[PATCH] HID: multitouch: fix calculation of last slot field in multi-touch reports

[Ben Chan]

According to [1] and also seemingly agreed by [2], the Scan Time usage
(0x0D 0x56) is a report level usage, not a contact level usage.

However, the hid-multitouch driver currently includes HID_DG_SCANTIME
when calculating `td->last_slot_field', which may lead to
mt_complete_slot() being prematurely called in certain cases (e.g. when
each touch input report includes more than one contact and the Scan Time
usage appears before any contact logical collection).

This patch fixes the issue by skipping mt_store_field() on
HID_DG_SCANTIME, similar to how HID_DG_CONTACTCOUNT and
HID_DG_CONTACTMAX are handled.

[1] 
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-required-hid-top-level-collections#windows-precision-touchpad-input-reports
[2] https://patchwork.kernel.org/patch/1742181/

Fixes: 29cc309d8bf19 ("HID: hid-multitouch: forward MSC_TIMESTAMP")
Signed-off-by: Ben Chan 
---
 drivers/hid/hid-multitouch.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index dad2fbb0e3f8..161551aab496 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -620,13 +620,16 @@ static int mt_touch_input_mapping(struct hid_device 
*hdev, struct hid_input *hi,
hid_map_usage(hi, usage, bit, max,
EV_MSC, MSC_TIMESTAMP);
input_set_capability(hi->input, EV_MSC, MSC_TIMESTAMP);
-   mt_store_field(usage, td, hi);
/* Ignore if indexes are out of bounds. */
if (field->index >= field->report->maxfield ||
usage->usage_index >= field->report_count)
return 1;
td->scantime_index = field->index;
td->scantime_val_index = usage->usage_index;
+   /*
+* We don't set td->last_slot_field as scan time is
+* global to the report.
+*/
return 1;
case HID_DG_CONTACTCOUNT:
/* Ignore if indexes are out of bounds. */
-- 
2.17.0.921.gf22659ad46-goog

[PATCH] HID: multitouch: fix calculation of last slot field in multi-touch reports

[Ben Chan]

According to [1] and also seemingly agreed by [2], the Scan Time usage
(0x0D 0x56) is a report level usage, not a contact level usage.

However, the hid-multitouch driver currently includes HID_DG_SCANTIME
when calculating `td->last_slot_field', which may lead to
mt_complete_slot() being prematurely called in certain cases (e.g. when
each touch input report includes more than one contact and the Scan Time
usage appears before any contact logical collection).

This patch fixes the issue by skipping mt_store_field() on
HID_DG_SCANTIME, similar to how HID_DG_CONTACTCOUNT and
HID_DG_CONTACTMAX are handled.

[1] 
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-required-hid-top-level-collections#windows-precision-touchpad-input-reports
[2] https://patchwork.kernel.org/patch/1742181/

Fixes: 29cc309d8bf19 ("HID: hid-multitouch: forward MSC_TIMESTAMP")
Signed-off-by: Ben Chan 
---
 drivers/hid/hid-multitouch.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index dad2fbb0e3f8..161551aab496 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -620,13 +620,16 @@ static int mt_touch_input_mapping(struct hid_device 
*hdev, struct hid_input *hi,
hid_map_usage(hi, usage, bit, max,
EV_MSC, MSC_TIMESTAMP);
input_set_capability(hi->input, EV_MSC, MSC_TIMESTAMP);
-   mt_store_field(usage, td, hi);
/* Ignore if indexes are out of bounds. */
if (field->index >= field->report->maxfield ||
usage->usage_index >= field->report_count)
return 1;
td->scantime_index = field->index;
td->scantime_val_index = usage->usage_index;
+   /*
+* We don't set td->last_slot_field as scan time is
+* global to the report.
+*/
return 1;
case HID_DG_CONTACTCOUNT:
/* Ignore if indexes are out of bounds. */
-- 
2.17.0.921.gf22659ad46-goog

[PATCH 1/1] staging: skein: Delete a useless white space line

[Ben Chan]

From: Kroderia 

Delete a useless white space line according to the coding style.

Signed-off-by: Ben Chan 
---
 drivers/staging/skein/skein_generic.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/skein/skein_generic.c 
b/drivers/staging/skein/skein_generic.c
index 85bd7d0..899078f 100644
--- a/drivers/staging/skein/skein_generic.c
+++ b/drivers/staging/skein/skein_generic.c
@@ -191,7 +191,6 @@ static int __init skein_generic_init(void)
 
return 0;
 
-   
 unreg512:
crypto_unregister_shash();
 unreg256:
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/1] staging: skein: Delete a useless white space line

[Ben Chan]

From: Kroderia krode...@gmail.com

Delete a useless white space line according to the coding style.

Signed-off-by: Ben Chan krode...@gmail.com
---
 drivers/staging/skein/skein_generic.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/skein/skein_generic.c 
b/drivers/staging/skein/skein_generic.c
index 85bd7d0..899078f 100644
--- a/drivers/staging/skein/skein_generic.c
+++ b/drivers/staging/skein/skein_generic.c
@@ -191,7 +191,6 @@ static int __init skein_generic_init(void)
 
return 0;
 
-   
 unreg512:
crypto_unregister_shash(alg512);
 unreg256:
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: add help text to Kconfig

[Ben Chan]

The descriptions are provided by GCT Semiconductor, Inc.

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/Kconfig | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index dd8a391..5836503 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -6,21 +6,29 @@ menuconfig WIMAX_GDM72XX
tristate "GCT GDM72xx WiMAX support"
depends on NET && (USB || MMC)
help
- Support for the GCT GDM72xx WiMAX chip
+ Support a WiMAX module based on the GCT GDM72xx WiMAX chip.
 
 if WIMAX_GDM72XX
 
 config WIMAX_GDM72XX_QOS
bool "Enable QoS support"
default n
+   help
+ Enable Quality of Service support based on the data protocol of
+ transmitting packets.
 
 config WIMAX_GDM72XX_K_MODE
bool "Enable K mode"
default n
+   help
+ Enable support for proprietary functions for KT (Korea Telecom).
 
 config WIMAX_GDM72XX_WIMAX2
bool "Enable WiMAX2 support"
default n
+   help
+ Enable support for transmitting multiple packets (packet
+ aggregation) from the WiMAX module to the host processor.
 
 choice
prompt "Select interface"
@@ -28,10 +36,16 @@ choice
 config WIMAX_GDM72XX_USB
bool "USB interface"
depends on (USB = y || USB = WIMAX_GDM72XX)
+   help
+ Select this option if the WiMAX module interfaces with the host
+ processor via USB.
 
 config WIMAX_GDM72XX_SDIO
bool "SDIO interface"
depends on (MMC = y || MMC = WIMAX_GDM72XX)
+   help
+ Select this option if the WiMAX module interfaces with the host
+ processor via SDIO.
 
 endchoice
 
@@ -40,6 +54,9 @@ if WIMAX_GDM72XX_USB
 config WIMAX_GDM72XX_USB_PM
bool "Enable power management support"
depends on PM_RUNTIME
+   help
+ Enable USB power management in order to reduce power consumption
+ while the interface is not in use.
 
 endif # WIMAX_GDM72XX_USB
 
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: add help text to Kconfig

[Ben Chan]

The descriptions are provided by GCT Semiconductor, Inc.

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/Kconfig | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index dd8a391..5836503 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -6,21 +6,29 @@ menuconfig WIMAX_GDM72XX
tristate GCT GDM72xx WiMAX support
depends on NET  (USB || MMC)
help
- Support for the GCT GDM72xx WiMAX chip
+ Support a WiMAX module based on the GCT GDM72xx WiMAX chip.
 
 if WIMAX_GDM72XX
 
 config WIMAX_GDM72XX_QOS
bool Enable QoS support
default n
+   help
+ Enable Quality of Service support based on the data protocol of
+ transmitting packets.
 
 config WIMAX_GDM72XX_K_MODE
bool Enable K mode
default n
+   help
+ Enable support for proprietary functions for KT (Korea Telecom).
 
 config WIMAX_GDM72XX_WIMAX2
bool Enable WiMAX2 support
default n
+   help
+ Enable support for transmitting multiple packets (packet
+ aggregation) from the WiMAX module to the host processor.
 
 choice
prompt Select interface
@@ -28,10 +36,16 @@ choice
 config WIMAX_GDM72XX_USB
bool USB interface
depends on (USB = y || USB = WIMAX_GDM72XX)
+   help
+ Select this option if the WiMAX module interfaces with the host
+ processor via USB.
 
 config WIMAX_GDM72XX_SDIO
bool SDIO interface
depends on (MMC = y || MMC = WIMAX_GDM72XX)
+   help
+ Select this option if the WiMAX module interfaces with the host
+ processor via SDIO.
 
 endchoice
 
@@ -40,6 +54,9 @@ if WIMAX_GDM72XX_USB
 config WIMAX_GDM72XX_USB_PM
bool Enable power management support
depends on PM_RUNTIME
+   help
+ Enable USB power management in order to reduce power consumption
+ while the interface is not in use.
 
 endif # WIMAX_GDM72XX_USB
 
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: move T_CAPABILITY bit definitions to hci.h

[Ben Chan]

On Tue, Jul 1, 2014 at 6:00 AM, Michalis Pappas  wrote:
> Signed-off-by: Michalis Pappas 
> ---
>  drivers/staging/gdm72xx/gdm_wimax.c | 10 +++---
>  drivers/staging/gdm72xx/hci.h   |  6 ++
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
> b/drivers/staging/gdm72xx/gdm_wimax.c
> index 63a760b..1fc64a9 100644
> --- a/drivers/staging/gdm72xx/gdm_wimax.c
> +++ b/drivers/staging/gdm72xx/gdm_wimax.c
> @@ -600,10 +600,6 @@ static void gdm_wimax_prepare_device(struct net_device 
> *dev)
> u16 len = 0;
> u32 val = 0;
>
> -   #define BIT_MULTI_CS0
> -   #define BIT_WIMAX   1
> -   #define BIT_QOS 2
> -   #define BIT_AGGREGATION 3
>
> /* GetInformation mac address */
> len = 0;
> @@ -612,12 +608,12 @@ static void gdm_wimax_prepare_device(struct net_device 
> *dev)
> hci->length = H2B(len);
> gdm_wimax_send(nic, hci, HCI_HEADER_SIZE+len);
>
> -   val = (1< +   val = (1 << T_CAPABILITY_BIT_WIMAX) | (1 << 
> T_CAPABILITY_BIT_MULTI_CS);
> #if defined(CONFIG_WIMAX_GDM72XX_QOS)
> -   val |= (1< +   val |= (1 << T_CAPABILITY_BIT_QOS);
> #endif
> #if defined(CONFIG_WIMAX_GDM72XX_WIMAX2)
> -   val |= (1< +   val |= (1 << T_CAPABILITY_BIT_AGGREGATION);
> #endif

[Ben] We can simply the code by defining these constants as bitmasks
instead, e.g.

T_CAPABILITY_MULTI_CS  (1 << 0)
T_CAPABILITY_WIMAX  (1 << 1)
T_CAPABILITY_QOS  (1 << 2)
T_CAPABILITY_AGGREGATION  (1 << 3)

> /* Set capability */
> diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
> index 059ba00..0cdd1fc 100644
> --- a/drivers/staging/gdm72xx/hci.h
> +++ b/drivers/staging/gdm72xx/hci.h
> @@ -198,6 +198,12 @@
>  #define T_FFTSIZE  (0xda   | (4 << 16))
>  #define T_DUPLEX_MODE  (0xdb   | (4 << 16))
>
> +/* T_CAPABILITY */
> +#define T_CAPABILITY_BIT_MULTI_CS  0
> +#define T_CAPABILITY_BIT_WIMAX 1
> +#define T_CAPABILITY_BIT_QOS   2
> +#define T_CAPABILITY_BIT_AGGREGATION   3
> +
>  struct hci_s {
> unsigned short cmd_evt;
> unsigned short length;
> --
> 1.8.4
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: clean up endianness conversions

[Ben Chan]

This patch cleans up the endianness conversions in the gdm72xx driver:
- Directly use the generic byte-reordering macros for endianness
  conversion instead of some custom-defined macros.
- Convert values on the constant side instead of the variable side when
  appropriate.
- Add endianness annotations.

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_qos.c   |  4 +--
 drivers/staging/gdm72xx/gdm_sdio.c  |  5 +--
 drivers/staging/gdm72xx/gdm_usb.c   | 12 +++
 drivers/staging/gdm72xx/gdm_wimax.c | 64 +++--
 drivers/staging/gdm72xx/gdm_wimax.h | 10 --
 drivers/staging/gdm72xx/hci.h   |  6 ++--
 drivers/staging/gdm72xx/usb_boot.c  | 10 +++---
 7 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index b08c8e1..af24c57 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -24,8 +24,6 @@
 #include "hci.h"
 #include "gdm_qos.h"
 
-#define B2H(x) __be16_to_cpu(x)
-
 #define MAX_FREE_LIST_CNT  32
 static struct {
struct list_head head;
@@ -266,7 +264,7 @@ int gdm_qos_send_hci_pkt(struct sk_buff *skb, struct 
net_device *dev)
 
tcph = (struct tcphdr *)iph + iph->ihl*4;
 
-   if (B2H(ethh->h_proto) == ETH_P_IP) {
+   if (ethh->h_proto == cpu_to_be16(ETH_P_IP)) {
if (qcb->qos_list_cnt && !qos_free_list.cnt) {
entry = alloc_qos_entry();
entry->skb = skb;
diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 9d2de6f..a903173 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -275,8 +275,9 @@ static void send_sdu(struct sdio_func *func, struct tx_cxt 
*tx)
aggr_len = pos;
 
hci = (struct hci_s *)(tx->sdu_buf + TYPE_A_HEADER_SIZE);
-   hci->cmd_evt = H2B(WIMAX_TX_SDU_AGGR);
-   hci->length = H2B(aggr_len - TYPE_A_HEADER_SIZE - HCI_HEADER_SIZE);
+   hci->cmd_evt = cpu_to_be16(WIMAX_TX_SDU_AGGR);
+   hci->length = cpu_to_be16(aggr_len - TYPE_A_HEADER_SIZE -
+ HCI_HEADER_SIZE);
 
spin_unlock_irqrestore(>lock, flags);
 
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 971976c..5a6b86a 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -36,10 +36,6 @@ MODULE_DEVICE_TABLE(usb, id_table);
 
 #define GDM7205_PADDING256
 
-#define H2B(x) __cpu_to_be16(x)
-#define B2H(x) __be16_to_cpu(x)
-#define DB2H(x)__be32_to_cpu(x)
-
 #define DOWNLOAD_CONF_VALUE0x21
 
 #ifdef CONFIG_WIMAX_GDM72XX_K_MODE
@@ -541,9 +537,9 @@ static int gdm_usb_probe(struct usb_interface *intf,
bConfigurationValue = usbdev->actconfig->desc.bConfigurationValue;
 
/*USB description is set up with Little-Endian*/
-   idVendor = L2H(usbdev->descriptor.idVendor);
-   idProduct = L2H(usbdev->descriptor.idProduct);
-   bcdDevice = L2H(usbdev->descriptor.bcdDevice);
+   idVendor = le16_to_cpu(usbdev->descriptor.idVendor);
+   idProduct = le16_to_cpu(usbdev->descriptor.idProduct);
+   bcdDevice = le16_to_cpu(usbdev->descriptor.bcdDevice);
 
dev_info(>dev, "Found GDM USB VID = 0x%04x PID = 0x%04x...\n",
 idVendor, idProduct);
@@ -626,7 +622,7 @@ static void gdm_usb_disconnect(struct usb_interface *intf)
phy_dev = usb_get_intfdata(intf);
 
/*USB description is set up with Little-Endian*/
-   idProduct = L2H(usbdev->descriptor.idProduct);
+   idProduct = le16_to_cpu(usbdev->descriptor.idProduct);
 
if (idProduct != EMERGENCY_PID &&
bConfigurationValue != DOWNLOAD_CONF_VALUE &&
diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index 4148013..cc3e80a 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -166,6 +166,7 @@ static void dump_eth_packet(struct net_device *dev, const 
char *title,
 static inline int gdm_wimax_header(struct sk_buff **pskb)
 {
u16 buf[HCI_HEADER_SIZE / sizeof(u16)];
+   struct hci_s *hci = (struct hci_s *)buf;
struct sk_buff *skb = *pskb;
 
if (unlikely(skb_headroom(skb) < HCI_HEADER_SIZE)) {
@@ -181,8 +182,8 @@ static inline int gdm_wimax_header(struct sk_buff **pskb)
}
 
skb_push(skb, HCI_HEADER_SIZE);
-   buf[0] = H2B(WIMAX_TX_SDU);
-   buf[1] = H2B(skb->len - HCI_HEADER_SIZE);
+   hci->cmd_evt = cpu_to_be16(WIMAX_TX_SDU);
+   hci->length = cpu_to_be16(skb->len - HCI_HEADER_SIZE);
memcpy(skb->data, buf, HCI_HEADER_SIZE);
 
*pskb = skb;
@@ -409,7 +410,7 @@ static int gdm_wimax_set_config(struct net_device *dev

[PATCH] staging: gdm72xx: clean up endianness conversions

[Ben Chan]

This patch cleans up the endianness conversions in the gdm72xx driver:
- Directly use the generic byte-reordering macros for endianness
  conversion instead of some custom-defined macros.
- Convert values on the constant side instead of the variable side when
  appropriate.
- Add endianness annotations.

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_qos.c   |  4 +--
 drivers/staging/gdm72xx/gdm_sdio.c  |  5 +--
 drivers/staging/gdm72xx/gdm_usb.c   | 12 +++
 drivers/staging/gdm72xx/gdm_wimax.c | 64 +++--
 drivers/staging/gdm72xx/gdm_wimax.h | 10 --
 drivers/staging/gdm72xx/hci.h   |  6 ++--
 drivers/staging/gdm72xx/usb_boot.c  | 10 +++---
 7 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index b08c8e1..af24c57 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -24,8 +24,6 @@
 #include hci.h
 #include gdm_qos.h
 
-#define B2H(x) __be16_to_cpu(x)
-
 #define MAX_FREE_LIST_CNT  32
 static struct {
struct list_head head;
@@ -266,7 +264,7 @@ int gdm_qos_send_hci_pkt(struct sk_buff *skb, struct 
net_device *dev)
 
tcph = (struct tcphdr *)iph + iph-ihl*4;
 
-   if (B2H(ethh-h_proto) == ETH_P_IP) {
+   if (ethh-h_proto == cpu_to_be16(ETH_P_IP)) {
if (qcb-qos_list_cnt  !qos_free_list.cnt) {
entry = alloc_qos_entry();
entry-skb = skb;
diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 9d2de6f..a903173 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -275,8 +275,9 @@ static void send_sdu(struct sdio_func *func, struct tx_cxt 
*tx)
aggr_len = pos;
 
hci = (struct hci_s *)(tx-sdu_buf + TYPE_A_HEADER_SIZE);
-   hci-cmd_evt = H2B(WIMAX_TX_SDU_AGGR);
-   hci-length = H2B(aggr_len - TYPE_A_HEADER_SIZE - HCI_HEADER_SIZE);
+   hci-cmd_evt = cpu_to_be16(WIMAX_TX_SDU_AGGR);
+   hci-length = cpu_to_be16(aggr_len - TYPE_A_HEADER_SIZE -
+ HCI_HEADER_SIZE);
 
spin_unlock_irqrestore(tx-lock, flags);
 
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 971976c..5a6b86a 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -36,10 +36,6 @@ MODULE_DEVICE_TABLE(usb, id_table);
 
 #define GDM7205_PADDING256
 
-#define H2B(x) __cpu_to_be16(x)
-#define B2H(x) __be16_to_cpu(x)
-#define DB2H(x)__be32_to_cpu(x)
-
 #define DOWNLOAD_CONF_VALUE0x21
 
 #ifdef CONFIG_WIMAX_GDM72XX_K_MODE
@@ -541,9 +537,9 @@ static int gdm_usb_probe(struct usb_interface *intf,
bConfigurationValue = usbdev-actconfig-desc.bConfigurationValue;
 
/*USB description is set up with Little-Endian*/
-   idVendor = L2H(usbdev-descriptor.idVendor);
-   idProduct = L2H(usbdev-descriptor.idProduct);
-   bcdDevice = L2H(usbdev-descriptor.bcdDevice);
+   idVendor = le16_to_cpu(usbdev-descriptor.idVendor);
+   idProduct = le16_to_cpu(usbdev-descriptor.idProduct);
+   bcdDevice = le16_to_cpu(usbdev-descriptor.bcdDevice);
 
dev_info(intf-dev, Found GDM USB VID = 0x%04x PID = 0x%04x...\n,
 idVendor, idProduct);
@@ -626,7 +622,7 @@ static void gdm_usb_disconnect(struct usb_interface *intf)
phy_dev = usb_get_intfdata(intf);
 
/*USB description is set up with Little-Endian*/
-   idProduct = L2H(usbdev-descriptor.idProduct);
+   idProduct = le16_to_cpu(usbdev-descriptor.idProduct);
 
if (idProduct != EMERGENCY_PID 
bConfigurationValue != DOWNLOAD_CONF_VALUE 
diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index 4148013..cc3e80a 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -166,6 +166,7 @@ static void dump_eth_packet(struct net_device *dev, const 
char *title,
 static inline int gdm_wimax_header(struct sk_buff **pskb)
 {
u16 buf[HCI_HEADER_SIZE / sizeof(u16)];
+   struct hci_s *hci = (struct hci_s *)buf;
struct sk_buff *skb = *pskb;
 
if (unlikely(skb_headroom(skb)  HCI_HEADER_SIZE)) {
@@ -181,8 +182,8 @@ static inline int gdm_wimax_header(struct sk_buff **pskb)
}
 
skb_push(skb, HCI_HEADER_SIZE);
-   buf[0] = H2B(WIMAX_TX_SDU);
-   buf[1] = H2B(skb-len - HCI_HEADER_SIZE);
+   hci-cmd_evt = cpu_to_be16(WIMAX_TX_SDU);
+   hci-length = cpu_to_be16(skb-len - HCI_HEADER_SIZE);
memcpy(skb-data, buf, HCI_HEADER_SIZE);
 
*pskb = skb;
@@ -409,7 +410,7 @@ static int gdm_wimax_set_config(struct net_device *dev, 
struct ifmap *map)
 static void __gdm_wimax_set_mac_addr(struct net_device *dev, char *mac_addr)
 {
u16 hci_pkt_buf[32 / sizeof(u16

Re: [PATCH] staging: gdm72xx: move T_CAPABILITY bit definitions to hci.h

[Ben Chan]

On Tue, Jul 1, 2014 at 6:00 AM, Michalis Pappas mpap...@fastmail.fm wrote:
 Signed-off-by: Michalis Pappas mpap...@fastmail.fm
 ---
  drivers/staging/gdm72xx/gdm_wimax.c | 10 +++---
  drivers/staging/gdm72xx/hci.h   |  6 ++
  2 files changed, 9 insertions(+), 7 deletions(-)

 diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
 b/drivers/staging/gdm72xx/gdm_wimax.c
 index 63a760b..1fc64a9 100644
 --- a/drivers/staging/gdm72xx/gdm_wimax.c
 +++ b/drivers/staging/gdm72xx/gdm_wimax.c
 @@ -600,10 +600,6 @@ static void gdm_wimax_prepare_device(struct net_device 
 *dev)
 u16 len = 0;
 u32 val = 0;

 -   #define BIT_MULTI_CS0
 -   #define BIT_WIMAX   1
 -   #define BIT_QOS 2
 -   #define BIT_AGGREGATION 3

 /* GetInformation mac address */
 len = 0;
 @@ -612,12 +608,12 @@ static void gdm_wimax_prepare_device(struct net_device 
 *dev)
 hci-length = H2B(len);
 gdm_wimax_send(nic, hci, HCI_HEADER_SIZE+len);

 -   val = (1BIT_WIMAX) | (1BIT_MULTI_CS);
 +   val = (1  T_CAPABILITY_BIT_WIMAX) | (1  
 T_CAPABILITY_BIT_MULTI_CS);
 #if defined(CONFIG_WIMAX_GDM72XX_QOS)
 -   val |= (1BIT_QOS);
 +   val |= (1  T_CAPABILITY_BIT_QOS);
 #endif
 #if defined(CONFIG_WIMAX_GDM72XX_WIMAX2)
 -   val |= (1BIT_AGGREGATION);
 +   val |= (1  T_CAPABILITY_BIT_AGGREGATION);
 #endif

[Ben] We can simply the code by defining these constants as bitmasks
instead, e.g.

T_CAPABILITY_MULTI_CS  (1  0)
T_CAPABILITY_WIMAX  (1  1)
T_CAPABILITY_QOS  (1  2)
T_CAPABILITY_AGGREGATION  (1  3)

 /* Set capability */
 diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
 index 059ba00..0cdd1fc 100644
 --- a/drivers/staging/gdm72xx/hci.h
 +++ b/drivers/staging/gdm72xx/hci.h
 @@ -198,6 +198,12 @@
  #define T_FFTSIZE  (0xda   | (4  16))
  #define T_DUPLEX_MODE  (0xdb   | (4  16))

 +/* T_CAPABILITY */
 +#define T_CAPABILITY_BIT_MULTI_CS  0
 +#define T_CAPABILITY_BIT_WIMAX 1
 +#define T_CAPABILITY_BIT_QOS   2
 +#define T_CAPABILITY_BIT_AGGREGATION   3
 +
  struct hci_s {
 unsigned short cmd_evt;
 unsigned short length;
 --
 1.8.4

 ___
 devel mailing list
 de...@linuxdriverproject.org
 http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] staging: gdm72xx: use consistent style for header guards

[Ben Chan]

Signed-off-by: Ben Chan 
---
I forgot to add the Signed-off-by stanza in the original patch.

 drivers/staging/gdm72xx/gdm_qos.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_sdio.h  | 6 +++---
 drivers/staging/gdm72xx/gdm_usb.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_wimax.h | 6 +++---
 drivers/staging/gdm72xx/hci.h   | 6 +++---
 drivers/staging/gdm72xx/netlink_k.h | 7 ---
 drivers/staging/gdm72xx/sdio_boot.h | 6 +++---
 drivers/staging/gdm72xx/usb_boot.h  | 6 +++---
 drivers/staging/gdm72xx/usb_ids.h   | 6 +++---
 drivers/staging/gdm72xx/wm_ioctl.h  | 7 ---
 10 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 8f742f3..bbc8aab 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(GDM_QOS_H_20090403)
-#define GDM_QOS_H_20090403
+#ifndef __GDM72XX_GDM_QOS_H__
+#define __GDM72XX_GDM_QOS_H__
 
 #include 
 #include 
@@ -71,4 +71,4 @@ void gdm_qos_release_list(void *nic_ptr);
 int gdm_qos_send_hci_pkt(struct sk_buff *skb, struct net_device *dev);
 void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int size);
 
-#endif
+#endif /* __GDM72XX_GDM_QOS_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_sdio.h 
b/drivers/staging/gdm72xx/gdm_sdio.h
index 0c0e2cb..77ad9d6 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.h
+++ b/drivers/staging/gdm72xx/gdm_sdio.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_SDIO_H__
-#define __GDM_SDIO_H__
+#ifndef __GDM72XX_GDM_SDIO_H__
+#define __GDM72XX_GDM_SDIO_H__
 
 #include 
 #include 
@@ -60,4 +60,4 @@ struct sdiowm_dev {
struct work_struct  ws;
 };
 
-#endif /* __GDM_SDIO_H__ */
+#endif /* __GDM72XX_GDM_SDIO_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_usb.h 
b/drivers/staging/gdm72xx/gdm_usb.h
index 3050652..8e58a25 100644
--- a/drivers/staging/gdm72xx/gdm_usb.h
+++ b/drivers/staging/gdm72xx/gdm_usb.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_USB_H__
-#define __GDM_USB_H__
+#ifndef __GDM72XX_GDM_USB_H__
+#define __GDM72XX_GDM_USB_H__
 
 #include 
 #include 
@@ -75,4 +75,4 @@ struct usbwm_dev {
int padding;
 };
 
-#endif /* __GDM_USB_H__ */
+#endif /* __GDM72XX_GDM_USB_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_wimax.h 
b/drivers/staging/gdm72xx/gdm_wimax.h
index 7e2c888..c640d2c 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.h
+++ b/drivers/staging/gdm72xx/gdm_wimax.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_WIMAX_H__
-#define __GDM_WIMAX_H__
+#ifndef __GDM72XX_GDM_WIMAX_H__
+#define __GDM72XX_GDM_WIMAX_H__
 
 #include 
 #include 
@@ -57,4 +57,4 @@ int register_wimax_device(struct phy_dev *phy_dev, struct 
device *pdev);
 int gdm_wimax_send_tx(struct sk_buff *skb, struct net_device *dev);
 void unregister_wimax_device(struct phy_dev *phy_dev);
 
-#endif
+#endif /* __GDM72XX_GDM_WIMAX_H__ */
diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
index 059ba00..ff6941e 100644
--- a/drivers/staging/gdm72xx/hci.h
+++ b/drivers/staging/gdm72xx/hci.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef HCI_H_20080801
-#define HCI_H_20080801
+#ifndef __GDM72XX_HCI_H__
+#define __GDM72XX_HCI_H__
 
 #define HCI_HEADER_SIZE4
 #define HCI_VALUE_OFFS (HCI_HEADER_SIZE)
@@ -204,4 +204,4 @@ struct hci_s {
unsigned char  data[0];
 } __packed;
 
-#endif
+#endif /* __GDM72XX_HCI_H__ */
diff --git a/drivers/staging/gdm72xx/netlink_k.h 
b/drivers/staging/gdm72xx/netlink_k.h
index b6caac1..1fe7198 100644
--- a/drivers/staging/gdm72xx/netlink_k.h
+++ b/drivers/staging/gdm72xx/netlink_k.h
@@ -11,8 +11,9 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(NETLINK_H_20081202)
-#define NETLINK_H_20081202
+#ifndef __GDM72XX_NETLINK_K_H__
+#define __GDM72XX_NETLINK_K_H__
+
 #include 
 #include 
 
@@ -21,4 +22,4 @@ struct sock *netlink_init(int unit, void (*cb)(struct 
net_device *dev, u16 type,
 void netlink_exit(struct sock *sock);
 int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len);
 
-#endif
+#endif /* __GDM72XX_NETLINK_K_H__ */
diff --git a/drivers/staging/gdm72xx/sdio_boot.h 
b/drivers/staging/gdm72xx/sdio_boot.h
index 045c1f4..e0800c6 100644
--- a/drivers/staging/gdm72xx/sdio_boot.h
+++ b/drivers/staging/gdm72xx/sdio_boot.h
@@ -11,11 +11,11 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __SDIO_BOOT_H__
-#define __SDIO_BOOT_H__
+#ifndef __GDM72XX_SDIO_BOOT_H__
+#define __GDM72XX_SDIO_BOOT_H__
 
 struct sdio_func;
 
 int sdio_boot(struct sdio_func *func);
 
-#endif /* __SDIO_BOOT_H__ */
+#endif /* __GDM72XX_SDIO_BOOT_H__ */
diff --git a/drivers/staging/gdm72xx/usb_boot.h 
b/drivers/staging/gdm72xx/usb_boot.h
index 05308e2..5bf7190 100644
--- a/drivers

[PATCH] staging: gdm72xx: use consistent style for header guards

[Ben Chan]

---
 drivers/staging/gdm72xx/gdm_qos.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_sdio.h  | 6 +++---
 drivers/staging/gdm72xx/gdm_usb.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_wimax.h | 6 +++---
 drivers/staging/gdm72xx/hci.h   | 6 +++---
 drivers/staging/gdm72xx/netlink_k.h | 7 ---
 drivers/staging/gdm72xx/sdio_boot.h | 6 +++---
 drivers/staging/gdm72xx/usb_boot.h  | 6 +++---
 drivers/staging/gdm72xx/usb_ids.h   | 6 +++---
 drivers/staging/gdm72xx/wm_ioctl.h  | 7 ---
 10 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 8f742f3..bbc8aab 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(GDM_QOS_H_20090403)
-#define GDM_QOS_H_20090403
+#ifndef __GDM72XX_GDM_QOS_H__
+#define __GDM72XX_GDM_QOS_H__
 
 #include 
 #include 
@@ -71,4 +71,4 @@ void gdm_qos_release_list(void *nic_ptr);
 int gdm_qos_send_hci_pkt(struct sk_buff *skb, struct net_device *dev);
 void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int size);
 
-#endif
+#endif /* __GDM72XX_GDM_QOS_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_sdio.h 
b/drivers/staging/gdm72xx/gdm_sdio.h
index 0c0e2cb..77ad9d6 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.h
+++ b/drivers/staging/gdm72xx/gdm_sdio.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_SDIO_H__
-#define __GDM_SDIO_H__
+#ifndef __GDM72XX_GDM_SDIO_H__
+#define __GDM72XX_GDM_SDIO_H__
 
 #include 
 #include 
@@ -60,4 +60,4 @@ struct sdiowm_dev {
struct work_struct  ws;
 };
 
-#endif /* __GDM_SDIO_H__ */
+#endif /* __GDM72XX_GDM_SDIO_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_usb.h 
b/drivers/staging/gdm72xx/gdm_usb.h
index 3050652..8e58a25 100644
--- a/drivers/staging/gdm72xx/gdm_usb.h
+++ b/drivers/staging/gdm72xx/gdm_usb.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_USB_H__
-#define __GDM_USB_H__
+#ifndef __GDM72XX_GDM_USB_H__
+#define __GDM72XX_GDM_USB_H__
 
 #include 
 #include 
@@ -75,4 +75,4 @@ struct usbwm_dev {
int padding;
 };
 
-#endif /* __GDM_USB_H__ */
+#endif /* __GDM72XX_GDM_USB_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_wimax.h 
b/drivers/staging/gdm72xx/gdm_wimax.h
index 7e2c888..c640d2c 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.h
+++ b/drivers/staging/gdm72xx/gdm_wimax.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_WIMAX_H__
-#define __GDM_WIMAX_H__
+#ifndef __GDM72XX_GDM_WIMAX_H__
+#define __GDM72XX_GDM_WIMAX_H__
 
 #include 
 #include 
@@ -57,4 +57,4 @@ int register_wimax_device(struct phy_dev *phy_dev, struct 
device *pdev);
 int gdm_wimax_send_tx(struct sk_buff *skb, struct net_device *dev);
 void unregister_wimax_device(struct phy_dev *phy_dev);
 
-#endif
+#endif /* __GDM72XX_GDM_WIMAX_H__ */
diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
index 059ba00..ff6941e 100644
--- a/drivers/staging/gdm72xx/hci.h
+++ b/drivers/staging/gdm72xx/hci.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef HCI_H_20080801
-#define HCI_H_20080801
+#ifndef __GDM72XX_HCI_H__
+#define __GDM72XX_HCI_H__
 
 #define HCI_HEADER_SIZE4
 #define HCI_VALUE_OFFS (HCI_HEADER_SIZE)
@@ -204,4 +204,4 @@ struct hci_s {
unsigned char  data[0];
 } __packed;
 
-#endif
+#endif /* __GDM72XX_HCI_H__ */
diff --git a/drivers/staging/gdm72xx/netlink_k.h 
b/drivers/staging/gdm72xx/netlink_k.h
index b6caac1..1fe7198 100644
--- a/drivers/staging/gdm72xx/netlink_k.h
+++ b/drivers/staging/gdm72xx/netlink_k.h
@@ -11,8 +11,9 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(NETLINK_H_20081202)
-#define NETLINK_H_20081202
+#ifndef __GDM72XX_NETLINK_K_H__
+#define __GDM72XX_NETLINK_K_H__
+
 #include 
 #include 
 
@@ -21,4 +22,4 @@ struct sock *netlink_init(int unit, void (*cb)(struct 
net_device *dev, u16 type,
 void netlink_exit(struct sock *sock);
 int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len);
 
-#endif
+#endif /* __GDM72XX_NETLINK_K_H__ */
diff --git a/drivers/staging/gdm72xx/sdio_boot.h 
b/drivers/staging/gdm72xx/sdio_boot.h
index 045c1f4..e0800c6 100644
--- a/drivers/staging/gdm72xx/sdio_boot.h
+++ b/drivers/staging/gdm72xx/sdio_boot.h
@@ -11,11 +11,11 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __SDIO_BOOT_H__
-#define __SDIO_BOOT_H__
+#ifndef __GDM72XX_SDIO_BOOT_H__
+#define __GDM72XX_SDIO_BOOT_H__
 
 struct sdio_func;
 
 int sdio_boot(struct sdio_func *func);
 
-#endif /* __SDIO_BOOT_H__ */
+#endif /* __GDM72XX_SDIO_BOOT_H__ */
diff --git a/drivers/staging/gdm72xx/usb_boot.h 
b/drivers/staging/gdm72xx/usb_boot.h
index 05308e2..5bf7190 100644
--- a/drivers/staging/gdm72xx/usb_boot.h
+++ b/drivers/staging/gdm72xx/usb_boot.h
@@ -11,12 +11,12 @@
  * 

[PATCH] staging: gdm72xx: use consistent style for header guards

[Ben Chan]

---
 drivers/staging/gdm72xx/gdm_qos.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_sdio.h  | 6 +++---
 drivers/staging/gdm72xx/gdm_usb.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_wimax.h | 6 +++---
 drivers/staging/gdm72xx/hci.h   | 6 +++---
 drivers/staging/gdm72xx/netlink_k.h | 7 ---
 drivers/staging/gdm72xx/sdio_boot.h | 6 +++---
 drivers/staging/gdm72xx/usb_boot.h  | 6 +++---
 drivers/staging/gdm72xx/usb_ids.h   | 6 +++---
 drivers/staging/gdm72xx/wm_ioctl.h  | 7 ---
 10 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 8f742f3..bbc8aab 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(GDM_QOS_H_20090403)
-#define GDM_QOS_H_20090403
+#ifndef __GDM72XX_GDM_QOS_H__
+#define __GDM72XX_GDM_QOS_H__
 
 #include linux/types.h
 #include linux/usb.h
@@ -71,4 +71,4 @@ void gdm_qos_release_list(void *nic_ptr);
 int gdm_qos_send_hci_pkt(struct sk_buff *skb, struct net_device *dev);
 void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int size);
 
-#endif
+#endif /* __GDM72XX_GDM_QOS_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_sdio.h 
b/drivers/staging/gdm72xx/gdm_sdio.h
index 0c0e2cb..77ad9d6 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.h
+++ b/drivers/staging/gdm72xx/gdm_sdio.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_SDIO_H__
-#define __GDM_SDIO_H__
+#ifndef __GDM72XX_GDM_SDIO_H__
+#define __GDM72XX_GDM_SDIO_H__
 
 #include linux/types.h
 #include linux/time.h
@@ -60,4 +60,4 @@ struct sdiowm_dev {
struct work_struct  ws;
 };
 
-#endif /* __GDM_SDIO_H__ */
+#endif /* __GDM72XX_GDM_SDIO_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_usb.h 
b/drivers/staging/gdm72xx/gdm_usb.h
index 3050652..8e58a25 100644
--- a/drivers/staging/gdm72xx/gdm_usb.h
+++ b/drivers/staging/gdm72xx/gdm_usb.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_USB_H__
-#define __GDM_USB_H__
+#ifndef __GDM72XX_GDM_USB_H__
+#define __GDM72XX_GDM_USB_H__
 
 #include linux/types.h
 #include linux/usb.h
@@ -75,4 +75,4 @@ struct usbwm_dev {
int padding;
 };
 
-#endif /* __GDM_USB_H__ */
+#endif /* __GDM72XX_GDM_USB_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_wimax.h 
b/drivers/staging/gdm72xx/gdm_wimax.h
index 7e2c888..c640d2c 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.h
+++ b/drivers/staging/gdm72xx/gdm_wimax.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_WIMAX_H__
-#define __GDM_WIMAX_H__
+#ifndef __GDM72XX_GDM_WIMAX_H__
+#define __GDM72XX_GDM_WIMAX_H__
 
 #include linux/netdevice.h
 #include linux/types.h
@@ -57,4 +57,4 @@ int register_wimax_device(struct phy_dev *phy_dev, struct 
device *pdev);
 int gdm_wimax_send_tx(struct sk_buff *skb, struct net_device *dev);
 void unregister_wimax_device(struct phy_dev *phy_dev);
 
-#endif
+#endif /* __GDM72XX_GDM_WIMAX_H__ */
diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
index 059ba00..ff6941e 100644
--- a/drivers/staging/gdm72xx/hci.h
+++ b/drivers/staging/gdm72xx/hci.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef HCI_H_20080801
-#define HCI_H_20080801
+#ifndef __GDM72XX_HCI_H__
+#define __GDM72XX_HCI_H__
 
 #define HCI_HEADER_SIZE4
 #define HCI_VALUE_OFFS (HCI_HEADER_SIZE)
@@ -204,4 +204,4 @@ struct hci_s {
unsigned char  data[0];
 } __packed;
 
-#endif
+#endif /* __GDM72XX_HCI_H__ */
diff --git a/drivers/staging/gdm72xx/netlink_k.h 
b/drivers/staging/gdm72xx/netlink_k.h
index b6caac1..1fe7198 100644
--- a/drivers/staging/gdm72xx/netlink_k.h
+++ b/drivers/staging/gdm72xx/netlink_k.h
@@ -11,8 +11,9 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(NETLINK_H_20081202)
-#define NETLINK_H_20081202
+#ifndef __GDM72XX_NETLINK_K_H__
+#define __GDM72XX_NETLINK_K_H__
+
 #include linux/netdevice.h
 #include net/sock.h
 
@@ -21,4 +22,4 @@ struct sock *netlink_init(int unit, void (*cb)(struct 
net_device *dev, u16 type,
 void netlink_exit(struct sock *sock);
 int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len);
 
-#endif
+#endif /* __GDM72XX_NETLINK_K_H__ */
diff --git a/drivers/staging/gdm72xx/sdio_boot.h 
b/drivers/staging/gdm72xx/sdio_boot.h
index 045c1f4..e0800c6 100644
--- a/drivers/staging/gdm72xx/sdio_boot.h
+++ b/drivers/staging/gdm72xx/sdio_boot.h
@@ -11,11 +11,11 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __SDIO_BOOT_H__
-#define __SDIO_BOOT_H__
+#ifndef __GDM72XX_SDIO_BOOT_H__
+#define __GDM72XX_SDIO_BOOT_H__
 
 struct sdio_func;
 
 int sdio_boot(struct sdio_func *func);
 
-#endif /* __SDIO_BOOT_H__ */
+#endif /* __GDM72XX_SDIO_BOOT_H__ */
diff --git a/drivers/staging/gdm72xx/usb_boot.h 
b/drivers/staging/gdm72xx/usb_boot.h
index 

[PATCH v2] staging: gdm72xx: use consistent style for header guards

[Ben Chan]

Signed-off-by: Ben Chan benc...@chromium.org
---
I forgot to add the Signed-off-by stanza in the original patch.

 drivers/staging/gdm72xx/gdm_qos.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_sdio.h  | 6 +++---
 drivers/staging/gdm72xx/gdm_usb.h   | 6 +++---
 drivers/staging/gdm72xx/gdm_wimax.h | 6 +++---
 drivers/staging/gdm72xx/hci.h   | 6 +++---
 drivers/staging/gdm72xx/netlink_k.h | 7 ---
 drivers/staging/gdm72xx/sdio_boot.h | 6 +++---
 drivers/staging/gdm72xx/usb_boot.h  | 6 +++---
 drivers/staging/gdm72xx/usb_ids.h   | 6 +++---
 drivers/staging/gdm72xx/wm_ioctl.h  | 7 ---
 10 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 8f742f3..bbc8aab 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(GDM_QOS_H_20090403)
-#define GDM_QOS_H_20090403
+#ifndef __GDM72XX_GDM_QOS_H__
+#define __GDM72XX_GDM_QOS_H__
 
 #include linux/types.h
 #include linux/usb.h
@@ -71,4 +71,4 @@ void gdm_qos_release_list(void *nic_ptr);
 int gdm_qos_send_hci_pkt(struct sk_buff *skb, struct net_device *dev);
 void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int size);
 
-#endif
+#endif /* __GDM72XX_GDM_QOS_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_sdio.h 
b/drivers/staging/gdm72xx/gdm_sdio.h
index 0c0e2cb..77ad9d6 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.h
+++ b/drivers/staging/gdm72xx/gdm_sdio.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_SDIO_H__
-#define __GDM_SDIO_H__
+#ifndef __GDM72XX_GDM_SDIO_H__
+#define __GDM72XX_GDM_SDIO_H__
 
 #include linux/types.h
 #include linux/time.h
@@ -60,4 +60,4 @@ struct sdiowm_dev {
struct work_struct  ws;
 };
 
-#endif /* __GDM_SDIO_H__ */
+#endif /* __GDM72XX_GDM_SDIO_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_usb.h 
b/drivers/staging/gdm72xx/gdm_usb.h
index 3050652..8e58a25 100644
--- a/drivers/staging/gdm72xx/gdm_usb.h
+++ b/drivers/staging/gdm72xx/gdm_usb.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_USB_H__
-#define __GDM_USB_H__
+#ifndef __GDM72XX_GDM_USB_H__
+#define __GDM72XX_GDM_USB_H__
 
 #include linux/types.h
 #include linux/usb.h
@@ -75,4 +75,4 @@ struct usbwm_dev {
int padding;
 };
 
-#endif /* __GDM_USB_H__ */
+#endif /* __GDM72XX_GDM_USB_H__ */
diff --git a/drivers/staging/gdm72xx/gdm_wimax.h 
b/drivers/staging/gdm72xx/gdm_wimax.h
index 7e2c888..c640d2c 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.h
+++ b/drivers/staging/gdm72xx/gdm_wimax.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __GDM_WIMAX_H__
-#define __GDM_WIMAX_H__
+#ifndef __GDM72XX_GDM_WIMAX_H__
+#define __GDM72XX_GDM_WIMAX_H__
 
 #include linux/netdevice.h
 #include linux/types.h
@@ -57,4 +57,4 @@ int register_wimax_device(struct phy_dev *phy_dev, struct 
device *pdev);
 int gdm_wimax_send_tx(struct sk_buff *skb, struct net_device *dev);
 void unregister_wimax_device(struct phy_dev *phy_dev);
 
-#endif
+#endif /* __GDM72XX_GDM_WIMAX_H__ */
diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
index 059ba00..ff6941e 100644
--- a/drivers/staging/gdm72xx/hci.h
+++ b/drivers/staging/gdm72xx/hci.h
@@ -11,8 +11,8 @@
  * GNU General Public License for more details.
  */
 
-#ifndef HCI_H_20080801
-#define HCI_H_20080801
+#ifndef __GDM72XX_HCI_H__
+#define __GDM72XX_HCI_H__
 
 #define HCI_HEADER_SIZE4
 #define HCI_VALUE_OFFS (HCI_HEADER_SIZE)
@@ -204,4 +204,4 @@ struct hci_s {
unsigned char  data[0];
 } __packed;
 
-#endif
+#endif /* __GDM72XX_HCI_H__ */
diff --git a/drivers/staging/gdm72xx/netlink_k.h 
b/drivers/staging/gdm72xx/netlink_k.h
index b6caac1..1fe7198 100644
--- a/drivers/staging/gdm72xx/netlink_k.h
+++ b/drivers/staging/gdm72xx/netlink_k.h
@@ -11,8 +11,9 @@
  * GNU General Public License for more details.
  */
 
-#if !defined(NETLINK_H_20081202)
-#define NETLINK_H_20081202
+#ifndef __GDM72XX_NETLINK_K_H__
+#define __GDM72XX_NETLINK_K_H__
+
 #include linux/netdevice.h
 #include net/sock.h
 
@@ -21,4 +22,4 @@ struct sock *netlink_init(int unit, void (*cb)(struct 
net_device *dev, u16 type,
 void netlink_exit(struct sock *sock);
 int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len);
 
-#endif
+#endif /* __GDM72XX_NETLINK_K_H__ */
diff --git a/drivers/staging/gdm72xx/sdio_boot.h 
b/drivers/staging/gdm72xx/sdio_boot.h
index 045c1f4..e0800c6 100644
--- a/drivers/staging/gdm72xx/sdio_boot.h
+++ b/drivers/staging/gdm72xx/sdio_boot.h
@@ -11,11 +11,11 @@
  * GNU General Public License for more details.
  */
 
-#ifndef __SDIO_BOOT_H__
-#define __SDIO_BOOT_H__
+#ifndef __GDM72XX_SDIO_BOOT_H__
+#define __GDM72XX_SDIO_BOOT_H__
 
 struct sdio_func;
 
 int sdio_boot(struct sdio_func *func);
 
-#endif /* __SDIO_BOOT_H__ */
+#endif

[PATCH 4/4] staging: gdm72xx: use lower case for variable names for consistency

[Ben Chan]

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_qos.c | 38 +++---
 drivers/staging/gdm72xx/gdm_qos.h |  2 +-
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index a2efc5c..b08c8e1 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -190,15 +190,15 @@ static int chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 
*stream, u8 *port)
 
 static int get_qos_index(struct nic *nic, u8 *iph, u8 *tcpudph)
 {
-   int IP_ver, i;
+   int ip_ver, i;
struct qos_cb_s *qcb = >qos;
 
if (iph == NULL || tcpudph == NULL)
return -1;
 
-   IP_ver = (iph[0]>>4)&0xf;
+   ip_ver = (iph[0]>>4)&0xf;
 
-   if (IP_ver != 4)
+   if (ip_ver != 4)
return -1;
 
for (i = 0; i < QOS_MAX; i++) {
@@ -303,12 +303,12 @@ out:
return ret;
 }
 
-static int get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
+static int get_csr(struct qos_cb_s *qcb, u32 sfid, int mode)
 {
int i;
 
for (i = 0; i < qcb->qos_list_cnt; i++) {
-   if (qcb->csr[i].SFID == SFID)
+   if (qcb->csr[i].sfid == sfid)
return i;
}
 
@@ -332,7 +332,7 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
 {
struct nic *nic = nic_ptr;
int i, index, pos;
-   u32 SFID;
+   u32 sfid;
u8 sub_cmd_evt;
struct qos_cb_s *qcb = >qos;
struct qos_entry_s *entry, *n;
@@ -345,11 +345,11 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
if (sub_cmd_evt == QOS_REPORT) {
spin_lock_irqsave(>qos_lock, flags);
for (i = 0; i < qcb->qos_list_cnt; i++) {
-   SFID = ((buf[(i*5)+6]<<24)&0xff00);
-   SFID += ((buf[(i*5)+7]<<16)&0xff);
-   SFID += ((buf[(i*5)+8]<<8)&0xff00);
-   SFID += (buf[(i*5)+9]);
-   index = get_csr(qcb, SFID, 0);
+   sfid = ((buf[(i*5)+6]<<24)&0xff00);
+   sfid += ((buf[(i*5)+7]<<16)&0xff);
+   sfid += ((buf[(i*5)+8]<<8)&0xff00);
+   sfid += (buf[(i*5)+9]);
+   index = get_csr(qcb, sfid, 0);
if (index == -1) {
spin_unlock_irqrestore(>qos_lock, flags);
netdev_err(nic->netdev, "QoS ERROR: No SF\n");
@@ -366,12 +366,12 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
 
/* sub_cmd_evt == QOS_ADD || sub_cmd_evt == QOS_CHANG_DEL */
pos = 6;
-   SFID = ((buf[pos++]<<24)&0xff00);
-   SFID += ((buf[pos++]<<16)&0xff);
-   SFID += ((buf[pos++]<<8)&0xff00);
-   SFID += (buf[pos++]);
+   sfid = ((buf[pos++]<<24)&0xff00);
+   sfid += ((buf[pos++]<<16)&0xff);
+   sfid += ((buf[pos++]<<8)&0xff00);
+   sfid += (buf[pos++]);
 
-   index = get_csr(qcb, SFID, 1);
+   index = get_csr(qcb, sfid, 1);
if (index == -1) {
netdev_err(nic->netdev,
   "QoS ERROR: csr Update Error / Wrong index (%d)\n",
@@ -381,10 +381,10 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
 
if (sub_cmd_evt == QOS_ADD) {
netdev_dbg(nic->netdev, "QOS_ADD SFID = 0x%x, index=%d\n",
-  SFID, index);
+  sfid, index);
 
spin_lock_irqsave(>qos_lock, flags);
-   qcb->csr[index].SFID = SFID;
+   qcb->csr[index].sfid = sfid;
qcb->csr[index].classifier_rule_en = ((buf[pos++]<<8)&0xff00);
qcb->csr[index].classifier_rule_en += buf[pos++];
if (qcb->csr[index].classifier_rule_en == 0)
@@ -422,7 +422,7 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
spin_unlock_irqrestore(>qos_lock, flags);
} else if (sub_cmd_evt == QOS_CHANGE_DEL) {
netdev_dbg(nic->netdev, "QOS_CHANGE_DEL SFID = 0x%x, 
index=%d\n",
-  SFID, index);
+  sfid, index);
 
INIT_LIST_HEAD(_list);
 
diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index ab03d33..8f742f3 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -33,7 +33,7 @@
 
 struct gdm_wimax_csr_s {
boolenabled;
-   u32 SFID;
+   u32 sfid;
u8  qos_buf_count;
  

[PATCH 3/4] staging: gdm72xx: use int instead of u32 whenever makes sense

[Ben Chan]

This patch addresses the following issues:
- Use int instead of u32 whenever makes sense
- Turn extract_qos_list() in gdm_qos.c, which previously always returned
  0, into a void function.

Reported-by: Dan Carpenter 
Reported-by: Michalis Pappas 
Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_qos.c | 15 +++
 drivers/staging/gdm72xx/gdm_qos.h |  6 +++---
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index 732f009..a2efc5c 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -142,7 +142,7 @@ void gdm_qos_release_list(void *nic_ptr)
free_qos_entry_list(_list);
 }
 
-static u32 chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 *stream, u8 *port)
+static int chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 *stream, u8 *port)
 {
int i;
 
@@ -188,9 +188,9 @@ static u32 chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 
*stream, u8 *port)
return 0;
 }
 
-static u32 get_qos_index(struct nic *nic, u8 *iph, u8 *tcpudph)
+static int get_qos_index(struct nic *nic, u8 *iph, u8 *tcpudph)
 {
-   u32 IP_ver, i;
+   int IP_ver, i;
struct qos_cb_s *qcb = >qos;
 
if (iph == NULL || tcpudph == NULL)
@@ -213,7 +213,7 @@ static u32 get_qos_index(struct nic *nic, u8 *iph, u8 
*tcpudph)
return -1;
 }
 
-static u32 extract_qos_list(struct nic *nic, struct list_head *head)
+static void extract_qos_list(struct nic *nic, struct list_head *head)
 {
struct qos_cb_s *qcb = >qos;
struct qos_entry_s *entry;
@@ -238,8 +238,6 @@ static u32 extract_qos_list(struct nic *nic, struct 
list_head *head)
if (!list_empty(>qos_list[i]))
netdev_warn(nic->netdev, "Index(%d) is piled!!\n", i);
}
-
-   return 0;
 }
 
 static void send_qos_list(struct nic *nic, struct list_head *head)
@@ -305,7 +303,7 @@ out:
return ret;
 }
 
-static u32 get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
+static int get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
 {
int i;
 
@@ -333,7 +331,8 @@ static u32 get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
 void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int size)
 {
struct nic *nic = nic_ptr;
-   u32 i, SFID, index, pos;
+   int i, index, pos;
+   u32 SFID;
u8 sub_cmd_evt;
struct qos_cb_s *qcb = >qos;
struct qos_entry_s *entry, *n;
diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 50aa191..ab03d33 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -59,11 +59,11 @@ struct qos_entry_s {
 
 struct qos_cb_s {
struct list_headqos_list[QOS_MAX];
-   u32 qos_list_cnt;
-   u32 qos_null_idx;
+   int qos_list_cnt;
+   int qos_null_idx;
struct gdm_wimax_csr_s  csr[QOS_MAX];
spinlock_t  qos_lock;
-   u32 qos_limit_size;
+   int qos_limit_size;
 };
 
 void gdm_qos_init(void *nic_ptr);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] staging: gdm72xx: return -EINVAL instead of BUG_ON for invalid data length

[Ben Chan]

This patch changes gdm_usb_send() and gdm_sdio_send() to return -EINVAL instead
of calling BUG_ON if an invalid data length is passed to the functions.

Reported-by: Dan Carpenter 
Reported-by: Michalis Pappas 
Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_sdio.c | 3 ++-
 drivers/staging/gdm72xx/gdm_usb.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 0c6a3eb..9d2de6f 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -390,7 +390,8 @@ static int gdm_sdio_send(void *priv_dev, void *data, int 
len,
u16 cmd_evt;
unsigned long flags;
 
-   BUG_ON(len > TX_BUF_SIZE - TYPE_A_HEADER_SIZE);
+   if (len > TX_BUF_SIZE - TYPE_A_HEADER_SIZE)
+   return -EINVAL;
 
spin_lock_irqsave(>lock, flags);
 
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index cd8e6e4..971976c 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -312,7 +312,8 @@ static int gdm_usb_send(void *priv_dev, void *data, int len,
return -ENODEV;
}
 
-   BUG_ON(len > TX_BUF_SIZE - padding - 1);
+   if (len > TX_BUF_SIZE - padding - 1)
+   return -EINVAL;
 
spin_lock_irqsave(>lock, flags);
 
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] staging: gdm72xx: use bool instead of custom-defined BOOLEAN

[Ben Chan]

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_qos.c | 10 +-
 drivers/staging/gdm72xx/gdm_qos.h |  4 +---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index df6f000..732f009 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -100,7 +100,7 @@ void gdm_qos_init(void *nic_ptr)
for (i = 0; i < QOS_MAX; i++) {
INIT_LIST_HEAD(>qos_list[i]);
qcb->csr[i].qos_buf_count = 0;
-   qcb->csr[i].enabled = 0;
+   qcb->csr[i].enabled = false;
}
 
qcb->qos_list_cnt = 0;
@@ -127,7 +127,7 @@ void gdm_qos_release_list(void *nic_ptr)
 
for (i = 0; i < QOS_MAX; i++) {
qcb->csr[i].qos_buf_count = 0;
-   qcb->csr[i].enabled = 0;
+   qcb->csr[i].enabled = false;
}
 
qcb->qos_list_cnt = 0;
@@ -316,8 +316,8 @@ static u32 get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
 
if (mode) {
for (i = 0; i < QOS_MAX; i++) {
-   if (qcb->csr[i].enabled == 0) {
-   qcb->csr[i].enabled = 1;
+   if (!qcb->csr[i].enabled) {
+   qcb->csr[i].enabled = true;
qcb->qos_list_cnt++;
return i;
}
@@ -428,7 +428,7 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
INIT_LIST_HEAD(_list);
 
spin_lock_irqsave(>qos_lock, flags);
-   qcb->csr[index].enabled = 0;
+   qcb->csr[index].enabled = false;
qcb->qos_list_cnt--;
qcb->qos_limit_size = 254/qcb->qos_list_cnt;
 
diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 6543cff..50aa191 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -18,8 +18,6 @@
 #include 
 #include 
 
-#define BOOLEANu8
-
 #define QOS_MAX16
 #define IPTYPEOFSERVICE0x8000
 #definePROTOCOL0x4000
@@ -34,7 +32,7 @@
 #defineIEEE802_1QVLANID0x10
 
 struct gdm_wimax_csr_s {
-   BOOLEAN enabled;
+   boolenabled;
u32 SFID;
u8  qos_buf_count;
u16 classifier_rule_en;
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] staging: gdm72xx: use int instead of u32 whenever makes sense

[Ben Chan]

This patch addresses the following issues:
- Use int instead of u32 whenever makes sense
- Turn extract_qos_list() in gdm_qos.c, which previously always returned
  0, into a void function.

Reported-by: Dan Carpenter dan.carpen...@oracle.com
Reported-by: Michalis Pappas mpap...@fastmail.fm
Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_qos.c | 15 +++
 drivers/staging/gdm72xx/gdm_qos.h |  6 +++---
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index 732f009..a2efc5c 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -142,7 +142,7 @@ void gdm_qos_release_list(void *nic_ptr)
free_qos_entry_list(free_list);
 }
 
-static u32 chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 *stream, u8 *port)
+static int chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 *stream, u8 *port)
 {
int i;
 
@@ -188,9 +188,9 @@ static u32 chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 
*stream, u8 *port)
return 0;
 }
 
-static u32 get_qos_index(struct nic *nic, u8 *iph, u8 *tcpudph)
+static int get_qos_index(struct nic *nic, u8 *iph, u8 *tcpudph)
 {
-   u32 IP_ver, i;
+   int IP_ver, i;
struct qos_cb_s *qcb = nic-qos;
 
if (iph == NULL || tcpudph == NULL)
@@ -213,7 +213,7 @@ static u32 get_qos_index(struct nic *nic, u8 *iph, u8 
*tcpudph)
return -1;
 }
 
-static u32 extract_qos_list(struct nic *nic, struct list_head *head)
+static void extract_qos_list(struct nic *nic, struct list_head *head)
 {
struct qos_cb_s *qcb = nic-qos;
struct qos_entry_s *entry;
@@ -238,8 +238,6 @@ static u32 extract_qos_list(struct nic *nic, struct 
list_head *head)
if (!list_empty(qcb-qos_list[i]))
netdev_warn(nic-netdev, Index(%d) is piled!!\n, i);
}
-
-   return 0;
 }
 
 static void send_qos_list(struct nic *nic, struct list_head *head)
@@ -305,7 +303,7 @@ out:
return ret;
 }
 
-static u32 get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
+static int get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
 {
int i;
 
@@ -333,7 +331,8 @@ static u32 get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
 void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int size)
 {
struct nic *nic = nic_ptr;
-   u32 i, SFID, index, pos;
+   int i, index, pos;
+   u32 SFID;
u8 sub_cmd_evt;
struct qos_cb_s *qcb = nic-qos;
struct qos_entry_s *entry, *n;
diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 50aa191..ab03d33 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -59,11 +59,11 @@ struct qos_entry_s {
 
 struct qos_cb_s {
struct list_headqos_list[QOS_MAX];
-   u32 qos_list_cnt;
-   u32 qos_null_idx;
+   int qos_list_cnt;
+   int qos_null_idx;
struct gdm_wimax_csr_s  csr[QOS_MAX];
spinlock_t  qos_lock;
-   u32 qos_limit_size;
+   int qos_limit_size;
 };
 
 void gdm_qos_init(void *nic_ptr);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] staging: gdm72xx: return -EINVAL instead of BUG_ON for invalid data length

[Ben Chan]

This patch changes gdm_usb_send() and gdm_sdio_send() to return -EINVAL instead
of calling BUG_ON if an invalid data length is passed to the functions.

Reported-by: Dan Carpenter dan.carpen...@oracle.com
Reported-by: Michalis Pappas mpap...@fastmail.fm
Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_sdio.c | 3 ++-
 drivers/staging/gdm72xx/gdm_usb.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 0c6a3eb..9d2de6f 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -390,7 +390,8 @@ static int gdm_sdio_send(void *priv_dev, void *data, int 
len,
u16 cmd_evt;
unsigned long flags;
 
-   BUG_ON(len  TX_BUF_SIZE - TYPE_A_HEADER_SIZE);
+   if (len  TX_BUF_SIZE - TYPE_A_HEADER_SIZE)
+   return -EINVAL;
 
spin_lock_irqsave(tx-lock, flags);
 
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index cd8e6e4..971976c 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -312,7 +312,8 @@ static int gdm_usb_send(void *priv_dev, void *data, int len,
return -ENODEV;
}
 
-   BUG_ON(len  TX_BUF_SIZE - padding - 1);
+   if (len  TX_BUF_SIZE - padding - 1)
+   return -EINVAL;
 
spin_lock_irqsave(tx-lock, flags);
 
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] staging: gdm72xx: use bool instead of custom-defined BOOLEAN

[Ben Chan]

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_qos.c | 10 +-
 drivers/staging/gdm72xx/gdm_qos.h |  4 +---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index df6f000..732f009 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -100,7 +100,7 @@ void gdm_qos_init(void *nic_ptr)
for (i = 0; i  QOS_MAX; i++) {
INIT_LIST_HEAD(qcb-qos_list[i]);
qcb-csr[i].qos_buf_count = 0;
-   qcb-csr[i].enabled = 0;
+   qcb-csr[i].enabled = false;
}
 
qcb-qos_list_cnt = 0;
@@ -127,7 +127,7 @@ void gdm_qos_release_list(void *nic_ptr)
 
for (i = 0; i  QOS_MAX; i++) {
qcb-csr[i].qos_buf_count = 0;
-   qcb-csr[i].enabled = 0;
+   qcb-csr[i].enabled = false;
}
 
qcb-qos_list_cnt = 0;
@@ -316,8 +316,8 @@ static u32 get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
 
if (mode) {
for (i = 0; i  QOS_MAX; i++) {
-   if (qcb-csr[i].enabled == 0) {
-   qcb-csr[i].enabled = 1;
+   if (!qcb-csr[i].enabled) {
+   qcb-csr[i].enabled = true;
qcb-qos_list_cnt++;
return i;
}
@@ -428,7 +428,7 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
INIT_LIST_HEAD(free_list);
 
spin_lock_irqsave(qcb-qos_lock, flags);
-   qcb-csr[index].enabled = 0;
+   qcb-csr[index].enabled = false;
qcb-qos_list_cnt--;
qcb-qos_limit_size = 254/qcb-qos_list_cnt;
 
diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index 6543cff..50aa191 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -18,8 +18,6 @@
 #include linux/usb.h
 #include linux/list.h
 
-#define BOOLEANu8
-
 #define QOS_MAX16
 #define IPTYPEOFSERVICE0x8000
 #definePROTOCOL0x4000
@@ -34,7 +32,7 @@
 #defineIEEE802_1QVLANID0x10
 
 struct gdm_wimax_csr_s {
-   BOOLEAN enabled;
+   boolenabled;
u32 SFID;
u8  qos_buf_count;
u16 classifier_rule_en;
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] staging: gdm72xx: use lower case for variable names for consistency

[Ben Chan]

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_qos.c | 38 +++---
 drivers/staging/gdm72xx/gdm_qos.h |  2 +-
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_qos.c 
b/drivers/staging/gdm72xx/gdm_qos.c
index a2efc5c..b08c8e1 100644
--- a/drivers/staging/gdm72xx/gdm_qos.c
+++ b/drivers/staging/gdm72xx/gdm_qos.c
@@ -190,15 +190,15 @@ static int chk_ipv4_rule(struct gdm_wimax_csr_s *csr, u8 
*stream, u8 *port)
 
 static int get_qos_index(struct nic *nic, u8 *iph, u8 *tcpudph)
 {
-   int IP_ver, i;
+   int ip_ver, i;
struct qos_cb_s *qcb = nic-qos;
 
if (iph == NULL || tcpudph == NULL)
return -1;
 
-   IP_ver = (iph[0]4)0xf;
+   ip_ver = (iph[0]4)0xf;
 
-   if (IP_ver != 4)
+   if (ip_ver != 4)
return -1;
 
for (i = 0; i  QOS_MAX; i++) {
@@ -303,12 +303,12 @@ out:
return ret;
 }
 
-static int get_csr(struct qos_cb_s *qcb, u32 SFID, int mode)
+static int get_csr(struct qos_cb_s *qcb, u32 sfid, int mode)
 {
int i;
 
for (i = 0; i  qcb-qos_list_cnt; i++) {
-   if (qcb-csr[i].SFID == SFID)
+   if (qcb-csr[i].sfid == sfid)
return i;
}
 
@@ -332,7 +332,7 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
 {
struct nic *nic = nic_ptr;
int i, index, pos;
-   u32 SFID;
+   u32 sfid;
u8 sub_cmd_evt;
struct qos_cb_s *qcb = nic-qos;
struct qos_entry_s *entry, *n;
@@ -345,11 +345,11 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
if (sub_cmd_evt == QOS_REPORT) {
spin_lock_irqsave(qcb-qos_lock, flags);
for (i = 0; i  qcb-qos_list_cnt; i++) {
-   SFID = ((buf[(i*5)+6]24)0xff00);
-   SFID += ((buf[(i*5)+7]16)0xff);
-   SFID += ((buf[(i*5)+8]8)0xff00);
-   SFID += (buf[(i*5)+9]);
-   index = get_csr(qcb, SFID, 0);
+   sfid = ((buf[(i*5)+6]24)0xff00);
+   sfid += ((buf[(i*5)+7]16)0xff);
+   sfid += ((buf[(i*5)+8]8)0xff00);
+   sfid += (buf[(i*5)+9]);
+   index = get_csr(qcb, sfid, 0);
if (index == -1) {
spin_unlock_irqrestore(qcb-qos_lock, flags);
netdev_err(nic-netdev, QoS ERROR: No SF\n);
@@ -366,12 +366,12 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
 
/* sub_cmd_evt == QOS_ADD || sub_cmd_evt == QOS_CHANG_DEL */
pos = 6;
-   SFID = ((buf[pos++]24)0xff00);
-   SFID += ((buf[pos++]16)0xff);
-   SFID += ((buf[pos++]8)0xff00);
-   SFID += (buf[pos++]);
+   sfid = ((buf[pos++]24)0xff00);
+   sfid += ((buf[pos++]16)0xff);
+   sfid += ((buf[pos++]8)0xff00);
+   sfid += (buf[pos++]);
 
-   index = get_csr(qcb, SFID, 1);
+   index = get_csr(qcb, sfid, 1);
if (index == -1) {
netdev_err(nic-netdev,
   QoS ERROR: csr Update Error / Wrong index (%d)\n,
@@ -381,10 +381,10 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
 
if (sub_cmd_evt == QOS_ADD) {
netdev_dbg(nic-netdev, QOS_ADD SFID = 0x%x, index=%d\n,
-  SFID, index);
+  sfid, index);
 
spin_lock_irqsave(qcb-qos_lock, flags);
-   qcb-csr[index].SFID = SFID;
+   qcb-csr[index].sfid = sfid;
qcb-csr[index].classifier_rule_en = ((buf[pos++]8)0xff00);
qcb-csr[index].classifier_rule_en += buf[pos++];
if (qcb-csr[index].classifier_rule_en == 0)
@@ -422,7 +422,7 @@ void gdm_recv_qos_hci_packet(void *nic_ptr, u8 *buf, int 
size)
spin_unlock_irqrestore(qcb-qos_lock, flags);
} else if (sub_cmd_evt == QOS_CHANGE_DEL) {
netdev_dbg(nic-netdev, QOS_CHANGE_DEL SFID = 0x%x, 
index=%d\n,
-  SFID, index);
+  sfid, index);
 
INIT_LIST_HEAD(free_list);
 
diff --git a/drivers/staging/gdm72xx/gdm_qos.h 
b/drivers/staging/gdm72xx/gdm_qos.h
index ab03d33..8f742f3 100644
--- a/drivers/staging/gdm72xx/gdm_qos.h
+++ b/drivers/staging/gdm72xx/gdm_qos.h
@@ -33,7 +33,7 @@
 
 struct gdm_wimax_csr_s {
boolenabled;
-   u32 SFID;
+   u32 sfid;
u8  qos_buf_count;
u16 classifier_rule_en;
u8  ip2s_lo;
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] staging: gdm72xx: check return value of sscanf

[Ben Chan]

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_wimax.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index e5e5115..3081fd4 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -291,8 +291,9 @@ static void __gdm_wimax_event_send(struct work_struct *work)
e = list_entry(wm_event.evtq.next, struct evt_entry, list);
spin_unlock_irqrestore(_event.evt_lock, flags);
 
-   sscanf(e->dev->name, "wm%d", );
-   netlink_send(wm_event.sock, idx, 0, e->evt_data, e->size);
+   if (sscanf(e->dev->name, "wm%d", ) == 1)
+   netlink_send(wm_event.sock, idx, 0, e->evt_data,
+e->size);
 
spin_lock_irqsave(_event.evt_lock, flags);
list_del(>list);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] MIPS: ZBOOT: implement stack protector in compressed boot phase

[Ben Chan]

This patch implements the stack protector code in MIPS compressed boot
phase based on the same code added to arm in commit
8779657d29c0ebcc0c94ede4df2f497baf1b563f "stackprotector: Introduce
CONFIG_CC_STACKPROTECTOR_STRONG" by Kees Cook 

Signed-off-by: Ben Chan 
Cc: Kees Cook 
Cc: Olof Johansson 
---
 arch/mips/boot/compressed/decompress.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/mips/boot/compressed/decompress.c 
b/arch/mips/boot/compressed/decompress.c
index c00c4dd..b49c7ad 100644
--- a/arch/mips/boot/compressed/decompress.c
+++ b/arch/mips/boot/compressed/decompress.c
@@ -67,10 +67,24 @@ void error(char *x)
 #include "../../../../lib/decompress_unxz.c"
 #endif
 
+unsigned long __stack_chk_guard;
+
+void __stack_chk_guard_setup(void)
+{
+   __stack_chk_guard = 0x000a0dff;
+}
+
+void __stack_chk_fail(void)
+{
+   error("stack-protector: Kernel stack is corrupted\n");
+}
+
 void decompress_kernel(unsigned long boot_heap_start)
 {
unsigned long zimage_start, zimage_size;
 
+   __stack_chk_guard_setup();
+
zimage_start = (unsigned long)(&__image_begin);
zimage_size = (unsigned long)(&__image_end) -
(unsigned long)(&__image_begin);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH staging-next] staging: gdm72xx: fix block comment style

[Ben Chan]

This patch fixes the following checkpatch warnings, which are issued
when the gdm72xx driver is moved out of staging into drivers/net/wimax:

  WARNING: networking block comments don't use an empty /* line, use /* 
Comment...
  WARNING: networking block comments start with * on subsequent lines
  WARNING: networking block comments put the trailing */ on a separate line

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_sdio.c  | 7 ++-
 drivers/staging/gdm72xx/gdm_usb.c   | 6 ++
 drivers/staging/gdm72xx/gdm_wimax.c | 5 +++--
 drivers/staging/gdm72xx/hci.h   | 4 +---
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 7398d45..0c6a3eb 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -439,9 +439,7 @@ static int gdm_sdio_send(void *priv_dev, void *data, int 
len,
return 0;
 }
 
-/*
- * Handle the HCI, WIMAX_SDU_TX_FLOW.
- */
+/* Handle the HCI, WIMAX_SDU_TX_FLOW. */
 static int control_sdu_tx_flow(struct sdiowm_dev *sdev, u8 *hci_data, int len)
 {
struct tx_cxt *tx = >tx;
@@ -462,8 +460,7 @@ static int control_sdu_tx_flow(struct sdiowm_dev *sdev, u8 
*hci_data, int len)
tx->stop_sdu_tx = 0;
if (tx->can_send)
schedule_work(>ws);
-   /*
-* If free buffer for sdu tx doesn't exist, then tx queue
+   /* If free buffer for sdu tx doesn't exist, then tx queue
 * should not be woken. For this reason, don't pass the command,
 * START_SDU_TX.
 */
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 78d6667..2325d41 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -338,8 +338,7 @@ static int gdm_usb_send(void *priv_dev, void *data, int len,
t->callback = cb;
t->cb_data = cb_data;
 
-   /*
-* In some cases, USB Module of WiMax is blocked when data size is
+   /* In some cases, USB Module of WiMax is blocked when data size is
 * the multiple of 512. So, increment length by one in that case.
 */
if ((len % 512) == 0)
@@ -439,8 +438,7 @@ static void gdm_usb_rcv_complete(struct urb *urb)
list_for_each_entry(t, >sdu_list, list) {
usb_submit_urb(t->urb, GFP_ATOMIC);
}
-   /*
-* If free buffer for sdu tx doesn't
+   /* If free buffer for sdu tx doesn't
 * exist, then tx queue should not be
 * woken. For this reason, don't pass
 * the command, START_SDU_TX.
diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index e5e5115..833d0d4 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -580,8 +580,9 @@ static int gdm_wimax_ioctl(struct net_device *dev, struct 
ifreq *ifr, int cmd)
return ret;
} else if (req->cmd == SIOCS_DATA) {
if (req->data_id == SIOC_DATA_FSM) {
-   /*NOTE: gdm_update_fsm should be called
-   before gdm_wimax_ioctl_set_data is called*/
+   /* NOTE: gdm_update_fsm should be called
+* before gdm_wimax_ioctl_set_data is called.
+*/
gdm_update_fsm(dev,
   (struct fsm_s *)req->data.buf);
}
diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
index 2485a37..059ba00 100644
--- a/drivers/staging/gdm72xx/hci.h
+++ b/drivers/staging/gdm72xx/hci.h
@@ -112,13 +112,11 @@
 #define W_SCAN_ALL_SUBSCRIPTION1
 #define W_SCAN_SPECIFIED_SUBSCRIPTION  2
 
-/*
- * TLV
+/* TLV
  *
  * [31:31] indicates the type is composite.
  * [30:16] is the length of the type. 0 length means length is variable.
  * [15:0] is the actual type.
- *
  */
 #define TLV_L(x)   (((x) >> 16) & 0xff)
 #define TLV_T(x)   ((x) & 0xff)
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH staging-next] staging: gdm72xx: remove blank lines after an open brace

[Ben Chan]

This patch fixes the following checkpatch warning:

  CHECK: Blank lines aren't necessary after an open brace '{'

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_usb.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 78d6667..256059c 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -632,7 +632,6 @@ static void gdm_usb_disconnect(struct usb_interface *intf)
if (idProduct != EMERGENCY_PID &&
bConfigurationValue != DOWNLOAD_CONF_VALUE &&
(idProduct & B_DOWNLOAD) == 0) {
-
udev = phy_dev->priv_dev;
udev->usbdev = NULL;
 
@@ -710,10 +709,8 @@ static int k_mode_thread(void *arg)
int ret;
 
while (!k_mode_stop) {
-
spin_lock_irqsave(_lock, flags2);
while (!list_empty(_list)) {
-
udev = list_entry(k_list.next, struct usbwm_dev, list);
tx = >tx;
rx = >rx;
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH staging-next] staging: gdm72xx: remove blank lines after an open brace

[Ben Chan]

This patch fixes the following checkpatch warning:

  CHECK: Blank lines aren't necessary after an open brace '{'

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_usb.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 78d6667..256059c 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -632,7 +632,6 @@ static void gdm_usb_disconnect(struct usb_interface *intf)
if (idProduct != EMERGENCY_PID 
bConfigurationValue != DOWNLOAD_CONF_VALUE 
(idProduct  B_DOWNLOAD) == 0) {
-
udev = phy_dev-priv_dev;
udev-usbdev = NULL;
 
@@ -710,10 +709,8 @@ static int k_mode_thread(void *arg)
int ret;
 
while (!k_mode_stop) {
-
spin_lock_irqsave(k_lock, flags2);
while (!list_empty(k_list)) {
-
udev = list_entry(k_list.next, struct usbwm_dev, list);
tx = udev-tx;
rx = udev-rx;
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH staging-next] staging: gdm72xx: fix block comment style

[Ben Chan]

This patch fixes the following checkpatch warnings, which are issued
when the gdm72xx driver is moved out of staging into drivers/net/wimax:

  WARNING: networking block comments don't use an empty /* line, use /* 
Comment...
  WARNING: networking block comments start with * on subsequent lines
  WARNING: networking block comments put the trailing */ on a separate line

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_sdio.c  | 7 ++-
 drivers/staging/gdm72xx/gdm_usb.c   | 6 ++
 drivers/staging/gdm72xx/gdm_wimax.c | 5 +++--
 drivers/staging/gdm72xx/hci.h   | 4 +---
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index 7398d45..0c6a3eb 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -439,9 +439,7 @@ static int gdm_sdio_send(void *priv_dev, void *data, int 
len,
return 0;
 }
 
-/*
- * Handle the HCI, WIMAX_SDU_TX_FLOW.
- */
+/* Handle the HCI, WIMAX_SDU_TX_FLOW. */
 static int control_sdu_tx_flow(struct sdiowm_dev *sdev, u8 *hci_data, int len)
 {
struct tx_cxt *tx = sdev-tx;
@@ -462,8 +460,7 @@ static int control_sdu_tx_flow(struct sdiowm_dev *sdev, u8 
*hci_data, int len)
tx-stop_sdu_tx = 0;
if (tx-can_send)
schedule_work(sdev-ws);
-   /*
-* If free buffer for sdu tx doesn't exist, then tx queue
+   /* If free buffer for sdu tx doesn't exist, then tx queue
 * should not be woken. For this reason, don't pass the command,
 * START_SDU_TX.
 */
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 78d6667..2325d41 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -338,8 +338,7 @@ static int gdm_usb_send(void *priv_dev, void *data, int len,
t-callback = cb;
t-cb_data = cb_data;
 
-   /*
-* In some cases, USB Module of WiMax is blocked when data size is
+   /* In some cases, USB Module of WiMax is blocked when data size is
 * the multiple of 512. So, increment length by one in that case.
 */
if ((len % 512) == 0)
@@ -439,8 +438,7 @@ static void gdm_usb_rcv_complete(struct urb *urb)
list_for_each_entry(t, tx-sdu_list, list) {
usb_submit_urb(t-urb, GFP_ATOMIC);
}
-   /*
-* If free buffer for sdu tx doesn't
+   /* If free buffer for sdu tx doesn't
 * exist, then tx queue should not be
 * woken. For this reason, don't pass
 * the command, START_SDU_TX.
diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index e5e5115..833d0d4 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -580,8 +580,9 @@ static int gdm_wimax_ioctl(struct net_device *dev, struct 
ifreq *ifr, int cmd)
return ret;
} else if (req-cmd == SIOCS_DATA) {
if (req-data_id == SIOC_DATA_FSM) {
-   /*NOTE: gdm_update_fsm should be called
-   before gdm_wimax_ioctl_set_data is called*/
+   /* NOTE: gdm_update_fsm should be called
+* before gdm_wimax_ioctl_set_data is called.
+*/
gdm_update_fsm(dev,
   (struct fsm_s *)req-data.buf);
}
diff --git a/drivers/staging/gdm72xx/hci.h b/drivers/staging/gdm72xx/hci.h
index 2485a37..059ba00 100644
--- a/drivers/staging/gdm72xx/hci.h
+++ b/drivers/staging/gdm72xx/hci.h
@@ -112,13 +112,11 @@
 #define W_SCAN_ALL_SUBSCRIPTION1
 #define W_SCAN_SPECIFIED_SUBSCRIPTION  2
 
-/*
- * TLV
+/* TLV
  *
  * [31:31] indicates the type is composite.
  * [30:16] is the length of the type. 0 length means length is variable.
  * [15:0] is the actual type.
- *
  */
 #define TLV_L(x)   (((x)  16)  0xff)
 #define TLV_T(x)   ((x)  0xff)
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] MIPS: ZBOOT: implement stack protector in compressed boot phase

[Ben Chan]

This patch implements the stack protector code in MIPS compressed boot
phase based on the same code added to arm in commit
8779657d29c0ebcc0c94ede4df2f497baf1b563f stackprotector: Introduce
CONFIG_CC_STACKPROTECTOR_STRONG by Kees Cook keesc...@chromium.org

Signed-off-by: Ben Chan benc...@chromium.org
Cc: Kees Cook keesc...@chromium.org
Cc: Olof Johansson ol...@chromium.org
---
 arch/mips/boot/compressed/decompress.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/mips/boot/compressed/decompress.c 
b/arch/mips/boot/compressed/decompress.c
index c00c4dd..b49c7ad 100644
--- a/arch/mips/boot/compressed/decompress.c
+++ b/arch/mips/boot/compressed/decompress.c
@@ -67,10 +67,24 @@ void error(char *x)
 #include ../../../../lib/decompress_unxz.c
 #endif
 
+unsigned long __stack_chk_guard;
+
+void __stack_chk_guard_setup(void)
+{
+   __stack_chk_guard = 0x000a0dff;
+}
+
+void __stack_chk_fail(void)
+{
+   error(stack-protector: Kernel stack is corrupted\n);
+}
+
 void decompress_kernel(unsigned long boot_heap_start)
 {
unsigned long zimage_start, zimage_size;
 
+   __stack_chk_guard_setup();
+
zimage_start = (unsigned long)(__image_begin);
zimage_size = (unsigned long)(__image_end) -
(unsigned long)(__image_begin);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: check return value of sscanf

[Ben Chan]

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_wimax.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index e5e5115..3081fd4 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -291,8 +291,9 @@ static void __gdm_wimax_event_send(struct work_struct *work)
e = list_entry(wm_event.evtq.next, struct evt_entry, list);
spin_unlock_irqrestore(wm_event.evt_lock, flags);
 
-   sscanf(e-dev-name, wm%d, idx);
-   netlink_send(wm_event.sock, idx, 0, e-evt_data, e-size);
+   if (sscanf(e-dev-name, wm%d, idx) == 1)
+   netlink_send(wm_event.sock, idx, 0, e-evt_data,
+e-size);
 
spin_lock_irqsave(wm_event.evt_lock, flags);
list_del(e-list);
-- 
2.0.0.526.g5318336

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next v4 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan 
---
 drivers/net/usb/cdc_ncm.c   | 17 +
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..e8711a8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -74,6 +74,7 @@ static int cdc_ncm_setup(struct usbnet *dev)
u8 iface_no;
int err;
int eth_hlen;
+   u16 mbim_mtu;
u16 ntb_fmt_supported;
__le16 max_datagram_size;
 
@@ -261,6 +262,14 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev->net->mtu > ctx->max_datagram_size - eth_hlen)
dev->net->mtu = ctx->max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx->mbim_extended_desc) {
+   mbim_mtu = le16_to_cpu(ctx->mbim_extended_desc->wMTU);
+   if (mbim_mtu != 0 && mbim_mtu < dev->net->mtu)
+   dev->net->mtu = mbim_mtu;
+   }
+
return 0;
 }
 
@@ -399,6 +408,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx->mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0] < sizeof(*(ctx->mbim_extended_desc)))
+   break;
+
+   ctx->mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next v4 0/2] adjust MTU as indicated by MBIM extended functional descriptor

[Ben Chan]

The MBIM extended functional descriptor, defined in "Universal Serial Bus
Communications Class Subclass Specification for Mobile Broadband Interface
Model, Revision 1.0, Errata-1" by USB-IF, indicates the operator preferred MTU
value via a wMTU field.

This patch set ensures that the initial MTU value set by cdc_ncm on a MBIM net
device does not exceed the wMTU value, provided the MBIM device exposes a MBIM
extended functional descriptor.

* Changelog
v2: Fixed a le16_to_cpu conversion issue in patch 2/2 pointed out by
Bjørn Mork 
v3: No code changes. Resubmitted to include patch 1/2 as suggested by
David Miller 
v4: No code changes. Resubmitted as suggested by David Miller:
- Added a summary of the patch set
- Carried the ACK from Greg Kroah-Hartman 
- Added a specified the tree (net-next) to apply the patch set to

Ben Chan (2):
  USB: cdc: add MBIM extended functional descriptor structure
  net: cdc_ncm: respect operator preferred MTU reported by MBIM

 drivers/net/usb/cdc_ncm.c| 17 +
 include/linux/usb/cdc_ncm.h  |  1 +
 include/uapi/linux/usb/cdc.h | 12 
 3 files changed, 30 insertions(+)

-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next v4 1/2] USB: cdc: add MBIM extended functional descriptor structure

[Ben Chan]

This patch adds the MBIM extended functional descriptor structure
defined in "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF.

Signed-off-by: Ben Chan 
Acked-by: Greg Kroah-Hartman 
---
 include/uapi/linux/usb/cdc.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index f35aa0a..b6a9cdd 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -56,6 +56,7 @@
 #define USB_CDC_OBEX_TYPE  0x15
 #define USB_CDC_NCM_TYPE   0x1a
 #define USB_CDC_MBIM_TYPE  0x1b
+#define USB_CDC_MBIM_EXTENDED_TYPE 0x1c
 
 /* "Header Functional Descriptor" from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -205,6 +206,17 @@ struct usb_cdc_mbim_desc {
__u8bmNetworkCapabilities;
 } __attribute__ ((packed));
 
+/* "MBIM Extended Functional Descriptor" from CDC MBIM spec 1.0 errata-1 */
+struct usb_cdc_mbim_extended_desc {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8bDescriptorSubType;
+
+   __le16  bcdMBIMExtendedVersion;
+   __u8bMaxOutstandingCommandMessages;
+   __le16  wMTU;
+} __attribute__ ((packed));
+
 /*-*/
 
 /*
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next v4 1/2] USB: cdc: add MBIM extended functional descriptor structure

[Ben Chan]

This patch adds the MBIM extended functional descriptor structure
defined in Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF.

Signed-off-by: Ben Chan benc...@chromium.org
Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
 include/uapi/linux/usb/cdc.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index f35aa0a..b6a9cdd 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -56,6 +56,7 @@
 #define USB_CDC_OBEX_TYPE  0x15
 #define USB_CDC_NCM_TYPE   0x1a
 #define USB_CDC_MBIM_TYPE  0x1b
+#define USB_CDC_MBIM_EXTENDED_TYPE 0x1c
 
 /* Header Functional Descriptor from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -205,6 +206,17 @@ struct usb_cdc_mbim_desc {
__u8bmNetworkCapabilities;
 } __attribute__ ((packed));
 
+/* MBIM Extended Functional Descriptor from CDC MBIM spec 1.0 errata-1 */
+struct usb_cdc_mbim_extended_desc {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8bDescriptorSubType;
+
+   __le16  bcdMBIMExtendedVersion;
+   __u8bMaxOutstandingCommandMessages;
+   __le16  wMTU;
+} __attribute__ ((packed));
+
 /*-*/
 
 /*
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next v4 0/2] adjust MTU as indicated by MBIM extended functional descriptor

[Ben Chan]

The MBIM extended functional descriptor, defined in Universal Serial Bus
Communications Class Subclass Specification for Mobile Broadband Interface
Model, Revision 1.0, Errata-1 by USB-IF, indicates the operator preferred MTU
value via a wMTU field.

This patch set ensures that the initial MTU value set by cdc_ncm on a MBIM net
device does not exceed the wMTU value, provided the MBIM device exposes a MBIM
extended functional descriptor.

* Changelog
v2: Fixed a le16_to_cpu conversion issue in patch 2/2 pointed out by
Bjørn Mork bj...@mork.no
v3: No code changes. Resubmitted to include patch 1/2 as suggested by
David Miller da...@davemloft.net
v4: No code changes. Resubmitted as suggested by David Miller:
- Added a summary of the patch set
- Carried the ACK from Greg Kroah-Hartman gre...@linuxfoundation.org
- Added a specified the tree (net-next) to apply the patch set to

Ben Chan (2):
  USB: cdc: add MBIM extended functional descriptor structure
  net: cdc_ncm: respect operator preferred MTU reported by MBIM

 drivers/net/usb/cdc_ncm.c| 17 +
 include/linux/usb/cdc_ncm.h  |  1 +
 include/uapi/linux/usb/cdc.h | 12 
 3 files changed, 30 insertions(+)

-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH net-next v4 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/net/usb/cdc_ncm.c   | 17 +
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..e8711a8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -74,6 +74,7 @@ static int cdc_ncm_setup(struct usbnet *dev)
u8 iface_no;
int err;
int eth_hlen;
+   u16 mbim_mtu;
u16 ntb_fmt_supported;
__le16 max_datagram_size;
 
@@ -261,6 +262,14 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev-net-mtu  ctx-max_datagram_size - eth_hlen)
dev-net-mtu = ctx-max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx-mbim_extended_desc) {
+   mbim_mtu = le16_to_cpu(ctx-mbim_extended_desc-wMTU);
+   if (mbim_mtu != 0  mbim_mtu  dev-net-mtu)
+   dev-net-mtu = mbim_mtu;
+   }
+
return 0;
 }
 
@@ -399,6 +408,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx-mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0]  sizeof(*(ctx-mbim_extended_desc)))
+   break;
+
+   ctx-mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

On Mon, Mar 17, 2014 at 6:41 PM, David Miller  wrote:
> From: Ben Chan 
> Date: Mon, 17 Mar 2014 17:46:27 -0700
>
>> Thanks again for the review and tip. I've submitted patch v2 to
>> address the le16_to_cpu conversion.
>
> When you update a patch from a series, you should repost the entire
> patch set, rather than just the patch which changes.
>
> This avoids any and all ambiguity of which patches go with which
> others.

Thanks for pointing that out. I've submitted the whole patch set as v3
(bumped the revision to avoid confusion).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/2] USB: cdc: add MBIM extended functional descriptor structure

[Ben Chan]

This patch adds the MBIM extended functional descriptor structure
defined in "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF.

Signed-off-by: Ben Chan 
---
No changes from patch v1.

 include/uapi/linux/usb/cdc.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index f35aa0a..b6a9cdd 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -56,6 +56,7 @@
 #define USB_CDC_OBEX_TYPE  0x15
 #define USB_CDC_NCM_TYPE   0x1a
 #define USB_CDC_MBIM_TYPE  0x1b
+#define USB_CDC_MBIM_EXTENDED_TYPE 0x1c
 
 /* "Header Functional Descriptor" from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -205,6 +206,17 @@ struct usb_cdc_mbim_desc {
__u8bmNetworkCapabilities;
 } __attribute__ ((packed));
 
+/* "MBIM Extended Functional Descriptor" from CDC MBIM spec 1.0 errata-1 */
+struct usb_cdc_mbim_extended_desc {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8bDescriptorSubType;
+
+   __le16  bcdMBIMExtendedVersion;
+   __u8bMaxOutstandingCommandMessages;
+   __le16  wMTU;
+} __attribute__ ((packed));
+
 /*-*/
 
 /*
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan 
---
Patch v3 (which is same as v2) adds the le16_to_cpu conversion on
ctx->mbim_extended_desc->wMTU as pointed out and suggested by
'Bjørn Mork '.

 drivers/net/usb/cdc_ncm.c   | 17 +
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..e8711a8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -74,6 +74,7 @@ static int cdc_ncm_setup(struct usbnet *dev)
u8 iface_no;
int err;
int eth_hlen;
+   u16 mbim_mtu;
u16 ntb_fmt_supported;
__le16 max_datagram_size;
 
@@ -261,6 +262,14 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev->net->mtu > ctx->max_datagram_size - eth_hlen)
dev->net->mtu = ctx->max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx->mbim_extended_desc) {
+   mbim_mtu = le16_to_cpu(ctx->mbim_extended_desc->wMTU);
+   if (mbim_mtu != 0 && mbim_mtu < dev->net->mtu)
+   dev->net->mtu = mbim_mtu;
+   }
+
return 0;
 }
 
@@ -399,6 +408,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx->mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0] < sizeof(*(ctx->mbim_extended_desc)))
+   break;
+
+   ctx->mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

On Mon, Mar 17, 2014 at 2:27 PM, Bjørn Mork  wrote:

>
> This sounds all reasonable to me. Thanks for taking the time to explain
> it in such detail. I did know that some vendors set wMaxSegmentSize too
> low, but had no idea that vendors were using the extended descriptor
> instead of MBIM_CID_IP_CONFIGURATION.
>
> If so, then yes, it does make sense for the driver to base the default
> MTU on this descriptor.

Hopefully, subsequent MBIM specifications would further clarify and
simplify things a bit, but it's gonna be a slow process as we all know
:-/

>
>>> wMTU access needs le16_to_cpu.
>>
>> Good catch. I will fix it in patch v2.
>
> Tip: I found this because my test script/makefile includes
> "C=1 CF=-D__CHECK_ENDIAN__"
>
> I find that very useful when dealing with USB on a little endian system,
> like most of us have.  It's all too easy to miss a conversion otherwise.
>

Thanks again for the review and tip. I've submitted patch v2 to
address the le16_to_cpu conversion.


>>> Could we move this final MTU correction from cdc_ncm_setup to
>>> cdc_ncm_bind_common to avoid bloating the device struct with another
>>> descriptor pointer we donæt really need to keep around?
>>>
>>> I know we look into descriptors in cdc_ncm_setup, because we have to,
>>> but ideally I would have loved to see cdc_ncm_setup dealing with *just*
>>> the NCM/MBIM specific control setup messages and cdc_ncm_bind_common
>>> dealing with all the functional descriptors.  That seems most logic to
>>> me (but is of course only my personal opinion and nothing else - I do
>>> not know what the original cdc_ncm author intended)
>>>
>>
>> I understand the argument against the extra descriptor pointer. But I
>> think it's better to keep the mtu related code together so that one
>> can easily see how MTU is determined when trying to change or refactor
>> the code. I haven't looked into what cdc_ncm_setup was originally
>> intended for. If we'd like to avoid adding an extra pointer in
>> cdc_ncm_ctx, we could have cdc_ncm_bind passing a locally scoped
>> context to cdc_ncm_setup.
>
> No, the extra pointer doesn't matter much. Just keep it as it is.
>
>
> Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan 
---
Patch v2 adds the le16_to_cpu conversion on ctx->mbim_extended_desc->wMTU as
pointed out and suggested by 'Bjørn Mork '.

 drivers/net/usb/cdc_ncm.c   | 17 +
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..e8711a8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -74,6 +74,7 @@ static int cdc_ncm_setup(struct usbnet *dev)
u8 iface_no;
int err;
int eth_hlen;
+   u16 mbim_mtu;
u16 ntb_fmt_supported;
__le16 max_datagram_size;
 
@@ -261,6 +262,14 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev->net->mtu > ctx->max_datagram_size - eth_hlen)
dev->net->mtu = ctx->max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx->mbim_extended_desc) {
+   mbim_mtu = le16_to_cpu(ctx->mbim_extended_desc->wMTU);
+   if (mbim_mtu != 0 && mbim_mtu < dev->net->mtu)
+   dev->net->mtu = mbim_mtu;
+   }
+
return 0;
 }
 
@@ -399,6 +408,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx->mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0] < sizeof(*(ctx->mbim_extended_desc)))
+   break;
+
+   ctx->mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

On Mon, Mar 17, 2014 at 2:42 AM, Bjørn Mork  wrote:
> Is this *really* driver material, or should we just leave the IP MTU
> hint handling up to the userspace management application?
>
> There are no less than 3(!) different ways for a device to specify the
> MBIM MTU:
>
>  1) wMaxSegmentSize field of the "MBIM Control Model Functional
>   Descriptor"
>   (mandatory, pre-errata1, per-device, both IP and DSS)
>
>  2) IPv4Mtu/IPv6Mtu fields of the MBIM_IP_CONFIGURATION_INFO response to
>   MBIM_CID_IP_CONFIGURATION
>   (mandatory, pre-errata1, per-session, IP only)
>
>  3) wMTU field of the "MBIM Extended Functional Descriptor"
>   (optional, errata1, per-operator, IP only)
>
> I see the optional extended MBIM descriptor as no more than a hint, and
> given that MBIM_CID_IP_CONFIGURATION is mandatory I see no real use case
> at all.
>
> If the extended decriptor is there, and the application supports it,
> then fine - let the application base the default IP MTU on this
> value. But I cannot see anything possibly break for an application and
> driver which does not support it.  The fact that the USB-IF didn't
> bother to update the MBIM version number when they added this descriptor
> supports that claim IMHO.  It tells us that it is perfectly OK for any
> MBIM v1.0 compliant application/driver to not even parse this
> descriptor, because the application/driver can predate Errata1.  The
> descriptor was added out of the blue in something called an "Errata",
> and is IMHO best ignored until a proper new version of the standard is
> published.
>
> The driver currently set the MTU based on the wMaxSegmentSize from the
> mandatory MBIM descriptor.  AFAICS this is the only field the driver
> *can* use.  The MBIM management application may make use of the extended
> descriptor, because it will know about "operator" and the relation to
> "IP data streams".  The driver does not have this knowledge, and it must
> be capable of supporting non-IP streams (DSS) multiplexed over the same
> network device.
>
> AFAICS, the MTU information provided by the extended descriptor is
> completely redundant for all cases where it has a meaning (the mandatory
> MBIM_CID_IP_CONFIGURATION will provde the IPv4 and IPv6 per-session
> MTUs), and possily completely wrong for any other case (wMaxSegmentSize
> sets the maximum size for non-IP sessions)
>
> My advice is to leave parsing of this descriptor to userspace.  That is
> also the only place where we can make use of the
> bMaxOutstandingCommandMessages, which actually is useful information. I
> just don't understand why they had to make that part of a descriptor
> when they have defined a new dedicated and extensible management
> protocol
>
> In case you disagree with the above (and do feel free to do so... I am
> often wrong), some minor nits regarding the actual implementation:
>

It's a bit messy how MTU is currently handled in MBIM. While wMTU may
seem optional and redundant, it addresses some issues with
wMaxSegmentSize and MBIM_CID_IP_CONFIGURATION, and hence why I suggest
using wMTU when available:

(1) wMaxSegmentSize

The MBIM 1.0 errata-1 spec does suggest that wMaxSegmentSize must be
at least 2048 and should not be used for determining IP MTU. However,
some MBIM devices follow Microsoft's guideline, which suggests using
wMaxSegmentSize to determine link MTU and its value should be between
1280 and 1500. The guideline may have been made before MBIM 1.0, but
it clearly contradicts and violates the current spec. Unfortunately,
it's followed by device vendors in practice. We could modify cdc_ncm
not to have the lower bound (i.e. CDC_MBIM_MIN_DATAGRAM_SIZE = 2048)
that it currently enforces. I don't feel like we should violate the
spec in the driver if there are alternative solutions.

(2) MBIM_CID_IP_CONFIGURATION

MBIM_CID_IP_CONFIGURATION doesn't necessarily contain MTU information
according to the spec. Bit 3 of IPv4ConfigurationAvailable /
IPv6ConfigurationAvailable of MBIM_IP_CONFIGURATION_INFO indicates
whether MTU information is available. As the Microsoft guideline also
suggests that MBIM_CID_IP_CONFIGURATION wouldn't be used for MTU
purpose, I wouldn't be too surprised that devices just don't bother to
notify MTU via MBIM_CID_IP_CONFIGURATION.

(3) wMTU

The MBIM extended functional descriptor is optional, but device
vendors do use it to indicate the MTU (mostly due to aforementioned
confusion around wMaxSegmentSize). Using the wMTU field wouldn't add
too much code or runtime overhead in kernel, so why don't we use it to
set up the initial MTU when available? We could handle it in
userspace, but I see the cdc_ncm driver is a better fit as it (like
other net drivers) already sets up mtu and leaving the wMTU case would
seem incomplete to me.

While (1) and (2) are fixable, it's hard to convince device vendors to
update their firmware just for that as carrier certifications impose a
heavy cost of firmware changes.


> wMTU access needs 

Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

On Mon, Mar 17, 2014 at 2:42 AM, Bjørn Mork bj...@mork.no wrote:
 Is this *really* driver material, or should we just leave the IP MTU
 hint handling up to the userspace management application?

 There are no less than 3(!) different ways for a device to specify the
 MBIM MTU:

  1) wMaxSegmentSize field of the MBIM Control Model Functional
   Descriptor
   (mandatory, pre-errata1, per-device, both IP and DSS)

  2) IPv4Mtu/IPv6Mtu fields of the MBIM_IP_CONFIGURATION_INFO response to
   MBIM_CID_IP_CONFIGURATION
   (mandatory, pre-errata1, per-session, IP only)

  3) wMTU field of the MBIM Extended Functional Descriptor
   (optional, errata1, per-operator, IP only)

 I see the optional extended MBIM descriptor as no more than a hint, and
 given that MBIM_CID_IP_CONFIGURATION is mandatory I see no real use case
 at all.

 If the extended decriptor is there, and the application supports it,
 then fine - let the application base the default IP MTU on this
 value. But I cannot see anything possibly break for an application and
 driver which does not support it.  The fact that the USB-IF didn't
 bother to update the MBIM version number when they added this descriptor
 supports that claim IMHO.  It tells us that it is perfectly OK for any
 MBIM v1.0 compliant application/driver to not even parse this
 descriptor, because the application/driver can predate Errata1.  The
 descriptor was added out of the blue in something called an Errata,
 and is IMHO best ignored until a proper new version of the standard is
 published.

 The driver currently set the MTU based on the wMaxSegmentSize from the
 mandatory MBIM descriptor.  AFAICS this is the only field the driver
 *can* use.  The MBIM management application may make use of the extended
 descriptor, because it will know about operator and the relation to
 IP data streams.  The driver does not have this knowledge, and it must
 be capable of supporting non-IP streams (DSS) multiplexed over the same
 network device.

 AFAICS, the MTU information provided by the extended descriptor is
 completely redundant for all cases where it has a meaning (the mandatory
 MBIM_CID_IP_CONFIGURATION will provde the IPv4 and IPv6 per-session
 MTUs), and possily completely wrong for any other case (wMaxSegmentSize
 sets the maximum size for non-IP sessions)

 My advice is to leave parsing of this descriptor to userspace.  That is
 also the only place where we can make use of the
 bMaxOutstandingCommandMessages, which actually is useful information. I
 just don't understand why they had to make that part of a descriptor
 when they have defined a new dedicated and extensible management
 protocol

 In case you disagree with the above (and do feel free to do so... I am
 often wrong), some minor nits regarding the actual implementation:


It's a bit messy how MTU is currently handled in MBIM. While wMTU may
seem optional and redundant, it addresses some issues with
wMaxSegmentSize and MBIM_CID_IP_CONFIGURATION, and hence why I suggest
using wMTU when available:

(1) wMaxSegmentSize

The MBIM 1.0 errata-1 spec does suggest that wMaxSegmentSize must be
at least 2048 and should not be used for determining IP MTU. However,
some MBIM devices follow Microsoft's guideline, which suggests using
wMaxSegmentSize to determine link MTU and its value should be between
1280 and 1500. The guideline may have been made before MBIM 1.0, but
it clearly contradicts and violates the current spec. Unfortunately,
it's followed by device vendors in practice. We could modify cdc_ncm
not to have the lower bound (i.e. CDC_MBIM_MIN_DATAGRAM_SIZE = 2048)
that it currently enforces. I don't feel like we should violate the
spec in the driver if there are alternative solutions.

(2) MBIM_CID_IP_CONFIGURATION

MBIM_CID_IP_CONFIGURATION doesn't necessarily contain MTU information
according to the spec. Bit 3 of IPv4ConfigurationAvailable /
IPv6ConfigurationAvailable of MBIM_IP_CONFIGURATION_INFO indicates
whether MTU information is available. As the Microsoft guideline also
suggests that MBIM_CID_IP_CONFIGURATION wouldn't be used for MTU
purpose, I wouldn't be too surprised that devices just don't bother to
notify MTU via MBIM_CID_IP_CONFIGURATION.

(3) wMTU

The MBIM extended functional descriptor is optional, but device
vendors do use it to indicate the MTU (mostly due to aforementioned
confusion around wMaxSegmentSize). Using the wMTU field wouldn't add
too much code or runtime overhead in kernel, so why don't we use it to
set up the initial MTU when available? We could handle it in
userspace, but I see the cdc_ncm driver is a better fit as it (like
other net drivers) already sets up mtu and leaving the wMTU case would
seem incomplete to me.

While (1) and (2) are fixable, it's hard to convince device vendors to
update their firmware just for that as carrier certifications impose a
heavy cost of firmware changes.


 wMTU access needs le16_to_cpu.

Good catch. I will fix it in patch v2.


 Could we 

[PATCH v2 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan benc...@chromium.org
---
Patch v2 adds the le16_to_cpu conversion on ctx-mbim_extended_desc-wMTU as
pointed out and suggested by 'Bjørn Mork bj...@mork.no'.

 drivers/net/usb/cdc_ncm.c   | 17 +
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..e8711a8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -74,6 +74,7 @@ static int cdc_ncm_setup(struct usbnet *dev)
u8 iface_no;
int err;
int eth_hlen;
+   u16 mbim_mtu;
u16 ntb_fmt_supported;
__le16 max_datagram_size;
 
@@ -261,6 +262,14 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev-net-mtu  ctx-max_datagram_size - eth_hlen)
dev-net-mtu = ctx-max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx-mbim_extended_desc) {
+   mbim_mtu = le16_to_cpu(ctx-mbim_extended_desc-wMTU);
+   if (mbim_mtu != 0  mbim_mtu  dev-net-mtu)
+   dev-net-mtu = mbim_mtu;
+   }
+
return 0;
 }
 
@@ -399,6 +408,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx-mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0]  sizeof(*(ctx-mbim_extended_desc)))
+   break;
+
+   ctx-mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

On Mon, Mar 17, 2014 at 2:27 PM, Bjørn Mork bj...@mork.no wrote:


 This sounds all reasonable to me. Thanks for taking the time to explain
 it in such detail. I did know that some vendors set wMaxSegmentSize too
 low, but had no idea that vendors were using the extended descriptor
 instead of MBIM_CID_IP_CONFIGURATION.

 If so, then yes, it does make sense for the driver to base the default
 MTU on this descriptor.

Hopefully, subsequent MBIM specifications would further clarify and
simplify things a bit, but it's gonna be a slow process as we all know
:-/


 wMTU access needs le16_to_cpu.

 Good catch. I will fix it in patch v2.

 Tip: I found this because my test script/makefile includes
 C=1 CF=-D__CHECK_ENDIAN__

 I find that very useful when dealing with USB on a little endian system,
 like most of us have.  It's all too easy to miss a conversion otherwise.


Thanks again for the review and tip. I've submitted patch v2 to
address the le16_to_cpu conversion.


 Could we move this final MTU correction from cdc_ncm_setup to
 cdc_ncm_bind_common to avoid bloating the device struct with another
 descriptor pointer we donæt really need to keep around?

 I know we look into descriptors in cdc_ncm_setup, because we have to,
 but ideally I would have loved to see cdc_ncm_setup dealing with *just*
 the NCM/MBIM specific control setup messages and cdc_ncm_bind_common
 dealing with all the functional descriptors.  That seems most logic to
 me (but is of course only my personal opinion and nothing else - I do
 not know what the original cdc_ncm author intended)


 I understand the argument against the extra descriptor pointer. But I
 think it's better to keep the mtu related code together so that one
 can easily see how MTU is determined when trying to change or refactor
 the code. I haven't looked into what cdc_ncm_setup was originally
 intended for. If we'd like to avoid adding an extra pointer in
 cdc_ncm_ctx, we could have cdc_ncm_bind passing a locally scoped
 context to cdc_ncm_setup.

 No, the extra pointer doesn't matter much. Just keep it as it is.


 Bjørn
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/2] USB: cdc: add MBIM extended functional descriptor structure

[Ben Chan]

This patch adds the MBIM extended functional descriptor structure
defined in Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF.

Signed-off-by: Ben Chan benc...@chromium.org
---
No changes from patch v1.

 include/uapi/linux/usb/cdc.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index f35aa0a..b6a9cdd 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -56,6 +56,7 @@
 #define USB_CDC_OBEX_TYPE  0x15
 #define USB_CDC_NCM_TYPE   0x1a
 #define USB_CDC_MBIM_TYPE  0x1b
+#define USB_CDC_MBIM_EXTENDED_TYPE 0x1c
 
 /* Header Functional Descriptor from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -205,6 +206,17 @@ struct usb_cdc_mbim_desc {
__u8bmNetworkCapabilities;
 } __attribute__ ((packed));
 
+/* MBIM Extended Functional Descriptor from CDC MBIM spec 1.0 errata-1 */
+struct usb_cdc_mbim_extended_desc {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8bDescriptorSubType;
+
+   __le16  bcdMBIMExtendedVersion;
+   __u8bMaxOutstandingCommandMessages;
+   __le16  wMTU;
+} __attribute__ ((packed));
+
 /*-*/
 
 /*
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan benc...@chromium.org
---
Patch v3 (which is same as v2) adds the le16_to_cpu conversion on
ctx-mbim_extended_desc-wMTU as pointed out and suggested by
'Bjørn Mork bj...@mork.no'.

 drivers/net/usb/cdc_ncm.c   | 17 +
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..e8711a8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -74,6 +74,7 @@ static int cdc_ncm_setup(struct usbnet *dev)
u8 iface_no;
int err;
int eth_hlen;
+   u16 mbim_mtu;
u16 ntb_fmt_supported;
__le16 max_datagram_size;
 
@@ -261,6 +262,14 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev-net-mtu  ctx-max_datagram_size - eth_hlen)
dev-net-mtu = ctx-max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx-mbim_extended_desc) {
+   mbim_mtu = le16_to_cpu(ctx-mbim_extended_desc-wMTU);
+   if (mbim_mtu != 0  mbim_mtu  dev-net-mtu)
+   dev-net-mtu = mbim_mtu;
+   }
+
return 0;
 }
 
@@ -399,6 +408,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx-mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0]  sizeof(*(ctx-mbim_extended_desc)))
+   break;
+
+   ctx-mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

On Mon, Mar 17, 2014 at 6:41 PM, David Miller da...@davemloft.net wrote:
 From: Ben Chan benc...@chromium.org
 Date: Mon, 17 Mar 2014 17:46:27 -0700

 Thanks again for the review and tip. I've submitted patch v2 to
 address the le16_to_cpu conversion.

 When you update a patch from a series, you should repost the entire
 patch set, rather than just the patch which changes.

 This avoids any and all ambiguity of which patches go with which
 others.

Thanks for pointing that out. I've submitted the whole patch set as v3
(bumped the revision to avoid confusion).
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] USB: cdc: add MBIM extended functional descriptor structure

[Ben Chan]

This patch adds the MBIM extended functional descriptor structure
defined in "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF.

Signed-off-by: Ben Chan 
---
 include/uapi/linux/usb/cdc.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index f35aa0a..b6a9cdd 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -56,6 +56,7 @@
 #define USB_CDC_OBEX_TYPE  0x15
 #define USB_CDC_NCM_TYPE   0x1a
 #define USB_CDC_MBIM_TYPE  0x1b
+#define USB_CDC_MBIM_EXTENDED_TYPE 0x1c
 
 /* "Header Functional Descriptor" from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -205,6 +206,17 @@ struct usb_cdc_mbim_desc {
__u8bmNetworkCapabilities;
 } __attribute__ ((packed));
 
+/* "MBIM Extended Functional Descriptor" from CDC MBIM spec 1.0 errata-1 */
+struct usb_cdc_mbim_extended_desc {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8bDescriptorSubType;
+
+   __le16  bcdMBIMExtendedVersion;
+   __u8bMaxOutstandingCommandMessages;
+   __le16  wMTU;
+} __attribute__ ((packed));
+
 /*-*/
 
 /*
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to "Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1" published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan 
---
 drivers/net/usb/cdc_ncm.c   | 15 +++
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..0b036ed 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -261,6 +261,13 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev->net->mtu > ctx->max_datagram_size - eth_hlen)
dev->net->mtu = ctx->max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx->mbim_extended_desc &&
+   ctx->mbim_extended_desc->wMTU != 0 &&
+   dev->net->mtu > ctx->mbim_extended_desc->wMTU)
+   dev->net->mtu = ctx->mbim_extended_desc->wMTU;
+
return 0;
 }
 
@@ -399,6 +406,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx->mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0] < sizeof(*(ctx->mbim_extended_desc)))
+   break;
+
+   ctx->mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

[Ben Chan]

According to Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF, the wMTU field of the MBIM extended
functional descriptor indicates the operator preferred MTU for IP data
streams.

This patch modifies cdc_ncm_setup to ensure that the MTU value set on
the usbnet device does not exceed the operator preferred MTU indicated
by wMTU if the MBIM device exposes a MBIM extended functional
descriptor.

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/net/usb/cdc_ncm.c   | 15 +++
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index dbff290..0b036ed 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -261,6 +261,13 @@ out:
/* set MTU to max supported by the device if necessary */
if (dev-net-mtu  ctx-max_datagram_size - eth_hlen)
dev-net-mtu = ctx-max_datagram_size - eth_hlen;
+
+   /* do not exceed operater preferred MTU */
+   if (ctx-mbim_extended_desc 
+   ctx-mbim_extended_desc-wMTU != 0 
+   dev-net-mtu  ctx-mbim_extended_desc-wMTU)
+   dev-net-mtu = ctx-mbim_extended_desc-wMTU;
+
return 0;
 }
 
@@ -399,6 +406,14 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
ctx-mbim_desc = (const struct usb_cdc_mbim_desc *)buf;
break;
 
+   case USB_CDC_MBIM_EXTENDED_TYPE:
+   if (buf[0]  sizeof(*(ctx-mbim_extended_desc)))
+   break;
+
+   ctx-mbim_extended_desc =
+   (const struct usb_cdc_mbim_extended_desc *)buf;
+   break;
+
default:
break;
}
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c3fa807..bdf05fb 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
 
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
+   const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
const struct usb_cdc_ether_desc *ether_desc;
 
struct usb_interface *control;
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] USB: cdc: add MBIM extended functional descriptor structure

[Ben Chan]

This patch adds the MBIM extended functional descriptor structure
defined in Universal Serial Bus Communications Class Subclass
Specification for Mobile Broadband Interface Model, Revision 1.0,
Errata-1 published by USB-IF.

Signed-off-by: Ben Chan benc...@chromium.org
---
 include/uapi/linux/usb/cdc.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index f35aa0a..b6a9cdd 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -56,6 +56,7 @@
 #define USB_CDC_OBEX_TYPE  0x15
 #define USB_CDC_NCM_TYPE   0x1a
 #define USB_CDC_MBIM_TYPE  0x1b
+#define USB_CDC_MBIM_EXTENDED_TYPE 0x1c
 
 /* Header Functional Descriptor from CDC spec  5.2.3.1 */
 struct usb_cdc_header_desc {
@@ -205,6 +206,17 @@ struct usb_cdc_mbim_desc {
__u8bmNetworkCapabilities;
 } __attribute__ ((packed));
 
+/* MBIM Extended Functional Descriptor from CDC MBIM spec 1.0 errata-1 */
+struct usb_cdc_mbim_extended_desc {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8bDescriptorSubType;
+
+   __le16  bcdMBIMExtendedVersion;
+   __u8bMaxOutstandingCommandMessages;
+   __le16  wMTU;
+} __attribute__ ((packed));
+
 /*-*/
 
 /*
-- 
1.9.0.279.gdc9e3eb

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] staging: gdm72xx: fix typos in Kconfig

[Ben Chan]

Signed-off-by: Ben Chan 
Cc: Sage Ahn 
---
 drivers/staging/gdm72xx/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index dd47bd1..dd8a391 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -19,7 +19,7 @@ config WIMAX_GDM72XX_K_MODE
default n
 
 config WIMAX_GDM72XX_WIMAX2
-   bool "Enable WIMAX2 support"
+   bool "Enable WiMAX2 support"
default n
 
 choice
@@ -38,7 +38,7 @@ endchoice
 if WIMAX_GDM72XX_USB
 
 config WIMAX_GDM72XX_USB_PM
-   bool "Enable power managerment support"
+   bool "Enable power management support"
depends on PM_RUNTIME
 
 endif # WIMAX_GDM72XX_USB
-- 
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 1/2] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

The gdm72xx driver needs to have either the USB or SDIO implementation
enabled to provide useful functionalities, so the driver should depend
on either USB or MMC. This patch makes WIMAX_GDM72XX depend on either
USB or MMC.

Also, WIMAX_GDM72XX needs to be built as a module if its dependent
interface, either USB or MMC, is built as a module. This patch enforces
that in the WIMAX_GDM72XX_USB and WIMAX_GDM72XX_SDIO dependency.

Reported-by: Alan Stern 
Signed-off-by: Ben Chan 
Cc: Sage Ahn 
---
 drivers/staging/gdm72xx/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index 6905913..dd47bd1 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig WIMAX_GDM72XX
tristate "GCT GDM72xx WiMAX support"
-   depends on NET
+   depends on NET && (USB || MMC)
help
  Support for the GCT GDM72xx WiMAX chip
 
@@ -27,11 +27,11 @@ choice
 
 config WIMAX_GDM72XX_USB
bool "USB interface"
-   depends on USB
+   depends on (USB = y || USB = WIMAX_GDM72XX)
 
 config WIMAX_GDM72XX_SDIO
bool "SDIO interface"
-   depends on MMC
+   depends on (MMC = y || MMC = WIMAX_GDM72XX)
 
 endchoice
 
-- 
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

No worries. I've attached a revised patch to the original email
thread, which includes Alan.

Thanks again,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

Nope... I didn't resend the patch but replied to the thread.

Ben

On Mon, Jun 3, 2013 at 11:18 AM, Ben Chan  wrote:
> Sorry, I meant to send the revised patch but got the wrong file.
>
> I'll add the Reported-by field.
>
> Thanks,
> Ben
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

Sorry, I meant to send the revised patch but got the wrong file.

I'll add the Reported-by field.

Thanks,
Ben

On Mon, Jun 3, 2013 at 10:48 AM, Greg Kroah-Hartman
 wrote:
> On Mon, Jun 03, 2013 at 10:23:45AM -0700, Ben Chan wrote:
>> The gdm72xx driver needs to have either the USB or SDIO implementation
>> enabled to provide useful functionalities, so the driver should depend
>> on either USB or MMC.
>>
>> Signed-off-by: Ben Chan 
>> Cc: Sage Ahn 
>> ---
>>  drivers/staging/gdm72xx/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/gdm72xx/Kconfig 
>> b/drivers/staging/gdm72xx/Kconfig
>> index 6905913..8af0dd4 100644
>> --- a/drivers/staging/gdm72xx/Kconfig
>> +++ b/drivers/staging/gdm72xx/Kconfig
>> @@ -4,7 +4,7 @@
>>
>>  menuconfig WIMAX_GDM72XX
>>   tristate "GCT GDM72xx WiMAX support"
>> - depends on NET
>> + depends on NET && (USB || MMC)
>
> Alan Stern already told you how this would not solve the problem, why
> did you resend it to me again?
>
> And you forgot the "Reported-by:" line above, giving proper credit to
> the person who told you about this problem.
>
> Please fix both of them and resend.
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

The gdm72xx driver needs to have either the USB or SDIO implementation
enabled to provide useful functionalities, so the driver should depend
on either USB or MMC.

Signed-off-by: Ben Chan 
Cc: Sage Ahn 
---
 drivers/staging/gdm72xx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index 6905913..8af0dd4 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig WIMAX_GDM72XX
tristate "GCT GDM72xx WiMAX support"
-   depends on NET
+   depends on NET && (USB || MMC)
help
  Support for the GCT GDM72xx WiMAX chip
 
-- 
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

No worries. I've attached a revised patch to the original email
thread, which includes Alan.

Thanks again,
Ben
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] staging: gdm72xx: fix typos in Kconfig

[Ben Chan]

Signed-off-by: Ben Chan benc...@chromium.org
Cc: Sage Ahn sy...@gctsemi.com
---
 drivers/staging/gdm72xx/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index dd47bd1..dd8a391 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -19,7 +19,7 @@ config WIMAX_GDM72XX_K_MODE
default n
 
 config WIMAX_GDM72XX_WIMAX2
-   bool Enable WIMAX2 support
+   bool Enable WiMAX2 support
default n
 
 choice
@@ -38,7 +38,7 @@ endchoice
 if WIMAX_GDM72XX_USB
 
 config WIMAX_GDM72XX_USB_PM
-   bool Enable power managerment support
+   bool Enable power management support
depends on PM_RUNTIME
 
 endif # WIMAX_GDM72XX_USB
-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 1/2] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

The gdm72xx driver needs to have either the USB or SDIO implementation
enabled to provide useful functionalities, so the driver should depend
on either USB or MMC. This patch makes WIMAX_GDM72XX depend on either
USB or MMC.

Also, WIMAX_GDM72XX needs to be built as a module if its dependent
interface, either USB or MMC, is built as a module. This patch enforces
that in the WIMAX_GDM72XX_USB and WIMAX_GDM72XX_SDIO dependency.

Reported-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ben Chan benc...@chromium.org
Cc: Sage Ahn sy...@gctsemi.com
---
 drivers/staging/gdm72xx/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index 6905913..dd47bd1 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig WIMAX_GDM72XX
tristate GCT GDM72xx WiMAX support
-   depends on NET
+   depends on NET  (USB || MMC)
help
  Support for the GCT GDM72xx WiMAX chip
 
@@ -27,11 +27,11 @@ choice
 
 config WIMAX_GDM72XX_USB
bool USB interface
-   depends on USB
+   depends on (USB = y || USB = WIMAX_GDM72XX)
 
 config WIMAX_GDM72XX_SDIO
bool SDIO interface
-   depends on MMC
+   depends on (MMC = y || MMC = WIMAX_GDM72XX)
 
 endchoice
 
-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

The gdm72xx driver needs to have either the USB or SDIO implementation
enabled to provide useful functionalities, so the driver should depend
on either USB or MMC.

Signed-off-by: Ben Chan benc...@chromium.org
Cc: Sage Ahn sy...@gctsemi.com
---
 drivers/staging/gdm72xx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gdm72xx/Kconfig b/drivers/staging/gdm72xx/Kconfig
index 6905913..8af0dd4 100644
--- a/drivers/staging/gdm72xx/Kconfig
+++ b/drivers/staging/gdm72xx/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig WIMAX_GDM72XX
tristate GCT GDM72xx WiMAX support
-   depends on NET
+   depends on NET  (USB || MMC)
help
  Support for the GCT GDM72xx WiMAX chip
 
-- 
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

Sorry, I meant to send the revised patch but got the wrong file.

I'll add the Reported-by field.

Thanks,
Ben

On Mon, Jun 3, 2013 at 10:48 AM, Greg Kroah-Hartman
gre...@linuxfoundation.org wrote:
 On Mon, Jun 03, 2013 at 10:23:45AM -0700, Ben Chan wrote:
 The gdm72xx driver needs to have either the USB or SDIO implementation
 enabled to provide useful functionalities, so the driver should depend
 on either USB or MMC.

 Signed-off-by: Ben Chan benc...@chromium.org
 Cc: Sage Ahn sy...@gctsemi.com
 ---
  drivers/staging/gdm72xx/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/staging/gdm72xx/Kconfig 
 b/drivers/staging/gdm72xx/Kconfig
 index 6905913..8af0dd4 100644
 --- a/drivers/staging/gdm72xx/Kconfig
 +++ b/drivers/staging/gdm72xx/Kconfig
 @@ -4,7 +4,7 @@

  menuconfig WIMAX_GDM72XX
   tristate GCT GDM72xx WiMAX support
 - depends on NET
 + depends on NET  (USB || MMC)

 Alan Stern already told you how this would not solve the problem, why
 did you resend it to me again?

 And you forgot the Reported-by: line above, giving proper credit to
 the person who told you about this problem.

 Please fix both of them and resend.

 thanks,

 greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: gdm72xx: WIMAX_GDM72XX should depend on either USB or MMC

[Ben Chan]

Nope... I didn't resend the patch but replied to the thread.

Ben

On Mon, Jun 3, 2013 at 11:18 AM, Ben Chan benc...@chromium.org wrote:
 Sorry, I meant to send the revised patch but got the wrong file.

 I'll add the Reported-by field.

 Thanks,
 Ben

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: fix unused variable warning in gdm_usb_send

[Ben Chan]

This patch fixes an unused variable warning in gdm_usb_send
(when CONFIG_WIMAX_GDM72XX_K_MODE=n), which was introduced in
commit 1a276b80466bbd195cf94ec7178f68f2ab351467 (staging:
gdm72xx: protect access of rx / tx structs).

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_usb.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 4426941..3709824 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -316,7 +316,10 @@ static int gdm_usb_send(void *priv_dev, void *data, int 
len,
int no_spc = 0, ret;
u8 *pkt = data;
u16 cmd_evt;
-   unsigned long flags, flags2;
+   unsigned long flags;
+#ifdef CONFIG_WIMAX_GDM72XX_K_MODE
+   unsigned long flags2;
+#endif /* CONFIG_WIMAX_GDM72XX_K_MODE */
 
if (!udev->usbdev) {
dev_err(>dev, "%s: No such device\n", __func__);
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: fix unused variable warning in gdm_usb_send

[Ben Chan]

This patch fixes an unused variable warning in gdm_usb_send
(when CONFIG_WIMAX_GDM72XX_K_MODE=n), which was introduced in
commit 1a276b80466bbd195cf94ec7178f68f2ab351467 (staging:
gdm72xx: protect access of rx / tx structs).

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_usb.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 4426941..3709824 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -316,7 +316,10 @@ static int gdm_usb_send(void *priv_dev, void *data, int 
len,
int no_spc = 0, ret;
u8 *pkt = data;
u16 cmd_evt;
-   unsigned long flags, flags2;
+   unsigned long flags;
+#ifdef CONFIG_WIMAX_GDM72XX_K_MODE
+   unsigned long flags2;
+#endif /* CONFIG_WIMAX_GDM72XX_K_MODE */
 
if (!udev-usbdev) {
dev_err(usbdev-dev, %s: No such device\n, __func__);
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: protect access of rx / tx structs

[Ben Chan]

This patch applies spinlock to protect access to rx / tx structs in
certain call sites, which fixes the following crash in gdm_suspend.
It also fixes usb_set_intfdata() in gdm_usb_probe to avoid setting an
already freed phy_dev.

<5>[ 4996.815018] [<7f0074b0>] (gdm_suspend+0x1c/0x2b4 [gdmwm]) from 
[<803020a4>] (usb_suspend_both+0x80/0x1a0)
<5>[ 4996.815055] [<803020a4>] (usb_suspend_both+0x80/0x1a0) from [<80302c84>] 
(usb_runtime_suspend+0x38/0x64)
<5>[ 4996.815089] [<80302c84>] (usb_runtime_suspend+0x38/0x64) from 
[<802becc0>] (__rpm_callback+0x48/0x78)
<5>[ 4996.815118] [<802becc0>] (__rpm_callback+0x48/0x78) from [<802bf8dc>] 
(rpm_suspend+0x394/0x5ec)
<5>[ 4996.815145] [<802bf8dc>] (rpm_suspend+0x394/0x5ec) from [<802c0550>] 
(pm_runtime_work+0x8c/0xa4)
<5>[ 4996.815177] [<802c0550>] (pm_runtime_work+0x8c/0xa4) from [<800456cc>] 
(process_one_work+0x264/0x438)
<5>[ 4996.815209] [<800456cc>] (process_one_work+0x264/0x438) from [<80045acc>] 
(worker_thread+0x22c/0x3b8)
<5>[ 4996.815239] [<80045acc>] (worker_thread+0x22c/0x3b8) from [<8004a43c>] 
(kthread+0x9c/0xa8)
<5>[ 4996.815270] [<8004a43c>] (kthread+0x9c/0xa8) from [<8000f160>] 
(kernel_thread_exit+0x0/0x8)
<0>[ 4996.815295] Code: e92d4000 e8bd4000 e2800020 eb4ab9a1 (e5905000)

Signed-off-by: Ben Chan 
Signed-off-by: Sameer Nanda 
---
 drivers/staging/gdm72xx/gdm_usb.c |   52 -
 1 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 0cc6317..4426941 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -186,6 +186,7 @@ static int init_usb(struct usbwm_dev *udev)
struct rx_cxt   *rx = >rx;
struct usb_tx   *t;
struct usb_rx   *r;
+   unsigned long flags;
 
INIT_LIST_HEAD(>free_list);
INIT_LIST_HEAD(>sdu_list);
@@ -200,6 +201,7 @@ static int init_usb(struct usbwm_dev *udev)
spin_lock_init(>lock);
spin_lock_init(>lock);
 
+   spin_lock_irqsave(>lock, flags);
for (i = 0; i < MAX_NR_SDU_BUF; i++) {
t = alloc_tx_struct(tx);
if (t == NULL) {
@@ -208,6 +210,7 @@ static int init_usb(struct usbwm_dev *udev)
}
list_add(>list, >free_list);
}
+   spin_unlock_irqrestore(>lock, flags);
 
r = alloc_rx_struct(rx);
if (r == NULL) {
@@ -215,7 +218,9 @@ static int init_usb(struct usbwm_dev *udev)
goto fail;
}
 
+   spin_lock_irqsave(>lock, flags);
list_add(>list, >free_list);
+   spin_unlock_irqrestore(>lock, flags);
return ret;
 
 fail:
@@ -229,6 +234,9 @@ static void release_usb(struct usbwm_dev *udev)
struct rx_cxt   *rx = >rx;
struct usb_tx   *t, *t_next;
struct usb_rx   *r, *r_next;
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
 
list_for_each_entry_safe(t, t_next, >sdu_list, list) {
list_del(>list);
@@ -245,6 +253,10 @@ static void release_usb(struct usbwm_dev *udev)
free_tx_struct(t);
}
 
+   spin_unlock_irqrestore(>lock, flags);
+
+   spin_lock_irqsave(>lock, flags);
+
list_for_each_entry_safe(r, r_next, >free_list, list) {
list_del(>list);
free_rx_struct(r);
@@ -254,6 +266,8 @@ static void release_usb(struct usbwm_dev *udev)
list_del(>list);
free_rx_struct(r);
}
+
+   spin_unlock_irqrestore(>lock, flags);
 }
 
 static void __gdm_usb_send_complete(struct urb *urb)
@@ -302,7 +316,7 @@ static int gdm_usb_send(void *priv_dev, void *data, int len,
int no_spc = 0, ret;
u8 *pkt = data;
u16 cmd_evt;
-   unsigned long flags;
+   unsigned long flags, flags2;
 
if (!udev->usbdev) {
dev_err(>dev, "%s: No such device\n", __func__);
@@ -371,13 +385,16 @@ static int gdm_usb_send(void *priv_dev, void *data, int 
len,
 
rx = >rx;
 
+   spin_lock_irqsave(>lock, flags2);
list_for_each_entry(r, >used_list, list)
usb_unlink_urb(r->urb);
+   spin_unlock_irqrestore(>lock, flags2);
+
udev->bw_switch = 1;
 
-   spin_lock(_lock);
+   spin_lock_irqsave(_lock, flags2);
list_add_tail(>list, _list);
-   spin_unlock(_lock);
+   spin_unlock_irqrestore(_lock, flags2);
 
wake_up(_wait);
}
@@ -416,7 +433,7 @@ static void gdm_usb_rcv_complete(struct urb *urb)
struct tx_cxt *tx = >tx;
struct usb_tx *t;
u16 cmd_

[PATCH] staging: gdm72xx: protect access of rx / tx structs

[Ben Chan]

This patch applies spinlock to protect access to rx / tx structs in
certain call sites, which fixes the following crash in gdm_suspend.
It also fixes usb_set_intfdata() in gdm_usb_probe to avoid setting an
already freed phy_dev.

5[ 4996.815018] [7f0074b0] (gdm_suspend+0x1c/0x2b4 [gdmwm]) from 
[803020a4] (usb_suspend_both+0x80/0x1a0)
5[ 4996.815055] [803020a4] (usb_suspend_both+0x80/0x1a0) from [80302c84] 
(usb_runtime_suspend+0x38/0x64)
5[ 4996.815089] [80302c84] (usb_runtime_suspend+0x38/0x64) from 
[802becc0] (__rpm_callback+0x48/0x78)
5[ 4996.815118] [802becc0] (__rpm_callback+0x48/0x78) from [802bf8dc] 
(rpm_suspend+0x394/0x5ec)
5[ 4996.815145] [802bf8dc] (rpm_suspend+0x394/0x5ec) from [802c0550] 
(pm_runtime_work+0x8c/0xa4)
5[ 4996.815177] [802c0550] (pm_runtime_work+0x8c/0xa4) from [800456cc] 
(process_one_work+0x264/0x438)
5[ 4996.815209] [800456cc] (process_one_work+0x264/0x438) from [80045acc] 
(worker_thread+0x22c/0x3b8)
5[ 4996.815239] [80045acc] (worker_thread+0x22c/0x3b8) from [8004a43c] 
(kthread+0x9c/0xa8)
5[ 4996.815270] [8004a43c] (kthread+0x9c/0xa8) from [8000f160] 
(kernel_thread_exit+0x0/0x8)
0[ 4996.815295] Code: e92d4000 e8bd4000 e2800020 eb4ab9a1 (e5905000)

Signed-off-by: Ben Chan benc...@chromium.org
Signed-off-by: Sameer Nanda sna...@chromium.org
---
 drivers/staging/gdm72xx/gdm_usb.c |   52 -
 1 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 0cc6317..4426941 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -186,6 +186,7 @@ static int init_usb(struct usbwm_dev *udev)
struct rx_cxt   *rx = udev-rx;
struct usb_tx   *t;
struct usb_rx   *r;
+   unsigned long flags;
 
INIT_LIST_HEAD(tx-free_list);
INIT_LIST_HEAD(tx-sdu_list);
@@ -200,6 +201,7 @@ static int init_usb(struct usbwm_dev *udev)
spin_lock_init(tx-lock);
spin_lock_init(rx-lock);
 
+   spin_lock_irqsave(tx-lock, flags);
for (i = 0; i  MAX_NR_SDU_BUF; i++) {
t = alloc_tx_struct(tx);
if (t == NULL) {
@@ -208,6 +210,7 @@ static int init_usb(struct usbwm_dev *udev)
}
list_add(t-list, tx-free_list);
}
+   spin_unlock_irqrestore(tx-lock, flags);
 
r = alloc_rx_struct(rx);
if (r == NULL) {
@@ -215,7 +218,9 @@ static int init_usb(struct usbwm_dev *udev)
goto fail;
}
 
+   spin_lock_irqsave(rx-lock, flags);
list_add(r-list, rx-free_list);
+   spin_unlock_irqrestore(rx-lock, flags);
return ret;
 
 fail:
@@ -229,6 +234,9 @@ static void release_usb(struct usbwm_dev *udev)
struct rx_cxt   *rx = udev-rx;
struct usb_tx   *t, *t_next;
struct usb_rx   *r, *r_next;
+   unsigned long flags;
+
+   spin_lock_irqsave(tx-lock, flags);
 
list_for_each_entry_safe(t, t_next, tx-sdu_list, list) {
list_del(t-list);
@@ -245,6 +253,10 @@ static void release_usb(struct usbwm_dev *udev)
free_tx_struct(t);
}
 
+   spin_unlock_irqrestore(tx-lock, flags);
+
+   spin_lock_irqsave(rx-lock, flags);
+
list_for_each_entry_safe(r, r_next, rx-free_list, list) {
list_del(r-list);
free_rx_struct(r);
@@ -254,6 +266,8 @@ static void release_usb(struct usbwm_dev *udev)
list_del(r-list);
free_rx_struct(r);
}
+
+   spin_unlock_irqrestore(rx-lock, flags);
 }
 
 static void __gdm_usb_send_complete(struct urb *urb)
@@ -302,7 +316,7 @@ static int gdm_usb_send(void *priv_dev, void *data, int len,
int no_spc = 0, ret;
u8 *pkt = data;
u16 cmd_evt;
-   unsigned long flags;
+   unsigned long flags, flags2;
 
if (!udev-usbdev) {
dev_err(usbdev-dev, %s: No such device\n, __func__);
@@ -371,13 +385,16 @@ static int gdm_usb_send(void *priv_dev, void *data, int 
len,
 
rx = udev-rx;
 
+   spin_lock_irqsave(rx-lock, flags2);
list_for_each_entry(r, rx-used_list, list)
usb_unlink_urb(r-urb);
+   spin_unlock_irqrestore(rx-lock, flags2);
+
udev-bw_switch = 1;
 
-   spin_lock(k_lock);
+   spin_lock_irqsave(k_lock, flags2);
list_add_tail(udev-list, k_list);
-   spin_unlock(k_lock);
+   spin_unlock_irqrestore(k_lock, flags2);
 
wake_up(k_wait);
}
@@ -416,7 +433,7 @@ static void gdm_usb_rcv_complete(struct urb *urb)
struct tx_cxt *tx = udev-tx;
struct usb_tx *t;
u16 cmd_evt;
-   unsigned long flags;
+   unsigned long flags, flags2;
 
 #ifdef CONFIG_WIMAX_GDM72XX_USB_PM
struct usb_device *dev = urb-dev;
@@ -462,9 +479,9 @@ static void gdm_usb_rcv_complete(struct urb *urb

Re: [PATCH] drivers/staging/gdm72xx/gdm_sdio.c: Replace kmalloc+memset for kzalloc

[Ben Chan]

FYI, a similar patch has already been applied: "staging: gdm72xx:
simplify alloc_tx_struct and alloc_rx_struct (commit
129575f2a8958a1e90780b0d5b80702bb45b5aac)"

Thanks,
Ben

On Mon, Sep 17, 2012 at 6:45 AM, Peter Senna Tschudin
 wrote:
> Replace kmalloc+memset for kzalloc and cleanup related code.
>
> To be applied after 47ad3428a1086af425447f763705e06b16ae905d:
> [PATCH 5/9] drivers/staging/gdm72xx/gdm_sdio.c: Remove useless kfree
>
> Signed-off-by: Peter Senna Tschudin 
> ---
>  drivers/staging/gdm72xx/gdm_sdio.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
> b/drivers/staging/gdm72xx/gdm_sdio.c
> index a0621d9..2e1a964 100644
> --- a/drivers/staging/gdm72xx/gdm_sdio.c
> +++ b/drivers/staging/gdm72xx/gdm_sdio.c
> @@ -91,14 +91,12 @@ static void free_tx_struct(struct sdio_tx *t)
>
>  static struct sdio_rx *alloc_rx_struct(struct rx_cxt *rx)
>  {
> -   struct sdio_rx *r = NULL;
> +   struct sdio_rx *r;
>
> -   r = kmalloc(sizeof(*r), GFP_ATOMIC);
> +   r = kzalloc(sizeof(*r), GFP_ATOMIC);
> if (!r)
> return NULL;
>
> -   memset(r, 0, sizeof(*r));
> -
> r->rx_cxt = rx;
>
> return r;
> --
> 1.7.11.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] drivers/staging/gdm72xx/gdm_sdio.c: Replace kmalloc+memset for kzalloc

[Ben Chan]

FYI, a similar patch has already been applied: staging: gdm72xx:
simplify alloc_tx_struct and alloc_rx_struct (commit
129575f2a8958a1e90780b0d5b80702bb45b5aac)

Thanks,
Ben

On Mon, Sep 17, 2012 at 6:45 AM, Peter Senna Tschudin
peter.se...@gmail.com wrote:
 Replace kmalloc+memset for kzalloc and cleanup related code.

 To be applied after 47ad3428a1086af425447f763705e06b16ae905d:
 [PATCH 5/9] drivers/staging/gdm72xx/gdm_sdio.c: Remove useless kfree

 Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
 ---
  drivers/staging/gdm72xx/gdm_sdio.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

 diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
 b/drivers/staging/gdm72xx/gdm_sdio.c
 index a0621d9..2e1a964 100644
 --- a/drivers/staging/gdm72xx/gdm_sdio.c
 +++ b/drivers/staging/gdm72xx/gdm_sdio.c
 @@ -91,14 +91,12 @@ static void free_tx_struct(struct sdio_tx *t)

  static struct sdio_rx *alloc_rx_struct(struct rx_cxt *rx)
  {
 -   struct sdio_rx *r = NULL;
 +   struct sdio_rx *r;

 -   r = kmalloc(sizeof(*r), GFP_ATOMIC);
 +   r = kzalloc(sizeof(*r), GFP_ATOMIC);
 if (!r)
 return NULL;

 -   memset(r, 0, sizeof(*r));
 -
 r-rx_cxt = rx;

 return r;
 --
 1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: simplify alloc_tx_struct and alloc_rx_struct

[Ben Chan]

This patch simplifies alloc_tx_struct and alloc_rx_struct in gdm_sdio.c
and gdm_usb.c by replacing kmalloc+memset with kzalloc and reorganizing
the code.

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_sdio.c |   30 
 drivers/staging/gdm72xx/gdm_usb.c  |   44 ++--
 2 files changed, 27 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index a0621d9..ca38d71 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -60,25 +60,20 @@ static void hexdump(char *title, u8 *data, int len)
 
 static struct sdio_tx *alloc_tx_struct(struct tx_cxt *tx)
 {
-   struct sdio_tx *t = NULL;
+   struct sdio_tx *t = kzalloc(sizeof(*t), GFP_ATOMIC);
 
-   t = kzalloc(sizeof(*t), GFP_ATOMIC);
-   if (t == NULL)
-   goto out;
+   if (!t)
+   return NULL;
 
t->buf = kmalloc(TX_BUF_SIZE, GFP_ATOMIC);
-   if (t->buf == NULL)
-   goto out;
+   if (!t->buf) {
+   kfree(t);
+   return NULL;
+   }
 
t->tx_cxt = tx;
 
return t;
-out:
-   if (t) {
-   kfree(t->buf);
-   kfree(t);
-   }
-   return NULL;
 }
 
 static void free_tx_struct(struct sdio_tx *t)
@@ -91,15 +86,10 @@ static void free_tx_struct(struct sdio_tx *t)
 
 static struct sdio_rx *alloc_rx_struct(struct rx_cxt *rx)
 {
-   struct sdio_rx *r = NULL;
-
-   r = kmalloc(sizeof(*r), GFP_ATOMIC);
-   if (!r)
-   return NULL;
-
-   memset(r, 0, sizeof(*r));
+   struct sdio_rx *r = kzalloc(sizeof(*r), GFP_ATOMIC);
 
-   r->rx_cxt = rx;
+   if (r)
+   r->rx_cxt = rx;
 
return r;
 }
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 6d306f7..0c9e895 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -73,27 +73,23 @@ static void hexdump(char *title, u8 *data, int len)
 
 static struct usb_tx *alloc_tx_struct(struct tx_cxt *tx)
 {
-   struct usb_tx *t = NULL;
+   struct usb_tx *t = kzalloc(sizeof(*t), GFP_ATOMIC);
 
-   t = kzalloc(sizeof(*t), GFP_ATOMIC);
-   if (t == NULL)
-   goto out;
+   if (!t)
+   return NULL;
 
t->urb = usb_alloc_urb(0, GFP_ATOMIC);
t->buf = kmalloc(TX_BUF_SIZE, GFP_ATOMIC);
-   if (t->urb == NULL || t->buf == NULL)
-   goto out;
-
-   t->tx_cxt = tx;
-
-   return t;
-out:
-   if (t) {
+   if (!t->urb || !t->buf) {
usb_free_urb(t->urb);
kfree(t->buf);
kfree(t);
+   return NULL;
}
-   return NULL;
+
+   t->tx_cxt = tx;
+
+   return t;
 }
 
 static void free_tx_struct(struct usb_tx *t)
@@ -107,28 +103,22 @@ static void free_tx_struct(struct usb_tx *t)
 
 static struct usb_rx *alloc_rx_struct(struct rx_cxt *rx)
 {
-   struct usb_rx *r = NULL;
+   struct usb_rx *r = kzalloc(sizeof(*r), GFP_ATOMIC);
 
-   r = kmalloc(sizeof(*r), GFP_ATOMIC);
-   if (r == NULL)
-   goto out;
-
-   memset(r, 0, sizeof(*r));
+   if (!r)
+   return NULL;
 
r->urb = usb_alloc_urb(0, GFP_ATOMIC);
r->buf = kmalloc(RX_BUF_SIZE, GFP_ATOMIC);
-   if (r->urb == NULL || r->buf == NULL)
-   goto out;
-
-   r->rx_cxt = rx;
-   return r;
-out:
-   if (r) {
+   if (!r->urb || !r->buf) {
usb_free_urb(r->urb);
kfree(r->buf);
kfree(r);
+   return NULL;
}
-   return NULL;
+
+   r->rx_cxt = rx;
+   return r;
 }
 
 static void free_rx_struct(struct usb_rx *r)
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: simplify alloc_tx_struct and alloc_rx_struct

[Ben Chan]

This patch simplifies alloc_tx_struct and alloc_rx_struct in gdm_sdio.c
and gdm_usb.c by replacing kmalloc+memset with kzalloc and reorganizing
the code.

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_sdio.c |   30 
 drivers/staging/gdm72xx/gdm_usb.c  |   44 ++--
 2 files changed, 27 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c 
b/drivers/staging/gdm72xx/gdm_sdio.c
index a0621d9..ca38d71 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -60,25 +60,20 @@ static void hexdump(char *title, u8 *data, int len)
 
 static struct sdio_tx *alloc_tx_struct(struct tx_cxt *tx)
 {
-   struct sdio_tx *t = NULL;
+   struct sdio_tx *t = kzalloc(sizeof(*t), GFP_ATOMIC);
 
-   t = kzalloc(sizeof(*t), GFP_ATOMIC);
-   if (t == NULL)
-   goto out;
+   if (!t)
+   return NULL;
 
t-buf = kmalloc(TX_BUF_SIZE, GFP_ATOMIC);
-   if (t-buf == NULL)
-   goto out;
+   if (!t-buf) {
+   kfree(t);
+   return NULL;
+   }
 
t-tx_cxt = tx;
 
return t;
-out:
-   if (t) {
-   kfree(t-buf);
-   kfree(t);
-   }
-   return NULL;
 }
 
 static void free_tx_struct(struct sdio_tx *t)
@@ -91,15 +86,10 @@ static void free_tx_struct(struct sdio_tx *t)
 
 static struct sdio_rx *alloc_rx_struct(struct rx_cxt *rx)
 {
-   struct sdio_rx *r = NULL;
-
-   r = kmalloc(sizeof(*r), GFP_ATOMIC);
-   if (!r)
-   return NULL;
-
-   memset(r, 0, sizeof(*r));
+   struct sdio_rx *r = kzalloc(sizeof(*r), GFP_ATOMIC);
 
-   r-rx_cxt = rx;
+   if (r)
+   r-rx_cxt = rx;
 
return r;
 }
diff --git a/drivers/staging/gdm72xx/gdm_usb.c 
b/drivers/staging/gdm72xx/gdm_usb.c
index 6d306f7..0c9e895 100644
--- a/drivers/staging/gdm72xx/gdm_usb.c
+++ b/drivers/staging/gdm72xx/gdm_usb.c
@@ -73,27 +73,23 @@ static void hexdump(char *title, u8 *data, int len)
 
 static struct usb_tx *alloc_tx_struct(struct tx_cxt *tx)
 {
-   struct usb_tx *t = NULL;
+   struct usb_tx *t = kzalloc(sizeof(*t), GFP_ATOMIC);
 
-   t = kzalloc(sizeof(*t), GFP_ATOMIC);
-   if (t == NULL)
-   goto out;
+   if (!t)
+   return NULL;
 
t-urb = usb_alloc_urb(0, GFP_ATOMIC);
t-buf = kmalloc(TX_BUF_SIZE, GFP_ATOMIC);
-   if (t-urb == NULL || t-buf == NULL)
-   goto out;
-
-   t-tx_cxt = tx;
-
-   return t;
-out:
-   if (t) {
+   if (!t-urb || !t-buf) {
usb_free_urb(t-urb);
kfree(t-buf);
kfree(t);
+   return NULL;
}
-   return NULL;
+
+   t-tx_cxt = tx;
+
+   return t;
 }
 
 static void free_tx_struct(struct usb_tx *t)
@@ -107,28 +103,22 @@ static void free_tx_struct(struct usb_tx *t)
 
 static struct usb_rx *alloc_rx_struct(struct rx_cxt *rx)
 {
-   struct usb_rx *r = NULL;
+   struct usb_rx *r = kzalloc(sizeof(*r), GFP_ATOMIC);
 
-   r = kmalloc(sizeof(*r), GFP_ATOMIC);
-   if (r == NULL)
-   goto out;
-
-   memset(r, 0, sizeof(*r));
+   if (!r)
+   return NULL;
 
r-urb = usb_alloc_urb(0, GFP_ATOMIC);
r-buf = kmalloc(RX_BUF_SIZE, GFP_ATOMIC);
-   if (r-urb == NULL || r-buf == NULL)
-   goto out;
-
-   r-rx_cxt = rx;
-   return r;
-out:
-   if (r) {
+   if (!r-urb || !r-buf) {
usb_free_urb(r-urb);
kfree(r-buf);
kfree(r);
+   return NULL;
}
-   return NULL;
+
+   r-rx_cxt = rx;
+   return r;
 }
 
 static void free_rx_struct(struct usb_rx *r)
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

Hi Dan,

I manually walked through the driver code and spotted the issue. But
this morning I was able to get an extra module to verify my patch on
hardware.

I tested the following patterns using two identical modules, and
checked the creation/destruction/ref_cnt of wm_event:
- insert module A, remove A
- insert A, insert B, remove A, remove B
- insert A, insert B, remove B, remove A
- insert A, insert B, remove A, remove B
- insert A, insert B, remove B, insert B, remove B, remove A

Thanks,
Ben

On Thu, Aug 9, 2012 at 11:28 PM, Dan Carpenter  wrote:
> Ben, I'm confused.  Do you have a way to test this, or are you just
> doing manual review?
>
> regards,
> dan carpenter
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

Hi Dan,

I manually walked through the driver code and spotted the issue. But
this morning I was able to get an extra module to verify my patch on
hardware.

I tested the following patterns using two identical modules, and
checked the creation/destruction/ref_cnt of wm_event:
- insert module A, remove A
- insert A, insert B, remove A, remove B
- insert A, insert B, remove B, remove A
- insert A, insert B, remove A, remove B
- insert A, insert B, remove B, insert B, remove B, remove A

Thanks,
Ben

On Thu, Aug 9, 2012 at 11:28 PM, Dan Carpenter dan.carpen...@oracle.com wrote:
 Ben, I'm confused.  Do you have a way to test this, or are you just
 doing manual review?

 regards,
 dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

Hi,

Does patch v2 make sense?

Thanks,
Ben

On Wed, Jul 25, 2012 at 6:53 AM, Ben Chan  wrote:
> Hi Devendra,
>
> Thanks for cleaning up the driver.  If I understand the code
> correctly, the original author wanted to initialize wm_event once and
> reuse it for multiple devices, and thus reference counted it with
> ref_cnt.
>
> For instance, each time gdm_usb_probe() is called, it may call
> register_wimax_device() / gdm_wimax_event_init(). wm_event is
> initialized the first time when wm_event.ref_cnt == 0 (alternatively,
> the code could check !wm_event.sock). Subsequent calls to
> gdm_wimax_event_init() will simply increase the ref count. Similarly,
> gdm_usb_disconnect() calls unregister_wimax_device() /
> gdm_wimax_event_exit(), which decreases the ref count and disposes
> wm_event when ref_cnt becomes zero.
>
> The code change in commit 8df858ea76b76dde9a39d4edd9aaded983582cfe
> only prevents ref_cnt from increasing beyond one. So the code no
> longer work when there are multiple devices (i.e. wm_event could be
> disposed even when there is an active device).
>
> Thanks,
> Ben
>
>
> On Tue, Jul 24, 2012 at 9:50 PM, devendra.aaru  
> wrote:
>> On Tue, Jul 24, 2012 at 8:34 PM, Ben Chan  wrote:
>>> This patch fixes the commit "staging/gdm72xx: cleanup little at
>>> gdm_wimax_event_rcv" (8df858ea76b76dde9a39d4edd9aaded983582cfe),
>>> which mishandles the reference counting of wm_event.
>>>
>>> Signed-off-by: Ben Chan 
>>> ---
>>> Fixed the commit message as suggested by Dan Carpenter.
>>>
>>>  drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
>>>  1 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
>>> b/drivers/staging/gdm72xx/gdm_wimax.c
>>> index 0716efc..6cb8107 100644
>>> --- a/drivers/staging/gdm72xx/gdm_wimax.c
>>> +++ b/drivers/staging/gdm72xx/gdm_wimax.c
>>> @@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
>>> if (!wm_event.ref_cnt) {
>>> wm_event.sock = netlink_init(NETLINK_WIMAX,
>>> gdm_wimax_event_rcv);
>>> -   if (wm_event.sock)
>>> -   wm_event.ref_cnt++;
>>> -   INIT_LIST_HEAD(_event.evtq);
>>> -   INIT_LIST_HEAD(_event.freeq);
>>> -   INIT_WORK(_event.ws, __gdm_wimax_event_send);
>>> -   spin_lock_init(_event.evt_lock);
>>> +   if (wm_event.sock) {
>>> +   INIT_LIST_HEAD(_event.evtq);
>>> +   INIT_LIST_HEAD(_event.freeq);
>>> +   INIT_WORK(_event.ws, __gdm_wimax_event_send);
>>> +   spin_lock_init(_event.evt_lock);
>>> +   }
>>> +   }
>>> +
>>> +   if (wm_event.sock) {
>>> +   wm_event.ref_cnt++;
>>> return 0;
>>> }
>>>
>>> --
>>> 1.7.7.3
>>>
>>
>> Hi Ben,
>>
>> I have some basic understanding about this flow of the function,
>>
>> Here is my understanding of the thing i have done when i was doing
>> some cleanups to this driver.
>>
>> The ref_cnt will be 0 at first time. if so the sock creation happens
>> only once, and register_wimax_device only calls this function.
>> so i think doing the ref_cnt++ inside the if (!wm_event.ref_cnt) is valid.
>>
>> Please suggest me if i am wrong.
>>
>> Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

Hi,

Does patch v2 make sense?

Thanks,
Ben

On Wed, Jul 25, 2012 at 6:53 AM, Ben Chan benc...@chromium.org wrote:
 Hi Devendra,

 Thanks for cleaning up the driver.  If I understand the code
 correctly, the original author wanted to initialize wm_event once and
 reuse it for multiple devices, and thus reference counted it with
 ref_cnt.

 For instance, each time gdm_usb_probe() is called, it may call
 register_wimax_device() / gdm_wimax_event_init(). wm_event is
 initialized the first time when wm_event.ref_cnt == 0 (alternatively,
 the code could check !wm_event.sock). Subsequent calls to
 gdm_wimax_event_init() will simply increase the ref count. Similarly,
 gdm_usb_disconnect() calls unregister_wimax_device() /
 gdm_wimax_event_exit(), which decreases the ref count and disposes
 wm_event when ref_cnt becomes zero.

 The code change in commit 8df858ea76b76dde9a39d4edd9aaded983582cfe
 only prevents ref_cnt from increasing beyond one. So the code no
 longer work when there are multiple devices (i.e. wm_event could be
 disposed even when there is an active device).

 Thanks,
 Ben


 On Tue, Jul 24, 2012 at 9:50 PM, devendra.aaru devendra.a...@gmail.com 
 wrote:
 On Tue, Jul 24, 2012 at 8:34 PM, Ben Chan benc...@chromium.org wrote:
 This patch fixes the commit staging/gdm72xx: cleanup little at
 gdm_wimax_event_rcv (8df858ea76b76dde9a39d4edd9aaded983582cfe),
 which mishandles the reference counting of wm_event.

 Signed-off-by: Ben Chan benc...@chromium.org
 ---
 Fixed the commit message as suggested by Dan Carpenter.

  drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
  1 files changed, 10 insertions(+), 6 deletions(-)

 diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
 b/drivers/staging/gdm72xx/gdm_wimax.c
 index 0716efc..6cb8107 100644
 --- a/drivers/staging/gdm72xx/gdm_wimax.c
 +++ b/drivers/staging/gdm72xx/gdm_wimax.c
 @@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
 if (!wm_event.ref_cnt) {
 wm_event.sock = netlink_init(NETLINK_WIMAX,
 gdm_wimax_event_rcv);
 -   if (wm_event.sock)
 -   wm_event.ref_cnt++;
 -   INIT_LIST_HEAD(wm_event.evtq);
 -   INIT_LIST_HEAD(wm_event.freeq);
 -   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
 -   spin_lock_init(wm_event.evt_lock);
 +   if (wm_event.sock) {
 +   INIT_LIST_HEAD(wm_event.evtq);
 +   INIT_LIST_HEAD(wm_event.freeq);
 +   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
 +   spin_lock_init(wm_event.evt_lock);
 +   }
 +   }
 +
 +   if (wm_event.sock) {
 +   wm_event.ref_cnt++;
 return 0;
 }

 --
 1.7.7.3


 Hi Ben,

 I have some basic understanding about this flow of the function,

 Here is my understanding of the thing i have done when i was doing
 some cleanups to this driver.

 The ref_cnt will be 0 at first time. if so the sock creation happens
 only once, and register_wimax_device only calls this function.
 so i think doing the ref_cnt++ inside the if (!wm_event.ref_cnt) is valid.

 Please suggest me if i am wrong.

 Thanks,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

Hi Devendra,

Thanks for cleaning up the driver.  If I understand the code
correctly, the original author wanted to initialize wm_event once and
reuse it for multiple devices, and thus reference counted it with
ref_cnt.

For instance, each time gdm_usb_probe() is called, it may call
register_wimax_device() / gdm_wimax_event_init(). wm_event is
initialized the first time when wm_event.ref_cnt == 0 (alternatively,
the code could check !wm_event.sock). Subsequent calls to
gdm_wimax_event_init() will simply increase the ref count. Similarly,
gdm_usb_disconnect() calls unregister_wimax_device() /
gdm_wimax_event_exit(), which decreases the ref count and disposes
wm_event when ref_cnt becomes zero.

The code change in commit 8df858ea76b76dde9a39d4edd9aaded983582cfe
only prevents ref_cnt from increasing beyond one. So the code no
longer work when there are multiple devices (i.e. wm_event could be
disposed even when there is an active device).

Thanks,
Ben


On Tue, Jul 24, 2012 at 9:50 PM, devendra.aaru  wrote:
> On Tue, Jul 24, 2012 at 8:34 PM, Ben Chan  wrote:
>> This patch fixes the commit "staging/gdm72xx: cleanup little at
>> gdm_wimax_event_rcv" (8df858ea76b76dde9a39d4edd9aaded983582cfe),
>> which mishandles the reference counting of wm_event.
>>
>> Signed-off-by: Ben Chan 
>> ---
>> Fixed the commit message as suggested by Dan Carpenter.
>>
>>  drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
>>  1 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
>> b/drivers/staging/gdm72xx/gdm_wimax.c
>> index 0716efc..6cb8107 100644
>> --- a/drivers/staging/gdm72xx/gdm_wimax.c
>> +++ b/drivers/staging/gdm72xx/gdm_wimax.c
>> @@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
>> if (!wm_event.ref_cnt) {
>> wm_event.sock = netlink_init(NETLINK_WIMAX,
>> gdm_wimax_event_rcv);
>> -   if (wm_event.sock)
>> -   wm_event.ref_cnt++;
>> -   INIT_LIST_HEAD(_event.evtq);
>> -   INIT_LIST_HEAD(_event.freeq);
>> -   INIT_WORK(_event.ws, __gdm_wimax_event_send);
>> -   spin_lock_init(_event.evt_lock);
>> +   if (wm_event.sock) {
>> +   INIT_LIST_HEAD(_event.evtq);
>> +   INIT_LIST_HEAD(_event.freeq);
>> +   INIT_WORK(_event.ws, __gdm_wimax_event_send);
>> +   spin_lock_init(_event.evt_lock);
>> +   }
>> +   }
>> +
>> +   if (wm_event.sock) {
>> +   wm_event.ref_cnt++;
>> return 0;
>> }
>>
>> --
>> 1.7.7.3
>>
>
> Hi Ben,
>
> I have some basic understanding about this flow of the function,
>
> Here is my understanding of the thing i have done when i was doing
> some cleanups to this driver.
>
> The ref_cnt will be 0 at first time. if so the sock creation happens
> only once, and register_wimax_device only calls this function.
> so i think doing the ref_cnt++ inside the if (!wm_event.ref_cnt) is valid.
>
> Please suggest me if i am wrong.
>
> Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

Hi Devendra,

Thanks for cleaning up the driver.  If I understand the code
correctly, the original author wanted to initialize wm_event once and
reuse it for multiple devices, and thus reference counted it with
ref_cnt.

For instance, each time gdm_usb_probe() is called, it may call
register_wimax_device() / gdm_wimax_event_init(). wm_event is
initialized the first time when wm_event.ref_cnt == 0 (alternatively,
the code could check !wm_event.sock). Subsequent calls to
gdm_wimax_event_init() will simply increase the ref count. Similarly,
gdm_usb_disconnect() calls unregister_wimax_device() /
gdm_wimax_event_exit(), which decreases the ref count and disposes
wm_event when ref_cnt becomes zero.

The code change in commit 8df858ea76b76dde9a39d4edd9aaded983582cfe
only prevents ref_cnt from increasing beyond one. So the code no
longer work when there are multiple devices (i.e. wm_event could be
disposed even when there is an active device).

Thanks,
Ben


On Tue, Jul 24, 2012 at 9:50 PM, devendra.aaru devendra.a...@gmail.com wrote:
 On Tue, Jul 24, 2012 at 8:34 PM, Ben Chan benc...@chromium.org wrote:
 This patch fixes the commit staging/gdm72xx: cleanup little at
 gdm_wimax_event_rcv (8df858ea76b76dde9a39d4edd9aaded983582cfe),
 which mishandles the reference counting of wm_event.

 Signed-off-by: Ben Chan benc...@chromium.org
 ---
 Fixed the commit message as suggested by Dan Carpenter.

  drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
  1 files changed, 10 insertions(+), 6 deletions(-)

 diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
 b/drivers/staging/gdm72xx/gdm_wimax.c
 index 0716efc..6cb8107 100644
 --- a/drivers/staging/gdm72xx/gdm_wimax.c
 +++ b/drivers/staging/gdm72xx/gdm_wimax.c
 @@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
 if (!wm_event.ref_cnt) {
 wm_event.sock = netlink_init(NETLINK_WIMAX,
 gdm_wimax_event_rcv);
 -   if (wm_event.sock)
 -   wm_event.ref_cnt++;
 -   INIT_LIST_HEAD(wm_event.evtq);
 -   INIT_LIST_HEAD(wm_event.freeq);
 -   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
 -   spin_lock_init(wm_event.evt_lock);
 +   if (wm_event.sock) {
 +   INIT_LIST_HEAD(wm_event.evtq);
 +   INIT_LIST_HEAD(wm_event.freeq);
 +   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
 +   spin_lock_init(wm_event.evt_lock);
 +   }
 +   }
 +
 +   if (wm_event.sock) {
 +   wm_event.ref_cnt++;
 return 0;
 }

 --
 1.7.7.3


 Hi Ben,

 I have some basic understanding about this flow of the function,

 Here is my understanding of the thing i have done when i was doing
 some cleanups to this driver.

 The ref_cnt will be 0 at first time. if so the sock creation happens
 only once, and register_wimax_device only calls this function.
 so i think doing the ref_cnt++ inside the if (!wm_event.ref_cnt) is valid.

 Please suggest me if i am wrong.

 Thanks,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

This patch fixes the commit "staging/gdm72xx: cleanup little at
gdm_wimax_event_rcv" (8df858ea76b76dde9a39d4edd9aaded983582cfe),
which mishandles the reference counting of wm_event.

Signed-off-by: Ben Chan 
---
Fixed the commit message as suggested by Dan Carpenter.

 drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index 0716efc..6cb8107 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
if (!wm_event.ref_cnt) {
wm_event.sock = netlink_init(NETLINK_WIMAX,
gdm_wimax_event_rcv);
-   if (wm_event.sock)
-   wm_event.ref_cnt++;
-   INIT_LIST_HEAD(_event.evtq);
-   INIT_LIST_HEAD(_event.freeq);
-   INIT_WORK(_event.ws, __gdm_wimax_event_send);
-   spin_lock_init(_event.evt_lock);
+   if (wm_event.sock) {
+   INIT_LIST_HEAD(_event.evtq);
+   INIT_LIST_HEAD(_event.freeq);
+   INIT_WORK(_event.ws, __gdm_wimax_event_send);
+   spin_lock_init(_event.evt_lock);
+   }
+   }
+
+   if (wm_event.sock) {
+   wm_event.ref_cnt++;
return 0;
}
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

This patch fixes the commit staging/gdm72xx: cleanup little at
gdm_wimax_event_rcv (8df858ea76b76dde9a39d4edd9aaded983582cfe),
which mishandles the reference counting of wm_event.

Signed-off-by: Ben Chan benc...@chromium.org
---
Fixed the commit message as suggested by Dan Carpenter.

 drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index 0716efc..6cb8107 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
if (!wm_event.ref_cnt) {
wm_event.sock = netlink_init(NETLINK_WIMAX,
gdm_wimax_event_rcv);
-   if (wm_event.sock)
-   wm_event.ref_cnt++;
-   INIT_LIST_HEAD(wm_event.evtq);
-   INIT_LIST_HEAD(wm_event.freeq);
-   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
-   spin_lock_init(wm_event.evt_lock);
+   if (wm_event.sock) {
+   INIT_LIST_HEAD(wm_event.evtq);
+   INIT_LIST_HEAD(wm_event.freeq);
+   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
+   spin_lock_init(wm_event.evt_lock);
+   }
+   }
+
+   if (wm_event.sock) {
+   wm_event.ref_cnt++;
return 0;
}
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

This patch fixes commit 8df858ea76b76dde9a39d4edd9aaded983582cfe, which
mishandles the reference counting of wm_event.

Signed-off-by: Ben Chan 
---
 drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index 0716efc..6cb8107 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
if (!wm_event.ref_cnt) {
wm_event.sock = netlink_init(NETLINK_WIMAX,
gdm_wimax_event_rcv);
-   if (wm_event.sock)
-   wm_event.ref_cnt++;
-   INIT_LIST_HEAD(_event.evtq);
-   INIT_LIST_HEAD(_event.freeq);
-   INIT_WORK(_event.ws, __gdm_wimax_event_send);
-   spin_lock_init(_event.evt_lock);
+   if (wm_event.sock) {
+   INIT_LIST_HEAD(_event.evtq);
+   INIT_LIST_HEAD(_event.freeq);
+   INIT_WORK(_event.ws, __gdm_wimax_event_send);
+   spin_lock_init(_event.evt_lock);
+   }
+   }
+
+   if (wm_event.sock) {
+   wm_event.ref_cnt++;
return 0;
}
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] staging: gdm72xx: fix reference counting in gdm_wimax_event_init

[Ben Chan]

This patch fixes commit 8df858ea76b76dde9a39d4edd9aaded983582cfe, which
mishandles the reference counting of wm_event.

Signed-off-by: Ben Chan benc...@chromium.org
---
 drivers/staging/gdm72xx/gdm_wimax.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_wimax.c 
b/drivers/staging/gdm72xx/gdm_wimax.c
index 0716efc..6cb8107 100644
--- a/drivers/staging/gdm72xx/gdm_wimax.c
+++ b/drivers/staging/gdm72xx/gdm_wimax.c
@@ -258,12 +258,16 @@ static int gdm_wimax_event_init(void)
if (!wm_event.ref_cnt) {
wm_event.sock = netlink_init(NETLINK_WIMAX,
gdm_wimax_event_rcv);
-   if (wm_event.sock)
-   wm_event.ref_cnt++;
-   INIT_LIST_HEAD(wm_event.evtq);
-   INIT_LIST_HEAD(wm_event.freeq);
-   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
-   spin_lock_init(wm_event.evt_lock);
+   if (wm_event.sock) {
+   INIT_LIST_HEAD(wm_event.evtq);
+   INIT_LIST_HEAD(wm_event.freeq);
+   INIT_WORK(wm_event.ws, __gdm_wimax_event_send);
+   spin_lock_init(wm_event.evt_lock);
+   }
+   }
+
+   if (wm_event.sock) {
+   wm_event.ref_cnt++;
return 0;
}
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/