Re: [PATCH] [v5] wireless: Initial driver submission for pureLiFi STA devices
From: Joe Perches <joe@perches.com>
Date: 2020-10-19 04:55:43
Also in:
linux-wireless, lkml
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:
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
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