[PATCH net-next v8 00/15] ACPI support for dpaa2 driver

Ioana Ciornei posted 15 patches 1 week, 4 days ago
Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++
MAINTAINERS                                   |   2 +
drivers/acpi/utils.c                          |  14 ++
.../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  88 ++++++-----
.../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   2 +-
drivers/net/ethernet/freescale/xgmac_mdio.c   |  30 ++--
drivers/net/mdio/Kconfig                      |  14 ++
drivers/net/mdio/Makefile                     |   4 +-
drivers/net/mdio/acpi_mdio.c                  |  56 +++++++
drivers/net/mdio/fwnode_mdio.c                | 144 ++++++++++++++++++
drivers/net/mdio/of_mdio.c                    | 138 ++---------------
drivers/net/phy/mii_timestamper.c             |   3 +
drivers/net/phy/phy_device.c                  | 109 ++++++++++++-
drivers/net/phy/phylink.c                     |  41 +++--
include/linux/acpi.h                          |   7 +
include/linux/acpi_mdio.h                     |  26 ++++
include/linux/fwnode_mdio.h                   |  35 +++++
include/linux/phy.h                           |  32 ++++
include/linux/phylink.h                       |   3 +
19 files changed, 691 insertions(+), 190 deletions(-)
create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
create mode 100644 drivers/net/mdio/acpi_mdio.c
create mode 100644 drivers/net/mdio/fwnode_mdio.c
create mode 100644 include/linux/acpi_mdio.h
create mode 100644 include/linux/fwnode_mdio.h

[PATCH net-next v8 00/15] ACPI support for dpaa2 driver

Posted by Ioana Ciornei 1 week, 4 days ago
From: Ioana Ciornei <ioana.ciornei@nxp.com>

This patch set provides ACPI support to DPAA2 network drivers.

It also introduces new fwnode based APIs to support phylink and phy
layers
    Following functions are defined:
      phylink_fwnode_phy_connect()
      fwnode_mdiobus_register_phy()
      fwnode_get_phy_id()
      fwnode_phy_find_device()
      device_phy_find_device()
      fwnode_get_phy_node()
      fwnode_mdio_find_device()
      acpi_get_local_address()

    First one helps in connecting phy to phylink instance.
    Next three helps in getting phy_id and registering phy to mdiobus
    Next two help in finding a phy on a mdiobus.
    Next one helps in getting phy_node from a fwnode.
    Last one is used to get local address from _ADR object.

    Corresponding OF functions are refactored.

Tested-on: LX2160ARDB

Changes in v8:
 - fixed some checkpatch warnings/checks
 - included linux/fwnode_mdio.h in fwnode_mdio.c (fixed the build warnings)
 - added fwnode_find_mii_timestamper() and
   fwnode_mdiobus_phy_device_register() in order to get rid of the cycle
   dependency.
 - change to 'depends on (ACPI || OF) || COMPILE_TEST (for FWNODE_MDIO)
 - remove the fwnode_mdiobus_register from fwnode_mdio.c since it
   introduces a cycle of dependencies.

Changes in v7:
- correct fwnode_mdio_find_device() description
- check NULL in unregister_mii_timestamper()
- Call unregister_mii_timestamper() without NULL check
- Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
- include fwnode_mdio.h
- Include headers directly used in acpi_mdio.c
- Move fwnode_mdiobus_register() to fwnode_mdio.c
- Include fwnode_mdio.h
- Alphabetically sort header inclusions
- remove unnecassary checks

Changes in v6:
- Minor cleanup
- fix warning for function parameter of fwnode_mdio_find_device()
- Initialize mii_ts to NULL
- use GENMASK() and ACPI_COMPANION_SET()
- some cleanup
- remove unwanted header inclusion
- remove OF check for fixed-link
- use dev_fwnode()
- remove useless else
- replace of_device_is_available() to fwnode_device_is_available()

Changes in v5:
- More cleanup
- Replace fwnode_get_id() with acpi_get_local_address()
- add missing MODULE_LICENSE()
- replace fwnode_get_id() with OF and ACPI function calls
- replace fwnode_get_id() with OF and ACPI function calls

Changes in v4:
- More cleanup
- Improve code structure to handle all cases
- Remove redundant else from fwnode_mdiobus_register()
- Cleanup xgmac_mdio_probe()
- call phy_device_free() before returning

Changes in v3:
- Add more info on legacy DT properties "phy" and "phy-device"
- Redefine fwnode_phy_find_device() to follow of_phy_find_device()
- Use traditional comparison pattern
- Use GENMASK
- Modified to retrieve reg property value for ACPI as well
- Resolved compilation issue with CONFIG_ACPI = n
- Added more info into documentation
- Use acpi_mdiobus_register()
- Avoid unnecessary line removal
- Remove unused inclusion of acpi.h

Changes in v2:
- Updated with more description in document
- use reverse christmas tree ordering for local variables
- Refactor OF functions to use fwnode functions

Calvin Johnson (15):
  Documentation: ACPI: DSD: Document MDIO PHY
  net: phy: Introduce fwnode_mdio_find_device()
  net: phy: Introduce phy related fwnode functions
  of: mdio: Refactor of_phy_find_device()
  net: phy: Introduce fwnode_get_phy_id()
  of: mdio: Refactor of_get_phy_id()
  net: mii_timestamper: check NULL in unregister_mii_timestamper()
  net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  of: mdio: Refactor of_mdiobus_register_phy()
  ACPI: utils: Introduce acpi_get_local_address()
  net: mdio: Add ACPI support code for mdio
  net/fsl: Use [acpi|of]_mdiobus_register
  net: phylink: introduce phylink_fwnode_phy_connect()
  net: phylink: Refactor phylink_of_phy_connect()
  net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver

 Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++
 MAINTAINERS                                   |   2 +
 drivers/acpi/utils.c                          |  14 ++
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  88 ++++++-----
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   2 +-
 drivers/net/ethernet/freescale/xgmac_mdio.c   |  30 ++--
 drivers/net/mdio/Kconfig                      |  14 ++
 drivers/net/mdio/Makefile                     |   4 +-
 drivers/net/mdio/acpi_mdio.c                  |  56 +++++++
 drivers/net/mdio/fwnode_mdio.c                | 144 ++++++++++++++++++
 drivers/net/mdio/of_mdio.c                    | 138 ++---------------
 drivers/net/phy/mii_timestamper.c             |   3 +
 drivers/net/phy/phy_device.c                  | 109 ++++++++++++-
 drivers/net/phy/phylink.c                     |  41 +++--
 include/linux/acpi.h                          |   7 +
 include/linux/acpi_mdio.h                     |  26 ++++
 include/linux/fwnode_mdio.h                   |  35 +++++
 include/linux/phy.h                           |  32 ++++
 include/linux/phylink.h                       |   3 +
 19 files changed, 691 insertions(+), 190 deletions(-)
 create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
 create mode 100644 drivers/net/mdio/acpi_mdio.c
 create mode 100644 drivers/net/mdio/fwnode_mdio.c
 create mode 100644 include/linux/acpi_mdio.h
 create mode 100644 include/linux/fwnode_mdio.h

-- 
2.31.1

Re: [PATCH net-next v8 00/15] ACPI support for dpaa2 driver

Posted by Andy Shevchenko 1 week, 3 days ago
On Thu, Jun 10, 2021 at 7:40 PM Ioana Ciornei <ciorneiioana@gmail.com> wrote:
>
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
>
> This patch set provides ACPI support to DPAA2 network drivers.
>
> It also introduces new fwnode based APIs to support phylink and phy
> layers
>     Following functions are defined:
>       phylink_fwnode_phy_connect()
>       fwnode_mdiobus_register_phy()
>       fwnode_get_phy_id()
>       fwnode_phy_find_device()
>       device_phy_find_device()
>       fwnode_get_phy_node()
>       fwnode_mdio_find_device()
>       acpi_get_local_address()
>
>     First one helps in connecting phy to phylink instance.
>     Next three helps in getting phy_id and registering phy to mdiobus
>     Next two help in finding a phy on a mdiobus.
>     Next one helps in getting phy_node from a fwnode.
>     Last one is used to get local address from _ADR object.
>
>     Corresponding OF functions are refactored.

In general it looks fine to me. What really worries me is the calls like

of_foo -> fwnode_bar -> of_baz.

As I have commented in one patch the idea of fwnode APIs is to have a
common ground for all resource providers. So, at the end it shouldn't
be a chain of calls like above mentioned. Either fix the name (so, the
first one will be in fwnode or device namespace) or fix the API that
it will be fwnode/device API.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v8 00/15] ACPI support for dpaa2 driver

Posted by Ioana Ciornei 1 week, 3 days ago
On Fri, Jun 11, 2021 at 12:00:02PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 10, 2021 at 7:40 PM Ioana Ciornei <ciorneiioana@gmail.com> wrote:
> >
> > From: Ioana Ciornei <ioana.ciornei@nxp.com>
> >
> > This patch set provides ACPI support to DPAA2 network drivers.
> >
> > It also introduces new fwnode based APIs to support phylink and phy
> > layers
> >     Following functions are defined:
> >       phylink_fwnode_phy_connect()
> >       fwnode_mdiobus_register_phy()
> >       fwnode_get_phy_id()
> >       fwnode_phy_find_device()
> >       device_phy_find_device()
> >       fwnode_get_phy_node()
> >       fwnode_mdio_find_device()
> >       acpi_get_local_address()
> >
> >     First one helps in connecting phy to phylink instance.
> >     Next three helps in getting phy_id and registering phy to mdiobus
> >     Next two help in finding a phy on a mdiobus.
> >     Next one helps in getting phy_node from a fwnode.
> >     Last one is used to get local address from _ADR object.
> >
> >     Corresponding OF functions are refactored.
> 
> In general it looks fine to me. What really worries me is the calls like
> 
> of_foo -> fwnode_bar -> of_baz.
> 
> As I have commented in one patch the idea of fwnode APIs is to have a
> common ground for all resource providers. So, at the end it shouldn't
> be a chain of calls like above mentioned. Either fix the name (so, the
> first one will be in fwnode or device namespace) or fix the API that
> it will be fwnode/device API.


These types of cyclic calls do not exist anymore.
The fwnode_mdio does not call back into of_mdio but instead it directly
implements any necessary operations using the fwnode_handle.

The only calls happening are 'of_mdio -> fwnode_mdio' so that we
leverage the common fwnode handling and do not duplicate code.

Ioana

Re: [PATCH net-next v8 00/15] ACPI support for dpaa2 driver

Posted by Grant Likely 1 week, 4 days ago
On 10/06/2021 17:39, Ioana Ciornei wrote:
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> This patch set provides ACPI support to DPAA2 network drivers.
> 
> It also introduces new fwnode based APIs to support phylink and phy
> layers
>      Following functions are defined:
>        phylink_fwnode_phy_connect()
>        fwnode_mdiobus_register_phy()
>        fwnode_get_phy_id()
>        fwnode_phy_find_device()
>        device_phy_find_device()
>        fwnode_get_phy_node()
>        fwnode_mdio_find_device()
>        acpi_get_local_address()
> 
>      First one helps in connecting phy to phylink instance.
>      Next three helps in getting phy_id and registering phy to mdiobus
>      Next two help in finding a phy on a mdiobus.
>      Next one helps in getting phy_node from a fwnode.
>      Last one is used to get local address from _ADR object.
> 
>      Corresponding OF functions are refactored.

In terms of design, this whole series looks right to me. The way data is 
encoded in ACPI is entirely appropriate. As long as Rob Herring is okay 
with the DT code changes I support this series being merged.

For the whole series:
Acked-by: Grant Likely <grant.likely@arm.com>


> 
> Tested-on: LX2160ARDB
> 
> Changes in v8:
>   - fixed some checkpatch warnings/checks
>   - included linux/fwnode_mdio.h in fwnode_mdio.c (fixed the build warnings)
>   - added fwnode_find_mii_timestamper() and
>     fwnode_mdiobus_phy_device_register() in order to get rid of the cycle
>     dependency.
>   - change to 'depends on (ACPI || OF) || COMPILE_TEST (for FWNODE_MDIO)
>   - remove the fwnode_mdiobus_register from fwnode_mdio.c since it
>     introduces a cycle of dependencies.
> 
> Changes in v7:
> - correct fwnode_mdio_find_device() description
> - check NULL in unregister_mii_timestamper()
> - Call unregister_mii_timestamper() without NULL check
> - Create fwnode_mdio.c and move fwnode_mdiobus_register_phy()
> - include fwnode_mdio.h
> - Include headers directly used in acpi_mdio.c
> - Move fwnode_mdiobus_register() to fwnode_mdio.c
> - Include fwnode_mdio.h
> - Alphabetically sort header inclusions
> - remove unnecassary checks
> 
> Changes in v6:
> - Minor cleanup
> - fix warning for function parameter of fwnode_mdio_find_device()
> - Initialize mii_ts to NULL
> - use GENMASK() and ACPI_COMPANION_SET()
> - some cleanup
> - remove unwanted header inclusion
> - remove OF check for fixed-link
> - use dev_fwnode()
> - remove useless else
> - replace of_device_is_available() to fwnode_device_is_available()
> 
> Changes in v5:
> - More cleanup
> - Replace fwnode_get_id() with acpi_get_local_address()
> - add missing MODULE_LICENSE()
> - replace fwnode_get_id() with OF and ACPI function calls
> - replace fwnode_get_id() with OF and ACPI function calls
> 
> Changes in v4:
> - More cleanup
> - Improve code structure to handle all cases
> - Remove redundant else from fwnode_mdiobus_register()
> - Cleanup xgmac_mdio_probe()
> - call phy_device_free() before returning
> 
> Changes in v3:
> - Add more info on legacy DT properties "phy" and "phy-device"
> - Redefine fwnode_phy_find_device() to follow of_phy_find_device()
> - Use traditional comparison pattern
> - Use GENMASK
> - Modified to retrieve reg property value for ACPI as well
> - Resolved compilation issue with CONFIG_ACPI = n
> - Added more info into documentation
> - Use acpi_mdiobus_register()
> - Avoid unnecessary line removal
> - Remove unused inclusion of acpi.h
> 
> Changes in v2:
> - Updated with more description in document
> - use reverse christmas tree ordering for local variables
> - Refactor OF functions to use fwnode functions
> 
> Calvin Johnson (15):
>    Documentation: ACPI: DSD: Document MDIO PHY
>    net: phy: Introduce fwnode_mdio_find_device()
>    net: phy: Introduce phy related fwnode functions
>    of: mdio: Refactor of_phy_find_device()
>    net: phy: Introduce fwnode_get_phy_id()
>    of: mdio: Refactor of_get_phy_id()
>    net: mii_timestamper: check NULL in unregister_mii_timestamper()
>    net: mdiobus: Introduce fwnode_mdiobus_register_phy()
>    of: mdio: Refactor of_mdiobus_register_phy()
>    ACPI: utils: Introduce acpi_get_local_address()
>    net: mdio: Add ACPI support code for mdio
>    net/fsl: Use [acpi|of]_mdiobus_register
>    net: phylink: introduce phylink_fwnode_phy_connect()
>    net: phylink: Refactor phylink_of_phy_connect()
>    net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver
> 
>   Documentation/firmware-guide/acpi/dsd/phy.rst | 133 ++++++++++++++++
>   MAINTAINERS                                   |   2 +
>   drivers/acpi/utils.c                          |  14 ++
>   .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  88 ++++++-----
>   .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   2 +-
>   drivers/net/ethernet/freescale/xgmac_mdio.c   |  30 ++--
>   drivers/net/mdio/Kconfig                      |  14 ++
>   drivers/net/mdio/Makefile                     |   4 +-
>   drivers/net/mdio/acpi_mdio.c                  |  56 +++++++
>   drivers/net/mdio/fwnode_mdio.c                | 144 ++++++++++++++++++
>   drivers/net/mdio/of_mdio.c                    | 138 ++---------------
>   drivers/net/phy/mii_timestamper.c             |   3 +
>   drivers/net/phy/phy_device.c                  | 109 ++++++++++++-
>   drivers/net/phy/phylink.c                     |  41 +++--
>   include/linux/acpi.h                          |   7 +
>   include/linux/acpi_mdio.h                     |  26 ++++
>   include/linux/fwnode_mdio.h                   |  35 +++++
>   include/linux/phy.h                           |  32 ++++
>   include/linux/phylink.h                       |   3 +
>   19 files changed, 691 insertions(+), 190 deletions(-)
>   create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
>   create mode 100644 drivers/net/mdio/acpi_mdio.c
>   create mode 100644 drivers/net/mdio/fwnode_mdio.c
>   create mode 100644 include/linux/acpi_mdio.h
>   create mode 100644 include/linux/fwnode_mdio.h
>