Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Srinivasan Raju <srini.raju@purelifi.com>
Cc: mostafa.afgani@purelifi.com, Kalle Valo <kvalo@codeaurora.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:NETWORKING DRIVERS (WIRELESS)" 
	<linux-wireless@vger.kernel.org>,
	"open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>
Subject: Re: [PATCH] [v5] wireless: Initial driver submission for pureLiFi STA devices
Date: Sun, 18 Oct 2020 21:55:33 -0700	[thread overview]
Message-ID: <e59c0c575f9d9e1af8c6fdf2911cd9d028de257f.camel@perches.com> (raw)
In-Reply-To: <20201019031744.17916-1-srini.raju@purelifi.com>

On Mon, 2020-10-19 at 08:47 +0530, Srinivasan Raju wrote:
> This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC
> and LiFi-XL USB devices.

Mostly trivial comments:

> diff --git a/drivers/net/wireless/purelifi/chip.c b/drivers/net/wireless/purelifi/chip.c
[]
> +int purelifi_chip_set_rate(struct purelifi_chip *chip, u8 rate)
> +{
> +	int r;
> +	static struct purelifi_chip *chip_p;

Isn't chip_p pointless?

> +
> +	if (chip)
> +		chip_p = chip;
> +	if (!chip_p)
> +		return -EINVAL;
> +

chip_p is otherwise unused.

> diff --git a/drivers/net/wireless/purelifi/mac.c b/drivers/net/wireless/purelifi/mac.c
[]
> +int purelifi_mac_init_hw(struct ieee80211_hw *hw)
> +{
> +	int r;
> +	struct purelifi_mac *mac = purelifi_hw_mac(hw);
> +	struct purelifi_chip *chip = &mac->chip;
> +
> +	r = purelifi_chip_init_hw(chip);
> +	if (r) {
> +		dev_warn(purelifi_mac_dev(mac), "init hw failed (%d)\n", r);
> +		goto out;
> +	}
> +
> +	dev_dbg(purelifi_mac_dev(mac), "irq_disabled %d\n", irqs_disabled());
> +
> +	r = regulatory_hint(hw->wiphy, "CA");
> +out:
> +	return r;
> +}

Simpler without the out: label and a direct return

	if (r) {
		dev_warn(...)
		return r;
	}

	...

	return regulator_hint(hw->wiphy, "CA");
}

> +static int download_fpga(struct usb_interface *intf)
> +{
[]
> +	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 = "purelifi/li_fi_x/fpga.bit";
> +		dev_info(&intf->dev, "bit 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 = "purelifi/li_fi_x/fpga_xc.bit";
> +		dev_info(&intf->dev, "bit filefor XC selected.\n");

Inconsistent dev_info spacing: "file for" vs "filefor"

> +	for (fw_data_i = 0; fw_data_i < fw->size;) {
> +		int tbuf_idx;
> +
> +		if ((fw->size - fw_data_i) < blk_tran_len)
> +			blk_tran_len = fw->size - fw_data_i;
> +
> +		fw_data = kmalloc(blk_tran_len, GFP_KERNEL);
> +
> +		memcpy(fw_data, &fw->data[fw_data_i], blk_tran_len);
> +
> +		for (tbuf_idx = 0; tbuf_idx < blk_tran_len; tbuf_idx++) {
> +			fw_data[tbuf_idx] =
> +				((fw_data[tbuf_idx] & 128) >> 7) |
> +				((fw_data[tbuf_idx] &  64) >> 5) |
> +				((fw_data[tbuf_idx] &  32) >> 3) |
> +				((fw_data[tbuf_idx] &  16) >> 1) |
> +				((fw_data[tbuf_idx] &   8) << 1) |
> +				((fw_data[tbuf_idx] &   4) << 3) |
> +				((fw_data[tbuf_idx] &   2) << 5) |
> +				((fw_data[tbuf_idx] &   1) << 7);
> +		}

pity there isn't an u8_bit_reverse function/mechanism

> +static void pretty_print_mac(struct usb_interface *intf, char *serial_number,
> +			     unsigned char *hw_address
> +			    )
> +{
> +	unsigned char i;
> +
> +	for (i = 0; i < ETH_ALEN; i++)
> +		dev_info(&intf->dev, "hw_address[%d]=%x\n", i, hw_address[i]);

I don't think this prettier than %pM

> +}
> +
> +static int upload_mac_and_serial_number(struct usb_interface *intf,
> +					unsigned char *hw_address,
> +					unsigned char *serial_number)
> +{
[]
> +	do {
> +		unsigned char buf[8];
> +
> +		msleep(200);
> +
> +		send_vendor_request(udev, 0x40, buf, sizeof(buf));
> +		flash.enabled = buf[0] & 0xFF;
> +
> +		if (flash.enabled) {
> +			flash.sectors = ((buf[6] & 255) << 24) |

buf is unsigned char[], rather pointless using & 255

> diff --git a/drivers/net/wireless/purelifi/usb.h b/drivers/net/wireless/purelifi/usb.h
[]
> +struct station_t {
> +   //  7...3    |    2     |     1     |     0     |
> +   // Reserved  | Hearbeat | FIFO full | Connected |

heartbeat



  reply	other threads:[~2020-10-19  4:55 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200924151910.21693-1-srini.raju@purelifi.com>
2020-09-28 10:19 ` [PATCH] [v2] wireless: Initial driver submission for pureLiFi devices Srinivasan Raju
2020-09-28 12:07   ` Joe Perches
2020-09-28 12:53     ` Srinivasan Raju
2020-09-30  5:16   ` Leon Romanovsky
2020-09-30  5:29     ` Srinivasan Raju
2020-09-30  8:01     ` Kalle Valo
2020-09-30  9:55       ` Leon Romanovsky
2020-09-30 10:11         ` Johannes Berg
2020-09-30 10:44           ` Leon Romanovsky
2020-10-16  8:23             ` Kalle Valo
2020-09-30  8:05     ` Kalle Valo
2020-09-30 10:04       ` Leon Romanovsky
2020-10-14  6:19   ` [PATCH] [PATCH] [v3] wireless: Initial driver submission for pureLiFi STA devices Srinivasan Raju
2020-10-15 22:35     ` Joe Perches
2020-10-16  6:36       ` Srinivasan Raju
2020-10-16  6:34   ` [PATCH] [v4] " Srinivasan Raju
2020-10-16  8:58     ` Joe Perches
2020-10-16 10:13       ` Srinivasan Raju
2020-10-19  3:17     ` [PATCH] [v5] " Srinivasan Raju
2020-10-19  4:55       ` Joe Perches [this message]
2020-10-19  6:05         ` Srinivasan Raju
2020-10-19  8:38       ` [PATCH] [v6] " Srinivasan Raju
2020-10-19 16:07         ` Krishna Chaitanya
2020-10-19 16:40           ` Srinivasan Raju
2020-10-19 16:54             ` Joe Perches
2020-10-19 17:05               ` Srinivasan Raju
2020-11-16  9:22     ` [PATCH] [v7] " Srinivasan Raju
2020-11-16 20:45       ` Joe Perches
2020-11-18  3:24         ` Srinivasan Raju
2020-11-24 14:44       ` Kalle Valo
     [not found]       ` <20201124144448.4E95EC43460@smtp.codeaurora.org>
2020-11-26  5:01         ` Srinivasan Raju
2020-12-03  4:43           ` Srinivasan Raju
2020-12-03 15:58             ` Kalle Valo
2020-12-03 16:50               ` Srinivasan Raju
2020-12-19 13:15                 ` Kalle Valo
2020-12-03  4:38   ` [PATCH] [v8] " Srinivasan Raju
2020-12-03  5:09   ` [PATCH] [v9] " Srinivasan Raju
2020-12-03  7:53     ` Joe Perches
2020-12-08  5:53   ` [PATCH] [v10] " Srinivasan Raju
2020-12-08 11:57   ` [PATCH] [v11] " Srinivasan Raju
2020-12-08 14:37     ` Kalle Valo
2020-12-19 12:51     ` Kalle Valo
2020-12-19 13:06     ` Kalle Valo
2020-12-19 13:14     ` Kalle Valo
2020-12-21  5:52       ` Srinivasan Raju
2020-12-21  5:57         ` Kalle Valo
2021-01-15 12:13           ` Srinivasan Raju
2021-01-05 13:19   ` [PATCH] [PATCH] [v12] " Srinivasan Raju
2021-02-12 11:49   ` [PATCH] [v13] " Srinivasan Raju
2021-02-12 13:44     ` Johannes Berg
2021-02-17 10:05       ` Kalle Valo
2021-02-19  5:15       ` Srinivasan Raju
2021-02-19  8:25         ` Johannes Berg
2021-02-24 10:41           ` Srinivasan Raju
2021-02-17 10:02     ` Kalle Valo
2021-02-17 10:13       ` Kalle Valo
2021-02-17 10:16         ` Srinivasan Raju
2021-02-17 10:09     ` Kalle Valo
2021-02-17 10:19     ` Kalle Valo
2021-02-24 10:44       ` Srinivasan Raju
2021-02-26 13:07   ` [PATCH] [v14] " Srinivasan Raju
2021-04-19 11:52     ` Srinivasan Raju
2021-08-10 13:02       ` Srinivasan Raju
2021-08-21 13:42         ` Kalle Valo
2021-08-18 14:13     ` [PATCH] [v15] " Srinivasan Raju
2021-09-20 13:05       ` Kalle Valo
     [not found]         ` <CWLP265MB3217BB5AA5F102629A3AD204E0A19@CWLP265MB3217.GBRP265.PROD.OUTLOOK.COM>
2021-09-21 12:30           ` [EXTERNAL] " Kalle Valo
2021-09-22  7:33             ` Johannes Berg
2021-09-24 13:27               ` [EXTERNAL] " Srinivasan Raju
2021-09-20 14:11       ` Kalle Valo
2021-09-24 11:11       ` Kalle Valo
2021-09-24 13:26   ` [PATCH] [v16] wireless: Initial driver submission for pureLiFi LiFi Station Srinivasan Raju
2021-09-24 13:40     ` Kalle Valo
2021-10-05 11:22   ` [PATCH] [v17] wireless: Initial driver submission for pureLiFi STA devices Srinivasan Raju
2021-10-05 11:26     ` Johannes Berg
2021-10-05 12:30   ` [PATCH] [v18 1/2] nl80211: Add LC placeholder band definition to enum nl80211_band Srinivasan Raju
2021-10-05 12:31   ` [PATCH] [v18 2/2] wireless: Initial driver submission for pureLiFi STA devices Srinivasan Raju
2021-10-05 22:09     ` Jeff Johnson
2021-10-06 10:04   ` [PATCH] [v19 " Srinivasan Raju
2021-10-11  6:16     ` Kalle Valo
2021-10-12 12:50   ` [PATCH 0/2] wireless: New Driver " Srinivasan Raju
2021-10-12 12:50     ` [PATCH 1/2] [v19 1/2] nl80211: Add LC placeholder band definition to enum nl80211_band Srinivasan Raju
2021-10-12 12:50     ` [PATCH 2/2] [v19 2/2] wireless: Initial driver submission for pureLiFi STA devices Srinivasan Raju
2021-10-18 10:00   ` [PATCH v20 0/2] wireless: New Driver " Srinivasan Raju
2021-10-18 10:00     ` [PATCH v20 1/2] nl80211: Add LC placeholder band definition to nl80211_band Srinivasan Raju
2021-10-18 10:00     ` [PATCH v20 2/2] wireless: Initial driver submission for pureLiFi STA devices Srinivasan Raju
2021-10-25  9:59       ` Kari Argillander
     [not found]         ` <CWLP265MB321780AB502EF147F6AAF197E0839@CWLP265MB3217.GBRP265.PROD.OUTLOOK.COM>
2021-10-25 12:17           ` [EXTERNAL] " Kalle Valo
2021-10-27 12:38       ` Kari Argillander
2021-10-28  7:24         ` Kalle Valo
2021-10-31 13:10   ` [PATCH v21 0/2] wireless: New Driver " Srinivasan Raju
2021-10-31 13:10     ` [PATCH 1/2] nl80211: Add LC placeholder band definition to nl80211_band Srinivasan Raju
2021-10-31 13:10     ` [PATCH 2/2] wireless: Initial driver submission for pureLiFi STA devices Srinivasan Raju
2021-12-20 19:13       ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e59c0c575f9d9e1af8c6fdf2911cd9d028de257f.camel@perches.com \
    --to=joe@perches.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=mchehab+huawei@kernel.org \
    --cc=mostafa.afgani@purelifi.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=srini.raju@purelifi.com \
    --subject='Re: [PATCH] [v5] wireless: Initial driver submission for pureLiFi STA devices' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).