linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] Revert "Prevent NUll pointer dereference with two PHYs"
@ 2016-04-20 14:42 Andrew Goodbody
  2016-04-20 14:42 ` [PATCH 1/1] Revert "Prevent NUll pointer dereference with two PHYs on cpsw" Andrew Goodbody
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Goodbody @ 2016-04-20 14:42 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-omap, mugunthanvnm, grygorii.strashko, tony,
	davem, Andrew Goodbody

Revert this patch as not only did it use an unitialised member of a struct
but there is also a pre-existing patch that does it better.

Andrew Goodbody (1):
  Revert "Prevent NUll pointer dereference with two PHYs on cpsw"

 drivers/net/ethernet/ti/cpsw.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

-- 
2.5.0

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

* [PATCH 1/1] Revert "Prevent NUll pointer dereference with two PHYs on cpsw"
  2016-04-20 14:42 [PATCH 0/1] Revert "Prevent NUll pointer dereference with two PHYs" Andrew Goodbody
@ 2016-04-20 14:42 ` Andrew Goodbody
  2016-04-20 15:05   ` Tony Lindgren
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Goodbody @ 2016-04-20 14:42 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-omap, mugunthanvnm, grygorii.strashko, tony,
	davem, Andrew Goodbody

This reverts commit cfe255600154f0072d4a8695590dbd194dfd1aeb

This can result in a "Unable to handle kernel paging request"
during boot. This was due to using an uninitialised struct member,
data->slaves.
---
 drivers/net/ethernet/ti/cpsw.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 2cd67a5..54bcc38 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -349,7 +349,6 @@ struct cpsw_slave {
 	struct cpsw_slave_data		*data;
 	struct phy_device		*phy;
 	struct net_device		*ndev;
-	struct device_node		*phy_node;
 	u32				port_vlan;
 	u32				open_stat;
 };
@@ -368,6 +367,7 @@ 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;
@@ -1142,8 +1142,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 		cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
 				   1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
 
-	if (slave->phy_node)
-		slave->phy = of_phy_connect(priv->ndev, slave->phy_node,
+	if (priv->phy_node)
+		slave->phy = of_phy_connect(priv->ndev, priv->phy_node,
 				 &cpsw_adjust_link, 0, slave->data->phy_if);
 	else
 		slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
@@ -2025,8 +2025,7 @@ static int cpsw_probe_dt(struct cpsw_priv *priv,
 		if (strcmp(slave_node->name, "slave"))
 			continue;
 
-		priv->slaves[i].phy_node =
-			of_parse_phandle(slave_node, "phy-handle", 0);
+		priv->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;
@@ -2267,22 +2266,12 @@ static int cpsw_probe(struct platform_device *pdev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(&pdev->dev);
 
-	data = &priv->data;
-	priv->slaves = devm_kzalloc(&pdev->dev,
-				    sizeof(struct cpsw_slave) * data->slaves,
-				    GFP_KERNEL);
-	if (!priv->slaves) {
-		ret = -ENOMEM;
-		goto clean_runtime_disable_ret;
-	}
-	for (i = 0; i < data->slaves; i++)
-		priv->slaves[i].slave_num = i;
-
 	if (cpsw_probe_dt(priv, 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)) {
 		memcpy(priv->mac_addr, data->slave_data[0].mac_addr, ETH_ALEN);
@@ -2294,6 +2283,16 @@ static int cpsw_probe(struct platform_device *pdev)
 
 	memcpy(ndev->dev_addr, priv->mac_addr, ETH_ALEN);
 
+	priv->slaves = devm_kzalloc(&pdev->dev,
+				    sizeof(struct cpsw_slave) * data->slaves,
+				    GFP_KERNEL);
+	if (!priv->slaves) {
+		ret = -ENOMEM;
+		goto clean_runtime_disable_ret;
+	}
+	for (i = 0; i < data->slaves; i++)
+		priv->slaves[i].slave_num = i;
+
 	priv->slaves[0].ndev = ndev;
 	priv->emac_port = 0;
 
-- 
2.5.0

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

* Re: [PATCH 1/1] Revert "Prevent NUll pointer dereference with two PHYs on cpsw"
  2016-04-20 14:42 ` [PATCH 1/1] Revert "Prevent NUll pointer dereference with two PHYs on cpsw" Andrew Goodbody
@ 2016-04-20 15:05   ` Tony Lindgren
  0 siblings, 0 replies; 3+ messages in thread
From: Tony Lindgren @ 2016-04-20 15:05 UTC (permalink / raw)
  To: Andrew Goodbody
  Cc: netdev, linux-kernel, linux-omap, mugunthanvnm, grygorii.strashko,
	davem

* Andrew Goodbody <andrew.goodbody@cambrionix.com> [160420 07:51]:
> This reverts commit cfe255600154f0072d4a8695590dbd194dfd1aeb
> 
> This can result in a "Unable to handle kernel paging request"
> during boot. This was due to using an uninitialised struct member,
> data->slaves.

Missing Signed-off-by?

This gets cpsw boards working in next for me again:

Tested-by: Tony Lindgren <tony@atomide.com>

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

end of thread, other threads:[~2016-04-20 15:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-20 14:42 [PATCH 0/1] Revert "Prevent NUll pointer dereference with two PHYs" Andrew Goodbody
2016-04-20 14:42 ` [PATCH 1/1] Revert "Prevent NUll pointer dereference with two PHYs on cpsw" Andrew Goodbody
2016-04-20 15:05   ` Tony Lindgren

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