Re: [PATCH v2 2/4] wifi: wilc1000: Fold wilc_get_chipid() into wlan.c

[Simon Horman <horms@kernel.org>]

+ Dan

On Fri, Aug 23, 2024 at 10:38:59PM +0200, Marek Vasut wrote:
> On 8/23/24 7:46 PM, Simon Horman wrote:
> > On Fri, Aug 23, 2024 at 06:08:57PM +0200, Marek Vasut wrote:
> > > Do not use wilc_get_chipid() outside of wlan.c . Instead, call
> > > wilc_get_chipid() right after the SDIO/SPI interface has been
> > > initialized to cache the device chipid, and then use the cached
> > > chipid throughout the driver. Make wilc_get_chipid() static and
> > > remove its prototype from wlan.h .
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > 
> > ...
> > 
> > > diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
> > 
> > ...
> > 
> > > @@ -1535,9 +1537,18 @@ int wilc_wlan_init(struct net_device *dev)
> > >   	if (!wilc->hif_func->hif_is_init(wilc)) {
> > >   		acquire_bus(wilc, WILC_BUS_ACQUIRE_ONLY);
> > >   		ret = wilc->hif_func->hif_init(wilc, false);
> > > +		if (!ret)
> > > +			chipid = wilc_get_chipid(wilc);
> > >   		release_bus(wilc, WILC_BUS_RELEASE_ONLY);
> > >   		if (ret)
> > >   			goto fail;
> > > +
> > > +		if (!is_wilc1000(chipid)) {
> > > +			netdev_err(dev, "Unsupported chipid: %x\n", chipid);
> > > +			return -EINVAL;
> > 
> > Hi Marek,
> > 
> > Should this unwind as is the case elsewhere in this function?
> 
> It should, will fix in V3, thanks.
> 
> > 			ret = -EINVAL;
> > 			goto fail;
> > 
> > Flagged by Smatch.
> 
> What's the trick here ?

Smatch is here. I don't think there is much of a trick other than running it :)

https://github.com/error27/smatch