On Wed, 2020-10-14 at 11:49 +0530, Srinivasan Raju wrote:
> This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC
> and LiFi-XL USB devices.
trivia: netdev_<level> might be better than dev_<level>.
> diff --git a/drivers/net/wireless/purelifi/chip.c b/drivers/net/wireless/purelifi/chip.c
[]
> +int purelifi_chip_switch_radio(struct purelifi_chip *chip, u32 value)
> +{
> + int r;
> +
> + r = usb_write_req((const u8 *)&value, sizeof(value), USB_REQ_POWER_WR);
> + if (r)
> + dev_err(purelifi_chip_dev(chip), "%s r=%d", __func__, r);
missing '\n' termination.
[]
> > +int purelifi_chip_set_rate(struct purelifi_chip *chip, u8 rate)
> +{
> + int r;
> + static struct purelifi_chip *chip_p;
> +
> + if (chip)
> + chip_p = chip;
> + if (!chip_p)
> + return -EINVAL;
> +
> + r = usb_write_req((const u8 *)&rate, sizeof(rate), USB_REQ_RATE_WR);
> + if (r)
> + dev_err(purelifi_chip_dev(chip), "set rate failed r=%d", r);
same missing newline, etc
It might also be better to use a consistent error message like
dev_err(purelifi_chip_dev(chip), "%s: failed, r=%d\n", r);
for both.
> diff --git a/drivers/net/wireless/purelifi/mac.c b/drivers/net/wireless/purelifi/mac.c
[]
> +static const struct ieee80211_rate purelifi_rates[] = {
> + { .bitrate = 10,
> + .hw_value = PURELIFI_CCK_RATE_1M, },
[]
> + { .bitrate = 60,
> + .hw_value = PURELIFI_OFDM_RATE_6M,
> + .flags = 0 },
Seems odd to set .flags to 0 for only some of the entries.
Perhaps more readable to always leave it off if unset.
> + { .bitrate = 90,
> + .hw_value = PURELIFI_OFDM_RATE_9M,
> + .flags = 0 },
> + { .bitrate = 120,
> + .hw_value = PURELIFI_OFDM_RATE_12M,
> + .flags = 0 },
> + { .bitrate = 180,
> + .hw_value = PURELIFI_OFDM_RATE_18M,
> + .flags = 0 },
> + { .bitrate = 240,
> + .hw_value = PURELIFI_OFDM_RATE_24M,
> + .flags = 0 },
> + { .bitrate = 360,
> + .hw_value = PURELIFI_OFDM_RATE_36M,
> + .flags = 0 },
> + { .bitrate = 480,
> + .hw_value = PURELIFI_OFDM_RATE_48M,
> + .flags = 0 },
> + { .bitrate = 540,
> + .hw_value = PURELIFI_OFDM_RATE_54M,
> + .flags = 0 },
> +};
[]
> +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", irqs_disabled());
missing newline
> +static int filter_ack(struct ieee80211_hw *hw, struct ieee80211_hdr *rx_hdr,
> + struct ieee80211_rx_status *stats)
> +{
> + struct purelifi_mac *mac = purelifi_hw_mac(hw);
> + struct sk_buff *skb;
> + struct sk_buff_head *q;
> + unsigned long flags;
> + int found = 0;
> + int i, position = 0;
> +
> + if (!ieee80211_is_ack(rx_hdr->frame_control))
> + return 0;
> + dev_info(purelifi_mac_dev(mac), "%s::ACK Received", __func__);
missing newline
> +static int purelifi_op_add_interface(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif)
> +{
> + struct purelifi_mac *mac = purelifi_hw_mac(hw);
> + static const char * const iftype80211[] = {
> + [NL80211_IFTYPE_STATION] = "Station",
> + [NL80211_IFTYPE_ADHOC] = "Adhoc"
> + };
> +
> + if (mac->type != NL80211_IFTYPE_UNSPECIFIED)
> + return -EOPNOTSUPP;
> +
> + if (vif->type == NL80211_IFTYPE_ADHOC ||
> + vif->type == NL80211_IFTYPE_STATION) {
> + dev_info(purelifi_mac_dev(mac), "%s %s\n", __func__,
> + iftype80211[vif->type]);
> + mac->type = vif->type;
> + mac->vif = vif;
> + } else {
> + dev_info(purelifi_mac_dev(mac), "unsupported iftype");
etc...
