netdev - Re: [net-next PATCH v14 07/16] net: mdio: regmap: add support for C45 read/write

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: <50c7328d-b8f7-4b07-9e34-6d7c34923335@lunn.ch>
Date: Thu, 10 Apr 2025 19:08:53 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Vladimir Oltean <olteanv@...il.com>,
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	"Chester A. Unal" <chester.a.unal@...nc9.com>,
	Daniel Golle <daniel@...rotopia.org>,
	DENG Qingfang <dqfext@...il.com>,
	Sean Wang <sean.wang@...iatek.com>, Simon Horman <horms@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	upstream@...oha.com
Subject: Re: [net-next PATCH v14 07/16] net: mdio: regmap: add support for
 C45 read/write

On Tue, Apr 08, 2025 at 11:51:14AM +0200, Christian Marangi wrote:
> Add support for C45 read/write for mdio regmap. This can be done
> by enabling the support_encoded_addr bool in mdio regmap config and by
> using the new API devm_mdio_regmap_init to init a regmap.
> 
> To support C45, additional info needs to be appended to the regmap
> address passed to regmap OPs.
> 
> The logic applied to the regmap address value:
> - First the regnum value (20, 16)
> - Second the devnum value (25, 21)
> - A bit to signal if it's C45 (26)
> 
> devm_mdio_regmap_init MUST be used to register a regmap for this to
> correctly handle internally the encode/decode of the address.
> 
> Drivers needs to define a mdio_regmap_init_config where an optional regmap
> name can be defined and MUST define C22 OPs (mdio_read/write).
> To support C45 operation also C45 OPs (mdio_read/write_c45).
> 
> The regmap from devm_mdio_regmap_init will internally decode the encoded
> regmap address and extract the various info (addr, devnum if C45 and
> regnum). It will then call the related OP and pass the extracted values to
> the function.
> 
> Example for a C45 read operation:
> - With an encoded address with C45 bit enabled, it will call the
>   .mdio_read_c45 and addr, devnum and regnum will be passed.
>   .mdio_read_c45 will then return the val and val will be stored in the
>   regmap_read pointer and will return 0. If .mdio_read_c45 returns
>   any error, then the regmap_read will return such error.
> 
> With support_encoded_addr enabled, also C22 will encode the address in
> the regmap address and .mdio_read/write will called accordingly similar
> to C45 operation.

This patchset needs pulling apart, there are two many things going on.

You are adding at least two different features here. The current code
only supports a single device on the bus, and it assumes the regmap
provider knows what device that is. That is probably because all
current users only have a single device. You now appear to want to
pass that address to the regmap provider. I don't see the need for
that, since it is still a single device on the bus. So adding this
feature on its own, with a good commit message, will explain that.

You want to add C45 support. So that is another patch.

C22 and C45 are different address spaces. To me, it seems logical to
have different regmaps. That makes the regmap provider simpler. A C22
regmap provider probably is just a straight access. A C45 regmap
provider might need to handle the hardware having a sparse register
map, only some of these 32 block of 65536 are implemented, etc.

So i think:

struct mdio_regmap_config {
        struct device *parent;
        struct regmap *regmap;
        char name[MII_BUS_ID_SIZE];
        u8 valid_addr;
        bool autoscan;
};

should be extended with a second regmap, used for C45.

	Andrew