linux-kernel - Re: [PATCH RFC] wifi: wilc1000: fix reset line assert/deassert polarity

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]

Date: Tue, 13 Feb 2024 16:42:04 +0000 (UTC)
From: David Mosberger-Tang <davidm@...uge.net>
To: Alexis Lothoré <alexis.lothore@...tlin.com>,
	linux-wireless@...r.kernel.org
Cc: Ajay Singh <ajay.kathat@...rochip.com>, Claudiu Beznea
	<claudiu.beznea@...on.dev>, Kalle Valo <kvalo@...nel.org>, Thomas Petazzoni
	<thomas.petazzoni@...tlin.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] wifi: wilc1000: fix reset line assert/deassert
 polarity

On Tue, 2024-02-13 at 16:22 +0100, Alexis Lothoré wrote:
> When using a wilc1000 chip over a spi bus, users can optionally define a
> reset gpio and a chip enable gpio. The reset line of wilc1000 is active
> low, so to hold the chip in reset, a low (physical) value must be applied.
> 
> The corresponding device tree binding documentation was introduced by
> commit f31ee3c0a555 ("wilc1000: Document enable-gpios and reset-gpios
> properties") and correctly indicates that the reset line is an active-low
> signal. However, the corresponding driver part, brought by commit
> ec031ac4792c ("wilc1000: Add reset/enable GPIO support to SPI driver"), is
> misusing the gpiod APIs and apply an inverted logic when powering up/down
> the chip (for example, setting the reset line to a logic "1" during power
> up, which in fact asserts the reset line when device tree describes the
> reset line as GPIO_ACTIVE_LOW).

Note that commit ec031ac4792c is doing the right thing in regards to an
ACTIVE_LOW RESET pin and the binding documentation is consistent with that code.

It was later on that commit fcf690b0 flipped the RESET line polarity to treat it
as GPIO_ACTIVE_HIGH.  I never understood why that was done and, as you noted, it
introduced in inconsistency with the binding documentation.

On our platform, we never merged commit fcf690b0 and hence our DTS already
defines the RESET pin as GPIO_ACTIVE_LOW.  So, I don't have any issues at all
with your patch! :-)

  --david
                                                                              gi