Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Marek,

On 8/23/24 18:08, 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@xxxxxxx>
> ---
> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
> Cc: Adham Abozaeid <adham.abozaeid@xxxxxxxxxxxxx>
> Cc: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx>
> Cc: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx>
> Cc: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> Cc: Conor Dooley <conor+dt@xxxxxxxxxx>
> Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: Kalle Valo <kvalo@xxxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>
> Cc: Marek Vasut <marex@xxxxxxx>
> Cc: Paolo Abeni <pabeni@xxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx
> ---
> V2: New patch
> ---

[...]

> +static u32 wilc_get_chipid(struct wilc *wilc)
> +{
> +	u32 chipid = 0;
> +	u32 rfrevid = 0;
> +
> +	if (wilc->chipid == 0) {
> +		wilc->hif_func->hif_read_reg(wilc, WILC_CHIPID, &chipid);
If we search for WILC_CHIPID in the whole driver, there are still two places
manually reading this register. Shouldn't those places also benefit from
wilc_get_chipid ?

> +		wilc->hif_func->hif_read_reg(wilc, WILC_RF_REVISION_ID,
> +					     &rfrevid);
> +		if (!is_wilc1000(chipid)) {
> +			wilc->chipid = 0;

While at it, since you have trimmed the update parameter, it would be nice to
also fix this return value (ie make wilc_getchipid() not return 0 but a real
error code if we can not read the chip id.

Thanks,

Alexis
-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux