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

[Charles Perry]

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!