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

Christian Marangi
2025-04-09T07:24:19Z

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.

Any hint on where to better document this?

>
> As I said, I think this could work and even be re-used in other places,
> so I'm ok with that, it's just not really clear from the commit log what
> problem this solves.
>
> Maxime

--
Ansuel