Re: [PATCH RFC 0/7] phylib MMD accessor cleanups

[Andrew Lunn]

> Thanks.  When I posted this last time around (19th Jan) I mentioned
> about marking the old _indirect() accessors with __deprecated - is
> that still something we want to do?
> 
> I haven't tested this against net-next yet, so I don't know if there
> are any new users of the indirect accessors - going down the deprecated
> route would avoid breakage, but means having to submit a patch later to
> actually remove them.
> 
> How would people want this handled?

Hi Russell

We can get patches into net-next very quickly. So i suggest you rebase
and resubmit and get it in. If something breaks, we add followup
patches to fix it.

Andrew

Re: [PATCH RFC 0/7] phylib MMD accessor cleanups

[Russell King - ARM Linux]

On Sun, Mar 19, 2017 at 03:30:38PM -0700, Florian Fainelli wrote:
> Le 03/19/17 à 03:59, Russell King - ARM Linux a écrit :
> > This series of patches does exactly that - we merge the functionality
> > of the indirect accesses into the clause 45 accessors, and use these
> > exclusively to access MMD registers.  Internally, the new clause
> > independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd
> > methods if present, otherwise fall back to using clause 45 accesses if
> > the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY
> > is clause 22.
> 
> LGTM:
> 
> Reviewed-by: Florian Fainelli 

Thanks.  When I posted this last time around (19th Jan) I mentioned
about marking the old _indirect() accessors with __deprecated - is
that still something we want to do?

I haven't tested this against net-next yet, so I don't know if there
are any new users of the indirect accessors - going down the deprecated
route would avoid breakage, but means having to submit a patch later to
actually remove them.

How would people want this handled?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH RFC 0/7] phylib MMD accessor cleanups

[Florian Fainelli]

Le 03/19/17 à 03:59, Russell King - ARM Linux a écrit :
> Hi,
> 
> This series cleans up the phylib MMD accessors.  We have two accessors
> at present, phy_(read|write)_mmd() and phy_(read|write)_mmd_indirect()
> 
> The _indirect methods access the MMD registers via a clause 22 phy,
> whereas the non-_indirect methods acess via clause 45 accesses.
> 
> Current PHY-independent parts of phylib (such as EEE) access the MMD
> registers via the _indirect methods, which is no good if you have a
> clause 45 PHY that doesn't respond to clause 22 accesses.
> 
> In order to make these features available, we need to rework these
> accessors such that they can access the MMD registers using a method
> dependent on the clause that the PHY conforms with.
> 
> This series of patches does exactly that - we merge the functionality
> of the indirect accesses into the clause 45 accessors, and use these
> exclusively to access MMD registers.  Internally, the new clause
> independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd
> methods if present, otherwise fall back to using clause 45 accesses if
> the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY
> is clause 22.

LGTM:

Reviewed-by: Florian Fainelli 

> 
> Note: confusingly, phy_read_mmd_indirect() vs phy_read_mmd() switches
> the order of prtad and devad, which means that converting between the
> two is not a simple matter of just replacing the function name.  Same
> applies for the write methods.
> 
>  drivers/net/phy/Makefile  |   2 +-
>  drivers/net/phy/bcm-phy-lib.c |  12 ++---
>  drivers/net/phy/dp83867.c |  25 +-
>  drivers/net/phy/intel-xway.c  |  26 +-
>  drivers/net/phy/micrel.c  |  13 +++--
>  drivers/net/phy/microchip.c   |   5 +-
>  drivers/net/phy/phy-core.c| 101 ++
>  drivers/net/phy/phy.c | 110 
> --
>  drivers/net/phy/phy_device.c  |   4 +-
>  drivers/net/usb/lan78xx.c |  10 ++--
>  include/linux/phy.h   |  80 +-
>  11 files changed, 178 insertions(+), 210 deletions(-)
> 


-- 
Florian

Re: [PATCH RFC 0/7] phylib MMD accessor cleanups

[Andrew Lunn]

On Sun, Mar 19, 2017 at 10:59:44AM +, Russell King - ARM Linux wrote:
> Hi,
> 
> This series cleans up the phylib MMD accessors.  We have two accessors
> at present, phy_(read|write)_mmd() and phy_(read|write)_mmd_indirect()
> 
> The _indirect methods access the MMD registers via a clause 22 phy,
> whereas the non-_indirect methods acess via clause 45 accesses.
> 
> Current PHY-independent parts of phylib (such as EEE) access the MMD
> registers via the _indirect methods, which is no good if you have a
> clause 45 PHY that doesn't respond to clause 22 accesses.
> 
> In order to make these features available, we need to rework these
> accessors such that they can access the MMD registers using a method
> dependent on the clause that the PHY conforms with.
> 
> This series of patches does exactly that - we merge the functionality
> of the indirect accesses into the clause 45 accessors, and use these
> exclusively to access MMD registers.  Internally, the new clause
> independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd
> methods if present, otherwise fall back to using clause 45 accesses if
> the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY
> is clause 22.
> 
> Note: confusingly, phy_read_mmd_indirect() vs phy_read_mmd() switches
> the order of prtad and devad, which means that converting between the
> two is not a simple matter of just replacing the function name.  Same
> applies for the write methods.

Hi Russell

This all looks good, apart from the one typo i spotted.

Reviewed-by: Andrew Lunn 

Andrew

[PATCH RFC 0/7] phylib MMD accessor cleanups

[Russell King - ARM Linux]

Hi,

This series cleans up the phylib MMD accessors.  We have two accessors
at present, phy_(read|write)_mmd() and phy_(read|write)_mmd_indirect()

The _indirect methods access the MMD registers via a clause 22 phy,
whereas the non-_indirect methods acess via clause 45 accesses.

Current PHY-independent parts of phylib (such as EEE) access the MMD
registers via the _indirect methods, which is no good if you have a
clause 45 PHY that doesn't respond to clause 22 accesses.

In order to make these features available, we need to rework these
accessors such that they can access the MMD registers using a method
dependent on the clause that the PHY conforms with.

This series of patches does exactly that - we merge the functionality
of the indirect accesses into the clause 45 accessors, and use these
exclusively to access MMD registers.  Internally, the new clause
independent MMD accessors indirect via the PHY drivers read_mmd/write_mmd
methods if present, otherwise fall back to using clause 45 accesses if
the PHY is a clause 45 phy, or clause 22 indirect accesses if the PHY
is clause 22.

Note: confusingly, phy_read_mmd_indirect() vs phy_read_mmd() switches
the order of prtad and devad, which means that converting between the
two is not a simple matter of just replacing the function name.  Same
applies for the write methods.

 drivers/net/phy/Makefile  |   2 +-
 drivers/net/phy/bcm-phy-lib.c |  12 ++---
 drivers/net/phy/dp83867.c |  25 +-
 drivers/net/phy/intel-xway.c  |  26 +-
 drivers/net/phy/micrel.c  |  13 +++--
 drivers/net/phy/microchip.c   |   5 +-
 drivers/net/phy/phy-core.c| 101 ++
 drivers/net/phy/phy.c | 110 --
 drivers/net/phy/phy_device.c  |   4 +-
 drivers/net/usb/lan78xx.c |  10 ++--
 include/linux/phy.h   |  80 +-
 11 files changed, 178 insertions(+), 210 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.