Re: [PATCH net-next v3 2/2] net: mdio: add a driver for PIC64-HPSC/HX MDIO controller

Charles Perry
2026-03-31T13:42:45Z

On Tue, Mar 31, 2026 at 01:57:10PM +0100, Russell King (Oracle) wrote:
> On Tue, Mar 31, 2026 at 05:38:54AM -0700, Charles Perry wrote:
> > + if (!(bus->phy_ignore_ta_mask & 1 << mii_id) &&
> > + !FIELD_GET(MDIO_READOK_BIT, val)) {
> > + dev_dbg(&bus->dev, "READOK bit cleared\n");
> > + return -EIO;
> > + }
> > +
> > + ret = FIELD_GET(MDIO_RDATA_MASK, val);
> > +
> > + return ret;
>
> You don't need "ret" here, this can simply be:
>
> return FIELD_GET(MDIO_RDATA_MASK, val);

Ok

>
> ...
>
> > + writel(MDIO_TRIGGER_BIT | FIELD_PREP(MDIO_REG_DEV_ADDR_MASK, regnum) |
> > + FIELD_PREP(MDIO_PHY_PRT_ADDR_MASK, mii_id) |
> > + FIELD_PREP(MDIO_OPERATION_MASK, MDIO_OPERATION_WRITE) |
> > + FIELD_PREP(MDIO_START_OF_FRAME_MASK, 1),
> > + priv->regs + MDIO_REG_FRAME_CFG_2);
>
> Shouldn't this wait for the write to complete?
>

As it is, the write transaction has NOT finished when the function returns,
because there's no call to pic64hpsc_mdio_wait_trigger().

It works because there's a pic64hpsc_mdio_wait_trigger() at the beginning
of ->read() and ->write(), so the completion will be waited for on the next
read or write. I've taken this from mdio-mscc-miim.c.

I don't know if there's any value in waiting for write completion here as
write completion doesn't mean that the effects of the write are available
right now. I also didn't run into any issues in my testing. Let me know if
you know of a use case where this wouldn't work.

I can add a wait for transaction completion if that's expected by phylib.

Thanks,
Charles

> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!