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

[Krzysztof Kozlowski]

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@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.

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