Re: [PATCH v9 5/7] dt-bindings: pinctrl: s32g2-siul2: describe GPIO and EIRQ resources

[Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>]

On 5/5/2026 9:36 AM, Krzysztof Kozlowski wrote:
> On Mon, May 04, 2026 at 03:11:46PM +0200, Khristine Andreea Barbulescu wrote:
>> Extend the S32G2 SIUL2 pinctrl binding to describe the additional
>> resources used by the updated SIUL2 pinctrl driver.
> 
> Please describe hardware, not drivers. Statement is not even true -
> drivers do not use these resources, unless you organized your patchset
> wrong (see submitting bindings documents, both).
> 
> Nothing above explains why you need new compatible.
> 
>>
>> Besides the MSCR and IMCR register ranges used for pinmux and
>> pin configuration, the SIUL2 block also contains PGPDO and PGPDI
>> registers for GPIO output and input operations, as well as EIRQ
>> registers used for external interrupt configuration.
>>
>> Add GPIO controller properties:
>>   - gpio-controller
>>   - #gpio-cells
>>   - gpio-ranges
>>
>> Add interrupt controller properties:
>>   - interrupt-controller
>>   - #interrupt-cells
>>   - interrupts
> 
> Do not explain what you did, but say why.
> 
>>
>> Also update the binding example to show the complete SIUL2 register
>> layout, including the GPIO data and EIRQ register windows.
>>
>> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
>> ---
>>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 107 ++++++++++++++++--
>>  1 file changed, 98 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>> index a24286e4def6..0bd341f1e84d 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>> @@ -1,5 +1,5 @@
>>  # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> -# Copyright 2022 NXP
>> +# Copyright 2022, 2026 NXP
>>  %YAML 1.2
>>  ---
>>  $id: http://devicetree.org/schemas/pinctrl/nxp,s32g2-siul2-pinctrl.yaml#
>> @@ -17,26 +17,39 @@ description: |
>>      SIUL2_0 @ 0x4009c000
>>      SIUL2_1 @ 0x44010000
>>  
>> -  Every SIUL2 region has multiple register types, and here only MSCR and
>> -  IMCR registers need to be revealed for kernel to configure pinmux.
>> +  Every SIUL2 region has multiple register types. MSCR and IMCR registers
>> +  need to be revealed for kernel to configure pinmux. PGPDO and PGPDI
>> +  registers are used for GPIO output/input operations. EIRQ registers
>> +  are used for external interrupt configuration.
>>  
>>    Please note that some register indexes are reserved in S32G2, such as
>>    MSCR102-MSCR111, MSCR123-MSCR143, IMCR84-IMCR118 and IMCR398-IMCR429.
>>  
>>  properties:
>>    compatible:
>> -    enum:
>> -      - nxp,s32g2-siul2-pinctrl
>> +    oneOf:
>> +      - const: nxp,s32g2-siul2-pinctrl
>> +      - items:
>> +          - const: nxp,s32g2-siul2-pinctrl-gpio
>> +          - const: nxp,s32g2-siul2-pinctrl
> 
> I don't get how this binding develops. You were asked to grow existing
> binding instead of deprecating it and now you did not grow it: you added
> completely new compatible.
> 
> Please go back to previous version comments.
> 
> Best regards,
> Krzysztof
> 

Hi Krzysztof,

Thanks for your review.

I will rework the binding and the commit message
accordingly in the next version.

Best regards,
Khristine