[PATCH v3] staging: wlan-ng: Fix struct definition's and variable type

2017-06-15 Thread sunil . m 
From: Suniel Mahesh 

le16_to_cpu() accepts argument of type __le16 and cpu_to_le16()
returns an argument of type __le16. This patch fixes:
(a) the type of the variable that end's up getting return from
cpu_to_le16().
(b) the member types of struct hfa384x_host_scan_request_data,
struct hfa384x_bytestr32 and struct hfa384x_hscan_result_sub.

The following type mismatch warnings reported by sparse
have been fixed:
warning: incorrect type in assignment (different base types)
warning: cast to restricted __le16

Signed-off-by: Suniel Mahesh 
---
Changes for v3:
- Edited subject and description of the patch to fit with the changes
  done in the code base, as suggested by Frans Klaver.

Changes for v2:
- Reworked on the patch and modified commit message as per the
  recommendations from Frans Klaver and Greg K-H.

- Patch was tested and built on next-20170609 and staging-testing.
---
 drivers/staging/wlan-ng/hfa384x.h| 18 +-
 drivers/staging/wlan-ng/prism2mgmt.c |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x.h 
b/drivers/staging/wlan-ng/hfa384x.h
index 310e2c4..f99cc04 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -358,7 +358,7 @@ struct hfa384x_bytestr {
 } __packed;
 
 struct hfa384x_bytestr32 {
-   u16 len;
+   __le16 len;
u8 data[32];
 } __packed;
 
@@ -399,8 +399,8 @@ struct hfa384x_caplevel {
 
 /*-- Configuration Record: HostScanRequest (data portion only) --*/
 struct hfa384x_host_scan_request_data {
-   u16 channel_list;
-   u16 tx_rate;
+   __le16 channel_list;
+   __le16 tx_rate;
struct hfa384x_bytestr32 ssid;
 } __packed;
 
@@ -682,16 +682,16 @@ struct hfa384x_ch_info_result {
 
 /*--  Inquiry Frame, Diagnose: Host Scan Results & Subfields--*/
 struct hfa384x_hscan_result_sub {
-   u16 chid;
-   u16 anl;
-   u16 sl;
+   __le16 chid;
+   __le16 anl;
+   __le16 sl;
u8 bssid[WLAN_BSSID_LEN];
-   u16 bcnint;
-   u16 capinfo;
+   __le16 bcnint;
+   __le16 capinfo;
struct hfa384x_bytestr32 ssid;
u8 supprates[10];   /* 802.11 info element */
u16 proberesp_rate;
-   u16 atim;
+   __le16 atim;
 } __packed;
 
 struct hfa384x_hscan_result {
diff --git a/drivers/staging/wlan-ng/prism2mgmt.c 
b/drivers/staging/wlan-ng/prism2mgmt.c
index f4d6e48..c4aa9e7 100644
--- a/drivers/staging/wlan-ng/prism2mgmt.c
+++ b/drivers/staging/wlan-ng/prism2mgmt.c
@@ -213,7 +213,7 @@ int prism2mgmt_scan(struct wlandevice *wlandev, void *msgp)
goto exit;
}
if (word == HFA384x_PORTSTATUS_DISABLED) {
-   u16 wordbuf[17];
+   __le16 wordbuf[17];
 
result = hfa384x_drvr_setconfig16(hw,
HFA384x_RID_CNFROAMINGMODE,
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: lustre: lustre: resolve "use spaces between elements" checkpatch errors

2017-06-15 Thread James Simmons 

> On Thu, 2017-06-15 at 17:57 +0100, James Simmons wrote:
> > > On Thu, 2017-06-15 at 17:03 +0100, James Simmons wrote:
> > > > > On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > > > > > Due to the way the DFID was embedded in our debug strings checkpatch
> > > > > > would report the following error:
> > > > > 
> > > > > unrelated trivia
> > > > > 
> > > > > > diff --git 
> > > > > > a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h 
> > > > > > b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
> > > > > 
> > > > > []
> > > > > > @@ -532,7 +532,7 @@ static inline void obd_uuid2fsname(char *buf, 
> > > > > > char *uuid, int buflen)
> > > > > >  #define FID_NOBRACE_LEN 40
> > > > > >  #define FID_LEN (FID_NOBRACE_LEN + 2)
> > > > > >  #define DFID_NOBRACE "%#llx:0x%x:0x%x"
> > > > > 
> > > > > It's odd to use a mixture of %#x and 0x%x.
> > > > > 
> > > > > Using
> > > > >   #define DFID_NOBRACE "%#llx:%#x:%#x"
> > > > > would also save a couple bytes per use.
> > > > 
> > > > Changing that format would break things very badly. This is used in 
> > > > user 
> > > > land utilities and the kernel code. 
> > > 
> > > Really?  Why would anything break?
> > 
> > It shouldn't break anything but I'm paranoid.
> 
> Which is an entirely different thing than
> writing "would break things very badly".
> 
> Paranoia is fine, incorrect statements of fact
> like that less so.

Meant no offense. When you said "would also save a couple bytes per use"
I took that as meaning it would change the data format. Should of tried
it myself to see if that was the case. 
 
> > In the past I have change 
> > "simple" things to have it blow up. I pushed the change to our test 
> > harness just to make sure. 
> 
> Understandable.
> 
> cheers, Joe
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: lustre: lustre: resolve "use spaces between elements" checkpatch errors

2017-06-15 Thread Joe Perches 
On Thu, 2017-06-15 at 17:57 +0100, James Simmons wrote:
> > On Thu, 2017-06-15 at 17:03 +0100, James Simmons wrote:
> > > > On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > > > > Due to the way the DFID was embedded in our debug strings checkpatch
> > > > > would report the following error:
> > > > 
> > > > unrelated trivia
> > > > 
> > > > > diff --git 
> > > > > a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h 
> > > > > b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
> > > > 
> > > > []
> > > > > @@ -532,7 +532,7 @@ static inline void obd_uuid2fsname(char *buf, 
> > > > > char *uuid, int buflen)
> > > > >  #define FID_NOBRACE_LEN 40
> > > > >  #define FID_LEN (FID_NOBRACE_LEN + 2)
> > > > >  #define DFID_NOBRACE "%#llx:0x%x:0x%x"
> > > > 
> > > > It's odd to use a mixture of %#x and 0x%x.
> > > > 
> > > > Using
> > > > #define DFID_NOBRACE "%#llx:%#x:%#x"
> > > > would also save a couple bytes per use.
> > > 
> > > Changing that format would break things very badly. This is used in user 
> > > land utilities and the kernel code. 
> > 
> > Really?  Why would anything break?
> 
> It shouldn't break anything but I'm paranoid.

Which is an entirely different thing than
writing "would break things very badly".

Paranoia is fine, incorrect statements of fact
like that less so.

> In the past I have change 
> "simple" things to have it blow up. I pushed the change to our test 
> harness just to make sure. 

Understandable.

cheers, Joe

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 6/6] staging: lustre: lustre: fix all braces issues reported by checkpatch

2017-06-15 Thread Joe Perches 
On Thu, 2017-06-15 at 17:38 +0100, James Simmons wrote:
> > On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > > Cleanup all braces that was reported by checkpatch. The only
> > > issue not fixed up is in mdc_lock.c. Removing the braces in
> > > the case of mdc_lock.c will break the build.
> > 
> > []
> > 
> > > diff --git a/drivers/staging/lustre/lustre/llite/vvp_dev.c 
> > > b/drivers/staging/lustre/lustre/llite/vvp_dev.c
> > 
> > []
> > > @@ -591,9 +591,10 @@ static void *vvp_pgcache_start(struct seq_file *f, 
> > > loff_t *pos)
> > >   env = cl_env_get();
> > >   if (!IS_ERR(env)) {
> > >   sbi = f->private;
> > > - if (sbi->ll_site->ls_obj_hash->hs_cur_bits > 64 - PGC_OBJ_SHIFT)
> > > + if (sbi->ll_site->ls_obj_hash->hs_cur_bits >
> > > + 64 - PGC_OBJ_SHIFT) {
> > >   pos = ERR_PTR(-EFBIG);
> > > - else {
> > > + } else {
> > >   *pos = vvp_pgcache_find(env, >ll_cl->cd_lu_dev,
> > >   *pos);
> > >   if (*pos == ~0ULL)
> > 
> > Sometimes is nicer to rearrange the code with smaller
> > indentation by using early returns and/or goto .
> 
> Do you mind if I submit a separate patch for this?
> It would be nice to land the current cleanups as is.

Of course not.

> Looking at the code I see where more
> simplication along this line can be done. I submitted a patch for our
> test harness:

Oh good.

> In the near future it will be pushed here.

Swell.

One of the blocks in vvp_pgcache_show looks like it
has asymmetric braces though.

 } else

Perhaps you should be using the linux-next version
of checkpatch which warns about this.

And don't take any of what I wrote as a requirement.

All were merely suggestions and could be implemented
an appropriate time or ignored completely.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: lustre: lustre: resolve "use spaces between elements" checkpatch errors

2017-06-15 Thread James Simmons 

> On Thu, 2017-06-15 at 17:03 +0100, James Simmons wrote:
> > > On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > > > Due to the way the DFID was embedded in our debug strings checkpatch
> > > > would report the following error:
> > > 
> > > unrelated trivia
> > > 
> > > > diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h 
> > > > b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
> > > 
> > > []
> > > > @@ -532,7 +532,7 @@ static inline void obd_uuid2fsname(char *buf, char 
> > > > *uuid, int buflen)
> > > >  #define FID_NOBRACE_LEN 40
> > > >  #define FID_LEN (FID_NOBRACE_LEN + 2)
> > > >  #define DFID_NOBRACE "%#llx:0x%x:0x%x"
> > > 
> > > It's odd to use a mixture of %#x and 0x%x.
> > > 
> > > Using
> > >   #define DFID_NOBRACE "%#llx:%#x:%#x"
> > > would also save a couple bytes per use.
> > 
> > Changing that format would break things very badly. This is used in user 
> > land utilities and the kernel code. 
> 
> Really?  Why would anything break?

It shouldn't break anything but I'm paranoid. In the past I have change 
"simple" things to have it blow up. I pushed the change to our test 
harness just to make sure. 
 
> $ cat fmt.c
> #include 
> #include 
> 
> int main(int argc, char **argv)
> {
>   printf("%#llx:0x%x:0x%x\n", (unsigned long long)1, 2, 3);
>   printf("%#llx:%#x:%#x\n", (unsigned long long)1, 2, 3);
>   return 0;
> }
> 
> $ gcc fmt.c
> $ ./a.out
> 0x1:0x2:0x3
> 0x1:0x2:0x3
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [lustre-devel] [PATCH] staging: lustre: headers: potential UAPI headers

2017-06-15 Thread Greg Kroah-Hartman 
On Thu, Jun 15, 2017 at 04:48:20PM +0100, James Simmons wrote:
> So this is coming from trying to understand the "merge them together" 
> part. Some people reading this it implies all the headers would be 
> eventually merged into one big header and placed into include/uapi/linux. 

Sounds like a good plan, why wouldn't you want something nice and simple
like that?  :)

> We are getting to the point where some sites are migrating from the out
> of tree branch to this client. This also means they will be moving 
> external open source userland applications shortly. If we expose UAPI 
> headers that are completely different that it breaks their tools users
> will dump this client and go back to the out source tree. We really like
> to avoid that.

Note, as the code is still in staging, files will get renamed and moved
around, you know that, nothing we can do here at the moment.

I have yet to see any patches yet, so I don't know why people are
complaining...

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 6/6] staging: lustre: lustre: fix all braces issues reported by checkpatch

2017-06-15 Thread James Simmons 

> On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > Cleanup all braces that was reported by checkpatch. The only
> > issue not fixed up is in mdc_lock.c. Removing the braces in
> > the case of mdc_lock.c will break the build.
> 
> []
> 
> > diff --git a/drivers/staging/lustre/lustre/llite/vvp_dev.c 
> > b/drivers/staging/lustre/lustre/llite/vvp_dev.c
> []
> > @@ -591,9 +591,10 @@ static void *vvp_pgcache_start(struct seq_file *f, 
> > loff_t *pos)
> > env = cl_env_get();
> > if (!IS_ERR(env)) {
> > sbi = f->private;
> > -   if (sbi->ll_site->ls_obj_hash->hs_cur_bits > 64 - PGC_OBJ_SHIFT)
> > +   if (sbi->ll_site->ls_obj_hash->hs_cur_bits >
> > +   64 - PGC_OBJ_SHIFT) {
> > pos = ERR_PTR(-EFBIG);
> > -   else {
> > +   } else {
> > *pos = vvp_pgcache_find(env, >ll_cl->cd_lu_dev,
> > *pos);
> > if (*pos == ~0ULL)
> 
> Sometimes is nicer to rearrange the code with smaller
> indentation by using early returns and/or goto .

Do you mind if I submit a separate patch for this? It would be nice to
land the current cleanups as is. Looking at the code I see where more
simplication along this line can be done. I submitted a patch for our
test harness:

https://review.whamcloud.com/#/c/27664

In the near future it will be pushed here.

> Something like:
> 
> static void *vvp_pgcache_start(struct seq_file *f, loff_t *pos)
> {
>   struct ll_sb_info *sbi;
>   struct lu_env *env;
>   u16 refcheck;
> 
>   sbi = f->private;
> 
>   env = cl_env_get();
>   if (IS_ERR(env))
>   return pos;
> 
>   sbi = f->private;
>   if (sbi->ll_site->ls_obj_hash->hs_cur_bits > 64 - PGC_OBJ_SHIFT) {
>   pos = ERR_PTR(-EFBIG);
>   goto out;
>   }
> 
>   *pos = vvp_pgcache_find(env, >ll_cl->cd_lu_dev, *pos);
>   if (*pos == ~0ULL)
>   pos = NULL;
> 
> out:
>   cl_env_put(env, );
> 
>   return pos;
> }
> 
> ___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: vc04_services: bcm2835-audio: bcm2835-ctl.c: Fixed alignment to match open paranthesis.

2017-06-15 Thread srishti sharma 
On Thu, Jun 15, 2017 at 12:29 AM, Joe Perches  wrote:
> On Wed, 2017-06-14 at 19:36 +0530, srishti sharma wrote:
>> Fixed alignment so that it matched open paranthesis.
>
> Please try to avoid typos in your commit message and as
> well try to do all of alignment warnings in a single patch.
>
> Ideally, all the files in a driver would be modified.
>
> $ ./scripts/checkpatch.pl -f 
> drivers/staging/vc04_services/bcm2835-audio/*.[ch]  
> --types=parenthesis_alignment --terse
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:108: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:188: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:196: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:213: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:233: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:241: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:252: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:260: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c:277: CHECK: 
> Alignment should match open parenthesis
> total: 0 errors, 0 warnings, 9 checks, 426 lines checked
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:70: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:85: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:256: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:305: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:328: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:336: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:371: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:378: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:421: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:431: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:436: CHECK: 
> Alignment should match open parenthesis
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c:525: CHECK: 
> Alignment should match open parenthesis
> total: 0 errors, 0 warnings, 12 checks, 567 lines checked
>
>


Hey,

Thanks for this , I have re-sent the patch .

Regards,
srishti
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: lustre: lustre: resolve "use spaces between elements" checkpatch errors

2017-06-15 Thread Joe Perches 
On Thu, 2017-06-15 at 17:03 +0100, James Simmons wrote:
> > On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > > Due to the way the DFID was embedded in our debug strings checkpatch
> > > would report the following error:
> > 
> > unrelated trivia
> > 
> > > diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h 
> > > b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
> > 
> > []
> > > @@ -532,7 +532,7 @@ static inline void obd_uuid2fsname(char *buf, char 
> > > *uuid, int buflen)
> > >  #define FID_NOBRACE_LEN 40
> > >  #define FID_LEN (FID_NOBRACE_LEN + 2)
> > >  #define DFID_NOBRACE "%#llx:0x%x:0x%x"
> > 
> > It's odd to use a mixture of %#x and 0x%x.
> > 
> > Using
> > #define DFID_NOBRACE "%#llx:%#x:%#x"
> > would also save a couple bytes per use.
> 
> Changing that format would break things very badly. This is used in user 
> land utilities and the kernel code. 

Really?  Why would anything break?

$ cat fmt.c
#include 
#include 

int main(int argc, char **argv)
{
printf("%#llx:0x%x:0x%x\n", (unsigned long long)1, 2, 3);
printf("%#llx:%#x:%#x\n", (unsigned long long)1, 2, 3);
return 0;
}

$ gcc fmt.c
$ ./a.out
0x1:0x2:0x3
0x1:0x2:0x3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/6] staging: lustre: lustre: several over 80 characters cleanups

2017-06-15 Thread Joe Perches 
On Thu, 2017-06-15 at 17:06 +0100, James Simmons wrote:
> > On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > > Cleanup many of the over 80 characters reported by checkpatch
> > 
> > Please don't let checkpatch get in the way of lustre
> > readability.
> > 
> > lustre commonly uses very long identifiers.
> > Long identifiers and 80 columns don't mix well.
> > 
> > It might be simpler to declare in some document that
> > lustre uses lines of up to whatever length and require
> > that checkpatch should be used with the --max-line-length
> > option when run on lustre code.
> 
> Greg would you be okay with this?

I trust Greg isn't a zealot.

Linus Torvalds has said he prefers a longer line length
(up to 100 cols)

https://lkml.org/lkml/2016/12/15/749

> If we changed to a max-line-length to 
> say 128 thay would mean very few checkpatch issues would remain.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [lustre-devel] [PATCH] staging: lustre: headers: potential UAPI headers

2017-06-15 Thread James Simmons 

> On Mon, Jun 12, 2017 at 08:20:15PM +, Dilger, Andreas wrote:
> > On Jan 21, 2017, at 02:24, Greg Kroah-Hartman  
> > wrote:
> > > 
> > > On Fri, Jan 20, 2017 at 11:33:11PM +, James Simmons wrote:
> > >> 
> > > On Mon, Dec 19, 2016 at 12:06:47PM -0500, James Simmons wrote:
> > >> Not for landing. This is the purposed UAPI headers
> > >> with the removal of unlikely and debugging macros.
> > >> This is just for feedback to see if this is acceptable
> > >> for the upstream client.
> > >> 
> > >> Signed-off-by: James Simmons 
> > >> ---
> > >> .../lustre/lustre/include/lustre/lustre_fid.h  | 353 
> > >> +
> > >> .../lustre/lustre/include/lustre/lustre_ostid.h| 233 
> > >> ++
> > > 
> > > Can you make a lustre "uapi" directory so we can see which files you
> > > really want to be UAPI and which you don't as time goes on?
> 
> I said that ^^^
> 
> >  Where do you want them placed? In uapi/linux/lustre or uapi/lustre. 
> >  Does
> >  it matter to you? The below was to forth coming UAPI headers which from
> >  your response you seem okay with in general.
> > >>> 
> > >>> How many .h files are there going to be?  It's just a single filesystem,
> > >>> shouldn't you just need a single file?  If so, how about
> > >>> drivers/staging/lustre/include/uapi/lustre.h
> > >>> ?
> > >>> 
> > >>> If you really need multiple .h files, put them all in the same uapi/
> > >>> directory with a lustre_ prefix, you don't need a whole subdir just for
> > >>> yourself, right?
> > >> 
> > >> We have 12 UAPI headers and yes they all begin with lustre_*. Okay I will
> > >> create a driver/staging/lustre/include/uapi/linux directory and start
> > >> moving headers there.
> > > 
> > > 12 seems like a lot just for a single, tiny, filesystem :)
> > > 
> > > But moving them all to a single directory will see where we can later
> > > merge them together, sounds like a good plan.
> > 
> > Greg,
> > are you really adamantly against moving the Lustre headers into their own 
> > lustre/
> > subdirectory?
> 
> I did not say that.
> 
> Please, when responding to 5 month old email messages, get your quoting
> correct...

So this is coming from trying to understand the "merge them together" 
part. Some people reading this it implies all the headers would be 
eventually merged into one big header and placed into include/uapi/linux. 

We are getting to the point where some sites are migrating from the out
of tree branch to this client. This also means they will be moving 
external open source userland applications shortly. If we expose UAPI 
headers that are completely different that it breaks their tools users
will dump this client and go back to the out source tree. We really like
to avoid that.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/6] staging: lustre: lustre: several over 80 characters cleanups

2017-06-15 Thread James Simmons 

> On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > Cleanup many of the over 80 characters reported by checkpatch
> 
> Please don't let checkpatch get in the way of lustre
> readability.
> 
> lustre commonly uses very long identifiers.
> Long identifiers and 80 columns don't mix well.
> 
> It might be simpler to declare in some document that
> lustre uses lines of up to whatever length and require
> that checkpatch should be used with the --max-line-length
> option when run on lustre code.

Greg would you be okay with this? If we changed to a max-line-length to 
say 128 thay would mean very few checkpatch issues would remain.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: lustre: lustre: resolve "use spaces between elements" checkpatch errors

2017-06-15 Thread James Simmons 

> On Wed, 2017-06-14 at 11:01 -0400, James Simmons wrote:
> > Due to the way the DFID was embedded in our debug strings checkpatch
> > would report the following error:
> 
> unrelated trivia
> 
> > diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h 
> > b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
> []
> > @@ -532,7 +532,7 @@ static inline void obd_uuid2fsname(char *buf, char 
> > *uuid, int buflen)
> >  #define FID_NOBRACE_LEN 40
> >  #define FID_LEN (FID_NOBRACE_LEN + 2)
> >  #define DFID_NOBRACE "%#llx:0x%x:0x%x"
> 
> It's odd to use a mixture of %#x and 0x%x.
> 
> Using
>   #define DFID_NOBRACE "%#llx:%#x:%#x"
> would also save a couple bytes per use.

Changing that format would break things very badly. This is used in user 
land utilities and the kernel code. 
 
> Does there need to be a difference between an SFID
> and a DFID_NOBRACE?

 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/1] fix coding style

2017-06-15 Thread Greg Kroah-Hartman 
On Thu, Jun 15, 2017 at 10:52:03PM +0800, jm wrote:
> ---
>  drivers/staging/android/ion/ion.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c 
> b/drivers/staging/android/ion/ion.c
> index 43ecb4a..a2d36b3 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
>   p = &(*p)->rb_right;
>   } else {
>   pr_err("%s: buffer already found.", __func__);
> - BUG();
> + WARN_ON();
>   }
>   }
>  
> @@ -103,7 +103,7 @@ static struct ion_buffer *ion_buffer_create(struct 
> ion_heap *heap,
>   goto err2;
>   }
>  
> - if (buffer->sg_table == NULL) {
> + if (!buffer->sg_table) {
>   WARN_ONCE(1, "This heap needs to set the sgtable");
>   ret = -EINVAL;
>   goto err1;
> @@ -161,7 +161,7 @@ static void *ion_buffer_kmap_get(struct ion_buffer 
> *buffer)
>   return buffer->vaddr;
>   }
>   vaddr = buffer->heap->ops->map_kernel(buffer->heap, buffer);
> - if (WARN_ONCE(vaddr == NULL,
> + if (WARN_ONCE(!vaddr,
> "heap->ops->map_kernel should return ERR_PTR on error"))
>   return ERR_PTR(-EINVAL);
>   if (IS_ERR(vaddr))
> @@ -425,7 +425,7 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, 
> unsigned int flags)
>   }
>   up_read(>lock);
>  
> - if (buffer == NULL)
> + if (!buffer)
>   return -ENODEV;
>  
>   if (IS_ERR(buffer))
> -- 
> 2.7.4


Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/SubmittingPatches and resend it after
  adding that line.  Note, the line needs to be in the body of the
  email, before the patch, not at the bottom of the patch or in the
  email signature.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

- You did not use your real name.  Signed-off-by: and From: has to have
  a real name associated with it.  Use whatever you sign legal documents
  with, no anonymous patches are allowed.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: wlan-ng: Amend type mismatch warnings

2017-06-15 Thread Frans Klaver 
> Subject: [PATCH v2] staging: wlan-ng: Amend type mismatch warnings

I think it would be better to state that you fix the types of some
struct fields. That's a much more important goal of this patch than
getting sparse to spout slightly fewer warnings.

On Thu, Jun 15, 2017 at 8:41 AM,   wrote:
> From: Suniel Mahesh 
>
> le16_to_cpu() accepts argument of type __le16 and cpu_to_le16()
> returns an argument of type __le16. This patch fixes warnings
> related to incorrect type in assignment and changes the types
> in the corresponding header file.
> The following type mismatch warnings reported by sparse
> have been amended:

You didn't amend them; you removed them.

> warning: cast to restricted __le16
> warning: incorrect type in assignment (different base types)
>
> Signed-off-by: Suniel Mahesh 
> ---
> Changes for v2:
> - Reworked on the patch and modified commit message as per the
>   recommendations from Frans Klaver and Greg K-H.
>
> - Patch was tested and built on next-20170609 and staging-testing.
> ---
>  drivers/staging/wlan-ng/hfa384x.h| 18 +-
>  drivers/staging/wlan-ng/prism2mgmt.c |  2 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/hfa384x.h 
> b/drivers/staging/wlan-ng/hfa384x.h
> index 310e2c4..f99cc04 100644
> --- a/drivers/staging/wlan-ng/hfa384x.h
> +++ b/drivers/staging/wlan-ng/hfa384x.h
> @@ -358,7 +358,7 @@ struct hfa384x_bytestr {
>  } __packed;
>
>  struct hfa384x_bytestr32 {
> -   u16 len;
> +   __le16 len;
> u8 data[32];
>  } __packed;
>
> @@ -399,8 +399,8 @@ struct hfa384x_caplevel {
>
>  /*-- Configuration Record: HostScanRequest (data portion only) --*/
>  struct hfa384x_host_scan_request_data {
> -   u16 channel_list;
> -   u16 tx_rate;
> +   __le16 channel_list;
> +   __le16 tx_rate;
> struct hfa384x_bytestr32 ssid;
>  } __packed;
>
> @@ -682,16 +682,16 @@ struct hfa384x_ch_info_result {
>
>  /*--  Inquiry Frame, Diagnose: Host Scan Results & Subfields--*/
>  struct hfa384x_hscan_result_sub {
> -   u16 chid;
> -   u16 anl;
> -   u16 sl;
> +   __le16 chid;
> +   __le16 anl;
> +   __le16 sl;
> u8 bssid[WLAN_BSSID_LEN];
> -   u16 bcnint;
> -   u16 capinfo;
> +   __le16 bcnint;
> +   __le16 capinfo;
> struct hfa384x_bytestr32 ssid;
> u8 supprates[10];   /* 802.11 info element */
> u16 proberesp_rate;
> -   u16 atim;
> +   __le16 atim;
>  } __packed;
>
>  struct hfa384x_hscan_result {
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.c 
> b/drivers/staging/wlan-ng/prism2mgmt.c
> index f4d6e48..c4aa9e7 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.c
> +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> @@ -213,7 +213,7 @@ int prism2mgmt_scan(struct wlandevice *wlandev, void 
> *msgp)
> goto exit;
> }
> if (word == HFA384x_PORTSTATUS_DISABLED) {
> -   u16 wordbuf[17];
> +   __le16 wordbuf[17];
>
> result = hfa384x_drvr_setconfig16(hw,
> HFA384x_RID_CNFROAMINGMODE,
> --
> 1.9.1
>

Cheers,
Frans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] staging: fsl-mc/dpio: Propagate error code

2017-06-15 Thread Bogdan Purcareata 
> -Original Message-
> From: Ioana Radulescu [mailto:ruxandra.radule...@nxp.com]
> Sent: Thursday, June 15, 2017 11:55 AM
> To: gre...@linuxfoundation.org
> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org;
> ag...@suse.de; a...@arndb.de; linux-arm-ker...@lists.infradead.org; Bogdan
> Purcareata ; stuyo...@gmail.com; Laurentiu Tudor
> ; Ruxandra Ioana Radulescu
> ; Roy Pledge ; Haiying Wang
> 
> Subject: [PATCH] staging: fsl-mc/dpio: Propagate error code
> 
> dpaa2_io_service_register() returns zero even if
> qbman_swp_CDAN_set() encountered an error. Fix this
> by propagating the error code so the caller is informed
> data availability notifications are not properly set
> for a channel.
> 
> Signed-off-by: Ioana Radulescu 

Acked-by: Bogdan Purcareata 

> ---
>  drivers/staging/fsl-mc/bus/dpio/dpio-service.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> index e5d66749614c..762f045f53f7 100644
> --- a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> +++ b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
> @@ -260,9 +260,9 @@ int dpaa2_io_service_register(struct dpaa2_io *d,
> 
>   /* Enable the generation of CDAN notifications */
>   if (ctx->is_cdan)
> - qbman_swp_CDAN_set_context_enable(d->swp,
> -   (u16)ctx->id,
> -   ctx->qman64);
> + return qbman_swp_CDAN_set_context_enable(d->swp,
> +  (u16)ctx->id,
> +  ctx->qman64);
>   return 0;
>  }
>  EXPORT_SYMBOL(dpaa2_io_service_register);
> --
> 2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] taging: bcm2835-audio: fix longer than 80 chars lines

2017-06-15 Thread Gao Zhuang 
Clip longer than 80 chars lines.
Fix more "80 char" warnings than [PATCH v1].
Keep 4 "80 char" warnings for readability.

Signed-off-by: Gao Zhuang 
---
 .../staging/vc04_services/bcm2835-audio/bcm2835-ctl.c  | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
index f484bb0..70e7283 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
@@ -72,11 +72,13 @@ static int toggle_mute(struct bcm2835_chip *chip, int nmute)
/* if the sound is muted then we need to unmute */
if (chip->mute == CTRL_VOL_MUTE) {
chip->volume = chip->old_volume; /* copy the old volume back */
-   audio_info("Unmuting, old_volume = %d, volume = %d ...\n", 
chip->old_volume, chip->volume);
+   audio_info("Unmuting, old_volume = %d, volume = %d ...\n",
+  chip->old_volume, chip->volume);
} else /* otherwise we mute */ {
chip->old_volume = chip->volume;
chip->volume = 26214; /* set volume to minimum level AKA mute */
-   audio_info("Muting, old_volume = %d, volume = %d ...\n", 
chip->old_volume, chip->volume);
+   audio_info("Muting, old_volume = %d, volume = %d ...\n",
+  chip->old_volume, chip->volume);
}
 
chip->mute = nmute;
@@ -114,10 +116,15 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol 
*kcontrol,
return -EINTR;
 
if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) {
-   audio_info("Volume change attempted.. volume = %d new_volume = 
%d\n", chip->volume, (int)ucontrol->value.integer.value[0]);
+   audio_info("Volume change attempted.. volume = %d new_volume = 
%d\n",
+  chip->volume, (int)ucontrol->value.integer.value[0]);
if (chip->mute == CTRL_VOL_MUTE) {
/* changed = toggle_mute(chip, CTRL_VOL_UNMUTE); */
-   changed = 1; /* should return 0 to signify no change 
but the mixer takes this as the opposite sign (no idea why) */
+   changed = 1; /* should return 0 to signify
+ * no change but the mixer
+ * takes this as the opposite
+ * sign (no idea why)
+ */
goto unlock;
}
if (changed || (ucontrol->value.integer.value[0] != 
chip2alsa(chip->volume))) {
@@ -152,7 +159,8 @@ static struct snd_kcontrol_new snd_bcm2835_ctl[] = {
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.name = "PCM Playback Volume",
.index = 0,
-   .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | 
SNDRV_CTL_ELEM_ACCESS_TLV_READ,
+   .access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
+ SNDRV_CTL_ELEM_ACCESS_TLV_READ,
.private_value = PCM_PLAYBACK_VOLUME,
.info = snd_bcm2835_ctl_info,
.get = snd_bcm2835_ctl_get,
-- 
2.5.0



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: octeon-usb: fix coding style error

2017-06-15 Thread bincy 


I have updated the comments here and resend the new patch
On 13/06/17 18:07, Greg KH wrote:

On Tue, Jun 13, 2017 at 03:45:32PM +0530, bincy_k_phi...@yahoo.co.in wrote:

From: bincy 

trivial fix for space alignment error

Signed-off-by: bincy k philip 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


First off, please slow down.  Take a break, and come back to this in a
few days when you have had a chance to think exactly about what you are
doing here.

Hint, your from: and signed-off-by: names do not match at all.

There is no rush here at all, please do not resend this for a few more
days at the earliest.

greg k-h


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fsl-mc/dpio: Propagate error code

2017-06-15 Thread Ioana Radulescu 
dpaa2_io_service_register() returns zero even if
qbman_swp_CDAN_set() encountered an error. Fix this
by propagating the error code so the caller is informed
data availability notifications are not properly set
for a channel.

Signed-off-by: Ioana Radulescu 
---
 drivers/staging/fsl-mc/bus/dpio/dpio-service.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c 
b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
index e5d66749614c..762f045f53f7 100644
--- a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
+++ b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
@@ -260,9 +260,9 @@ int dpaa2_io_service_register(struct dpaa2_io *d,
 
/* Enable the generation of CDAN notifications */
if (ctx->is_cdan)
-   qbman_swp_CDAN_set_context_enable(d->swp,
- (u16)ctx->id,
- ctx->qman64);
+   return qbman_swp_CDAN_set_context_enable(d->swp,
+(u16)ctx->id,
+ctx->qman64);
return 0;
 }
 EXPORT_SYMBOL(dpaa2_io_service_register);
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

2017-06-15 Thread Okash Khawaja 
Hi,

On Wed, Jun 14, 2017 at 9:23 AM, Dan Carpenter  wrote:
[...]
>
> Could you call it "dev_name" instead?  I normally expect "dev" to be a
> device struct.

Thanks for the feedback. Will keep these in mind for next version of the patch.

Okash
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

2017-06-15 Thread Okash Khawaja 
Hi,

On Wed, Jun 14, 2017 at 03:04:18PM +0200, Greg Kroah-Hartman wrote: 
> The console stuff is odd though, but that is each driver defining the
> name for itself, major/minor does not apply.  Also, a separate driver is
> not having to figure this all out.  I wonder if we could just add a tty
> core function for this as it does know the name of everything that has
> been registered with it, right?

I can start working on a patch for this. I'm still new to the tty code
so will follow up with any questions I might have.

Thanks,
Okash
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: wlan-ng: Amend type mismatch warnings

2017-06-15 Thread sunil . m 
From: Suniel Mahesh 

le16_to_cpu() accepts argument of type __le16 and cpu_to_le16()
returns an argument of type __le16. This patch fixes warnings
related to incorrect type in assignment and changes the types
in the corresponding header file.
The following type mismatch warnings reported by sparse
have been amended:
warning: cast to restricted __le16
warning: incorrect type in assignment (different base types)

Signed-off-by: Suniel Mahesh 
---
Changes for v2:
- Reworked on the patch and modified commit message as per the
  recommendations from Frans Klaver and Greg K-H.

- Patch was tested and built on next-20170609 and staging-testing.
---
 drivers/staging/wlan-ng/hfa384x.h| 18 +-
 drivers/staging/wlan-ng/prism2mgmt.c |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x.h 
b/drivers/staging/wlan-ng/hfa384x.h
index 310e2c4..f99cc04 100644
--- a/drivers/staging/wlan-ng/hfa384x.h
+++ b/drivers/staging/wlan-ng/hfa384x.h
@@ -358,7 +358,7 @@ struct hfa384x_bytestr {
 } __packed;
 
 struct hfa384x_bytestr32 {
-   u16 len;
+   __le16 len;
u8 data[32];
 } __packed;
 
@@ -399,8 +399,8 @@ struct hfa384x_caplevel {
 
 /*-- Configuration Record: HostScanRequest (data portion only) --*/
 struct hfa384x_host_scan_request_data {
-   u16 channel_list;
-   u16 tx_rate;
+   __le16 channel_list;
+   __le16 tx_rate;
struct hfa384x_bytestr32 ssid;
 } __packed;
 
@@ -682,16 +682,16 @@ struct hfa384x_ch_info_result {
 
 /*--  Inquiry Frame, Diagnose: Host Scan Results & Subfields--*/
 struct hfa384x_hscan_result_sub {
-   u16 chid;
-   u16 anl;
-   u16 sl;
+   __le16 chid;
+   __le16 anl;
+   __le16 sl;
u8 bssid[WLAN_BSSID_LEN];
-   u16 bcnint;
-   u16 capinfo;
+   __le16 bcnint;
+   __le16 capinfo;
struct hfa384x_bytestr32 ssid;
u8 supprates[10];   /* 802.11 info element */
u16 proberesp_rate;
-   u16 atim;
+   __le16 atim;
 } __packed;
 
 struct hfa384x_hscan_result {
diff --git a/drivers/staging/wlan-ng/prism2mgmt.c 
b/drivers/staging/wlan-ng/prism2mgmt.c
index f4d6e48..c4aa9e7 100644
--- a/drivers/staging/wlan-ng/prism2mgmt.c
+++ b/drivers/staging/wlan-ng/prism2mgmt.c
@@ -213,7 +213,7 @@ int prism2mgmt_scan(struct wlandevice *wlandev, void *msgp)
goto exit;
}
if (word == HFA384x_PORTSTATUS_DISABLED) {
-   u16 wordbuf[17];
+   __le16 wordbuf[17];
 
result = hfa384x_drvr_setconfig16(hw,
HFA384x_RID_CNFROAMINGMODE,
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel