[net-next: PATCH v2 0/5] Armada 7k/8k PP2 ACPI support

[net-next: PATCH v2 0/5] Armada 7k/8k PP2 ACPI support

[Marcin Wojtas]

Hi,

This a second version of a patchset, which introduces ACPI support
in mvpp2 driver. Comparing to the initial one, all patches
touching generic ACPI MDIO bus / PHY handling were removed
and after some modifications will be resend separately. They
may require a longer discussion in terms of phylink support
and ACPI specification extensions.

This way mvpp2 driver is able to operate using the link interrupt
capability (a.k.a. in-band management) on all ports, 1000BaseT RGMII
included. Driver operation was tested on top of the net-next branch
with both DT and ACPI on MacchiatoBin and Armada 7040 DB boards.

mvpp2 driver can work with the ACPI representation, as exposed
on a public branch:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/marvell-armada-wip
It was compiled together with the most recent Tianocore EDK2 revision.
Please refer to the firmware build instruction on MacchiatoBin board:
http://wiki.macchiatobin.net/tiki-index.php?page=Build+from+source+-+UEFI+EDK+II

ACPI representation of PP2 controllers (withouth PHY support) can
be viewed in the github:
* MacchiatoBin:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/marvell-armada-wip/Platforms/Marvell/Armada/AcpiTables/Armada80x0McBin/Dsdt.asl#L201

* Armada 7040 DB:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/marvell-armada-wip/Platforms/Marvell/Armada/AcpiTables/Armada70x0/Dsdt.asl#L131

I will appreciate any comments or remarks.

Best regards,
Marcin

Changelog:
v1 -> v2:
* Remove MDIO patches
* Use PP2 ports only with link interrupts
* Release second region resources in mvpp2 driver (code moved from
  mvmdio), as explained in details in 5/5 commit message.

Marcin Wojtas (5):
  device property: Introduce fwnode_get_mac_address()
  device property: Introduce fwnode_get_phy_mode()
  net: mvpp2: simplify maintaining enabled ports' list
  net: mvpp2: use device_*/fwnode_* APIs instead of of_*
  net: mvpp2: enable ACPI support in the driver

 drivers/base/property.c              |  52 +++--
 drivers/net/ethernet/marvell/mvpp2.c | 222 ++++++++++++--------
 include/linux/property.h             |   3 +
 3 files changed, 180 insertions(+), 97 deletions(-)

-- 
2.7.4

[net-next: PATCH v2 1/5] device property: Introduce fwnode_get_mac_address()

[Marcin Wojtas]

Until now there were two almost identical functions for
obtaining MAC address - of_get_mac_address() and, more generic,
device_get_mac_address(). However it is not uncommon,
that the network interface is represented as a child
of the actual controller, hence it is not associated
directly to any struct device, required by the latter
routine.

This commit allows for getting the MAC address for
children nodes in the ACPI world by introducing a new function -
fwnode_get_mac_address(). This commit also changes
device_get_mac_address() routine to be its wrapper, in order
to prevent unnecessary duplication.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/base/property.c  | 28 ++++++++++++++------
 include/linux/property.h |  2 ++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 851b1b6..f261d1a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1153,11 +1153,11 @@ int device_get_phy_mode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(device_get_phy_mode);
 
-static void *device_get_mac_addr(struct device *dev,
+static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode,
 				 const char *name, char *addr,
 				 int alen)
 {
-	int ret = device_property_read_u8_array(dev, name, addr, alen);
+	int ret = fwnode_property_read_u8_array(fwnode, name, addr, alen);
 
 	if (ret == 0 && alen == ETH_ALEN && is_valid_ether_addr(addr))
 		return addr;
@@ -1165,8 +1165,8 @@ static void *device_get_mac_addr(struct device *dev,
 }
 
 /**
- * device_get_mac_address - Get the MAC for a given device
- * @dev:	Pointer to the device
+ * fwnode_get_mac_address - Get the MAC from the firmware node
+ * @fwnode:	Pointer to the firmware node
  * @addr:	Address of buffer to store the MAC in
  * @alen:	Length of the buffer pointed to by addr, should be ETH_ALEN
  *
@@ -1187,19 +1187,31 @@ static void *device_get_mac_addr(struct device *dev,
  * In this case, the real MAC is in 'local-mac-address', and 'mac-address'
  * exists but is all zeros.
 */
-void *device_get_mac_address(struct device *dev, char *addr, int alen)
+void *fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr, int alen)
 {
 	char *res;
 
-	res = device_get_mac_addr(dev, "mac-address", addr, alen);
+	res = fwnode_get_mac_addr(fwnode, "mac-address", addr, alen);
 	if (res)
 		return res;
 
-	res = device_get_mac_addr(dev, "local-mac-address", addr, alen);
+	res = fwnode_get_mac_addr(fwnode, "local-mac-address", addr, alen);
 	if (res)
 		return res;
 
-	return device_get_mac_addr(dev, "address", addr, alen);
+	return fwnode_get_mac_addr(fwnode, "address", addr, alen);
+}
+EXPORT_SYMBOL(fwnode_get_mac_address);
+
+/**
+ * device_get_mac_address - Get the MAC for a given device
+ * @dev:	Pointer to the device
+ * @addr:	Address of buffer to store the MAC in
+ * @alen:	Length of the buffer pointed to by addr, should be ETH_ALEN
+ */
+void *device_get_mac_address(struct device *dev, char *addr, int alen)
+{
+	return fwnode_get_mac_address(dev_fwnode(dev), addr, alen);
 }
 EXPORT_SYMBOL(device_get_mac_address);
 
diff --git a/include/linux/property.h b/include/linux/property.h
index f6189a3..35620e0 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -279,6 +279,8 @@ int device_get_phy_mode(struct device *dev);
 
 void *device_get_mac_address(struct device *dev, char *addr, int alen);
 
+void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
+			     char *addr, int alen);
 struct fwnode_handle *fwnode_graph_get_next_endpoint(
 	const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
 struct fwnode_handle *
-- 
2.7.4

[net-next: PATCH v2 1/5] device property: Introduce fwnode_get_mac_address()

[Rafael J. Wysocki]

On Sunday, December 31, 2017 12:58:36 PM CET Marcin Wojtas wrote:
> Until now there were two almost identical functions for
> obtaining MAC address - of_get_mac_address() and, more generic,
> device_get_mac_address(). However it is not uncommon,
> that the network interface is represented as a child
> of the actual controller, hence it is not associated
> directly to any struct device, required by the latter
> routine.
> 
> This commit allows for getting the MAC address for
> children nodes in the ACPI world by introducing a new function -
> fwnode_get_mac_address(). This commit also changes
> device_get_mac_address() routine to be its wrapper, in order
> to prevent unnecessary duplication.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

The changes look reasonable to me, so

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/base/property.c  | 28 ++++++++++++++------
>  include/linux/property.h |  2 ++
>  2 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 851b1b6..f261d1a 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1153,11 +1153,11 @@ int device_get_phy_mode(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(device_get_phy_mode);
>  
> -static void *device_get_mac_addr(struct device *dev,
> +static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode,
>  				 const char *name, char *addr,
>  				 int alen)
>  {
> -	int ret = device_property_read_u8_array(dev, name, addr, alen);
> +	int ret = fwnode_property_read_u8_array(fwnode, name, addr, alen);
>  
>  	if (ret == 0 && alen == ETH_ALEN && is_valid_ether_addr(addr))
>  		return addr;
> @@ -1165,8 +1165,8 @@ static void *device_get_mac_addr(struct device *dev,
>  }
>  
>  /**
> - * device_get_mac_address - Get the MAC for a given device
> - * @dev:	Pointer to the device
> + * fwnode_get_mac_address - Get the MAC from the firmware node
> + * @fwnode:	Pointer to the firmware node
>   * @addr:	Address of buffer to store the MAC in
>   * @alen:	Length of the buffer pointed to by addr, should be ETH_ALEN
>   *
> @@ -1187,19 +1187,31 @@ static void *device_get_mac_addr(struct device *dev,
>   * In this case, the real MAC is in 'local-mac-address', and 'mac-address'
>   * exists but is all zeros.
>  */
> -void *device_get_mac_address(struct device *dev, char *addr, int alen)
> +void *fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr, int alen)
>  {
>  	char *res;
>  
> -	res = device_get_mac_addr(dev, "mac-address", addr, alen);
> +	res = fwnode_get_mac_addr(fwnode, "mac-address", addr, alen);
>  	if (res)
>  		return res;
>  
> -	res = device_get_mac_addr(dev, "local-mac-address", addr, alen);
> +	res = fwnode_get_mac_addr(fwnode, "local-mac-address", addr, alen);
>  	if (res)
>  		return res;
>  
> -	return device_get_mac_addr(dev, "address", addr, alen);
> +	return fwnode_get_mac_addr(fwnode, "address", addr, alen);
> +}
> +EXPORT_SYMBOL(fwnode_get_mac_address);
> +
> +/**
> + * device_get_mac_address - Get the MAC for a given device
> + * @dev:	Pointer to the device
> + * @addr:	Address of buffer to store the MAC in
> + * @alen:	Length of the buffer pointed to by addr, should be ETH_ALEN
> + */
> +void *device_get_mac_address(struct device *dev, char *addr, int alen)
> +{
> +	return fwnode_get_mac_address(dev_fwnode(dev), addr, alen);
>  }
>  EXPORT_SYMBOL(device_get_mac_address);
>  
> diff --git a/include/linux/property.h b/include/linux/property.h
> index f6189a3..35620e0 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -279,6 +279,8 @@ int device_get_phy_mode(struct device *dev);
>  
>  void *device_get_mac_address(struct device *dev, char *addr, int alen);
>  
> +void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
> +			     char *addr, int alen);
>  struct fwnode_handle *fwnode_graph_get_next_endpoint(
>  	const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
>  struct fwnode_handle *
> 

[net-next: PATCH v2 2/5] device property: Introduce fwnode_get_phy_mode()

[Marcin Wojtas]

Until now there were two almost identical functions for
obtaining network PHY mode - of_get_phy_mode() and,
more generic, device_get_phy_mode(). However it is not uncommon,
that the network interface is represented as a child
of the actual controller, hence it is not associated
directly to any struct device, required by the latter
routine.

This commit allows for getting the PHY mode for
children nodes in the ACPI world by introducing a new function -
fwnode_get_phy_mode(). This commit also changes
device_get_phy_mode() routine to be its wrapper, in order
to prevent unnecessary duplication.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/base/property.c  | 24 ++++++++++++++++----
 include/linux/property.h |  1 +
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index f261d1a..7c4a53d 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1126,21 +1126,21 @@ enum dev_dma_attr device_get_dma_attr(struct device *dev)
 EXPORT_SYMBOL_GPL(device_get_dma_attr);
 
 /**
- * device_get_phy_mode - Get phy mode for given device
- * @dev:	Pointer to the given device
+ * fwnode_get_phy_mode - Get phy mode for given firmware node
+ * @fwnode:	Pointer to the given node
  *
  * The function gets phy interface string from property 'phy-mode' or
  * 'phy-connection-type', and return its index in phy_modes table, or errno in
  * error case.
  */
-int device_get_phy_mode(struct device *dev)
+int fwnode_get_phy_mode(struct fwnode_handle *fwnode)
 {
 	const char *pm;
 	int err, i;
 
-	err = device_property_read_string(dev, "phy-mode", &pm);
+	err = fwnode_property_read_string(fwnode, "phy-mode", &pm);
 	if (err < 0)
-		err = device_property_read_string(dev,
+		err = fwnode_property_read_string(fwnode,
 						  "phy-connection-type", &pm);
 	if (err < 0)
 		return err;
@@ -1151,6 +1151,20 @@ int device_get_phy_mode(struct device *dev)
 
 	return -ENODEV;
 }
+EXPORT_SYMBOL_GPL(fwnode_get_phy_mode);
+
+/**
+ * device_get_phy_mode - Get phy mode for given device
+ * @dev:	Pointer to the given device
+ *
+ * The function gets phy interface string from property 'phy-mode' or
+ * 'phy-connection-type', and return its index in phy_modes table, or errno in
+ * error case.
+ */
+int device_get_phy_mode(struct device *dev)
+{
+	return fwnode_get_phy_mode(dev_fwnode(dev));
+}
 EXPORT_SYMBOL_GPL(device_get_phy_mode);
 
 static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index 35620e0..9b13332 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -279,6 +279,7 @@ int device_get_phy_mode(struct device *dev);
 
 void *device_get_mac_address(struct device *dev, char *addr, int alen);
 
+int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
 void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
 			     char *addr, int alen);
 struct fwnode_handle *fwnode_graph_get_next_endpoint(
-- 
2.7.4

[net-next: PATCH v2 2/5] device property: Introduce fwnode_get_phy_mode()

[Rafael J. Wysocki]

On Sunday, December 31, 2017 12:58:37 PM CET Marcin Wojtas wrote:
> Until now there were two almost identical functions for
> obtaining network PHY mode - of_get_phy_mode() and,
> more generic, device_get_phy_mode(). However it is not uncommon,
> that the network interface is represented as a child
> of the actual controller, hence it is not associated
> directly to any struct device, required by the latter
> routine.
> 
> This commit allows for getting the PHY mode for
> children nodes in the ACPI world by introducing a new function -
> fwnode_get_phy_mode(). This commit also changes
> device_get_phy_mode() routine to be its wrapper, in order
> to prevent unnecessary duplication.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/base/property.c  | 24 ++++++++++++++++----
>  include/linux/property.h |  1 +
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index f261d1a..7c4a53d 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1126,21 +1126,21 @@ enum dev_dma_attr device_get_dma_attr(struct device *dev)
>  EXPORT_SYMBOL_GPL(device_get_dma_attr);
>  
>  /**
> - * device_get_phy_mode - Get phy mode for given device
> - * @dev:	Pointer to the given device
> + * fwnode_get_phy_mode - Get phy mode for given firmware node
> + * @fwnode:	Pointer to the given node
>   *
>   * The function gets phy interface string from property 'phy-mode' or
>   * 'phy-connection-type', and return its index in phy_modes table, or errno in
>   * error case.
>   */
> -int device_get_phy_mode(struct device *dev)
> +int fwnode_get_phy_mode(struct fwnode_handle *fwnode)
>  {
>  	const char *pm;
>  	int err, i;
>  
> -	err = device_property_read_string(dev, "phy-mode", &pm);
> +	err = fwnode_property_read_string(fwnode, "phy-mode", &pm);
>  	if (err < 0)
> -		err = device_property_read_string(dev,
> +		err = fwnode_property_read_string(fwnode,
>  						  "phy-connection-type", &pm);
>  	if (err < 0)
>  		return err;
> @@ -1151,6 +1151,20 @@ int device_get_phy_mode(struct device *dev)
>  
>  	return -ENODEV;
>  }
> +EXPORT_SYMBOL_GPL(fwnode_get_phy_mode);
> +
> +/**
> + * device_get_phy_mode - Get phy mode for given device
> + * @dev:	Pointer to the given device
> + *
> + * The function gets phy interface string from property 'phy-mode' or
> + * 'phy-connection-type', and return its index in phy_modes table, or errno in
> + * error case.
> + */
> +int device_get_phy_mode(struct device *dev)
> +{
> +	return fwnode_get_phy_mode(dev_fwnode(dev));
> +}
>  EXPORT_SYMBOL_GPL(device_get_phy_mode);
>  
>  static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode,
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 35620e0..9b13332 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -279,6 +279,7 @@ int device_get_phy_mode(struct device *dev);
>  
>  void *device_get_mac_address(struct device *dev, char *addr, int alen);
>  
> +int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
>  void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
>  			     char *addr, int alen);
>  struct fwnode_handle *fwnode_graph_get_next_endpoint(
> 

[net-next: PATCH v2 3/5] net: mvpp2: simplify maintaining enabled ports' list

[Marcin Wojtas]

'port_count' field of the mvpp2 structure holds an overall amount
of available ports, based on DT nodes status. In order to be prepared
to support other HW description, obtain the value by incrementing it
upon each successful port initialization. This allowed for simplifying
port indexing in the controller's private array, whose size is now not
dynamically allocated, but fixed to MVPP2_MAX_PORTS.

This patch simplifies creating and filling list of enabled ports and
is a part of the preparation for adding ACPI support in the mvpp2 driver.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 32 +++++++-------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index a197607..7f42d90 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -865,7 +865,7 @@ struct mvpp2 {
 
 	/* List of pointers to port structures */
 	int port_count;
-	struct mvpp2_port **port_list;
+	struct mvpp2_port *port_list[MVPP2_MAX_PORTS];
 
 	/* Aggregated TXQs */
 	struct mvpp2_tx_queue *aggr_txqs;
@@ -7741,7 +7741,7 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 /* Ports initialization */
 static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct device_node *port_node,
-			    struct mvpp2 *priv, int index)
+			    struct mvpp2 *priv)
 {
 	struct device_node *phy_node;
 	struct phy *comphy;
@@ -7934,7 +7934,8 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	}
 	netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr);
 
-	priv->port_list[index] = port;
+	priv->port_list[priv->port_count++] = port;
+
 	return 0;
 
 err_free_port_pcpu:
@@ -8313,28 +8314,17 @@ static int mvpp2_probe(struct platform_device *pdev)
 		goto err_mg_clk;
 	}
 
-	priv->port_count = of_get_available_child_count(dn);
-	if (priv->port_count == 0) {
-		dev_err(&pdev->dev, "no ports enabled\n");
-		err = -ENODEV;
-		goto err_mg_clk;
-	}
-
-	priv->port_list = devm_kcalloc(&pdev->dev, priv->port_count,
-				       sizeof(*priv->port_list),
-				       GFP_KERNEL);
-	if (!priv->port_list) {
-		err = -ENOMEM;
-		goto err_mg_clk;
-	}
-
 	/* Initialize ports */
-	i = 0;
 	for_each_available_child_of_node(dn, port_node) {
-		err = mvpp2_port_probe(pdev, port_node, priv, i);
+		err = mvpp2_port_probe(pdev, port_node, priv);
 		if (err < 0)
 			goto err_port_probe;
-		i++;
+	}
+
+	if (priv->port_count == 0) {
+		dev_err(&pdev->dev, "no ports enabled\n");
+		err = -ENODEV;
+		goto err_mg_clk;
 	}
 
 	/* Statistics must be gathered regularly because some of them (like
-- 
2.7.4

[net-next: PATCH v2 4/5] net: mvpp2: use device_*/fwnode_* APIs instead of of_*

[Marcin Wojtas]

OF functions can be used only for the driver using DT.
As a preparation for introducing ACPI support in mvpp2
driver, use struct fwnode_handle in order to obtain
properties from the hardware description.

Because there is no equivalent for for_each_available_child_of_node(),
use device_for_each_child_node() and check the port availability
inside the mvpp2_port_probe() routine.

This patch replaces of_* function with device_*/fwnode_*
where possible in the mvpp2.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 47 +++++++++++---------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 7f42d90..537474f 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -932,6 +932,9 @@ struct mvpp2_port {
 
 	struct mvpp2 *priv;
 
+	/* Firmware node associated to the port */
+	struct fwnode_handle *fwnode;
+
 	/* Per-port registers' base address */
 	void __iomem *base;
 	void __iomem *stats_base;
@@ -7711,17 +7714,16 @@ static bool mvpp2_port_has_tx_irqs(struct mvpp2 *priv,
 }
 
 static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
-				     struct device_node *port_node,
+				     struct fwnode_handle *fwnode,
 				     char **mac_from)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 	char hw_mac_addr[ETH_ALEN] = {0};
-	const char *dt_mac_addr;
+	char fw_mac_addr[ETH_ALEN];
 
-	dt_mac_addr = of_get_mac_address(port_node);
-	if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
-		*mac_from = "device tree";
-		ether_addr_copy(dev->dev_addr, dt_mac_addr);
+	if (fwnode_get_mac_address(fwnode, fw_mac_addr, ETH_ALEN)) {
+		*mac_from = "firmware node";
+		ether_addr_copy(dev->dev_addr, fw_mac_addr);
 		return;
 	}
 
@@ -7740,13 +7742,14 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 
 /* Ports initialization */
 static int mvpp2_port_probe(struct platform_device *pdev,
-			    struct device_node *port_node,
+			    struct fwnode_handle *port_fwnode,
 			    struct mvpp2 *priv)
 {
 	struct device_node *phy_node;
 	struct phy *comphy;
 	struct mvpp2_port *port;
 	struct mvpp2_port_pcpu *port_pcpu;
+	struct device_node *port_node = to_of_node(port_fwnode);
 	struct net_device *dev;
 	struct resource *res;
 	char *mac_from = "";
@@ -7757,6 +7760,10 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	int phy_mode;
 	int err, i, cpu;
 
+	/* Silently exit, if the port node turns out to be disabled. */
+	if (!fwnode_device_is_available(port_fwnode))
+		return 0;
+
 	has_tx_irqs = mvpp2_port_has_tx_irqs(priv, port_node);
 
 	if (!has_tx_irqs)
@@ -7773,7 +7780,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		return -ENOMEM;
 
 	phy_node = of_parse_phandle(port_node, "phy", 0);
-	phy_mode = of_get_phy_mode(port_node);
+	phy_mode = fwnode_get_phy_mode(port_fwnode);
 	if (phy_mode < 0) {
 		dev_err(&pdev->dev, "incorrect phy mode\n");
 		err = phy_mode;
@@ -7789,7 +7796,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		comphy = NULL;
 	}
 
-	if (of_property_read_u32(port_node, "port-id", &id)) {
+	if (fwnode_property_read_u32(port_fwnode, "port-id", &id)) {
 		err = -EINVAL;
 		dev_err(&pdev->dev, "missing port-id value\n");
 		goto err_free_netdev;
@@ -7820,7 +7827,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		/* the link irq is optional */
 		port->link_irq = 0;
 
-	if (of_property_read_bool(port_node, "marvell,loopback"))
+	if (fwnode_property_read_bool(port_fwnode, "marvell,loopback"))
 		port->flags |= MVPP2_F_LOOPBACK;
 
 	port->id = id;
@@ -7845,8 +7852,8 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 				   MVPP21_MIB_COUNTERS_OFFSET +
 				   port->gop_id * MVPP21_MIB_COUNTERS_PORT_SZ;
 	} else {
-		if (of_property_read_u32(port_node, "gop-port-id",
-					 &port->gop_id)) {
+		if (fwnode_property_read_u32(port_fwnode, "gop-port-id",
+					     &port->gop_id)) {
 			err = -EINVAL;
 			dev_err(&pdev->dev, "missing gop-port-id value\n");
 			goto err_deinit_qvecs;
@@ -7876,7 +7883,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	mutex_init(&port->gather_stats_lock);
 	INIT_DELAYED_WORK(&port->stats_work, mvpp2_gather_hw_statistics);
 
-	mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
+	mvpp2_port_copy_mac_addr(dev, priv, port_fwnode, &mac_from);
 
 	port->tx_ring_size = MVPP2_MAX_TXD_DFLT;
 	port->rx_ring_size = MVPP2_MAX_RXD_DFLT;
@@ -8194,8 +8201,7 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 
 static int mvpp2_probe(struct platform_device *pdev)
 {
-	struct device_node *dn = pdev->dev.of_node;
-	struct device_node *port_node;
+	struct fwnode_handle *port_fwnode;
 	struct mvpp2 *priv;
 	struct resource *res;
 	void __iomem *base;
@@ -8315,8 +8321,8 @@ static int mvpp2_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize ports */
-	for_each_available_child_of_node(dn, port_node) {
-		err = mvpp2_port_probe(pdev, port_node, priv);
+	device_for_each_child_node(&pdev->dev, port_fwnode) {
+		err = mvpp2_port_probe(pdev, port_fwnode, priv);
 		if (err < 0)
 			goto err_port_probe;
 	}
@@ -8347,7 +8353,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 
 err_port_probe:
 	i = 0;
-	for_each_available_child_of_node(dn, port_node) {
+	device_for_each_child_node(&pdev->dev, port_fwnode) {
 		if (priv->port_list[i])
 			mvpp2_port_remove(priv->port_list[i]);
 		i++;
@@ -8366,14 +8372,13 @@ static int mvpp2_probe(struct platform_device *pdev)
 static int mvpp2_remove(struct platform_device *pdev)
 {
 	struct mvpp2 *priv = platform_get_drvdata(pdev);
-	struct device_node *dn = pdev->dev.of_node;
-	struct device_node *port_node;
+	struct fwnode_handle *port_fwnode;
 	int i = 0;
 
 	flush_workqueue(priv->stats_queue);
 	destroy_workqueue(priv->stats_queue);
 
-	for_each_available_child_of_node(dn, port_node) {
+	device_for_each_child_node(&pdev->dev, port_fwnode) {
 		if (priv->port_list[i]) {
 			mutex_destroy(&priv->port_list[i]->gather_stats_lock);
 			mvpp2_port_remove(priv->port_list[i]);
-- 
2.7.4

[net-next: PATCH v2 4/5] net: mvpp2: use device_*/fwnode_* APIs instead of of_*

[Andrew Lunn]

On Sun, Dec 31, 2017 at 12:58:39PM +0100, Marcin Wojtas wrote:
Hi Marcin

> Because there is no equivalent for for_each_available_child_of_node(),
> use device_for_each_child_node() and check the port availability
> inside the mvpp2_port_probe() routine.

Could device_each_available_child_node() be added? I guess this will
not be the last driver converted in this way, and having that macro
will help with future ports.

     Andrew

[net-next: PATCH v2 4/5] net: mvpp2: use device_*/fwnode_* APIs instead of of_*

[Marcin Wojtas]

Hi Andrew,

2017-12-31 20:18 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
> On Sun, Dec 31, 2017 at 12:58:39PM +0100, Marcin Wojtas wrote:
> Hi Marcin
>
>> Because there is no equivalent for for_each_available_child_of_node(),
>> use device_for_each_child_node() and check the port availability
>> inside the mvpp2_port_probe() routine.
>
> Could device_each_available_child_node() be added? I guess this will
> not be the last driver converted in this way, and having that macro
> will help with future ports.
>

Sure, I can add it in the next round.

Thanks,
Marcin

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Marcin Wojtas]

This patch introduces an alternative way of obtaining resources - via
ACPI tables provided by firmware. Enabling coexistence with the DT
support, in addition to the OF_*->device_*/fwnode_* API replacement,
required following steps to be taken:

* Add mvpp2_acpi_match table
* Omit clock configuration and obtain tclk from the property - in ACPI
  world, the firmware is responsible for clock maintenance.
* Disable comphy and syscon handling as they are not available for ACPI.
* Modify way of obtaining interrupts - with ACPI they are resources
  bound to struct platform_device and it's not possible to obtain
  them directly from the child node. Hence a formula is used, depending
  on the port_id and number of possible CPUs.
* Until proper MDIO bus and PHY handling with ACPI is established in the
  kernel, use only link interrupts feature in the driver.
* When booting with ACPI MVPP2_QDIST_MULTI_MODE is picked by
  default, as there is no need to keep any kind of the backward
  compatibility.

Moreover, a memory region used by mvmdio driver is usually placed in
the middle of the address space of the PP2 network controller.
The MDIO base address is obtained without requesting memory region
(by devm_ioremap() call) in mvmdio.c, later overlapping resources are
requested by the network driver, which is responsible for avoiding
a concurrent access.

In case the MDIO memory region is declared in the ACPI, it can
already appear as 'in-use' in the OS. Because it is overlapped by second
region of the network controller, make sure it is released, before
requesting it again. The care is taken by mvpp2 driver to avoid
concurrent access to this memory region.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 147 ++++++++++++++------
 1 file changed, 103 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 537474f..8b1c9a3 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -10,6 +10,7 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/acpi.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
@@ -7469,7 +7470,8 @@ static int mvpp2_simple_queue_vectors_init(struct mvpp2_port *port,
 	return 0;
 }
 
-static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
+static int mvpp2_multi_queue_vectors_init(struct platform_device *pdev,
+					  struct mvpp2_port *port,
 					  struct device_node *port_node)
 {
 	struct mvpp2_queue_vector *v;
@@ -7502,7 +7504,11 @@ static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
 			strncpy(irqname, "rx-shared", sizeof(irqname));
 		}
 
-		v->irq = of_irq_get_byname(port_node, irqname);
+		if (port_node)
+			v->irq = of_irq_get_byname(port_node, irqname);
+		else
+			v->irq = platform_get_irq(pdev, port->id *
+						  (port->nqvecs + 2) + i);
 		if (v->irq <= 0) {
 			ret = -EINVAL;
 			goto err;
@@ -7520,11 +7526,12 @@ static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
 	return ret;
 }
 
-static int mvpp2_queue_vectors_init(struct mvpp2_port *port,
+static int mvpp2_queue_vectors_init(struct platform_device *pdev,
+				    struct mvpp2_port *port,
 				    struct device_node *port_node)
 {
 	if (port->has_tx_irqs)
-		return mvpp2_multi_queue_vectors_init(port, port_node);
+		return mvpp2_multi_queue_vectors_init(pdev, port, port_node);
 	else
 		return mvpp2_simple_queue_vectors_init(port, port_node);
 }
@@ -7746,7 +7753,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct mvpp2 *priv)
 {
 	struct device_node *phy_node;
-	struct phy *comphy;
+	struct phy *comphy = NULL;
 	struct mvpp2_port *port;
 	struct mvpp2_port_pcpu *port_pcpu;
 	struct device_node *port_node = to_of_node(port_fwnode);
@@ -7764,7 +7771,12 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	if (!fwnode_device_is_available(port_fwnode))
 		return 0;
 
-	has_tx_irqs = mvpp2_port_has_tx_irqs(priv, port_node);
+	if (port_node) {
+		has_tx_irqs = mvpp2_port_has_tx_irqs(priv, port_node);
+	} else {
+		has_tx_irqs = true;
+		queue_mode = MVPP2_QDIST_MULTI_MODE;
+	}
 
 	if (!has_tx_irqs)
 		queue_mode = MVPP2_QDIST_SINGLE_MODE;
@@ -7779,7 +7791,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	if (!dev)
 		return -ENOMEM;
 
-	phy_node = of_parse_phandle(port_node, "phy", 0);
+	if (port_node)
+		phy_node = of_parse_phandle(port_node, "phy", 0);
+	else
+		phy_node = NULL;
+
 	phy_mode = fwnode_get_phy_mode(port_fwnode);
 	if (phy_mode < 0) {
 		dev_err(&pdev->dev, "incorrect phy mode\n");
@@ -7787,13 +7803,15 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		goto err_free_netdev;
 	}
 
-	comphy = devm_of_phy_get(&pdev->dev, port_node, NULL);
-	if (IS_ERR(comphy)) {
-		if (PTR_ERR(comphy) == -EPROBE_DEFER) {
-			err = -EPROBE_DEFER;
-			goto err_free_netdev;
+	if (port_node) {
+		comphy = devm_of_phy_get(&pdev->dev, port_node, NULL);
+		if (IS_ERR(comphy)) {
+			if (PTR_ERR(comphy) == -EPROBE_DEFER) {
+				err = -EPROBE_DEFER;
+				goto err_free_netdev;
+			}
+			comphy = NULL;
 		}
-		comphy = NULL;
 	}
 
 	if (fwnode_property_read_u32(port_fwnode, "port-id", &id)) {
@@ -7813,12 +7831,18 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	port->nrxqs = nrxqs;
 	port->priv = priv;
 	port->has_tx_irqs = has_tx_irqs;
+	port->id = id;
 
-	err = mvpp2_queue_vectors_init(port, port_node);
+	err = mvpp2_queue_vectors_init(pdev, port, port_node);
 	if (err)
 		goto err_free_netdev;
 
-	port->link_irq = of_irq_get_byname(port_node, "link");
+	if (port_node)
+		port->link_irq = of_irq_get_byname(port_node, "link");
+	else
+		port->link_irq = platform_get_irq(pdev, port->id *
+							(port->nqvecs + 2) +
+							port->nqvecs + 1);
 	if (port->link_irq == -EPROBE_DEFER) {
 		err = -EPROBE_DEFER;
 		goto err_deinit_qvecs;
@@ -7830,7 +7854,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	if (fwnode_property_read_bool(port_fwnode, "marvell,loopback"))
 		port->flags |= MVPP2_F_LOOPBACK;
 
-	port->id = id;
 	if (priv->hw_version == MVPP21)
 		port->first_rxq = port->id * port->nrxqs;
 	else
@@ -8201,6 +8224,7 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 
 static int mvpp2_probe(struct platform_device *pdev)
 {
+	const struct acpi_device_id *acpi_id;
 	struct fwnode_handle *port_fwnode;
 	struct mvpp2 *priv;
 	struct resource *res;
@@ -8212,8 +8236,14 @@ static int mvpp2_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	priv->hw_version =
-		(unsigned long)of_device_get_match_data(&pdev->dev);
+	if (has_acpi_companion(&pdev->dev)) {
+		acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table,
+					    &pdev->dev);
+		priv->hw_version = (unsigned long)acpi_id->driver_data;
+	} else {
+		priv->hw_version =
+			(unsigned long)of_device_get_match_data(&pdev->dev);
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(&pdev->dev, res);
@@ -8227,10 +8257,23 @@ static int mvpp2_probe(struct platform_device *pdev)
 			return PTR_ERR(priv->lms_base);
 	} else {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		if (has_acpi_companion(&pdev->dev)) {
+			/* In case the MDIO memory region is declared in
+			 * the ACPI, it can already appear as 'in-use'
+			 * in the OS. Because it is overlapped by second
+			 * region of the network controller, make
+			 * sure it is released, before requesting it again.
+			 * The care is taken by mvpp2 driver to avoid
+			 * concurrent access to this memory region.
+			 */
+			release_resource(res);
+		}
 		priv->iface_base = devm_ioremap_resource(&pdev->dev, res);
 		if (IS_ERR(priv->iface_base))
 			return PTR_ERR(priv->iface_base);
+	}
 
+	if (priv->hw_version == MVPP22 && dev_of_node(&pdev->dev)) {
 		priv->sysctrl_base =
 			syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
 							"marvell,system-controller");
@@ -8256,32 +8299,34 @@ static int mvpp2_probe(struct platform_device *pdev)
 	else
 		priv->max_port_rxqs = 32;
 
-	priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
-	if (IS_ERR(priv->pp_clk))
-		return PTR_ERR(priv->pp_clk);
-	err = clk_prepare_enable(priv->pp_clk);
-	if (err < 0)
-		return err;
-
-	priv->gop_clk = devm_clk_get(&pdev->dev, "gop_clk");
-	if (IS_ERR(priv->gop_clk)) {
-		err = PTR_ERR(priv->gop_clk);
-		goto err_pp_clk;
-	}
-	err = clk_prepare_enable(priv->gop_clk);
-	if (err < 0)
-		goto err_pp_clk;
+	if (dev_of_node(&pdev->dev)) {
+		priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
+		if (IS_ERR(priv->pp_clk))
+			return PTR_ERR(priv->pp_clk);
+		err = clk_prepare_enable(priv->pp_clk);
+		if (err < 0)
+			return err;
 
-	if (priv->hw_version == MVPP22) {
-		priv->mg_clk = devm_clk_get(&pdev->dev, "mg_clk");
-		if (IS_ERR(priv->mg_clk)) {
-			err = PTR_ERR(priv->mg_clk);
-			goto err_gop_clk;
+		priv->gop_clk = devm_clk_get(&pdev->dev, "gop_clk");
+		if (IS_ERR(priv->gop_clk)) {
+			err = PTR_ERR(priv->gop_clk);
+			goto err_pp_clk;
 		}
-
-		err = clk_prepare_enable(priv->mg_clk);
+		err = clk_prepare_enable(priv->gop_clk);
 		if (err < 0)
-			goto err_gop_clk;
+			goto err_pp_clk;
+
+		if (priv->hw_version == MVPP22) {
+			priv->mg_clk = devm_clk_get(&pdev->dev, "mg_clk");
+			if (IS_ERR(priv->mg_clk)) {
+				err = PTR_ERR(priv->mg_clk);
+				goto err_gop_clk;
+			}
+
+			err = clk_prepare_enable(priv->mg_clk);
+			if (err < 0)
+				goto err_gop_clk;
+		}
 
 		priv->axi_clk = devm_clk_get(&pdev->dev, "axi_clk");
 		if (IS_ERR(priv->axi_clk)) {
@@ -8294,10 +8339,14 @@ static int mvpp2_probe(struct platform_device *pdev)
 			if (err < 0)
 				goto err_gop_clk;
 		}
-	}
 
-	/* Get system's tclk rate */
-	priv->tclk = clk_get_rate(priv->pp_clk);
+		/* Get system's tclk rate */
+		priv->tclk = clk_get_rate(priv->pp_clk);
+	} else if (device_property_read_u32(&pdev->dev, "clock-frequency",
+					    &priv->tclk)) {
+		dev_err(&pdev->dev, "missing clock-frequency value\n");
+		return -EINVAL;
+	}
 
 	if (priv->hw_version == MVPP22) {
 		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(40));
@@ -8401,6 +8450,9 @@ static int mvpp2_remove(struct platform_device *pdev)
 				  aggr_txq->descs_dma);
 	}
 
+	if (is_acpi_node(port_fwnode))
+		return 0;
+
 	clk_disable_unprepare(priv->axi_clk);
 	clk_disable_unprepare(priv->mg_clk);
 	clk_disable_unprepare(priv->pp_clk);
@@ -8422,12 +8474,19 @@ static const struct of_device_id mvpp2_match[] = {
 };
 MODULE_DEVICE_TABLE(of, mvpp2_match);
 
+static const struct acpi_device_id mvpp2_acpi_match[] = {
+	{ "MRVL0110", MVPP22 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, mvpp2_acpi_match);
+
 static struct platform_driver mvpp2_driver = {
 	.probe = mvpp2_probe,
 	.remove = mvpp2_remove,
 	.driver = {
 		.name = MVPP2_DRIVER_NAME,
 		.of_match_table = mvpp2_match,
+		.acpi_match_table = ACPI_PTR(mvpp2_acpi_match),
 	},
 };
 
-- 
2.7.4

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Andrew Lunn]

> * Modify way of obtaining interrupts - with ACPI they are resources
>   bound to struct platform_device and it's not possible to obtain
>   them directly from the child node. Hence a formula is used, depending
>   on the port_id and number of possible CPUs.

Hi Marcin

I know nothing about ACPI. Is this limitation with respect to
interrupts fundamental to ACPI, or just that nobody has implemented
flexible interrupt support yet?

> * Until proper MDIO bus and PHY handling with ACPI is established in the
>   kernel, use only link interrupts feature in the driver.

I think interrupts might be interesting with PHY devices, since they
are child nodes of the MDIO device....

    Andrew

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Marcin Wojtas]

HI Andrew,

2017-12-31 20:23 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
>> * Modify way of obtaining interrupts - with ACPI they are resources
>>   bound to struct platform_device and it's not possible to obtain
>>   them directly from the child node. Hence a formula is used, depending
>>   on the port_id and number of possible CPUs.
>
> Hi Marcin
>
> I know nothing about ACPI. Is this limitation with respect to
> interrupts fundamental to ACPI, or just that nobody has implemented
> flexible interrupt support yet?

I think it's a limitation, however it would be great, if some real
ACPI expert was able to give an opinion here. I'd really prefer to
declare IRQ's in the child nodes, but it seems not possible.

>
>> * Until proper MDIO bus and PHY handling with ACPI is established in the
>>   kernel, use only link interrupts feature in the driver.
>
> I think interrupts might be interesting with PHY devices, since they
> are child nodes of the MDIO device....
>

Apart from the phylink's SFP support that may require in-band
management, it's an alternative to the normal PHY handling. Once MDIO
bus + PHYs are supported for ACPI, phylib support will be used instead
of the IRQs, so there should be no problem here.

Best regards,
Marcin

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Andrew Lunn]

> Apart from the phylink's SFP support that may require in-band
> management, it's an alternative to the normal PHY handling. Once MDIO
> bus + PHYs are supported for ACPI, phylib support will be used instead
> of the IRQs, so there should be no problem here.

Hi Marcin

However, phylib and phylink can use IRQs. The PHY can interrupt when
there is a change of state. This can be seen in the DT binding
documentation example:

ethernet-phy at 0 {
        compatible = "ethernet-phy-id0141.0e90", "ethernet-phy-ieee802.3-c22";
        interrupt-parent = <&PIC>;
        interrupts = <35 IRQ_TYPE_EDGE_RISING>;
        reg = <0>;

Whatever ACPI support you propose needs to include interrupts.

May i suggest you take a look at
arch/arm/boot/dts/vf610-zii-dev-rev-c.dts and ensure your ACPI work
can support this. I know you tend to concentrate of Marvell parts.
Although it is a Freescale SoC, the Ethernet parts are all Marvell.

The SoC exports an MDIO bus. We then have an MDIO multiplexer, which
exports 8 MDIO busses. Of these only 2 are used in this design. Each
bus has an Ethernet switch. Each switch has an MDIO bus, which the
embedded PHYs are on. The Ethernet switch is also an interrupt
controller for the PHYs interrupts. So the PHYs have interrupt
properties pointing back to the switch.

	   Andrew

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Marcin Wojtas]

Hi Andrew,

2018-01-02 14:33 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
>> Apart from the phylink's SFP support that may require in-band
>> management, it's an alternative to the normal PHY handling. Once MDIO
>> bus + PHYs are supported for ACPI, phylib support will be used instead
>> of the IRQs, so there should be no problem here.
>
> Hi Marcin
>
> However, phylib and phylink can use IRQs. The PHY can interrupt when
> there is a change of state. This can be seen in the DT binding
> documentation example:
>
> ethernet-phy at 0 {
>         compatible = "ethernet-phy-id0141.0e90", "ethernet-phy-ieee802.3-c22";
>         interrupt-parent = <&PIC>;
>         interrupts = <35 IRQ_TYPE_EDGE_RISING>;
>         reg = <0>;
>
> Whatever ACPI support you propose needs to include interrupts.
>
> May i suggest you take a look at
> arch/arm/boot/dts/vf610-zii-dev-rev-c.dts and ensure your ACPI work
> can support this. I know you tend to concentrate of Marvell parts.
> Although it is a Freescale SoC, the Ethernet parts are all Marvell.
>
> The SoC exports an MDIO bus. We then have an MDIO multiplexer, which
> exports 8 MDIO busses. Of these only 2 are used in this design. Each
> bus has an Ethernet switch. Each switch has an MDIO bus, which the
> embedded PHYs are on. The Ethernet switch is also an interrupt
> controller for the PHYs interrupts. So the PHYs have interrupt
> properties pointing back to the switch.
>

I thought you were pointing possible problems in mvpp2 with PHY/link
interrupts, sorry. Now I get it :)

Indeed in of_mdio_bus_register_phy, there is of_irq_get. This is more
a discussion for a MDIO bus / ACPI patchset, but we either find a way
to use IRQs with ACPI obtained from child nodes or for this world the
functionality will be limited (at least for the beginning).

Best regards,
Marcin

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Andrew Lunn]

> Indeed in of_mdio_bus_register_phy, there is of_irq_get. This is more
> a discussion for a MDIO bus / ACPI patchset, but we either find a way
> to use IRQs with ACPI obtained from child nodes or for this world the
> functionality will be limited (at least for the beginning).

Hi Marcin

What i want to avoid is adding something which partially works, and
then have to throw it all away and start again in order to add full
support.

If ACPI really limits interrupts to devices, maybe we need a totally
different representation of MDIO and PHYs in ACPI to what it used in
device tree? The same may be true for the Ethernet ports of the mvpp2?
They might have to be represented as real devices, not children of a
device? Maybe trying to map DT to ACPI on a one-to-one basis is the
wrong approach?

	Andrew

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Marcin Wojtas]

2018-01-02 15:08 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
>> Indeed in of_mdio_bus_register_phy, there is of_irq_get. This is more
>> a discussion for a MDIO bus / ACPI patchset, but we either find a way
>> to use IRQs with ACPI obtained from child nodes or for this world the
>> functionality will be limited (at least for the beginning).
>
> Hi Marcin
>
> What i want to avoid is adding something which partially works, and
> then have to throw it all away and start again in order to add full
> support.
>
> If ACPI really limits interrupts to devices, maybe we need a totally
> different representation of MDIO and PHYs in ACPI to what it used in
> device tree? The same may be true for the Ethernet ports of the mvpp2?
> They might have to be represented as real devices, not children of a
> device? Maybe trying to map DT to ACPI on a one-to-one basis is the
> wrong approach?
>

In terms of PP2 controller, I'd prefer to keep as much as possible to
describing how real hardware looks like, i.e. single common controller
with multiple ports as its children. Those considerations are
reflected in the DT description shape and how the driver enumerates,
which was part of the design of the initial support. Bending the
driver (huge amount of shared initialization and resources) to
multiple instances just for the sake of possible avoidance of IRQ
description in ACPI is IMO a huge and unnecessary overkill.

Anyway, I'll do a more research on the resources / ACPI representation
and will get back with some conclusions. I hope that someone from this
thread recipents will be able to give some advice too :)

Best regards,
Marcin

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Florian Fainelli]

On January 2, 2018 7:05:35 AM PST, Marcin Wojtas <mw@semihalf.com> wrote:
>2018-01-02 15:08 GMT+01:00 Andrew Lunn <andrew@lunn.ch>:
>>> Indeed in of_mdio_bus_register_phy, there is of_irq_get. This is
>more
>>> a discussion for a MDIO bus / ACPI patchset, but we either find a
>way
>>> to use IRQs with ACPI obtained from child nodes or for this world
>the
>>> functionality will be limited (at least for the beginning).
>>
>> Hi Marcin
>>
>> What i want to avoid is adding something which partially works, and
>> then have to throw it all away and start again in order to add full
>> support.
>>
>> If ACPI really limits interrupts to devices, maybe we need a totally
>> different representation of MDIO and PHYs in ACPI to what it used in
>> device tree? The same may be true for the Ethernet ports of the
>mvpp2?
>> They might have to be represented as real devices, not children of a
>> device? Maybe trying to map DT to ACPI on a one-to-one basis is the
>> wrong approach?
>>
>
>In terms of PP2 controller, I'd prefer to keep as much as possible to
>describing how real hardware looks like, i.e. single common controller
>with multiple ports as its children. Those considerations are
>reflected in the DT description shape and how the driver enumerates,
>which was part of the design of the initial support. Bending the
>driver (huge amount of shared initialization and resources) to
>multiple instances just for the sake of possible avoidance of IRQ
>description in ACPI is IMO a huge and unnecessary overkill.

True, although keep in mind that Device Tree, as implemented in Linux allows for a lot of flexibility in how parent/child nodes are represented and backed or not by a corresponding struct device. I suspect ACPI is much less permissive than that and we might want to have a struct device for the whole mvpp2 controller as well as for the child ports within that controller (something you could today with device tree too, just it would be more overhead). This does not necessarily have to influence the representation within the description language but we should not be biased by how the current implementation using Device Tree has shaped both representation and implementation.

-- 
Florian

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[graeme.gregory at linaro.org]

On Sun, Dec 31, 2017 at 08:23:54PM +0100, Andrew Lunn wrote:
> > * Modify way of obtaining interrupts - with ACPI they are resources
> >   bound to struct platform_device and it's not possible to obtain
> >   them directly from the child node. Hence a formula is used, depending
> >   on the port_id and number of possible CPUs.
> 
> Hi Marcin
> 
> I know nothing about ACPI. Is this limitation with respect to
> interrupts fundamental to ACPI, or just that nobody has implemented
> flexible interrupt support yet?
> 
The infrastructure is there to traverse trees of children, but I don't
think there any helper functions.

Graeme

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180103/bf301cec/attachment-0001.sig>

[net-next: PATCH v2 5/5] net: mvpp2: enable ACPI support in the driver

[Marcin Wojtas]

Graeme,

2018-01-03 12:16 GMT+01:00  <graeme.gregory@linaro.org>:
> On Sun, Dec 31, 2017 at 08:23:54PM +0100, Andrew Lunn wrote:
>> > * Modify way of obtaining interrupts - with ACPI they are resources
>> >   bound to struct platform_device and it's not possible to obtain
>> >   them directly from the child node. Hence a formula is used, depending
>> >   on the port_id and number of possible CPUs.
>>
>> Hi Marcin
>>
>> I know nothing about ACPI. Is this limitation with respect to
>> interrupts fundamental to ACPI, or just that nobody has implemented
>> flexible interrupt support yet?
>>
> The infrastructure is there to traverse trees of children, but I don't
> think there any helper functions.
>

Thanks, so if I implement such, do you expect any formal issues that
prevent its acceptance?

Best regards,
Marcin