lkml.org 
[lkml]   [2020]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] [PATCH] [v3] wireless: Initial driver submission for pureLiFi STA devices
    From
    Date
    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...


    \
     
     \ /
      Last update: 2020-10-16 00:36    [W:3.216 / U:1.444 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site