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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

On Wed, 9 Apr 2025 09:24:13 +0200
Christian Marangi <ansuelsmth@gmail.com> wrote:

> On Wed, Apr 09, 2025 at 09:07:51AM +0200, Maxime Chevallier wrote:
> > Hi Christian,
> > 
> > On Tue,  8 Apr 2025 11:51:14 +0200
> > Christian Marangi <ansuelsmth@gmail.com> 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 driver's orginal goal is to address the case where we have a
> > PHY-like device that has the same register layout and behaviour as a
> > C22 PHY, but where the registers are not accesses through MDIO (MMIO
> > for example, as in altera-tse or dwmac-socfpga, or potentially SPI even
> > though  there's no example upstream).
> > 
> > What is done here is quite different, I guess it could work if we have
> > MMIO C45 phys that understand the proposed encoding, but I don't really
> > understand the dance where C45 accesses are wrapped by this mdio-regmap
> > driver into regmap accesss, but the regmap itself converts it back to
> > C45 accesses. Is it just so that it fits well with MFD ?  
> 
> The main task of this wrapping is to remove from the dev side having to
> handle the encode/decode part. regmap address is still a single value
> but if a phy is mmio mapped is difficult to support c45 since you need 3
> different values (phy id, mmd and addr)
> 
> With this implementation a c45 that is mmio mapped can implement
> whatever way he wants to configure each parameter for read/write
> operation.
> 
> Example the ecoding might be on different mask and with the additional
> function it can be reorganized following the specific mask.
> 
> > 
> > I'm not really against that, it still converts mdio access to regmap so
> > there's that, but is there a way to elaborate or document somewhere why
> > we need to do go through C45 -> regmap -> C45 instead of just
> > writing a mii_bus driver in the first place ?  
> 
> This was askek to prevent creating additional ""trivial"" mdio driver
> that would all do the same task. Since mdio-regmap was already in place
> it could have been extended with a more generic approach.

Ah yes that's the point I was missing, I've browsed more in depth and
indeed in V11 Vlad suggested to use this.

> Any hint on where to better document this?

Given the simplicity of the driver, I think this commit log is good
enough then :)

Let's go for it and hopefully this can be reused elsewhere !

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime