Re: [net-next PATCH v11 6/9] net: mdio: Add Airoha AN8855 Switch MDIO Passtrough

[Christian Marangi <ansuelsmth@gmail.com>]

On Tue, Dec 10, 2024 at 02:53:34AM +0100, Andrew Lunn wrote:
> > +static int an855_phy_restore_page(struct an8855_mfd_priv *priv,
> > +				  int phy) __must_hold(&priv->bus->mdio_lock)
> > +{
> > +	/* Check PHY page only for addr shared with switch */
> > +	if (phy != priv->switch_addr)
> > +		return 0;
> > +
> > +	/* Don't restore page if it's not set to switch page */
> > +	if (priv->current_page != FIELD_GET(AN8855_PHY_PAGE,
> > +					    AN8855_PHY_PAGE_EXTENDED_4))
> > +		return 0;
> > +
> > +	/* Restore page to 0, PHY might change page right after but that
> > +	 * will be ignored as it won't be a switch page.
> > +	 */
> > +	return an8855_mii_set_page(priv, phy, AN8855_PHY_PAGE_STANDARD);
> > +}
> 
> I don't really understand what is going on here. Maybe the commit
> message needs expanding, or the function names changing.
> 
> Generally, i would expect a save/restore action. Save the current
> page, swap to the PHY page, do the PHY access, and then restore to the
> saved page.
>

Idea is to save on extra read/write on subsequent write on the same
page.

Idea here is that PHY will receive most of the read (for status
poll) hence in 90% of the time page will be 0.

And switch will receive read/write only on setup or fdb/vlan access on
configuration so it will receive subsequent write on the same page.
(page 4)

PHY might also need to write on page 1 on setup but never on page 4 as
that is reserved for switch.

Making the read/swap/write/restore adds 2 additional operation that can
really be skipped 90% of the time.

Also curret_page cache is indirectly protected by the mdio lock.

So in short this function makes sure PHY for port 0 is configured on the
right page based on the prev page set.

An alternative way might be assume PHY is always on page 0 and any
switch operation save and restore the page.

Hope it's clear now why this is needed. Is this ok or you prefer the
alternative way? 

-- 
	Ansuel