* [PATCH v2 0/5] Add GMAC support for S32 SoC family
@ 2022-11-28 5:49 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Jan Petrous, Ondrej Spacek,
Ghennadi Procopciuc, Andrew Lunn
Cc: Chester Lin, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Andreas Färber, Matthias Brugger
Hello,
Here I want to introduce a new patch series, which aims to support GMAC
functions on S32 SoCs, such as S32G2. This series is originally from NXP's
implementation on CodeAurora[1] and it will be required by upstream kernel
to configure platform settings at the DWMAC glue layer before activating
the DWMAC core on different S32G boards. In this patchset I also introduce
more register fields needed by S32 SoCs, such as higher CSR clock ranges
and cache coherency settings. For more information please see NXP's
GMACSUBSYS Reference Manual[2].
Currently, the whole architecture relies on FDTs offered by ATF[3] on
CodeAurora to keep the flexibility of handling multiple S32 platforms since
now S32 clks can be triggered via the ARM SCMI clock protocol and clk IDs/
settings can vary according to different board designs. To ensure that the
driver can work properly, the dt-binding schemas in this patchset is still
required as a reference.
Thanks,
Chester
Changes in v2:
- Fix schema issues.
- Add minItems to clocks & clock-names.
- Replace all sgmii/SGMII terms with pcs/PCS.
- clock-names: tx_sgmii -> tx_pcs, rx_sgmii -> rx_pcs
- Adjust error handlings while calling devm_clk_get().
- Remove redundant dev_info messages.
- Remove unnecessary if conditions.
- Fix the copyright format suggested by NXP.
[1] https://source.codeaurora.org/external/autobsps32/linux/tree/drivers/net/ethernet/stmicro/stmmac?h=bsp34.0-5.10.120-rt
[2] https://www.nxp.com/webapp/Download?colCode=GMACSUBSYSRM
[3] https://source.codeaurora.org/external/autobsps32/arm-trusted-firmware/tag/?h=bsp34.0-2.5
Chester Lin (5):
dt-bindings: net: snps, dwmac: add NXP S32CC support
dt-bindings: net: add schema for NXP S32CC dwmac glue driver
net: stmmac: Add CSR clock 500Mhz/800Mhz support
net: stmmac: Add AXI4 ACE control support
net: stmmac: Add NXP S32 SoC family support
.../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++
.../devicetree/bindings/net/snps,dwmac.yaml | 5 +-
drivers/net/ethernet/stmicro/stmmac/Kconfig | 13 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/common.h | 2 +
.../net/ethernet/stmicro/stmmac/dwmac-s32cc.c | 304 ++++++++++++++++++
.../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 10 +
.../net/ethernet/stmicro/stmmac/dwmac4_dma.h | 4 +-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 5 +
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +
include/linux/stmmac.h | 9 +
11 files changed, 492 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
--
2.37.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/5] dt-bindings: net: snps, dwmac: add NXP S32CC support
2022-11-28 5:49 ` Chester Lin
@ 2022-11-28 5:49 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, Rob Herring,
Krzysztof Kozlowski
Cc: Chester Lin, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Andreas Färber, Matthias Brugger,
Jan Petrous
Add a new compatible string for NXP S32CC DWMAC glue driver. The maxItems
of clock and clock-names need be increased because S32CC has up to 11
clocks for its DWMAC.
Signed-off-by: Chester Lin <clin@suse.com>
---
No change in v2.
Documentation/devicetree/bindings/net/snps,dwmac.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 13b984076af5..c174d173591e 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -65,6 +65,7 @@ properties:
- ingenic,x2000-mac
- loongson,ls2k-dwmac
- loongson,ls7a-dwmac
+ - nxp,s32cc-dwmac
- renesas,r9a06g032-gmac
- renesas,rzn1-gmac
- rockchip,px30-gmac
@@ -110,7 +111,7 @@ properties:
clocks:
minItems: 1
- maxItems: 8
+ maxItems: 11
additionalItems: true
items:
- description: GMAC main clock
@@ -122,7 +123,7 @@ properties:
clock-names:
minItems: 1
- maxItems: 8
+ maxItems: 11
additionalItems: true
contains:
enum:
--
2.37.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 1/5] dt-bindings: net: snps, dwmac: add NXP S32CC support
@ 2022-11-28 5:49 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, Rob Herring,
Krzysztof Kozlowski
Cc: Chester Lin, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Andreas Färber, Matthias Brugger,
Jan Petrous
Add a new compatible string for NXP S32CC DWMAC glue driver. The maxItems
of clock and clock-names need be increased because S32CC has up to 11
clocks for its DWMAC.
Signed-off-by: Chester Lin <clin@suse.com>
---
No change in v2.
Documentation/devicetree/bindings/net/snps,dwmac.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 13b984076af5..c174d173591e 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -65,6 +65,7 @@ properties:
- ingenic,x2000-mac
- loongson,ls2k-dwmac
- loongson,ls7a-dwmac
+ - nxp,s32cc-dwmac
- renesas,r9a06g032-gmac
- renesas,rzn1-gmac
- rockchip,px30-gmac
@@ -110,7 +111,7 @@ properties:
clocks:
minItems: 1
- maxItems: 8
+ maxItems: 11
additionalItems: true
items:
- description: GMAC main clock
@@ -122,7 +123,7 @@ properties:
clock-names:
minItems: 1
- maxItems: 8
+ maxItems: 11
additionalItems: true
contains:
enum:
--
2.37.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 1/5] dt-bindings: net: snps, dwmac: add NXP S32CC support
2022-11-28 5:49 ` Chester Lin
@ 2022-11-29 14:10 ` Andreas Färber
-1 siblings, 0 replies; 30+ messages in thread
From: Andreas Färber @ 2022-11-29 14:10 UTC (permalink / raw)
To: Chester Lin, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu,
Rob Herring, Krzysztof Kozlowski
Cc: netdev, s32, devicetree, linux-kernel, linux-arm-kernel,
Matthias Brugger, Jan Petrous
Hi Chester,
Am 28.11.22 um 06:49 schrieb Chester Lin:
> Add a new compatible string for NXP S32CC DWMAC glue driver. The maxItems
> of clock and clock-names need be increased because S32CC has up to 11
> clocks for its DWMAC.
>
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>
> No change in v2.
>
> Documentation/devicetree/bindings/net/snps,dwmac.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 13b984076af5..c174d173591e 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -65,6 +65,7 @@ properties:
> - ingenic,x2000-mac
> - loongson,ls2k-dwmac
> - loongson,ls7a-dwmac
> + - nxp,s32cc-dwmac
As we had discussed offline, please change this to nxp,s32g2-dwmac.
S32G3 and S32R45 can then reuse it if they don't require changes; there
is no difference here to how i.MX family or other vendors inherit IP
across SoC models, so Rob's rules apply equally.
Also affects the following patches.
Thanks,
Andreas
> - renesas,r9a06g032-gmac
> - renesas,rzn1-gmac
> - rockchip,px30-gmac
[snip]
--
SUSE Software Solutions Germany GmbH
Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: net: snps, dwmac: add NXP S32CC support
@ 2022-11-29 14:10 ` Andreas Färber
0 siblings, 0 replies; 30+ messages in thread
From: Andreas Färber @ 2022-11-29 14:10 UTC (permalink / raw)
To: Chester Lin, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu,
Rob Herring, Krzysztof Kozlowski
Cc: netdev, s32, devicetree, linux-kernel, linux-arm-kernel,
Matthias Brugger, Jan Petrous
Hi Chester,
Am 28.11.22 um 06:49 schrieb Chester Lin:
> Add a new compatible string for NXP S32CC DWMAC glue driver. The maxItems
> of clock and clock-names need be increased because S32CC has up to 11
> clocks for its DWMAC.
>
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>
> No change in v2.
>
> Documentation/devicetree/bindings/net/snps,dwmac.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 13b984076af5..c174d173591e 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -65,6 +65,7 @@ properties:
> - ingenic,x2000-mac
> - loongson,ls2k-dwmac
> - loongson,ls7a-dwmac
> + - nxp,s32cc-dwmac
As we had discussed offline, please change this to nxp,s32g2-dwmac.
S32G3 and S32R45 can then reuse it if they don't require changes; there
is no difference here to how i.MX family or other vendors inherit IP
across SoC models, so Rob's rules apply equally.
Also affects the following patches.
Thanks,
Andreas
> - renesas,r9a06g032-gmac
> - renesas,rzn1-gmac
> - rockchip,px30-gmac
[snip]
--
SUSE Software Solutions Germany GmbH
Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nürnberg)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-11-28 5:49 ` Chester Lin
@ 2022-11-28 5:49 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Jan Petrous, Andrew Lunn
Cc: Chester Lin, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu,
netdev, s32, devicetree, linux-kernel, linux-arm-kernel,
Andreas Färber, Matthias Brugger
Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
Chassis.
Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
Changes in v2:
- Fix schema issues.
- Add minItems to clocks & clock-names.
- Replace all sgmii/SGMII terms with pcs/PCS.
.../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
1 file changed, 135 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
new file mode 100644
index 000000000000..c6839fd3df40
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
@@ -0,0 +1,135 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2021-2022 NXP
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: NXP S32CC DWMAC Ethernet controller
+
+maintainers:
+ - Jan Petrous <jan.petrous@nxp.com>
+ - Chester Lin <clin@suse.com>
+
+allOf:
+ - $ref: "snps,dwmac.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - nxp,s32cc-dwmac
+
+ reg:
+ items:
+ - description: Main GMAC registers
+ - description: S32 MAC control registers
+
+ dma-coherent: true
+
+ clocks:
+ minItems: 5
+ items:
+ - description: Main GMAC clock
+ - description: Peripheral registers clock
+ - description: Transmit PCS clock
+ - description: Transmit RGMII clock
+ - description: Transmit RMII clock
+ - description: Transmit MII clock
+ - description: Receive PCS clock
+ - description: Receive RGMII clock
+ - description: Receive RMII clock
+ - description: Receive MII clock
+ - description:
+ PTP reference clock. This clock is used for programming the
+ Timestamp Addend Register. If not passed then the system
+ clock will be used.
+
+ clock-names:
+ minItems: 5
+ items:
+ - const: stmmaceth
+ - const: pclk
+ - const: tx_pcs
+ - const: tx_rgmii
+ - const: tx_rmii
+ - const: tx_mii
+ - const: rx_pcs
+ - const: rx_rgmii
+ - const: rx_rmii
+ - const: rx_mii
+ - const: ptp_ref
+
+ tx-fifo-depth:
+ const: 20480
+
+ rx-fifo-depth:
+ const: 20480
+
+required:
+ - compatible
+ - reg
+ - tx-fifo-depth
+ - rx-fifo-depth
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ #define S32GEN1_SCMI_CLK_GMAC0_AXI
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
+ #define S32GEN1_SCMI_CLK_GMAC0_TS
+
+ soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ gmac0: ethernet@4033c000 {
+ compatible = "nxp,s32cc-dwmac";
+ reg = <0x4033c000 0x2000>, /* gmac IP */
+ <0x4007C004 0x4>; /* S32 CTRL_STS reg */
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ phy-mode = "rgmii-id";
+ tx-fifo-depth = <20480>;
+ rx-fifo-depth = <20480>;
+ dma-coherent;
+ clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_PCS>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_PCS>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
+ clock-names = "stmmaceth", "pclk",
+ "tx_pcs", "tx_rgmii", "tx_rmii", "tx_mii",
+ "rx_pcs", "rx_rgmii", "rx_rmii", "rx_mii",
+ "ptp_ref";
+
+ gmac0_mdio: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+
+ ethernet-phy@4 {
+ reg = <0x04>;
+ };
+ };
+ };
+ };
--
2.37.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-11-28 5:49 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Jan Petrous, Andrew Lunn
Cc: Chester Lin, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu,
netdev, s32, devicetree, linux-kernel, linux-arm-kernel,
Andreas Färber, Matthias Brugger
Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
Chassis.
Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
Changes in v2:
- Fix schema issues.
- Add minItems to clocks & clock-names.
- Replace all sgmii/SGMII terms with pcs/PCS.
.../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
1 file changed, 135 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
new file mode 100644
index 000000000000..c6839fd3df40
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
@@ -0,0 +1,135 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2021-2022 NXP
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: NXP S32CC DWMAC Ethernet controller
+
+maintainers:
+ - Jan Petrous <jan.petrous@nxp.com>
+ - Chester Lin <clin@suse.com>
+
+allOf:
+ - $ref: "snps,dwmac.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - nxp,s32cc-dwmac
+
+ reg:
+ items:
+ - description: Main GMAC registers
+ - description: S32 MAC control registers
+
+ dma-coherent: true
+
+ clocks:
+ minItems: 5
+ items:
+ - description: Main GMAC clock
+ - description: Peripheral registers clock
+ - description: Transmit PCS clock
+ - description: Transmit RGMII clock
+ - description: Transmit RMII clock
+ - description: Transmit MII clock
+ - description: Receive PCS clock
+ - description: Receive RGMII clock
+ - description: Receive RMII clock
+ - description: Receive MII clock
+ - description:
+ PTP reference clock. This clock is used for programming the
+ Timestamp Addend Register. If not passed then the system
+ clock will be used.
+
+ clock-names:
+ minItems: 5
+ items:
+ - const: stmmaceth
+ - const: pclk
+ - const: tx_pcs
+ - const: tx_rgmii
+ - const: tx_rmii
+ - const: tx_mii
+ - const: rx_pcs
+ - const: rx_rgmii
+ - const: rx_rmii
+ - const: rx_mii
+ - const: ptp_ref
+
+ tx-fifo-depth:
+ const: 20480
+
+ rx-fifo-depth:
+ const: 20480
+
+required:
+ - compatible
+ - reg
+ - tx-fifo-depth
+ - rx-fifo-depth
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ #define S32GEN1_SCMI_CLK_GMAC0_AXI
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
+ #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
+ #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
+ #define S32GEN1_SCMI_CLK_GMAC0_TS
+
+ soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ gmac0: ethernet@4033c000 {
+ compatible = "nxp,s32cc-dwmac";
+ reg = <0x4033c000 0x2000>, /* gmac IP */
+ <0x4007C004 0x4>; /* S32 CTRL_STS reg */
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ phy-mode = "rgmii-id";
+ tx-fifo-depth = <20480>;
+ rx-fifo-depth = <20480>;
+ dma-coherent;
+ clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_PCS>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_PCS>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
+ <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
+ clock-names = "stmmaceth", "pclk",
+ "tx_pcs", "tx_rgmii", "tx_rmii", "tx_mii",
+ "rx_pcs", "rx_rgmii", "rx_rmii", "rx_mii",
+ "ptp_ref";
+
+ gmac0_mdio: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+
+ ethernet-phy@4 {
+ reg = <0x04>;
+ };
+ };
+ };
+ };
--
2.37.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-11-28 5:49 ` Chester Lin
@ 2022-11-30 15:51 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-30 15:51 UTC (permalink / raw)
To: Chester Lin, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Jan Petrous,
Andrew Lunn
Cc: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, netdev, s32,
devicetree, linux-kernel, linux-arm-kernel, Andreas Färber,
Matthias Brugger
On 28/11/2022 06:49, Chester Lin wrote:
> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> Chassis.
>
> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> Signed-off-by: Chester Lin <clin@suse.com>
Thank you for your patch. There is something to discuss/improve.
> ---
>
> Changes in v2:
> - Fix schema issues.
> - Add minItems to clocks & clock-names.
> - Replace all sgmii/SGMII terms with pcs/PCS.
>
> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
> 1 file changed, 135 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> new file mode 100644
> index 000000000000..c6839fd3df40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2021-2022 NXP
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
Drop quotes from both.
> +
> +title: NXP S32CC DWMAC Ethernet controller
> +
> +maintainers:
> + - Jan Petrous <jan.petrous@nxp.com>
> + - Chester Lin <clin@suse.com>
> +
> +allOf:
> + - $ref: "snps,dwmac.yaml#"
Drop quotes.
> +
> +properties:
> + compatible:
> + enum:
> + - nxp,s32cc-dwmac
> +
> + reg:
> + items:
> + - description: Main GMAC registers
> + - description: S32 MAC control registers
> +
> + dma-coherent: true
> +
> + clocks:
> + minItems: 5
Why only 5 clocks are required? Receive clocks don't have to be there?
Is such system - only with clocks for transmit - usable?
> + items:
> + - description: Main GMAC clock
> + - description: Peripheral registers clock
> + - description: Transmit PCS clock
> + - description: Transmit RGMII clock
> + - description: Transmit RMII clock
> + - description: Transmit MII clock
> + - description: Receive PCS clock
> + - description: Receive RGMII clock
> + - description: Receive RMII clock
> + - description: Receive MII clock
> + - description:
> + PTP reference clock. This clock is used for programming the
> + Timestamp Addend Register. If not passed then the system
> + clock will be used.
> +
> + clock-names:
> + minItems: 5
> + items:
> + - const: stmmaceth
> + - const: pclk
> + - const: tx_pcs
> + - const: tx_rgmii
> + - const: tx_rmii
> + - const: tx_mii
> + - const: rx_pcs
> + - const: rx_rgmii
> + - const: rx_rmii
> + - const: rx_mii
> + - const: ptp_ref
> +
> + tx-fifo-depth:
> + const: 20480
> +
> + rx-fifo-depth:
> + const: 20480
> +
> +required:
> + - compatible
> + - reg
> + - tx-fifo-depth
> + - rx-fifo-depth
> + - clocks
> + - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> + #define S32GEN1_SCMI_CLK_GMAC0_TS
Why defines? Your clock controller is not ready? If so, just use raw
numbers.
> +
> + soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + gmac0: ethernet@4033c000 {
> + compatible = "nxp,s32cc-dwmac";
> + reg = <0x4033c000 0x2000>, /* gmac IP */
> + <0x4007C004 0x4>; /* S32 CTRL_STS reg */
Lowercase hex.
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + phy-mode = "rgmii-id";
> + tx-fifo-depth = <20480>;
> + rx-fifo-depth = <20480>;
> + dma-coherent;
> + clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_PCS>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_PCS>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-11-30 15:51 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-30 15:51 UTC (permalink / raw)
To: Chester Lin, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Jan Petrous,
Andrew Lunn
Cc: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, netdev, s32,
devicetree, linux-kernel, linux-arm-kernel, Andreas Färber,
Matthias Brugger
On 28/11/2022 06:49, Chester Lin wrote:
> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> Chassis.
>
> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> Signed-off-by: Chester Lin <clin@suse.com>
Thank you for your patch. There is something to discuss/improve.
> ---
>
> Changes in v2:
> - Fix schema issues.
> - Add minItems to clocks & clock-names.
> - Replace all sgmii/SGMII terms with pcs/PCS.
>
> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
> 1 file changed, 135 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> new file mode 100644
> index 000000000000..c6839fd3df40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2021-2022 NXP
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
Drop quotes from both.
> +
> +title: NXP S32CC DWMAC Ethernet controller
> +
> +maintainers:
> + - Jan Petrous <jan.petrous@nxp.com>
> + - Chester Lin <clin@suse.com>
> +
> +allOf:
> + - $ref: "snps,dwmac.yaml#"
Drop quotes.
> +
> +properties:
> + compatible:
> + enum:
> + - nxp,s32cc-dwmac
> +
> + reg:
> + items:
> + - description: Main GMAC registers
> + - description: S32 MAC control registers
> +
> + dma-coherent: true
> +
> + clocks:
> + minItems: 5
Why only 5 clocks are required? Receive clocks don't have to be there?
Is such system - only with clocks for transmit - usable?
> + items:
> + - description: Main GMAC clock
> + - description: Peripheral registers clock
> + - description: Transmit PCS clock
> + - description: Transmit RGMII clock
> + - description: Transmit RMII clock
> + - description: Transmit MII clock
> + - description: Receive PCS clock
> + - description: Receive RGMII clock
> + - description: Receive RMII clock
> + - description: Receive MII clock
> + - description:
> + PTP reference clock. This clock is used for programming the
> + Timestamp Addend Register. If not passed then the system
> + clock will be used.
> +
> + clock-names:
> + minItems: 5
> + items:
> + - const: stmmaceth
> + - const: pclk
> + - const: tx_pcs
> + - const: tx_rgmii
> + - const: tx_rmii
> + - const: tx_mii
> + - const: rx_pcs
> + - const: rx_rgmii
> + - const: rx_rmii
> + - const: rx_mii
> + - const: ptp_ref
> +
> + tx-fifo-depth:
> + const: 20480
> +
> + rx-fifo-depth:
> + const: 20480
> +
> +required:
> + - compatible
> + - reg
> + - tx-fifo-depth
> + - rx-fifo-depth
> + - clocks
> + - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> + #define S32GEN1_SCMI_CLK_GMAC0_TS
Why defines? Your clock controller is not ready? If so, just use raw
numbers.
> +
> + soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + gmac0: ethernet@4033c000 {
> + compatible = "nxp,s32cc-dwmac";
> + reg = <0x4033c000 0x2000>, /* gmac IP */
> + <0x4007C004 0x4>; /* S32 CTRL_STS reg */
Lowercase hex.
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + phy-mode = "rgmii-id";
> + tx-fifo-depth = <20480>;
> + rx-fifo-depth = <20480>;
> + dma-coherent;
> + clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_PCS>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_PCS>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
> + <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-11-30 15:51 ` Krzysztof Kozlowski
@ 2022-11-30 17:33 ` Andreas Färber
-1 siblings, 0 replies; 30+ messages in thread
From: Andreas Färber @ 2022-11-30 17:33 UTC (permalink / raw)
To: Krzysztof Kozlowski, Chester Lin, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn
Cc: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, netdev, s32,
devicetree, linux-kernel, linux-arm-kernel, Matthias Brugger
Hi Krysztof,
Am 30.11.22 um 16:51 schrieb Krzysztof Kozlowski:
> On 28/11/2022 06:49, Chester Lin wrote:
>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
>> Chassis.
>>
>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
>> Signed-off-by: Chester Lin <clin@suse.com>
>
> Thank you for your patch. There is something to discuss/improve.
>
>> ---
>>
>> Changes in v2:
>> - Fix schema issues.
>> - Add minItems to clocks & clock-names.
>> - Replace all sgmii/SGMII terms with pcs/PCS.
>>
>> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
>> 1 file changed, 135 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>> new file mode 100644
>> index 000000000000..c6839fd3df40
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
[...]
>> +properties:
>> + compatible:
>> + enum:
>> + - nxp,s32cc-dwmac
>> +
>> + reg:
>> + items:
>> + - description: Main GMAC registers
>> + - description: S32 MAC control registers
>> +
>> + dma-coherent: true
>> +
>> + clocks:
>> + minItems: 5
>
> Why only 5 clocks are required? Receive clocks don't have to be there?
> Is such system - only with clocks for transmit - usable?
>
>> + items:
>> + - description: Main GMAC clock
>> + - description: Peripheral registers clock
>> + - description: Transmit PCS clock
>> + - description: Transmit RGMII clock
>> + - description: Transmit RMII clock
>> + - description: Transmit MII clock
>> + - description: Receive PCS clock
>> + - description: Receive RGMII clock
>> + - description: Receive RMII clock
>> + - description: Receive MII clock
>> + - description:
>> + PTP reference clock. This clock is used for programming the
>> + Timestamp Addend Register. If not passed then the system
>> + clock will be used.
>> +
>> + clock-names:
>> + minItems: 5
>> + items:
>> + - const: stmmaceth
>> + - const: pclk
>> + - const: tx_pcs
>> + - const: tx_rgmii
>> + - const: tx_rmii
>> + - const: tx_mii
>> + - const: rx_pcs
>> + - const: rx_rgmii
>> + - const: rx_rmii
>> + - const: rx_mii
>> + - const: ptp_ref
>> +
>> + tx-fifo-depth:
>> + const: 20480
>> +
>> + rx-fifo-depth:
>> + const: 20480
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - tx-fifo-depth
>> + - rx-fifo-depth
>> + - clocks
>> + - clock-names
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>
> Why defines? Your clock controller is not ready? If so, just use raw
> numbers.
Please compare v1: There is no Linux-driven clock controller here but
rather a fluid SCMI firmware interface. Work towards getting clocks into
a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
also explains the ugly examples here and for pinctrl.
Logically there are only 5 input clocks; however due to SCMI not
supporting re-parenting today, some clocks got duplicated at SCMI level.
Andrew appeared to approve of that approach. I still dislike it but
don't have a better proposal that would work today. So the two values
above indeed seem wrong and should be 11 rather than 5.
Cheers,
Andreas
>> +
>> + soc {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + gmac0: ethernet@4033c000 {
>> + compatible = "nxp,s32cc-dwmac";
>> + reg = <0x4033c000 0x2000>, /* gmac IP */
>> + <0x4007C004 0x4>; /* S32 CTRL_STS reg */
>
> Lowercase hex.
>
>> + interrupt-parent = <&gic>;
>> + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "macirq";
>> + phy-mode = "rgmii-id";
>> + tx-fifo-depth = <20480>;
>> + rx-fifo-depth = <20480>;
>> + dma-coherent;
>> + clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_PCS>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_PCS>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
>
>
> Best regards,
> Krzysztof
>
--
SUSE Software Solutions Germany GmbH
Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nürnberg)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-11-30 17:33 ` Andreas Färber
0 siblings, 0 replies; 30+ messages in thread
From: Andreas Färber @ 2022-11-30 17:33 UTC (permalink / raw)
To: Krzysztof Kozlowski, Chester Lin, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn
Cc: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, netdev, s32,
devicetree, linux-kernel, linux-arm-kernel, Matthias Brugger
Hi Krysztof,
Am 30.11.22 um 16:51 schrieb Krzysztof Kozlowski:
> On 28/11/2022 06:49, Chester Lin wrote:
>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
>> Chassis.
>>
>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
>> Signed-off-by: Chester Lin <clin@suse.com>
>
> Thank you for your patch. There is something to discuss/improve.
>
>> ---
>>
>> Changes in v2:
>> - Fix schema issues.
>> - Add minItems to clocks & clock-names.
>> - Replace all sgmii/SGMII terms with pcs/PCS.
>>
>> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
>> 1 file changed, 135 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>> new file mode 100644
>> index 000000000000..c6839fd3df40
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
[...]
>> +properties:
>> + compatible:
>> + enum:
>> + - nxp,s32cc-dwmac
>> +
>> + reg:
>> + items:
>> + - description: Main GMAC registers
>> + - description: S32 MAC control registers
>> +
>> + dma-coherent: true
>> +
>> + clocks:
>> + minItems: 5
>
> Why only 5 clocks are required? Receive clocks don't have to be there?
> Is such system - only with clocks for transmit - usable?
>
>> + items:
>> + - description: Main GMAC clock
>> + - description: Peripheral registers clock
>> + - description: Transmit PCS clock
>> + - description: Transmit RGMII clock
>> + - description: Transmit RMII clock
>> + - description: Transmit MII clock
>> + - description: Receive PCS clock
>> + - description: Receive RGMII clock
>> + - description: Receive RMII clock
>> + - description: Receive MII clock
>> + - description:
>> + PTP reference clock. This clock is used for programming the
>> + Timestamp Addend Register. If not passed then the system
>> + clock will be used.
>> +
>> + clock-names:
>> + minItems: 5
>> + items:
>> + - const: stmmaceth
>> + - const: pclk
>> + - const: tx_pcs
>> + - const: tx_rgmii
>> + - const: tx_rmii
>> + - const: tx_mii
>> + - const: rx_pcs
>> + - const: rx_rgmii
>> + - const: rx_rmii
>> + - const: rx_mii
>> + - const: ptp_ref
>> +
>> + tx-fifo-depth:
>> + const: 20480
>> +
>> + rx-fifo-depth:
>> + const: 20480
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - tx-fifo-depth
>> + - rx-fifo-depth
>> + - clocks
>> + - clock-names
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>
> Why defines? Your clock controller is not ready? If so, just use raw
> numbers.
Please compare v1: There is no Linux-driven clock controller here but
rather a fluid SCMI firmware interface. Work towards getting clocks into
a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
also explains the ugly examples here and for pinctrl.
Logically there are only 5 input clocks; however due to SCMI not
supporting re-parenting today, some clocks got duplicated at SCMI level.
Andrew appeared to approve of that approach. I still dislike it but
don't have a better proposal that would work today. So the two values
above indeed seem wrong and should be 11 rather than 5.
Cheers,
Andreas
>> +
>> + soc {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + gmac0: ethernet@4033c000 {
>> + compatible = "nxp,s32cc-dwmac";
>> + reg = <0x4033c000 0x2000>, /* gmac IP */
>> + <0x4007C004 0x4>; /* S32 CTRL_STS reg */
>
> Lowercase hex.
>
>> + interrupt-parent = <&gic>;
>> + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "macirq";
>> + phy-mode = "rgmii-id";
>> + tx-fifo-depth = <20480>;
>> + rx-fifo-depth = <20480>;
>> + dma-coherent;
>> + clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_PCS>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_PCS>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
>> + <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
>
>
> Best regards,
> Krzysztof
>
--
SUSE Software Solutions Germany GmbH
Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nürnberg)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-11-30 17:33 ` Andreas Färber
@ 2022-11-30 18:14 ` Andrew Lunn
-1 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2022-11-30 18:14 UTC (permalink / raw)
To: Andreas Färber
Cc: Krzysztof Kozlowski, Chester Lin, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu,
netdev, s32, devicetree, linux-kernel, linux-arm-kernel,
Matthias Brugger
> Please compare v1: There is no Linux-driven clock controller here but rather
> a fluid SCMI firmware interface. Work towards getting clocks into a
> kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which also
> explains the ugly examples here and for pinctrl.
>
> Logically there are only 5 input clocks; however due to SCMI not supporting
> re-parenting today, some clocks got duplicated at SCMI level. Andrew
> appeared to approve of that approach. I still dislike it but don't have a
> better proposal that would work today. So the two values above indeed seem
> wrong and should be 11 rather than 5.
Just be aware, you are setting an ABI here. So your fluid SCMI
firmware interface must forever support this.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-11-30 18:14 ` Andrew Lunn
0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2022-11-30 18:14 UTC (permalink / raw)
To: Andreas Färber
Cc: Krzysztof Kozlowski, Chester Lin, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu,
netdev, s32, devicetree, linux-kernel, linux-arm-kernel,
Matthias Brugger
> Please compare v1: There is no Linux-driven clock controller here but rather
> a fluid SCMI firmware interface. Work towards getting clocks into a
> kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which also
> explains the ugly examples here and for pinctrl.
>
> Logically there are only 5 input clocks; however due to SCMI not supporting
> re-parenting today, some clocks got duplicated at SCMI level. Andrew
> appeared to approve of that approach. I still dislike it but don't have a
> better proposal that would work today. So the two values above indeed seem
> wrong and should be 11 rather than 5.
Just be aware, you are setting an ABI here. So your fluid SCMI
firmware interface must forever support this.
Andrew
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-11-30 17:33 ` Andreas Färber
@ 2022-12-01 10:18 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-01 10:18 UTC (permalink / raw)
To: Andreas Färber, Chester Lin, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn
Cc: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, netdev, s32,
devicetree, linux-kernel, linux-arm-kernel, Matthias Brugger
On 30/11/2022 18:33, Andreas Färber wrote:
> Hi Krysztof,
>
> Am 30.11.22 um 16:51 schrieb Krzysztof Kozlowski:
>> On 28/11/2022 06:49, Chester Lin wrote:
>>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
>>> Chassis.
>>>
>>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> ---
>>>
>>> Changes in v2:
>>> - Fix schema issues.
>>> - Add minItems to clocks & clock-names.
>>> - Replace all sgmii/SGMII terms with pcs/PCS.
>>>
>>> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
>>> 1 file changed, 135 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>> new file mode 100644
>>> index 000000000000..c6839fd3df40
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> [...]
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - nxp,s32cc-dwmac
>>> +
>>> + reg:
>>> + items:
>>> + - description: Main GMAC registers
>>> + - description: S32 MAC control registers
>>> +
>>> + dma-coherent: true
>>> +
>>> + clocks:
>>> + minItems: 5
>>
>> Why only 5 clocks are required? Receive clocks don't have to be there?
>> Is such system - only with clocks for transmit - usable?
Any comments here? If not, drop minItems.
>>
>>> + items:
>>> + - description: Main GMAC clock
>>> + - description: Peripheral registers clock
>>> + - description: Transmit PCS clock
>>> + - description: Transmit RGMII clock
>>> + - description: Transmit RMII clock
>>> + - description: Transmit MII clock
>>> + - description: Receive PCS clock
>>> + - description: Receive RGMII clock
>>> + - description: Receive RMII clock
>>> + - description: Receive MII clock
>>> + - description:
>>> + PTP reference clock. This clock is used for programming the
>>> + Timestamp Addend Register. If not passed then the system
>>> + clock will be used.
>>> +
>>> + clock-names:
>>> + minItems: 5
>>> + items:
>>> + - const: stmmaceth
>>> + - const: pclk
>>> + - const: tx_pcs
>>> + - const: tx_rgmii
>>> + - const: tx_rmii
>>> + - const: tx_mii
>>> + - const: rx_pcs
>>> + - const: rx_rgmii
>>> + - const: rx_rmii
>>> + - const: rx_mii
>>> + - const: ptp_ref
>>> +
>>> + tx-fifo-depth:
>>> + const: 20480
>>> +
>>> + rx-fifo-depth:
>>> + const: 20480
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - tx-fifo-depth
>>> + - rx-fifo-depth
>>> + - clocks
>>> + - clock-names
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>>
>> Why defines? Your clock controller is not ready? If so, just use raw
>> numbers.
>
> Please compare v1: There is no Linux-driven clock controller here but
> rather a fluid SCMI firmware interface. Work towards getting clocks into
> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
> also explains the ugly examples here and for pinctrl.
This does not explain to me why you added defines in the example. Are
you saying these can change any moment?
>
> Logically there are only 5 input clocks; however due to SCMI not
> supporting re-parenting today, some clocks got duplicated at SCMI level.
> Andrew appeared to approve of that approach. I still dislike it but
> don't have a better proposal that would work today. So the two values
> above indeed seem wrong and should be 11 rather than 5.
You should rather fix firmware then create some incorrect bindings as a
workaround...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-12-01 10:18 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-01 10:18 UTC (permalink / raw)
To: Andreas Färber, Chester Lin, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn
Cc: Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, netdev, s32,
devicetree, linux-kernel, linux-arm-kernel, Matthias Brugger
On 30/11/2022 18:33, Andreas Färber wrote:
> Hi Krysztof,
>
> Am 30.11.22 um 16:51 schrieb Krzysztof Kozlowski:
>> On 28/11/2022 06:49, Chester Lin wrote:
>>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
>>> Chassis.
>>>
>>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> ---
>>>
>>> Changes in v2:
>>> - Fix schema issues.
>>> - Add minItems to clocks & clock-names.
>>> - Replace all sgmii/SGMII terms with pcs/PCS.
>>>
>>> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
>>> 1 file changed, 135 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>> new file mode 100644
>>> index 000000000000..c6839fd3df40
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> [...]
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - nxp,s32cc-dwmac
>>> +
>>> + reg:
>>> + items:
>>> + - description: Main GMAC registers
>>> + - description: S32 MAC control registers
>>> +
>>> + dma-coherent: true
>>> +
>>> + clocks:
>>> + minItems: 5
>>
>> Why only 5 clocks are required? Receive clocks don't have to be there?
>> Is such system - only with clocks for transmit - usable?
Any comments here? If not, drop minItems.
>>
>>> + items:
>>> + - description: Main GMAC clock
>>> + - description: Peripheral registers clock
>>> + - description: Transmit PCS clock
>>> + - description: Transmit RGMII clock
>>> + - description: Transmit RMII clock
>>> + - description: Transmit MII clock
>>> + - description: Receive PCS clock
>>> + - description: Receive RGMII clock
>>> + - description: Receive RMII clock
>>> + - description: Receive MII clock
>>> + - description:
>>> + PTP reference clock. This clock is used for programming the
>>> + Timestamp Addend Register. If not passed then the system
>>> + clock will be used.
>>> +
>>> + clock-names:
>>> + minItems: 5
>>> + items:
>>> + - const: stmmaceth
>>> + - const: pclk
>>> + - const: tx_pcs
>>> + - const: tx_rgmii
>>> + - const: tx_rmii
>>> + - const: tx_mii
>>> + - const: rx_pcs
>>> + - const: rx_rgmii
>>> + - const: rx_rmii
>>> + - const: rx_mii
>>> + - const: ptp_ref
>>> +
>>> + tx-fifo-depth:
>>> + const: 20480
>>> +
>>> + rx-fifo-depth:
>>> + const: 20480
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - tx-fifo-depth
>>> + - rx-fifo-depth
>>> + - clocks
>>> + - clock-names
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>>
>> Why defines? Your clock controller is not ready? If so, just use raw
>> numbers.
>
> Please compare v1: There is no Linux-driven clock controller here but
> rather a fluid SCMI firmware interface. Work towards getting clocks into
> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
> also explains the ugly examples here and for pinctrl.
This does not explain to me why you added defines in the example. Are
you saying these can change any moment?
>
> Logically there are only 5 input clocks; however due to SCMI not
> supporting re-parenting today, some clocks got duplicated at SCMI level.
> Andrew appeared to approve of that approach. I still dislike it but
> don't have a better proposal that would work today. So the two values
> above indeed seem wrong and should be 11 rather than 5.
You should rather fix firmware then create some incorrect bindings as a
workaround...
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-12-01 10:18 ` Krzysztof Kozlowski
@ 2022-12-05 7:54 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-12-05 7:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, Chester Lin,
ghennadi.procopciuc
Hi Krzysztof,
On Thu, Dec 01, 2022 at 11:18:57AM +0100, Krzysztof Kozlowski wrote:
> On 30/11/2022 18:33, Andreas Färber wrote:
> > Hi Krysztof,
> >
> > Am 30.11.22 um 16:51 schrieb Krzysztof Kozlowski:
> >> On 28/11/2022 06:49, Chester Lin wrote:
> >>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> >>> Chassis.
> >>>
> >>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> >>> Signed-off-by: Chester Lin <clin@suse.com>
> >>
> >> Thank you for your patch. There is something to discuss/improve.
> >>
Thanks for taking time to review this patch!
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - Fix schema issues.
> >>> - Add minItems to clocks & clock-names.
> >>> - Replace all sgmii/SGMII terms with pcs/PCS.
> >>>
> >>> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
> >>> 1 file changed, 135 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> >>> new file mode 100644
> >>> index 000000000000..c6839fd3df40
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > [...]
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - nxp,s32cc-dwmac
> >>> +
> >>> + reg:
> >>> + items:
> >>> + - description: Main GMAC registers
> >>> + - description: S32 MAC control registers
> >>> +
> >>> + dma-coherent: true
> >>> +
> >>> + clocks:
> >>> + minItems: 5
> >>
> >> Why only 5 clocks are required? Receive clocks don't have to be there?
> >> Is such system - only with clocks for transmit - usable?
>
> Any comments here? If not, drop minItems.
>
> >>
> >>> + items:
> >>> + - description: Main GMAC clock
> >>> + - description: Peripheral registers clock
> >>> + - description: Transmit PCS clock
> >>> + - description: Transmit RGMII clock
> >>> + - description: Transmit RMII clock
> >>> + - description: Transmit MII clock
> >>> + - description: Receive PCS clock
> >>> + - description: Receive RGMII clock
> >>> + - description: Receive RMII clock
> >>> + - description: Receive MII clock
> >>> + - description:
> >>> + PTP reference clock. This clock is used for programming the
> >>> + Timestamp Addend Register. If not passed then the system
> >>> + clock will be used.
> >>> +
> >>> + clock-names:
> >>> + minItems: 5
> >>> + items:
> >>> + - const: stmmaceth
> >>> + - const: pclk
> >>> + - const: tx_pcs
> >>> + - const: tx_rgmii
> >>> + - const: tx_rmii
> >>> + - const: tx_mii
> >>> + - const: rx_pcs
> >>> + - const: rx_rgmii
> >>> + - const: rx_rmii
> >>> + - const: rx_mii
> >>> + - const: ptp_ref
> >>> +
> >>> + tx-fifo-depth:
> >>> + const: 20480
> >>> +
> >>> + rx-fifo-depth:
> >>> + const: 20480
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - tx-fifo-depth
> >>> + - rx-fifo-depth
> >>> + - clocks
> >>> + - clock-names
> >>> +
> >>> +unevaluatedProperties: false
> >>> +
> >>> +examples:
> >>> + - |
> >>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>> +
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
> >>
> >> Why defines? Your clock controller is not ready? If so, just use raw
> >> numbers.
> >
> > Please compare v1: There is no Linux-driven clock controller here but
> > rather a fluid SCMI firmware interface. Work towards getting clocks into
> > a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
> > also explains the ugly examples here and for pinctrl.
>
> This does not explain to me why you added defines in the example. Are
> you saying these can change any moment?
>
Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
some redundant TS clock IDs were removed and the rest of clock IDs were all moved
forward. Apart from GMAC-related IDs, some other clock IDs were also appended
in both base-clock IDs and platform-specific clock IDs [The first plat ID =
The last base ID + 1]. Due to the current design of the clk-scmi driver and the
SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
a blank in order to avoid query miss, which could affect the probe speed.
> >
> > Logically there are only 5 input clocks; however due to SCMI not
> > supporting re-parenting today, some clocks got duplicated at SCMI level.
> > Andrew appeared to approve of that approach. I still dislike it but
> > don't have a better proposal that would work today. So the two values
> > above indeed seem wrong and should be 11 rather than 5.
>
> You should rather fix firmware then create some incorrect bindings as a
> workaround...
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-12-05 7:54 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-12-05 7:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, Chester Lin,
ghennadi.procopciuc
Hi Krzysztof,
On Thu, Dec 01, 2022 at 11:18:57AM +0100, Krzysztof Kozlowski wrote:
> On 30/11/2022 18:33, Andreas Färber wrote:
> > Hi Krysztof,
> >
> > Am 30.11.22 um 16:51 schrieb Krzysztof Kozlowski:
> >> On 28/11/2022 06:49, Chester Lin wrote:
> >>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> >>> Chassis.
> >>>
> >>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> >>> Signed-off-by: Chester Lin <clin@suse.com>
> >>
> >> Thank you for your patch. There is something to discuss/improve.
> >>
Thanks for taking time to review this patch!
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - Fix schema issues.
> >>> - Add minItems to clocks & clock-names.
> >>> - Replace all sgmii/SGMII terms with pcs/PCS.
> >>>
> >>> .../bindings/net/nxp,s32cc-dwmac.yaml | 135 ++++++++++++++++++
> >>> 1 file changed, 135 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> >>> new file mode 100644
> >>> index 000000000000..c6839fd3df40
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > [...]
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - nxp,s32cc-dwmac
> >>> +
> >>> + reg:
> >>> + items:
> >>> + - description: Main GMAC registers
> >>> + - description: S32 MAC control registers
> >>> +
> >>> + dma-coherent: true
> >>> +
> >>> + clocks:
> >>> + minItems: 5
> >>
> >> Why only 5 clocks are required? Receive clocks don't have to be there?
> >> Is such system - only with clocks for transmit - usable?
>
> Any comments here? If not, drop minItems.
>
> >>
> >>> + items:
> >>> + - description: Main GMAC clock
> >>> + - description: Peripheral registers clock
> >>> + - description: Transmit PCS clock
> >>> + - description: Transmit RGMII clock
> >>> + - description: Transmit RMII clock
> >>> + - description: Transmit MII clock
> >>> + - description: Receive PCS clock
> >>> + - description: Receive RGMII clock
> >>> + - description: Receive RMII clock
> >>> + - description: Receive MII clock
> >>> + - description:
> >>> + PTP reference clock. This clock is used for programming the
> >>> + Timestamp Addend Register. If not passed then the system
> >>> + clock will be used.
> >>> +
> >>> + clock-names:
> >>> + minItems: 5
> >>> + items:
> >>> + - const: stmmaceth
> >>> + - const: pclk
> >>> + - const: tx_pcs
> >>> + - const: tx_rgmii
> >>> + - const: tx_rmii
> >>> + - const: tx_mii
> >>> + - const: rx_pcs
> >>> + - const: rx_rgmii
> >>> + - const: rx_rmii
> >>> + - const: rx_mii
> >>> + - const: ptp_ref
> >>> +
> >>> + tx-fifo-depth:
> >>> + const: 20480
> >>> +
> >>> + rx-fifo-depth:
> >>> + const: 20480
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - tx-fifo-depth
> >>> + - rx-fifo-depth
> >>> + - clocks
> >>> + - clock-names
> >>> +
> >>> +unevaluatedProperties: false
> >>> +
> >>> +examples:
> >>> + - |
> >>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>> +
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> >>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
> >>
> >> Why defines? Your clock controller is not ready? If so, just use raw
> >> numbers.
> >
> > Please compare v1: There is no Linux-driven clock controller here but
> > rather a fluid SCMI firmware interface. Work towards getting clocks into
> > a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
> > also explains the ugly examples here and for pinctrl.
>
> This does not explain to me why you added defines in the example. Are
> you saying these can change any moment?
>
Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
some redundant TS clock IDs were removed and the rest of clock IDs were all moved
forward. Apart from GMAC-related IDs, some other clock IDs were also appended
in both base-clock IDs and platform-specific clock IDs [The first plat ID =
The last base ID + 1]. Due to the current design of the clk-scmi driver and the
SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
a blank in order to avoid query miss, which could affect the probe speed.
> >
> > Logically there are only 5 input clocks; however due to SCMI not
> > supporting re-parenting today, some clocks got duplicated at SCMI level.
> > Andrew appeared to approve of that approach. I still dislike it but
> > don't have a better proposal that would work today. So the two values
> > above indeed seem wrong and should be 11 rather than 5.
>
> You should rather fix firmware then create some incorrect bindings as a
> workaround...
>
> Best regards,
> Krzysztof
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-12-05 7:54 ` Chester Lin
@ 2022-12-05 8:55 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-05 8:55 UTC (permalink / raw)
To: Chester Lin
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, ghennadi.procopciuc
On 05/12/2022 08:54, Chester Lin wrote:
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>>> +
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>>>>
>>>> Why defines? Your clock controller is not ready? If so, just use raw
>>>> numbers.
>>>
>>> Please compare v1: There is no Linux-driven clock controller here but
>>> rather a fluid SCMI firmware interface. Work towards getting clocks into
>>> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
>>> also explains the ugly examples here and for pinctrl.
>>
>> This does not explain to me why you added defines in the example. Are
>> you saying these can change any moment?
>>
>
> Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
> some redundant TS clock IDs were removed and the rest of clock IDs were all moved
> forward.
This is not accepted. Your downstream TF-A cannot change bindings. As an
upstream contributor you should push this back and definitely not try to
upstream such approach.
> Apart from GMAC-related IDs, some other clock IDs were also appended
> in both base-clock IDs and platform-specific clock IDs [The first plat ID =
> The last base ID + 1]. Due to the current design of the clk-scmi driver and the
> SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
> a blank in order to avoid query miss, which could affect the probe speed.
You miss here broken ABI! Any change in IDs causes all DTBs to be
broken. Downstream, upstream, other projects, everywhere.
Therefore thank you for clarifying that we need to be more careful about
stuff coming from (or for) NXP. Here you need to drop all defines and
all your patches must assume the ID is fixed. Once there, it's
unchangeable without breaking the ABI.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-12-05 8:55 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-05 8:55 UTC (permalink / raw)
To: Chester Lin
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, ghennadi.procopciuc
On 05/12/2022 08:54, Chester Lin wrote:
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>>> +
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>>>>
>>>> Why defines? Your clock controller is not ready? If so, just use raw
>>>> numbers.
>>>
>>> Please compare v1: There is no Linux-driven clock controller here but
>>> rather a fluid SCMI firmware interface. Work towards getting clocks into
>>> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
>>> also explains the ugly examples here and for pinctrl.
>>
>> This does not explain to me why you added defines in the example. Are
>> you saying these can change any moment?
>>
>
> Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
> some redundant TS clock IDs were removed and the rest of clock IDs were all moved
> forward.
This is not accepted. Your downstream TF-A cannot change bindings. As an
upstream contributor you should push this back and definitely not try to
upstream such approach.
> Apart from GMAC-related IDs, some other clock IDs were also appended
> in both base-clock IDs and platform-specific clock IDs [The first plat ID =
> The last base ID + 1]. Due to the current design of the clk-scmi driver and the
> SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
> a blank in order to avoid query miss, which could affect the probe speed.
You miss here broken ABI! Any change in IDs causes all DTBs to be
broken. Downstream, upstream, other projects, everywhere.
Therefore thank you for clarifying that we need to be more careful about
stuff coming from (or for) NXP. Here you need to drop all defines and
all your patches must assume the ID is fixed. Once there, it's
unchangeable without breaking the ABI.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-12-05 8:55 ` Krzysztof Kozlowski
@ 2022-12-13 2:46 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-12-13 2:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, ghennadi.procopciuc
Hi Krzysztof,
Sorry for the late reply.
On Mon, Dec 05, 2022 at 09:55:40AM +0100, Krzysztof Kozlowski wrote:
> On 05/12/2022 08:54, Chester Lin wrote:
> >>>>> +examples:
> >>>>> + - |
> >>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>>>> +
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
> >>>>
> >>>> Why defines? Your clock controller is not ready? If so, just use raw
> >>>> numbers.
> >>>
> >>> Please compare v1: There is no Linux-driven clock controller here but
> >>> rather a fluid SCMI firmware interface. Work towards getting clocks into
> >>> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
> >>> also explains the ugly examples here and for pinctrl.
> >>
> >> This does not explain to me why you added defines in the example. Are
> >> you saying these can change any moment?
> >>
> >
> > Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
> > some redundant TS clock IDs were removed and the rest of clock IDs were all moved
> > forward.
>
> This is not accepted. Your downstream TF-A cannot change bindings. As an
> upstream contributor you should push this back and definitely not try to
> upstream such approach.
>
> > Apart from GMAC-related IDs, some other clock IDs were also appended
> > in both base-clock IDs and platform-specific clock IDs [The first plat ID =
> > The last base ID + 1]. Due to the current design of the clk-scmi driver and the
> > SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
> > a blank in order to avoid query miss, which could affect the probe speed.
>
> You miss here broken ABI! Any change in IDs causes all DTBs to be
> broken. Downstream, upstream, other projects, everywhere.
>
> Therefore thank you for clarifying that we need to be more careful about
> stuff coming from (or for) NXP. Here you need to drop all defines and
> all your patches must assume the ID is fixed. Once there, it's
> unchangeable without breaking the ABI.
>
Please accept my apologies for submitting these bad patches. We were just
confused since we thought that this approach might be acceptable because
there were some similar examples got merged in the kernel tree:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/intel,dwmac-plat.yaml#L57
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml#L55
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml#L73
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml#L39
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/watchdog/intel,keembay-wdt.yaml#L46
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml#L75
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml#L282
The defines in these yaml files are not actually referred by kernel DTs or
drivers so I assume that they should be provided by firmware as pure integer
numbers and the clk-scmi driver should just take them to ask firmware for doing
clk stuff.
Regards,
Chester
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-12-13 2:46 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-12-13 2:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, ghennadi.procopciuc
Hi Krzysztof,
Sorry for the late reply.
On Mon, Dec 05, 2022 at 09:55:40AM +0100, Krzysztof Kozlowski wrote:
> On 05/12/2022 08:54, Chester Lin wrote:
> >>>>> +examples:
> >>>>> + - |
> >>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>>>> +
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> >>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
> >>>>
> >>>> Why defines? Your clock controller is not ready? If so, just use raw
> >>>> numbers.
> >>>
> >>> Please compare v1: There is no Linux-driven clock controller here but
> >>> rather a fluid SCMI firmware interface. Work towards getting clocks into
> >>> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
> >>> also explains the ugly examples here and for pinctrl.
> >>
> >> This does not explain to me why you added defines in the example. Are
> >> you saying these can change any moment?
> >>
> >
> > Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
> > some redundant TS clock IDs were removed and the rest of clock IDs were all moved
> > forward.
>
> This is not accepted. Your downstream TF-A cannot change bindings. As an
> upstream contributor you should push this back and definitely not try to
> upstream such approach.
>
> > Apart from GMAC-related IDs, some other clock IDs were also appended
> > in both base-clock IDs and platform-specific clock IDs [The first plat ID =
> > The last base ID + 1]. Due to the current design of the clk-scmi driver and the
> > SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
> > a blank in order to avoid query miss, which could affect the probe speed.
>
> You miss here broken ABI! Any change in IDs causes all DTBs to be
> broken. Downstream, upstream, other projects, everywhere.
>
> Therefore thank you for clarifying that we need to be more careful about
> stuff coming from (or for) NXP. Here you need to drop all defines and
> all your patches must assume the ID is fixed. Once there, it's
> unchangeable without breaking the ABI.
>
Please accept my apologies for submitting these bad patches. We were just
confused since we thought that this approach might be acceptable because
there were some similar examples got merged in the kernel tree:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/intel,dwmac-plat.yaml#L57
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml#L55
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml#L73
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml#L39
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/watchdog/intel,keembay-wdt.yaml#L46
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml#L75
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml#L282
The defines in these yaml files are not actually referred by kernel DTs or
drivers so I assume that they should be provided by firmware as pure integer
numbers and the clk-scmi driver should just take them to ask firmware for doing
clk stuff.
Regards,
Chester
> Best regards,
> Krzysztof
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
2022-12-13 2:46 ` Chester Lin
@ 2022-12-13 7:50 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 7:50 UTC (permalink / raw)
To: Chester Lin
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, ghennadi.procopciuc
On 13/12/2022 03:46, Chester Lin wrote:
> Hi Krzysztof,
>
> Sorry for the late reply.
>
> On Mon, Dec 05, 2022 at 09:55:40AM +0100, Krzysztof Kozlowski wrote:
>> On 05/12/2022 08:54, Chester Lin wrote:
>>>>>>> +examples:
>>>>>>> + - |
>>>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>> +
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>>>>>>
>>>>>> Why defines? Your clock controller is not ready? If so, just use raw
>>>>>> numbers.
>>>>>
>>>>> Please compare v1: There is no Linux-driven clock controller here but
>>>>> rather a fluid SCMI firmware interface. Work towards getting clocks into
>>>>> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
>>>>> also explains the ugly examples here and for pinctrl.
>>>>
>>>> This does not explain to me why you added defines in the example. Are
>>>> you saying these can change any moment?
>>>>
>>>
>>> Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
>>> some redundant TS clock IDs were removed and the rest of clock IDs were all moved
>>> forward.
>>
>> This is not accepted. Your downstream TF-A cannot change bindings. As an
>> upstream contributor you should push this back and definitely not try to
>> upstream such approach.
>>
>>> Apart from GMAC-related IDs, some other clock IDs were also appended
>>> in both base-clock IDs and platform-specific clock IDs [The first plat ID =
>>> The last base ID + 1]. Due to the current design of the clk-scmi driver and the
>>> SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
>>> a blank in order to avoid query miss, which could affect the probe speed.
>>
>> You miss here broken ABI! Any change in IDs causes all DTBs to be
>> broken. Downstream, upstream, other projects, everywhere.
>>
>> Therefore thank you for clarifying that we need to be more careful about
>> stuff coming from (or for) NXP. Here you need to drop all defines and
>> all your patches must assume the ID is fixed. Once there, it's
>> unchangeable without breaking the ABI.
>>
>
> Please accept my apologies for submitting these bad patches. We were just
> confused since we thought that this approach might be acceptable because
> there were some similar examples got merged in the kernel tree:
How are these related to the talk of ABI break?
>
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/intel,dwmac-plat.yaml#L57
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml#L55
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml#L73
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml#L39
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/watchdog/intel,keembay-wdt.yaml#L46
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml#L75
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml#L282
How are these even relevant here? The support for this platform is
incomplete, right? These are just few scattered pieces - only bindings -
without drivers and DTS. It proves nothing. You cannot take some
incomplete platform and build on top of it theory that you can keep
changing ABI!
And anyway it is entirely independent problem. You just said you want to
change defines which is not allowed. ABI break. No one changes the
Keembay defines, whatever they are (you even do not know what they are...).
>
> The defines in these yaml files are not actually referred by kernel DTs or
> drivers so I assume that they should be provided by firmware as pure integer
> numbers and the clk-scmi driver should just take them to ask firmware for doing
> clk stuff.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver
@ 2022-12-13 7:50 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 7:50 UTC (permalink / raw)
To: Chester Lin
Cc: Andreas Färber, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Jan Petrous, Andrew Lunn, Alexandre Torgue, Giuseppe Cavallaro,
Jose Abreu, netdev, s32, devicetree, linux-kernel,
linux-arm-kernel, Matthias Brugger, ghennadi.procopciuc
On 13/12/2022 03:46, Chester Lin wrote:
> Hi Krzysztof,
>
> Sorry for the late reply.
>
> On Mon, Dec 05, 2022 at 09:55:40AM +0100, Krzysztof Kozlowski wrote:
>> On 05/12/2022 08:54, Chester Lin wrote:
>>>>>>> +examples:
>>>>>>> + - |
>>>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>>>>> +
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_AXI
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_PCS
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_PCS
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
>>>>>>> + #define S32GEN1_SCMI_CLK_GMAC0_TS
>>>>>>
>>>>>> Why defines? Your clock controller is not ready? If so, just use raw
>>>>>> numbers.
>>>>>
>>>>> Please compare v1: There is no Linux-driven clock controller here but
>>>>> rather a fluid SCMI firmware interface. Work towards getting clocks into
>>>>> a kernel-hosted .dtsi was halted in favor of (downstream) TF-A, which
>>>>> also explains the ugly examples here and for pinctrl.
>>>>
>>>> This does not explain to me why you added defines in the example. Are
>>>> you saying these can change any moment?
>>>>
>>>
>>> Actually these GMAC-related SCMI clock IDs changed once in NXP's downstream TF-A,
>>> some redundant TS clock IDs were removed and the rest of clock IDs were all moved
>>> forward.
>>
>> This is not accepted. Your downstream TF-A cannot change bindings. As an
>> upstream contributor you should push this back and definitely not try to
>> upstream such approach.
>>
>>> Apart from GMAC-related IDs, some other clock IDs were also appended
>>> in both base-clock IDs and platform-specific clock IDs [The first plat ID =
>>> The last base ID + 1]. Due to the current design of the clk-scmi driver and the
>>> SCMI clock protocol, IIUC, it's better to keep all clock IDs in sequence without
>>> a blank in order to avoid query miss, which could affect the probe speed.
>>
>> You miss here broken ABI! Any change in IDs causes all DTBs to be
>> broken. Downstream, upstream, other projects, everywhere.
>>
>> Therefore thank you for clarifying that we need to be more careful about
>> stuff coming from (or for) NXP. Here you need to drop all defines and
>> all your patches must assume the ID is fixed. Once there, it's
>> unchangeable without breaking the ABI.
>>
>
> Please accept my apologies for submitting these bad patches. We were just
> confused since we thought that this approach might be acceptable because
> there were some similar examples got merged in the kernel tree:
How are these related to the talk of ABI break?
>
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/intel,dwmac-plat.yaml#L57
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml#L55
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml#L73
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml#L39
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/watchdog/intel,keembay-wdt.yaml#L46
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml#L75
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml#L282
How are these even relevant here? The support for this platform is
incomplete, right? These are just few scattered pieces - only bindings -
without drivers and DTS. It proves nothing. You cannot take some
incomplete platform and build on top of it theory that you can keep
changing ABI!
And anyway it is entirely independent problem. You just said you want to
change defines which is not allowed. ABI break. No one changes the
Keembay defines, whatever they are (you even do not know what they are...).
>
> The defines in these yaml files are not actually referred by kernel DTs or
> drivers so I assume that they should be provided by firmware as pure integer
> numbers and the clk-scmi driver should just take them to ask firmware for doing
> clk stuff.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 3/5] net: stmmac: Add CSR clock 500Mhz/800Mhz support
2022-11-28 5:49 ` Chester Lin
@ 2022-11-28 5:49 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: Chester Lin, netdev, linux-kernel, linux-arm-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jan Petrous, Andreas Färber, Matthias Brugger
Add additional 500Mhz/800Mhz CSR clock ranges since NXP S32CC DWMAC can
support higher frequencies.
Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
No change in v2.
drivers/net/ethernet/stmicro/stmmac/common.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++
include/linux/stmmac.h | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 6b5d96bced47..5b7e8cc70439 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -222,6 +222,8 @@ struct stmmac_safety_stats {
#define CSR_F_150M 150000000
#define CSR_F_250M 250000000
#define CSR_F_300M 300000000
+#define CSR_F_500M 500000000
+#define CSR_F_800M 800000000
#define MAC_CSR_H_FRQ_MASK 0x20
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6b43da78cdf0..ff0b32c9e748 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -323,6 +323,10 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
priv->clk_csr = STMMAC_CSR_150_250M;
else if ((clk_rate >= CSR_F_250M) && (clk_rate <= CSR_F_300M))
priv->clk_csr = STMMAC_CSR_250_300M;
+ else if ((clk_rate >= CSR_F_300M) && (clk_rate < CSR_F_500M))
+ priv->clk_csr = STMMAC_CSR_300_500M;
+ else if ((clk_rate >= CSR_F_500M) && (clk_rate < CSR_F_800M))
+ priv->clk_csr = STMMAC_CSR_500_800M;
}
if (priv->plat->has_sun8i) {
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index fb2e88614f5d..307980c808f7 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -34,6 +34,8 @@
#define STMMAC_CSR_35_60M 0x3 /* MDC = clk_scr_i/26 */
#define STMMAC_CSR_150_250M 0x4 /* MDC = clk_scr_i/102 */
#define STMMAC_CSR_250_300M 0x5 /* MDC = clk_scr_i/122 */
+#define STMMAC_CSR_300_500M 0x6 /* MDC = clk_scr_i/204 */
+#define STMMAC_CSR_500_800M 0x7 /* MDC = clk_scr_i/324 */
/* MTL algorithms identifiers */
#define MTL_TX_ALGORITHM_WRR 0x0
--
2.37.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 3/5] net: stmmac: Add CSR clock 500Mhz/800Mhz support
@ 2022-11-28 5:49 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: Chester Lin, netdev, linux-kernel, linux-arm-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jan Petrous, Andreas Färber, Matthias Brugger
Add additional 500Mhz/800Mhz CSR clock ranges since NXP S32CC DWMAC can
support higher frequencies.
Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
No change in v2.
drivers/net/ethernet/stmicro/stmmac/common.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++
include/linux/stmmac.h | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 6b5d96bced47..5b7e8cc70439 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -222,6 +222,8 @@ struct stmmac_safety_stats {
#define CSR_F_150M 150000000
#define CSR_F_250M 250000000
#define CSR_F_300M 300000000
+#define CSR_F_500M 500000000
+#define CSR_F_800M 800000000
#define MAC_CSR_H_FRQ_MASK 0x20
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6b43da78cdf0..ff0b32c9e748 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -323,6 +323,10 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
priv->clk_csr = STMMAC_CSR_150_250M;
else if ((clk_rate >= CSR_F_250M) && (clk_rate <= CSR_F_300M))
priv->clk_csr = STMMAC_CSR_250_300M;
+ else if ((clk_rate >= CSR_F_300M) && (clk_rate < CSR_F_500M))
+ priv->clk_csr = STMMAC_CSR_300_500M;
+ else if ((clk_rate >= CSR_F_500M) && (clk_rate < CSR_F_800M))
+ priv->clk_csr = STMMAC_CSR_500_800M;
}
if (priv->plat->has_sun8i) {
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index fb2e88614f5d..307980c808f7 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -34,6 +34,8 @@
#define STMMAC_CSR_35_60M 0x3 /* MDC = clk_scr_i/26 */
#define STMMAC_CSR_150_250M 0x4 /* MDC = clk_scr_i/102 */
#define STMMAC_CSR_250_300M 0x5 /* MDC = clk_scr_i/122 */
+#define STMMAC_CSR_300_500M 0x6 /* MDC = clk_scr_i/204 */
+#define STMMAC_CSR_500_800M 0x7 /* MDC = clk_scr_i/324 */
/* MTL algorithms identifiers */
#define MTL_TX_ALGORITHM_WRR 0x0
--
2.37.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 4/5] net: stmmac: Add AXI4 ACE control support
2022-11-28 5:49 ` Chester Lin
@ 2022-11-28 5:49 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: Chester Lin, netdev, linux-kernel, linux-arm-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jan Petrous, Ondrej Spacek, Andreas Färber,
Matthias Brugger
Add AXI4 cache coherency control in dwmac4 DMA core.
Signed-off-by: Ondrej Spacek <ondrej.spacek@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
No change in v2.
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 10 ++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 4 +++-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 5 +++++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
include/linux/stmmac.h | 7 +++++++
5 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index d99fa028c646..4e6e2952abfd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -517,6 +517,15 @@ static int dwmac4_enable_tbs(void __iomem *ioaddr, bool en, u32 chan)
return 0;
}
+static void dwmac4_axi4_cc(void __iomem *ioaddr,
+ struct stmmac_axi4_ace_ctrl *acecfg)
+{
+ /* Configure AXI4 cache coherency for Tx/Rx DMA channels */
+ writel(acecfg->tx_ar_reg, ioaddr + DMA_AXI4_TX_AR_ACE_CONTROL);
+ writel(acecfg->rx_aw_reg, ioaddr + DMA_AXI4_RX_AW_ACE_CONTROL);
+ writel(acecfg->txrx_awar_reg, ioaddr + DMA_AXI4_TXRX_AWAR_ACE_CONTROL);
+}
+
const struct stmmac_dma_ops dwmac4_dma_ops = {
.reset = dwmac4_dma_reset,
.init = dwmac4_dma_init,
@@ -574,4 +583,5 @@ const struct stmmac_dma_ops dwmac410_dma_ops = {
.set_bfsize = dwmac4_set_bfsize,
.enable_sph = dwmac4_enable_sph,
.enable_tbs = dwmac4_enable_tbs,
+ .axi4_cc = dwmac4_axi4_cc,
};
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
index 9321879b599c..7f491f2651b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
@@ -21,7 +21,9 @@
#define DMA_DEBUG_STATUS_0 0x0000100c
#define DMA_DEBUG_STATUS_1 0x00001010
#define DMA_DEBUG_STATUS_2 0x00001014
-#define DMA_AXI_BUS_MODE 0x00001028
+#define DMA_AXI4_TX_AR_ACE_CONTROL 0x00001020
+#define DMA_AXI4_RX_AW_ACE_CONTROL 0x00001024
+#define DMA_AXI4_TXRX_AWAR_ACE_CONTROL 0x00001028
#define DMA_TBS_CTRL 0x00001050
/* DMA Bus Mode bitmap */
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 592b4067f9b8..bffe2ec36bb3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -212,6 +212,9 @@ struct stmmac_dma_ops {
void (*set_bfsize)(void __iomem *ioaddr, int bfsize, u32 chan);
void (*enable_sph)(void __iomem *ioaddr, bool en, u32 chan);
int (*enable_tbs)(void __iomem *ioaddr, bool en, u32 chan);
+ /* Configure AXI4 cache coherency for Tx and Rx DMA channels */
+ void (*axi4_cc)(void __iomem *ioaddr,
+ struct stmmac_axi4_ace_ctrl *acecfg);
};
#define stmmac_reset(__priv, __args...) \
@@ -272,6 +275,8 @@ struct stmmac_dma_ops {
stmmac_do_void_callback(__priv, dma, enable_sph, __args)
#define stmmac_enable_tbs(__priv, __args...) \
stmmac_do_callback(__priv, dma, enable_tbs, __args)
+#define stmmac_axi4_cc(__priv, __args...) \
+ stmmac_do_void_callback(__priv, dma, axi4_cc, __args)
struct mac_device_info;
struct net_device;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ff0b32c9e748..c689723c7d93 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2917,6 +2917,9 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
if (priv->plat->axi)
stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
+ if (priv->plat->axi4_ace_ctrl)
+ stmmac_axi4_cc(priv, priv->ioaddr, priv->plat->axi4_ace_ctrl);
+
/* DMA CSR Channel configuration */
for (chan = 0; chan < dma_csr_ch; chan++) {
stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 307980c808f7..23e740c6c7b8 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -115,6 +115,12 @@ struct stmmac_axi {
bool axi_rb;
};
+struct stmmac_axi4_ace_ctrl {
+ u32 tx_ar_reg;
+ u32 rx_aw_reg;
+ u32 txrx_awar_reg;
+};
+
#define EST_GCL 1024
struct stmmac_est {
struct mutex lock;
@@ -248,6 +254,7 @@ struct plat_stmmacenet_data {
struct reset_control *stmmac_rst;
struct reset_control *stmmac_ahb_rst;
struct stmmac_axi *axi;
+ struct stmmac_axi4_ace_ctrl *axi4_ace_ctrl;
int has_gmac4;
bool has_sun8i;
bool tso_en;
--
2.37.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 4/5] net: stmmac: Add AXI4 ACE control support
@ 2022-11-28 5:49 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: Chester Lin, netdev, linux-kernel, linux-arm-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jan Petrous, Ondrej Spacek, Andreas Färber,
Matthias Brugger
Add AXI4 cache coherency control in dwmac4 DMA core.
Signed-off-by: Ondrej Spacek <ondrej.spacek@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
No change in v2.
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 10 ++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 4 +++-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 5 +++++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
include/linux/stmmac.h | 7 +++++++
5 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index d99fa028c646..4e6e2952abfd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -517,6 +517,15 @@ static int dwmac4_enable_tbs(void __iomem *ioaddr, bool en, u32 chan)
return 0;
}
+static void dwmac4_axi4_cc(void __iomem *ioaddr,
+ struct stmmac_axi4_ace_ctrl *acecfg)
+{
+ /* Configure AXI4 cache coherency for Tx/Rx DMA channels */
+ writel(acecfg->tx_ar_reg, ioaddr + DMA_AXI4_TX_AR_ACE_CONTROL);
+ writel(acecfg->rx_aw_reg, ioaddr + DMA_AXI4_RX_AW_ACE_CONTROL);
+ writel(acecfg->txrx_awar_reg, ioaddr + DMA_AXI4_TXRX_AWAR_ACE_CONTROL);
+}
+
const struct stmmac_dma_ops dwmac4_dma_ops = {
.reset = dwmac4_dma_reset,
.init = dwmac4_dma_init,
@@ -574,4 +583,5 @@ const struct stmmac_dma_ops dwmac410_dma_ops = {
.set_bfsize = dwmac4_set_bfsize,
.enable_sph = dwmac4_enable_sph,
.enable_tbs = dwmac4_enable_tbs,
+ .axi4_cc = dwmac4_axi4_cc,
};
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
index 9321879b599c..7f491f2651b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
@@ -21,7 +21,9 @@
#define DMA_DEBUG_STATUS_0 0x0000100c
#define DMA_DEBUG_STATUS_1 0x00001010
#define DMA_DEBUG_STATUS_2 0x00001014
-#define DMA_AXI_BUS_MODE 0x00001028
+#define DMA_AXI4_TX_AR_ACE_CONTROL 0x00001020
+#define DMA_AXI4_RX_AW_ACE_CONTROL 0x00001024
+#define DMA_AXI4_TXRX_AWAR_ACE_CONTROL 0x00001028
#define DMA_TBS_CTRL 0x00001050
/* DMA Bus Mode bitmap */
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 592b4067f9b8..bffe2ec36bb3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -212,6 +212,9 @@ struct stmmac_dma_ops {
void (*set_bfsize)(void __iomem *ioaddr, int bfsize, u32 chan);
void (*enable_sph)(void __iomem *ioaddr, bool en, u32 chan);
int (*enable_tbs)(void __iomem *ioaddr, bool en, u32 chan);
+ /* Configure AXI4 cache coherency for Tx and Rx DMA channels */
+ void (*axi4_cc)(void __iomem *ioaddr,
+ struct stmmac_axi4_ace_ctrl *acecfg);
};
#define stmmac_reset(__priv, __args...) \
@@ -272,6 +275,8 @@ struct stmmac_dma_ops {
stmmac_do_void_callback(__priv, dma, enable_sph, __args)
#define stmmac_enable_tbs(__priv, __args...) \
stmmac_do_callback(__priv, dma, enable_tbs, __args)
+#define stmmac_axi4_cc(__priv, __args...) \
+ stmmac_do_void_callback(__priv, dma, axi4_cc, __args)
struct mac_device_info;
struct net_device;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ff0b32c9e748..c689723c7d93 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2917,6 +2917,9 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
if (priv->plat->axi)
stmmac_axi(priv, priv->ioaddr, priv->plat->axi);
+ if (priv->plat->axi4_ace_ctrl)
+ stmmac_axi4_cc(priv, priv->ioaddr, priv->plat->axi4_ace_ctrl);
+
/* DMA CSR Channel configuration */
for (chan = 0; chan < dma_csr_ch; chan++) {
stmmac_init_chan(priv, priv->ioaddr, priv->plat->dma_cfg, chan);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 307980c808f7..23e740c6c7b8 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -115,6 +115,12 @@ struct stmmac_axi {
bool axi_rb;
};
+struct stmmac_axi4_ace_ctrl {
+ u32 tx_ar_reg;
+ u32 rx_aw_reg;
+ u32 txrx_awar_reg;
+};
+
#define EST_GCL 1024
struct stmmac_est {
struct mutex lock;
@@ -248,6 +254,7 @@ struct plat_stmmacenet_data {
struct reset_control *stmmac_rst;
struct reset_control *stmmac_ahb_rst;
struct stmmac_axi *axi;
+ struct stmmac_axi4_ace_ctrl *axi4_ace_ctrl;
int has_gmac4;
bool has_sun8i;
bool tso_en;
--
2.37.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 5/5] net: stmmac: Add NXP S32 SoC family support
2022-11-28 5:49 ` Chester Lin
@ 2022-11-28 5:49 ` Chester Lin
-1 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Andrew Lunn
Cc: Chester Lin, netdev, linux-kernel, linux-arm-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jan Petrous, Ondrej Spacek, Ghennadi Procopciuc,
Andra-Teodora Ilie, Andreas Färber, Matthias Brugger
Add GMAC support for NXP S32 SoC family. This driver is mainly based on
NXP's downstream implementation on CodeAurora[1].
[1] https://source.codeaurora.org/external/autobsps32/linux/tree/drivers/net/ethernet/stmicro/stmmac?h=bsp34.0-5.10.120-rt
Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
Signed-off-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
Changes in v2:
- Replace clock names tx_sgmii/rx_sgmii with tx_pcs/rx_pcs.
- Adjust error handlings while calling devm_clk_get().
- Remove redundant dev_info messages.
- Remove unnecessary if conditions.
- Fix the copyright format suggested by NXP.
drivers/net/ethernet/stmicro/stmmac/Kconfig | 13 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-s32cc.c | 304 ++++++++++++++++++
3 files changed, 318 insertions(+)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 31ff35174034..dd3fb5e462b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -153,6 +153,19 @@ config DWMAC_ROCKCHIP
This selects the Rockchip RK3288 SoC glue layer support for
the stmmac device driver.
+config DWMAC_S32CC
+ tristate "NXP S32 series GMAC support"
+ default ARCH_S32
+ depends on OF && (ARCH_S32 || COMPILE_TEST)
+ select MFD_SYSCON
+ select PHYLINK
+ help
+ Support for ethernet controller on NXP S32 series SOCs.
+
+ This selects NXP SoC glue layer support for the stmmac
+ device driver. This driver is used for the S32 series
+ SOCs GMAC ethernet controller.
+
config DWMAC_SOCFPGA
tristate "SOCFPGA dwmac support"
default ARCH_INTEL_SOCFPGA
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..ec92cc2becd7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o
obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o
obj-$(CONFIG_DWMAC_IMX8) += dwmac-imx.o
obj-$(CONFIG_DWMAC_VISCONTI) += dwmac-visconti.o
+obj-$(CONFIG_DWMAC_S32CC) += dwmac-s32cc.o
stmmac-platform-objs:= stmmac_platform.o
dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
new file mode 100644
index 000000000000..92a132ad985a
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DWMAC Specific Glue layer for NXP S32 Common Chassis
+ *
+ * Copyright 2019-2022 NXP
+ * Copyright (C) 2022 SUSE LLC
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/ethtool.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/of_address.h>
+#include <linux/stmmac.h>
+
+#include "stmmac_platform.h"
+
+#define GMAC_TX_RATE_125M 125000000 /* 125MHz */
+#define GMAC_TX_RATE_25M 25000000 /* 25MHz */
+#define GMAC_TX_RATE_2M5 2500000 /* 2.5MHz */
+
+/* S32 SRC register for phyif selection */
+#define PHY_INTF_SEL_MII 0x00
+#define PHY_INTF_SEL_SGMII 0x01
+#define PHY_INTF_SEL_RGMII 0x02
+#define PHY_INTF_SEL_RMII 0x08
+
+/* AXI4 ACE control settings */
+#define ACE_DOMAIN_SIGNAL 0x2
+#define ACE_CACHE_SIGNAL 0xf
+#define ACE_CONTROL_SIGNALS ((ACE_DOMAIN_SIGNAL << 4) | ACE_CACHE_SIGNAL)
+#define ACE_PROTECTION 0x2
+
+struct s32cc_priv_data {
+ void __iomem *ctrl_sts;
+ struct device *dev;
+ phy_interface_t intf_mode;
+ struct clk *tx_clk;
+ struct clk *rx_clk;
+};
+
+static int s32cc_gmac_init(struct platform_device *pdev, void *priv)
+{
+ struct s32cc_priv_data *gmac = priv;
+ u32 intf_sel;
+ int ret;
+
+ ret = clk_prepare_enable(gmac->tx_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Can't enable tx clock\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(gmac->rx_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Can't enable rx clock\n");
+ return ret;
+ }
+
+ /* set interface mode */
+ switch (gmac->intf_mode) {
+ case PHY_INTERFACE_MODE_SGMII:
+ intf_sel = PHY_INTF_SEL_SGMII;
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ intf_sel = PHY_INTF_SEL_RGMII;
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ intf_sel = PHY_INTF_SEL_RMII;
+ break;
+ case PHY_INTERFACE_MODE_MII:
+ intf_sel = PHY_INTF_SEL_MII;
+ break;
+ default:
+ dev_err(&pdev->dev, "Unsupported PHY interface: %s\n",
+ phy_modes(gmac->intf_mode));
+ return -EINVAL;
+ }
+
+ writel(intf_sel, gmac->ctrl_sts);
+
+ dev_dbg(&pdev->dev, "PHY mode set to %s\n", phy_modes(gmac->intf_mode));
+
+ return 0;
+}
+
+static void s32cc_gmac_exit(struct platform_device *pdev, void *priv)
+{
+ struct s32cc_priv_data *gmac = priv;
+
+ clk_disable_unprepare(gmac->tx_clk);
+ clk_disable_unprepare(gmac->rx_clk);
+}
+
+static void s32cc_fix_speed(void *priv, unsigned int speed)
+{
+ struct s32cc_priv_data *gmac = priv;
+
+ /* SGMII mode doesn't support the clock reconfiguration */
+ if (gmac->intf_mode == PHY_INTERFACE_MODE_SGMII)
+ return;
+
+ switch (speed) {
+ case SPEED_1000:
+ dev_info(gmac->dev, "Set TX clock to 125M\n");
+ clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_125M);
+ break;
+ case SPEED_100:
+ dev_info(gmac->dev, "Set TX clock to 25M\n");
+ clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_25M);
+ break;
+ case SPEED_10:
+ dev_info(gmac->dev, "Set TX clock to 2.5M\n");
+ clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_2M5);
+ break;
+ default:
+ dev_err(gmac->dev, "Unsupported/Invalid speed: %d\n", speed);
+ return;
+ }
+}
+
+static int s32cc_config_cache_coherency(struct platform_device *pdev,
+ struct plat_stmmacenet_data *plat_dat)
+{
+ plat_dat->axi4_ace_ctrl =
+ devm_kzalloc(&pdev->dev,
+ sizeof(struct stmmac_axi4_ace_ctrl),
+ GFP_KERNEL);
+
+ if (!plat_dat->axi4_ace_ctrl)
+ return -ENOMEM;
+
+ plat_dat->axi4_ace_ctrl->tx_ar_reg = (ACE_CONTROL_SIGNALS << 16)
+ | (ACE_CONTROL_SIGNALS << 8) | ACE_CONTROL_SIGNALS;
+
+ plat_dat->axi4_ace_ctrl->rx_aw_reg = (ACE_CONTROL_SIGNALS << 24)
+ | (ACE_CONTROL_SIGNALS << 16) | (ACE_CONTROL_SIGNALS << 8)
+ | ACE_CONTROL_SIGNALS;
+
+ plat_dat->axi4_ace_ctrl->txrx_awar_reg = (ACE_PROTECTION << 20)
+ | (ACE_PROTECTION << 16) | (ACE_CONTROL_SIGNALS << 8)
+ | ACE_CONTROL_SIGNALS;
+
+ return 0;
+}
+
+static int s32cc_dwmac_probe(struct platform_device *pdev)
+{
+ struct plat_stmmacenet_data *plat_dat;
+ struct stmmac_resources stmmac_res;
+ struct s32cc_priv_data *gmac;
+ struct resource *res;
+ const char *tx_clk, *rx_clk;
+ int ret;
+
+ ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (ret)
+ return ret;
+
+ gmac = devm_kzalloc(&pdev->dev, sizeof(*gmac), GFP_KERNEL);
+ if (!gmac)
+ return PTR_ERR(gmac);
+
+ gmac->dev = &pdev->dev;
+
+ /* S32G control reg */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ gmac->ctrl_sts = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR_OR_NULL(gmac->ctrl_sts)) {
+ dev_err(&pdev->dev, "S32CC config region is missing\n");
+ return PTR_ERR(gmac->ctrl_sts);
+ }
+
+ plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return PTR_ERR(plat_dat);
+
+ plat_dat->bsp_priv = gmac;
+
+ switch (plat_dat->phy_interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ tx_clk = "tx_pcs";
+ rx_clk = "rx_pcs";
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ tx_clk = "tx_rgmii";
+ rx_clk = "rx_rgmii";
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ tx_clk = "tx_rmii";
+ rx_clk = "rx_rmii";
+ break;
+ case PHY_INTERFACE_MODE_MII:
+ tx_clk = "tx_mii";
+ rx_clk = "rx_mii";
+ break;
+ default:
+ dev_err(&pdev->dev, "Not supported phy interface mode: [%s]\n",
+ phy_modes(plat_dat->phy_interface));
+ return -EINVAL;
+ };
+
+ gmac->intf_mode = plat_dat->phy_interface;
+
+ /* DMA cache coherency settings */
+ if (of_dma_is_coherent(pdev->dev.of_node)) {
+ ret = s32cc_config_cache_coherency(pdev, plat_dat);
+ if (ret)
+ goto err_remove_config_dt;
+ }
+
+ /* tx clock */
+ gmac->tx_clk = devm_clk_get(&pdev->dev, tx_clk);
+ if (IS_ERR(gmac->tx_clk)) {
+ dev_err(&pdev->dev, "Get TX clock failed\n");
+ ret = PTR_ERR(gmac->tx_clk);
+ goto err_remove_config_dt;
+ }
+
+ /* rx clock */
+ gmac->rx_clk = devm_clk_get(&pdev->dev, rx_clk);
+ if (IS_ERR(gmac->rx_clk)) {
+ dev_err(&pdev->dev, "Get RX clock failed\n");
+ ret = PTR_ERR(gmac->rx_clk);
+ goto err_remove_config_dt;
+ }
+
+ ret = s32cc_gmac_init(pdev, gmac);
+ if (ret)
+ goto err_remove_config_dt;
+
+ /* core feature set */
+ plat_dat->has_gmac4 = true;
+ plat_dat->pmt = 1;
+
+ plat_dat->init = s32cc_gmac_init;
+ plat_dat->exit = s32cc_gmac_exit;
+ plat_dat->fix_mac_speed = s32cc_fix_speed;
+
+ /* safety feature config */
+ plat_dat->safety_feat_cfg =
+ devm_kzalloc(&pdev->dev, sizeof(*plat_dat->safety_feat_cfg),
+ GFP_KERNEL);
+
+ if (!plat_dat->safety_feat_cfg) {
+ dev_err(&pdev->dev, "Allocate safety_feat_cfg failed\n");
+ goto err_gmac_exit;
+ }
+
+ plat_dat->safety_feat_cfg->tsoee = 1;
+ plat_dat->safety_feat_cfg->mrxpee = 1;
+ plat_dat->safety_feat_cfg->mestee = 1;
+ plat_dat->safety_feat_cfg->mrxee = 1;
+ plat_dat->safety_feat_cfg->mtxee = 1;
+ plat_dat->safety_feat_cfg->epsi = 1;
+ plat_dat->safety_feat_cfg->edpp = 1;
+ plat_dat->safety_feat_cfg->prtyen = 1;
+ plat_dat->safety_feat_cfg->tmouten = 1;
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret)
+ goto err_gmac_exit;
+
+ return 0;
+
+err_gmac_exit:
+ s32cc_gmac_exit(pdev, plat_dat->bsp_priv);
+err_remove_config_dt:
+ stmmac_remove_config_dt(pdev, plat_dat);
+ return ret;
+}
+
+static const struct of_device_id s32_dwmac_match[] = {
+ { .compatible = "nxp,s32cc-dwmac" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, s32_dwmac_match);
+
+static struct platform_driver s32_dwmac_driver = {
+ .probe = s32cc_dwmac_probe,
+ .remove = stmmac_pltfr_remove,
+ .driver = {
+ .name = "s32cc-dwmac",
+ .pm = &stmmac_pltfr_pm_ops,
+ .of_match_table = s32_dwmac_match,
+ },
+};
+module_platform_driver(s32_dwmac_driver);
+
+MODULE_AUTHOR("Jan Petrous <jan.petrous@nxp.com>");
+MODULE_DESCRIPTION("NXP S32 common chassis GMAC driver");
+MODULE_LICENSE("GPL v2");
--
2.37.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 5/5] net: stmmac: Add NXP S32 SoC family support
@ 2022-11-28 5:49 ` Chester Lin
0 siblings, 0 replies; 30+ messages in thread
From: Chester Lin @ 2022-11-28 5:49 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Andrew Lunn
Cc: Chester Lin, netdev, linux-kernel, linux-arm-kernel,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jan Petrous, Ondrej Spacek, Ghennadi Procopciuc,
Andra-Teodora Ilie, Andreas Färber, Matthias Brugger
Add GMAC support for NXP S32 SoC family. This driver is mainly based on
NXP's downstream implementation on CodeAurora[1].
[1] https://source.codeaurora.org/external/autobsps32/linux/tree/drivers/net/ethernet/stmicro/stmmac?h=bsp34.0-5.10.120-rt
Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
Signed-off-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
Changes in v2:
- Replace clock names tx_sgmii/rx_sgmii with tx_pcs/rx_pcs.
- Adjust error handlings while calling devm_clk_get().
- Remove redundant dev_info messages.
- Remove unnecessary if conditions.
- Fix the copyright format suggested by NXP.
drivers/net/ethernet/stmicro/stmmac/Kconfig | 13 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-s32cc.c | 304 ++++++++++++++++++
3 files changed, 318 insertions(+)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 31ff35174034..dd3fb5e462b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -153,6 +153,19 @@ config DWMAC_ROCKCHIP
This selects the Rockchip RK3288 SoC glue layer support for
the stmmac device driver.
+config DWMAC_S32CC
+ tristate "NXP S32 series GMAC support"
+ default ARCH_S32
+ depends on OF && (ARCH_S32 || COMPILE_TEST)
+ select MFD_SYSCON
+ select PHYLINK
+ help
+ Support for ethernet controller on NXP S32 series SOCs.
+
+ This selects NXP SoC glue layer support for the stmmac
+ device driver. This driver is used for the S32 series
+ SOCs GMAC ethernet controller.
+
config DWMAC_SOCFPGA
tristate "SOCFPGA dwmac support"
default ARCH_INTEL_SOCFPGA
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..ec92cc2becd7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o
obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o
obj-$(CONFIG_DWMAC_IMX8) += dwmac-imx.o
obj-$(CONFIG_DWMAC_VISCONTI) += dwmac-visconti.o
+obj-$(CONFIG_DWMAC_S32CC) += dwmac-s32cc.o
stmmac-platform-objs:= stmmac_platform.o
dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
new file mode 100644
index 000000000000..92a132ad985a
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DWMAC Specific Glue layer for NXP S32 Common Chassis
+ *
+ * Copyright 2019-2022 NXP
+ * Copyright (C) 2022 SUSE LLC
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/ethtool.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/of_address.h>
+#include <linux/stmmac.h>
+
+#include "stmmac_platform.h"
+
+#define GMAC_TX_RATE_125M 125000000 /* 125MHz */
+#define GMAC_TX_RATE_25M 25000000 /* 25MHz */
+#define GMAC_TX_RATE_2M5 2500000 /* 2.5MHz */
+
+/* S32 SRC register for phyif selection */
+#define PHY_INTF_SEL_MII 0x00
+#define PHY_INTF_SEL_SGMII 0x01
+#define PHY_INTF_SEL_RGMII 0x02
+#define PHY_INTF_SEL_RMII 0x08
+
+/* AXI4 ACE control settings */
+#define ACE_DOMAIN_SIGNAL 0x2
+#define ACE_CACHE_SIGNAL 0xf
+#define ACE_CONTROL_SIGNALS ((ACE_DOMAIN_SIGNAL << 4) | ACE_CACHE_SIGNAL)
+#define ACE_PROTECTION 0x2
+
+struct s32cc_priv_data {
+ void __iomem *ctrl_sts;
+ struct device *dev;
+ phy_interface_t intf_mode;
+ struct clk *tx_clk;
+ struct clk *rx_clk;
+};
+
+static int s32cc_gmac_init(struct platform_device *pdev, void *priv)
+{
+ struct s32cc_priv_data *gmac = priv;
+ u32 intf_sel;
+ int ret;
+
+ ret = clk_prepare_enable(gmac->tx_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Can't enable tx clock\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(gmac->rx_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Can't enable rx clock\n");
+ return ret;
+ }
+
+ /* set interface mode */
+ switch (gmac->intf_mode) {
+ case PHY_INTERFACE_MODE_SGMII:
+ intf_sel = PHY_INTF_SEL_SGMII;
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ intf_sel = PHY_INTF_SEL_RGMII;
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ intf_sel = PHY_INTF_SEL_RMII;
+ break;
+ case PHY_INTERFACE_MODE_MII:
+ intf_sel = PHY_INTF_SEL_MII;
+ break;
+ default:
+ dev_err(&pdev->dev, "Unsupported PHY interface: %s\n",
+ phy_modes(gmac->intf_mode));
+ return -EINVAL;
+ }
+
+ writel(intf_sel, gmac->ctrl_sts);
+
+ dev_dbg(&pdev->dev, "PHY mode set to %s\n", phy_modes(gmac->intf_mode));
+
+ return 0;
+}
+
+static void s32cc_gmac_exit(struct platform_device *pdev, void *priv)
+{
+ struct s32cc_priv_data *gmac = priv;
+
+ clk_disable_unprepare(gmac->tx_clk);
+ clk_disable_unprepare(gmac->rx_clk);
+}
+
+static void s32cc_fix_speed(void *priv, unsigned int speed)
+{
+ struct s32cc_priv_data *gmac = priv;
+
+ /* SGMII mode doesn't support the clock reconfiguration */
+ if (gmac->intf_mode == PHY_INTERFACE_MODE_SGMII)
+ return;
+
+ switch (speed) {
+ case SPEED_1000:
+ dev_info(gmac->dev, "Set TX clock to 125M\n");
+ clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_125M);
+ break;
+ case SPEED_100:
+ dev_info(gmac->dev, "Set TX clock to 25M\n");
+ clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_25M);
+ break;
+ case SPEED_10:
+ dev_info(gmac->dev, "Set TX clock to 2.5M\n");
+ clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_2M5);
+ break;
+ default:
+ dev_err(gmac->dev, "Unsupported/Invalid speed: %d\n", speed);
+ return;
+ }
+}
+
+static int s32cc_config_cache_coherency(struct platform_device *pdev,
+ struct plat_stmmacenet_data *plat_dat)
+{
+ plat_dat->axi4_ace_ctrl =
+ devm_kzalloc(&pdev->dev,
+ sizeof(struct stmmac_axi4_ace_ctrl),
+ GFP_KERNEL);
+
+ if (!plat_dat->axi4_ace_ctrl)
+ return -ENOMEM;
+
+ plat_dat->axi4_ace_ctrl->tx_ar_reg = (ACE_CONTROL_SIGNALS << 16)
+ | (ACE_CONTROL_SIGNALS << 8) | ACE_CONTROL_SIGNALS;
+
+ plat_dat->axi4_ace_ctrl->rx_aw_reg = (ACE_CONTROL_SIGNALS << 24)
+ | (ACE_CONTROL_SIGNALS << 16) | (ACE_CONTROL_SIGNALS << 8)
+ | ACE_CONTROL_SIGNALS;
+
+ plat_dat->axi4_ace_ctrl->txrx_awar_reg = (ACE_PROTECTION << 20)
+ | (ACE_PROTECTION << 16) | (ACE_CONTROL_SIGNALS << 8)
+ | ACE_CONTROL_SIGNALS;
+
+ return 0;
+}
+
+static int s32cc_dwmac_probe(struct platform_device *pdev)
+{
+ struct plat_stmmacenet_data *plat_dat;
+ struct stmmac_resources stmmac_res;
+ struct s32cc_priv_data *gmac;
+ struct resource *res;
+ const char *tx_clk, *rx_clk;
+ int ret;
+
+ ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (ret)
+ return ret;
+
+ gmac = devm_kzalloc(&pdev->dev, sizeof(*gmac), GFP_KERNEL);
+ if (!gmac)
+ return PTR_ERR(gmac);
+
+ gmac->dev = &pdev->dev;
+
+ /* S32G control reg */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ gmac->ctrl_sts = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR_OR_NULL(gmac->ctrl_sts)) {
+ dev_err(&pdev->dev, "S32CC config region is missing\n");
+ return PTR_ERR(gmac->ctrl_sts);
+ }
+
+ plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return PTR_ERR(plat_dat);
+
+ plat_dat->bsp_priv = gmac;
+
+ switch (plat_dat->phy_interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ tx_clk = "tx_pcs";
+ rx_clk = "rx_pcs";
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ tx_clk = "tx_rgmii";
+ rx_clk = "rx_rgmii";
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ tx_clk = "tx_rmii";
+ rx_clk = "rx_rmii";
+ break;
+ case PHY_INTERFACE_MODE_MII:
+ tx_clk = "tx_mii";
+ rx_clk = "rx_mii";
+ break;
+ default:
+ dev_err(&pdev->dev, "Not supported phy interface mode: [%s]\n",
+ phy_modes(plat_dat->phy_interface));
+ return -EINVAL;
+ };
+
+ gmac->intf_mode = plat_dat->phy_interface;
+
+ /* DMA cache coherency settings */
+ if (of_dma_is_coherent(pdev->dev.of_node)) {
+ ret = s32cc_config_cache_coherency(pdev, plat_dat);
+ if (ret)
+ goto err_remove_config_dt;
+ }
+
+ /* tx clock */
+ gmac->tx_clk = devm_clk_get(&pdev->dev, tx_clk);
+ if (IS_ERR(gmac->tx_clk)) {
+ dev_err(&pdev->dev, "Get TX clock failed\n");
+ ret = PTR_ERR(gmac->tx_clk);
+ goto err_remove_config_dt;
+ }
+
+ /* rx clock */
+ gmac->rx_clk = devm_clk_get(&pdev->dev, rx_clk);
+ if (IS_ERR(gmac->rx_clk)) {
+ dev_err(&pdev->dev, "Get RX clock failed\n");
+ ret = PTR_ERR(gmac->rx_clk);
+ goto err_remove_config_dt;
+ }
+
+ ret = s32cc_gmac_init(pdev, gmac);
+ if (ret)
+ goto err_remove_config_dt;
+
+ /* core feature set */
+ plat_dat->has_gmac4 = true;
+ plat_dat->pmt = 1;
+
+ plat_dat->init = s32cc_gmac_init;
+ plat_dat->exit = s32cc_gmac_exit;
+ plat_dat->fix_mac_speed = s32cc_fix_speed;
+
+ /* safety feature config */
+ plat_dat->safety_feat_cfg =
+ devm_kzalloc(&pdev->dev, sizeof(*plat_dat->safety_feat_cfg),
+ GFP_KERNEL);
+
+ if (!plat_dat->safety_feat_cfg) {
+ dev_err(&pdev->dev, "Allocate safety_feat_cfg failed\n");
+ goto err_gmac_exit;
+ }
+
+ plat_dat->safety_feat_cfg->tsoee = 1;
+ plat_dat->safety_feat_cfg->mrxpee = 1;
+ plat_dat->safety_feat_cfg->mestee = 1;
+ plat_dat->safety_feat_cfg->mrxee = 1;
+ plat_dat->safety_feat_cfg->mtxee = 1;
+ plat_dat->safety_feat_cfg->epsi = 1;
+ plat_dat->safety_feat_cfg->edpp = 1;
+ plat_dat->safety_feat_cfg->prtyen = 1;
+ plat_dat->safety_feat_cfg->tmouten = 1;
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret)
+ goto err_gmac_exit;
+
+ return 0;
+
+err_gmac_exit:
+ s32cc_gmac_exit(pdev, plat_dat->bsp_priv);
+err_remove_config_dt:
+ stmmac_remove_config_dt(pdev, plat_dat);
+ return ret;
+}
+
+static const struct of_device_id s32_dwmac_match[] = {
+ { .compatible = "nxp,s32cc-dwmac" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, s32_dwmac_match);
+
+static struct platform_driver s32_dwmac_driver = {
+ .probe = s32cc_dwmac_probe,
+ .remove = stmmac_pltfr_remove,
+ .driver = {
+ .name = "s32cc-dwmac",
+ .pm = &stmmac_pltfr_pm_ops,
+ .of_match_table = s32_dwmac_match,
+ },
+};
+module_platform_driver(s32_dwmac_driver);
+
+MODULE_AUTHOR("Jan Petrous <jan.petrous@nxp.com>");
+MODULE_DESCRIPTION("NXP S32 common chassis GMAC driver");
+MODULE_LICENSE("GPL v2");
--
2.37.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread