| Message ID | 20210218052654.28995-1-calvin.johnson@oss.nxp.com |
|---|---|
| Headers | show |
| Series | ACPI support for dpaa2 driver | expand |
On Thu, Feb 18, 2021 at 7:28 AM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > > Define acpi_mdiobus_register() to Register mii_bus and create PHYs for > each ACPI child node. > +#include <linux/acpi.h> > +#include <linux/acpi_mdio.h> Perhaps it's better to provide the headers that this file is direct user of, i.e. bits.h dev_printk.h module.h types.h The rest seems fine because they are guaranteed to be included by acpi.h (IIUC about fwnode API and acpi_mdio includes MDIO PHY APIs).
On Thu, Feb 18, 2021 at 05:08:05PM +0200, Andy Shevchenko wrote: > On Thu, Feb 18, 2021 at 7:28 AM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > > > Define acpi_mdiobus_register() to Register mii_bus and create PHYs for > > each ACPI child node. > > > +#include <linux/acpi.h> > > +#include <linux/acpi_mdio.h> > > Perhaps it's better to provide the headers that this file is direct > user of, i.e. > bits.h > dev_printk.h Looks like device.h needs to be used instead of dev_printk.h. Please let me know if you've a different opinion. > module.h > types.h > > The rest seems fine because they are guaranteed to be included by > acpi.h (IIUC about fwnode API and acpi_mdio includes MDIO PHY APIs). > Thanks Calvin
On Mon, Mar 8, 2021 at 4:11 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > On Thu, Feb 18, 2021 at 05:08:05PM +0200, Andy Shevchenko wrote: > > On Thu, Feb 18, 2021 at 7:28 AM Calvin Johnson > > <calvin.johnson@oss.nxp.com> wrote: > > > Define acpi_mdiobus_register() to Register mii_bus and create PHYs for > > > each ACPI child node. > > > > > +#include <linux/acpi.h> > > > +#include <linux/acpi_mdio.h> > > > > Perhaps it's better to provide the headers that this file is direct > > user of, i.e. > > bits.h > > dev_printk.h > > Looks like device.h needs to be used instead of dev_printk.h. Please > let me know if you've a different opinion. I don't see the user of device.h. dev_printk.h is definitely in use here... Do you see a user for device.h? Which line in your code requires it? It might be that I don't see something quite obvious... > > module.h > > types.h > > > > The rest seems fine because they are guaranteed to be included by > > acpi.h (IIUC about fwnode API and acpi_mdio includes MDIO PHY APIs). -- With Best Regards, Andy Shevchenko
On Mon, Mar 08, 2021 at 04:57:35PM +0200, Andy Shevchenko wrote: > On Mon, Mar 8, 2021 at 4:11 PM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > On Thu, Feb 18, 2021 at 05:08:05PM +0200, Andy Shevchenko wrote: > > > On Thu, Feb 18, 2021 at 7:28 AM Calvin Johnson > > > <calvin.johnson@oss.nxp.com> wrote: > > > > > Define acpi_mdiobus_register() to Register mii_bus and create PHYs for > > > > each ACPI child node. > > > > > > > +#include <linux/acpi.h> > > > > +#include <linux/acpi_mdio.h> > > > > > > Perhaps it's better to provide the headers that this file is direct > > > user of, i.e. > > > bits.h > > > dev_printk.h > > > > Looks like device.h needs to be used instead of dev_printk.h. Please > > let me know if you've a different opinion. > > I don't see the user of device.h. dev_printk.h is definitely in use here... > Do you see a user for device.h? Which line in your code requires it? > > It might be that I don't see something quite obvious... I thought of including device.h instead of dev_printk.h because, it is the only file that includes dev_printk.h and device.h is widely used. Of course, it will mean that dev_printk.h is indirectly called. Regards Calvin
On Mon, Mar 8, 2021 at 6:28 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > On Mon, Mar 08, 2021 at 04:57:35PM +0200, Andy Shevchenko wrote: .... > I thought of including device.h instead of dev_printk.h because, it is the > only file that includes dev_printk.h and device.h is widely used. Of course, > it will mean that dev_printk.h is indirectly called. The split happened recently, not every developer knows about it and definitely most of the contributors are too lazy to properly write the inclusion block in their code.
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_mdiobus_register() 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: T2080RDB, LS1046ARDB, LS2088ARDB and LX2160ARDB Changes in v6: - Minor cleanup - 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: 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: mdiobus: Introduce fwnode_mdiobus_register() net/fsl: Use fwnode_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 | 1 + drivers/acpi/utils.c | 14 ++ .../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 91 +++++++----- drivers/net/ethernet/freescale/xgmac_mdio.c | 11 +- drivers/net/mdio/Kconfig | 7 + drivers/net/mdio/Makefile | 1 + drivers/net/mdio/acpi_mdio.c | 51 +++++++ drivers/net/mdio/of_mdio.c | 79 +---------- drivers/net/phy/mdio_bus.c | 86 +++++++++++ drivers/net/phy/phy_device.c | 106 ++++++++++++++ drivers/net/phy/phylink.c | 41 ++++-- include/linux/acpi.h | 7 + include/linux/acpi_mdio.h | 25 ++++ include/linux/mdio.h | 2 + include/linux/of_mdio.h | 6 +- include/linux/phy.h | 32 +++++ include/linux/phylink.h | 3 + 18 files changed, 570 insertions(+), 126 deletions(-) create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst create mode 100644 drivers/net/mdio/acpi_mdio.c create mode 100644 include/linux/acpi_mdio.h