linux-kernel - Re: [PATCH 2/9] dt-bindings: fsl,fpga-qixis-i2c: extend support to also cover the LX2160ARDB FPGA

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <6qeqx64uwhkooe3f6g2eid567rg4ubh6djdtybwlg3oc4xdyaf@sllekrl7wzea>
Date: Wed, 9 Jul 2025 17:31:51 +0300
From: Ioana Ciornei <ioana.ciornei@....com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-gpio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Shawn Guo <shawnguo@...nel.org>, 
	Michael Walle <mwalle@...nel.org>, Lee Jones <lee@...nel.org>, Frank Li <Frank.Li@....com>
Subject: Re: [PATCH 2/9] dt-bindings: fsl,fpga-qixis-i2c: extend support to
 also cover the LX2160ARDB FPGA

On Wed, Jul 09, 2025 at 02:17:27PM +0200, Krzysztof Kozlowski wrote:
> On 09/07/2025 13:26, Ioana Ciornei wrote:
> > Extend the list of supported compatible strings with fsl,lx2160ardb-fpga.
> > 
> > Since the register map exposed by the LX2160ARDB's FPGA also contains
> > two GPIO controllers, accept the necessary GPIO pattern property. At the
> > same time, add the #address-cells and #size-cells properties as valid
> > ones.
> > 
> > This is needed because when defining child devices such as the GPIO
> > controller described in the added example, the child device needs a the
> > reg property to properly identify its register location.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@....com>
> > ---
> >  .../bindings/board/fsl,fpga-qixis-i2c.yaml    | 35 +++++++++++++++++++
> 
> So here is the board? Why FPGA is in the board...

I think because its usage and integration is very much dependant on the
board? I am really not sure why it was added there in the first place as
a .txt file.

> 
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> > index 28b37772fb65..e8981f974210 100644
> > --- a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> > @@ -22,6 +22,13 @@ properties:
> >                - fsl,lx2160aqds-fpga
> >            - const: fsl,fpga-qixis-i2c
> >            - const: simple-mfd
> > +      - const: fsl,lx2160ardb-fpga
> 
> Weird, your first patch added three compatibles, this adds only one.

The first patch added 3 compatibles for the registers exposed by this
FPGA that act as a GPIO controller. There are 3 compatibles and not just
one because the registers backing them have different layouts, each
exposing different control/status bits.
As you have pointed out in your last reply on patch 1/9, two of those
compatibles can be merged into a single one.

In this patch I am adding a new compatible for the QIXIS FPGA found on
the LX2160ARDB board so that the simple-mfd-i2c driver has something to
probe on and expose its regmap to the child devices - the gpio
controllers.

> 
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> >  
> >    interrupts:
> >      maxItems: 1
> > @@ -32,6 +39,10 @@ properties:
> >    mux-controller:
> >      $ref: /schemas/mux/reg-mux.yaml
> >  
> > +patternProperties:
> > +  "^gpio(@[0-9a-f]+)?$":
> 
> Why unit address is optional? Anyway, this is wrong. You do not have
> ranges here and earlier you already said children do not have any
> addressing. Look at mux.

Agree on the '?' not being needed here since my plan is to enforce that
if the dts has a GPIO controller defined as a child device then it needs
a unit address.

The unit address is there to convey to the driver what is the address of
the register backing the GPIO controller. I am not sure how else could I
cleanly do that.

My current plan is to:
 - Not change how the board DT files which already define their QIXIS FPGAs
   look like, meaning that they will keep their FPGA child nodes without
   addressing. Very much like the mux is used currently in the
   fsl-lx2160a-qds.dts.
 - For any new boards that need a GPIO driver to be probed on one of the
   FPGA's registers, impose the use of the unit address.

I acknowledge the fact that this a bit confusing, I am open to
suggestions, but I currently do not know another way forward which
cleanly does what I need.

> 
> > +    $ref: /schemas/gpio/fsl,fpga-gpio.yaml
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -68,3 +79,27 @@ examples:
> >          };
> >      };
> >  
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        board-control@66 {
> > +            compatible = "fsl,lx2160ardb-fpga";
> > +            reg = <0x66>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            gpio@19 {
> 
> And what is the meaning of @19?

The register found at address 0x19 is the one backing this GPIO
controller.