linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] sti: Conflict in node name for an IP supporting both I2C and SPI
@ 2021-11-25 21:04 Alain Volmat
  2021-11-26 22:47 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Alain Volmat @ 2021-11-25 21:04 UTC (permalink / raw)
  To: wsa, broonie, robh+dt
  Cc: linux-i2c, linux-spi, linux-kernel, linux-arm-kernel, Patrice Chotard

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)

In order to fix this, I could think of only having one node (for example
I2C) in the SoC DT (dtsi) and then, within each board DT override some of the
properties (including the compatible) to make it be a SPI node when necessary.
However in such case I think this would leads to an issue regarding Yaml.
The YAML for I2C controller [2] mentions that the node name pattern should
be "^i2c(@.*)?", while in case of a SPI controller [3] it should be
"^spi(@.*|-[0-9a-f])*$".
For that reason, this way doesn't seem possible.

What would you advice to handle such case ?

Regards,
Alain

[1] arch/arm/boot/dts/stih407-family.dtsi
[2] schemas/i2c/i2c-controller.yaml
[3] Documentation/devicetree/bindings/spi/spi-controller.yaml

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] sti: Conflict in node name for an IP supporting both I2C and SPI
  2021-11-25 21:04 [RFC] sti: Conflict in node name for an IP supporting both I2C and SPI Alain Volmat
@ 2021-11-26 22:47 ` Andrew Lunn
  2021-11-29 11:28   ` Alain Volmat
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2021-11-26 22:47 UTC (permalink / raw)
  To: wsa, broonie, robh+dt, linux-i2c, linux-spi, linux-kernel,
	linux-arm-kernel, Patrice Chotard

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.

     Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] sti: Conflict in node name for an IP supporting both I2C and SPI
  2021-11-26 22:47 ` Andrew Lunn
@ 2021-11-29 11:28   ` Alain Volmat
  0 siblings, 0 replies; 3+ messages in thread
From: Alain Volmat @ 2021-11-29 11:28 UTC (permalink / raw)
  To: Andrew Lunn, robh+dt
  Cc: wsa, linux-spi, linux-kernel, linux-arm-kernel, Patrice Chotard,
	Alain Volmat

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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-29 11:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 21:04 [RFC] sti: Conflict in node name for an IP supporting both I2C and SPI Alain Volmat
2021-11-26 22:47 ` Andrew Lunn
2021-11-29 11:28   ` Alain Volmat

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).