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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20211129112824.GA17122@gnbcxl0045.gnb.st.com>
Date:   Mon, 29 Nov 2021 12:28:24 +0100
From:   Alain Volmat <avolmat@...com>
To:     Andrew Lunn <andrew@...n.ch>, robh+dt@...nel.org
Cc:     wsa@...nel.org, linux-spi@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Patrice Chotard <patrice.chotard@...s.st.com>,
        Alain Volmat <avolmat@...com>
Subject: Re: [RFC] sti: Conflict in node name for an IP supporting both I2C
 and SPI

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@...0000 {
> >                         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@...0000 {
> >                         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@...0000: duplicate unit-address (also used in node /soc/spi@...0000)
> 
> 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
>