* [PATCH net v3 1/5] drivers: net: cpsw: fix parsing of phy-handle DT property in dual_emac config
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
@ 2016-04-28 1:25 ` David Rivshin (Allworx)
2016-04-28 1:32 ` [PATCH net v3 2/5] drivers: net: cpsw: fix segfault in case of bad phy-handle David Rivshin (Allworx)
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: David Rivshin (Allworx) @ 2016-04-28 1:25 UTC (permalink / raw)
To: netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Grygorii Strashko, Andrew Goodbody, Markus Brunner,
Nicolas Chauvet
From: David Rivshin <drivshin@allworx.com>
Commit 9e42f715264ff158478fa30eaed847f6e131366b ("drivers: net: cpsw: add
phy-handle parsing") saved the "phy-handle" phandle into a new cpsw_priv
field. However, phy connections are per-slave, so the phy_node field should
be in cpsw_slave_data rather than cpsw_priv.
This would go unnoticed in a single emac configuration. But in dual_emac
mode, the last "phy-handle" property parsed for either slave would be used
by both of them, causing them both to refer to the same phy_device.
Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")
Signed-off-by: David Rivshin <drivshin@allworx.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Andrew Goodbody <andrew.goodbody@cambrionix.com>
Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
I would suggest this for -stable. It should apply cleanly as far back
as 4.4.
Changes since v2 [1]:
- Added Tested-by from Andrew Goodbody [3]
- Added Reviewed-by from Mugunthan V N [4]
- Added Reviewed-by from Grygorii Strashko [5]
Changes since v1 [2]:
- Rebased (no conflicts)
- Added Tested-by from Nicolas Chauvet
[1] http://patchwork.ozlabs.org/patch/613237/
[2] http://patchwork.ozlabs.org/patch/560326/
[3] https://lkml.org/lkml/2016/4/22/537
[4] https://lkml.org/lkml/2016/4/22/63
[5] https://lkml.org/lkml/2016/4/22/496
drivers/net/ethernet/ti/cpsw.c | 13 ++++++-------
drivers/net/ethernet/ti/cpsw.h | 1 +
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index bbb77cd..ce0b0ca 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -363,15 +363,14 @@ static inline void slave_write(struct cpsw_slave *slave, u32 val, u32 offset)
__raw_writel(val, slave->regs + offset);
}
struct cpsw_priv {
spinlock_t lock;
struct platform_device *pdev;
struct net_device *ndev;
- struct device_node *phy_node;
struct napi_struct napi_rx;
struct napi_struct napi_tx;
struct device *dev;
struct cpsw_platform_data data;
struct cpsw_ss_regs __iomem *regs;
struct cpsw_wr_regs __iomem *wr_regs;
u8 __iomem *hw_stats;
@@ -1144,16 +1143,16 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
if (priv->data.dual_emac)
cpsw_add_dual_emac_def_ale_entries(priv, slave, slave_port);
else
cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
- if (priv->phy_node)
- slave->phy = of_phy_connect(priv->ndev, priv->phy_node,
+ if (slave->data->phy_node)
+ slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
&cpsw_adjust_link, 0, slave->data->phy_if);
else
slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
&cpsw_adjust_link, slave->data->phy_if);
if (IS_ERR(slave->phy)) {
dev_err(priv->dev, "phy %s not found on slave %d\n",
slave->data->phy_id, slave->slave_num);
@@ -1936,20 +1935,19 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
slave->data = data;
slave->regs = regs + slave_reg_ofs;
slave->sliver = regs + sliver_reg_ofs;
slave->port_vlan = data->dual_emac_res_vlan;
}
-static int cpsw_probe_dt(struct cpsw_priv *priv,
+static int cpsw_probe_dt(struct cpsw_platform_data *data,
struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
struct device_node *slave_node;
- struct cpsw_platform_data *data = &priv->data;
int i = 0, ret;
u32 prop;
if (!node)
return -EINVAL;
if (of_property_read_u32(node, "slaves", &prop)) {
@@ -2029,15 +2027,16 @@ static int cpsw_probe_dt(struct cpsw_priv *priv,
int lenp;
const __be32 *parp;
/* This is no slave child node, continue */
if (strcmp(slave_node->name, "slave"))
continue;
- priv->phy_node = of_parse_phandle(slave_node, "phy-handle", 0);
+ slave_data->phy_node = of_parse_phandle(slave_node,
+ "phy-handle", 0);
parp = of_get_property(slave_node, "phy_id", &lenp);
if (of_phy_is_fixed_link(slave_node)) {
struct device_node *phy_node;
struct phy_device *phy_dev;
/* In the case of a fixed PHY, the DT node associated
* to the PHY is the Ethernet MAC DT node.
@@ -2271,15 +2270,15 @@ static int cpsw_probe(struct platform_device *pdev)
* This may be required here for child devices.
*/
pm_runtime_enable(&pdev->dev);
/* Select default pin state */
pinctrl_pm_select_default_state(&pdev->dev);
- if (cpsw_probe_dt(priv, pdev)) {
+ if (cpsw_probe_dt(&priv->data, pdev)) {
dev_err(&pdev->dev, "cpsw: platform data missing\n");
ret = -ENODEV;
goto clean_runtime_disable_ret;
}
data = &priv->data;
if (is_valid_ether_addr(data->slave_data[0].mac_addr)) {
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 442a703..e50afd1 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -14,14 +14,15 @@
#ifndef __CPSW_H__
#define __CPSW_H__
#include <linux/if_ether.h>
#include <linux/phy.h>
struct cpsw_slave_data {
+ struct device_node *phy_node;
char phy_id[MII_BUS_ID_SIZE];
int phy_if;
u8 mac_addr[ETH_ALEN];
u16 dual_emac_res_vlan; /* Reserved VLAN for DualEMAC */
};
struct cpsw_platform_data {
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net v3 2/5] drivers: net: cpsw: fix segfault in case of bad phy-handle
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
2016-04-28 1:25 ` [PATCH net v3 1/5] drivers: net: cpsw: fix parsing of phy-handle DT property in dual_emac config David Rivshin (Allworx)
@ 2016-04-28 1:32 ` David Rivshin (Allworx)
2016-04-28 1:38 ` [PATCH net v3 3/5] drivers: net: cpsw: don't ignore phy-mode if phy-handle is used David Rivshin (Allworx)
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: David Rivshin (Allworx) @ 2016-04-28 1:32 UTC (permalink / raw)
To: netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Grygorii Strashko, Andrew Goodbody, Markus Brunner,
Nicolas Chauvet
From: David Rivshin <drivshin@allworx.com>
If an emac node has a phy-handle property that points to something
which is not a phy, then a segmentation fault will occur when the
interface is brought up. This is because while phy_connect() will
return ERR_PTR() on failure, of_phy_connect() will return NULL.
The common error check uses IS_ERR(), and so missed when
of_phy_connect() fails. The NULL pointer is then dereferenced.
Also, the common error message referenced slave->data->phy_id,
which would be empty in the case of phy-handle. Instead, use the
name of the device_node as a useful identifier. And in the phy_id
case add the error code for completeness.
Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")
Signed-off-by: David Rivshin <drivshin@allworx.com>
---
I would suggest this for -stable. It should apply cleanly as far back
as 4.5, although there is a trivial conflict in 4.4. I can produce a
separate patch against linux-4.4.y if preferred.
Changes since v2:
- new patch, although fixing part of previous patch 2 [1]
Changes since v1 [2]:
- Rebased (no conflicts)
- Added Tested-by from Nicolas Chauvet
- Added Acked-by from Rob Herring for the binding change
[1] http://patchwork.ozlabs.org/patch/613260/
[2] http://patchwork.ozlabs.org/patch/560324/
drivers/net/ethernet/ti/cpsw.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ce0b0ca..5903448 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1143,33 +1143,42 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
if (priv->data.dual_emac)
cpsw_add_dual_emac_def_ale_entries(priv, slave, slave_port);
else
cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
- if (slave->data->phy_node)
+ if (slave->data->phy_node) {
slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
&cpsw_adjust_link, 0, slave->data->phy_if);
- else
+ if (!slave->phy) {
+ dev_err(priv->dev, "phy \"%s\" not found on slave %d\n",
+ slave->data->phy_node->full_name,
+ slave->slave_num);
+ return;
+ }
+ } else {
slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
&cpsw_adjust_link, slave->data->phy_if);
- if (IS_ERR(slave->phy)) {
- dev_err(priv->dev, "phy %s not found on slave %d\n",
- slave->data->phy_id, slave->slave_num);
- slave->phy = NULL;
- } else {
- phy_attached_info(slave->phy);
-
- phy_start(slave->phy);
-
- /* Configure GMII_SEL register */
- cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface,
- slave->slave_num);
+ if (IS_ERR(slave->phy)) {
+ dev_err(priv->dev,
+ "phy \"%s\" not found on slave %d, err %ld\n",
+ slave->data->phy_id, slave->slave_num,
+ PTR_ERR(slave->phy));
+ slave->phy = NULL;
+ return;
+ }
}
+
+ phy_attached_info(slave->phy);
+
+ phy_start(slave->phy);
+
+ /* Configure GMII_SEL register */
+ cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface, slave->slave_num);
}
static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
{
const int vlan = priv->data.default_vlan;
const int port = priv->host_port;
u32 reg;
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net v3 3/5] drivers: net: cpsw: don't ignore phy-mode if phy-handle is used
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
2016-04-28 1:25 ` [PATCH net v3 1/5] drivers: net: cpsw: fix parsing of phy-handle DT property in dual_emac config David Rivshin (Allworx)
2016-04-28 1:32 ` [PATCH net v3 2/5] drivers: net: cpsw: fix segfault in case of bad phy-handle David Rivshin (Allworx)
@ 2016-04-28 1:38 ` David Rivshin (Allworx)
2016-04-28 1:42 ` [PATCH net v3 4/5] dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive David Rivshin (Allworx)
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: David Rivshin (Allworx) @ 2016-04-28 1:38 UTC (permalink / raw)
To: netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Grygorii Strashko, Andrew Goodbody, Markus Brunner,
Nicolas Chauvet
From: David Rivshin <drivshin@allworx.com>
The phy-mode emac property was only being processed in the phy_id
or fixed-link cases. However if phy-handle was specified instead,
an error message would complain about the lack of phy_id or
fixed-link, and then jump past the of_get_phy_mode(). This would
result in the PHY mode defaulting to MII, regardless of what the
devicetree specified.
Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")
Signed-off-by: David Rivshin <drivshin@allworx.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Andrew Goodbody <andrew.goodbody@cambrionix.com>
Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>
---
I would suggest this for -stable. It should apply cleanly as far back
as 4.4.
Changes since v2 [1]:
- split from previous patch 2
- Added Tested-by from Andrew Goodbody [3]
- Added Reviewed-by from Mugunthan V N [4]
- rewrote commit log to focus on the functional bug fixed, rather
than the bogus error message
Changes since v1 [2]:
- Rebased (no conflicts)
- Added Tested-by from Nicolas Chauvet
- Added Acked-by from Rob Herring for the binding change
[1] http://patchwork.ozlabs.org/patch/613260/
[2] http://patchwork.ozlabs.org/patch/560324/
[3] https://lkml.org/lkml/2016/4/22/537
[4] https://lkml.org/lkml/2016/4/22/63
drivers/net/ethernet/ti/cpsw.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 5903448..712bc6d 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2039,15 +2039,19 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
/* This is no slave child node, continue */
if (strcmp(slave_node->name, "slave"))
continue;
slave_data->phy_node = of_parse_phandle(slave_node,
"phy-handle", 0);
parp = of_get_property(slave_node, "phy_id", &lenp);
- if (of_phy_is_fixed_link(slave_node)) {
+ if (slave_data->phy_node) {
+ dev_dbg(&pdev->dev,
+ "slave[%d] using phy-handle=\"%s\"\n",
+ i, slave_data->phy_node->full_name);
+ } else if (of_phy_is_fixed_link(slave_node)) {
struct device_node *phy_node;
struct phy_device *phy_dev;
/* In the case of a fixed PHY, the DT node associated
* to the PHY is the Ethernet MAC DT node.
*/
ret = of_phy_register_fixed_link(slave_node);
@@ -2076,15 +2080,17 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
if (!mdio) {
dev_err(&pdev->dev, "Missing mdio platform device\n");
return -EINVAL;
}
snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
PHY_ID_FMT, mdio->name, phyid);
} else {
- dev_err(&pdev->dev, "No slave[%d] phy_id or fixed-link property\n", i);
+ dev_err(&pdev->dev,
+ "No slave[%d] phy_id, phy-handle, or fixed-link property\n",
+ i);
goto no_phy_slave;
}
slave_data->phy_if = of_get_phy_mode(slave_node);
if (slave_data->phy_if < 0) {
dev_err(&pdev->dev, "Missing or malformed slave[%d] phy-mode property\n",
i);
return slave_data->phy_if;
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net v3 4/5] dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
` (2 preceding siblings ...)
2016-04-28 1:38 ` [PATCH net v3 3/5] drivers: net: cpsw: don't ignore phy-mode if phy-handle is used David Rivshin (Allworx)
@ 2016-04-28 1:42 ` David Rivshin (Allworx)
[not found] ` <1461807767-4563-1-git-send-email-drivshin.allworx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-04-28 1:45 ` [PATCH net v3 5/5] drivers: net: cpsw: use of_phy_connect() in fixed-link case David Rivshin (Allworx)
` (3 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: David Rivshin (Allworx) @ 2016-04-28 1:42 UTC (permalink / raw)
To: netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Grygorii Strashko, Andrew Goodbody, Markus Brunner,
Nicolas Chauvet
From: David Rivshin <drivshin@allworx.com>
The phy-handle, phy_id, and fixed-link properties are mutually exclusive,
and only one need be specified. Make this clear in the binding doc.
Also mark the phy_id property as deprecated, as phy-handle should be
used instead.
Signed-off-by: David Rivshin <drivshin@allworx.com>
---
Changes since v2 [1]:
- split from previous patch 2
- marked the phy_id property as deprecated [3]
- removed Rob Herring's Acked-by due to above change
Changes since v1 [2]:
- Rebased (no conflicts)
- Added Tested-by from Nicolas Chauvet
- Added Acked-by from Rob Herring for the binding change
[1] http://patchwork.ozlabs.org/patch/613260/
[2] http://patchwork.ozlabs.org/patch/560324/
[3] https://lkml.org/lkml/2016/4/22/494
Documentation/devicetree/bindings/net/cpsw.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 28a4781..0ae0649 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -41,21 +41,21 @@ Optional properties:
Slave Properties:
Required properties:
- phy-mode : See ethernet.txt file in the same directory
Optional properties:
- dual_emac_res_vlan : Specifies VID to be used to segregate the ports
- mac-address : See ethernet.txt file in the same directory
-- phy_id : Specifies slave phy id
+- phy_id : Specifies slave phy id (deprecated, use phy-handle)
- phy-handle : See ethernet.txt file in the same directory
Slave sub-nodes:
- fixed-link : See fixed-link.txt file in the same directory
- Either the property phy_id, or the sub-node
- fixed-link can be specified
+
+Note: Exactly one of phy_id, phy-handle, or fixed-link must be specified.
Note: "ti,hwmods" field is used to fetch the base address and irq
resources from TI, omap hwmod data base during device registration.
Future plan is to migrate hwmod data base contents into device tree
blob so that, all the required data will be used from device tree dts
file.
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH net v3 5/5] drivers: net: cpsw: use of_phy_connect() in fixed-link case
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
` (3 preceding siblings ...)
2016-04-28 1:42 ` [PATCH net v3 4/5] dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive David Rivshin (Allworx)
@ 2016-04-28 1:45 ` David Rivshin (Allworx)
2016-04-28 14:28 ` [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes Tony Lindgren
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: David Rivshin (Allworx) @ 2016-04-28 1:45 UTC (permalink / raw)
To: netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Grygorii Strashko, Andrew Goodbody, Markus Brunner,
Nicolas Chauvet
From: David Rivshin <drivshin@allworx.com>
If a fixed-link DT subnode is used, the phy_device was looked up so
that a PHY ID string could be constructed and passed to phy_connect().
This is not necessary, as the device_node can be passed directly to
of_phy_connect() instead. This reuses the same codepath as if the
phy-handle DT property was used.
Signed-off-by: David Rivshin <drivshin@allworx.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Andrew Goodbody <andrew.goodbody@cambrionix.com>
Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Changes since v2 [1]:
- Added Tested-by from Andrew Goodbody [3]
- Added Reviewed-by from Mugunthan V N [4]
- Added Reviewed-by from Grygorii Strashko [5]
Changes since v1 [2]:
- Rebased (trivial conflict, e5a03bfd modified the deleted snprintf)
- Added Tested-by from Nicolas Chauvet
[1] http://patchwork.ozlabs.org/patch/613276/
[2] http://patchwork.ozlabs.org/patch/560327/
[3] https://lkml.org/lkml/2016/4/22/537
[4] https://lkml.org/lkml/2016/4/22/63
[5] https://lkml.org/lkml/2016/4/22/529
drivers/net/ethernet/ti/cpsw.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 712bc6d..e2fcdf1 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2044,30 +2044,21 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
"phy-handle", 0);
parp = of_get_property(slave_node, "phy_id", &lenp);
if (slave_data->phy_node) {
dev_dbg(&pdev->dev,
"slave[%d] using phy-handle=\"%s\"\n",
i, slave_data->phy_node->full_name);
} else if (of_phy_is_fixed_link(slave_node)) {
- struct device_node *phy_node;
- struct phy_device *phy_dev;
-
/* In the case of a fixed PHY, the DT node associated
* to the PHY is the Ethernet MAC DT node.
*/
ret = of_phy_register_fixed_link(slave_node);
if (ret)
return ret;
- phy_node = of_node_get(slave_node);
- phy_dev = of_phy_find_device(phy_node);
- if (!phy_dev)
- return -ENODEV;
- snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
- PHY_ID_FMT, phy_dev->mdio.bus->id,
- phy_dev->mdio.addr);
+ slave_data->phy_node = of_node_get(slave_node);
} else if (parp) {
u32 phyid;
struct device_node *mdio_node;
struct platform_device *mdio;
if (lenp != (sizeof(__be32) * 2)) {
dev_err(&pdev->dev, "Invalid slave[%d] phy_id property\n", i);
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
` (4 preceding siblings ...)
2016-04-28 1:45 ` [PATCH net v3 5/5] drivers: net: cpsw: use of_phy_connect() in fixed-link case David Rivshin (Allworx)
@ 2016-04-28 14:28 ` Tony Lindgren
2016-04-28 15:55 ` Grygorii Strashko
2016-04-28 21:27 ` David Miller
7 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2016-04-28 14:28 UTC (permalink / raw)
To: David Rivshin (Allworx)
Cc: netdev, linux-omap, Markus Brunner, devicetree, Grygorii Strashko,
Mugunthan V N, Nicolas Chauvet, linux-kernel, Andrew Goodbody,
David Miller, linux-arm-kernel
* David Rivshin (Allworx) <drivshin.allworx@gmail.com> [160427 18:13]:
> From: David Rivshin <drivshin@allworx.com>
>
> This series fixes a number of related issues around using phy-handle
> properties in cpsw emac nodes.
>
> Patch 1 fixes a bug if more than one slave is used, and either
> slave uses the phy-handle property in the devicetree.
>
> Patch 2 fixes a NULL pointer dereference which can occur if a
> phy-handle property is used and of_phy_connect() return NULL,
> such as with a bad devicetree.
>
> Patch 3 fixes an issue where the phy-mode property would be ignored
> if a phy-handle property was used. This also fixes a bogus error
> message that would be emitted.
>
> Patch 4 fixes makes the binding documentation more explicit that
> exactly one PHY property should be used, and also marks phy_id as
> deprecated.
>
> Patch 5 cleans up the fixed-link case to work like the now-fixed
> phy-handle case.
>
> I have tested on the following hardware configurations:
> - (EVMSK) dual emac, phy_id property in both slaves
> - (EVMSK) dual emac, phy-handle property in both slaves
> - (EVMSK) a bad phy-handle property pointing to &mmc1
> - (EVMSK) phy_id property with incorrect PHY address
> - (BeagleBoneBlack) single emac, phy_id property
> - (custom) single emac, fixed-link subnode
>
> Andrew Goodbody reported testing v2 on a board that doesn't use
> dual_emac mode, but with 2 PHYs using phy-handle properties [1].
>
> Nicolas Chauvet reported testing v2 on an HP t410 (dm8148).
>
> Markus Brunner reported testing v1 on the following [2]:
> - emac0 with phy_id and emac1 with fixed phy
> - emac0 with phy-handle and emac1 with fixed phy
> - emac0 with fixed phy and emac1 with fixed phy
Quickly boot tested these against next on dra62x-j5eco EVM:
Tested-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
` (5 preceding siblings ...)
2016-04-28 14:28 ` [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes Tony Lindgren
@ 2016-04-28 15:55 ` Grygorii Strashko
2016-04-28 21:27 ` David Miller
7 siblings, 0 replies; 11+ messages in thread
From: Grygorii Strashko @ 2016-04-28 15:55 UTC (permalink / raw)
To: David Rivshin (Allworx), netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Andrew Goodbody, Markus Brunner, Nicolas Chauvet
On 04/28/2016 04:10 AM, David Rivshin (Allworx) wrote:
> From: David Rivshin <drivshin@allworx.com>
>
> This series fixes a number of related issues around using phy-handle
> properties in cpsw emac nodes.
>
> Patch 1 fixes a bug if more than one slave is used, and either
> slave uses the phy-handle property in the devicetree.
>
> Patch 2 fixes a NULL pointer dereference which can occur if a
> phy-handle property is used and of_phy_connect() return NULL,
> such as with a bad devicetree.
>
> Patch 3 fixes an issue where the phy-mode property would be ignored
> if a phy-handle property was used. This also fixes a bogus error
> message that would be emitted.
>
> Patch 4 fixes makes the binding documentation more explicit that
> exactly one PHY property should be used, and also marks phy_id as
> deprecated.
>
> Patch 5 cleans up the fixed-link case to work like the now-fixed
> phy-handle case.
>
> I have tested on the following hardware configurations:
> - (EVMSK) dual emac, phy_id property in both slaves
> - (EVMSK) dual emac, phy-handle property in both slaves
> - (EVMSK) a bad phy-handle property pointing to &mmc1
> - (EVMSK) phy_id property with incorrect PHY address
> - (BeagleBoneBlack) single emac, phy_id property
> - (custom) single emac, fixed-link subnode
>
> Andrew Goodbody reported testing v2 on a board that doesn't use
> dual_emac mode, but with 2 PHYs using phy-handle properties [1].
>
> Nicolas Chauvet reported testing v2 on an HP t410 (dm8148).
>
> Markus Brunner reported testing v1 on the following [2]:
> - emac0 with phy_id and emac1 with fixed phy
> - emac0 with phy-handle and emac1 with fixed phy
> - emac0 with fixed phy and emac1 with fixed phy
>
> [1] https://lkml.org/lkml/2016/4/22/537
> [2] http://www.spinics.net/lists/netdev/msg357890.html
>
> David Rivshin (5):
> drivers: net: cpsw: fix parsing of phy-handle DT property in dual_emac
> config
> drivers: net: cpsw: fix segfault in case of bad phy-handle
> drivers: net: cpsw: don't ignore phy-mode if phy-handle is used
> dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive
> drivers: net: cpsw: use of_phy_connect() in fixed-link case
>
> Documentation/devicetree/bindings/net/cpsw.txt | 6 +--
> drivers/net/ethernet/ti/cpsw.c | 69 ++++++++++++++------------
> drivers/net/ethernet/ti/cpsw.h | 1 +
> 3 files changed, 41 insertions(+), 35 deletions(-)
>
Thanks a lot.
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes
2016-04-28 1:10 [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes David Rivshin (Allworx)
` (6 preceding siblings ...)
2016-04-28 15:55 ` Grygorii Strashko
@ 2016-04-28 21:27 ` David Miller
7 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2016-04-28 21:27 UTC (permalink / raw)
To: drivshin.allworx
Cc: netdev, linux-omap, linux-arm-kernel, devicetree, linux-kernel,
mugunthanvnm, grygorii.strashko, andrew.goodbody,
systemprogrammierung.brunner, kwizart
From: "David Rivshin (Allworx)" <drivshin.allworx@gmail.com>
Date: Wed, 27 Apr 2016 21:10:03 -0400
> This series fixes a number of related issues around using phy-handle
> properties in cpsw emac nodes.
Series applied, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread