Re: [PATCH] ARM: dts: microchip: sam9x60: Move i2c address/size to dtsi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
    Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
    Conor Dooley <conor+dt@xxxxxxxxxx> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
    Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx> (supporter:ARM/Microchip (AT91) SoC support)
    Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> (supporter:ARM/Microchip (AT91) SoC support)
    Claudiu Beznea <claudiu.beznea@xxxxxxxxx> (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@xxxxxxxxxxx>
> >> ---
> >>  .../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
> >>
> >>
> > 
> 




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux