2023-03-03 08:59:55

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 00/12] Add Ethernet driver for StarFive JH7110 SoC

This series adds ethernet support for the StarFive JH7110 RISC-V SoC.
The series includes MAC driver. The MAC version is dwmac-5.20 (from
Synopsys DesignWare). 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], [5].
[1]: https://wiki.rvspace.org/
[2]: https://github.com/SaminGuo/linux/tree/vf2-6.2-gmac
[3]: https://lore.kernel.org/all/[email protected]/
[4]: https://lore.kernel.org/all/[email protected]/
[5]: https://lore.kernel.org/all/[email protected]/

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.

Patch 12:
- No update
Patch 11:
- Configuration of gmac and phy for visionfive 2 v1.2A.
Patch 10:
- Configuration of gmac and phy for visionfive 2 v1.3B.
Patch 9:
- Added starfive,syscon for gmac nodes in jh7110.dtsi.
Patch 8:
- Added starfive_dwmac_set_mode to set PHY interface mode.
Patch 7:
- Added starfive,syscon item in StarFive-dwmac dt-bindings.
Patch 6:
- Moved SOC_STARFIVE to ARCH_STARFIVE in Kconfig.
- Dropped the _PLAT/_plat from the config/function/struct names. (by Emil)
- Added MODULE_DEVICE_TABLE() and udev will load the module automatically. (by Emil)
- Used { /* sentinel */ } for the last entry of starfive_eth_match. (by Emil)
- Added 'tx_use_rgmii_rxin_clk' to struct starfive_dwmac, to mark the clk_tx'parent is rgmii.
- Suggested by Emil, dropped clk_gtxclk and use clk_tx_inv to set the clock frequency.
Patch 5:
- Suggested by Emil, dropped mdio0/1 labels because there is no reference elsewhere.
Patch 4:
- Removed GTXC clk in StarFive-dwmac dt-bindings.
- Added starfive,tx-use-rgmii-clk item in StarFive-dwmac dt-bindings.
Patch 3:
- Added an optional reset single 'ahb' in 'snps,dwmac.yaml', according to
stmmac_probe_config_dt/stmmac_dvr_probe.
Patch 2:
- No update
Patch 1:
- No update

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/[email protected]/
v2 - https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
v3 - https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
v4 - https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/

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 (8):
dt-bindings: net: snps,dwmac: Add an optional resets single 'ahb'
riscv: dts: starfive: jh7110: Add ethernet device nodes
net: stmmac: Add glue layer for StarFive JH7110 SoC
dt-bindings: net: starfive,jh7110-dwmac: Add starfive,syscon
net: stmmac: starfive_dmac: Add phy interface settings
riscv: dts: starfive: jh7110: Add syscon to support phy interface
settings
riscv: dts: starfive: visionfive-2-v1.3b: Add gmac+phy's delay
configuration
riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay
configuration

Yanhong Wang (2):
dt-bindings: net: Add support StarFive dwmac
riscv: dts: starfive: visionfive 2: Enable gmac device tree node

.../devicetree/bindings/net/snps,dwmac.yaml | 19 +-
.../bindings/net/starfive,jh7110-dwmac.yaml | 130 +++++++++++++
MAINTAINERS | 7 +
.../jh7110-starfive-visionfive-2-v1.2a.dts | 13 ++
.../jh7110-starfive-visionfive-2-v1.3b.dts | 27 +++
.../jh7110-starfive-visionfive-2.dtsi | 10 +
arch/riscv/boot/dts/starfive/jh7110.dtsi | 93 ++++++++++
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 +-
11 files changed, 481 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: 11934a315b671ddb09bc7ac5f505649e9f2623c7
prerequisite-patch-id: ad56ef54d3f2a18025abc9e27321c25beda16422
prerequisite-patch-id: 1be0fb49e0fbe293ca8fa94601e191b13c8c67d9
prerequisite-patch-id: 8b402a8d97294a9b568595816b0dc96afc5e6f5d
prerequisite-patch-id: 5c149662674f9e7dd888e2028fd8c9772948273f
prerequisite-patch-id: 0caf8a313a9f161447e0480a93b42467378b2164
prerequisite-patch-id: b2422f7a12f1e86e38c563139f3c1dbafc158efd
prerequisite-patch-id: be612664eca7049e987bfae15bb460caa82eb211
prerequisite-patch-id: 8300965cc6c55cad69f009da7916cf9e8ce628e7
--
2.17.1



2023-03-03 08:59:59

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings

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, mask>

Signed-off-by: Samin Guo <[email protected]>
---
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 46 +++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
index 566378306f67..40fdd7036127 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -7,10 +7,15 @@
*
*/

+#include <linux/mfd/syscon.h>
#include <linux/of_device.h>
+#include <linux/regmap.h>

#include "stmmac_platform.h"

+#define MACPHYC_PHY_INFT_RMII 0x4
+#define MACPHYC_PHY_INFT_RGMII 0x1
+
struct starfive_dwmac {
struct device *dev;
struct clk *clk_tx;
@@ -53,6 +58,46 @@ static void starfive_eth_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 of_phandle_args args;
+ struct regmap *regmap;
+ unsigned int reg, mask, mode;
+ int err;
+
+ switch (plat_dat->interface) {
+ case PHY_INTERFACE_MODE_RMII:
+ mode = MACPHYC_PHY_INFT_RMII;
+ break;
+
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ mode = MACPHYC_PHY_INFT_RGMII;
+ break;
+
+ default:
+ dev_err(dwmac->dev, "Unsupported interface %d\n",
+ plat_dat->interface);
+ }
+
+ err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
+ "starfive,syscon", 2, 0, &args);
+ if (err) {
+ dev_dbg(dwmac->dev, "syscon reg not found\n");
+ return -EINVAL;
+ }
+
+ reg = args.args[0];
+ mask = args.args[1];
+ regmap = syscon_node_to_regmap(args.np);
+ of_node_put(args.np);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));
+}
+
static int starfive_dwmac_probe(struct platform_device *pdev)
{
struct plat_stmmacenet_data *plat_dat;
@@ -93,6 +138,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
plat_dat->bsp_priv = dwmac;
plat_dat->dma_cfg->dche = true;

+ starfive_dwmac_set_mode(plat_dat);
err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
if (err) {
stmmac_remove_config_dt(pdev, plat_dat);
--
2.17.1


2023-03-03 09:00:03

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 02/12] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

From: Emil Renner Berthing <[email protected]>

Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
to define some platform data in the glue layer.

Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Samin Guo <[email protected]>
---
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 0046a4ee6e64..807eca7edf53 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


2023-03-03 09:00:05

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 09/12] riscv: dts: starfive: jh7110: Add syscon to support phy interface settings

The phy interface needs to be set in syscon, the format is as follows:
starfive,syscon: <&syscon, offset, mask>

Signed-off-by: Samin Guo <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 2ce28292b721..c1c5085dab72 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -554,6 +554,7 @@
snps,en-tx-lpi-clockgating;
snps,txpbl = <16>;
snps,rxpbl = <16>;
+ starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
status = "disabled";
phy-handle = <&phy0>;

@@ -596,6 +597,7 @@
snps,en-tx-lpi-clockgating;
snps,txpbl = <16>;
snps,rxpbl = <16>;
+ starfive,syscon = <&sys_syscon 0x90 0x1c>;
status = "disabled";
phy-handle = <&phy1>;

--
2.17.1


2023-03-03 09:00:10

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 10/12] riscv: dts: starfive: visionfive-2-v1.3b: Add gmac+phy's delay configuration

v1.3B uses motorcomm YT8531(rgmii-id phy) x2, need delay and
inverse configurations.

The tx_clk of v1.3B uses an external clock and needs to be
switched to an external clock source.

Signed-off-by: Samin Guo <[email protected]>
---
.../jh7110-starfive-visionfive-2-v1.3b.dts | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
index 9230cc3d8946..32fae0de9a44 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dts
@@ -11,3 +11,30 @@
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";
};
+
+&gmac0 {
+ starfive,tx-use-rgmii-clk;
+ assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
+ assigned-clock-parents = <&aoncrg JH7110_AONCLK_GMAC0_RMII_RTX>;
+};
+
+&gmac1 {
+ starfive,tx-use-rgmii-clk;
+ assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>;
+ assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
+};
+
+&phy0 {
+ motorcomm,tx-clk-adj-enabled;
+ motorcomm,tx-clk-100-inverted;
+ motorcomm,tx-clk-1000-inverted;
+ rx-internal-delay-ps = <1900>;
+ tx-internal-delay-ps = <1500>;
+};
+
+&phy1 {
+ motorcomm,tx-clk-adj-enabled;
+ motorcomm,tx-clk-100-inverted;
+ rx-internal-delay-ps = <0>;
+ tx-internal-delay-ps = <0>;
+};
--
2.17.1


2023-03-03 09:00:14

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration

v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay
configurations.

v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to
switch rx and rx to external clock sources.

Signed-off-by: Samin Guo <[email protected]>
---
.../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
index 4af3300f3cf3..205a13d8c8b1 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
@@ -11,3 +11,16 @@
model = "StarFive VisionFive 2 v1.2A";
compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110";
};
+
+&gmac1 {
+ phy-mode = "rmii";
+ assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>,
+ <&syscrg JH7110_SYSCLK_GMAC1_RX>;
+ assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>,
+ <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
+};
+
+&phy0 {
+ rx-internal-delay-ps = <1900>;
+ tx-internal-delay-ps = <1350>;
+};
--
2.17.1


2023-03-03 09:00:17

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 12/12] riscv: dts: starfive: visionfive 2: Enable gmac device tree node

From: Yanhong Wang <[email protected]>

Update gmac device tree node status to okay.

Signed-off-by: Yanhong Wang <[email protected]>
Signed-off-by: Samin Guo <[email protected]>
---
.../dts/starfive/jh7110-starfive-visionfive-2.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index c2aa8946a0f1..d1c409f40014 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -12,6 +12,8 @@
/ {
aliases {
serial0 = &uart0;
+ ethernet0 = &gmac0;
+ ethernet1 = &gmac1;
i2c0 = &i2c0;
i2c2 = &i2c2;
i2c5 = &i2c5;
@@ -92,6 +94,14 @@
status = "okay";
};

+&gmac0 {
+ status = "okay";
+};
+
+&gmac1 {
+ status = "okay";
+};
+
&i2c0 {
clock-frequency = <100000>;
i2c-sda-hold-time-ns = <300>;
--
2.17.1


2023-03-03 09:00:19

by Guo Samin

[permalink] [raw]
Subject: [PATCH v5 07/12] dt-bindings: net: starfive,jh7110-dwmac: Add starfive,syscon

A phandle to syscon with two arguments that configure phy mode.

Signed-off-by: Samin Guo <[email protected]>
---
.../bindings/net/starfive,jh7110-dwmac.yaml | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
index ca49f08d50dd..79ae635db0a5 100644
--- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
@@ -58,6 +58,18 @@ properties:
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: Mask 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 mask of phy mode selection.
+
allOf:
- $ref: snps,dwmac.yaml#

@@ -96,6 +108,7 @@ examples:
snps,en-tx-lpi-clockgating;
snps,txpbl = <16>;
snps,rxpbl = <16>;
+ starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
phy-handle = <&phy0>;

mdio {
--
2.17.1


2023-03-03 13:37:22

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings

> +static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> + struct starfive_dwmac *dwmac = plat_dat->bsp_priv;
> + struct of_phandle_args args;
> + struct regmap *regmap;
> + unsigned int reg, mask, mode;
> + int err;
> +
> + switch (plat_dat->interface) {
> + case PHY_INTERFACE_MODE_RMII:
> + mode = MACPHYC_PHY_INFT_RMII;
> + break;
> +
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + mode = MACPHYC_PHY_INFT_RGMII;
> + break;
> +
> + default:
> + dev_err(dwmac->dev, "Unsupported interface %d\n",
> + plat_dat->interface);
> + }

Please add a return -EINVAL;

> +
> + err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
> + "starfive,syscon", 2, 0, &args);
> + if (err) {
> + dev_dbg(dwmac->dev, "syscon reg not found\n");
> + return -EINVAL;
> + }
> +
> + reg = args.args[0];
> + mask = args.args[1];
> + regmap = syscon_node_to_regmap(args.np);
> + of_node_put(args.np);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));

This is a poor device tree binding. We generally don't allow bindings
which say put value X in register Y.

Could you add a table: interface mode, reg, mask? You can then do a
simple lookup based on the interface mode? No device tree binding
needed at all?

Andrew

2023-03-03 16:52:14

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings

On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> 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, mask>
>
> Signed-off-by: Samin Guo <[email protected]>
> ---
> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 46 +++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> index 566378306f67..40fdd7036127 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> @@ -7,10 +7,15 @@
> *
> */
>
> +#include <linux/mfd/syscon.h>
> #include <linux/of_device.h>
> +#include <linux/regmap.h>
>
> #include "stmmac_platform.h"
>
> +#define MACPHYC_PHY_INFT_RMII 0x4
> +#define MACPHYC_PHY_INFT_RGMII 0x1

Please prefix these with something like STARFIVE_DWMAC_

> struct starfive_dwmac {
> struct device *dev;
> struct clk *clk_tx;
> @@ -53,6 +58,46 @@ static void starfive_eth_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 of_phandle_args args;
> + struct regmap *regmap;
> + unsigned int reg, mask, mode;
> + int err;
> +
> + switch (plat_dat->interface) {
> + case PHY_INTERFACE_MODE_RMII:
> + mode = MACPHYC_PHY_INFT_RMII;
> + break;
> +
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + mode = MACPHYC_PHY_INFT_RGMII;
> + break;
> +
> + default:
> + dev_err(dwmac->dev, "Unsupported interface %d\n",
> + plat_dat->interface);
> + }
> +
> + err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
> + "starfive,syscon", 2, 0, &args);
> + if (err) {
> + dev_dbg(dwmac->dev, "syscon reg not found\n");
> + return -EINVAL;
> + }
> +
> + reg = args.args[0];
> + mask = args.args[1];
> + regmap = syscon_node_to_regmap(args.np);
> + of_node_put(args.np);

I think the above is basically
unsigned int args[2];
syscon_regmap_lookup_by_phandle_args(dwmac->dev_of_node,
"starfive,syscon", 2, args);

..but as Andrew points out another solution is to use platform match
data for this. Eg.

static const struct starfive_dwmac_match_data starfive_dwmac_jh7110_data {
.phy_interface_offset = 0xc,
.phy_interface_mask = 0x1c0000,
};

static const struct of_device_id starfive_dwmac_match[] = {
{ .compatible = "starfive,jh7110-dwmac", .data =
&starfive_dwmac_jh7110_data },
{ /* sentinel */ }
};

and in the probe function:

struct starfive_dwmac_match_data *pdata = device_get_match_data(&pdev->dev);

> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));
> +}
> +
> static int starfive_dwmac_probe(struct platform_device *pdev)
> {
> struct plat_stmmacenet_data *plat_dat;
> @@ -93,6 +138,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
> plat_dat->bsp_priv = dwmac;
> plat_dat->dma_cfg->dche = true;
>
> + starfive_dwmac_set_mode(plat_dat);

The function returns errors in an int, but you never check it :(

> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-03-06 03:07:47

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings



在 2023/3/4 0:50:54, Emil Renner Berthing 写道:
> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> 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, mask>
>>
>> Signed-off-by: Samin Guo <[email protected]>
>> ---
>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 46 +++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> index 566378306f67..40fdd7036127 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> @@ -7,10 +7,15 @@
>> *
>> */
>>
>> +#include <linux/mfd/syscon.h>
>> #include <linux/of_device.h>
>> +#include <linux/regmap.h>
>>
>> #include "stmmac_platform.h"
>>
>> +#define MACPHYC_PHY_INFT_RMII 0x4
>> +#define MACPHYC_PHY_INFT_RGMII 0x1
>
> Please prefix these with something like STARFIVE_DWMAC_
>
Hi, Emil, These definitions come from the datasheet of dwmac. However, add STARDRIVE_ DWMAC is a good idea. I will fix it,thanks.
>> struct starfive_dwmac {
>> struct device *dev;
>> struct clk *clk_tx;
>> @@ -53,6 +58,46 @@ static void starfive_eth_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 of_phandle_args args;
>> + struct regmap *regmap;
>> + unsigned int reg, mask, mode;
>> + int err;
>> +
>> + switch (plat_dat->interface) {
>> + case PHY_INTERFACE_MODE_RMII:
>> + mode = MACPHYC_PHY_INFT_RMII;
>> + break;
>> +
>> + case PHY_INTERFACE_MODE_RGMII:
>> + case PHY_INTERFACE_MODE_RGMII_ID:
>> + mode = MACPHYC_PHY_INFT_RGMII;
>> + break;
>> +
>> + default:
>> + dev_err(dwmac->dev, "Unsupported interface %d\n",
>> + plat_dat->interface);
>> + }
>> +
>> + err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
>> + "starfive,syscon", 2, 0, &args);
>> + if (err) {
>> + dev_dbg(dwmac->dev, "syscon reg not found\n");
>> + return -EINVAL;
>> + }
>> +
>> + reg = args.args[0];
>> + mask = args.args[1];
>> + regmap = syscon_node_to_regmap(args.np);
>> + of_node_put(args.np);
>
> I think the above is basically
> unsigned int args[2];
> syscon_regmap_lookup_by_phandle_args(dwmac->dev_of_node,
> "starfive,syscon", 2, args);
>
> ..but as Andrew points out another solution is to use platform match
> data for this. Eg.
>
> static const struct starfive_dwmac_match_data starfive_dwmac_jh7110_data {
> .phy_interface_offset = 0xc,
> .phy_interface_mask = 0x1c0000,
> };
>
> static const struct of_device_id starfive_dwmac_match[] = {
> { .compatible = "starfive,jh7110-dwmac", .data =
> &starfive_dwmac_jh7110_data },
> { /* sentinel */ }
> };
>
> and in the probe function:
>
Hi Emil, Yes,this is usually a good solution, and I have considered this plan before.
However, gmac0 of jh7110 is different from the reg/mask of gmac1.
You can find it in patch-9:

&gmac0 {
starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
};

&gmac1 {
starfive,syscon = <&sys_syscon 0x90 0x1c>;
};

In this case, using match_data of starfive,jh7110-dwma does not seem to be compatible.

> struct starfive_dwmac_match_data *pdata = device_get_match_data(&pdev->dev);
>
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));
>> +}
>> +
>> static int starfive_dwmac_probe(struct platform_device *pdev)
>> {
>> struct plat_stmmacenet_data *plat_dat;
>> @@ -93,6 +138,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>> plat_dat->bsp_priv = dwmac;
>> plat_dat->dma_cfg->dche = true;
>>
>> + starfive_dwmac_set_mode(plat_dat);
>
> The function returns errors in an int, but you never check it :(
>
Thank you for pointing out that it will be added in the next version.
>> err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> if (err) {
>> stmmac_remove_config_dt(pdev, plat_dat);


Best regards,
Samin

>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

--
Best regards,
Samin

2023-03-06 12:50:42

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings

On Mon, 6 Mar 2023 at 04:07, Guo Samin <[email protected]> wrote:
> 在 2023/3/4 0:50:54, Emil Renner Berthing 写道:
> > On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> 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, mask>
> >>
> >> Signed-off-by: Samin Guo <[email protected]>
> >> ---
> >> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 46 +++++++++++++++++++
> >> 1 file changed, 46 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >> index 566378306f67..40fdd7036127 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >> @@ -7,10 +7,15 @@
> >> *
> >> */
> >>
> >> +#include <linux/mfd/syscon.h>
> >> #include <linux/of_device.h>
> >> +#include <linux/regmap.h>
> >>
> >> #include "stmmac_platform.h"
> >>
> >> +#define MACPHYC_PHY_INFT_RMII 0x4
> >> +#define MACPHYC_PHY_INFT_RGMII 0x1
> >
> > Please prefix these with something like STARFIVE_DWMAC_
> >
> Hi, Emil, These definitions come from the datasheet of dwmac. However, add STARDRIVE_ DWMAC is a good idea. I will fix it,thanks.
> >> struct starfive_dwmac {
> >> struct device *dev;
> >> struct clk *clk_tx;
> >> @@ -53,6 +58,46 @@ static void starfive_eth_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 of_phandle_args args;
> >> + struct regmap *regmap;
> >> + unsigned int reg, mask, mode;
> >> + int err;
> >> +
> >> + switch (plat_dat->interface) {
> >> + case PHY_INTERFACE_MODE_RMII:
> >> + mode = MACPHYC_PHY_INFT_RMII;
> >> + break;
> >> +
> >> + case PHY_INTERFACE_MODE_RGMII:
> >> + case PHY_INTERFACE_MODE_RGMII_ID:
> >> + mode = MACPHYC_PHY_INFT_RGMII;
> >> + break;
> >> +
> >> + default:
> >> + dev_err(dwmac->dev, "Unsupported interface %d\n",
> >> + plat_dat->interface);
> >> + }
> >> +
> >> + err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
> >> + "starfive,syscon", 2, 0, &args);
> >> + if (err) {
> >> + dev_dbg(dwmac->dev, "syscon reg not found\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + reg = args.args[0];
> >> + mask = args.args[1];
> >> + regmap = syscon_node_to_regmap(args.np);
> >> + of_node_put(args.np);
> >
> > I think the above is basically
> > unsigned int args[2];
> > syscon_regmap_lookup_by_phandle_args(dwmac->dev_of_node,
> > "starfive,syscon", 2, args);
> >
> > ..but as Andrew points out another solution is to use platform match
> > data for this. Eg.
> >
> > static const struct starfive_dwmac_match_data starfive_dwmac_jh7110_data {
> > .phy_interface_offset = 0xc,
> > .phy_interface_mask = 0x1c0000,
> > };
> >
> > static const struct of_device_id starfive_dwmac_match[] = {
> > { .compatible = "starfive,jh7110-dwmac", .data =
> > &starfive_dwmac_jh7110_data },
> > { /* sentinel */ }
> > };
> >
> > and in the probe function:
> >
> Hi Emil, Yes,this is usually a good solution, and I have considered this plan before.
> However, gmac0 of jh7110 is different from the reg/mask of gmac1.
> You can find it in patch-9:
>
> &gmac0 {
> starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
> };
>
> &gmac1 {
> starfive,syscon = <&sys_syscon 0x90 0x1c>;
> };
>
> In this case, using match_data of starfive,jh7110-dwma does not seem to be compatible.

Ugh, you're right. Both the syscon block, the register offset and the
bit position in those registers are different from gmac0 to gmac1, and
since we need a phandle to the syscon block anyway passing those two
other parameters as arguments is probably the nicest solution. For the
next version I'd change the 2nd argument from mask to the bit position
though. It seems the field is always 3 bits wide and this makes it a
little clearer that we're not just putting register values in the
device tree. Eg. something like

regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
"starfive,syscon", 2, args);
...
err = regmap_update_bits(regmap, args[0], 7U << args[1], mode << args[1]);
...

Alternatively we'd put data for each gmac interface in the platform
data including the syscon compatible string, and use
syscon_regmap_lookup_by_compatible("starfive,jh7110-aon-syscon"); for
gmac0 fx. This way the dependency from the gmac nodes to the syscon
nodes won't be recorded is the device tree though.

@Andrew is this what you were suggesting?

> > struct starfive_dwmac_match_data *pdata = device_get_match_data(&pdev->dev);
> >
> >> + if (IS_ERR(regmap))
> >> + return PTR_ERR(regmap);
> >> +
> >> + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));
> >> +}
> >> +
> >> static int starfive_dwmac_probe(struct platform_device *pdev)
> >> {
> >> struct plat_stmmacenet_data *plat_dat;
> >> @@ -93,6 +138,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
> >> plat_dat->bsp_priv = dwmac;
> >> plat_dat->dma_cfg->dche = true;
> >>
> >> + starfive_dwmac_set_mode(plat_dat);
> >
> > The function returns errors in an int, but you never check it :(
> >
> Thank you for pointing out that it will be added in the next version.
> >> err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> >> if (err) {
> >> stmmac_remove_config_dt(pdev, plat_dat);
>
>
> Best regards,
> Samin
>
> >> --
> >> 2.17.1
> >>
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> --
> Best regards,
> Samin

2023-03-06 13:01:28

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration

On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay
> configurations.
>
> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to
> switch rx and rx to external clock sources.
>
> Signed-off-by: Samin Guo <[email protected]>
> ---
> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
> index 4af3300f3cf3..205a13d8c8b1 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
> @@ -11,3 +11,16 @@
> model = "StarFive VisionFive 2 v1.2A";
> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110";
> };
> +
> +&gmac1 {
> + phy-mode = "rmii";
> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>,
> + <&syscrg JH7110_SYSCLK_GMAC1_RX>;
> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>,
> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
> +};
> +
> +&phy0 {
> + rx-internal-delay-ps = <1900>;
> + tx-internal-delay-ps = <1350>;
> +};

Here you're not specifying the internal delays for phy1 which means it
defaults to 1950ps for both rx and tx. Is that right or did you mean
to set them to 0 like the v1.3b phy1?

Also your u-boot seems to set what the linux phy driver calls
motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all
the phys. Did you leave those out on purpose?

> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-03-06 13:04:57

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v5 12/12] riscv: dts: starfive: visionfive 2: Enable gmac device tree node

On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> From: Yanhong Wang <[email protected]>
>
> Update gmac device tree node status to okay.
>
> Signed-off-by: Yanhong Wang <[email protected]>
> Signed-off-by: Samin Guo <[email protected]>
> ---
> .../dts/starfive/jh7110-starfive-visionfive-2.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index c2aa8946a0f1..d1c409f40014 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -12,6 +12,8 @@
> / {
> aliases {
> serial0 = &uart0;
> + ethernet0 = &gmac0;
> + ethernet1 = &gmac1;

Please sort these alphabetically.

> i2c0 = &i2c0;
> i2c2 = &i2c2;
> i2c5 = &i2c5;
> @@ -92,6 +94,14 @@
> status = "okay";
> };
>
> +&gmac0 {
> + status = "okay";
> +};
> +
> +&gmac1 {
> + status = "okay";
> +};

Since you'll need to add to the gmac0 and gmac1 nodes in the board
specific files too and it's only one line, consider just dropping this
here and add the status = "okay" there instead.

> &i2c0 {
> clock-frequency = <100000>;
> i2c-sda-hold-time-ns = <300>;
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-03-06 13:06:33

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings

> Ugh, you're right. Both the syscon block, the register offset and the
> bit position in those registers are different from gmac0 to gmac1, and
> since we need a phandle to the syscon block anyway passing those two
> other parameters as arguments is probably the nicest solution. For the
> next version I'd change the 2nd argument from mask to the bit position
> though. It seems the field is always 3 bits wide and this makes it a
> little clearer that we're not just putting register values in the
> device tree.

I prefer bit position over mask.

But please fully document this in the device tree. This is something a
board developer is going to get wrong, because they assume MAC blocks
are identical, and normally need identical configuration.

I assume this is also a hardware 'bug', and the next generation of the
silicon will have this fixed? So this will go away?

Andrew

2023-03-07 01:21:34

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 12/12] riscv: dts: starfive: visionfive 2: Enable gmac device tree node



在 2023/3/6 21:04:28, Emil Renner Berthing 写道:
> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>> From: Yanhong Wang <[email protected]>
>>
>> Update gmac device tree node status to okay.
>>
>> Signed-off-by: Yanhong Wang <[email protected]>
>> Signed-off-by: Samin Guo <[email protected]>
>> ---
>> .../dts/starfive/jh7110-starfive-visionfive-2.dtsi | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index c2aa8946a0f1..d1c409f40014 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -12,6 +12,8 @@
>> / {
>> aliases {
>> serial0 = &uart0;
>> + ethernet0 = &gmac0;
>> + ethernet1 = &gmac1;
>
> Please sort these alphabetically.
Thanks, will fix.
>
>> i2c0 = &i2c0;
>> i2c2 = &i2c2;
>> i2c5 = &i2c5;
>> @@ -92,6 +94,14 @@
>> status = "okay";
>> };
>>
>> +&gmac0 {
>> + status = "okay";
>> +};
>> +
>> +&gmac1 {
>> + status = "okay";
>> +};
>
> Since you'll need to add to the gmac0 and gmac1 nodes in the board
> specific files too and it's only one line, consider just dropping this
> here and add the status = "okay" there instead.
>
According to Andrew's suggestion, can I put the nodes of mdio and phy here?
>> &i2c0 {
>> clock-frequency = <100000>;
>> i2c-sda-hold-time-ns = <300>;
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Best regards,
Samin
--
Best regards,
Samin

2023-03-07 01:43:59

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration



在 2023/3/6 21:00:19, Emil Renner Berthing 写道:
> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay
>> configurations.
>>
>> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to
>> switch rx and rx to external clock sources.
>>
>> Signed-off-by: Samin Guo <[email protected]>
>> ---
>> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
>> index 4af3300f3cf3..205a13d8c8b1 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
>> @@ -11,3 +11,16 @@
>> model = "StarFive VisionFive 2 v1.2A";
>> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110";
>> };
>> +
>> +&gmac1 {
>> + phy-mode = "rmii";
>> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>,
>> + <&syscrg JH7110_SYSCLK_GMAC1_RX>;
>> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>,
>> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
>> +};
>> +
>> +&phy0 {
>> + rx-internal-delay-ps = <1900>;
>> + tx-internal-delay-ps = <1350>;
>> +};
>
> Here you're not specifying the internal delays for phy1 which means it
> defaults to 1950ps for both rx and tx. Is that right or did you mean
> to set them to 0 like the v1.3b phy1?

Hi, emil, usually, only 1000M (rgmii) needs to configure the delay, and 100M(rmii) does not.
>
> Also your u-boot seems to set what the linux phy driver calls
> motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all
> the phys. Did you leave those out on purpose?

Hi, Emil, We did configure motorcomm,auto-sleep-disabled for yt8512 in uboot,
but Yutai upstream's Linux driver only yt8521/yt8531 supports this property.
Yt8512 is a Generic PHY driver and does not support the configuration of
motorcomm,auto-sleep-disabled and motorcomm,keep-pll-enabled.

And without configuring these two attributes, vf2-1.2a gmac1 also works normally.


Best regards,
Samin
>
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

--
Best regards,
Samin

2023-03-07 01:50:56

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings



在 2023/3/6 21:06:20, Andrew Lunn 写道:
>> Ugh, you're right. Both the syscon block, the register offset and the
>> bit position in those registers are different from gmac0 to gmac1, and
>> since we need a phandle to the syscon block anyway passing those two
>> other parameters as arguments is probably the nicest solution. For the
>> next version I'd change the 2nd argument from mask to the bit position
>> though. It seems the field is always 3 bits wide and this makes it a
>> little clearer that we're not just putting register values in the
>> device tree.
>
> I prefer bit position over mask.
>
> But please fully document this in the device tree. This is something a
> board developer is going to get wrong, because they assume MAC blocks
> are identical, and normally need identical configuration.
>
> I assume this is also a hardware 'bug', and the next generation of the
> silicon will have this fixed? So this will go away?
>
> Andrew


Hi Andrew, Yes, the hardware design does not take into account the feasibility of the software.
The next version will be fixed. Thank you.
I will use bit position instead of mask, which is described in detail in the document.

Best regards,
Samin

--
Best regards,
Samin

2023-03-07 02:17:08

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 08/12] net: stmmac: starfive_dmac: Add phy interface settings



在 2023/3/6 20:49:57, Emil Renner Berthing 写道:
> On Mon, 6 Mar 2023 at 04:07, Guo Samin <[email protected]> wrote:
>> 在 2023/3/4 0:50:54, Emil Renner Berthing 写道:
>>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> 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, mask>
>>>>
>>>> Signed-off-by: Samin Guo <[email protected]>
>>>> ---
>>>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 46 +++++++++++++++++++
>>>> 1 file changed, 46 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>> index 566378306f67..40fdd7036127 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>>> @@ -7,10 +7,15 @@
>>>> *
>>>> */
>>>>
>>>> +#include <linux/mfd/syscon.h>
>>>> #include <linux/of_device.h>
>>>> +#include <linux/regmap.h>
>>>>
>>>> #include "stmmac_platform.h"
>>>>
>>>> +#define MACPHYC_PHY_INFT_RMII 0x4
>>>> +#define MACPHYC_PHY_INFT_RGMII 0x1
>>>
>>> Please prefix these with something like STARFIVE_DWMAC_
>>>
>> Hi, Emil, These definitions come from the datasheet of dwmac. However, add STARDRIVE_ DWMAC is a good idea. I will fix it,thanks.
>>>> struct starfive_dwmac {
>>>> struct device *dev;
>>>> struct clk *clk_tx;
>>>> @@ -53,6 +58,46 @@ static void starfive_eth_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 of_phandle_args args;
>>>> + struct regmap *regmap;
>>>> + unsigned int reg, mask, mode;
>>>> + int err;
>>>> +
>>>> + switch (plat_dat->interface) {
>>>> + case PHY_INTERFACE_MODE_RMII:
>>>> + mode = MACPHYC_PHY_INFT_RMII;
>>>> + break;
>>>> +
>>>> + case PHY_INTERFACE_MODE_RGMII:
>>>> + case PHY_INTERFACE_MODE_RGMII_ID:
>>>> + mode = MACPHYC_PHY_INFT_RGMII;
>>>> + break;
>>>> +
>>>> + default:
>>>> + dev_err(dwmac->dev, "Unsupported interface %d\n",
>>>> + plat_dat->interface);
>>>> + }
>>>> +
>>>> + err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
>>>> + "starfive,syscon", 2, 0, &args);
>>>> + if (err) {
>>>> + dev_dbg(dwmac->dev, "syscon reg not found\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + reg = args.args[0];
>>>> + mask = args.args[1];
>>>> + regmap = syscon_node_to_regmap(args.np);
>>>> + of_node_put(args.np);
>>>
>>> I think the above is basically
>>> unsigned int args[2];
>>> syscon_regmap_lookup_by_phandle_args(dwmac->dev_of_node,
>>> "starfive,syscon", 2, args);
>>>
>>> ..but as Andrew points out another solution is to use platform match
>>> data for this. Eg.
>>>
>>> static const struct starfive_dwmac_match_data starfive_dwmac_jh7110_data {
>>> .phy_interface_offset = 0xc,
>>> .phy_interface_mask = 0x1c0000,
>>> };
>>>
>>> static const struct of_device_id starfive_dwmac_match[] = {
>>> { .compatible = "starfive,jh7110-dwmac", .data =
>>> &starfive_dwmac_jh7110_data },
>>> { /* sentinel */ }
>>> };
>>>
>>> and in the probe function:
>>>
>> Hi Emil, Yes,this is usually a good solution, and I have considered this plan before.
>> However, gmac0 of jh7110 is different from the reg/mask of gmac1.
>> You can find it in patch-9:
>>
>> &gmac0 {
>> starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
>> };
>>
>> &gmac1 {
>> starfive,syscon = <&sys_syscon 0x90 0x1c>;
>> };
>>
>> In this case, using match_data of starfive,jh7110-dwma does not seem to be compatible.
>
> Ugh, you're right. Both the syscon block, the register offset and the
> bit position in those registers are different from gmac0 to gmac1, and
> since we need a phandle to the syscon block anyway passing those two
> other parameters as arguments is probably the nicest solution. For the
> next version I'd change the 2nd argument from mask to the bit position
> though. It seems the field is always 3 bits wide and this makes it a
> little clearer that we're not just putting register values in the
> device tree. Eg. something like
>
Yes,the field is always 3 bits wide, the next version will use bit position instead of mask.
Thank you for your advice.
> regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
> "starfive,syscon", 2, args);
> ...
> err = regmap_update_bits(regmap, args[0], 7U << args[1], mode << args[1]);
> ...
>
I also think the current method is relatively simple and compatible.


Best regards,
Samin
> Alternatively we'd put data for each gmac interface in the platform
> data including the syscon compatible string, and use
> syscon_regmap_lookup_by_compatible("starfive,jh7110-aon-syscon"); for
> gmac0 fx. This way the dependency from the gmac nodes to the syscon
> nodes won't be recorded is the device tree though.
>
> @Andrew is this what you were suggesting?
>


>>> struct starfive_dwmac_match_data *pdata = device_get_match_data(&pdev->dev);
>>>
>>>> + if (IS_ERR(regmap))
>>>> + return PTR_ERR(regmap);
>>>> +
>>>> + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));
>>>> +}
>>>> +
>>>> static int starfive_dwmac_probe(struct platform_device *pdev)
>>>> {
>>>> struct plat_stmmacenet_data *plat_dat;
>>>> @@ -93,6 +138,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>>>> plat_dat->bsp_priv = dwmac;
>>>> plat_dat->dma_cfg->dche = true;
>>>>
>>>> + starfive_dwmac_set_mode(plat_dat);
>>>
>>> The function returns errors in an int, but you never check it :(
>>>
>> Thank you for pointing out that it will be added in the next version.
>>>> err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>>>> if (err) {
>>>> stmmac_remove_config_dt(pdev, plat_dat);
>>
>>
>> Best regards,
>> Samin
>>
>>>> --
>>>> 2.17.1
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-riscv mailing list
>>>> [email protected]
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>> --
>> Best regards,
>> Samin

--
Best regards,
Samin

2023-03-07 12:23:55

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v5 12/12] riscv: dts: starfive: visionfive 2: Enable gmac device tree node

On Tue, 7 Mar 2023 at 02:21, Guo Samin <[email protected]> wrote:

> 在 2023/3/6 21:04:28, Emil Renner Berthing 写道:
> > On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> >> From: Yanhong Wang <[email protected]>
> >>
> >> Update gmac device tree node status to okay.
> >>
> >> Signed-off-by: Yanhong Wang <[email protected]>
> >> Signed-off-by: Samin Guo <[email protected]>
> >> ---
> >> .../dts/starfive/jh7110-starfive-visionfive-2.dtsi | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> >> index c2aa8946a0f1..d1c409f40014 100644
> >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> >> @@ -12,6 +12,8 @@
> >> / {
> >> aliases {
> >> serial0 = &uart0;
> >> + ethernet0 = &gmac0;
> >> + ethernet1 = &gmac1;
> >
> > Please sort these alphabetically.
> Thanks, will fix.
> >
> >> i2c0 = &i2c0;
> >> i2c2 = &i2c2;
> >> i2c5 = &i2c5;
> >> @@ -92,6 +94,14 @@
> >> status = "okay";
> >> };
> >>
> >> +&gmac0 {
> >> + status = "okay";
> >> +};
> >> +
> >> +&gmac1 {
> >> + status = "okay";
> >> +};
> >
> > Since you'll need to add to the gmac0 and gmac1 nodes in the board
> > specific files too and it's only one line, consider just dropping this
> > here and add the status = "okay" there instead.
> >
> According to Andrew's suggestion, can I put the nodes of mdio and phy here?

Yeah, if the boards then end up sharing more information it's fine to
put it here. It just seemed a little much to add 8 lines here when all
the boards shared was a status = "okay";

> >> &i2c0 {
> >> clock-frequency = <100000>;
> >> i2c-sda-hold-time-ns = <300>;
> >> --
> >> 2.17.1
> >>
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> Best regards,
> Samin
> --
> Best regards,
> Samin

2023-03-07 12:41:23

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration

On Tue, 7 Mar 2023 at 02:43, Guo Samin <[email protected]> wrote:
> 在 2023/3/6 21:00:19, Emil Renner Berthing 写道:
> > On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> >> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay
> >> configurations.
> >>
> >> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to
> >> switch rx and rx to external clock sources.
> >>
> >> Signed-off-by: Samin Guo <[email protected]>
> >> ---
> >> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++
> >> 1 file changed, 13 insertions(+)
> >>
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
> >> index 4af3300f3cf3..205a13d8c8b1 100644
> >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
> >> @@ -11,3 +11,16 @@
> >> model = "StarFive VisionFive 2 v1.2A";
> >> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110";
> >> };
> >> +
> >> +&gmac1 {
> >> + phy-mode = "rmii";
> >> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>,
> >> + <&syscrg JH7110_SYSCLK_GMAC1_RX>;
> >> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>,
> >> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
> >> +};
> >> +
> >> +&phy0 {
> >> + rx-internal-delay-ps = <1900>;
> >> + tx-internal-delay-ps = <1350>;
> >> +};
> >
> > Here you're not specifying the internal delays for phy1 which means it
> > defaults to 1950ps for both rx and tx. Is that right or did you mean
> > to set them to 0 like the v1.3b phy1?
>
> Hi, emil, usually, only 1000M (rgmii) needs to configure the delay, and 100M(rmii) does not.

Ah, I see.

> > Also your u-boot seems to set what the linux phy driver calls
> > motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all
> > the phys. Did you leave those out on purpose?
>
> Hi, Emil, We did configure motorcomm,auto-sleep-disabled for yt8512 in uboot,
> but Yutai upstream's Linux driver only yt8521/yt8531 supports this property.

I'm confused. Is Yutai also Frank Sae? Because he is the one who added
support for the yt8531 upstream.

> Yt8512 is a Generic PHY driver and does not support the configuration of
> motorcomm,auto-sleep-disabled and motorcomm,keep-pll-enabled.

Right phy1 of the 1.2a might use a different phy, but I'm also talking
about phy0 and the v1.3b which does use the yt8531 right?

> And without configuring these two attributes, vf2-1.2a gmac1 also works normally.

Yes, but what I'm worried about is that it only works because u-boot
initialises the PHYs and ethernet may stop working if you're using a
different bootloader or Linux gains support for resetting the PHYs
before use.

>
> Best regards,
> Samin
> >
> >> --
> >> 2.17.1
> >>
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> --
> Best regards,
> Samin

2023-03-08 03:01:54

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration



-------- 原始信息 --------
主题: Re: [PATCH v5 11/12] riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay configuration
From: Emil Renner Berthing <[email protected]>
收件人: Guo Samin <[email protected]>
日期: 2023/3/7

> On Tue, 7 Mar 2023 at 02:43, Guo Samin <[email protected]> wrote:
>> 在 2023/3/6 21:00:19, Emil Renner Berthing 写道:
>>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>>>> v1.2A gmac0 uses motorcomm YT8531(rgmii-id) PHY, and needs delay
>>>> configurations.
>>>>
>>>> v1.2A gmac1 uses motorcomm YT8512(rmii) PHY, and needs to
>>>> switch rx and rx to external clock sources.
>>>>
>>>> Signed-off-by: Samin Guo <[email protected]>
>>>> ---
>>>> .../starfive/jh7110-starfive-visionfive-2-v1.2a.dts | 13 +++++++++++++
>>>> 1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
>>>> index 4af3300f3cf3..205a13d8c8b1 100644
>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dts
>>>> @@ -11,3 +11,16 @@
>>>> model = "StarFive VisionFive 2 v1.2A";
>>>> compatible = "starfive,visionfive-2-v1.2a", "starfive,jh7110";
>>>> };
>>>> +
>>>> +&gmac1 {
>>>> + phy-mode = "rmii";
>>>> + assigned-clocks = <&syscrg JH7110_SYSCLK_GMAC1_TX>,
>>>> + <&syscrg JH7110_SYSCLK_GMAC1_RX>;
>>>> + assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>,
>>>> + <&syscrg JH7110_SYSCLK_GMAC1_RMII_RTX>;
>>>> +};
>>>> +
>>>> +&phy0 {
>>>> + rx-internal-delay-ps = <1900>;
>>>> + tx-internal-delay-ps = <1350>;
>>>> +};
>>>
>>> Here you're not specifying the internal delays for phy1 which means it
>>> defaults to 1950ps for both rx and tx. Is that right or did you mean
>>> to set them to 0 like the v1.3b phy1?
>>
>> Hi, emil, usually, only 1000M (rgmii) needs to configure the delay, and 100M(rmii) does not.
>
> Ah, I see.
>
>>> Also your u-boot seems to set what the linux phy driver calls
>>> motorcomm,keep-pll-enabled and motorcomm,auto-sleep-disabled for all
>>> the phys. Did you leave those out on purpose?
>>
>> Hi, Emil, We did configure motorcomm,auto-sleep-disabled for yt8512 in uboot,
>> but Yutai upstream's Linux driver only yt8521/yt8531 supports this property.
>
> I'm confused. Is Yutai also Frank Sae? Because he is the one who added
> support for the yt8531 upstream.

My fault , Frank Sae is from Motorcomm, also known as Yutai.
yt8531 ==> Yutai 8531
>
>> Yt8512 is a Generic PHY driver and does not support the configuration of
>> motorcomm,auto-sleep-disabled and motorcomm,keep-pll-enabled.
>
> Right phy1 of the 1.2a might use a different phy, but I'm also talking
> about phy0 and the v1.3b which does use the yt8531 right?
Right:
v1.3b: gmac0:yt8531 gmac1:yt8531
v1.2a: gmac0:yt8531 gmac1:yt8512
>
>> And without configuring these two attributes, vf2-1.2a gmac1 also works normally.
>
> Yes, but what I'm worried about is that it only works because u-boot
> initialises the PHYs and ethernet may stop working if you're using a
> different bootloader or Linux gains support for resetting the PHYs
> before use.
>
I have tested that in uboot, use the sd card to start Linux, do not run network programs (do not use uboot to initialize phy and gmac),
and the network works normally in Linux.

Best regards,
Samin

>>
>> Best regards,
>> Samin
>>>
>>>> --
>>>> 2.17.1
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-riscv mailing list
>>>> [email protected]
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>> --
>> Best regards,
>> Samin

--
Best regards,
Samin

2023-03-08 22:03:26

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 07/12] dt-bindings: net: starfive,jh7110-dwmac: Add starfive,syscon

On Fri, Mar 03, 2023 at 04:59:23PM +0800, Samin Guo wrote:
> A phandle to syscon with two arguments that configure phy mode.

This change belongs in patch 4.

>
> Signed-off-by: Samin Guo <[email protected]>
> ---
> .../bindings/net/starfive,jh7110-dwmac.yaml | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> index ca49f08d50dd..79ae635db0a5 100644
> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> @@ -58,6 +58,18 @@ properties:
> 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: Mask 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 mask of phy mode selection.
> +
> allOf:
> - $ref: snps,dwmac.yaml#
>
> @@ -96,6 +108,7 @@ examples:
> snps,en-tx-lpi-clockgating;
> snps,txpbl = <16>;
> snps,rxpbl = <16>;
> + starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
> phy-handle = <&phy0>;
>
> mdio {
> --
> 2.17.1
>

2023-03-09 01:16:54

by Guo Samin

[permalink] [raw]
Subject: Re: [PATCH v5 07/12] dt-bindings: net: starfive,jh7110-dwmac: Add starfive,syscon



-------- 原始信息 --------
主题: Re: [PATCH v5 07/12] dt-bindings: net: starfive,jh7110-dwmac: Add starfive,syscon
From: Rob Herring <[email protected]>

> On Fri, Mar 03, 2023 at 04:59:23PM +0800, Samin Guo wrote:
>> A phandle to syscon with two arguments that configure phy mode.
>
> This change belongs in patch 4.
>
Thank you for pointing out that the next version will be merged into patch4


Best regards,
Samin

>>
>> Signed-off-by: Samin Guo <[email protected]>
>> ---
>> .../bindings/net/starfive,jh7110-dwmac.yaml | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
>> index ca49f08d50dd..79ae635db0a5 100644
>> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
>> @@ -58,6 +58,18 @@ properties:
>> 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: Mask 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 mask of phy mode selection.
>> +
>> allOf:
>> - $ref: snps,dwmac.yaml#
>>
>> @@ -96,6 +108,7 @@ examples:
>> snps,en-tx-lpi-clockgating;
>> snps,txpbl = <16>;
>> snps,rxpbl = <16>;
>> + starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
>> phy-handle = <&phy0>;
>>
>> mdio {
>> --
>> 2.17.1
>>


2023-03-10 08:10:55

by Tommaso Merciai

[permalink] [raw]
Subject: Re: [PATCH v5 00/12] Add Ethernet driver for StarFive JH7110 SoC

Hello Samin,

On Fri, Mar 03, 2023 at 04:59:16PM +0800, Samin Guo wrote:
> This series adds ethernet support for the StarFive JH7110 RISC-V SoC.
> The series includes MAC driver. The MAC version is dwmac-5.20 (from
> Synopsys DesignWare). 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], [5].
> [1]: https://wiki.rvspace.org/
> [2]: https://github.com/SaminGuo/linux/tree/vf2-6.2-gmac
> [3]: https://lore.kernel.org/all/[email protected]/
> [4]: https://lore.kernel.org/all/[email protected]/
> [5]: https://lore.kernel.org/all/[email protected]/

Thanks for this series.
I'm able to boot Linux over nfs into jh7110-starfive-visionfive-2-v1.3b board

Tested-by: Tommaso Merciai <[email protected]>

Regards,
Tommaso

>
> 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.
>
> Patch 12:
> - No update
> Patch 11:
> - Configuration of gmac and phy for visionfive 2 v1.2A.
> Patch 10:
> - Configuration of gmac and phy for visionfive 2 v1.3B.
> Patch 9:
> - Added starfive,syscon for gmac nodes in jh7110.dtsi.
> Patch 8:
> - Added starfive_dwmac_set_mode to set PHY interface mode.
> Patch 7:
> - Added starfive,syscon item in StarFive-dwmac dt-bindings.
> Patch 6:
> - Moved SOC_STARFIVE to ARCH_STARFIVE in Kconfig.
> - Dropped the _PLAT/_plat from the config/function/struct names. (by Emil)
> - Added MODULE_DEVICE_TABLE() and udev will load the module automatically. (by Emil)
> - Used { /* sentinel */ } for the last entry of starfive_eth_match. (by Emil)
> - Added 'tx_use_rgmii_rxin_clk' to struct starfive_dwmac, to mark the clk_tx'parent is rgmii.
> - Suggested by Emil, dropped clk_gtxclk and use clk_tx_inv to set the clock frequency.
> Patch 5:
> - Suggested by Emil, dropped mdio0/1 labels because there is no reference elsewhere.
> Patch 4:
> - Removed GTXC clk in StarFive-dwmac dt-bindings.
> - Added starfive,tx-use-rgmii-clk item in StarFive-dwmac dt-bindings.
> Patch 3:
> - Added an optional reset single 'ahb' in 'snps,dwmac.yaml', according to
> stmmac_probe_config_dt/stmmac_dvr_probe.
> Patch 2:
> - No update
> Patch 1:
> - No update
>
> 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/[email protected]/
> v2 - https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
> v3 - https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
> v4 - https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
>
> 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 (8):
> dt-bindings: net: snps,dwmac: Add an optional resets single 'ahb'
> riscv: dts: starfive: jh7110: Add ethernet device nodes
> net: stmmac: Add glue layer for StarFive JH7110 SoC
> dt-bindings: net: starfive,jh7110-dwmac: Add starfive,syscon
> net: stmmac: starfive_dmac: Add phy interface settings
> riscv: dts: starfive: jh7110: Add syscon to support phy interface
> settings
> riscv: dts: starfive: visionfive-2-v1.3b: Add gmac+phy's delay
> configuration
> riscv: dts: starfive: visionfive-2-v1.2a: Add gmac+phy's delay
> configuration
>
> Yanhong Wang (2):
> dt-bindings: net: Add support StarFive dwmac
> riscv: dts: starfive: visionfive 2: Enable gmac device tree node
>
> .../devicetree/bindings/net/snps,dwmac.yaml | 19 +-
> .../bindings/net/starfive,jh7110-dwmac.yaml | 130 +++++++++++++
> MAINTAINERS | 7 +
> .../jh7110-starfive-visionfive-2-v1.2a.dts | 13 ++
> .../jh7110-starfive-visionfive-2-v1.3b.dts | 27 +++
> .../jh7110-starfive-visionfive-2.dtsi | 10 +
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 93 ++++++++++
> 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 +-
> 11 files changed, 481 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: 11934a315b671ddb09bc7ac5f505649e9f2623c7
> prerequisite-patch-id: ad56ef54d3f2a18025abc9e27321c25beda16422
> prerequisite-patch-id: 1be0fb49e0fbe293ca8fa94601e191b13c8c67d9
> prerequisite-patch-id: 8b402a8d97294a9b568595816b0dc96afc5e6f5d
> prerequisite-patch-id: 5c149662674f9e7dd888e2028fd8c9772948273f
> prerequisite-patch-id: 0caf8a313a9f161447e0480a93b42467378b2164
> prerequisite-patch-id: b2422f7a12f1e86e38c563139f3c1dbafc158efd
> prerequisite-patch-id: be612664eca7049e987bfae15bb460caa82eb211
> prerequisite-patch-id: 8300965cc6c55cad69f009da7916cf9e8ce628e7
> --
> 2.17.1
>