[PATCH SPEAKUP v2 0/3] cleanup error and initilization
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
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
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
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
> 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
> 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
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
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
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
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
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
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
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
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.
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
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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.
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
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.
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.
> 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.
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
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.
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
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.
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.
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
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
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
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
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
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.
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