Hi Andrew,
Thanks for your review.
On 2024-04-16 00:55:12 +0200, Andrew Lunn wrote:
<snip>
> > +static int rtsn_mii_access_indirect(struct mii_bus *bus, bool read,
> > int phyad,
> > + int devnum, int regnum, u16 data)
> > +{
> > + int ret;
> > +
> > + ret = rtsn_mii_access(bus, false, phyad, MII_MMD_CTRL, devnum);
> > + if (ret)
> > + return ret;
> > +
> > + ret = rtsn_mii_access(bus, false, phyad, MII_MMD_DATA, regnum);
> > + if (ret)
> > + return ret;
> > +
> > + ret = rtsn_mii_access(bus, false, phyad, MII_MMD_CTRL,
> > + devnum | MII_MMD_CTRL_NOINCR);
>
> This looks to be C45 over C22. phylib core knows how to do this, since
> it should be the same for all PHYs which implement C45 over C22. So
> there is no need for you to implement it again.
>
> > +static int rtsn_mii_register(struct rtsn_private *priv)
> > +{
> > + struct platform_device *pdev = priv->pdev;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *mdio_node;
> > + struct mii_bus *mii;
> > + int ret;
> > +
> > + mii = mdiobus_alloc();
> > + if (!mii)
> > + return -ENOMEM;
> > +
> > + mdio_node = of_get_child_by_name(dev->of_node, "mdio");
> > + if (!mdio_node) {
> > + ret = -ENODEV;
> > + goto out_free_bus;
> > + };
> > +
> > + mii->name = "rtsn_mii";
> > + sprintf(mii->id, "%s-%x", pdev->name, pdev->id);
> > + mii->priv = priv;
> > + mii->read = rtsn_mii_read;
> > + mii->write = rtsn_mii_write;
> > + mii->read_c45 = rtsn_mii_read_c45;
> > + mii->write_c45 = rtsn_mii_write_c45;
>
> Just leave these two empty, and the core will do C45 over C22 for you.
Does this not require the bus to be created/allocated with an
implementation that support this, for example mdio_i2c_alloc() or
alloc_mdio_bitbang()? This bus is allocated with mdiobus_alloc() which
do not implement this. Removing the C45 functions here result in
__mdiobus_c45_read() returning -EOPNOTSUPP as bus->read_c45 is not set.
The allocator in question here could possibly be alloc_mdio_bitbang(),
but I see no easy way for me to implement the mdiobb_ops struct. Is
there a different bus implementation I should use that is able to talk
C45 over C22 ?
--
Kind Regards,
Niklas Söderlund
