[v7,1/7] dt-bindings: wireless: wilc1000: Document WILC3000 compatible string

Document compatible string for the WILC3000 chip. The chip is similar
to WILC1000, except that the register layout is slightly different and
it does not support WPA3/SAE.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
Cc: Ajay Singh <ajay.kathat@microchip.com>
Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
V2: - Use WILC1000 as fallback compatible string for WILC3000
V3: - Swap the wilc1000/wilc3000 compatible order
V4: - Add RB from Krzysztof
V5: No change
V6: - Rebase on next-20240926
V7: - Rebase on next-20241003 / wireless-next/main 5a4d42c1688c
      with v2 wifi: wilc1000: Keep slot powered on during suspend/resume
---
 .../bindings/net/wireless/microchip,wilc1000.yaml           | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

On 10/3/24 7:33 PM, Alexis Lothoré wrote:
> On 10/3/24 13:14, Marek Vasut wrote:
>> Reduce the use of wilc_get_chipid(), use cached chip ID wherever
>> possible. Remove duplicated partial chip ID read implementations
>> from the driver. Update wilc_get_chipid() to always read the chip
>> ID out of the hardware and update the cached chip ID, and make it
>> return a proper return value instead of a chipid. Call wilc_get_chipid()
>> early to make the cached chip ID available to various sites using
>> is_wilc1000() to access the cached chip ID.
>>
>> Reviewed-by: Alexis Lothoré <alexis.lothore@bootlin.com>
>> Signed-off-by: Marek Vasut <marex@denx.de>
> 
> [...]
> 
>> +int 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);
>> +		wilc->hif_func->hif_read_reg(wilc, WILC_RF_REVISION_ID,
>> +					     &rfrevid);
>> +		if (!is_wilc1000(chipid)) {
>> +			wilc->chipid = 0;
>> +			return -EINVAL;
>> +		}
>> +		if (chipid == WILC_1000_BASE_ID_2A) { /* 0x1002A0 */
>> +			if (rfrevid != 0x1)
>> +				chipid = WILC_1000_BASE_ID_2A_REV1;
>> +		} else if (chipid == WILC_1000_BASE_ID_2B) { /* 0x1002B0 */
>> +			if (rfrevid == 0x4)
>> +				chipid = WILC_1000_BASE_ID_2B_REV1;
>> +			else if (rfrevid != 0x3)
>> +				chipid = WILC_1000_BASE_ID_2B_REV2;
>> +		}
>> +
>> +		wilc->chipid = chipid;
>> +	}
>> +
>> +	return 0;
>> +}
> 
> My bad for not having spotted it in v6, but you are still missing an
> EXPORT_SYMBOL_GPL(wilc_get_chipid) here, making the build fail if wilc support
> is built as module:
> 
> ERROR: modpost: "wilc_get_chipid"
> [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] undefined!
> ERROR: modpost: "wilc_get_chipid"
> [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined!
> make[2]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> make[1]: *** [/home/alexis/src/microchip/linux/Makefile:1878: modpost] Error 2
> make: *** [Makefile:224: __sub-make] Error 2
Fixed in V8, thanks.

Before I send V8, can you have a look at the last two patches in this 
series? They need some RB/TB.

Thanks !

On 10/4/24 10:39 AM, Alexis Lothoré wrote:
> Hello Marek,

Hi,

> On 10/3/24 13:14, Marek Vasut wrote:
>> Register wiphy after reading out chipid, so the chipid can be
>> used to determine chip features and not advertise WPA3/SAE
>> support to userspace on WILC3000. Note that wilc_netdev_cleanup()
>> will deregister the wiphy in fail path.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
> 
> [...]
> 
>> diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
>> index 11e0f8a473467..30015c5920467 100644
>> --- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
>> +++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
>> @@ -1761,7 +1761,6 @@ static struct wilc *wilc_create_wiphy(struct device *dev)
>>   {
>>   	struct wiphy *wiphy;
>>   	struct wilc *wl;
>> -	int ret;
>>   
>>   	wiphy = wiphy_new(&wilc_cfg80211_ops, sizeof(*wl));
>>   	if (!wiphy)
>> @@ -1804,14 +1803,8 @@ static struct wilc *wilc_create_wiphy(struct device *dev)
>>   				BIT(NL80211_IFTYPE_P2P_GO) |
>>   				BIT(NL80211_IFTYPE_P2P_CLIENT);
>>   	wiphy->flags |= WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
>> -	wiphy->features |= NL80211_FEATURE_SAE;
>>   	set_wiphy_dev(wiphy, dev);
>>   	wl->wiphy = wiphy;
>> -	ret = wiphy_register(wiphy);
> 
> I think you did not address one of my comment in v5 about this change: in
> wilc_cfg80211_init (wilc_create_wiphy's caller), there is still a
> wiphy_unregister in the failure path, which does not make sense anymore since
> this function does not register wiphy anymore.
> 
> Once fixed: with this patch iw phy shows correctly on my platform that wilc3000
> does not support SAE authenticate command while wilc1000 does. And wilc1000
> still works correctly, even with wpa3.
> 
> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> Tested-on: WILC1000SD 07 SDIO WILC_WIFI_FW_REL_16_1_2
> Tested-on: WILC3000 A SDIO WILC_WIFI_FW_REL_16_1_1
Fixed in V8, thank you.