Re: [net-next RFC PATCH v2 01/11] net: phy: extend PHY package API to support multiple global address

[Andrew Lunn]

On Sat, Nov 25, 2023 at 01:11:17AM +0100, Christian Marangi wrote:
> Current API for PHY package are limited to single address to configure
> global settings for the PHY package.
> 
> It was found that some PHY package (for example the qca807x, a PHY
> package that is shipped with a bundle of 5 PHY) require multiple PHY
> address to configure global settings. An example scenario is a PHY that
> have a dedicated PHY for PSGMII/serdes calibrarion and have a specific
> PHY in the package where the global PHY mode is set and affects every
> other PHY in the package.
> 
> Change the API in the following way:
> - Make phy_package_join() require a list of address to be passed and the
>   number of address in the list
> - On shared data init, each address is the list is checked and added to
>   the shared struct.
> - Make __/phy_package_write/read() require an additional arg that
>   select what global PHY address to use in the provided list.

I think this is overly complex.

I would rename struct phy_package_shared addr to base_addr.
phy_package_join() would then pass the base address of the package,
which is the same as your reg property for the package in DT.

I think all current users of devm_phy_package_join() already do pass
the lowest address in the package, so this should not cause any
problems. Most drivers even call it base address, rather than cookie,
which the documentation uses.

I would then extend __phy_package_read() etc to take an offset, which
is added to base_addr, and the read is performed on that address. All
the existing users would pass 0, and your new driver can pass other
values.

I also think you can split this out from the DT binding. Make it two
patch sets. One patch set is about extended the package concept to
allow access to global registers at addresses other than the base. The
DT patch is about properties which are shared by the package. These
seems like two mostly orthogonal concepts.

    Andrew