[PATCH 1/2] dt-bindings: gpio: nxp,pcf8575: add reset GPIO

Quentin Schulz posted 2 patches 10 months ago
There is a newer version of this series
[PATCH 1/2] dt-bindings: gpio: nxp,pcf8575: add reset GPIO
Posted by Quentin Schulz 10 months ago
From: Quentin Schulz <quentin.schulz@cherry.de>

A few of the I2C GPIO expander chips supported by this binding have a
RESETN pin to be able to reset the chip. The chip is held in reset while
the pin is low, therefore the polarity of reset-gpios is expected to
reflect that, i.e. a GPIO_ACTIVE_HIGH means the GPIO will be held low
for reset and released high, GPIO_ACTIVE_LOW means the GPIO will be held
high for reset and released low.

Out of the supported chips, only PCA9670, PCA9671, PCA9672 and PCA9673
show a RESETN pin in their datasheets. They all share the same reset
timings, that is 4+us reset pulse[0] and 100+us reset time[0].

When performing a reset, "The PCA9670 registers and I2C-bus state
machine will be held in their default state until the RESET input is
once again HIGH."[1] meaning we now know the state of each line
controlled by the GPIO expander. Therefore, setting lines-initial-states
and reset-gpios both does not make sense and their presence is XOR'ed.

[0] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf Fig 22.
[1] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf 8.5

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 .../devicetree/bindings/gpio/nxp,pcf8575.yaml      | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml b/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
index 3718103e966a13e1d77f73335ff73c18a3199469..d08d3f848f82e74de949da16d26a810dc52a74e5 100644
--- a/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
+++ b/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
@@ -73,6 +73,39 @@ properties:
 
   wakeup-source: true
 
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO controlling the (reset active LOW) RESET# pin.
+
+      Performing a reset makes all lines initialized to their input (pulled-up)
+      state.
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              enum:
+                - nxp,pca9670
+                - nxp,pca9671
+                - nxp,pca9672
+                - nxp,pca9673
+    then:
+      properties:
+        reset-gpios: false
+
+  # lines-initial-states XOR reset-gpios
+  # Performing a reset reinitializes all lines to a known state which
+  # may not match passed lines-initial-states
+  - if:
+      required:
+        - lines-initial-states
+    then:
+      properties:
+        reset-gpios: false
+
 patternProperties:
   "^(.+-hog(-[0-9]+)?)$":
     type: object

-- 
2.48.1
Re: [PATCH 1/2] dt-bindings: gpio: nxp,pcf8575: add reset GPIO
Posted by Heiko Stübner 10 months ago
Am Donnerstag, 20. Februar 2025, 10:56:51 MEZ schrieb Quentin Schulz:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> A few of the I2C GPIO expander chips supported by this binding have a
> RESETN pin to be able to reset the chip. The chip is held in reset while
> the pin is low, therefore the polarity of reset-gpios is expected to
> reflect that, i.e. a GPIO_ACTIVE_HIGH means the GPIO will be held low
> for reset and released high, GPIO_ACTIVE_LOW means the GPIO will be held
> high for reset and released low.
> 
> Out of the supported chips, only PCA9670, PCA9671, PCA9672 and PCA9673
> show a RESETN pin in their datasheets. They all share the same reset
> timings, that is 4+us reset pulse[0] and 100+us reset time[0].
> 
> When performing a reset, "The PCA9670 registers and I2C-bus state
> machine will be held in their default state until the RESET input is
> once again HIGH."[1] meaning we now know the state of each line
> controlled by the GPIO expander. Therefore, setting lines-initial-states
> and reset-gpios both does not make sense and their presence is XOR'ed.
> 
> [0] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf Fig 22.
> [1] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf 8.5
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  .../devicetree/bindings/gpio/nxp,pcf8575.yaml      | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml b/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
> index 3718103e966a13e1d77f73335ff73c18a3199469..d08d3f848f82e74de949da16d26a810dc52a74e5 100644
> --- a/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
> +++ b/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
[...]
> +  # lines-initial-states XOR reset-gpios
> +  # Performing a reset reinitializes all lines to a known state which
> +  # may not match passed lines-initial-states
> +  - if:
> +      required:
> +        - lines-initial-states
> +    then:
> +      properties:
> +        reset-gpios: false
> +

exclusion logic
Tested-by: Heiko Stuebner <heiko@sntech.de>

dtbscheck is happy when either reset-gpios or lines-initial-states is
present, but when both are present complains like

    rk3588-tiger-haikou-video-demo.dtb: gpio@27: reset-gpios: False schema does not allow [[205, 17, 1]]

as expected.
Re: [PATCH 1/2] dt-bindings: gpio: nxp,pcf8575: add reset GPIO
Posted by Laurent Pinchart 10 months ago
Hi Quentin,

Thank you for the patch.

On Thu, Feb 20, 2025 at 10:56:51AM +0100, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> A few of the I2C GPIO expander chips supported by this binding have a
> RESETN pin to be able to reset the chip. The chip is held in reset while
> the pin is low, therefore the polarity of reset-gpios is expected to
> reflect that, i.e. a GPIO_ACTIVE_HIGH means the GPIO will be held low
> for reset and released high, GPIO_ACTIVE_LOW means the GPIO will be held
> high for reset and released low.

I think the convention in DT is the opposite. The DT property is
"reset-gpios", no "resetn-gpio", so the polarity should indicate how to
drive the GPIO to assert a logical "reset". GPIO_ACTIVE_LOW should mean
that the chip will be in reset when the physical GPIO is 0.

Could DT maintainers confirm this ?

> Out of the supported chips, only PCA9670, PCA9671, PCA9672 and PCA9673
> show a RESETN pin in their datasheets. They all share the same reset
> timings, that is 4+us reset pulse[0] and 100+us reset time[0].
> 
> When performing a reset, "The PCA9670 registers and I2C-bus state
> machine will be held in their default state until the RESET input is
> once again HIGH."[1] meaning we now know the state of each line
> controlled by the GPIO expander. Therefore, setting lines-initial-states
> and reset-gpios both does not make sense and their presence is XOR'ed.
> 
> [0] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf Fig 22.
> [1] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf 8.5
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  .../devicetree/bindings/gpio/nxp,pcf8575.yaml      | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml b/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
> index 3718103e966a13e1d77f73335ff73c18a3199469..d08d3f848f82e74de949da16d26a810dc52a74e5 100644
> --- a/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
> +++ b/Documentation/devicetree/bindings/gpio/nxp,pcf8575.yaml
> @@ -73,6 +73,39 @@ properties:
>  
>    wakeup-source: true
>  
> +  reset-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO controlling the (reset active LOW) RESET# pin.
> +
> +      Performing a reset makes all lines initialized to their input (pulled-up)
> +      state.
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              enum:
> +                - nxp,pca9670
> +                - nxp,pca9671
> +                - nxp,pca9672
> +                - nxp,pca9673
> +    then:
> +      properties:
> +        reset-gpios: false
> +
> +  # lines-initial-states XOR reset-gpios
> +  # Performing a reset reinitializes all lines to a known state which
> +  # may not match passed lines-initial-states
> +  - if:
> +      required:
> +        - lines-initial-states
> +    then:
> +      properties:
> +        reset-gpios: false
> +
>  patternProperties:
>    "^(.+-hog(-[0-9]+)?)$":
>      type: object
> 

-- 
Regards,

Laurent Pinchart
Re: [PATCH 1/2] dt-bindings: gpio: nxp,pcf8575: add reset GPIO
Posted by Quentin Schulz 10 months ago
Hi Laurent,

On 2/20/25 1:24 PM, Laurent Pinchart wrote:
> Hi Quentin,
> 
> Thank you for the patch.
> 
> On Thu, Feb 20, 2025 at 10:56:51AM +0100, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> A few of the I2C GPIO expander chips supported by this binding have a
>> RESETN pin to be able to reset the chip. The chip is held in reset while
>> the pin is low, therefore the polarity of reset-gpios is expected to
>> reflect that, i.e. a GPIO_ACTIVE_HIGH means the GPIO will be held low
>> for reset and released high, GPIO_ACTIVE_LOW means the GPIO will be held
>> high for reset and released low.
> 
> I think the convention in DT is the opposite. The DT property is
> "reset-gpios", no "resetn-gpio", so the polarity should indicate how to
> drive the GPIO to assert a logical "reset". GPIO_ACTIVE_LOW should mean
> that the chip will be in reset when the physical GPIO is 0.
> 

Oh boy. I actually meant the opposite. What a brain fart. You can see 
the implementation in the driver too, if I am not having a second brain 
fart, it should follow what you're saying. I activate/assert 
(GPIOD_OUT_HIGH) then release/deassert (gpiod_set_value(x, 0)), so if 
you have a line straight from the SoC to the RESETN pin, you'd need 
GPIO_ACTIVE_LOW in DT to model that.

The polarity of the line should match the reset state. I.e. if 
GPIO_ACTIVE_LOW for reset-gpio, it means the chip is in reset when the 
line is low. It exits reset when high.

I got confused by the GPIOD_OUT_HIGH flag I used in the driver to 
*assert* the reset, which is putting the line in logical high (or rather 
"active"), which is "drive low" for me on all my devices that'll make 
use of it (no inverter on the line, so RESETN meaning is kept, 0V = 
reset; I have GPIO_ACTIVE_LOW for my reset GPIO and it does reflect 
that, c.f. 
https://lore.kernel.org/linux-rockchip/20250220-ringneck-dtbos-v1-4-25c97f2385e6@cherry.de/T/#u).

Cheers,
Quentin