devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH v4 00/15] ACPI support for dpaa2 driver
@ 2021-01-22 15:42 Calvin Johnson
  2021-01-22 15:42 ` [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
  0 siblings, 1 reply; 7+ messages in thread
From: Calvin Johnson @ 2021-01-22 15:42 UTC (permalink / raw)
  To: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Andy Shevchenko, Florian Fainelli,
	Russell King - ARM Linux admin, Cristi Sovaiala,
	Florin Laurentiu Chiculita, Ioana Ciornei, Madalin Bucur,
	Heikki Krogerus, Marcin Wojtas, Pieter Jansen Van Vuuren, Jon,
	Saravana Kannan, Randy Dunlap
  Cc: linux.cj, Diana Madalina Craciun, linux-acpi, linux-arm-kernel,
	linux-kernel, netdev, Laurentiu Tudor, Calvin Johnson,
	Andy Shevchenko, Bartosz Golaszewski, David S. Miller,
	Frank Rowand, Greg Kroah-Hartman, Heiner Kallweit,
	Ioana Radulescu, Jakub Kicinski, Jamie Iles, Kieran Bingham,
	Laurent Pinchart, Len Brown, Rafael J. Wysocki, Rob Herring,
	devicetree


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()
      fwnode_get_id()

    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 fwnode ID.

    Corresponding OF functions are refactored.

Tested-on: LS2088ARDB and LX2160ARDB


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()
  device property: Introduce fwnode_get_id()
  net: mdio: Add ACPI support code for mdio
  net: mdiobus: Introduce fwnode_mdiobus_register()
  net/fsl: Use fwnode_mdiobus_register()
  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 | 129 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 drivers/base/property.c                       |  34 +++++
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  87 +++++++-----
 drivers/net/ethernet/freescale/xgmac_mdio.c   |  11 +-
 drivers/net/mdio/Kconfig                      |   7 +
 drivers/net/mdio/Makefile                     |   1 +
 drivers/net/mdio/acpi_mdio.c                  |  49 +++++++
 drivers/net/mdio/of_mdio.c                    |  79 +----------
 drivers/net/phy/mdio_bus.c                    |  88 ++++++++++++
 drivers/net/phy/phy_device.c                  | 106 ++++++++++++++
 drivers/net/phy/phylink.c                     |  53 ++++---
 include/linux/acpi_mdio.h                     |  27 ++++
 include/linux/mdio.h                          |   2 +
 include/linux/of_mdio.h                       |   6 +-
 include/linux/phy.h                           |  32 +++++
 include/linux/phylink.h                       |   3 +
 include/linux/property.h                      |   1 +
 18 files changed, 584 insertions(+), 132 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

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  2021-01-22 15:42 [net-next PATCH v4 00/15] ACPI support for dpaa2 driver Calvin Johnson
@ 2021-01-22 15:42 ` Calvin Johnson
  2021-02-05 17:25   ` Calvin Johnson
  0 siblings, 1 reply; 7+ messages in thread
From: Calvin Johnson @ 2021-01-22 15:42 UTC (permalink / raw)
  To: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Andy Shevchenko, Florian Fainelli,
	Russell King - ARM Linux admin, Cristi Sovaiala,
	Florin Laurentiu Chiculita, Ioana Ciornei, Madalin Bucur,
	Heikki Krogerus, Marcin Wojtas, Pieter Jansen Van Vuuren, Jon,
	Saravana Kannan, Randy Dunlap
  Cc: linux.cj, Diana Madalina Craciun, linux-acpi, linux-arm-kernel,
	linux-kernel, netdev, Laurentiu Tudor, Calvin Johnson,
	David S. Miller, Frank Rowand, Heiner Kallweit, Jakub Kicinski,
	Rob Herring, devicetree

Introduce fwnode_mdiobus_register_phy() to register PHYs on the
mdiobus. From the compatible string, identify whether the PHY is
c45 and based on this create a PHY device instance which is
registered on the mdiobus.

Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/mdio/of_mdio.c |  3 +-
 drivers/net/phy/mdio_bus.c | 67 ++++++++++++++++++++++++++++++++++++++
 include/linux/mdio.h       |  2 ++
 include/linux/of_mdio.h    |  6 +++-
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index d4cc293358f7..cd7da38ae763 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -32,7 +32,7 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
 	return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
 }
 
-static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
+struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
 {
 	struct of_phandle_args arg;
 	int err;
@@ -49,6 +49,7 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
 
 	return register_mii_timestamper(arg.np, arg.args[0]);
 }
+EXPORT_SYMBOL(of_find_mii_timestamper);
 
 int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
 			      struct device_node *child, u32 addr)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 040509b81f02..44ddfb0ba99f 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -8,6 +8,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/errno.h>
@@ -106,6 +107,72 @@ int mdiobus_unregister_device(struct mdio_device *mdiodev)
 }
 EXPORT_SYMBOL(mdiobus_unregister_device);
 
+int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+				struct fwnode_handle *child, u32 addr)
+{
+	struct mii_timestamper *mii_ts;
+	struct phy_device *phy;
+	bool is_c45 = false;
+	u32 phy_id;
+	int rc;
+
+	if (is_of_node(child)) {
+		mii_ts = of_find_mii_timestamper(to_of_node(child));
+		if (IS_ERR(mii_ts))
+			return PTR_ERR(mii_ts);
+	}
+
+	rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
+	if (rc >= 0)
+		is_c45 = true;
+
+	if (is_c45 || fwnode_get_phy_id(child, &phy_id))
+		phy = get_phy_device(bus, addr, is_c45);
+	else
+		phy = phy_device_create(bus, addr, phy_id, 0, NULL);
+	if (IS_ERR(phy)) {
+		if (mii_ts && is_of_node(child))
+			unregister_mii_timestamper(mii_ts);
+		return PTR_ERR(phy);
+	}
+
+	if (is_acpi_node(child)) {
+		phy->irq = bus->irq[addr];
+
+		/* Associate the fwnode with the device structure so it
+		 * can be looked up later.
+		 */
+		phy->mdio.dev.fwnode = child;
+
+		/* All data is now stored in the phy struct, so register it */
+		rc = phy_device_register(phy);
+		if (rc) {
+			phy_device_free(phy);
+			fwnode_handle_put(phy->mdio.dev.fwnode);
+			return rc;
+		}
+
+		dev_dbg(&bus->dev, "registered phy at address %i\n", addr);
+	} else if (is_of_node(child)) {
+		rc = of_mdiobus_phy_device_register(bus, phy, to_of_node(child), addr);
+		if (rc) {
+			if (mii_ts)
+				unregister_mii_timestamper(mii_ts);
+			phy_device_free(phy);
+			return rc;
+		}
+
+		/* phy->mii_ts may already be defined by the PHY driver. A
+		 * mii_timestamper probed via the device tree will still have
+		 * precedence.
+		 */
+		if (mii_ts)
+			phy->mii_ts = mii_ts;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
 struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
 {
 	struct mdio_device *mdiodev = bus->mdio_map[addr];
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index ffb787d5ebde..7f4215c069fe 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -381,6 +381,8 @@ int mdiobus_register_device(struct mdio_device *mdiodev);
 int mdiobus_unregister_device(struct mdio_device *mdiodev);
 bool mdiobus_is_registered_device(struct mii_bus *bus, int addr);
 struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr);
+int fwnode_mdiobus_register_phy(struct mii_bus *bus,
+				      struct fwnode_handle *child, u32 addr);
 
 /**
  * mdio_module_driver() - Helper macro for registering mdio drivers
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index cfe8c607a628..3b66016f18aa 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -34,6 +34,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
 int of_phy_register_fixed_link(struct device_node *np);
 void of_phy_deregister_fixed_link(struct device_node *np);
 bool of_phy_is_fixed_link(struct device_node *np);
+struct mii_timestamper *of_find_mii_timestamper(struct device_node *np);
 int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
 				   struct device_node *child, u32 addr);
 
@@ -128,7 +129,10 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
 {
 	return false;
 }
-
+static inline struct mii_timestamper *of_find_mii_timestamper(struct device_node *np)
+{
+	return NULL;
+}
 static inline int of_mdiobus_phy_device_register(struct mii_bus *mdio,
 					    struct phy_device *phy,
 					    struct device_node *child, u32 addr)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  2021-01-22 15:42 ` [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
@ 2021-02-05 17:25   ` Calvin Johnson
  2021-02-05 18:25     ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Calvin Johnson @ 2021-02-05 17:25 UTC (permalink / raw)
  To: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Andy Shevchenko, Florian Fainelli,
	Russell King - ARM Linux admin, Cristi Sovaiala,
	Florin Laurentiu Chiculita, Ioana Ciornei, Madalin Bucur,
	Heikki Krogerus, Marcin Wojtas, Pieter Jansen Van Vuuren, Jon,
	Saravana Kannan, Randy Dunlap
  Cc: linux.cj, Diana Madalina Craciun, linux-acpi, linux-arm-kernel,
	linux-kernel, netdev, Laurentiu Tudor, David S. Miller,
	Frank Rowand, Heiner Kallweit, Jakub Kicinski, Rob Herring,
	devicetree

On Fri, Jan 22, 2021 at 09:12:52PM +0530, Calvin Johnson wrote:
> Introduce fwnode_mdiobus_register_phy() to register PHYs on the
> mdiobus. From the compatible string, identify whether the PHY is
> c45 and based on this create a PHY device instance which is
> registered on the mdiobus.
> 
> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> ---
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/net/mdio/of_mdio.c |  3 +-
>  drivers/net/phy/mdio_bus.c | 67 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mdio.h       |  2 ++
>  include/linux/of_mdio.h    |  6 +++-
>  4 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
> index d4cc293358f7..cd7da38ae763 100644
> --- a/drivers/net/mdio/of_mdio.c
> +++ b/drivers/net/mdio/of_mdio.c
> @@ -32,7 +32,7 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
>  	return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
>  }
>  
> -static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> +struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
>  {
>  	struct of_phandle_args arg;
>  	int err;
> @@ -49,6 +49,7 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
>  
>  	return register_mii_timestamper(arg.np, arg.args[0]);
>  }
> +EXPORT_SYMBOL(of_find_mii_timestamper);
>  
>  int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
>  			      struct device_node *child, u32 addr)
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 040509b81f02..44ddfb0ba99f 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -8,6 +8,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/errno.h>
> @@ -106,6 +107,72 @@ int mdiobus_unregister_device(struct mdio_device *mdiodev)
>  }
>  EXPORT_SYMBOL(mdiobus_unregister_device);
>  
> +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> +				struct fwnode_handle *child, u32 addr)
> +{
> +	struct mii_timestamper *mii_ts;
> +	struct phy_device *phy;
> +	bool is_c45 = false;
> +	u32 phy_id;
> +	int rc;
> +
> +	if (is_of_node(child)) {
> +		mii_ts = of_find_mii_timestamper(to_of_node(child));
> +		if (IS_ERR(mii_ts))
> +			return PTR_ERR(mii_ts);
> +	}
> +
> +	rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
With ACPI, I'm facing some problem with fwnode_property_match_string(). It is
unable to detect the compatible string and returns -EPROTO.

ACPI node for PHY4 is as below:

 Device(PHY4) {
    Name (_ADR, 0x4)
    Name(_CRS, ResourceTemplate() {
    Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
    {
      AQR_PHY4_IT
    }
    }) // end of _CRS for PHY4
    Name (_DSD, Package () {
      ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package () {
          Package () {"compatible", "ethernet-phy-ieee802.3-c45"}
       }
    })
  } // end of PHY4

 What is see is that in acpi_data_get_property(),
propvalue->type = 0x2(ACPI_TYPE_STRING) and type = 0x4(ACPI_TYPE_PACKAGE).

Any help please?

fwnode_property_match_string() works fine for DT.

Thanks
Calvin

> +	if (rc >= 0)
> +		is_c45 = true;
> +
> +	if (is_c45 || fwnode_get_phy_id(child, &phy_id))
> +		phy = get_phy_device(bus, addr, is_c45);
> +	else
> +		phy = phy_device_create(bus, addr, phy_id, 0, NULL);
> +	if (IS_ERR(phy)) {
> +		if (mii_ts && is_of_node(child))
> +			unregister_mii_timestamper(mii_ts);
> +		return PTR_ERR(phy);
> +	}
> +
> +	if (is_acpi_node(child)) {
> +		phy->irq = bus->irq[addr];
> +
> +		/* Associate the fwnode with the device structure so it
> +		 * can be looked up later.
> +		 */
> +		phy->mdio.dev.fwnode = child;
> +
> +		/* All data is now stored in the phy struct, so register it */
> +		rc = phy_device_register(phy);
> +		if (rc) {
> +			phy_device_free(phy);
> +			fwnode_handle_put(phy->mdio.dev.fwnode);
> +			return rc;
> +		}
> +
> +		dev_dbg(&bus->dev, "registered phy at address %i\n", addr);
> +	} else if (is_of_node(child)) {
> +		rc = of_mdiobus_phy_device_register(bus, phy, to_of_node(child), addr);
> +		if (rc) {
> +			if (mii_ts)
> +				unregister_mii_timestamper(mii_ts);
> +			phy_device_free(phy);
> +			return rc;
> +		}
> +
> +		/* phy->mii_ts may already be defined by the PHY driver. A
> +		 * mii_timestamper probed via the device tree will still have
> +		 * precedence.
> +		 */
> +		if (mii_ts)
> +			phy->mii_ts = mii_ts;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
> +
>  struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
>  {
>  	struct mdio_device *mdiodev = bus->mdio_map[addr];
> diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> index ffb787d5ebde..7f4215c069fe 100644
> --- a/include/linux/mdio.h
> +++ b/include/linux/mdio.h
> @@ -381,6 +381,8 @@ int mdiobus_register_device(struct mdio_device *mdiodev);
>  int mdiobus_unregister_device(struct mdio_device *mdiodev);
>  bool mdiobus_is_registered_device(struct mii_bus *bus, int addr);
>  struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr);
> +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> +				      struct fwnode_handle *child, u32 addr);
>  
>  /**
>   * mdio_module_driver() - Helper macro for registering mdio drivers
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index cfe8c607a628..3b66016f18aa 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -34,6 +34,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
>  int of_phy_register_fixed_link(struct device_node *np);
>  void of_phy_deregister_fixed_link(struct device_node *np);
>  bool of_phy_is_fixed_link(struct device_node *np);
> +struct mii_timestamper *of_find_mii_timestamper(struct device_node *np);
>  int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
>  				   struct device_node *child, u32 addr);
>  
> @@ -128,7 +129,10 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
>  {
>  	return false;
>  }
> -
> +static inline struct mii_timestamper *of_find_mii_timestamper(struct device_node *np)
> +{
> +	return NULL;
> +}
>  static inline int of_mdiobus_phy_device_register(struct mii_bus *mdio,
>  					    struct phy_device *phy,
>  					    struct device_node *child, u32 addr)
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  2021-02-05 17:25   ` Calvin Johnson
@ 2021-02-05 18:25     ` Andy Shevchenko
  2021-02-05 18:41       ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2021-02-05 18:25 UTC (permalink / raw)
  To: Calvin Johnson
  Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Florian Fainelli, Russell King - ARM Linux admin,
	Cristi Sovaiala, Florin Laurentiu Chiculita, Ioana Ciornei,
	Madalin Bucur, Heikki Krogerus, Marcin Wojtas,
	Pieter Jansen Van Vuuren, Jon, Saravana Kannan, Randy Dunlap,
	linux.cj, Diana Madalina Craciun, ACPI Devel Maling List,
	linux-arm Mailing List, Linux Kernel Mailing List, netdev,
	Laurentiu Tudor, David S. Miller, Frank Rowand, Heiner Kallweit,
	Jakub Kicinski, Rob Herring, devicetree

On Fri, Feb 5, 2021 at 7:25 PM Calvin Johnson
<calvin.johnson@oss.nxp.com> wrote:
> On Fri, Jan 22, 2021 at 09:12:52PM +0530, Calvin Johnson wrote:

...

> > +     rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
> With ACPI, I'm facing some problem with fwnode_property_match_string(). It is
> unable to detect the compatible string and returns -EPROTO.
>
> ACPI node for PHY4 is as below:
>
>  Device(PHY4) {
>     Name (_ADR, 0x4)
>     Name(_CRS, ResourceTemplate() {
>     Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
>     {
>       AQR_PHY4_IT
>     }
>     }) // end of _CRS for PHY4
>     Name (_DSD, Package () {
>       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>         Package () {
>           Package () {"compatible", "ethernet-phy-ieee802.3-c45"}
>        }
>     })
>   } // end of PHY4
>
>  What is see is that in acpi_data_get_property(),
> propvalue->type = 0x2(ACPI_TYPE_STRING) and type = 0x4(ACPI_TYPE_PACKAGE).
>
> Any help please?
>
> fwnode_property_match_string() works fine for DT.

Can you show the DT node which works and also input for the
)match_string() (i.o.w what exactly you are trying to match with)?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  2021-02-05 18:25     ` Andy Shevchenko
@ 2021-02-05 18:41       ` Andy Shevchenko
  2021-02-05 18:58         ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2021-02-05 18:41 UTC (permalink / raw)
  To: Calvin Johnson
  Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Florian Fainelli, Russell King - ARM Linux admin,
	Cristi Sovaiala, Florin Laurentiu Chiculita, Ioana Ciornei,
	Madalin Bucur, Heikki Krogerus, Marcin Wojtas,
	Pieter Jansen Van Vuuren, Jon, Saravana Kannan, Randy Dunlap,
	linux.cj, Diana Madalina Craciun, ACPI Devel Maling List,
	linux-arm Mailing List, Linux Kernel Mailing List, netdev,
	Laurentiu Tudor, David S. Miller, Frank Rowand, Heiner Kallweit,
	Jakub Kicinski, Rob Herring, devicetree

On Fri, Feb 5, 2021 at 8:25 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Feb 5, 2021 at 7:25 PM Calvin Johnson
> <calvin.johnson@oss.nxp.com> wrote:
> > On Fri, Jan 22, 2021 at 09:12:52PM +0530, Calvin Johnson wrote:
>
> ...
>
> > > +     rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
> > With ACPI, I'm facing some problem with fwnode_property_match_string(). It is
> > unable to detect the compatible string and returns -EPROTO.
> >
> > ACPI node for PHY4 is as below:
> >
> >  Device(PHY4) {
> >     Name (_ADR, 0x4)
> >     Name(_CRS, ResourceTemplate() {
> >     Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
> >     {
> >       AQR_PHY4_IT
> >     }
> >     }) // end of _CRS for PHY4
> >     Name (_DSD, Package () {
> >       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >         Package () {

> >           Package () {"compatible", "ethernet-phy-ieee802.3-c45"}

I guess converting this to
           Package () {"compatible", Package() {"ethernet-phy-ieee802.3-c45"}}
will solve it.

> >        }

> >     })
> >   } // end of PHY4
> >
> >  What is see is that in acpi_data_get_property(),
> > propvalue->type = 0x2(ACPI_TYPE_STRING) and type = 0x4(ACPI_TYPE_PACKAGE).
> >
> > Any help please?
> >
> > fwnode_property_match_string() works fine for DT.
>
> Can you show the DT node which works and also input for the
> )match_string() (i.o.w what exactly you are trying to match with)?
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  2021-02-05 18:41       ` Andy Shevchenko
@ 2021-02-05 18:58         ` Andy Shevchenko
  2021-02-06 17:14           ` Calvin Johnson
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2021-02-05 18:58 UTC (permalink / raw)
  To: Calvin Johnson
  Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Florian Fainelli, Russell King - ARM Linux admin,
	Cristi Sovaiala, Florin Laurentiu Chiculita, Ioana Ciornei,
	Madalin Bucur, Heikki Krogerus, Marcin Wojtas,
	Pieter Jansen Van Vuuren, Jon, Saravana Kannan, Randy Dunlap,
	linux.cj, Diana Madalina Craciun, ACPI Devel Maling List,
	linux-arm Mailing List, Linux Kernel Mailing List, netdev,
	Laurentiu Tudor, David S. Miller, Frank Rowand, Heiner Kallweit,
	Jakub Kicinski, Rob Herring, devicetree

On Fri, Feb 5, 2021 at 8:41 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Feb 5, 2021 at 8:25 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Feb 5, 2021 at 7:25 PM Calvin Johnson
> > <calvin.johnson@oss.nxp.com> wrote:
> > > On Fri, Jan 22, 2021 at 09:12:52PM +0530, Calvin Johnson wrote:
> >
> > ...
> >
> > > > +     rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
> > > With ACPI, I'm facing some problem with fwnode_property_match_string(). It is
> > > unable to detect the compatible string and returns -EPROTO.
> > >
> > > ACPI node for PHY4 is as below:
> > >
> > >  Device(PHY4) {
> > >     Name (_ADR, 0x4)
> > >     Name(_CRS, ResourceTemplate() {
> > >     Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
> > >     {
> > >       AQR_PHY4_IT
> > >     }
> > >     }) // end of _CRS for PHY4
> > >     Name (_DSD, Package () {
> > >       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > >         Package () {
>
> > >           Package () {"compatible", "ethernet-phy-ieee802.3-c45"}
>
> I guess converting this to
>            Package () {"compatible", Package() {"ethernet-phy-ieee802.3-c45"}}
> will solve it.

For the record, it doesn't mean there is no bug in the code. DT treats
a single string as an array, but ACPI doesn't.
And this is specific to _match_string() because it has two passes. And
the first one fails.
While reading a single string as an array of 1 element will work I believe.

> > >        }
>
> > >     })
> > >   } // end of PHY4
> > >
> > >  What is see is that in acpi_data_get_property(),
> > > propvalue->type = 0x2(ACPI_TYPE_STRING) and type = 0x4(ACPI_TYPE_PACKAGE).
> > >
> > > Any help please?
> > >
> > > fwnode_property_match_string() works fine for DT.
> >
> > Can you show the DT node which works and also input for the
> > )match_string() (i.o.w what exactly you are trying to match with)?


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
  2021-02-05 18:58         ` Andy Shevchenko
@ 2021-02-06 17:14           ` Calvin Johnson
  0 siblings, 0 replies; 7+ messages in thread
From: Calvin Johnson @ 2021-02-06 17:14 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Grant Likely, Rafael J . Wysocki, Jeremy Linton, Andrew Lunn,
	Florian Fainelli, Russell King - ARM Linux admin,
	Cristi Sovaiala, Florin Laurentiu Chiculita, Ioana Ciornei,
	Madalin Bucur, Heikki Krogerus, Marcin Wojtas,
	Pieter Jansen Van Vuuren, Jon, Saravana Kannan, Randy Dunlap,
	linux.cj, Diana Madalina Craciun, ACPI Devel Maling List,
	linux-arm Mailing List, Linux Kernel Mailing List, netdev,
	Laurentiu Tudor, David S. Miller, Frank Rowand, Heiner Kallweit,
	Jakub Kicinski, Rob Herring, devicetree

On Fri, Feb 05, 2021 at 08:58:06PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 5, 2021 at 8:41 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Feb 5, 2021 at 8:25 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Fri, Feb 5, 2021 at 7:25 PM Calvin Johnson
> > > <calvin.johnson@oss.nxp.com> wrote:
> > > > On Fri, Jan 22, 2021 at 09:12:52PM +0530, Calvin Johnson wrote:
> > >
> > > ...
> > >
> > > > > +     rc = fwnode_property_match_string(child, "compatible", "ethernet-phy-ieee802.3-c45");
> > > > With ACPI, I'm facing some problem with fwnode_property_match_string(). It is
> > > > unable to detect the compatible string and returns -EPROTO.
> > > >
> > > > ACPI node for PHY4 is as below:
> > > >
> > > >  Device(PHY4) {
> > > >     Name (_ADR, 0x4)
> > > >     Name(_CRS, ResourceTemplate() {
> > > >     Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
> > > >     {
> > > >       AQR_PHY4_IT
> > > >     }
> > > >     }) // end of _CRS for PHY4
> > > >     Name (_DSD, Package () {
> > > >       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > > >         Package () {
> >
> > > >           Package () {"compatible", "ethernet-phy-ieee802.3-c45"}
> >
> > I guess converting this to
> >            Package () {"compatible", Package() {"ethernet-phy-ieee802.3-c45"}}
> > will solve it.

Thanks a lot Andy! This helped. But is this the correct way to define compatible
string value, i.e as a sub package. 
> 
> For the record, it doesn't mean there is no bug in the code. DT treats
> a single string as an array, but ACPI doesn't.
> And this is specific to _match_string() because it has two passes. And
> the first one fails.
> While reading a single string as an array of 1 element will work I believe.
> 
> > > >        }
> >
> > > >     })
> > > >   } // end of PHY4
> > > >
> > > >  What is see is that in acpi_data_get_property(),
> > > > propvalue->type = 0x2(ACPI_TYPE_STRING) and type = 0x4(ACPI_TYPE_PACKAGE).
> > > >
> > > > Any help please?
> > > >
> > > > fwnode_property_match_string() works fine for DT.
> > >
> > > Can you show the DT node which works and also input for the
> > > )match_string() (i.o.w what exactly you are trying to match with)?
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-02-06 17:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 15:42 [net-next PATCH v4 00/15] ACPI support for dpaa2 driver Calvin Johnson
2021-01-22 15:42 ` [net-next PATCH v4 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
2021-02-05 17:25   ` Calvin Johnson
2021-02-05 18:25     ` Andy Shevchenko
2021-02-05 18:41       ` Andy Shevchenko
2021-02-05 18:58         ` Andy Shevchenko
2021-02-06 17:14           ` Calvin Johnson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).