* [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC
@ 2023-04-07 11:03 Samin Guo
2023-04-07 11:03 ` [-net-next v11 1/6] dt-bindings: net: snps,dwmac: Add dwmac-5.20 version Samin Guo
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
This series adds ethernet support for the StarFive JH7110 RISC-V SoC,
which includes a dwmac-5.20 MAC driver (from Synopsys DesignWare).
This series has been tested and works fine on VisionFive-2 v1.2A and
v1.3B SBC boards.
For more information and support, you can visit RVspace wiki[1].
You can simply review or test the patches at the link [2].
This patchset should be applied after the patchset [3] [4].
[1]: https://wiki.rvspace.org/
[2]: https://github.com/saminGuo/linux/tree/vf2-6.3rc4-gmac-net-next
[3]: https://patchwork.kernel.org/project/linux-riscv/cover/20230401111934.130844-1-hal.feng@starfivetech.com
[4]: https://patchwork.kernel.org/project/linux-riscv/cover/20230315055813.94740-1-william.qiu@starfivetech.com
Changes since v10:
Patch 4:
- Required goes after properties (by Krzysztof Kozlowski)
Patch 5:
- Fixed the wrong binding filename (by Conor Dooley)
Changes history:
Changes since v9:
- Rebased on tag v6.3-rc4.
Patch 5:
- Cleaned up all the error handling code by dev_err_probe (by Emil).
- Fixed the warning that clang uses W=1 because 'rate' may not be initialized (by Jakub Kicinski)
Changes since v8:
Patch 4:
- Dropped irrelevant MAINTAINERS context (by Jakub Kicinski)
Patch 6:
- Moved error messages to starfive_dwmac_set_mode function to avoid getting error
messages twice when syscon_regmap_lookup_by_phandle_args fails. (by Emil)
- Renamed patch header to "net: stmmac: dwmac-starfive:xxx" (by Emil)
Changes since v7:
Patch 4:
- Constrained the reg/interrupts/interrupt-names properties (by Krzysztof Kozlowski)
Changes since v6:
- Sended network patches[patch 1,2,3,4,5,6] and riscv trees patches[patch 7,8] separately (by Jakub Kicinski)
Changes since v5:
- Dropped "depends on STMMAC_ETH" and compiled DWMAC_STARFIVE to m by default (by Emil)
- Removed clk_gtx in struct starfive_dwmac due to this pointer is only set, but never read. (by Emil)
- Only setting the plat_dat->fix_mac_speed callback when it is needed (by Emil)
- Moved mdio/phy nodes from SOC .dtsi into board .dtsi (by Andrew)
- Modified the parameters passed by starfive,syscon (by Andrew && Emil)
<syscon, offset, mask> ==> <syscon, offset, shift>
- Optimized the patchs(Fewer patches from 12 to 8)
1)merged patch-7 into patch-4 (by Rob)
2)merged patch-9 into patch-5
2)merged patch-11,12 into patch-10
3)Adjusted the patchs order
- Fixed the unevaluatedProperties property from true to false (by Rob)
- Replaced contains:enum with items:const for reset-names in snps,dwmac.yaml (by Rob)
- Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
Changes since v4:
- Supported both visionfive 2 v1.2A and visionfive 2 v1.3B.
- Reworded the maxitems number of resets property in 'snps,dwmac.yaml'.
- Suggested by Emil, dropped the _PLAT/_plat from the config/function/struct/file names.
- Suggested by Emil, added MODULE_DEVICE_TABLE().
- Suggested by Emil, dropped clk_gtxclk and use clk_tx_inv to set the clock frequency.
- Added phy interface mode configuration function.
- Rebased on tag v6.2.
Changes since v3:
- Reworded the maxitems number of resets property in 'snps,dwmac.yaml'
- Removed the unused code in 'dwmac-starfive-plat.c'.
- Reworded the return statement in 'starfive_eth_plat_fix_mac_speed' function.
Changes since v2:
- Renamed the dt-bindings 'starfive,jh71x0-dwmac.yaml' to 'starfive,jh7110-dwmac.yaml'.
- Reworded the commit messages.
- Reworded the example context in the dt-binding 'starfive,jh7110-dwmac.yaml'.
- Removed "starfive,jh7100-dwmac" compatible string and special initialization of jh7100.
- Removed the parts of YT8531,so dropped patch 5 and 6.
- Reworded the maxitems number of resets property in 'snps,dwmac.yaml'.
Changes since v1:
- Recovered the author of the 1st and 3rd patches back to Emil Renner Berthing.
- Added a new patch to update maxitems number of resets property in 'snps,dwmac.yaml'.
- Fixed the check errors reported by "make dt_binding_check".
- Renamed the dt-binding 'starfive,dwmac-plat.yaml' to 'starfive,jh71x0-dwmac.yaml'.
- Updated the example context in the dt-binding 'starfive,jh71x0-dwmac.yaml'.
- Added new dt-binding 'motorcomm,yt8531.yaml' to describe details of phy clock
delay configuration parameters.
- Added more comments for PHY driver setting. For more details, see
'motorcomm,yt8531.yaml'.
- Moved mdio device tree node from 'jh7110-starfive-visionfive-v2.dts' to 'jh7110.dtsi'.
- Re-worded the commit message of several patches.
- Renamed all the functions with starfive_eth_plat prefix in 'dwmac-starfive-plat.c'.
- Added "starfive,jh7100-dwmac" compatible string and special init to support JH7100.
Previous versions:
v1 - https://patchwork.kernel.org/project/linux-riscv/cover/20221201090242.2381-1-yanhong.wang@starfivetech.com/
v2 - https://patchwork.kernel.org/project/linux-riscv/cover/20221216070632.11444-1-yanhong.wang@starfivetech.com/
v3 - https://patchwork.kernel.org/project/linux-riscv/cover/20230106030001.1952-1-yanhong.wang@starfivetech.com/
v4 - https://patchwork.kernel.org/project/linux-riscv/cover/20230118061701.30047-1-yanhong.wang@starfivetech.com/
v5 - https://patchwork.kernel.org/project/netdevbpf/cover/20230303085928.4535-1-samin.guo@starfivetech.com/
v6 - https://patchwork.kernel.org/project/netdevbpf/cover/20230313034645.5469-1-samin.guo@starfivetech.com/
v7 - https://patchwork.kernel.org/project/netdevbpf/cover/20230316043714.24279-1-samin.guo@starfivetech.com/
v8 - https://patchwork.kernel.org/project/linux-riscv/cover/20230324022819.2324-1-samin.guo@starfivetech.com/
v9 - https://patchwork.kernel.org/project/linux-riscv/cover/20230328062009.25454-1-samin.guo@starfivetech.com/
v10 - https://patchwork.kernel.org/project/linux-riscv/cover/20230403065932.7187-1-samin.guo@starfivetech.com/
Emil Renner Berthing (2):
dt-bindings: net: snps,dwmac: Add dwmac-5.20 version
net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string
Samin Guo (3):
dt-bindings: net: snps,dwmac: Add 'ahb' reset/reset-name
net: stmmac: Add glue layer for StarFive JH7110 SoC
net: stmmac: starfive-dmac: Add phy interface settings
Yanhong Wang (1):
dt-bindings: net: Add support StarFive dwmac
.../devicetree/bindings/net/snps,dwmac.yaml | 17 +-
.../bindings/net/starfive,jh7110-dwmac.yaml | 144 +++++++++++++++
MAINTAINERS | 7 +
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 171 ++++++++++++++++++
.../ethernet/stmicro/stmmac/stmmac_platform.c | 3 +-
7 files changed, 350 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
base-commit: 522c3534e9e1123cd03999016b08ce811058dfe0
prerequisite-patch-id: 55390537360f25c8b9cbfdc30b73ade004f436f7
prerequisite-patch-id: bb939c0c7c26b08addfccd890f9d3974b6eaec53
prerequisite-patch-id: 8a6f135bcabdad4a4bfb21f0c6a0ffd2bb57efe7
prerequisite-patch-id: c2366f993a9d85e28c06d8d09f064dd5e8b29a61
prerequisite-patch-id: 50d53a21f91f4087fc80b6f1f72864adfb0002b9
prerequisite-patch-id: 0df3703af91c30f1ca2c47f5609012f2d7200028
prerequisite-patch-id: 89f049f951e5acf75aab92541992f816fd0acc0d
prerequisite-patch-id: 551fae54377090044c3612fca9740a9b359abdd2
prerequisite-patch-id: c7fdf904f398d478f0ed6d57eb878982bc73329d
prerequisite-patch-id: 1b2d0982b18da060c82134f05bf3ce16425bac8d
prerequisite-patch-id: 090ba4b78d47bc19204916e76fdbc70021785388
prerequisite-patch-id: a5d9e0f7d4f8163f566678894cf693015119f2d9
prerequisite-patch-id: 4637a8fa2334a45fa6b64351f4e9e28d3e2d60d3
prerequisite-patch-id: 32647ec60a3b614e1c59ec8e54cb511ae832c22f
prerequisite-patch-id: aa06658ecf89c92d0dfdd6a4ba6d9e6e67532971
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: e7773c977a7b37692e9792b21cc4f17fa58f9215
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 9f911969d0a550648493952c99096d26e05d4d83
prerequisite-patch-id: 41eddeabff082d08a76d8da523f90da4b5218d28
prerequisite-patch-id: 999c243dca89d56d452aa52ea3e181358b5c1d80
prerequisite-patch-id: 1be0fb49e0fbe293ca8fa94601e191b13c8c67d9
--
2.17.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [-net-next v11 1/6] dt-bindings: net: snps,dwmac: Add dwmac-5.20 version
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
@ 2023-04-07 11:03 ` Samin Guo
2023-04-07 11:03 ` [-net-next v11 2/6] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string Samin Guo
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
From: Emil Renner Berthing <kernel@esmil.dk>
Add dwmac-5.20 IP version to snps.dwmac.yaml
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
Documentation/devicetree/bindings/net/snps,dwmac.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 16b7d2904696..01b056ab71f7 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -30,6 +30,7 @@ select:
- snps,dwmac-4.10a
- snps,dwmac-4.20a
- snps,dwmac-5.10a
+ - snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
@@ -87,6 +88,7 @@ properties:
- snps,dwmac-4.10a
- snps,dwmac-4.20a
- snps,dwmac-5.10a
+ - snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
@@ -575,6 +577,7 @@ allOf:
- snps,dwmac-3.50a
- snps,dwmac-4.10a
- snps,dwmac-4.20a
+ - snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
- st,spear600-gmac
@@ -629,6 +632,7 @@ allOf:
- snps,dwmac-4.10a
- snps,dwmac-4.20a
- snps,dwmac-5.10a
+ - snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
- st,spear600-gmac
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [-net-next v11 2/6] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
2023-04-07 11:03 ` [-net-next v11 1/6] dt-bindings: net: snps,dwmac: Add dwmac-5.20 version Samin Guo
@ 2023-04-07 11:03 ` Samin Guo
2023-04-07 11:03 ` [-net-next v11 3/6] dt-bindings: net: snps,dwmac: Add 'ahb' reset/reset-name Samin Guo
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
From: Emil Renner Berthing <kernel@esmil.dk>
Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
to define some platform data in the glue layer.
Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 067a40fe0a23..eb0b2898daa3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
of_device_is_compatible(np, "snps,dwmac-4.10a") ||
of_device_is_compatible(np, "snps,dwmac-4.20a") ||
- of_device_is_compatible(np, "snps,dwmac-5.10a")) {
+ of_device_is_compatible(np, "snps,dwmac-5.10a") ||
+ of_device_is_compatible(np, "snps,dwmac-5.20")) {
plat->has_gmac4 = 1;
plat->has_gmac = 0;
plat->pmt = 1;
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [-net-next v11 3/6] dt-bindings: net: snps,dwmac: Add 'ahb' reset/reset-name
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
2023-04-07 11:03 ` [-net-next v11 1/6] dt-bindings: net: snps,dwmac: Add dwmac-5.20 version Samin Guo
2023-04-07 11:03 ` [-net-next v11 2/6] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string Samin Guo
@ 2023-04-07 11:03 ` Samin Guo
2023-04-07 11:03 ` [-net-next v11 4/6] dt-bindings: net: Add support StarFive dwmac Samin Guo
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
According to:
stmmac_platform.c: stmmac_probe_config_dt
stmmac_main.c: stmmac_dvr_probe
dwmac controller may require one (stmmaceth) or two (stmmaceth+ahb)
reset signals, and the maxItems of resets/reset-names is going to be 2.
The gmac of Starfive Jh7110 SOC must have two resets.
it uses snps,dwmac-5.20 IP.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 01b056ab71f7..e4519cf722ab 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -133,12 +133,16 @@ properties:
- ptp_ref
resets:
- maxItems: 1
- description:
- MAC Reset signal.
+ minItems: 1
+ items:
+ - description: GMAC stmmaceth reset
+ - description: AHB reset
reset-names:
- const: stmmaceth
+ minItems: 1
+ items:
+ - const: stmmaceth
+ - const: ahb
power-domains:
maxItems: 1
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [-net-next v11 4/6] dt-bindings: net: Add support StarFive dwmac
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
` (2 preceding siblings ...)
2023-04-07 11:03 ` [-net-next v11 3/6] dt-bindings: net: snps,dwmac: Add 'ahb' reset/reset-name Samin Guo
@ 2023-04-07 11:03 ` Samin Guo
2023-04-07 11:03 ` [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC Samin Guo
2023-04-07 11:03 ` [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings Samin Guo
5 siblings, 0 replies; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
From: Yanhong Wang <yanhong.wang@starfivetech.com>
Add documentation to describe StarFive dwmac driver(GMAC).
Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 1 +
.../bindings/net/starfive,jh7110-dwmac.yaml | 144 ++++++++++++++++++
MAINTAINERS | 6 +
3 files changed, 151 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e4519cf722ab..245f7d713261 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -91,6 +91,7 @@ properties:
- snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
+ - starfive,jh7110-dwmac
reg:
minItems: 1
diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
new file mode 100644
index 000000000000..5e7cfbbebce6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
@@ -0,0 +1,144 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 StarFive Technology Co., Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/starfive,jh7110-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 DWMAC glue layer
+
+maintainers:
+ - Emil Renner Berthing <kernel@esmil.dk>
+ - Samin Guo <samin.guo@starfivetech.com>
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - starfive,jh7110-dwmac
+ required:
+ - compatible
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - starfive,jh7110-dwmac
+ - const: snps,dwmac-5.20
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: GMAC main clock
+ - description: GMAC AHB clock
+ - description: PTP clock
+ - description: TX clock
+ - description: GTX clock
+
+ clock-names:
+ items:
+ - const: stmmaceth
+ - const: pclk
+ - const: ptp_ref
+ - const: tx
+ - const: gtx
+
+ interrupts:
+ minItems: 3
+ maxItems: 3
+
+ interrupt-names:
+ minItems: 3
+ maxItems: 3
+
+ resets:
+ items:
+ - description: MAC Reset signal.
+ - description: AHB Reset signal.
+
+ reset-names:
+ items:
+ - const: stmmaceth
+ - const: ahb
+
+ starfive,tx-use-rgmii-clk:
+ description:
+ Tx clock is provided by external rgmii clock.
+ type: boolean
+
+ starfive,syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ - items:
+ - description: phandle to syscon that configures phy mode
+ - description: Offset of phy mode selection
+ - description: Shift of phy mode selection
+ description:
+ A phandle to syscon with two arguments that configure phy mode.
+ The argument one is the offset of phy mode selection, the
+ argument two is the shift of phy mode selection.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - resets
+ - reset-names
+
+allOf:
+ - $ref: snps,dwmac.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ ethernet@16030000 {
+ compatible = "starfive,jh7110-dwmac", "snps,dwmac-5.20";
+ reg = <0x16030000 0x10000>;
+ clocks = <&clk 3>, <&clk 2>, <&clk 109>,
+ <&clk 6>, <&clk 111>;
+ clock-names = "stmmaceth", "pclk", "ptp_ref",
+ "tx", "gtx";
+ resets = <&rst 1>, <&rst 2>;
+ reset-names = "stmmaceth", "ahb";
+ interrupts = <7>, <6>, <5>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+ phy-mode = "rgmii-id";
+ snps,multicast-filter-bins = <64>;
+ snps,perfect-filter-entries = <8>;
+ rx-fifo-depth = <2048>;
+ tx-fifo-depth = <2048>;
+ snps,fixed-burst;
+ snps,no-pbl-x8;
+ snps,tso;
+ snps,force_thresh_dma_mode;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,en-tx-lpi-clockgating;
+ snps,txpbl = <16>;
+ snps,rxpbl = <16>;
+ starfive,syscon = <&aon_syscon 0xc 0x12>;
+ phy-handle = <&phy0>;
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+
+ phy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+ };
+
+ stmmac_axi_setup: stmmac-axi-config {
+ snps,lpi_en;
+ snps,wr_osr_lmt = <4>;
+ snps,rd_osr_lmt = <4>;
+ snps,blen = <256 128 64 32 0 0 0>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 1dc8bd26b6cf..6b6b67468b8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19905,6 +19905,12 @@ M: Emil Renner Berthing <kernel@esmil.dk>
S: Maintained
F: arch/riscv/boot/dts/starfive/
+STARFIVE DWMAC GLUE LAYER
+M: Emil Renner Berthing <kernel@esmil.dk>
+M: Samin Guo <samin.guo@starfivetech.com>
+S: Maintained
+F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+
STARFIVE JH7100 CLOCK DRIVERS
M: Emil Renner Berthing <kernel@esmil.dk>
S: Maintained
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
` (3 preceding siblings ...)
2023-04-07 11:03 ` [-net-next v11 4/6] dt-bindings: net: Add support StarFive dwmac Samin Guo
@ 2023-04-07 11:03 ` Samin Guo
2023-04-07 19:11 ` Emil Renner Berthing
2023-04-07 11:03 ` [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings Samin Guo
5 siblings, 1 reply; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
MAINTAINERS | 1 +
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
4 files changed, 137 insertions(+)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6b6b67468b8f..46b366456cee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
M: Samin Guo <samin.guo@starfivetech.com>
S: Maintained
F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
STARFIVE JH7100 CLOCK DRIVERS
M: Emil Renner Berthing <kernel@esmil.dk>
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index f77511fe4e87..5f5a997f21f3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
for the stmmac device driver. This driver is used for
arria5 and cyclone5 FPGA SoCs.
+config DWMAC_STARFIVE
+ tristate "StarFive dwmac support"
+ depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
+ select MFD_SYSCON
+ default m if ARCH_STARFIVE
+ help
+ Support for ethernet controllers on StarFive RISC-V SoCs
+
+ This selects the StarFive platform specific glue layer support for
+ the stmmac device driver. This driver is used for StarFive JH7110
+ ethernet controller.
+
config DWMAC_STI
tristate "STi GMAC support"
default ARCH_STI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 057e4bab5c08..8738fdbb4b2d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
+obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
new file mode 100644
index 000000000000..4963d4008485
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * StarFive DWMAC platform driver
+ *
+ * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ *
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+#include "stmmac_platform.h"
+
+struct starfive_dwmac {
+ struct device *dev;
+ struct clk *clk_tx;
+};
+
+static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
+{
+ struct starfive_dwmac *dwmac = priv;
+ unsigned long rate;
+ int err;
+
+ rate = clk_get_rate(dwmac->clk_tx);
+
+ switch (speed) {
+ case SPEED_1000:
+ rate = 125000000;
+ break;
+ case SPEED_100:
+ rate = 25000000;
+ break;
+ case SPEED_10:
+ rate = 2500000;
+ break;
+ default:
+ dev_err(dwmac->dev, "invalid speed %u\n", speed);
+ break;
+ }
+
+ err = clk_set_rate(dwmac->clk_tx, rate);
+ if (err)
+ dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
+}
+
+static int starfive_dwmac_probe(struct platform_device *pdev)
+{
+ struct plat_stmmacenet_data *plat_dat;
+ struct stmmac_resources stmmac_res;
+ struct starfive_dwmac *dwmac;
+ struct clk *clk_gtx;
+ int err;
+
+ err = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (err)
+ return dev_err_probe(&pdev->dev, err,
+ "failed to get resources\n");
+
+ plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
+ "dt configuration failed\n");
+
+ dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+ if (!dwmac)
+ return -ENOMEM;
+
+ dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
+ if (IS_ERR(dwmac->clk_tx))
+ return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
+ "error getting tx clock\n");
+
+ clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
+ if (IS_ERR(clk_gtx))
+ return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
+ "error getting gtx clock\n");
+
+ /* Generally, the rgmii_tx clock is provided by the internal clock,
+ * which needs to match the corresponding clock frequency according
+ * to different speeds. If the rgmii_tx clock is provided by the
+ * external rgmii_rxin, there is no need to configure the clock
+ * internally, because rgmii_rxin will be adaptively adjusted.
+ */
+ if (!device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
+ plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed;
+
+ dwmac->dev = &pdev->dev;
+ plat_dat->bsp_priv = dwmac;
+ plat_dat->dma_cfg->dche = true;
+
+ err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (err) {
+ stmmac_remove_config_dt(pdev, plat_dat);
+ return err;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id starfive_dwmac_match[] = {
+ { .compatible = "starfive,jh7110-dwmac" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
+
+static struct platform_driver starfive_dwmac_driver = {
+ .probe = starfive_dwmac_probe,
+ .remove = stmmac_pltfr_remove,
+ .driver = {
+ .name = "starfive-dwmac",
+ .pm = &stmmac_pltfr_pm_ops,
+ .of_match_table = starfive_dwmac_match,
+ },
+};
+module_platform_driver(starfive_dwmac_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("StarFive DWMAC platform driver");
+MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
+MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>");
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
` (4 preceding siblings ...)
2023-04-07 11:03 ` [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC Samin Guo
@ 2023-04-07 11:03 ` Samin Guo
2023-04-07 19:16 ` Emil Renner Berthing
5 siblings, 1 reply; 15+ messages in thread
From: Samin Guo @ 2023-04-07 11:03 UTC (permalink / raw)
To: linux-kernel, linux-riscv, devicetree, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Emil Renner Berthing,
Pedro Moreira, Richard Cochran, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Andrew Lunn, Heiner Kallweit,
Peter Geis, Yanhong Wang, Samin Guo, Tommaso Merciai
dwmac supports multiple modess. When working under rmii and rgmii,
you need to set different phy interfaces.
According to the dwmac document, when working in rmii, it needs to be
set to 0x4, and rgmii needs to be set to 0x1.
The phy interface needs to be set in syscon, the format is as follows:
starfive,syscon: <&syscon, offset, shift>
Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
---
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 48 +++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
index 4963d4008485..d6a1eddb51e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -13,6 +13,10 @@
#include "stmmac_platform.h"
+#define STARFIVE_DWMAC_PHY_INFT_RGMII 0x1
+#define STARFIVE_DWMAC_PHY_INFT_RMII 0x4
+#define STARFIVE_DWMAC_PHY_INFT_FIELD 0x7U
+
struct starfive_dwmac {
struct device *dev;
struct clk *clk_tx;
@@ -46,6 +50,46 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
}
+static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+ struct starfive_dwmac *dwmac = plat_dat->bsp_priv;
+ struct regmap *regmap;
+ unsigned int args[2];
+ unsigned int mode;
+ int err;
+
+ switch (plat_dat->interface) {
+ case PHY_INTERFACE_MODE_RMII:
+ mode = STARFIVE_DWMAC_PHY_INFT_RMII;
+ break;
+
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ mode = STARFIVE_DWMAC_PHY_INFT_RGMII;
+ break;
+
+ default:
+ dev_err(dwmac->dev, "unsupported interface %d\n",
+ plat_dat->interface);
+ return -EINVAL;
+ }
+
+ regmap = syscon_regmap_lookup_by_phandle_args(dwmac->dev->of_node,
+ "starfive,syscon",
+ 2, args);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dwmac->dev, PTR_ERR(regmap), "syscon regmap failed\n");
+
+ /* args[0]:offset args[1]: shift */
+ err = regmap_update_bits(regmap, args[0],
+ STARFIVE_DWMAC_PHY_INFT_FIELD << args[1],
+ mode << args[1]);
+ if (err)
+ return dev_err_probe(dwmac->dev, err, "error setting phy mode\n");
+
+ return 0;
+}
+
static int starfive_dwmac_probe(struct platform_device *pdev)
{
struct plat_stmmacenet_data *plat_dat;
@@ -91,6 +135,10 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
plat_dat->bsp_priv = dwmac;
plat_dat->dma_cfg->dche = true;
+ err = starfive_dwmac_set_mode(plat_dat);
+ if (err)
+ return err;
+
err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
if (err) {
stmmac_remove_config_dt(pdev, plat_dat);
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-07 11:03 ` [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC Samin Guo
@ 2023-04-07 19:11 ` Emil Renner Berthing
2023-04-08 1:15 ` Guo Samin
0 siblings, 1 reply; 15+ messages in thread
From: Emil Renner Berthing @ 2023-04-07 19:11 UTC (permalink / raw)
To: Samin Guo
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Emil Renner Berthing, Pedro Moreira,
Richard Cochran, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Andrew Lunn, Heiner Kallweit, Peter Geis,
Yanhong Wang, Tommaso Merciai
On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
>
> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>
> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> ---
> MAINTAINERS | 1 +
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
> 4 files changed, 137 insertions(+)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6b6b67468b8f..46b366456cee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
> M: Samin Guo <samin.guo@starfivetech.com>
> S: Maintained
> F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>
> STARFIVE JH7100 CLOCK DRIVERS
> M: Emil Renner Berthing <kernel@esmil.dk>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index f77511fe4e87..5f5a997f21f3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
> for the stmmac device driver. This driver is used for
> arria5 and cyclone5 FPGA SoCs.
>
> +config DWMAC_STARFIVE
> + tristate "StarFive dwmac support"
> + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
> + select MFD_SYSCON
> + default m if ARCH_STARFIVE
> + help
> + Support for ethernet controllers on StarFive RISC-V SoCs
> +
> + This selects the StarFive platform specific glue layer support for
> + the stmmac device driver. This driver is used for StarFive JH7110
> + ethernet controller.
> +
> config DWMAC_STI
> tristate "STi GMAC support"
> default ARCH_STI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index 057e4bab5c08..8738fdbb4b2d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
> obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
> obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
> obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
> +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
> obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
> obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
> obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> new file mode 100644
> index 000000000000..4963d4008485
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * StarFive DWMAC platform driver
> + *
> + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + *
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +#include "stmmac_platform.h"
> +
> +struct starfive_dwmac {
> + struct device *dev;
> + struct clk *clk_tx;
> +};
> +
> +static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
> +{
> + struct starfive_dwmac *dwmac = priv;
> + unsigned long rate;
> + int err;
> +
> + rate = clk_get_rate(dwmac->clk_tx);
Hi Samin,
I'm not sure why you added this line in this revision. If it's just to
not call clk_set_rate on the uninitialized rate, I'd much rather you
just returned early and don't call clk_set_rate at all in case of
errors.
> +
> + switch (speed) {
> + case SPEED_1000:
> + rate = 125000000;
> + break;
> + case SPEED_100:
> + rate = 25000000;
> + break;
> + case SPEED_10:
> + rate = 2500000;
> + break;
> + default:
> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> + break;
That is skip the clk_get_rate above and just change this break to a return.
> + }
> +
> + err = clk_set_rate(dwmac->clk_tx, rate);
> + if (err)
> + dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
> +}
> +
> +static int starfive_dwmac_probe(struct platform_device *pdev)
> +{
> + struct plat_stmmacenet_data *plat_dat;
> + struct stmmac_resources stmmac_res;
> + struct starfive_dwmac *dwmac;
> + struct clk *clk_gtx;
> + int err;
> +
> + err = stmmac_get_platform_resources(pdev, &stmmac_res);
> + if (err)
> + return dev_err_probe(&pdev->dev, err,
> + "failed to get resources\n");
> +
> + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> + if (IS_ERR(plat_dat))
> + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
> + "dt configuration failed\n");
> +
> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> + if (!dwmac)
> + return -ENOMEM;
> +
> + dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
> + if (IS_ERR(dwmac->clk_tx))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
> + "error getting tx clock\n");
> +
> + clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
> + if (IS_ERR(clk_gtx))
> + return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
> + "error getting gtx clock\n");
> +
> + /* Generally, the rgmii_tx clock is provided by the internal clock,
> + * which needs to match the corresponding clock frequency according
> + * to different speeds. If the rgmii_tx clock is provided by the
> + * external rgmii_rxin, there is no need to configure the clock
> + * internally, because rgmii_rxin will be adaptively adjusted.
> + */
> + if (!device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
> + plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed;
> +
> + dwmac->dev = &pdev->dev;
> + plat_dat->bsp_priv = dwmac;
> + plat_dat->dma_cfg->dche = true;
> +
> + err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> + if (err) {
> + stmmac_remove_config_dt(pdev, plat_dat);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id starfive_dwmac_match[] = {
> + { .compatible = "starfive,jh7110-dwmac" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
> +
> +static struct platform_driver starfive_dwmac_driver = {
> + .probe = starfive_dwmac_probe,
> + .remove = stmmac_pltfr_remove,
> + .driver = {
> + .name = "starfive-dwmac",
> + .pm = &stmmac_pltfr_pm_ops,
> + .of_match_table = starfive_dwmac_match,
> + },
> +};
> +module_platform_driver(starfive_dwmac_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
> +MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
> +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>");
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings
2023-04-07 11:03 ` [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings Samin Guo
@ 2023-04-07 19:16 ` Emil Renner Berthing
2023-04-08 1:24 ` Guo Samin
0 siblings, 1 reply; 15+ messages in thread
From: Emil Renner Berthing @ 2023-04-07 19:16 UTC (permalink / raw)
To: Samin Guo
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Emil Renner Berthing, Pedro Moreira,
Richard Cochran, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Andrew Lunn, Heiner Kallweit, Peter Geis,
Yanhong Wang, Tommaso Merciai
Hi Samin,
If you're respinning this series anyway, please use "net: stmmac:
dwmac-starfive:" to match the filename.
On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
>
> dwmac supports multiple modess. When working under rmii and rgmii,
> you need to set different phy interfaces.
>
> According to the dwmac document, when working in rmii, it needs to be
> set to 0x4, and rgmii needs to be set to 0x1.
>
> The phy interface needs to be set in syscon, the format is as follows:
> starfive,syscon: <&syscon, offset, shift>
>
> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> ---
> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 48 +++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> index 4963d4008485..d6a1eddb51e8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> @@ -13,6 +13,10 @@
>
> #include "stmmac_platform.h"
>
> +#define STARFIVE_DWMAC_PHY_INFT_RGMII 0x1
> +#define STARFIVE_DWMAC_PHY_INFT_RMII 0x4
> +#define STARFIVE_DWMAC_PHY_INFT_FIELD 0x7U
> +
> struct starfive_dwmac {
> struct device *dev;
> struct clk *clk_tx;
> @@ -46,6 +50,46 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
> dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
> }
>
> +static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> + struct starfive_dwmac *dwmac = plat_dat->bsp_priv;
> + struct regmap *regmap;
> + unsigned int args[2];
> + unsigned int mode;
> + int err;
> +
> + switch (plat_dat->interface) {
> + case PHY_INTERFACE_MODE_RMII:
> + mode = STARFIVE_DWMAC_PHY_INFT_RMII;
> + break;
> +
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + mode = STARFIVE_DWMAC_PHY_INFT_RGMII;
> + break;
> +
> + default:
> + dev_err(dwmac->dev, "unsupported interface %d\n",
> + plat_dat->interface);
> + return -EINVAL;
> + }
> +
> + regmap = syscon_regmap_lookup_by_phandle_args(dwmac->dev->of_node,
> + "starfive,syscon",
> + 2, args);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dwmac->dev, PTR_ERR(regmap), "syscon regmap failed\n");
This message is a bit misleading. It's not actually that the regmap
failed, but getting/looking up the regmap failed.
> + /* args[0]:offset args[1]: shift */
> + err = regmap_update_bits(regmap, args[0],
> + STARFIVE_DWMAC_PHY_INFT_FIELD << args[1],
> + mode << args[1]);
> + if (err)
> + return dev_err_probe(dwmac->dev, err, "error setting phy mode\n");
> +
> + return 0;
> +}
> +
> static int starfive_dwmac_probe(struct platform_device *pdev)
> {
> struct plat_stmmacenet_data *plat_dat;
> @@ -91,6 +135,10 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
> plat_dat->bsp_priv = dwmac;
> plat_dat->dma_cfg->dche = true;
>
> + err = starfive_dwmac_set_mode(plat_dat);
> + if (err)
> + return err;
> +
> err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> if (err) {
> stmmac_remove_config_dt(pdev, plat_dat);
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-07 19:11 ` Emil Renner Berthing
@ 2023-04-08 1:15 ` Guo Samin
2023-04-08 17:30 ` Emil Renner Berthing
0 siblings, 1 reply; 15+ messages in thread
From: Guo Samin @ 2023-04-08 1:15 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Emil Renner Berthing, Pedro Moreira,
Richard Cochran, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Andrew Lunn, Heiner Kallweit, Peter Geis,
Yanhong Wang, Tommaso Merciai
Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
to: Samin Guo <samin.guo@starfivetech.com>
data: 2023/4/8
> On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
>>
>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>
>> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
>> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
>> 4 files changed, 137 insertions(+)
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6b6b67468b8f..46b366456cee 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
>> M: Samin Guo <samin.guo@starfivetech.com>
>> S: Maintained
>> F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
>> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>
>> STARFIVE JH7100 CLOCK DRIVERS
>> M: Emil Renner Berthing <kernel@esmil.dk>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index f77511fe4e87..5f5a997f21f3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
>> for the stmmac device driver. This driver is used for
>> arria5 and cyclone5 FPGA SoCs.
>>
>> +config DWMAC_STARFIVE
>> + tristate "StarFive dwmac support"
>> + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
>> + select MFD_SYSCON
>> + default m if ARCH_STARFIVE
>> + help
>> + Support for ethernet controllers on StarFive RISC-V SoCs
>> +
>> + This selects the StarFive platform specific glue layer support for
>> + the stmmac device driver. This driver is used for StarFive JH7110
>> + ethernet controller.
>> +
>> config DWMAC_STI
>> tristate "STi GMAC support"
>> default ARCH_STI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index 057e4bab5c08..8738fdbb4b2d 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
>> obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
>> obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
>> obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
>> +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
>> obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
>> obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
>> obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> new file mode 100644
>> index 000000000000..4963d4008485
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> @@ -0,0 +1,123 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * StarFive DWMAC platform driver
>> + *
>> + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + *
>> + */
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +struct starfive_dwmac {
>> + struct device *dev;
>> + struct clk *clk_tx;
>> +};
>> +
>> +static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
>> +{
>> + struct starfive_dwmac *dwmac = priv;
>> + unsigned long rate;
>> + int err;
>> +
>> + rate = clk_get_rate(dwmac->clk_tx);
>
> Hi Samin,
>
> I'm not sure why you added this line in this revision. If it's just to
> not call clk_set_rate on the uninitialized rate, I'd much rather you
> just returned early and don't call clk_set_rate at all in case of
> errors.
>
>> +
>> + switch (speed) {
>> + case SPEED_1000:
>> + rate = 125000000;
>> + break;
>> + case SPEED_100:
>> + rate = 25000000;
>> + break;
>> + case SPEED_10:
>> + rate = 2500000;
>> + break;
>> + default:
>> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
>> + break;
>
> That is skip the clk_get_rate above and just change this break to a return.
>
Hi Emil,
We used the solution you mentioned before V3, but Arun Ramadoss doesn't think that's great.
(https://patchwork.kernel.org/project/linux-riscv/patch/20230106030001.1952-6-yanhong.wang@starfivetech.com)
> +static void starfive_eth_plat_fix_mac_speed(void *priv, unsigned int
> speed)
> +{
> + struct starfive_dwmac *dwmac = priv;
> + unsigned long rate;
> + int err;
> +
> + switch (speed) {
> + case SPEED_1000:
> + rate = 125000000;
> + break;
> + case SPEED_100:
> + rate = 25000000;
> + break;
> + case SPEED_10:
> + rate = 2500000;
> + break;
> + default:
> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> + return;
Do we need to return value, since it is invalid speed. But the return
value of function is void.(Arun Ramadoss)
So in v9, after discussing with Jakub Kicinski, the clk_set_rate was used to initialize the rate.
(It is a reference to Intel's scheme: dwmac-intel-plat.c: kmb_eth_fix_mac_speed)
(https://patchwork.kernel.org/project/linux-riscv/patch/20230328062009.25454-6-samin.guo@starfivetech.com)
Best regards,
Samin
>> + }
>> +
>> + err = clk_set_rate(dwmac->clk_tx, rate);
>> + if (err)
>> + dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>> +}
>> +
>> +static int starfive_dwmac_probe(struct platform_device *pdev)
>> +{
>> + struct plat_stmmacenet_data *plat_dat;
>> + struct stmmac_resources stmmac_res;
>> + struct starfive_dwmac *dwmac;
>> + struct clk *clk_gtx;
>> + int err;
>> +
>> + err = stmmac_get_platform_resources(pdev, &stmmac_res);
>> + if (err)
>> + return dev_err_probe(&pdev->dev, err,
>> + "failed to get resources\n");
>> +
>> + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
>> + if (IS_ERR(plat_dat))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
>> + "dt configuration failed\n");
>> +
>> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> + if (!dwmac)
>> + return -ENOMEM;
>> +
>> + dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
>> + if (IS_ERR(dwmac->clk_tx))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
>> + "error getting tx clock\n");
>> +
>> + clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
>> + if (IS_ERR(clk_gtx))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
>> + "error getting gtx clock\n");
>> +
>> + /* Generally, the rgmii_tx clock is provided by the internal clock,
>> + * which needs to match the corresponding clock frequency according
>> + * to different speeds. If the rgmii_tx clock is provided by the
>> + * external rgmii_rxin, there is no need to configure the clock
>> + * internally, because rgmii_rxin will be adaptively adjusted.
>> + */
>> + if (!device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
>> + plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed;
>> +
>> + dwmac->dev = &pdev->dev;
>> + plat_dat->bsp_priv = dwmac;
>> + plat_dat->dma_cfg->dche = true;
>> +
>> + err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> + if (err) {
>> + stmmac_remove_config_dt(pdev, plat_dat);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id starfive_dwmac_match[] = {
>> + { .compatible = "starfive,jh7110-dwmac" },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
>> +
>> +static struct platform_driver starfive_dwmac_driver = {
>> + .probe = starfive_dwmac_probe,
>> + .remove = stmmac_pltfr_remove,
>> + .driver = {
>> + .name = "starfive-dwmac",
>> + .pm = &stmmac_pltfr_pm_ops,
>> + .of_match_table = starfive_dwmac_match,
>> + },
>> +};
>> +module_platform_driver(starfive_dwmac_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
>> +MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
>> +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>");
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Best regards,
Samin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings
2023-04-07 19:16 ` Emil Renner Berthing
@ 2023-04-08 1:24 ` Guo Samin
0 siblings, 0 replies; 15+ messages in thread
From: Guo Samin @ 2023-04-08 1:24 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Emil Renner Berthing, Pedro Moreira,
Richard Cochran, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Andrew Lunn, Heiner Kallweit, Peter Geis,
Yanhong Wang, Tommaso Merciai
Re: [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings
From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
to: Samin Guo <samin.guo@starfivetech.com>
data: 2023/4/8
> Hi Samin,
>
> If you're respinning this series anyway, please use "net: stmmac:
> dwmac-starfive:" to match the filename.
>
Thanks, will fix.
> On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
>>
>> dwmac supports multiple modess. When working under rmii and rgmii,
>> you need to set different phy interfaces.
>>
>> According to the dwmac document, when working in rmii, it needs to be
>> set to 0x4, and rgmii needs to be set to 0x1.
>>
>> The phy interface needs to be set in syscon, the format is as follows:
>> starfive,syscon: <&syscon, offset, shift>
>>
>> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>> ---
>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 48 +++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> index 4963d4008485..d6a1eddb51e8 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> @@ -13,6 +13,10 @@
>>
>> #include "stmmac_platform.h"
>>
>> +#define STARFIVE_DWMAC_PHY_INFT_RGMII 0x1
>> +#define STARFIVE_DWMAC_PHY_INFT_RMII 0x4
>> +#define STARFIVE_DWMAC_PHY_INFT_FIELD 0x7U
>> +
>> struct starfive_dwmac {
>> struct device *dev;
>> struct clk *clk_tx;
>> @@ -46,6 +50,46 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
>> dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>> }
>>
>> +static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> + struct starfive_dwmac *dwmac = plat_dat->bsp_priv;
>> + struct regmap *regmap;
>> + unsigned int args[2];
>> + unsigned int mode;
>> + int err;
>> +
>> + switch (plat_dat->interface) {
>> + case PHY_INTERFACE_MODE_RMII:
>> + mode = STARFIVE_DWMAC_PHY_INFT_RMII;
>> + break;
>> +
>> + case PHY_INTERFACE_MODE_RGMII:
>> + case PHY_INTERFACE_MODE_RGMII_ID:
>> + mode = STARFIVE_DWMAC_PHY_INFT_RGMII;
>> + break;
>> +
>> + default:
>> + dev_err(dwmac->dev, "unsupported interface %d\n",
>> + plat_dat->interface);
>> + return -EINVAL;
>> + }
>> +
>> + regmap = syscon_regmap_lookup_by_phandle_args(dwmac->dev->of_node,
>> + "starfive,syscon",
>> + 2, args);
>> + if (IS_ERR(regmap))
>> + return dev_err_probe(dwmac->dev, PTR_ERR(regmap), "syscon regmap failed\n");
>
> This message is a bit misleading. It's not actually that the regmap
> failed, but getting/looking up the regmap failed.
Will fix.
Best regards,
Samin
>
>> + /* args[0]:offset args[1]: shift */
>> + err = regmap_update_bits(regmap, args[0],
>> + STARFIVE_DWMAC_PHY_INFT_FIELD << args[1],
>> + mode << args[1]);
>> + if (err)
>> + return dev_err_probe(dwmac->dev, err, "error setting phy mode\n");
>> +
>> + return 0;
>> +}
>> +
>> static int starfive_dwmac_probe(struct platform_device *pdev)
>> {
>> struct plat_stmmacenet_data *plat_dat;
>> @@ -91,6 +135,10 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>> plat_dat->bsp_priv = dwmac;
>> plat_dat->dma_cfg->dche = true;
>>
>> + err = starfive_dwmac_set_mode(plat_dat);
>> + if (err)
>> + return err;
>> +
>> err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> if (err) {
>> stmmac_remove_config_dt(pdev, plat_dat);
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-08 1:15 ` Guo Samin
@ 2023-04-08 17:30 ` Emil Renner Berthing
2023-04-10 8:29 ` Guo Samin
0 siblings, 1 reply; 15+ messages in thread
From: Emil Renner Berthing @ 2023-04-08 17:30 UTC (permalink / raw)
To: Guo Samin
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Emil Renner Berthing, Pedro Moreira,
Richard Cochran, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Andrew Lunn, Heiner Kallweit, Peter Geis,
Yanhong Wang, Tommaso Merciai
On Sat, 8 Apr 2023 at 03:16, Guo Samin <samin.guo@starfivetech.com> wrote:
>
> Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
> From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> to: Samin Guo <samin.guo@starfivetech.com>
> data: 2023/4/8
>
> > On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
> >>
> >> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
> >>
> >> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
> >> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> >> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> >> ---
> >> MAINTAINERS | 1 +
> >> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> >> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> >> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
> >> 4 files changed, 137 insertions(+)
> >> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 6b6b67468b8f..46b366456cee 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
> >> M: Samin Guo <samin.guo@starfivetech.com>
> >> S: Maintained
> >> F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> >> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >>
> >> STARFIVE JH7100 CLOCK DRIVERS
> >> M: Emil Renner Berthing <kernel@esmil.dk>
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> >> index f77511fe4e87..5f5a997f21f3 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> >> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
> >> for the stmmac device driver. This driver is used for
> >> arria5 and cyclone5 FPGA SoCs.
> >>
> >> +config DWMAC_STARFIVE
> >> + tristate "StarFive dwmac support"
> >> + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
> >> + select MFD_SYSCON
> >> + default m if ARCH_STARFIVE
> >> + help
> >> + Support for ethernet controllers on StarFive RISC-V SoCs
> >> +
> >> + This selects the StarFive platform specific glue layer support for
> >> + the stmmac device driver. This driver is used for StarFive JH7110
> >> + ethernet controller.
> >> +
> >> config DWMAC_STI
> >> tristate "STi GMAC support"
> >> default ARCH_STI
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> >> index 057e4bab5c08..8738fdbb4b2d 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> >> @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
> >> obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
> >> obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
> >> obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
> >> +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
> >> obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
> >> obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
> >> obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >> new file mode 100644
> >> index 000000000000..4963d4008485
> >> --- /dev/null
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >> @@ -0,0 +1,123 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * StarFive DWMAC platform driver
> >> + *
> >> + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
> >> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> + *
> >> + */
> >> +
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/regmap.h>
> >> +
> >> +#include "stmmac_platform.h"
> >> +
> >> +struct starfive_dwmac {
> >> + struct device *dev;
> >> + struct clk *clk_tx;
> >> +};
> >> +
> >> +static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
> >> +{
> >> + struct starfive_dwmac *dwmac = priv;
> >> + unsigned long rate;
> >> + int err;
> >> +
> >> + rate = clk_get_rate(dwmac->clk_tx);
> >
> > Hi Samin,
> >
> > I'm not sure why you added this line in this revision. If it's just to
> > not call clk_set_rate on the uninitialized rate, I'd much rather you
> > just returned early and don't call clk_set_rate at all in case of
> > errors.
> >
> >> +
> >> + switch (speed) {
> >> + case SPEED_1000:
> >> + rate = 125000000;
> >> + break;
> >> + case SPEED_100:
> >> + rate = 25000000;
> >> + break;
> >> + case SPEED_10:
> >> + rate = 2500000;
> >> + break;
> >> + default:
> >> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> >> + break;
> >
> > That is skip the clk_get_rate above and just change this break to a return.
> >
>
> Hi Emil,
>
> We used the solution you mentioned before V3, but Arun Ramadoss doesn't think that's great.
> (https://patchwork.kernel.org/project/linux-riscv/patch/20230106030001.1952-6-yanhong.wang@starfivetech.com)
>
>
> > +static void starfive_eth_plat_fix_mac_speed(void *priv, unsigned int
> > speed)
> > +{
> > + struct starfive_dwmac *dwmac = priv;
> > + unsigned long rate;
> > + int err;
> > +
> > + switch (speed) {
> > + case SPEED_1000:
> > + rate = 125000000;
> > + break;
> > + case SPEED_100:
> > + rate = 25000000;
> > + break;
> > + case SPEED_10:
> > + rate = 2500000;
> > + break;
> > + default:
> > + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> > + return;
>
> Do we need to return value, since it is invalid speed. But the return
> value of function is void.(Arun Ramadoss)
>
>
> So in v9, after discussing with Jakub Kicinski, the clk_set_rate was used to initialize the rate.
> (It is a reference to Intel's scheme: dwmac-intel-plat.c: kmb_eth_fix_mac_speed)
> (https://patchwork.kernel.org/project/linux-riscv/patch/20230328062009.25454-6-samin.guo@starfivetech.com)
>
Yeah, I think this is a misunderstanding and Arun is considering if we
ought to return the error which we can't without changing generic
dwmac code, and Jakub is rightly concerned about using a local
variable uninitialized. I don't think anyone is suggesting that
getting the rate just to set it to the exact same value is better than
just leaving the clock alone.
> Best regards,
> Samin
> >> + }
> >> +
> >> + err = clk_set_rate(dwmac->clk_tx, rate);
> >> + if (err)
> >> + dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
> >> +}
> >> +
> >> +static int starfive_dwmac_probe(struct platform_device *pdev)
> >> +{
> >> + struct plat_stmmacenet_data *plat_dat;
> cons>> + struct stmmac_resources stmmac_res;
> >> + struct starfive_dwmac *dwmac;
> >> + struct clk *clk_gtx;
> >> + int err;
> >> +
> >> + err = stmmac_get_platform_resources(pdev, &stmmac_res);
> >> + if (err)
> >> + return dev_err_probe(&pdev->dev, err,
> >> + "failed to get resources\n");
> >> +
> >> + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> >> + if (IS_ERR(plat_dat))
> >> + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
> >> + "dt configuration failed\n");
> >> +
> >> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> >> + if (!dwmac)
> >> + return -ENOMEM;
> >> +
> >> + dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
> >> + if (IS_ERR(dwmac->clk_tx))
> >> + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
> >> + "error getting tx clock\n");
> >> +
> >> + clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
> >> + if (IS_ERR(clk_gtx))
> >> + return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
> >> + "error getting gtx clock\n");
> >> +
> >> + /* Generally, the rgmii_tx clock is provided by the internal clock,
> >> + * which needs to match the corresponding clock frequency according
> >> + * to different speeds. If the rgmii_tx clock is provided by the
> >> + * external rgmii_rxin, there is no need to configure the clock
> >> + * internally, because rgmii_rxin will be adaptively adjusted.
> >> + */
> >> + if (!device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
> >> + plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed;
> >> +
> >> + dwmac->dev = &pdev->dev;
> >> + plat_dat->bsp_priv = dwmac;
> >> + plat_dat->dma_cfg->dche = true;
> >> +
> >> + err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> >> + if (err) {
> >> + stmmac_remove_config_dt(pdev, plat_dat);
> >> + return err;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct of_device_id starfive_dwmac_match[] = {
> >> + { .compatible = "starfive,jh7110-dwmac" },
> >> + { /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
> >> +
> >> +static struct platform_driver starfive_dwmac_driver = {
> >> + .probe = starfive_dwmac_probe,
> >> + .remove = stmmac_pltfr_remove,
> >> + .driver = {
> >> + .name = "starfive-dwmac",
> >> + .pm = &stmmac_pltfr_pm_ops,
> >> + .of_match_table = starfive_dwmac_match,
> >> + },
> >> +};
> >> +module_platform_driver(starfive_dwmac_driver);
> >> +
> >> +MODULE_LICENSE("GPL");
> >> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
> >> +MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
> >> +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>");
> >> --
> >> 2.17.1
> >>
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> linux-riscv@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> --
> Best regards,
> Samin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-08 17:30 ` Emil Renner Berthing
@ 2023-04-10 8:29 ` Guo Samin
2023-04-11 13:16 ` Paolo Abeni
0 siblings, 1 reply; 15+ messages in thread
From: Guo Samin @ 2023-04-10 8:29 UTC (permalink / raw)
To: Emil Renner Berthing, Arun.Ramadoss
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Emil Renner Berthing, Pedro Moreira,
Richard Cochran, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Andrew Lunn, Heiner Kallweit, Peter Geis,
Yanhong Wang, Tommaso Merciai
Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
to: Guo Samin <samin.guo@starfivetech.com>
data: 2023/4/9
> On Sat, 8 Apr 2023 at 03:16, Guo Samin <samin.guo@starfivetech.com> wrote:
>>
>> Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
>> From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
>> to: Samin Guo <samin.guo@starfivetech.com>
>> data: 2023/4/8
>>
>>> On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
>>>>
>>>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>>>
>>>> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
>>>> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
>>>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>>>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>>>> ---
>>>> MAINTAINERS | 1 +
>>>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
>>>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>>>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
>>>> 4 files changed, 137 insertions(+)
>>>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 6b6b67468b8f..46b366456cee 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
>>>> M: Samin Guo <samin.guo@starfivetech.com>
>>>> S: Maintained
>>>> F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
>>>> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>>
>>>> STARFIVE JH7100 CLOCK DRIVERS
>>>> M: Emil Renner Berthing <kernel@esmil.dk>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>>>> index f77511fe4e87..5f5a997f21f3 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>>>> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
>>>> for the stmmac device driver. This driver is used for
>>>> arria5 and cyclone5 FPGA SoCs.
>>>>
>>>> +config DWMAC_STARFIVE
>>>> + tristate "StarFive dwmac support"
>>>> + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
>>>> + select MFD_SYSCON
>>>> + default m if ARCH_STARFIVE
>>>> + help
>>>> + Support for ethernet controllers on StarFive RISC-V SoCs
>>>> +
>>>> + This selects the StarFive platform specific glue layer support for
>>>> + the stmmac device driver. This driver is used for StarFive JH7110
>>>> + ethernet controller.
>>>> +
>>>> config DWMAC_STI
>>>> tristate "STi GMAC support"
>>>> default ARCH_STI
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>>>> index 057e4bab5c08..8738fdbb4b2d 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>>>> @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
>>>> obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
>>>> obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
>>>> obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
>>>> +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
>>>> obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
>>>> obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
>>>> obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>> new file mode 100644
>>>> index 000000000000..4963d4008485
>>>> --- /dev/null
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>> @@ -0,0 +1,123 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * StarFive DWMAC platform driver
>>>> + *
>>>> + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
>>>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/mfd/syscon.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/regmap.h>
>>>> +
>>>> +#include "stmmac_platform.h"
>>>> +
>>>> +struct starfive_dwmac {
>>>> + struct device *dev;
>>>> + struct clk *clk_tx;
>>>> +};
>>>> +
>>>> +static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
>>>> +{
>>>> + struct starfive_dwmac *dwmac = priv;
>>>> + unsigned long rate;
>>>> + int err;
>>>> +
>>>> + rate = clk_get_rate(dwmac->clk_tx);
>>>
>>> Hi Samin,
>>>
>>> I'm not sure why you added this line in this revision. If it's just to
>>> not call clk_set_rate on the uninitialized rate, I'd much rather you
>>> just returned early and don't call clk_set_rate at all in case of
>>> errors.
>>>
>>>> +
>>>> + switch (speed) {
>>>> + case SPEED_1000:
>>>> + rate = 125000000;
>>>> + break;
>>>> + case SPEED_100:
>>>> + rate = 25000000;
>>>> + break;
>>>> + case SPEED_10:
>>>> + rate = 2500000;
>>>> + break;
>>>> + default:
>>>> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
>>>> + break;
>>>
>>> That is skip the clk_get_rate above and just change this break to a return.
>>>
>>
>> Hi Emil,
>>
>> We used the solution you mentioned before V3, but Arun Ramadoss doesn't think that's great.
>> (https://patchwork.kernel.org/project/linux-riscv/patch/20230106030001.1952-6-yanhong.wang@starfivetech.com)
>>
>>
>>> +static void starfive_eth_plat_fix_mac_speed(void *priv, unsigned int
>>> speed)
>>> +{
>>> + struct starfive_dwmac *dwmac = priv;
>>> + unsigned long rate;
>>> + int err;
>>> +
>>> + switch (speed) {
>>> + case SPEED_1000:
>>> + rate = 125000000;
>>> + break;
>>> + case SPEED_100:
>>> + rate = 25000000;
>>> + break;
>>> + case SPEED_10:
>>> + rate = 2500000;
>>> + break;
>>> + default:
>>> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
>>> + return;
>>
>> Do we need to return value, since it is invalid speed. But the return
>> value of function is void.(Arun Ramadoss)
>>
>>
>> So in v9, after discussing with Jakub Kicinski, the clk_set_rate was used to initialize the rate.
>> (It is a reference to Intel's scheme: dwmac-intel-plat.c: kmb_eth_fix_mac_speed)
>> (https://patchwork.kernel.org/project/linux-riscv/patch/20230328062009.25454-6-samin.guo@starfivetech.com)
>>
>
> Yeah, I think this is a misunderstanding and Arun is considering if we
> ought to return the error which we can't without changing generic
> dwmac code, and Jakub is rightly concerned about using a local
> variable uninitialized. I don't think anyone is suggesting that
> getting the rate just to set it to the exact same value is better than
> just leaving the clock alone.
>
HI Emil,
Yeah, return early saves time and code complexity, and seems like a good solution so Yanhong did the same before v3. (Jakub has suggested it before),
I wonder if Arun or other maintainers accept this solution or if there are other solutions?
Best regards,
Samin
>> Best regards,
>> Samin
>>>> + }
>>>> +
>>>> + err = clk_set_rate(dwmac->clk_tx, rate);
>>>> + if (err)
>>>> + dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>>>> +}
>>>> +
>>>> +static int starfive_dwmac_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct plat_stmmacenet_data *plat_dat;
>> cons>> + struct stmmac_resources stmmac_res;
>>>> + struct starfive_dwmac *dwmac;
>>>> + struct clk *clk_gtx;
>>>> + int err;
>>>> +
>>>> + err = stmmac_get_platform_resources(pdev, &stmmac_res);
>>>> + if (err)
>>>> + return dev_err_probe(&pdev->dev, err,
>>>> + "failed to get resources\n");
>>>> +
>>>> + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
>>>> + if (IS_ERR(plat_dat))
>>>> + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
>>>> + "dt configuration failed\n");
>>>> +
>>>> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>>>> + if (!dwmac)
>>>> + return -ENOMEM;
>>>> +
>>>> + dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
>>>> + if (IS_ERR(dwmac->clk_tx))
>>>> + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
>>>> + "error getting tx clock\n");
>>>> +
>>>> + clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
>>>> + if (IS_ERR(clk_gtx))
>>>> + return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx),
>>>> + "error getting gtx clock\n");
>>>> +
>>>> + /* Generally, the rgmii_tx clock is provided by the internal clock,
>>>> + * which needs to match the corresponding clock frequency according
>>>> + * to different speeds. If the rgmii_tx clock is provided by the
>>>> + * external rgmii_rxin, there is no need to configure the clock
>>>> + * internally, because rgmii_rxin will be adaptively adjusted.
>>>> + */
>>>> + if (!device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
>>>> + plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed;
>>>> +
>>>> + dwmac->dev = &pdev->dev;
>>>> + plat_dat->bsp_priv = dwmac;
>>>> + plat_dat->dma_cfg->dche = true;
>>>> +
>>>> + err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>>>> + if (err) {
>>>> + stmmac_remove_config_dt(pdev, plat_dat);
>>>> + return err;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id starfive_dwmac_match[] = {
>>>> + { .compatible = "starfive,jh7110-dwmac" },
>>>> + { /* sentinel */ }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
>>>> +
>>>> +static struct platform_driver starfive_dwmac_driver = {
>>>> + .probe = starfive_dwmac_probe,
>>>> + .remove = stmmac_pltfr_remove,
>>>> + .driver = {
>>>> + .name = "starfive-dwmac",
>>>> + .pm = &stmmac_pltfr_pm_ops,
>>>> + .of_match_table = starfive_dwmac_match,
>>>> + },
>>>> +};
>>>> +module_platform_driver(starfive_dwmac_driver);
>>>> +
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
>>>> +MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
>>>> +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>");
>>>> --
>>>> 2.17.1
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-riscv mailing list
>>>> linux-riscv@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>> --
>> Best regards,
>> Samin
--
Best regards,
Samin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-10 8:29 ` Guo Samin
@ 2023-04-11 13:16 ` Paolo Abeni
2023-04-11 14:37 ` Guo Samin
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2023-04-11 13:16 UTC (permalink / raw)
To: Guo Samin, Emil Renner Berthing, Arun.Ramadoss
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Rob Herring, Krzysztof Kozlowski,
Emil Renner Berthing, Pedro Moreira, Richard Cochran,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Andrew Lunn, Heiner Kallweit, Peter Geis, Yanhong Wang,
Tommaso Merciai
On Mon, 2023-04-10 at 16:29 +0800, Guo Samin wrote:
> Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
> From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> to: Guo Samin <samin.guo@starfivetech.com>
> data: 2023/4/9
>
> > On Sat, 8 Apr 2023 at 03:16, Guo Samin <samin.guo@starfivetech.com> wrote:
> > >
> > > Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
> > > From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > > to: Samin Guo <samin.guo@starfivetech.com>
> > > data: 2023/4/8
> > >
> > > > On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
> > > > >
> > > > > This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
> > > > >
> > > > > Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > > > > Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > > > Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> > > > > ---
> > > > > MAINTAINERS | 1 +
> > > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> > > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> > > > > .../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
> > > > > 4 files changed, 137 insertions(+)
> > > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 6b6b67468b8f..46b366456cee 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
> > > > > M: Samin Guo <samin.guo@starfivetech.com>
> > > > > S: Maintained
> > > > > F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > > > > +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> > > > >
> > > > > STARFIVE JH7100 CLOCK DRIVERS
> > > > > M: Emil Renner Berthing <kernel@esmil.dk>
> > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > > > index f77511fe4e87..5f5a997f21f3 100644
> > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > > > @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
> > > > > for the stmmac device driver. This driver is used for
> > > > > arria5 and cyclone5 FPGA SoCs.
> > > > >
> > > > > +config DWMAC_STARFIVE
> > > > > + tristate "StarFive dwmac support"
> > > > > + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
> > > > > + select MFD_SYSCON
> > > > > + default m if ARCH_STARFIVE
> > > > > + help
> > > > > + Support for ethernet controllers on StarFive RISC-V SoCs
> > > > > +
> > > > > + This selects the StarFive platform specific glue layer support for
> > > > > + the stmmac device driver. This driver is used for StarFive JH7110
> > > > > + ethernet controller.
> > > > > +
> > > > > config DWMAC_STI
> > > > > tristate "STi GMAC support"
> > > > > default ARCH_STI
> > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > > > index 057e4bab5c08..8738fdbb4b2d 100644
> > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > > > @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
> > > > > obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
> > > > > obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
> > > > > obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
> > > > > +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
> > > > > obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
> > > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
> > > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
> > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> > > > > new file mode 100644
> > > > > index 000000000000..4963d4008485
> > > > > --- /dev/null
> > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> > > > > @@ -0,0 +1,123 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * StarFive DWMAC platform driver
> > > > > + *
> > > > > + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
> > > > > + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#include <linux/mfd/syscon.h>
> > > > > +#include <linux/of_device.h>
> > > > > +#include <linux/regmap.h>
> > > > > +
> > > > > +#include "stmmac_platform.h"
> > > > > +
> > > > > +struct starfive_dwmac {
> > > > > + struct device *dev;
> > > > > + struct clk *clk_tx;
> > > > > +};
> > > > > +
> > > > > +static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
> > > > > +{
> > > > > + struct starfive_dwmac *dwmac = priv;
> > > > > + unsigned long rate;
> > > > > + int err;
> > > > > +
> > > > > + rate = clk_get_rate(dwmac->clk_tx);
> > > >
> > > > Hi Samin,
> > > >
> > > > I'm not sure why you added this line in this revision. If it's just to
> > > > not call clk_set_rate on the uninitialized rate, I'd much rather you
> > > > just returned early and don't call clk_set_rate at all in case of
> > > > errors.
> > > >
> > > > > +
> > > > > + switch (speed) {
> > > > > + case SPEED_1000:
> > > > > + rate = 125000000;
> > > > > + break;
> > > > > + case SPEED_100:
> > > > > + rate = 25000000;
> > > > > + break;
> > > > > + case SPEED_10:
> > > > > + rate = 2500000;
> > > > > + break;
> > > > > + default:
> > > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> > > > > + break;
> > > >
> > > > That is skip the clk_get_rate above and just change this break to a return.
> > > >
> > >
> > > Hi Emil,
> > >
> > > We used the solution you mentioned before V3, but Arun Ramadoss doesn't think that's great.
> > > (https://patchwork.kernel.org/project/linux-riscv/patch/20230106030001.1952-6-yanhong.wang@starfivetech.com)
> > >
> > >
> > > > +static void starfive_eth_plat_fix_mac_speed(void *priv, unsigned int
> > > > speed)
> > > > +{
> > > > + struct starfive_dwmac *dwmac = priv;
> > > > + unsigned long rate;
> > > > + int err;
> > > > +
> > > > + switch (speed) {
> > > > + case SPEED_1000:
> > > > + rate = 125000000;
> > > > + break;
> > > > + case SPEED_100:
> > > > + rate = 25000000;
> > > > + break;
> > > > + case SPEED_10:
> > > > + rate = 2500000;
> > > > + break;
> > > > + default:
> > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> > > > + return;
> > >
> > > Do we need to return value, since it is invalid speed. But the return
> > > value of function is void.(Arun Ramadoss)
> > >
> > >
> > > So in v9, after discussing with Jakub Kicinski, the clk_set_rate was used to initialize the rate.
> > > (It is a reference to Intel's scheme: dwmac-intel-plat.c: kmb_eth_fix_mac_speed)
> > > (https://patchwork.kernel.org/project/linux-riscv/patch/20230328062009.25454-6-samin.guo@starfivetech.com)
> > >
> >
> > Yeah, I think this is a misunderstanding and Arun is considering if we
> > ought to return the error which we can't without changing generic
> > dwmac code, and Jakub is rightly concerned about using a local
> > variable uninitialized. I don't think anyone is suggesting that
> > getting the rate just to set it to the exact same value is better than
> > just leaving the clock alone.
> >
> HI Emil,
>
> Yeah, return early saves time and code complexity, and seems like a good solution so Yanhong did the same before v3. (Jakub has suggested it before),
> I wonder if Arun or other maintainers accept this solution or if there are other solutions?
I think is not a big deal either way.
To avoid too much back and forth I'll stick to the current code.
Please address Emil comment on patch 6/6
Thanks!
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
2023-04-11 13:16 ` Paolo Abeni
@ 2023-04-11 14:37 ` Guo Samin
0 siblings, 0 replies; 15+ messages in thread
From: Guo Samin @ 2023-04-11 14:37 UTC (permalink / raw)
To: Paolo Abeni, Emil Renner Berthing, Arun.Ramadoss
Cc: linux-kernel, linux-riscv, devicetree, netdev, David S . Miller,
Eric Dumazet, Jakub Kicinski, Rob Herring, Krzysztof Kozlowski,
Emil Renner Berthing, Pedro Moreira, Richard Cochran,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Andrew Lunn, Heiner Kallweit, Peter Geis, Yanhong Wang,
Tommaso Merciai
Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
From: Paolo Abeni <pabeni@redhat.com>
to: Guo Samin <samin.guo@starfivetech.com>, Emil Renner Berthing <emil.renner.berthing@canonical.com>, Arun.Ramadoss@microchip.com
data: 2023/4/11
> On Mon, 2023-04-10 at 16:29 +0800, Guo Samin wrote:
>> Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
>> From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
>> to: Guo Samin <samin.guo@starfivetech.com>
>> data: 2023/4/9
>>
>>> On Sat, 8 Apr 2023 at 03:16, Guo Samin <samin.guo@starfivetech.com> wrote:
>>>>
>>>> Re: [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC
>>>> From: Emil Renner Berthing <emil.renner.berthing@canonical.com>
>>>> to: Samin Guo <samin.guo@starfivetech.com>
>>>> data: 2023/4/8
>>>>
>>>>> On Fri, 7 Apr 2023 at 13:05, Samin Guo <samin.guo@starfivetech.com> wrote:
>>>>>>
>>>>>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>>>>>
>>>>>> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
>>>>>> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
>>>>>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>>>>>> Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
>>>>>> ---
>>>>>> MAINTAINERS | 1 +
>>>>>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
>>>>>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>>>>>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 123 ++++++++++++++++++
>>>>>> 4 files changed, 137 insertions(+)
>>>>>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>>>>
>>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>>> index 6b6b67468b8f..46b366456cee 100644
>>>>>> --- a/MAINTAINERS
>>>>>> +++ b/MAINTAINERS
>>>>>> @@ -19910,6 +19910,7 @@ M: Emil Renner Berthing <kernel@esmil.dk>
>>>>>> M: Samin Guo <samin.guo@starfivetech.com>
>>>>>> S: Maintained
>>>>>> F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
>>>>>> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>>>>
>>>>>> STARFIVE JH7100 CLOCK DRIVERS
>>>>>> M: Emil Renner Berthing <kernel@esmil.dk>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>>>>>> index f77511fe4e87..5f5a997f21f3 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>>>>>> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
>>>>>> for the stmmac device driver. This driver is used for
>>>>>> arria5 and cyclone5 FPGA SoCs.
>>>>>>
>>>>>> +config DWMAC_STARFIVE
>>>>>> + tristate "StarFive dwmac support"
>>>>>> + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
>>>>>> + select MFD_SYSCON
>>>>>> + default m if ARCH_STARFIVE
>>>>>> + help
>>>>>> + Support for ethernet controllers on StarFive RISC-V SoCs
>>>>>> +
>>>>>> + This selects the StarFive platform specific glue layer support for
>>>>>> + the stmmac device driver. This driver is used for StarFive JH7110
>>>>>> + ethernet controller.
>>>>>> +
>>>>>> config DWMAC_STI
>>>>>> tristate "STi GMAC support"
>>>>>> default ARCH_STI
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>>>>>> index 057e4bab5c08..8738fdbb4b2d 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>>>>>> @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
>>>>>> obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
>>>>>> obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
>>>>>> obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
>>>>>> +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
>>>>>> obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
>>>>>> obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
>>>>>> obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..4963d4008485
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>>>> @@ -0,0 +1,123 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>>> +/*
>>>>>> + * StarFive DWMAC platform driver
>>>>>> + *
>>>>>> + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
>>>>>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/mfd/syscon.h>
>>>>>> +#include <linux/of_device.h>
>>>>>> +#include <linux/regmap.h>
>>>>>> +
>>>>>> +#include "stmmac_platform.h"
>>>>>> +
>>>>>> +struct starfive_dwmac {
>>>>>> + struct device *dev;
>>>>>> + struct clk *clk_tx;
>>>>>> +};
>>>>>> +
>>>>>> +static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed)
>>>>>> +{
>>>>>> + struct starfive_dwmac *dwmac = priv;
>>>>>> + unsigned long rate;
>>>>>> + int err;
>>>>>> +
>>>>>> + rate = clk_get_rate(dwmac->clk_tx);
>>>>>
>>>>> Hi Samin,
>>>>>
>>>>> I'm not sure why you added this line in this revision. If it's just to
>>>>> not call clk_set_rate on the uninitialized rate, I'd much rather you
>>>>> just returned early and don't call clk_set_rate at all in case of
>>>>> errors.
>>>>>
>>>>>> +
>>>>>> + switch (speed) {
>>>>>> + case SPEED_1000:
>>>>>> + rate = 125000000;
>>>>>> + break;
>>>>>> + case SPEED_100:
>>>>>> + rate = 25000000;
>>>>>> + break;
>>>>>> + case SPEED_10:
>>>>>> + rate = 2500000;
>>>>>> + break;
>>>>>> + default:
>>>>>> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
>>>>>> + break;
>>>>>
>>>>> That is skip the clk_get_rate above and just change this break to a return.
>>>>>
>>>>
>>>> Hi Emil,
>>>>
>>>> We used the solution you mentioned before V3, but Arun Ramadoss doesn't think that's great.
>>>> (https://patchwork.kernel.org/project/linux-riscv/patch/20230106030001.1952-6-yanhong.wang@starfivetech.com)
>>>>
>>>>
>>>>> +static void starfive_eth_plat_fix_mac_speed(void *priv, unsigned int
>>>>> speed)
>>>>> +{
>>>>> + struct starfive_dwmac *dwmac = priv;
>>>>> + unsigned long rate;
>>>>> + int err;
>>>>> +
>>>>> + switch (speed) {
>>>>> + case SPEED_1000:
>>>>> + rate = 125000000;
>>>>> + break;
>>>>> + case SPEED_100:
>>>>> + rate = 25000000;
>>>>> + break;
>>>>> + case SPEED_10:
>>>>> + rate = 2500000;
>>>>> + break;
>>>>> + default:
>>>>> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
>>>>> + return;
>>>>
>>>> Do we need to return value, since it is invalid speed. But the return
>>>> value of function is void.(Arun Ramadoss)
>>>>
>>>>
>>>> So in v9, after discussing with Jakub Kicinski, the clk_set_rate was used to initialize the rate.
>>>> (It is a reference to Intel's scheme: dwmac-intel-plat.c: kmb_eth_fix_mac_speed)
>>>> (https://patchwork.kernel.org/project/linux-riscv/patch/20230328062009.25454-6-samin.guo@starfivetech.com)
>>>>
>>>
>>> Yeah, I think this is a misunderstanding and Arun is considering if we
>>> ought to return the error which we can't without changing generic
>>> dwmac code, and Jakub is rightly concerned about using a local
>>> variable uninitialized. I don't think anyone is suggesting that
>>> getting the rate just to set it to the exact same value is better than
>>> just leaving the clock alone.
>>>
>> HI Emil,
>>
>> Yeah, return early saves time and code complexity, and seems like a good solution so Yanhong did the same before v3. (Jakub has suggested it before),
>> I wonder if Arun or other maintainers accept this solution or if there are other solutions?
>
> I think is not a big deal either way.
>
> To avoid too much back and forth I'll stick to the current code.
>
> Please address Emil comment on patch 6/6
>
> Thanks!
>
> Paolo
>
Hi Paolo,
Thanks! I'll fix it on patch 6/6 as suggested by Emil.
Best regards,
Samin
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-04-11 14:38 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-07 11:03 [-net-next v11 0/6] Add Ethernet driver for StarFive JH7110 SoC Samin Guo
2023-04-07 11:03 ` [-net-next v11 1/6] dt-bindings: net: snps,dwmac: Add dwmac-5.20 version Samin Guo
2023-04-07 11:03 ` [-net-next v11 2/6] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string Samin Guo
2023-04-07 11:03 ` [-net-next v11 3/6] dt-bindings: net: snps,dwmac: Add 'ahb' reset/reset-name Samin Guo
2023-04-07 11:03 ` [-net-next v11 4/6] dt-bindings: net: Add support StarFive dwmac Samin Guo
2023-04-07 11:03 ` [-net-next v11 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC Samin Guo
2023-04-07 19:11 ` Emil Renner Berthing
2023-04-08 1:15 ` Guo Samin
2023-04-08 17:30 ` Emil Renner Berthing
2023-04-10 8:29 ` Guo Samin
2023-04-11 13:16 ` Paolo Abeni
2023-04-11 14:37 ` Guo Samin
2023-04-07 11:03 ` [-net-next v11 6/6] net: stmmac: starfive-dmac: Add phy interface settings Samin Guo
2023-04-07 19:16 ` Emil Renner Berthing
2023-04-08 1:24 ` Guo Samin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).