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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8ce4503-c24d-4d6e-91ec-d03624b31fe0@lunn.ch>
Date: Sat, 25 Nov 2023 18:51:54 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, Andy Gross <agross@...nel.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	Daniel Golle <daniel@...rotopia.org>,
	Qingfang Deng <dqfext@...il.com>,
	SkyLake Huang <SkyLake.Huang@...iatek.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Vladimir Oltean <olteanv@...il.com>,
	David Epping <david.epping@...singlinkelectronics.com>,
	Harini Katakam <harini.katakam@....com>,
	"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
	Robert Marko <robert.marko@...tura.hr>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [net-next RFC PATCH v2 01/11] net: phy: extend PHY package API
 to support multiple global address

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