Received-SPF: pass (google.com: domain of linux-wireless+bounces-3556-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <2ff1c701f3443e1c612a81f4077b0280850f57c6.camel@egauge.net>
Subject: Re: [PATCH RFC] wifi: wilc1000: fix reset line assert/deassert
 polarity
From: David Mosberger-Tang <davidm@egauge.net>
Date: Tue, 13 Feb 2024 16:42:04 +0000 (UTC)
In-Reply-To: <20240213-wilc_1000_reset_line-v1-1-e01da2b23fed@bootlin.com>
References: <20240213-wilc_1000_reset_line-v1-1-e01da2b23fed@bootlin.com>
Organization: eGauge Systems LLC
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0
To: Alexis =?iso-8859-1?q?Lothor=E9?= <alexis.lothore@bootlin.com>,
	linux-wireless@vger.kernel.org
Cc: Ajay Singh <ajay.kathat@microchip.com>, Claudiu Beznea
	<claudiu.beznea@tuxon.dev>, Kalle Valo <kvalo@kernel.org>, Thomas Petazzoni
	<thomas.petazzoni@bootlin.com>, linux-kernel@vger.kernel.org

On Tue, 2024-02-13 at 16:22 +0100, Alexis Lothor=E9 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=
.
>=20
> 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"), i=
s
> 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 tre=
at it
as GPIO_ACTIVE_HIGH.  I never understood why that was done and, as you note=
d, 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 a=
ll
with your patch! :-)

  --david
                                                                           =
   gi