Re: [PATCH 3/3] staging/ozwpan: coding style ether_addr_copy

2014-03-13 Thread Jérôme Pinot 
On 03/13/14 02:28, Greg Kroah-Hartman wrote:
 On Thu, Mar 13, 2014 at 10:21:44AM +0900, Jérôme Pinot wrote:
[...]
  diff --git a/drivers/staging/ozwpan/ozcdev.c 
  b/drivers/staging/ozwpan/ozcdev.c
  index 5de5981..10c0a96 100644
  --- a/drivers/staging/ozwpan/ozcdev.c
  +++ b/drivers/staging/ozwpan/ozcdev.c
  @@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
  pd = oz_pd_find(addr);
  if (pd) {
  spin_lock_bh(g_cdev.lock);
  -   memcpy(g_cdev.active_addr, addr, ETH_ALEN);
  +   ether_addr_copy(g_cdev.active_addr, addr);
 
 Are you sure this will work?

No. But the ozwpan driver uses already ether_addr_equal which is not
alignment-safe. As
https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt 
said:

This alignment-unsafe function is still useful as it is a decent
optimization for the cases when you can ensure alignment, which is
true almost all of the time in ethernet networking context.

I expected the maintainer to confirm/infirm this. I'm just seeing that's
actually Chris Kelly who did write this part of code, so I'm CC'ing him
too.

 You have to check the alignment of the variable.
 
 Also, this breaks the build, you need to include some kind of .h file to
 get this to work, please ALWAYS test your patches to make sure they
 don't break things.

My bad, sorry. I'll send a better patch. Thanks for your time.

 greg k-h

-- 
Jérôme Pinot
http://ngc891.blogdns.net/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging/ozwpan: coding style ether_addr_copy

2014-03-13 Thread Jérôme Pinot 
This fixes the following issues detected by checkpatch.pl:

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #220: FILE: drivers/staging/ozwpan/ozcdev.c:220:
 +  memcpy(g_cdev.active_addr, addr, ETH_ALEN);

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #286: FILE: drivers/staging/ozwpan/ozcdev.c:286:
 +  memcpy(addr, g_cdev.active_addr, ETH_ALEN);

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #176: FILE: drivers/staging/ozwpan/ozpd.c:176:
 +  memcpy(pd-mac_addr, mac_addr, ETH_ALEN);

Signed-off-by: Jerome Pinot ngc...@gmail.com
---
 drivers/staging/ozwpan/ozcdev.c | 4 ++--
 drivers/staging/ozwpan/ozpd.c   | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index 5de5981..10c0a96 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
pd = oz_pd_find(addr);
if (pd) {
spin_lock_bh(g_cdev.lock);
-   memcpy(g_cdev.active_addr, addr, ETH_ALEN);
+   ether_addr_copy(g_cdev.active_addr, addr);
old_pd = g_cdev.active_pd;
g_cdev.active_pd = pd;
spin_unlock_bh(g_cdev.lock);
@@ -283,7 +283,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int 
cmd,
u8 addr[ETH_ALEN];
oz_dbg(ON, OZ_IOCTL_GET_ACTIVE_PD\n);
spin_lock_bh(g_cdev.lock);
-   memcpy(addr, g_cdev.active_addr, ETH_ALEN);
+   ether_addr_copy(addr, g_cdev.active_addr);
spin_unlock_bh(g_cdev.lock);
if (copy_to_user((void __user *)arg, addr, ETH_ALEN))
return -EFAULT;
diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c
index 29a23a3..10f1b3a 100644
--- a/drivers/staging/ozwpan/ozpd.c
+++ b/drivers/staging/ozwpan/ozpd.c
@@ -8,6 +8,7 @@
 #include linux/timer.h
 #include linux/sched.h
 #include linux/netdevice.h
+#include linux/etherdevice.h
 #include linux/errno.h
 #include ozdbg.h
 #include ozprotocol.h
@@ -173,7 +174,7 @@ struct oz_pd *oz_pd_alloc(const u8 *mac_addr)
pd-last_rx_pkt_num = 0x;
oz_pd_set_state(pd, OZ_PD_S_IDLE);
pd-max_tx_size = OZ_MAX_TX_SIZE;
-   memcpy(pd-mac_addr, mac_addr, ETH_ALEN);
+   ether_addr_copy(pd-mac_addr, mac_addr);
if (0 != oz_elt_buf_init(pd-elt_buff)) {
kfree(pd);
pd = NULL;
-- 
Jérôme Pinot
http://ngc891.blogdns.net/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging/ozwpan: coding style

2014-03-12 Thread Jérôme Pinot 
Hi,

This commit fixes warnings reported by checkpatch.pl

Signed-off-by: Jerome Pinot ngc...@gmail.com
---
 drivers/staging/ozwpan/ozcdev.c  | 4 ++--
 drivers/staging/ozwpan/ozhcd.c   | 8 
 drivers/staging/ozwpan/ozpd.c| 2 +-
 drivers/staging/ozwpan/ozpd.h| 2 +-
 drivers/staging/ozwpan/ozproto.c | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index 5de5981..10c0a96 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
pd = oz_pd_find(addr);
if (pd) {
spin_lock_bh(g_cdev.lock);
-   memcpy(g_cdev.active_addr, addr, ETH_ALEN);
+   ether_addr_copy(g_cdev.active_addr, addr);
old_pd = g_cdev.active_pd;
g_cdev.active_pd = pd;
spin_unlock_bh(g_cdev.lock);
@@ -283,7 +283,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int 
cmd,
u8 addr[ETH_ALEN];
oz_dbg(ON, OZ_IOCTL_GET_ACTIVE_PD\n);
spin_lock_bh(g_cdev.lock);
-   memcpy(addr, g_cdev.active_addr, ETH_ALEN);
+   ether_addr_copy(addr, g_cdev.active_addr);
spin_unlock_bh(g_cdev.lock);
if (copy_to_user((void __user *)arg, addr, ETH_ALEN))
return -EFAULT;
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index efaf26f..ec61973 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -354,7 +354,8 @@ static struct oz_endpoint *oz_ep_alloc(int buffer_size, 
gfp_t mem_flags)
  * disabled.
  * Context: softirq or process
  */
-static struct oz_urb_link *oz_uncancel_urb(struct oz_hcd *ozhcd, struct urb 
*urb)
+static struct oz_urb_link *oz_uncancel_urb(struct oz_hcd *ozhcd,
+   struct urb *urb)
 {
struct oz_urb_link *urbl;
struct list_head *e;
@@ -1986,8 +1987,7 @@ static void oz_get_hub_descriptor(struct usb_hcd *hcd,
memset(desc, 0, sizeof(*desc));
desc-bDescriptorType = 0x29;
desc-bDescLength = 9;
-   desc-wHubCharacteristics = (__force __u16)
-   __constant_cpu_to_le16(0x0001);
+   desc-wHubCharacteristics = (__force __u16)cpu_to_le16(0x0001);
desc-bNbrPorts = OZ_NB_PORTS;
 }
 
@@ -2181,7 +2181,7 @@ static int oz_hcd_hub_control(struct usb_hcd *hcd, u16 
req_type, u16 wvalue,
break;
case GetHubStatus:
oz_dbg(HUB, GetHubStatus: req_type = 0x%x\n, req_type);
-   put_unaligned(__constant_cpu_to_le32(0), (__le32 *)buf);
+   put_unaligned(cpu_to_le32(0), (__le32 *)buf);
break;
case GetPortStatus:
err = oz_get_port_status(hcd, windex, buf);
diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c
index 29a23a3..4740238 100644
--- a/drivers/staging/ozwpan/ozpd.c
+++ b/drivers/staging/ozwpan/ozpd.c
@@ -173,7 +173,7 @@ struct oz_pd *oz_pd_alloc(const u8 *mac_addr)
pd-last_rx_pkt_num = 0x;
oz_pd_set_state(pd, OZ_PD_S_IDLE);
pd-max_tx_size = OZ_MAX_TX_SIZE;
-   memcpy(pd-mac_addr, mac_addr, ETH_ALEN);
+   ether_addr_copy(pd-mac_addr, mac_addr);
if (0 != oz_elt_buf_init(pd-elt_buff)) {
kfree(pd);
pd = NULL;
diff --git a/drivers/staging/ozwpan/ozpd.h b/drivers/staging/ozwpan/ozpd.h
index 56e6fdf..ad5fe7a 100644
--- a/drivers/staging/ozwpan/ozpd.h
+++ b/drivers/staging/ozwpan/ozpd.h
@@ -22,7 +22,7 @@
 #define OZ_TIMER_HEARTBEAT 2
 #define OZ_TIMER_STOP  3
 
-/* 
+/*
  *External spinlock variable
  */
 extern spinlock_t g_polling_lock;
diff --git a/drivers/staging/ozwpan/ozproto.c b/drivers/staging/ozwpan/ozproto.c
index c1325a6..a7e953d 100644
--- a/drivers/staging/ozwpan/ozproto.c
+++ b/drivers/staging/ozwpan/ozproto.c
@@ -672,7 +672,7 @@ void oz_binding_add(const char *net_dev)
if (!binding)
return;
 
-   binding-ptype.type = __constant_htons(OZ_ETHERTYPE);
+   binding-ptype.type = htons(OZ_ETHERTYPE);
binding-ptype.func = oz_pkt_recv;
if (net_dev  *net_dev) {
memcpy(binding-name, net_dev, OZ_MAX_BINDING_LEN);
@@ -792,7 +792,7 @@ int oz_get_pd_list(struct oz_mac_addr *addr, int max_count)
if (count = max_count)
break;
pd = container_of(e, struct oz_pd, link);
-   memcpy(addr[count++], pd-mac_addr, ETH_ALEN);
+   ether_addr_copy(addr[count++], pd-mac_addr);
}
spin_unlock_bh(g_polling_lock);
return count;
-- 
1.8.4


-- 
Jérôme Pinot
http://ngc891.blogdns.net

[PATCH 1/3] staging/ozwpan: formatting coding style

2014-03-12 Thread Jérôme Pinot 
This fixes the following spacing issues detected by checkpatch.pl:

 WARNING: line over 80 characters
 #357: FILE: drivers/staging/ozwpan/ozhcd.c:357:
 +static struct oz_urb_link *oz_uncancel_urb(struct oz_hcd *ozhcd, struct urb 
*urb)

 ERROR: trailing whitespace
 #25: FILE: drivers/staging/ozwpan/ozpd.h:25:
 +/* $

Signed-off-by: Jerome Pinot ngc...@gmail.com
---
 drivers/staging/ozwpan/ozhcd.c | 3 ++-
 drivers/staging/ozwpan/ozpd.h  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index efaf26f..e7901aa 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -354,7 +354,8 @@ static struct oz_endpoint *oz_ep_alloc(int buffer_size, 
gfp_t mem_flags)
  * disabled.
  * Context: softirq or process
  */
-static struct oz_urb_link *oz_uncancel_urb(struct oz_hcd *ozhcd, struct urb 
*urb)
+static struct oz_urb_link *oz_uncancel_urb(struct oz_hcd *ozhcd,
+   struct urb *urb)
 {
struct oz_urb_link *urbl;
struct list_head *e;
diff --git a/drivers/staging/ozwpan/ozpd.h b/drivers/staging/ozwpan/ozpd.h
index 56e6fdf..ad5fe7a 100644
--- a/drivers/staging/ozwpan/ozpd.h
+++ b/drivers/staging/ozwpan/ozpd.h
@@ -22,7 +22,7 @@
 #define OZ_TIMER_HEARTBEAT 2
 #define OZ_TIMER_STOP  3
 
-/* 
+/*
  *External spinlock variable
  */
 extern spinlock_t g_polling_lock;
-- 
1.8.4

-- 
Jérôme Pinot
http://ngc891.blogdns.net/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging/ozwpan: coding style __constant_

2014-03-12 Thread Jérôme Pinot 
This fixes the following issues detected by checkpatch.pl:

 WARNING: __constant_cpu_to_le16 should be cpu_to_le16
 #1991: FILE: drivers/staging/ozwpan/ozhcd.c:1991:
 +  __constant_cpu_to_le16(0x0001);

 WARNING: __constant_cpu_to_le32 should be cpu_to_le32
 #2185: FILE: drivers/staging/ozwpan/ozhcd.c:2185:
 +  put_unaligned(__constant_cpu_to_le32(0), (__le32 *)buf);

 WARNING: __constant_htons should be htons
 #675: FILE: drivers/staging/ozwpan/ozproto.c:675:
 +  binding-ptype.type = __constant_htons(OZ_ETHERTYPE);

Signed-off-by: Jerome Pinot ngc...@gmail.com
---
 drivers/staging/ozwpan/ozhcd.c   | 5 ++---
 drivers/staging/ozwpan/ozproto.c | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index e7901aa..b3d401a 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -1987,8 +1987,7 @@ static void oz_get_hub_descriptor(struct usb_hcd *hcd,
memset(desc, 0, sizeof(*desc));
desc-bDescriptorType = 0x29;
desc-bDescLength = 9;
-   desc-wHubCharacteristics = (__force __u16)
-   __constant_cpu_to_le16(0x0001);
+   desc-wHubCharacteristics = (__force __u16)cpu_to_le16(0x0001);
desc-bNbrPorts = OZ_NB_PORTS;
 }
 
@@ -2182,7 +2181,7 @@ static int oz_hcd_hub_control(struct usb_hcd *hcd, u16 
req_type, u16 wvalue,
break;
case GetHubStatus:
oz_dbg(HUB, GetHubStatus: req_type = 0x%x\n, req_type);
-   put_unaligned(__constant_cpu_to_le32(0), (__le32 *)buf);
+   put_unaligned(cpu_to_le32(0), (__le32 *)buf);
break;
case GetPortStatus:
err = oz_get_port_status(hcd, windex, buf);
diff --git a/drivers/staging/ozwpan/ozproto.c b/drivers/staging/ozwpan/ozproto.c
index c1325a6..f09acd0 100644
--- a/drivers/staging/ozwpan/ozproto.c
+++ b/drivers/staging/ozwpan/ozproto.c
@@ -672,7 +672,7 @@ void oz_binding_add(const char *net_dev)
if (!binding)
return;
 
-   binding-ptype.type = __constant_htons(OZ_ETHERTYPE);
+   binding-ptype.type = htons(OZ_ETHERTYPE);
binding-ptype.func = oz_pkt_recv;
if (net_dev  *net_dev) {
memcpy(binding-name, net_dev, OZ_MAX_BINDING_LEN);
-- 
1.8.4

-- 
Jérôme Pinot
http://ngc891.blogdns.net/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging/ozwpan: coding style ether_addr_copy

2014-03-12 Thread Jérôme Pinot 
This fixes the following issues detected by checkpatch.pl:

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #220: FILE: drivers/staging/ozwpan/ozcdev.c:220:
 +  memcpy(g_cdev.active_addr, addr, ETH_ALEN);

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #286: FILE: drivers/staging/ozwpan/ozcdev.c:286:
 +  memcpy(addr, g_cdev.active_addr, ETH_ALEN);

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #176: FILE: drivers/staging/ozwpan/ozpd.c:176:
 +  memcpy(pd-mac_addr, mac_addr, ETH_ALEN);

 WARNING: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are 
__aligned(2)
 #795: FILE: drivers/staging/ozwpan/ozproto.c:795:
 +  memcpy(addr[count++], pd-mac_addr, ETH_ALEN);

Signed-off-by: Jerome Pinot ngc...@gmail.com
---
 drivers/staging/ozwpan/ozcdev.c  | 4 ++--
 drivers/staging/ozwpan/ozpd.c| 2 +-
 drivers/staging/ozwpan/ozproto.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index 5de5981..10c0a96 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
pd = oz_pd_find(addr);
if (pd) {
spin_lock_bh(g_cdev.lock);
-   memcpy(g_cdev.active_addr, addr, ETH_ALEN);
+   ether_addr_copy(g_cdev.active_addr, addr);
old_pd = g_cdev.active_pd;
g_cdev.active_pd = pd;
spin_unlock_bh(g_cdev.lock);
@@ -283,7 +283,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int 
cmd,
u8 addr[ETH_ALEN];
oz_dbg(ON, OZ_IOCTL_GET_ACTIVE_PD\n);
spin_lock_bh(g_cdev.lock);
-   memcpy(addr, g_cdev.active_addr, ETH_ALEN);
+   ether_addr_copy(addr, g_cdev.active_addr);
spin_unlock_bh(g_cdev.lock);
if (copy_to_user((void __user *)arg, addr, ETH_ALEN))
return -EFAULT;
diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c
index 29a23a3..4740238 100644
--- a/drivers/staging/ozwpan/ozpd.c
+++ b/drivers/staging/ozwpan/ozpd.c
@@ -173,7 +173,7 @@ struct oz_pd *oz_pd_alloc(const u8 *mac_addr)
pd-last_rx_pkt_num = 0x;
oz_pd_set_state(pd, OZ_PD_S_IDLE);
pd-max_tx_size = OZ_MAX_TX_SIZE;
-   memcpy(pd-mac_addr, mac_addr, ETH_ALEN);
+   ether_addr_copy(pd-mac_addr, mac_addr);
if (0 != oz_elt_buf_init(pd-elt_buff)) {
kfree(pd);
pd = NULL;
diff --git a/drivers/staging/ozwpan/ozproto.c b/drivers/staging/ozwpan/ozproto.c
index f09acd0..a7e953d 100644
--- a/drivers/staging/ozwpan/ozproto.c
+++ b/drivers/staging/ozwpan/ozproto.c
@@ -792,7 +792,7 @@ int oz_get_pd_list(struct oz_mac_addr *addr, int max_count)
if (count = max_count)
break;
pd = container_of(e, struct oz_pd, link);
-   memcpy(addr[count++], pd-mac_addr, ETH_ALEN);
+   ether_addr_copy(addr[count++], pd-mac_addr);
}
spin_unlock_bh(g_polling_lock);
return count;
-- 
1.8.4

-- 
Jérôme Pinot
http://ngc891.blogdns.net/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel