[PATCH 0/8] Add dwc_eth_qos support for rockchip

older
[PATCH V2 0/7] imx: drivers: ddr:...

David Wu

30 Apr 2020 30 Apr '20

12:36 p.m.

Rockchip Socs can support two controllers "snps, dwmac-4.20a" and "snps, dwmac-3.50". In order to support two at gmac-rockchip.c, export public interface functions and struct data, it will be more general for others.

David Wu (8): net: dwc_eth_qos: Use dev_ functions calls to get FDT data net: dwc_eth_qos: Fix the software reset net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32 net: dwc_eth_qos: Move interface() to eqos_ops struct net: dwc_eth_qos: Make clk_rx and clk_tx optional net: dwc_eth_qos: Split eqos_start() to get link speed net: dwc_eth_qos: Export common struct and interface at head file net: gmac_rockchip: Add dwc_eth_qos support

drivers/net/Kconfig | 2 +- drivers/net/dwc_eth_qos.c | 264 +++++++++++++++++------------------- drivers/net/gmac_rockchip.c | 160 ++++++++++++++++++---- 3 files changed, 263 insertions(+), 163 deletions(-)

-- 2.19.1

Show replies by date

David Wu

30 Apr 30 Apr

12:36 p.m.

New subject: [PATCH 1/8] net: dwc_eth_qos: Use dev_ functions calls to get FDT data

It seems dev_ functions are more general than fdt_ functions.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 63f2086dec..a72132cacf 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1728,8 +1728,7 @@ static phy_interface_t eqos_get_interface_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

- phy_mode = fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "phy-mode", - NULL); + phy_mode = dev_read_string(dev, "phy-mode"); if (phy_mode) interface = phy_get_interface_by_name(phy_mode);

@@ -1788,9 +1787,9 @@ static int eqos_probe(struct udevice *dev) eqos->dev = dev; eqos->config = (void *)dev_get_driver_data(dev);

- eqos->regs = devfdt_get_addr(dev); + eqos->regs = dev_read_addr(dev); if (eqos->regs == FDT_ADDR_T_NONE) { - pr_err("devfdt_get_addr() failed"); + pr_err("dev_read_addr() failed"); return -ENODEV; } eqos->mac_regs = (void *)(eqos->regs + EQOS_MAC_REGS_BASE);

-- 2.19.1

David Wu

12:36 p.m.

New subject: [PATCH 2/8] net: dwc_eth_qos: Fix the software reset

When using rgmii Gigabit mode, the wait_for_bit_le32() reset method resulting in RX can not receive data, after this patch, works well.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index a72132cacf..16988f6bdc 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1034,7 +1034,7 @@ static int eqos_write_hwaddr(struct udevice *dev) static int eqos_start(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); - int ret, i; + int ret, i, limit; ulong rate; u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; ulong last_rx_desc; @@ -1060,12 +1060,21 @@ static int eqos_start(struct udevice *dev)

eqos->reg_access_ok = true;

- ret = wait_for_bit_le32(&eqos->dma_regs->mode, - EQOS_DMA_MODE_SWR, false, - eqos->config->swr_wait, false); - if (ret) { + /* DMA SW reset */ + val = readl(&eqos->dma_regs->mode); + val |= EQOS_DMA_MODE_SWR; + writel(val, &eqos->dma_regs->mode); + limit = eqos->config->swr_wait / 10; + while (limit--) { + if (!(readl(&eqos->dma_regs->mode) & EQOS_DMA_MODE_SWR)) + break; + mdelay(10000); + } + + if (limit < 0) { pr_err("EQOS_DMA_MODE_SWR stuck"); - goto err_stop_resets; + goto err_stop_clks; + return -ETIMEDOUT; }

ret = eqos->config->ops->eqos_calibrate_pads(dev);

-- 2.19.1

Patrice CHOTARD

5:28 p.m.

New subject: [PATCH 2/8] net: dwc_eth_qos: Fix the software reset

Hi David

On 4/30/20 12:36 PM, David Wu wrote:

...

When using rgmii Gigabit mode, the wait_for_bit_le32() reset method resulting in RX can not receive data, after this patch, works well.

Signed-off-by: David Wu david.wu@rock-chips.com

drivers/net/dwc_eth_qos.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index a72132cacf..16988f6bdc 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1034,7 +1034,7 @@ static int eqos_write_hwaddr(struct udevice *dev) static int eqos_start(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev);

int ret, i;

int ret, i, limit; ulong rate; u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; ulong last_rx_desc;

@@ -1060,12 +1060,21 @@ static int eqos_start(struct udevice *dev)

eqos->reg_access_ok = true;
ret = wait_for_bit_le32(&eqos->dma_regs->mode,
		EQOS_DMA_MODE_SWR, false,
		eqos->config->swr_wait, false);
if (ret) {
/* DMA SW reset */

val = readl(&eqos->dma_regs->mode);

val |= EQOS_DMA_MODE_SWR;

writel(val, &eqos->dma_regs->mode);

limit = eqos->config->swr_wait / 10;

while (limit--) {
if (!(readl(&eqos->dma_regs->mode) & EQOS_DMA_MODE_SWR))
	break;
mdelay(10000);
}

usage of wait_for_bit_le32() must still work, if the timeout is no enough in you case,

create a dedicated rockchip struct eqos_config

...

if (limit < 0) { pr_err("EQOS_DMA_MODE_SWR stuck");
goto err_stop_resets;

why are you updating the error path here ?

...

```
goto err_stop_clks;
```
```
return -ETIMEDOUT;
```

the return after the goto is useless

...

}

ret = eqos->config->ops->eqos_calibrate_pads(dev);

Stephen Warren

1 May 1 May

12:25 a.m.

New subject: [PATCH 2/8] net: dwc_eth_qos: Fix the software reset

On 4/30/20 4:36 AM, David Wu wrote:

...

When using rgmii Gigabit mode, the wait_for_bit_le32() reset method resulting in RX can not receive data, after this patch, works well.

...

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c

...

limit = eqos->config->swr_wait / 10;

while (limit--) {
if (!(readl(&eqos->dma_regs->mode) & EQOS_DMA_MODE_SWR))
	break;
mdelay(10000);
}

mdelay()'s parameter is in milliseconds judging by its implementation in include/linux/delay.h. So, this delays 10 seconds in each loop iteration. That can't possibly be right.

David Wu

30 Apr 30 Apr

12:36 p.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32

It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 40 ++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 16988f6bdc..06a8d924a7 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -298,6 +298,7 @@ struct eqos_priv { struct eqos_tegra186_regs *tegra186_regs; struct reset_ctl reset_ctl; struct gpio_desc phy_reset_gpio; + u32 reset_delays[3]; struct clk clk_master_bus; struct clk clk_rx; struct clk clk_ptp_ref; @@ -701,6 +702,15 @@ static int eqos_start_resets_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev); if (dm_gpio_is_valid(&eqos->phy_reset_gpio)) { + ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0); + if (ret < 0) { + pr_err("dm_gpio_set_value(phy_reset, deassert) failed: %d", + ret); + return ret; + } + + udelay(eqos->reset_delays[0]); + ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 1); if (ret < 0) { pr_err("dm_gpio_set_value(phy_reset, assert) failed: %d", @@ -708,7 +718,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) return ret; }

- udelay(2); + udelay(eqos->reset_delays[1]);

ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0); if (ret < 0) { @@ -716,6 +726,8 @@ static int eqos_start_resets_stm32(struct udevice *dev) ret); return ret; } + + udelay(eqos->reset_delays[2]); } debug("%s: OK\n", __func__);

@@ -1065,16 +1077,16 @@ static int eqos_start(struct udevice *dev) val |= EQOS_DMA_MODE_SWR; writel(val, &eqos->dma_regs->mode); limit = eqos->config->swr_wait / 10; - while (limit--) { + do { if (!(readl(&eqos->dma_regs->mode) & EQOS_DMA_MODE_SWR)) break; mdelay(10000); - } + } while (limit--);

if (limit < 0) { pr_err("EQOS_DMA_MODE_SWR stuck"); - goto err_stop_clks; - return -ETIMEDOUT; + ret = -ETIMEDOUT; + goto err_stop_resets; }

ret = eqos->config->ops->eqos_calibrate_pads(dev); @@ -1712,11 +1724,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret); + else + eqos->reset_delays[1] = 2;

eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, "reg", -1); }

+ if (!dm_gpio_is_valid(&eqos->phy_reset_gpio)) { + int reset_flags = GPIOD_IS_OUT; + + if (dev_read_bool(dev, "snps,reset-active-low")) + reset_flags |= GPIOD_ACTIVE_LOW; + + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, + &eqos->phy_reset_gpio, reset_flags); + if (ret == 0) + ret = dev_read_u32_array(dev, "snps,reset-delays-us", + eqos->reset_delays, 3); + else + pr_warn("gpio_request_by_name(snps,reset-gpio) failed: %d", + ret); + } + debug("%s: OK\n", __func__); return 0;

-- 2.19.1

Patrice CHOTARD

5:47 p.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32

Hi David

On 4/30/20 12:36 PM, David Wu wrote:

...

It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general.

Signed-off-by: David Wu david.wu@rock-chips.com

drivers/net/dwc_eth_qos.c | 40 ++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 16988f6bdc..06a8d924a7 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -298,6 +298,7 @@ struct eqos_priv { struct eqos_tegra186_regs *tegra186_regs; struct reset_ctl reset_ctl; struct gpio_desc phy_reset_gpio;

u32 reset_delays[3]; struct clk clk_master_bus; struct clk clk_rx; struct clk clk_ptp_ref;

@@ -701,6 +702,15 @@ static int eqos_start_resets_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev); if (dm_gpio_is_valid(&eqos->phy_reset_gpio)) {
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0);
if (ret < 0) {
	pr_err("dm_gpio_set_value(phy_reset, deassert) failed: %d",
	       ret);
	return ret;
}
udelay(eqos->reset_delays[0]);

not related to this patch subject

...

ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 1);
if (ret < 0) {
	pr_err("dm_gpio_set_value(phy_reset, assert) failed: %d",
@@ -708,7 +718,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) return ret; }
udelay(2);
udelay(eqos->reset_delays[1]);
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0); if (ret < 0) {
@@ -716,6 +726,8 @@ static int eqos_start_resets_stm32(struct udevice *dev) ret); return ret; }
udelay(eqos->reset_delays[2]);
} debug("%s: OK\n", __func__);
@@ -1065,16 +1077,16 @@ static int eqos_start(struct udevice *dev) val |= EQOS_DMA_MODE_SWR; writel(val, &eqos->dma_regs->mode); limit = eqos->config->swr_wait / 10;

while (limit--) {

do { if (!(readl(&eqos->dma_regs->mode) & EQOS_DMA_MODE_SWR)) break; mdelay(10000);

}

} while (limit--);

why are you updating this ? it's not related to the purpose of this patch

...

if (limit < 0) { pr_err("EQOS_DMA_MODE_SWR stuck");
goto err_stop_clks;
return -ETIMEDOUT;
ret = -ETIMEDOUT;
goto err_stop_resets;

ditto

...

}

ret = eqos->config->ops->eqos_calibrate_pads(dev); @@ -1712,11 +1724,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret);
else
	eqos->reset_delays[1] = 2;

this is not the correct place to set default value. It must be set in case we can't get value from DT below

...

eqos->phyaddr = ofnode_read_u32_default(phandle_args.node,
					"reg", -1);

}

if (!dm_gpio_is_valid(&eqos->phy_reset_gpio)) {
```
int reset_flags = GPIOD_IS_OUT;
```

if (dev_read_bool(dev, "snps,reset-active-low"))

```
	reset_flags |= GPIOD_ACTIVE_LOW;
```

ret = gpio_request_by_name(dev, "snps,reset-gpio", 0,

			   &eqos->phy_reset_gpio, reset_flags);

```
if (ret == 0)
```

	ret = dev_read_u32_array(dev, "snps,reset-delays-us",

```
				 eqos->reset_delays, 3);
```

in case "snps,reset-delays-us" is not in present DT, all resets-delays are set to 0, see my remark above

...

else
	pr_warn("gpio_request_by_name(snps,reset-gpio) failed: %d",
		ret);
}

debug("%s: OK\n", __func__); return 0;

David Wu

9 May 9 May

4:59 a.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps,reset-gpio" phy-rst gpio for stm32

Hi Patrice,

在 2020/4/30 下午11:47, Patrice CHOTARD 写道:

...

...
@@ -701,6 +702,15 @@ static int eqos_start_resets_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev); if (dm_gpio_is_valid(&eqos->phy_reset_gpio)) {
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0);
if (ret < 0) {
	pr_err("dm_gpio_set_value(phy_reset, deassert) failed: %d",
	       ret);
	return ret;
}
udelay(eqos->reset_delays[0]);
not related to this patch subject

...
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 1);
if (ret < 0) {
	pr_err("dm_gpio_set_value(phy_reset, assert) failed: %d",
@@ -708,7 +718,7 @@ static int eqos_start_resets_stm32(struct udevice *dev) return ret; }
udelay(2);
udelay(eqos->reset_delays[1]);
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0); if (ret < 0) {

...

...
@@ -1712,11 +1724,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret);
else
	eqos->reset_delays[1] = 2;
this is not the correct place to set default value. It must be set in case we can't get value from DT below

No, three cases below, it is second case, and we can see udelay(2) in eqos_start_resets_stm32(), here we are to be compatible with the original.

- If there is not phy rst, reset_delays is 0; - If "reset-gpios exists in phy node, reset_delays [1] = 2; - "snps, reset-gpio" exists in DT, reset_delays is obtained from DT

...

eqos->phyaddr = ofnode_read_u32_default(phandle_args.node,
					"reg", -1);

}

if (!dm_gpio_is_valid(&eqos->phy_reset_gpio)) {
```
int reset_flags = GPIOD_IS_OUT;
```

if (dev_read_bool(dev, "snps,reset-active-low"))

```
	reset_flags |= GPIOD_ACTIVE_LOW;
```

ret = gpio_request_by_name(dev, "snps,reset-gpio", 0,

			   &eqos->phy_reset_gpio, reset_flags);

```
if (ret == 0)
```

	ret = dev_read_u32_array(dev, "snps,reset-delays-us",

```
				 eqos->reset_delays, 3);
```

in case "snps,reset-delays-us" is not in present DT, all resets-delays are set to 0, see my remark above

...

else
	pr_warn("gpio_request_by_name(snps,reset-gpio) failed: %d",
		ret);
}

debug("%s: OK\n", __func__); return 0;

Stephen Warren

1 May 1 May

12:36 a.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32

On 4/30/20 4:36 AM, David Wu wrote:

...

It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general.

...

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c

...

@@ -1712,11 +1724,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret);
else
	eqos->reset_delays[1] = 2;
eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, "reg", -1); }
if (!dm_gpio_is_valid(&eqos->phy_reset_gpio)) {
int reset_flags = GPIOD_IS_OUT;
if (dev_read_bool(dev, "snps,reset-active-low"))
	reset_flags |= GPIOD_ACTIVE_LOW;
ret = gpio_request_by_name(dev, "snps,reset-gpio", 0,
			   &eqos->phy_reset_gpio, reset_flags);

The kernel's bindings/net/snps,dwmac.yaml does not mention any reset-gpios property (which is what the existing code parses just above the portion that is quoted by this patch as context). I suspect that this patch should simply change the name of the property that this function parses to align with the binding, and fix any DTs in U-Boot that also don't match the binding?

Stephen Warren

12:42 a.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32

On 4/30/20 4:36 PM, Stephen Warren wrote:

...

On 4/30/20 4:36 AM, David Wu wrote:

...
It can be seen that most of the Socs using STM mac, "snps,reset-gpio" gpio is used, adding this option makes reset function more general.

...
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c

...
@@ -1712,11 +1724,29 @@ static int eqos_probe_resources_stm32(struct udevice *dev) if (ret) pr_warn("gpio_request_by_name(phy reset) not provided %d", ret);
else
	eqos->reset_delays[1] = 2;
eqos->phyaddr = ofnode_read_u32_default(phandle_args.node, "reg", -1); }
if (!dm_gpio_is_valid(&eqos->phy_reset_gpio)) {
int reset_flags = GPIOD_IS_OUT;
if (dev_read_bool(dev, "snps,reset-active-low"))
	reset_flags |= GPIOD_ACTIVE_LOW;
ret = gpio_request_by_name(dev, "snps,reset-gpio", 0,
			   &eqos->phy_reset_gpio, reset_flags);
The kernel's bindings/net/snps,dwmac.yaml does not mention any reset-gpios property (which is what the existing code parses just above the portion that is quoted by this patch as context). I suspect that this patch should simply change the name of the property that this function parses to align with the binding, and fix any DTs in U-Boot that also don't match the binding?

Oops, the relevant YAML file is probably ./bindings/net/rockchip-dwmac.txt, although this makes no difference to my statement luckily.

David Wu

9 May 9 May

4:41 a.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32

Hi Stephen,

在 2020/5/1 上午6:36, Stephen Warren 写道:

...

The kernel's bindings/net/snps,dwmac.yaml does not mention any reset-gpios property (which is what the existing code parses just above the portion that is quoted by this patch as context). I suspect that this patch should simply change the name of the property that this function parses to align with the binding, and fix any DTs in U-Boot that also don't match the binding?

The kernel's ./Documentation/devicetree/bindings/net/stmmac.txt mentions that Required properties:

- phy-mode: See ethernet.txt file in the same directory. - snps,reset-gpio gpio number for phy reset. - snps,reset-active-low boolean flag to indicate if phy reset is active low. - snps,reset-delays-us is triplet of delays The 1st cell is reset pre-delay in micro seconds. The 2nd cell is reset pulse in micro seconds. The 3rd cell is reset post-delay in micro seconds.

David Wu

5:32 a.m.

New subject: [PATCH 3/8] net: dwc_eth_qos: Add option "snps, reset-gpio" phy-rst gpio for stm32

Hi Stephen,

在 2020/5/9 上午10:41, David Wu 写道:

...

The kernel's ./Documentation/devicetree/bindings/net/stmmac.txt mentions that Required properties:

phy-mode: See ethernet.txt file in the same directory.

snps,reset-gpio gpio number for phy reset.

snps,reset-active-low boolean flag to indicate if phy reset is active

low.

snps,reset-delays-us is triplet of delays

The 1st cell is reset pre-delay in micro seconds. The 2nd cell is reset pulse in micro seconds. The 3rd cell is reset post-delay in micro seconds.

Sorry, I just saw you replying again before, stmmac.txt was found, this reply email please discard.

David Wu

30 Apr 30 Apr

12:36 p.m.

New subject: [PATCH 4/8] net: dwc_eth_qos: Move interface() to eqos_ops struct

After moving to eqos_ops, if eqos_config is defined outside, can not export interface() definition.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 06a8d924a7..fbd6caf85b 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -267,7 +267,6 @@ struct eqos_config { int swr_wait; int config_mac; int config_mac_mdio; - phy_interface_t (*interface)(struct udevice *dev); struct eqos_ops *ops; };

@@ -286,6 +285,7 @@ struct eqos_ops { int (*eqos_disable_calibration)(struct udevice *dev); int (*eqos_set_tx_clk_speed)(struct udevice *dev); ulong (*eqos_get_tick_clk_rate)(struct udevice *dev); + phy_interface_t (*eqos_get_interface)(struct udevice *dev); };

struct eqos_priv { @@ -1105,7 +1105,7 @@ static int eqos_start(struct udevice *dev) */ if (!eqos->phy) { eqos->phy = phy_connect(eqos->mii, eqos->phyaddr, dev, - eqos->config->interface(dev)); + eqos->config->ops->eqos_get_interface(dev)); if (!eqos->phy) { pr_err("phy_connect() failed"); goto err_stop_resets; @@ -1675,7 +1675,7 @@ static int eqos_probe_resources_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

- interface = eqos->config->interface(dev); + interface = eqos->config->ops->eqos_get_interface(dev);

if (interface == PHY_INTERFACE_MODE_NONE) { pr_err("Invalid PHY interface\n"); @@ -1918,7 +1918,8 @@ static struct eqos_ops eqos_tegra186_ops = { .eqos_calibrate_pads = eqos_calibrate_pads_tegra186, .eqos_disable_calibration = eqos_disable_calibration_tegra186, .eqos_set_tx_clk_speed = eqos_set_tx_clk_speed_tegra186, - .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_tegra186 + .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_tegra186, + .eqos_get_interface = eqos_get_interface_tegra186 };

static const struct eqos_config eqos_tegra186_config = { @@ -1927,7 +1928,6 @@ static const struct eqos_config eqos_tegra186_config = { .swr_wait = 10, .config_mac = EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB, .config_mac_mdio = EQOS_MAC_MDIO_ADDRESS_CR_20_35, - .interface = eqos_get_interface_tegra186, .ops = &eqos_tegra186_ops };

@@ -1945,7 +1945,8 @@ static struct eqos_ops eqos_stm32_ops = { .eqos_calibrate_pads = eqos_calibrate_pads_stm32, .eqos_disable_calibration = eqos_disable_calibration_stm32, .eqos_set_tx_clk_speed = eqos_set_tx_clk_speed_stm32, - .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_stm32 + .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_stm32, + .eqos_get_interface = eqos_get_interface_stm32 };

static const struct eqos_config eqos_stm32_config = { @@ -1954,7 +1955,6 @@ static const struct eqos_config eqos_stm32_config = { .swr_wait = 50, .config_mac = EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_AV, .config_mac_mdio = EQOS_MAC_MDIO_ADDRESS_CR_250_300, - .interface = eqos_get_interface_stm32, .ops = &eqos_stm32_ops };

-- 2.19.1

Stephen Warren

1 May 1 May

12:39 a.m.

New subject: [PATCH 4/8] net: dwc_eth_qos: Move interface() to eqos_ops struct

On 4/30/20 4:36 AM, David Wu wrote:

...

After moving to eqos_ops, if eqos_config is defined outside, can not export interface() definition.

Looking at the patch itself, I think this patch just moves a function pointer from the config to the ops structure which makes sense. However, I can't understand the patch description at all, so I worry there's intended to be some other justification/implication for this patch, and that may not be correct...

In particular, defined outside of what, and what does this have to do with exporting things?

David Wu

9 May 9 May

5:22 a.m.

New subject: [PATCH 4/8] net: dwc_eth_qos: Move interface() to eqos_ops struct

Hi Stephen,

在 2020/5/1 上午6:39, Stephen Warren 写道:

...

On 4/30/20 4:36 AM, David Wu wrote:

...
After moving to eqos_ops, if eqos_config is defined outside, can not export interface() definition.

Looking at the patch itself, I think this patch just moves a function pointer from the config to the ops structure which makes sense. However, I can't understand the patch description at all, so I worry there's intended to be some other justification/implication for this patch, and that may not be correct...

In particular, defined outside of what, and what does this have to do with exporting things

Yes, if define eqos_config structure in gmac_rockchip.c, need to export an eqos_get_interface function, or redefine a similar function in gmac_rockchip.c, but this function is the same implementation as eqos_get_interface_stm32(), so we can share this function. Move interface() to eqos_ops structure, no need to export interface() in the head file. I lost a patch to define eqos_ops structure at curent file, then only exprot eqos_rockchip_ops, so it would be simpler?

...

David Wu

30 Apr 30 Apr

12:43 p.m.

New subject: [PATCH 5/8] net: dwc_eth_qos: Make clk_rx and clk_tx optional

For others using, clk_rx and clk_tx may not be necessary, and their clock names are different.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 65 +++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index fbd6caf85b..b5d5156292 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -592,16 +592,20 @@ static int eqos_start_clks_stm32(struct udevice *dev) goto err; }

- ret = clk_enable(&eqos->clk_rx); - if (ret < 0) { - pr_err("clk_enable(clk_rx) failed: %d", ret); - goto err_disable_clk_master_bus; + if (clk_valid(&eqos->clk_rx)) { + ret = clk_enable(&eqos->clk_rx); + if (ret < 0) { + pr_err("clk_enable(clk_rx) failed: %d", ret); + goto err_disable_clk_master_bus; + } }

- ret = clk_enable(&eqos->clk_tx); - if (ret < 0) { - pr_err("clk_enable(clk_tx) failed: %d", ret); - goto err_disable_clk_rx; + if (clk_valid(&eqos->clk_tx)) { + ret = clk_enable(&eqos->clk_tx); + if (ret < 0) { + pr_err("clk_enable(clk_tx) failed: %d", ret); + goto err_disable_clk_rx; + } }

if (clk_valid(&eqos->clk_ck)) { @@ -616,9 +620,11 @@ static int eqos_start_clks_stm32(struct udevice *dev) return 0;

err_disable_clk_tx: - clk_disable(&eqos->clk_tx); + if (clk_valid(&eqos->clk_tx)) + clk_disable(&eqos->clk_tx); err_disable_clk_rx: - clk_disable(&eqos->clk_rx); + if (clk_valid(&eqos->clk_rx)) + clk_disable(&eqos->clk_rx); err_disable_clk_master_bus: clk_disable(&eqos->clk_master_bus); err: @@ -647,8 +653,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

- clk_disable(&eqos->clk_tx); - clk_disable(&eqos->clk_rx); + if (clk_valid(&eqos->clk_tx)) + clk_disable(&eqos->clk_tx); + if (clk_valid(&eqos->clk_rx)) + clk_disable(&eqos->clk_rx); clk_disable(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_disable(&eqos->clk_ck); @@ -1691,20 +1699,16 @@ static int eqos_probe_resources_stm32(struct udevice *dev) ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); if (ret) { pr_err("clk_get_by_name(master_bus) failed: %d", ret); - goto err_probe; + return ret; }

- ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx); - if (ret) { - pr_err("clk_get_by_name(rx) failed: %d", ret); - goto err_free_clk_master_bus; - } + ret = clk_get_by_name(dev, "mac_clk_rx", &eqos->clk_rx); + if (ret) + pr_warn("clk_get_by_name(rx) failed: %d", ret);

- ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx); - if (ret) { - pr_err("clk_get_by_name(tx) failed: %d", ret); - goto err_free_clk_rx; - } + ret = clk_get_by_name(dev, "mac_clk_tx", &eqos->clk_tx); + if (ret) + pr_warn("clk_get_by_name(tx) failed: %d", ret);

/* Get ETH_CLK clocks (optional) */ ret = clk_get_by_name(dev, "eth-ck", &eqos->clk_ck); @@ -1749,15 +1753,6 @@ static int eqos_probe_resources_stm32(struct udevice *dev)

debug("%s: OK\n", __func__); return 0; - -err_free_clk_rx: - clk_free(&eqos->clk_rx); -err_free_clk_master_bus: - clk_free(&eqos->clk_master_bus); -err_probe: - - debug("%s: returns %d\n", __func__, ret); - return ret; }

static phy_interface_t eqos_get_interface_stm32(struct udevice *dev) @@ -1803,8 +1798,10 @@ static int eqos_remove_resources_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

- clk_free(&eqos->clk_tx); - clk_free(&eqos->clk_rx); + if (clk_valid(&eqos->clk_tx)) + clk_free(&eqos->clk_tx); + if (clk_valid(&eqos->clk_rx)) + clk_free(&eqos->clk_rx); clk_free(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_free(&eqos->clk_ck);

-- 2.19.1

Patrice CHOTARD

4 p.m.

New subject: [PATCH 5/8] net: dwc_eth_qos: Make clk_rx and clk_tx optional

Hi David

On 4/30/20 12:43 PM, David Wu wrote:

...

For others using, clk_rx and clk_tx may not be necessary, and their clock names are different.

Signed-off-by: David Wu david.wu@rock-chips.com

drivers/net/dwc_eth_qos.c | 65 +++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index fbd6caf85b..b5d5156292 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -592,16 +592,20 @@ static int eqos_start_clks_stm32(struct udevice *dev) goto err; }
ret = clk_enable(&eqos->clk_rx);

if (ret < 0) {
pr_err("clk_enable(clk_rx) failed: %d", ret);
goto err_disable_clk_master_bus;
if (clk_valid(&eqos->clk_rx)) {
ret = clk_enable(&eqos->clk_rx);
if (ret < 0) {
	pr_err("clk_enable(clk_rx) failed: %d", ret);
	goto err_disable_clk_master_bus;
}
}
ret = clk_enable(&eqos->clk_tx);

if (ret < 0) {
pr_err("clk_enable(clk_tx) failed: %d", ret);
goto err_disable_clk_rx;
if (clk_valid(&eqos->clk_tx)) {
ret = clk_enable(&eqos->clk_tx);
if (ret < 0) {
	pr_err("clk_enable(clk_tx) failed: %d", ret);
	goto err_disable_clk_rx;
}
}

if (clk_valid(&eqos->clk_ck)) {
@@ -616,9 +620,11 @@ static int eqos_start_clks_stm32(struct udevice *dev) return 0;

err_disable_clk_tx:

clk_disable(&eqos->clk_tx);
if (clk_valid(&eqos->clk_tx))
clk_disable(&eqos->clk_tx);
err_disable_clk_rx:

clk_disable(&eqos->clk_rx);
if (clk_valid(&eqos->clk_rx))
clk_disable(&eqos->clk_rx);
err_disable_clk_master_bus: clk_disable(&eqos->clk_master_bus); err: @@ -647,8 +653,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

clk_disable(&eqos->clk_tx);

clk_disable(&eqos->clk_rx);
if (clk_valid(&eqos->clk_tx))
clk_disable(&eqos->clk_tx);
if (clk_valid(&eqos->clk_rx))
clk_disable(&eqos->clk_rx);
clk_disable(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_disable(&eqos->clk_ck);
@@ -1691,20 +1699,16 @@ static int eqos_probe_resources_stm32(struct udevice *dev) ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); if (ret) { pr_err("clk_get_by_name(master_bus) failed: %d", ret);
goto err_probe;
return ret;
}

why are you changing the error path here ?

...

ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx);

if (ret) {
pr_err("clk_get_by_name(rx) failed: %d", ret);
goto err_free_clk_master_bus;
}
ret = clk_get_by_name(dev, "mac_clk_rx", &eqos->clk_rx);

if (ret)
pr_warn("clk_get_by_name(rx) failed: %d", ret);
ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx);

if (ret) {
pr_err("clk_get_by_name(tx) failed: %d", ret);
goto err_free_clk_rx;
}
ret = clk_get_by_name(dev, "mac_clk_tx", &eqos->clk_tx);

if (ret)
pr_warn("clk_get_by_name(tx) failed: %d", ret);

Nak

Why are you changing the Rx and Tx clock names ?

for information, check with the kernel dt bindings regarding this driver:

Documentation/devicetree/bindings/net/stm32-dwmac.txt

This patch is breaking ethernet on STM32MP1 boards

...

/* Get ETH_CLK clocks (optional) */ ret = clk_get_by_name(dev, "eth-ck", &eqos->clk_ck); @@ -1749,15 +1753,6 @@ static int eqos_probe_resources_stm32(struct udevice *dev)

debug("%s: OK\n", __func__); return 0;

-err_free_clk_rx:

clk_free(&eqos->clk_rx);

-err_free_clk_master_bus:

clk_free(&eqos->clk_master_bus);

-err_probe:

debug("%s: returns %d\n", __func__, ret);

return ret;

}

static phy_interface_t eqos_get_interface_stm32(struct udevice *dev) @@ -1803,8 +1798,10 @@ static int eqos_remove_resources_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

clk_free(&eqos->clk_tx);

clk_free(&eqos->clk_rx);
if (clk_valid(&eqos->clk_tx))
clk_free(&eqos->clk_tx);
if (clk_valid(&eqos->clk_rx))
clk_free(&eqos->clk_rx);
clk_free(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_free(&eqos->clk_ck);

David Wu

9 May 9 May

8:31 a.m.

New subject: [PATCH 5/8] net: dwc_eth_qos: Make clk_rx and clk_tx optional

Hi Patrice,

在 2020/4/30 下午10:00, Patrice CHOTARD 写道:

...

...
@@ -647,8 +653,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev)

debug("%s(dev=%p):\n", __func__, dev);

clk_disable(&eqos->clk_tx);

clk_disable(&eqos->clk_rx);
if (clk_valid(&eqos->clk_tx))
clk_disable(&eqos->clk_tx);
if (clk_valid(&eqos->clk_rx))
clk_disable(&eqos->clk_rx);
clk_disable(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_disable(&eqos->clk_ck);
@@ -1691,20 +1699,16 @@ static int eqos_probe_resources_stm32(struct udevice *dev) ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); if (ret) { pr_err("clk_get_by_name(master_bus) failed: %d", ret);
goto err_probe;
return ret;
}
why are you changing the error path here ?

The following code has not returned, so for the sake of simpler code, return directly.

...

...
ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx);

if (ret) {
pr_err("clk_get_by_name(rx) failed: %d", ret);
goto err_free_clk_master_bus;
}
ret = clk_get_by_name(dev, "mac_clk_rx", &eqos->clk_rx);

if (ret)
pr_warn("clk_get_by_name(rx) failed: %d", ret);
ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx);

if (ret) {
pr_err("clk_get_by_name(tx) failed: %d", ret);
goto err_free_clk_rx;
}
ret = clk_get_by_name(dev, "mac_clk_tx", &eqos->clk_tx);

if (ret)
pr_warn("clk_get_by_name(tx) failed: %d", ret);
Nak

Why are you changing the Rx and Tx clock names ?

for information, check with the kernel dt bindings regarding this driver:

Documentation/devicetree/bindings/net/stm32-dwmac.txt

This patch is breaking ethernet on STM32MP1 boards

I should have made a mistake here. In fact, for Rockchip, there is no need to obtain this two clock. My intention is to make these two clocks optional.

...

Stephen Warren

1 May 1 May

12:45 a.m.

New subject: [PATCH 5/8] net: dwc_eth_qos: Make clk_rx and clk_tx optional

On 4/30/20 4:43 AM, David Wu wrote:

...

For others using, clk_rx and clk_tx may not be necessary, and their clock names are different.

...

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c

...

@@ -1691,20 +1699,16 @@ static int eqos_probe_resources_stm32(struct udevice *dev) ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); if (ret) { pr_err("clk_get_by_name(master_bus) failed: %d", ret);
goto err_probe;
return ret;
}
ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx);

if (ret) {
pr_err("clk_get_by_name(rx) failed: %d", ret);
goto err_free_clk_master_bus;
}
ret = clk_get_by_name(dev, "mac_clk_rx", &eqos->clk_rx);

if (ret)
pr_warn("clk_get_by_name(rx) failed: %d", ret);

Oh... Judging by your email, you're trying to make this driver work on a Rockchip system. However, you're editing an STM32-specific probe function. You should introduce a new probe function for Rockchip if it needs to work differently to the existing STM32 code.

Also, mac_clk_rx isn't a valid DT property name; they aren't supposed to have _ in them. I don't see mac_clk_rx or mac-clk-rx in the DT binding file in Documentation/bindings/net/rockchip-dwmac.txt the kernel. That should probably be submitted/reviewed/applied before using the binding...

David Wu

9 May 9 May

8:33 a.m.

New subject: [PATCH 5/8] net: dwc_eth_qos: Make clk_rx and clk_tx optional

Hi Stephen,

在 2020/5/1 上午6:45, Stephen Warren 写道:

...

Oh... Judging by your email, you're trying to make this driver work on a Rockchip system. However, you're editing an STM32-specific probe function. You should introduce a new probe function for Rockchip if it needs to work differently to the existing STM32 code.

Also, mac_clk_rx isn't a valid DT property name; they aren't supposed to have _ in them. I don't see mac_clk_rx or mac-clk-rx in the DT binding file in Documentation/bindings/net/rockchip-dwmac.txt the kernel. That should probably be submitted/reviewed/applied before using the binding...

If necessary, I can rewrite a function to obtain resources, but I think the function of rockchip and stm should be very similar, and can be shared.

David Wu

30 Apr 30 Apr

12:44 p.m.

New subject: [PATCH 6/8] net: dwc_eth_qos: Split eqos_start() to get link speed

Before enabling mac and mac working, we need to obtain the current link speed to configure the clock, so split eqos_start into two functions.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 56 ++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index b5d5156292..25b3449047 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1051,19 +1051,15 @@ static int eqos_write_hwaddr(struct udevice *dev) return 0; }

-static int eqos_start(struct udevice *dev) +static int eqos_init(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); - int ret, i, limit; + int ret, limit; ulong rate; - u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; - ulong last_rx_desc; + u32 val;

debug("%s(dev=%p):\n", __func__, dev);

- eqos->tx_desc_idx = 0; - eqos->rx_desc_idx = 0; - ret = eqos->config->ops->eqos_start_clks(dev); if (ret < 0) { pr_err("eqos_start_clks() failed: %d", ret); @@ -1151,6 +1147,30 @@ static int eqos_start(struct udevice *dev) goto err_shutdown_phy; }

+ debug("%s: OK\n", __func__); + return 0; + +err_shutdown_phy: + phy_shutdown(eqos->phy); +err_stop_resets: + eqos->config->ops->eqos_stop_resets(dev); +err_stop_clks: + eqos->config->ops->eqos_stop_clks(dev); +err: + pr_err("FAILED: %d", ret); + return ret; +} + +static void eqos_enable(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; + ulong last_rx_desc; + int i; + + eqos->tx_desc_idx = 0; + eqos->rx_desc_idx = 0; + /* Configure MTL */

/* Enable Store and Forward mode for TX */ @@ -1352,19 +1372,19 @@ static int eqos_start(struct udevice *dev) writel(last_rx_desc, &eqos->dma_regs->ch0_rxdesc_tail_pointer);

eqos->started = true; +}

- debug("%s: OK\n", __func__); - return 0; +static int eqos_start(struct udevice *dev) +{ + int ret;

-err_shutdown_phy: - phy_shutdown(eqos->phy); -err_stop_resets: - eqos->config->ops->eqos_stop_resets(dev); -err_stop_clks: - eqos->config->ops->eqos_stop_clks(dev); -err: - pr_err("FAILED: %d", ret); - return ret; + ret = eqos_init(dev); + if (ret) + return ret; + + eqos_enable(dev); + + return 0; }

static void eqos_stop(struct udevice *dev)

-- 2.19.1

Patrice CHOTARD

5:33 p.m.

New subject: [PATCH 6/8] net: dwc_eth_qos: Split eqos_start() to get link speed

Hi David

On 4/30/20 12:44 PM, David Wu wrote:

...

Before enabling mac and mac working, we need to obtain the current link speed to configure the clock, so split eqos_start into two functions.

Signed-off-by: David Wu david.wu@rock-chips.com

drivers/net/dwc_eth_qos.c | 56 ++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index b5d5156292..25b3449047 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1051,19 +1051,15 @@ static int eqos_write_hwaddr(struct udevice *dev) return 0; }

-static int eqos_start(struct udevice *dev) +static int eqos_init(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev);

int ret, i, limit;

int ret, limit; ulong rate;

u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl;

ulong last_rx_desc;

u32 val;

debug("%s(dev=%p):\n", __func__, dev);

eqos->tx_desc_idx = 0;

eqos->rx_desc_idx = 0;

ret = eqos->config->ops->eqos_start_clks(dev); if (ret < 0) { pr_err("eqos_start_clks() failed: %d", ret);

@@ -1151,6 +1147,30 @@ static int eqos_start(struct udevice *dev) goto err_shutdown_phy; }

debug("%s: OK\n", __func__);

return 0;

+err_shutdown_phy:

phy_shutdown(eqos->phy);

+err_stop_resets:

eqos->config->ops->eqos_stop_resets(dev);

+err_stop_clks:

eqos->config->ops->eqos_stop_clks(dev);

+err:

pr_err("FAILED: %d", ret);

return ret;

+}

+static void eqos_enable(struct udevice *dev) +{

struct eqos_priv *eqos = dev_get_priv(dev);

u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl;

ulong last_rx_desc;

int i;

eqos->tx_desc_idx = 0;

eqos->rx_desc_idx = 0;

/* Configure MTL */

/* Enable Store and Forward mode for TX */

@@ -1352,19 +1372,19 @@ static int eqos_start(struct udevice *dev) writel(last_rx_desc, &eqos->dma_regs->ch0_rxdesc_tail_pointer);

eqos->started = true; +}

debug("%s: OK\n", __func__);

return 0;

+static int eqos_start(struct udevice *dev) +{

int ret;

-err_shutdown_phy:

phy_shutdown(eqos->phy);

-err_stop_resets:

eqos->config->ops->eqos_stop_resets(dev);

-err_stop_clks:

eqos->config->ops->eqos_stop_clks(dev);

-err:

pr_err("FAILED: %d", ret);

return ret;
ret = eqos_init(dev);

if (ret)
return ret;
eqos_enable(dev);

return 0;

Can you explain why you are splitting this function in 2 parts and calling these parts sequentially ?

Please elaborate

Thanks

Patrice

...

}

static void eqos_stop(struct udevice *dev)

David Wu

9 May 9 May

8:42 a.m.

New subject: [PATCH 6/8] net: dwc_eth_qos: Split eqos_start() to get link speed

Hi Patrice,

在 2020/4/30 下午11:33, Patrice CHOTARD 写道:

...

Can you explain why you are splitting this function in 2 parts and calling these parts sequentially ?

For rockchip, need to obtain the current link speed to configure the tx clocks, (for example, in rgmii mode, 1000M link: 125M, 100M link: 25M, 10M link is 2.5M rate) and then enable gmac. So after the adjust_link(), before the start gamc, this intermediate stage needs to configure the clock according to the current link speed.

Patrice CHOTARD

11 May 11 May

2:48 p.m.

New subject: [PATCH 6/8] net: dwc_eth_qos: Split eqos_start() to get link speed

Hi David

On 5/9/20 8:42 AM, David Wu wrote:

...

Hi Patrice,

在 2020/4/30 下午11:33, Patrice CHOTARD 写道:

...
Can you explain why you are splitting this function in 2 parts and calling these parts sequentially ?

For rockchip, need to obtain the current link speed to configure the tx clocks, (for example, in rgmii mode, 1000M link: 125M, 100M link: 25M, 10M link is 2.5M rate) and then enable gmac. So after the adjust_link(), before the start gamc, this intermediate stage needs to configure the clock according to the current link speed.

Please, add these informations in the commit message

Thanks

Patrice

David Wu

12 May 12 May

3:56 a.m.

New subject: [PATCH 6/8] net: dwc_eth_qos: Split eqos_start() to get link speed

Hi Patrice,

在 2020/5/11 下午8:48, Patrice CHOTARD 写道:

...

Hi David

On 5/9/20 8:42 AM, David Wu wrote:

...
Hi Patrice,

在 2020/4/30 下午11:33, Patrice CHOTARD 写道:

...
Can you explain why you are splitting this function in 2 parts and calling these parts sequentially ?

For rockchip, need to obtain the current link speed to configure the tx clocks, (for example, in rgmii mode, 1000M link: 125M, 100M link: 25M, 10M link is 2.5M rate) and then enable gmac. So after the adjust_link(), before the start gamc, this intermediate stage needs to configure the clock according to the current link speed.

Please, add these informations in the commit message

I will add it at the next version, Thanks.

...

Thanks

Patrice

David Wu

30 Apr 30 Apr

12:45 p.m.

New subject: [PATCH 7/8] net: dwc_eth_qos: Export common struct and interface at head file

Open structure data and interface, so that Soc using dw_eth_qos controller can reference.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/dwc_eth_qos.c | 81 +++++---------------------------------- 1 file changed, 9 insertions(+), 72 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 25b3449047..7f47e5f505 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -41,6 +41,7 @@ #include <wait_bit.h> #include <asm/gpio.h> #include <asm/io.h> +#include "dwc_eth_qos.h"

/* Core registers */

@@ -94,9 +95,6 @@ struct eqos_mac_regs {

#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT 0 #define EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK 3 -#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_NOT_ENABLED 0 -#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB 2 -#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_AV 1

#define EQOS_MAC_RXQ_CTRL2_PSRQ0_SHIFT 0 #define EQOS_MAC_RXQ_CTRL2_PSRQ0_MASK 0xff @@ -109,8 +107,6 @@ struct eqos_mac_regs { #define EQOS_MAC_MDIO_ADDRESS_PA_SHIFT 21 #define EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT 16 #define EQOS_MAC_MDIO_ADDRESS_CR_SHIFT 8 -#define EQOS_MAC_MDIO_ADDRESS_CR_20_35 2 -#define EQOS_MAC_MDIO_ADDRESS_CR_250_300 5 #define EQOS_MAC_MDIO_ADDRESS_SKAP BIT(4) #define EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT 2 #define EQOS_MAC_MDIO_ADDRESS_GOC_READ 3 @@ -261,65 +257,6 @@ struct eqos_desc { #define EQOS_DESC3_LD BIT(28) #define EQOS_DESC3_BUF1V BIT(24)

-struct eqos_config { - bool reg_access_always_ok; - int mdio_wait; - int swr_wait; - int config_mac; - int config_mac_mdio; - struct eqos_ops *ops; -}; - -struct eqos_ops { - void (*eqos_inval_desc)(void *desc); - void (*eqos_flush_desc)(void *desc); - void (*eqos_inval_buffer)(void *buf, size_t size); - void (*eqos_flush_buffer)(void *buf, size_t size); - int (*eqos_probe_resources)(struct udevice *dev); - int (*eqos_remove_resources)(struct udevice *dev); - int (*eqos_stop_resets)(struct udevice *dev); - int (*eqos_start_resets)(struct udevice *dev); - void (*eqos_stop_clks)(struct udevice *dev); - int (*eqos_start_clks)(struct udevice *dev); - int (*eqos_calibrate_pads)(struct udevice *dev); - int (*eqos_disable_calibration)(struct udevice *dev); - int (*eqos_set_tx_clk_speed)(struct udevice *dev); - ulong (*eqos_get_tick_clk_rate)(struct udevice *dev); - phy_interface_t (*eqos_get_interface)(struct udevice *dev); -}; - -struct eqos_priv { - struct udevice *dev; - const struct eqos_config *config; - fdt_addr_t regs; - struct eqos_mac_regs *mac_regs; - struct eqos_mtl_regs *mtl_regs; - struct eqos_dma_regs *dma_regs; - struct eqos_tegra186_regs *tegra186_regs; - struct reset_ctl reset_ctl; - struct gpio_desc phy_reset_gpio; - u32 reset_delays[3]; - struct clk clk_master_bus; - struct clk clk_rx; - struct clk clk_ptp_ref; - struct clk clk_tx; - struct clk clk_ck; - struct clk clk_slave_bus; - struct mii_dev *mii; - struct phy_device *phy; - int phyaddr; - u32 max_speed; - void *descs; - struct eqos_desc *tx_descs; - struct eqos_desc *rx_descs; - int tx_desc_idx, rx_desc_idx; - void *tx_dma_buf; - void *rx_dma_buf; - void *rx_pkt; - bool started; - bool reg_access_ok; -}; - /* * TX and RX descriptors are 16 bytes. This causes problems with the cache * maintenance on CPUs where the cache-line size exceeds the size of these @@ -1007,7 +944,7 @@ static int eqos_adjust_link(struct udevice *dev) return 0; }

-static int eqos_write_hwaddr(struct udevice *dev) +int eqos_write_hwaddr(struct udevice *dev) { struct eth_pdata *plat = dev_get_platdata(dev); struct eqos_priv *eqos = dev_get_priv(dev); @@ -1051,7 +988,7 @@ static int eqos_write_hwaddr(struct udevice *dev) return 0; }

-static int eqos_init(struct udevice *dev) +int eqos_init(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); int ret, limit; @@ -1161,7 +1098,7 @@ err: return ret; }

-static void eqos_enable(struct udevice *dev) +void eqos_enable(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; @@ -1387,7 +1324,7 @@ static int eqos_start(struct udevice *dev) return 0; }

-static void eqos_stop(struct udevice *dev) +void eqos_stop(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); int i; @@ -1441,7 +1378,7 @@ static void eqos_stop(struct udevice *dev) debug("%s: OK\n", __func__); }

-static int eqos_send(struct udevice *dev, void *packet, int length) +int eqos_send(struct udevice *dev, void *packet, int length) { struct eqos_priv *eqos = dev_get_priv(dev); struct eqos_desc *tx_desc; @@ -1482,7 +1419,7 @@ static int eqos_send(struct udevice *dev, void *packet, int length) return -ETIMEDOUT; }

-static int eqos_recv(struct udevice *dev, int flags, uchar **packetp) +int eqos_recv(struct udevice *dev, int flags, uchar **packetp) { struct eqos_priv *eqos = dev_get_priv(dev); struct eqos_desc *rx_desc; @@ -1506,7 +1443,7 @@ static int eqos_recv(struct udevice *dev, int flags, uchar **packetp) return length; }

-static int eqos_free_pkt(struct udevice *dev, uchar *packet, int length) +int eqos_free_pkt(struct udevice *dev, uchar *packet, int length) { struct eqos_priv *eqos = dev_get_priv(dev); uchar *packet_expected; @@ -1833,7 +1770,7 @@ static int eqos_remove_resources_stm32(struct udevice *dev) return 0; }

-static int eqos_probe(struct udevice *dev) +int eqos_probe(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); int ret;

-- 2.19.1

Patrice CHOTARD

2:06 p.m.

New subject: [PATCH 7/8] net: dwc_eth_qos: Export common struct and interface at head file

Hi David

On 4/30/20 12:45 PM, David Wu wrote:

...

Open structure data and interface, so that Soc using dw_eth_qos controller can reference.

Signed-off-by: David Wu david.wu@rock-chips.com

drivers/net/dwc_eth_qos.c | 81 +++++---------------------------------- 1 file changed, 9 insertions(+), 72 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 25b3449047..7f47e5f505 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -41,6 +41,7 @@ #include <wait_bit.h> #include <asm/gpio.h> #include <asm/io.h> +#include "dwc_eth_qos.h"

The new dwc_eth_qos.h file is missing in your series and can't compile

Patrice

...

/* Core registers */

@@ -94,9 +95,6 @@ struct eqos_mac_regs {

#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT 0 #define EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK 3 -#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_NOT_ENABLED 0 -#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB 2 -#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_AV 1

#define EQOS_MAC_RXQ_CTRL2_PSRQ0_SHIFT 0 #define EQOS_MAC_RXQ_CTRL2_PSRQ0_MASK 0xff @@ -109,8 +107,6 @@ struct eqos_mac_regs { #define EQOS_MAC_MDIO_ADDRESS_PA_SHIFT 21 #define EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT 16 #define EQOS_MAC_MDIO_ADDRESS_CR_SHIFT 8 -#define EQOS_MAC_MDIO_ADDRESS_CR_20_35 2 -#define EQOS_MAC_MDIO_ADDRESS_CR_250_300 5 #define EQOS_MAC_MDIO_ADDRESS_SKAP BIT(4) #define EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT 2 #define EQOS_MAC_MDIO_ADDRESS_GOC_READ 3 @@ -261,65 +257,6 @@ struct eqos_desc { #define EQOS_DESC3_LD BIT(28) #define EQOS_DESC3_BUF1V BIT(24)

-struct eqos_config {

bool reg_access_always_ok;

int mdio_wait;

int swr_wait;

int config_mac;

int config_mac_mdio;

struct eqos_ops *ops;

-};

-struct eqos_ops {

void (*eqos_inval_desc)(void *desc);

void (*eqos_flush_desc)(void *desc);

void (*eqos_inval_buffer)(void *buf, size_t size);

void (*eqos_flush_buffer)(void *buf, size_t size);

int (*eqos_probe_resources)(struct udevice *dev);

int (*eqos_remove_resources)(struct udevice *dev);

int (*eqos_stop_resets)(struct udevice *dev);

int (*eqos_start_resets)(struct udevice *dev);

void (*eqos_stop_clks)(struct udevice *dev);

int (*eqos_start_clks)(struct udevice *dev);

int (*eqos_calibrate_pads)(struct udevice *dev);

int (*eqos_disable_calibration)(struct udevice *dev);

int (*eqos_set_tx_clk_speed)(struct udevice *dev);

ulong (*eqos_get_tick_clk_rate)(struct udevice *dev);

phy_interface_t (*eqos_get_interface)(struct udevice *dev);

-};

-struct eqos_priv {

struct udevice *dev;

const struct eqos_config *config;

fdt_addr_t regs;

struct eqos_mac_regs *mac_regs;

struct eqos_mtl_regs *mtl_regs;

struct eqos_dma_regs *dma_regs;

struct eqos_tegra186_regs *tegra186_regs;

struct reset_ctl reset_ctl;

struct gpio_desc phy_reset_gpio;

u32 reset_delays[3];

struct clk clk_master_bus;

struct clk clk_rx;

struct clk clk_ptp_ref;

struct clk clk_tx;

struct clk clk_ck;

struct clk clk_slave_bus;

struct mii_dev *mii;

struct phy_device *phy;

int phyaddr;

u32 max_speed;

void *descs;

struct eqos_desc *tx_descs;

struct eqos_desc *rx_descs;

int tx_desc_idx, rx_desc_idx;

void *tx_dma_buf;

void *rx_dma_buf;

void *rx_pkt;

bool started;

bool reg_access_ok;

-};

/*

TX and RX descriptors are 16 bytes. This causes problems with the cache

maintenance on CPUs where the cache-line size exceeds the size of these

@@ -1007,7 +944,7 @@ static int eqos_adjust_link(struct udevice *dev) return 0; }

-static int eqos_write_hwaddr(struct udevice *dev) +int eqos_write_hwaddr(struct udevice *dev) { struct eth_pdata *plat = dev_get_platdata(dev); struct eqos_priv *eqos = dev_get_priv(dev); @@ -1051,7 +988,7 @@ static int eqos_write_hwaddr(struct udevice *dev) return 0; }

-static int eqos_init(struct udevice *dev) +int eqos_init(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); int ret, limit; @@ -1161,7 +1098,7 @@ err: return ret; }

-static void eqos_enable(struct udevice *dev) +void eqos_enable(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; @@ -1387,7 +1324,7 @@ static int eqos_start(struct udevice *dev) return 0; }

-static void eqos_stop(struct udevice *dev) +void eqos_stop(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); int i; @@ -1441,7 +1378,7 @@ static void eqos_stop(struct udevice *dev) debug("%s: OK\n", __func__); }

-static int eqos_send(struct udevice *dev, void *packet, int length) +int eqos_send(struct udevice *dev, void *packet, int length) { struct eqos_priv *eqos = dev_get_priv(dev); struct eqos_desc *tx_desc; @@ -1482,7 +1419,7 @@ static int eqos_send(struct udevice *dev, void *packet, int length) return -ETIMEDOUT; }

-static int eqos_recv(struct udevice *dev, int flags, uchar **packetp) +int eqos_recv(struct udevice *dev, int flags, uchar **packetp) { struct eqos_priv *eqos = dev_get_priv(dev); struct eqos_desc *rx_desc; @@ -1506,7 +1443,7 @@ static int eqos_recv(struct udevice *dev, int flags, uchar **packetp) return length; }

-static int eqos_free_pkt(struct udevice *dev, uchar *packet, int length) +int eqos_free_pkt(struct udevice *dev, uchar *packet, int length) { struct eqos_priv *eqos = dev_get_priv(dev); uchar *packet_expected; @@ -1833,7 +1770,7 @@ static int eqos_remove_resources_stm32(struct udevice *dev) return 0; }

-static int eqos_probe(struct udevice *dev) +int eqos_probe(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); int ret;

Stephen Warren

1 May 1 May

12:47 a.m.

New subject: [PATCH 7/8] net: dwc_eth_qos: Export common struct and interface at head file

On 4/30/20 4:45 AM, David Wu wrote:

...

Open structure data and interface, so that Soc using dw_eth_qos controller can reference.

This shouldn't be necessary; nothing outside of this driver should need access to the registers.

At most, perhaps implement some additional functions so that other code can call into the driver to perform specific actions. However, even that's probably not a good idea.

David Wu

30 Apr 30 Apr

12:45 p.m.

New subject: [PATCH 8/8] net: gmac_rockchip: Add dwc_eth_qos support

Change the original data structure so that Rockchip's Soc gmac controller can support the designware.c and dwc_eth_qos.c drivers, a Soc can only support one.

Signed-off-by: David Wu david.wu@rock-chips.com ---

drivers/net/Kconfig | 2 +- drivers/net/gmac_rockchip.c | 160 ++++++++++++++++++++++++++++++------ 2 files changed, 135 insertions(+), 27 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 4d1013c984..07d2b0787c 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -482,7 +482,7 @@ config PIC32_ETH

config GMAC_ROCKCHIP bool "Rockchip Synopsys Designware Ethernet MAC" - depends on DM_ETH && ETH_DESIGNWARE + depends on DM_ETH && (ETH_DESIGNWARE || DWC_ETH_QOS) help This driver provides Rockchip SoCs network support based on the Synopsys Designware driver. diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c index e152faf083..aa2bab4203 100644 --- a/drivers/net/gmac_rockchip.c +++ b/drivers/net/gmac_rockchip.c @@ -25,26 +25,39 @@ #include <dm/pinctrl.h> #include <dt-bindings/clock/rk3288-cru.h> #include "designware.h" +#include "dwc_eth_qos.h"

DECLARE_GLOBAL_DATA_PTR; #define DELAY_ENABLE(soc, tx, rx) \ (((tx) ? soc##_TXCLK_DLY_ENA_GMAC_ENABLE : soc##_TXCLK_DLY_ENA_GMAC_DISABLE) | \ ((rx) ? soc##_RXCLK_DLY_ENA_GMAC_ENABLE : soc##_RXCLK_DLY_ENA_GMAC_DISABLE))

+struct rockchip_eth_dev { + union { + struct eqos_priv eqos; + struct dw_eth_dev dw; + }; +}; + /* * Platform data for the gmac * * dw_eth_pdata: Required platform data for designware driver (must be first) */ struct gmac_rockchip_platdata { - struct dw_eth_pdata dw_eth_pdata; + union { + struct dw_eth_pdata dw_eth_pdata; + struct eth_pdata eth_pdata; + }; + bool has_gmac4; bool clock_input; int tx_delay; int rx_delay; };

struct rk_gmac_ops { - int (*fix_mac_speed)(struct dw_eth_dev *priv); + const struct eqos_config config; + int (*fix_mac_speed)(struct rockchip_eth_dev *dev); void (*set_to_rmii)(struct gmac_rockchip_platdata *pdata); void (*set_to_rgmii)(struct gmac_rockchip_platdata *pdata); }; @@ -55,6 +68,9 @@ static int gmac_rockchip_ofdata_to_platdata(struct udevice *dev) struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); const char *string;

+ if (device_is_compatible(dev, "snps,dwmac-4.20a")) + pdata->has_gmac4 = true; + string = dev_read_string(dev, "clock_in_out"); if (!strcmp(string, "input")) pdata->clock_input = true; @@ -71,11 +87,15 @@ static int gmac_rockchip_ofdata_to_platdata(struct udevice *dev) if (pdata->rx_delay == -ENOENT) pdata->rx_delay = dev_read_u32_default(dev, "rx-delay", 0x10);

- return designware_eth_ofdata_to_platdata(dev); + if (!pdata->has_gmac4) + return designware_eth_ofdata_to_platdata(dev); + + return 0; }

-static int px30_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int px30_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct px30_grf *grf; struct clk clk_speed; int speed, ret; @@ -115,8 +135,9 @@ static int px30_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rk3228_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int rk3228_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rk322x_grf *grf; int clk; enum { @@ -148,8 +169,9 @@ static int rk3228_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rk3288_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int rk3288_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rk3288_grf *grf; int clk;

@@ -174,8 +196,9 @@ static int rk3288_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rk3308_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int rk3308_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rk3308_grf *grf; struct clk clk_speed; int speed, ret; @@ -215,8 +238,9 @@ static int rk3308_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rk3328_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int rk3328_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rk3328_grf_regs *grf; int clk; enum { @@ -248,8 +272,9 @@ static int rk3328_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rk3368_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int rk3368_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rk3368_grf *grf; int clk; enum { @@ -280,8 +305,9 @@ static int rk3368_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev *priv) +static int rk3399_gmac_fix_mac_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rk3399_grf_regs *grf; int clk;

@@ -306,8 +332,9 @@ static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev *priv) return 0; }

-static int rv1108_set_rmii_speed(struct dw_eth_dev *priv) +static int rv1108_set_rmii_speed(struct rockchip_eth_dev *dev) { + struct dw_eth_dev *priv = &dev->dw; struct rv1108_grf *grf; int clk, speed; enum { @@ -555,12 +582,22 @@ static int gmac_rockchip_probe(struct udevice *dev) struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); struct rk_gmac_ops *ops = (struct rk_gmac_ops *)dev_get_driver_data(dev); - struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev); - struct eth_pdata *eth_pdata = &dw_pdata->eth_pdata; + struct dw_eth_pdata *dw_pdata; + struct eth_pdata *eth_pdata; + struct eqos_config *config; struct clk clk; ulong rate; int ret;

+ if (pdata->has_gmac4) { + eth_pdata = &pdata->eth_pdata; + config = (struct eqos_config *)&ops->config; + eth_pdata->phy_interface = config->ops->eqos_get_interface(dev); + } else { + dw_pdata = &pdata->dw_eth_pdata; + eth_pdata = &dw_pdata->eth_pdata; + } + ret = clk_set_defaults(dev, 0); if (ret) debug("%s clk_set_defaults failed %d\n", __func__, ret); @@ -656,37 +693,108 @@ static int gmac_rockchip_probe(struct udevice *dev) return -ENXIO; }

- return designware_eth_probe(dev); + if (pdata->has_gmac4) + return eqos_probe(dev); + else + return designware_eth_probe(dev); +} + +static int gmac_rockchip_eth_write_hwaddr(struct udevice *dev) +{ + struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); + + if (pdata->has_gmac4) + return eqos_write_hwaddr(dev); + else + return designware_eth_write_hwaddr(dev); +} + +static int gmac_rockchip_eth_free_pkt(struct udevice *dev, uchar *packet, + int length) +{ + struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); + + if (pdata->has_gmac4) + return eqos_free_pkt(dev, packet, length); + else + return designware_eth_free_pkt(dev, packet, length); +} + +static int gmac_rockchip_eth_send(struct udevice *dev, void *packet, + int length) +{ + struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); + + if (pdata->has_gmac4) + return eqos_send(dev, packet, length); + else + return designware_eth_send(dev, packet, length); +} + +static int gmac_rockchip_eth_recv(struct udevice *dev, int flags, + uchar **packetp) +{ + struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); + + if (pdata->has_gmac4) + return eqos_recv(dev, flags, packetp); + else + return designware_eth_recv(dev, flags, packetp); }

static int gmac_rockchip_eth_start(struct udevice *dev) { - struct eth_pdata *pdata = dev_get_platdata(dev); - struct dw_eth_dev *priv = dev_get_priv(dev); + struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); + struct rockchip_eth_dev *priv = dev_get_priv(dev); struct rk_gmac_ops *ops = (struct rk_gmac_ops *)dev_get_driver_data(dev); + struct dw_eth_pdata *dw_pdata; + struct eth_pdata *eth_pdata; int ret;

- ret = designware_eth_init(priv, pdata->enetaddr); + if (pdata->has_gmac4) { + ret = eqos_init(dev); + } else { + dw_pdata = &pdata->dw_eth_pdata; + eth_pdata = &dw_pdata->eth_pdata; + ret = designware_eth_init((struct dw_eth_dev *)priv, + eth_pdata->enetaddr); + } if (ret) return ret; + ret = ops->fix_mac_speed(priv); if (ret) return ret; - ret = designware_eth_enable(priv); - if (ret) - return ret; + + if (pdata->has_gmac4) { + eqos_enable(dev); + } else { + ret = designware_eth_enable((struct dw_eth_dev *)priv); + if (ret) + return ret; + }

return 0; }

+static void gmac_rockchip_eth_stop(struct udevice *dev) +{ + struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev); + + if (pdata->has_gmac4) + eqos_stop(dev); + else + designware_eth_stop(dev); +} + const struct eth_ops gmac_rockchip_eth_ops = { .start = gmac_rockchip_eth_start, - .send = designware_eth_send, - .recv = designware_eth_recv, - .free_pkt = designware_eth_free_pkt, - .stop = designware_eth_stop, - .write_hwaddr = designware_eth_write_hwaddr, + .send = gmac_rockchip_eth_send, + .recv = gmac_rockchip_eth_recv, + .free_pkt = gmac_rockchip_eth_free_pkt, + .stop = gmac_rockchip_eth_stop, + .write_hwaddr = gmac_rockchip_eth_write_hwaddr, };

const struct rk_gmac_ops px30_gmac_ops = { @@ -756,7 +864,7 @@ U_BOOT_DRIVER(eth_gmac_rockchip) = { .ofdata_to_platdata = gmac_rockchip_ofdata_to_platdata, .probe = gmac_rockchip_probe, .ops = &gmac_rockchip_eth_ops, - .priv_auto_alloc_size = sizeof(struct dw_eth_dev), + .priv_auto_alloc_size = sizeof(struct rockchip_eth_dev), .platdata_auto_alloc_size = sizeof(struct gmac_rockchip_platdata), .flags = DM_FLAG_ALLOC_PRIV_DMA, };

-- 2.19.1

Stephen Warren

1 May 1 May

12:52 a.m.

New subject: [PATCH 8/8] net: gmac_rockchip: Add dwc_eth_qos support

On 4/30/20 4:45 AM, David Wu wrote:

...

Change the original data structure so that Rockchip's Soc gmac controller can support the designware.c and dwc_eth_qos.c drivers, a Soc can only support one.

I'm really confused; with a filename like gmac_rockchip.c that sounds like it's driver for a MAC device. DWC EQoS is also a MAC device. The two shouldn't be related or coupled in any way.

I think what you need is to completely drop this patch (and the patch which creates dwc_eth_qos.h), and instead make the DWC EQoS driver itself directly support the Rockchip SoC by adding RK's compatible value to the list of compatible values that the EQoS driver supports, along with new probe functions etc.

Maybe this requires splitting some PHY code out of gmac_rockchip into a common/separate PHY driver? I haven't looked at the code to know if that's required.

David Wu

9 May 9 May

8:56 a.m.

New subject: [PATCH 8/8] net: gmac_rockchip: Add dwc_eth_qos support

Hi Stephen,

在 2020/5/1 上午6:52, Stephen Warren 写道:

...

I'm really confused; with a filename like gmac_rockchip.c that sounds like it's driver for a MAC device. DWC EQoS is also a MAC device. The two shouldn't be related or coupled in any way.

I think what you need is to completely drop this patch (and the patch which creates dwc_eth_qos.h), and instead make the DWC EQoS driver itself directly support the Rockchip SoC by adding RK's compatible value to the list of compatible values that the EQoS driver supports, along with new probe functions etc.

Maybe this requires splitting some PHY code out of gmac_rockchip into a common/separate PHY driver? I haven't looked at the code to know if that's required.

I think this relationship is like the current designware.c and gmac_rockchip.c, except that the designware driver becomes DWC EQoS. In fact, most of the code is the same, because it is the same controller, and there are a few differences related to Soc implemented in gmac_rockchip.c, this is the purpose of my series of patches, and the code must be compatible with the previous Rockchip Socs.

1828

Age (days ago)

1840

Last active (days ago)

List overview

Download

30 comments

3 participants

tags (0)

participants (3)

David Wu
Patrice CHOTARD
Stephen Warren