[PATCH SPEAKUP v2 0/3] cleanup error and initilization

2017-02-28 Thread Pranay Kr. Srivastava
Changelog from v1:
1. fixed kbuild warning for i386 build as reported by kbuild robot
2. split initialization code in two patches.

Pranay Kr. Srivastava (3):
  return same error value from spk_set_key_info
  remove unecessary initial allocation of vc
  use spkeaup_allocate as per required context

 drivers/staging/speakup/main.c | 46 +-
 1 file changed, 23 insertions(+), 23 deletions(-)

-- 
2.10.2

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


[PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context

2017-02-28 Thread Pranay Kr. Srivastava
speakup_allocate used GFP_ATOMIC for allocations
even while during initialization due to it's use
in notifier call.

Pass GFP_ flags as well to speakup_allocate depending
on the context it is called in.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/staging/speakup/main.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index ca817ca..ede842e 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, 
u_char ch, u_short key)
 }
 
 /* Allocation concurrency is protected by the console semaphore */
-static int speakup_allocate(struct vc_data *vc)
+static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags)
 {
int vc_num;
 
vc_num = vc->vc_num;
if (speakup_console[vc_num] == NULL) {
speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
- GFP_ATOMIC);
+ gfp_flags);
if (speakup_console[vc_num] == NULL)
return -ENOMEM;
speakup_date(vc);
@@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb,
switch (code) {
case VT_ALLOCATE:
if (vc->vc_mode == KD_TEXT)
-   speakup_allocate(vc);
+   speakup_allocate(vc, GFP_ATOMIC);
break;
case VT_DEALLOCATE:
speakup_deallocate(vc);
@@ -2343,7 +2343,7 @@ static int __init speakup_init(void)
 
for (i = 0; i < MAX_NR_CONSOLES; i++)
if (vc_cons[i].d) {
-   err = speakup_allocate(vc_cons[i].d);
+   err = speakup_allocate(vc_cons[i].d, GFP_KERNEL);
if (err)
goto error_kobjects;
}
-- 
2.10.2

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


[PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info

2017-02-28 Thread Pranay Kr. Srivastava
This patch makes spk_set_key_info return -EINVAL
in case of failure instead of returning 4 different
values for the type of error that occurred.

Print the offending values instead as debug message.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/staging/speakup/main.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index c2f70ef..a1d5b66 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char 
*k_buffer)
u_char ch, version, num_keys;
 
version = *cp++;
-   if (version != KEY_MAP_VER)
-   return -1;
+   if (version != KEY_MAP_VER) {
+   pr_debug("version found %d should be %d\n",
+version, KEY_MAP_VER);
+   return -EINVAL;
+   }
num_keys = *cp;
states = (int)cp[1];
key_data_len = (states + 1) * (num_keys + 1);
-   if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf))
-   return -2;
+   if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) {
+   pr_debug("too many key_infos (%d over %u)\n",
+key_data_len + SHIFT_TBL_SIZE + 4, (unsigned 
int)(sizeof(spk_key_buf)));
+   return -EINVAL;
+   }
memset(k_buffer, 0, SHIFT_TBL_SIZE);
memset(spk_our_keys, 0, sizeof(spk_our_keys));
spk_shift_table = k_buffer;
@@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char 
*k_buffer)
cp1 += 2;   /* now pointing at shift states */
for (i = 1; i <= states; i++) {
ch = *cp1++;
-   if (ch >= SHIFT_TBL_SIZE)
-   return -3;
+   if (ch >= SHIFT_TBL_SIZE) {
+   pr_debug("(%d) not valid shift state (max_allowed = 
%d)\n", ch,
+SHIFT_TBL_SIZE);
+   return -EINVAL;
+   }
spk_shift_table[ch] = i;
}
keymap_flags = *cp1++;
while ((ch = *cp1)) {
-   if (ch >= MAX_KEY)
-   return -4;
+   if (ch >= MAX_KEY) {
+   pr_debug("(%d), not valid key, (max_allowed = %d)\n", 
ch, MAX_KEY);
+   return -EINVAL;
+   }
spk_our_keys[ch] = cp1;
cp1 += states + 1;
}
-- 
2.10.2

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


[PATCH SPEAKUP v2 2/3] remove unecessary initial allocation of vc

2017-02-28 Thread Pranay Kr. Srivastava
This patch removes the unnecessary allocation of
current foreground vc during initialization.

This initialization is already handled in the loop
that follows it for all available virtual consoles.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/staging/speakup/main.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index a1d5b66..ca817ca 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2317,7 +2317,6 @@ static int __init speakup_init(void)
 {
int i;
long err = 0;
-   struct st_spk_t *first_console;
struct vc_data *vc = vc_cons[fg_console].d;
struct var_t *var;
 
@@ -2342,15 +2341,6 @@ static int __init speakup_init(void)
if (err)
goto error_virtkeyboard;
 
-   first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
-   if (!first_console) {
-   err = -ENOMEM;
-   goto error_alloc;
-   }
-
-   speakup_console[vc->vc_num] = first_console;
-   speakup_date(vc);
-
for (i = 0; i < MAX_NR_CONSOLES; i++)
if (vc_cons[i].d) {
err = speakup_allocate(vc_cons[i].d);
@@ -2412,7 +2402,6 @@ static int __init speakup_init(void)
for (i = 0; i < MAX_NR_CONSOLES; i++)
kfree(speakup_console[i]);
 
-error_alloc:
speakup_remove_virtual_keyboard();
 
 error_virtkeyboard:
-- 
2.10.2

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


Re: [Outreachy kernel] Re: [PATCH v6] staging: xgifb: correct the multiple line dereference

2017-02-28 Thread Julia Lawall


On Tue, 28 Feb 2017, Arushi Singhal wrote:

>
>
> On Tue, Feb 28, 2017 at 11:21 AM, Greg Kroah-Hartman 
>  wrote:
>   On Tue, Feb 28, 2017 at 10:35:30AM +0530, Arushi Singhal wrote:
>   > Error reported by checkpatch.pl as "avoid multiple line dereference".
>   > Addition of new variables to make the code more readable and also to
>   > correct about mentioned error as by itroducing new variables line is
>   > not exceeding 80 characters.
>   >
>   > Signed-off-by: Arushi Singhal 
>   > ---
>   > changes in v6
>   >   - changes done such that no other errors can generate.
>   >   - Improve the coding style.
>   >   - Introduced new variables.
>   >   - type of the variable is changed.
>   >
>   >  drivers/staging/xgifb/XGI_main_26.c | 29 
> ++---
>   >  drivers/staging/xgifb/vb_setmode.c  | 17 +++--
>   >  2 files changed, 17 insertions(+), 29 deletions(-)
>   >
>   > diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/XGI_main_26.c
>   > index 69ed137337ce..9870ea3b76b4 100644
>   > --- a/drivers/staging/xgifb/XGI_main_26.c
>   > +++ b/drivers/staging/xgifb/XGI_main_26.c
>   > @@ -878,30 +878,13 @@ static void XGIfb_post_setmode(struct 
> xgifb_video_info *xgifb_info)
>   >                       }
>   >
>   >                       if ((filter >= 0) && (filter <= 7)) {
>   > +                             const u8 *f = 
> XGI_TV_filter[filter_tb].filter[filter];
>   >                               pr_debug("FilterTable[%d]-%d: %*ph\n",
>   > -                                      filter_tb, filter,
>   > -                                      4, XGI_TV_filter[filter_tb].
>   > -                                                filter[filter]);
>   > -                             xgifb_reg_set(
>   > -                                     XGIPART2,
>   > -                                     0x35,
>   > -                                     (XGI_TV_filter[filter_tb].
>   > -                                             filter[filter][0]));
>   > -                             xgifb_reg_set(
>   > -                                     XGIPART2,
>   > -                                     0x36,
>   > -                                     (XGI_TV_filter[filter_tb].
>   > -                                             filter[filter][1]));
>   > -                             xgifb_reg_set(
>   > -                                     XGIPART2,
>   > -                                     0x37,
>   > -                                     (XGI_TV_filter[filter_tb].
>   > -                                             filter[filter][2]));
>   > -                             xgifb_reg_set(
>   > -                                     XGIPART2,
>   > -                                     0x38,
>   > -                                     (XGI_TV_filter[filter_tb].
>   > -                                             filter[filter][3]));
>   > +                                      filter_tb, filter, 4, f);
>   > +                             xgifb_reg_set(XGIPART2, 0x35, f[0]);
>   > +                             xgifb_reg_set(XGIPART2, 0x36, f[1]);
>   > +                             xgifb_reg_set(XGIPART2, 0x37, f[2]);
>   > +                             xgifb_reg_set(XGIPART2, 0x38, f[3]);
>   >                       }
>   >               }
>   >       }
>   > diff --git a/drivers/staging/xgifb/vb_setmode.c 
> b/drivers/staging/xgifb/vb_setmode.c
>   > index 7c7c8c8f1df3..249a32804c06 100644
>   > --- a/drivers/staging/xgifb/vb_setmode.c
>   > +++ b/drivers/staging/xgifb/vb_setmode.c
>   > @@ -221,8 +221,11 @@ static unsigned char XGI_AjustCRT2Rate(unsigned 
> short ModeIdIndex,
>   >
>   >       for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
>   >              tempbx; (*i)--) {
>   > -             infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
> (*i)].
>   > -                             Ext_InfoFlag;
>   > +             unsigned short j;
>   > +
>   > +             j = XGI330_RefIndex[RefreshRateTableIndex + 
> (*i)].Ext_InfoFlag;
>   > +             infoflag = j;
>   > +
>   >               if (infoflag & tempax)
>   >                       return 1;
>
>
> Why are you using a temporary variable 'j' here?  It's not needed at
> all, and just is confusing to read the code now, don't you agree?
>
>
> I am using temporary variable of small size(character) so that when
> I fixed the multiple line dereference then the line number of characters in a 
> line will
> not increase by 80.

I agree with Greg that this is not a readable solution.  Putting the whole
thing on one line would be better, even if it goes over 80 characters.
Having the field on a li

Re: [PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info

2017-02-28 Thread Samuel Thibault
Pranay Kr. Srivastava, on mar. 28 févr. 2017 13:57:53 +0530, wrote:
> This patch makes spk_set_key_info return -EINVAL
> in case of failure instead of returning 4 different
> values for the type of error that occurred.
> 
> Print the offending values instead as debug message.
> 
> Signed-off-by: Pranay Kr. Srivastava 

Reviewed-by: Samuel Thibault 

> ---
>  drivers/staging/speakup/main.c | 27 +++
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index c2f70ef..a1d5b66 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char 
> *k_buffer)
>   u_char ch, version, num_keys;
>  
>   version = *cp++;
> - if (version != KEY_MAP_VER)
> - return -1;
> + if (version != KEY_MAP_VER) {
> + pr_debug("version found %d should be %d\n",
> +  version, KEY_MAP_VER);
> + return -EINVAL;
> + }
>   num_keys = *cp;
>   states = (int)cp[1];
>   key_data_len = (states + 1) * (num_keys + 1);
> - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf))
> - return -2;
> + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) {
> + pr_debug("too many key_infos (%d over %u)\n",
> +  key_data_len + SHIFT_TBL_SIZE + 4, (unsigned 
> int)(sizeof(spk_key_buf)));
> + return -EINVAL;
> + }
>   memset(k_buffer, 0, SHIFT_TBL_SIZE);
>   memset(spk_our_keys, 0, sizeof(spk_our_keys));
>   spk_shift_table = k_buffer;
> @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char 
> *k_buffer)
>   cp1 += 2;   /* now pointing at shift states */
>   for (i = 1; i <= states; i++) {
>   ch = *cp1++;
> - if (ch >= SHIFT_TBL_SIZE)
> - return -3;
> + if (ch >= SHIFT_TBL_SIZE) {
> + pr_debug("(%d) not valid shift state (max_allowed = 
> %d)\n", ch,
> +  SHIFT_TBL_SIZE);
> + return -EINVAL;
> + }
>   spk_shift_table[ch] = i;
>   }
>   keymap_flags = *cp1++;
>   while ((ch = *cp1)) {
> - if (ch >= MAX_KEY)
> - return -4;
> + if (ch >= MAX_KEY) {
> + pr_debug("(%d), not valid key, (max_allowed = %d)\n", 
> ch, MAX_KEY);
> + return -EINVAL;
> + }
>   spk_our_keys[ch] = cp1;
>   cp1 += states + 1;
>   }
> -- 
> 2.10.2
> 

-- 
Samuel
Progress (n.): The process through which the Internet has evolved from
smart people in front of dumb terminals to dumb people in front of smart
terminals.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH SPEAKUP v2 2/3] remove unecessary initial allocation of vc

2017-02-28 Thread Samuel Thibault
Pranay Kr. Srivastava, on mar. 28 févr. 2017 13:57:54 +0530, wrote:
> This patch removes the unnecessary allocation of
> current foreground vc during initialization.
> 
> This initialization is already handled in the loop
> that follows it for all available virtual consoles.
> 
> Signed-off-by: Pranay Kr. Srivastava 

Reviewed-by: Samuel Thibault 

> ---
>  drivers/staging/speakup/main.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index a1d5b66..ca817ca 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -2317,7 +2317,6 @@ static int __init speakup_init(void)
>  {
>   int i;
>   long err = 0;
> - struct st_spk_t *first_console;
>   struct vc_data *vc = vc_cons[fg_console].d;
>   struct var_t *var;
>  
> @@ -2342,15 +2341,6 @@ static int __init speakup_init(void)
>   if (err)
>   goto error_virtkeyboard;
>  
> - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
> - if (!first_console) {
> - err = -ENOMEM;
> - goto error_alloc;
> - }
> -
> - speakup_console[vc->vc_num] = first_console;
> - speakup_date(vc);
> -
>   for (i = 0; i < MAX_NR_CONSOLES; i++)
>   if (vc_cons[i].d) {
>   err = speakup_allocate(vc_cons[i].d);
> @@ -2412,7 +2402,6 @@ static int __init speakup_init(void)
>   for (i = 0; i < MAX_NR_CONSOLES; i++)
>   kfree(speakup_console[i]);
>  
> -error_alloc:
>   speakup_remove_virtual_keyboard();
>  
>  error_virtkeyboard:
> -- 
> 2.10.2
> 

-- 
Samuel
 ben oui ce serait idiot, mais osb
  -+- m'en fous de faire un truc débile ! -+-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context

2017-02-28 Thread Samuel Thibault
Pranay Kr. Srivastava, on mar. 28 févr. 2017 13:57:55 +0530, wrote:
> speakup_allocate used GFP_ATOMIC for allocations
> even while during initialization due to it's use
> in notifier call.
> 
> Pass GFP_ flags as well to speakup_allocate depending
> on the context it is called in.
> 
> Signed-off-by: Pranay Kr. Srivastava 

Reviewed-by: Samuel Thibault 

> ---
>  drivers/staging/speakup/main.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index ca817ca..ede842e 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, 
> u_char ch, u_short key)
>  }
>  
>  /* Allocation concurrency is protected by the console semaphore */
> -static int speakup_allocate(struct vc_data *vc)
> +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags)
>  {
>   int vc_num;
>  
>   vc_num = vc->vc_num;
>   if (speakup_console[vc_num] == NULL) {
>   speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
> -   GFP_ATOMIC);
> +   gfp_flags);
>   if (speakup_console[vc_num] == NULL)
>   return -ENOMEM;
>   speakup_date(vc);
> @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb,
>   switch (code) {
>   case VT_ALLOCATE:
>   if (vc->vc_mode == KD_TEXT)
> - speakup_allocate(vc);
> + speakup_allocate(vc, GFP_ATOMIC);
>   break;
>   case VT_DEALLOCATE:
>   speakup_deallocate(vc);
> @@ -2343,7 +2343,7 @@ static int __init speakup_init(void)
>  
>   for (i = 0; i < MAX_NR_CONSOLES; i++)
>   if (vc_cons[i].d) {
> - err = speakup_allocate(vc_cons[i].d);
> + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL);
>   if (err)
>   goto error_kobjects;
>   }
> -- 
> 2.10.2
> 

-- 
Samuel
(03:13:14)  bon
(03:13:19)  il est tard :p
(03:13:25)  c'est l'heure de manger
(03:13:38)  hm j'ai mangé à 1h moi, j'ai des horaires raisonnables
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v6] staging: xgifb: correct the multiple line dereference

2017-02-28 Thread Greg Kroah-Hartman
On Tue, Feb 28, 2017 at 01:59:53PM +0530, Arushi Singhal wrote:

Please fix your email client to not send html mail, otherwise the
mailing lists reject them, and your quoting is all messed up :(

> On Tue, Feb 28, 2017 at 11:21 AM, Greg Kroah-Hartman <
> gre...@linuxfoundation.org> wrote:
> 
> On Tue, Feb 28, 2017 at 10:35:30AM +0530, Arushi Singhal wrote:
> > Error reported by checkpatch.pl as "avoid multiple line dereference".
> > Addition of new variables to make the code more readable and also to
> > correct about mentioned error as by itroducing new variables line is
> > not exceeding 80 characters.
> >
> > Signed-off-by: Arushi Singhal 
> > ---
> > changes in v6
> >   - changes done such that no other errors can generate.
> >   - Improve the coding style.
> >   - Introduced new variables.
> >   - type of the variable is changed.
> >
> >  drivers/staging/xgifb/XGI_main_26.c | 29 ++---
> >  drivers/staging/xgifb/vb_setmode.c  | 17 +++--
> >  2 files changed, 17 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/
> XGI_main_26.c
> > index 69ed137337ce..9870ea3b76b4 100644
> > --- a/drivers/staging/xgifb/XGI_main_26.c
> > +++ b/drivers/staging/xgifb/XGI_main_26.c
> > @@ -878,30 +878,13 @@ static void XGIfb_post_setmode(struct
> xgifb_video_info *xgifb_info)
> >                       }
> >
> >                       if ((filter >= 0) && (filter <= 7)) {
> > +                             const u8 *f = XGI_TV_filter[filter_tb].
> filter[filter];
> >                               pr_debug("FilterTable[%d]-%d: %*ph\n",
> > -                                      filter_tb, filter,
> > -                                      4, XGI_TV_filter[filter_tb].
> > -                                                filter[filter]);
> > -                             xgifb_reg_set(
> > -                                     XGIPART2,
> > -                                     0x35,
> > -                                     (XGI_TV_filter[filter_tb].
> > -                                             filter[filter][0]));
> > -                             xgifb_reg_set(
> > -                                     XGIPART2,
> > -                                     0x36,
> > -                                     (XGI_TV_filter[filter_tb].
> > -                                             filter[filter][1]));
> > -                             xgifb_reg_set(
> > -                                     XGIPART2,
> > -                                     0x37,
> > -                                     (XGI_TV_filter[filter_tb].
> > -                                             filter[filter][2]));
> > -                             xgifb_reg_set(
> > -                                     XGIPART2,
> > -                                     0x38,
> > -                                     (XGI_TV_filter[filter_tb].
> > -                                             filter[filter][3]));
> > +                                      filter_tb, filter, 4, f);
> > +                             xgifb_reg_set(XGIPART2, 0x35, f[0]);
> > +                             xgifb_reg_set(XGIPART2, 0x36, f[1]);
> > +                             xgifb_reg_set(XGIPART2, 0x37, f[2]);
> > +                             xgifb_reg_set(XGIPART2, 0x38, f[3]);
> >                       }
> >               }
> >       }
> > diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/
> vb_setmode.c
> > index 7c7c8c8f1df3..249a32804c06 100644
> > --- a/drivers/staging/xgifb/vb_setmode.c
> > +++ b/drivers/staging/xgifb/vb_setmode.c
> > @@ -221,8 +221,11 @@ static unsigned char XGI_AjustCRT2Rate(unsigned
> short ModeIdIndex,
> >
> >       for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
> >              tempbx; (*i)--) {
> > -             infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> > -                             Ext_InfoFlag;
> > +             unsigned short j;
> > +
> > +             j = XGI330_RefIndex[RefreshRateTableIndex +
> (*i)].Ext_InfoFlag;
> > +             infoflag = j;
> > +
> >               if (infoflag & tempax)
> >                       return 1;
> 
> 
> Why are you using a temporary variable 'j' here?  It's not needed at
> all, and just is confusing to read the code now, don't you agree?
> 
> I am using temporary variable of small size(character) so that when
> I fixed the multiple line dereference then the line number of characters in a
> line will not increase by 80.

I know _why_ you are doing it, sorry, the point is that it makes no
sense at all to _do_ that.  Please don't just blindly

Re: [PATCH] staging: fsl-mc: fix warning in DT ranges parser

2017-02-28 Thread Laurentiu Tudor
Hi Arnd,

On 02/27/2017 10:42 PM, Arnd Bergmann wrote:
> The fsl-mc-bus driver in staging contains a copy of the standard
> 'ranges' property parsing algorithm with a hack to treat a missing
> property the same way as an empty one. This code produces false-positive
> warnings for me in an allmodconfig build:
>
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c: In function 'fsl_mc_bus_probe':
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:645:6: error: 'mc_size_cells' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:682:8: error: 'mc_addr_cells' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:644:6: note: 'mc_addr_cells' was 
> declared here
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:684:8: error: 'paddr_cells' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:643:6: note: 'paddr_cells' was 
> declared here
>
> To avoid the warnings, I'm simplifying the argument handling to pass
> the number of valid ranges in the property as the function return code
> rather than passing it by reference. With this change, gcc can see that
> we don't evaluate the cell numbers for an missing ranges property.
>
> Signed-off-by: Arnd Bergmann 

Looks good to me, i've tested it and did not see any issues, so here's an:

Acked-by: Laurentiu Tudor 

---
Thanks & Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v7] staging: xgifb: correct the multiple line dereference

2017-02-28 Thread Arushi Singhal
Error reported by checkpatch.pl as "avoid multiple line dereference".
Addition of new variables to make the code more readable.

Signed-off-by: Arushi Singhal 
---
changes in v7
  - Improve the coding style.
  - Introduced new variables.

 drivers/staging/xgifb/XGI_main_26.c | 29 ++---
 drivers/staging/xgifb/vb_setmode.c  | 11 +--
 2 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c 
b/drivers/staging/xgifb/XGI_main_26.c
index 69ed137337ce..9870ea3b76b4 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -878,30 +878,13 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
*xgifb_info)
}
 
if ((filter >= 0) && (filter <= 7)) {
+   const u8 *f = 
XGI_TV_filter[filter_tb].filter[filter];
pr_debug("FilterTable[%d]-%d: %*ph\n",
-filter_tb, filter,
-4, XGI_TV_filter[filter_tb].
-  filter[filter]);
-   xgifb_reg_set(
-   XGIPART2,
-   0x35,
-   (XGI_TV_filter[filter_tb].
-   filter[filter][0]));
-   xgifb_reg_set(
-   XGIPART2,
-   0x36,
-   (XGI_TV_filter[filter_tb].
-   filter[filter][1]));
-   xgifb_reg_set(
-   XGIPART2,
-   0x37,
-   (XGI_TV_filter[filter_tb].
-   filter[filter][2]));
-   xgifb_reg_set(
-   XGIPART2,
-   0x38,
-   (XGI_TV_filter[filter_tb].
-   filter[filter][3]));
+filter_tb, filter, 4, f);
+   xgifb_reg_set(XGIPART2, 0x35, f[0]);
+   xgifb_reg_set(XGIPART2, 0x36, f[1]);
+   xgifb_reg_set(XGIPART2, 0x37, f[2]);
+   xgifb_reg_set(XGIPART2, 0x38, f[3]);
}
}
}
diff --git a/drivers/staging/xgifb/vb_setmode.c 
b/drivers/staging/xgifb/vb_setmode.c
index 7c7c8c8f1df3..ab84adbc542d 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -221,8 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
 
for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
   tempbx; (*i)--) {
-   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
-   Ext_InfoFlag;
+   infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
(*i)].Ext_InfoFlag;
+
if (infoflag & tempax)
return 1;
 
@@ -231,8 +231,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
}
 
for ((*i) = 0;; (*i)++) {
-   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
-   Ext_InfoFlag;
+   infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
(*i)].Ext_InfoFlag;
+
if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
!= tempbx) {
return 0;
@@ -5092,8 +5092,7 @@ unsigned short XGI_GetRatePtrCRT2(struct 
xgi_hw_device_info *pXGIHWDE,
 
i = 0;
do {
-   if (XGI330_RefIndex[RefreshRateTableIndex + i].
-   ModeID != ModeNo)
+   if (XGI330_RefIndex[RefreshRateTableIndex + i].ModeID != ModeNo)
break;
temp = XGI330_RefIndex[RefreshRateTableIndex + i].Ext_InfoFlag;
temp &= ModeTypeMask;
-- 
2.11.0

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


Re: [PATCH v7] staging: xgifb: correct the multiple line dereference

2017-02-28 Thread Joe Perches
On Tue, 2017-02-28 at 16:15 +0530, Arushi Singhal wrote:
> Error reported by checkpatch.pl as "avoid multiple line dereference".
> Addition of new variables to make the code more readable.

You should probably split this into 2 patches, one for
each file.

What Julia said about finding a better/shorter identifier
than RefreshRateTableIndex is true but I still suggest
something like:

> diff --git a/drivers/staging/xgifb/vb_setmode.c 
> b/drivers/staging/xgifb/vb_setmode.c@@ -221,8 +221,8 @@ static unsigned char 
> XGI_AjustCRT2Rate(unsigned short ModeIdIndex,
>  
>   for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
>  tempbx; (*i)--) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
> (*i)].Ext_InfoFlag;
> +

I suggest something like:
---
 drivers/staging/xgifb/vb_setmode.c | 41 ++
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/xgifb/vb_setmode.c 
b/drivers/staging/xgifb/vb_setmode.c
index 7c7c8c8f1df3..daa3318aab41 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -172,10 +172,12 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
struct vb_device_info *pVBInfo)
 {
unsigned short tempax, tempbx, resinfo, modeflag, infoflag;
+   const struct XGI_Ext2Struct *ext2;
 
modeflag = XGI330_EModeIDTable[ModeIdIndex].Ext_ModeFlag;
resinfo = XGI330_EModeIDTable[ModeIdIndex].Ext_RESINFO;
-   tempbx = XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID;
+   ext2 = &XGI330_RefIndex[RefreshRateTableIndex];
+   tempbx = ext2[*i].ModeID;
tempax = 0;
 
if (pVBInfo->VBInfo & SetCRT2ToRAMDAC) {
@@ -219,10 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
return 0;
}
 
-   for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
-  tempbx; (*i)--) {
-   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
-   Ext_InfoFlag;
+   for (; ext2[*i].ModeID == tempbx; (*i)--) {
+   infoflag = ext2[*i].Ext_InfoFlag;
if (infoflag & tempax)
return 1;
 
@@ -231,12 +231,9 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
}
 
for ((*i) = 0;; (*i)++) {
-   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
-   Ext_InfoFlag;
-   if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
-   != tempbx) {
+   infoflag = ext2[*i].Ext_InfoFlag;
+   if (ext2[*i].ModeID != tempbx)
return 0;
-   }
 
if (infoflag & tempax)
return 1;
@@ -5050,6 +5047,7 @@ unsigned short XGI_GetRatePtrCRT2(struct 
xgi_hw_device_info *pXGIHWDE,
 {
const u8 LCDARefreshIndex[] = {
0x00, 0x00, 0x03, 0x01, 0x01, 0x01, 0x01, 0x00 };
+   const struct XGI_Ext2Struct *ext2;
 
unsigned short RefreshRateTableIndex, i, index, temp;
 
@@ -5073,29 +5071,29 @@ unsigned short XGI_GetRatePtrCRT2(struct 
xgi_hw_device_info *pXGIHWDE,
}
 
RefreshRateTableIndex = XGI330_EModeIDTable[ModeIdIndex].REFindex;
-   ModeNo = XGI330_RefIndex[RefreshRateTableIndex].ModeID;
+   ext2 = &XGI330_RefIndex[RefreshRateTableIndex];
+   ModeNo = ext2->ModeID;
if (pXGIHWDE->jChipType >= XG20) { /* for XG20, XG21, XG27 */
-   if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 800) &&
-   (XGI330_RefIndex[RefreshRateTableIndex].YRes == 600)) {
+   if ((ext2->XRes == 800) &&
+   (ext2->YRes == 600)) {
index++;
}
/* do the similar adjustment like XGISearchCRT1Rate() */
-   if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 1024) &&
-   (XGI330_RefIndex[RefreshRateTableIndex].YRes == 768)) {
+   if ((ext2->XRes == 1024) &&
+   (ext2->YRes == 768)) {
index++;
}
-   if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 1280) &&
-   (XGI330_RefIndex[RefreshRateTableIndex].YRes == 1024)) {
+   if ((ext2->XRes == 1280) &&
+   (ext2->YRes == 1024)) {
index++;
}
}
 
i = 0;
do {
-   if (XGI330_RefIndex[RefreshRateTableIndex + i].
-   ModeID != ModeNo)
+   if (ext2[i].ModeID != ModeNo)
break;
-   temp = XGI330_RefIndex[RefreshRateTableIndex + i].Ext_InfoFlag;
+   temp = ext

Re: [Outreachy kernel] Re: [PATCH v7] staging: xgifb: correct the multiple line dereference

2017-02-28 Thread Julia Lawall


On Tue, 28 Feb 2017, Joe Perches wrote:

> On Tue, 2017-02-28 at 16:15 +0530, Arushi Singhal wrote:
> > Error reported by checkpatch.pl as "avoid multiple line dereference".
> > Addition of new variables to make the code more readable.
>
> You should probably split this into 2 patches, one for
> each file.
>
> What Julia said about finding a better/shorter identifier
> than RefreshRateTableIndex is true but I still suggest
> something like:
>
> > diff --git a/drivers/staging/xgifb/vb_setmode.c 
> > b/drivers/staging/xgifb/vb_setmode.c@@ -221,8 +221,8 @@ static unsigned 
> > char XGI_AjustCRT2Rate(unsigned short ModeIdIndex,
> >
> > for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
> >tempbx; (*i)--) {
> > -   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> > -   Ext_InfoFlag;
> > +   infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
> > (*i)].Ext_InfoFlag;
> > +
>
> I suggest something like:
> ---
>  drivers/staging/xgifb/vb_setmode.c | 41 
> ++
>  1 file changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/xgifb/vb_setmode.c 
> b/drivers/staging/xgifb/vb_setmode.c
> index 7c7c8c8f1df3..daa3318aab41 100644
> --- a/drivers/staging/xgifb/vb_setmode.c
> +++ b/drivers/staging/xgifb/vb_setmode.c
> @@ -172,10 +172,12 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>   struct vb_device_info *pVBInfo)
>  {
>   unsigned short tempax, tempbx, resinfo, modeflag, infoflag;
> + const struct XGI_Ext2Struct *ext2;
>
>   modeflag = XGI330_EModeIDTable[ModeIdIndex].Ext_ModeFlag;
>   resinfo = XGI330_EModeIDTable[ModeIdIndex].Ext_RESINFO;
> - tempbx = XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID;
> + ext2 = &XGI330_RefIndex[RefreshRateTableIndex];
> + tempbx = ext2[*i].ModeID;
>   tempax = 0;
>
>   if (pVBInfo->VBInfo & SetCRT2ToRAMDAC) {
> @@ -219,10 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>   return 0;
>   }
>
> - for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
> -tempbx; (*i)--) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + for (; ext2[*i].ModeID == tempbx; (*i)--) {
> + infoflag = ext2[*i].Ext_InfoFlag;
>   if (infoflag & tempax)
>   return 1;
>
> @@ -231,12 +231,9 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>   }
>
>   for ((*i) = 0;; (*i)++) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> - if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
> - != tempbx) {
> + infoflag = ext2[*i].Ext_InfoFlag;
> + if (ext2[*i].ModeID != tempbx)
>   return 0;
> - }

It's drifting a little bit from the original issue, but the whole *i thing
is not very nice.  The function is used in only one place, and the call
looks like this:

   temp = XGI_AjustCRT2Rate(ModeIdIndex, RefreshRateTableIndex,
 &i, pVBInfo);

immediately followed by:

return RefreshRateTableIndex + i;

temp is never used.  Could the function be made to return a negative error
code on failure and the updated value of i on success?  The (*i) could
become just i, which would even be a little shorter.

julia

>
>   if (infoflag & tempax)
>   return 1;
> @@ -5050,6 +5047,7 @@ unsigned short XGI_GetRatePtrCRT2(struct 
> xgi_hw_device_info *pXGIHWDE,
>  {
>   const u8 LCDARefreshIndex[] = {
>   0x00, 0x00, 0x03, 0x01, 0x01, 0x01, 0x01, 0x00 };
> + const struct XGI_Ext2Struct *ext2;
>
>   unsigned short RefreshRateTableIndex, i, index, temp;
>
> @@ -5073,29 +5071,29 @@ unsigned short XGI_GetRatePtrCRT2(struct 
> xgi_hw_device_info *pXGIHWDE,
>   }
>
>   RefreshRateTableIndex = XGI330_EModeIDTable[ModeIdIndex].REFindex;
> - ModeNo = XGI330_RefIndex[RefreshRateTableIndex].ModeID;
> + ext2 = &XGI330_RefIndex[RefreshRateTableIndex];
> + ModeNo = ext2->ModeID;
>   if (pXGIHWDE->jChipType >= XG20) { /* for XG20, XG21, XG27 */
> - if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 800) &&
> - (XGI330_RefIndex[RefreshRateTableIndex].YRes == 600)) {
> + if ((ext2->XRes == 800) &&
> + (ext2->YRes == 600)) {
>   index++;
>   }
>   /* do the similar adjustment like XGISearchCRT1Rate() */
> - if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 1024) &&
> - (XGI330_RefIndex[RefreshRateTableIndex].YRes == 768)) {
> + if ((ext2->XRes == 1024) &&
> + (ext2->YRes

Re: [Outreachy kernel] Re: [PATCH v7] staging: xgifb: correct the multiple line dereference

2017-02-28 Thread Joe Perches
On Tue, 2017-02-28 at 12:24 +0100, Julia Lawall wrote:
> It's drifting a little bit from the original issue, but the whole *i thing
> is not very nice.  The function is used in only one place, and the call
> looks like this:
> 
>temp = XGI_AjustCRT2Rate(ModeIdIndex, RefreshRateTableIndex,
>&i, pVBInfo);
> 
> immediately followed by:
> 
> return RefreshRateTableIndex + i;
> 
> temp is never used.  Could the function be made to return a negative error
> code on failure and the updated value of i on success?  The (*i) could
> become just i, which would even be a little shorter.

I don't see why not.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: ks7010: Unnecessary parentheses removed and improved coding style.

2017-02-28 Thread Arushi Singhal
Unnecessary parentheses are removed as reported by checkpatch.pl
to make coder nicer and to improve readability.
Also coding style is improved as it's often nicer to read if
 &(foo[0]) is converted to foo like:
 memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN);
 memcpy(ap->bssid, ap_info->bssid, ETH_ALEN);

Signed-off-by: Arushi Singhal 
---
 drivers/staging/ks7010/ks_hostif.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index a533408cf0b9..ae7cf3ffae8e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -113,7 +113,7 @@ int get_current_ap(struct ks_wlan_private *priv, struct 
link_ap_info_t *ap_info)
int rc = 0;
 
DPRINTK(3, "\n");
-   ap = &(priv->current_ap);
+   ap = &priv->current_ap;
 
if ((priv->connect_status & CONNECT_STATUS_MASK) == DISCONNECT_STATUS) {
memset(ap, 0, sizeof(struct local_ap_t));
@@ -121,19 +121,19 @@ int get_current_ap(struct ks_wlan_private *priv, struct 
link_ap_info_t *ap_info)
}
 
/* bssid */
-   memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN);
+   memcpy(ap->bssid, ap_info->bssid, ETH_ALEN);
/* essid */
-   memcpy(&(ap->ssid.body[0]), &(priv->reg.ssid.body[0]),
+   memcpy(ap->ssid.body, priv->reg.ssid.body,
   priv->reg.ssid.size);
ap->ssid.size = priv->reg.ssid.size;
/* rate_set */
-   memcpy(&(ap->rate_set.body[0]), &(ap_info->rate_set.body[0]),
+   memcpy(ap->rate_set.body, ap_info->rate_set.body,
   ap_info->rate_set.size);
ap->rate_set.size = ap_info->rate_set.size;
if (ap_info->ext_rate_set.size) {
/* rate_set */
-   memcpy(&(ap->rate_set.body[ap->rate_set.size]),
-  &(ap_info->ext_rate_set.body[0]),
+   memcpy(&ap->rate_set.body[ap->rate_set.size],
+  ap_info->ext_rate_set.body,
   ap_info->ext_rate_set.size);
ap->rate_set.size += ap_info->ext_rate_set.size;
}
@@ -153,11 +153,11 @@ int get_current_ap(struct ks_wlan_private *priv, struct 
link_ap_info_t *ap_info)
ap->rsn_ie.id = 0x30;
if (ap_info->rsn.size <= RSN_IE_BODY_MAX) {
ap->rsn_ie.size = ap_info->rsn.size;
-   memcpy(&(ap->rsn_ie.body[0]), &(ap_info->rsn.body[0]),
+   memcpy(ap->rsn_ie.body, ap_info->rsn.body,
   ap_info->rsn.size);
} else {
ap->rsn_ie.size = RSN_IE_BODY_MAX;
-   memcpy(&(ap->rsn_ie.body[0]), &(ap_info->rsn.body[0]),
+   memcpy(ap->rsn_ie.body, ap_info->rsn.body,
   RSN_IE_BODY_MAX);
}
} else if ((ap_info->rsn_mode & RSN_MODE_WPA)
@@ -165,11 +165,11 @@ int get_current_ap(struct ks_wlan_private *priv, struct 
link_ap_info_t *ap_info)
ap->wpa_ie.id = 0xdd;
if (ap_info->rsn.size <= RSN_IE_BODY_MAX) {
ap->wpa_ie.size = ap_info->rsn.size;
-   memcpy(&(ap->wpa_ie.body[0]), &(ap_info->rsn.body[0]),
+   memcpy(ap->wpa_ie.body, ap_info->rsn.body,
   ap_info->rsn.size);
} else {
ap->wpa_ie.size = RSN_IE_BODY_MAX;
-   memcpy(&(ap->wpa_ie.body[0]), &(ap_info->rsn.body[0]),
+   memcpy(ap->wpa_ie.body, ap_info->rsn.body,
   RSN_IE_BODY_MAX);
}
} else {
@@ -184,7 +184,7 @@ int get_current_ap(struct ks_wlan_private *priv, struct 
link_ap_info_t *ap_info)
wrqu.ap_addr.sa_family = ARPHRD_ETHER;
if ((priv->connect_status & CONNECT_STATUS_MASK) == CONNECT_STATUS) {
memcpy(wrqu.ap_addr.sa_data,
-  &(priv->current_ap.bssid[0]), ETH_ALEN);
+  priv->current_ap.bssid, ETH_ALEN);
DPRINTK(3,
"IWEVENT: connect bssid=%pM\n", wrqu.ap_addr.sa_data);
wireless_send_event(netdev, SIOCGIWAP, &wrqu, NULL);
@@ -212,7 +212,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct 
ap_info_t *ap_info,
memset(ap, 0, sizeof(struct local_ap_t));
 
/* bssid */
-   memcpy(&(ap->bssid[0]), &(ap_info->bssid[0]), ETH_ALEN);
+   memcpy(ap->bssid, ap_info->bssid, ETH_ALEN);
/* rssi */
ap->rssi = ap_info->rssi;
/* sq */
@@ -224,7 +224,7 @@ int get_ap_information(struct ks_wlan_private *priv, struct 
ap_info_t *ap_info,
/* channel */
ap->channel = ap_info->ch_info;
 
-   bp = &(ap_info->body[0]);
+   bp = ap_info->body;
bsize = ap_info->body_size;
offset = 0;
 
-- 
2.11.0


[PATCH] staging: wilc1000: add check for kmalloc allocation failure.

2017-02-28 Thread Colin King
From: Colin Ian King 

Add a sanity check that wid.val has been allocated, fixes a null
pointer deference on stamac when calling ether_add_copy.

Detected by CoverityScan, CID#1369537 ("Dereference null return value")

Signed-off-by: Colin Ian King 
---
 drivers/staging/wilc1000/host_interface.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index c307cce..e83fa21 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1928,6 +1928,8 @@ static s32 Handle_Get_InActiveTime(struct wilc_vif *vif,
wid.type = WID_STR;
wid.size = ETH_ALEN;
wid.val = kmalloc(wid.size, GFP_KERNEL);
+   if (!wid.val)
+   return -ENOMEM;
 
stamac = wid.val;
ether_addr_copy(stamac, strHostIfStaInactiveT->mac);
-- 
2.10.2

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


[PATCH] staging: wilc1000: remove redundant result < 0 check

2017-02-28 Thread Colin King
From: Colin Ian King 

The check for result < 0 is redundant because at that point result
is always zero, hence we can remove this check and the netdev_err
message.

Detected by CoverityScan, CID#1357147 ("Logically Dead Code")

Signed-off-by: Colin Ian King 
---
 drivers/staging/wilc1000/host_interface.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index e83fa21..f848bb8 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3350,10 +3350,6 @@ int wilc_init(struct net_device *dev, struct host_if_drv 
**hif_drv_handler)
init_completion(&hif_drv->comp_inactive_time);
 
if (clients_count == 0) {
-   if (result < 0) {
-   netdev_err(vif->ndev, "Failed to creat MQ\n");
-   goto _fail_;
-   }
hif_workqueue = create_singlethread_workqueue("WILC_wq");
if (!hif_workqueue) {
netdev_err(vif->ndev, "Failed to create workqueue\n");
-- 
2.10.2

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


Re: [PATCH] staging: fixed style in greybus/tools/loopback_test.c

2017-02-28 Thread Johan Hovold
On Mon, Feb 27, 2017 at 12:52:17PM -0800, Jonathan Bowie wrote:
> fixed a simple style issue in tools/loopback_test.c

Please mention what kind of style issue you're addressing.

Also make sure your patch summary (Subject) is on the format

staging: greybus: loopback_test: add missing space

and remember to CC the staging list (de...@driverdev.osuosl.org).

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


Re: [PATCH-SPEAKUP 1/2] return same error value from spk_set_key_info

2017-02-28 Thread kbuild test robot
Hi Pranay,

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.10 next-20170228]
[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/Pranay-Kr-Srivastava/return-same-error-value-from-spk_set_key_info/20170222-152440
config: i386-randconfig-c0-02281459 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
from include/linux/kernel.h:6,
from drivers/staging/speakup/main.c:21:
   drivers/staging/speakup/main.c: In function 'spk_set_key_info':
>> include/linux/compiler.h:117:18: warning: format '%lu' expects argument of 
>> type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
  __r = !!(cond); \
   ^
   include/linux/dynamic_debug.h:125:2: note: in expansion of macro 'if'
 if (DYNAMIC_DEBUG_BRANCH(descriptor))   \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 
__branch_check__(x, 0))
 ^
   include/linux/dynamic_debug.h:117:2: note: in expansion of macro 'unlikely'
 unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)
 ^
   include/linux/dynamic_debug.h:125:6: note: in expansion of macro 
'DYNAMIC_DEBUG_BRANCH'
 if (DYNAMIC_DEBUG_BRANCH(descriptor))   \
 ^
   include/linux/printk.h:324:2: note: in expansion of macro 'dynamic_pr_debug'
 dynamic_pr_debug(fmt, ##__VA_ARGS__)
 ^
   drivers/staging/speakup/main.c:1228:3: note: in expansion of macro 'pr_debug'
  pr_debug("too many key_infos (%d over %lu)\n",
  ^

vim +117 include/linux/compiler.h

2bcd521a Steven Rostedt  2008-11-21  101};
2bcd521a Steven Rostedt  2008-11-21  102  };
2ed84eeb Steven Rostedt  2008-11-12  103  
2ed84eeb Steven Rostedt  2008-11-12  104  /*
2ed84eeb Steven Rostedt  2008-11-12  105   * Note: DISABLE_BRANCH_PROFILING can 
be used by special lowlevel code
2ed84eeb Steven Rostedt  2008-11-12  106   * to disable branch tracing on a per 
file basis.
2ed84eeb Steven Rostedt  2008-11-12  107   */
d9ad8bc0 Bart Van Assche 2009-04-05  108  #if 
defined(CONFIG_TRACE_BRANCH_PROFILING) \
d9ad8bc0 Bart Van Assche 2009-04-05  109  && 
!defined(DISABLE_BRANCH_PROFILING) && !defined(__CHECKER__)
2ed84eeb Steven Rostedt  2008-11-12  110  void ftrace_likely_update(struct 
ftrace_branch_data *f, int val, int expect);
1f0d69a9 Steven Rostedt  2008-11-12  111  
1f0d69a9 Steven Rostedt  2008-11-12  112  #define likely_notrace(x) 
__builtin_expect(!!(x), 1)
1f0d69a9 Steven Rostedt  2008-11-12  113  #define unlikely_notrace(x)   
__builtin_expect(!!(x), 0)
1f0d69a9 Steven Rostedt  2008-11-12  114  
45b79749 Steven Rostedt  2008-11-21  115  #define __branch_check__(x, expect) 
({\
1f0d69a9 Steven Rostedt  2008-11-12  116int __r;
\
2ed84eeb Steven Rostedt  2008-11-12 @117static struct 
ftrace_branch_data\
1f0d69a9 Steven Rostedt  2008-11-12  118
__attribute__((__aligned__(4))) \
45b79749 Steven Rostedt  2008-11-21  119
__attribute__((section("_ftrace_annotated_branch"))) \
1f0d69a9 Steven Rostedt  2008-11-12  120__f 
= { \
1f0d69a9 Steven Rostedt  2008-11-12  121.func = 
__func__,   \
1f0d69a9 Steven Rostedt  2008-11-12  122.file = 
__FILE__,   \
1f0d69a9 Steven Rostedt  2008-11-12  123.line = 
__LINE__,   \
1f0d69a9 Steven Rostedt  2008-11-12  124};  
\
1f0d69a9 Steven Rostedt  2008-11-12  125__r = 
likely_notrace(x);\

:: The code at line 117 was first introduced by commit
:: 2ed84eeb8808cf3c9f039213ca137ffd7d753f0e trace: rename unlikely profiler 
to branch profiler

:: TO: Steven Rostedt 
:: CC: Ingo Molnar 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all

[PATCH 2/2] Staging: xgifb: vb_setmode.c: Fix checkpath warnings

2017-02-28 Thread Manoj Sawai
Fix the following checkpatch.pl warnings:

1: Avoid multiple line dereference - prefer
   'XGI330_RefIndex[RefreshRateTableIndex+i].ModeID'

2: Avoid multiple line dereference - prefer
   'XGI330_RefIndex[RefreshRateTableIndex+(*i)].Ext_InfoFlag'

Signed-off-by: Manoj Sawai 
---
 drivers/staging/xgifb/vb_setmode.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/xgifb/vb_setmode.c 
b/drivers/staging/xgifb/vb_setmode.c
index c359734..cea128b 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -5090,8 +5090,7 @@ unsigned short XGI_GetRatePtrCRT2(struct 
xgi_hw_device_info *pXGIHWDE,
 
i = 0;
do {
-   if (XGI330_RefIndex[RefreshRateTableIndex + i].
-   ModeID != ModeNo)
+   if (XGI330_RefIndex[RefreshRateTableIndex + i].ModeID != ModeNo)
break;
temp = XGI330_RefIndex[RefreshRateTableIndex + i].Ext_InfoFlag;
temp &= ModeTypeMask;
@@ -5103,8 +5102,7 @@ unsigned short XGI_GetRatePtrCRT2(struct 
xgi_hw_device_info *pXGIHWDE,
} while (index != 0x);
if (!(pVBInfo->VBInfo & SetCRT2ToRAMDAC)) {
if (pVBInfo->VBInfo & SetInSlaveMode) {
-   temp = XGI330_RefIndex[RefreshRateTableIndex + i - 1].
-   Ext_InfoFlag;
+   temp = XGI330_RefIndex[RefreshRateTableIndex + i - 
1].Ext_InfoFlag;
if (temp & InterlaceMode)
i++;
}
-- 
2.7.4

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


[PATCH 1/2] Staging: xgifb: vb_setmode.c: Fix checkpath warning

2017-02-28 Thread Manoj Sawai
Fix the following checkpatch.pl warning:

Avoid multiple line dereference - prefer
'XGI330_RefIndex[RefreshRateTableIndex+(*i)].Ext_InfoFlag'

Signed-off-by: Manoj Sawai 
---
 drivers/staging/xgifb/vb_setmode.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/xgifb/vb_setmode.c 
b/drivers/staging/xgifb/vb_setmode.c
index 7c7c8c8..c359734 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -221,8 +221,7 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
 
for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
   tempbx; (*i)--) {
-   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
-   Ext_InfoFlag;
+   infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
(*i)].Ext_InfoFlag;
if (infoflag & tempax)
return 1;
 
@@ -231,8 +230,7 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
ModeIdIndex,
}
 
for ((*i) = 0;; (*i)++) {
-   infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
-   Ext_InfoFlag;
+   infoflag = XGI330_RefIndex[RefreshRateTableIndex + 
(*i)].Ext_InfoFlag;
if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
!= tempbx) {
return 0;
-- 
2.7.4

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


Re: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

2017-02-28 Thread Vitaly Kuznetsov
Dexuan Cui  writes:

> If the daemon is NOT running at all, when we disable the util device from
> Hyper-V Manager (or sometimes the host can rescind a util device and then
> re-offer it), we'll hang in util_remove -> hv_kvp_deinit ->
> wait_for_completion(&release_event), because this code path doesn't run:
> hvt_op_release -> ... -> kvp_on_reset -> complete(&release_event).
>
> Due to this, we even can't reboot the VM properly.
>
> The patch tracks if the dev file is opened or not, and we only need to
> wait if it's opened.
>
> Fixes: 5a66fecbf6aa ("Drivers: hv: util: kvp: Fix a rescind processing issue")
> Signed-off-by: Dexuan Cui 
> Cc: Vitaly Kuznetsov 
> Cc: "K. Y. Srinivasan" 
> Cc: Haiyang Zhang 
> Cc: Stephen Hemminger 
> ---
>  drivers/hv/hv_fcopy.c   | 5 -
>  drivers/hv/hv_kvp.c | 6 +-
>  drivers/hv/hv_snapshot.c| 5 -
>  drivers/hv/hv_utils_transport.c | 2 ++
>  drivers/hv/hv_utils_transport.h | 1 +
>  5 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
> index 9aee601..545cf43 100644
> --- a/drivers/hv/hv_fcopy.c
> +++ b/drivers/hv/hv_fcopy.c
> @@ -358,8 +358,11 @@ int hv_fcopy_init(struct hv_util_service *srv)
>
>  void hv_fcopy_deinit(void)
>  {
> + bool wait = hvt->dev_opened;
> +
>   fcopy_transaction.state = HVUTIL_DEVICE_DYING;
>   cancel_delayed_work_sync(&fcopy_timeout_work);
>   hvutil_transport_destroy(hvt);
> - wait_for_completion(&release_event);
> + if (wait)
> + wait_for_completion(&release_event);

This is racy I think. We need to prevent openning the device first and
then query its state:

bool wait;

fcopy_transaction.state = HVUTIL_DEVICE_DYING;
/* make sure state is set */
mb();
wait = hvt->dev_opened;
cancel_delayed_work_sync(&fcopy_timeout_work);
hvutil_transport_destroy(hvt);
if (wait)
wait_for_completion(&release_event);

otherwise someone could open the device before we manage to update its
state.

>  }
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> index de26371..15c7873 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -742,10 +742,14 @@ hv_kvp_init(struct hv_util_service *srv)
>
>  void hv_kvp_deinit(void)
>  {
> + bool wait = hvt->dev_opened;
> +
>   kvp_transaction.state = HVUTIL_DEVICE_DYING;
>   cancel_delayed_work_sync(&kvp_host_handshake_work);
>   cancel_delayed_work_sync(&kvp_timeout_work);
>   cancel_work_sync(&kvp_sendkey_work);
>   hvutil_transport_destroy(hvt);
> - wait_for_completion(&release_event);
> +
> + if (wait)
> + wait_for_completion(&release_event);
>  }
> diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
> index bcc03f0..3847f19 100644
> --- a/drivers/hv/hv_snapshot.c
> +++ b/drivers/hv/hv_snapshot.c
> @@ -396,9 +396,12 @@ hv_vss_init(struct hv_util_service *srv)
>
>  void hv_vss_deinit(void)
>  {
> + bool wait = hvt->dev_opened;
> +
>   vss_transaction.state = HVUTIL_DEVICE_DYING;
>   cancel_delayed_work_sync(&vss_timeout_work);
>   cancel_work_sync(&vss_handle_request_work);
>   hvutil_transport_destroy(hvt);
> - wait_for_completion(&release_event);
> + if (wait)
> + wait_for_completion(&release_event);
>  }
> diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
> index c235a95..05e0648 100644
> --- a/drivers/hv/hv_utils_transport.c
> +++ b/drivers/hv/hv_utils_transport.c
> @@ -153,6 +153,7 @@ static int hvt_op_open(struct inode *inode, struct file 
> *file)
>
>   if (issue_reset)
>   hvt_reset(hvt);
> + hvt->dev_opened = (hvt->mode == HVUTIL_TRANSPORT_CHARDEV) && !ret;
>
>   mutex_unlock(&hvt->lock);
>
> @@ -182,6 +183,7 @@ static int hvt_op_release(struct inode *inode, struct 
> file *file)
>* connects back.
>*/
>   hvt_reset(hvt);
> + hvt->dev_opened = false;
>   mutex_unlock(&hvt->lock);
>

Not sure but it seems this may also be racy (what if we query the state
just before we reset it?).

>   if (mode_old == HVUTIL_TRANSPORT_DESTROY)
> diff --git a/drivers/hv/hv_utils_transport.h b/drivers/hv/hv_utils_transport.h
> index d98f522..9871283 100644
> --- a/drivers/hv/hv_utils_transport.h
> +++ b/drivers/hv/hv_utils_transport.h
> @@ -32,6 +32,7 @@ struct hvutil_transport {
>   int mode;   /* hvutil_transport_mode */
>   struct file_operations fops;/* file operations */
>   struct miscdevice mdev; /* misc device */
> + bool   dev_opened;  /* Is the device opened? */
>   struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
>   struct list_head list;  /* hvt_list */
>   int (*on_msg)(void *, int); /* callback on new user message */

I think we can get away without introducing this new flag, e.g. if we
replace rele

RE: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

2017-02-28 Thread Dexuan Cui
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> >  void hv_fcopy_deinit(void)
> >  {
> > +   bool wait = hvt->dev_opened;
> > +
> > fcopy_transaction.state = HVUTIL_DEVICE_DYING;
> > cancel_delayed_work_sync(&fcopy_timeout_work);
> > hvutil_transport_destroy(hvt);
> > -   wait_for_completion(&release_event);
> > +   if (wait)
> > +   wait_for_completion(&release_event);
> 
> This is racy I think. We need to prevent openning the device first and
> then query its state:
> 
>   bool wait;
> 
>   fcopy_transaction.state = HVUTIL_DEVICE_DYING;
> /* make sure state is set */
> mb();
> wait = hvt->dev_opened;
>   cancel_delayed_work_sync(&fcopy_timeout_work);
>   hvutil_transport_destroy(hvt);
> if (wait)
>   wait_for_completion(&release_event);
> 
> otherwise someone could open the device before we manage to update its
> state.

I agree.

> > @@ -182,6 +183,7 @@ static int hvt_op_release(struct inode *inode,
> struct file *file)
> >  * connects back.
> >  */
> > hvt_reset(hvt);
> > +   hvt->dev_opened = false;
> > mutex_unlock(&hvt->lock);
> >
> 
> Not sure but it seems this may also be racy (what if we query the state
> just before we reset it?).

Yeah, I agree.

> > if (mode_old == HVUTIL_TRANSPORT_DESTROY)
> > diff --git a/drivers/hv/hv_utils_transport.h
> b/drivers/hv/hv_utils_transport.h
> > index d98f522..9871283 100644
> > --- a/drivers/hv/hv_utils_transport.h
> > +++ b/drivers/hv/hv_utils_transport.h
> > @@ -32,6 +32,7 @@ struct hvutil_transport {
> > int mode;   /* hvutil_transport_mode */
> > struct file_operations fops;/* file operations */
> > struct miscdevice mdev; /* misc device */
> > +   bool   dev_opened;  /* Is the device opened? */
> > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
> > struct list_head list;  /* hvt_list */
> > int (*on_msg)(void *, int); /* callback on new user message */
> 
> I think we can get away without introducing this new flag, e.g. if we
> replace release_event with an atomic which will hold the state
> (open/closed). This will also elimenate possible races above. I can try
> prototyping a patch if you want me to.
> --
>   Vitaly

Thanks for offering the help! Please do. :-)

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


RE: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

2017-02-28 Thread Dexuan Cui
> From: devel [...] On Behalf Of Dexuan Cui
> > > --- a/drivers/hv/hv_utils_transport.h
> > > +++ b/drivers/hv/hv_utils_transport.h
> > > @@ -32,6 +32,7 @@ struct hvutil_transport {
> > > int mode;   /* hvutil_transport_mode */
> > > struct file_operations fops;/* file operations */
> > > struct miscdevice mdev; /* misc device */
> > > +   bool   dev_opened;  /* Is the device opened? */
> > > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
> > > struct list_head list;  /* hvt_list */
> > > int (*on_msg)(void *, int); /* callback on new user message */
> >
> > I think we can get away without introducing this new flag, e.g. if we
> > replace release_event with an atomic which will hold the state
> > (open/closed). This will also elimenate possible races above. I can try
> > prototyping a patch if you want me to.
> > --
> >   Vitaly
> 
> Thanks for offering the help! Please do. :-)

BTW, IMO I found another potential issue:
In hvt_op_open -> hvt_reset -> kvp_on_reset(), I think we should call
init_completion() instead of complete()?

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


Re: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

2017-02-28 Thread Vitaly Kuznetsov
Dexuan Cui  writes:

>> From: devel [...] On Behalf Of Dexuan Cui
>> > > --- a/drivers/hv/hv_utils_transport.h
>> > > +++ b/drivers/hv/hv_utils_transport.h
>> > > @@ -32,6 +32,7 @@ struct hvutil_transport {
>> > > int mode;   /* hvutil_transport_mode */
>> > > struct file_operations fops;/* file operations */
>> > > struct miscdevice mdev; /* misc device */
>> > > +   bool   dev_opened;  /* Is the device opened? */
>> > > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
>> > > struct list_head list;  /* hvt_list */
>> > > int (*on_msg)(void *, int); /* callback on new user message 
>> > > */
>> >
>> > I think we can get away without introducing this new flag, e.g. if we
>> > replace release_event with an atomic which will hold the state
>> > (open/closed). This will also elimenate possible races above. I can try
>> > prototyping a patch if you want me to.
>> > --
>> >   Vitaly
>> 
>> Thanks for offering the help! Please do. :-)
>
> BTW, IMO I found another potential issue:
> In hvt_op_open -> hvt_reset -> kvp_on_reset(), I think we should call
> init_completion() instead of complete()?
>

To me it looks like we can do better with something different from
struct completion, I'll take a look later today.

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


[PATCH] staging/rtl8192e fixed checkpatch.pl warnings in rtl_wx.c

2017-02-28 Thread Sumantro
Fixed Warning rasied by checkpatch.pl
Signed-off-by: Sumantro 
---
 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index 8ffb458..78eb871 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -475,12 +475,10 @@ static int _rtl92e_wx_set_scan(struct net_device *dev,
return ret;
 }
 
-
 static int _rtl92e_wx_get_scan(struct net_device *dev,
   struct iw_request_info *a,
   union iwreq_data *wrqu, char *b)
 {
-
int ret;
struct r8192_priv *priv = rtllib_priv(dev);
 
@@ -654,7 +652,6 @@ static int _rtl92e_wx_set_wap(struct net_device *dev,
 
 }
 
-
 static int _rtl92e_wx_get_wap(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *extra)
@@ -664,7 +661,6 @@ static int _rtl92e_wx_get_wap(struct net_device *dev,
return rtllib_wx_get_wap(priv->rtllib, info, wrqu, extra);
 }
 
-
 static int _rtl92e_wx_get_enc(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *key)
@@ -831,7 +827,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,
 {
struct r8192_priv *priv = rtllib_priv(dev);
 
-
wrqu->retry.disabled = 0; /* can't be disabled */
 
if ((wrqu->retry.flags & IW_RETRY_TYPE) ==
@@ -969,7 +964,6 @@ static int _rtl92e_wx_set_encode_ext(struct net_device *dev,
priv->rtllib->wx_set_enc = 0;
mutex_unlock(&priv->wx_mutex);
return ret;
-
 }
 
 static int _rtl92e_wx_set_auth(struct net_device *dev,
-- 
2.7.4

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


Re: [PATCH] staging/rtl8192e fixed checkpatch.pl warnings in rtl_wx.c

2017-02-28 Thread Vaishali Thakkar

On Tuesday 28 February 2017 07:21 PM, Sumantro wrote:

Fixed Warning rasied by checkpatch.pl


Hi,

Usually we prefer to have subject stating what change is done
and commit log stating why that change is done instead of
stating tool name. Though you can give credit to the tool
in the commit log after stating the reason behind the change.

Also, subject of a patch follows particular structure. You can
check that with 'git log --pretty=oneline --abbrev-commit
'.

May be you can check this[1] for a reference.

Thanks!

[1] https://kernelnewbies.org/PatchPhilosophy



Signed-off-by: Sumantro 
---
 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index 8ffb458..78eb871 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -475,12 +475,10 @@ static int _rtl92e_wx_set_scan(struct net_device *dev,
return ret;
 }

-
 static int _rtl92e_wx_get_scan(struct net_device *dev,
   struct iw_request_info *a,
   union iwreq_data *wrqu, char *b)
 {
-
int ret;
struct r8192_priv *priv = rtllib_priv(dev);

@@ -654,7 +652,6 @@ static int _rtl92e_wx_set_wap(struct net_device *dev,

 }

-
 static int _rtl92e_wx_get_wap(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *extra)
@@ -664,7 +661,6 @@ static int _rtl92e_wx_get_wap(struct net_device *dev,
return rtllib_wx_get_wap(priv->rtllib, info, wrqu, extra);
 }

-
 static int _rtl92e_wx_get_enc(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *key)
@@ -831,7 +827,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,
 {
struct r8192_priv *priv = rtllib_priv(dev);

-
wrqu->retry.disabled = 0; /* can't be disabled */

if ((wrqu->retry.flags & IW_RETRY_TYPE) ==
@@ -969,7 +964,6 @@ static int _rtl92e_wx_set_encode_ext(struct net_device *dev,
priv->rtllib->wx_set_enc = 0;
mutex_unlock(&priv->wx_mutex);
return ret;
-
 }

 static int _rtl92e_wx_set_auth(struct net_device *dev,



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


Re: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

2017-02-28 Thread Vitaly Kuznetsov
Vitaly Kuznetsov  writes:

> Dexuan Cui  writes:
>
>>> From: devel [...] On Behalf Of Dexuan Cui
>>> > > --- a/drivers/hv/hv_utils_transport.h
>>> > > +++ b/drivers/hv/hv_utils_transport.h
>>> > > @@ -32,6 +32,7 @@ struct hvutil_transport {
>>> > > int mode;   /* hvutil_transport_mode */
>>> > > struct file_operations fops;/* file operations */
>>> > > struct miscdevice mdev; /* misc device */
>>> > > +   bool   dev_opened;  /* Is the device opened? */
>>> > > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
>>> > > struct list_head list;  /* hvt_list */
>>> > > int (*on_msg)(void *, int); /* callback on new user message 
>>> > > */
>>> >
>>> > I think we can get away without introducing this new flag, e.g. if we
>>> > replace release_event with an atomic which will hold the state
>>> > (open/closed). This will also elimenate possible races above. I can try
>>> > prototyping a patch if you want me to.
>>> > --
>>> >   Vitaly
>>> 
>>> Thanks for offering the help! Please do. :-)
>>
>> BTW, IMO I found another potential issue:
>> In hvt_op_open -> hvt_reset -> kvp_on_reset(), I think we should call
>> init_completion() instead of complete()?
>>
>
> To me it looks like we can do better with something different from
> struct completion, I'll take a look later today.

Dexuan,

please take a look at the attached patch. After looking at the code
again it occured to me that it's going to be easier to move release wait
to the transport itself. Lightly tested.

Thanks!

-- 
  Vitaly

>From e1144c6127d4f9acffeb64f40449de365caec51e Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov 
Date: Tue, 28 Feb 2017 15:12:17 +0100
Subject: [PATCH] Drivers: hv: util: move waiting for release to
 hv_utils_transport itself

Waiting for release_event in all three drivers introduced issues on release
as on_reset() hook is not always called. E.g. if the device was never
opened we will never get the completion.

Move the waiting code to hvutil_transport_destroy() and make sure it is
only called when the device is open. hvt->lock serialization should
guarantee the absence of races.

Fixes: 5a66fecbf6aa ("Drivers: hv: util: kvp: Fix a rescind processing issue")
Fixes: 20951c7535b5 ("Drivers: hv: util: Fcopy: Fix a rescind processing issue")
Fixes: d77044d142e9 ("Drivers: hv: util: Backup: Fix a rescind processing issue")
Signed-off-by: Vitaly Kuznetsov 
---
 drivers/hv/hv_fcopy.c   |  4 
 drivers/hv/hv_kvp.c |  4 
 drivers/hv/hv_snapshot.c|  4 
 drivers/hv/hv_utils_transport.c | 12 
 drivers/hv/hv_utils_transport.h |  1 +
 5 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 9aee601..a5596a6 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -71,7 +71,6 @@ static DECLARE_WORK(fcopy_send_work, fcopy_send_data);
 static const char fcopy_devname[] = "vmbus/hv_fcopy";
 static u8 *recv_buffer;
 static struct hvutil_transport *hvt;
-static struct completion release_event;
 /*
  * This state maintains the version number registered by the daemon.
  */
@@ -331,7 +330,6 @@ static void fcopy_on_reset(void)
 
 	if (cancel_delayed_work_sync(&fcopy_timeout_work))
 		fcopy_respond_to_host(HV_E_FAIL);
-	complete(&release_event);
 }
 
 int hv_fcopy_init(struct hv_util_service *srv)
@@ -339,7 +337,6 @@ int hv_fcopy_init(struct hv_util_service *srv)
 	recv_buffer = srv->recv_buffer;
 	fcopy_transaction.recv_channel = srv->channel;
 
-	init_completion(&release_event);
 	/*
 	 * When this driver loads, the user level daemon that
 	 * processes the host requests may not yet be running.
@@ -361,5 +358,4 @@ void hv_fcopy_deinit(void)
 	fcopy_transaction.state = HVUTIL_DEVICE_DYING;
 	cancel_delayed_work_sync(&fcopy_timeout_work);
 	hvutil_transport_destroy(hvt);
-	wait_for_completion(&release_event);
 }
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index de26371..a1adfe2 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -101,7 +101,6 @@ static DECLARE_WORK(kvp_sendkey_work, kvp_send_key);
 static const char kvp_devname[] = "vmbus/hv_kvp";
 static u8 *recv_buffer;
 static struct hvutil_transport *hvt;
-static struct completion release_event;
 /*
  * Register the kernel component with the user-level daemon.
  * As part of this registration, pass the LIC version number.
@@ -714,7 +713,6 @@ static void kvp_on_reset(void)
 	if (cancel_delayed_work_sync(&kvp_timeout_work))
 		kvp_respond_to_host(NULL, HV_E_FAIL);
 	kvp_transaction.state = HVUTIL_DEVICE_INIT;
-	complete(&release_event);
 }
 
 int
@@ -723,7 +721,6 @@ hv_kvp_init(struct hv_util_service *srv)
 	recv_buffer = srv->recv_buffer;
 	kvp_transaction.recv_channel = srv->channel;
 
-	init_completion(&release_event);
 	/*
 	 * When this driver loads, the user level daemon that
 	 * processes the host requests may not yet be running.
@@ -747,5 +744,4 @@ vo

Re: [PATCH 1/2] staging: comedi: cb_pcidas64: refactor out function

2017-02-28 Thread Ian Abbott

On 27/02/17 21:08, Tobin C. Harding wrote:

Function contains code at differing levels of abstraction. This is
shown by the fact that conditional is loop-invariant. Code may be
refactored out into a separate function.

Refactor out loop body to separate function. Do not alter original
code structure of loop body (to ease review). Add local variables to
new function in identical manner to calling function.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/comedi/drivers/cb_pcidas64.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c 
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 3b98193..385f007 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -1480,12 +1480,29 @@ static void init_stc_registers(struct comedi_device 
*dev)
disable_ai_pacing(dev);
 };

+static int alloc_dma_member(int i, struct comedi_device *dev)


It would be better if the parameters were reversed for consistency with 
other functions, where the 'struct comedi_device *' parameter comes first.


Also, the function name could be improved as the function that calls its 
allocates various DMA things.  This is allocating a DMA buffer for the 
the AO subdevice, so perhaps the name could be 'alloc_dma_ao_buffer'.



+{
+   struct pci_dev *pcidev = comedi_to_pci_dev(dev);
+   struct pcidas64_private *devpriv = dev->private;
+
+   devpriv->ao_buffer[i] =
+   dma_alloc_coherent(&pcidev->dev,
+   DMA_BUFFER_SIZE,
+   &devpriv->
+   ao_buffer_bus_addr[i],
+   GFP_KERNEL);
+   if (!devpriv->ao_buffer[i])
+   return -ENOMEM;
+   return 0;
+}
+
 static int alloc_and_init_dma_members(struct comedi_device *dev)
 {
const struct pcidas64_board *board = dev->board_ptr;
struct pci_dev *pcidev = comedi_to_pci_dev(dev);
struct pcidas64_private *devpriv = dev->private;
int i;
+   int ret;

/* allocate pci dma buffers */
for (i = 0; i < ai_dma_ring_count(board); i++) {
@@ -1498,14 +1515,9 @@ static int alloc_and_init_dma_members(struct 
comedi_device *dev)
}
for (i = 0; i < AO_DMA_RING_COUNT; i++) {
if (ao_cmd_is_supported(board)) {
-   devpriv->ao_buffer[i] =
-   dma_alloc_coherent(&pcidev->dev,
-  DMA_BUFFER_SIZE,
-  &devpriv->
-  ao_buffer_bus_addr[i],
-  GFP_KERNEL);
-   if (!devpriv->ao_buffer[i])
-   return -ENOMEM;
+   ret = alloc_dma_member(i, dev);
+   if (ret)
+   return ret; /* -ENOMEM */
}
}
/* allocate dma descriptors */




--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/2] staging: comedi: cb_pcidas64: refactor relocated code

2017-02-28 Thread Ian Abbott

On 27/02/17 21:08, Tobin C. Harding wrote:

Code was moved to a separate function in a previous patch. Code
structure wast maintained to ease review. Code may now be refactored
to ease reading.

Move multi-line dereference onto single line. Give function parameter
more meaningful name. Fix minor alignment issue.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/comedi/drivers/cb_pcidas64.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c 
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 385f007..16d2491 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -1480,18 +1480,17 @@ static void init_stc_registers(struct comedi_device 
*dev)
disable_ai_pacing(dev);
 };

-static int alloc_dma_member(int i, struct comedi_device *dev)
+static int alloc_dma_member(int member, struct comedi_device *dev)
 {
struct pci_dev *pcidev = comedi_to_pci_dev(dev);
struct pcidas64_private *devpriv = dev->private;

-   devpriv->ao_buffer[i] =
+   devpriv->ao_buffer[member] =
dma_alloc_coherent(&pcidev->dev,
-   DMA_BUFFER_SIZE,
-   &devpriv->
-   ao_buffer_bus_addr[i],
-   GFP_KERNEL);
-   if (!devpriv->ao_buffer[i])
+  DMA_BUFFER_SIZE,
+  &devpriv->ao_buffer_bus_addr[member],
+  GFP_KERNEL);
+   if (!devpriv->ao_buffer[member])
return -ENOMEM;
return 0;
 }



This could be merged with the other patch.  Also, the parameter 'member' 
is an index into the list of buffers, so you could call it 'index' or 
'bufidx' or something.


--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: rtl8192e: removed unnecessary white lines from crtl_wx.c

2017-02-28 Thread Sumantro
staging:rtl8192e: removed extra whitelines which were giving warnings 
Signed-off-by: Sumantro 
---
 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index 8ffb458..78eb871 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -475,12 +475,10 @@ static int _rtl92e_wx_set_scan(struct net_device *dev,
return ret;
 }
 
-
 static int _rtl92e_wx_get_scan(struct net_device *dev,
   struct iw_request_info *a,
   union iwreq_data *wrqu, char *b)
 {
-
int ret;
struct r8192_priv *priv = rtllib_priv(dev);
 
@@ -654,7 +652,6 @@ static int _rtl92e_wx_set_wap(struct net_device *dev,
 
 }
 
-
 static int _rtl92e_wx_get_wap(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *extra)
@@ -664,7 +661,6 @@ static int _rtl92e_wx_get_wap(struct net_device *dev,
return rtllib_wx_get_wap(priv->rtllib, info, wrqu, extra);
 }
 
-
 static int _rtl92e_wx_get_enc(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *key)
@@ -831,7 +827,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,
 {
struct r8192_priv *priv = rtllib_priv(dev);
 
-
wrqu->retry.disabled = 0; /* can't be disabled */
 
if ((wrqu->retry.flags & IW_RETRY_TYPE) ==
@@ -969,7 +964,6 @@ static int _rtl92e_wx_set_encode_ext(struct net_device *dev,
priv->rtllib->wx_set_enc = 0;
mutex_unlock(&priv->wx_mutex);
return ret;
-
 }
 
 static int _rtl92e_wx_set_auth(struct net_device *dev,
-- 
2.7.4

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


Re: [PATCH] staging: rtl8192e: removed unnecessary white lines from crtl_wx.c

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 08:16:25PM +0530, Sumantro wrote:
> staging:rtl8192e: removed extra whitelines which were giving warnings 
> Signed-off-by: Sumantro 

We need a blank line before the signed-off-by line, as well as a
"real/full" name being used (unless you use only "Sumantro" on legal
documents.)

Also, you need to be a bit more verbose in the changelog text, no need
to repeat the driver and subsystem name, it's up there in the subject
line.

Look at other patches for the driver for examples of what needs to be
done here.

thanks,

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


[PATCH] scsi: storvsc: Add support for FC rport.

2017-02-28 Thread Cathy Avery
Included in the current storvsc driver for Hyper-V is the ability
to access luns on an FC fabric via a virtualized fiber channel
adapter exposed by the Hyper-V host. The driver also attaches to the FC
transport to allow host and port names to be published under
/sys/class/fc_host/hostX. Current customer tools running on the VM
require that these names be available in the well known standard
location under fc_host/hostX.

A problem arose when attaching to the FC transport. The scsi_scan
code attempts to call fc_user_scan which has basically become a no-op
due to the fact that the virtualized FC device does not expose rports.
At this point you cannot refresh the scsi bus after mapping or unmapping
luns on the SAN without a reboot of the VM.

This patch stubs in an rport per fc_host in storvsc so that the
requirement of a defined rport is now met within the fc_transport and
echo "- - -" > /sys/class/scsi_host/hostX/scan now works.

Signed-off-by: Cathy Avery 
---
 drivers/scsi/storvsc_drv.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 585e54f..6d7b932 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -478,6 +478,9 @@ struct storvsc_device {
 */
u64 node_name;
u64 port_name;
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+   struct fc_rport *rport;
+#endif
 };
 
 struct hv_host_device {
@@ -1823,8 +1826,16 @@ static int storvsc_probe(struct hv_device *device,
}
 #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
if (host->transportt == fc_transport_template) {
+   struct fc_rport_identifiers ids;
+
+   ids.node_name = 0;
+   ids.port_name = 0;
+   ids.port_id = 0;
+   ids.roles |= FC_PORT_ROLE_FCP_TARGET;
+
fc_host_node_name(host) = stor_device->node_name;
fc_host_port_name(host) = stor_device->port_name;
+   stor_device->rport = fc_remote_port_add(host, 0, &ids);
}
 #endif
return 0;
@@ -1854,8 +1865,10 @@ static int storvsc_remove(struct hv_device *dev)
struct Scsi_Host *host = stor_device->host;
 
 #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
-   if (host->transportt == fc_transport_template)
+   if (host->transportt == fc_transport_template) {
+   fc_remote_port_delete(stor_device->rport);
fc_remove_host(host);
+   }
 #endif
scsi_remove_host(host);
storvsc_dev_remove(dev);
-- 
2.5.0

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


[PATCH 0/3] staging: vc04-services: Create new toplevel config VIDEOCORE

2017-02-28 Thread Michael Zoran
The current toplevel menu item for vc04-services has this description:

Kernel to VideoCore communication interface for the
BCM2835 family of products.
Defaults to Y when the Broadcom Videocore services
are included in the build, N otherwise.

1. This isn't quite how things work today. Videocore is always included.
2. It's confusing and I don't believe anything about the brand VideoCore
   is specific to the BCM2835.

So what this patch set does is change the toplevel of the menu to be:

menuconfig VIDEOCORE
bool "Broadcom VideoCore support"
depends on HAS_DMA
depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWAR
E)
default y
help
Support for Broadcom VideoCore services including
the BCM2835 family of products which is used
by the Raspberry PI.

This makes it clear that VideoCore is a general set of Broadcom 
services that may not be exclusive to the BCM2835.

Also, I have kept the BCM2835_VCHIQ config, but changed it to a
simple config rather then a menuconfig.

Finally, I have changed the camera and the audio to select rather 
then depend on BCM2835_VCHIQ.  This allows BCM2835_VCHIQ to be changed
to say a module independenly, but yet the menu still works just like
the original description says that it should.  It is forced in if any
of the specific VideoCore services(like camera) are enabled.

Michael Zoran (3):
  staging: vc04_services: Create new CONFIG_VIDEOCORE setting for
VideoCore services.
  staging: bcm2835-audio: select BCM2835_VCHIQ rather then depending on
it.
  staging: bcm2835-camera: select BCM2835_VCHIQ rather then depending on
it.

 drivers/staging/vc04_services/Kconfig| 15 +++
 drivers/staging/vc04_services/bcm2835-audio/Kconfig  |  3 ++-
 drivers/staging/vc04_services/bcm2835-camera/Kconfig |  2 +-
 3 files changed, 14 insertions(+), 6 deletions(-)

-- 
2.11.0

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


[PATCH 2/3] staging: bcm2835-audio: select BCM2835_VCHIQ rather then depending on it.

2017-02-28 Thread Michael Zoran
Change the audio's dependency on BCM2835_VCHIQ to a select.

Signed-off-by: Michael Zoran 
---
 drivers/staging/vc04_services/bcm2835-audio/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/Kconfig 
b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
index b2e6d90ef1cb..479c9e3ace11 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
+++ b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
@@ -1,7 +1,8 @@
 config SND_BCM2835
 tristate "BCM2835 Audio"
-depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND
+depends on ARCH_BCM2835 && SND
 select SND_PCM
+   select BCM2835_VCHIQ
 help
   Say Y or M if you want to support BCM2835 built in audio
 
-- 
2.11.0

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


[PATCH 3/3] staging: bcm2835-camera: select BCM2835_VCHIQ rather then depending on it.

2017-02-28 Thread Michael Zoran
Change the camera's dependency on BCM2835_VCHIQ to a select.

Signed-off-by: Michael Zoran 
---
 drivers/staging/vc04_services/bcm2835-camera/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/Kconfig 
b/drivers/staging/vc04_services/bcm2835-camera/Kconfig
index 64be42ef344d..25e534cd3fd1 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/Kconfig
+++ b/drivers/staging/vc04_services/bcm2835-camera/Kconfig
@@ -2,8 +2,8 @@ config VIDEO_BCM2835
tristate "BCM2835 Camera"
depends on MEDIA_SUPPORT
depends on VIDEO_V4L2 && (ARCH_BCM2835 || COMPILE_TEST)
-   depends on BCM2835_VCHIQ
depends on ARM
+   select BCM2835_VCHIQ
select VIDEOBUF2_VMALLOC
help
  Say Y here to enable camera host interface devices for
-- 
2.11.0

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


[PATCH 1/3] staging: vc04_services: Create new CONFIG_VIDEOCORE setting for VideoCore services.

2017-02-28 Thread Michael Zoran
Create a new config for Broadcom VideoCore services.

Signed-off-by: Michael Zoran 
---
 drivers/staging/vc04_services/Kconfig | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vc04_services/Kconfig 
b/drivers/staging/vc04_services/Kconfig
index 3b576b0b49ae..08e46a91d365 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -1,16 +1,23 @@
-menuconfig BCM2835_VCHIQ
-   tristate "Videocore VCHIQ"
+menuconfig VIDEOCORE
+   bool "Broadcom VideoCore support"
depends on HAS_DMA
depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && 
!RASPBERRYPI_FIRMWARE)
default y
help
+   Support for Broadcom VideoCore services including
+   the BCM2835 family of products which is used
+   by the Raspberry PI.
+
+if VIDEOCORE
+
+config BCM2835_VCHIQ
+   tristate "BCM2835 VCHIQ"
+   help
Kernel to VideoCore communication interface for the
BCM2835 family of products.
Defaults to Y when the Broadcom Videocore services
are included in the build, N otherwise.
 
-if BCM2835_VCHIQ
-
 source "drivers/staging/vc04_services/bcm2835-audio/Kconfig"
 
 source "drivers/staging/vc04_services/bcm2835-camera/Kconfig"
-- 
2.11.0

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


[PATCH v2] staging: iio: accel: Move header file content to source file

2017-02-28 Thread simran singhal
The contents of the header file are used only by this single
source file. Move content into .c and remove .h.

Signed-off-by: simran singhal 
---

 v2:
   -Removing ifndef and define

 drivers/staging/iio/accel/adis16201.h  | 144 -
 drivers/staging/iio/accel/adis16201_core.c | 142 +++-
 2 files changed, 141 insertions(+), 145 deletions(-)
 delete mode 100644 drivers/staging/iio/accel/adis16201.h

diff --git a/drivers/staging/iio/accel/adis16201.h 
b/drivers/staging/iio/accel/adis16201.h
deleted file mode 100644
index 64844ad..000
--- a/drivers/staging/iio/accel/adis16201.h
+++ /dev/null
@@ -1,144 +0,0 @@
-#ifndef SPI_ADIS16201_H_
-#define SPI_ADIS16201_H_
-
-#define ADIS16201_STARTUP_DELAY220 /* ms */
-
-/* Flash memory write count */
-#define ADIS16201_FLASH_CNT  0x00
-
-/* Output, power supply */
-#define ADIS16201_SUPPLY_OUT 0x02
-
-/* Output, x-axis accelerometer */
-#define ADIS16201_XACCL_OUT  0x04
-
-/* Output, y-axis accelerometer */
-#define ADIS16201_YACCL_OUT  0x06
-
-/* Output, auxiliary ADC input */
-#define ADIS16201_AUX_ADC0x08
-
-/* Output, temperature */
-#define ADIS16201_TEMP_OUT   0x0A
-
-/* Output, x-axis inclination */
-#define ADIS16201_XINCL_OUT  0x0C
-
-/* Output, y-axis inclination */
-#define ADIS16201_YINCL_OUT  0x0E
-
-/* Calibration, x-axis acceleration offset */
-#define ADIS16201_XACCL_OFFS 0x10
-
-/* Calibration, y-axis acceleration offset */
-#define ADIS16201_YACCL_OFFS 0x12
-
-/* x-axis acceleration scale factor */
-#define ADIS16201_XACCL_SCALE0x14
-
-/* y-axis acceleration scale factor */
-#define ADIS16201_YACCL_SCALE0x16
-
-/* Calibration, x-axis inclination offset */
-#define ADIS16201_XINCL_OFFS 0x18
-
-/* Calibration, y-axis inclination offset */
-#define ADIS16201_YINCL_OFFS 0x1A
-
-/* x-axis inclination scale factor */
-#define ADIS16201_XINCL_SCALE0x1C
-
-/* y-axis inclination scale factor */
-#define ADIS16201_YINCL_SCALE0x1E
-
-/* Alarm 1 amplitude threshold */
-#define ADIS16201_ALM_MAG1   0x20
-
-/* Alarm 2 amplitude threshold */
-#define ADIS16201_ALM_MAG2   0x22
-
-/* Alarm 1, sample period */
-#define ADIS16201_ALM_SMPL1  0x24
-
-/* Alarm 2, sample period */
-#define ADIS16201_ALM_SMPL2  0x26
-
-/* Alarm control */
-#define ADIS16201_ALM_CTRL   0x28
-
-/* Auxiliary DAC data */
-#define ADIS16201_AUX_DAC0x30
-
-/* General-purpose digital input/output control */
-#define ADIS16201_GPIO_CTRL  0x32
-
-/* Miscellaneous control */
-#define ADIS16201_MSC_CTRL   0x34
-
-/* Internal sample period (rate) control */
-#define ADIS16201_SMPL_PRD   0x36
-
-/* Operation, filter configuration */
-#define ADIS16201_AVG_CNT0x38
-
-/* Operation, sleep mode control */
-#define ADIS16201_SLP_CNT0x3A
-
-/* Diagnostics, system status register */
-#define ADIS16201_DIAG_STAT  0x3C
-
-/* Operation, system command register */
-#define ADIS16201_GLOB_CMD   0x3E
-
-/* MSC_CTRL */
-
-/* Self-test enable */
-#define ADIS16201_MSC_CTRL_SELF_TEST_ENBIT(8)
-
-/* Data-ready enable: 1 = enabled, 0 = disabled */
-#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-
-/* Data-ready polarity: 1 = active high, 0 = active low */
-#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
-
-/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
-#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1   BIT(0)
-
-/* DIAG_STAT */
-
-/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
-#define ADIS16201_DIAG_STAT_ALARM2BIT(9)
-
-/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
-#define ADIS16201_DIAG_STAT_ALARM1BIT(8)
-
-/* SPI communications failure */
-#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
-
-/* Flash update failure */
-#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
-
-/* Power supply above 3.625 V */
-#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-
-/* Power supply below 3.15 V */
-#define ADIS16201_DIAG_STAT_POWER_LOW_BIT  0
-
-/* GLOB_CMD */
-
-#define ADIS16201_GLOB_CMD_SW_RESETBIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
-
-#define ADIS16201_ERROR_ACTIVE  BIT(14)
-
-enum adis16201_scan {
-   ADIS16201_SCAN_ACC_X,
-   ADIS16201_SCAN_ACC_Y,
-   ADIS16201_SCAN_INCLI_X,
-   ADIS16201_SCAN_INCLI_Y,
-   ADIS16201_SCAN_SUPPLY,
-   ADIS16201_SCAN_AUX_ADC,
-   ADIS16201_SCAN_TEMP,
-};
-
-#endif /* SPI_ADIS16201_H_ */
diff --git a/drivers/staging/iio/accel/adis16201_core.c 
b/drivers/staging/iio/accel/adis16201_core.c
index 7963d4a..dcf8463 100644
--- a/drivers/staging/iio/accel/adis16201_core.c
+++ b/drivers/staging/iio/accel/adis16201_core.c
@@ -20,7 +20,147 @@
 #include 
 #include 
 
-#include "adis16201.h"
+#define ADIS16201_STARTUP_DELAY220 /* ms */
+
+/* Flash memory write count */
+#define ADIS16201_FLASH_CNT  0x00
+
+/* Output, power supply */
+#define ADIS16201_SUPPLY_OUT 0x02
+
+/* Output, x-ax

Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

2017-02-28 Thread SIMRAN SINGHAL
On Tue, Feb 28, 2017 at 2:45 AM, Julia Lawall  wrote:
>
>
> On Mon, 27 Feb 2017, simran singhal wrote:
>
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>>
>> This was done using Coccinelle:
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>  s1
>>
>> Signed-off-by: simran singhal 
>> ---
>>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++
>>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++
>>  2 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
>> b/drivers/staging/sm750fb/ddk750_sii164.c
>> index 259006a..6906598 100644
>> --- a/drivers/staging/sm750fb/ddk750_sii164.c
>> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
>> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
>>   hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & 
>> SII164_DETECT_HOT_PLUG_STATUS_MASK;
>>   if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
>>   return 1;
>> - else
>> - return 0;
>> + return 0;
>
>
> Totally unrelated to what you are doing, but I wonder if these return
> values should be true and false?  Perhaps it would help to see how the
> return values are used at the calling context.
>
you want me to go ahead with what Joe mentioned and also do the same
changes here also.
> julia
>
>>  }
>>
>>  /*
>> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
>>   detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & 
>> SII164_DETECT_MONITOR_STATE_MASK;
>>   if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
>>   return 1;
>> - else
>> - return 0;
>> + return 0;
>>  }
>>
>>  /*
>> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c 
>> b/drivers/staging/sm750fb/ddk750_swi2c.c
>> index a4ac07c..5997349 100644
>> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
>> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
>> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
>>   gpio_data = peek32(sw_i2c_data_gpio_data_reg);
>>   if (gpio_data & (1 << sw_i2c_data_gpio))
>>   return 1;
>> - else
>> - return 0;
>> + return 0;
>>  }
>>
>>  /*
>> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>>
>>   if (i < 0xff)
>>   return 0;
>> - else
>> - return -1;
>> + return -1;
>>  }
>>
>>  /*
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

2017-02-28 Thread Julia Lawall


On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:45 AM, Julia Lawall  wrote:
> >
> >
> > On Mon, 27 Feb 2017, simran singhal wrote:
> >
> >> This patch fixes the checkpatch warning that else is not generally
> >> useful after a break or return.
> >>
> >> This was done using Coccinelle:
> >> @@
> >> expression e2;
> >> statement s1;
> >> @@
> >> if(e2) { ... return ...; }
> >> -else
> >>  s1
> >>
> >> Signed-off-by: simran singhal 
> >> ---
> >>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++
> >>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++
> >>  2 files changed, 4 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
> >> b/drivers/staging/sm750fb/ddk750_sii164.c
> >> index 259006a..6906598 100644
> >> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> >> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> >> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
> >>   hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & 
> >> SII164_DETECT_HOT_PLUG_STATUS_MASK;
> >>   if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
> >>   return 1;
> >> - else
> >> - return 0;
> >> + return 0;
> >
> >
> > Totally unrelated to what you are doing, but I wonder if these return
> > values should be true and false?  Perhaps it would help to see how the
> > return values are used at the calling context.
> >
> you want me to go ahead with what Joe mentioned and also do the same
> changes here also.

I think that it would be best to complete what you have underway already.
When those patches are picked up by Greg, you can look into the
possibility of using true and false.

julia

> > julia
> >
> >>  }
> >>
> >>  /*
> >> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
> >>   detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & 
> >> SII164_DETECT_MONITOR_STATE_MASK;
> >>   if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
> >>   return 1;
> >> - else
> >> - return 0;
> >> + return 0;
> >>  }
> >>
> >>  /*
> >> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c 
> >> b/drivers/staging/sm750fb/ddk750_swi2c.c
> >> index a4ac07c..5997349 100644
> >> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
> >> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
> >> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
> >>   gpio_data = peek32(sw_i2c_data_gpio_data_reg);
> >>   if (gpio_data & (1 << sw_i2c_data_gpio))
> >>   return 1;
> >> - else
> >> - return 0;
> >> + return 0;
> >>  }
> >>
> >>  /*
> >> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
> >>
> >>   if (i < 0xff)
> >>   return 0;
> >> - else
> >> - return -1;
> >> + return -1;
> >>  }
> >>
> >>  /*
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups 
> >> "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an 
> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
> >> To view this discussion on the web visit 
> >> https://groups.google.com/d/msgid/outreachy-kernel/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyNXbn1SZ8Daws7yKkYm-C-U9FHyJ_eRxvuem1Fk%2BMqs%3Dg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

2017-02-28 Thread SIMRAN SINGHAL
On Tue, Feb 28, 2017 at 2:13 AM, Joe Perches  wrote:
> On Tue, 2017-02-28 at 01:51 +0530, SIMRAN SINGHAL wrote:
>> On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches  wrote:
>> > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>> > > This patch fixes the checkpatch warning that else is not generally
>> > > useful after a break or return.
>> >
>> > checkpatch doesn't actually warn for this style
>> >
>> > if (foo)
>> > return bar;
>> > else
>> > return baz;
>> >
>>
>> ok, My bad
>> so, I have to change commit message as checkpatch doesn't warn for this 
>> style.
>
> Perhaps better would be to leave them unchanged instead.
>
>> > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c 
>> > > b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> []
>> > > @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct 
>> > > lnet_process_id id, __u32 ipaddr)
>> > >
>> > >   if (!count)
>> > >   return -ENOENT;
>> > > - else
>> > > - return 0;
>> > > + return 0;
>
> There might be a case for this one.
> error returns are generally in the form
>
> {
> [...]
>
> err = func(...);
> if (err < 0)
> return err;
>
> return 0;
> }
Not sure, what's the problem in removing else as according to me
there is no use of else.

In this case if (if condition) does not satisfy then else condition will
be satisfied and function will return 0.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH 4/5] staging: sm750fb: Remove unnecessary else after return

2017-02-28 Thread SIMRAN SINGHAL
On Wed, Mar 1, 2017 at 12:30 AM, Julia Lawall  wrote:
>
>
> On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:
>
>> On Tue, Feb 28, 2017 at 2:45 AM, Julia Lawall  wrote:
>> >
>> >
>> > On Mon, 27 Feb 2017, simran singhal wrote:
>> >
>> >> This patch fixes the checkpatch warning that else is not generally
>> >> useful after a break or return.
>> >>
>> >> This was done using Coccinelle:
>> >> @@
>> >> expression e2;
>> >> statement s1;
>> >> @@
>> >> if(e2) { ... return ...; }
>> >> -else
>> >>  s1
>> >>
>> >> Signed-off-by: simran singhal 
>> >> ---
>> >>  drivers/staging/sm750fb/ddk750_sii164.c | 6 ++
>> >>  drivers/staging/sm750fb/ddk750_swi2c.c  | 6 ++
>> >>  2 files changed, 4 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
>> >> b/drivers/staging/sm750fb/ddk750_sii164.c
>> >> index 259006a..6906598 100644
>> >> --- a/drivers/staging/sm750fb/ddk750_sii164.c
>> >> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
>> >> @@ -368,8 +368,7 @@ unsigned char sii164IsConnected(void)
>> >>   hotPlugValue = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & 
>> >> SII164_DETECT_HOT_PLUG_STATUS_MASK;
>> >>   if (hotPlugValue == SII164_DETECT_HOT_PLUG_STATUS_ON)
>> >>   return 1;
>> >> - else
>> >> - return 0;
>> >> + return 0;
>> >
>> >
>> > Totally unrelated to what you are doing, but I wonder if these return
>> > values should be true and false?  Perhaps it would help to see how the
>> > return values are used at the calling context.
>> >
>> you want me to go ahead with what Joe mentioned and also do the same
>> changes here also.
>
> I think that it would be best to complete what you have underway already.
> When those patches are picked up by Greg, you can look into the
> possibility of using true and false.
>
Thanks, so for now I can ignore this.
> julia
>
>> > julia
>> >
>> >>  }
>> >>
>> >>  /*
>> >> @@ -387,8 +386,7 @@ unsigned char sii164CheckInterrupt(void)
>> >>   detectReg = i2cReadReg(SII164_I2C_ADDRESS, SII164_DETECT) & 
>> >> SII164_DETECT_MONITOR_STATE_MASK;
>> >>   if (detectReg == SII164_DETECT_MONITOR_STATE_CHANGE)
>> >>   return 1;
>> >> - else
>> >> - return 0;
>> >> + return 0;
>> >>  }
>> >>
>> >>  /*
>> >> diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c 
>> >> b/drivers/staging/sm750fb/ddk750_swi2c.c
>> >> index a4ac07c..5997349 100644
>> >> --- a/drivers/staging/sm750fb/ddk750_swi2c.c
>> >> +++ b/drivers/staging/sm750fb/ddk750_swi2c.c
>> >> @@ -199,8 +199,7 @@ static unsigned char sw_i2c_read_sda(void)
>> >>   gpio_data = peek32(sw_i2c_data_gpio_data_reg);
>> >>   if (gpio_data & (1 << sw_i2c_data_gpio))
>> >>   return 1;
>> >> - else
>> >> - return 0;
>> >> + return 0;
>> >>  }
>> >>
>> >>  /*
>> >> @@ -295,8 +294,7 @@ static long sw_i2c_write_byte(unsigned char data)
>> >>
>> >>   if (i < 0xff)
>> >>   return 0;
>> >> - else
>> >> - return -1;
>> >> + return -1;
>> >>  }
>> >>
>> >>  /*
>> >> --
>> >> 2.7.4
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups 
>> >> "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an 
>> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> >> To view this discussion on the web visit 
>> >> https://groups.google.com/d/msgid/outreachy-kernel/1488219268-3006-4-git-send-email-singhalsimran0%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyNXbn1SZ8Daws7yKkYm-C-U9FHyJ_eRxvuem1Fk%2BMqs%3Dg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

2017-02-28 Thread SIMRAN SINGHAL
On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
 wrote:
> On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches  wrote:
>> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>>> This patch fixes the checkpatch warning that else is not generally
>>> useful after a break or return.
>>
>>> This was done using Coccinelle:
>>> @@
>>> expression e2;
>>> statement s1;
>>> @@
>>> if(e2) { ... return ...; }
>>> -else
>>>  s1
>> []
>>> diff --git a/drivers/staging/gdm724x/gdm_endian.c 
>>> b/drivers/staging/gdm724x/gdm_endian.c
>> []
>>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>>>  {
>>>   if (ed->dev_ed == ENDIANNESS_LITTLE)
>>>   return (__force __dev16)cpu_to_le16(x);
>>> - else
>>> - return (__force __dev16)cpu_to_be16(x);
>>> + return (__force __dev16)cpu_to_be16(x);
>>
>> again, not a checkpatch message for any of the
>> suggested modified hunks.
>>
I am not getting what's the problem in removing else or may be I
am wrong you just want to say that I should change the commit message.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] Re: [PATCH 1/5] staging: lustre: Remove unnecessary else after return

2017-02-28 Thread Julia Lawall


On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:13 AM, Joe Perches  wrote:
> > On Tue, 2017-02-28 at 01:51 +0530, SIMRAN SINGHAL wrote:
> >> On Tue, Feb 28, 2017 at 12:55 AM, Joe Perches  wrote:
> >> > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> >> > > This patch fixes the checkpatch warning that else is not generally
> >> > > useful after a break or return.
> >> >
> >> > checkpatch doesn't actually warn for this style
> >> >
> >> > if (foo)
> >> > return bar;
> >> > else
> >> > return baz;
> >> >
> >>
> >> ok, My bad
> >> so, I have to change commit message as checkpatch doesn't warn for this 
> >> style.
> >
> > Perhaps better would be to leave them unchanged instead.
> >
> >> > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c 
> >> > > b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> > []
> >> > > @@ -1806,8 +1806,7 @@ ksocknal_close_matching_conns(struct 
> >> > > lnet_process_id id, __u32 ipaddr)
> >> > >
> >> > >   if (!count)
> >> > >   return -ENOENT;
> >> > > - else
> >> > > - return 0;
> >> > > + return 0;
> >
> > There might be a case for this one.
> > error returns are generally in the form
> >
> > {
> > [...]
> >
> > err = func(...);
> > if (err < 0)
> > return err;
> >
> > return 0;
> > }
> Not sure, what's the problem in removing else as according to me
> there is no use of else.
>
> In this case if (if condition) does not satisfy then else condition will
> be satisfied and function will return 0.

I think that "there might be a case for" was a positive comment.  In any
case, it looks nicer to me if success is outside of a conditional and
failure is under a conditional.  Or to be more precise, Coccinelle bug
finding rules tend to work better if that property is respected.

julia

>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyP-BRPj4jEFgU%3DB_AV7E4-tJfgaXwwxRhUprLg_4gWQkg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return

2017-02-28 Thread SIMRAN SINGHAL
On Tue, Feb 28, 2017 at 2:49 AM, Julia Lawall  wrote:
>
>
> On Mon, 27 Feb 2017, simran singhal wrote:
>
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>>
>> This was done using Coccinelle:
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>  s1
>
> One might be surprised that the following code was detected using the
> above semantic patch, because in the code below there is no return in the
> if branches.  Actually, as a special feature, when one has an if branch
> that ends in return, Coccinelle will skip through any gotos and see if the
> return is matched afterward.  Indeed it is a common pattern to have
>
> if (...) {
>foo(x);
>bar(y);
>return -ENOMEM;
> }
>
> But the code can also be cut up as eg
>
> if (...) {
>ret = -ENOMEM;
>goto out;
> }
> ...
> out:
>foo(x);
>bar(y);
>return ret;
>
> To avoid having to write multiple patterns for these cases, Coccinelle
> will just jump through the return in the second case, allowing the same
> pattern to match both of them.
>
Julia, Thanks for explaination. Its really helpful.

But I think there is no problem in removing else.
Because it will execute this

 padapter->dvobjpriv.inirp_init(padapter);

when if condition will not satisfy.

> julia
>
>>
>> Signed-off-by: simran singhal 
>> ---
>>  drivers/staging/rtl8712/os_intfs.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8712/os_intfs.c 
>> b/drivers/staging/rtl8712/os_intfs.c
>> index 8836b31..3062167 100644
>> --- a/drivers/staging/rtl8712/os_intfs.c
>> +++ b/drivers/staging/rtl8712/os_intfs.c
>> @@ -411,8 +411,7 @@ static int netdev_open(struct net_device *pnetdev)
>>   goto netdev_open_error;
>>   if (!padapter->dvobjpriv.inirp_init)
>>   goto netdev_open_error;
>> - else
>> - padapter->dvobjpriv.inirp_init(padapter);
>> + padapter->dvobjpriv.inirp_init(padapter);
>>   r8712_set_ps_mode(padapter, padapter->registrypriv.power_mgnt,
>> padapter->registrypriv.smart_ps);
>>   }
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/outreachy-kernel/1488219268-3006-3-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] staging: iio: accel: Move header file content to source file

2017-02-28 Thread Jonathan Cameron
On 28/02/17 18:51, simran singhal wrote:
> The contents of the header file are used only by this single
> source file. Move content into .c and remove .h.
> 
> Signed-off-by: simran singhal 
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
> 
>  v2:
>-Removing ifndef and define
> 
>  drivers/staging/iio/accel/adis16201.h  | 144 
> -
>  drivers/staging/iio/accel/adis16201_core.c | 142 +++-
>  2 files changed, 141 insertions(+), 145 deletions(-)
>  delete mode 100644 drivers/staging/iio/accel/adis16201.h
> 
> diff --git a/drivers/staging/iio/accel/adis16201.h 
> b/drivers/staging/iio/accel/adis16201.h
> deleted file mode 100644
> index 64844ad..000
> --- a/drivers/staging/iio/accel/adis16201.h
> +++ /dev/null
> @@ -1,144 +0,0 @@
> -#ifndef SPI_ADIS16201_H_
> -#define SPI_ADIS16201_H_
> -
> -#define ADIS16201_STARTUP_DELAY  220 /* ms */
> -
> -/* Flash memory write count */
> -#define ADIS16201_FLASH_CNT  0x00
> -
> -/* Output, power supply */
> -#define ADIS16201_SUPPLY_OUT 0x02
> -
> -/* Output, x-axis accelerometer */
> -#define ADIS16201_XACCL_OUT  0x04
> -
> -/* Output, y-axis accelerometer */
> -#define ADIS16201_YACCL_OUT  0x06
> -
> -/* Output, auxiliary ADC input */
> -#define ADIS16201_AUX_ADC0x08
> -
> -/* Output, temperature */
> -#define ADIS16201_TEMP_OUT   0x0A
> -
> -/* Output, x-axis inclination */
> -#define ADIS16201_XINCL_OUT  0x0C
> -
> -/* Output, y-axis inclination */
> -#define ADIS16201_YINCL_OUT  0x0E
> -
> -/* Calibration, x-axis acceleration offset */
> -#define ADIS16201_XACCL_OFFS 0x10
> -
> -/* Calibration, y-axis acceleration offset */
> -#define ADIS16201_YACCL_OFFS 0x12
> -
> -/* x-axis acceleration scale factor */
> -#define ADIS16201_XACCL_SCALE0x14
> -
> -/* y-axis acceleration scale factor */
> -#define ADIS16201_YACCL_SCALE0x16
> -
> -/* Calibration, x-axis inclination offset */
> -#define ADIS16201_XINCL_OFFS 0x18
> -
> -/* Calibration, y-axis inclination offset */
> -#define ADIS16201_YINCL_OFFS 0x1A
> -
> -/* x-axis inclination scale factor */
> -#define ADIS16201_XINCL_SCALE0x1C
> -
> -/* y-axis inclination scale factor */
> -#define ADIS16201_YINCL_SCALE0x1E
> -
> -/* Alarm 1 amplitude threshold */
> -#define ADIS16201_ALM_MAG1   0x20
> -
> -/* Alarm 2 amplitude threshold */
> -#define ADIS16201_ALM_MAG2   0x22
> -
> -/* Alarm 1, sample period */
> -#define ADIS16201_ALM_SMPL1  0x24
> -
> -/* Alarm 2, sample period */
> -#define ADIS16201_ALM_SMPL2  0x26
> -
> -/* Alarm control */
> -#define ADIS16201_ALM_CTRL   0x28
> -
> -/* Auxiliary DAC data */
> -#define ADIS16201_AUX_DAC0x30
> -
> -/* General-purpose digital input/output control */
> -#define ADIS16201_GPIO_CTRL  0x32
> -
> -/* Miscellaneous control */
> -#define ADIS16201_MSC_CTRL   0x34
> -
> -/* Internal sample period (rate) control */
> -#define ADIS16201_SMPL_PRD   0x36
> -
> -/* Operation, filter configuration */
> -#define ADIS16201_AVG_CNT0x38
> -
> -/* Operation, sleep mode control */
> -#define ADIS16201_SLP_CNT0x3A
> -
> -/* Diagnostics, system status register */
> -#define ADIS16201_DIAG_STAT  0x3C
> -
> -/* Operation, system command register */
> -#define ADIS16201_GLOB_CMD   0x3E
> -
> -/* MSC_CTRL */
> -
> -/* Self-test enable */
> -#define ADIS16201_MSC_CTRL_SELF_TEST_EN  BIT(8)
> -
> -/* Data-ready enable: 1 = enabled, 0 = disabled */
> -#define ADIS16201_MSC_CTRL_DATA_RDY_EN   BIT(2)
> -
> -/* Data-ready polarity: 1 = active high, 0 = active low */
> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH   BIT(1)
> -
> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
> -#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
> -
> -/* DIAG_STAT */
> -
> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
> -#define ADIS16201_DIAG_STAT_ALARM2BIT(9)
> -
> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
> -#define ADIS16201_DIAG_STAT_ALARM1BIT(8)
> -
> -/* SPI communications failure */
> -#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
> -
> -/* Flash update failure */
> -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
> -
> -/* Power supply above 3.625 V */
> -#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
> -
> -/* Power supply below 3.15 V */
> -#define ADIS16201_DIAG_STAT_POWER_LOW_BIT  0
> -
> -/* GLOB_CMD */
> -
> -#define ADIS16201_GLOB_CMD_SW_RESET  BIT(7)
> -#define ADIS16201_GLOB_CMD_FACTORY_CAL   BIT(1)
> -
> -#define ADIS16201_ERROR_ACTIVE  BIT(14)
> -
> -enum adis16201_scan {
> - ADIS16201_SCAN_ACC_X,
> - ADIS16201_SCAN_ACC_Y,
> - ADIS16201_SCAN_INCLI_X,
> - ADIS16201_SCAN_INCLI_Y,
> - ADIS16201_SCAN_SUPPLY,
> - ADIS16201_SCAN_AUX_ADC,
> - ADIS16201_SCAN_TEMP,
> -};
> -
> -#endif /* SPI_ADIS16201_H_ */
> diff --git a/dri

Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

2017-02-28 Thread Joe Perches
On Wed, 2017-03-01 at 00:41 +0530, SIMRAN SINGHAL wrote:
> On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
>  wrote:
> > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches  wrote:
> > > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > > This patch fixes the checkpatch warning that else is not generally
> > > > useful after a break or return.
> > > > This was done using Coccinelle:
> > > > @@
> > > > expression e2;
> > > > statement s1;
> > > > @@
> > > > if(e2) { ... return ...; }
> > > > -else
> > > >  s1
> > > 
> > > []
> > > > diff --git a/drivers/staging/gdm724x/gdm_endian.c 
> > > > b/drivers/staging/gdm724x/gdm_endian.c
> > > 
> > > []
> > > > @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 
> > > > x)
> > > >  {
> > > >   if (ed->dev_ed == ENDIANNESS_LITTLE)
> > > >   return (__force __dev16)cpu_to_le16(x);
> > > > - else
> > > > - return (__force __dev16)cpu_to_be16(x);
> > > > + return (__force __dev16)cpu_to_be16(x);
> > > 
> > > again, not a checkpatch message for any of the
> > > suggested modified hunks.
> > > 
> 
> I am not getting what's the problem in removing else or may be I
> am wrong you just want to say that I should change the commit message.

2 things:

1: The commit message is incorrect.
2: This form is fundamentally OK:

if (foo)
return bar;
else
return baz;


So I think this patch is not good.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

2017-02-28 Thread Julia Lawall


On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
>  wrote:
> > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches  wrote:
> >> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> >>> This patch fixes the checkpatch warning that else is not generally
> >>> useful after a break or return.
> >>
> >>> This was done using Coccinelle:
> >>> @@
> >>> expression e2;
> >>> statement s1;
> >>> @@
> >>> if(e2) { ... return ...; }
> >>> -else
> >>>  s1
> >> []
> >>> diff --git a/drivers/staging/gdm724x/gdm_endian.c 
> >>> b/drivers/staging/gdm724x/gdm_endian.c
> >> []
> >>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> >>>  {
> >>>   if (ed->dev_ed == ENDIANNESS_LITTLE)
> >>>   return (__force __dev16)cpu_to_le16(x);
> >>> - else
> >>> - return (__force __dev16)cpu_to_be16(x);
> >>> + return (__force __dev16)cpu_to_be16(x);
> >>
> >> again, not a checkpatch message for any of the
> >> suggested modified hunks.
> >>
> I am not getting what's the problem in removing else or may be I
> am wrong you just want to say that I should change the commit message.

Yes, I think that the issue is just the commit message.  Was it really
checkpatch that motivated you to do this?  Joe maintains checkpatch, and
he doesn't think that it gives such a warning.

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


Re: [Outreachy kernel] [PATCH 3/5] staging: rtl8712: Remove unnecessary else after return

2017-02-28 Thread Julia Lawall


On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 2:49 AM, Julia Lawall  wrote:
> >
> >
> > On Mon, 27 Feb 2017, simran singhal wrote:
> >
> >> This patch fixes the checkpatch warning that else is not generally
> >> useful after a break or return.
> >>
> >> This was done using Coccinelle:
> >> @@
> >> expression e2;
> >> statement s1;
> >> @@
> >> if(e2) { ... return ...; }
> >> -else
> >>  s1
> >
> > One might be surprised that the following code was detected using the
> > above semantic patch, because in the code below there is no return in the
> > if branches.  Actually, as a special feature, when one has an if branch
> > that ends in return, Coccinelle will skip through any gotos and see if the
> > return is matched afterward.  Indeed it is a common pattern to have
> >
> > if (...) {
> >foo(x);
> >bar(y);
> >return -ENOMEM;
> > }
> >
> > But the code can also be cut up as eg
> >
> > if (...) {
> >ret = -ENOMEM;
> >goto out;
> > }
> > ...
> > out:
> >foo(x);
> >bar(y);
> >return ret;
> >
> > To avoid having to write multiple patterns for these cases, Coccinelle
> > will just jump through the return in the second case, allowing the same
> > pattern to match both of them.
> >
> Julia, Thanks for explaination. Its really helpful.
>
> But I think there is no problem in removing else.
> Because it will execute this
>
>  padapter->dvobjpriv.inirp_init(padapter);
>
> when if condition will not satisfy.

It seems ok to me.

julia

>
> > julia
> >
> >>
> >> Signed-off-by: simran singhal 
> >> ---
> >>  drivers/staging/rtl8712/os_intfs.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8712/os_intfs.c 
> >> b/drivers/staging/rtl8712/os_intfs.c
> >> index 8836b31..3062167 100644
> >> --- a/drivers/staging/rtl8712/os_intfs.c
> >> +++ b/drivers/staging/rtl8712/os_intfs.c
> >> @@ -411,8 +411,7 @@ static int netdev_open(struct net_device *pnetdev)
> >>   goto netdev_open_error;
> >>   if (!padapter->dvobjpriv.inirp_init)
> >>   goto netdev_open_error;
> >> - else
> >> - padapter->dvobjpriv.inirp_init(padapter);
> >> + padapter->dvobjpriv.inirp_init(padapter);
> >>   r8712_set_ps_mode(padapter, 
> >> padapter->registrypriv.power_mgnt,
> >> padapter->registrypriv.smart_ps);
> >>   }
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups 
> >> "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an 
> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
> >> To view this discussion on the web visit 
> >> https://groups.google.com/d/msgid/outreachy-kernel/1488219268-3006-3-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/3] staging: vc04_services: Create new CONFIG_VIDEOCORE setting for VideoCore services.

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 10:49:35AM -0800, Michael Zoran wrote:
> Create a new config for Broadcom VideoCore services.
> 
> Signed-off-by: Michael Zoran 
> ---
>  drivers/staging/vc04_services/Kconfig | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/Kconfig 
> b/drivers/staging/vc04_services/Kconfig
> index 3b576b0b49ae..08e46a91d365 100644
> --- a/drivers/staging/vc04_services/Kconfig
> +++ b/drivers/staging/vc04_services/Kconfig
> @@ -1,16 +1,23 @@
> -menuconfig BCM2835_VCHIQ
> - tristate "Videocore VCHIQ"
> +menuconfig VIDEOCORE

How about BCM_VIDEOCORE?  "CONFIG_VIDEOCORE" really is generic, the V4L
developers are going to get mad if they see this :)

thanks,

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


Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

2017-02-28 Thread Julia Lawall


On Tue, 28 Feb 2017, Joe Perches wrote:

> On Wed, 2017-03-01 at 00:41 +0530, SIMRAN SINGHAL wrote:
> > On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> >  wrote:
> > > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches  wrote:
> > > > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > > > This patch fixes the checkpatch warning that else is not generally
> > > > > useful after a break or return.
> > > > > This was done using Coccinelle:
> > > > > @@
> > > > > expression e2;
> > > > > statement s1;
> > > > > @@
> > > > > if(e2) { ... return ...; }
> > > > > -else
> > > > >  s1
> > > >
> > > > []
> > > > > diff --git a/drivers/staging/gdm724x/gdm_endian.c 
> > > > > b/drivers/staging/gdm724x/gdm_endian.c
> > > >
> > > > []
> > > > > @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, 
> > > > > u16 x)
> > > > >  {
> > > > >   if (ed->dev_ed == ENDIANNESS_LITTLE)
> > > > >   return (__force __dev16)cpu_to_le16(x);
> > > > > - else
> > > > > - return (__force __dev16)cpu_to_be16(x);
> > > > > + return (__force __dev16)cpu_to_be16(x);
> > > >
> > > > again, not a checkpatch message for any of the
> > > > suggested modified hunks.
> > > >
> >
> > I am not getting what's the problem in removing else or may be I
> > am wrong you just want to say that I should change the commit message.
>
> 2 things:
>
> 1: The commit message is incorrect.
> 2: This form is fundamentally OK:
>
>   if (foo)
>   return bar;
>   else
>   return baz;
>
>
> So I think this patch is not good.

I agree in this case.  The two branches are quite parallel.  In some of
the other patches, the if was looking for the absence of some resource or
the failure of something, so there was a clear distinction between one
branch being cleanup on failure and the other branch being the continuing
successful computation, even if it is just to return a success indicator.

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


Re: [PATCH v2] staging: iio: accel: Move header file content to source file

2017-02-28 Thread Jonathan Cameron
On 28/02/17 19:17, Jonathan Cameron wrote:
> On 28/02/17 18:51, simran singhal wrote:
>> The contents of the header file are used only by this single
>> source file. Move content into .c and remove .h.
>>
>> Signed-off-by: simran singhal 
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
> 
> Thanks,
> 
> Jonathan

One quick note:

The title should include the name of the driver being modified
as this is probably the bit interested people are most likely
to pick up on in a long list of patches.

I fixed this up.  Thanks,

Jonathan
>> ---
>>
>>  v2:
>>-Removing ifndef and define
>>
>>  drivers/staging/iio/accel/adis16201.h  | 144 
>> -
>>  drivers/staging/iio/accel/adis16201_core.c | 142 
>> +++-
>>  2 files changed, 141 insertions(+), 145 deletions(-)
>>  delete mode 100644 drivers/staging/iio/accel/adis16201.h
>>
>> diff --git a/drivers/staging/iio/accel/adis16201.h 
>> b/drivers/staging/iio/accel/adis16201.h
>> deleted file mode 100644
>> index 64844ad..000
>> --- a/drivers/staging/iio/accel/adis16201.h
>> +++ /dev/null
>> @@ -1,144 +0,0 @@
>> -#ifndef SPI_ADIS16201_H_
>> -#define SPI_ADIS16201_H_
>> -
>> -#define ADIS16201_STARTUP_DELAY 220 /* ms */
>> -
>> -/* Flash memory write count */
>> -#define ADIS16201_FLASH_CNT  0x00
>> -
>> -/* Output, power supply */
>> -#define ADIS16201_SUPPLY_OUT 0x02
>> -
>> -/* Output, x-axis accelerometer */
>> -#define ADIS16201_XACCL_OUT  0x04
>> -
>> -/* Output, y-axis accelerometer */
>> -#define ADIS16201_YACCL_OUT  0x06
>> -
>> -/* Output, auxiliary ADC input */
>> -#define ADIS16201_AUX_ADC0x08
>> -
>> -/* Output, temperature */
>> -#define ADIS16201_TEMP_OUT   0x0A
>> -
>> -/* Output, x-axis inclination */
>> -#define ADIS16201_XINCL_OUT  0x0C
>> -
>> -/* Output, y-axis inclination */
>> -#define ADIS16201_YINCL_OUT  0x0E
>> -
>> -/* Calibration, x-axis acceleration offset */
>> -#define ADIS16201_XACCL_OFFS 0x10
>> -
>> -/* Calibration, y-axis acceleration offset */
>> -#define ADIS16201_YACCL_OFFS 0x12
>> -
>> -/* x-axis acceleration scale factor */
>> -#define ADIS16201_XACCL_SCALE0x14
>> -
>> -/* y-axis acceleration scale factor */
>> -#define ADIS16201_YACCL_SCALE0x16
>> -
>> -/* Calibration, x-axis inclination offset */
>> -#define ADIS16201_XINCL_OFFS 0x18
>> -
>> -/* Calibration, y-axis inclination offset */
>> -#define ADIS16201_YINCL_OFFS 0x1A
>> -
>> -/* x-axis inclination scale factor */
>> -#define ADIS16201_XINCL_SCALE0x1C
>> -
>> -/* y-axis inclination scale factor */
>> -#define ADIS16201_YINCL_SCALE0x1E
>> -
>> -/* Alarm 1 amplitude threshold */
>> -#define ADIS16201_ALM_MAG1   0x20
>> -
>> -/* Alarm 2 amplitude threshold */
>> -#define ADIS16201_ALM_MAG2   0x22
>> -
>> -/* Alarm 1, sample period */
>> -#define ADIS16201_ALM_SMPL1  0x24
>> -
>> -/* Alarm 2, sample period */
>> -#define ADIS16201_ALM_SMPL2  0x26
>> -
>> -/* Alarm control */
>> -#define ADIS16201_ALM_CTRL   0x28
>> -
>> -/* Auxiliary DAC data */
>> -#define ADIS16201_AUX_DAC0x30
>> -
>> -/* General-purpose digital input/output control */
>> -#define ADIS16201_GPIO_CTRL  0x32
>> -
>> -/* Miscellaneous control */
>> -#define ADIS16201_MSC_CTRL   0x34
>> -
>> -/* Internal sample period (rate) control */
>> -#define ADIS16201_SMPL_PRD   0x36
>> -
>> -/* Operation, filter configuration */
>> -#define ADIS16201_AVG_CNT0x38
>> -
>> -/* Operation, sleep mode control */
>> -#define ADIS16201_SLP_CNT0x3A
>> -
>> -/* Diagnostics, system status register */
>> -#define ADIS16201_DIAG_STAT  0x3C
>> -
>> -/* Operation, system command register */
>> -#define ADIS16201_GLOB_CMD   0x3E
>> -
>> -/* MSC_CTRL */
>> -
>> -/* Self-test enable */
>> -#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
>> -
>> -/* Data-ready enable: 1 = enabled, 0 = disabled */
>> -#define ADIS16201_MSC_CTRL_DATA_RDY_EN  BIT(2)
>> -
>> -/* Data-ready polarity: 1 = active high, 0 = active low */
>> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH  BIT(1)
>> -
>> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
>> -#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1BIT(0)
>> -
>> -/* DIAG_STAT */
>> -
>> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
>> -#define ADIS16201_DIAG_STAT_ALARM2BIT(9)
>> -
>> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
>> -#define ADIS16201_DIAG_STAT_ALARM1BIT(8)
>> -
>> -/* SPI communications failure */
>> -#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
>> -
>> -/* Flash update failure */
>> -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
>> -
>> -/* Power supply above 3.625 V */
>> -#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>> -
>> -/* Power supply below 3.15 V */
>> -#define ADIS16201_DIAG_STAT_POWER_LOW_BIT  0
>> -
>> -/* GLOB_CMD */
>> -
>> -#define ADIS16201_GLOB_CMD_SW_RESET 

Re: [PATCH V2 5/9] Staging: rtl8712: rtl871x_mp_ioctl.h - style fix

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 06:12:35PM +1300, Derek Robson wrote:
> Fixed style of block comments
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
> Version #1 introduced lines over 80.
> This version moves comments to be above the line of code.
> 
> 
>  drivers/staging/rtl8712/rtl871x_mp_ioctl.h | 173 
> -
>  1 file changed, 98 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mp_ioctl.h 
> b/drivers/staging/rtl8712/rtl871x_mp_ioctl.h
> index 1102451a733d..d8b443b08bba 100644
> --- a/drivers/staging/rtl8712/rtl871x_mp_ioctl.h
> +++ b/drivers/staging/rtl8712/rtl871x_mp_ioctl.h
> @@ -150,83 +150,105 @@ uint oid_rt_get_power_mode_hdl(
>  #ifdef _RTL871X_MP_IOCTL_C_ /* CAUTION!!! */
>  /* This ifdef _MUST_ be left in!! */
>  static const struct oid_obj_priv oid_rtl_seg_81_80_00[] = {
> - {1, oid_null_function}, /*0x00  OID_RT_PRO_RESET_DUT */
> - {1, oid_rt_pro_set_data_rate_hdl},  /*0x01*/
> - {1, oid_rt_pro_start_test_hdl}, /*0x02*/
> - {1, oid_rt_pro_stop_test_hdl},  /*0x03*/
> - {1, oid_null_function}, /*0x04  OID_RT_PRO_SET_PREAMBLE*/
> - {1, oid_null_function}, /*0x05  OID_RT_PRO_SET_SCRAMBLER*/
> - {1, oid_null_function}, /*0x06  OID_RT_PRO_SET_FILTER_BB*/
> - {1, oid_null_function}, /*0x07
> -  * OID_RT_PRO_SET_MANUAL_DIVERS_BB
> -  */
> - {1, oid_rt_pro_set_channel_direct_call_hdl},/*0x08*/
> - {1, oid_null_function}, /*0x09
> - * OID_RT_PRO_SET_SLEEP_MODE_DIRECT_CALL
> - */
> - {1, oid_null_function}, /*0x0A
> - * OID_RT_PRO_SET_WAKE_MODE_DIRECT_CALL
> - */
> - {1, oid_rt_pro_set_continuous_tx_hdl},  /*0x0B
> - * OID_RT_PRO_SET_TX_CONTINUOUS_DIRECT_CALL
> - */
> - {1, oid_rt_pro_set_single_carrier_tx_hdl}, /*0x0C
> - * OID_RT_PRO_SET_SINGLE_CARRIER_TX_CONTINUOUS
> - */
> - {1, oid_null_function}, /*0x0D
> - * OID_RT_PRO_SET_TX_ANTENNA_BB
> - */
> - {1, oid_rt_pro_set_antenna_bb_hdl}, /*0x0E*/
> - {1, oid_null_function}, /*0x0F  OID_RT_PRO_SET_CR_SCRAMBLER*/
> - {1, oid_null_function}, /*0x10  OID_RT_PRO_SET_CR_NEW_FILTER*/
> - {1, oid_rt_pro_set_tx_power_control_hdl}, /*0x11
> - * OID_RT_PRO_SET_TX_POWER_CONTROL
> - */
> - {1, oid_null_function}, /*0x12  OID_RT_PRO_SET_CR_TX_CONFIG*/
> - {1, oid_null_function}, /*0x13
> -  * OID_RT_PRO_GET_TX_POWER_CONTROL
> -  */
> - {1, oid_null_function}, /*0x14
> -  * OID_RT_PRO_GET_CR_SIGNAL_QUALITY
> -  */
> - {1, oid_null_function}, /*0x15  OID_RT_PRO_SET_CR_SETPOINT*/
> - {1, oid_null_function}, /*0x16  OID_RT_PRO_SET_INTEGRATOR*/
> - {1, oid_null_function}, /*0x17  OID_RT_PRO_SET_SIGNAL_QUALITY*/
> - {1, oid_null_function}, /*0x18  OID_RT_PRO_GET_INTEGRATOR*/
> - {1, oid_null_function}, /*0x19  OID_RT_PRO_GET_SIGNAL_QUALITY*/
> - {1, oid_null_function}, /*0x1A  OID_RT_PRO_QUERY_EEPROM_TYPE*/
> - {1, oid_null_function}, /*0x1B  OID_RT_PRO_WRITE_MAC_ADDRESS*/
> - {1, oid_null_function}, /*0x1C  OID_RT_PRO_READ_MAC_ADDRESS*/
> - {1, oid_null_function}, /*0x1D  OID_RT_PRO_WRITE_CIS_DATA*/
> - {1, oid_null_function}, /*0x1E  OID_RT_PRO_READ_CIS_DATA*/
> - {1, oid_null_function}  /*0x1F  OID_RT_PRO_WRITE_POWER_CONTROL*/
> + /*0x00  OID_RT_PRO_RESET_DUT */

Please always put a ' ' in the comment after and before the '*'.  This
should look like:
/* 0x00 OID_RT_PRO_RESET_DUT */

And this:
> + /*0x04  OID_RT_PRO_SET_PREAMBLE*/

Should look like:
/* 0x04 OID_RT_PRO_SET_PREAMBLE */

thanks,

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


Re: [PATCH v5 1/5] staging: ks7010: fixed warning of avoiding line over 80 characters

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 11:59:36AM +0900, Chetan Sethi wrote:
> This patch fixes warning of line over 80 characters, as issued by
> checkpatch.pl
> 
> Signed-off-by: Chetan Sethi 
> ---
> v2:
>  - split multiple changes across different patches
> v3:
>  - mentioned patch revision in subject
> v4:
>  - modified description to exclude mention of patch number from changelog
> v5:
>  - updated series for removing additional checkpatch error introduced
> 
>  drivers/staging/ks7010/ks_wlan.h | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_wlan.h 
> b/drivers/staging/ks7010/ks_wlan.h
> index 9ab80e1..668202d 100644
> --- a/drivers/staging/ks7010/ks_wlan.h
> +++ b/drivers/staging/ks7010/ks_wlan.h
> @@ -18,10 +18,10 @@
>  #include 
>  #include 
>  
> -#include   /* spinlock_t   
> */
> -#include  /* wait_queue_head_t
> */
> -#include  /* pid_t
> */
> -#include  /* struct net_device_stats,  struct sk_buff 
> */
> +#include   /* spinlock_t */
> +#include  /* wait_queue_head_t */
> +#include  /* pid_t */
> +#include  /* struct net_device_stats,  struct sk_buff */
>  #include 
>  #include 
>  #include /* struct atomic_t */
> @@ -36,7 +36,8 @@
>  
>  #ifdef KS_WLAN_DEBUG
>  #define DPRINTK(n, fmt, args...) \
> - if (KS_WLAN_DEBUG > (n)) printk(KERN_NOTICE "%s: "fmt, 
> __FUNCTION__, ## args)
> + if (KS_WLAN_DEBUG > (n)) \

Why did you not use a tab here to indent this line as you were modifying
it?

thanks,

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


Re: [PATCH v5 1/5] staging: ks7010: fixed warning of avoiding line over 80 characters

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 08:26:19PM +0100, Greg KH wrote:
> On Tue, Feb 28, 2017 at 11:59:36AM +0900, Chetan Sethi wrote:
> > This patch fixes warning of line over 80 characters, as issued by
> > checkpatch.pl
> > 
> > Signed-off-by: Chetan Sethi 
> > ---
> > v2:
> >  - split multiple changes across different patches
> > v3:
> >  - mentioned patch revision in subject
> > v4:
> >  - modified description to exclude mention of patch number from changelog
> > v5:
> >  - updated series for removing additional checkpatch error introduced
> > 
> >  drivers/staging/ks7010/ks_wlan.h | 11 ++-
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/ks7010/ks_wlan.h 
> > b/drivers/staging/ks7010/ks_wlan.h
> > index 9ab80e1..668202d 100644
> > --- a/drivers/staging/ks7010/ks_wlan.h
> > +++ b/drivers/staging/ks7010/ks_wlan.h
> > @@ -18,10 +18,10 @@
> >  #include 
> >  #include 
> >  
> > -#include /* spinlock_t   
> > */
> > -#include/* wait_queue_head_t
> > */
> > -#include/* pid_t
> > */
> > -#include/* struct net_device_stats,  struct 
> > sk_buff */
> > +#include /* spinlock_t */
> > +#include/* wait_queue_head_t */
> > +#include/* pid_t */
> > +#include/* struct net_device_stats,  struct 
> > sk_buff */
> >  #include 
> >  #include 
> >  #include   /* struct atomic_t */
> > @@ -36,7 +36,8 @@
> >  
> >  #ifdef KS_WLAN_DEBUG
> >  #define DPRINTK(n, fmt, args...) \
> > - if (KS_WLAN_DEBUG > (n)) printk(KERN_NOTICE "%s: "fmt, 
> > __FUNCTION__, ## args)
> > + if (KS_WLAN_DEBUG > (n)) \
> 
> Why did you not use a tab here to indent this line as you were modifying
> it?

Oh nevermind, you change this up later on in the series...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] staging: iio: accel: Move header file content to source file

2017-02-28 Thread SIMRAN SINGHAL
On Wed, Mar 1, 2017 at 12:53 AM, Jonathan Cameron  wrote:
> On 28/02/17 19:17, Jonathan Cameron wrote:
>> On 28/02/17 18:51, simran singhal wrote:
>>> The contents of the header file are used only by this single
>>> source file. Move content into .c and remove .h.
>>>
>>> Signed-off-by: simran singhal 
>> Applied to the togreg branch of iio.git and pushed out as testing
>> for the autobuilders to play with it.
>>
>> Thanks,
>>
>> Jonathan
>
> One quick note:
>
> The title should include the name of the driver being modified
> as this is probably the bit interested people are most likely
> to pick up on in a long list of patches.
>
> I fixed this up.  Thanks,
>
> Jonathan

I have included the name of driver being modified.
Due to want me to include the name of file i.e adis16201_core.c

>>> ---
>>>
>>>  v2:
>>>-Removing ifndef and define
>>>
>>>  drivers/staging/iio/accel/adis16201.h  | 144 
>>> -
>>>  drivers/staging/iio/accel/adis16201_core.c | 142 
>>> +++-
>>>  2 files changed, 141 insertions(+), 145 deletions(-)
>>>  delete mode 100644 drivers/staging/iio/accel/adis16201.h
>>>
>>> diff --git a/drivers/staging/iio/accel/adis16201.h 
>>> b/drivers/staging/iio/accel/adis16201.h
>>> deleted file mode 100644
>>> index 64844ad..000
>>> --- a/drivers/staging/iio/accel/adis16201.h
>>> +++ /dev/null
>>> @@ -1,144 +0,0 @@
>>> -#ifndef SPI_ADIS16201_H_
>>> -#define SPI_ADIS16201_H_
>>> -
>>> -#define ADIS16201_STARTUP_DELAY 220 /* ms */
>>> -
>>> -/* Flash memory write count */
>>> -#define ADIS16201_FLASH_CNT  0x00
>>> -
>>> -/* Output, power supply */
>>> -#define ADIS16201_SUPPLY_OUT 0x02
>>> -
>>> -/* Output, x-axis accelerometer */
>>> -#define ADIS16201_XACCL_OUT  0x04
>>> -
>>> -/* Output, y-axis accelerometer */
>>> -#define ADIS16201_YACCL_OUT  0x06
>>> -
>>> -/* Output, auxiliary ADC input */
>>> -#define ADIS16201_AUX_ADC0x08
>>> -
>>> -/* Output, temperature */
>>> -#define ADIS16201_TEMP_OUT   0x0A
>>> -
>>> -/* Output, x-axis inclination */
>>> -#define ADIS16201_XINCL_OUT  0x0C
>>> -
>>> -/* Output, y-axis inclination */
>>> -#define ADIS16201_YINCL_OUT  0x0E
>>> -
>>> -/* Calibration, x-axis acceleration offset */
>>> -#define ADIS16201_XACCL_OFFS 0x10
>>> -
>>> -/* Calibration, y-axis acceleration offset */
>>> -#define ADIS16201_YACCL_OFFS 0x12
>>> -
>>> -/* x-axis acceleration scale factor */
>>> -#define ADIS16201_XACCL_SCALE0x14
>>> -
>>> -/* y-axis acceleration scale factor */
>>> -#define ADIS16201_YACCL_SCALE0x16
>>> -
>>> -/* Calibration, x-axis inclination offset */
>>> -#define ADIS16201_XINCL_OFFS 0x18
>>> -
>>> -/* Calibration, y-axis inclination offset */
>>> -#define ADIS16201_YINCL_OFFS 0x1A
>>> -
>>> -/* x-axis inclination scale factor */
>>> -#define ADIS16201_XINCL_SCALE0x1C
>>> -
>>> -/* y-axis inclination scale factor */
>>> -#define ADIS16201_YINCL_SCALE0x1E
>>> -
>>> -/* Alarm 1 amplitude threshold */
>>> -#define ADIS16201_ALM_MAG1   0x20
>>> -
>>> -/* Alarm 2 amplitude threshold */
>>> -#define ADIS16201_ALM_MAG2   0x22
>>> -
>>> -/* Alarm 1, sample period */
>>> -#define ADIS16201_ALM_SMPL1  0x24
>>> -
>>> -/* Alarm 2, sample period */
>>> -#define ADIS16201_ALM_SMPL2  0x26
>>> -
>>> -/* Alarm control */
>>> -#define ADIS16201_ALM_CTRL   0x28
>>> -
>>> -/* Auxiliary DAC data */
>>> -#define ADIS16201_AUX_DAC0x30
>>> -
>>> -/* General-purpose digital input/output control */
>>> -#define ADIS16201_GPIO_CTRL  0x32
>>> -
>>> -/* Miscellaneous control */
>>> -#define ADIS16201_MSC_CTRL   0x34
>>> -
>>> -/* Internal sample period (rate) control */
>>> -#define ADIS16201_SMPL_PRD   0x36
>>> -
>>> -/* Operation, filter configuration */
>>> -#define ADIS16201_AVG_CNT0x38
>>> -
>>> -/* Operation, sleep mode control */
>>> -#define ADIS16201_SLP_CNT0x3A
>>> -
>>> -/* Diagnostics, system status register */
>>> -#define ADIS16201_DIAG_STAT  0x3C
>>> -
>>> -/* Operation, system command register */
>>> -#define ADIS16201_GLOB_CMD   0x3E
>>> -
>>> -/* MSC_CTRL */
>>> -
>>> -/* Self-test enable */
>>> -#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
>>> -
>>> -/* Data-ready enable: 1 = enabled, 0 = disabled */
>>> -#define ADIS16201_MSC_CTRL_DATA_RDY_EN  BIT(2)
>>> -
>>> -/* Data-ready polarity: 1 = active high, 0 = active low */
>>> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH  BIT(1)
>>> -
>>> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
>>> -#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1BIT(0)
>>> -
>>> -/* DIAG_STAT */
>>> -
>>> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
>>> -#define ADIS16201_DIAG_STAT_ALARM2BIT(9)
>>> -
>>> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
>>> -#define ADIS16201_DIAG_STAT_ALARM1BIT(8)
>>> -
>>> -/* SPI communications failure */
>>> -#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   

Re: [PATCH v2] staging: iio: accel: Move header file content to source file

2017-02-28 Thread SIMRAN SINGHAL
On Wed, Mar 1, 2017 at 1:04 AM, Lars-Peter Clausen  wrote:
> On 02/28/2017 08:32 PM, SIMRAN SINGHAL wrote:
>> On Wed, Mar 1, 2017 at 12:53 AM, Jonathan Cameron  wrote:
>>> On 28/02/17 19:17, Jonathan Cameron wrote:
 On 28/02/17 18:51, simran singhal wrote:
> The contents of the header file are used only by this single
> source file. Move content into .c and remove .h.
>
> Signed-off-by: simran singhal 
 Applied to the togreg branch of iio.git and pushed out as testing
 for the autobuilders to play with it.

 Thanks,

 Jonathan
>>>
>>> One quick note:
>>>
>>> The title should include the name of the driver being modified
>>> as this is probably the bit interested people are most likely
>>> to pick up on in a long list of patches.
>>>
>>> I fixed this up.  Thanks,
>>>
>>> Jonathan
>>
>> I have included the name of driver being modified.
>> Due to want me to include the name of file i.e adis16201_core.c
>
> Not the full filename, but the partnumber. E.g.
>
> 'staging: iio: adis16201: ...'
>

Lars-Peter, I got it.

Jonathan, I got my mistake should I send v3.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] staging: iio: accel: Move header file content to source file

2017-02-28 Thread Lars-Peter Clausen
On 02/28/2017 08:32 PM, SIMRAN SINGHAL wrote:
> On Wed, Mar 1, 2017 at 12:53 AM, Jonathan Cameron  wrote:
>> On 28/02/17 19:17, Jonathan Cameron wrote:
>>> On 28/02/17 18:51, simran singhal wrote:
 The contents of the header file are used only by this single
 source file. Move content into .c and remove .h.

 Signed-off-by: simran singhal 
>>> Applied to the togreg branch of iio.git and pushed out as testing
>>> for the autobuilders to play with it.
>>>
>>> Thanks,
>>>
>>> Jonathan
>>
>> One quick note:
>>
>> The title should include the name of the driver being modified
>> as this is probably the bit interested people are most likely
>> to pick up on in a long list of patches.
>>
>> I fixed this up.  Thanks,
>>
>> Jonathan
> 
> I have included the name of driver being modified.
> Due to want me to include the name of file i.e adis16201_core.c

Not the full filename, but the partnumber. E.g.

'staging: iio: adis16201: ...'

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


Re: [PATCH 1/3] staging: vc04_services: Create new CONFIG_VIDEOCORE setting for VideoCore services.

2017-02-28 Thread Michael Zoran
On Tue, 2017-02-28 at 20:19 +0100, Greg KH wrote:
> On Tue, Feb 28, 2017 at 10:49:35AM -0800, Michael Zoran wrote:
> > Create a new config for Broadcom VideoCore services.
> > 
> > Signed-off-by: Michael Zoran 
> > ---
> >  drivers/staging/vc04_services/Kconfig | 15 +++
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/Kconfig
> > b/drivers/staging/vc04_services/Kconfig
> > index 3b576b0b49ae..08e46a91d365 100644
> > --- a/drivers/staging/vc04_services/Kconfig
> > +++ b/drivers/staging/vc04_services/Kconfig
> > @@ -1,16 +1,23 @@
> > -menuconfig BCM2835_VCHIQ
> > -   tristate "Videocore VCHIQ"
> > +menuconfig VIDEOCORE
> 
> How about BCM_VIDEOCORE?  "CONFIG_VIDEOCORE" really is generic, the
> V4L
> developers are going to get mad if they see this :)
> 
> thanks,
> 
> greg k-h

Sure no problem.  I'll change it to BCM_VIDEOCORE.

Also, I'm not sure if I need the word "services" in the title
description or not.   I'm thinking eventually this stuff is going to
get moved somewhere else like Drivers/Firmware at which point that
would be implied.  

I want to make it clear this isn't talking directly to hardware like
say the VC4 graphics driver does.



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


[PATCH v3] staging: iio: adis16201: Move header file content to source file

2017-02-28 Thread simran singhal
The contents of the header file are used only by this single
source file. Move content into .c and remove .h.

Signed-off-by: simran singhal 
---
 v3:
   -Removing endif as it is not needed
   -Changing subject  

 drivers/staging/iio/accel/adis16201.h  | 144 -
 drivers/staging/iio/accel/adis16201_core.c | 140 +++-
 2 files changed, 139 insertions(+), 145 deletions(-)
 delete mode 100644 drivers/staging/iio/accel/adis16201.h

diff --git a/drivers/staging/iio/accel/adis16201.h 
b/drivers/staging/iio/accel/adis16201.h
deleted file mode 100644
index 64844ad..000
--- a/drivers/staging/iio/accel/adis16201.h
+++ /dev/null
@@ -1,144 +0,0 @@
-#ifndef SPI_ADIS16201_H_
-#define SPI_ADIS16201_H_
-
-#define ADIS16201_STARTUP_DELAY220 /* ms */
-
-/* Flash memory write count */
-#define ADIS16201_FLASH_CNT  0x00
-
-/* Output, power supply */
-#define ADIS16201_SUPPLY_OUT 0x02
-
-/* Output, x-axis accelerometer */
-#define ADIS16201_XACCL_OUT  0x04
-
-/* Output, y-axis accelerometer */
-#define ADIS16201_YACCL_OUT  0x06
-
-/* Output, auxiliary ADC input */
-#define ADIS16201_AUX_ADC0x08
-
-/* Output, temperature */
-#define ADIS16201_TEMP_OUT   0x0A
-
-/* Output, x-axis inclination */
-#define ADIS16201_XINCL_OUT  0x0C
-
-/* Output, y-axis inclination */
-#define ADIS16201_YINCL_OUT  0x0E
-
-/* Calibration, x-axis acceleration offset */
-#define ADIS16201_XACCL_OFFS 0x10
-
-/* Calibration, y-axis acceleration offset */
-#define ADIS16201_YACCL_OFFS 0x12
-
-/* x-axis acceleration scale factor */
-#define ADIS16201_XACCL_SCALE0x14
-
-/* y-axis acceleration scale factor */
-#define ADIS16201_YACCL_SCALE0x16
-
-/* Calibration, x-axis inclination offset */
-#define ADIS16201_XINCL_OFFS 0x18
-
-/* Calibration, y-axis inclination offset */
-#define ADIS16201_YINCL_OFFS 0x1A
-
-/* x-axis inclination scale factor */
-#define ADIS16201_XINCL_SCALE0x1C
-
-/* y-axis inclination scale factor */
-#define ADIS16201_YINCL_SCALE0x1E
-
-/* Alarm 1 amplitude threshold */
-#define ADIS16201_ALM_MAG1   0x20
-
-/* Alarm 2 amplitude threshold */
-#define ADIS16201_ALM_MAG2   0x22
-
-/* Alarm 1, sample period */
-#define ADIS16201_ALM_SMPL1  0x24
-
-/* Alarm 2, sample period */
-#define ADIS16201_ALM_SMPL2  0x26
-
-/* Alarm control */
-#define ADIS16201_ALM_CTRL   0x28
-
-/* Auxiliary DAC data */
-#define ADIS16201_AUX_DAC0x30
-
-/* General-purpose digital input/output control */
-#define ADIS16201_GPIO_CTRL  0x32
-
-/* Miscellaneous control */
-#define ADIS16201_MSC_CTRL   0x34
-
-/* Internal sample period (rate) control */
-#define ADIS16201_SMPL_PRD   0x36
-
-/* Operation, filter configuration */
-#define ADIS16201_AVG_CNT0x38
-
-/* Operation, sleep mode control */
-#define ADIS16201_SLP_CNT0x3A
-
-/* Diagnostics, system status register */
-#define ADIS16201_DIAG_STAT  0x3C
-
-/* Operation, system command register */
-#define ADIS16201_GLOB_CMD   0x3E
-
-/* MSC_CTRL */
-
-/* Self-test enable */
-#define ADIS16201_MSC_CTRL_SELF_TEST_ENBIT(8)
-
-/* Data-ready enable: 1 = enabled, 0 = disabled */
-#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-
-/* Data-ready polarity: 1 = active high, 0 = active low */
-#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
-
-/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
-#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1   BIT(0)
-
-/* DIAG_STAT */
-
-/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
-#define ADIS16201_DIAG_STAT_ALARM2BIT(9)
-
-/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
-#define ADIS16201_DIAG_STAT_ALARM1BIT(8)
-
-/* SPI communications failure */
-#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT   3
-
-/* Flash update failure */
-#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT  2
-
-/* Power supply above 3.625 V */
-#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-
-/* Power supply below 3.15 V */
-#define ADIS16201_DIAG_STAT_POWER_LOW_BIT  0
-
-/* GLOB_CMD */
-
-#define ADIS16201_GLOB_CMD_SW_RESETBIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
-
-#define ADIS16201_ERROR_ACTIVE  BIT(14)
-
-enum adis16201_scan {
-   ADIS16201_SCAN_ACC_X,
-   ADIS16201_SCAN_ACC_Y,
-   ADIS16201_SCAN_INCLI_X,
-   ADIS16201_SCAN_INCLI_Y,
-   ADIS16201_SCAN_SUPPLY,
-   ADIS16201_SCAN_AUX_ADC,
-   ADIS16201_SCAN_TEMP,
-};
-
-#endif /* SPI_ADIS16201_H_ */
diff --git a/drivers/staging/iio/accel/adis16201_core.c 
b/drivers/staging/iio/accel/adis16201_core.c
index 7963d4a..d6c8658 100644
--- a/drivers/staging/iio/accel/adis16201_core.c
+++ b/drivers/staging/iio/accel/adis16201_core.c
@@ -20,7 +20,145 @@
 #include 
 #include 
 
-#include "adis16201.h"
+#define ADIS16201_STARTUP_DELAY220 /* ms */
+
+/* Flash memory write count */
+#define ADIS16201_FLASH_CNT  0x00
+
+/* Output, power supply */
+#define ADIS16201_SUPPLY_O

[PATCH] staging/fbtft: Hush checkpatch.pl warning about unnecessary line continuations.

2017-02-28 Thread Alexander Kapshuk
Use a single fmt string with appropriate verbs as conversion specifiers,
followed by the original string literals and the integer argument
instead of using a backslash to escape a new line embedded inbetween
quoted string literals passed as fmt arguments to dev_err() invoked in
drivers/staging/fbtft/fb_ssd1331.c.

Signed-off-by: Alexander Kapshuk 
---
 drivers/staging/fbtft/fb_ssd1331.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ssd1331.c 
b/drivers/staging/fbtft/fb_ssd1331.c
index 26f24e3..6d03041 100644
--- a/drivers/staging/fbtft/fb_ssd1331.c
+++ b/drivers/staging/fbtft/fb_ssd1331.c
@@ -129,17 +129,19 @@ static int set_gamma(struct fbtft_par *par, u32 *curves)
 
for (i = 0; i < 63; i++) {
if (i > 0 && curves[i] < 2) {
-   dev_err(par->info->device,
-   "Illegal value in Grayscale Lookup Table at 
index %d. " \
-   "Must be greater than 1\n", i);
+   dev_err(par->info->device, "%s %d. %s\n",
+   "Illegal value in Grayscale Lookup Table at 
index",
+   i,
+   "Must be greater than 1");
return -EINVAL;
}
acc += curves[i];
tmp[i] = acc;
if (acc > 180) {
-   dev_err(par->info->device,
-   "Illegal value(s) in Grayscale Lookup Table. " \
-   "At index=%d, the accumulated value has 
exceeded 180\n", i);
+   dev_err(par->info->device, "%s%d, %s\n",
+   "Illegal value(s) in Grayscale Lookup Table. At 
index=",
+   i,
+   "the accumulated value has exceeded 180");
return -EINVAL;
}
}
-- 
2.10.2

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


Re: [PATCH 2/3] staging: bcm2835-audio: select BCM2835_VCHIQ rather then depending on it.

2017-02-28 Thread Stefan Wahren

> Michael Zoran  hat am 28. Februar 2017 um 19:49 
> geschrieben:
> 
> 
> Change the audio's dependency on BCM2835_VCHIQ to a select.
> 
> Signed-off-by: Michael Zoran 
> ---
>  drivers/staging/vc04_services/bcm2835-audio/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/Kconfig 
> b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> index b2e6d90ef1cb..479c9e3ace11 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> +++ b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> @@ -1,7 +1,8 @@
>  config SND_BCM2835
>  tristate "BCM2835 Audio"
> -depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND
> +depends on ARCH_BCM2835 && SND
>  select SND_PCM
> + select BCM2835_VCHIQ
>  help
>Say Y or M if you want to support BCM2835 built in audio
>  

AFAIK "depends on" is perferred instead of "select", because it's causes less 
issues. Please explain in the commit messages instead of the cover letter why 
this patch and patch 3 is necessary.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/3] staging: vc04_services: Create new CONFIG_VIDEOCORE setting for VideoCore services.

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 11:43:16AM -0800, Michael Zoran wrote:
> On Tue, 2017-02-28 at 20:19 +0100, Greg KH wrote:
> > On Tue, Feb 28, 2017 at 10:49:35AM -0800, Michael Zoran wrote:
> > > Create a new config for Broadcom VideoCore services.
> > > 
> > > Signed-off-by: Michael Zoran 
> > > ---
> > >  drivers/staging/vc04_services/Kconfig | 15 +++
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/Kconfig
> > > b/drivers/staging/vc04_services/Kconfig
> > > index 3b576b0b49ae..08e46a91d365 100644
> > > --- a/drivers/staging/vc04_services/Kconfig
> > > +++ b/drivers/staging/vc04_services/Kconfig
> > > @@ -1,16 +1,23 @@
> > > -menuconfig BCM2835_VCHIQ
> > > - tristate "Videocore VCHIQ"
> > > +menuconfig VIDEOCORE
> > 
> > How about BCM_VIDEOCORE?  "CONFIG_VIDEOCORE" really is generic, the
> > V4L
> > developers are going to get mad if they see this :)
> > 
> > thanks,
> > 
> > greg k-h
> 
> Sure no problem.  I'll change it to BCM_VIDEOCORE.
> 
> Also, I'm not sure if I need the word "services" in the title
> description or not.   I'm thinking eventually this stuff is going to
> get moved somewhere else like Drivers/Firmware at which point that
> would be implied.  

It's ok, people don't read it very closely :)

thanks,

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


Re: [PATCH 2/2] staging: comedi: cb_pcidas64: refactor relocated code

2017-02-28 Thread Tobin C. Harding
On Tue, Feb 28, 2017 at 02:36:25PM +, Ian Abbott wrote:
> On 27/02/17 21:08, Tobin C. Harding wrote:
> >Code was moved to a separate function in a previous patch. Code
> >structure wast maintained to ease review. Code may now be refactored
> >to ease reading.
> >
> >Move multi-line dereference onto single line. Give function parameter
> >more meaningful name. Fix minor alignment issue.
> >
> >Signed-off-by: Tobin C. Harding 
> >---
> > drivers/staging/comedi/drivers/cb_pcidas64.c | 13 ++---
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c 
> >b/drivers/staging/comedi/drivers/cb_pcidas64.c
> >index 385f007..16d2491 100644
> >--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
> >+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
> >@@ -1480,18 +1480,17 @@ static void init_stc_registers(struct comedi_device 
> >*dev)
> > disable_ai_pacing(dev);
> > };
> >
> >-static int alloc_dma_member(int i, struct comedi_device *dev)
> >+static int alloc_dma_member(int member, struct comedi_device *dev)
> > {
> > struct pci_dev *pcidev = comedi_to_pci_dev(dev);
> > struct pcidas64_private *devpriv = dev->private;
> >
> >-devpriv->ao_buffer[i] =
> >+devpriv->ao_buffer[member] =
> > dma_alloc_coherent(&pcidev->dev,
> >-DMA_BUFFER_SIZE,
> >-&devpriv->
> >-ao_buffer_bus_addr[i],
> >-GFP_KERNEL);
> >-if (!devpriv->ao_buffer[i])
> >+   DMA_BUFFER_SIZE,
> >+   &devpriv->ao_buffer_bus_addr[member],
> >+   GFP_KERNEL);
> >+if (!devpriv->ao_buffer[member])
> > return -ENOMEM;
> > return 0;
> > }
> >
> 
> This could be merged with the other patch.  Also, the parameter 'member' is
> an index into the list of buffers, so you could call it 'index' or 'bufidx'
> or something.

Ok, thanks for the pointers. I'm going to wait until the merge window
closes then implement as suggested.

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


Re: [PATCH 2/3] staging: bcm2835-audio: select BCM2835_VCHIQ rather then depending on it.

2017-02-28 Thread Michael Zoran
On Tue, 2017-02-28 at 21:18 +0100, Stefan Wahren wrote:
> > Michael Zoran  hat am 28. Februar 2017 um
> > 19:49 geschrieben:
> > 
> > 
> > Change the audio's dependency on BCM2835_VCHIQ to a select.
> > 
> > Signed-off-by: Michael Zoran 
> > ---
> >  drivers/staging/vc04_services/bcm2835-audio/Kconfig | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > index b2e6d90ef1cb..479c9e3ace11 100644
> > --- a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > +++ b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > @@ -1,7 +1,8 @@
> >  config SND_BCM2835
> >  tristate "BCM2835 Audio"
> > -depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND
> > +depends on ARCH_BCM2835 && SND
> >  select SND_PCM
> > +   select BCM2835_VCHIQ
> >  help
> >    Say Y or M if you want to support BCM2835 built in audio
> >  
> 
> AFAIK "depends on" is perferred instead of "select", because it's
> causes less issues. Please explain in the commit messages instead of
> the cover letter why this patch and patch 3 is necessary.

Actually, when I was posting a very small fix to the vc4 driver to the
DRM aliases, I saw some very, very pointed words by Linus during the
4.11 merge directed at someone that used depends when he believed they
should have used select. So it isn't clear exactly what the guidance is
on that.

My understanding is that the issues with select happens if the item
being selected has dependencies itself.  Due to limitations of the
config system, it's possible to select something that can't have it's
dependencies satisfied. In this case BCM2835_VCHIQ doesn't have any
dependencies itself since I moved those to the top menu item.

Let me think about a good way to word this in the commit message.



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


[PATCH v2] staging: rtl8192e: remove unnecessary whitespace in rtl_wx.c

2017-02-28 Thread Sumantro
Remove unncessary whitespaces in rtl_wc.c.

Problem found by checkpatch.pl.

Signed-off-by: Sumantro Mukherjee 
---
Changes since v1:
-Add reason of change
-Add version number to patch
-Add full legal name
---
 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index 8ffb458..78eb871 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -475,12 +475,10 @@ static int _rtl92e_wx_set_scan(struct net_device *dev,
return ret;
 }
 
-
 static int _rtl92e_wx_get_scan(struct net_device *dev,
   struct iw_request_info *a,
   union iwreq_data *wrqu, char *b)
 {
-
int ret;
struct r8192_priv *priv = rtllib_priv(dev);
 
@@ -654,7 +652,6 @@ static int _rtl92e_wx_set_wap(struct net_device *dev,
 
 }
 
-
 static int _rtl92e_wx_get_wap(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *extra)
@@ -664,7 +661,6 @@ static int _rtl92e_wx_get_wap(struct net_device *dev,
return rtllib_wx_get_wap(priv->rtllib, info, wrqu, extra);
 }
 
-
 static int _rtl92e_wx_get_enc(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *key)
@@ -831,7 +827,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,
 {
struct r8192_priv *priv = rtllib_priv(dev);
 
-
wrqu->retry.disabled = 0; /* can't be disabled */
 
if ((wrqu->retry.flags & IW_RETRY_TYPE) ==
@@ -969,7 +964,6 @@ static int _rtl92e_wx_set_encode_ext(struct net_device *dev,
priv->rtllib->wx_set_enc = 0;
mutex_unlock(&priv->wx_mutex);
return ret;
-
 }
 
 static int _rtl92e_wx_set_auth(struct net_device *dev,
-- 
2.7.4

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


Re: [PATCH] staging/fbtft: Hush checkpatch.pl warning about unnecessary line continuations.

2017-02-28 Thread Alexander Kapshuk
On Tue, Feb 28, 2017 at 10:34 PM, Joe Perches  wrote:
> On Tue, 2017-02-28 at 21:52 +0200, Alexander Kapshuk wrote:
>> Use a single fmt string with appropriate verbs as conversion specifiers,
>> followed by the original string literals and the integer argument
>> instead of using a backslash to escape a new line embedded inbetween
>> quoted string literals passed as fmt arguments to dev_err() invoked in
>> drivers/staging/fbtft/fb_ssd1331.c.
>
> trivia:
>
>> diff --git a/drivers/staging/fbtft/fb_ssd1331.c 
>> b/drivers/staging/fbtft/fb_ssd1331.c
>> index 26f24e3..6d03041 100644
>> --- a/drivers/staging/fbtft/fb_ssd1331.c
>> +++ b/drivers/staging/fbtft/fb_ssd1331.c
>> @@ -129,17 +129,19 @@ static int set_gamma(struct fbtft_par *par, u32 
>> *curves)
>>
>>   for (i = 0; i < 63; i++) {
>>   if (i > 0 && curves[i] < 2) {
>> - dev_err(par->info->device,
>> - "Illegal value in Grayscale Lookup Table at 
>> index %d. " \
>> - "Must be greater than 1\n", i);
>> + dev_err(par->info->device, "%s %d. %s\n",
>> + "Illegal value in Grayscale Lookup Table at 
>> index",
>
> Most prefer the use of invalid not illegal.
>

Thanks for your feedback.

I don't mind substituting invalid for illegal, unless the maintainers
preferred to do that themselves instead.
My patch was addressing a checkpatch.pl warning only.

I'll wait a bit more to hear from the maintainers, before going ahead
and sending another patch in.

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


Re: [PATCH] staging/fbtft: Hush checkpatch.pl warning about unnecessary line continuations.

2017-02-28 Thread Joe Perches
On Tue, 2017-02-28 at 21:52 +0200, Alexander Kapshuk wrote:
> Use a single fmt string with appropriate verbs as conversion specifiers,
> followed by the original string literals and the integer argument
> instead of using a backslash to escape a new line embedded inbetween
> quoted string literals passed as fmt arguments to dev_err() invoked in
> drivers/staging/fbtft/fb_ssd1331.c.

trivia:

> diff --git a/drivers/staging/fbtft/fb_ssd1331.c 
> b/drivers/staging/fbtft/fb_ssd1331.c
> index 26f24e3..6d03041 100644
> --- a/drivers/staging/fbtft/fb_ssd1331.c
> +++ b/drivers/staging/fbtft/fb_ssd1331.c
> @@ -129,17 +129,19 @@ static int set_gamma(struct fbtft_par *par, u32 *curves)
>  
>   for (i = 0; i < 63; i++) {
>   if (i > 0 && curves[i] < 2) {
> - dev_err(par->info->device,
> - "Illegal value in Grayscale Lookup Table at 
> index %d. " \
> - "Must be greater than 1\n", i);
> + dev_err(par->info->device, "%s %d. %s\n",
> + "Illegal value in Grayscale Lookup Table at 
> index",

Most prefer the use of invalid not illegal.

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


Re: [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX media driver

2017-02-28 Thread Steve Longerbeam

Hi Rob,


On 02/27/2017 06:38 AM, Rob Herring wrote:

On Wed, Feb 15, 2017 at 06:19:03PM -0800, Steve Longerbeam wrote:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
  Documentation/devicetree/bindings/media/imx.txt | 66 +
  1 file changed, 66 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..fd5af50
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,66 @@
+Freescale i.MX Media Video Device
+=
+
+Video Media Controller node
+---
+
+This is the media controller node for video capture support. It is a
+virtual device that lists the camera serial interface nodes that the
+media device will control.
+
+Required properties:
+- compatible : "fsl,imx-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to camera
+   sensor interface ports of IPU devices
+
+example:
+
+capture-subsystem {
+   compatible = "fsl,capture-subsystem";
+   ports = <&ipu1_csi0>, <&ipu1_csi1>;
+};
+
+fim child node
+--
+
+This is an optional child node of the ipu_csi port nodes. If present and
+available, it enables the Frame Interval Monitor. Its properties can be
+used to modify the method in which the FIM measures frame intervals.
+Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
+Frame Interval Monitor.

This should have a compatible string.


I don't think so. The fim child node does not represent a device. The
CSI supports monitoring frame intervals (reporting via a v4l2 event
when a measured frame interval falls outside the nominal interval
by some tolerance value). The fim child node is only to group properties
for the FIM under a common child node.


+
+Optional properties:
+- fsl,input-capture-channel: an input capture channel and channel flags,
+specified as . The channel number
+must be 0 or 1. The flags can be
+IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
+IRQ_TYPE_EDGE_BOTH, and specify which input
+capture signal edge will trigger the input
+capture event. If an input capture channel is
+specified, the FIM will use this method to
+measure frame intervals instead of via the EOF
+interrupt. The input capture method is much
+preferred over EOF as it is not subject to
+interrupt latency errors. However it requires
+routing the VSYNC or FIELD output signals of
+the camera sensor to one of the i.MX input
+capture pads (SD1_DAT0, SD1_DAT1), which also
+gives up support for SD1.
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the DPHY clock), video_27m, and eim_podf;
+- clock-names  : must contain "dphy", "cfg", "pix";

Don't you need ports to describe the sensor and IPU connections?


Done.

Steve

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


Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice driver

2017-02-28 Thread Steve Longerbeam



On 02/27/2017 06:41 AM, Rob Herring wrote:

On Wed, Feb 15, 2017 at 06:19:17PM -0800, Steve Longerbeam wrote:

From: Philipp Zabel 

This driver can handle SoC internal and external video bus multiplexers,
controlled either by register bit fields or by a GPIO. The subdevice
passes through frame interval and mbus configuration of the active input
to the output side.

Signed-off-by: Sascha Hauer 
Signed-off-by: Philipp Zabel 

--

- fixed a cut&paste error in vidsw_remove(): v4l2_async_register_subdev()
   should be unregister.

- added media_entity_cleanup() and v4l2_device_unregister_subdev()
   to vidsw_remove().

- added missing MODULE_DEVICE_TABLE().
   Suggested-by: Javier Martinez Canillas 

- there was a line left over from a previous iteration that negated
   the new way of determining the pad count just before it which
   has been removed (num_pads = of_get_child_count(np)).

- Philipp Zabel has developed a set of patches that allow adding
   to the subdev async notifier waiting list using a chaining method
   from the async registered callbacks (v4l2_of_subdev_registered()
   and the prep patches for that). For now, I've removed the use of
   v4l2_of_subdev_registered() for the vidmux driver's registered
   callback. This doesn't affect the functionality of this driver,
   but allows for it to be merged now, before adding the chaining
   support.

Signed-off-by: Steve Longerbeam 
---
  .../bindings/media/video-multiplexer.txt   |  59 +++

Please make this a separate commit.


Done.

Steve

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


Re: [PATCH v4 24/36] [media] add Omnivision OV5640 sensor driver

2017-02-28 Thread Steve Longerbeam



On 02/27/2017 06:45 AM, Rob Herring wrote:

On Wed, Feb 15, 2017 at 06:19:26PM -0800, Steve Longerbeam wrote:

This driver is based on ov5640_mipi.c from Freescale imx_3.10.17_1.0.0_beta
branch, modified heavily to bring forward to latest interfaces and code
cleanup.

Signed-off-by: Steve Longerbeam 
---
  .../devicetree/bindings/media/i2c/ov5640.txt   |   43 +

Please split to separate commit.


Done.




  drivers/media/i2c/Kconfig  |7 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/ov5640.c | 2109 
  4 files changed, 2160 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt
  create mode 100644 drivers/media/i2c/ov5640.c

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt 
b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
new file mode 100644
index 000..4607bbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
@@ -0,0 +1,43 @@
+* Omnivision OV5640 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov5640"
+- clocks: reference to the xclk input clock.
+- clock-names: should be "xclk".
+- DOVDD-supply: Digital I/O voltage supply, 1.8 volts
+- AVDD-supply: Analog voltage supply, 2.8 volts
+- DVDD-supply: Digital core voltage supply, 1.5 volts
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the reset pin, if any.
+- pwdn-gpios: reference to the GPIO connected to the pwdn pin, if any.

Use powerdown-gpios here as that is a somewhat standard name.


Done.



Both need to state what is the active state.


Done.

Steve

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


Re: [PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 01:57:53PM +0530, Pranay Kr. Srivastava wrote:
> This patch makes spk_set_key_info return -EINVAL
> in case of failure instead of returning 4 different
> values for the type of error that occurred.
> 
> Print the offending values instead as debug message.
> 
> Signed-off-by: Pranay Kr. Srivastava 
> ---
>  drivers/staging/speakup/main.c | 27 +++
>  1 file changed, 19 insertions(+), 8 deletions(-)

Please fix up your subject line to contain the subsystem and driver it
is modifying.  For example, this message should read:
Subject: staging: speakup: return same error value from spk_set_key_info

Same thing goes for all 3 messages in this series.

Please fix up, keep the Ack, and resend.

thanks,

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


Re: [PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 01:57:55PM +0530, Pranay Kr. Srivastava wrote:
> speakup_allocate used GFP_ATOMIC for allocations
> even while during initialization due to it's use
> in notifier call.
> 
> Pass GFP_ flags as well to speakup_allocate depending
> on the context it is called in.
> 
> Signed-off-by: Pranay Kr. Srivastava 
> ---
>  drivers/staging/speakup/main.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Your subject line contains a misspelling :(

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


Re: [PATCH] staging/fbtft: Hush checkpatch.pl warning about unnecessary line continuations.

2017-02-28 Thread Greg KH
On Tue, Feb 28, 2017 at 09:52:44PM +0200, Alexander Kapshuk wrote:
> Use a single fmt string with appropriate verbs as conversion specifiers,
> followed by the original string literals and the integer argument
> instead of using a backslash to escape a new line embedded inbetween
> quoted string literals passed as fmt arguments to dev_err() invoked in
> drivers/staging/fbtft/fb_ssd1331.c.
> 
> Signed-off-by: Alexander Kapshuk 
> ---
>  drivers/staging/fbtft/fb_ssd1331.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fb_ssd1331.c 
> b/drivers/staging/fbtft/fb_ssd1331.c
> index 26f24e3..6d03041 100644
> --- a/drivers/staging/fbtft/fb_ssd1331.c
> +++ b/drivers/staging/fbtft/fb_ssd1331.c
> @@ -129,17 +129,19 @@ static int set_gamma(struct fbtft_par *par, u32 *curves)
>  
>   for (i = 0; i < 63; i++) {
>   if (i > 0 && curves[i] < 2) {
> - dev_err(par->info->device,
> - "Illegal value in Grayscale Lookup Table at 
> index %d. " \
> - "Must be greater than 1\n", i);
> + dev_err(par->info->device, "%s %d. %s\n",
> + "Illegal value in Grayscale Lookup Table at 
> index",
> + i,
> + "Must be greater than 1");

Huh?  This should just be:
"Illegal value in Grayscale Lookup Table at 
index %d. Must be greater than 1\n",
i);
Don't split the string for no good reason.

>   return -EINVAL;
>   }
>   acc += curves[i];
>   tmp[i] = acc;
>   if (acc > 180) {
> - dev_err(par->info->device,
> - "Illegal value(s) in Grayscale Lookup Table. " \
> - "At index=%d, the accumulated value has 
> exceeded 180\n", i);
> + dev_err(par->info->device, "%s%d, %s\n",
> + "Illegal value(s) in Grayscale Lookup Table. At 
> index=",
> + i,
> + "the accumulated value has exceeded 180");

Same here.

please fix.

thanks,

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