| Message ID | 20191119221925.28426-7-grygorii.strashko@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | None | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Tue, Nov 19, 2019 at 4:19 PM Grygorii Strashko <grygorii.strashko@ti.com> 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 | 240 ++++++++++++++++++ > 1 file changed, 240 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml I see David has applied this already, but it still has numerous problems. Please send a follow-up. > > 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..81ae8cafabc1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml > @@ -0,0 +1,240 @@ > +# SPDX-License-Identifier: GPL-2.0 For new bindings, please dual license: (GPL-2.0-only OR BSD-2-Clause) > +%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: > + items: > + - description: RX_THRESH interrupt > + - description: RX interrupt > + - description: TX interrupt > + - description: MISC interrupt > + > + interrupt-names: > + items: > + - const: "rx_thresh" > + - const: "rx" > + - const: "tx" > + - const: "misc" > + > + pinctrl-names: true > + > + syscon: > + $ref: /schemas/types.yaml#definitions/phandle > + description: > + Phandle to the system control device node which provides access to > + efuse IO range with MAC addresses Can't you use nvmem binding for this? > + > + Drop the extra blank line. > + ethernet-ports: > + type: object > + properties: > + '#address-cells': > + const: 1 > + '#size-cells': > + const: 0 > + > + patternProperties: > + "^port@[0-9]+$": > + type: object > + minItems: 1 > + maxItems: 2 These apply to arrays and don't make sense here. > + description: CPSW external ports > + > + allOf: > + - $ref: ethernet-controller.yaml# > + > + properties: > + reg: > + maxItems: 1 > + enum: [1, 2] You're mixing array and scalar constraints. What you want is: reg: items: - enum: [1, 2] > + description: CPSW port number > + > + phys: > + $ref: /schemas/types.yaml#definitions/phandle-array Drop this. 'phys' is standard and has a type definition already. > + maxItems: 1 > + description: phandle on phy-gmii-sel PHY > + > + label: > + $ref: /schemas/types.yaml#/definitions/string-array Same here. > + maxItems: 1 This too. 'label' should never be an array. > + description: label associated with this port > + > + ti,dual-emac-pvid: > + $ref: /schemas/types.yaml#/definitions/uint32 > + maxItems: 1 > + minimum: 1 > + maximum: 1024 allOf: - $ref: /schemas/types.yaml#/definitions/uint32 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 Can drop this. Implied by 'items' list length. > + items: > + - const: cpts > + > + cpts_clock_mult: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Numerator to convert input clock ticks into ns > + > + cpts_clock_shift: > + $ref: /schemas/types.yaml#/definitions/uint32 > + 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 = <ðphy0_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 = <ðphy1_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 = <1>; > + }; > + }; > + > + cpts { > + clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 25>; > + clock-names = "cpts"; > + }; > + }; > -- > 2.17.1 >
Hi Rob, On 21/11/2019 21:24, Rob Herring wrote: > On Tue, Nov 19, 2019 at 4:19 PM Grygorii Strashko > <grygorii.strashko@ti.com> 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 | 240 ++++++++++++++++++ >> 1 file changed, 240 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml > > I see David has applied this already, but it still has numerous > problems. Please send a follow-up. > >> >> 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..81ae8cafabc1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml >> @@ -0,0 +1,240 @@ >> +# SPDX-License-Identifier: GPL-2.0 > > For new bindings, please dual license: > > (GPL-2.0-only OR BSD-2-Clause) > >> +%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: >> + items: >> + - description: RX_THRESH interrupt >> + - description: RX interrupt >> + - description: TX interrupt >> + - description: MISC interrupt >> + >> + interrupt-names: >> + items: >> + - const: "rx_thresh" >> + - const: "rx" >> + - const: "tx" >> + - const: "misc" >> + >> + pinctrl-names: true >> + >> + syscon: >> + $ref: /schemas/types.yaml#definitions/phandle >> + description: >> + Phandle to the system control device node which provides access to >> + efuse IO range with MAC addresses > > Can't you use nvmem binding for this? > I've sent patch to fix other comments except this one. About nvmem,I've been thinking about it for a long time already, but in our case MAC address is encoded in eFuse register in a different way for different SoCs. So even if I'll try to use nvmem and define some MAC cell: efuse: efuse { compatible = "..."; #address-cells = <1>; #size-cells = <1>; eth_mac: eth_mac@34 { reg = <0x34 0x10>; }; }; portX { ... nvmem-cells = <ð_mac>; nvmem-cell-names = "mac-address"; }; the of_get_mac_address() will finally call nvmem->reg_read(priv, offset, val, bytes); and at this point nvmem driver will have no knowledge about the type of the cell (MAC address), so no decoding can not be done and returned mac will be incorrect. Not sure how to proceed here. One of the ways is to pass cell info in struct nvmem_device .reg_read()/.reg_write() callbacks, cell name could be use to perform some actions. Another thing which need to be considered is - MAC can be assigned per port, so dev->of_node != port_of_node (and of_get_mac_addr_nvmem() will fail).
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..81ae8cafabc1 --- /dev/null +++ b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml @@ -0,0 +1,240 @@ +# 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: + items: + - description: RX_THRESH interrupt + - description: RX interrupt + - description: TX interrupt + - description: MISC interrupt + + interrupt-names: + items: + - const: "rx_thresh" + - const: "rx" + - const: "tx" + - const: "misc" + + pinctrl-names: true + + syscon: + $ref: /schemas/types.yaml#definitions/phandle + 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 + description: + Numerator to convert input clock ticks into ns + + cpts_clock_shift: + $ref: /schemas/types.yaml#/definitions/uint32 + 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 = <ðphy0_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 = <ðphy1_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 = <1>; + }; + }; + + cpts { + clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 25>; + clock-names = "cpts"; + }; + };
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 | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml