Hi,
I quickly resend the series, thanks to Antoine Tenart's remark,
who spotted !CONFIG_ACPI compilation issue after introducing
the new fwnode_irq_get() routine. Please see the details in the changelog
below and the 3/7 commit log.
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/71ae395da1661374b0f07d1602afb1eee56e9794/Platforms/Marvell/Armada/AcpiTables/Armada80x0McBin/Dsdt.asl#L201
* Armada 7040 DB:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/71ae395da1661374b0f07d1602afb1eee56e9794/Platforms/Marvell/Armada/AcpiTables/Armada70x0/Dsdt.asl#L131
I will appreciate any comments or remarks.
Best regards,
Marcin
Changelog:
v3 -> v4:
* 3/7
- add new macro (ACPI_HANDLE_FWNODE) and fix
compilation with !CONFIG_ACPI
- extend commit log and mention usability of fwnode_irq_get
for the child nodes as well
v2 -> v3:
* 1/7, 2/7
- Add Rafael's Acked-by's
* 3/7, 4/7
- New patches
* 6/7, 7/7
- Update driver with new helper routines usage
- Improve commit log.
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 (7):
device property: Introduce fwnode_get_mac_address()
device property: Introduce fwnode_get_phy_mode()
device property: Introduce fwnode_irq_get()
device property: Allow iterating over available child fwnodes
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 | 104 ++++++++--
drivers/net/ethernet/marvell/mvpp2.c | 206 ++++++++++++--------
include/linux/acpi.h | 3 +
include/linux/property.h | 11 ++
4 files changed, 232 insertions(+), 92 deletions(-)
--
2.7.4
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 - use newly introduced
fwnode_irq_get() routine
* Until proper MDIO bus and PHY handling with ACPI is established in the
kernel, use only link interrupts feature in the driver. For the RGMII
port it results in depending on GMAC settings done during firmware
stage.
* 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 <[email protected]>
---
drivers/net/ethernet/marvell/mvpp2.c | 133 ++++++++++++++------
1 file changed, 94 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index f16448e..a1d7b88 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>
@@ -7502,7 +7503,10 @@ 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 = fwnode_irq_get(port->fwnode, i);
if (v->irq <= 0) {
ret = -EINVAL;
goto err;
@@ -7746,7 +7750,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);
@@ -7760,7 +7764,12 @@ static int mvpp2_port_probe(struct platform_device *pdev,
int phy_mode;
int err, i, cpu;
- 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;
@@ -7775,7 +7784,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");
@@ -7783,13 +7796,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)) {
@@ -7805,6 +7820,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
port = netdev_priv(dev);
port->dev = dev;
+ port->fwnode = port_fwnode;
port->ntxqs = ntxqs;
port->nrxqs = nrxqs;
port->priv = priv;
@@ -7814,7 +7830,10 @@ static int mvpp2_port_probe(struct platform_device *pdev,
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 = fwnode_irq_get(port_fwnode, port->nqvecs + 1);
if (port->link_irq == -EPROBE_DEFER) {
err = -EPROBE_DEFER;
goto err_deinit_qvecs;
@@ -8197,6 +8216,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 *fwnode = pdev->dev.fwnode;
struct fwnode_handle *port_fwnode;
struct mvpp2 *priv;
@@ -8209,8 +8229,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);
@@ -8224,10 +8250,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");
@@ -8253,32 +8292,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)) {
@@ -8291,10 +8332,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));
@@ -8399,6 +8444,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);
@@ -8420,12 +8468,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
Until now there were two very similar functions allowing
to get Linux IRQ number from ACPI handle (acpi_irq_get())
and OF node (of_irq_get()). The first one appeared to be used
only as a subroutine of platform_irq_get(), which (in the generic
code) limited IRQ obtaining from _CRS method only to nodes
associated to kernel's struct platform_device.
This patch introduces a new helper routine - fwnode_irq_get(),
which allows to get the IRQ number directly from the fwnode
to be used as common for OF/ACPI worlds. It is usable not
only for the parents fwnodes, but also for the child nodes
comprising their own _CRS methods with interrupts description.
In order to be able o satisfy compilation with !CONFIG_ACPI
and also simplify the new code, introduce a helper macro
(ACPI_HANDLE_FWNODE), with which it is possible to reach
an ACPI handle directly from its fwnode.
Signed-off-by: Marcin Wojtas <[email protected]>
---
drivers/base/property.c | 26 ++++++++++++++++++++
include/linux/acpi.h | 3 +++
include/linux/property.h | 2 ++
3 files changed, 31 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 7c4a53d..1d6c9d9 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -16,6 +16,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_graph.h>
+#include <linux/of_irq.h>
#include <linux/property.h>
#include <linux/etherdevice.h>
#include <linux/phy.h>
@@ -1230,6 +1231,31 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
EXPORT_SYMBOL(device_get_mac_address);
/**
+ * fwnode_irq_get - Get IRQ directly from a fwnode
+ * @fwnode: Pointer to the firmware node
+ * @index: Zero-based index of the IRQ
+ *
+ * Returns Linux IRQ number on success. Other values are determined
+ * accordingly to acpi_/of_ irq_get() operation.
+ */
+int fwnode_irq_get(struct fwnode_handle *fwnode, unsigned int index)
+{
+ struct device_node *of_node = to_of_node(fwnode);
+ struct resource res;
+ int ret;
+
+ if (IS_ENABLED(CONFIG_OF) && of_node)
+ return of_irq_get(of_node, index);
+
+ ret = acpi_irq_get(ACPI_HANDLE_FWNODE(fwnode), index, &res);
+ if (ret)
+ return ret;
+
+ return res.start;
+}
+EXPORT_SYMBOL(fwnode_irq_get);
+
+/**
* device_graph_get_next_endpoint - Get next endpoint firmware node
* @fwnode: Pointer to the parent firmware node
* @prev: Previous endpoint node or %NULL to get the first
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfe..f05b9b6 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -56,6 +56,8 @@ static inline acpi_handle acpi_device_handle(struct acpi_device *adev)
#define ACPI_COMPANION_SET(dev, adev) set_primary_fwnode(dev, (adev) ? \
acpi_fwnode_handle(adev) : NULL)
#define ACPI_HANDLE(dev) acpi_device_handle(ACPI_COMPANION(dev))
+#define ACPI_HANDLE_FWNODE(fwnode) \
+ acpi_device_handle(to_acpi_device_node(fwnode))
static inline struct fwnode_handle *acpi_alloc_fwnode_static(void)
{
@@ -626,6 +628,7 @@ int acpi_arch_timer_mem_init(struct arch_timer_mem *timer_mem, int *timer_count)
#define ACPI_COMPANION(dev) (NULL)
#define ACPI_COMPANION_SET(dev, adev) do { } while (0)
#define ACPI_HANDLE(dev) (NULL)
+#define ACPI_HANDLE_FWNODE(fwnode) (NULL)
#define ACPI_DEVICE_CLASS(_cls, _msk) .cls = (0), .cls_msk = (0),
struct fwnode_handle;
diff --git a/include/linux/property.h b/include/linux/property.h
index 9b13332..e05889f 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -103,6 +103,8 @@ struct fwnode_handle *device_get_named_child_node(struct device *dev,
struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
void fwnode_handle_put(struct fwnode_handle *fwnode);
+int fwnode_irq_get(struct fwnode_handle *fwnode, unsigned int index);
+
unsigned int device_get_child_node_count(struct device *dev);
static inline bool device_property_read_bool(struct device *dev,
--
2.7.4
'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 <[email protected]>
---
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
Implement a new helper function fwnode_get_next_available_child_node(),
which enables obtaining next enabled child fwnode, which
works on a similar basis to OF's of_get_next_available_child().
This commit also introduces a macro, thanks to which it is
possible to iterate over the available fwnodes, using the
new function described above.
Signed-off-by: Marcin Wojtas <[email protected]>
---
drivers/base/property.c | 26 ++++++++++++++++++++
include/linux/property.h | 6 +++++
2 files changed, 32 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1d6c9d9..613ba82 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -998,6 +998,32 @@ fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
/**
+ * fwnode_get_next_available_child_node - Return the next
+ * available child node handle for a node
+ * @fwnode: Firmware node to find the next child node for.
+ * @child: Handle to one of the node's child nodes or a %NULL handle.
+ */
+struct fwnode_handle *
+fwnode_get_next_available_child_node(const struct fwnode_handle *fwnode,
+ struct fwnode_handle *child)
+{
+ struct fwnode_handle *next_child = child;
+
+ if (!fwnode)
+ return NULL;
+
+ do {
+ next_child = fwnode_get_next_child_node(fwnode, next_child);
+
+ if (!next_child || fwnode_device_is_available(next_child))
+ break;
+ } while (next_child);
+
+ return next_child;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_next_available_child_node);
+
+/**
* device_get_next_child_node - Return the next child node handle for a device
* @dev: Device to find the next child node for.
* @child: Handle to one of the device's child nodes or a null handle.
diff --git a/include/linux/property.h b/include/linux/property.h
index e05889f..5b0563a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -83,11 +83,17 @@ struct fwnode_handle *fwnode_get_next_parent(
struct fwnode_handle *fwnode);
struct fwnode_handle *fwnode_get_next_child_node(
const struct fwnode_handle *fwnode, struct fwnode_handle *child);
+struct fwnode_handle *fwnode_get_next_available_child_node(
+ const struct fwnode_handle *fwnode, struct fwnode_handle *child);
#define fwnode_for_each_child_node(fwnode, child) \
for (child = fwnode_get_next_child_node(fwnode, NULL); child; \
child = fwnode_get_next_child_node(fwnode, child))
+#define fwnode_for_each_available_child_node(fwnode, child) \
+ for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
+ child = fwnode_get_next_available_child_node(fwnode, child))
+
struct fwnode_handle *device_get_next_child_node(
struct device *dev, struct fwnode_handle *child);
--
2.7.4
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 <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
---
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
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 <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
---
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
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.
This patch replaces of_* function with device_*/fwnode_*
where possible in the mvpp2.
Signed-off-by: Marcin Wojtas <[email protected]>
---
drivers/net/ethernet/marvell/mvpp2.c | 45 +++++++++++---------
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 7f42d90..f16448e 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 = "";
@@ -7773,7 +7776,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 +7792,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 +7823,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 +7848,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 +7879,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 +8197,8 @@ 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 *fwnode = pdev->dev.fwnode;
+ struct fwnode_handle *port_fwnode;
struct mvpp2 *priv;
struct resource *res;
void __iomem *base;
@@ -8315,8 +8318,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);
+ fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+ err = mvpp2_port_probe(pdev, port_fwnode, priv);
if (err < 0)
goto err_port_probe;
}
@@ -8347,7 +8350,7 @@ static int mvpp2_probe(struct platform_device *pdev)
err_port_probe:
i = 0;
- for_each_available_child_of_node(dn, port_node) {
+ fwnode_for_each_available_child_node(fwnode, port_fwnode) {
if (priv->port_list[i])
mvpp2_port_remove(priv->port_list[i]);
i++;
@@ -8366,14 +8369,14 @@ 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 *fwnode = pdev->dev.fwnode;
+ 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) {
+ fwnode_for_each_available_child_node(fwnode, 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
Hi Marcin,
I tested the series on a MacchiatoBin to ensure the mvpp2 DT support was
still working. I was able to use all supported ports as before, and saw
no issue.
For all mvpp2 patches, you can add:
Tested-by: Antoine Tenart <[email protected]>
Thanks!
Antoine
On Thu, Jan 18, 2018 at 01:31:37PM +0100, Marcin Wojtas wrote:
> Hi,
>
> I quickly resend the series, thanks to Antoine Tenart's remark,
> who spotted !CONFIG_ACPI compilation issue after introducing
> the new fwnode_irq_get() routine. Please see the details in the changelog
> below and the 3/7 commit log.
>
> 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/71ae395da1661374b0f07d1602afb1eee56e9794/Platforms/Marvell/Armada/AcpiTables/Armada80x0McBin/Dsdt.asl#L201
>
> * Armada 7040 DB:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/71ae395da1661374b0f07d1602afb1eee56e9794/Platforms/Marvell/Armada/AcpiTables/Armada70x0/Dsdt.asl#L131
>
> I will appreciate any comments or remarks.
>
> Best regards,
> Marcin
>
> Changelog:
> v3 -> v4:
> * 3/7
> - add new macro (ACPI_HANDLE_FWNODE) and fix
> compilation with !CONFIG_ACPI
> - extend commit log and mention usability of fwnode_irq_get
> for the child nodes as well
>
> v2 -> v3:
> * 1/7, 2/7
> - Add Rafael's Acked-by's
> * 3/7, 4/7
> - New patches
> * 6/7, 7/7
> - Update driver with new helper routines usage
> - Improve commit log.
>
> 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 (7):
> device property: Introduce fwnode_get_mac_address()
> device property: Introduce fwnode_get_phy_mode()
> device property: Introduce fwnode_irq_get()
> device property: Allow iterating over available child fwnodes
> 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 | 104 ++++++++--
> drivers/net/ethernet/marvell/mvpp2.c | 206 ++++++++++++--------
> include/linux/acpi.h | 3 +
> include/linux/property.h | 11 ++
> 4 files changed, 232 insertions(+), 92 deletions(-)
>
> --
> 2.7.4
>
--
Antoine T?nart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi David,
There's a discussion about the ACPI vs generic MDIO/PHY change under
initial version of the patchset, however the patches in question were
for now abandoned from further re-sends.
As the v4 has had no objections until now and:
- patches 1-2 were Acked-by: Rafael J. Wysocki <[email protected]>
- mvpp2 patches (5-7) were Tested-by: Antoine Tenart
<[email protected]>
- entire series was Reviewed-by: Graeme Gregory <[email protected]>
Do you see any chance that it lands in net-next before the coming
merge window? It would be really much appreciated.
Thanks,
Marcin
2018-01-18 14:23 GMT+01:00 Antoine Tenart <[email protected]>:
> Hi Marcin,
>
> I tested the series on a MacchiatoBin to ensure the mvpp2 DT support was
> still working. I was able to use all supported ports as before, and saw
> no issue.
>
> For all mvpp2 patches, you can add:
>
> Tested-by: Antoine Tenart <[email protected]>
>
> Thanks!
> Antoine
>
> On Thu, Jan 18, 2018 at 01:31:37PM +0100, Marcin Wojtas wrote:
>> Hi,
>>
>> I quickly resend the series, thanks to Antoine Tenart's remark,
>> who spotted !CONFIG_ACPI compilation issue after introducing
>> the new fwnode_irq_get() routine. Please see the details in the changelog
>> below and the 3/7 commit log.
>>
>> 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/71ae395da1661374b0f07d1602afb1eee56e9794/Platforms/Marvell/Armada/AcpiTables/Armada80x0McBin/Dsdt.asl#L201
>>
>> * Armada 7040 DB:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/71ae395da1661374b0f07d1602afb1eee56e9794/Platforms/Marvell/Armada/AcpiTables/Armada70x0/Dsdt.asl#L131
>>
>> I will appreciate any comments or remarks.
>>
>> Best regards,
>> Marcin
>>
>> Changelog:
>> v3 -> v4:
>> * 3/7
>> - add new macro (ACPI_HANDLE_FWNODE) and fix
>> compilation with !CONFIG_ACPI
>> - extend commit log and mention usability of fwnode_irq_get
>> for the child nodes as well
>>
>> v2 -> v3:
>> * 1/7, 2/7
>> - Add Rafael's Acked-by's
>> * 3/7, 4/7
>> - New patches
>> * 6/7, 7/7
>> - Update driver with new helper routines usage
>> - Improve commit log.
>>
>> 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 (7):
>> device property: Introduce fwnode_get_mac_address()
>> device property: Introduce fwnode_get_phy_mode()
>> device property: Introduce fwnode_irq_get()
>> device property: Allow iterating over available child fwnodes
>> 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 | 104 ++++++++--
>> drivers/net/ethernet/marvell/mvpp2.c | 206 ++++++++++++--------
>> include/linux/acpi.h | 3 +
>> include/linux/property.h | 11 ++
>> 4 files changed, 232 insertions(+), 92 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> --
> Antoine Ténart, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
From: Marcin Wojtas <[email protected]>
Date: Mon, 22 Jan 2018 14:00:37 +0100
> There's a discussion about the ACPI vs generic MDIO/PHY change under
> initial version of the patchset, however the patches in question were
> for now abandoned from further re-sends.
But doesn't the results of that discussion determine whether the way ACPI
is being used in this patch series is what we want to do or not?
On Mon, Jan 22, 2018 at 09:35:25AM -0500, David Miller wrote:
> From: Marcin Wojtas <[email protected]>
> Date: Mon, 22 Jan 2018 14:00:37 +0100
>
> > There's a discussion about the ACPI vs generic MDIO/PHY change under
> > initial version of the patchset, however the patches in question were
> > for now abandoned from further re-sends.
>
> But doesn't the results of that discussion determine whether the way ACPI
> is being used in this patch series is what we want to do or not?
Hi David
The patches submitted here don't involve any ACPI for MDIO or PHY. It
is all MAC. And it is using standard ACPI primitives for devices,
nothing new.
It is not setting any precedence for MDIO and PHY. That is totally out
of scope for these patches. Whatever is decided for MDIO and PHY can
be added later.
Andrew
2018-01-22 15:43 GMT+01:00 Andrew Lunn <[email protected]>:
> On Mon, Jan 22, 2018 at 09:35:25AM -0500, David Miller wrote:
>> From: Marcin Wojtas <[email protected]>
>> Date: Mon, 22 Jan 2018 14:00:37 +0100
>>
>> > There's a discussion about the ACPI vs generic MDIO/PHY change under
>> > initial version of the patchset, however the patches in question were
>> > for now abandoned from further re-sends.
>>
>> But doesn't the results of that discussion determine whether the way ACPI
>> is being used in this patch series is what we want to do or not?
>
> Hi David
>
> The patches submitted here don't involve any ACPI for MDIO or PHY. It
> is all MAC. And it is using standard ACPI primitives for devices,
> nothing new.
>
> It is not setting any precedence for MDIO and PHY. That is totally out
> of scope for these patches. Whatever is decided for MDIO and PHY can
> be added later.
>
Indeed, generic helper routines will be used in drivers (net and
others) and the mvpp2 with this patchset is ready to whatever shape
MDIO+ACPI goes in future, so there will be no need to revert any
changes there.
Best regards,
Marcin
From: Andrew Lunn <[email protected]>
Date: Mon, 22 Jan 2018 15:43:42 +0100
> On Mon, Jan 22, 2018 at 09:35:25AM -0500, David Miller wrote:
>> From: Marcin Wojtas <[email protected]>
>> Date: Mon, 22 Jan 2018 14:00:37 +0100
>>
>> > There's a discussion about the ACPI vs generic MDIO/PHY change under
>> > initial version of the patchset, however the patches in question were
>> > for now abandoned from further re-sends.
>>
>> But doesn't the results of that discussion determine whether the way ACPI
>> is being used in this patch series is what we want to do or not?
>
> Hi David
>
> The patches submitted here don't involve any ACPI for MDIO or PHY. It
> is all MAC. And it is using standard ACPI primitives for devices,
> nothing new.
>
> It is not setting any precedence for MDIO and PHY. That is totally out
> of scope for these patches. Whatever is decided for MDIO and PHY can
> be added later.
Thanks for all of the clarifications.
Series applied to net-next, thank you.
2018-01-22 16:57 GMT+01:00 David Miller <[email protected]>:
> From: Andrew Lunn <[email protected]>
> Date: Mon, 22 Jan 2018 15:43:42 +0100
>
>> On Mon, Jan 22, 2018 at 09:35:25AM -0500, David Miller wrote:
>>> From: Marcin Wojtas <[email protected]>
>>> Date: Mon, 22 Jan 2018 14:00:37 +0100
>>>
>>> > There's a discussion about the ACPI vs generic MDIO/PHY change under
>>> > initial version of the patchset, however the patches in question were
>>> > for now abandoned from further re-sends.
>>>
>>> But doesn't the results of that discussion determine whether the way ACPI
>>> is being used in this patch series is what we want to do or not?
>>
>> Hi David
>>
>> The patches submitted here don't involve any ACPI for MDIO or PHY. It
>> is all MAC. And it is using standard ACPI primitives for devices,
>> nothing new.
>>
>> It is not setting any precedence for MDIO and PHY. That is totally out
>> of scope for these patches. Whatever is decided for MDIO and PHY can
>> be added later.
>
> Thanks for all of the clarifications.
>
> Series applied to net-next, thank you.
Great, thanks!
On Thu, Jan 18, 2018 at 1:31 PM, Marcin Wojtas <[email protected]> wrote:
> Until now there were two very similar functions allowing
> to get Linux IRQ number from ACPI handle (acpi_irq_get())
> and OF node (of_irq_get()). The first one appeared to be used
> only as a subroutine of platform_irq_get(), which (in the generic
> code) limited IRQ obtaining from _CRS method only to nodes
> associated to kernel's struct platform_device.
>
> This patch introduces a new helper routine - fwnode_irq_get(),
> which allows to get the IRQ number directly from the fwnode
> to be used as common for OF/ACPI worlds. It is usable not
> only for the parents fwnodes, but also for the child nodes
> comprising their own _CRS methods with interrupts description.
>
> In order to be able o satisfy compilation with !CONFIG_ACPI
> and also simplify the new code, introduce a helper macro
> (ACPI_HANDLE_FWNODE), with which it is possible to reach
> an ACPI handle directly from its fwnode.
>
> Signed-off-by: Marcin Wojtas <[email protected]>
> ---
> drivers/base/property.c | 26 ++++++++++++++++++++
> include/linux/acpi.h | 3 +++
> include/linux/property.h | 2 ++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 7c4a53d..1d6c9d9 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -16,6 +16,7 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_graph.h>
> +#include <linux/of_irq.h>
> #include <linux/property.h>
> #include <linux/etherdevice.h>
> #include <linux/phy.h>
> @@ -1230,6 +1231,31 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
> EXPORT_SYMBOL(device_get_mac_address);
>
> /**
> + * fwnode_irq_get - Get IRQ directly from a fwnode
> + * @fwnode: Pointer to the firmware node
> + * @index: Zero-based index of the IRQ
> + *
> + * Returns Linux IRQ number on success. Other values are determined
> + * accordingly to acpi_/of_ irq_get() operation.
> + */
> +int fwnode_irq_get(struct fwnode_handle *fwnode, unsigned int index)
> +{
> + struct device_node *of_node = to_of_node(fwnode);
> + struct resource res;
> + int ret;
> +
> + if (IS_ENABLED(CONFIG_OF) && of_node)
> + return of_irq_get(of_node, index);
> +
> + ret = acpi_irq_get(ACPI_HANDLE_FWNODE(fwnode), index, &res);
> + if (ret)
> + return ret;
> +
> + return res.start;
> +}
> +EXPORT_SYMBOL(fwnode_irq_get);
EXPORT_SYMBOL_GPL(), please.
> +
> +/**
> * device_graph_get_next_endpoint - Get next endpoint firmware node
> * @fwnode: Pointer to the parent firmware node
> * @prev: Previous endpoint node or %NULL to get the first
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfe..f05b9b6 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -56,6 +56,8 @@ static inline acpi_handle acpi_device_handle(struct acpi_device *adev)
> #define ACPI_COMPANION_SET(dev, adev) set_primary_fwnode(dev, (adev) ? \
> acpi_fwnode_handle(adev) : NULL)
> #define ACPI_HANDLE(dev) acpi_device_handle(ACPI_COMPANION(dev))
> +#define ACPI_HANDLE_FWNODE(fwnode) \
> + acpi_device_handle(to_acpi_device_node(fwnode))
>
This change should go as a separate patch with a clear explanation why
it is needed in the changelog.
> static inline struct fwnode_handle *acpi_alloc_fwnode_static(void)
> {
> @@ -626,6 +628,7 @@ int acpi_arch_timer_mem_init(struct arch_timer_mem *timer_mem, int *timer_count)
> #define ACPI_COMPANION(dev) (NULL)
> #define ACPI_COMPANION_SET(dev, adev) do { } while (0)
> #define ACPI_HANDLE(dev) (NULL)
> +#define ACPI_HANDLE_FWNODE(fwnode) (NULL)
> #define ACPI_DEVICE_CLASS(_cls, _msk) .cls = (0), .cls_msk = (0),
>
> struct fwnode_handle;
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 9b13332..e05889f 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -103,6 +103,8 @@ struct fwnode_handle *device_get_named_child_node(struct device *dev,
> struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
> void fwnode_handle_put(struct fwnode_handle *fwnode);
>
> +int fwnode_irq_get(struct fwnode_handle *fwnode, unsigned int index);
> +
> unsigned int device_get_child_node_count(struct device *dev);
>
> static inline bool device_property_read_bool(struct device *dev,
> --
On Thu, Jan 18, 2018 at 1:31 PM, Marcin Wojtas <[email protected]> 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 <[email protected]>
> Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> 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);
That should be EXPORT_SYMBOL_GPL().
I have overlooked that previously, sorry about that.
> +
> +/**
> + * 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);
Same here.
Generally speaking, you should use EXPORT_SYMBOL_GPL() everywhere
unless there's a specific reason for not doing that in which cases
that specific reason has to be clearly spelled out at least in the
changelog of the patch, but really better in a code comment.
>
> 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 *
> --
On Thu, Jan 18, 2018 at 1:31 PM, Marcin Wojtas <[email protected]> wrote:
> Implement a new helper function fwnode_get_next_available_child_node(),
> which enables obtaining next enabled child fwnode, which
> works on a similar basis to OF's of_get_next_available_child().
>
> This commit also introduces a macro, thanks to which it is
> possible to iterate over the available fwnodes, using the
> new function described above.
>
> Signed-off-by: Marcin Wojtas <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/base/property.c | 26 ++++++++++++++++++++
> include/linux/property.h | 6 +++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 1d6c9d9..613ba82 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -998,6 +998,32 @@ fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
> EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
>
> /**
> + * fwnode_get_next_available_child_node - Return the next
> + * available child node handle for a node
> + * @fwnode: Firmware node to find the next child node for.
> + * @child: Handle to one of the node's child nodes or a %NULL handle.
> + */
> +struct fwnode_handle *
> +fwnode_get_next_available_child_node(const struct fwnode_handle *fwnode,
> + struct fwnode_handle *child)
> +{
> + struct fwnode_handle *next_child = child;
> +
> + if (!fwnode)
> + return NULL;
> +
> + do {
> + next_child = fwnode_get_next_child_node(fwnode, next_child);
> +
> + if (!next_child || fwnode_device_is_available(next_child))
> + break;
> + } while (next_child);
> +
> + return next_child;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_get_next_available_child_node);
> +
> +/**
> * device_get_next_child_node - Return the next child node handle for a device
> * @dev: Device to find the next child node for.
> * @child: Handle to one of the device's child nodes or a null handle.
> diff --git a/include/linux/property.h b/include/linux/property.h
> index e05889f..5b0563a 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -83,11 +83,17 @@ struct fwnode_handle *fwnode_get_next_parent(
> struct fwnode_handle *fwnode);
> struct fwnode_handle *fwnode_get_next_child_node(
> const struct fwnode_handle *fwnode, struct fwnode_handle *child);
> +struct fwnode_handle *fwnode_get_next_available_child_node(
> + const struct fwnode_handle *fwnode, struct fwnode_handle *child);
>
> #define fwnode_for_each_child_node(fwnode, child) \
> for (child = fwnode_get_next_child_node(fwnode, NULL); child; \
> child = fwnode_get_next_child_node(fwnode, child))
>
> +#define fwnode_for_each_available_child_node(fwnode, child) \
> + for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
> + child = fwnode_get_next_available_child_node(fwnode, child))
> +
> struct fwnode_handle *device_get_next_child_node(
> struct device *dev, struct fwnode_handle *child);
>
> --
Hi Rafael,
> > 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);
>
> That should be EXPORT_SYMBOL_GPL().
>
> I have overlooked that previously, sorry about that.
The series landed yesterday in net-next, so I need to send a fix on
top. Would you be ok with single patch fixing all EXPORT_SYMBOL()
occurences? Those would be 2 new routines:
- fwnode_get_mac_address
- fwnode_irq_get
and 2 already existing in the file:
- device_get_mac_address
- fwnode_graph_parse_endpoint
Please let know, how you prefer to handle it?
Best regards,
Marcin
>
> > +
> > +/**
> > + * 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);
>
> Same here.
>
> Generally speaking, you should use EXPORT_SYMBOL_GPL() everywhere
> unless there's a specific reason for not doing that in which cases
> that specific reason has to be clearly spelled out at least in the
> changelog of the patch, but really better in a code comment.
>
> >
> > 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 *
> > --
On Tue, Jan 23, 2018 at 7:12 AM, Marcin Wojtas <[email protected]> wrote:
> Hi Rafael,
>
>> > 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);
>>
>> That should be EXPORT_SYMBOL_GPL().
>>
>> I have overlooked that previously, sorry about that.
>
> The series landed yesterday in net-next, so I need to send a fix on
> top.
OK
> Would you be ok with single patch fixing all EXPORT_SYMBOL()
> occurences? Those would be 2 new routines:
> - fwnode_get_mac_address
> - fwnode_irq_get
> and 2 already existing in the file:
> - device_get_mac_address
> - fwnode_graph_parse_endpoint
>
> Please let know, how you prefer to handle it?
I guess it's better to fix this up when the series gets merged.
Thanks,
Rafael
Hi Rafael,
2018-01-24 3:08 GMT+01:00 Rafael J. Wysocki <[email protected]>:
> On Tue, Jan 23, 2018 at 7:12 AM, Marcin Wojtas <[email protected]> wrote:
>> Hi Rafael,
>>
>>> > 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);
>>>
>>> That should be EXPORT_SYMBOL_GPL().
>>>
>>> I have overlooked that previously, sorry about that.
>>
>> The series landed yesterday in net-next, so I need to send a fix on
>> top.
>
> OK
>
>> Would you be ok with single patch fixing all EXPORT_SYMBOL()
>> occurences? Those would be 2 new routines:
>> - fwnode_get_mac_address
>> - fwnode_irq_get
>> and 2 already existing in the file:
>> - device_get_mac_address
>> - fwnode_graph_parse_endpoint
>>
>> Please let know, how you prefer to handle it?
>
> I guess it's better to fix this up when the series gets merged.
>
Ok, I will do it at that time.
Thanks,
Marcin