Re: [PATCH net-next] net: phy: micrel: Add config_init for LAN8814

On Fri, Nov 26, 2021 at 12:57:33PM +0100, Heiner Kallweit wrote:

Not directly related to just this patch:
Did you consider implementing the read_page and write_page PHY driver
callbacks? Then you could use phylib functions like phy_modify_paged et al
and you wouldn't have to open-code the paged register operations.

I think write_page would just be
phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));

and read_page
phy_read(phydev, LAN_EXT_PAGE_ACCESS_CONTROL);

Remember that read_page() and write_page() must be implemented using
the unlocked accessors since the MDIO bus lock is held prior to calling
them. So these should be __phy_write() and __phy_read().

The use of the helpers you mention above also bring greater safety to
the read-modify-write accesses, since with these accessors, the whole
set of accesses are done while holding the bus lock.

Thanks.

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