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]

Message-ID: <aac398e4-d870-4ba2-8877-b98afecb8d1b@microchip.com>
Date: Thu, 15 Feb 2024 04:35:46 +0000
From: <Ajay.Kathat@...rochip.com>
To: <alexis.lothore@...tlin.com>, <davidm@...uge.net>,
	<linux-wireless@...r.kernel.org>
CC: <claudiu.beznea@...on.dev>, <kvalo@...nel.org>,
	<thomas.petazzoni@...tlin.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] wifi: wilc1000: fix reset line assert/deassert
 polarity

Hi,

On 2/13/24 09:58, Alexis Lothoré wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 2/13/24 17:42, David Mosberger-Tang wrote:
>> 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.
> 
> Ah, you are right, and I was wrong citing your GPIOs patch as faulty
> (git-blaming too fast !), thanks for the clarification. I missed this patch from
> Ajay (fcf690b0) flipping the reset logic. Maybe he had issues while missing
> proper device tree configuration and then submitted this flip ?

Indeed, it was done to align the code as per the DT entry suggested in
WILC1000/3000 porting guide[1 -page 18], which is already used by most
of the existing users. This change has impact on the users who are using
DT entry from porting guide. One approach is to retain the current code
and document this if needed.

1.
https://ww1.microchip.com/downloads/en/DeviceDoc/ATWILC1000-ATWILC3000-ATWILC-Devices-Linux-Porting-Guide-User-Guide-DS70005329C.pdf


Regards,
Ajay