* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-05-28 15:31 [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi Alexander Dahl
@ 2024-07-05 6:19 ` Alexander Dahl
2024-07-08 8:20 ` Mihai.Sain
` (2 more replies)
2024-07-08 13:04 ` Nicolas Ferre
` (2 subsequent siblings)
3 siblings, 3 replies; 11+ messages in thread
From: Alexander Dahl @ 2024-07-05 6:19 UTC (permalink / raw)
To: devicetree
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Nicolas Ferre,
linux-arm-kernel, linux-kernel, Mihai Sain
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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-07-05 6:19 ` Alexander Dahl
@ 2024-07-08 8:20 ` Mihai.Sain
2024-07-08 13:15 ` Nicolas Ferre
2024-07-08 16:23 ` claudiu beznea
2 siblings, 0 replies; 11+ messages in thread
From: Mihai.Sain @ 2024-07-08 8:20 UTC (permalink / raw)
To: ada, devicetree, claudiu.beznea
Cc: robh, krzk+dt, conor+dt, Nicolas.Ferre, linux-arm-kernel,
linux-kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-07-05 6:19 ` Alexander Dahl
2024-07-08 8:20 ` Mihai.Sain
@ 2024-07-08 13:15 ` Nicolas Ferre
2024-07-08 16:23 ` claudiu beznea
2 siblings, 0 replies; 11+ messages in thread
From: Nicolas Ferre @ 2024-07-08 13:15 UTC (permalink / raw)
To: devicetree, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-arm-kernel, linux-kernel, Claudiu Beznea
Cc: Manikandan M, Durai Manickam, Mihai Sain, Hari Prasath,
Patrice Vilchez
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
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-07-05 6:19 ` Alexander Dahl
2024-07-08 8:20 ` Mihai.Sain
2024-07-08 13:15 ` Nicolas Ferre
@ 2024-07-08 16:23 ` claudiu beznea
2024-07-09 10:41 ` Alexander Dahl
2 siblings, 1 reply; 11+ messages in thread
From: claudiu beznea @ 2024-07-08 16:23 UTC (permalink / raw)
To: devicetree, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nicolas Ferre, linux-arm-kernel, linux-kernel, Mihai Sain
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
>>
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-07-08 16:23 ` claudiu beznea
@ 2024-07-09 10:41 ` Alexander Dahl
2024-07-09 10:50 ` Conor Dooley
2024-07-09 14:40 ` Krzysztof Kozlowski
0 siblings, 2 replies; 11+ messages in thread
From: Alexander Dahl @ 2024-07-09 10:41 UTC (permalink / raw)
To: claudiu beznea
Cc: devicetree, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nicolas Ferre, linux-arm-kernel, linux-kernel, Mihai Sain
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
> >>
> >>
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-07-09 10:41 ` Alexander Dahl
@ 2024-07-09 10:50 ` Conor Dooley
2024-07-09 14:40 ` Krzysztof Kozlowski
1 sibling, 0 replies; 11+ messages in thread
From: Conor Dooley @ 2024-07-09 10:50 UTC (permalink / raw)
To: claudiu beznea, devicetree, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nicolas Ferre, linux-arm-kernel, linux-kernel,
Mihai Sain
[-- Attachment #1: Type: text/plain, Size: 3772 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-07-09 10:41 ` Alexander Dahl
2024-07-09 10:50 ` Conor Dooley
@ 2024-07-09 14:40 ` Krzysztof Kozlowski
1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-09 14:40 UTC (permalink / raw)
To: claudiu beznea, devicetree, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nicolas Ferre, linux-arm-kernel, linux-kernel,
Mihai Sain
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-05-28 15:31 [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi Alexander Dahl
2024-07-05 6:19 ` Alexander Dahl
@ 2024-07-08 13:04 ` Nicolas Ferre
2024-07-08 16:20 ` claudiu beznea
2024-08-09 6:15 ` claudiu beznea
3 siblings, 0 replies; 11+ messages in thread
From: Nicolas Ferre @ 2024-07-08 13:04 UTC (permalink / raw)
To: Alexander Dahl, devicetree, Claudiu Beznea
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel,
linux-kernel, Manikandan M, Hari Prasath, Durai Manickam
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-05-28 15:31 [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi Alexander Dahl
2024-07-05 6:19 ` Alexander Dahl
2024-07-08 13:04 ` Nicolas Ferre
@ 2024-07-08 16:20 ` claudiu beznea
2024-08-09 6:15 ` claudiu beznea
3 siblings, 0 replies; 11+ messages in thread
From: claudiu beznea @ 2024-07-08 16:20 UTC (permalink / raw)
To: Alexander Dahl, devicetree
Cc: Rob Herring, Conor Dooley, linux-kernel, Krzysztof Kozlowski,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi
2024-05-28 15:31 [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi Alexander Dahl
` (2 preceding siblings ...)
2024-07-08 16:20 ` claudiu beznea
@ 2024-08-09 6:15 ` claudiu beznea
3 siblings, 0 replies; 11+ messages in thread
From: claudiu beznea @ 2024-08-09 6:15 UTC (permalink / raw)
To: Alexander Dahl, devicetree
Cc: Rob Herring, Conor Dooley, linux-kernel, Krzysztof Kozlowski,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 11+ messages in thread