Use put_unaligned_le32 rather than using byte ordering function and
memcpy which makes code clear.
Also, add the header file where it is declared.
Done using Coccinelle and semantic patch used is :
@ rule1 @
identifier tmp; expression ptr,x; type T;
@@
- tmp = cpu_to_le32(x);
<+... when != tmp
- memcpy(ptr, (T)&tmp, ...);
+ put_unaligned_le32(x,ptr);
...+>
@ depends on rule1 @
type j; identifier tmp;
@@
- j tmp;
...when != tmp
Signed-off-by: Himanshu Jha <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 0edc5d6..e28e119 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -17,6 +17,7 @@
* this warranty disclaimer.
*/
+#include <linux/unaligned/access_ok.h>
#include "decl.h"
#include "ioctl.h"
#include "util.h"
@@ -183,7 +184,6 @@ static int mwifiex_dnld_cmd_to_fw(struct mwifiex_private *priv,
uint16_t cmd_code;
uint16_t cmd_size;
unsigned long flags;
- __le32 tmp;
if (!adapter || !cmd_node)
return -1;
@@ -249,9 +249,9 @@ static int mwifiex_dnld_cmd_to_fw(struct mwifiex_private *priv,
mwifiex_dbg_dump(adapter, CMD_D, "cmd buffer:", host_cmd, cmd_size);
if (adapter->iface_type == MWIFIEX_USB) {
- tmp = cpu_to_le32(MWIFIEX_USB_TYPE_CMD);
skb_push(cmd_node->cmd_skb, MWIFIEX_TYPE_LEN);
- memcpy(cmd_node->cmd_skb->data, &tmp, MWIFIEX_TYPE_LEN);
+ put_unaligned_le32(MWIFIEX_USB_TYPE_CMD,
+ cmd_node->cmd_skb->data);
adapter->cmd_sent = true;
ret = adapter->if_ops.host_to_card(adapter,
MWIFIEX_USB_EP_CMD_EVENT,
@@ -317,7 +317,6 @@ static int mwifiex_dnld_sleep_confirm_cmd(struct mwifiex_adapter *adapter)
(struct mwifiex_opt_sleep_confirm *)
adapter->sleep_cfm->data;
struct sk_buff *sleep_cfm_tmp;
- __le32 tmp;
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
@@ -342,8 +341,7 @@ static int mwifiex_dnld_sleep_confirm_cmd(struct mwifiex_adapter *adapter)
+ MWIFIEX_TYPE_LEN);
skb_put(sleep_cfm_tmp, sizeof(struct mwifiex_opt_sleep_confirm)
+ MWIFIEX_TYPE_LEN);
- tmp = cpu_to_le32(MWIFIEX_USB_TYPE_CMD);
- memcpy(sleep_cfm_tmp->data, &tmp, MWIFIEX_TYPE_LEN);
+ put_unaligned_le32(MWIFIEX_USB_TYPE_CMD, sleep_cfm_tmp->data);
memcpy(sleep_cfm_tmp->data + MWIFIEX_TYPE_LEN,
adapter->sleep_cfm->data,
sizeof(struct mwifiex_opt_sleep_confirm));
--
2.7.4
On 10/05/2017 12:07 PM, Himanshu Jha wrote:
>>
>> In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
>> seems that asm-generic/unaligned.h is set up to include different
>> headers, based on the expected architecture behavior.
>>
> Yes, asm-generic/unaligned.h looks more appopriate and is most generic
> implementation of unaligned accesses and arc specific.
>
Probably the idea is that each ARCH knows exactly what is the best way
to do unaligned access, so common code (like drivers) should include
ARCH-specific asm/unaligned.h only. It will then decide how to do that
and will include asm-generic/unaligned.h itself, if required.
On Thu, Oct 05, 2017 at 11:41:38AM +0300, Kalle Valo wrote:
> Himanshu Jha <[email protected]> writes:
>
> >> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> >> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> >> > @@ -17,6 +17,7 @@
> >> > * this warranty disclaimer.
> >> > */
> >> >
> >> > +#include <linux/unaligned/access_ok.h>
> >>
> >> I don't think this is correct. Should it be asm/unaligned.h?
> >
> > Would mind explainig me as to why it is incorrect! Also, it defined in
> > both the header files but, why is asm/unaligned.h preferred ?
>
> asm/unaligned.h seems to be the toplevel header file which includes
> header files based on arch configuration. Also grepping the sources
> support that, nobody from drivers/ include access_ok.h directly.
>
Yes, you are correct!
I will send v2 patch with asm/unaligned.h
> But I can't say that I fully understand how the header files work so
> please do correct me if I have mistaken.
>
It is confusing to me as well.
There are various instances where a function used in file say for eg
int func_align (void* a)
is used and it is defined in align.h
But many files don't *directly* include align.h and rather include
any other header which includes align.h
Is compiling the file the only way to check if apppropriate header is
included or is there some other way to check for it.
Thanks
Himanshu Jha <[email protected]> writes:
> Use put_unaligned_le32 rather than using byte ordering function and
> memcpy which makes code clear.
> Also, add the header file where it is declared.
>
> Done using Coccinelle and semantic patch used is :
>
> @ rule1 @
> identifier tmp; expression ptr,x; type T;
> @@
>
> - tmp = cpu_to_le32(x);
>
> <+... when != tmp
> - memcpy(ptr, (T)&tmp, ...);
> + put_unaligned_le32(x,ptr);
> ...+>
>
> @ depends on rule1 @
> type j; identifier tmp;
> @@
>
> - j tmp;
> ...when != tmp
>
> Signed-off-by: Himanshu Jha <[email protected]>
> ---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 0edc5d6..e28e119 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -17,6 +17,7 @@
> * this warranty disclaimer.
> */
>
> +#include <linux/unaligned/access_ok.h>
I don't think this is correct. Should it be asm/unaligned.h?
--
Kalle Valo
On Thu, Oct 05, 2017 at 10:23:37AM +0300, Kalle Valo wrote:
> Himanshu Jha <[email protected]> writes:
>
> > Use put_unaligned_le32 rather than using byte ordering function and
> > memcpy which makes code clear.
> > Also, add the header file where it is declared.
> >
> > Done using Coccinelle and semantic patch used is :
> >
> > @ rule1 @
> > identifier tmp; expression ptr,x; type T;
> > @@
> >
> > - tmp = cpu_to_le32(x);
> >
> > <+... when != tmp
> > - memcpy(ptr, (T)&tmp, ...);
> > + put_unaligned_le32(x,ptr);
> > ...+>
> >
> > @ depends on rule1 @
> > type j; identifier tmp;
> > @@
> >
> > - j tmp;
> > ...when != tmp
> >
> > Signed-off-by: Himanshu Jha <[email protected]>
> > ---
> > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 10 ++++------
> > 1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > index 0edc5d6..e28e119 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > @@ -17,6 +17,7 @@
> > * this warranty disclaimer.
> > */
> >
> > +#include <linux/unaligned/access_ok.h>
>
> I don't think this is correct. Should it be asm/unaligned.h?
Would mind explainig me as to why it is incorrect! Also, it defined in
both the header files but, why is asm/unaligned.h preferred ?
Thanks
> --
> Kalle Valo
Hi Himanshu,
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Himanshu-Jha/mwifiex-Use-put_unaligned_le32/20171007-095017
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
In file included from arch/xtensa/include/asm/unaligned.h:22:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_struct.h:6:19: error: redefinition of 'get_unaligned_be16'
static inline u16 get_unaligned_be16(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:22:28: note: previous definition of 'get_unaligned_be16' was here
static __always_inline u16 get_unaligned_be16(const void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:22:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_struct.h:11:19: error: redefinition of 'get_unaligned_be32'
static inline u32 get_unaligned_be32(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:27:28: note: previous definition of 'get_unaligned_be32' was here
static __always_inline u32 get_unaligned_be32(const void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:22:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_struct.h:16:19: error: redefinition of 'get_unaligned_be64'
static inline u64 get_unaligned_be64(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:32:28: note: previous definition of 'get_unaligned_be64' was here
static __always_inline u64 get_unaligned_be64(const void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:22:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_struct.h:21:20: error: redefinition of 'put_unaligned_be16'
static inline void put_unaligned_be16(u16 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:52:29: note: previous definition of 'put_unaligned_be16' was here
static __always_inline void put_unaligned_be16(u16 val, void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:22:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_struct.h:26:20: error: redefinition of 'put_unaligned_be32'
static inline void put_unaligned_be32(u32 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:57:29: note: previous definition of 'put_unaligned_be32' was here
static __always_inline void put_unaligned_be32(u32 val, void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:22:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_struct.h:31:20: error: redefinition of 'put_unaligned_be64'
static inline void put_unaligned_be64(u64 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:62:29: note: previous definition of 'put_unaligned_be64' was here
static __always_inline void put_unaligned_be64(u64 val, void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:23:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/le_byteshift.h:40:19: error: redefinition of 'get_unaligned_le16'
static inline u16 get_unaligned_le16(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:7:28: note: previous definition of 'get_unaligned_le16' was here
static __always_inline u16 get_unaligned_le16(const void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:23:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/le_byteshift.h:45:19: error: redefinition of 'get_unaligned_le32'
static inline u32 get_unaligned_le32(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:12:28: note: previous definition of 'get_unaligned_le32' was here
static __always_inline u32 get_unaligned_le32(const void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:23:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/le_byteshift.h:50:19: error: redefinition of 'get_unaligned_le64'
static inline u64 get_unaligned_le64(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:17:28: note: previous definition of 'get_unaligned_le64' was here
static __always_inline u64 get_unaligned_le64(const void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:23:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/le_byteshift.h:55:20: error: redefinition of 'put_unaligned_le16'
static inline void put_unaligned_le16(u16 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:37:29: note: previous definition of 'put_unaligned_le16' was here
static __always_inline void put_unaligned_le16(u16 val, void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:23:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/le_byteshift.h:60:20: error: redefinition of 'put_unaligned_le32'
static inline void put_unaligned_le32(u32 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:42:29: note: previous definition of 'put_unaligned_le32' was here
static __always_inline void put_unaligned_le32(u32 val, void *p)
^
In file included from arch/xtensa/include/asm/unaligned.h:23:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/le_byteshift.h:65:20: error: redefinition of 'put_unaligned_le64'
static inline void put_unaligned_le64(u64 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:47:29: note: previous definition of 'put_unaligned_le64' was here
static __always_inline void put_unaligned_le64(u64 val, void *p)
^
vim +/put_unaligned_le32 +60 include/linux/unaligned/le_byteshift.h
064106a9 Harvey Harrison 2008-04-29 39
064106a9 Harvey Harrison 2008-04-29 @40 static inline u16 get_unaligned_le16(const void *p)
064106a9 Harvey Harrison 2008-04-29 41 {
064106a9 Harvey Harrison 2008-04-29 42 return __get_unaligned_le16((const u8 *)p);
064106a9 Harvey Harrison 2008-04-29 43 }
064106a9 Harvey Harrison 2008-04-29 44
064106a9 Harvey Harrison 2008-04-29 @45 static inline u32 get_unaligned_le32(const void *p)
064106a9 Harvey Harrison 2008-04-29 46 {
064106a9 Harvey Harrison 2008-04-29 47 return __get_unaligned_le32((const u8 *)p);
064106a9 Harvey Harrison 2008-04-29 48 }
064106a9 Harvey Harrison 2008-04-29 49
064106a9 Harvey Harrison 2008-04-29 @50 static inline u64 get_unaligned_le64(const void *p)
064106a9 Harvey Harrison 2008-04-29 51 {
064106a9 Harvey Harrison 2008-04-29 52 return __get_unaligned_le64((const u8 *)p);
064106a9 Harvey Harrison 2008-04-29 53 }
064106a9 Harvey Harrison 2008-04-29 54
064106a9 Harvey Harrison 2008-04-29 @55 static inline void put_unaligned_le16(u16 val, void *p)
064106a9 Harvey Harrison 2008-04-29 56 {
064106a9 Harvey Harrison 2008-04-29 57 __put_unaligned_le16(val, p);
064106a9 Harvey Harrison 2008-04-29 58 }
064106a9 Harvey Harrison 2008-04-29 59
064106a9 Harvey Harrison 2008-04-29 @60 static inline void put_unaligned_le32(u32 val, void *p)
064106a9 Harvey Harrison 2008-04-29 61 {
064106a9 Harvey Harrison 2008-04-29 62 __put_unaligned_le32(val, p);
064106a9 Harvey Harrison 2008-04-29 63 }
064106a9 Harvey Harrison 2008-04-29 64
064106a9 Harvey Harrison 2008-04-29 @65 static inline void put_unaligned_le64(u64 val, void *p)
064106a9 Harvey Harrison 2008-04-29 66 {
064106a9 Harvey Harrison 2008-04-29 67 __put_unaligned_le64(val, p);
064106a9 Harvey Harrison 2008-04-29 68 }
064106a9 Harvey Harrison 2008-04-29 69
:::::: The code at line 60 was first introduced by commit
:::::: 064106a91be5e76cb42c1ddf5d3871e3a1bd2a23 kernel: add common infrastructure for unaligned access
:::::: TO: Harvey Harrison <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Oct 05, 2017 at 11:02:50AM -0700, Brian Norris wrote:
> On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
> > There are various instances where a function used in file say for eg
> > int func_align (void* a)
> > is used and it is defined in align.h
> > But many files don't *directly* include align.h and rather include
> > any other header which includes align.h
>
> I believe the general rule is that you should included headers for all
> symbols you use, and not rely on implicit includes.
>
> The modification to the general rule is that not all headers are
> intended to be included directly, and in such cases there's likely a
> parent header that is the more appropriate target.
>
> In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
> seems that asm-generic/unaligned.h is set up to include different
> headers, based on the expected architecture behavior.
>
Yes, asm-generic/unaligned.h looks more appopriate and is most generic
implementation of unaligned accesses and arc specific.
Let's see what Kalle Valo recommends! And then I will send v2 of the
patch.
Thanks for the information!
Himanshu Jha
> I wonder if include/linux/unaligned/access_ok.h should have a safety
> check (e.g., raise an #error if
> !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?).
>
> > Is compiling the file the only way to check if apppropriate header is
> > included or is there some other way to check for it.
>
> I believe it's mostly manual. Implicit includes have been a problem for
> anyone who refactors header files.
>
> Brian
On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
> There are various instances where a function used in file say for eg
> int func_align (void* a)
> is used and it is defined in align.h
> But many files don't *directly* include align.h and rather include
> any other header which includes align.h
I believe the general rule is that you should included headers for all
symbols you use, and not rely on implicit includes.
The modification to the general rule is that not all headers are
intended to be included directly, and in such cases there's likely a
parent header that is the more appropriate target.
In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
seems that asm-generic/unaligned.h is set up to include different
headers, based on the expected architecture behavior.
I wonder if include/linux/unaligned/access_ok.h should have a safety
check (e.g., raise an #error if
!CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS?).
> Is compiling the file the only way to check if apppropriate header is
> included or is there some other way to check for it.
I believe it's mostly manual. Implicit includes have been a problem for
anyone who refactors header files.
Brian
Himanshu Jha <[email protected]> writes:
>> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
>> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
>> > @@ -17,6 +17,7 @@
>> > * this warranty disclaimer.
>> > */
>> >
>> > +#include <linux/unaligned/access_ok.h>
>>
>> I don't think this is correct. Should it be asm/unaligned.h?
>
> Would mind explainig me as to why it is incorrect! Also, it defined in
> both the header files but, why is asm/unaligned.h preferred ?
asm/unaligned.h seems to be the toplevel header file which includes
header files based on arch configuration. Also grepping the sources
support that, nobody from drivers/ include access_ok.h directly.
But I can't say that I fully understand how the header files work so
please do correct me if I have mistaken.
--
Kalle Valo
Hi Himanshu,
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Himanshu-Jha/mwifiex-Use-put_unaligned_le32/20171007-095017
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: openrisc-allyesconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 5.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All errors (new ones prefixed by >>):
In file included from arch/openrisc/include/asm/unaligned.h:42:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_memmove.h:6:19: error: redefinition of 'get_unaligned_be16'
static inline u16 get_unaligned_be16(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:22:28: note: previous definition of 'get_unaligned_be16' was here
static __always_inline u16 get_unaligned_be16(const void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:42:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_memmove.h:11:19: error: redefinition of 'get_unaligned_be32'
static inline u32 get_unaligned_be32(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:27:28: note: previous definition of 'get_unaligned_be32' was here
static __always_inline u32 get_unaligned_be32(const void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:42:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_memmove.h:16:19: error: redefinition of 'get_unaligned_be64'
static inline u64 get_unaligned_be64(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:32:28: note: previous definition of 'get_unaligned_be64' was here
static __always_inline u64 get_unaligned_be64(const void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:42:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_memmove.h:21:20: error: redefinition of 'put_unaligned_be16'
static inline void put_unaligned_be16(u16 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:52:29: note: previous definition of 'put_unaligned_be16' was here
static __always_inline void put_unaligned_be16(u16 val, void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:42:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_memmove.h:26:20: error: redefinition of 'put_unaligned_be32'
static inline void put_unaligned_be32(u32 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:57:29: note: previous definition of 'put_unaligned_be32' was here
static __always_inline void put_unaligned_be32(u32 val, void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:42:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
>> include/linux/unaligned/be_memmove.h:31:20: error: redefinition of 'put_unaligned_be64'
static inline void put_unaligned_be64(u64 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:62:29: note: previous definition of 'put_unaligned_be64' was here
static __always_inline void put_unaligned_be64(u64 val, void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:43:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
include/linux/unaligned/le_byteshift.h:40:19: error: redefinition of 'get_unaligned_le16'
static inline u16 get_unaligned_le16(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:7:28: note: previous definition of 'get_unaligned_le16' was here
static __always_inline u16 get_unaligned_le16(const void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:43:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
include/linux/unaligned/le_byteshift.h:45:19: error: redefinition of 'get_unaligned_le32'
static inline u32 get_unaligned_le32(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:12:28: note: previous definition of 'get_unaligned_le32' was here
static __always_inline u32 get_unaligned_le32(const void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:43:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
include/linux/unaligned/le_byteshift.h:50:19: error: redefinition of 'get_unaligned_le64'
static inline u64 get_unaligned_le64(const void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:17:28: note: previous definition of 'get_unaligned_le64' was here
static __always_inline u64 get_unaligned_le64(const void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:43:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
include/linux/unaligned/le_byteshift.h:55:20: error: redefinition of 'put_unaligned_le16'
static inline void put_unaligned_le16(u16 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:37:29: note: previous definition of 'put_unaligned_le16' was here
static __always_inline void put_unaligned_le16(u16 val, void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:43:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
include/linux/unaligned/le_byteshift.h:60:20: error: redefinition of 'put_unaligned_le32'
static inline void put_unaligned_le32(u32 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:42:29: note: previous definition of 'put_unaligned_le32' was here
static __always_inline void put_unaligned_le32(u32 val, void *p)
^
In file included from arch/openrisc/include/asm/unaligned.h:43:0,
from include/linux/etherdevice.h:28,
from include/linux/ieee80211.h:22,
from drivers/net//wireless/marvell/mwifiex/decl.h:28,
from drivers/net//wireless/marvell/mwifiex/cmdevt.c:21:
include/linux/unaligned/le_byteshift.h:65:20: error: redefinition of 'put_unaligned_le64'
static inline void put_unaligned_le64(u64 val, void *p)
^
In file included from drivers/net//wireless/marvell/mwifiex/cmdevt.c:20:0:
include/linux/unaligned/access_ok.h:47:29: note: previous definition of 'put_unaligned_le64' was here
static __always_inline void put_unaligned_le64(u64 val, void *p)
^
vim +/get_unaligned_be16 +6 include/linux/unaligned/be_memmove.h
064106a9 Harvey Harrison 2008-04-29 5
064106a9 Harvey Harrison 2008-04-29 @6 static inline u16 get_unaligned_be16(const void *p)
064106a9 Harvey Harrison 2008-04-29 7 {
064106a9 Harvey Harrison 2008-04-29 8 return __get_unaligned_memmove16((const u8 *)p);
064106a9 Harvey Harrison 2008-04-29 9 }
064106a9 Harvey Harrison 2008-04-29 10
064106a9 Harvey Harrison 2008-04-29 @11 static inline u32 get_unaligned_be32(const void *p)
064106a9 Harvey Harrison 2008-04-29 12 {
064106a9 Harvey Harrison 2008-04-29 13 return __get_unaligned_memmove32((const u8 *)p);
064106a9 Harvey Harrison 2008-04-29 14 }
064106a9 Harvey Harrison 2008-04-29 15
064106a9 Harvey Harrison 2008-04-29 @16 static inline u64 get_unaligned_be64(const void *p)
064106a9 Harvey Harrison 2008-04-29 17 {
064106a9 Harvey Harrison 2008-04-29 18 return __get_unaligned_memmove64((const u8 *)p);
064106a9 Harvey Harrison 2008-04-29 19 }
064106a9 Harvey Harrison 2008-04-29 20
064106a9 Harvey Harrison 2008-04-29 @21 static inline void put_unaligned_be16(u16 val, void *p)
064106a9 Harvey Harrison 2008-04-29 22 {
064106a9 Harvey Harrison 2008-04-29 23 __put_unaligned_memmove16(val, p);
064106a9 Harvey Harrison 2008-04-29 24 }
064106a9 Harvey Harrison 2008-04-29 25
064106a9 Harvey Harrison 2008-04-29 @26 static inline void put_unaligned_be32(u32 val, void *p)
064106a9 Harvey Harrison 2008-04-29 27 {
064106a9 Harvey Harrison 2008-04-29 28 __put_unaligned_memmove32(val, p);
064106a9 Harvey Harrison 2008-04-29 29 }
064106a9 Harvey Harrison 2008-04-29 30
064106a9 Harvey Harrison 2008-04-29 @31 static inline void put_unaligned_be64(u64 val, void *p)
064106a9 Harvey Harrison 2008-04-29 32 {
064106a9 Harvey Harrison 2008-04-29 33 __put_unaligned_memmove64(val, p);
064106a9 Harvey Harrison 2008-04-29 34 }
064106a9 Harvey Harrison 2008-04-29 35
:::::: The code at line 6 was first introduced by commit
:::::: 064106a91be5e76cb42c1ddf5d3871e3a1bd2a23 kernel: add common infrastructure for unaligned access
:::::: TO: Harvey Harrison <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Himanshu Jha <[email protected]> writes:
> On Thu, Oct 05, 2017 at 11:02:50AM -0700, Brian Norris wrote:
>> On Thu, Oct 05, 2017 at 08:52:33PM +0530, Himanshu Jha wrote:
>> > There are various instances where a function used in file say for eg
>> > int func_align (void* a)
>> > is used and it is defined in align.h
>> > But many files don't *directly* include align.h and rather include
>> > any other header which includes align.h
>>
>> I believe the general rule is that you should included headers for all
>> symbols you use, and not rely on implicit includes.
>>
>> The modification to the general rule is that not all headers are
>> intended to be included directly, and in such cases there's likely a
>> parent header that is the more appropriate target.
>>
>> In this case, the key is CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS. It
>> seems that asm-generic/unaligned.h is set up to include different
>> headers, based on the expected architecture behavior.
>>
> Yes, asm-generic/unaligned.h looks more appopriate and is most generic
> implementation of unaligned accesses and arc specific.
>
> Let's see what Kalle Valo recommends! And then I will send v2 of the
> patch.
Not sure what you are asking from me. But if you are asking should it
be:
#include <asm/unaligned.h>
or:
#include <asm-generic/unaligned.h>
I think it should be the former.
--
Kalle Valo