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: 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ