[U-Boot] [PATCH 0/6] net: ti: net: to: set of fixes and improvements

Hi All,
This series introduces set of fixes and improvements for TI CPSW and AM654x CPSW networking drivers.
Patch 1 - Enables support of 10Mbit link speeds for TI CPSW driver. Patch 3 - Adds support of standard Ethernet "max-speed" DT property. Patches 4-5 - fix mac tx internal delay for rgmii-rxid mode Patches 2,6 - are code improvements
Grygorii Strashko (6): net: ti: cpsw: enable 10Mbps link speed support in rgmii mode net: ti: cpsw: move parsing of dt port's parameters in separate func net: ti: cpsw: add support for standard eth "max-speed" dt property net: ti: cpsw: fix mac tx internal delay for rgmii-rxid mode net: ti: am65x-cpsw: fix mac tx internal delay for rgmii-rxid mode net: ti: cpsw: convert to use dev/ofnode api
drivers/net/ti/am65-cpsw-nuss.c | 2 +- drivers/net/ti/cpsw.c | 154 +++++++++++++++++--------------- include/cpsw.h | 3 +- 3 files changed, 84 insertions(+), 75 deletions(-)

According to TRMs the 10Mbps link speed is supported in RGMII only when CPSW2G MAC SL is configured for External Control ("in band") mode CPSW_SL_MACCTRL.EXT_EN(18) = 1.
Hence update cpsw_slave_update_link() to follow documentation.
[1] https://patchwork.kernel.org/patch/10285239/ Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/ti/cpsw.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/ti/cpsw.c b/drivers/net/ti/cpsw.c index 20ddb44dd8..f0d008f0f5 100644 --- a/drivers/net/ti/cpsw.c +++ b/drivers/net/ti/cpsw.c @@ -33,6 +33,7 @@ DECLARE_GLOBAL_DATA_PTR; #define GIGABITEN BIT(7) #define FULLDUPLEXEN BIT(0) #define MIIEN BIT(15) +#define CTL_EXT_EN BIT(18) /* DMA Registers */ #define CPDMA_TXCONTROL 0x004 #define CPDMA_RXCONTROL 0x014 @@ -489,6 +490,8 @@ static int cpsw_slave_update_link(struct cpsw_slave *slave, mac_control |= FULLDUPLEXEN; if (phy->speed == 100) mac_control |= MIIEN; + if (phy->speed == 10 && phy_interface_is_rgmii(phy)) + mac_control |= CTL_EXT_EN; }
if (mac_control == slave->mac_control)

Move parsing of dt port's parameters in separate func for better code readability.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/ti/cpsw.c | 57 +++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ti/cpsw.c b/drivers/net/ti/cpsw.c index f0d008f0f5..533c167995 100644 --- a/drivers/net/ti/cpsw.c +++ b/drivers/net/ti/cpsw.c @@ -1179,12 +1179,40 @@ static int cpsw_eth_probe(struct udevice *dev) }
#if CONFIG_IS_ENABLED(OF_CONTROL) +static void cpsw_eth_of_parse_slave(struct cpsw_platform_data *data, + int slave_index, int subnode) +{ + struct cpsw_slave_data *slave_data; + const void *fdt = gd->fdt_blob; + const char *phy_mode; + u32 phy_id[2]; + + slave_data = &data->slave_data[slave_index]; + + phy_mode = fdt_getprop(fdt, subnode, "phy-mode", NULL); + if (phy_mode) + slave_data->phy_if = + phy_get_interface_by_name(phy_mode); + + slave_data->phy_of_handle = fdtdec_lookup_phandle(fdt, subnode, + "phy-handle"); + + if (data->slave_data[slave_index].phy_of_handle >= 0) { + slave_data->phy_addr = + fdtdec_get_int(fdt, slave_data->phy_of_handle, + "reg", -1); + } else { + fdtdec_get_int_array(fdt, subnode, "phy_id", + phy_id, 2); + slave_data->phy_addr = phy_id[1]; + } +} + static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) { struct eth_pdata *pdata = dev_get_platdata(dev); struct cpsw_platform_data *data; struct gpio_desc *mode_gpios; - const char *phy_mode; const void *fdt = gd->fdt_blob; int node = dev_of_offset(dev); int subnode; @@ -1267,30 +1295,10 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) }
if (!strncmp(name, "slave", 5)) { - u32 phy_id[2]; - if (slave_index >= data->slaves) continue; - phy_mode = fdt_getprop(fdt, subnode, "phy-mode", NULL); - if (phy_mode) - data->slave_data[slave_index].phy_if = - phy_get_interface_by_name(phy_mode); - - data->slave_data[slave_index].phy_of_handle = - fdtdec_lookup_phandle(fdt, subnode, - "phy-handle"); - - if (data->slave_data[slave_index].phy_of_handle >= 0) { - data->slave_data[slave_index].phy_addr = - fdtdec_get_int(gd->fdt_blob, - data->slave_data[slave_index].phy_of_handle, - "reg", -1); - } else { - fdtdec_get_int_array(fdt, subnode, "phy_id", - phy_id, 2); - data->slave_data[slave_index].phy_addr = - phy_id[1]; - } + + cpsw_eth_of_parse_slave(data, slave_index, subnode); slave_index++; }
@@ -1331,7 +1339,8 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev)
pdata->phy_interface = data->slave_data[active_slave].phy_if; if (pdata->phy_interface == -1) { - debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode); + debug("%s: Invalid PHY interface '%s'\n", __func__, + phy_string_for_interface(pdata->phy_interface)); return -EINVAL; }

This patch adds support for standard Ethernet "max-speed" DT property to allow PHY link speed limitation.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/ti/cpsw.c | 14 ++++++++++++++ include/cpsw.h | 1 + 2 files changed, 15 insertions(+)
diff --git a/drivers/net/ti/cpsw.c b/drivers/net/ti/cpsw.c index 533c167995..af4db89341 100644 --- a/drivers/net/ti/cpsw.c +++ b/drivers/net/ti/cpsw.c @@ -839,6 +839,7 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave) { struct phy_device *phydev; u32 supported = PHY_GBIT_FEATURES; + int ret;
phydev = phy_connect(priv->bus, slave->data->phy_addr, @@ -849,6 +850,13 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave) return -1;
phydev->supported &= supported; + if (slave->data->max_speed) { + ret = phy_set_supported(phydev, slave->data->max_speed); + if (ret) + return ret; + dev_dbg(priv->dev, "Port %u speed forced to %uMbit\n", + slave->slave_num + 1, slave->data->max_speed); + } phydev->advertising = phydev->supported;
#ifdef CONFIG_DM_ETH @@ -1185,6 +1193,7 @@ static void cpsw_eth_of_parse_slave(struct cpsw_platform_data *data, struct cpsw_slave_data *slave_data; const void *fdt = gd->fdt_blob; const char *phy_mode; + int max_speed = -1; u32 phy_id[2];
slave_data = &data->slave_data[slave_index]; @@ -1206,6 +1215,11 @@ static void cpsw_eth_of_parse_slave(struct cpsw_platform_data *data, phy_id, 2); slave_data->phy_addr = phy_id[1]; } + + max_speed = fdtdec_get_int(fdt, subnode, + "max-speed", max_speed); + if (max_speed > 0) + slave_data->max_speed = max_speed; }
static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) diff --git a/include/cpsw.h b/include/cpsw.h index 96ff254f98..c7532fc866 100644 --- a/include/cpsw.h +++ b/include/cpsw.h @@ -39,6 +39,7 @@ struct cpsw_slave_data { int phy_addr; int phy_if; int phy_of_handle; + int max_speed; };
enum {

On 09/09/2019 18:00, Grygorii Strashko wrote:
This patch adds support for standard Ethernet "max-speed" DT property to allow PHY link speed limitation.
This patch by itself will break network boot (see below). But with whole series issue will not be reproducible, because it's implicitly fixed by patch 6.
I'll send v2.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
drivers/net/ti/cpsw.c | 14 ++++++++++++++ include/cpsw.h | 1 + 2 files changed, 15 insertions(+)
diff --git a/drivers/net/ti/cpsw.c b/drivers/net/ti/cpsw.c index 533c167995..af4db89341 100644 --- a/drivers/net/ti/cpsw.c +++ b/drivers/net/ti/cpsw.c @@ -839,6 +839,7 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave) { struct phy_device *phydev; u32 supported = PHY_GBIT_FEATURES;
int ret;
phydev = phy_connect(priv->bus, slave->data->phy_addr,
@@ -849,6 +850,13 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave) return -1;
phydev->supported &= supported;
if (slave->data->max_speed) {
ret = phy_set_supported(phydev, slave->data->max_speed);
if (ret)
return ret;
dev_dbg(priv->dev, "Port %u speed forced to %uMbit\n",
slave->slave_num + 1, slave->data->max_speed);
} phydev->advertising = phydev->supported;
#ifdef CONFIG_DM_ETH
@@ -1185,6 +1193,7 @@ static void cpsw_eth_of_parse_slave(struct cpsw_platform_data *data, struct cpsw_slave_data *slave_data; const void *fdt = gd->fdt_blob; const char *phy_mode;
int max_speed = -1; u32 phy_id[2];
slave_data = &data->slave_data[slave_index];
@@ -1206,6 +1215,11 @@ static void cpsw_eth_of_parse_slave(struct cpsw_platform_data *data, phy_id, 2); slave_data->phy_addr = phy_id[1]; }
slave_data->max_speed has to be reset to 0 here.
- max_speed = fdtdec_get_int(fdt, subnode,
"max-speed", max_speed);
- if (max_speed > 0)
}slave_data->max_speed = max_speed;

Now TI CPSW driver will disable MAC TX internal delay for PHY interface mode "rgmii-rxid" which is incorrect.
Hence, fix it by keeping default value (enabled) for MAC TX internal delay when "rgmii-rxid" interface mode is selected.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/ti/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ti/cpsw.c b/drivers/net/ti/cpsw.c index af4db89341..55edff3b8d 100644 --- a/drivers/net/ti/cpsw.c +++ b/drivers/net/ti/cpsw.c @@ -1072,10 +1072,10 @@ static void cpsw_gmii_sel_am3352(struct cpsw_priv *priv, break;
case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_RXID: mode = AM33XX_GMII_SEL_MODE_RGMII; break; case PHY_INTERFACE_MODE_RGMII_ID: - case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: mode = AM33XX_GMII_SEL_MODE_RGMII; rgmii_id = true;

Now AM65x CPSW2G driver will disable MAC TX internal delay for PHY interface mode "rgmii-rxid" which is incorrect. Hence, fix it by keeping default value (enabled) for MAC TX internal delay when "rgmii-rxid" interface mode is selected.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/ti/am65-cpsw-nuss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index e11fbdeed3..06b0663950 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -234,11 +234,11 @@ static void am65_cpsw_gmii_sel_k3(struct am65_cpsw_priv *priv, break;
case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_RXID: mode = AM65_GMII_SEL_MODE_RGMII; break;
case PHY_INTERFACE_MODE_RGMII_ID: - case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: mode = AM65_GMII_SEL_MODE_RGMII; rgmii_id = true;

Conver TI CPSW driver to use dev/ofnode api.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/ti/cpsw.c | 118 ++++++++++++++++++------------------------ include/cpsw.h | 2 +- 2 files changed, 51 insertions(+), 69 deletions(-)
diff --git a/drivers/net/ti/cpsw.c b/drivers/net/ti/cpsw.c index 55edff3b8d..4a990be93e 100644 --- a/drivers/net/ti/cpsw.c +++ b/drivers/net/ti/cpsw.c @@ -19,12 +19,9 @@ #include <phy.h> #include <asm/arch/cpu.h> #include <dm.h> -#include <fdt_support.h>
#include "cpsw_mdio.h"
-DECLARE_GLOBAL_DATA_PTR; - #define BITMASK(bits) (BIT(bits) - 1) #define NUM_DESCS (PKTBUFSRX * 2) #define PKT_MIN 60 @@ -860,8 +857,8 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave) phydev->advertising = phydev->supported;
#ifdef CONFIG_DM_ETH - if (slave->data->phy_of_handle) - phydev->node = offset_to_ofnode(slave->data->phy_of_handle); + if (ofnode_valid(slave->data->phy_of_handle)) + phydev->node = slave->data->phy_of_handle; #endif
priv->phydev = phydev; @@ -1049,12 +1046,6 @@ static const struct eth_ops cpsw_eth_ops = { .stop = cpsw_eth_stop, };
-static inline fdt_addr_t cpsw_get_addr_by_node(const void *fdt, int node) -{ - return fdtdec_get_addr_size_auto_noparent(fdt, node, "reg", 0, NULL, - false); -} - static void cpsw_gmii_sel_am3352(struct cpsw_priv *priv, phy_interface_t phy_mode) { @@ -1188,38 +1179,37 @@ static int cpsw_eth_probe(struct udevice *dev)
#if CONFIG_IS_ENABLED(OF_CONTROL) static void cpsw_eth_of_parse_slave(struct cpsw_platform_data *data, - int slave_index, int subnode) + int slave_index, ofnode subnode) { - struct cpsw_slave_data *slave_data; - const void *fdt = gd->fdt_blob; + struct ofnode_phandle_args out_args; + struct cpsw_slave_data *slave_data; const char *phy_mode; - int max_speed = -1; u32 phy_id[2]; + int ret;
slave_data = &data->slave_data[slave_index];
- phy_mode = fdt_getprop(fdt, subnode, "phy-mode", NULL); + phy_mode = ofnode_read_string(subnode, "phy-mode"); if (phy_mode) - slave_data->phy_if = - phy_get_interface_by_name(phy_mode); + slave_data->phy_if = phy_get_interface_by_name(phy_mode);
- slave_data->phy_of_handle = fdtdec_lookup_phandle(fdt, subnode, - "phy-handle"); + ret = ofnode_parse_phandle_with_args(subnode, "phy-handle", + NULL, 0, 0, &out_args); + if (!ret) { + slave_data->phy_of_handle = out_args.node;
- if (data->slave_data[slave_index].phy_of_handle >= 0) { - slave_data->phy_addr = - fdtdec_get_int(fdt, slave_data->phy_of_handle, - "reg", -1); + ret = ofnode_read_s32(slave_data->phy_of_handle, "reg", + &slave_data->phy_addr); + if (ret) + printf("error: phy addr not found in dt\n"); } else { - fdtdec_get_int_array(fdt, subnode, "phy_id", - phy_id, 2); - slave_data->phy_addr = phy_id[1]; + ret = ofnode_read_u32_array(subnode, "phy_id", phy_id, 2); + if (ret) + printf("error: phy_id read failed\n"); }
- max_speed = fdtdec_get_int(fdt, subnode, - "max-speed", max_speed); - if (max_speed > 0) - slave_data->max_speed = max_speed; + slave_data->max_speed = ofnode_read_s32_default(subnode, + "max-speed", 0); }
static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) @@ -1227,17 +1217,14 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) struct eth_pdata *pdata = dev_get_platdata(dev); struct cpsw_platform_data *data; struct gpio_desc *mode_gpios; - const void *fdt = gd->fdt_blob; - int node = dev_of_offset(dev); - int subnode; int slave_index = 0; - int active_slave; int num_mode_gpios; + ofnode subnode; int ret;
data = calloc(1, sizeof(struct cpsw_platform_data)); pdata->priv_pdata = data; - pdata->iobase = devfdt_get_addr(dev); + pdata->iobase = dev_read_addr(dev); data->version = CPSW_CTRL_VERSION_2; data->bd_ram_ofs = CPSW_BD_OFFSET; data->ale_reg_ofs = CPSW_ALE_OFFSET; @@ -1248,36 +1235,37 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) pdata->phy_interface = -1;
data->cpsw_base = pdata->iobase; - data->channels = fdtdec_get_int(fdt, node, "cpdma_channels", -1); - if (data->channels <= 0) { + + ret = dev_read_s32(dev, "cpdma_channels", &data->channels); + if (ret) { printf("error: cpdma_channels not found in dt\n"); - return -ENOENT; + return ret; }
- data->slaves = fdtdec_get_int(fdt, node, "slaves", -1); - if (data->slaves <= 0) { + ret = dev_read_s32(dev, "slaves", &data->slaves); + if (ret) { printf("error: slaves not found in dt\n"); - return -ENOENT; + return ret; } data->slave_data = malloc(sizeof(struct cpsw_slave_data) * data->slaves);
- data->ale_entries = fdtdec_get_int(fdt, node, "ale_entries", -1); - if (data->ale_entries <= 0) { + ret = dev_read_s32(dev, "ale_entries", &data->ale_entries); + if (ret) { printf("error: ale_entries not found in dt\n"); - return -ENOENT; + return ret; }
- data->bd_ram_ofs = fdtdec_get_int(fdt, node, "bd_ram_size", -1); - if (data->bd_ram_ofs <= 0) { + ret = dev_read_u32(dev, "bd_ram_size", &data->bd_ram_ofs); + if (ret) { printf("error: bd_ram_size not found in dt\n"); - return -ENOENT; + return ret; }
- data->mac_control = fdtdec_get_int(fdt, node, "mac_control", -1); - if (data->mac_control <= 0) { + ret = dev_read_u32(dev, "mac_control", &data->mac_control); + if (ret) { printf("error: ale_entries not found in dt\n"); - return -ENOENT; + return ret; }
num_mode_gpios = gpio_get_list_count(dev, "mode-gpios"); @@ -1289,23 +1277,18 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) free(mode_gpios); }
- active_slave = fdtdec_get_int(fdt, node, "active_slave", 0); - data->active_slave = active_slave; + data->active_slave = dev_read_u32_default(dev, "active_slave", 0);
- fdt_for_each_subnode(subnode, fdt, node) { - int len; + ofnode_for_each_subnode(subnode, dev_ofnode(dev)) { const char *name;
- name = fdt_get_name(fdt, subnode, &len); + name = ofnode_get_name(subnode); if (!strncmp(name, "mdio", 4)) { - u32 mdio_base; - - mdio_base = cpsw_get_addr_by_node(fdt, subnode); - if (mdio_base == FDT_ADDR_T_NONE) { + data->mdio_base = ofnode_get_addr(subnode); + if (data->mdio_base == FDT_ADDR_T_NONE) { pr_err("Not able to get MDIO address space\n"); return -ENOENT; } - data->mdio_base = mdio_base; }
if (!strncmp(name, "slave", 5)) { @@ -1317,19 +1300,18 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) }
if (!strncmp(name, "cpsw-phy-sel", 12)) { - data->gmii_sel = cpsw_get_addr_by_node(fdt, subnode); + data->gmii_sel = ofnode_get_addr(subnode);
if (data->gmii_sel == FDT_ADDR_T_NONE) { pr_err("Not able to get gmii_sel reg address\n"); return -ENOENT; }
- if (fdt_get_property(fdt, subnode, "rmii-clock-ext", - NULL)) + if (ofnode_read_bool(subnode, "rmii-clock-ext")) data->rmii_clock_external = true;
- data->phy_sel_compat = fdt_getprop(fdt, subnode, - "compatible", NULL); + data->phy_sel_compat = ofnode_read_string(subnode, + "compatible"); if (!data->phy_sel_compat) { pr_err("Not able to get gmii_sel compatible\n"); return -ENOENT; @@ -1345,13 +1327,13 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) data->slave_data[1].sliver_reg_ofs = CPSW_SLIVER1_OFFSET; }
- ret = ti_cm_get_macid_addr(dev, active_slave, data); + ret = ti_cm_get_macid_addr(dev, data->active_slave, data); if (ret < 0) { pr_err("cpsw read efuse mac failed\n"); return ret; }
- pdata->phy_interface = data->slave_data[active_slave].phy_if; + pdata->phy_interface = data->slave_data[data->active_slave].phy_if; if (pdata->phy_interface == -1) { debug("%s: Invalid PHY interface '%s'\n", __func__, phy_string_for_interface(pdata->phy_interface)); diff --git a/include/cpsw.h b/include/cpsw.h index c7532fc866..82f90b0184 100644 --- a/include/cpsw.h +++ b/include/cpsw.h @@ -38,7 +38,7 @@ struct cpsw_slave_data { u32 sliver_reg_ofs; int phy_addr; int phy_if; - int phy_of_handle; + ofnode phy_of_handle; int max_speed; };
participants (1)
-
Grygorii Strashko