Message ID | 20240528153109.439407-1-ada@thorsis.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi | expand |
Hei hei, Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: > These properties are common for all i2c subnodes, and marked as > 'required' in atmel/microchip i2c bindings. Allows to add i2c device > nodes (like an rtc for example) in other .dts files including > sam9x60.dtsi without requiring to repeat these properties for each i2c > device again and again. > > Found on a custom board after adding this in .dts: > > &flx5 { > atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; > status = "okay"; > > i2c5: i2c@600 { > pinctrl-0 = <&pinctrl_flx5_default>; > status = "okay"; > > pcf8523: rtc@68 { > compatible = "nxp,pcf8523"; > reg = <0x68>; > }; > }; > }; > > … which created a warning like this: > > […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) > […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' > […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 > […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value > […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' > […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' > > This probably should have been done with commit 84f23f3284d5 ("ARM: dts: > at91: sam9x60: move flexcom definitions") already, where those > address-cells and size-cells properties were left in the board .dts > files instead of moving them to the dtsi. It's been a while. Is something wrong with the patch? Or with the commit message? at91 support seems to have slowed down somehow lately? :-/ Greets Alex > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > --- > .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- > .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- > arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ > 3 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > index c6fbdd29019f..b9ffd9e5faac 100644 > --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > @@ -198,8 +198,6 @@ i2c0: i2c@600 { > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx0_default>; > - #address-cells = <1>; > - #size-cells = <0>; > i2c-analog-filter; > i2c-digital-filter; > i2c-digital-filter-width-ns = <35>; > diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > index f3cbb675cea4..3b38707d736e 100644 > --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > @@ -207,8 +207,6 @@ &flx0 { > status = "okay"; > > i2c0: i2c@600 { > - #address-cells = <1>; > - #size-cells = <0>; > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx0_default>; > @@ -254,8 +252,6 @@ &flx6 { > status = "okay"; > > i2c6: i2c@600 { > - #address-cells = <1>; > - #size-cells = <0>; > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx6_default>; > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > index 291540e5d81e..551b46894f47 100644 > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > @@ -215,6 +215,8 @@ i2c4: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -284,6 +286,8 @@ i2c5: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -394,6 +398,8 @@ i2c11: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -443,6 +449,8 @@ i2c12: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -600,6 +608,8 @@ i2c6: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -649,6 +659,8 @@ i2c7: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -698,6 +710,8 @@ i2c8: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -766,6 +780,8 @@ i2c0: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -834,6 +850,8 @@ i2c1: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -902,6 +920,8 @@ i2c2: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -970,6 +990,8 @@ i2c3: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > -- > 2.39.2 > >
Hi Alex, I added Claudiu in the thread. Regards, Mihai ----------------------------------------------------------------- Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: > These properties are common for all i2c subnodes, and marked as > 'required' in atmel/microchip i2c bindings. Allows to add i2c device > nodes (like an rtc for example) in other .dts files including > sam9x60.dtsi without requiring to repeat these properties for each i2c > device again and again. > > Found on a custom board after adding this in .dts: > > &flx5 { > atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; > status = "okay"; > > i2c5: i2c@600 { > pinctrl-0 = <&pinctrl_flx5_default>; > status = "okay"; > > pcf8523: rtc@68 { > compatible = "nxp,pcf8523"; > reg = <0x68>; > }; > }; > }; > > … which created a warning like this: > > […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) > […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' > […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 > […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value > […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' > […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' > > This probably should have been done with commit 84f23f3284d5 ("ARM: dts: > at91: sam9x60: move flexcom definitions") already, where those > address-cells and size-cells properties were left in the board .dts > files instead of moving them to the dtsi. It's been a while. Is something wrong with the patch? Or with the commit message? at91 support seems to have slowed down somehow lately? :-/ Greets Alex > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > --- > .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- > .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- > arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ > 3 files changed, 26 insertions(+), 6 deletions(-) > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > -- > 2.39.2
Hi Alexander, On 28/05/2024 at 17:31, Alexander Dahl wrote: > These properties are common for all i2c subnodes, and marked as > 'required' in atmel/microchip i2c bindings. Yes, indeed. > Allows to add i2c device > nodes (like an rtc for example) in other .dts files including > sam9x60.dtsi without requiring to repeat these properties for each i2c > device again and again. > > Found on a custom board after adding this in .dts: > > &flx5 { > atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; > status = "okay"; > > i2c5: i2c@600 { > pinctrl-0 = <&pinctrl_flx5_default>; > status = "okay"; > > pcf8523: rtc@68 { > compatible = "nxp,pcf8523"; > reg = <0x68>; > }; > }; > }; > > … which created a warning like this: > > […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) > […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' > […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 > […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value > […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' > […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' > > This probably should have been done with commit 84f23f3284d5 ("ARM: dts: > at91: sam9x60: move flexcom definitions") already, where those Yes, and I think it was mentioned by Claudiu back then. > address-cells and size-cells properties were left in the board .dts > files instead of moving them to the dtsi. I agree with the move, plus you addressed both of the current mainline boards: Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> Thanks Alexander, best regards, Nicolas > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > --- > .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- > .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- > arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ > 3 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > index c6fbdd29019f..b9ffd9e5faac 100644 > --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > @@ -198,8 +198,6 @@ i2c0: i2c@600 { > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx0_default>; > - #address-cells = <1>; > - #size-cells = <0>; > i2c-analog-filter; > i2c-digital-filter; > i2c-digital-filter-width-ns = <35>; > diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > index f3cbb675cea4..3b38707d736e 100644 > --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > @@ -207,8 +207,6 @@ &flx0 { > status = "okay"; > > i2c0: i2c@600 { > - #address-cells = <1>; > - #size-cells = <0>; > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx0_default>; > @@ -254,8 +252,6 @@ &flx6 { > status = "okay"; > > i2c6: i2c@600 { > - #address-cells = <1>; > - #size-cells = <0>; > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx6_default>; > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > index 291540e5d81e..551b46894f47 100644 > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > @@ -215,6 +215,8 @@ i2c4: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -284,6 +286,8 @@ i2c5: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -394,6 +398,8 @@ i2c11: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -443,6 +449,8 @@ i2c12: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -600,6 +608,8 @@ i2c6: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -649,6 +659,8 @@ i2c7: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -698,6 +710,8 @@ i2c8: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -766,6 +780,8 @@ i2c0: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -834,6 +850,8 @@ i2c1: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -902,6 +920,8 @@ i2c2: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -970,6 +990,8 @@ i2c3: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > -- > 2.39.2 >
On 05/07/2024 at 08:19, Alexander Dahl wrote: > Hei hei, > > Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: >> These properties are common for all i2c subnodes, and marked as >> 'required' in atmel/microchip i2c bindings. Allows to add i2c device >> nodes (like an rtc for example) in other .dts files including >> sam9x60.dtsi without requiring to repeat these properties for each i2c >> device again and again. >> >> Found on a custom board after adding this in .dts: >> >> &flx5 { >> atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; >> status = "okay"; >> >> i2c5: i2c@600 { >> pinctrl-0 = <&pinctrl_flx5_default>; >> status = "okay"; >> >> pcf8523: rtc@68 { >> compatible = "nxp,pcf8523"; >> reg = <0x68>; >> }; >> }; >> }; >> >> … which created a warning like this: [..] >> This probably should have been done with commit 84f23f3284d5 ("ARM: dts: >> at91: sam9x60: move flexcom definitions") already, where those >> address-cells and size-cells properties were left in the board .dts >> files instead of moving them to the dtsi. > > It's been a while. Is something wrong with the patch? Or with the > commit message? Sorry about that. > at91 support seems to have slowed down somehow lately? :-/ Sorry for this feeling that you have. I guarantee it's not ;-) Thanks a lot for your contribution, we'll make sure to react in a timely manner to your efforts on Microchip platforms. Best regards, Nicolas >> Signed-off-by: Alexander Dahl <ada@thorsis.com> >> --- >> .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- >> .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- >> arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ >> 3 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts >> index c6fbdd29019f..b9ffd9e5faac 100644 >> --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts >> @@ -198,8 +198,6 @@ i2c0: i2c@600 { >> dmas = <0>, <0>; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_flx0_default>; >> - #address-cells = <1>; >> - #size-cells = <0>; >> i2c-analog-filter; >> i2c-digital-filter; >> i2c-digital-filter-width-ns = <35>; >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts >> index f3cbb675cea4..3b38707d736e 100644 >> --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts >> @@ -207,8 +207,6 @@ &flx0 { >> status = "okay"; >> >> i2c0: i2c@600 { >> - #address-cells = <1>; >> - #size-cells = <0>; >> dmas = <0>, <0>; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_flx0_default>; >> @@ -254,8 +252,6 @@ &flx6 { >> status = "okay"; >> >> i2c6: i2c@600 { >> - #address-cells = <1>; >> - #size-cells = <0>; >> dmas = <0>, <0>; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_flx6_default>; >> diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi >> index 291540e5d81e..551b46894f47 100644 >> --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi >> +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi >> @@ -215,6 +215,8 @@ i2c4: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -284,6 +286,8 @@ i2c5: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -394,6 +398,8 @@ i2c11: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -443,6 +449,8 @@ i2c12: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -600,6 +608,8 @@ i2c6: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -649,6 +659,8 @@ i2c7: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -698,6 +710,8 @@ i2c8: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -766,6 +780,8 @@ i2c0: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -834,6 +850,8 @@ i2c1: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -902,6 +920,8 @@ i2c2: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -970,6 +990,8 @@ i2c3: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> >> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 >> -- >> 2.39.2 >> >>
On 28.05.2024 18:31, Alexander Dahl wrote: > These properties are common for all i2c subnodes, and marked as > 'required' in atmel/microchip i2c bindings. Not required anymore: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=cf7feb642dc0e36bbbda9ec9fecc6601f439b8dd > Allows to add i2c device > nodes (like an rtc for example) in other .dts files including > sam9x60.dtsi without requiring to repeat these properties for each i2c > device again and again. > > Found on a custom board after adding this in .dts: > > &flx5 { > atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; > status = "okay"; > > i2c5: i2c@600 { > pinctrl-0 = <&pinctrl_flx5_default>; > status = "okay"; > > pcf8523: rtc@68 { > compatible = "nxp,pcf8523"; > reg = <0x68>; > }; > }; > }; > > … which created a warning like this: > > […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) > […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' > […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 > […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value > […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value > […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' > […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' > > This probably should have been done with commit 84f23f3284d5 ("ARM: dts: > at91: sam9x60: move flexcom definitions") already, where those > address-cells and size-cells properties were left in the board .dts > files instead of moving them to the dtsi. > > Signed-off-by: Alexander Dahl <ada@thorsis.com> Will apply it for the next PR. > --- > .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- > .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- > arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ > 3 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > index c6fbdd29019f..b9ffd9e5faac 100644 > --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > @@ -198,8 +198,6 @@ i2c0: i2c@600 { > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx0_default>; > - #address-cells = <1>; > - #size-cells = <0>; > i2c-analog-filter; > i2c-digital-filter; > i2c-digital-filter-width-ns = <35>; > diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > index f3cbb675cea4..3b38707d736e 100644 > --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > @@ -207,8 +207,6 @@ &flx0 { > status = "okay"; > > i2c0: i2c@600 { > - #address-cells = <1>; > - #size-cells = <0>; > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx0_default>; > @@ -254,8 +252,6 @@ &flx6 { > status = "okay"; > > i2c6: i2c@600 { > - #address-cells = <1>; > - #size-cells = <0>; > dmas = <0>, <0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_flx6_default>; > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > index 291540e5d81e..551b46894f47 100644 > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > @@ -215,6 +215,8 @@ i2c4: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -284,6 +286,8 @@ i2c5: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -394,6 +398,8 @@ i2c11: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -443,6 +449,8 @@ i2c12: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -600,6 +608,8 @@ i2c6: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -649,6 +659,8 @@ i2c7: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -698,6 +710,8 @@ i2c8: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -766,6 +780,8 @@ i2c0: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -834,6 +850,8 @@ i2c1: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -902,6 +920,8 @@ i2c2: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -970,6 +990,8 @@ i2c3: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { > compatible = "microchip,sam9x60-i2c"; > reg = <0x600 0x200>; > interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; > + #address-cells = <1>; > + #size-cells = <0>; > clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; > dmas = <&dma0 > (AT91_XDMAC_DT_MEM_IF(0) | > > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
On 05.07.2024 09:19, Alexander Dahl wrote: > Hei hei, > > Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: >> These properties are common for all i2c subnodes, and marked as >> 'required' in atmel/microchip i2c bindings. Allows to add i2c device >> nodes (like an rtc for example) in other .dts files including >> sam9x60.dtsi without requiring to repeat these properties for each i2c >> device again and again. >> >> Found on a custom board after adding this in .dts: >> >> &flx5 { >> atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; >> status = "okay"; >> >> i2c5: i2c@600 { >> pinctrl-0 = <&pinctrl_flx5_default>; >> status = "okay"; >> >> pcf8523: rtc@68 { >> compatible = "nxp,pcf8523"; >> reg = <0x68>; >> }; >> }; >> }; >> >> … which created a warning like this: >> >> […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) >> […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' >> […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' >> […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' >> […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 >> […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 >> […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' >> […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' >> […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' >> […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value >> […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value >> […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' >> […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' >> >> This probably should have been done with commit 84f23f3284d5 ("ARM: dts: >> at91: sam9x60: move flexcom definitions") already, where those >> address-cells and size-cells properties were left in the board .dts >> files instead of moving them to the dtsi. > > It's been a while. Is something wrong with the patch? Or with the > commit message? Please CC your patches to proper people (e.g., use ./script/get_maintainer.pl). I see no Microchip AT91 maintainers in the initial to/cc list of your patch. Thank you, Claudiu Beznea > > at91 support seems to have slowed down somehow lately? :-/ > > Greets > Alex > >> >> Signed-off-by: Alexander Dahl <ada@thorsis.com> >> --- >> .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- >> .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- >> arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ >> 3 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts >> index c6fbdd29019f..b9ffd9e5faac 100644 >> --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts >> @@ -198,8 +198,6 @@ i2c0: i2c@600 { >> dmas = <0>, <0>; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_flx0_default>; >> - #address-cells = <1>; >> - #size-cells = <0>; >> i2c-analog-filter; >> i2c-digital-filter; >> i2c-digital-filter-width-ns = <35>; >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts >> index f3cbb675cea4..3b38707d736e 100644 >> --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts >> @@ -207,8 +207,6 @@ &flx0 { >> status = "okay"; >> >> i2c0: i2c@600 { >> - #address-cells = <1>; >> - #size-cells = <0>; >> dmas = <0>, <0>; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_flx0_default>; >> @@ -254,8 +252,6 @@ &flx6 { >> status = "okay"; >> >> i2c6: i2c@600 { >> - #address-cells = <1>; >> - #size-cells = <0>; >> dmas = <0>, <0>; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_flx6_default>; >> diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi >> index 291540e5d81e..551b46894f47 100644 >> --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi >> +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi >> @@ -215,6 +215,8 @@ i2c4: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -284,6 +286,8 @@ i2c5: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -394,6 +398,8 @@ i2c11: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -443,6 +449,8 @@ i2c12: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -600,6 +608,8 @@ i2c6: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -649,6 +659,8 @@ i2c7: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -698,6 +710,8 @@ i2c8: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -766,6 +780,8 @@ i2c0: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -834,6 +850,8 @@ i2c1: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -902,6 +920,8 @@ i2c2: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -970,6 +990,8 @@ i2c3: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { >> compatible = "microchip,sam9x60-i2c"; >> reg = <0x600 0x200>; >> interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; >> dmas = <&dma0 >> (AT91_XDMAC_DT_MEM_IF(0) | >> >> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 >> -- >> 2.39.2 >> >> >
Hello Claudiu, Am Mon, Jul 08, 2024 at 07:23:47PM +0300 schrieb claudiu beznea: > > > On 05.07.2024 09:19, Alexander Dahl wrote: > > Hei hei, > > > > Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: > >> These properties are common for all i2c subnodes, and marked as > >> 'required' in atmel/microchip i2c bindings. Allows to add i2c device > >> nodes (like an rtc for example) in other .dts files including > >> sam9x60.dtsi without requiring to repeat these properties for each i2c > >> device again and again. > >> > >> Found on a custom board after adding this in .dts: > >> > >> &flx5 { > >> atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; > >> status = "okay"; > >> > >> i2c5: i2c@600 { > >> pinctrl-0 = <&pinctrl_flx5_default>; > >> status = "okay"; > >> > >> pcf8523: rtc@68 { > >> compatible = "nxp,pcf8523"; > >> reg = <0x68>; > >> }; > >> }; > >> }; > >> > >> … which created a warning like this: > >> > >> […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) > >> […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' > >> […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > >> […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > >> […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 > >> […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 > >> […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > >> […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > >> […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > >> […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value > >> […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value > >> […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' > >> […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' > >> > >> This probably should have been done with commit 84f23f3284d5 ("ARM: dts: > >> at91: sam9x60: move flexcom definitions") already, where those > >> address-cells and size-cells properties were left in the board .dts > >> files instead of moving them to the dtsi. > > > > It's been a while. Is something wrong with the patch? Or with the > > commit message? > > Please CC your patches to proper people (e.g., use > ./script/get_maintainer.pl). I see no Microchip AT91 maintainers in the > initial to/cc list of your patch. You can be sure I did. This is the list I got on my patch and you see I CCed everone listed as a _maintainer_ from that output: % ./scripts/get_maintainer.pl outgoing/arm-dts-microchip/0001-ARM-dts-microchip-sam9x60-Move-i2c-address-size-to-d.patch Rob Herring <robh@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Krzysztof Kozlowski <krzk+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Nicolas Ferre <nicolas.ferre@microchip.com> (supporter:ARM/Microchip (AT91) SoC support) Alexandre Belloni <alexandre.belloni@bootlin.com> (supporter:ARM/Microchip (AT91) SoC support) Claudiu Beznea <claudiu.beznea@tuxon.dev> (supporter:ARM/Microchip (AT91) SoC support,commit_signer:1/2=50%,authored:1/2=50%,added_lines:32/45=71%,removed_lines:32/45=71%) Not sure why Nicolas, Alexandre, and you are listed as "supporter" only? I think you should have been in the CC list in the first place, sorry about that. Besides, I just noticed arch/arm/boot/dts/microchip/sam9x60.dtsi is not covered by specific matches in MAINTAINERS file, just through a generic fallback for all dts. Lines in question are these, sam9 is not matched: F: arch/arm/boot/dts/microchip/at91* F: arch/arm/boot/dts/microchip/sama* Okay for the next time I will also CC supporters, but I found the output of get_maintainer.pl some kind of confusing here. Greets Alex > > Thank you, > Claudiu Beznea > > > > > at91 support seems to have slowed down somehow lately? :-/ > > > > Greets > > Alex > > > >> > >> Signed-off-by: Alexander Dahl <ada@thorsis.com> > >> --- > >> .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- > >> .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- > >> arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ > >> 3 files changed, 26 insertions(+), 6 deletions(-) > >> > >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > >> index c6fbdd29019f..b9ffd9e5faac 100644 > >> --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts > >> @@ -198,8 +198,6 @@ i2c0: i2c@600 { > >> dmas = <0>, <0>; > >> pinctrl-names = "default"; > >> pinctrl-0 = <&pinctrl_flx0_default>; > >> - #address-cells = <1>; > >> - #size-cells = <0>; > >> i2c-analog-filter; > >> i2c-digital-filter; > >> i2c-digital-filter-width-ns = <35>; > >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > >> index f3cbb675cea4..3b38707d736e 100644 > >> --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts > >> @@ -207,8 +207,6 @@ &flx0 { > >> status = "okay"; > >> > >> i2c0: i2c@600 { > >> - #address-cells = <1>; > >> - #size-cells = <0>; > >> dmas = <0>, <0>; > >> pinctrl-names = "default"; > >> pinctrl-0 = <&pinctrl_flx0_default>; > >> @@ -254,8 +252,6 @@ &flx6 { > >> status = "okay"; > >> > >> i2c6: i2c@600 { > >> - #address-cells = <1>; > >> - #size-cells = <0>; > >> dmas = <0>, <0>; > >> pinctrl-names = "default"; > >> pinctrl-0 = <&pinctrl_flx6_default>; > >> diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi > >> index 291540e5d81e..551b46894f47 100644 > >> --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > >> +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > >> @@ -215,6 +215,8 @@ i2c4: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -284,6 +286,8 @@ i2c5: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -394,6 +398,8 @@ i2c11: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -443,6 +449,8 @@ i2c12: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -600,6 +608,8 @@ i2c6: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -649,6 +659,8 @@ i2c7: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -698,6 +710,8 @@ i2c8: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -766,6 +780,8 @@ i2c0: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -834,6 +850,8 @@ i2c1: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -902,6 +920,8 @@ i2c2: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -970,6 +990,8 @@ i2c3: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { > >> compatible = "microchip,sam9x60-i2c"; > >> reg = <0x600 0x200>; > >> interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; > >> dmas = <&dma0 > >> (AT91_XDMAC_DT_MEM_IF(0) | > >> > >> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > >> -- > >> 2.39.2 > >> > >> > > >
On Tue, Jul 09, 2024 at 12:41:02PM +0200, Alexander Dahl wrote: > Am Mon, Jul 08, 2024 at 07:23:47PM +0300 schrieb claudiu beznea: > > On 05.07.2024 09:19, Alexander Dahl wrote: > > > Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: > > > It's been a while. Is something wrong with the patch? Or with the > > > commit message? > > > > Please CC your patches to proper people (e.g., use > > ./script/get_maintainer.pl). I see no Microchip AT91 maintainers in the > > initial to/cc list of your patch. > > You can be sure I did. This is the list I got on my patch and you see > I CCed everone listed as a _maintainer_ from that output: > > % ./scripts/get_maintainer.pl outgoing/arm-dts-microchip/0001-ARM-dts-microchip-sam9x60-Move-i2c-address-size-to-d.patch > Rob Herring <robh@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) > Krzysztof Kozlowski <krzk+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) > Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) > Nicolas Ferre <nicolas.ferre@microchip.com> (supporter:ARM/Microchip (AT91) SoC support) > Alexandre Belloni <alexandre.belloni@bootlin.com> (supporter:ARM/Microchip (AT91) SoC support) > Claudiu Beznea <claudiu.beznea@tuxon.dev> (supporter:ARM/Microchip (AT91) SoC support,commit_signer:1/2=50%,authored:1/2=50%,added_lines:32/45=71%,removed_lines:32/45=71%) > > Not sure why Nicolas, Alexandre, and you are listed as "supporter" > only? I think you should have been in the CC list in the first place, > sorry about that. You say only, but actually "supporter" is a stronger wording than "maintainer" as it means that the people are actually paid to look after the platform: S: *Status*, one of the following: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do much other than throw the odd patch in. See below.. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. Obsolete: Old code. Something tagged obsolete generally means it has been replaced by a better system and you should be using that. As an aside, it might be a bit inaccurate here though, because Claudiu is not paid to look after AT91 and Alexandre might not be either. > Besides, I just noticed arch/arm/boot/dts/microchip/sam9x60.dtsi is > not covered by specific matches in MAINTAINERS file, just through a > generic fallback for all dts. Lines in question are these, sam9 is > not matched: > > F: arch/arm/boot/dts/microchip/at91* > F: arch/arm/boot/dts/microchip/sama* I think this is a hangover from when all arm dts files used to be in arch/arm/boot/dts/, so the patter was needed to only match stuff the lads cared about. Now that arm is like other architectures and has subdirectories for vendors I think these could be simplified: diff --git a/MAINTAINERS b/MAINTAINERS index 691f98fe8ed3..1d2b201563e7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2534,8 +2534,7 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Supported W: http://www.linux4sam.org T: git git://git.kernel.org/pub/scm/linux/kernel/git/at91/linux.git -F: arch/arm/boot/dts/microchip/at91* -F: arch/arm/boot/dts/microchip/sama* +F: arch/arm/boot/dts/microchip/ F: arch/arm/include/debug/at91.S F: arch/arm/mach-at91/ F: drivers/memory/atmel* > > Okay for the next time I will also CC supporters, but I found the > output of get_maintainer.pl some kind of confusing here. Cheers, Conor.
On 09/07/2024 12:41, Alexander Dahl wrote: > Hello Claudiu, > > Am Mon, Jul 08, 2024 at 07:23:47PM +0300 schrieb claudiu beznea: >> >> >> On 05.07.2024 09:19, Alexander Dahl wrote: >>> Hei hei, >>> >>> Am Tue, May 28, 2024 at 05:31:09PM +0200 schrieb Alexander Dahl: >>>> These properties are common for all i2c subnodes, and marked as >>>> 'required' in atmel/microchip i2c bindings. Allows to add i2c device >>>> nodes (like an rtc for example) in other .dts files including >>>> sam9x60.dtsi without requiring to repeat these properties for each i2c >>>> device again and again. >>>> >>>> Found on a custom board after adding this in .dts: >>>> >>>> &flx5 { >>>> atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; >>>> status = "okay"; >>>> >>>> i2c5: i2c@600 { >>>> pinctrl-0 = <&pinctrl_flx5_default>; >>>> status = "okay"; >>>> >>>> pcf8523: rtc@68 { >>>> compatible = "nxp,pcf8523"; >>>> reg = <0x68>; >>>> }; >>>> }; >>>> }; >>>> >>>> … which created a warning like this: >>>> >>>> […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) >>>> […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' >>>> […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' >>>> […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' >>>> […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 >>>> […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 >>>> […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' >>>> […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' >>>> […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' >>>> […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value >>>> […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value >>>> […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' >>>> […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' >>>> >>>> This probably should have been done with commit 84f23f3284d5 ("ARM: dts: >>>> at91: sam9x60: move flexcom definitions") already, where those >>>> address-cells and size-cells properties were left in the board .dts >>>> files instead of moving them to the dtsi. >>> >>> It's been a while. Is something wrong with the patch? Or with the >>> commit message? >> >> Please CC your patches to proper people (e.g., use >> ./script/get_maintainer.pl). I see no Microchip AT91 maintainers in the >> initial to/cc list of your patch. > > You can be sure I did. This is the list I got on my patch and you see > I CCed everone listed as a _maintainer_ from that output: > > % ./scripts/get_maintainer.pl outgoing/arm-dts-microchip/0001-ARM-dts-microchip-sam9x60-Move-i2c-address-size-to-d.patch > Rob Herring <robh@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) > Krzysztof Kozlowski <krzk+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) > Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) > Nicolas Ferre <nicolas.ferre@microchip.com> (supporter:ARM/Microchip (AT91) SoC support) > Alexandre Belloni <alexandre.belloni@bootlin.com> (supporter:ARM/Microchip (AT91) SoC support) > Claudiu Beznea <claudiu.beznea@tuxon.dev> (supporter:ARM/Microchip (AT91) SoC support,commit_signer:1/2=50%,authored:1/2=50%,added_lines:32/45=71%,removed_lines:32/45=71%) > > Not sure why Nicolas, Alexandre, and you are listed as "supporter" > only? I think you should have been in the CC list in the first place, > sorry about that. > > Besides, I just noticed arch/arm/boot/dts/microchip/sam9x60.dtsi is > not covered by specific matches in MAINTAINERS file, just through a > generic fallback for all dts. Lines in question are these, sam9 is > not matched: > > F: arch/arm/boot/dts/microchip/at91* > F: arch/arm/boot/dts/microchip/sama* > > Okay for the next time I will also CC supporters, but I found the > output of get_maintainer.pl some kind of confusing here. get_maintainers is mostly (for typical cases) used through scripts, like cc-cmd or b4, thus no one cares about actual title. But if supported confused you, then why not checking its meaning in MAINTAINERS file? Best regards, Krzysztof
On 28.05.2024 18:31, Alexander Dahl wrote: > These properties are common for all i2c subnodes, and marked as > 'required' in atmel/microchip i2c bindings. Allows to add i2c device > nodes (like an rtc for example) in other .dts files including > sam9x60.dtsi without requiring to repeat these properties for each i2c > device again and again. > > Found on a custom board after adding this in .dts: > > &flx5 { > atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; > status = "okay"; > > i2c5: i2c@600 { > pinctrl-0 = <&pinctrl_flx5_default>; > status = "okay"; > > pcf8523: rtc@68 { > compatible = "nxp,pcf8523"; > reg = <0x68>; > }; > }; > }; > > … which created a warning like this: > > […]:236.4-17: Warning (reg_format): > /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid > length (4 bytes) (#address-cells == 2, #size-cells == 1) > […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' > […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' > […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' > > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: > Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect > #address-cells for I2C bus also defined at […]:228.16-238.4 > > […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: > Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect > #size-cells for I2C bus also defined at […]:228.16-238.4 > […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' > […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' > […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' > […]:234.19-237.5: Warning (avoid_default_addr_size): > /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default > #address-cells value > […]:234.19-237.5: Warning (avoid_default_addr_size): > /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells > value > […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite > 'avoid_default_addr_size' > […]: Warning (unique_unit_address): Failed prerequisite > 'avoid_default_addr_size' > > This probably should have been done with commit 84f23f3284d5 ("ARM: dts: > at91: sam9x60: move flexcom definitions") already, where those > address-cells and size-cells properties were left in the board .dts > files instead of moving them to the dtsi. > > Signed-off-by: Alexander Dahl <ada@thorsis.com> Applied to at91-dt, thanks! Please note that I've adjusted the commit message to reflect that the props you've adjusted are not required anymore in the validation schema. Thank you, Claudiu Beznea
diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts index c6fbdd29019f..b9ffd9e5faac 100644 --- a/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts +++ b/arch/arm/boot/dts/microchip/at91-sam9x60_curiosity.dts @@ -198,8 +198,6 @@ i2c0: i2c@600 { dmas = <0>, <0>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_flx0_default>; - #address-cells = <1>; - #size-cells = <0>; i2c-analog-filter; i2c-digital-filter; i2c-digital-filter-width-ns = <35>; diff --git a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts index f3cbb675cea4..3b38707d736e 100644 --- a/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts +++ b/arch/arm/boot/dts/microchip/at91-sam9x60ek.dts @@ -207,8 +207,6 @@ &flx0 { status = "okay"; i2c0: i2c@600 { - #address-cells = <1>; - #size-cells = <0>; dmas = <0>, <0>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_flx0_default>; @@ -254,8 +252,6 @@ &flx6 { status = "okay"; i2c6: i2c@600 { - #address-cells = <1>; - #size-cells = <0>; dmas = <0>, <0>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_flx6_default>; diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi index 291540e5d81e..551b46894f47 100644 --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi @@ -215,6 +215,8 @@ i2c4: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <13 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 13>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -284,6 +286,8 @@ i2c5: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <14 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 14>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -394,6 +398,8 @@ i2c11: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <32 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 32>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -443,6 +449,8 @@ i2c12: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <33 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 33>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -600,6 +608,8 @@ i2c6: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <9 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 9>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -649,6 +659,8 @@ i2c7: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <10 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 10>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -698,6 +710,8 @@ i2c8: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <11 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 11>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -766,6 +780,8 @@ i2c0: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <5 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 5>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -834,6 +850,8 @@ i2c1: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <6 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 6>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -902,6 +920,8 @@ i2c2: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <7 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 7>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -970,6 +990,8 @@ i2c3: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <8 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 8>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -1074,6 +1096,8 @@ i2c9: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <15 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 15>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | @@ -1123,6 +1147,8 @@ i2c10: i2c@600 { compatible = "microchip,sam9x60-i2c"; reg = <0x600 0x200>; interrupts = <16 IRQ_TYPE_LEVEL_HIGH 7>; + #address-cells = <1>; + #size-cells = <0>; clocks = <&pmc PMC_TYPE_PERIPHERAL 16>; dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) |
These properties are common for all i2c subnodes, and marked as 'required' in atmel/microchip i2c bindings. Allows to add i2c device nodes (like an rtc for example) in other .dts files including sam9x60.dtsi without requiring to repeat these properties for each i2c device again and again. Found on a custom board after adding this in .dts: &flx5 { atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; status = "okay"; i2c5: i2c@600 { pinctrl-0 = <&pinctrl_flx5_default>; status = "okay"; pcf8523: rtc@68 { compatible = "nxp,pcf8523"; reg = <0x68>; }; }; }; … which created a warning like this: […]:236.4-17: Warning (reg_format): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) […]: Warning (pci_device_reg): Failed prerequisite 'reg_format' […]: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' […]: Warning (simple_bus_reg): Failed prerequisite 'reg_format' […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #address-cells for I2C bus also defined at […]:228.16-238.4 […]/linux-6.6.25/arch/arm/boot/dts/microchip/sam9x60.dtsi:283.19-299.7: Warning (i2c_bus_bridge): /ahb/apb/flexcom@f0004000/i2c@600: incorrect #size-cells for I2C bus also defined at […]:228.16-238.4 […]: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' […]: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge' […]: Warning (spi_bus_reg): Failed prerequisite 'reg_format' […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #address-cells value […]:234.19-237.5: Warning (avoid_default_addr_size): /ahb/apb/flexcom@f0004000/i2c@600/rtc@68: Relying on default #size-cells value […]: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size' […]: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' This probably should have been done with commit 84f23f3284d5 ("ARM: dts: at91: sam9x60: move flexcom definitions") already, where those address-cells and size-cells properties were left in the board .dts files instead of moving them to the dtsi. Signed-off-by: Alexander Dahl <ada@thorsis.com> --- .../dts/microchip/at91-sam9x60_curiosity.dts | 2 -- .../arm/boot/dts/microchip/at91-sam9x60ek.dts | 4 --- arch/arm/boot/dts/microchip/sam9x60.dtsi | 26 +++++++++++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0