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

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: <3ce67283-e484-4450-a67c-23ed1e36945f@cherry.de>
Date: Thu, 20 Feb 2025 14:11:57 +0100
From: Quentin Schulz <quentin.schulz@...rry.de>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Quentin Schulz <foss+kernel@...il.net>
Cc: Linus Walleij <linus.walleij@...aro.org>,
 Bartosz Golaszewski <brgl@...ev.pl>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: gpio: nxp,pcf8575: add reset GPIO

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@...rry.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