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

From: Krzysztof Kozlowski
Date: Tue Jul 09 2024 - 10:40:41 EST


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