mbox series

[net-next,v4,0/5] net: phy: add Lynx PCS MDIO module

Message ID 20200724080143.12909-1-ioana.ciornei@nxp.com
Headers show
Series net: phy: add Lynx PCS MDIO module | expand

Message

Ioana Ciornei July 24, 2020, 8:01 a.m. UTC
Add support for the Lynx PCS as a separate module in drivers/net/phy/.
The advantage of this structure is that multiple ethernet or switch
drivers used on NXP hardware (ENETC, Seville, Felix DSA switch etc) can
share the same implementation of PCS configuration and runtime
management.

The module implements phylink_pcs_ops and exports a phylink_pcs
(incorporated into a lynx_pcs) which can be directly passed to phylink
through phylink_pcs_set.

The first 3 patches add some missing pieces in phylink and the locked
mdiobus write accessor. Next, the Lynx PCS MDIO module is added as a
standalone module. The majority of the code is extracted from the Felix
DSA driver. The last patch makes the necessary changes in the Felix and
Seville drivers in order to use the new common PCS implementation.

At the moment, USXGMII (only with in-band AN), SGMII, QSGMII (with and
without in-band AN) and 2500Base-X (only w/o in-band AN) are supported
by the Lynx PCS MDIO module since these were also supported by Felix and
no functional change is intended at this time.

Changes in v2:
 * got rid of the mdio_lynx_pcs structure and directly exported the
 functions without the need of an indirection
 * made the necessary adjustments for this in the Felix DSA driver
 * solved the broken allmodconfig build test by making the module
 tristate instead of bool
 * fixed a memory leakage in the Felix driver (the pcs structure was
 allocated twice)

Changes in v3:
 * added support for PHYLINK PCS ops in DSA (patch 5/9)
 * cleanup in Felix PHYLINK operations and migrate to
 phylink_mac_link_up() being the callback of choice for applying MAC
 configuration (patches 6-8)

Changes in v4:
 * use the newly introduced phylink PCS mechanism
 * install the phylink_pcs in the phylink_mac_config DSA ops
 * remove the direct implementations of the PCS ops
 * do no use the SGMII_ prefix when referring to the IF_MORE register
 * add a phylink helper to decode the USXGMII code word
 * remove cleanup patches for Felix (these have been already accepted)
 * Seville (recently introduced) now has PCS support through the same
 Lynx PCS module

Ioana Ciornei (5):
  net: phylink: add helper function to decode USXGMII word
  net: phylink: consider QSGMII interface mode in
    phylink_mii_c22_pcs_get_state
  net: mdiobus: add clause 45 mdiobus write accessor
  net: phy: add Lynx PCS module
  net: dsa: ocelot: use the Lynx PCS helpers in Felix and Seville

 MAINTAINERS                              |   7 +
 drivers/net/dsa/ocelot/Kconfig           |   1 +
 drivers/net/dsa/ocelot/felix.c           |  28 +-
 drivers/net/dsa/ocelot/felix.h           |  20 +-
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 374 ++---------------------
 drivers/net/dsa/ocelot/seville_vsc9953.c |  21 +-
 drivers/net/phy/Kconfig                  |   6 +
 drivers/net/phy/Makefile                 |   1 +
 drivers/net/phy/pcs-lynx.c               | 314 +++++++++++++++++++
 drivers/net/phy/phylink.c                |  44 +++
 include/linux/mdio.h                     |   6 +
 include/linux/pcs-lynx.h                 |  21 ++
 include/linux/phylink.h                  |   3 +
 13 files changed, 442 insertions(+), 404 deletions(-)
 create mode 100644 drivers/net/phy/pcs-lynx.c
 create mode 100644 include/linux/pcs-lynx.h

Comments

Vladimir Oltean July 24, 2020, 9:47 p.m. UTC | #1
On Fri, Jul 24, 2020 at 11:01:38AM +0300, Ioana Ciornei wrote:
> Add support for the Lynx PCS as a separate module in drivers/net/phy/.
> The advantage of this structure is that multiple ethernet or switch
> drivers used on NXP hardware (ENETC, Seville, Felix DSA switch etc) can
> share the same implementation of PCS configuration and runtime
> management.
> 
> The module implements phylink_pcs_ops and exports a phylink_pcs
> (incorporated into a lynx_pcs) which can be directly passed to phylink
> through phylink_pcs_set.
> 
> The first 3 patches add some missing pieces in phylink and the locked
> mdiobus write accessor. Next, the Lynx PCS MDIO module is added as a
> standalone module. The majority of the code is extracted from the Felix
> DSA driver. The last patch makes the necessary changes in the Felix and
> Seville drivers in order to use the new common PCS implementation.
> 
> At the moment, USXGMII (only with in-band AN), SGMII, QSGMII (with and
> without in-band AN) and 2500Base-X (only w/o in-band AN) are supported
> by the Lynx PCS MDIO module since these were also supported by Felix and
> no functional change is intended at this time.
> 
> Changes in v2:
>  * got rid of the mdio_lynx_pcs structure and directly exported the
>  functions without the need of an indirection
>  * made the necessary adjustments for this in the Felix DSA driver
>  * solved the broken allmodconfig build test by making the module
>  tristate instead of bool
>  * fixed a memory leakage in the Felix driver (the pcs structure was
>  allocated twice)
> 
> Changes in v3:
>  * added support for PHYLINK PCS ops in DSA (patch 5/9)
>  * cleanup in Felix PHYLINK operations and migrate to
>  phylink_mac_link_up() being the callback of choice for applying MAC
>  configuration (patches 6-8)
> 
> Changes in v4:
>  * use the newly introduced phylink PCS mechanism
>  * install the phylink_pcs in the phylink_mac_config DSA ops
>  * remove the direct implementations of the PCS ops
>  * do no use the SGMII_ prefix when referring to the IF_MORE register
>  * add a phylink helper to decode the USXGMII code word
>  * remove cleanup patches for Felix (these have been already accepted)
>  * Seville (recently introduced) now has PCS support through the same
>  Lynx PCS module
> 
> Ioana Ciornei (5):
>   net: phylink: add helper function to decode USXGMII word
>   net: phylink: consider QSGMII interface mode in
>     phylink_mii_c22_pcs_get_state
>   net: mdiobus: add clause 45 mdiobus write accessor
>   net: phy: add Lynx PCS module
>   net: dsa: ocelot: use the Lynx PCS helpers in Felix and Seville
> 
>  MAINTAINERS                              |   7 +
>  drivers/net/dsa/ocelot/Kconfig           |   1 +
>  drivers/net/dsa/ocelot/felix.c           |  28 +-
>  drivers/net/dsa/ocelot/felix.h           |  20 +-
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 374 ++---------------------
>  drivers/net/dsa/ocelot/seville_vsc9953.c |  21 +-
>  drivers/net/phy/Kconfig                  |   6 +
>  drivers/net/phy/Makefile                 |   1 +
>  drivers/net/phy/pcs-lynx.c               | 314 +++++++++++++++++++
>  drivers/net/phy/phylink.c                |  44 +++
>  include/linux/mdio.h                     |   6 +
>  include/linux/pcs-lynx.h                 |  21 ++
>  include/linux/phylink.h                  |   3 +
>  13 files changed, 442 insertions(+), 404 deletions(-)
>  create mode 100644 drivers/net/phy/pcs-lynx.c
>  create mode 100644 include/linux/pcs-lynx.h
> 
> -- 
> 2.25.1
> 

For the entire series:

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Seville QSGMII with in-band AN
Felix QSGMII with in-band AN
Felix SGMII without in-band AN
Felix USXGMII with in-band AN

Thanks,
-Vladimir
Ioana Ciornei July 27, 2020, 6:27 p.m. UTC | #2
> Subject: [PATCH net-next v4 0/5] net: phy: add Lynx PCS MDIO module
> 
> Add support for the Lynx PCS as a separate module in drivers/net/phy/.
> The advantage of this structure is that multiple ethernet or switch drivers used
> on NXP hardware (ENETC, Seville, Felix DSA switch etc) can share the same
> implementation of PCS configuration and runtime management.
> 
> The module implements phylink_pcs_ops and exports a phylink_pcs
> (incorporated into a lynx_pcs) which can be directly passed to phylink through
> phylink_pcs_set.
> 
> The first 3 patches add some missing pieces in phylink and the locked mdiobus
> write accessor. Next, the Lynx PCS MDIO module is added as a standalone
> module. The majority of the code is extracted from the Felix DSA driver. The last
> patch makes the necessary changes in the Felix and Seville drivers in order to use
> the new common PCS implementation.
> 
> At the moment, USXGMII (only with in-band AN), SGMII, QSGMII (with and
> without in-band AN) and 2500Base-X (only w/o in-band AN) are supported by the
> Lynx PCS MDIO module since these were also supported by Felix and no
> functional change is intended at this time.
> 

Any feedback on the use of phylink_pcs?

Thanks,
Ioana

> Changes in v2:
>  * got rid of the mdio_lynx_pcs structure and directly exported the  functions
> without the need of an indirection
>  * made the necessary adjustments for this in the Felix DSA driver
>  * solved the broken allmodconfig build test by making the module  tristate
> instead of bool
>  * fixed a memory leakage in the Felix driver (the pcs structure was  allocated
> twice)
> 
> Changes in v3:
>  * added support for PHYLINK PCS ops in DSA (patch 5/9)
>  * cleanup in Felix PHYLINK operations and migrate to
>  phylink_mac_link_up() being the callback of choice for applying MAC
> configuration (patches 6-8)
> 
> Changes in v4:
>  * use the newly introduced phylink PCS mechanism
>  * install the phylink_pcs in the phylink_mac_config DSA ops
>  * remove the direct implementations of the PCS ops
>  * do no use the SGMII_ prefix when referring to the IF_MORE register
>  * add a phylink helper to decode the USXGMII code word
>  * remove cleanup patches for Felix (these have been already accepted)
>  * Seville (recently introduced) now has PCS support through the same  Lynx PCS
> module
> 
> Ioana Ciornei (5):
>   net: phylink: add helper function to decode USXGMII word
>   net: phylink: consider QSGMII interface mode in
>     phylink_mii_c22_pcs_get_state
>   net: mdiobus: add clause 45 mdiobus write accessor
>   net: phy: add Lynx PCS module
>   net: dsa: ocelot: use the Lynx PCS helpers in Felix and Seville
> 
>  MAINTAINERS                              |   7 +
>  drivers/net/dsa/ocelot/Kconfig           |   1 +
>  drivers/net/dsa/ocelot/felix.c           |  28 +-
>  drivers/net/dsa/ocelot/felix.h           |  20 +-
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 374 ++---------------------
>  drivers/net/dsa/ocelot/seville_vsc9953.c |  21 +-
>  drivers/net/phy/Kconfig                  |   6 +
>  drivers/net/phy/Makefile                 |   1 +
>  drivers/net/phy/pcs-lynx.c               | 314 +++++++++++++++++++
>  drivers/net/phy/phylink.c                |  44 +++
>  include/linux/mdio.h                     |   6 +
>  include/linux/pcs-lynx.h                 |  21 ++
>  include/linux/phylink.h                  |   3 +
>  13 files changed, 442 insertions(+), 404 deletions(-)  create mode 100644
> drivers/net/phy/pcs-lynx.c  create mode 100644 include/linux/pcs-lynx.h
> 
> --
> 2.25.1
Florian Fainelli July 27, 2020, 6:29 p.m. UTC | #3
On 7/24/2020 1:01 AM, Ioana Ciornei wrote:
> Add support for the Lynx PCS as a separate module in drivers/net/phy/.
> The advantage of this structure is that multiple ethernet or switch
> drivers used on NXP hardware (ENETC, Seville, Felix DSA switch etc) can
> share the same implementation of PCS configuration and runtime
> management.
> 
> The module implements phylink_pcs_ops and exports a phylink_pcs
> (incorporated into a lynx_pcs) which can be directly passed to phylink
> through phylink_pcs_set.
> 
> The first 3 patches add some missing pieces in phylink and the locked
> mdiobus write accessor. Next, the Lynx PCS MDIO module is added as a
> standalone module. The majority of the code is extracted from the Felix
> DSA driver. The last patch makes the necessary changes in the Felix and
> Seville drivers in order to use the new common PCS implementation.
> 
> At the moment, USXGMII (only with in-band AN), SGMII, QSGMII (with and
> without in-band AN) and 2500Base-X (only w/o in-band AN) are supported
> by the Lynx PCS MDIO module since these were also supported by Felix and
> no functional change is intended at this time.
> 
> Changes in v2:
>  * got rid of the mdio_lynx_pcs structure and directly exported the
>  functions without the need of an indirection
>  * made the necessary adjustments for this in the Felix DSA driver
>  * solved the broken allmodconfig build test by making the module
>  tristate instead of bool
>  * fixed a memory leakage in the Felix driver (the pcs structure was
>  allocated twice)
> 
> Changes in v3:
>  * added support for PHYLINK PCS ops in DSA (patch 5/9)
>  * cleanup in Felix PHYLINK operations and migrate to
>  phylink_mac_link_up() being the callback of choice for applying MAC
>  configuration (patches 6-8)
> 
> Changes in v4:
>  * use the newly introduced phylink PCS mechanism
>  * install the phylink_pcs in the phylink_mac_config DSA ops
>  * remove the direct implementations of the PCS ops
>  * do no use the SGMII_ prefix when referring to the IF_MORE register
>  * add a phylink helper to decode the USXGMII code word
>  * remove cleanup patches for Felix (these have been already accepted)
>  * Seville (recently introduced) now has PCS support through the same
>  Lynx PCS module
> 
> Ioana Ciornei (5):
>   net: phylink: add helper function to decode USXGMII word
>   net: phylink: consider QSGMII interface mode in
>     phylink_mii_c22_pcs_get_state
>   net: mdiobus: add clause 45 mdiobus write accessor
>   net: phy: add Lynx PCS module
>   net: dsa: ocelot: use the Lynx PCS helpers in Felix and Seville
> 
>  MAINTAINERS                              |   7 +
>  drivers/net/dsa/ocelot/Kconfig           |   1 +
>  drivers/net/dsa/ocelot/felix.c           |  28 +-
>  drivers/net/dsa/ocelot/felix.h           |  20 +-
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 374 ++---------------------
>  drivers/net/dsa/ocelot/seville_vsc9953.c |  21 +-
>  drivers/net/phy/Kconfig                  |   6 +
>  drivers/net/phy/Makefile                 |   1 +
>  drivers/net/phy/pcs-lynx.c               | 314 +++++++++++++++++++

I believe Andrew had a plan to create a better organization within
drivers/net/phy, while this happens, maybe you can already create
drivers/net/phy/pcs/ regardless of the state of Andrew's work?

>  drivers/net/phy/phylink.c                |  44 +++
>  include/linux/mdio.h                     |   6 +
>  include/linux/pcs-lynx.h                 |  21 ++

And likewise for this header.
Ioana Ciornei July 27, 2020, 6:48 p.m. UTC | #4
> Subject: Re: [PATCH net-next v4 0/5] net: phy: add Lynx PCS MDIO module
> 
> 
> 
> On 7/24/2020 1:01 AM, Ioana Ciornei wrote:
> > Add support for the Lynx PCS as a separate module in drivers/net/phy/.
> > The advantage of this structure is that multiple ethernet or switch
> > drivers used on NXP hardware (ENETC, Seville, Felix DSA switch etc)
> > can share the same implementation of PCS configuration and runtime
> > management.
> >
> > The module implements phylink_pcs_ops and exports a phylink_pcs
> > (incorporated into a lynx_pcs) which can be directly passed to phylink
> > through phylink_pcs_set.
> >
> > The first 3 patches add some missing pieces in phylink and the locked
> > mdiobus write accessor. Next, the Lynx PCS MDIO module is added as a
> > standalone module. The majority of the code is extracted from the
> > Felix DSA driver. The last patch makes the necessary changes in the
> > Felix and Seville drivers in order to use the new common PCS implementation.
> >
> > At the moment, USXGMII (only with in-band AN), SGMII, QSGMII (with and
> > without in-band AN) and 2500Base-X (only w/o in-band AN) are supported
> > by the Lynx PCS MDIO module since these were also supported by Felix
> > and no functional change is intended at this time.
> >
> > Changes in v2:
> >  * got rid of the mdio_lynx_pcs structure and directly exported the
> > functions without the need of an indirection
> >  * made the necessary adjustments for this in the Felix DSA driver
> >  * solved the broken allmodconfig build test by making the module
> > tristate instead of bool
> >  * fixed a memory leakage in the Felix driver (the pcs structure was
> > allocated twice)
> >
> > Changes in v3:
> >  * added support for PHYLINK PCS ops in DSA (patch 5/9)
> >  * cleanup in Felix PHYLINK operations and migrate to
> >  phylink_mac_link_up() being the callback of choice for applying MAC
> > configuration (patches 6-8)
> >
> > Changes in v4:
> >  * use the newly introduced phylink PCS mechanism
> >  * install the phylink_pcs in the phylink_mac_config DSA ops
> >  * remove the direct implementations of the PCS ops
> >  * do no use the SGMII_ prefix when referring to the IF_MORE register
> >  * add a phylink helper to decode the USXGMII code word
> >  * remove cleanup patches for Felix (these have been already accepted)
> >  * Seville (recently introduced) now has PCS support through the same
> > Lynx PCS module
> >
> > Ioana Ciornei (5):
> >   net: phylink: add helper function to decode USXGMII word
> >   net: phylink: consider QSGMII interface mode in
> >     phylink_mii_c22_pcs_get_state
> >   net: mdiobus: add clause 45 mdiobus write accessor
> >   net: phy: add Lynx PCS module
> >   net: dsa: ocelot: use the Lynx PCS helpers in Felix and Seville
> >
> >  MAINTAINERS                              |   7 +
> >  drivers/net/dsa/ocelot/Kconfig           |   1 +
> >  drivers/net/dsa/ocelot/felix.c           |  28 +-
> >  drivers/net/dsa/ocelot/felix.h           |  20 +-
> >  drivers/net/dsa/ocelot/felix_vsc9959.c   | 374 ++---------------------
> >  drivers/net/dsa/ocelot/seville_vsc9953.c |  21 +-
> >  drivers/net/phy/Kconfig                  |   6 +
> >  drivers/net/phy/Makefile                 |   1 +
> >  drivers/net/phy/pcs-lynx.c               | 314 +++++++++++++++++++
> 
> I believe Andrew had a plan to create a better organization within
> drivers/net/phy, while this happens, maybe you can already create
> drivers/net/phy/pcs/ regardless of the state of Andrew's work?
> 

I got the impression from Andrew that the plan was to do this at a later stage
together with the Synopsys XPCS. I could certainly do what you say, I am just
not very keen to add such things into this patch set. 

Ioana

> >  drivers/net/phy/phylink.c                |  44 +++
> >  include/linux/mdio.h                     |   6 +
> >  include/linux/pcs-lynx.h                 |  21 ++
> 
> And likewise for this header.
> --
> Florian
Florian Fainelli July 27, 2020, 7:47 p.m. UTC | #5
On 7/27/20 11:48 AM, Ioana Ciornei wrote:
>> Subject: Re: [PATCH net-next v4 0/5] net: phy: add Lynx PCS MDIO module
>>
>>
>>
>> On 7/24/2020 1:01 AM, Ioana Ciornei wrote:
>>> Add support for the Lynx PCS as a separate module in drivers/net/phy/.
>>> The advantage of this structure is that multiple ethernet or switch
>>> drivers used on NXP hardware (ENETC, Seville, Felix DSA switch etc)
>>> can share the same implementation of PCS configuration and runtime
>>> management.
>>>
>>> The module implements phylink_pcs_ops and exports a phylink_pcs
>>> (incorporated into a lynx_pcs) which can be directly passed to phylink
>>> through phylink_pcs_set.
>>>
>>> The first 3 patches add some missing pieces in phylink and the locked
>>> mdiobus write accessor. Next, the Lynx PCS MDIO module is added as a
>>> standalone module. The majority of the code is extracted from the
>>> Felix DSA driver. The last patch makes the necessary changes in the
>>> Felix and Seville drivers in order to use the new common PCS implementation.
>>>
>>> At the moment, USXGMII (only with in-band AN), SGMII, QSGMII (with and
>>> without in-band AN) and 2500Base-X (only w/o in-band AN) are supported
>>> by the Lynx PCS MDIO module since these were also supported by Felix
>>> and no functional change is intended at this time.
>>>
>>> Changes in v2:
>>>  * got rid of the mdio_lynx_pcs structure and directly exported the
>>> functions without the need of an indirection
>>>  * made the necessary adjustments for this in the Felix DSA driver
>>>  * solved the broken allmodconfig build test by making the module
>>> tristate instead of bool
>>>  * fixed a memory leakage in the Felix driver (the pcs structure was
>>> allocated twice)
>>>
>>> Changes in v3:
>>>  * added support for PHYLINK PCS ops in DSA (patch 5/9)
>>>  * cleanup in Felix PHYLINK operations and migrate to
>>>  phylink_mac_link_up() being the callback of choice for applying MAC
>>> configuration (patches 6-8)
>>>
>>> Changes in v4:
>>>  * use the newly introduced phylink PCS mechanism
>>>  * install the phylink_pcs in the phylink_mac_config DSA ops
>>>  * remove the direct implementations of the PCS ops
>>>  * do no use the SGMII_ prefix when referring to the IF_MORE register
>>>  * add a phylink helper to decode the USXGMII code word
>>>  * remove cleanup patches for Felix (these have been already accepted)
>>>  * Seville (recently introduced) now has PCS support through the same
>>> Lynx PCS module
>>>
>>> Ioana Ciornei (5):
>>>   net: phylink: add helper function to decode USXGMII word
>>>   net: phylink: consider QSGMII interface mode in
>>>     phylink_mii_c22_pcs_get_state
>>>   net: mdiobus: add clause 45 mdiobus write accessor
>>>   net: phy: add Lynx PCS module
>>>   net: dsa: ocelot: use the Lynx PCS helpers in Felix and Seville
>>>
>>>  MAINTAINERS                              |   7 +
>>>  drivers/net/dsa/ocelot/Kconfig           |   1 +
>>>  drivers/net/dsa/ocelot/felix.c           |  28 +-
>>>  drivers/net/dsa/ocelot/felix.h           |  20 +-
>>>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 374 ++---------------------
>>>  drivers/net/dsa/ocelot/seville_vsc9953.c |  21 +-
>>>  drivers/net/phy/Kconfig                  |   6 +
>>>  drivers/net/phy/Makefile                 |   1 +
>>>  drivers/net/phy/pcs-lynx.c               | 314 +++++++++++++++++++
>>
>> I believe Andrew had a plan to create a better organization within
>> drivers/net/phy, while this happens, maybe you can already create
>> drivers/net/phy/pcs/ regardless of the state of Andrew's work?
>>
> 
> I got the impression from Andrew that the plan was to do this at a later stage
> together with the Synopsys XPCS. I could certainly do what you say, I am just
> not very keen to add such things into this patch set. 

Andrew, what directory structure do you have in mind such that Ioana can
already start putting the PCS drivers in the right location?
Andrew Lunn July 27, 2020, 8:28 p.m. UTC | #6
> Andrew, what directory structure do you have in mind such that Ioana can
> already start putting the PCS drivers in the right location?

Hi Florian

I will push out an RFC soon. It will need some build testing, just to
make sure i've not broken any Kconfig dependencies.

     Andrew