Received-SPF: pass (google.com: domain of linux-wireless+bounces-3268-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: <e8aa38252c0700df16c0023b0cde81594259a5b5.camel@egauge.net>
Subject: Re: [PATCH] [v4] wifi: wilc1000: validate chip id during bus probe
From: David Mosberger-Tang <davidm@egauge.net>
To: Kalle Valo <kvalo@kernel.org>, Alexis =?ISO-8859-1?Q?Lothor=E9?=
	 <alexis.lothore@bootlin.com>
Cc: linux-wireless@vger.kernel.org, Ajay.Kathat@microchip.com
Date: Tue, 06 Feb 2024 21:59:55 -0700
In-Reply-To: <87cytgv6nt.fsf@kernel.org>
References: <20240127004331.1334804-1-davidm@egauge.net>
	 <415a0e6e-5824-44a2-af2a-a75115d5a62e@bootlin.com>
	 <87cytgv6nt.fsf@kernel.org>
Organization: eGauge Systems LLC
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0

On Thu, 2024-02-01 at 12:08 +0200, Kalle Valo wrote:
> Alexis Lothor=C3=A9 <alexis.lothore@bootlin.com> writes:
>=20
> > On 1/27/24 01:43, David Mosberger-Tang wrote:
> > > Previously, the driver created a net device (typically wlan0) as soon
> > > as the module was loaded.  This commit changes the driver to follow
> > > normal Linux convention of creating the net device only when bus
> > > probing detects a supported chip.
> >=20
> > As already mentioned multiple times, I am skeptical about the validity =
of
> > keeping netdev registration before chip presence check, but I am not th=
e
> > maintainer, so I let Ajay and Kalle decide for this.
>=20
> I haven't checked the code but as a general comment I agree with Alexis,
> registering netdev before the hardware is ready sounds odd to me.

I agree, but it's orthogonal to what my patch does.

I did a quick scan and it looks like the cleanest thing to do would be
to change all the code below "Spi Internal Read/Write Function"  and
"Spi interfaces" to take a spi_device pointer instead of the wilc
pointer.  The probe code could then safely call these lower-level
functions without having to worry that they might inadvertently access
a part of the wilc structure that hasn't been initialized yet.

From the looks of it, that would probably also shorten the code, since
many calls to to_spi_device(wilc->dev) would go away.

However, it'd be a major rewrite of spi.c so it better had the buy in
of the maintainer(s).

  --david