* [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC
@ 2023-08-27 9:17 Jisheng Zhang
2023-08-27 9:17 ` [PATCH net-next v2 1/3] dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties Jisheng Zhang
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Jisheng Zhang @ 2023-08-27 9:17 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime,
Coquelin, Simon Horman
Cc: netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel,
linux-riscv
Add the dwmac driver support for T-HEAD TH1520 SoC.
Since the clk part isn't mainlined, so SoC dts(i) changes are not
included in this series. However, it can be tested by using fixed-clock.
Since v1:
- rebase on the lastest net-next
- collect Reviewed-by tag
- address Krzysztof's comment of the dt binding
- fix "div is not initialised" issue pointed out by Simon
Jisheng Zhang (3):
dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties
dt-bindings: net: add T-HEAD dwmac support
net: stmmac: add glue layer for T-HEAD TH1520 SoC
.../devicetree/bindings/net/snps,dwmac.yaml | 2 +
.../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++
5 files changed, 393 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
--
2.40.1
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH net-next v2 1/3] dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties 2023-08-27 9:17 [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC Jisheng Zhang @ 2023-08-27 9:17 ` Jisheng Zhang 2023-08-27 9:17 ` [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support Jisheng Zhang ` (2 subsequent siblings) 3 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-27 9:17 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman Cc: netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv, Krzysztof Kozlowski snps dwmac 3.70a also supports setting pbl related properties, such as "snps,pbl", "snps,txpbl", "snps,rxpbl" and "snps,no-pbl-x8". Signed-off-by: Jisheng Zhang <jszhang@kernel.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Documentation/devicetree/bindings/net/snps,dwmac.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index ddf9522a5dc2..b196c5de2061 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -586,6 +586,7 @@ allOf: - qcom,sa8775p-ethqos - qcom,sc8280xp-ethqos - snps,dwmac-3.50a + - snps,dwmac-3.70a - snps,dwmac-4.10a - snps,dwmac-4.20a - snps,dwmac-5.20 -- 2.40.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-27 9:17 [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC Jisheng Zhang 2023-08-27 9:17 ` [PATCH net-next v2 1/3] dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties Jisheng Zhang @ 2023-08-27 9:17 ` Jisheng Zhang 2023-08-28 13:13 ` Serge Semin 2023-08-28 13:16 ` Serge Semin 2023-08-27 9:17 ` [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC Jisheng Zhang 2023-09-04 5:41 ` [PATCH net-next v2 0/3] add the dwmac driver " Jiexun Wang 3 siblings, 2 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-27 9:17 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman Cc: netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv Add documentation to describe T-HEAD dwmac. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- .../devicetree/bindings/net/snps,dwmac.yaml | 1 + .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index b196c5de2061..73821f86a609 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -96,6 +96,7 @@ properties: - snps,dwxgmac - snps,dwxgmac-2.10 - starfive,jh7110-dwmac + - thead,th1520-dwmac reg: minItems: 1 diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml new file mode 100644 index 000000000000..bf8ec8ca2753 --- /dev/null +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: T-HEAD DWMAC Ethernet controller + +maintainers: + - Jisheng Zhang <jszhang@kernel.org> + +select: + properties: + compatible: + contains: + enum: + - thead,th1520-dwmac + required: + - compatible + +properties: + compatible: + items: + - enum: + - thead,th1520-dwmac + - const: snps,dwmac-3.70a + + reg: + maxItems: 1 + + thead,gmacapb: + $ref: /schemas/types.yaml#/definitions/phandle + description: + The phandle to the syscon node that control ethernet + interface and timing delay. + +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + - interrupt-names + - phy-mode + - thead,gmacapb + +allOf: + - $ref: snps,dwmac.yaml# + +unevaluatedProperties: false + +examples: + - | + gmac0: ethernet@e7070000 { + compatible = "thead,th1520-dwmac", "snps,dwmac-3.70a"; + reg = <0xe7070000 0x2000>; + clocks = <&clk 1>, <&clk 2>; + clock-names = "stmmaceth", "pclk"; + interrupts = <66>; + interrupt-names = "macirq"; + phy-mode = "rgmii-id"; + snps,fixed-burst; + snps,axi-config = <&stmmac_axi_setup>; + snps,pbl = <32>; + thead,gmacapb = <&gmacapb_syscon>; + phy-handle = <&phy0>; + + mdio { + #address-cells = <1>; + #size-cells = <0>; + compatible = "snps,dwmac-mdio"; + + phy0: ethernet-phy@0 { + reg = <0>; + }; + }; + }; -- 2.40.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-27 9:17 ` [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support Jisheng Zhang @ 2023-08-28 13:13 ` Serge Semin 2023-08-28 15:17 ` Jisheng Zhang 2023-08-28 13:16 ` Serge Semin 1 sibling, 1 reply; 22+ messages in thread From: Serge Semin @ 2023-08-28 13:13 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > Add documentation to describe T-HEAD dwmac. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > 2 files changed, 78 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index b196c5de2061..73821f86a609 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -96,6 +96,7 @@ properties: > - snps,dwxgmac > - snps,dwxgmac-2.10 > - starfive,jh7110-dwmac > + - thead,th1520-dwmac > > reg: > minItems: 1 > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > new file mode 100644 > index 000000000000..bf8ec8ca2753 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml see further regarding using dwmac in the names here. > @@ -0,0 +1,77 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: T-HEAD DWMAC Ethernet controller Additionally would be nice to have a brief controller "description:" having the next info: the SoCs the controllers can be found on, the DW (G)MAC IP-core version the ethernet controller is based on and some data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC Management counters (MMC). In addition to that for DW QoS ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues and DMA channels, MTL queues capabilities (QoS-related), TSO availability, SPO availability. Note DMA FIFO sizes can be also constrained in the properties "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > + > +maintainers: > + - Jisheng Zhang <jszhang@kernel.org> > + > +select: > + properties: > + compatible: > + contains: > + enum: > + - thead,th1520-dwmac Referring to the DW IP-core in the compatible string isn't very much useful especially seeing you have a generic fallback compatible. Name like "thead,th1520-gmac" looks more informative indicating its speed capability. > + required: > + - compatible > + > +properties: > + compatible: > + items: > + - enum: > + - thead,th1520-dwmac ditto. -Serge(y) > + - const: snps,dwmac-3.70a > + > + reg: > + maxItems: 1 > + > + thead,gmacapb: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + The phandle to the syscon node that control ethernet > + interface and timing delay. > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - interrupts > + - interrupt-names > + - phy-mode > + - thead,gmacapb > + > +allOf: > + - $ref: snps,dwmac.yaml# > + > +unevaluatedProperties: false > + > +examples: > + - | > + gmac0: ethernet@e7070000 { > + compatible = "thead,th1520-dwmac", "snps,dwmac-3.70a"; > + reg = <0xe7070000 0x2000>; > + clocks = <&clk 1>, <&clk 2>; > + clock-names = "stmmaceth", "pclk"; > + interrupts = <66>; > + interrupt-names = "macirq"; > + phy-mode = "rgmii-id"; > + snps,fixed-burst; > + snps,axi-config = <&stmmac_axi_setup>; > + snps,pbl = <32>; > + thead,gmacapb = <&gmacapb_syscon>; > + phy-handle = <&phy0>; > + > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "snps,dwmac-mdio"; > + > + phy0: ethernet-phy@0 { > + reg = <0>; > + }; > + }; > + }; > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-28 13:13 ` Serge Semin @ 2023-08-28 15:17 ` Jisheng Zhang 2023-08-28 15:51 ` Serge Semin 0 siblings, 1 reply; 22+ messages in thread From: Jisheng Zhang @ 2023-08-28 15:17 UTC (permalink / raw) To: Serge Semin Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > > Add documentation to describe T-HEAD dwmac. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > > 2 files changed, 78 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > index b196c5de2061..73821f86a609 100644 > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > @@ -96,6 +96,7 @@ properties: > > - snps,dwxgmac > > - snps,dwxgmac-2.10 > > - starfive,jh7110-dwmac > > + - thead,th1520-dwmac > > > > reg: > > minItems: 1 > > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > new file mode 100644 > > index 000000000000..bf8ec8ca2753 > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > see further regarding using dwmac in the names here. > > > @@ -0,0 +1,77 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > > +title: T-HEAD DWMAC Ethernet controller > > Additionally would be nice to have a brief controller "description:" > having the next info: the SoCs the controllers can be found on, the DW > (G)MAC IP-core version the ethernet controller is based on and some > data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > Management counters (MMC). In addition to that for DW QoS > ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > and DMA channels, MTL queues capabilities (QoS-related), TSO > availability, SPO availability. > > Note DMA FIFO sizes can be also constrained in the properties > "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". Hi Serge, Thank you for your code review. I have different views here: If we only support the gmac controller in one specific SoC, these detailed information is nice to have, but what about if the driver/dt-binding supports the gmac controller in different SoCs? These detailed information will be outdated. what's more, I think the purpose of dt-binding is different from the one of documentation. So I prefer to put these GMAC IP related detailed information into the SoC's dtsi commit msg rather than polluting the dt-binding. > > > + > > +maintainers: > > + - Jisheng Zhang <jszhang@kernel.org> > > + > > +select: > > + properties: > > + compatible: > > + contains: > > + enum: > > > + - thead,th1520-dwmac > > Referring to the DW IP-core in the compatible string isn't very > much useful especially seeing you have a generic fallback compatible. > Name like "thead,th1520-gmac" looks more informative indicating its > speed capability. This is just to follow the common style as those dwmac-* does. I'm not sure which is better, but personally, I'd like to keep current common style. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-28 15:17 ` Jisheng Zhang @ 2023-08-28 15:51 ` Serge Semin 2023-08-28 16:06 ` Jisheng Zhang 2023-08-28 17:55 ` Krzysztof Kozlowski 0 siblings, 2 replies; 22+ messages in thread From: Serge Semin @ 2023-08-28 15:51 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: > On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > > On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > > > Add documentation to describe T-HEAD dwmac. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > > > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > > > 2 files changed, 78 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > index b196c5de2061..73821f86a609 100644 > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > @@ -96,6 +96,7 @@ properties: > > > - snps,dwxgmac > > > - snps,dwxgmac-2.10 > > > - starfive,jh7110-dwmac > > > + - thead,th1520-dwmac > > > > > > reg: > > > minItems: 1 > > > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > new file mode 100644 > > > index 000000000000..bf8ec8ca2753 > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > see further regarding using dwmac in the names here. > > > > > @@ -0,0 +1,77 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > > > +title: T-HEAD DWMAC Ethernet controller > > > > Additionally would be nice to have a brief controller "description:" > > having the next info: the SoCs the controllers can be found on, the DW > > (G)MAC IP-core version the ethernet controller is based on and some > > data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > > FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > > availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > > engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > > 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > > Management counters (MMC). In addition to that for DW QoS > > ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > > and DMA channels, MTL queues capabilities (QoS-related), TSO > > availability, SPO availability. > > > > Note DMA FIFO sizes can be also constrained in the properties > > "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > > in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". BTW plus to this you may wish to add the "rx-internal-delay-ps" and "tx-internal-delay-ps" properties constraints seeing they device supports internal Tx/Rx delays. > > Hi Serge, > > Thank you for your code review. I have different views here: If we > only support the gmac controller in one specific SoC, these detailed > information is nice to have, but what about if the driver/dt-binding > supports the gmac controller in different SoCs? These detailed > information will be outdated. First they won't. Second then you can either add more info to the description for instance in a separate paragraph or create a dedicated DT-bindings. Such information would be very much useful for the generic STMMAC driver code maintenance. > > what's more, I think the purpose of dt-binding is different from > the one of documentation. The purpose of the DT-bindings is a hardware "description". The info I listed describes your hardware. > > So I prefer to put these GMAC IP related detailed information into > the SoC's dtsi commit msg rather than polluting the dt-binding. > > > > > + > > > +maintainers: > > > + - Jisheng Zhang <jszhang@kernel.org> > > > + > > > +select: > > > + properties: > > > + compatible: > > > + contains: > > > + enum: > > > > > + - thead,th1520-dwmac > > > > Referring to the DW IP-core in the compatible string isn't very > > much useful especially seeing you have a generic fallback compatible. > > Name like "thead,th1520-gmac" looks more informative indicating its > > speed capability. > > This is just to follow the common style as those dwmac-* does. > I'm not sure which is better, but personally, I'd like to keep current > common style. It's not that common. Half the compatible strings use the notation suggested by me and it has more sense then a dwmac suffix. It's ok to use the suffix in the STMMAC driver-related things because the glue code is supposed to work with the DW *MAC generic code. Using it in the compatible string especially together with the generic fallback compatible just useless. -Serge(y) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-28 15:51 ` Serge Semin @ 2023-08-28 16:06 ` Jisheng Zhang 2023-08-28 17:55 ` Krzysztof Kozlowski 1 sibling, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-28 16:06 UTC (permalink / raw) To: Serge Semin Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 06:51:49PM +0300, Serge Semin wrote: > On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: > > On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > > > On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > > > > Add documentation to describe T-HEAD dwmac. > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > > --- > > > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > > > > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > > > > 2 files changed, 78 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > index b196c5de2061..73821f86a609 100644 > > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > @@ -96,6 +96,7 @@ properties: > > > > - snps,dwxgmac > > > > - snps,dwxgmac-2.10 > > > > - starfive,jh7110-dwmac > > > > + - thead,th1520-dwmac > > > > > > > > reg: > > > > minItems: 1 > > > > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > new file mode 100644 > > > > index 000000000000..bf8ec8ca2753 > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > > > see further regarding using dwmac in the names here. > > > > > > > @@ -0,0 +1,77 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > > > > +title: T-HEAD DWMAC Ethernet controller > > > > > > Additionally would be nice to have a brief controller "description:" > > > having the next info: the SoCs the controllers can be found on, the DW > > > (G)MAC IP-core version the ethernet controller is based on and some > > > data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > > > FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > > > availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > > > engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > > > 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > > > Management counters (MMC). In addition to that for DW QoS > > > ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > > > and DMA channels, MTL queues capabilities (QoS-related), TSO > > > availability, SPO availability. > > > > > > > Note DMA FIFO sizes can be also constrained in the properties > > > "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > > > in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > > BTW plus to this you may wish to add the "rx-internal-delay-ps" and > "tx-internal-delay-ps" properties constraints seeing they device > supports internal Tx/Rx delays. > > > > > Hi Serge, > > > > > Thank you for your code review. I have different views here: If we > > only support the gmac controller in one specific SoC, these detailed > > information is nice to have, but what about if the driver/dt-binding > > supports the gmac controller in different SoCs? These detailed > > information will be outdated. > > First they won't. Second then you can either add more info to the > description for instance in a separate paragraph or create a dedicated > DT-bindings. Such information would be very much useful for the > generic STMMAC driver code maintenance. > > > > > what's more, I think the purpose of dt-binding is different from > > the one of documentation. > > The purpose of the DT-bindings is a hardware "description". The info I > listed describes your hardware. dt-binding VS. dts(i), they are different things. Part of what you listed belong dts(i), that's the reason why I prefer to put those into dts(i) commit msg. The HW description is in dts(i) itself rather than dt-binding. Anyway I will add generic decriptions to the dt-binding. > > > > > So I prefer to put these GMAC IP related detailed information into > > the SoC's dtsi commit msg rather than polluting the dt-binding. > > > > > > > + > > > > +maintainers: > > > > + - Jisheng Zhang <jszhang@kernel.org> > > > > + > > > > +select: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > > > > + - thead,th1520-dwmac > > > > > > Referring to the DW IP-core in the compatible string isn't very > > > much useful especially seeing you have a generic fallback compatible. > > > Name like "thead,th1520-gmac" looks more informative indicating its > > > speed capability. > > > > > This is just to follow the common style as those dwmac-* does. > > I'm not sure which is better, but personally, I'd like to keep current > > common style. > > It's not that common. Half the compatible strings use the notation > suggested by me and it has more sense then a dwmac suffix. It's ok to > use the suffix in the STMMAC driver-related things because the glue > code is supposed to work with the DW *MAC generic code. Using it in > the compatible string especially together with the generic fallback > compatible just useless. > > -Serge(y) > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-28 15:51 ` Serge Semin 2023-08-28 16:06 ` Jisheng Zhang @ 2023-08-28 17:55 ` Krzysztof Kozlowski 2023-08-29 3:02 ` Jisheng Zhang 1 sibling, 1 reply; 22+ messages in thread From: Krzysztof Kozlowski @ 2023-08-28 17:55 UTC (permalink / raw) To: Serge Semin, Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On 28/08/2023 17:51, Serge Semin wrote: > On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: >> On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: >>> On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: >>>> Add documentation to describe T-HEAD dwmac. >>>> >>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >>>> --- >>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >>>> .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ >>>> 2 files changed, 78 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> index b196c5de2061..73821f86a609 100644 >>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> @@ -96,6 +96,7 @@ properties: >>>> - snps,dwxgmac >>>> - snps,dwxgmac-2.10 >>>> - starfive,jh7110-dwmac >>>> + - thead,th1520-dwmac >>>> >>>> reg: >>>> minItems: 1 >>>> diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml >>>> new file mode 100644 >>>> index 000000000000..bf8ec8ca2753 >>>> --- /dev/null >>> >>>> +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml >>> >>> see further regarding using dwmac in the names here. >>> >>>> @@ -0,0 +1,77 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>> >>>> +title: T-HEAD DWMAC Ethernet controller >>> >>> Additionally would be nice to have a brief controller "description:" >>> having the next info: the SoCs the controllers can be found on, the DW >>> (G)MAC IP-core version the ethernet controller is based on and some >>> data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA >>> FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters >>> availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload >>> engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE >>> 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC >>> Management counters (MMC). In addition to that for DW QoS >>> ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues >>> and DMA channels, MTL queues capabilities (QoS-related), TSO >>> availability, SPO availability. >>> > >>> Note DMA FIFO sizes can be also constrained in the properties >>> "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - >>> in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > > BTW plus to this you may wish to add the "rx-internal-delay-ps" and > "tx-internal-delay-ps" properties constraints seeing they device > supports internal Tx/Rx delays. > >> >> Hi Serge, >> > >> Thank you for your code review. I have different views here: If we >> only support the gmac controller in one specific SoC, these detailed >> information is nice to have, but what about if the driver/dt-binding >> supports the gmac controller in different SoCs? These detailed >> information will be outdated. > > First they won't. Second then you can either add more info to the > description for instance in a separate paragraph or create a dedicated > DT-bindings. Such information would be very much useful for the > generic STMMAC driver code maintenance. > >> >> what's more, I think the purpose of dt-binding is different from >> the one of documentation. > > The purpose of the DT-bindings is a hardware "description". The info I > listed describes your hardware. > >> >> So I prefer to put these GMAC IP related detailed information into >> the SoC's dtsi commit msg rather than polluting the dt-binding. >>> >>>> + >>>> +maintainers: >>>> + - Jisheng Zhang <jszhang@kernel.org> >>>> + >>>> +select: >>>> + properties: >>>> + compatible: >>>> + contains: >>>> + enum: >>> >>>> + - thead,th1520-dwmac >>> >>> Referring to the DW IP-core in the compatible string isn't very >>> much useful especially seeing you have a generic fallback compatible. >>> Name like "thead,th1520-gmac" looks more informative indicating its >>> speed capability. >> > >> This is just to follow the common style as those dwmac-* does. >> I'm not sure which is better, but personally, I'd like to keep current >> common style. > > It's not that common. Half the compatible strings use the notation > suggested by me and it has more sense then a dwmac suffix. It's ok to > use the suffix in the STMMAC driver-related things because the glue > code is supposed to work with the DW *MAC generic code. Using it in > the compatible string especially together with the generic fallback > compatible just useless. THEAD did not make dwmac here, but a gmac. dwmac does not exist in the context of Thead and Th1520, so the naming suggested by Serge makes sense. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-28 17:55 ` Krzysztof Kozlowski @ 2023-08-29 3:02 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-29 3:02 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Serge Semin, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 07:55:44PM +0200, Krzysztof Kozlowski wrote: > On 28/08/2023 17:51, Serge Semin wrote: > > On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: > >> On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > >>> On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > >>>> Add documentation to describe T-HEAD dwmac. > >>>> > >>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > >>>> --- > >>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > >>>> .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > >>>> 2 files changed, 78 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > >>>> index b196c5de2061..73821f86a609 100644 > >>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > >>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > >>>> @@ -96,6 +96,7 @@ properties: > >>>> - snps,dwxgmac > >>>> - snps,dwxgmac-2.10 > >>>> - starfive,jh7110-dwmac > >>>> + - thead,th1520-dwmac > >>>> > >>>> reg: > >>>> minItems: 1 > >>>> diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > >>>> new file mode 100644 > >>>> index 000000000000..bf8ec8ca2753 > >>>> --- /dev/null > >>> > >>>> +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > >>> > >>> see further regarding using dwmac in the names here. > >>> > >>>> @@ -0,0 +1,77 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>> > >>>> +title: T-HEAD DWMAC Ethernet controller > >>> > >>> Additionally would be nice to have a brief controller "description:" > >>> having the next info: the SoCs the controllers can be found on, the DW > >>> (G)MAC IP-core version the ethernet controller is based on and some > >>> data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > >>> FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > >>> availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > >>> engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > >>> 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > >>> Management counters (MMC). In addition to that for DW QoS > >>> ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > >>> and DMA channels, MTL queues capabilities (QoS-related), TSO > >>> availability, SPO availability. > >>> > > > >>> Note DMA FIFO sizes can be also constrained in the properties > >>> "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > >>> in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > > > > BTW plus to this you may wish to add the "rx-internal-delay-ps" and > > "tx-internal-delay-ps" properties constraints seeing they device > > supports internal Tx/Rx delays. > > > >> > >> Hi Serge, > >> > > > >> Thank you for your code review. I have different views here: If we > >> only support the gmac controller in one specific SoC, these detailed > >> information is nice to have, but what about if the driver/dt-binding > >> supports the gmac controller in different SoCs? These detailed > >> information will be outdated. > > > > First they won't. Second then you can either add more info to the > > description for instance in a separate paragraph or create a dedicated > > DT-bindings. Such information would be very much useful for the > > generic STMMAC driver code maintenance. > > > >> > >> what's more, I think the purpose of dt-binding is different from > >> the one of documentation. > > > > The purpose of the DT-bindings is a hardware "description". The info I > > listed describes your hardware. > > > >> > >> So I prefer to put these GMAC IP related detailed information into > >> the SoC's dtsi commit msg rather than polluting the dt-binding. > >>> > >>>> + > >>>> +maintainers: > >>>> + - Jisheng Zhang <jszhang@kernel.org> > >>>> + > >>>> +select: > >>>> + properties: > >>>> + compatible: > >>>> + contains: > >>>> + enum: > >>> > >>>> + - thead,th1520-dwmac > >>> > >>> Referring to the DW IP-core in the compatible string isn't very > >>> much useful especially seeing you have a generic fallback compatible. > >>> Name like "thead,th1520-gmac" looks more informative indicating its > >>> speed capability. > >> > > > >> This is just to follow the common style as those dwmac-* does. > >> I'm not sure which is better, but personally, I'd like to keep current > >> common style. > > > > It's not that common. Half the compatible strings use the notation > > suggested by me and it has more sense then a dwmac suffix. It's ok to > > use the suffix in the STMMAC driver-related things because the glue > > code is supposed to work with the DW *MAC generic code. Using it in > > the compatible string especially together with the generic fallback > > compatible just useless. > > THEAD did not make dwmac here, but a gmac. dwmac does not exist in the > context of Thead and Th1520, so the naming suggested by Serge makes sense. > I have no preference. But just want to confirm: the th1520 ethernet controller doesn't always function as GMAC, but can act as MII, so "thead,th1520-gmac" is still OK? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support 2023-08-27 9:17 ` [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support Jisheng Zhang 2023-08-28 13:13 ` Serge Semin @ 2023-08-28 13:16 ` Serge Semin 1 sibling, 0 replies; 22+ messages in thread From: Serge Semin @ 2023-08-28 13:16 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > Add documentation to describe T-HEAD dwmac. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > 2 files changed, 78 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index b196c5de2061..73821f86a609 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -96,6 +96,7 @@ properties: > - snps,dwxgmac > - snps,dwxgmac-2.10 > - starfive,jh7110-dwmac > + - thead,th1520-dwmac > > reg: > minItems: 1 > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > new file mode 100644 > index 000000000000..bf8ec8ca2753 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > @@ -0,0 +1,77 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: T-HEAD DWMAC Ethernet controller > + > +maintainers: > + - Jisheng Zhang <jszhang@kernel.org> > + > +select: > + properties: > + compatible: > + contains: > + enum: > + - thead,th1520-dwmac > + required: > + - compatible > + > +properties: > + compatible: > + items: > + - enum: > + - thead,th1520-dwmac > + - const: snps,dwmac-3.70a > + > + reg: > + maxItems: 1 > + > + thead,gmacapb: BTW what is a point in having the "apb" prefix in the name? The property name like "thead,gmac-syscon" looks much more suitable since it refers to the actual property content. -Serge(y) > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + The phandle to the syscon node that control ethernet > + interface and timing delay. > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - interrupts > + - interrupt-names > + - phy-mode > + - thead,gmacapb > + > +allOf: > + - $ref: snps,dwmac.yaml# > + > +unevaluatedProperties: false > + > +examples: > + - | > + gmac0: ethernet@e7070000 { > + compatible = "thead,th1520-dwmac", "snps,dwmac-3.70a"; > + reg = <0xe7070000 0x2000>; > + clocks = <&clk 1>, <&clk 2>; > + clock-names = "stmmaceth", "pclk"; > + interrupts = <66>; > + interrupt-names = "macirq"; > + phy-mode = "rgmii-id"; > + snps,fixed-burst; > + snps,axi-config = <&stmmac_axi_setup>; > + snps,pbl = <32>; > + thead,gmacapb = <&gmacapb_syscon>; > + phy-handle = <&phy0>; > + > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "snps,dwmac-mdio"; > + > + phy0: ethernet-phy@0 { > + reg = <0>; > + }; > + }; > + }; > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-27 9:17 [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC Jisheng Zhang 2023-08-27 9:17 ` [PATCH net-next v2 1/3] dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties Jisheng Zhang 2023-08-27 9:17 ` [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support Jisheng Zhang @ 2023-08-27 9:17 ` Jisheng Zhang 2023-08-28 13:40 ` Serge Semin 2023-08-28 15:58 ` Serge Semin 2023-09-04 5:41 ` [PATCH net-next v2 0/3] add the dwmac driver " Jiexun Wang 3 siblings, 2 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-27 9:17 UTC (permalink / raw) To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman Cc: netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ 3 files changed, 314 insertions(+) create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig index 06c6871f8788..1bf71804c270 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig @@ -216,6 +216,17 @@ config DWMAC_SUN8I stmmac device driver. This driver is used for H3/A83T/A64 EMAC ethernet controller. +config DWMAC_THEAD + tristate "T-HEAD dwmac support" + depends on OF && (ARCH_THEAD || COMPILE_TEST) + select MFD_SYSCON + help + Support for ethernet controllers on T-HEAD RISC-V SoCs + + This selects the T-HEAD platform specific glue layer support for + the stmmac device driver. This driver is used for T-HEAD TH1520 + ethernet controller. + config DWMAC_IMX8 tristate "NXP IMX8 DWMAC support" default ARCH_MXC diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile index 5b57aee19267..d73171ed6ad7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Makefile +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c new file mode 100644 index 000000000000..85135ef05906 --- /dev/null +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c @@ -0,0 +1,302 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * T-HEAD DWMAC platform driver + * + * Copyright (C) 2021 Alibaba Group Holding Limited. + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> + * + */ + +#include <linux/bitfield.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_net.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +#include "stmmac_platform.h" + +#define GMAC_CLK_EN 0x00 +#define GMAC_TX_CLK_EN BIT(1) +#define GMAC_TX_CLK_N_EN BIT(2) +#define GMAC_TX_CLK_OUT_EN BIT(3) +#define GMAC_RX_CLK_EN BIT(4) +#define GMAC_RX_CLK_N_EN BIT(5) +#define GMAC_EPHY_REF_CLK_EN BIT(6) +#define GMAC_RXCLK_DELAY_CTRL 0x04 +#define GMAC_RXCLK_BYPASS BIT(15) +#define GMAC_RXCLK_INVERT BIT(14) +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) +#define GMAC_TXCLK_DELAY_CTRL 0x08 +#define GMAC_TXCLK_BYPASS BIT(15) +#define GMAC_TXCLK_INVERT BIT(14) +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) +#define GMAC_PLLCLK_DIV 0x0c +#define GMAC_PLLCLK_DIV_EN BIT(31) +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) +#define GMAC_GTXCLK_SEL 0x18 +#define GMAC_GTXCLK_SEL_PLL BIT(0) +#define GMAC_INTF_CTRL 0x1c +#define PHY_INTF_MASK BIT(0) +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) +#define GMAC_TXCLK_OEN 0x20 +#define TXCLK_DIR_MASK BIT(0) +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) + +#define GMAC_GMII_RGMII_RATE 125000000 +#define GMAC_MII_RATE 25000000 + +struct thead_dwmac { + struct plat_stmmacenet_data *plat; + struct regmap *apb_regmap; + struct device *dev; + u32 rx_delay; + u32 tx_delay; +}; + +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) +{ + struct thead_dwmac *dwmac = plat->bsp_priv; + u32 phyif; + + switch (plat->interface) { + case PHY_INTERFACE_MODE_MII: + phyif = PHY_INTF_MII_GMII; + break; + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_TXID: + case PHY_INTERFACE_MODE_RGMII_RXID: + phyif = PHY_INTF_RGMII; + break; + default: + dev_err(dwmac->dev, "unsupported phy interface %d\n", + plat->interface); + return -EINVAL; + }; + + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); + + return 0; +} + +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) +{ + struct thead_dwmac *dwmac = plat->bsp_priv; + u32 txclk_dir; + + switch (plat->interface) { + case PHY_INTERFACE_MODE_MII: + txclk_dir = TXCLK_DIR_INPUT; + break; + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_TXID: + case PHY_INTERFACE_MODE_RGMII_RXID: + txclk_dir = TXCLK_DIR_OUTPUT; + break; + default: + dev_err(dwmac->dev, "unsupported phy interface %d\n", + plat->interface); + return -EINVAL; + }; + + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); + + return 0; +} + +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) +{ + struct thead_dwmac *dwmac = priv; + struct plat_stmmacenet_data *plat = dwmac->plat; + unsigned long rate; + u32 div; + + switch (plat->interface) { + /* For MII, rxc/txc is provided by phy */ + case PHY_INTERFACE_MODE_MII: + return; + + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_RXID: + case PHY_INTERFACE_MODE_RGMII_TXID: + rate = clk_get_rate(plat->stmmac_clk); + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || + rate % GMAC_MII_RATE != 0) { + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); + return; + } + + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); + + switch (speed) { + case SPEED_1000: + div = rate / GMAC_GMII_RGMII_RATE; + break; + case SPEED_100: + div = rate / GMAC_MII_RATE; + break; + case SPEED_10: + div = rate * 10 / GMAC_MII_RATE; + break; + default: + dev_err(dwmac->dev, "invalid speed %u\n", speed); + return; + } + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); + + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); + break; + default: + dev_err(dwmac->dev, "unsupported phy interface %d\n", + plat->interface); + return; + } +} + +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) +{ + struct thead_dwmac *dwmac = plat->bsp_priv; + u32 reg; + + switch (plat->interface) { + case PHY_INTERFACE_MODE_MII: + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; + break; + + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_RXID: + case PHY_INTERFACE_MODE_RGMII_TXID: + /* use pll */ + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); + + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; + break; + + default: + dev_err(dwmac->dev, "unsupported phy interface %d\n", + plat->interface); + return -EINVAL; + } + + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); + + return 0; +} + +static int thead_dwmac_init(struct platform_device *pdev, + struct plat_stmmacenet_data *plat) +{ + struct thead_dwmac *dwmac = plat->bsp_priv; + int ret; + + ret = thead_dwmac_set_phy_if(plat); + if (ret) + return ret; + + ret = thead_dwmac_set_txclk_dir(plat); + if (ret) + return ret; + + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); + + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); + + return thead_dwmac_enable_clk(plat); +} + +static int thead_dwmac_probe(struct platform_device *pdev) +{ + struct plat_stmmacenet_data *plat; + struct stmmac_resources stmmac_res; + struct thead_dwmac *dwmac; + struct device_node *np = pdev->dev.of_node; + u32 delay_ps; + int ret; + + ret = stmmac_get_platform_resources(pdev, &stmmac_res); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "failed to get resources\n"); + + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); + if (IS_ERR(plat)) + return dev_err_probe(&pdev->dev, PTR_ERR(plat), + "dt configuration failed\n"); + + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); + if (!dwmac) { + ret = -ENOMEM; + goto err_remove_config_dt; + } + + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) + dwmac->rx_delay = delay_ps; + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) + dwmac->tx_delay = delay_ps; + + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); + if (IS_ERR(dwmac->apb_regmap)) { + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), + "Failed to get gmac apb syscon\n"); + goto err_remove_config_dt; + } + + dwmac->dev = &pdev->dev; + dwmac->plat = plat; + plat->bsp_priv = dwmac; + plat->fix_mac_speed = thead_dwmac_fix_speed; + + ret = thead_dwmac_init(pdev, plat); + if (ret) + goto err_remove_config_dt; + + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); + if (ret) + goto err_remove_config_dt; + + return 0; + +err_remove_config_dt: + stmmac_remove_config_dt(pdev, plat); + + return ret; +} + +static const struct of_device_id thead_dwmac_match[] = { + { .compatible = "thead,th1520-dwmac" }, + { } +}; +MODULE_DEVICE_TABLE(of, thead_dwmac_match); + +static struct platform_driver thead_dwmac_driver = { + .probe = thead_dwmac_probe, + .remove_new = stmmac_pltfr_remove, + .driver = { + .name = "thead-dwmac", + .pm = &stmmac_pltfr_pm_ops, + .of_match_table = thead_dwmac_match, + }, +}; +module_platform_driver(thead_dwmac_driver); + +MODULE_AUTHOR("T-HEAD"); +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); +MODULE_LICENSE("GPL"); -- 2.40.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-27 9:17 ` [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC Jisheng Zhang @ 2023-08-28 13:40 ` Serge Semin 2023-08-28 15:41 ` Jisheng Zhang 2023-08-28 15:58 ` Serge Semin 1 sibling, 1 reply; 22+ messages in thread From: Serge Semin @ 2023-08-28 13:40 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > 3 files changed, 314 insertions(+) > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > index 06c6871f8788..1bf71804c270 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > stmmac device driver. This driver is used for H3/A83T/A64 > EMAC ethernet controller. > > +config DWMAC_THEAD > + tristate "T-HEAD dwmac support" > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > + select MFD_SYSCON > + help > + Support for ethernet controllers on T-HEAD RISC-V SoCs > + > + This selects the T-HEAD platform specific glue layer support for > + the stmmac device driver. This driver is used for T-HEAD TH1520 > + ethernet controller. > + > config DWMAC_IMX8 > tristate "NXP IMX8 DWMAC support" > default ARCH_MXC > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > index 5b57aee19267..d73171ed6ad7 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > new file mode 100644 > index 000000000000..85135ef05906 > --- /dev/null > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > @@ -0,0 +1,302 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * T-HEAD DWMAC platform driver > + * > + * Copyright (C) 2021 Alibaba Group Holding Limited. > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + * > + */ > + > +#include <linux/bitfield.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_net.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include "stmmac_platform.h" > + > +#define GMAC_CLK_EN 0x00 > +#define GMAC_TX_CLK_EN BIT(1) > +#define GMAC_TX_CLK_N_EN BIT(2) > +#define GMAC_TX_CLK_OUT_EN BIT(3) > +#define GMAC_RX_CLK_EN BIT(4) > +#define GMAC_RX_CLK_N_EN BIT(5) > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > +#define GMAC_RXCLK_BYPASS BIT(15) > +#define GMAC_RXCLK_INVERT BIT(14) > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > +#define GMAC_TXCLK_BYPASS BIT(15) > +#define GMAC_TXCLK_INVERT BIT(14) > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > +#define GMAC_PLLCLK_DIV 0x0c > +#define GMAC_PLLCLK_DIV_EN BIT(31) > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > +#define GMAC_GTXCLK_SEL 0x18 > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > +#define GMAC_INTF_CTRL 0x1c > +#define PHY_INTF_MASK BIT(0) > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > +#define GMAC_TXCLK_OEN 0x20 > +#define TXCLK_DIR_MASK BIT(0) > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > + > +#define GMAC_GMII_RGMII_RATE 125000000 > +#define GMAC_MII_RATE 25000000 > + > +struct thead_dwmac { > + struct plat_stmmacenet_data *plat; > + struct regmap *apb_regmap; > + struct device *dev; > + u32 rx_delay; > + u32 tx_delay; > +}; > + > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + u32 phyif; > + > + switch (plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + phyif = PHY_INTF_MII_GMII; > + break; > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + phyif = PHY_INTF_RGMII; > + break; > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return -EINVAL; > + }; > + > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > + > + return 0; > +} > + > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + u32 txclk_dir; > + > + switch (plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + txclk_dir = TXCLK_DIR_INPUT; > + break; > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + txclk_dir = TXCLK_DIR_OUTPUT; > + break; > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return -EINVAL; > + }; > + > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > + > + return 0; > +} > + > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > +{ > + struct thead_dwmac *dwmac = priv; > + struct plat_stmmacenet_data *plat = dwmac->plat; > + unsigned long rate; > + u32 div; > + > + switch (plat->interface) { > + /* For MII, rxc/txc is provided by phy */ > + case PHY_INTERFACE_MODE_MII: > + return; > + > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + rate = clk_get_rate(plat->stmmac_clk); > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > + rate % GMAC_MII_RATE != 0) { > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > + return; > + } > + > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > + > + switch (speed) { > + case SPEED_1000: > + div = rate / GMAC_GMII_RGMII_RATE; > + break; > + case SPEED_100: > + div = rate / GMAC_MII_RATE; > + break; > + case SPEED_10: > + div = rate * 10 / GMAC_MII_RATE; > + break; > + default: > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > + return; > + } > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > + > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); This chunk looks like a hard-coded implementation of the CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the "stmmaceth" clock. I suggest to move it to the respective driver, add a "tx" clock to the bindings and use the common clock kernel API methods here only. > + break; > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return; > + } > +} > + > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + u32 reg; > + > + switch (plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > + break; > + > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + /* use pll */ > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > + > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; Similarly settings these flags looks like just clock gates which can also be moved to the clock driver. > + break; > + > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return -EINVAL; > + } > + > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > + > + return 0; > +} > + > +static int thead_dwmac_init(struct platform_device *pdev, > + struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + int ret; > + > + ret = thead_dwmac_set_phy_if(plat); > + if (ret) > + return ret; > + > + ret = thead_dwmac_set_txclk_dir(plat); > + if (ret) > + return ret; > + > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > + > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > + > + return thead_dwmac_enable_clk(plat); > +} > + > +static int thead_dwmac_probe(struct platform_device *pdev) > +{ > + struct plat_stmmacenet_data *plat; > + struct stmmac_resources stmmac_res; > + struct thead_dwmac *dwmac; > + struct device_node *np = pdev->dev.of_node; > + u32 delay_ps; > + int ret; > + > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to get resources\n"); > + > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); This can be replaced with devm_stmmac_probe_config_dt() so the stmmac_remove_config_dt() invocation would be dropped. > + if (IS_ERR(plat)) > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > + "dt configuration failed\n"); > + > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > + if (!dwmac) { > + ret = -ENOMEM; > + goto err_remove_config_dt; > + } > + > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > + dwmac->rx_delay = delay_ps; > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > + dwmac->tx_delay = delay_ps; > + > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > + if (IS_ERR(dwmac->apb_regmap)) { > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > + "Failed to get gmac apb syscon\n"); > + goto err_remove_config_dt; > + } > + > + dwmac->dev = &pdev->dev; > + dwmac->plat = plat; > + plat->bsp_priv = dwmac; > + plat->fix_mac_speed = thead_dwmac_fix_speed; > + > + ret = thead_dwmac_init(pdev, plat); > + if (ret) > + goto err_remove_config_dt; > + > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); This can be replaced with: plat->init = thead_dwmac_init; ret = devm_stmmac_pltfr_probe(); > + if (ret) > + goto err_remove_config_dt; > + > + return 0; > + > +err_remove_config_dt: > + stmmac_remove_config_dt(pdev, plat); > + > + return ret; This can be dropped if devm_stmmac_probe_config_dt() is utilized. > +} > + > +static const struct of_device_id thead_dwmac_match[] = { > + { .compatible = "thead,th1520-dwmac" }, See my comment to the bindings about the compatible string suffix. > + { } > +}; > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > + > +static struct platform_driver thead_dwmac_driver = { > + .probe = thead_dwmac_probe, > + .remove_new = stmmac_pltfr_remove, This can be dropped if devm_stmmac_probe_config_dt() and devm_stmmac_pltfr_probe() are utilized. > + .driver = { > + .name = "thead-dwmac", > + .pm = &stmmac_pltfr_pm_ops, > + .of_match_table = thead_dwmac_match, > + }, > +}; > +module_platform_driver(thead_dwmac_driver); > + > +MODULE_AUTHOR("T-HEAD"); > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); ^ | Capitalize? ------------------+ -Serge(y) > +MODULE_LICENSE("GPL"); > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-28 13:40 ` Serge Semin @ 2023-08-28 15:41 ` Jisheng Zhang 2023-08-28 16:56 ` Emil Renner Berthing 2023-08-28 17:30 ` Serge Semin 0 siblings, 2 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-28 15:41 UTC (permalink / raw) To: Serge Semin Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 04:40:19PM +0300, Serge Semin wrote: > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > 3 files changed, 314 insertions(+) > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > index 06c6871f8788..1bf71804c270 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > stmmac device driver. This driver is used for H3/A83T/A64 > > EMAC ethernet controller. > > > > +config DWMAC_THEAD > > + tristate "T-HEAD dwmac support" > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > + select MFD_SYSCON > > + help > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > + > > + This selects the T-HEAD platform specific glue layer support for > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > + ethernet controller. > > + > > config DWMAC_IMX8 > > tristate "NXP IMX8 DWMAC support" > > default ARCH_MXC > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > index 5b57aee19267..d73171ed6ad7 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > new file mode 100644 > > index 000000000000..85135ef05906 > > --- /dev/null > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > @@ -0,0 +1,302 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * T-HEAD DWMAC platform driver > > + * > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > + * > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/of_net.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > + > > +#include "stmmac_platform.h" > > + > > +#define GMAC_CLK_EN 0x00 > > +#define GMAC_TX_CLK_EN BIT(1) > > +#define GMAC_TX_CLK_N_EN BIT(2) > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > +#define GMAC_RX_CLK_EN BIT(4) > > +#define GMAC_RX_CLK_N_EN BIT(5) > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > +#define GMAC_RXCLK_BYPASS BIT(15) > > +#define GMAC_RXCLK_INVERT BIT(14) > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > +#define GMAC_TXCLK_BYPASS BIT(15) > > +#define GMAC_TXCLK_INVERT BIT(14) > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > +#define GMAC_PLLCLK_DIV 0x0c > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > +#define GMAC_GTXCLK_SEL 0x18 > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > +#define GMAC_INTF_CTRL 0x1c > > +#define PHY_INTF_MASK BIT(0) > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > +#define GMAC_TXCLK_OEN 0x20 > > +#define TXCLK_DIR_MASK BIT(0) > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > + > > +#define GMAC_GMII_RGMII_RATE 125000000 > > +#define GMAC_MII_RATE 25000000 > > + > > +struct thead_dwmac { > > + struct plat_stmmacenet_data *plat; > > + struct regmap *apb_regmap; > > + struct device *dev; > > + u32 rx_delay; > > + u32 tx_delay; > > +}; > > + > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + u32 phyif; > > + > > + switch (plat->interface) { > > + case PHY_INTERFACE_MODE_MII: > > + phyif = PHY_INTF_MII_GMII; > > + break; > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + phyif = PHY_INTF_RGMII; > > + break; > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return -EINVAL; > > + }; > > + > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > + > > + return 0; > > +} > > + > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + u32 txclk_dir; > > + > > + switch (plat->interface) { > > + case PHY_INTERFACE_MODE_MII: > > + txclk_dir = TXCLK_DIR_INPUT; > > + break; > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + txclk_dir = TXCLK_DIR_OUTPUT; > > + break; > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return -EINVAL; > > + }; > > + > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > + > > + return 0; > > +} > > + > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > +{ > > + struct thead_dwmac *dwmac = priv; > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > + unsigned long rate; > > + u32 div; > > + > > + switch (plat->interface) { > > + /* For MII, rxc/txc is provided by phy */ > > + case PHY_INTERFACE_MODE_MII: > > + return; > > + > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + rate = clk_get_rate(plat->stmmac_clk); > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > + rate % GMAC_MII_RATE != 0) { > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > + return; > > + } > > + > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > + > > + switch (speed) { > > + case SPEED_1000: > > + div = rate / GMAC_GMII_RGMII_RATE; > > + break; > > + case SPEED_100: > > + div = rate / GMAC_MII_RATE; > > + break; > > + case SPEED_10: > > + div = rate * 10 / GMAC_MII_RATE; > > + break; > > + default: > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > + return; > > + } > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > + > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > This chunk looks like a hard-coded implementation of the > CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the > "stmmaceth" clock. I suggest to move it to the respective driver, add > a "tx" clock to the bindings and use the common clock kernel API > methods here only. I did consider your solution before writing the code, here are the reasons why I dropped it: There's no any clk IP here, the HW just puts several gmac related control bits here, such as rx/tx delay, bypass, invert interface choice, clk direction. From this point of view, it looks more like a syscon rather than clk. Secondly, I see other SoCs did similar for this case, such as dwmac-visconti, dwmac-meson8b, dwmac-ipq806x, dwmac-socfpga and so on. They met similar issue as the above. PS: here is the initial th1520 clk driver, as is seen, the clk IP is different from the control logic here. https://lore.kernel.org/linux-riscv/20230515054402.27633-1-frank.li@vivo.com/ Thanks > > > + break; > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return; > > + } > > +} > > + > > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + u32 reg; > > + > > + switch (plat->interface) { > > + case PHY_INTERFACE_MODE_MII: > > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > > + break; > > + > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + /* use pll */ > > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > > + > > > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; > > Similarly settings these flags looks like just clock gates which can > also be moved to the clock driver. > > > + break; > > + > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return -EINVAL; > > + } > > + > > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > > + > > + return 0; > > +} > > + > > +static int thead_dwmac_init(struct platform_device *pdev, > > + struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + int ret; > > + > > + ret = thead_dwmac_set_phy_if(plat); > > + if (ret) > > + return ret; > > + > > + ret = thead_dwmac_set_txclk_dir(plat); > > + if (ret) > > + return ret; > > + > > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > > + > > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > > + > > + return thead_dwmac_enable_clk(plat); > > +} > > + > > +static int thead_dwmac_probe(struct platform_device *pdev) > > +{ > > + struct plat_stmmacenet_data *plat; > > + struct stmmac_resources stmmac_res; > > + struct thead_dwmac *dwmac; > > + struct device_node *np = pdev->dev.of_node; > > + u32 delay_ps; > > + int ret; > > + > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, > > + "failed to get resources\n"); > > + > > > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); > > This can be replaced with devm_stmmac_probe_config_dt() so the > stmmac_remove_config_dt() invocation would be dropped. > > > + if (IS_ERR(plat)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > > + "dt configuration failed\n"); > > + > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > > + if (!dwmac) { > > + ret = -ENOMEM; > > + goto err_remove_config_dt; > > + } > > + > > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > > + dwmac->rx_delay = delay_ps; > > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > > + dwmac->tx_delay = delay_ps; > > + > > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > > + if (IS_ERR(dwmac->apb_regmap)) { > > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > > + "Failed to get gmac apb syscon\n"); > > + goto err_remove_config_dt; > > + } > > + > > + dwmac->dev = &pdev->dev; > > + dwmac->plat = plat; > > + plat->bsp_priv = dwmac; > > + plat->fix_mac_speed = thead_dwmac_fix_speed; > > + > > > + ret = thead_dwmac_init(pdev, plat); > > + if (ret) > > + goto err_remove_config_dt; > > + > > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); > > This can be replaced with: > plat->init = thead_dwmac_init; > ret = devm_stmmac_pltfr_probe(); > > > + if (ret) > > + goto err_remove_config_dt; > > + > > + return 0; > > + > > > +err_remove_config_dt: > > + stmmac_remove_config_dt(pdev, plat); > > + > > + return ret; > > This can be dropped if devm_stmmac_probe_config_dt() is utilized. > > > +} > > + > > +static const struct of_device_id thead_dwmac_match[] = { > > > + { .compatible = "thead,th1520-dwmac" }, > > See my comment to the bindings about the compatible string suffix. > > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > > + > > +static struct platform_driver thead_dwmac_driver = { > > + .probe = thead_dwmac_probe, > > > + .remove_new = stmmac_pltfr_remove, > > This can be dropped if devm_stmmac_probe_config_dt() and > devm_stmmac_pltfr_probe() are utilized. > > > + .driver = { > > + .name = "thead-dwmac", > > + .pm = &stmmac_pltfr_pm_ops, > > + .of_match_table = thead_dwmac_match, > > + }, > > +}; > > +module_platform_driver(thead_dwmac_driver); > > + > > +MODULE_AUTHOR("T-HEAD"); > > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > > > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); > ^ > | > Capitalize? ------------------+ > > -Serge(y) > > > +MODULE_LICENSE("GPL"); > > -- > > 2.40.1 > > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-28 15:41 ` Jisheng Zhang @ 2023-08-28 16:56 ` Emil Renner Berthing 2023-08-28 17:30 ` Serge Semin 1 sibling, 0 replies; 22+ messages in thread From: Emil Renner Berthing @ 2023-08-28 16:56 UTC (permalink / raw) To: Jisheng Zhang Cc: Serge Semin, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, 28 Aug 2023 at 17:55, Jisheng Zhang <jszhang@kernel.org> wrote: > On Mon, Aug 28, 2023 at 04:40:19PM +0300, Serge Semin wrote: > > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > > 3 files changed, 314 insertions(+) > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > index 06c6871f8788..1bf71804c270 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > > stmmac device driver. This driver is used for H3/A83T/A64 > > > EMAC ethernet controller. > > > > > > +config DWMAC_THEAD > > > + tristate "T-HEAD dwmac support" > > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > > + select MFD_SYSCON > > > + help > > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > > + > > > + This selects the T-HEAD platform specific glue layer support for > > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > > + ethernet controller. > > > + > > > config DWMAC_IMX8 > > > tristate "NXP IMX8 DWMAC support" > > > default ARCH_MXC > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > index 5b57aee19267..d73171ed6ad7 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > new file mode 100644 > > > index 000000000000..85135ef05906 > > > --- /dev/null > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > @@ -0,0 +1,302 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * T-HEAD DWMAC platform driver > > > + * > > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > + * > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/mfd/syscon.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > +#include <linux/of_net.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/regmap.h> > > > + > > > +#include "stmmac_platform.h" > > > + > > > +#define GMAC_CLK_EN 0x00 > > > +#define GMAC_TX_CLK_EN BIT(1) > > > +#define GMAC_TX_CLK_N_EN BIT(2) > > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > > +#define GMAC_RX_CLK_EN BIT(4) > > > +#define GMAC_RX_CLK_N_EN BIT(5) > > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > > +#define GMAC_RXCLK_BYPASS BIT(15) > > > +#define GMAC_RXCLK_INVERT BIT(14) > > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > > +#define GMAC_TXCLK_BYPASS BIT(15) > > > +#define GMAC_TXCLK_INVERT BIT(14) > > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > +#define GMAC_PLLCLK_DIV 0x0c > > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > > +#define GMAC_GTXCLK_SEL 0x18 > > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > > +#define GMAC_INTF_CTRL 0x1c > > > +#define PHY_INTF_MASK BIT(0) > > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > > +#define GMAC_TXCLK_OEN 0x20 > > > +#define TXCLK_DIR_MASK BIT(0) > > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > > + > > > +#define GMAC_GMII_RGMII_RATE 125000000 > > > +#define GMAC_MII_RATE 25000000 > > > + > > > +struct thead_dwmac { > > > + struct plat_stmmacenet_data *plat; > > > + struct regmap *apb_regmap; > > > + struct device *dev; > > > + u32 rx_delay; > > > + u32 tx_delay; > > > +}; > > > + > > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 phyif; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + phyif = PHY_INTF_MII_GMII; > > > + break; > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + phyif = PHY_INTF_RGMII; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + }; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > > + > > > + return 0; > > > +} > > > + > > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 txclk_dir; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + txclk_dir = TXCLK_DIR_INPUT; > > > + break; > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + txclk_dir = TXCLK_DIR_OUTPUT; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + }; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > > + > > > + return 0; > > > +} > > > + > > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > > +{ > > > + struct thead_dwmac *dwmac = priv; > > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > > + unsigned long rate; > > > + u32 div; > > > + > > > + switch (plat->interface) { > > > + /* For MII, rxc/txc is provided by phy */ > > > + case PHY_INTERFACE_MODE_MII: > > > + return; > > > + > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > + rate = clk_get_rate(plat->stmmac_clk); > > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > > + rate % GMAC_MII_RATE != 0) { > > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > > + return; > > > + } > > > + > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > > + > > > + switch (speed) { > > > + case SPEED_1000: > > > + div = rate / GMAC_GMII_RGMII_RATE; > > > + break; > > > + case SPEED_100: > > > + div = rate / GMAC_MII_RATE; > > > + break; > > > + case SPEED_10: > > > + div = rate * 10 / GMAC_MII_RATE; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > > + return; > > > + } > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > > + > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > > > This chunk looks like a hard-coded implementation of the > > CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the > > "stmmaceth" clock. I suggest to move it to the respective driver, add > > a "tx" clock to the bindings and use the common clock kernel API > > methods here only. > > I did consider your solution before writing the code, here are the > reasons why I dropped it: > > There's no any clk IP here, the HW just puts several > gmac related control bits here, such as rx/tx delay, bypass, invert > interface choice, clk direction. From this point of view, it looks more > like a syscon rather than clk. Hi Jisheng. That's not really a compelling argument to do the clock manipulation here. Look at the StarFive JH7110 PLL clock driver. It's exactly a clock driver for just the registers controlling the PLLs among the other syscon registers. > Secondly, I see other SoCs did similar for this case, such as > dwmac-visconti, dwmac-meson8b, dwmac-ipq806x, dwmac-socfpga and so on. > They met similar issue as the above. > > PS: here is the initial th1520 clk driver, as is seen, the clk IP is > different from the control logic here. > > https://lore.kernel.org/linux-riscv/20230515054402.27633-1-frank.li@vivo.com/ > > Thanks > > > > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return; > > > + } > > > +} > > > + > > > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 reg; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > > > + break; > > > + > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + /* use pll */ > > > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > > > + > > > > > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > > > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; > > > > Similarly settings these flags looks like just clock gates which can > > also be moved to the clock driver. > > > > > + break; > > > + > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + } > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > > > + > > > + return 0; > > > +} > > > + > > > +static int thead_dwmac_init(struct platform_device *pdev, > > > + struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + int ret; > > > + > > > + ret = thead_dwmac_set_phy_if(plat); > > > + if (ret) > > > + return ret; > > > + > > > + ret = thead_dwmac_set_txclk_dir(plat); > > > + if (ret) > > > + return ret; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > > > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > > > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > > > + > > > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > > > + > > > + return thead_dwmac_enable_clk(plat); > > > +} > > > + > > > +static int thead_dwmac_probe(struct platform_device *pdev) > > > +{ > > > + struct plat_stmmacenet_data *plat; > > > + struct stmmac_resources stmmac_res; > > > + struct thead_dwmac *dwmac; > > > + struct device_node *np = pdev->dev.of_node; > > > + u32 delay_ps; > > > + int ret; > > > + > > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > > > + if (ret) > > > + return dev_err_probe(&pdev->dev, ret, > > > + "failed to get resources\n"); > > > + > > > > > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); > > > > This can be replaced with devm_stmmac_probe_config_dt() so the > > stmmac_remove_config_dt() invocation would be dropped. > > > > > + if (IS_ERR(plat)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > > > + "dt configuration failed\n"); > > > + > > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > > > + if (!dwmac) { > > > + ret = -ENOMEM; > > > + goto err_remove_config_dt; > > > + } > > > + > > > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > > > + dwmac->rx_delay = delay_ps; > > > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > > > + dwmac->tx_delay = delay_ps; > > > + > > > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > > > + if (IS_ERR(dwmac->apb_regmap)) { > > > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > > > + "Failed to get gmac apb syscon\n"); > > > + goto err_remove_config_dt; > > > + } > > > + > > > + dwmac->dev = &pdev->dev; > > > + dwmac->plat = plat; > > > + plat->bsp_priv = dwmac; > > > + plat->fix_mac_speed = thead_dwmac_fix_speed; > > > + > > > > > + ret = thead_dwmac_init(pdev, plat); > > > + if (ret) > > > + goto err_remove_config_dt; > > > + > > > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); > > > > This can be replaced with: > > plat->init = thead_dwmac_init; > > ret = devm_stmmac_pltfr_probe(); > > > > > + if (ret) > > > + goto err_remove_config_dt; > > > + > > > + return 0; > > > + > > > > > +err_remove_config_dt: > > > + stmmac_remove_config_dt(pdev, plat); > > > + > > > + return ret; > > > > This can be dropped if devm_stmmac_probe_config_dt() is utilized. > > > > > +} > > > + > > > +static const struct of_device_id thead_dwmac_match[] = { > > > > > + { .compatible = "thead,th1520-dwmac" }, > > > > See my comment to the bindings about the compatible string suffix. > > > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > > > + > > > +static struct platform_driver thead_dwmac_driver = { > > > + .probe = thead_dwmac_probe, > > > > > + .remove_new = stmmac_pltfr_remove, > > > > This can be dropped if devm_stmmac_probe_config_dt() and > > devm_stmmac_pltfr_probe() are utilized. > > > > > + .driver = { > > > + .name = "thead-dwmac", > > > + .pm = &stmmac_pltfr_pm_ops, > > > + .of_match_table = thead_dwmac_match, > > > + }, > > > +}; > > > +module_platform_driver(thead_dwmac_driver); > > > + > > > +MODULE_AUTHOR("T-HEAD"); > > > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > > > > > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); > > ^ > > | > > Capitalize? ------------------+ > > > > -Serge(y) > > > > > +MODULE_LICENSE("GPL"); > > > -- > > > 2.40.1 > > > > > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-28 15:41 ` Jisheng Zhang 2023-08-28 16:56 ` Emil Renner Berthing @ 2023-08-28 17:30 ` Serge Semin 2023-08-29 3:17 ` Jisheng Zhang 1 sibling, 1 reply; 22+ messages in thread From: Serge Semin @ 2023-08-28 17:30 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 11:41:47PM +0800, Jisheng Zhang wrote: > On Mon, Aug 28, 2023 at 04:40:19PM +0300, Serge Semin wrote: > > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > > 3 files changed, 314 insertions(+) > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > index 06c6871f8788..1bf71804c270 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > > stmmac device driver. This driver is used for H3/A83T/A64 > > > EMAC ethernet controller. > > > > > > +config DWMAC_THEAD > > > + tristate "T-HEAD dwmac support" > > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > > + select MFD_SYSCON > > > + help > > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > > + > > > + This selects the T-HEAD platform specific glue layer support for > > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > > + ethernet controller. > > > + > > > config DWMAC_IMX8 > > > tristate "NXP IMX8 DWMAC support" > > > default ARCH_MXC > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > index 5b57aee19267..d73171ed6ad7 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > new file mode 100644 > > > index 000000000000..85135ef05906 > > > --- /dev/null > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > @@ -0,0 +1,302 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * T-HEAD DWMAC platform driver > > > + * > > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > + * > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/mfd/syscon.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > +#include <linux/of_net.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/regmap.h> > > > + > > > +#include "stmmac_platform.h" > > > + > > > +#define GMAC_CLK_EN 0x00 > > > +#define GMAC_TX_CLK_EN BIT(1) > > > +#define GMAC_TX_CLK_N_EN BIT(2) > > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > > +#define GMAC_RX_CLK_EN BIT(4) > > > +#define GMAC_RX_CLK_N_EN BIT(5) > > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > > +#define GMAC_RXCLK_BYPASS BIT(15) > > > +#define GMAC_RXCLK_INVERT BIT(14) > > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > > +#define GMAC_TXCLK_BYPASS BIT(15) > > > +#define GMAC_TXCLK_INVERT BIT(14) > > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > +#define GMAC_PLLCLK_DIV 0x0c > > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > > +#define GMAC_GTXCLK_SEL 0x18 > > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > > +#define GMAC_INTF_CTRL 0x1c > > > +#define PHY_INTF_MASK BIT(0) > > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > > +#define GMAC_TXCLK_OEN 0x20 > > > +#define TXCLK_DIR_MASK BIT(0) > > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > > + > > > +#define GMAC_GMII_RGMII_RATE 125000000 > > > +#define GMAC_MII_RATE 25000000 > > > + > > > +struct thead_dwmac { > > > + struct plat_stmmacenet_data *plat; > > > + struct regmap *apb_regmap; > > > + struct device *dev; > > > + u32 rx_delay; > > > + u32 tx_delay; > > > +}; > > > + > > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 phyif; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + phyif = PHY_INTF_MII_GMII; > > > + break; > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + phyif = PHY_INTF_RGMII; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + }; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > > + > > > + return 0; > > > +} > > > + > > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 txclk_dir; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + txclk_dir = TXCLK_DIR_INPUT; > > > + break; > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + txclk_dir = TXCLK_DIR_OUTPUT; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + }; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > > + > > > + return 0; > > > +} > > > + > > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > > +{ > > > + struct thead_dwmac *dwmac = priv; > > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > > + unsigned long rate; > > > + u32 div; > > > + > > > + switch (plat->interface) { > > > + /* For MII, rxc/txc is provided by phy */ > > > + case PHY_INTERFACE_MODE_MII: > > > + return; > > > + > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > + rate = clk_get_rate(plat->stmmac_clk); > > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > > + rate % GMAC_MII_RATE != 0) { > > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > > + return; > > > + } > > > + > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > > + > > > + switch (speed) { > > > + case SPEED_1000: > > > + div = rate / GMAC_GMII_RGMII_RATE; > > > + break; > > > + case SPEED_100: > > > + div = rate / GMAC_MII_RATE; > > > + break; > > > + case SPEED_10: > > > + div = rate * 10 / GMAC_MII_RATE; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > > + return; > > > + } > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > > + > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > > > This chunk looks like a hard-coded implementation of the > > CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the > > "stmmaceth" clock. I suggest to move it to the respective driver, add > > a "tx" clock to the bindings and use the common clock kernel API > > methods here only. > > I did consider your solution before writing the code, here are the > reasons why I dropped it: > > There's no any clk IP here, the HW just puts several > gmac related control bits here, such as rx/tx delay, bypass, invert > interface choice, clk direction. You omitted the essential part of your code which I pointed out. > From this point of view, it looks more > like a syscon rather than clk. Toggling control bits is surely the syscon work. But gating a parental clock, settings up the parental clock _divider_ and ungating the clock back is the clock controller function. So it means your syscon is just a normal multi-function device, which one of the function is the clock controller. It's not like your situation is unique. For instance in case of a SoC I was working with recently Clock Control Unit (CCU) was actually a multi-function device which had: 1. PLLs and Dividers supplying the clocks to the SoC components. 2. SoC components reset controller. 3. I2C-interface controller. 4. AXI-bus errors report registers. 5. PCIe-controller tunings (LTSSM, link up/down, etc) 6. SATA-controller tunings. 7. Full SoC reset controller (syscon reboot), 8. L2-cache tunings controller. with the sub-functions CSRs joint in a single space. In that case the PCIe-controller tunings and a lot of its reference clocks settings were intermixed in a single chunk of the registers. So I had to create a driver for the clocks anyway including all the PCIe reference clock and refer to the syscon in the PCIe-controller device node for the respective PCIe platform-specific tunings. > > Secondly, I see other SoCs did similar for this case, such as > dwmac-visconti, dwmac-meson8b, dwmac-ipq806x, dwmac-socfpga and so on. > They met similar issue as the above. First I failed to find any clock-related things in the dwmac-socfpga driver looking in anyway as yours. Second the dwmac-meson8b driver creates a generic clock handler right in the driver. I don't think it's a great solution but at the very least it registers the clock handler in the kernel. But seeing the PROG_ETHERNET CSR is of 8 bytes long there (0xc8834540 0x8) and defined at looking random base address it's definitely a part of a Meson system controller which just directly passed to the device driver. It's not correct. That part should have been at least specified as a syscon too. Third the dwmac-visconti driver is not a good example seeing it defines some specific registers way away from the NIC CSR space. It's most likely a separate device like syscon. Fourth dwmac-ipq806x driver implementation looks indeed like yours. In anyway I don't say your solution is fully wrong. At the very least you have a syscon node defined. But it just makes you adding incomplete device/platform bindings. Your network device do have the Tx reference clock as a part of the separate system controller, but you have to omit it because of the syscon property. You do have a syscon node, but don't have its clock function exported. So AFAICS in your case things can be implemented in a more canonical way than they are now. -Serge(y) > > PS: here is the initial th1520 clk driver, as is seen, the clk IP is > different from the control logic here. > > https://lore.kernel.org/linux-riscv/20230515054402.27633-1-frank.li@vivo.com/ > > Thanks > > > > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return; > > > + } > > > +} > > > + > > > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 reg; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > > > + break; > > > + > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + /* use pll */ > > > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > > > + > > > > > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > > > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; > > > > Similarly settings these flags looks like just clock gates which can > > also be moved to the clock driver. > > > > > + break; > > > + > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + } > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > > > + > > > + return 0; > > > +} > > > + > > > +static int thead_dwmac_init(struct platform_device *pdev, > > > + struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + int ret; > > > + > > > + ret = thead_dwmac_set_phy_if(plat); > > > + if (ret) > > > + return ret; > > > + > > > + ret = thead_dwmac_set_txclk_dir(plat); > > > + if (ret) > > > + return ret; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > > > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > > > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > > > + > > > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > > > + > > > + return thead_dwmac_enable_clk(plat); > > > +} > > > + > > > +static int thead_dwmac_probe(struct platform_device *pdev) > > > +{ > > > + struct plat_stmmacenet_data *plat; > > > + struct stmmac_resources stmmac_res; > > > + struct thead_dwmac *dwmac; > > > + struct device_node *np = pdev->dev.of_node; > > > + u32 delay_ps; > > > + int ret; > > > + > > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > > > + if (ret) > > > + return dev_err_probe(&pdev->dev, ret, > > > + "failed to get resources\n"); > > > + > > > > > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); > > > > This can be replaced with devm_stmmac_probe_config_dt() so the > > stmmac_remove_config_dt() invocation would be dropped. > > > > > + if (IS_ERR(plat)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > > > + "dt configuration failed\n"); > > > + > > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > > > + if (!dwmac) { > > > + ret = -ENOMEM; > > > + goto err_remove_config_dt; > > > + } > > > + > > > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > > > + dwmac->rx_delay = delay_ps; > > > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > > > + dwmac->tx_delay = delay_ps; > > > + > > > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > > > + if (IS_ERR(dwmac->apb_regmap)) { > > > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > > > + "Failed to get gmac apb syscon\n"); > > > + goto err_remove_config_dt; > > > + } > > > + > > > + dwmac->dev = &pdev->dev; > > > + dwmac->plat = plat; > > > + plat->bsp_priv = dwmac; > > > + plat->fix_mac_speed = thead_dwmac_fix_speed; > > > + > > > > > + ret = thead_dwmac_init(pdev, plat); > > > + if (ret) > > > + goto err_remove_config_dt; > > > + > > > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); > > > > This can be replaced with: > > plat->init = thead_dwmac_init; > > ret = devm_stmmac_pltfr_probe(); > > > > > + if (ret) > > > + goto err_remove_config_dt; > > > + > > > + return 0; > > > + > > > > > +err_remove_config_dt: > > > + stmmac_remove_config_dt(pdev, plat); > > > + > > > + return ret; > > > > This can be dropped if devm_stmmac_probe_config_dt() is utilized. > > > > > +} > > > + > > > +static const struct of_device_id thead_dwmac_match[] = { > > > > > + { .compatible = "thead,th1520-dwmac" }, > > > > See my comment to the bindings about the compatible string suffix. > > > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > > > + > > > +static struct platform_driver thead_dwmac_driver = { > > > + .probe = thead_dwmac_probe, > > > > > + .remove_new = stmmac_pltfr_remove, > > > > This can be dropped if devm_stmmac_probe_config_dt() and > > devm_stmmac_pltfr_probe() are utilized. > > > > > + .driver = { > > > + .name = "thead-dwmac", > > > + .pm = &stmmac_pltfr_pm_ops, > > > + .of_match_table = thead_dwmac_match, > > > + }, > > > +}; > > > +module_platform_driver(thead_dwmac_driver); > > > + > > > +MODULE_AUTHOR("T-HEAD"); > > > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > > > > > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); > > ^ > > | > > Capitalize? ------------------+ > > > > -Serge(y) > > > > > +MODULE_LICENSE("GPL"); > > > -- > > > 2.40.1 > > > > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-28 17:30 ` Serge Semin @ 2023-08-29 3:17 ` Jisheng Zhang 2023-08-29 10:07 ` Serge Semin 0 siblings, 1 reply; 22+ messages in thread From: Jisheng Zhang @ 2023-08-29 3:17 UTC (permalink / raw) To: Serge Semin, Emil Renner Berthing Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 08:30:50PM +0300, Serge Semin wrote: > On Mon, Aug 28, 2023 at 11:41:47PM +0800, Jisheng Zhang wrote: > > On Mon, Aug 28, 2023 at 04:40:19PM +0300, Serge Semin wrote: > > > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > > --- > > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > > > 3 files changed, 314 insertions(+) > > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > index 06c6871f8788..1bf71804c270 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > > > stmmac device driver. This driver is used for H3/A83T/A64 > > > > EMAC ethernet controller. > > > > > > > > +config DWMAC_THEAD > > > > + tristate "T-HEAD dwmac support" > > > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > > > + select MFD_SYSCON > > > > + help > > > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > > > + > > > > + This selects the T-HEAD platform specific glue layer support for > > > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > > > + ethernet controller. > > > > + > > > > config DWMAC_IMX8 > > > > tristate "NXP IMX8 DWMAC support" > > > > default ARCH_MXC > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > index 5b57aee19267..d73171ed6ad7 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > new file mode 100644 > > > > index 000000000000..85135ef05906 > > > > --- /dev/null > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > @@ -0,0 +1,302 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * T-HEAD DWMAC platform driver > > > > + * > > > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > > + * > > > > + */ > > > > + > > > > +#include <linux/bitfield.h> > > > > +#include <linux/mfd/syscon.h> > > > > +#include <linux/module.h> > > > > +#include <linux/of.h> > > > > +#include <linux/of_device.h> > > > > +#include <linux/of_net.h> > > > > +#include <linux/platform_device.h> > > > > +#include <linux/regmap.h> > > > > + > > > > +#include "stmmac_platform.h" > > > > + > > > > +#define GMAC_CLK_EN 0x00 > > > > +#define GMAC_TX_CLK_EN BIT(1) > > > > +#define GMAC_TX_CLK_N_EN BIT(2) > > > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > > > +#define GMAC_RX_CLK_EN BIT(4) > > > > +#define GMAC_RX_CLK_N_EN BIT(5) > > > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > > > +#define GMAC_RXCLK_BYPASS BIT(15) > > > > +#define GMAC_RXCLK_INVERT BIT(14) > > > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > > > +#define GMAC_TXCLK_BYPASS BIT(15) > > > > +#define GMAC_TXCLK_INVERT BIT(14) > > > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > > +#define GMAC_PLLCLK_DIV 0x0c > > > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > > > +#define GMAC_GTXCLK_SEL 0x18 > > > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > > > +#define GMAC_INTF_CTRL 0x1c > > > > +#define PHY_INTF_MASK BIT(0) > > > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > > > +#define GMAC_TXCLK_OEN 0x20 > > > > +#define TXCLK_DIR_MASK BIT(0) > > > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > > > + > > > > +#define GMAC_GMII_RGMII_RATE 125000000 > > > > +#define GMAC_MII_RATE 25000000 > > > > + > > > > +struct thead_dwmac { > > > > + struct plat_stmmacenet_data *plat; > > > > + struct regmap *apb_regmap; > > > > + struct device *dev; > > > > + u32 rx_delay; > > > > + u32 tx_delay; > > > > +}; > > > > + > > > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > > > +{ > > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > > + u32 phyif; > > > > + > > > > + switch (plat->interface) { > > > > + case PHY_INTERFACE_MODE_MII: > > > > + phyif = PHY_INTF_MII_GMII; > > > > + break; > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > + phyif = PHY_INTF_RGMII; > > > > + break; > > > > + default: > > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > > + plat->interface); > > > > + return -EINVAL; > > > > + }; > > > > + > > > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > > > +{ > > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > > + u32 txclk_dir; > > > > + > > > > + switch (plat->interface) { > > > > + case PHY_INTERFACE_MODE_MII: > > > > + txclk_dir = TXCLK_DIR_INPUT; > > > > + break; > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > + txclk_dir = TXCLK_DIR_OUTPUT; > > > > + break; > > > > + default: > > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > > + plat->interface); > > > > + return -EINVAL; > > > > + }; > > > > + > > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > > > +{ > > > > + struct thead_dwmac *dwmac = priv; > > > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > > > + unsigned long rate; > > > > + u32 div; > > > > + > > > > + switch (plat->interface) { > > > > + /* For MII, rxc/txc is provided by phy */ > > > > + case PHY_INTERFACE_MODE_MII: > > > > + return; > > > > + > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > > > + rate = clk_get_rate(plat->stmmac_clk); > > > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > > > + rate % GMAC_MII_RATE != 0) { > > > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > > > + return; > > > > + } > > > > + > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > > > + > > > > + switch (speed) { > > > > + case SPEED_1000: > > > > + div = rate / GMAC_GMII_RGMII_RATE; > > > > + break; > > > > + case SPEED_100: > > > > + div = rate / GMAC_MII_RATE; > > > > + break; > > > > + case SPEED_10: > > > > + div = rate * 10 / GMAC_MII_RATE; > > > > + break; > > > > + default: > > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > > > + return; > > > > + } > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > > > + > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > > > > > This chunk looks like a hard-coded implementation of the > > > CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the > > > "stmmaceth" clock. I suggest to move it to the respective driver, add > > > a "tx" clock to the bindings and use the common clock kernel API > > > methods here only. > > > > I did consider your solution before writing the code, here are the > > reasons why I dropped it: > > > > > There's no any clk IP here, the HW just puts several > > gmac related control bits here, such as rx/tx delay, bypass, invert > > interface choice, clk direction. > > You omitted the essential part of your code which I pointed out. > > > From this point of view, it looks more > > like a syscon rather than clk. > > Toggling control bits is surely the syscon work. But gating a parental > clock, settings up the parental clock _divider_ and ungating the clock > back is the clock controller function. So it means your syscon is just > a normal multi-function device, which one of the function is the clock > controller. > > It's not like your situation is unique. For instance in case of a SoC > I was working with recently Clock Control Unit (CCU) was actually a > multi-function device which had: > 1. PLLs and Dividers supplying the clocks to the SoC components. Hi Serge, This is the big difference between your case and TH1520 gmac. (PS: @Emil, I read your comments in another reply. IIUC, jh7110 puts a real clk IP for gmac tx clock purpose) However, There's no real clk IP in the TH1520 gmac related syscon, yep, div and enable are some what clock related bits, but that's all, no more, no less. So even in this case, another abstraction layer via. clk subsystem is still preferred? IOW, a seperate clk driver for the gmac? Thanks > > 2. SoC components reset controller. > 3. I2C-interface controller. > 4. AXI-bus errors report registers. > 5. PCIe-controller tunings (LTSSM, link up/down, etc) > 6. SATA-controller tunings. > 7. Full SoC reset controller (syscon reboot), > 8. L2-cache tunings controller. > with the sub-functions CSRs joint in a single space. In that case the > PCIe-controller tunings and a lot of its reference clocks settings > were intermixed in a single chunk of the registers. So I had to create > a driver for the clocks anyway including all the PCIe reference > clock and refer to the syscon in the PCIe-controller device node for > the respective PCIe platform-specific tunings. > > > > > Secondly, I see other SoCs did similar for this case, such as > > dwmac-visconti, dwmac-meson8b, dwmac-ipq806x, dwmac-socfpga and so on. > > They met similar issue as the above. > > First I failed to find any clock-related things in the dwmac-socfpga I believe the ptp ref clk related is just to enable the clk by toggling corresponding bit. Anyway that's not important part here. > driver looking in anyway as yours. Second the dwmac-meson8b driver > creates a generic clock handler right in the driver. I don't think > it's a great solution but at the very least it registers the clock > handler in the kernel. But seeing the PROG_ETHERNET CSR is of 8 bytes > long there (0xc8834540 0x8) and defined at looking random base address > it's definitely a part of a Meson system controller which just > directly passed to the device driver. It's not correct. That part > should have been at least specified as a syscon too. Third the > dwmac-visconti driver is not a good example seeing it defines some > specific registers way away from the NIC CSR space. It's most likely a > separate device like syscon. Fourth dwmac-ipq806x driver > implementation looks indeed like yours. > In anyway I don't say your solution is fully wrong. At the very least > you have a syscon node defined. But it just makes you adding > incomplete device/platform bindings. Your network device do have the > Tx reference clock as a part of the separate system controller, but > you have to omit it because of the syscon property. You do have a > syscon node, but don't have its clock function exported. So AFAICS in > your case things can be implemented in a more canonical way than they > are now. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-29 3:17 ` Jisheng Zhang @ 2023-08-29 10:07 ` Serge Semin 2023-08-29 11:09 ` Jisheng Zhang 0 siblings, 1 reply; 22+ messages in thread From: Serge Semin @ 2023-08-29 10:07 UTC (permalink / raw) To: Jisheng Zhang Cc: Emil Renner Berthing, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Tue, Aug 29, 2023 at 11:17:45AM +0800, Jisheng Zhang wrote: > On Mon, Aug 28, 2023 at 08:30:50PM +0300, Serge Semin wrote: > > On Mon, Aug 28, 2023 at 11:41:47PM +0800, Jisheng Zhang wrote: > > > On Mon, Aug 28, 2023 at 04:40:19PM +0300, Serge Semin wrote: > > > > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > > > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > > > --- > > > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > > > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > > > > 3 files changed, 314 insertions(+) > > > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > > index 06c6871f8788..1bf71804c270 100644 > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > > > > stmmac device driver. This driver is used for H3/A83T/A64 > > > > > EMAC ethernet controller. > > > > > > > > > > +config DWMAC_THEAD > > > > > + tristate "T-HEAD dwmac support" > > > > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > > > > + select MFD_SYSCON > > > > > + help > > > > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > > > > + > > > > > + This selects the T-HEAD platform specific glue layer support for > > > > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > > > > + ethernet controller. > > > > > + > > > > > config DWMAC_IMX8 > > > > > tristate "NXP IMX8 DWMAC support" > > > > > default ARCH_MXC > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > > index 5b57aee19267..d73171ed6ad7 100644 > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > > > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > new file mode 100644 > > > > > index 000000000000..85135ef05906 > > > > > --- /dev/null > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > @@ -0,0 +1,302 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +/* > > > > > + * T-HEAD DWMAC platform driver > > > > > + * > > > > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > > > + * > > > > > + */ > > > > > + > > > > > +#include <linux/bitfield.h> > > > > > +#include <linux/mfd/syscon.h> > > > > > +#include <linux/module.h> > > > > > +#include <linux/of.h> > > > > > +#include <linux/of_device.h> > > > > > +#include <linux/of_net.h> > > > > > +#include <linux/platform_device.h> > > > > > +#include <linux/regmap.h> > > > > > + > > > > > +#include "stmmac_platform.h" > > > > > + > > > > > +#define GMAC_CLK_EN 0x00 > > > > > +#define GMAC_TX_CLK_EN BIT(1) > > > > > +#define GMAC_TX_CLK_N_EN BIT(2) > > > > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > > > > +#define GMAC_RX_CLK_EN BIT(4) > > > > > +#define GMAC_RX_CLK_N_EN BIT(5) > > > > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > > > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > > > > +#define GMAC_RXCLK_BYPASS BIT(15) > > > > > +#define GMAC_RXCLK_INVERT BIT(14) > > > > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > > > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > > > > +#define GMAC_TXCLK_BYPASS BIT(15) > > > > > +#define GMAC_TXCLK_INVERT BIT(14) > > > > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > > > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > > > +#define GMAC_PLLCLK_DIV 0x0c > > > > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > > > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > > > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > > > > +#define GMAC_GTXCLK_SEL 0x18 > > > > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > > > > +#define GMAC_INTF_CTRL 0x1c > > > > > +#define PHY_INTF_MASK BIT(0) > > > > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > > > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > > > > +#define GMAC_TXCLK_OEN 0x20 > > > > > +#define TXCLK_DIR_MASK BIT(0) > > > > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > > > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > > > > + > > > > > +#define GMAC_GMII_RGMII_RATE 125000000 > > > > > +#define GMAC_MII_RATE 25000000 > > > > > + > > > > > +struct thead_dwmac { > > > > > + struct plat_stmmacenet_data *plat; > > > > > + struct regmap *apb_regmap; > > > > > + struct device *dev; > > > > > + u32 rx_delay; > > > > > + u32 tx_delay; > > > > > +}; > > > > > + > > > > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > > > > +{ > > > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > > > + u32 phyif; > > > > > + > > > > > + switch (plat->interface) { > > > > > + case PHY_INTERFACE_MODE_MII: > > > > > + phyif = PHY_INTF_MII_GMII; > > > > > + break; > > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > > + phyif = PHY_INTF_RGMII; > > > > > + break; > > > > > + default: > > > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > > > + plat->interface); > > > > > + return -EINVAL; > > > > > + }; > > > > > + > > > > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > > > > +{ > > > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > > > + u32 txclk_dir; > > > > > + > > > > > + switch (plat->interface) { > > > > > + case PHY_INTERFACE_MODE_MII: > > > > > + txclk_dir = TXCLK_DIR_INPUT; > > > > > + break; > > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > > + txclk_dir = TXCLK_DIR_OUTPUT; > > > > > + break; > > > > > + default: > > > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > > > + plat->interface); > > > > > + return -EINVAL; > > > > > + }; > > > > > + > > > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > > > > +{ > > > > > + struct thead_dwmac *dwmac = priv; > > > > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > > > > + unsigned long rate; > > > > > + u32 div; > > > > > + > > > > > + switch (plat->interface) { > > > > > + /* For MII, rxc/txc is provided by phy */ > > > > > + case PHY_INTERFACE_MODE_MII: > > > > > + return; > > > > > + > > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > > > > > + rate = clk_get_rate(plat->stmmac_clk); > > > > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > > > > + rate % GMAC_MII_RATE != 0) { > > > > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > > > > + return; > > > > > + } > > > > > + > > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > > > > + > > > > > + switch (speed) { > > > > > + case SPEED_1000: > > > > > + div = rate / GMAC_GMII_RGMII_RATE; > > > > > + break; > > > > > + case SPEED_100: > > > > > + div = rate / GMAC_MII_RATE; > > > > > + break; > > > > > + case SPEED_10: > > > > > + div = rate * 10 / GMAC_MII_RATE; > > > > > + break; > > > > > + default: > > > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > > > > + return; > > > > > + } > > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > > > > + > > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > > > > > > > This chunk looks like a hard-coded implementation of the > > > > CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the > > > > "stmmaceth" clock. I suggest to move it to the respective driver, add > > > > a "tx" clock to the bindings and use the common clock kernel API > > > > methods here only. > > > > > > I did consider your solution before writing the code, here are the > > > reasons why I dropped it: > > > > > > > > There's no any clk IP here, the HW just puts several > > > gmac related control bits here, such as rx/tx delay, bypass, invert > > > interface choice, clk direction. > > > > You omitted the essential part of your code which I pointed out. > > > > > From this point of view, it looks more > > > like a syscon rather than clk. > > > > Toggling control bits is surely the syscon work. But gating a parental > > clock, settings up the parental clock _divider_ and ungating the clock > > back is the clock controller function. So it means your syscon is just > > a normal multi-function device, which one of the function is the clock > > controller. > > > > It's not like your situation is unique. For instance in case of a SoC > > I was working with recently Clock Control Unit (CCU) was actually a > > multi-function device which had: > > 1. PLLs and Dividers supplying the clocks to the SoC components. > > Hi Serge, > > This is the big difference between your case and TH1520 gmac. AFAICS my PCIe-part of the syscon is a similar story as your GMAC CSRs: enabled/disable several clock gates, setup a ref-clock divider, multiple flags with reset function and multiple flags/fields for the PCIe-controller tunings. All of that intermixed in a few registers. A similar thing was for the SATA-controller. > (PS: @Emil, I read your comments in another reply. IIUC, jh7110 puts a > real clk IP for gmac tx clock purpose) > > However, There's no real clk IP in the TH1520 gmac related syscon, yep, div > and enable are some what clock related bits, but that's all, no more, no less. These bits/fields are the clock-controller function of the syscon. Based on that your syscon is a multi-functional device which support the GMAC tunings and controls a reference clock gating/dividing. The outputs from the clock gate and divider gets to be supplied to the TH1520 GMAC. BTW Seeing you have only 0x20 bytes utilized in the glue driver I guess those registers chunk is a part of a bigger CSR space. Is it? > So even in this case, another abstraction layer via. clk subsystem is still > preferred? IOW, a seperate clk driver for the gmac? That's what I was told in a situation when I was trying to use a syscon to switch between the two parental clocks: https://lore.kernel.org/linux-ide/20220517201332.GB1462130-robh@kernel.org/ In your case IMO it is more preferred at the very least because it gives a more correct device description. Your bindings currently do not define the Tx/Rx reference clocks meanwhile the TH1520 GMAC do have them. -Serge(y) > > Thanks > > > > 2. SoC components reset controller. > > 3. I2C-interface controller. > > 4. AXI-bus errors report registers. > > 5. PCIe-controller tunings (LTSSM, link up/down, etc) > > 6. SATA-controller tunings. > > 7. Full SoC reset controller (syscon reboot), > > 8. L2-cache tunings controller. > > with the sub-functions CSRs joint in a single space. In that case the > > PCIe-controller tunings and a lot of its reference clocks settings > > were intermixed in a single chunk of the registers. So I had to create > > a driver for the clocks anyway including all the PCIe reference > > clock and refer to the syscon in the PCIe-controller device node for > > the respective PCIe platform-specific tunings. > > > > > > > > Secondly, I see other SoCs did similar for this case, such as > > > dwmac-visconti, dwmac-meson8b, dwmac-ipq806x, dwmac-socfpga and so on. > > > They met similar issue as the above. > > > > First I failed to find any clock-related things in the dwmac-socfpga > > I believe the ptp ref clk related is just to enable the clk by toggling > corresponding bit. Anyway that's not important part here. > > > driver looking in anyway as yours. Second the dwmac-meson8b driver > > creates a generic clock handler right in the driver. I don't think > > it's a great solution but at the very least it registers the clock > > handler in the kernel. But seeing the PROG_ETHERNET CSR is of 8 bytes > > long there (0xc8834540 0x8) and defined at looking random base address > > it's definitely a part of a Meson system controller which just > > directly passed to the device driver. It's not correct. That part > > should have been at least specified as a syscon too. Third the > > dwmac-visconti driver is not a good example seeing it defines some > > specific registers way away from the NIC CSR space. It's most likely a > > separate device like syscon. Fourth dwmac-ipq806x driver > > implementation looks indeed like yours. > > > In anyway I don't say your solution is fully wrong. At the very least > > you have a syscon node defined. But it just makes you adding > > incomplete device/platform bindings. Your network device do have the > > Tx reference clock as a part of the separate system controller, but > > you have to omit it because of the syscon property. You do have a > > syscon node, but don't have its clock function exported. So AFAICS in > > your case things can be implemented in a more canonical way than they > > are now. > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-29 10:07 ` Serge Semin @ 2023-08-29 11:09 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2023-08-29 11:09 UTC (permalink / raw) To: Serge Semin Cc: Emil Renner Berthing, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Tue, Aug 29, 2023 at 01:07:39PM +0300, Serge Semin wrote: > On Tue, Aug 29, 2023 at 11:17:45AM +0800, Jisheng Zhang wrote: > > On Mon, Aug 28, 2023 at 08:30:50PM +0300, Serge Semin wrote: > > > On Mon, Aug 28, 2023 at 11:41:47PM +0800, Jisheng Zhang wrote: > > > > On Mon, Aug 28, 2023 at 04:40:19PM +0300, Serge Semin wrote: > > > > > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > > > > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > > > > --- > > > > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > > > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > > > > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > > > > > 3 files changed, 314 insertions(+) > > > > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > > > index 06c6871f8788..1bf71804c270 100644 > > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > > > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > > > > > stmmac device driver. This driver is used for H3/A83T/A64 > > > > > > EMAC ethernet controller. > > > > > > > > > > > > +config DWMAC_THEAD > > > > > > + tristate "T-HEAD dwmac support" > > > > > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > > > > > + select MFD_SYSCON > > > > > > + help > > > > > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > > > > > + > > > > > > + This selects the T-HEAD platform specific glue layer support for > > > > > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > > > > > + ethernet controller. > > > > > > + > > > > > > config DWMAC_IMX8 > > > > > > tristate "NXP IMX8 DWMAC support" > > > > > > default ARCH_MXC > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > > > index 5b57aee19267..d73171ed6ad7 100644 > > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > > > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > > > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > > > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > > > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > > > > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > > > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > > > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > > > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > new file mode 100644 > > > > > > index 000000000000..85135ef05906 > > > > > > --- /dev/null > > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > @@ -0,0 +1,302 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > +/* > > > > > > + * T-HEAD DWMAC platform driver > > > > > > + * > > > > > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > > > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > > > > + * > > > > > > + */ > > > > > > + > > > > > > +#include <linux/bitfield.h> > > > > > > +#include <linux/mfd/syscon.h> > > > > > > +#include <linux/module.h> > > > > > > +#include <linux/of.h> > > > > > > +#include <linux/of_device.h> > > > > > > +#include <linux/of_net.h> > > > > > > +#include <linux/platform_device.h> > > > > > > +#include <linux/regmap.h> > > > > > > + > > > > > > +#include "stmmac_platform.h" > > > > > > + > > > > > > +#define GMAC_CLK_EN 0x00 > > > > > > +#define GMAC_TX_CLK_EN BIT(1) > > > > > > +#define GMAC_TX_CLK_N_EN BIT(2) > > > > > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > > > > > +#define GMAC_RX_CLK_EN BIT(4) > > > > > > +#define GMAC_RX_CLK_N_EN BIT(5) > > > > > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > > > > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > > > > > +#define GMAC_RXCLK_BYPASS BIT(15) > > > > > > +#define GMAC_RXCLK_INVERT BIT(14) > > > > > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > > > > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > > > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > > > > > +#define GMAC_TXCLK_BYPASS BIT(15) > > > > > > +#define GMAC_TXCLK_INVERT BIT(14) > > > > > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > > > > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > > > > +#define GMAC_PLLCLK_DIV 0x0c > > > > > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > > > > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > > > > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > > > > > +#define GMAC_GTXCLK_SEL 0x18 > > > > > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > > > > > +#define GMAC_INTF_CTRL 0x1c > > > > > > +#define PHY_INTF_MASK BIT(0) > > > > > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > > > > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > > > > > +#define GMAC_TXCLK_OEN 0x20 > > > > > > +#define TXCLK_DIR_MASK BIT(0) > > > > > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > > > > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > > > > > + > > > > > > +#define GMAC_GMII_RGMII_RATE 125000000 > > > > > > +#define GMAC_MII_RATE 25000000 > > > > > > + > > > > > > +struct thead_dwmac { > > > > > > + struct plat_stmmacenet_data *plat; > > > > > > + struct regmap *apb_regmap; > > > > > > + struct device *dev; > > > > > > + u32 rx_delay; > > > > > > + u32 tx_delay; > > > > > > +}; > > > > > > + > > > > > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > > > > > +{ > > > > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > > > > + u32 phyif; > > > > > > + > > > > > > + switch (plat->interface) { > > > > > > + case PHY_INTERFACE_MODE_MII: > > > > > > + phyif = PHY_INTF_MII_GMII; > > > > > > + break; > > > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > > > + phyif = PHY_INTF_RGMII; > > > > > > + break; > > > > > > + default: > > > > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > > > > + plat->interface); > > > > > > + return -EINVAL; > > > > > > + }; > > > > > > + > > > > > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > > > > > +{ > > > > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > > > > + u32 txclk_dir; > > > > > > + > > > > > > + switch (plat->interface) { > > > > > > + case PHY_INTERFACE_MODE_MII: > > > > > > + txclk_dir = TXCLK_DIR_INPUT; > > > > > > + break; > > > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > > > + txclk_dir = TXCLK_DIR_OUTPUT; > > > > > > + break; > > > > > > + default: > > > > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > > > > + plat->interface); > > > > > > + return -EINVAL; > > > > > > + }; > > > > > > + > > > > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > > > > > +{ > > > > > > + struct thead_dwmac *dwmac = priv; > > > > > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > > > > > + unsigned long rate; > > > > > > + u32 div; > > > > > > + > > > > > > + switch (plat->interface) { > > > > > > + /* For MII, rxc/txc is provided by phy */ > > > > > > + case PHY_INTERFACE_MODE_MII: > > > > > > + return; > > > > > > + > > > > > > + case PHY_INTERFACE_MODE_RGMII: > > > > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > > > > > > > > > + rate = clk_get_rate(plat->stmmac_clk); > > > > > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > > > > > + rate % GMAC_MII_RATE != 0) { > > > > > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > > > > > + > > > > > > + switch (speed) { > > > > > > + case SPEED_1000: > > > > > > + div = rate / GMAC_GMII_RGMII_RATE; > > > > > > + break; > > > > > > + case SPEED_100: > > > > > > + div = rate / GMAC_MII_RATE; > > > > > > + break; > > > > > > + case SPEED_10: > > > > > > + div = rate * 10 / GMAC_MII_RATE; > > > > > > + break; > > > > > > + default: > > > > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > > > > > + return; > > > > > > + } > > > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > > > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > > > > > + > > > > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > > > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > > > > > > > > > This chunk looks like a hard-coded implementation of the > > > > > CLK_SET_RATE_GATE Tx-clocks rate setup which parental clock is the > > > > > "stmmaceth" clock. I suggest to move it to the respective driver, add > > > > > a "tx" clock to the bindings and use the common clock kernel API > > > > > methods here only. > > > > > > > > I did consider your solution before writing the code, here are the > > > > reasons why I dropped it: > > > > > > > > > > > There's no any clk IP here, the HW just puts several > > > > gmac related control bits here, such as rx/tx delay, bypass, invert > > > > interface choice, clk direction. > > > > > > You omitted the essential part of your code which I pointed out. > > > > > > > From this point of view, it looks more > > > > like a syscon rather than clk. > > > > > > > Toggling control bits is surely the syscon work. But gating a parental > > > clock, settings up the parental clock _divider_ and ungating the clock > > > back is the clock controller function. So it means your syscon is just > > > a normal multi-function device, which one of the function is the clock > > > controller. > > > > > > It's not like your situation is unique. For instance in case of a SoC > > > I was working with recently Clock Control Unit (CCU) was actually a > > > multi-function device which had: > > > 1. PLLs and Dividers supplying the clocks to the SoC components. > > > > Hi Serge, > > > > This is the big difference between your case and TH1520 gmac. > > AFAICS my PCIe-part of the syscon is a similar story as your GMAC > CSRs: enabled/disable several clock gates, setup a ref-clock divider, > multiple flags with reset function and multiple flags/fields for the > PCIe-controller tunings. All of that intermixed in a few registers. A > similar thing was for the SATA-controller. > > > (PS: @Emil, I read your comments in another reply. IIUC, jh7110 puts a > > real clk IP for gmac tx clock purpose) > > > > > However, There's no real clk IP in the TH1520 gmac related syscon, yep, div > > and enable are some what clock related bits, but that's all, no more, no less. > > These bits/fields are the clock-controller function of the syscon. > Based on that your syscon is a multi-functional device which support > the GMAC tunings and controls a reference clock gating/dividing. The > outputs from the clock gate and divider gets to be supplied to the > TH1520 GMAC. > > BTW Seeing you have only 0x20 bytes utilized in the glue driver I > guess those registers chunk is a part of a bigger CSR space. Is it? Nope, the 4KB reg space is dedicated to gmac control, and isn't a part of a big reg space, and only 0~0x20 of the address space is used. > > > So even in this case, another abstraction layer via. clk subsystem is still > > preferred? IOW, a seperate clk driver for the gmac? > > That's what I was told in a situation when I was trying to use a > syscon to switch between the two parental clocks: > https://lore.kernel.org/linux-ide/20220517201332.GB1462130-robh@kernel.org/ > > In your case IMO it is more preferred at the very least because it > gives a more correct device description. Your bindings currently do > not define the Tx/Rx reference clocks meanwhile the TH1520 GMAC do > have them. > Thank you, I will modify the code towards clk direction. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-27 9:17 ` [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC Jisheng Zhang 2023-08-28 13:40 ` Serge Semin @ 2023-08-28 15:58 ` Serge Semin 2023-08-28 16:13 ` Jisheng Zhang 1 sibling, 1 reply; 22+ messages in thread From: Serge Semin @ 2023-08-28 15:58 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > 3 files changed, 314 insertions(+) > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > index 06c6871f8788..1bf71804c270 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > stmmac device driver. This driver is used for H3/A83T/A64 > EMAC ethernet controller. > > +config DWMAC_THEAD > + tristate "T-HEAD dwmac support" > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > + select MFD_SYSCON > + help > + Support for ethernet controllers on T-HEAD RISC-V SoCs > + > + This selects the T-HEAD platform specific glue layer support for > + the stmmac device driver. This driver is used for T-HEAD TH1520 > + ethernet controller. > + > config DWMAC_IMX8 > tristate "NXP IMX8 DWMAC support" > default ARCH_MXC > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > index 5b57aee19267..d73171ed6ad7 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > new file mode 100644 > index 000000000000..85135ef05906 > --- /dev/null > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > @@ -0,0 +1,302 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * T-HEAD DWMAC platform driver > + * > + * Copyright (C) 2021 Alibaba Group Holding Limited. > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + * > + */ > + > +#include <linux/bitfield.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_net.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > + > +#include "stmmac_platform.h" > + > +#define GMAC_CLK_EN 0x00 > +#define GMAC_TX_CLK_EN BIT(1) > +#define GMAC_TX_CLK_N_EN BIT(2) > +#define GMAC_TX_CLK_OUT_EN BIT(3) > +#define GMAC_RX_CLK_EN BIT(4) > +#define GMAC_RX_CLK_N_EN BIT(5) > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > +#define GMAC_RXCLK_BYPASS BIT(15) > +#define GMAC_RXCLK_INVERT BIT(14) > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > +#define GMAC_TXCLK_BYPASS BIT(15) > +#define GMAC_TXCLK_INVERT BIT(14) > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > +#define GMAC_PLLCLK_DIV 0x0c > +#define GMAC_PLLCLK_DIV_EN BIT(31) > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > +#define GMAC_GTXCLK_SEL 0x18 > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > +#define GMAC_INTF_CTRL 0x1c > +#define PHY_INTF_MASK BIT(0) > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > +#define GMAC_TXCLK_OEN 0x20 > +#define TXCLK_DIR_MASK BIT(0) > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > + > +#define GMAC_GMII_RGMII_RATE 125000000 > +#define GMAC_MII_RATE 25000000 > + > +struct thead_dwmac { > + struct plat_stmmacenet_data *plat; > + struct regmap *apb_regmap; > + struct device *dev; > + u32 rx_delay; > + u32 tx_delay; > +}; > + > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + u32 phyif; > + > + switch (plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + phyif = PHY_INTF_MII_GMII; > + break; > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + phyif = PHY_INTF_RGMII; > + break; > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return -EINVAL; > + }; > + > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > + > + return 0; > +} > + > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + u32 txclk_dir; > + > + switch (plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + txclk_dir = TXCLK_DIR_INPUT; > + break; > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + txclk_dir = TXCLK_DIR_OUTPUT; > + break; > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return -EINVAL; > + }; > + > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > + > + return 0; > +} > + > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > +{ > + struct thead_dwmac *dwmac = priv; > + struct plat_stmmacenet_data *plat = dwmac->plat; > + unsigned long rate; > + u32 div; > + > + switch (plat->interface) { > + /* For MII, rxc/txc is provided by phy */ > + case PHY_INTERFACE_MODE_MII: > + return; > + > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + rate = clk_get_rate(plat->stmmac_clk); > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > + rate % GMAC_MII_RATE != 0) { > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > + return; > + } > + > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > + > + switch (speed) { > + case SPEED_1000: > + div = rate / GMAC_GMII_RGMII_RATE; > + break; > + case SPEED_100: > + div = rate / GMAC_MII_RATE; > + break; > + case SPEED_10: > + div = rate * 10 / GMAC_MII_RATE; > + break; > + default: > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > + return; > + } > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > + > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > + break; > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return; > + } > +} > + > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + u32 reg; > + > + switch (plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > + break; > + > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + /* use pll */ > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > + > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; > + break; > + > + default: > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > + plat->interface); > + return -EINVAL; > + } > + > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > + > + return 0; > +} > + > +static int thead_dwmac_init(struct platform_device *pdev, > + struct plat_stmmacenet_data *plat) > +{ > + struct thead_dwmac *dwmac = plat->bsp_priv; > + int ret; > + > + ret = thead_dwmac_set_phy_if(plat); > + if (ret) > + return ret; > + > + ret = thead_dwmac_set_txclk_dir(plat); > + if (ret) > + return ret; > + > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); Just noticed. This doesn't look right because: 1. Are you sure your hardware expects the delays being specified in picoseconds. It's most likely a number of reference clock cycles and thus the DT-properties values should be properly converted. 2. Both delays should be added only for "rgmii" phy-mode, Tx clock delay - for "rgmii-rxid" phy-mode, Rx clock delay - for "rgmii-txid" phy-mode. -Serge(y) > + > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > + > + return thead_dwmac_enable_clk(plat); > +} > + > +static int thead_dwmac_probe(struct platform_device *pdev) > +{ > + struct plat_stmmacenet_data *plat; > + struct stmmac_resources stmmac_res; > + struct thead_dwmac *dwmac; > + struct device_node *np = pdev->dev.of_node; > + u32 delay_ps; > + int ret; > + > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to get resources\n"); > + > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); > + if (IS_ERR(plat)) > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > + "dt configuration failed\n"); > + > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > + if (!dwmac) { > + ret = -ENOMEM; > + goto err_remove_config_dt; > + } > + > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > + dwmac->rx_delay = delay_ps; > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > + dwmac->tx_delay = delay_ps; > + > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > + if (IS_ERR(dwmac->apb_regmap)) { > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > + "Failed to get gmac apb syscon\n"); > + goto err_remove_config_dt; > + } > + > + dwmac->dev = &pdev->dev; > + dwmac->plat = plat; > + plat->bsp_priv = dwmac; > + plat->fix_mac_speed = thead_dwmac_fix_speed; > + > + ret = thead_dwmac_init(pdev, plat); > + if (ret) > + goto err_remove_config_dt; > + > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); > + if (ret) > + goto err_remove_config_dt; > + > + return 0; > + > +err_remove_config_dt: > + stmmac_remove_config_dt(pdev, plat); > + > + return ret; > +} > + > +static const struct of_device_id thead_dwmac_match[] = { > + { .compatible = "thead,th1520-dwmac" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > + > +static struct platform_driver thead_dwmac_driver = { > + .probe = thead_dwmac_probe, > + .remove_new = stmmac_pltfr_remove, > + .driver = { > + .name = "thead-dwmac", > + .pm = &stmmac_pltfr_pm_ops, > + .of_match_table = thead_dwmac_match, > + }, > +}; > +module_platform_driver(thead_dwmac_driver); > + > +MODULE_AUTHOR("T-HEAD"); > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); > +MODULE_LICENSE("GPL"); > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-28 15:58 ` Serge Semin @ 2023-08-28 16:13 ` Jisheng Zhang 2023-08-28 17:46 ` Serge Semin 0 siblings, 1 reply; 22+ messages in thread From: Jisheng Zhang @ 2023-08-28 16:13 UTC (permalink / raw) To: Serge Semin Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Mon, Aug 28, 2023 at 06:58:11PM +0300, Serge Semin wrote: > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > 3 files changed, 314 insertions(+) > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > index 06c6871f8788..1bf71804c270 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > stmmac device driver. This driver is used for H3/A83T/A64 > > EMAC ethernet controller. > > > > +config DWMAC_THEAD > > + tristate "T-HEAD dwmac support" > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > + select MFD_SYSCON > > + help > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > + > > + This selects the T-HEAD platform specific glue layer support for > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > + ethernet controller. > > + > > config DWMAC_IMX8 > > tristate "NXP IMX8 DWMAC support" > > default ARCH_MXC > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > index 5b57aee19267..d73171ed6ad7 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > new file mode 100644 > > index 000000000000..85135ef05906 > > --- /dev/null > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > @@ -0,0 +1,302 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * T-HEAD DWMAC platform driver > > + * > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > + * > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/of_net.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > + > > +#include "stmmac_platform.h" > > + > > +#define GMAC_CLK_EN 0x00 > > +#define GMAC_TX_CLK_EN BIT(1) > > +#define GMAC_TX_CLK_N_EN BIT(2) > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > +#define GMAC_RX_CLK_EN BIT(4) > > +#define GMAC_RX_CLK_N_EN BIT(5) > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > +#define GMAC_RXCLK_BYPASS BIT(15) > > +#define GMAC_RXCLK_INVERT BIT(14) > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > +#define GMAC_TXCLK_BYPASS BIT(15) > > +#define GMAC_TXCLK_INVERT BIT(14) > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > +#define GMAC_PLLCLK_DIV 0x0c > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > +#define GMAC_GTXCLK_SEL 0x18 > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > +#define GMAC_INTF_CTRL 0x1c > > +#define PHY_INTF_MASK BIT(0) > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > +#define GMAC_TXCLK_OEN 0x20 > > +#define TXCLK_DIR_MASK BIT(0) > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > + > > +#define GMAC_GMII_RGMII_RATE 125000000 > > +#define GMAC_MII_RATE 25000000 > > + > > +struct thead_dwmac { > > + struct plat_stmmacenet_data *plat; > > + struct regmap *apb_regmap; > > + struct device *dev; > > + u32 rx_delay; > > + u32 tx_delay; > > +}; > > + > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + u32 phyif; > > + > > + switch (plat->interface) { > > + case PHY_INTERFACE_MODE_MII: > > + phyif = PHY_INTF_MII_GMII; > > + break; > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + phyif = PHY_INTF_RGMII; > > + break; > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return -EINVAL; > > + }; > > + > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > + > > + return 0; > > +} > > + > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + u32 txclk_dir; > > + > > + switch (plat->interface) { > > + case PHY_INTERFACE_MODE_MII: > > + txclk_dir = TXCLK_DIR_INPUT; > > + break; > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + txclk_dir = TXCLK_DIR_OUTPUT; > > + break; > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return -EINVAL; > > + }; > > + > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > + > > + return 0; > > +} > > + > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > +{ > > + struct thead_dwmac *dwmac = priv; > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > + unsigned long rate; > > + u32 div; > > + > > + switch (plat->interface) { > > + /* For MII, rxc/txc is provided by phy */ > > + case PHY_INTERFACE_MODE_MII: > > + return; > > + > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + rate = clk_get_rate(plat->stmmac_clk); > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > + rate % GMAC_MII_RATE != 0) { > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > + return; > > + } > > + > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > + > > + switch (speed) { > > + case SPEED_1000: > > + div = rate / GMAC_GMII_RGMII_RATE; > > + break; > > + case SPEED_100: > > + div = rate / GMAC_MII_RATE; > > + break; > > + case SPEED_10: > > + div = rate * 10 / GMAC_MII_RATE; > > + break; > > + default: > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > + return; > > + } > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > + > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > + break; > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return; > > + } > > +} > > + > > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + u32 reg; > > + > > + switch (plat->interface) { > > + case PHY_INTERFACE_MODE_MII: > > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > > + break; > > + > > + case PHY_INTERFACE_MODE_RGMII: > > + case PHY_INTERFACE_MODE_RGMII_ID: > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > + /* use pll */ > > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > > + > > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; > > + break; > > + > > + default: > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > + plat->interface); > > + return -EINVAL; > > + } > > + > > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > > + > > + return 0; > > +} > > + > > +static int thead_dwmac_init(struct platform_device *pdev, > > + struct plat_stmmacenet_data *plat) > > +{ > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > + int ret; > > + > > + ret = thead_dwmac_set_phy_if(plat); > > + if (ret) > > + return ret; > > + > > + ret = thead_dwmac_set_txclk_dir(plat); > > + if (ret) > > + return ret; > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > > Just noticed. This doesn't look right because: > 1. Are you sure your hardware expects the delays being specified in > picoseconds. It's most likely a number of reference clock cycles and > thus the DT-properties values should be properly converted. Good catch! I need to check further. > 2. Both delays should be added only for "rgmii" phy-mode, Tx clock > delay - for "rgmii-rxid" phy-mode, Rx clock delay - for "rgmii-txid" > phy-mode. For MII, the rxc/txc is provided by phy, so setting those delays won't impact MII. > > -Serge(y) > > > + > > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > > + > > + return thead_dwmac_enable_clk(plat); > > +} > > + > > +static int thead_dwmac_probe(struct platform_device *pdev) > > +{ > > + struct plat_stmmacenet_data *plat; > > + struct stmmac_resources stmmac_res; > > + struct thead_dwmac *dwmac; > > + struct device_node *np = pdev->dev.of_node; > > + u32 delay_ps; > > + int ret; > > + > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, > > + "failed to get resources\n"); > > + > > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); > > + if (IS_ERR(plat)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > > + "dt configuration failed\n"); > > + > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > > + if (!dwmac) { > > + ret = -ENOMEM; > > + goto err_remove_config_dt; > > + } > > + > > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > > + dwmac->rx_delay = delay_ps; > > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > > + dwmac->tx_delay = delay_ps; > > + > > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > > + if (IS_ERR(dwmac->apb_regmap)) { > > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > > + "Failed to get gmac apb syscon\n"); > > + goto err_remove_config_dt; > > + } > > + > > + dwmac->dev = &pdev->dev; > > + dwmac->plat = plat; > > + plat->bsp_priv = dwmac; > > + plat->fix_mac_speed = thead_dwmac_fix_speed; > > + > > + ret = thead_dwmac_init(pdev, plat); > > + if (ret) > > + goto err_remove_config_dt; > > + > > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); > > + if (ret) > > + goto err_remove_config_dt; > > + > > + return 0; > > + > > +err_remove_config_dt: > > + stmmac_remove_config_dt(pdev, plat); > > + > > + return ret; > > +} > > + > > +static const struct of_device_id thead_dwmac_match[] = { > > + { .compatible = "thead,th1520-dwmac" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > > + > > +static struct platform_driver thead_dwmac_driver = { > > + .probe = thead_dwmac_probe, > > + .remove_new = stmmac_pltfr_remove, > > + .driver = { > > + .name = "thead-dwmac", > > + .pm = &stmmac_pltfr_pm_ops, > > + .of_match_table = thead_dwmac_match, > > + }, > > +}; > > +module_platform_driver(thead_dwmac_driver); > > + > > +MODULE_AUTHOR("T-HEAD"); > > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); > > +MODULE_LICENSE("GPL"); > > -- > > 2.40.1 > > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC 2023-08-28 16:13 ` Jisheng Zhang @ 2023-08-28 17:46 ` Serge Semin 0 siblings, 0 replies; 22+ messages in thread From: Serge Semin @ 2023-08-28 17:46 UTC (permalink / raw) To: Jisheng Zhang Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime, Coquelin, Simon Horman, netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv On Tue, Aug 29, 2023 at 12:13:58AM +0800, Jisheng Zhang wrote: > On Mon, Aug 28, 2023 at 06:58:11PM +0300, Serge Semin wrote: > > On Sun, Aug 27, 2023 at 05:17:10PM +0800, Jisheng Zhang wrote: > > > Add dwmac glue driver to support the dwmac on the T-HEAD TH1520 SoC. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > > > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > > > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > > > 3 files changed, 314 insertions(+) > > > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > index 06c6871f8788..1bf71804c270 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > > > @@ -216,6 +216,17 @@ config DWMAC_SUN8I > > > stmmac device driver. This driver is used for H3/A83T/A64 > > > EMAC ethernet controller. > > > > > > +config DWMAC_THEAD > > > + tristate "T-HEAD dwmac support" > > > + depends on OF && (ARCH_THEAD || COMPILE_TEST) > > > + select MFD_SYSCON > > > + help > > > + Support for ethernet controllers on T-HEAD RISC-V SoCs > > > + > > > + This selects the T-HEAD platform specific glue layer support for > > > + the stmmac device driver. This driver is used for T-HEAD TH1520 > > > + ethernet controller. > > > + > > > config DWMAC_IMX8 > > > tristate "NXP IMX8 DWMAC support" > > > default ARCH_MXC > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > index 5b57aee19267..d73171ed6ad7 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > > > @@ -27,6 +27,7 @@ obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o > > > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o > > > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o > > > obj-$(CONFIG_DWMAC_SUN8I) += dwmac-sun8i.o > > > +obj-$(CONFIG_DWMAC_THEAD) += dwmac-thead.o > > > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) += dwmac-dwc-qos-eth.o > > > obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o > > > obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > new file mode 100644 > > > index 000000000000..85135ef05906 > > > --- /dev/null > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c > > > @@ -0,0 +1,302 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * T-HEAD DWMAC platform driver > > > + * > > > + * Copyright (C) 2021 Alibaba Group Holding Limited. > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > + * > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/mfd/syscon.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > +#include <linux/of_net.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/regmap.h> > > > + > > > +#include "stmmac_platform.h" > > > + > > > +#define GMAC_CLK_EN 0x00 > > > +#define GMAC_TX_CLK_EN BIT(1) > > > +#define GMAC_TX_CLK_N_EN BIT(2) > > > +#define GMAC_TX_CLK_OUT_EN BIT(3) > > > +#define GMAC_RX_CLK_EN BIT(4) > > > +#define GMAC_RX_CLK_N_EN BIT(5) > > > +#define GMAC_EPHY_REF_CLK_EN BIT(6) > > > +#define GMAC_RXCLK_DELAY_CTRL 0x04 > > > +#define GMAC_RXCLK_BYPASS BIT(15) > > > +#define GMAC_RXCLK_INVERT BIT(14) > > > +#define GMAC_RXCLK_DELAY_MASK GENMASK(4, 0) > > > +#define GMAC_RXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > +#define GMAC_TXCLK_DELAY_CTRL 0x08 > > > +#define GMAC_TXCLK_BYPASS BIT(15) > > > +#define GMAC_TXCLK_INVERT BIT(14) > > > +#define GMAC_TXCLK_DELAY_MASK GENMASK(4, 0) > > > +#define GMAC_TXCLK_DELAY_VAL(x) FIELD_PREP(GMAC_RXCLK_DELAY_MASK, (x)) > > > +#define GMAC_PLLCLK_DIV 0x0c > > > +#define GMAC_PLLCLK_DIV_EN BIT(31) > > > +#define GMAC_PLLCLK_DIV_MASK GENMASK(7, 0) > > > +#define GMAC_PLLCLK_DIV_NUM(x) FIELD_PREP(GMAC_PLLCLK_DIV_MASK, (x)) > > > +#define GMAC_GTXCLK_SEL 0x18 > > > +#define GMAC_GTXCLK_SEL_PLL BIT(0) > > > +#define GMAC_INTF_CTRL 0x1c > > > +#define PHY_INTF_MASK BIT(0) > > > +#define PHY_INTF_RGMII FIELD_PREP(PHY_INTF_MASK, 1) > > > +#define PHY_INTF_MII_GMII FIELD_PREP(PHY_INTF_MASK, 0) > > > +#define GMAC_TXCLK_OEN 0x20 > > > +#define TXCLK_DIR_MASK BIT(0) > > > +#define TXCLK_DIR_OUTPUT FIELD_PREP(TXCLK_DIR_MASK, 0) > > > +#define TXCLK_DIR_INPUT FIELD_PREP(TXCLK_DIR_MASK, 1) > > > + > > > +#define GMAC_GMII_RGMII_RATE 125000000 > > > +#define GMAC_MII_RATE 25000000 > > > + > > > +struct thead_dwmac { > > > + struct plat_stmmacenet_data *plat; > > > + struct regmap *apb_regmap; > > > + struct device *dev; > > > + u32 rx_delay; > > > + u32 tx_delay; > > > +}; > > > + > > > +static int thead_dwmac_set_phy_if(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 phyif; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + phyif = PHY_INTF_MII_GMII; > > > + break; > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + phyif = PHY_INTF_RGMII; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + }; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_INTF_CTRL, phyif); > > > + > > > + return 0; > > > +} > > > + > > > +static int thead_dwmac_set_txclk_dir(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 txclk_dir; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + txclk_dir = TXCLK_DIR_INPUT; > > > + break; > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + txclk_dir = TXCLK_DIR_OUTPUT; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + }; > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_OEN, txclk_dir); > > > + > > > + return 0; > > > +} > > > + > > > +static void thead_dwmac_fix_speed(void *priv, unsigned int speed, unsigned int mode) > > > +{ > > > + struct thead_dwmac *dwmac = priv; > > > + struct plat_stmmacenet_data *plat = dwmac->plat; > > > + unsigned long rate; > > > + u32 div; > > > + > > > + switch (plat->interface) { > > > + /* For MII, rxc/txc is provided by phy */ > > > + case PHY_INTERFACE_MODE_MII: > > > + return; > > > + > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + rate = clk_get_rate(plat->stmmac_clk); > > > + if (!rate || rate % GMAC_GMII_RGMII_RATE != 0 || > > > + rate % GMAC_MII_RATE != 0) { > > > + dev_err(dwmac->dev, "invalid gmac rate %ld\n", rate); > > > + return; > > > + } > > > + > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, GMAC_PLLCLK_DIV_EN, 0); > > > + > > > + switch (speed) { > > > + case SPEED_1000: > > > + div = rate / GMAC_GMII_RGMII_RATE; > > > + break; > > > + case SPEED_100: > > > + div = rate / GMAC_MII_RATE; > > > + break; > > > + case SPEED_10: > > > + div = rate * 10 / GMAC_MII_RATE; > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "invalid speed %u\n", speed); > > > + return; > > > + } > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > + GMAC_PLLCLK_DIV_MASK, GMAC_PLLCLK_DIV_NUM(div)); > > > + > > > + regmap_update_bits(dwmac->apb_regmap, GMAC_PLLCLK_DIV, > > > + GMAC_PLLCLK_DIV_EN, GMAC_PLLCLK_DIV_EN); > > > + break; > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return; > > > + } > > > +} > > > + > > > +static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + u32 reg; > > > + > > > + switch (plat->interface) { > > > + case PHY_INTERFACE_MODE_MII: > > > + reg = GMAC_RX_CLK_EN | GMAC_TX_CLK_EN; > > > + break; > > > + > > > + case PHY_INTERFACE_MODE_RGMII: > > > + case PHY_INTERFACE_MODE_RGMII_ID: > > > + case PHY_INTERFACE_MODE_RGMII_RXID: > > > + case PHY_INTERFACE_MODE_RGMII_TXID: > > > + /* use pll */ > > > + regmap_write(dwmac->apb_regmap, GMAC_GTXCLK_SEL, GMAC_GTXCLK_SEL_PLL); > > > + > > > + reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN | > > > + GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN; > > > + break; > > > + > > > + default: > > > + dev_err(dwmac->dev, "unsupported phy interface %d\n", > > > + plat->interface); > > > + return -EINVAL; > > > + } > > > + > > > + regmap_write(dwmac->apb_regmap, GMAC_CLK_EN, reg); > > > + > > > + return 0; > > > +} > > > + > > > +static int thead_dwmac_init(struct platform_device *pdev, > > > + struct plat_stmmacenet_data *plat) > > > +{ > > > + struct thead_dwmac *dwmac = plat->bsp_priv; > > > + int ret; > > > + > > > + ret = thead_dwmac_set_phy_if(plat); > > > + if (ret) > > > + return ret; > > > + > > > + ret = thead_dwmac_set_txclk_dir(plat); > > > + if (ret) > > > + return ret; > > > + > > > > > + regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > > > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > > > + regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > > > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > > > > Just noticed. This doesn't look right because: > > 1. Are you sure your hardware expects the delays being specified in > > picoseconds. It's most likely a number of reference clock cycles and > > thus the DT-properties values should be properly converted. > > Good catch! I need to check further. > > 2. Both delays should be added only for "rgmii" phy-mode, Tx clock > > delay - for "rgmii-rxid" phy-mode, Rx clock delay - for "rgmii-txid" > > phy-mode. > > For MII, the rxc/txc is provided by phy, so setting those delays > won't impact MII. I am talking about _RGMII_ in this case which based on the thead_dwmac_set_phy_if() method is supported by this controller. rgmii{-(tx|rx)id} modes are provided so to prevent both MAC and PHY adding the delays in the RGMII interface. Imagine you have a PHY with the fixed delays meanwhile you have them also added in the MAC. You may and likely will end up with the traffic loss then. As a reference take a closer look at for instance rk_gmac_powerup() method implemented in the dwmac-rk.c driver. -Serge(y) > > > > -Serge(y) > > > > > + > > > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > > > + > > > + return thead_dwmac_enable_clk(plat); > > > +} > > > + > > > +static int thead_dwmac_probe(struct platform_device *pdev) > > > +{ > > > + struct plat_stmmacenet_data *plat; > > > + struct stmmac_resources stmmac_res; > > > + struct thead_dwmac *dwmac; > > > + struct device_node *np = pdev->dev.of_node; > > > + u32 delay_ps; > > > + int ret; > > > + > > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > > > + if (ret) > > > + return dev_err_probe(&pdev->dev, ret, > > > + "failed to get resources\n"); > > > + > > > + plat = stmmac_probe_config_dt(pdev, stmmac_res.mac); > > > + if (IS_ERR(plat)) > > > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > > > + "dt configuration failed\n"); > > > + > > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > > > + if (!dwmac) { > > > + ret = -ENOMEM; > > > + goto err_remove_config_dt; > > > + } > > > + > > > + if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay_ps)) > > > + dwmac->rx_delay = delay_ps; > > > + if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay_ps)) > > > + dwmac->tx_delay = delay_ps; > > > + > > > + dwmac->apb_regmap = syscon_regmap_lookup_by_phandle(np, "thead,gmacapb"); > > > + if (IS_ERR(dwmac->apb_regmap)) { > > > + ret = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->apb_regmap), > > > + "Failed to get gmac apb syscon\n"); > > > + goto err_remove_config_dt; > > > + } > > > + > > > + dwmac->dev = &pdev->dev; > > > + dwmac->plat = plat; > > > + plat->bsp_priv = dwmac; > > > + plat->fix_mac_speed = thead_dwmac_fix_speed; > > > + > > > + ret = thead_dwmac_init(pdev, plat); > > > + if (ret) > > > + goto err_remove_config_dt; > > > + > > > + ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res); > > > + if (ret) > > > + goto err_remove_config_dt; > > > + > > > + return 0; > > > + > > > +err_remove_config_dt: > > > + stmmac_remove_config_dt(pdev, plat); > > > + > > > + return ret; > > > +} > > > + > > > +static const struct of_device_id thead_dwmac_match[] = { > > > + { .compatible = "thead,th1520-dwmac" }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(of, thead_dwmac_match); > > > + > > > +static struct platform_driver thead_dwmac_driver = { > > > + .probe = thead_dwmac_probe, > > > + .remove_new = stmmac_pltfr_remove, > > > + .driver = { > > > + .name = "thead-dwmac", > > > + .pm = &stmmac_pltfr_pm_ops, > > > + .of_match_table = thead_dwmac_match, > > > + }, > > > +}; > > > +module_platform_driver(thead_dwmac_driver); > > > + > > > +MODULE_AUTHOR("T-HEAD"); > > > +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); > > > +MODULE_DESCRIPTION("T-HEAD dwmac platform driver"); > > > +MODULE_LICENSE("GPL"); > > > -- > > > 2.40.1 > > > > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC 2023-08-27 9:17 [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC Jisheng Zhang ` (2 preceding siblings ...) 2023-08-27 9:17 ` [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC Jisheng Zhang @ 2023-09-04 5:41 ` Jiexun Wang 3 siblings, 0 replies; 22+ messages in thread From: Jiexun Wang @ 2023-09-04 5:41 UTC (permalink / raw) To: jszhang Cc: alexandre.torgue, conor+dt, davem, devicetree, edumazet, joabreu, krzysztof.kozlowski+dt, kuba, linux-arm-kernel, linux-kernel, linux-riscv, linux-stm32, netdev, pabeni, peppe.cavallaro, robh+dt, simon.horman On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: >Add the dwmac driver support for T-HEAD TH1520 SoC. > >Since the clk part isn't mainlined, so SoC dts(i) changes are not >included in this series. However, it can be tested by using fixed-clock. > >Since v1: > - rebase on the lastest net-next > - collect Reviewed-by tag > - address Krzysztof's comment of the dt binding > - fix "div is not initialised" issue pointed out by Simon > >Jisheng Zhang (3): > dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties > dt-bindings: net: add T-HEAD dwmac support > net: stmmac: add glue layer for T-HEAD TH1520 SoC > > .../devicetree/bindings/net/snps,dwmac.yaml | 2 + > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++ > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > .../net/ethernet/stmicro/stmmac/dwmac-thead.c | 302 ++++++++++++++++++ > 5 files changed, 393 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml I would like to try this patch on my LicheePi 4A board, but I don't know how to modify the dts file. Could you please share your modifications to the dts file? Thank you very much. Best regards, Jiexun Wang ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-09-04 5:41 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-27 9:17 [PATCH net-next v2 0/3] add the dwmac driver for T-HEAD TH1520 SoC Jisheng Zhang 2023-08-27 9:17 ` [PATCH net-next v2 1/3] dt-bindings: net: snps,dwmac: allow dwmac-3.70a to set pbl properties Jisheng Zhang 2023-08-27 9:17 ` [PATCH net-next v2 2/3] dt-bindings: net: add T-HEAD dwmac support Jisheng Zhang 2023-08-28 13:13 ` Serge Semin 2023-08-28 15:17 ` Jisheng Zhang 2023-08-28 15:51 ` Serge Semin 2023-08-28 16:06 ` Jisheng Zhang 2023-08-28 17:55 ` Krzysztof Kozlowski 2023-08-29 3:02 ` Jisheng Zhang 2023-08-28 13:16 ` Serge Semin 2023-08-27 9:17 ` [PATCH net-next v2 3/3] net: stmmac: add glue layer for T-HEAD TH1520 SoC Jisheng Zhang 2023-08-28 13:40 ` Serge Semin 2023-08-28 15:41 ` Jisheng Zhang 2023-08-28 16:56 ` Emil Renner Berthing 2023-08-28 17:30 ` Serge Semin 2023-08-29 3:17 ` Jisheng Zhang 2023-08-29 10:07 ` Serge Semin 2023-08-29 11:09 ` Jisheng Zhang 2023-08-28 15:58 ` Serge Semin 2023-08-28 16:13 ` Jisheng Zhang 2023-08-28 17:46 ` Serge Semin 2023-09-04 5:41 ` [PATCH net-next v2 0/3] add the dwmac driver " Jiexun Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).