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
