Re: [PATCH v4 6/7] riscv: dts: eswin: add I2C controller support

[Samuel Holland]

Hi Pinkesh,

On 2026-09-30 3:06 PM, Pinkesh Vaghela wrote:
>>> +             aon_i2c0: i2c@51830000 {
>>
>> Please keep nodes sorted (the aon block controllers go below GPIO, etc.).
> 
> As per DTS coding style document [1], "Order of Nodes" states that
> "Nodes on any bus, thus using unit addresses for children, shall be ordered by unit
> address in ascending order. Alternatively for some subarchitectures, nodes of the 
> same type can be grouped together, e.g. all I2C controllers one after another even
> if this breaks unit address ordering."
> 
> Do you still prefer to change order?

Yes. I believe the alternative option is allowed because a few platforms were
already doing things that way, to avoid forcing them to change. However, for a
new platform, it is best to follow the recommended pattern; simple numerical
order is easier to understand, verify, and automate with tooling (which is on
the way).

Regards,
Samuel

> [1] https://docs.kernel.org/devicetree/bindings/dts-coding-style.html
> 
> Regards,
> Pinkesh
> 
>>
>> Regards,
>> Samuel
>>
>>> +                     compatible = "eswin,eic7700-i2c", "snps,designware-i2c";
>>> +                     reg = <0x0 0x51830000 0x0 0x8000>;
>>> +                     interrupts = <290>;
>>> +                     clocks = <&clk EIC7700_CLK_GATE_AON_I2C0_PCLK>;
>>> +                     clock-names = "ref";
>>> +                     clock-frequency = <100000>;
>>> +                     resets = <&reset EIC7700_RESET_ANO0>;
>>> +                     #address-cells = <1>;
>>> +                     #size-cells = <0>;
>>> +                     status = "disabled";
>>> +             };
>>> +
>>> +             aon_i2c1: i2c@51838000 {
>>> +                     compatible = "eswin,eic7700-i2c", "snps,designware-i2c";
>>> +                     reg = <0x0 0x51838000 0x0 0x8000>;
>>> +                     interrupts = <291>;
>>> +                     clocks = <&clk EIC7700_CLK_GATE_AON_I2C1_PCLK>;
>>> +                     clock-names = "ref";
>>> +                     clock-frequency = <100000>;
>>> +                     resets = <&reset EIC7700_RESET_ANO1>;
>>> +                     #address-cells = <1>;
>>> +                     #size-cells = <0>;
>>> +                     status = "disabled";
>>> +             };
>>> +
>>>               gpio@51600000 {
>>>                       compatible = "snps,dw-apb-gpio";
>>>                       reg = <0x0 0x51600000 0x0 0x80>;
>