Am 2020-04-20 17:36, schrieb Andrew Lunn:
Ok I see, but what locking do you have in mind? We could have
something
like
__phy_package_write(struct phy_device *dev, u32 regnum, u16 val)
{
return __mdiobus_write(phydev->mdio.bus, phydev->shared->addr,
regnum, val);
}
and its phy_package_write() equivalent. But that would just be
convenience functions, nothing where you actually help the user with
locking. Am I missing something?
In general, drivers should not be using __foo functions. We want
drivers to make use of phy_package_write() which would do the bus
locking. Look at a typical PHY driver. There is no locking what so
ever. Just lots of phy_read() and phy write(). The locking is done by
the core and so should be correct.
Ok, but for example the BCM54140 uses indirect register access and thus
need to lock the mdio bus itself in which case I need the __funcs.
> > > Get the core to do reference counting on the structure?
> > > Add helpers phy_read_shared(), phy_write_shared(), etc, which does
> > > MDIO accesses on the base device, taking care of the locking.
> > >
> > The "base" access is another thing, I guess, which has nothing to do
> > with the shared structure.
> >
> I'm making the assumption that all global addresses are at the base
> address. If we don't want to make that assumption, we need the change
> the API above so you pass a cookie, and all PHYs need to use the same
> cookie to identify the package.
how would a phy driver deduce a common cookie? And how would that be a
difference to using a PHY address.
For a cookie, i don't care how the driver decides on the cookie. The
core never uses it, other than comparing cookies to combine individual
PHYs into a package. It could be a PHY address. It could be the PHY
address where the global registers are. Or it could be anything else.
> Maybe base is the wrong name, since MSCC can have the base as the high
> address of the four, not the low?
I'd say it might be any of the four addresses as long as it is the
same
across the PHYs in the same package. And in that case you can also
have
the phy_package_read/write() functions.
Yes. That is the semantics which is think is most useful. But then we
don't have a cookie, the value has real significance, and we need to
document what is should mean.
Agreed.
I will post a RFC shortly.
-michael
