[v6,net-next,06/13] dt-bindings: net: ti: add new cpsw switch driver bindings

Add bindings for the new TI CPSW switch driver. Comparing to the legacy
bindings (net/cpsw.txt):
- ports definition follows DSA bindings (net/dsa/dsa.txt) and ports can be
marked as "disabled" if not physically wired.
- all deprecated properties dropped;
- all legacy propertiies dropped which represent constant HW cpapbilities
(cpdma_channels, ale_entries, bd_ram_size, mac_control, slaves,
active_slave)
- TI CPTS DT properties are reused as is, but grouped in "cpts" sub-node
- TI Davinci MDIO DT bindings are reused as is, because Davinci MDIO is
reused.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../bindings/net/ti,cpsw-switch.yaml          | 245 ++++++++++++++++++
 1 file changed, 245 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml

Hi,

* Grygorii Strashko <grygorii.strashko@ti.com> [191109 15:17]:
> +    mac_sw: switch@0 {
> +        compatible = "ti,dra7-cpsw-switch","ti,cpsw-switch";
> +        reg = <0x0 0x4000>;
> +        ranges = <0 0 0x4000>;
> +        clocks = <&gmac_main_clk>;
> +        clock-names = "fck";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        syscon = <&scm_conf>;
> +        inctrl-names = "default", "sleep";
> +
> +        interrupts = <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "rx_thresh", "rx", "tx", "misc";

I think with the ti-sysc managing the interconnect target module as the
parent of this, you should be able add all the modules as direct children
of ti-sysc with minor fixups. This would simplify things, and makes it
easier to update the driver later on when the child modules get
changed/updated/moved around.

The child modules just need to call PM runtime to have access to their
registers, and whatever cpsw control module part could be a separate
driver providing Linux standard services for example for clock gating :)

> +        davinci_mdio_sw: mdio@1000 {
> +                compatible = "ti,cpsw-mdio","ti,davinci_mdio";
> +                reg = <0x1000 0x100>;
> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0>;
> +                clock-names = "fck";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                bus_freq = <1000000>;
> +
> +                ethphy0_sw: ethernet-phy@0 {
> +                        reg = <0>;
> +                };
> +
> +                ethphy1_sw: ethernet-phy@1 {
> +                        reg = <41>;
> +                };
> +        };

And in this case, mdio above would just move up one level.

This goes back to my earlier comments saying the cpsw is really just
a private interconnect with a collection of various mostly independent
modules. Sounds like you're heading that way already though at the
driver level :)

Regards,

Tony

On 11/11/2019 19:26, Tony Lindgren wrote:
> Hi,
> 
> * Grygorii Strashko <grygorii.strashko@ti.com> [191109 15:17]:
>> +    mac_sw: switch@0 {
>> +        compatible = "ti,dra7-cpsw-switch","ti,cpsw-switch";
>> +        reg = <0x0 0x4000>;
>> +        ranges = <0 0 0x4000>;
>> +        clocks = <&gmac_main_clk>;
>> +        clock-names = "fck";
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +        syscon = <&scm_conf>;
>> +        inctrl-names = "default", "sleep";
>> +
>> +        interrupts = <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>;
>> +        interrupt-names = "rx_thresh", "rx", "tx", "misc";
> 
> I think with the ti-sysc managing the interconnect target module as the
> parent of this, you should be able add all the modules as direct children
> of ti-sysc with minor fixups. This would simplify things, and makes it
> easier to update the driver later on when the child modules get
> changed/updated/moved around.
> 
> The child modules just need to call PM runtime to have access to their
> registers, and whatever cpsw control module part could be a separate
> driver providing Linux standard services for example for clock gating :)
> 
>> +        davinci_mdio_sw: mdio@1000 {
>> +                compatible = "ti,cpsw-mdio","ti,davinci_mdio";
>> +                reg = <0x1000 0x100>;
>> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0>;
>> +                clock-names = "fck";
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +                bus_freq = <1000000>;
>> +
>> +                ethphy0_sw: ethernet-phy@0 {
>> +                        reg = <0>;
>> +                };
>> +
>> +                ethphy1_sw: ethernet-phy@1 {
>> +                        reg = <41>;
>> +                };
>> +        };
> 
> And in this case, mdio above would just move up one level.
> 
> This goes back to my earlier comments saying the cpsw is really just
> a private interconnect with a collection of various mostly independent
> modules. Sounds like you're heading that way already though at the
> driver level :)

No, sorry I do not agree. The MDIO is inseparable part of CPSW and it's enabled when CPSW is enabled
(on interconnect level), more over I want to get rid of platform device in MDIO for most of the cases
as it only introduces boot/probing complexity.

The same is valid for CPTS.

Hi,

* Grygorii Strashko <grygorii.strashko@ti.com> [191112 09:54]:
> No, sorry I do not agree. The MDIO is inseparable part of CPSW and it's enabled when CPSW is enabled
> (on interconnect level), more over I want to get rid of platform device in MDIO for most of the cases
> as it only introduces boot/probing complexity.

Well the fact that mdio is enabled at the interconnect level is why
I think the cpsw child modules are independent components :)

So I did the following quick test on pocketbeagle with Linux next,
it has no Ethernet wired up, and by default we have ethernet@0
set to status = "disabled".

Manually enable the target module at 0x4a100000:
# echo on > /sys/devices/platform/ocp/4a000000.interconnect/\
4a000000.interconnect:segment@0/4a101200.target-module/power/control

Dump out mdio registers at offset 0x1000:
# rwmem 0x4a101000+0x100
0x4a101000 = 0x40070106
0x4a101004 = 0x810000ff
0x4a101008 = 0000000000
0x4a10100c = 0000000000
0x4a101010 = 0000000000
0x4a101014 = 0000000000
0x4a101018 = 0000000000
...

So yup, it seems quite independent of the other child devices
on the same interconnect target mdoule. I'm quessing it's the
same story for other modules like cppi_dma and so on, this
should be easy to check.

Hmm and isn't the some version of mdio also used stuffed into
davinci_emac and netcp too?

Anyways, up to you. But my experience is that having separate
driver modules is the way to go than trying to treat any TI
"subsystem" as a single device. This is because the child modules
tend to get updated and changed and moved around over time.

Regards,

Tony

On Sat, Nov 09, 2019 at 05:15:18PM +0200, Grygorii Strashko wrote:
> Add bindings for the new TI CPSW switch driver. Comparing to the legacy
> bindings (net/cpsw.txt):
> - ports definition follows DSA bindings (net/dsa/dsa.txt) and ports can be
> marked as "disabled" if not physically wired.
> - all deprecated properties dropped;
> - all legacy propertiies dropped which represent constant HW cpapbilities
> (cpdma_channels, ale_entries, bd_ram_size, mac_control, slaves,
> active_slave)
> - TI CPTS DT properties are reused as is, but grouped in "cpts" sub-node
> - TI Davinci MDIO DT bindings are reused as is, because Davinci MDIO is
> reused.
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  .../bindings/net/ti,cpsw-switch.yaml          | 245 ++++++++++++++++++
>  1 file changed, 245 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
> new file mode 100644
> index 000000000000..afeb6a4f1ada
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
> @@ -0,0 +1,245 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ti,cpsw-switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI SoC Ethernet Switch Controller (CPSW) Device Tree Bindings
> +
> +maintainers:
> +  - Grygorii Strashko <grygorii.strashko@ti.com>
> +  - Sekhar Nori <nsekhar@ti.com>
> +
> +description:
> +  The 3-port switch gigabit ethernet subsystem provides ethernet packet
> +  communication and can be configured as an ethernet switch. It provides the
> +  gigabit media independent interface (GMII),reduced gigabit media
> +  independent interface (RGMII), reduced media independent interface (RMII),
> +  the management data input output (MDIO) for physical layer device (PHY)
> +  management.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,cpsw-switch
> +      - items:
> +         - const: ti,am335x-cpsw-switch
> +         - const: ti,cpsw-switch
> +      - items:
> +        - const: ti,am4372-cpsw-switch
> +        - const: ti,cpsw-switch
> +      - items:
> +        - const: ti,dra7-cpsw-switch
> +        - const: ti,cpsw-switch
> +
> +  reg:
> +    maxItems: 1
> +    description:
> +       The physical base address and size of full the CPSW module IO range
> +
> +  ranges: true
> +
> +  clocks:
> +    maxItems: 1
> +    description: CPSW functional clock
> +
> +  clock-names:
> +    maxItems: 1
> +    items:
> +      - const: fck
> +
> +  interrupts:
> +    maxItems: 4

Implied by 'items' list.

> +    items:
> +      - description: RX_THRESH interrupt
> +      - description: RX interrupt
> +      - description: TX interrupt
> +      - description: MISC interrupt
> +
> +  interrupt-names:
> +    maxItems: 4

Implied by 'items' list.

> +    items:
> +      - const: "rx_thresh"
> +      - const: "rx"
> +      - const: "tx"
> +      - const: "misc"
> +
> +  pinctrl-names: true
> +
> +  syscon:
> +    $ref: /schemas/types.yaml#definitions/phandle
> +    maxItems: 1

Not an array, so not needed.

> +    description:
> +      Phandle to the system control device node which provides access to
> +      efuse IO range with MAC addresses
> +
> +
> +  ethernet-ports:
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      "^port@[0-9]+$":
> +          type: object
> +          minItems: 1
> +          maxItems: 2
> +          description: CPSW external ports
> +
> +          allOf:
> +            - $ref: ethernet-controller.yaml#
> +
> +          properties:
> +            reg:
> +              maxItems: 1
> +              enum: [1, 2]
> +              description: CPSW port number
> +
> +            phys:
> +              $ref: /schemas/types.yaml#definitions/phandle-array
> +              maxItems: 1
> +              description:  phandle on phy-gmii-sel PHY
> +
> +            label:
> +              $ref: /schemas/types.yaml#/definitions/string-array
> +              maxItems: 1
> +              description: label associated with this port
> +
> +            ti,dual-emac-pvid:
> +              $ref: /schemas/types.yaml#/definitions/uint32
> +              maxItems: 1
> +              minimum: 1
> +              maximum: 1024
> +              description:
> +                Specifies default PORT VID to be used to segregate
> +                ports. Default value - CPSW port number.
> +
> +          required:
> +            - reg
> +            - phys
> +
> +  mdio:
> +    type: object
> +    allOf:
> +      - $ref: "ti,davinci-mdio.yaml#"
> +    description:
> +      CPSW MDIO bus.
> +
> +  cpts:
> +    type: object
> +    description:
> +      The Common Platform Time Sync (CPTS) module
> +
> +    properties:
> +      clocks:
> +        maxItems: 1
> +        description: CPTS reference clock
> +
> +      clock-names:
> +        maxItems: 1
> +        items:
> +          - const: cpts
> +
> +      cpts_clock_mult:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        maxItems: 1

Not an array, so not needed.

Is there a set or range of values you can define?

> +        description:
> +          Numerator to convert input clock ticks into ns
> +
> +      cpts_clock_shift:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        maxItems: 1

Same comments here.

> +        description:
> +          Denominator to convert input clock ticks into ns.
> +          Mult and shift will be calculated basing on CPTS rftclk frequency if
> +          both cpts_clock_shift and cpts_clock_mult properties are not provided.
> +
> +    required:
> +      - clocks
> +      - clock-names
> +
> +required:
> +  - compatible
> +  - reg
> +  - ranges
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interrupt-names
> +  - '#address-cells'
> +  - '#size-cells'
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/dra7.h>
> +
> +    mac_sw: switch@0 {
> +        compatible = "ti,dra7-cpsw-switch","ti,cpsw-switch";
> +        reg = <0x0 0x4000>;
> +        ranges = <0 0 0x4000>;
> +        clocks = <&gmac_main_clk>;
> +        clock-names = "fck";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        syscon = <&scm_conf>;
> +        inctrl-names = "default", "sleep";
> +
> +        interrupts = <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "rx_thresh", "rx", "tx", "misc";
> +
> +        ethernet-ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                cpsw_port1: port@1 {
> +                        reg = <1>;
> +                        label = "port1";
> +                        mac-address = [ 00 00 00 00 00 00 ];
> +                        phys = <&phy_gmii_sel 1>;
> +                        phy-handle = <&ethphy0_sw>;
> +                        phy-mode = "rgmii";
> +                        ti,dual_emac_pvid = <1>;
> +                };
> +
> +                cpsw_port2: port@2 {
> +                        reg = <2>;
> +                        label = "wan";
> +                        mac-address = [ 00 00 00 00 00 00 ];
> +                        phys = <&phy_gmii_sel 2>;
> +                        phy-handle = <&ethphy1_sw>;
> +                        phy-mode = "rgmii";
> +                        ti,dual_emac_pvid = <2>;
> +                };
> +        };
> +
> +        davinci_mdio_sw: mdio@1000 {
> +                compatible = "ti,cpsw-mdio","ti,davinci_mdio";
> +                reg = <0x1000 0x100>;
> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0>;
> +                clock-names = "fck";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                bus_freq = <1000000>;
> +
> +                ethphy0_sw: ethernet-phy@0 {
> +                        reg = <0>;
> +                };
> +
> +                ethphy1_sw: ethernet-phy@1 {
> +                        reg = <41>;

make dt_binding_check fails:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: 
mdio@1000: ethernet-phy@1:reg:0:0: 41 is greater than the maximum of 31
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: 
ethernet-phy@1: reg:0:0: 41 is greater than the maximum of 31

> +                };
> +        };
> +
> +        cpts {
> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 25>;
> +                clock-names = "cpts";
> +        };
> +    };
> -- 
> 2.17.1
>