Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Shirish Gajera
On Sun, Mar 29, 2015 at 12:54:45AM +0100, Richard Weinberger wrote:
> Am 29.03.2015 um 00:44 schrieb Shirish Gajera:
> > On Sat, Mar 28, 2015 at 02:35:19PM -0700, Joe Perches wrote:
> >> On Sat, 2015-03-28 at 22:22 +0100, Richard Weinberger wrote:
> >>> Am 28.03.2015 um 22:18 schrieb Joe Perches:
>  On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote:
> > On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera 
> >  wrote:
> >> This patch fixes the checkpatch.pl warning:
> >> []
> > Instead of blindly adding newlines to silence checkpatch.pl, what
> > about reworking the code?
> > printf("%s\n", ..) cries for a puts().
> 
>  There is no synth_puts
> >>>
> >>> So what?
> >>> Fix it! :-)
> >>
> >> Not sure that'd make the code better... ;-p
> >>
> >>> the whole code is horrible and lines other 80 chars are the *least*
> >>> problem.
> >>
> >> Dunno about how horrible it is, my guess is it works.
> >>
> >>> Submitting a patch just for the sake of silencing checkpatch.pl is a 
> >>> waste of time.
> >>> After applying this patch the driver 0 better than before.
> >>
> >> Agree with that.
> >>
> >> And truly, checkpatch is only a guide.
> >>
> >> Making the code better instead of merely style conforming
> >> should be the primary goal of patches.
> > 
> > This is my first patch.
> 
> Are you sure?
> http://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-January/013187.html
Yup, this patch never got merge because I was doing changes against
wrong repo.
> 
> > Actually on the website it's return that 
> > "Pick a warning, and try to fix it. For your first patch, only pick one
> > warning. In the future you can group multiple changes into one patch,
> > but only if you follow the PatchPhilosophy of breaking each patch into
> > logical changes."
> > 
> > My main aim is to get the patch in and get familier with the full system
> > (code checking,flow etc.). So, I am fixing simple warning.
> > 
> > If this code require changes then I can do as part of future changes.
> 
> The future is now, please address these issues now. :-)
> Again, blindly fixing checkpatch.pl warnings is worthless.
> 
> Thanks,
> //richard

Are you sure you want me to do this changes. Because it will conflict
the things written on http://kernelnewbies.org/

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


Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Richard Weinberger
Am 29.03.2015 um 00:44 schrieb Shirish Gajera:
> On Sat, Mar 28, 2015 at 02:35:19PM -0700, Joe Perches wrote:
>> On Sat, 2015-03-28 at 22:22 +0100, Richard Weinberger wrote:
>>> Am 28.03.2015 um 22:18 schrieb Joe Perches:
 On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote:
> On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera  
> wrote:
>> This patch fixes the checkpatch.pl warning:
>> []
> Instead of blindly adding newlines to silence checkpatch.pl, what
> about reworking the code?
> printf("%s\n", ..) cries for a puts().

 There is no synth_puts
>>>
>>> So what?
>>> Fix it! :-)
>>
>> Not sure that'd make the code better... ;-p
>>
>>> the whole code is horrible and lines other 80 chars are the *least*
>>> problem.
>>
>> Dunno about how horrible it is, my guess is it works.
>>
>>> Submitting a patch just for the sake of silencing checkpatch.pl is a waste 
>>> of time.
>>> After applying this patch the driver 0 better than before.
>>
>> Agree with that.
>>
>> And truly, checkpatch is only a guide.
>>
>> Making the code better instead of merely style conforming
>> should be the primary goal of patches.
> 
> This is my first patch.

Are you sure?
http://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-January/013187.html

> Actually on the website it's return that 
> "Pick a warning, and try to fix it. For your first patch, only pick one
> warning. In the future you can group multiple changes into one patch,
> but only if you follow the PatchPhilosophy of breaking each patch into
> logical changes."
> 
> My main aim is to get the patch in and get familier with the full system
> (code checking,flow etc.). So, I am fixing simple warning.
> 
> If this code require changes then I can do as part of future changes.

The future is now, please address these issues now. :-)
Again, blindly fixing checkpatch.pl warnings is worthless.

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


Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Shirish Gajera
On Sat, Mar 28, 2015 at 02:35:19PM -0700, Joe Perches wrote:
> On Sat, 2015-03-28 at 22:22 +0100, Richard Weinberger wrote:
> > Am 28.03.2015 um 22:18 schrieb Joe Perches:
> > > On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote:
> > >> On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera  
> > >> wrote:
> > >>> This patch fixes the checkpatch.pl warning:
> []
> > >> Instead of blindly adding newlines to silence checkpatch.pl, what
> > >> about reworking the code?
> > >> printf("%s\n", ..) cries for a puts().
> > > 
> > > There is no synth_puts
> > 
> > So what?
> > Fix it! :-)
> 
> Not sure that'd make the code better... ;-p
> 
> > the whole code is horrible and lines other 80 chars are the *least*
> > problem.
> 
> Dunno about how horrible it is, my guess is it works.
> 
> > Submitting a patch just for the sake of silencing checkpatch.pl is a waste 
> > of time.
> > After applying this patch the driver 0 better than before.
> 
> Agree with that.
> 
> And truly, checkpatch is only a guide.
> 
> Making the code better instead of merely style conforming
> should be the primary goal of patches.

This is my first patch.

Actually on the website it's return that 
"Pick a warning, and try to fix it. For your first patch, only pick one
warning. In the future you can group multiple changes into one patch,
but only if you follow the PatchPhilosophy of breaking each patch into
logical changes."

My main aim is to get the patch in and get familier with the full system
(code checking,flow etc.). So, I am fixing simple warning.

If this code require changes then I can do as part of future changes.

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


Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Joe Perches
On Sat, 2015-03-28 at 22:22 +0100, Richard Weinberger wrote:
> Am 28.03.2015 um 22:18 schrieb Joe Perches:
> > On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote:
> >> On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera  
> >> wrote:
> >>> This patch fixes the checkpatch.pl warning:
[]
> >> Instead of blindly adding newlines to silence checkpatch.pl, what
> >> about reworking the code?
> >> printf("%s\n", ..) cries for a puts().
> > 
> > There is no synth_puts
> 
> So what?
> Fix it! :-)

Not sure that'd make the code better... ;-p

> the whole code is horrible and lines other 80 chars are the *least*
> problem.

Dunno about how horrible it is, my guess is it works.

> Submitting a patch just for the sake of silencing checkpatch.pl is a waste of 
> time.
> After applying this patch the driver 0 better than before.

Agree with that.

And truly, checkpatch is only a guide.

Making the code better instead of merely style conforming
should be the primary goal of patches.

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


Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Richard Weinberger
Am 28.03.2015 um 22:18 schrieb Joe Perches:
> On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote:
>> On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera  
>> wrote:
>>> This patch fixes the checkpatch.pl warning:
> 
> []
> 
>>> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> []
>>> @@ -423,7 +423,8 @@ static void announce_edge(struct vc_data *vc, int 
>>> msg_id)
>>> if (spk_bleeps & 1)
>>> bleep(spk_y);
>>> if ((spk_bleeps & 2) && (msg_id < edge_quiet))
>>> -   synth_printf("%s\n", spk_msg_get(MSG_EDGE_MSGS_START + 
>>> msg_id - 1));
>>> +   synth_printf("%s\n",
>>> +   spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 1));
>>
>> Instead of blindly adding newlines to silence checkpatch.pl, what
>> about reworking the code?
>> printf("%s\n", ..) cries for a puts().
> 
> There is no synth_puts

So what?
Fix it! :-)

>>> @@ -1131,7 +1132,8 @@ static void spkup_write(const char *in_buf, int count)
>>> if (in_count > 2 && rep_count > 2) {
>>> if (last_type & CH_RPT) {
>>> synth_printf(" ");
>>> -   synth_printf(spk_msg_get(MSG_REPEAT_DESC2), 
>>> ++rep_count);
>>> +   synth_printf(spk_msg_get(MSG_REPEAT_DESC2),
>>> +   ++rep_count);
>>> synth_printf(" ");
>>
>> This printf stuff looks odd. synth_printf() seems to take a format
>> string, in this case the format string
>> is returned by spk_msg_get(), smells like a format string bug.
> 
> Nope, but it would be nicer to avoid these spk_msg_get
> functions for the indices that are used with printf style
> formatting.
> 
>>> }
>>> rep_count = 0;
>>> @@ -1847,7 +1849,8 @@ static void speakup_win_set(struct vc_data *vc)
>>> win_right = spk_x;
>>> }
>>> snprintf(info, sizeof(info), 
>>> spk_msg_get(MSG_WINDOW_BOUNDARY),
>>> -(win_start) ? spk_msg_get(MSG_END) : 
>>> spk_msg_get(MSG_START),
>>> +(win_start) ?
>>> +   spk_msg_get(MSG_END) : 
>>> spk_msg_get(MSG_START),
>>>  (int)spk_y + 1, (int)spk_x + 1);
>>
>> Same here. Also please resolve the ?: mess.
> 
> I don't think there's a ?: mess, but the code looks wrong.  
> 
>   win_start ? MSG_END : MSG_START

Face it, the whole code is horrible and lines other 80 chars are the *least*
problem.
Submitting a patch just for the sake of silencing checkpatch.pl is a waste of 
time.
After applying this patch the driver 0 better than before.

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


Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Joe Perches
On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote:
> On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera  
> wrote:
> > This patch fixes the checkpatch.pl warning:

[]

> > diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
[]
> > @@ -423,7 +423,8 @@ static void announce_edge(struct vc_data *vc, int 
> > msg_id)
> > if (spk_bleeps & 1)
> > bleep(spk_y);
> > if ((spk_bleeps & 2) && (msg_id < edge_quiet))
> > -   synth_printf("%s\n", spk_msg_get(MSG_EDGE_MSGS_START + 
> > msg_id - 1));
> > +   synth_printf("%s\n",
> > +   spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 1));
> 
> Instead of blindly adding newlines to silence checkpatch.pl, what
> about reworking the code?
> printf("%s\n", ..) cries for a puts().

There is no synth_puts

> > @@ -1131,7 +1132,8 @@ static void spkup_write(const char *in_buf, int count)
> > if (in_count > 2 && rep_count > 2) {
> > if (last_type & CH_RPT) {
> > synth_printf(" ");
> > -   synth_printf(spk_msg_get(MSG_REPEAT_DESC2), 
> > ++rep_count);
> > +   synth_printf(spk_msg_get(MSG_REPEAT_DESC2),
> > +   ++rep_count);
> > synth_printf(" ");
> 
> This printf stuff looks odd. synth_printf() seems to take a format
> string, in this case the format string
> is returned by spk_msg_get(), smells like a format string bug.

Nope, but it would be nicer to avoid these spk_msg_get
functions for the indices that are used with printf style
formatting.

> > }
> > rep_count = 0;
> > @@ -1847,7 +1849,8 @@ static void speakup_win_set(struct vc_data *vc)
> > win_right = spk_x;
> > }
> > snprintf(info, sizeof(info), 
> > spk_msg_get(MSG_WINDOW_BOUNDARY),
> > -(win_start) ? spk_msg_get(MSG_END) : 
> > spk_msg_get(MSG_START),
> > +(win_start) ?
> > +   spk_msg_get(MSG_END) : 
> > spk_msg_get(MSG_START),
> >  (int)spk_y + 1, (int)spk_x + 1);
> 
> Same here. Also please resolve the ?: mess.

I don't think there's a ?: mess, but the code looks wrong.  

win_start ? MSG_END : MSG_START

sure looks backwards.


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


Re: [PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Richard Weinberger
On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera  wrote:
> This patch fixes the checkpatch.pl warning:
> WARNING: line over 80 characters
>
> All line over 80 characters in driver/staging/speakup/* are fixed.
>
> Signed-off-by: Shirish Gajera 
> ---
>  drivers/staging/speakup/main.c   | 9 ++---
>  drivers/staging/speakup/serialio.h   | 3 ++-
>  drivers/staging/speakup/speakup.h| 6 --
>  drivers/staging/speakup/speakup_decext.c | 6 --
>  drivers/staging/speakup/speakup_decpc.c  | 6 --
>  drivers/staging/speakup/spk_types.h  | 3 ++-
>  6 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index 1249f91..c955976 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -423,7 +423,8 @@ static void announce_edge(struct vc_data *vc, int msg_id)
> if (spk_bleeps & 1)
> bleep(spk_y);
> if ((spk_bleeps & 2) && (msg_id < edge_quiet))
> -   synth_printf("%s\n", spk_msg_get(MSG_EDGE_MSGS_START + msg_id 
> - 1));
> +   synth_printf("%s\n",
> +   spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 1));

Instead of blindly adding newlines to silence checkpatch.pl, what
about reworking the code?
printf("%s\n", ..) cries for a puts().

>  }
>
>  static void speak_char(u_char ch)
> @@ -1131,7 +1132,8 @@ static void spkup_write(const char *in_buf, int count)
> if (in_count > 2 && rep_count > 2) {
> if (last_type & CH_RPT) {
> synth_printf(" ");
> -   synth_printf(spk_msg_get(MSG_REPEAT_DESC2), 
> ++rep_count);
> +   synth_printf(spk_msg_get(MSG_REPEAT_DESC2),
> +   ++rep_count);
> synth_printf(" ");

This printf stuff looks odd. synth_printf() seems to take a format
string, in this case the format string
is returned by spk_msg_get(), smells like a format string bug.

> }
> rep_count = 0;
> @@ -1847,7 +1849,8 @@ static void speakup_win_set(struct vc_data *vc)
> win_right = spk_x;
> }
> snprintf(info, sizeof(info), spk_msg_get(MSG_WINDOW_BOUNDARY),
> -(win_start) ? spk_msg_get(MSG_END) : 
> spk_msg_get(MSG_START),
> +(win_start) ?
> +   spk_msg_get(MSG_END) : spk_msg_get(MSG_START),
>  (int)spk_y + 1, (int)spk_x + 1);

Same here. Also please resolve the ?: mess.

> }
> synth_printf("%s\n", info);
> diff --git a/drivers/staging/speakup/serialio.h 
> b/drivers/staging/speakup/serialio.h
> index 317bb84..1b39921 100644
> --- a/drivers/staging/speakup/serialio.h
> +++ b/drivers/staging/speakup/serialio.h
> @@ -34,6 +34,7 @@ struct old_serial_port {
>  #define SPK_TIMEOUT 100
>  #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
>
> -#define spk_serial_tx_busy() ((inb(speakup_info.port_tts + UART_LSR) & 
> BOTH_EMPTY) != BOTH_EMPTY)
> +#define spk_serial_tx_busy() \
> +   ((inb(speakup_info.port_tts + UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)

This makro is ugly in many ways.
Why can't this be an static inline function without a dependency on
global state?

>
>  #endif
> diff --git a/drivers/staging/speakup/speakup.h 
> b/drivers/staging/speakup/speakup.h
> index 898dce5..a7f4962 100644
> --- a/drivers/staging/speakup/speakup.h
> +++ b/drivers/staging/speakup/speakup.h
> @@ -61,10 +61,12 @@ extern struct st_var_header *spk_get_var_header(enum 
> var_id_t var_id);
>  extern struct st_var_header *spk_var_header_by_name(const char *name);
>  extern struct punc_var_t *spk_get_punc_var(enum var_id_t var_id);
>  extern int spk_set_num_var(int val, struct st_var_header *var, int how);
> -extern int spk_set_string_var(const char *page, struct st_var_header *var, 
> int len);
> +extern int spk_set_string_var(const char *page, struct st_var_header *var,
> +   int len);
>  extern int spk_set_mask_bits(const char *input, const int which, const int 
> how);
>  extern special_func spk_special_handler;
> -extern int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, 
> u_short key);
> +extern int spk_handle_help(struct vc_data *vc, u_char type, u_char ch,
> +   u_short key);
>  extern int synth_init(char *name);
>  extern void synth_release(void);
>
> diff --git a/drivers/staging/speakup/speakup_decext.c 
> b/drivers/staging/speakup/speakup_decext.c
> index 2b772f8..e0b5db9 100644
> --- a/drivers/staging/speakup/speakup_decext.c
> +++ b/drivers/staging/speakup/speakup_decext.c
> @@ -207,10 +207,12 @@ static void do_catch_up(struct spk_synth *synth)
> if (time_after_eq(jiffies, jiff_max)) {
> if (!in_escape)
> spk_serial_out(PRO

[PATCH] staging: speakup: Fix warning of line over 80 characters.

2015-03-28 Thread Shirish Gajera
This patch fixes the checkpatch.pl warning:
WARNING: line over 80 characters

All line over 80 characters in driver/staging/speakup/* are fixed.

Signed-off-by: Shirish Gajera 
---
 drivers/staging/speakup/main.c   | 9 ++---
 drivers/staging/speakup/serialio.h   | 3 ++-
 drivers/staging/speakup/speakup.h| 6 --
 drivers/staging/speakup/speakup_decext.c | 6 --
 drivers/staging/speakup/speakup_decpc.c  | 6 --
 drivers/staging/speakup/spk_types.h  | 3 ++-
 6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 1249f91..c955976 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -423,7 +423,8 @@ static void announce_edge(struct vc_data *vc, int msg_id)
if (spk_bleeps & 1)
bleep(spk_y);
if ((spk_bleeps & 2) && (msg_id < edge_quiet))
-   synth_printf("%s\n", spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 
1));
+   synth_printf("%s\n",
+   spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 1));
 }
 
 static void speak_char(u_char ch)
@@ -1131,7 +1132,8 @@ static void spkup_write(const char *in_buf, int count)
if (in_count > 2 && rep_count > 2) {
if (last_type & CH_RPT) {
synth_printf(" ");
-   synth_printf(spk_msg_get(MSG_REPEAT_DESC2), 
++rep_count);
+   synth_printf(spk_msg_get(MSG_REPEAT_DESC2),
+   ++rep_count);
synth_printf(" ");
}
rep_count = 0;
@@ -1847,7 +1849,8 @@ static void speakup_win_set(struct vc_data *vc)
win_right = spk_x;
}
snprintf(info, sizeof(info), spk_msg_get(MSG_WINDOW_BOUNDARY),
-(win_start) ? spk_msg_get(MSG_END) : 
spk_msg_get(MSG_START),
+(win_start) ?
+   spk_msg_get(MSG_END) : spk_msg_get(MSG_START),
 (int)spk_y + 1, (int)spk_x + 1);
}
synth_printf("%s\n", info);
diff --git a/drivers/staging/speakup/serialio.h 
b/drivers/staging/speakup/serialio.h
index 317bb84..1b39921 100644
--- a/drivers/staging/speakup/serialio.h
+++ b/drivers/staging/speakup/serialio.h
@@ -34,6 +34,7 @@ struct old_serial_port {
 #define SPK_TIMEOUT 100
 #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
 
-#define spk_serial_tx_busy() ((inb(speakup_info.port_tts + UART_LSR) & 
BOTH_EMPTY) != BOTH_EMPTY)
+#define spk_serial_tx_busy() \
+   ((inb(speakup_info.port_tts + UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
 
 #endif
diff --git a/drivers/staging/speakup/speakup.h 
b/drivers/staging/speakup/speakup.h
index 898dce5..a7f4962 100644
--- a/drivers/staging/speakup/speakup.h
+++ b/drivers/staging/speakup/speakup.h
@@ -61,10 +61,12 @@ extern struct st_var_header *spk_get_var_header(enum 
var_id_t var_id);
 extern struct st_var_header *spk_var_header_by_name(const char *name);
 extern struct punc_var_t *spk_get_punc_var(enum var_id_t var_id);
 extern int spk_set_num_var(int val, struct st_var_header *var, int how);
-extern int spk_set_string_var(const char *page, struct st_var_header *var, int 
len);
+extern int spk_set_string_var(const char *page, struct st_var_header *var,
+   int len);
 extern int spk_set_mask_bits(const char *input, const int which, const int 
how);
 extern special_func spk_special_handler;
-extern int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short 
key);
+extern int spk_handle_help(struct vc_data *vc, u_char type, u_char ch,
+   u_short key);
 extern int synth_init(char *name);
 extern void synth_release(void);
 
diff --git a/drivers/staging/speakup/speakup_decext.c 
b/drivers/staging/speakup/speakup_decext.c
index 2b772f8..e0b5db9 100644
--- a/drivers/staging/speakup/speakup_decext.c
+++ b/drivers/staging/speakup/speakup_decext.c
@@ -207,10 +207,12 @@ static void do_catch_up(struct spk_synth *synth)
if (time_after_eq(jiffies, jiff_max)) {
if (!in_escape)
spk_serial_out(PROCSPEECH);
-   spin_lock_irqsave(&speakup_info.spinlock, 
flags);
+   spin_lock_irqsave(&speakup_info.spinlock,
+   flags);
jiffy_delta_val = jiffy_delta->u.n.value;
delay_time_val = delay_time->u.n.value;
-   spin_unlock_irqrestore(&speakup_info.spinlock, 
flags);
+   spin_unlock_irqrestore(&speakup_info.spinlock,
+   flags);
schedule_timeout(msecs_to_jiffies
  

[PATCH] staging: octeon-ethernet: delete cvm_oct_set_carrier()

2015-03-28 Thread Aaro Koskinen
Delete unused function cvm_oct_set_carrier().

Signed-off-by: Aaro Koskinen 
---
 drivers/staging/octeon/ethernet-mdio.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-mdio.c 
b/drivers/staging/octeon/ethernet-mdio.c
index ebfa9c9..40dab11 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -130,19 +130,6 @@ static void cvm_oct_note_carrier(struct octeon_ethernet 
*priv,
}
 }
 
-void cvm_oct_set_carrier(struct octeon_ethernet *priv,
-cvmx_helper_link_info_t link_info)
-{
-   cvm_oct_note_carrier(priv, link_info);
-   if (link_info.s.link_up) {
-   if (!netif_carrier_ok(priv->netdev))
-   netif_carrier_on(priv->netdev);
-   } else {
-   if (netif_carrier_ok(priv->netdev))
-   netif_carrier_off(priv->netdev);
-   }
-}
-
 void cvm_oct_adjust_link(struct net_device *dev)
 {
struct octeon_ethernet *priv = netdev_priv(dev);
-- 
2.2.0

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


[PATCH 1/4] staging: octeon-usb: make cvmx_fifo_setup void

2015-03-28 Thread Aaro Koskinen
Make cvmx_fifo_setup void, it does not return any value.

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

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index feaaafc..ca8be66 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -608,7 +608,7 @@ static inline int cvmx_usb_get_data_pid(struct 
cvmx_usb_pipe *pipe)
return 0; /* Data0 */
 }
 
-static int cvmx_fifo_setup(struct cvmx_usb_state *usb)
+static void cvmx_fifo_setup(struct cvmx_usb_state *usb)
 {
union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
union cvmx_usbcx_gnptxfsiz npsiz;
-- 
2.2.0

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


[PATCH 4/4] staging: octeon-usb: make CVMX_WAIT_FOR_FIELD32 to take condition expression

2015-03-28 Thread Aaro Koskinen
Make CVMX_WAIT_FOR_FIELD32 to take full condition expression.
This should make the usage simpler, and the macro more readable.

Signed-off-by: Aaro Koskinen 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 3e89265..9e5476e 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -393,8 +393,8 @@ struct octeon_hcd {
struct cvmx_usb_state usb;
 };
 
-/* This macro spins on a field waiting for it to reach a value */
-#define CVMX_WAIT_FOR_FIELD32(address, _union, field, op, value, timeout_usec)\
+/* This macro spins on a register waiting for it to reach a condition. */
+#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \
({int result;   \
do {\
uint64_t done = cvmx_get_cycle() + (uint64_t)timeout_usec * \
@@ -403,7 +403,8 @@ struct octeon_hcd {
\
while (1) { \
c.u32 = cvmx_usb_read_csr32(usb, address);  \
-   if (c.s.field op (value)) { \
+   \
+   if (cond) { \
result = 0; \
break;  \
} else if (cvmx_get_cycle() > done) {   \
@@ -652,11 +653,11 @@ static void cvmx_fifo_setup(struct cvmx_usb_state *usb)
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
cvmx_usbcx_grstctl, txfflsh, 1);
CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, txfflsh, ==, 0, 100);
+ cvmx_usbcx_grstctl, c.s.txfflsh == 0, 100);
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
cvmx_usbcx_grstctl, rxfflsh, 1);
CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, rxfflsh, ==, 0, 100);
+ cvmx_usbcx_grstctl, c.s.rxfflsh == 0, 100);
 }
 
 /**
-- 
2.2.0

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


[PATCH 3/4] staging: octeon-usb: assume union type for FIELD32 macros

2015-03-28 Thread Aaro Koskinen
Assume union type for FIELD32 macros to simplify usage.

Signed-off-by: Aaro Koskinen 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 79 +++--
 1 file changed, 36 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 7312597..3e89265 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -394,12 +394,13 @@ struct octeon_hcd {
 };
 
 /* This macro spins on a field waiting for it to reach a value */
-#define CVMX_WAIT_FOR_FIELD32(address, type, field, op, value, timeout_usec)\
+#define CVMX_WAIT_FOR_FIELD32(address, _union, field, op, value, timeout_usec)\
({int result;   \
do {\
uint64_t done = cvmx_get_cycle() + (uint64_t)timeout_usec * \
octeon_get_clock_rate() / 100;  \
-   type c; \
+   union _union c; \
+   \
while (1) { \
c.u32 = cvmx_usb_read_csr32(usb, address);  \
if (c.s.field op (value)) { \
@@ -418,9 +419,10 @@ struct octeon_hcd {
  * This macro logically sets a single field in a CSR. It does the sequence
  * read, modify, and write
  */
-#define USB_SET_FIELD32(address, type, field, value)   \
+#define USB_SET_FIELD32(address, _union, field, value) \
do {\
-   type c; \
+   union _union c; \
+   \
c.u32 = cvmx_usb_read_csr32(usb, address);  \
c.s.field = value;  \
cvmx_usb_write_csr32(usb, address, c.u32);  \
@@ -621,9 +623,8 @@ static void cvmx_fifo_setup(struct cvmx_usb_state *usb)
 * Program the USBC_GRXFSIZ register to select the size of the receive
 * FIFO (25%).
 */
-   USB_SET_FIELD32(CVMX_USBCX_GRXFSIZ(usb->index),
-   union cvmx_usbcx_grxfsiz, rxfdep,
-   usbcx_ghwcfg3.s.dfifodepth / 4);
+   USB_SET_FIELD32(CVMX_USBCX_GRXFSIZ(usb->index), cvmx_usbcx_grxfsiz,
+   rxfdep, usbcx_ghwcfg3.s.dfifodepth / 4);
 
/*
 * Program the USBC_GNPTXFSIZ register to select the size and the start
@@ -647,17 +648,15 @@ static void cvmx_fifo_setup(struct cvmx_usb_state *usb)
 
/* Flush all FIFOs */
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   union cvmx_usbcx_grstctl, txfnum, 0x10);
+   cvmx_usbcx_grstctl, txfnum, 0x10);
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   union cvmx_usbcx_grstctl, txfflsh, 1);
+   cvmx_usbcx_grstctl, txfflsh, 1);
CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- union cvmx_usbcx_grstctl,
- txfflsh, ==, 0, 100);
+ cvmx_usbcx_grstctl, txfflsh, ==, 0, 100);
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
-   union cvmx_usbcx_grstctl, rxfflsh, 1);
+   cvmx_usbcx_grstctl, rxfflsh, 1);
CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- union cvmx_usbcx_grstctl,
- rxfflsh, ==, 0, 100);
+ cvmx_usbcx_grstctl, rxfflsh, ==, 0, 100);
 }
 
 /**
@@ -926,9 +925,9 @@ retry:
 *USBC_GINTMSK[PRTINT] = 1
 */
USB_SET_FIELD32(CVMX_USBCX_GINTMSK(usb->index),
-   union cvmx_usbcx_gintmsk, prtintmsk, 1);
+   cvmx_usbcx_gintmsk, prtintmsk, 1);
USB_SET_FIELD32(CVMX_USBCX_GINTMSK(usb->index),
-   union cvmx_usbcx_gintmsk, disconnintmsk, 1);
+   cvmx_usbcx_gintmsk, disconnintmsk, 1);
 
/*
 * 2. Program the USBC_HCFG register to select full-speed host
@@ -975,7 +974,7 @@ static void cvmx_usb_reset_port(struct cvmx_usb_state *usb)
  CVMX_USBCX_HPRT(usb->index));
 
/* Program the port reset bit to start the reset process */
-   USB_SET_FIELD32(CVMX_USBCX_HPRT(usb->index), union cvmx_usbcx_hprt,
+   USB_SET_FIELD32(CVMX_USBCX_HPRT(usb->index), cvmx_usbcx_hprt,
prtrst, 1);
 
/*
@@ -985,7 +984,7 @@ static void cvmx_usb_reset_port(struct cvmx_usb_state

[PATCH 2/4] staging: octeon-usb: octeon_usb_probe: delete unused variable

2015-03-28 Thread Aaro Koskinen
"flags" is not used, delete it.

Signed-off-by: Aaro Koskinen 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index ca8be66..7312597 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -3581,7 +3581,6 @@ static int octeon_usb_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct octeon_hcd *priv;
struct usb_hcd *hcd;
-   unsigned long flags;
u32 clock_rate = 4800;
bool is_crystal_clock = false;
const char *clock_type;
-- 
2.2.0

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


[PATCH 2/2] staging: sm7xxfb: disable pci device

2015-03-28 Thread Sudip Mukherjee
disable the pci device when the module exits.

Signed-off-by: Sudip Mukherjee 
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c 
b/drivers/staging/sm7xxfb/sm7xxfb.c
index 149286e..77f51a0 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -943,6 +943,7 @@ static void smtcfb_pci_remove(struct pci_dev *pdev)
unregister_framebuffer(&sfb->fb);
smtc_free_fb_info(sfb);
pci_release_region(pdev, 0);
+   pci_disable_device(pdev);
 }
 
 #ifdef CONFIG_PM
-- 
1.8.1.2

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


[PATCH 1/2] staging: sm7xxfb: reserve PCI resource

2015-03-28 Thread Sudip Mukherjee
before starting to access any address inside the PCI region we should
reserve the resource and release the resource when the module exits.

Signed-off-by: Sudip Mukherjee 
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c 
b/drivers/staging/sm7xxfb/sm7xxfb.c
index 5b3e614..149286e 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -776,6 +776,12 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
if (err)
return err;
 
+   err = pci_request_region(pdev, 0, "sm7xxfb");
+   if (err < 0) {
+   dev_err(&pdev->dev, "cannot reserve framebuffer region\n");
+   goto failed_regions;
+   }
+
sprintf(smtcfb_fix.id, "sm%Xfb", ent->device);
 
sfb = smtc_alloc_fb_info(pdev);
@@ -905,6 +911,9 @@ failed_fb:
smtc_free_fb_info(sfb);
 
 failed_free:
+   pci_release_region(pdev, 0);
+
+failed_regions:
pci_disable_device(pdev);
 
return err;
@@ -933,6 +942,7 @@ static void smtcfb_pci_remove(struct pci_dev *pdev)
smtc_unmap_mmio(sfb);
unregister_framebuffer(&sfb->fb);
smtc_free_fb_info(sfb);
+   pci_release_region(pdev, 0);
 }
 
 #ifdef CONFIG_PM
-- 
1.8.1.2

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


Re: [PATCH] drivers: staging: speakup: fix sparse "warning: expression using sizeof bool"

2015-03-28 Thread Piotr Witosławski
On Sat, Mar 28, 2015 at 11:56:40AM +0530, Sudip Mukherjee wrote:
> On Fri, Mar 27, 2015 at 09:36:07PM +0100, Witos wrote:
> > Changed bool to u8 to get rid of sparse warning.
> but i am not getting this warning. which version of sparse are you
> using?

sparse 0.5.0, see: http://yarchive.net/comp/linux/bool.html

> and why you have sent the same patch two times?

sorry my mistake, first time I wanted send it to myself, I forgot
that I filled CC list. 

> 
> regards
> sudip
> 
> > 
> > Signed-off-by: Piotr Witoslawski 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel