2023-12-15 20:41:14

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 0/9] Enable networking support for StarFive JH7100 SoC

This patch series adds ethernet support for the StarFive JH7100 SoC and makes it
available for the StarFive VisionFive V1 and BeagleV Starlight boards, although
I could only validate on the former SBC. Thank you Emil and Geert for helping
with tests on BeagleV!

The work is heavily based on the reference implementation [1] and depends the
SiFive Composable Cache controller and non-coherent DMA support provided by Emil
via [2] and [3].

*Update*: as of next-20231214, all dependencies have been merged.

[1] https://github.com/starfive-tech/linux/commits/visionfive
[2] https://lore.kernel.org/all/CAJM55Z_pdoGxRXbmBgJ5GbVWyeM1N6+LHihbNdT26Oo_qA5VYA@mail.gmail.com/
[3] https://lore.kernel.org/all/[email protected]/

Changes in v3:
- Rebased series onto next-20231214 and dropped the ccache & DMA coherency
related patches (v2 06-08/12) handled by Emil via [3]
- Squashed PATCH v2 01/12 into PATCH v3 2/9, per Krzysztof's review
- Dropped incorrect PATCH v2 02/12
- Incorporated Emil's feedback; also added his Co-developed-by on all dts
patches
- Documented the need of adjusting RX internal delay in PATCH v3 8/9, per
Andrew's request
- Added clock fixes from Emil (PATCH v3 8-9/9) required to support 10/100Mb
link speeds
- v2:
https://lore.kernel.org/lkml/[email protected]/

Changes in v2:
- Dropped ccache PATCH 01-05 reworked by Emil via [2]
- Dropped already applied PATCH 06/12
- Added PATCH v2 01 to prepare snps-dwmac binding for JH7100 support
- Added PATCH v2 02-03 to provide some jh7110-dwmac binding optimizations
- Handled JH7110 conflicting work in PATCH 07 via PATCH v2 04
- Reworked PATCH 8 via PATCH v2 05, adding JH7100 quirk and dropped
starfive,gtxclk-dlychain DT property; also fixed register naming
- Added PATCH v2 08 providing DMA coherency related DT changes
- Updated PATCH 9 commit msg:
s/OF_DMA_DEFAULT_COHERENT/ARCH_DMA_DEFAULT_COHERENT/
- Replaced 'uncached-offset' property with 'sifive,cache-ops' in PATCH 10/12
and dropped 'sideband' reg
- Add new patch providing coherent DMA memory pool (PATCH v2 10)
- Updated PATCH 11/12 according to the stmmac glue layer changes in upstream
- Split PATCH 12/12 into PATCH v2 10-12 to handle individual gmac setup of
VisionFive v1 and BeagleV boards as they use different PHYs; also switched
phy-mode from "rgmii-tx" to "rgmii-id" (requires a reduction of
rx-internal-delay-ps by ~50%)
- Rebased series onto next-20231024
- v1:
https://lore.kernel.org/lkml/[email protected]/

Cristian Ciocaltea (7):
dt-bindings: net: starfive,jh7110-dwmac: Drop redundant reset
description
dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible
net: stmmac: dwmac-starfive: Add support for JH7100 SoC
riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes
riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac
riscv: dts: starfive: visionfive-v1: Setup ethernet phy
riscv: dts: starfive: beaglev-starlight: Setup phy reset gpio

Emil Renner Berthing (2):
clk: starfive: Add flags argument to JH71X0__MUX macro
clk: starfive: jh7100: Add CLK_SET_RATE_PARENT to gmac_tx

.../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
.../bindings/net/starfive,jh7110-dwmac.yaml | 75 +++++++++++-----
.../dts/starfive/jh7100-beaglev-starlight.dts | 7 ++
.../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++
.../jh7100-starfive-visionfive-v1.dts | 7 ++
arch/riscv/boot/dts/starfive/jh7100.dtsi | 37 ++++++++
.../clk/starfive/clk-starfive-jh7100-audio.c | 2 +-
drivers/clk/starfive/clk-starfive-jh7100.c | 32 +++----
.../clk/starfive/clk-starfive-jh7110-aon.c | 6 +-
.../clk/starfive/clk-starfive-jh7110-isp.c | 2 +-
.../clk/starfive/clk-starfive-jh7110-sys.c | 26 +++---
drivers/clk/starfive/clk-starfive-jh71x0.h | 4 +-
drivers/net/ethernet/stmicro/stmmac/Kconfig | 6 +-
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 32 ++++++-
14 files changed, 258 insertions(+), 66 deletions(-)

--
2.43.0



2023-12-15 20:41:25

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 1/9] dt-bindings: net: starfive,jh7110-dwmac: Drop redundant reset description

The reset description items are already provided by the referenced
snps,dwmac.yaml schema, hence replace them with the necessary
{min,max}Items.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../devicetree/bindings/net/starfive,jh7110-dwmac.yaml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
index 5e7cfbbebce6..d90cb82c1424 100644
--- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
@@ -55,9 +55,8 @@ properties:
maxItems: 3

resets:
- items:
- - description: MAC Reset signal.
- - description: AHB Reset signal.
+ minItems: 2
+ maxItems: 2

reset-names:
items:
--
2.43.0


2023-12-15 20:41:35

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
similar to the newer JH7110, but it requires only two interrupts and a
single reset line, which is 'ahb' instead of the commonly used
'stmmaceth'.

Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
conjunction with 'stmmaceth', extend the logic to also permit exclusive
usage of the 'ahb' reset name. This ensures the following use cases are
supported:

JH7110: reset-names = "stmmaceth", "ahb";
JH7100: reset-names = "ahb";
other: reset-names = "stmmaceth";

Also note the need to use a different dwmac fallback, as v5.20 applies
to JH7110 only, while JH7100 relies on v3.7x.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
.../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
2 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 5c2769dc689a..c1380ff1c054 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -95,6 +95,7 @@ properties:
- snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
+ - starfive,jh7100-dwmac
- starfive,jh7110-dwmac

reg:
@@ -146,7 +147,7 @@ properties:
reset-names:
minItems: 1
items:
- - const: stmmaceth
+ - enum: [stmmaceth, ahb]
- const: ahb

power-domains:
diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
index d90cb82c1424..b01602738fc3 100644
--- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
@@ -22,10 +22,14 @@ select:

properties:
compatible:
- items:
- - enum:
- - starfive,jh7110-dwmac
- - const: snps,dwmac-5.20
+ oneOf:
+ - items:
+ - const: starfive,jh7100-dwmac
+ - const: snps,dwmac
+ - items:
+ - enum:
+ - starfive,jh7110-dwmac
+ - const: snps,dwmac-5.20

reg:
maxItems: 1
@@ -46,23 +50,6 @@ properties:
- const: tx
- const: gtx

- interrupts:
- minItems: 3
- maxItems: 3
-
- interrupt-names:
- minItems: 3
- maxItems: 3
-
- resets:
- minItems: 2
- maxItems: 2
-
- reset-names:
- items:
- - const: stmmaceth
- - const: ahb
-
starfive,tx-use-rgmii-clk:
description:
Tx clock is provided by external rgmii clock.
@@ -93,6 +80,51 @@ required:
allOf:
- $ref: snps,dwmac.yaml#

+ - if:
+ properties:
+ compatible:
+ contains:
+ const: starfive,jh7100-dwmac
+ then:
+ properties:
+ interrupts:
+ minItems: 2
+ maxItems: 2
+
+ interrupt-names:
+ minItems: 2
+ maxItems: 2
+
+ resets:
+ maxItems: 1
+
+ reset-names:
+ const: ahb
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: starfive,jh7110-dwmac
+ then:
+ properties:
+ interrupts:
+ minItems: 3
+ maxItems: 3
+
+ interrupt-names:
+ minItems: 3
+ maxItems: 3
+
+ resets:
+ minItems: 2
+ maxItems: 2
+
+ reset-names:
+ items:
+ - const: stmmaceth
+ - const: ahb
+
unevaluatedProperties: false

examples:
--
2.43.0


2023-12-15 20:42:03

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 3/9] net: stmmac: dwmac-starfive: Add support for JH7100 SoC

Add a missing quirk to enable support for the StarFive JH7100 SoC.

Additionally, for greater flexibility in operation, allow using the
rgmii-rxid and rgmii-txid phy modes.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 6 ++--
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 32 ++++++++++++++++---
2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 85dcda51df05..4ec61f1ee71a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -165,9 +165,9 @@ config DWMAC_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.
+ This selects the StarFive platform specific glue layer support
+ for the stmmac device driver. This driver is used for the
+ StarFive JH7100 and JH7110 ethernet controllers.

config DWMAC_STI
tristate "STi GMAC support"
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
index 5d630affb4d1..4e1076faee0c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -15,13 +15,20 @@

#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
+#define STARFIVE_DWMAC_PHY_INFT_RGMII 0x1
+#define STARFIVE_DWMAC_PHY_INFT_RMII 0x4
+#define STARFIVE_DWMAC_PHY_INFT_FIELD 0x7U
+
+#define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8
+
+struct starfive_dwmac_data {
+ unsigned int gtxclk_dlychain;
+};

struct starfive_dwmac {
struct device *dev;
struct clk *clk_tx;
+ const struct starfive_dwmac_data *data;
};

static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
@@ -67,6 +74,8 @@ static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)

case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
mode = STARFIVE_DWMAC_PHY_INFT_RGMII;
break;

@@ -89,6 +98,14 @@ static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
if (err)
return dev_err_probe(dwmac->dev, err, "error setting phy mode\n");

+ if (dwmac->data) {
+ err = regmap_write(regmap, JH7100_SYSMAIN_REGISTER49_DLYCHAIN,
+ dwmac->data->gtxclk_dlychain);
+ if (err)
+ return dev_err_probe(dwmac->dev, err,
+ "error selecting gtxclk delay chain\n");
+ }
+
return 0;
}

@@ -114,6 +131,8 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
if (!dwmac)
return -ENOMEM;

+ dwmac->data = device_get_match_data(&pdev->dev);
+
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),
@@ -144,8 +163,13 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
}

+static const struct starfive_dwmac_data jh7100_data = {
+ .gtxclk_dlychain = 4,
+};
+
static const struct of_device_id starfive_dwmac_match[] = {
- { .compatible = "starfive,jh7110-dwmac" },
+ { .compatible = "starfive,jh7100-dwmac", .data = &jh7100_data },
+ { .compatible = "starfive,jh7110-dwmac" },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
--
2.43.0


2023-12-15 20:42:26

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 4/9] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

Provide the sysmain and gmac DT nodes supporting the DWMAC found on the
StarFive JH7100 SoC.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7100.dtsi | 37 ++++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index c216aaecac53..985383745e77 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -204,6 +204,38 @@ sdio1: mmc@10010000 {
status = "disabled";
};

+ gmac: ethernet@10020000 {
+ compatible = "starfive,jh7100-dwmac", "snps,dwmac";
+ reg = <0x0 0x10020000 0x0 0x10000>;
+ clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
+ <&clkgen JH7100_CLK_GMAC_AHB>,
+ <&clkgen JH7100_CLK_GMAC_PTP_REF>,
+ <&clkgen JH7100_CLK_GMAC_TX_INV>,
+ <&clkgen JH7100_CLK_GMAC_GTX>;
+ clock-names = "stmmaceth", "pclk", "ptp_ref", "tx", "gtx";
+ resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
+ reset-names = "ahb";
+ interrupts = <6>, <7>;
+ interrupt-names = "macirq", "eth_wake_irq";
+ max-frame-size = <9000>;
+ snps,multicast-filter-bins = <32>;
+ snps,perfect-filter-entries = <128>;
+ starfive,syscon = <&sysmain 0x70 0>;
+ rx-fifo-depth = <32768>;
+ tx-fifo-depth = <16384>;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,fixed-burst;
+ snps,force_thresh_dma_mode;
+ snps,no-pbl-x8;
+ status = "disabled";
+
+ stmmac_axi_setup: stmmac-axi-config {
+ snps,wr_osr_lmt = <16>;
+ snps,rd_osr_lmt = <16>;
+ snps,blen = <256 128 64 32 0 0 0>;
+ };
+ };
+
clkgen: clock-controller@11800000 {
compatible = "starfive,jh7100-clkgen";
reg = <0x0 0x11800000 0x0 0x10000>;
@@ -218,6 +250,11 @@ rstgen: reset-controller@11840000 {
#reset-cells = <1>;
};

+ sysmain: syscon@11850000 {
+ compatible = "starfive,jh7100-sysmain", "syscon";
+ reg = <0x0 0x11850000 0x0 0x10000>;
+ };
+
i2c0: i2c@118b0000 {
compatible = "snps,designware-i2c";
reg = <0x0 0x118b0000 0x0 0x10000>;
--
2.43.0


2023-12-15 20:42:46

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 5/9] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

Add pinmux configuration for DWMAC found on the JH7100 based boards and
enable the related DT node, providing a basic PHY configuration.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++
1 file changed, 85 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index 42fb61c36068..5cafe8f5c2e7 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq {
};
};

+&gmac {
+ pinctrl-names = "default";
+ pinctrl-0 = <&gmac_pins>;
+ phy-mode = "rgmii-id";
+ phy-handle = <&phy>;
+ status = "okay";
+
+ mdio: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+ };
+};
+
&gpio {
+ gmac_pins: gmac-0 {
+ gtxclk-pins {
+ pins = <PAD_FUNC_SHARE(115)>;
+ bias-pull-up;
+ drive-strength = <35>;
+ input-enable;
+ input-schmitt-enable;
+ slew-rate = <0>;
+ };
+ miitxclk-pins {
+ pins = <PAD_FUNC_SHARE(116)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ tx-pins {
+ pins = <PAD_FUNC_SHARE(117)>,
+ <PAD_FUNC_SHARE(119)>,
+ <PAD_FUNC_SHARE(120)>,
+ <PAD_FUNC_SHARE(121)>,
+ <PAD_FUNC_SHARE(122)>,
+ <PAD_FUNC_SHARE(123)>,
+ <PAD_FUNC_SHARE(124)>,
+ <PAD_FUNC_SHARE(125)>,
+ <PAD_FUNC_SHARE(126)>;
+ bias-pull-up;
+ drive-strength = <35>;
+ input-disable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ rxclk-pins {
+ pins = <PAD_FUNC_SHARE(127)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-disable;
+ slew-rate = <6>;
+ };
+ rxer-pins {
+ pins = <PAD_FUNC_SHARE(129)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ rx-pins {
+ pins = <PAD_FUNC_SHARE(128)>,
+ <PAD_FUNC_SHARE(130)>,
+ <PAD_FUNC_SHARE(131)>,
+ <PAD_FUNC_SHARE(132)>,
+ <PAD_FUNC_SHARE(133)>,
+ <PAD_FUNC_SHARE(134)>,
+ <PAD_FUNC_SHARE(135)>,
+ <PAD_FUNC_SHARE(136)>,
+ <PAD_FUNC_SHARE(137)>,
+ <PAD_FUNC_SHARE(138)>,
+ <PAD_FUNC_SHARE(139)>,
+ <PAD_FUNC_SHARE(140)>,
+ <PAD_FUNC_SHARE(141)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-enable;
+ slew-rate = <0>;
+ };
+ };
+
i2c0_pins: i2c0-0 {
i2c-pins {
pinmux = <GPIOMUX(62, GPO_LOW,
--
2.43.0


2023-12-15 20:43:09

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 6/9] riscv: dts: starfive: visionfive-v1: Setup ethernet phy

The StarFive VisionFive V1 SBC uses a Motorcomm YT8521 PHY supporting
RGMII-ID, but requires manual adjustment of the RX internal delay to
work properly.

The default RX delay provided by the driver is 1.95 ns, which proves to
be too high. Applying a 50% reduction seems to mitigate the issue.

Also note this adjustment is not necessary on BeagleV Starlight SBC,
which uses a Microchip PHY. Hence, there is no indication of a
miss-behaviour on the GMAC side, but most likely the issue stems from
the Motorcomm PHY.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../boot/dts/starfive/jh7100-starfive-visionfive-v1.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-starfive-visionfive-v1.dts b/arch/riscv/boot/dts/starfive/jh7100-starfive-visionfive-v1.dts
index e82af72f1aaf..ca134b9f11bf 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-starfive-visionfive-v1.dts
+++ b/arch/riscv/boot/dts/starfive/jh7100-starfive-visionfive-v1.dts
@@ -18,3 +18,10 @@ gpio-restart {
priority = <224>;
};
};
+
+&mdio {
+ phy: ethernet-phy@0 {
+ reg = <0>;
+ rx-internal-delay-ps = <900>;
+ };
+};
--
2.43.0


2023-12-15 20:43:32

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 7/9] riscv: dts: starfive: beaglev-starlight: Setup phy reset gpio

The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting
RGMII-ID which doesn't require any particular setup, other than defining
a reset gpio, as opposed to VisionFive V1 for which the RX internal
delay had to be adjusted.

Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts
index 7cda3a89020a..5ac9961226fc 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts
+++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts
@@ -11,3 +11,10 @@ / {
model = "BeagleV Starlight Beta";
compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100";
};
+
+&mdio {
+ phy: ethernet-phy@7 {
+ reg = <7>;
+ reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW>;
+ };
+};
--
2.43.0


2023-12-15 20:43:59

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 8/9] clk: starfive: Add flags argument to JH71X0__MUX macro

From: Emil Renner Berthing <[email protected]>

This flag is needed to add the CLK_SET_RATE_PARENT flag on the gmac_tx
clock on the JH7100, which in turn is needed by the dwmac-starfive
driver to set the clock properly for 1000, 100 and 10 Mbps links.

This change was mostly made using coccinelle:

@ match @
expression idx, name, nparents;
@@
JH71X0__MUX(
-idx, name, nparents,
+idx, name, 0, nparents,
...)

Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../clk/starfive/clk-starfive-jh7100-audio.c | 2 +-
drivers/clk/starfive/clk-starfive-jh7100.c | 32 +++++++++----------
.../clk/starfive/clk-starfive-jh7110-aon.c | 6 ++--
.../clk/starfive/clk-starfive-jh7110-isp.c | 2 +-
.../clk/starfive/clk-starfive-jh7110-sys.c | 26 +++++++--------
drivers/clk/starfive/clk-starfive-jh71x0.h | 4 +--
6 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/clk/starfive/clk-starfive-jh7100-audio.c b/drivers/clk/starfive/clk-starfive-jh7100-audio.c
index ee4bda14a40e..1fcf4e62f347 100644
--- a/drivers/clk/starfive/clk-starfive-jh7100-audio.c
+++ b/drivers/clk/starfive/clk-starfive-jh7100-audio.c
@@ -79,7 +79,7 @@ static const struct jh71x0_clk_data jh7100_audclk_data[] = {
JH71X0_GDIV(JH7100_AUDCLK_USB_LPM, "usb_lpm", CLK_IGNORE_UNUSED, 4, JH7100_AUDCLK_USB_APB),
JH71X0_GDIV(JH7100_AUDCLK_USB_STB, "usb_stb", CLK_IGNORE_UNUSED, 3, JH7100_AUDCLK_USB_APB),
JH71X0__DIV(JH7100_AUDCLK_APB_EN, "apb_en", 8, JH7100_AUDCLK_DOM7AHB_BUS),
- JH71X0__MUX(JH7100_AUDCLK_VAD_MEM, "vad_mem", 2,
+ JH71X0__MUX(JH7100_AUDCLK_VAD_MEM, "vad_mem", 0, 2,
JH7100_AUDCLK_VAD_INTMEM,
JH7100_AUDCLK_AUDIO_12288),
};
diff --git a/drivers/clk/starfive/clk-starfive-jh7100.c b/drivers/clk/starfive/clk-starfive-jh7100.c
index 69cc11ea7e33..d3b260c01d5c 100644
--- a/drivers/clk/starfive/clk-starfive-jh7100.c
+++ b/drivers/clk/starfive/clk-starfive-jh7100.c
@@ -24,48 +24,48 @@
#define JH7100_CLK_GMAC_GR_MII_RX (JH7100_CLK_END + 3)

static const struct jh71x0_clk_data jh7100_clk_data[] __initconst = {
- JH71X0__MUX(JH7100_CLK_CPUNDBUS_ROOT, "cpundbus_root", 4,
+ JH71X0__MUX(JH7100_CLK_CPUNDBUS_ROOT, "cpundbus_root", 0, 4,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL0_OUT,
JH7100_CLK_PLL1_OUT,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_DLA_ROOT, "dla_root", 3,
+ JH71X0__MUX(JH7100_CLK_DLA_ROOT, "dla_root", 0, 3,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL1_OUT,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_DSP_ROOT, "dsp_root", 4,
+ JH71X0__MUX(JH7100_CLK_DSP_ROOT, "dsp_root", 0, 4,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL0_OUT,
JH7100_CLK_PLL1_OUT,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_GMACUSB_ROOT, "gmacusb_root", 3,
+ JH71X0__MUX(JH7100_CLK_GMACUSB_ROOT, "gmacusb_root", 0, 3,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL0_OUT,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_PERH0_ROOT, "perh0_root", 2,
+ JH71X0__MUX(JH7100_CLK_PERH0_ROOT, "perh0_root", 0, 2,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL0_OUT),
- JH71X0__MUX(JH7100_CLK_PERH1_ROOT, "perh1_root", 2,
+ JH71X0__MUX(JH7100_CLK_PERH1_ROOT, "perh1_root", 0, 2,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_VIN_ROOT, "vin_root", 3,
+ JH71X0__MUX(JH7100_CLK_VIN_ROOT, "vin_root", 0, 3,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL1_OUT,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_VOUT_ROOT, "vout_root", 3,
+ JH71X0__MUX(JH7100_CLK_VOUT_ROOT, "vout_root", 0, 3,
JH7100_CLK_OSC_AUD,
JH7100_CLK_PLL0_OUT,
JH7100_CLK_PLL2_OUT),
JH71X0_GDIV(JH7100_CLK_AUDIO_ROOT, "audio_root", 0, 8, JH7100_CLK_PLL0_OUT),
- JH71X0__MUX(JH7100_CLK_CDECHIFI4_ROOT, "cdechifi4_root", 3,
+ JH71X0__MUX(JH7100_CLK_CDECHIFI4_ROOT, "cdechifi4_root", 0, 3,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL1_OUT,
JH7100_CLK_PLL2_OUT),
- JH71X0__MUX(JH7100_CLK_CDEC_ROOT, "cdec_root", 3,
+ JH71X0__MUX(JH7100_CLK_CDEC_ROOT, "cdec_root", 0, 3,
JH7100_CLK_OSC_SYS,
JH7100_CLK_PLL0_OUT,
JH7100_CLK_PLL1_OUT),
- JH71X0__MUX(JH7100_CLK_VOUTBUS_ROOT, "voutbus_root", 3,
+ JH71X0__MUX(JH7100_CLK_VOUTBUS_ROOT, "voutbus_root", 0, 3,
JH7100_CLK_OSC_AUD,
JH7100_CLK_PLL0_OUT,
JH7100_CLK_PLL2_OUT),
@@ -76,7 +76,7 @@ static const struct jh71x0_clk_data jh7100_clk_data[] __initconst = {
JH71X0_GDIV(JH7100_CLK_PLL0_TESTOUT, "pll0_testout", 0, 31, JH7100_CLK_PERH0_SRC),
JH71X0_GDIV(JH7100_CLK_PLL1_TESTOUT, "pll1_testout", 0, 31, JH7100_CLK_DLA_ROOT),
JH71X0_GDIV(JH7100_CLK_PLL2_TESTOUT, "pll2_testout", 0, 31, JH7100_CLK_PERH1_SRC),
- JH71X0__MUX(JH7100_CLK_PLL2_REF, "pll2_refclk", 2,
+ JH71X0__MUX(JH7100_CLK_PLL2_REF, "pll2_refclk", 0, 2,
JH7100_CLK_OSC_SYS,
JH7100_CLK_OSC_AUD),
JH71X0__DIV(JH7100_CLK_CPU_CORE, "cpu_core", 8, JH7100_CLK_CPUNBUS_ROOT_DIV),
@@ -142,7 +142,7 @@ static const struct jh71x0_clk_data jh7100_clk_data[] __initconst = {
JH71X0__DIV(JH7100_CLK_NOC_COG, "noc_cog", 8, JH7100_CLK_DLA_ROOT),
JH71X0_GATE(JH7100_CLK_NNE_AHB, "nne_ahb", 0, JH7100_CLK_AHB_BUS),
JH71X0__DIV(JH7100_CLK_NNEBUS_SRC1, "nnebus_src1", 4, JH7100_CLK_DSP_ROOT),
- JH71X0__MUX(JH7100_CLK_NNE_BUS, "nne_bus", 2,
+ JH71X0__MUX(JH7100_CLK_NNE_BUS, "nne_bus", 0, 2,
JH7100_CLK_CPU_AXI,
JH7100_CLK_NNEBUS_SRC1),
JH71X0_GATE(JH7100_CLK_NNE_AXI, "nne_axi", 0, JH7100_CLK_NNE_BUS),
@@ -166,7 +166,7 @@ static const struct jh71x0_clk_data jh7100_clk_data[] __initconst = {
JH71X0_GDIV(JH7100_CLK_USBPHY_125M, "usbphy_125m", 0, 8, JH7100_CLK_USBPHY_ROOTDIV),
JH71X0_GDIV(JH7100_CLK_USBPHY_PLLDIV25M, "usbphy_plldiv25m", 0, 32,
JH7100_CLK_USBPHY_ROOTDIV),
- JH71X0__MUX(JH7100_CLK_USBPHY_25M, "usbphy_25m", 2,
+ JH71X0__MUX(JH7100_CLK_USBPHY_25M, "usbphy_25m", 0, 2,
JH7100_CLK_OSC_SYS,
JH7100_CLK_USBPHY_PLLDIV25M),
JH71X0_FDIV(JH7100_CLK_AUDIO_DIV, "audio_div", JH7100_CLK_AUDIO_ROOT),
@@ -200,12 +200,12 @@ static const struct jh71x0_clk_data jh7100_clk_data[] __initconst = {
JH71X0_GDIV(JH7100_CLK_GMAC_GTX, "gmac_gtxclk", 0, 255, JH7100_CLK_GMAC_ROOT_DIV),
JH71X0_GDIV(JH7100_CLK_GMAC_RMII_TX, "gmac_rmii_txclk", 0, 8, JH7100_CLK_GMAC_RMII_REF),
JH71X0_GDIV(JH7100_CLK_GMAC_RMII_RX, "gmac_rmii_rxclk", 0, 8, JH7100_CLK_GMAC_RMII_REF),
- JH71X0__MUX(JH7100_CLK_GMAC_TX, "gmac_tx", 3,
+ JH71X0__MUX(JH7100_CLK_GMAC_TX, "gmac_tx", 0, 3,
JH7100_CLK_GMAC_GTX,
JH7100_CLK_GMAC_TX_INV,
JH7100_CLK_GMAC_RMII_TX),
JH71X0__INV(JH7100_CLK_GMAC_TX_INV, "gmac_tx_inv", JH7100_CLK_GMAC_TX),
- JH71X0__MUX(JH7100_CLK_GMAC_RX_PRE, "gmac_rx_pre", 2,
+ JH71X0__MUX(JH7100_CLK_GMAC_RX_PRE, "gmac_rx_pre", 0, 2,
JH7100_CLK_GMAC_GR_MII_RX,
JH7100_CLK_GMAC_RMII_RX),
JH71X0__INV(JH7100_CLK_GMAC_RX_INV, "gmac_rx_inv", JH7100_CLK_GMAC_RX_PRE),
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-aon.c b/drivers/clk/starfive/clk-starfive-jh7110-aon.c
index 62954eb7b50a..418efdad719b 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-aon.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-aon.c
@@ -26,7 +26,7 @@
static const struct jh71x0_clk_data jh7110_aonclk_data[] = {
/* source */
JH71X0__DIV(JH7110_AONCLK_OSC_DIV4, "osc_div4", 4, JH7110_AONCLK_OSC),
- JH71X0__MUX(JH7110_AONCLK_APB_FUNC, "apb_func", 2,
+ JH71X0__MUX(JH7110_AONCLK_APB_FUNC, "apb_func", 0, 2,
JH7110_AONCLK_OSC_DIV4,
JH7110_AONCLK_OSC),
/* gmac0 */
@@ -39,7 +39,7 @@ static const struct jh71x0_clk_data jh7110_aonclk_data[] = {
JH7110_AONCLK_GMAC0_GTXCLK,
JH7110_AONCLK_GMAC0_RMII_RTX),
JH71X0__INV(JH7110_AONCLK_GMAC0_TX_INV, "gmac0_tx_inv", JH7110_AONCLK_GMAC0_TX),
- JH71X0__MUX(JH7110_AONCLK_GMAC0_RX, "gmac0_rx", 2,
+ JH71X0__MUX(JH7110_AONCLK_GMAC0_RX, "gmac0_rx", 0, 2,
JH7110_AONCLK_GMAC0_RGMII_RXIN,
JH7110_AONCLK_GMAC0_RMII_RTX),
JH71X0__INV(JH7110_AONCLK_GMAC0_RX_INV, "gmac0_rx_inv", JH7110_AONCLK_GMAC0_RX),
@@ -48,7 +48,7 @@ static const struct jh71x0_clk_data jh7110_aonclk_data[] = {
/* rtc */
JH71X0_GATE(JH7110_AONCLK_RTC_APB, "rtc_apb", 0, JH7110_AONCLK_APB_BUS),
JH71X0__DIV(JH7110_AONCLK_RTC_INTERNAL, "rtc_internal", 1022, JH7110_AONCLK_OSC),
- JH71X0__MUX(JH7110_AONCLK_RTC_32K, "rtc_32k", 2,
+ JH71X0__MUX(JH7110_AONCLK_RTC_32K, "rtc_32k", 0, 2,
JH7110_AONCLK_RTC_OSC,
JH7110_AONCLK_RTC_INTERNAL),
JH71X0_GATE(JH7110_AONCLK_RTC_CAL, "rtc_cal", 0, JH7110_AONCLK_OSC),
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-isp.c b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
index ce034ed28532..929b8788279e 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-isp.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
@@ -53,7 +53,7 @@ static const struct jh71x0_clk_data jh7110_ispclk_data[] = {
JH7110_ISPCLK_MIPI_RX0_PXL),
JH71X0_GATE(JH7110_ISPCLK_VIN_PIXEL_IF3, "vin_pixel_if3", 0,
JH7110_ISPCLK_MIPI_RX0_PXL),
- JH71X0__MUX(JH7110_ISPCLK_VIN_P_AXI_WR, "vin_p_axi_wr", 2,
+ JH71X0__MUX(JH7110_ISPCLK_VIN_P_AXI_WR, "vin_p_axi_wr", 0, 2,
JH7110_ISPCLK_MIPI_RX0_PXL,
JH7110_ISPCLK_DVP_INV),
/* ispv2_top_wrapper */
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
index 3884eff9fe93..8f5e5abfa178 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
@@ -36,18 +36,18 @@

static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
/* root */
- JH71X0__MUX(JH7110_SYSCLK_CPU_ROOT, "cpu_root", 2,
+ JH71X0__MUX(JH7110_SYSCLK_CPU_ROOT, "cpu_root", 0, 2,
JH7110_SYSCLK_OSC,
JH7110_SYSCLK_PLL0_OUT),
JH71X0__DIV(JH7110_SYSCLK_CPU_CORE, "cpu_core", 7, JH7110_SYSCLK_CPU_ROOT),
JH71X0__DIV(JH7110_SYSCLK_CPU_BUS, "cpu_bus", 2, JH7110_SYSCLK_CPU_CORE),
- JH71X0__MUX(JH7110_SYSCLK_GPU_ROOT, "gpu_root", 2,
+ JH71X0__MUX(JH7110_SYSCLK_GPU_ROOT, "gpu_root", 0, 2,
JH7110_SYSCLK_PLL2_OUT,
JH7110_SYSCLK_PLL1_OUT),
JH71X0_MDIV(JH7110_SYSCLK_PERH_ROOT, "perh_root", 2, 2,
JH7110_SYSCLK_PLL0_OUT,
JH7110_SYSCLK_PLL2_OUT),
- JH71X0__MUX(JH7110_SYSCLK_BUS_ROOT, "bus_root", 2,
+ JH71X0__MUX(JH7110_SYSCLK_BUS_ROOT, "bus_root", 0, 2,
JH7110_SYSCLK_OSC,
JH7110_SYSCLK_PLL2_OUT),
JH71X0__DIV(JH7110_SYSCLK_NOCSTG_BUS, "nocstg_bus", 3, JH7110_SYSCLK_BUS_ROOT),
@@ -62,7 +62,7 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0__DIV(JH7110_SYSCLK_PLL2_DIV2, "pll2_div2", 2, JH7110_SYSCLK_PLL2_OUT),
JH71X0__DIV(JH7110_SYSCLK_AUDIO_ROOT, "audio_root", 8, JH7110_SYSCLK_PLL2_OUT),
JH71X0__DIV(JH7110_SYSCLK_MCLK_INNER, "mclk_inner", 64, JH7110_SYSCLK_AUDIO_ROOT),
- JH71X0__MUX(JH7110_SYSCLK_MCLK, "mclk", 2,
+ JH71X0__MUX(JH7110_SYSCLK_MCLK, "mclk", 0, 2,
JH7110_SYSCLK_MCLK_INNER,
JH7110_SYSCLK_MCLK_EXT),
JH71X0_GATE(JH7110_SYSCLK_MCLK_OUT, "mclk_out", 0, JH7110_SYSCLK_MCLK_INNER),
@@ -96,7 +96,7 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0__DIV(JH7110_SYSCLK_OSC_DIV2, "osc_div2", 2, JH7110_SYSCLK_OSC),
JH71X0__DIV(JH7110_SYSCLK_PLL1_DIV4, "pll1_div4", 2, JH7110_SYSCLK_PLL1_DIV2),
JH71X0__DIV(JH7110_SYSCLK_PLL1_DIV8, "pll1_div8", 2, JH7110_SYSCLK_PLL1_DIV4),
- JH71X0__MUX(JH7110_SYSCLK_DDR_BUS, "ddr_bus", 4,
+ JH71X0__MUX(JH7110_SYSCLK_DDR_BUS, "ddr_bus", 0, 4,
JH7110_SYSCLK_OSC_DIV2,
JH7110_SYSCLK_PLL1_DIV2,
JH7110_SYSCLK_PLL1_DIV4,
@@ -186,7 +186,7 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0__DIV(JH7110_SYSCLK_GMAC1_RMII_RTX, "gmac1_rmii_rtx", 30,
JH7110_SYSCLK_GMAC1_RMII_REFIN),
JH71X0_GDIV(JH7110_SYSCLK_GMAC1_PTP, "gmac1_ptp", 0, 31, JH7110_SYSCLK_GMAC_SRC),
- JH71X0__MUX(JH7110_SYSCLK_GMAC1_RX, "gmac1_rx", 2,
+ JH71X0__MUX(JH7110_SYSCLK_GMAC1_RX, "gmac1_rx", 0, 2,
JH7110_SYSCLK_GMAC1_RGMII_RXIN,
JH7110_SYSCLK_GMAC1_RMII_RTX),
JH71X0__INV(JH7110_SYSCLK_GMAC1_RX_INV, "gmac1_rx_inv", JH7110_SYSCLK_GMAC1_RX),
@@ -270,11 +270,11 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0_MDIV(JH7110_SYSCLK_I2STX0_LRCK_MST, "i2stx0_lrck_mst", 64, 2,
JH7110_SYSCLK_I2STX0_BCLK_MST_INV,
JH7110_SYSCLK_I2STX0_BCLK_MST),
- JH71X0__MUX(JH7110_SYSCLK_I2STX0_BCLK, "i2stx0_bclk", 2,
+ JH71X0__MUX(JH7110_SYSCLK_I2STX0_BCLK, "i2stx0_bclk", 0, 2,
JH7110_SYSCLK_I2STX0_BCLK_MST,
JH7110_SYSCLK_I2STX_BCLK_EXT),
JH71X0__INV(JH7110_SYSCLK_I2STX0_BCLK_INV, "i2stx0_bclk_inv", JH7110_SYSCLK_I2STX0_BCLK),
- JH71X0__MUX(JH7110_SYSCLK_I2STX0_LRCK, "i2stx0_lrck", 2,
+ JH71X0__MUX(JH7110_SYSCLK_I2STX0_LRCK, "i2stx0_lrck", 0, 2,
JH7110_SYSCLK_I2STX0_LRCK_MST,
JH7110_SYSCLK_I2STX_LRCK_EXT),
/* i2stx1 */
@@ -285,11 +285,11 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0_MDIV(JH7110_SYSCLK_I2STX1_LRCK_MST, "i2stx1_lrck_mst", 64, 2,
JH7110_SYSCLK_I2STX1_BCLK_MST_INV,
JH7110_SYSCLK_I2STX1_BCLK_MST),
- JH71X0__MUX(JH7110_SYSCLK_I2STX1_BCLK, "i2stx1_bclk", 2,
+ JH71X0__MUX(JH7110_SYSCLK_I2STX1_BCLK, "i2stx1_bclk", 0, 2,
JH7110_SYSCLK_I2STX1_BCLK_MST,
JH7110_SYSCLK_I2STX_BCLK_EXT),
JH71X0__INV(JH7110_SYSCLK_I2STX1_BCLK_INV, "i2stx1_bclk_inv", JH7110_SYSCLK_I2STX1_BCLK),
- JH71X0__MUX(JH7110_SYSCLK_I2STX1_LRCK, "i2stx1_lrck", 2,
+ JH71X0__MUX(JH7110_SYSCLK_I2STX1_LRCK, "i2stx1_lrck", 0, 2,
JH7110_SYSCLK_I2STX1_LRCK_MST,
JH7110_SYSCLK_I2STX_LRCK_EXT),
/* i2srx */
@@ -300,11 +300,11 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0_MDIV(JH7110_SYSCLK_I2SRX_LRCK_MST, "i2srx_lrck_mst", 64, 2,
JH7110_SYSCLK_I2SRX_BCLK_MST_INV,
JH7110_SYSCLK_I2SRX_BCLK_MST),
- JH71X0__MUX(JH7110_SYSCLK_I2SRX_BCLK, "i2srx_bclk", 2,
+ JH71X0__MUX(JH7110_SYSCLK_I2SRX_BCLK, "i2srx_bclk", 0, 2,
JH7110_SYSCLK_I2SRX_BCLK_MST,
JH7110_SYSCLK_I2SRX_BCLK_EXT),
JH71X0__INV(JH7110_SYSCLK_I2SRX_BCLK_INV, "i2srx_bclk_inv", JH7110_SYSCLK_I2SRX_BCLK),
- JH71X0__MUX(JH7110_SYSCLK_I2SRX_LRCK, "i2srx_lrck", 2,
+ JH71X0__MUX(JH7110_SYSCLK_I2SRX_LRCK, "i2srx_lrck", 0, 2,
JH7110_SYSCLK_I2SRX_LRCK_MST,
JH7110_SYSCLK_I2SRX_LRCK_EXT),
/* pdm */
@@ -314,7 +314,7 @@ static const struct jh71x0_clk_data jh7110_sysclk_data[] __initconst = {
JH71X0_GATE(JH7110_SYSCLK_TDM_AHB, "tdm_ahb", 0, JH7110_SYSCLK_AHB0),
JH71X0_GATE(JH7110_SYSCLK_TDM_APB, "tdm_apb", 0, JH7110_SYSCLK_APB0),
JH71X0_GDIV(JH7110_SYSCLK_TDM_INTERNAL, "tdm_internal", 0, 64, JH7110_SYSCLK_MCLK),
- JH71X0__MUX(JH7110_SYSCLK_TDM_TDM, "tdm_tdm", 2,
+ JH71X0__MUX(JH7110_SYSCLK_TDM_TDM, "tdm_tdm", 0, 2,
JH7110_SYSCLK_TDM_INTERNAL,
JH7110_SYSCLK_TDM_EXT),
JH71X0__INV(JH7110_SYSCLK_TDM_TDM_INV, "tdm_tdm_inv", JH7110_SYSCLK_TDM_TDM),
diff --git a/drivers/clk/starfive/clk-starfive-jh71x0.h b/drivers/clk/starfive/clk-starfive-jh71x0.h
index 34bb11c72eb7..23e052fc1549 100644
--- a/drivers/clk/starfive/clk-starfive-jh71x0.h
+++ b/drivers/clk/starfive/clk-starfive-jh71x0.h
@@ -61,10 +61,10 @@ struct jh71x0_clk_data {
.parents = { [0] = _parent }, \
}

-#define JH71X0__MUX(_idx, _name, _nparents, ...) \
+#define JH71X0__MUX(_idx, _name, _flags, _nparents, ...) \
[_idx] = { \
.name = _name, \
- .flags = 0, \
+ .flags = _flags, \
.max = ((_nparents) - 1) << JH71X0_CLK_MUX_SHIFT, \
.parents = { __VA_ARGS__ }, \
}
--
2.43.0


2023-12-15 20:44:12

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH v3 9/9] clk: starfive: jh7100: Add CLK_SET_RATE_PARENT to gmac_tx

From: Emil Renner Berthing <[email protected]>

This is needed by the dwmac-starfive ethernet driver to set the clock
for 1000, 100 and 10 Mbps links properly.

Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
drivers/clk/starfive/clk-starfive-jh7100.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/starfive/clk-starfive-jh7100.c b/drivers/clk/starfive/clk-starfive-jh7100.c
index d3b260c01d5c..03f6f26a15d8 100644
--- a/drivers/clk/starfive/clk-starfive-jh7100.c
+++ b/drivers/clk/starfive/clk-starfive-jh7100.c
@@ -200,7 +200,7 @@ static const struct jh71x0_clk_data jh7100_clk_data[] __initconst = {
JH71X0_GDIV(JH7100_CLK_GMAC_GTX, "gmac_gtxclk", 0, 255, JH7100_CLK_GMAC_ROOT_DIV),
JH71X0_GDIV(JH7100_CLK_GMAC_RMII_TX, "gmac_rmii_txclk", 0, 8, JH7100_CLK_GMAC_RMII_REF),
JH71X0_GDIV(JH7100_CLK_GMAC_RMII_RX, "gmac_rmii_rxclk", 0, 8, JH7100_CLK_GMAC_RMII_REF),
- JH71X0__MUX(JH7100_CLK_GMAC_TX, "gmac_tx", 0, 3,
+ JH71X0__MUX(JH7100_CLK_GMAC_TX, "gmac_tx", CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT, 3,
JH7100_CLK_GMAC_GTX,
JH7100_CLK_GMAC_TX_INV,
JH7100_CLK_GMAC_RMII_TX),
--
2.43.0


2023-12-15 20:48:22

by Jessica Clarke

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
>
> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
> similar to the newer JH7110, but it requires only two interrupts and a
> single reset line, which is 'ahb' instead of the commonly used
> 'stmmaceth'.
>
> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
> conjunction with 'stmmaceth', extend the logic to also permit exclusive
> usage of the 'ahb' reset name. This ensures the following use cases are
> supported:
>
> JH7110: reset-names = "stmmaceth", "ahb";
> JH7100: reset-names = "ahb";
> other: reset-names = "stmmaceth";
>
> Also note the need to use a different dwmac fallback, as v5.20 applies
> to JH7110 only, while JH7100 relies on v3.7x.
>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
> 2 files changed, 55 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 5c2769dc689a..c1380ff1c054 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -95,6 +95,7 @@ properties:
> - snps,dwmac-5.20
> - snps,dwxgmac
> - snps,dwxgmac-2.10
> + - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
>
> reg:
> @@ -146,7 +147,7 @@ properties:
> reset-names:
> minItems: 1
> items:
> - - const: stmmaceth
> + - enum: [stmmaceth, ahb]
> - const: ahb

I’m not so well-versed in the YAML bindings, but would this not allow
reset-names = "ahb", "ahb"?

Jess


2023-12-15 20:57:22

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On 12/15/23 22:47, Jessica Clarke wrote:
> On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
>>
>> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
>> similar to the newer JH7110, but it requires only two interrupts and a
>> single reset line, which is 'ahb' instead of the commonly used
>> 'stmmaceth'.
>>
>> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
>> conjunction with 'stmmaceth', extend the logic to also permit exclusive
>> usage of the 'ahb' reset name. This ensures the following use cases are
>> supported:
>>
>> JH7110: reset-names = "stmmaceth", "ahb";
>> JH7100: reset-names = "ahb";
>> other: reset-names = "stmmaceth";
>>
>> Also note the need to use a different dwmac fallback, as v5.20 applies
>> to JH7110 only, while JH7100 relies on v3.7x.
>>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
>> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
>> 2 files changed, 55 insertions(+), 22 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index 5c2769dc689a..c1380ff1c054 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -95,6 +95,7 @@ properties:
>> - snps,dwmac-5.20
>> - snps,dwxgmac
>> - snps,dwxgmac-2.10
>> + - starfive,jh7100-dwmac
>> - starfive,jh7110-dwmac
>>
>> reg:
>> @@ -146,7 +147,7 @@ properties:
>> reset-names:
>> minItems: 1
>> items:
>> - - const: stmmaceth
>> + - enum: [stmmaceth, ahb]
>> - const: ahb
>
> I’m not so well-versed in the YAML bindings, but would this not allow
> reset-names = "ahb", "ahb"?

Yes, as I already pointed out in [1], I wasn't able to come up with a
proper solution to avoid that.

Thanks,
Cristian

> Jess
>

2023-12-15 20:59:30

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On 2023-12-15 2:47 PM, Jessica Clarke wrote:
> On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
>>
>> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
>> similar to the newer JH7110, but it requires only two interrupts and a
>> single reset line, which is 'ahb' instead of the commonly used
>> 'stmmaceth'.
>>
>> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
>> conjunction with 'stmmaceth', extend the logic to also permit exclusive
>> usage of the 'ahb' reset name. This ensures the following use cases are
>> supported:
>>
>> JH7110: reset-names = "stmmaceth", "ahb";
>> JH7100: reset-names = "ahb";
>> other: reset-names = "stmmaceth";
>>
>> Also note the need to use a different dwmac fallback, as v5.20 applies
>> to JH7110 only, while JH7100 relies on v3.7x.
>>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
>> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
>> 2 files changed, 55 insertions(+), 22 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index 5c2769dc689a..c1380ff1c054 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -95,6 +95,7 @@ properties:
>> - snps,dwmac-5.20
>> - snps,dwxgmac
>> - snps,dwxgmac-2.10
>> + - starfive,jh7100-dwmac
>> - starfive,jh7110-dwmac
>>
>> reg:
>> @@ -146,7 +147,7 @@ properties:
>> reset-names:
>> minItems: 1
>> items:
>> - - const: stmmaceth
>> + - enum: [stmmaceth, ahb]
>> - const: ahb
>
> I’m not so well-versed in the YAML bindings, but would this not allow
> reset-names = "ahb", "ahb"?

Yes, it would. You need something like:

reset-names:
oneOf:
- enum: [stmmaceth, ahb]
- items:
- const: stmmaceth
- const: ahb

Regards,
Samuel


2023-12-15 21:00:17

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On 12/15/23 22:56, Cristian Ciocaltea wrote:
> On 12/15/23 22:47, Jessica Clarke wrote:
>> On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
>>>
>>> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
>>> similar to the newer JH7110, but it requires only two interrupts and a
>>> single reset line, which is 'ahb' instead of the commonly used
>>> 'stmmaceth'.
>>>
>>> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
>>> conjunction with 'stmmaceth', extend the logic to also permit exclusive
>>> usage of the 'ahb' reset name. This ensures the following use cases are
>>> supported:
>>>
>>> JH7110: reset-names = "stmmaceth", "ahb";
>>> JH7100: reset-names = "ahb";
>>> other: reset-names = "stmmaceth";
>>>
>>> Also note the need to use a different dwmac fallback, as v5.20 applies
>>> to JH7110 only, while JH7100 relies on v3.7x.
>>>
>>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>>> ---
>>> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
>>> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
>>> 2 files changed, 55 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> index 5c2769dc689a..c1380ff1c054 100644
>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> @@ -95,6 +95,7 @@ properties:
>>> - snps,dwmac-5.20
>>> - snps,dwxgmac
>>> - snps,dwxgmac-2.10
>>> + - starfive,jh7100-dwmac
>>> - starfive,jh7110-dwmac
>>>
>>> reg:
>>> @@ -146,7 +147,7 @@ properties:
>>> reset-names:
>>> minItems: 1
>>> items:
>>> - - const: stmmaceth
>>> + - enum: [stmmaceth, ahb]
>>> - const: ahb
>>
>> I’m not so well-versed in the YAML bindings, but would this not allow
>> reset-names = "ahb", "ahb"?
>
> Yes, as I already pointed out in [1], I wasn't able to come up with a
> proper solution to avoid that.

Sorry, I've sent the previous email too early..

[1]: https://lore.kernel.org/lkml/[email protected]/

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

2023-12-15 21:03:42

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On 12/15/23 22:59, Samuel Holland wrote:
> On 2023-12-15 2:47 PM, Jessica Clarke wrote:
>> On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
>>>
>>> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
>>> similar to the newer JH7110, but it requires only two interrupts and a
>>> single reset line, which is 'ahb' instead of the commonly used
>>> 'stmmaceth'.
>>>
>>> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
>>> conjunction with 'stmmaceth', extend the logic to also permit exclusive
>>> usage of the 'ahb' reset name. This ensures the following use cases are
>>> supported:
>>>
>>> JH7110: reset-names = "stmmaceth", "ahb";
>>> JH7100: reset-names = "ahb";
>>> other: reset-names = "stmmaceth";
>>>
>>> Also note the need to use a different dwmac fallback, as v5.20 applies
>>> to JH7110 only, while JH7100 relies on v3.7x.
>>>
>>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>>> ---
>>> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
>>> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
>>> 2 files changed, 55 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> index 5c2769dc689a..c1380ff1c054 100644
>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> @@ -95,6 +95,7 @@ properties:
>>> - snps,dwmac-5.20
>>> - snps,dwxgmac
>>> - snps,dwxgmac-2.10
>>> + - starfive,jh7100-dwmac
>>> - starfive,jh7110-dwmac
>>>
>>> reg:
>>> @@ -146,7 +147,7 @@ properties:
>>> reset-names:
>>> minItems: 1
>>> items:
>>> - - const: stmmaceth
>>> + - enum: [stmmaceth, ahb]
>>> - const: ahb
>>
>> I’m not so well-versed in the YAML bindings, but would this not allow
>> reset-names = "ahb", "ahb"?
>
> Yes, it would. You need something like:
>
> reset-names:
> oneOf:
> - enum: [stmmaceth, ahb]
> - items:
> - const: stmmaceth
> - const: ahb

Oh yes, I always forget about the "oneOf" thing. Thanks!

> Regards,
> Samuel
>

2023-12-16 19:39:26

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

Cristian Ciocaltea wrote:
> Add pinmux configuration for DWMAC found on the JH7100 based boards and
> enable the related DT node, providing a basic PHY configuration.
>
> Co-developed-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index 42fb61c36068..5cafe8f5c2e7 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq {
> };
> };
>
> +&gmac {
> + pinctrl-names = "default";
> + pinctrl-0 = <&gmac_pins>;
> + phy-mode = "rgmii-id";
> + phy-handle = <&phy>;

I'm not sure if it's a generic policy or not, but I don't really like adding a
reference to a non-existant node here. I'd move this property to the board
files where the phy node is actually defined.

/Emil

2023-12-17 17:55:56

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 6/9] riscv: dts: starfive: visionfive-v1: Setup ethernet phy

On Fri, Dec 15, 2023 at 10:40:45PM +0200, Cristian Ciocaltea wrote:
> The StarFive VisionFive V1 SBC uses a Motorcomm YT8521 PHY supporting
> RGMII-ID, but requires manual adjustment of the RX internal delay to
> work properly.
>
> The default RX delay provided by the driver is 1.95 ns, which proves to
> be too high. Applying a 50% reduction seems to mitigate the issue.
>
> Also note this adjustment is not necessary on BeagleV Starlight SBC,
> which uses a Microchip PHY. Hence, there is no indication of a
> miss-behaviour on the GMAC side, but most likely the issue stems from
> the Motorcomm PHY.

I suggest you make a similar comment in the .dts file, just to explain
the odd setting.

Andrew

2023-12-17 21:03:55

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

On Sat, Dec 16, 2023 at 11:38:53AM -0800, Emil Renner Berthing wrote:
> Cristian Ciocaltea wrote:
> > Add pinmux configuration for DWMAC found on the JH7100 based boards and
> > enable the related DT node, providing a basic PHY configuration.
> >
> > Co-developed-by: Emil Renner Berthing <[email protected]>
> > Signed-off-by: Emil Renner Berthing <[email protected]>
> > Signed-off-by: Cristian Ciocaltea <[email protected]>
> > ---
> > .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++
> > 1 file changed, 85 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > index 42fb61c36068..5cafe8f5c2e7 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq {
> > };
> > };
> >
> > +&gmac {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&gmac_pins>;
> > + phy-mode = "rgmii-id";
> > + phy-handle = <&phy>;
>
> I'm not sure if it's a generic policy or not, but I don't really like adding a
> reference to a non-existant node here. I'd move this property to the board
> files where the phy node is actually defined.

FWIW, I don't like the reference-in-the-wrong-place thing either.


Attachments:
(No filename) (1.36 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-17 21:10:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On Fri, Dec 15, 2023 at 11:03:24PM +0200, Cristian Ciocaltea wrote:
> On 12/15/23 22:59, Samuel Holland wrote:
> > On 2023-12-15 2:47 PM, Jessica Clarke wrote:
> >> On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
> >>>
> >>> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
> >>> similar to the newer JH7110, but it requires only two interrupts and a
> >>> single reset line, which is 'ahb' instead of the commonly used
> >>> 'stmmaceth'.
> >>>
> >>> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
> >>> conjunction with 'stmmaceth', extend the logic to also permit exclusive
> >>> usage of the 'ahb' reset name. This ensures the following use cases are
> >>> supported:
> >>>
> >>> JH7110: reset-names = "stmmaceth", "ahb";
> >>> JH7100: reset-names = "ahb";
> >>> other: reset-names = "stmmaceth";
> >>>
> >>> Also note the need to use a different dwmac fallback, as v5.20 applies
> >>> to JH7110 only, while JH7100 relies on v3.7x.
> >>>
> >>> Signed-off-by: Cristian Ciocaltea <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
> >>> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
> >>> 2 files changed, 55 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> >>> index 5c2769dc689a..c1380ff1c054 100644
> >>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> >>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> >>> @@ -95,6 +95,7 @@ properties:
> >>> - snps,dwmac-5.20
> >>> - snps,dwxgmac
> >>> - snps,dwxgmac-2.10
> >>> + - starfive,jh7100-dwmac
> >>> - starfive,jh7110-dwmac
> >>>
> >>> reg:
> >>> @@ -146,7 +147,7 @@ properties:
> >>> reset-names:
> >>> minItems: 1
> >>> items:
> >>> - - const: stmmaceth
> >>> + - enum: [stmmaceth, ahb]
> >>> - const: ahb
> >>
> >> I’m not so well-versed in the YAML bindings, but would this not allow
> >> reset-names = "ahb", "ahb"?
> >
> > Yes, it would. You need something like:
> >
> > reset-names:
> > oneOf:
> > - enum: [stmmaceth, ahb]
> > - items:
> > - const: stmmaceth
> > - const: ahb
>
> Oh yes, I always forget about the "oneOf" thing. Thanks!

Won't this also relax the naming for all devices that allow a single
reset, but expect the stmmaceth one? I'm not sure that that actually
matters, I think the consumer bindings have constraints themselves.


Attachments:
(No filename) (2.61 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-18 11:55:28

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

On 12/16/23 21:38, Emil Renner Berthing wrote:
> Cristian Ciocaltea wrote:
>> Add pinmux configuration for DWMAC found on the JH7100 based boards and
>> enable the related DT node, providing a basic PHY configuration.
>>
>> Co-developed-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++
>> 1 file changed, 85 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
>> index 42fb61c36068..5cafe8f5c2e7 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
>> @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq {
>> };
>> };
>>
>> +&gmac {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&gmac_pins>;
>> + phy-mode = "rgmii-id";
>> + phy-handle = <&phy>;
>
> I'm not sure if it's a generic policy or not, but I don't really like adding a
> reference to a non-existant node here. I'd move this property to the board
> files where the phy node is actually defined.

Totally agree, I simply went too far while dropping duplicated code and
didn't realize the mistake. Thanks for noticing!

2023-12-18 11:56:39

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 6/9] riscv: dts: starfive: visionfive-v1: Setup ethernet phy

On 12/17/23 19:55, Andrew Lunn wrote:
> On Fri, Dec 15, 2023 at 10:40:45PM +0200, Cristian Ciocaltea wrote:
>> The StarFive VisionFive V1 SBC uses a Motorcomm YT8521 PHY supporting
>> RGMII-ID, but requires manual adjustment of the RX internal delay to
>> work properly.
>>
>> The default RX delay provided by the driver is 1.95 ns, which proves to
>> be too high. Applying a 50% reduction seems to mitigate the issue.
>>
>> Also note this adjustment is not necessary on BeagleV Starlight SBC,
>> which uses a Microchip PHY. Hence, there is no indication of a
>> miss-behaviour on the GMAC side, but most likely the issue stems from
>> the Motorcomm PHY.
>
> I suggest you make a similar comment in the .dts file, just to explain
> the odd setting.

Sure, will do, we need a v4 anyway.

Thanks,
Cristian

2023-12-18 11:57:56

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

On 12/17/23 23:03, Conor Dooley wrote:
> On Sat, Dec 16, 2023 at 11:38:53AM -0800, Emil Renner Berthing wrote:
>> Cristian Ciocaltea wrote:
>>> Add pinmux configuration for DWMAC found on the JH7100 based boards and
>>> enable the related DT node, providing a basic PHY configuration.
>>>
>>> Co-developed-by: Emil Renner Berthing <[email protected]>
>>> Signed-off-by: Emil Renner Berthing <[email protected]>
>>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>>> ---
>>> .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++
>>> 1 file changed, 85 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
>>> index 42fb61c36068..5cafe8f5c2e7 100644
>>> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
>>> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
>>> @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq {
>>> };
>>> };
>>>
>>> +&gmac {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&gmac_pins>;
>>> + phy-mode = "rgmii-id";
>>> + phy-handle = <&phy>;
>>
>> I'm not sure if it's a generic policy or not, but I don't really like adding a
>> reference to a non-existant node here. I'd move this property to the board
>> files where the phy node is actually defined.
>
> FWIW, I don't like the reference-in-the-wrong-place thing either.

Yeah, as I already mentioned this was unintentional, will fix in v4.

Thanks,
Cristian

2023-12-18 12:03:34

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible

On 12/17/23 23:09, Conor Dooley wrote:
> On Fri, Dec 15, 2023 at 11:03:24PM +0200, Cristian Ciocaltea wrote:
>> On 12/15/23 22:59, Samuel Holland wrote:
>>> On 2023-12-15 2:47 PM, Jessica Clarke wrote:
>>>> On 15 Dec 2023, at 20:40, Cristian Ciocaltea <[email protected]> wrote:
>>>>>
>>>>> The Synopsys DesignWare MAC found on StarFive JH7100 SoC is mostly
>>>>> similar to the newer JH7110, but it requires only two interrupts and a
>>>>> single reset line, which is 'ahb' instead of the commonly used
>>>>> 'stmmaceth'.
>>>>>
>>>>> Since the common binding 'snps,dwmac' allows selecting 'ahb' only in
>>>>> conjunction with 'stmmaceth', extend the logic to also permit exclusive
>>>>> usage of the 'ahb' reset name. This ensures the following use cases are
>>>>> supported:
>>>>>
>>>>> JH7110: reset-names = "stmmaceth", "ahb";
>>>>> JH7100: reset-names = "ahb";
>>>>> other: reset-names = "stmmaceth";
>>>>>
>>>>> Also note the need to use a different dwmac fallback, as v5.20 applies
>>>>> to JH7110 only, while JH7100 relies on v3.7x.
>>>>>
>>>>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/net/snps,dwmac.yaml | 3 +-
>>>>> .../bindings/net/starfive,jh7110-dwmac.yaml | 74 +++++++++++++------
>>>>> 2 files changed, 55 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> index 5c2769dc689a..c1380ff1c054 100644
>>>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> @@ -95,6 +95,7 @@ properties:
>>>>> - snps,dwmac-5.20
>>>>> - snps,dwxgmac
>>>>> - snps,dwxgmac-2.10
>>>>> + - starfive,jh7100-dwmac
>>>>> - starfive,jh7110-dwmac
>>>>>
>>>>> reg:
>>>>> @@ -146,7 +147,7 @@ properties:
>>>>> reset-names:
>>>>> minItems: 1
>>>>> items:
>>>>> - - const: stmmaceth
>>>>> + - enum: [stmmaceth, ahb]
>>>>> - const: ahb
>>>>
>>>> I’m not so well-versed in the YAML bindings, but would this not allow
>>>> reset-names = "ahb", "ahb"?
>>>
>>> Yes, it would. You need something like:
>>>
>>> reset-names:
>>> oneOf:
>>> - enum: [stmmaceth, ahb]
>>> - items:
>>> - const: stmmaceth
>>> - const: ahb
>>
>> Oh yes, I always forget about the "oneOf" thing. Thanks!
>
> Won't this also relax the naming for all devices that allow a single
> reset, but expect the stmmaceth one? I'm not sure that that actually
> matters, I think the consumer bindings have constraints themselves.

Before commit 843f603762a5 ("dt-bindings: net: snps,dwmac: Add 'ahb'
reset/reset-name"), the 'stmmaceth' was the only possible option, hence
there was no need for any constraints on the consumer bindings. But
afterwards it was allowed to use both resets, hence I think the bindings
should have been updated at that time by adding 'maxItems: 1' to prevent
using the 2nd reset.

I could fix this in a separate series, if it's not something mandatory
to be handled here.

Thanks for reviewing,
Cristian

2023-12-18 15:00:15

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] dt-bindings: net: starfive,jh7110-dwmac: Add JH7100 SoC compatible


> > Won't this also relax the naming for all devices that allow a single
> > reset, but expect the stmmaceth one? I'm not sure that that actually
> > matters, I think the consumer bindings have constraints themselves.
>
> Before commit 843f603762a5 ("dt-bindings: net: snps,dwmac: Add 'ahb'
> reset/reset-name"), the 'stmmaceth' was the only possible option, hence
> there was no need for any constraints on the consumer bindings. But
> afterwards it was allowed to use both resets, hence I think the bindings
> should have been updated at that time by adding 'maxItems: 1' to prevent
> using the 2nd reset.
>
> I could fix this in a separate series, if it's not something mandatory
> to be handled here.

If it is not introduced by this series, I don't see why it could not be
handled separately if needed.


Attachments:
(No filename) (829.00 B)
signature.asc (235.00 B)
Download all attachments