Re: [PATCH 13/19] wilc: add linux_wlan.c
On 10/11/2018 12:33 PM, Johannes Berg wrote:
> On Thu, 2018-10-11 at 12:30 +0530, Ajay Singh wrote:
+ if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
+ netif_stop_queue(wilc->vif[0]->ndev);
+ netif_stop_queue(wilc->vif[1]->ndev);
+ }
>>> It seems like a pretty bad idea to hard-code two interfaces, we do
>>> dynamic addition/removal these days, in *particular* for P2P.
>>>
>> Did you mean it not good to call stop queue for both the interfaces.
>> Can you please provide some more details about this comments.
> No, I mean you should be more dynamic and have e.g. a list of interfaces
> (actually, you can use cfg80211's list, I believe!), instead of hard-
> coding that you have "wlan0" and "p2p0".
I got your point now. I will check on this to add the support.
Regards,
Ajay
Re: [PATCH 13/19] wilc: add linux_wlan.c
On Thu, 2018-10-11 at 12:30 +0530, Ajay Singh wrote:
>
> > > + if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
> > > + netif_stop_queue(wilc->vif[0]->ndev);
> > > + netif_stop_queue(wilc->vif[1]->ndev);
> > > + }
> >
> > It seems like a pretty bad idea to hard-code two interfaces, we do
> > dynamic addition/removal these days, in *particular* for P2P.
> >
>
> Did you mean it not good to call stop queue for both the interfaces.
> Can you please provide some more details about this comments.
No, I mean you should be more dynamic and have e.g. a list of interfaces
(actually, you can use cfg80211's list, I believe!), instead of hard-
coding that you have "wlan0" and "p2p0".
johannes
Re: [PATCH 13/19] wilc: add linux_wlan.c
On 10/8/2018 8:11 PM, Johannes Berg wrote:
> On Wed, 2018-09-26 at 15:55 +0530, Ajay Singh wrote:
>> Moved '/driver/staging/wilc1000/linux_wlan.c' to
>> 'drivers/net/wireless/microchip/wilc/'.
>>
>> Signed-off-by: Ajay Singh
>> ---
>> drivers/net/wireless/microchip/wilc/linux_wlan.c | 1161
>> ++
>> 1 file changed, 1161 insertions(+)
>> create mode 100644 drivers/net/wireless/microchip/wilc/linux_wlan.c
> Hmm. It's pretty obviously a linux driver, what's the point?
>
Agree, will rename by avoiding the 'linux' prefix.
>> diff --git a/drivers/net/wireless/microchip/wilc/linux_wlan.c
>> b/drivers/net/wireless/microchip/wilc/linux_wlan.c
>> new file mode 100644
>> index 000..76c9012
>> --- /dev/null
>> +++ b/drivers/net/wireless/microchip/wilc/linux_wlan.c
>> @@ -0,0 +1,1161 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2012 - 2018 Microchip Technology Inc., and its
>> subsidiaries.
>> + * All rights reserved.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "wilc_wfi_cfgoperations.h"
>> +
>> +static int dev_state_ev_handler(struct notifier_block *this,
>> +unsigned long event, void *ptr)
>> +{
>> +struct in_ifaddr *dev_iface = ptr;
>> +struct wilc_priv *priv;
>> +struct host_if_drv *hif_drv;
>> +struct net_device *dev;
>> +u8 *ip_addr_buf;
>> +struct wilc_vif *vif;
>> +u8 null_ip[4] = {0};
>> +char wlan_dev_name[5] = "wlan0";
> Regardless of what you're trying to do, thta seems like a bad idea.
>
>> +if (!dev_iface || !dev_iface->ifa_dev || !dev_iface->ifa_dev->dev)
>> +return NOTIFY_DONE;
>> +
>> +if (memcmp(dev_iface->ifa_label, "wlan0", 5) &&
>> +memcmp(dev_iface->ifa_label, "p2p0", 4))
>> +return NOTIFY_DONE;
> That too. What???
Purpose of this function is to deferred going into powsersave till the
IP address is acquired by the interfaces. This is to avoid any issue in
acquiring the IP address(due to power save mode).
We will investigate and check the better to handle this condition.
>> +dev = (struct net_device *)dev_iface->ifa_dev->dev;
>> +if (!dev->ieee80211_ptr || !dev->ieee80211_ptr->wiphy)
>> +return NOTIFY_DONE;
>> +
>> +priv = wiphy_priv(dev->ieee80211_ptr->wiphy);
>> +if (!priv)
>> +return NOTIFY_DONE;
>> +
>> +hif_drv = (struct host_if_drv *)priv->hif_drv;
>> +vif = netdev_priv(dev);
>> +if (!vif || !hif_drv)
>> +return NOTIFY_DONE;
>> +
>> +switch (event) {
>> +case NETDEV_UP:
>> +if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
>> +hif_drv->ifc_up = 1;
>> +vif->obtaining_ip = false;
>> +del_timer(>during_ip_timer);
>> +}
>> +
>> +if (vif->wilc->enable_ps)
>> +wilc_set_power_mgmt(vif, 1, 0);
>> +
>> +netdev_dbg(dev, "[%s] Up IP\n", dev_iface->ifa_label);
>> +
>> +ip_addr_buf = (char *)_iface->ifa_address;
>> +netdev_dbg(dev, "IP add=%d:%d:%d:%d\n",
>> + ip_addr_buf[0], ip_addr_buf[1],
>> + ip_addr_buf[2], ip_addr_buf[3]);
> %pI4, I believe, but I think you should just remove it, it likely won't
> have an IP address anyway, and you might have multiple, and so this is
> just broken.
>
>> +eth_h = (struct ethhdr *)(skb->data);
>> +if (eth_h->h_proto == cpu_to_be16(0x8e88))
>> +netdev_dbg(ndev, "EAPOL transmitted\n");
> Err, no, just remove that.
Ok.
>> +ih = (struct iphdr *)(skb->data + sizeof(struct ethhdr));
> Sure, everything is IP. You just checked that it wasn't EAPOL?
>
>> +udp_buf = (char *)ih + sizeof(struct iphdr);
>> +if ((udp_buf[1] == 68 && udp_buf[3] == 67) ||
>> +(udp_buf[1] == 67 && udp_buf[3] == 68))
>> +netdev_dbg(ndev, "DHCP Message transmitted, type:%x %x %x\n",
>> + udp_buf[248], udp_buf[249], udp_buf[250]);
> Umm... no. Just remove that too.
>
Ok
>> +vif->netstats.tx_packets++;
>> +vif->netstats.tx_bytes += tx_data->size;
>> +tx_data->bssid = wilc->vif[vif->idx]->bssid;
>> +queue_count = wilc_wlan_txq_add_net_pkt(ndev, (void *)tx_data,
>> +tx_data->buff, tx_data->size,
>> +linux_wlan_tx_complete);
>> +
>> +if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
>> +netif_stop_queue(wilc->vif[0]->ndev);
>> +netif_stop_queue(wilc->vif[1]->ndev);
>> +}
> It seems like a pretty bad idea to hard-code two interfaces, we do
> dynamic addition/removal these days, in *particular* for P2P.
>
Did you mean it not good to call stop queue for both the interfaces.
Can you please provide some more details about this comments.
>> +static int wilc_mac_close(struct net_device *ndev)
>> +{
>> +struct
Re: [PATCH 13/19] wilc: add linux_wlan.c
On Wed, 2018-09-26 at 15:55 +0530, Ajay Singh wrote:
> Moved '/driver/staging/wilc1000/linux_wlan.c' to
> 'drivers/net/wireless/microchip/wilc/'.
>
> Signed-off-by: Ajay Singh
> ---
> drivers/net/wireless/microchip/wilc/linux_wlan.c | 1161
> ++
> 1 file changed, 1161 insertions(+)
> create mode 100644 drivers/net/wireless/microchip/wilc/linux_wlan.c
Hmm. It's pretty obviously a linux driver, what's the point?
> diff --git a/drivers/net/wireless/microchip/wilc/linux_wlan.c
> b/drivers/net/wireless/microchip/wilc/linux_wlan.c
> new file mode 100644
> index 000..76c9012
> --- /dev/null
> +++ b/drivers/net/wireless/microchip/wilc/linux_wlan.c
> @@ -0,0 +1,1161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2012 - 2018 Microchip Technology Inc., and its subsidiaries.
> + * All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "wilc_wfi_cfgoperations.h"
> +
> +static int dev_state_ev_handler(struct notifier_block *this,
> + unsigned long event, void *ptr)
> +{
> + struct in_ifaddr *dev_iface = ptr;
> + struct wilc_priv *priv;
> + struct host_if_drv *hif_drv;
> + struct net_device *dev;
> + u8 *ip_addr_buf;
> + struct wilc_vif *vif;
> + u8 null_ip[4] = {0};
> + char wlan_dev_name[5] = "wlan0";
Regardless of what you're trying to do, thta seems like a bad idea.
> + if (!dev_iface || !dev_iface->ifa_dev || !dev_iface->ifa_dev->dev)
> + return NOTIFY_DONE;
> +
> + if (memcmp(dev_iface->ifa_label, "wlan0", 5) &&
> + memcmp(dev_iface->ifa_label, "p2p0", 4))
> + return NOTIFY_DONE;
That too. What???
> + dev = (struct net_device *)dev_iface->ifa_dev->dev;
> + if (!dev->ieee80211_ptr || !dev->ieee80211_ptr->wiphy)
> + return NOTIFY_DONE;
> +
> + priv = wiphy_priv(dev->ieee80211_ptr->wiphy);
> + if (!priv)
> + return NOTIFY_DONE;
> +
> + hif_drv = (struct host_if_drv *)priv->hif_drv;
> + vif = netdev_priv(dev);
> + if (!vif || !hif_drv)
> + return NOTIFY_DONE;
> +
> + switch (event) {
> + case NETDEV_UP:
> + if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
> + hif_drv->ifc_up = 1;
> + vif->obtaining_ip = false;
> + del_timer(>during_ip_timer);
> + }
> +
> + if (vif->wilc->enable_ps)
> + wilc_set_power_mgmt(vif, 1, 0);
> +
> + netdev_dbg(dev, "[%s] Up IP\n", dev_iface->ifa_label);
> +
> + ip_addr_buf = (char *)_iface->ifa_address;
> + netdev_dbg(dev, "IP add=%d:%d:%d:%d\n",
> +ip_addr_buf[0], ip_addr_buf[1],
> +ip_addr_buf[2], ip_addr_buf[3]);
%pI4, I believe, but I think you should just remove it, it likely won't
have an IP address anyway, and you might have multiple, and so this is
just broken.
> + eth_h = (struct ethhdr *)(skb->data);
> + if (eth_h->h_proto == cpu_to_be16(0x8e88))
> + netdev_dbg(ndev, "EAPOL transmitted\n");
Err, no, just remove that.
> + ih = (struct iphdr *)(skb->data + sizeof(struct ethhdr));
Sure, everything is IP. You just checked that it wasn't EAPOL?
> + udp_buf = (char *)ih + sizeof(struct iphdr);
> + if ((udp_buf[1] == 68 && udp_buf[3] == 67) ||
> + (udp_buf[1] == 67 && udp_buf[3] == 68))
> + netdev_dbg(ndev, "DHCP Message transmitted, type:%x %x %x\n",
> +udp_buf[248], udp_buf[249], udp_buf[250]);
Umm... no. Just remove that too.
> + vif->netstats.tx_packets++;
> + vif->netstats.tx_bytes += tx_data->size;
> + tx_data->bssid = wilc->vif[vif->idx]->bssid;
> + queue_count = wilc_wlan_txq_add_net_pkt(ndev, (void *)tx_data,
> + tx_data->buff, tx_data->size,
> + linux_wlan_tx_complete);
> +
> + if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
> + netif_stop_queue(wilc->vif[0]->ndev);
> + netif_stop_queue(wilc->vif[1]->ndev);
> + }
It seems like a pretty bad idea to hard-code two interfaces, we do
dynamic addition/removal these days, in *particular* for P2P.
> +static int wilc_mac_close(struct net_device *ndev)
> +{
> + struct wilc_priv *priv;
> + struct wilc_vif *vif = netdev_priv(ndev);
> + struct host_if_drv *hif_drv;
> + struct wilc *wl;
> +
> + if (!vif || !vif->ndev || !vif->ndev->ieee80211_ptr ||
> + !vif->ndev->ieee80211_ptr->wiphy)
> + return 0;
I'm not really sure why you're so paranoid, none of that can possibly
happen.
> + priv = wiphy_priv(vif->ndev->ieee80211_ptr->wiphy);
> + wl = vif->wilc;
> +
> + if (!priv)
> + return 0;
Nor can this.
> + hif_drv = (struct host_if_drv *)priv->hif_drv;
> +
> +
[PATCH 13/19] wilc: add linux_wlan.c
Moved '/driver/staging/wilc1000/linux_wlan.c' to
'drivers/net/wireless/microchip/wilc/'.
Signed-off-by: Ajay Singh
---
drivers/net/wireless/microchip/wilc/linux_wlan.c | 1161 ++
1 file changed, 1161 insertions(+)
create mode 100644 drivers/net/wireless/microchip/wilc/linux_wlan.c
diff --git a/drivers/net/wireless/microchip/wilc/linux_wlan.c
b/drivers/net/wireless/microchip/wilc/linux_wlan.c
new file mode 100644
index 000..76c9012
--- /dev/null
+++ b/drivers/net/wireless/microchip/wilc/linux_wlan.c
@@ -0,0 +1,1161 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2012 - 2018 Microchip Technology Inc., and its subsidiaries.
+ * All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "wilc_wfi_cfgoperations.h"
+
+static int dev_state_ev_handler(struct notifier_block *this,
+ unsigned long event, void *ptr)
+{
+ struct in_ifaddr *dev_iface = ptr;
+ struct wilc_priv *priv;
+ struct host_if_drv *hif_drv;
+ struct net_device *dev;
+ u8 *ip_addr_buf;
+ struct wilc_vif *vif;
+ u8 null_ip[4] = {0};
+ char wlan_dev_name[5] = "wlan0";
+
+ if (!dev_iface || !dev_iface->ifa_dev || !dev_iface->ifa_dev->dev)
+ return NOTIFY_DONE;
+
+ if (memcmp(dev_iface->ifa_label, "wlan0", 5) &&
+ memcmp(dev_iface->ifa_label, "p2p0", 4))
+ return NOTIFY_DONE;
+
+ dev = (struct net_device *)dev_iface->ifa_dev->dev;
+ if (!dev->ieee80211_ptr || !dev->ieee80211_ptr->wiphy)
+ return NOTIFY_DONE;
+
+ priv = wiphy_priv(dev->ieee80211_ptr->wiphy);
+ if (!priv)
+ return NOTIFY_DONE;
+
+ hif_drv = (struct host_if_drv *)priv->hif_drv;
+ vif = netdev_priv(dev);
+ if (!vif || !hif_drv)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_UP:
+ if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
+ hif_drv->ifc_up = 1;
+ vif->obtaining_ip = false;
+ del_timer(>during_ip_timer);
+ }
+
+ if (vif->wilc->enable_ps)
+ wilc_set_power_mgmt(vif, 1, 0);
+
+ netdev_dbg(dev, "[%s] Up IP\n", dev_iface->ifa_label);
+
+ ip_addr_buf = (char *)_iface->ifa_address;
+ netdev_dbg(dev, "IP add=%d:%d:%d:%d\n",
+ ip_addr_buf[0], ip_addr_buf[1],
+ ip_addr_buf[2], ip_addr_buf[3]);
+
+ break;
+
+ case NETDEV_DOWN:
+ if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
+ hif_drv->ifc_up = 0;
+ vif->obtaining_ip = false;
+ }
+
+ if (memcmp(dev_iface->ifa_label, wlan_dev_name, 5) == 0)
+ wilc_set_power_mgmt(vif, 0, 0);
+
+ wilc_resolve_disconnect_aberration(vif);
+
+ netdev_dbg(dev, "[%s] Down IP\n", dev_iface->ifa_label);
+
+ ip_addr_buf = null_ip;
+ netdev_dbg(dev, "IP add=%d:%d:%d:%d\n",
+ ip_addr_buf[0], ip_addr_buf[1],
+ ip_addr_buf[2], ip_addr_buf[3]);
+
+ break;
+
+ default:
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static irqreturn_t isr_uh_routine(int irq, void *user_data)
+{
+ struct net_device *dev = user_data;
+ struct wilc_vif *vif = netdev_priv(dev);
+ struct wilc *wilc = vif->wilc;
+
+ if (wilc->close) {
+ netdev_err(dev, "Can't handle UH interrupt\n");
+ return IRQ_HANDLED;
+ }
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t isr_bh_routine(int irq, void *userdata)
+{
+ struct net_device *dev = userdata;
+ struct wilc_vif *vif = netdev_priv(userdata);
+ struct wilc *wilc = vif->wilc;
+
+ if (wilc->close) {
+ netdev_err(dev, "Can't handle BH interrupt\n");
+ return IRQ_HANDLED;
+ }
+
+ wilc_handle_isr(wilc);
+
+ return IRQ_HANDLED;
+}
+
+static int init_irq(struct net_device *dev)
+{
+ int ret = 0;
+ struct wilc_vif *vif = netdev_priv(dev);
+ struct wilc *wl = vif->wilc;
+
+ ret = gpiod_direction_input(wl->gpio_irq);
+ if (ret) {
+ netdev_err(dev, "could not obtain gpio for WILC_INTR\n");
+ return ret;
+ }
+
+ wl->dev_irq_num = gpiod_to_irq(wl->gpio_irq);
+
+ ret = request_threaded_irq(wl->dev_irq_num, isr_uh_routine,
+ isr_bh_routine,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "WILC_IRQ", dev);
+ if (ret < 0)
+ netdev_err(dev, "Failed to request IRQ\n");
+ else
+ netdev_dbg(dev, "IRQ request
