Re: [RFC] sti: Conflict in node name for an IP supporting both I2C and SPI

[Alain Volmat <avolmat@me.com>]

Hello,

On Fri, Nov 26, 2021 at 11:47:55PM +0100, Andrew Lunn wrote:
> On Thu, Nov 25, 2021 at 10:04:28PM +0100, Alain Volmat wrote:
> > Hello,
> > 
> > in the STi platform [1], the I2C and SPI controllers are handled by the
> > same IP, which can be configured in either one or the other mode.
> > This leads to warnings during the DT build and I was wondering if you could
> > give me some hints about how such situation should be handled since this
> > concern DT warnings but also bindings and YAML.
> > 
> > In the SoC DT (dtsi), for each IP, there are 2 entries:
> > 
> > One for the I2C mode (implemented by the driver i2c/busses/i2c-st.c)
> >                 i2c@9840000 {
> >                         compatible = "st,comms-ssc4-i2c";
> >                         interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> >                         reg = <0x9840000 0x110>;
> >                         clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> >                         clock-names = "ssc";
> >                         clock-frequency = <400000>;
> >                         pinctrl-names = "default";
> >                         pinctrl-0 = <&pinctrl_i2c0_default>;
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> > 
> >                         status = "disabled";
> >                 };
> > 
> > One for the SPI mode (implemented by the driver spi/spi-st-ssc4.c)
> >                 spi@9840000 {
> >                         compatible = "st,comms-ssc4-spi";
> >                         reg = <0x9840000 0x110>;
> >                         interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> >                         clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>;
> >                         clock-names = "ssc";
> >                         pinctrl-0 = <&pinctrl_spi0_default>;
> >                         pinctrl-names = "default";
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> > 
> >                         status = "disabled";
> >                 };
> > 
> > So basically, there are 2 nodes, one for each mode, and enabling one or the
> > other mode is done within the board DT.
> > Since the address is the same, this obviously leads to warning during the build
> > of the DT.
> > 
> > arch/arm/boot/dts/stih407-family.dtsi:363.15-376.5: Warning (unique_unit_address): /soc/i2c@9840000: duplicate unit-address (also used in node /soc/spi@9840000)
> 
> How about making the compiler look at the status value. So long as
> only zero or one is enabled, it should not be an issue. If you have
> two or more nodes enabled for an address, then you want a warning or
> error.

From the compiler point of view it seems this behavior is already
possible, probably with something like:
   -Wno-unique_unit_address -Wunique_unit_address_if_enabled

Rob, is current behavior (checking unique unit address even if node
is disabled) on purpose or could this be changed to only checking if the
node is enabled ?

Alain

> 
>      Andrew
>