Commit bd34cf66 authored by David S. Miller's avatar David S. Miller

Merge branch 'cpsw-phy-handle-fixes'

David Rivshin says:

====================
drivers: net: cpsw: phy-handle fixes

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
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents bbdd09eb 06cd6d6e
...@@ -45,13 +45,13 @@ Required properties: ...@@ -45,13 +45,13 @@ Required properties:
Optional properties: Optional properties:
- dual_emac_res_vlan : Specifies VID to be used to segregate the ports - dual_emac_res_vlan : Specifies VID to be used to segregate the ports
- mac-address : See ethernet.txt file in the same directory - 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 - phy-handle : See ethernet.txt file in the same directory
Slave sub-nodes: Slave sub-nodes:
- fixed-link : See fixed-link.txt file in the same directory - 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 Note: "ti,hwmods" field is used to fetch the base address and irq
resources from TI, omap hwmod data base during device registration. resources from TI, omap hwmod data base during device registration.
......
...@@ -367,7 +367,6 @@ struct cpsw_priv { ...@@ -367,7 +367,6 @@ struct cpsw_priv {
spinlock_t lock; spinlock_t lock;
struct platform_device *pdev; struct platform_device *pdev;
struct net_device *ndev; struct net_device *ndev;
struct device_node *phy_node;
struct napi_struct napi_rx; struct napi_struct napi_rx;
struct napi_struct napi_tx; struct napi_struct napi_tx;
struct device *dev; struct device *dev;
...@@ -1148,25 +1147,34 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv) ...@@ -1148,25 +1147,34 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast, cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
1 << slave_port, 0, 0, ALE_MCAST_FWD_2); 1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
if (priv->phy_node) if (slave->data->phy_node) {
slave->phy = of_phy_connect(priv->ndev, priv->phy_node, slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
&cpsw_adjust_link, 0, slave->data->phy_if); &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, slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
&cpsw_adjust_link, slave->data->phy_if); &cpsw_adjust_link, slave->data->phy_if);
if (IS_ERR(slave->phy)) { if (IS_ERR(slave->phy)) {
dev_err(priv->dev, "phy %s not found on slave %d\n", dev_err(priv->dev,
slave->data->phy_id, slave->slave_num); "phy \"%s\" not found on slave %d, err %ld\n",
slave->data->phy_id, slave->slave_num,
PTR_ERR(slave->phy));
slave->phy = NULL; slave->phy = NULL;
} else { return;
}
}
phy_attached_info(slave->phy); phy_attached_info(slave->phy);
phy_start(slave->phy); phy_start(slave->phy);
/* Configure GMII_SEL register */ /* Configure GMII_SEL register */
cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface, cpsw_phy_sel(&priv->pdev->dev, slave->phy->interface, slave->slave_num);
slave->slave_num);
}
} }
static inline void cpsw_add_default_vlan(struct cpsw_priv *priv) static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
...@@ -1940,12 +1948,11 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv, ...@@ -1940,12 +1948,11 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
slave->port_vlan = data->dual_emac_res_vlan; 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 platform_device *pdev)
{ {
struct device_node *node = pdev->dev.of_node; struct device_node *node = pdev->dev.of_node;
struct device_node *slave_node; struct device_node *slave_node;
struct cpsw_platform_data *data = &priv->data;
int i = 0, ret; int i = 0, ret;
u32 prop; u32 prop;
...@@ -2033,25 +2040,21 @@ static int cpsw_probe_dt(struct cpsw_priv *priv, ...@@ -2033,25 +2040,21 @@ static int cpsw_probe_dt(struct cpsw_priv *priv,
if (strcmp(slave_node->name, "slave")) if (strcmp(slave_node->name, "slave"))
continue; 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); parp = of_get_property(slave_node, "phy_id", &lenp);
if (of_phy_is_fixed_link(slave_node)) { if (slave_data->phy_node) {
struct device_node *phy_node; dev_dbg(&pdev->dev,
struct phy_device *phy_dev; "slave[%d] using phy-handle=\"%s\"\n",
i, slave_data->phy_node->full_name);
} else if (of_phy_is_fixed_link(slave_node)) {
/* In the case of a fixed PHY, the DT node associated /* In the case of a fixed PHY, the DT node associated
* to the PHY is the Ethernet MAC DT node. * to the PHY is the Ethernet MAC DT node.
*/ */
ret = of_phy_register_fixed_link(slave_node); ret = of_phy_register_fixed_link(slave_node);
if (ret) if (ret)
return ret; return ret;
phy_node = of_node_get(slave_node); slave_data->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);
} else if (parp) { } else if (parp) {
u32 phyid; u32 phyid;
struct device_node *mdio_node; struct device_node *mdio_node;
...@@ -2072,7 +2075,9 @@ static int cpsw_probe_dt(struct cpsw_priv *priv, ...@@ -2072,7 +2075,9 @@ static int cpsw_probe_dt(struct cpsw_priv *priv,
snprintf(slave_data->phy_id, sizeof(slave_data->phy_id), snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
PHY_ID_FMT, mdio->name, phyid); PHY_ID_FMT, mdio->name, phyid);
} else { } 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; goto no_phy_slave;
} }
slave_data->phy_if = of_get_phy_mode(slave_node); slave_data->phy_if = of_get_phy_mode(slave_node);
...@@ -2275,7 +2280,7 @@ static int cpsw_probe(struct platform_device *pdev) ...@@ -2275,7 +2280,7 @@ static int cpsw_probe(struct platform_device *pdev)
/* Select default pin state */ /* Select default pin state */
pinctrl_pm_select_default_state(&pdev->dev); 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"); dev_err(&pdev->dev, "cpsw: platform data missing\n");
ret = -ENODEV; ret = -ENODEV;
goto clean_runtime_disable_ret; goto clean_runtime_disable_ret;
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include <linux/phy.h> #include <linux/phy.h>
struct cpsw_slave_data { struct cpsw_slave_data {
struct device_node *phy_node;
char phy_id[MII_BUS_ID_SIZE]; char phy_id[MII_BUS_ID_SIZE];
int phy_if; int phy_if;
u8 mac_addr[ETH_ALEN]; u8 mac_addr[ETH_ALEN];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment