| Message ID | 20240919134732.2626144-3-andrei.stefanescu@oss.nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: siul2-s32g2: add initial GPIO driver | expand |
Hi Conor, Thank you for your review! On 20/09/2024 15:46, Conor Dooley wrote: > On Thu, Sep 19, 2024 at 04:47:22PM +0300, Andrei Stefanescu wrote: >> Add support for the GPIO driver of the NXP S32G2/S32G3 SoCs. >> >> Signed-off-by: Phu Luu An <phu.luuan@nxp.com> >> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com> >> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> >> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> >> --- >> .../bindings/gpio/nxp,s32g2-siul2-gpio.yaml | 107 ++++++++++++++++++ >> 1 file changed, 107 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml >> >> diff --git a/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml >> new file mode 100644 >> index 000000000000..0548028e6745 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml >> @@ -0,0 +1,107 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause >> +# Copyright 2024 NXP >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/gpio/nxp,s32g2-siul2-gpio.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: NXP S32G2 SIUL2 GPIO controller >> + >> +maintainers: >> + - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com> >> + - Larisa Grigore <larisa.grigore@nxp.com> >> + - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> >> + >> +description: >> + Support for the SIUL2 GPIOs found on the S32G2 and S32G3 >> + chips. It includes an IRQ controller for all pins which have >> + an EIRQ associated. >> + >> +properties: >> + compatible: >> + items: >> + - const: nxp,s32g2-siul2-gpio > > Commit message and binding description say s32g2 and s32g3, but there's > only a compatible here for g2. Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan to reuse the same compatible when I add the SIUL2 GPIO device tree node for the S32G3 boards. Would that be ok? Best regards, Andrei
On Fri, Sep 20, 2024 at 03:40:31PM +0200, Krzysztof Kozlowski wrote: > On 20/09/2024 15:33, Andrei Stefanescu wrote: > > Hi Conor, > > > > Thank you for your review! > > > > On 20/09/2024 15:46, Conor Dooley wrote: > >> On Thu, Sep 19, 2024 at 04:47:22PM +0300, Andrei Stefanescu wrote: > >>> Add support for the GPIO driver of the NXP S32G2/S32G3 SoCs. > >>> > >>> Signed-off-by: Phu Luu An <phu.luuan@nxp.com> > >>> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com> > >>> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > >>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> > >>> --- > >>> .../bindings/gpio/nxp,s32g2-siul2-gpio.yaml | 107 ++++++++++++++++++ > >>> 1 file changed, 107 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > >>> new file mode 100644 > >>> index 000000000000..0548028e6745 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > >>> @@ -0,0 +1,107 @@ > >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause > >>> +# Copyright 2024 NXP > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/gpio/nxp,s32g2-siul2-gpio.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: NXP S32G2 SIUL2 GPIO controller > >>> + > >>> +maintainers: > >>> + - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com> > >>> + - Larisa Grigore <larisa.grigore@nxp.com> > >>> + - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> > >>> + > >>> +description: > >>> + Support for the SIUL2 GPIOs found on the S32G2 and S32G3 > >>> + chips. It includes an IRQ controller for all pins which have > >>> + an EIRQ associated. > >>> + > >>> +properties: > >>> + compatible: > >>> + items: > >>> + - const: nxp,s32g2-siul2-gpio > >> > >> Commit message and binding description say s32g2 and s32g3, but there's > >> only a compatible here for g2. > > > > Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan > > to reuse the same compatible when I add the SIUL2 GPIO device tree node for > > the S32G3 boards. Would that be ok? > > There are only few exceptions where re-using compatible is allowed. Was > S32G on them? Please consult existing practice/maintainers and past reviews. Pretty sure I had a similar conversation about another peripheral on these devices, and it was established that these are not different fusings etc, but rather are independent SoCs that reuse an IP core. Given that, I'd expect to see a fallback compatible used here, as is the norm. Cheers, Conor.
On Sat, Sep 21, 2024 at 10:58:46PM +0100, Conor Dooley wrote: > On Fri, Sep 20, 2024 at 03:40:31PM +0200, Krzysztof Kozlowski wrote: > > On 20/09/2024 15:33, Andrei Stefanescu wrote: > > > Hi Conor, > > > > > > Thank you for your review! > > > > > > On 20/09/2024 15:46, Conor Dooley wrote: > > >> On Thu, Sep 19, 2024 at 04:47:22PM +0300, Andrei Stefanescu wrote: > > >>> Add support for the GPIO driver of the NXP S32G2/S32G3 SoCs. > > >>> > > >>> Signed-off-by: Phu Luu An <phu.luuan@nxp.com> > > >>> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com> > > >>> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > > >>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> > > >>> --- > > >>> .../bindings/gpio/nxp,s32g2-siul2-gpio.yaml | 107 ++++++++++++++++++ > > >>> 1 file changed, 107 insertions(+) > > >>> create mode 100644 Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > > >>> new file mode 100644 > > >>> index 000000000000..0548028e6745 > > >>> --- /dev/null > > >>> +++ b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > > >>> @@ -0,0 +1,107 @@ > > >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause > > >>> +# Copyright 2024 NXP > > >>> +%YAML 1.2 > > >>> +--- > > >>> +$id: http://devicetree.org/schemas/gpio/nxp,s32g2-siul2-gpio.yaml# > > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >>> + > > >>> +title: NXP S32G2 SIUL2 GPIO controller > > >>> + > > >>> +maintainers: > > >>> + - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com> > > >>> + - Larisa Grigore <larisa.grigore@nxp.com> > > >>> + - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> > > >>> + > > >>> +description: > > >>> + Support for the SIUL2 GPIOs found on the S32G2 and S32G3 > > >>> + chips. It includes an IRQ controller for all pins which have > > >>> + an EIRQ associated. > > >>> + > > >>> +properties: > > >>> + compatible: > > >>> + items: > > >>> + - const: nxp,s32g2-siul2-gpio > > >> > > >> Commit message and binding description say s32g2 and s32g3, but there's > > >> only a compatible here for g2. > > > > > > Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan > > > to reuse the same compatible when I add the SIUL2 GPIO device tree node for > > > the S32G3 boards. Would that be ok? > > > > There are only few exceptions where re-using compatible is allowed. Was > > S32G on them? Please consult existing practice/maintainers and past reviews. Just in case this was not clear - comment "please consult existing..." was towards Andrei, not you Conor. > > Pretty sure I had a similar conversation about another peripheral on > these devices, and it was established that these are not different fusings > etc, but rather are independent SoCs that reuse an IP core. Given that, > I'd expect to see a fallback compatible used here, as is the norm. Yep. Best regards, Krzysztof
On Thu, Sep 19, 2024 at 04:47:22PM +0300, Andrei Stefanescu wrote: > Add support for the GPIO driver of the NXP S32G2/S32G3 SoCs. > > Signed-off-by: Phu Luu An <phu.luuan@nxp.com> > Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com> > Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> > --- > .../bindings/gpio/nxp,s32g2-siul2-gpio.yaml | 107 ++++++++++++++++++ > 1 file changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > > diff --git a/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > new file mode 100644 > index 000000000000..0548028e6745 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause Different license - see checkpatch. Best regards, Krzysztof
On Sun, Sep 22, 2024 at 11:04:22PM +0200, Krzysztof Kozlowski wrote: > On Sat, Sep 21, 2024 at 10:58:46PM +0100, Conor Dooley wrote: > > On Fri, Sep 20, 2024 at 03:40:31PM +0200, Krzysztof Kozlowski wrote: > > > On 20/09/2024 15:33, Andrei Stefanescu wrote: > > > >>> +properties: > > > >>> + compatible: > > > >>> + items: > > > >>> + - const: nxp,s32g2-siul2-gpio > > > >> > > > >> Commit message and binding description say s32g2 and s32g3, but there's > > > >> only a compatible here for g2. > > > > > > > > Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan > > > > to reuse the same compatible when I add the SIUL2 GPIO device tree node for > > > > the S32G3 boards. Would that be ok? > > > > > > There are only few exceptions where re-using compatible is allowed. Was > > > S32G on them? Please consult existing practice/maintainers and past reviews. > > Just in case this was not clear - comment "please consult existing..." > was towards Andrei, not you Conor. Oh I know, I was just passing through and figured I may as well leave a comment repeating what I said on the other devices :) > > Pretty sure I had a similar conversation about another peripheral on > > these devices, and it was established that these are not different fusings > > etc, but rather are independent SoCs that reuse an IP core. Given that, > > I'd expect to see a fallback compatible used here, as is the norm. > > Yep. > > Best regards, > Krzysztof >
Hi, On 23/09/2024 00:07, Conor Dooley wrote: > On Sun, Sep 22, 2024 at 11:04:22PM +0200, Krzysztof Kozlowski wrote: >> On Sat, Sep 21, 2024 at 10:58:46PM +0100, Conor Dooley wrote: >>> On Fri, Sep 20, 2024 at 03:40:31PM +0200, Krzysztof Kozlowski wrote: >>>> On 20/09/2024 15:33, Andrei Stefanescu wrote: > >>>>>>> +properties: >>>>>>> + compatible: >>>>>>> + items: >>>>>>> + - const: nxp,s32g2-siul2-gpio >>>>>> >>>>>> Commit message and binding description say s32g2 and s32g3, but there's >>>>>> only a compatible here for g2. >>>>> >>>>> Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan >>>>> to reuse the same compatible when I add the SIUL2 GPIO device tree node for >>>>> the S32G3 boards. Would that be ok? >>>> >>>> There are only few exceptions where re-using compatible is allowed. Was >>>> S32G on them? Please consult existing practice/maintainers and past reviews. I will add another compatible: "nxp,s32g3-siul2-gpio" for the S32G3 SoC. We currently also have the SIUL2 pinctrl driver in upstream with only one compatible: "nxp,s32g2-siul2-pinctrl". Should I also send a separate patch to add an S32G3 compatible to it? >> >> Just in case this was not clear - comment "please consult existing..." >> was towards Andrei, not you Conor. > > Oh I know, I was just passing through and figured I may as well leave a > comment repeating what I said on the other devices :) > >>> Pretty sure I had a similar conversation about another peripheral on >>> these devices, and it was established that these are not different fusings >>> etc, but rather are independent SoCs that reuse an IP core. Given that, >>> I'd expect to see a fallback compatible used here, as is the norm. >> >> Yep. >> >> Best regards, >> Krzysztof >> Best regards, Andrei
Hi Krzysztof, On 23/09/2024 00:04, Krzysztof Kozlowski wrote: > On Thu, Sep 19, 2024 at 04:47:22PM +0300, Andrei Stefanescu wrote: >> Add support for the GPIO driver of the NXP S32G2/S32G3 SoCs. >> >> Signed-off-by: Phu Luu An <phu.luuan@nxp.com> >> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com> >> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> >> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> >> --- >> .../bindings/gpio/nxp,s32g2-siul2-gpio.yaml | 107 ++++++++++++++++++ >> 1 file changed, 107 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml >> >> diff --git a/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml >> new file mode 100644 >> index 000000000000..0548028e6745 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml >> @@ -0,0 +1,107 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause > > Different license - see checkpatch. > Thank you for pointing it out! I will fix it in v4. > Best regards, > Krzysztof Best regards, Andrei
On Mon, Sep 23, 2024 at 01:47:25PM +0300, Andrei Stefanescu wrote: > Hi, > > On 23/09/2024 00:07, Conor Dooley wrote: > > On Sun, Sep 22, 2024 at 11:04:22PM +0200, Krzysztof Kozlowski wrote: > >> On Sat, Sep 21, 2024 at 10:58:46PM +0100, Conor Dooley wrote: > >>> On Fri, Sep 20, 2024 at 03:40:31PM +0200, Krzysztof Kozlowski wrote: > >>>> On 20/09/2024 15:33, Andrei Stefanescu wrote: > > > >>>>>>> +properties: > >>>>>>> + compatible: > >>>>>>> + items: > >>>>>>> + - const: nxp,s32g2-siul2-gpio > >>>>>> > >>>>>> Commit message and binding description say s32g2 and s32g3, but there's > >>>>>> only a compatible here for g2. > >>>>> > >>>>> Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan > >>>>> to reuse the same compatible when I add the SIUL2 GPIO device tree node for > >>>>> the S32G3 boards. Would that be ok? > >>>> > >>>> There are only few exceptions where re-using compatible is allowed. Was > >>>> S32G on them? Please consult existing practice/maintainers and past reviews. > > I will add another compatible: "nxp,s32g3-siul2-gpio" for the S32G3 SoC. We currently > also have the SIUL2 pinctrl driver in upstream with only one compatible: > "nxp,s32g2-siul2-pinctrl". Should I also send a separate patch to add an S32G3 compatible > to it? > That would be great, thanks.
On Mon, Sep 23, 2024 at 10:34:05PM +0100, Conor Dooley wrote: > On Mon, Sep 23, 2024 at 01:47:25PM +0300, Andrei Stefanescu wrote: > > Hi, > > > > On 23/09/2024 00:07, Conor Dooley wrote: > > > On Sun, Sep 22, 2024 at 11:04:22PM +0200, Krzysztof Kozlowski wrote: > > >> On Sat, Sep 21, 2024 at 10:58:46PM +0100, Conor Dooley wrote: > > >>> On Fri, Sep 20, 2024 at 03:40:31PM +0200, Krzysztof Kozlowski wrote: > > >>>> On 20/09/2024 15:33, Andrei Stefanescu wrote: > > > > > >>>>>>> +properties: > > >>>>>>> + compatible: > > >>>>>>> + items: > > >>>>>>> + - const: nxp,s32g2-siul2-gpio > > >>>>>> > > >>>>>> Commit message and binding description say s32g2 and s32g3, but there's > > >>>>>> only a compatible here for g2. > > >>>>> > > >>>>> Yes, the SIUL2 GPIO hardware is the same for both S32G2 and S32G3 SoCs. I plan > > >>>>> to reuse the same compatible when I add the SIUL2 GPIO device tree node for > > >>>>> the S32G3 boards. Would that be ok? > > >>>> > > >>>> There are only few exceptions where re-using compatible is allowed. Was > > >>>> S32G on them? Please consult existing practice/maintainers and past reviews. > > > > I will add another compatible: "nxp,s32g3-siul2-gpio" for the S32G3 SoC. We currently > > also have the SIUL2 pinctrl driver in upstream with only one compatible: > > "nxp,s32g2-siul2-pinctrl". Should I also send a separate patch to add an S32G3 compatible > > to it? > > > > That would be great, thanks. Wait, the driver doesn't need to have 2 compatibles, only the binding does. Make the g3 compatible fall back to the g2 one, and the driver doesn't need to change.
diff --git a/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml new file mode 100644 index 000000000000..0548028e6745 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/nxp,s32g2-siul2-gpio.yaml @@ -0,0 +1,107 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause +# Copyright 2024 NXP +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpio/nxp,s32g2-siul2-gpio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP S32G2 SIUL2 GPIO controller + +maintainers: + - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com> + - Larisa Grigore <larisa.grigore@nxp.com> + - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> + +description: + Support for the SIUL2 GPIOs found on the S32G2 and S32G3 + chips. It includes an IRQ controller for all pins which have + an EIRQ associated. + +properties: + compatible: + items: + - const: nxp,s32g2-siul2-gpio + + reg: + items: + - description: PGPDO (output value) registers for SIUL2_0 + - description: PGPDO (output value) registers for SIUL2_1 + - description: PGPDI (input value) registers for SIUL2_0 + - description: PGPDI (input value) registers for SIUL2_1 + - description: EIRQ (interrupt) configuration registers from SIUL2_1 + - description: EIRQ IMCR registers for interrupt muxing between pads + + reg-names: + items: + - const: opads0 + - const: opads1 + - const: ipads0 + - const: ipads1 + - const: eirqs + - const: eirq-imcrs + + gpio-controller: true + + '#gpio-cells': + const: 2 + + interrupts: + maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": + const: 2 + + gpio-ranges: + minItems: 2 + maxItems: 2 + + gpio-reserved-ranges: + minItems: 2 + +patternProperties: + "-hog(-[0-9]+)?$": + required: + - gpio-hog + +required: + - compatible + - reg + - reg-names + - gpio-controller + - "#gpio-cells" + - gpio-ranges + - gpio-reserved-ranges + - interrupts + - interrupt-controller + - "#interrupt-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + gpio@4009d700 { + compatible = "nxp,s32g2-siul2-gpio"; + reg = <0x4009d700 0x10>, + <0x44011700 0x18>, + <0x4009d740 0x10>, + <0x44011740 0x18>, + <0x44010010 0xb4>, + <0x44011078 0x80>; + reg-names = "opads0", "opads1", "ipads0", + "ipads1", "eirqs", "eirq-imcrs"; + gpio-controller; + #gpio-cells = <2>; + /* GPIO 0-101 */ + gpio-ranges = <&pinctrl 0 0 102>, + /* GPIO 112-190 */ + <&pinctrl 112 112 79>; + gpio-reserved-ranges = <102 10>, <123 21>; + interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <2>; + };