Re: [PATCH] [v13] wireless: Initial driver submission for pureLiFi STA devices

From: Kalle Valo <kvalo@codeaurora.org>
To: Srinivasan Raju <srini.raju@purelifi.com>
Cc: mostafa.afgani@purelifi.com,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-kernel@vger.kernel.org (open list),
	linux-wireless@vger.kernel.org (open list:NETWORKING DRIVERS
	(WIRELESS)),
	netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
Subject: Re: [PATCH] [v13] wireless: Initial driver submission for pureLiFi STA devices
Date: Wed, 17 Feb 2021 12:19:25 +0200	[thread overview]
Message-ID: <87im6rov2q.fsf@codeaurora.org> (raw)
In-Reply-To: <20210212115030.124490-1-srini.raju@purelifi.com> (Srinivasan Raju's message of "Fri, 12 Feb 2021 17:19:39 +0530")

Srinivasan Raju <srini.raju@purelifi.com> writes:

> This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC
> and LiFi-XL USB devices.
>
> This driver implementation has been based on the zd1211rw driver.
>
> Driver is based on 802.11 softMAC Architecture and uses
> native 802.11 for configuration and management.
>
> The driver is compiled and tested in ARM, x86 architectures and
> compiled in powerpc architecture.
>
> Signed-off-by: Srinivasan Raju <srini.raju@purelifi.com>

[...]

> +	if ((le16_to_cpu(udev->descriptor.idVendor) ==
> +				PURELIFI_X_VENDOR_ID_0) &&
> +	    (le16_to_cpu(udev->descriptor.idProduct) ==
> +				PURELIFI_X_PRODUCT_ID_0)) {
> +		fw_name = "plfxlc/LiFi-X.bin";
> +		dev_dbg(&intf->dev, "bin file for X selected\n");
> +
> +	} else if ((le16_to_cpu(udev->descriptor.idVendor)) ==
> +					PURELIFI_XC_VENDOR_ID_0 &&
> +		   (le16_to_cpu(udev->descriptor.idProduct) ==
> +					PURELIFI_XC_PRODUCT_ID_0)) {
> +		fw_name = "plfxlc/LiFi-XC.bin";
> +		dev_dbg(&intf->dev, "bin file for XC selected\n");
> +
> +	} else {
> +		r = -EINVAL;
> +		goto error;
> +	}
> +
> +	r = request_firmware((const struct firmware **)&fw, fw_name,
> +			     &intf->dev);
> +	if (r) {
> +		dev_err(&intf->dev, "request_firmware failed (%d)\n", r);
> +		goto error;
> +	}

[...]

> +	/* Code for single pack file download */
> +
> +	fw_pack = "plfxlc/LiFi-XL.bin";
> +
> +	r = request_firmware((const struct firmware **)&fw_packed, fw_pack,
> +			     &intf->dev);
> +	if (r) {
> +		dev_err(&intf->dev, "request_firmware failed (%d)\n", r);
> +		goto error;
> +	}

Having the firmware files under plfxlc/ directory looks good to me. But
I'm not really a fan of upper case filenames, is there a reason for
that? I would prefer have all lowercase filenames.

Also the cast (const struct firmware **) looks very wrong, but I didn't
investigate why you do it. I'm sure there is a proper way to implement
whatever you need here, without the cast.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches