lkml.org 
[lkml]   [2020]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net-next 3/3] net: phy: bcm54140: add hwmon support
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

\
 
 \ /
  Last update: 2020-04-20 18:11    [W:1.268 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site