| Message ID | 20191024100914.16840-6-grygorii.strashko@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | net: ethernet: ti: introduce new cpsw switchdev based driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On 10/24/19 3:09 AM, 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> > --- [snip] > +- mdio : CPSW MDIO bus block description > + - bus_freq : MDIO Bus frequency clock-frequency is a more typical property to describe the bus clock's frequency, that is what i2c and spi do. > + See bindings/net/mdio.txt and davinci-mdio.txt > + > +- cpts : The Common Platform Time Sync (CPTS) module description > + - clocks : should contain the CPTS reference clock > + - clock-names : should be "cpts" > + See bindings/clock/clock-bindings.txt > + > + Optional properties - all ports: > + - cpts_clock_mult : Numerator to convert input clock ticks into ns > + - cpts_clock_shift : 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. Why would those two be needed that would be modeled in the Linux Common Clock Framework?
> +TI SoC Ethernet Switch Controller Device Tree Bindings (new) > +------------------------------------------------------ > + > +The 3-port switch gigabit ethernet subsystem provides ethernet packet > +communication and can be configured as an ethernet switch. Hi Grygorii Maybe referring it to a 3-port switch will cause confusion, since in this use case, it only has 2 ports, and you only list two ports in the device tree. > 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. > + > +Required properties: > +- compatible : be one of the below: > + "ti,cpsw-switch" for backward compatible > + "ti,am335x-cpsw-switch" for AM335x controllers > + "ti,am4372-cpsw-switch" for AM437x controllers > + "ti,dra7-cpsw-switch" for DRA7x controllers > +- reg : physical base address and size of the CPSW module IO range > +- ranges : shall contain the CPSW module IO range available for child devices > +- clocks : should contain the CPSW functional clock > +- clock-names : should be "fck" > + See bindings/clock/clock-bindings.txt > +- interrupts : should contain CPSW RX_THRESH, RX, TX, MISC interrupts > +- interrupt-names : should contain "rx_thresh", "rx", "tx", "misc" > + See bindings/interrupt-controller/interrupts.txt > + > +Optional properties: > +- syscon : phandle to the system control device node which provides access to > + efuse IO range with MAC addresses > + > +Required Sub-nodes: > +- ethernet-ports : contains CPSW external ports descriptions > + Required properties: > + - #address-cells : Must be 1 > + - #size-cells : Must be 0 > + - reg : CPSW port number. Should be 1 or 2 > + - phys : phandle on phy-gmii-sel PHY (see phy/ti-phy-gmii-sel.txt) > + - phy-mode : See [1] > + - phy-handle : See [1] > + > + Optional properties: > + - label : Describes the label associated with this port > + - ti,dual-emac-pvid : Specifies default PORT VID to be used to segregate > + ports. Default value - CPSW port number. > + - mac-address : See [1] > + - local-mac-address : See [1] > + > +- mdio : CPSW MDIO bus block description > + - bus_freq : MDIO Bus frequency > + See bindings/net/mdio.txt and davinci-mdio.txt > + > +- cpts : The Common Platform Time Sync (CPTS) module description > + - clocks : should contain the CPTS reference clock > + - clock-names : should be "cpts" > + See bindings/clock/clock-bindings.txt > + > + Optional properties - all ports: > + - cpts_clock_mult : Numerator to convert input clock ticks into ns > + - cpts_clock_shift : 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. > + > +[1] See Documentation/devicetree/bindings/net/ethernet-controller.yaml > + > +Examples: > + > +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>; > + status = "disabled"; > + > + 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"; > + /* Filled in by U-Boot */ > + mac-address = [ 00 00 00 00 00 00 ]; > + phys = <&phy_gmii_sel 1>; > + }; > + > + cpsw_port2: port@2 { > + reg = <2>; > + label = "wan"; > + /* Filled in by U-Boot */ > + mac-address = [ 00 00 00 00 00 00 ]; > + phys = <&phy_gmii_sel 2>; > + }; > + }; > + > + davinci_mdio_sw: mdio@1000 { > + compatible = "ti,cpsw-mdio","ti,davinci_mdio"; > + #address-cells = <1>; > + #size-cells = <0>; > + ti,hwmods = "davinci_mdio"; > + bus_freq = <1000000>; > + reg = <0x1000 0x100>; > + }; > + > + cpts { > + clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 25>; > + clock-names = "cpts"; > + }; > +}; > + > +&mac_sw { > + pinctrl-names = "default", "sleep"; > + status = "okay"; > +}; > + > +&cpsw_port1 { > + phy-handle = <ðphy0_sw>; > + phy-mode = "rgmii"; > + ti,dual_emac_pvid = <1>; > +}; > + > +&cpsw_port2 { > + phy-handle = <ðphy1_sw>; > + phy-mode = "rgmii"; > + ti,dual_emac_pvid = <2>; > +}; > + > +&davinci_mdio_sw { > + ethphy0_sw: ethernet-phy@0 { > + reg = <0>; > + }; > + > + ethphy1_sw: ethernet-phy@1 { > + reg = <1>; > + }; > +}; In an example, it is unusual to split things up like this. I understand that parts of this will be in the dtsi file, and parts in the .dts file, but examples generally keep it all as one. And when you re-write this in YAML so it can be used to validated real DTs, you will have to combine it. Andrew
Hi Florian, On 25/10/2019 20:47, Florian Fainelli wrote: > On 10/24/19 3:09 AM, 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> >> --- > > [snip] >> +- mdio : CPSW MDIO bus block description >> + - bus_freq : MDIO Bus frequency > > clock-frequency is a more typical property to describe the bus clock's > frequency, that is what i2c and spi do. The MDIO is re-used here unchanged (including bindings). i think, I could try to add standard optional property "bus-frequency" to MDIO bindings as separate series, and deprecate "bus_freq". > >> + See bindings/net/mdio.txt and davinci-mdio.txt >> + >> +- cpts : The Common Platform Time Sync (CPTS) module description >> + - clocks : should contain the CPTS reference clock >> + - clock-names : should be "cpts" >> + See bindings/clock/clock-bindings.txt >> + >> + Optional properties - all ports: >> + - cpts_clock_mult : Numerator to convert input clock ticks into ns >> + - cpts_clock_shift : 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. > > Why would those two be needed that would be modeled in the Linux Common > Clock Framework? The CPTS is re-used here unchanged (including bindings). This is very specific tuning options for PHC clock (cyclecounter) which intended to be used in very rare cases with some ref frequencies when automatic calculation is not working properly.
hi Andrew, On 29/10/2019 04:23, Andrew Lunn wrote: >> +TI SoC Ethernet Switch Controller Device Tree Bindings (new) >> +------------------------------------------------------ >> + >> +The 3-port switch gigabit ethernet subsystem provides ethernet packet >> +communication and can be configured as an ethernet switch. > > Hi Grygorii > > Maybe referring it to a 3-port switch will cause confusion, since in > this use case, it only has 2 ports, and you only list two ports in the > device tree. Yeah. This is how it's defined in TRM - Port 0 (CPU port) is the same as external Port from CPSW switch core point of view. > >> It provides the >> +gigabit media independent interface (GMII),reduced gigabit media >> +independent interface (RGMII), reduced media independent interface (RMII), [...] >> + >> +&mac_sw { >> + pinctrl-names = "default", "sleep"; >> + status = "okay"; >> +}; >> + >> +&cpsw_port1 { >> + phy-handle = <ðphy0_sw>; >> + phy-mode = "rgmii"; >> + ti,dual_emac_pvid = <1>; >> +}; >> + >> +&cpsw_port2 { >> + phy-handle = <ðphy1_sw>; >> + phy-mode = "rgmii"; >> + ti,dual_emac_pvid = <2>; >> +}; >> + >> +&davinci_mdio_sw { >> + ethphy0_sw: ethernet-phy@0 { >> + reg = <0>; >> + }; >> + >> + ethphy1_sw: ethernet-phy@1 { >> + reg = <1>; >> + }; >> +}; > > In an example, it is unusual to split things up like this. I > understand that parts of this will be in the dtsi file, and parts in > the .dts file, but examples generally keep it all as one. And when you > re-write this in YAML so it can be used to validated real DTs, you > will have to combine it. Thank you. I'll update.
On 11/1/19 10:25 AM, Grygorii Strashko wrote: > Hi Florian, > > On 25/10/2019 20:47, Florian Fainelli wrote: >> On 10/24/19 3:09 AM, 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> >>> --- >> >> [snip] >>> +- mdio : CPSW MDIO bus block description >>> + - bus_freq : MDIO Bus frequency >> >> clock-frequency is a more typical property to describe the bus clock's >> frequency, that is what i2c and spi do. > > The MDIO is re-used here unchanged (including bindings). > i think, I could try to add standard optional property "bus-frequency" > to MDIO bindings > as separate series, and deprecate "bus_freq". What is wrong with 'clock-frequency'? Documentation/devicetree/bindings/i2c/i2c.txt: - clock-frequency frequency of bus clock in Hz. Documentation/devicetree/bindings/net/brcm,unimac-mdio.txt: - clock-frequency: the MDIO bus clock that must be output by the MDIO bus hardware, if absent, the default hardware values are used Maybe this is a bit of a misnomer as it is usually considered a replacement for the lack of a proper "clocks" property with a clock provider, but we can flip the coin around any way we want, it looks almost the same.
On 01/11/2019 19:36, Florian Fainelli wrote: > On 11/1/19 10:25 AM, Grygorii Strashko wrote: >> Hi Florian, >> >> On 25/10/2019 20:47, Florian Fainelli wrote: >>> On 10/24/19 3:09 AM, 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> >>>> --- >>> >>> [snip] >>>> +- mdio : CPSW MDIO bus block description >>>> + - bus_freq : MDIO Bus frequency >>> >>> clock-frequency is a more typical property to describe the bus clock's >>> frequency, that is what i2c and spi do. >> >> The MDIO is re-used here unchanged (including bindings). >> i think, I could try to add standard optional property "bus-frequency" >> to MDIO bindings >> as separate series, and deprecate "bus_freq". > > What is wrong with 'clock-frequency'? > > Documentation/devicetree/bindings/i2c/i2c.txt: > > - clock-frequency > frequency of bus clock in Hz. > > Documentation/devicetree/bindings/net/brcm,unimac-mdio.txt: > > - clock-frequency: the MDIO bus clock that must be output by the MDIO bus > hardware, if absent, the default hardware values are used > > Maybe this is a bit of a misnomer as it is usually considered a > replacement for the lack of a proper "clocks" property with a clock > provider, but we can flip the coin around any way we want, it looks > almost the same. > I can do clock-frequency, but I like more bus-frequency (personally) due to more understandable meaning, and because in "Devicetree Specification v0.2" clock-frequency is defined as more related to clocks. Any way I hope you agree that it should be part separate discussion?
diff --git a/Documentation/devicetree/bindings/net/ti,cpsw-switch.txt b/Documentation/devicetree/bindings/net/ti,cpsw-switch.txt new file mode 100644 index 000000000000..c0110391be9d --- /dev/null +++ b/Documentation/devicetree/bindings/net/ti,cpsw-switch.txt @@ -0,0 +1,145 @@ +TI SoC Ethernet Switch Controller Device Tree Bindings (new) +------------------------------------------------------ + +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. + +Required properties: +- compatible : be one of the below: + "ti,cpsw-switch" for backward compatible + "ti,am335x-cpsw-switch" for AM335x controllers + "ti,am4372-cpsw-switch" for AM437x controllers + "ti,dra7-cpsw-switch" for DRA7x controllers +- reg : physical base address and size of the CPSW module IO range +- ranges : shall contain the CPSW module IO range available for child devices +- clocks : should contain the CPSW functional clock +- clock-names : should be "fck" + See bindings/clock/clock-bindings.txt +- interrupts : should contain CPSW RX_THRESH, RX, TX, MISC interrupts +- interrupt-names : should contain "rx_thresh", "rx", "tx", "misc" + See bindings/interrupt-controller/interrupts.txt + +Optional properties: +- syscon : phandle to the system control device node which provides access to + efuse IO range with MAC addresses + +Required Sub-nodes: +- ethernet-ports : contains CPSW external ports descriptions + Required properties: + - #address-cells : Must be 1 + - #size-cells : Must be 0 + - reg : CPSW port number. Should be 1 or 2 + - phys : phandle on phy-gmii-sel PHY (see phy/ti-phy-gmii-sel.txt) + - phy-mode : See [1] + - phy-handle : See [1] + + Optional properties: + - label : Describes the label associated with this port + - ti,dual-emac-pvid : Specifies default PORT VID to be used to segregate + ports. Default value - CPSW port number. + - mac-address : See [1] + - local-mac-address : See [1] + +- mdio : CPSW MDIO bus block description + - bus_freq : MDIO Bus frequency + See bindings/net/mdio.txt and davinci-mdio.txt + +- cpts : The Common Platform Time Sync (CPTS) module description + - clocks : should contain the CPTS reference clock + - clock-names : should be "cpts" + See bindings/clock/clock-bindings.txt + + Optional properties - all ports: + - cpts_clock_mult : Numerator to convert input clock ticks into ns + - cpts_clock_shift : 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. + +[1] See Documentation/devicetree/bindings/net/ethernet-controller.yaml + +Examples: + +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>; + status = "disabled"; + + 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"; + /* Filled in by U-Boot */ + mac-address = [ 00 00 00 00 00 00 ]; + phys = <&phy_gmii_sel 1>; + }; + + cpsw_port2: port@2 { + reg = <2>; + label = "wan"; + /* Filled in by U-Boot */ + mac-address = [ 00 00 00 00 00 00 ]; + phys = <&phy_gmii_sel 2>; + }; + }; + + davinci_mdio_sw: mdio@1000 { + compatible = "ti,cpsw-mdio","ti,davinci_mdio"; + #address-cells = <1>; + #size-cells = <0>; + ti,hwmods = "davinci_mdio"; + bus_freq = <1000000>; + reg = <0x1000 0x100>; + }; + + cpts { + clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 25>; + clock-names = "cpts"; + }; +}; + +&mac_sw { + pinctrl-names = "default", "sleep"; + status = "okay"; +}; + +&cpsw_port1 { + phy-handle = <ðphy0_sw>; + phy-mode = "rgmii"; + ti,dual_emac_pvid = <1>; +}; + +&cpsw_port2 { + phy-handle = <ðphy1_sw>; + phy-mode = "rgmii"; + ti,dual_emac_pvid = <2>; +}; + +&davinci_mdio_sw { + ethphy0_sw: ethernet-phy@0 { + reg = <0>; + }; + + ethphy1_sw: ethernet-phy@1 { + reg = <1>; + }; +};
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.txt | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.txt