Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Chewie Lin]

On Sat, Apr 01, 2017 at 04:32:39AM +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > Replace string with formatted arguments in the dev_warn() call. It removes
> > the checkpatch warning:
> > 
> > WARNING: Prefer using "%s", __func__ to embedded function names
> > #417: FILE: main_usb.c:417:
> > +"usb_device_reset fail status=%d\n", status);
> > 
> > total: 0 errors, 1 warnings, 1058 lines checked
> > 
> > And after fix:
> > 
> > main_usb.c has no obvious style problems and is ready for submission.
> 
> 
> > -"usb_device_reset fail status=%d\n", status);
> > +"%s=%d\n", "usb_device_reset fail status", status);
> 
> Would you mind explaining the meaning of that warning?  Incidentally, why not
> "%se_reset fail status=%d\n", "usb_devic", status);
> - it would produce the identical output and silences checkpatch even more
> reliably.  Discuss.
> 
> Sarcasm aside, when you are proposing a change, there are several questions 
> you
> really must ask yourself and be able to answer.  First and foremost, of 
> course,
> is
>   * Why is it an improvement?
> If the answer to that was "it makes a warning go away", the next questions are
>   * What are we warned about?
>   * What is problematic in the original variant?
>   * Is the replacement free from the problem we had in the original?
> and if the answer to any of that is "I don't know", the next step is _not_
> "send it anyway".  "Try to figure out" might be a good idea, with usual
> heuristics regarding the reading comprehension ("if a sentence remains
> a mystery, see if there are any unfamiliar words skipped at the first reading
> and find what they mean", etc.) applying.
> 
> In this particular case the part you've apparently skipped was, indeed,
> critical - "__func__".  I am not trying to defend the quality of checkpatch -
> the warning is badly worded, the tool is badly written and more often than
> not its suggestions reflect nothing but authors' arbitrary preferences.
> 
> This one, however, does have some rationale behind it.  Namely, if some
> debugging output contains the name of a function that has produced it,
> having that name spelled out in format string is not a good idea.  If
> that code gets moved elsewhere one would have to replace the name in format
> string or face confusing messages refering to the function where that
> code used to be.  Since __func__ is interpreted as a constant string
> initialized with the name of the function it's used in, it is possible
> to avoid spelling the function name out - instead of
> "my_function: pink elephants; reduce the daily intake to %d glasses\n", n
> one could use
> "%s: pink elephants; reduce the daily intake to %d glasses\n", __func__, n
> producing the same output and avoiding the need to adjust anything when
> the whole thing is moved to another function.  It's not particularly big
> deal, but it's a more or less reasonable suggestion.
> 
> Your change, of course, has achieved only one thing - it has moved the
> explicitly spelled out function name out of format string.  It's still
> just as explicitly spelled out, still would need to be adjusted if moved
> to another function and the whole thing has become harder to read and
> understand since you've buried other parts of message in the place where
> it's harder to follow.
> 
> Again, checkpatch warning is badly written, but the main problem with
> your posting is not that you had been confused by checkpatch - it's
> that you have posted it based on an incomprehensible message with no better
> rationale than "it shuts checkpatch up, dunno why and what about".

Al, brilliant, that's exactly what I was trying to do on my first try. 
The checkpatch *is* confusing. It's fine with a simple string but doesn't 
like it when it's formatted string. From what you said, I 
think this may work better and more portable: 

"%s: fail status = %d\n", "usb_device_reset", status)



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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Joe Perches]

On Sat, 2017-04-01 at 05:08 +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 08:52:50PM -0700, Joe Perches wrote:
> 
> > > MILD SUGGESTION: don't spell the function name out in format strings;
> > >   "this_function: foo is %d", n
> > > might be better off as
> > >   "%s: foo is %d", __func__, n
> > > in case you ever move it to another function or rename your function.
> > 
> > Thank you sir, may I have another.
> > 
> > checkpatch messages are single line.
> 
> Too bad... Incidentally, being able to get more detailed explanation of
> a warning might be a serious improvement, especially if it contains
> the rationale.  Hell, something like TeX handling of errors might be
> a good idea - warning printed, offered actions include 'give more help',
> 'continue', 'exit', 'from now on suppress this kind of warning', 'from
> now on just dump this kind of warning into log and keep going', 'from
> now on dump all warnings into log and keep going'.

Well, there is the possibility to have longer messages.
It's just the --terse option has to be somewhat sensible.

> And yes, I'm serious about having something like "mild suggestion" as
> possible severity - people are using that thing to look for potential
> improvements to make and 'such and such change might be useful for such
> and such reasons' is a lot more useful than 'this needs to be thus because
> it must be thus or I'll keep warning'.

I agree about checkpatch and ERROR/WARNING/CHECK vs some other wording.

https://lkml.org/lkml/2016/8/27/180
https://lkml.org/lkml/2015/7/16/568

The other thing that might help is for people to take
the warnings the script produces less seriously.

Maybe convert:

ERROR -> defect
WARNING -> unstylish
CHECK -> nitpick

or some such.

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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Joe Perches]

On Sat, 2017-04-01 at 04:32 +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > Replace string with formatted arguments in the dev_warn() call. It removes
> > the checkpatch warning:
> > 
> > WARNING: Prefer using "%s", __func__ to embedded function names
[]
> Again, checkpatch warning is badly written

In your opinion, what wording would be better?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Al Viro]

On Fri, Mar 31, 2017 at 08:52:50PM -0700, Joe Perches wrote:

> > MILD SUGGESTION: don't spell the function name out in format strings;
> > "this_function: foo is %d", n
> > might be better off as
> > "%s: foo is %d", __func__, n
> > in case you ever move it to another function or rename your function.
> 
> Thank you sir, may I have another.
> 
> checkpatch messages are single line.

Too bad... Incidentally, being able to get more detailed explanation of
a warning might be a serious improvement, especially if it contains
the rationale.  Hell, something like TeX handling of errors might be
a good idea - warning printed, offered actions include 'give more help',
'continue', 'exit', 'from now on suppress this kind of warning', 'from
now on just dump this kind of warning into log and keep going', 'from
now on dump all warnings into log and keep going'.

And yes, I'm serious about having something like "mild suggestion" as
possible severity - people are using that thing to look for potential
improvements to make and 'such and such change might be useful for such
and such reasons' is a lot more useful than 'this needs to be thus because
it must be thus or I'll keep warning'.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Joe Perches]

On Sat, 2017-04-01 at 04:46 +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 08:36:22PM -0700, Joe Perches wrote:
> > On Sat, 2017-04-01 at 04:32 +0100, Al Viro wrote:
> > > On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > > > Replace string with formatted arguments in the dev_warn() call. It 
> > > > removes
> > > > the checkpatch warning:
> > > > 
> > > > WARNING: Prefer using "%s", __func__ to embedded function names
> > 
> > []
> > > Again, checkpatch warning is badly written
> > 
> > In your opinion, what wording would be better?
> 
> MILD SUGGESTION: don't spell the function name out in format strings;
>   "this_function: foo is %d", n
> might be better off as
>   "%s: foo is %d", __func__, n
> in case you ever move it to another function or rename your function.

Thank you sir, may I have another.

checkpatch messages are single line.

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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Al Viro]

On Fri, Mar 31, 2017 at 08:36:22PM -0700, Joe Perches wrote:
> On Sat, 2017-04-01 at 04:32 +0100, Al Viro wrote:
> > On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > > Replace string with formatted arguments in the dev_warn() call. It removes
> > > the checkpatch warning:
> > > 
> > >   WARNING: Prefer using "%s", __func__ to embedded function names
> []
> > Again, checkpatch warning is badly written
> 
> In your opinion, what wording would be better?

MILD SUGGESTION: don't spell the function name out in format strings;
"this_function: foo is %d", n
might be better off as
"%s: foo is %d", __func__, n
in case you ever move it to another function or rename your function.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Joe Perches]

On Fri, 2017-03-31 at 20:18 -0700, Chewie Lin wrote:
> These are good points, but any feedback on the dev_warn() call itself?
> I was trying to fix the checkpatch warning on my first try.

Using "%s" with a long string is generally inefficient.

Compare the compiled object size of
printf("%s is %d\n", "Long string", 1);
to
printf("Long string is %d\n", 1); 

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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Al Viro]

On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning:
> 
>   WARNING: Prefer using "%s", __func__ to embedded function names
>   #417: FILE: main_usb.c:417:
>   +"usb_device_reset fail status=%d\n", status);
> 
>   total: 0 errors, 1 warnings, 1058 lines checked
> 
> And after fix:
> 
>   main_usb.c has no obvious style problems and is ready for submission.


> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

Would you mind explaining the meaning of that warning?  Incidentally, why not
  "%se_reset fail status=%d\n", "usb_devic", status);
- it would produce the identical output and silences checkpatch even more
reliably.  Discuss.

Sarcasm aside, when you are proposing a change, there are several questions you
really must ask yourself and be able to answer.  First and foremost, of course,
is
* Why is it an improvement?
If the answer to that was "it makes a warning go away", the next questions are
* What are we warned about?
* What is problematic in the original variant?
* Is the replacement free from the problem we had in the original?
and if the answer to any of that is "I don't know", the next step is _not_
"send it anyway".  "Try to figure out" might be a good idea, with usual
heuristics regarding the reading comprehension ("if a sentence remains
a mystery, see if there are any unfamiliar words skipped at the first reading
and find what they mean", etc.) applying.

In this particular case the part you've apparently skipped was, indeed,
critical - "__func__".  I am not trying to defend the quality of checkpatch -
the warning is badly worded, the tool is badly written and more often than
not its suggestions reflect nothing but authors' arbitrary preferences.

This one, however, does have some rationale behind it.  Namely, if some
debugging output contains the name of a function that has produced it,
having that name spelled out in format string is not a good idea.  If
that code gets moved elsewhere one would have to replace the name in format
string or face confusing messages refering to the function where that
code used to be.  Since __func__ is interpreted as a constant string
initialized with the name of the function it's used in, it is possible
to avoid spelling the function name out - instead of
"my_function: pink elephants; reduce the daily intake to %d glasses\n", n
one could use
"%s: pink elephants; reduce the daily intake to %d glasses\n", __func__, n
producing the same output and avoiding the need to adjust anything when
the whole thing is moved to another function.  It's not particularly big
deal, but it's a more or less reasonable suggestion.

Your change, of course, has achieved only one thing - it has moved the
explicitly spelled out function name out of format string.  It's still
just as explicitly spelled out, still would need to be adjusted if moved
to another function and the whole thing has become harder to read and
understand since you've buried other parts of message in the place where
it's harder to follow.

Again, checkpatch warning is badly written, but the main problem with
your posting is not that you had been confused by checkpatch - it's
that you have posted it based on an incomprehensible message with no better
rationale than "it shuts checkpatch up, dunno why and what about".
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] drivers: fixed a checkpatch warning

[Chewie Lin]

On Fri, Mar 31, 2017 at 11:53:55AM -0700, Joe Perches wrote:
> On Fri, 2017-03-31 at 01:40 -0700, Chewie Lin wrote:
> > fixed a coding style error/warning.
> > 
> > Signed-off-by: Chewie Lin 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 9e074e9..2d9e7af 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
> > status = usb_reset_device(priv->usb);
> > if (status)
> > dev_warn(>usb->dev,
> > -"usb_device_reset fail status=%d\n", status);
> > +"%s=%d\n", "usb_device_reset fail status", status);
> 
> Yeah, what Greg said.
> 
> Also most likely this should be
> 
>   dev_warn(>usb->dev,
>"usb_reset_device fail status=%d\n", status);
> 
> Note the function is usb_device_reset, but the
> function call that failed is usb_reset_device.

yep, I had the comic book guy from the Simpsons voice when I read that. :)
I submitted a separate patch, maybe I should have reply-to this instead?

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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Chewie Lin]

On Fri, Mar 31, 2017 at 07:45:09PM -0700, Joe Perches wrote:
> On Fri, 2017-03-31 at 19:15 -0700, Randy Dunlap wrote:
> > On 03/31/17 18:59, Chewie Lin wrote:
> > > Replace string with formatted arguments in the dev_warn() call. It removes
> > > the checkpatch warning:
> > > 
> > >   WARNING: Prefer using "%s", __func__ to embedded function names
> > >   #417: FILE: main_usb.c:417:
> > >   +"usb_device_reset fail status=%d\n", status);
> > > 
> > >   total: 0 errors, 1 warnings, 1058 lines checked
> > > 
> > > And after fix:
> > > 
> > >   main_usb.c has no obvious style problems and is ready for submission.
> > > 
> > > Signed-off-by: Chewie Lin 
> > > ---
> > >  drivers/staging/vt6656/main_usb.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/vt6656/main_usb.c 
> > > b/drivers/staging/vt6656/main_usb.c
> > > index 9e074e9..2d9e7af 100644
> > > --- a/drivers/staging/vt6656/main_usb.c
> > > +++ b/drivers/staging/vt6656/main_usb.c
> > > @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
> > >   status = usb_reset_device(priv->usb);
> > >   if (status)
> > >   dev_warn(>usb->dev,
> > > -  "usb_device_reset fail status=%d\n", status);
> > > +  "%s=%d\n", "usb_device_reset fail status", status);
> > >  }
> > >  
> > >  static void vnt_free_int_bufs(struct vnt_private *priv)
> > > 
> > 
> > As other people have said:
> > 
> > This function is usb_device_reset().  If that function name string is to be
> > used in the message, it should be done so by using __func__.
> > See http://marc.info/?l=linux-driver-devel=149095639202492=2
> > 
> > Or is the called (failing) function name is to be kept in the message (as it
> > is currently), then the message should contain "usb_reset_device" instead of
> > "usb_device_reset".  See 
> > http://marc.info/?l=linux-driver-devel=149098680312723=2
> 
> If this is to be changed at all, I suggest
> just getting rid of the function.

These are good points, but any feedback on the dev_warn() call itself? 
I was trying to fix the checkpatch warning on my first try.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Joe Perches]

On Fri, 2017-03-31 at 19:15 -0700, Randy Dunlap wrote:
> On 03/31/17 18:59, Chewie Lin wrote:
> > Replace string with formatted arguments in the dev_warn() call. It removes
> > the checkpatch warning:
> > 
> > WARNING: Prefer using "%s", __func__ to embedded function names
> > #417: FILE: main_usb.c:417:
> > +"usb_device_reset fail status=%d\n", status);
> > 
> > total: 0 errors, 1 warnings, 1058 lines checked
> > 
> > And after fix:
> > 
> > main_usb.c has no obvious style problems and is ready for submission.
> > 
> > Signed-off-by: Chewie Lin 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 9e074e9..2d9e7af 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
> > status = usb_reset_device(priv->usb);
> > if (status)
> > dev_warn(>usb->dev,
> > -"usb_device_reset fail status=%d\n", status);
> > +"%s=%d\n", "usb_device_reset fail status", status);
> >  }
> >  
> >  static void vnt_free_int_bufs(struct vnt_private *priv)
> > 
> 
> As other people have said:
> 
> This function is usb_device_reset().  If that function name string is to be
> used in the message, it should be done so by using __func__.
> See http://marc.info/?l=linux-driver-devel=149095639202492=2
> 
> Or is the called (failing) function name is to be kept in the message (as it
> is currently), then the message should contain "usb_reset_device" instead of
> "usb_device_reset".  See 
> http://marc.info/?l=linux-driver-devel=149098680312723=2

If this is to be changed at all, I suggest
just getting rid of the function.
---
 drivers/staging/vt6656/main_usb.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9daf4e..799aeeb6812d 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -407,16 +407,6 @@ static void vnt_free_rx_bufs(struct vnt_private *priv)
}
 }
 
-static void usb_device_reset(struct vnt_private *priv)
-{
-   int status;
-
-   status = usb_reset_device(priv->usb);
-   if (status)
-   dev_warn(>usb->dev,
-"usb_device_reset fail status=%d\n", status);
-}
-
 static void vnt_free_int_bufs(struct vnt_private *priv)
 {
kfree(priv->int_buf.data_buf);
@@ -995,7 +985,9 @@ vt6656_probe(struct usb_interface *intf, const struct 
usb_device_id *id)
 
SET_IEEE80211_DEV(priv->hw, >dev);
 
-   usb_device_reset(priv);
+   rc = usb_reset_device(priv->usb);
+   if (rc)
+   dev_warn(>usb->dev, "usb_reset_device failed: %d\n", rc);
 
clear_bit(DEVICE_FLAGS_DISCONNECTED, >flags);
vnt_reset_command_timer(priv);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: light: constify attribute_group structures

[simran singhal]

As the event_attrs field of iio_info structures is constant, so these
attribute_group structures can also be declared constant.

File size before:
   textdata bss dec hex filename
  150641528   0   1659240d0
drivers/staging/iio/light/tsl2x7x_core.o

File size after:
   textdata bss dec hex filename
  151921400   0   1659240d0
drivers/staging/iio/light/tsl2x7x_core.o

Signed-off-by: simran singhal 
---
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c 
b/drivers/staging/iio/light/tsl2x7x_core.c
index 0490c1d..9a0046a 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1676,7 +1676,7 @@ static const struct attribute_group 
tsl2X7X_device_attr_group_tbl[] = {
},
 };
 
-static struct attribute_group tsl2X7X_event_attr_group_tbl[] = {
+static const struct attribute_group tsl2X7X_event_attr_group_tbl[] = {
[ALS] = {
.attrs = tsl2X7X_ALS_event_attrs,
.name = "events",
-- 
2.7.4

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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Joe Perches]

On Fri, 2017-03-31 at 18:59 -0700, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning:
> 
>   WARNING: Prefer using "%s", __func__ to embedded function names
>   #417: FILE: main_usb.c:417:
>   +"usb_device_reset fail status=%d\n", status);
> 
>   total: 0 errors, 1 warnings, 1058 lines checked
> 
> And after fix:
> 
>   main_usb.c has no obvious style problems and is ready for submission.
> 
> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(>usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

So, what is failing?  usb_reset_device or usb_device_reset?

And this function is used exactly once, is trivial, and
would likely be better as direct code in the one place
it's used.

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

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Randy Dunlap]

On 03/31/17 18:59, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning:
> 
>   WARNING: Prefer using "%s", __func__ to embedded function names
>   #417: FILE: main_usb.c:417:
>   +"usb_device_reset fail status=%d\n", status);
> 
>   total: 0 errors, 1 warnings, 1058 lines checked
> 
> And after fix:
> 
>   main_usb.c has no obvious style problems and is ready for submission.
> 
> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(>usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);
>  }
>  
>  static void vnt_free_int_bufs(struct vnt_private *priv)
> 

As other people have said:

This function is usb_device_reset().  If that function name string is to be
used in the message, it should be done so by using __func__.
See http://marc.info/?l=linux-driver-devel=149095639202492=2

Or is the called (failing) function name is to be kept in the message (as it
is currently), then the message should contain "usb_reset_device" instead of
"usb_device_reset".  See 
http://marc.info/?l=linux-driver-devel=149098680312723=2



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

[PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Chewie Lin]

Replace string with formatted arguments in the dev_warn() call. It removes
the checkpatch warning:

WARNING: Prefer using "%s", __func__ to embedded function names
#417: FILE: main_usb.c:417:
+"usb_device_reset fail status=%d\n", status);

total: 0 errors, 1 warnings, 1058 lines checked

And after fix:

main_usb.c has no obvious style problems and is ready for submission.

Signed-off-by: Chewie Lin 
---
 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9..2d9e7af 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
status = usb_reset_device(priv->usb);
if (status)
dev_warn(>usb->dev,
-"usb_device_reset fail status=%d\n", status);
+"%s=%d\n", "usb_device_reset fail status", status);
 }
 
 static void vnt_free_int_bufs(struct vnt_private *priv)
-- 
2.10.0

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

[PATCH] eudyptula challenge

[Chewie Lin]

Hi all:

Better and improved changelog version 3. 

I'm submitting this patch as part of Eudyptula challenge to fix a
coding style problem.  Thanks for taking time on this trivial patch.

sl424

Chewie Lin (1):
drivers/staging/vt6656/main_usb.c: checkpatch warning

 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.10.0

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

Re: [PATCH 01/01] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Randy Dunlap]

On 03/31/17 13:58, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning.

Tell us what the warning was.
and then see if the patch fixes that warning.

> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(>usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);
>  }
>  
>  static void vnt_free_int_bufs(struct vnt_private *priv)
> 


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

Re: [RFC PATCH] binder: Don't require the binder lock when killed in binder_thread_read()

[Doug Anderson]

Hi,

On Fri, Mar 31, 2017 at 12:29 PM, Greg KH  wrote:
> On Fri, Mar 31, 2017 at 10:53:41AM -0700, Douglas Anderson wrote:
>> Sometimes when we're out of memory the OOM killer decides to kill a
>> process that's in binder_thread_read().  If we happen to be waiting
>> for work we'll get the kill signal and wake up.  That's good.  ...but
>> then we try to grab the binder lock before we return.  That's bad.
>>
>> The problem is that someone else might be holding the one true global
>> binder lock.  If that one other process is blocked then we can't
>> finish exiting.  In the worst case, the other process might be blocked
>> waiting for memory.  In that case we'll have a really hard time
>> exiting.
>>
>> On older kernels that don't have the OOM reaper (or something
>> similar), like kernel 4.4, this is a really big problem and we end up
>> with a simple deadlock because:
>> * Once we pick a process to OOM kill we won't pick another--we first
>>   wait for the process we picked to die.  The reasoning is that we've
>>   given the doomed process access to special memory pools so it can
>>   quit quickly and we don't have special pool memory to go around.
>> * We don't have any type of "special access donation" that would give
>>   the mutex holder our special access.
>>
>> On kernel 4.4 w/ binder patches, we easily see this happen:
>
> 
>
> How does your change interact with the recent "break up the binder big
> lock" patchset:
> https://android-review.googlesource.com/#/c/354698/
>
> Have you tried that series out to see if it helps out any?

I wasn't aware of that patchset.  Someone else on my team mentioned
that fine-grained locking was being worked on but I didn't know
patches were actually posted...  Probably it makes sense to just drop
my patch, then.  It was only making things marginally better even on
kernel 4.4 because I would just hit the next task that would refuse to
quit for a non-binder related reason.  :(

BTW: I presume that nobody has decided that it would be a wise idea to
pick the OOM reaper code back to any stable trees?  It seemed a bit
too scary to me, so I wrote a dumber (but easier to backport) solution
that avoided the deadlocks I was seeing.  http://crosreview.com/465189
and the 3 patches above it in case anyone else stumbles on this thread
and is curious.


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

[PATCH 01/01] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Chewie Lin]

Replace string with formatted arguments in the dev_warn() call. It removes
the checkpatch warning.

Signed-off-by: Chewie Lin 
---
 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9..2d9e7af 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
status = usb_reset_device(priv->usb);
if (status)
dev_warn(>usb->dev,
-"usb_device_reset fail status=%d\n", status);
+"%s=%d\n", "usb_device_reset fail status", status);
 }
 
 static void vnt_free_int_bufs(struct vnt_private *priv)
-- 
2.10.0

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

[PATCH] eudyptula challenge

[Chewie Lin]

Hi greg and forest:

I hate to bother but one more, with better changelog.

I'm submitting this patch as part of Eudyptula challenge to fix a
coding style problem.  Thanks for taking time on this trivial patch.

sl424

Chewie Lin (1):
drivers/staging/vt6656/main_usb.c: checkpatch warning

 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.10.0

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

Re: [RFC PATCH] binder: Don't require the binder lock when killed in binder_thread_read()

[Greg KH]

On Fri, Mar 31, 2017 at 10:53:41AM -0700, Douglas Anderson wrote:
> Sometimes when we're out of memory the OOM killer decides to kill a
> process that's in binder_thread_read().  If we happen to be waiting
> for work we'll get the kill signal and wake up.  That's good.  ...but
> then we try to grab the binder lock before we return.  That's bad.
> 
> The problem is that someone else might be holding the one true global
> binder lock.  If that one other process is blocked then we can't
> finish exiting.  In the worst case, the other process might be blocked
> waiting for memory.  In that case we'll have a really hard time
> exiting.
> 
> On older kernels that don't have the OOM reaper (or something
> similar), like kernel 4.4, this is a really big problem and we end up
> with a simple deadlock because:
> * Once we pick a process to OOM kill we won't pick another--we first
>   wait for the process we picked to die.  The reasoning is that we've
>   given the doomed process access to special memory pools so it can
>   quit quickly and we don't have special pool memory to go around.
> * We don't have any type of "special access donation" that would give
>   the mutex holder our special access.
> 
> On kernel 4.4 w/ binder patches, we easily see this happen:



How does your change interact with the recent "break up the binder big
lock" patchset:
https://android-review.googlesource.com/#/c/354698/

Have you tried that series out to see if it helps out any?

thanks,

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

[PATCH 2/2] staging: sm750fb: removed line continuations in quoted strings

[Prasant Jalan]

checkpatch gives WARNING: Avoid line continuations in quoted strings.

Trivial fix by removing line continuations and adding another quote at
the start of next line.

Signed-off-by: Prasant Jalan 
---
 drivers/staging/sm750fb/sm750.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 445c68d..386d4ad 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -837,15 +837,15 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int 
index)
 
/* some member of info->var had been set by fb_find_mode */
 
-   pr_info("Member of info->var is :\n\
-   xres=%d\n\
-   yres=%d\n\
-   xres_virtual=%d\n\
-   yres_virtual=%d\n\
-   xoffset=%d\n\
-   yoffset=%d\n\
-   bits_per_pixel=%d\n \
-   ...\n",
+   pr_info("Member of info->var is :\n"
+   "xres=%d\n"
+   "yres=%d\n"
+   "xres_virtual=%d\n"
+   "yres_virtual=%d\n"
+   "xoffset=%d\n"
+   "yoffset=%d\n"
+   "bits_per_pixel=%d\n"
+   " ...\n",
var->xres,
var->yres,
var->xres_virtual,
-- 
2.7.4

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

[PATCH 1/2] staging: sm750fb: fixing function return with lock held

[Prasant Jalan]

lynxfb_suspend() & lynxfb_resume() return on errors while holding
console_lock.

Adding 'goto' such that proper cleanups can be done before returning
from function and therefore console_lock can be released before
returning.

Signed-off-by: Prasant Jalan 
---
 drivers/staging/sm750fb/sm750.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c618c56..445c68d 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -419,7 +419,7 @@ static int lynxfb_suspend(struct pci_dev *pdev, 
pm_message_t mesg)
if (ret) {
dev_err(>dev,
"error:%d occurred in pci_save_state\n", ret);
-   return ret;
+   goto lynxfb_suspend_err;
}
 
ret = pci_set_power_state(pdev, pci_choose_state(pdev, mesg));
@@ -427,11 +427,13 @@ static int lynxfb_suspend(struct pci_dev *pdev, 
pm_message_t mesg)
dev_err(>dev,
"error:%d occurred in pci_set_power_state\n",
ret);
-   return ret;
+   goto lynxfb_suspend_err;
}
}
 
pdev->dev.power.power_state = mesg;
+
+lynxfb_suspend_err:
console_unlock();
return ret;
 }
@@ -456,7 +458,7 @@ static int lynxfb_resume(struct pci_dev *pdev)
if (ret) {
dev_err(>dev,
"error:%d occurred in pci_set_power_state\n", ret);
-   return ret;
+   goto lynxfb_resume_err;
}
 
if (pdev->dev.power.power_state.event != PM_EVENT_FREEZE) {
@@ -466,7 +468,7 @@ static int lynxfb_resume(struct pci_dev *pdev)
dev_err(>dev,
"error:%d occurred in pci_enable_device\n",
ret);
-   return ret;
+   goto lynxfb_resume_err;
}
pci_set_master(pdev);
}
@@ -498,6 +500,8 @@ static int lynxfb_resume(struct pci_dev *pdev)
}
 
pdev->dev.power.power_state.event = PM_EVENT_RESUME;
+
+lynxfb_resume_err:
console_unlock();
return ret;
 }
-- 
2.7.4

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

Re: [PATCH] drivers: fixed a checkpatch warning

[Joe Perches]

On Fri, 2017-03-31 at 01:40 -0700, Chewie Lin wrote:
> fixed a coding style error/warning.
> 
> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(>usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

Yeah, what Greg said.

Also most likely this should be

dev_warn(>usb->dev,
 "usb_reset_device fail status=%d\n", status);

Note the function is usb_device_reset, but the
function call that failed is usb_reset_device.

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

[PATCH v3] staging: unisys: visorbus: fix kernel BUG discovered by day0 testing

[David Kershner]

Fixes: 5b6f9b95f7ae ("staging: unisys: visorbus: get rid of create_bus_type.")

Kernel day0 testing robot reported a kernel BUG at drivers/base/driver.c!
with the following call stack:

[   14.963563] [ cut here ]
[   14.967298] kernel BUG at drivers/base/driver.c:153!
[   14.970948] invalid opcode:  [#1] SMP
[   14.974013] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.11.0-rc4-00790-g0789e2c #1
[   14.978221] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.9.3-20161025_171302-gandalf 04/01/2014
[   14.983417] task: 88001ea46040 task.stack: c9008000
[   14.987315] RIP: 0010:driver_register+0xa1/0xd0
[   14.990044] RSP: :c900be60 EFLAGS: 00010246
[   14.993039] RAX:  RBX: 831d4c20 RCX: 
[   14.997040] RDX: 004d RSI: 831d47c0 RDI: 831d4c20
[   15.001511] RBP: c900be78 R08: c900be78 R09: c900be7c
[   15.006163] R10:  R11: 0001 R12: 
[   15.010068] R13:  R14: 832f3923 R15: 
[   15.013715] FS:  () GS:88001fa0() 
knlGS:
[   15.017460] CS:  0010 DS:  ES:  CR0: 80050033
[   15.021268] CR2:  CR3: 03009000 CR4: 06b0
[   15.025633] Call Trace:
[   15.028069]  ? visorbus_register_visor_driver+0x3f/0x60
[   15.031065]  ? init_unisys+0x3a/0x90
[   15.033562]  ? device_resume_response+0x50/0x50
[   15.036083]  visorinput_init+0x10/0x20
[   15.038937]  do_one_initcall+0x9a/0x164
[   15.041838]  ? set_debug_rodata+0x12/0x12
[   15.045333]  kernel_init_freeable+0x11e/0x1a1
[   15.048369]  ? rest_init+0x80/0x80
[   15.050813]  kernel_init+0x9/0x100
[   15.053353]  ret_from_fork+0x2c/0x40
[   15.056009] Code: ff 85 c0 41 89 c4 75 13 48 8b 7b 70 31 f6 e8 97 16 be ff 
44 89 e0 5b 41 5c 5d c3 48 89 df e8 57 e1 ff ff 44 89 e0 5b 41 5c 5d c3 <0f> 0b 
48 8b 33 48 c7 c7 a0 dd d5 82 e8 ec f0 6f ff 48 8b 73 08
[   15.065144] RIP: driver_register+0xa1/0xd0 RSP: c900be60
[   15.068360] ---[ end trace 7d13369c38d80a8f ]---

This bug will occur if the visorbus driver is built-in to the kernel, and
the resulting kernel is run in an environment where visorbus devices are
NOT supported, and an attempt is made to load any of the drivers: visorhba,
visornic, or visorinput.

Checked to see if we have called bus_register, if not do not call
driver_register.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
Reviewed-by: David Binder 
---
 - Changes from v1 - Made changes suggested by GregKH. Instead of reverting
   patch, just added the check for initialization.
 - Changes from v2 - Fixed the spacing in a comment that was missed in v2.
   Sorry about that GregKH.  

 drivers/staging/unisys/visorbus/visorbus_main.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
b/drivers/staging/unisys/visorbus/visorbus_main.c
index 4348072..08acfe8 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -29,6 +29,7 @@
 #define CURRENT_FILE_PC VISOR_BUS_PC_visorbus_main_c
 #define POLLJIFFIES_NORMALCHANNEL 10
 
+static bool initialized; /* stores whether bus_registration was successful */
 static struct dentry *visorbus_debugfs_dir;
 
 /*
@@ -959,6 +960,9 @@ static void bus_device_info_init(
  */
 int visorbus_register_visor_driver(struct visor_driver *drv)
 {
+   if (!initialized)
+   return -ENODEV; /* can't register on a nonexistent bus */
+
drv->driver.name = drv->name;
drv->driver.bus = _type;
drv->driver.probe = visordriver_probe_device;
@@ -1297,6 +1301,7 @@ int visorbus_register_visor_driver(struct visor_driver 
*drv)
POSTCODE_LINUX(BUS_CREATE_ENTRY_PC, 0, 0, DIAG_SEVERITY_ERR);
goto error;
}
+   initialized = true;
 
bus_device_info_init(_driverinfo, "chipset", "visorchipset");
 
@@ -1322,5 +1327,6 @@ int visorbus_register_visor_driver(struct visor_driver 
*drv)
}
 
bus_unregister(_type);
+   initialized = false;
debugfs_remove_recursive(visorbus_debugfs_dir);
 }
-- 
1.9.1

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

[RFC PATCH] binder: Don't require the binder lock when killed in binder_thread_read()

[Douglas Anderson]

Sometimes when we're out of memory the OOM killer decides to kill a
process that's in binder_thread_read().  If we happen to be waiting
for work we'll get the kill signal and wake up.  That's good.  ...but
then we try to grab the binder lock before we return.  That's bad.

The problem is that someone else might be holding the one true global
binder lock.  If that one other process is blocked then we can't
finish exiting.  In the worst case, the other process might be blocked
waiting for memory.  In that case we'll have a really hard time
exiting.

On older kernels that don't have the OOM reaper (or something
similar), like kernel 4.4, this is a really big problem and we end up
with a simple deadlock because:
* Once we pick a process to OOM kill we won't pick another--we first
  wait for the process we picked to die.  The reasoning is that we've
  given the doomed process access to special memory pools so it can
  quit quickly and we don't have special pool memory to go around.
* We don't have any type of "special access donation" that would give
  the mutex holder our special access.

On kernel 4.4 w/ binder patches, we easily see this happen:

We just attempted this OOM kill:
  Killed process 4132 (Binder_1) total-vm:949904kB, anon-rss:4kB, file-rss:0kB

The doomed task:
  Stack traceback for pid 4132
   4132 3380  00   D  Binder_1
  Call trace:
   __switch_to+0x9c/0xa8
   __schedule+0x504/0x740
   schedule+0x88/0xa8
   schedule_preempt_disabled+0x28/0x44
   __mutex_lock_slowpath+0xf8/0x1a4
   mutex_lock+0x4c/0x68
   binder_thread_read+0x68c/0x11bc
   binder_ioctl_write_read.constprop.46+0x1e8/0x318
   binder_ioctl+0x370/0x778
   compat_SyS_ioctl+0x134/0x10ac
   el0_svc_naked+0x24/0x28

The binder lock holder:
   4001 3380  04   RBinder_7
  Call trace:
   __switch_to+0x9c/0xa8
   __schedule+0x504/0x740
   preempt_schedule_common+0x28/0x48
   preempt_schedule+0x24/0x2c
   _raw_spin_unlock+0x44/0x50
   list_lru_count_one+0x40/0x50
   super_cache_count+0x68/0xa0
   shrink_slab.part.54+0xfc/0x4a4
   shrink_zone+0xa8/0x198
   try_to_free_pages+0x408/0x590
   __alloc_pages_nodemask+0x60c/0x95c
   __read_swap_cache_async+0x98/0x214
   read_swap_cache_async+0x48/0x7c
   swapin_readahead+0x188/0x1b8
   handle_mm_fault+0xbf8/0x1050
   do_page_fault+0x140/0x2dc
   do_mem_abort+0x64/0xd8
  Exception stack: < ... cut ... >
   el1_da+0x18/0x78
   binder_ioctl+0x370/0x778
   compat_SyS_ioctl+0x134/0x10ac
   el0_svc_naked+0x24/0x28

There's really not a lot of reason to grab the binder lock when we're
just going to exit anyway.  Add a little bit of complexity to the code
to allow binder_thread_read() to sometimes return without the lock
held.

NOTE: to do this safely we need to make sure that we can safely do
these things _without_ the global binder lock:
* Muck with proc->ready_threads
* Clear a bitfield in thread->looper

It appears that those two operations don't need to be done together
and it's OK to have different locking solutions for the two.  Thus:

1. We change thread->looper to atomic_t and use atomic ops to muck
   with it.  This means we're not clobbering someone else's work with
   our read/modify/write.

   Note that I haven't confirmed that every modify of "thread->looper"
   can be done without the binder mutex or without some
   coarser-grained lock.  ...but clearing the
   BINDER_LOOPER_STATE_WAITING bit should be fine.  The only place
   looking at it is binder_deferred_flush().  Also: while erroring out
   we also may clear BINDER_LOOPER_STATE_NEED_RETURN.  This looks to
   be OK, though the code isn't trivial to follow.

2. We add a new fine-grained mutex (per "proc" structure) to guard all
   of the "_threads" counters.  Technically the only value we're
   modifying is "proc->ready_threads" and we're just decrementing it
   (so maybe we could use atomic_t here, too?).  ...but it appears
   that all of the "_threads" work together so protecting them
   together seems to make the most sense.

   Note that to avoid deadlock:
   * We never first grab the fine grained mutex and _then_ the binder
 mutex.
   * We always grab the fine grained mutex for short periods and never
 do anything blocking while holding it.

To sum it all up:

1. This patch does improve the chances that we will be able to kill a
   task quickly when we want to.  That means this patch has some
   merit.
2. Though this patch has merit, it isn't isn't actually all that
   critical because:
   2a) On newer kernels the OOM reaper should keep us from getting
   deadlocked.
   2b) Even on old kernels there are other deadlock cases we hit even
   if we fix binder in this way.

Signed-off-by: Douglas Anderson 
---
It's unclear to me if we really want this patch since, as per the
description, it's not all that critical.  I also don't personally have
enough context with Binder to prove that every change it makes is
guaranteed not to screw something up.

I post it in case 

Re: [PATCH v2] staging: unisys: visorbus: fix kernel BUG discovered by day0 testing

[Greg KH]

On Fri, Mar 31, 2017 at 10:56:28AM -0400, David Kershner wrote:
> Fixes: 5b6f9b95f7ae ("staging: unisys: visorbus: get rid of create_bus_type.")
> 
> Kernel day0 testing robot reported a kernel BUG at drivers/base/driver.c!
> with the following call stack:
> 
> [   14.963563] [ cut here ]
> [   14.967298] kernel BUG at drivers/base/driver.c:153!
> [   14.970948] invalid opcode:  [#1] SMP
> [   14.974013] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 4.11.0-rc4-00790-g0789e2c #1
> [   14.978221] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.9.3-20161025_171302-gandalf 04/01/2014
> [   14.983417] task: 88001ea46040 task.stack: c9008000
> [   14.987315] RIP: 0010:driver_register+0xa1/0xd0
> [   14.990044] RSP: :c900be60 EFLAGS: 00010246
> [   14.993039] RAX:  RBX: 831d4c20 RCX: 
> 
> [   14.997040] RDX: 004d RSI: 831d47c0 RDI: 
> 831d4c20
> [   15.001511] RBP: c900be78 R08: c900be78 R09: 
> c900be7c
> [   15.006163] R10:  R11: 0001 R12: 
> 
> [   15.010068] R13:  R14: 832f3923 R15: 
> 
> [   15.013715] FS:  () GS:88001fa0() 
> knlGS:
> [   15.017460] CS:  0010 DS:  ES:  CR0: 80050033
> [   15.021268] CR2:  CR3: 03009000 CR4: 
> 06b0
> [   15.025633] Call Trace:
> [   15.028069]  ? visorbus_register_visor_driver+0x3f/0x60
> [   15.031065]  ? init_unisys+0x3a/0x90
> [   15.033562]  ? device_resume_response+0x50/0x50
> [   15.036083]  visorinput_init+0x10/0x20
> [   15.038937]  do_one_initcall+0x9a/0x164
> [   15.041838]  ? set_debug_rodata+0x12/0x12
> [   15.045333]  kernel_init_freeable+0x11e/0x1a1
> [   15.048369]  ? rest_init+0x80/0x80
> [   15.050813]  kernel_init+0x9/0x100
> [   15.053353]  ret_from_fork+0x2c/0x40
> [   15.056009] Code: ff 85 c0 41 89 c4 75 13 48 8b 7b 70 31 f6 e8 97 16 be ff 
> 44 89 e0 5b 41 5c 5d c3 48 89 df e8 57 e1 ff ff 44 89 e0 5b 41 5c 5d c3 <0f> 
> 0b 48 8b 33 48 c7 c7 a0 dd d5 82 e8 ec f0 6f ff 48 8b 73 08
> [   15.065144] RIP: driver_register+0xa1/0xd0 RSP: c900be60
> [   15.068360] ---[ end trace 7d13369c38d80a8f ]---
> 
> This bug will occur if the visorbus driver is built-in to the kernel, and
> the resulting kernel is run in an environment where visorbus devices are
> NOT supported, and an attempt is made to load any of the drivers: visorhba,
> visornic, or visorinput.
> 
> Checked to see if we have called bus_register, if not do not call
> driver_register.
> 
> Signed-off-by: David Kershner 
> Reviewed-by: Tim Sell 
> ---
>  - Changes from v1 - Made changes suggested by GregKH. Instead of reverting
>patch, just added the check for initialization.
>  drivers/staging/unisys/visorbus/visorbus_main.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index 4348072..9490977 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -29,6 +29,7 @@
>  #define CURRENT_FILE_PC VISOR_BUS_PC_visorbus_main_c
>  #define POLLJIFFIES_NORMALCHANNEL 10
>  
> +static bool initialized; /* stores whether bus_registration was successful */
>  static struct dentry *visorbus_debugfs_dir;
>  
>  /*
> @@ -959,6 +960,9 @@ static void bus_device_info_init(
>   */
>  int visorbus_register_visor_driver(struct visor_driver *drv)
>  {
> + if (!initialized)
> + return -ENODEV; /*can't register on a nonexistent bus*/

Same question about the lack of ' ' in your comment :(

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

[PATCH 1/3] staging: iio: accel: Remove useless type conversion

[simran singhal]

Some type conversions like casting a pointer to a pointer of same type,
casting to the original type using addressof(&) operator etc. are not
needed. Therefore, remove them. Done using coccinelle:

@@
type t;
t *p;
t a;
@@
(
- (t)(a)
+ a
|
- (t *)(p)
+ p
|
- (t *)()
+ 
)

Signed-off-by: simran singhal 
---
 drivers/staging/iio/accel/adis16201.c | 2 +-
 drivers/staging/iio/accel/adis16203.c | 2 +-
 drivers/staging/iio/accel/adis16209.c | 2 +-
 drivers/staging/iio/accel/adis16240.c | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/accel/adis16201.c 
b/drivers/staging/iio/accel/adis16201.c
index fbc2406..b7381d3 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -228,7 +228,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
if (ret)
return ret;
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
}
diff --git a/drivers/staging/iio/accel/adis16203.c 
b/drivers/staging/iio/accel/adis16203.c
index b59755a..25e5e52 100644
--- a/drivers/staging/iio/accel/adis16203.c
+++ b/drivers/staging/iio/accel/adis16203.c
@@ -211,7 +211,7 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
mutex_unlock(_dev->mlock);
return IIO_VAL_INT;
diff --git a/drivers/staging/iio/accel/adis16209.c 
b/drivers/staging/iio/accel/adis16209.c
index 52fa2e0..7aac310 100644
--- a/drivers/staging/iio/accel/adis16209.c
+++ b/drivers/staging/iio/accel/adis16209.c
@@ -259,7 +259,7 @@ static int adis16209_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
}
diff --git a/drivers/staging/iio/accel/adis16240.c 
b/drivers/staging/iio/accel/adis16240.c
index 6e3c95c..2c55b65 100644
--- a/drivers/staging/iio/accel/adis16240.c
+++ b/drivers/staging/iio/accel/adis16240.c
@@ -220,7 +220,7 @@ static ssize_t adis16240_spi_read_signed(struct device *dev,
if (val & ADIS16240_ERROR_ACTIVE)
adis_check_status(st);
 
-   val = (s16)(val << shift) >> shift;
+   val = val << shift >> shift;
return sprintf(buf, "%d\n", val);
 }
 
@@ -294,7 +294,7 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
case IIO_CHAN_INFO_PEAK:
@@ -305,7 +305,7 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
return ret;
}
val16 &= (1 << bits) - 1;
-   val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
+   val16 = val16 << (16 - bits) >> (16 - bits);
*val = val16;
return IIO_VAL_INT;
}
-- 
2.7.4

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

[PATCH 2/3] staging: iio: frequency: Remove useless type conversion

[simran singhal]

Some type conversions like casting a pointer to a pointer of same type,
casting to the original type using addressof(&) operator etc. are not
needed. Therefore, remove them. Done using coccinelle:

@@
type t;
t *p;
t a;
@@
(
- (t)(a)
+ a
|
- (t *)(p)
+ p
|
- (t *)()
+ 
)

Signed-off-by: simran singhal 
---
 drivers/staging/iio/frequency/ad9832.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c 
b/drivers/staging/iio/frequency/ad9832.c
index 425b8ab..01bdf8e 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -119,7 +119,7 @@ struct ad9832_state {
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long 
fout)
 {
unsigned long long freqreg = (u64)fout *
-(u64)((u64)1L << AD9832_FREQ_BITS);
+(u64)1L << AD9832_FREQ_BITS;
do_div(freqreg, mclk);
return freqreg;
 }
-- 
2.7.4

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

[PATCH 3/3] staging: iio: light: Remove useless type conversion

[simran singhal]

Some type conversions like casting a pointer to a pointer of same type,
casting to the original type using addressof(&) operator etc. are not
needed. Therefore, remove them. Done using coccinelle:

@@
type t;
t *p;
t a;
@@
(
- (t)(a)
+ a
|
- (t *)(p)
+ p
|
- (t *)()
+ 
)

Signed-off-by: simran singhal 
---
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c 
b/drivers/staging/iio/light/tsl2x7x_core.c
index ea15bc1..0490c1d 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -624,7 +624,7 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
dev_info(>client->dev,
 "%s als_calibrate completed\n", chip->client->name);
 
-   return (int)gain_trim_val;
+   return gain_trim_val;
 }
 
 static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
-- 
2.7.4

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

[PATCH 0/3] staging: iio: Remove useless type conversion

[simran singhal]

This patch-series removes some type conversions like casting a pointer to a 
pointer of same type,
casting to the original type using addressof(&) operator etc.

simran singhal (3):
  staging: iio: accel: Remove useless type conversion
  staging: iio: frequency: Remove useless type conversion
  staging: iio: light: Remove useless type conversion

 drivers/staging/iio/accel/adis16201.c| 2 +-
 drivers/staging/iio/accel/adis16203.c| 2 +-
 drivers/staging/iio/accel/adis16209.c| 2 +-
 drivers/staging/iio/accel/adis16240.c| 6 +++---
 drivers/staging/iio/frequency/ad9832.c   | 2 +-
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.7.4

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

[PATCH v2] staging: unisys: visorbus: fix kernel BUG discovered by day0 testing

[David Kershner]

Fixes: 5b6f9b95f7ae ("staging: unisys: visorbus: get rid of create_bus_type.")

Kernel day0 testing robot reported a kernel BUG at drivers/base/driver.c!
with the following call stack:

[   14.963563] [ cut here ]
[   14.967298] kernel BUG at drivers/base/driver.c:153!
[   14.970948] invalid opcode:  [#1] SMP
[   14.974013] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.11.0-rc4-00790-g0789e2c #1
[   14.978221] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.9.3-20161025_171302-gandalf 04/01/2014
[   14.983417] task: 88001ea46040 task.stack: c9008000
[   14.987315] RIP: 0010:driver_register+0xa1/0xd0
[   14.990044] RSP: :c900be60 EFLAGS: 00010246
[   14.993039] RAX:  RBX: 831d4c20 RCX: 
[   14.997040] RDX: 004d RSI: 831d47c0 RDI: 831d4c20
[   15.001511] RBP: c900be78 R08: c900be78 R09: c900be7c
[   15.006163] R10:  R11: 0001 R12: 
[   15.010068] R13:  R14: 832f3923 R15: 
[   15.013715] FS:  () GS:88001fa0() 
knlGS:
[   15.017460] CS:  0010 DS:  ES:  CR0: 80050033
[   15.021268] CR2:  CR3: 03009000 CR4: 06b0
[   15.025633] Call Trace:
[   15.028069]  ? visorbus_register_visor_driver+0x3f/0x60
[   15.031065]  ? init_unisys+0x3a/0x90
[   15.033562]  ? device_resume_response+0x50/0x50
[   15.036083]  visorinput_init+0x10/0x20
[   15.038937]  do_one_initcall+0x9a/0x164
[   15.041838]  ? set_debug_rodata+0x12/0x12
[   15.045333]  kernel_init_freeable+0x11e/0x1a1
[   15.048369]  ? rest_init+0x80/0x80
[   15.050813]  kernel_init+0x9/0x100
[   15.053353]  ret_from_fork+0x2c/0x40
[   15.056009] Code: ff 85 c0 41 89 c4 75 13 48 8b 7b 70 31 f6 e8 97 16 be ff 
44 89 e0 5b 41 5c 5d c3 48 89 df e8 57 e1 ff ff 44 89 e0 5b 41 5c 5d c3 <0f> 0b 
48 8b 33 48 c7 c7 a0 dd d5 82 e8 ec f0 6f ff 48 8b 73 08
[   15.065144] RIP: driver_register+0xa1/0xd0 RSP: c900be60
[   15.068360] ---[ end trace 7d13369c38d80a8f ]---

This bug will occur if the visorbus driver is built-in to the kernel, and
the resulting kernel is run in an environment where visorbus devices are
NOT supported, and an attempt is made to load any of the drivers: visorhba,
visornic, or visorinput.

Checked to see if we have called bus_register, if not do not call
driver_register.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 - Changes from v1 - Made changes suggested by GregKH. Instead of reverting
   patch, just added the check for initialization.
 drivers/staging/unisys/visorbus/visorbus_main.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
b/drivers/staging/unisys/visorbus/visorbus_main.c
index 4348072..9490977 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -29,6 +29,7 @@
 #define CURRENT_FILE_PC VISOR_BUS_PC_visorbus_main_c
 #define POLLJIFFIES_NORMALCHANNEL 10
 
+static bool initialized; /* stores whether bus_registration was successful */
 static struct dentry *visorbus_debugfs_dir;
 
 /*
@@ -959,6 +960,9 @@ static void bus_device_info_init(
  */
 int visorbus_register_visor_driver(struct visor_driver *drv)
 {
+   if (!initialized)
+   return -ENODEV; /*can't register on a nonexistent bus*/
+
drv->driver.name = drv->name;
drv->driver.bus = _type;
drv->driver.probe = visordriver_probe_device;
@@ -1297,6 +1301,7 @@ int visorbus_register_visor_driver(struct visor_driver 
*drv)
POSTCODE_LINUX(BUS_CREATE_ENTRY_PC, 0, 0, DIAG_SEVERITY_ERR);
goto error;
}
+   initialized = true;
 
bus_device_info_init(_driverinfo, "chipset", "visorchipset");
 
@@ -1322,5 +1327,6 @@ int visorbus_register_visor_driver(struct visor_driver 
*drv)
}
 
bus_unregister(_type);
+   initialized = false;
debugfs_remove_recursive(visorbus_debugfs_dir);
 }
-- 
1.9.1

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

[PATCH 12/13] staging: most: usb: fix size overflow

[Christian Gromm]

From: Andrey Shvetsov 

Despite the user payload may not be bigger than (2**16 - 1) bytes, the
final packet size may be bigger because of the gap space needed for the
controller.

This patch removes the temporary variables of the type u16 that are used
to hold the offsets that may be bigger than 2**16 bytes.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index 6e94ee2b4fb3..ad907e9c59bb 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -281,7 +281,6 @@ static int hdm_add_padding(struct most_dev *mdev, int 
channel, struct mbo *mbo)
struct most_channel_config *conf = >conf[channel];
unsigned int frame_size = get_stream_frame_size(conf);
unsigned int j, num_frames;
-   u16 rd_addr, wr_addr;
 
if (!frame_size)
return -EIO;
@@ -293,13 +292,10 @@ static int hdm_add_padding(struct most_dev *mdev, int 
channel, struct mbo *mbo)
return -EIO;
}
 
-   for (j = 1; j < num_frames; j++) {
-   wr_addr = (num_frames - j) * USB_MTU;
-   rd_addr = (num_frames - j) * frame_size;
-   memmove(mbo->virt_address + wr_addr,
-   mbo->virt_address + rd_addr,
+   for (j = num_frames - 1; j > 0; j--)
+   memmove(mbo->virt_address + j * USB_MTU,
+   mbo->virt_address + j * frame_size,
frame_size);
-   }
mbo->buffer_length = num_frames * USB_MTU;
return 0;
 }
-- 
2.11.0

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

[PATCH 10/13] staging: most: destroy cdev by channel disconnect

[Christian Gromm]

From: Andrey Shvetsov 

Currently, when the channel disappears whereas the character device is
open by an application, the character device will be destroyed only
after the application closes the character device.

When the channel appears again before the application closes the
channel, the channel cannot be linked to the character device with the
same name.

This patch changes the described behavior and destroys the character
device by the disconnecting the channel from the AIM.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/aim-cdev/cdev.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c 
b/drivers/staging/most/aim-cdev/cdev.c
index 7f51024dc5eb..1e5cbc893496 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -99,11 +99,16 @@ static void destroy_cdev(struct aim_channel *c)
 
device_destroy(aim_class, c->devno);
cdev_del(>cdev);
-   kfifo_free(>fifo);
spin_lock_irqsave(_list_lock, flags);
list_del(>list);
spin_unlock_irqrestore(_list_lock, flags);
+}
+
+static void destroy_channel(struct aim_channel *c)
+{
ida_simple_remove(_id, MINOR(c->devno));
+   kfifo_free(>fifo);
+   kfree(c);
 }
 
 /**
@@ -170,9 +175,8 @@ static int aim_close(struct inode *inode, struct file *filp)
stop_channel(c);
mutex_unlock(>io_mutex);
} else {
-   destroy_cdev(c);
mutex_unlock(>io_mutex);
-   kfree(c);
+   destroy_channel(c);
}
return 0;
 }
@@ -337,14 +341,14 @@ static int aim_disconnect_channel(struct most_interface 
*iface, int channel_id)
spin_lock(>unlink);
c->dev = NULL;
spin_unlock(>unlink);
+   destroy_cdev(c);
if (c->access_ref) {
stop_channel(c);
wake_up_interruptible(>wq);
mutex_unlock(>io_mutex);
} else {
-   destroy_cdev(c);
mutex_unlock(>io_mutex);
-   kfree(c);
+   destroy_channel(c);
}
return 0;
 }
@@ -546,7 +550,7 @@ static void __exit mod_exit(void)
 
list_for_each_entry_safe(c, tmp, _list, list) {
destroy_cdev(c);
-   kfree(c);
+   destroy_channel(c);
}
class_destroy(aim_class);
unregister_chrdev_region(aim_devno, 1);
-- 
2.11.0

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

[PATCH 13/13] staging: most: usb: Fix setting of writable DCI registers

[Christian Gromm]

The function store_value is about writing a value, but the code has been
passing ro_regs array to get_static_reg_addr, which prevented setting
the writable registers.

Noticed when trying to setup the EUI48.

Signed-off-by: Alex Riesen 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index ad907e9c59bb..477c0ed305d3 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -1005,7 +1005,7 @@ static ssize_t store_value(struct most_dci_obj *dci_obj,
err = drci_wr_reg(usb_dev, dci_obj->reg_addr, val);
else if (!strcmp(name, "sync_ep"))
err = start_sync_ep(usb_dev, val);
-   else if (!get_static_reg_addr(ro_regs, name, _addr))
+   else if (!get_static_reg_addr(rw_regs, name, _addr))
err = drci_wr_reg(usb_dev, reg_addr, val);
else
return -EFAULT;
-- 
2.11.0

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

[PATCH 11/13] staging: most: usb: fix calculation of the extra_len

[Christian Gromm]

From: Andrey Shvetsov 

The final size of the buffer used for the streaming transfer consists of
the size for the user payload (buffer_size) and the size for the gaps
needed by the controller (extra_len).

The current implementation of the hdm_configure_channel() corrects the
buffer size down to the next appropriate for the hardware value, that is
the whole number of frames, but uses the old unaligned value to
calculate the extra_len.

Current patch fixes the described problem.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 27 +--
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c 
b/drivers/staging/most/hdm-usb/hdm_usb.c
index 65211d1824b7..6e94ee2b4fb3 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -649,8 +649,6 @@ static int hdm_configure_channel(struct most_interface 
*iface, int channel,
 {
unsigned int num_frames;
unsigned int frame_size;
-   unsigned int temp_size;
-   unsigned int tail_space;
struct most_dev *mdev = to_mdev(iface);
struct device *dev = >usb_device->dev;
 
@@ -685,7 +683,6 @@ static int hdm_configure_channel(struct most_interface 
*iface, int channel,
}
 
mdev->padding_active[channel] = true;
-   temp_size = conf->buffer_size;
 
frame_size = get_stream_frame_size(conf);
if (frame_size == 0 || frame_size > USB_MTU) {
@@ -693,25 +690,19 @@ static int hdm_configure_channel(struct most_interface 
*iface, int channel,
return -EINVAL;
}
 
+   num_frames = conf->buffer_size / frame_size;
+
if (conf->buffer_size % frame_size) {
-   u16 tmp_val;
-
-   tmp_val = conf->buffer_size / frame_size;
-   conf->buffer_size = tmp_val * frame_size;
-   dev_notice(dev,
-  "Channel %d - rounding buffer size to %d bytes, 
channel config says %d bytes\n",
-  channel,
-  conf->buffer_size,
-  temp_size);
-   }
+   u16 old_size = conf->buffer_size;
 
-   num_frames = conf->buffer_size / frame_size;
-   tail_space = num_frames * (USB_MTU - frame_size);
-   temp_size += tail_space;
+   conf->buffer_size = num_frames * frame_size;
+   dev_warn(dev, "%s: fixed buffer size (%d -> %d)\n",
+mdev->suffix[channel], old_size, conf->buffer_size);
+   }
 
/* calculate extra length to comply w/ HW padding */
-   conf->extra_len = (DIV_ROUND_UP(temp_size, USB_MTU) * USB_MTU)
- - conf->buffer_size;
+   conf->extra_len = num_frames * (USB_MTU - frame_size);
+
 exit:
mdev->conf[channel] = *conf;
if (conf->data_type == MOST_CH_ASYNC) {
-- 
2.11.0

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

[PATCH 07/13] staging: most: consolidate channel attributes

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces 13 temporary variables representing the attributes
to control the channel with an array of 13 elements to keep the
corresponding code compact.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 61 +---
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index 9cbd893989b3..720b9ced1a9d 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -333,17 +333,6 @@ static ssize_t channel_starving_show(struct most_c_obj *c,
return snprintf(buf, PAGE_SIZE, "%d\n", c->is_starving);
 }
 
-#define create_show_channel_attribute(val) \
-   static struct most_c_attr most_chnl_attr_##val = __ATTR_RO(val)
-
-create_show_channel_attribute(available_directions);
-create_show_channel_attribute(available_datatypes);
-create_show_channel_attribute(number_of_packet_buffers);
-create_show_channel_attribute(number_of_stream_buffers);
-create_show_channel_attribute(size_of_stream_buffer);
-create_show_channel_attribute(size_of_packet_buffer);
-create_show_channel_attribute(channel_starving);
-
 static ssize_t set_number_of_buffers_show(struct most_c_obj *c,
  struct most_c_attr *attr,
  char *buf)
@@ -485,33 +474,39 @@ static ssize_t set_packets_per_xact_store(struct 
most_c_obj *c,
return count;
 }
 
-#define create_channel_attribute(value) \
-   static struct most_c_attr most_chnl_attr_##value = __ATTR_RW(value)
-
-create_channel_attribute(set_buffer_size);
-create_channel_attribute(set_number_of_buffers);
-create_channel_attribute(set_direction);
-create_channel_attribute(set_datatype);
-create_channel_attribute(set_subbuffer_size);
-create_channel_attribute(set_packets_per_xact);
+static struct most_c_attr most_c_attrs[] = {
+   __ATTR_RO(available_directions),
+   __ATTR_RO(available_datatypes),
+   __ATTR_RO(number_of_packet_buffers),
+   __ATTR_RO(number_of_stream_buffers),
+   __ATTR_RO(size_of_stream_buffer),
+   __ATTR_RO(size_of_packet_buffer),
+   __ATTR_RO(channel_starving),
+   __ATTR_RW(set_buffer_size),
+   __ATTR_RW(set_number_of_buffers),
+   __ATTR_RW(set_direction),
+   __ATTR_RW(set_datatype),
+   __ATTR_RW(set_subbuffer_size),
+   __ATTR_RW(set_packets_per_xact),
+};
 
 /**
  * most_channel_def_attrs - array of default attributes of channel object
  */
 static struct attribute *most_channel_def_attrs[] = {
-   _chnl_attr_available_directions.attr,
-   _chnl_attr_available_datatypes.attr,
-   _chnl_attr_number_of_packet_buffers.attr,
-   _chnl_attr_number_of_stream_buffers.attr,
-   _chnl_attr_size_of_packet_buffer.attr,
-   _chnl_attr_size_of_stream_buffer.attr,
-   _chnl_attr_set_number_of_buffers.attr,
-   _chnl_attr_set_buffer_size.attr,
-   _chnl_attr_set_direction.attr,
-   _chnl_attr_set_datatype.attr,
-   _chnl_attr_set_subbuffer_size.attr,
-   _chnl_attr_set_packets_per_xact.attr,
-   _chnl_attr_channel_starving.attr,
+   _c_attrs[0].attr,
+   _c_attrs[1].attr,
+   _c_attrs[2].attr,
+   _c_attrs[3].attr,
+   _c_attrs[4].attr,
+   _c_attrs[5].attr,
+   _c_attrs[6].attr,
+   _c_attrs[7].attr,
+   _c_attrs[8].attr,
+   _c_attrs[9].attr,
+   _c_attrs[10].attr,
+   _c_attrs[11].attr,
+   _c_attrs[12].attr,
NULL,
 };
 
-- 
2.11.0

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

[PATCH 09/13] staging: most: consolidate attributes for list of links

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces three temporary variables representing the
attributes to control the links between the AIMs and HDMs with an array
of three elements to keep the corresponding code compact.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index 7fc7cb39ea2b..675b2a9e66c1 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -854,9 +854,6 @@ static ssize_t links_show(struct most_aim_obj *aim_obj,
return offs;
 }
 
-static struct most_aim_attribute most_aim_attr_links =
-   __ATTR_RO(links);
-
 /**
  * split_string - parses and changes string in the buffer buf and
  * splits it into two mandatory and one optional substrings.
@@ -1000,9 +997,6 @@ static ssize_t add_link_store(struct most_aim_obj *aim_obj,
return len;
 }
 
-static struct most_aim_attribute most_aim_attr_add_link =
-   __ATTR_WO(add_link);
-
 /**
  * remove_link_store - store function for remove_link attribute
  * @aim_obj: pointer to AIM object
@@ -1043,13 +1037,16 @@ static ssize_t remove_link_store(struct most_aim_obj 
*aim_obj,
return len;
 }
 
-static struct most_aim_attribute most_aim_attr_remove_link =
-   __ATTR_WO(remove_link);
+static struct most_aim_attribute most_aim_attrs[] = {
+   __ATTR_RO(links),
+   __ATTR_WO(add_link),
+   __ATTR_WO(remove_link),
+};
 
 static struct attribute *most_aim_def_attrs[] = {
-   _aim_attr_links.attr,
-   _aim_attr_add_link.attr,
-   _aim_attr_remove_link.attr,
+   _aim_attrs[0].attr,
+   _aim_attrs[1].attr,
+   _aim_attrs[2].attr,
NULL,
 };
 
-- 
2.11.0

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

[PATCH 08/13] staging: most: separate property showing links

[Christian Gromm]

From: Andrey Shvetsov 

Currently there are following properties of the AIM to control the links
to the HDMs:
  - write-only "remove_link" used to remove one link from a list,
  - read/write "add_link" used to add one link into a list and list all
links.

This patch moves the read functionality of the "add_link" into the new
read-only property "links" to build consistent set of properties to
control the list of links.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index 720b9ced1a9d..7fc7cb39ea2b 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -831,9 +831,9 @@ static void most_aim_release(struct kobject *kobj)
kfree(aim_obj);
 }
 
-static ssize_t add_link_show(struct most_aim_obj *aim_obj,
-struct most_aim_attribute *attr,
-char *buf)
+static ssize_t links_show(struct most_aim_obj *aim_obj,
+ struct most_aim_attribute *attr,
+ char *buf)
 {
struct most_c_obj *c;
struct most_inst_obj *i;
@@ -854,6 +854,9 @@ static ssize_t add_link_show(struct most_aim_obj *aim_obj,
return offs;
 }
 
+static struct most_aim_attribute most_aim_attr_links =
+   __ATTR_RO(links);
+
 /**
  * split_string - parses and changes string in the buffer buf and
  * splits it into two mandatory and one optional substrings.
@@ -998,7 +1001,7 @@ static ssize_t add_link_store(struct most_aim_obj *aim_obj,
 }
 
 static struct most_aim_attribute most_aim_attr_add_link =
-   __ATTR_RW(add_link);
+   __ATTR_WO(add_link);
 
 /**
  * remove_link_store - store function for remove_link attribute
@@ -1044,6 +1047,7 @@ static struct most_aim_attribute 
most_aim_attr_remove_link =
__ATTR_WO(remove_link);
 
 static struct attribute *most_aim_def_attrs[] = {
+   _aim_attr_links.attr,
_aim_attr_add_link.attr,
_aim_attr_remove_link.attr,
NULL,
-- 
2.11.0

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

[PATCH 05/13] staging: most: use __ATTR_RW in create_channel_attribute

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces the use of a macro MOST_CHNL_ATTR in the macro
create_channel_attribute with a macro __ATTR_RW.

Additionally, this patch removes not anymore used macro MOST_CHNL_ATTR.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index f3a2dc4f4b47..826984c2424d 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -127,10 +127,6 @@ struct most_c_attr {
 
 #define to_channel_attr(a) container_of(a, struct most_c_attr, attr)
 
-#define MOST_CHNL_ATTR(_name, _mode, _show, _store) \
-   struct most_c_attr most_chnl_attr_##_name = \
-   __ATTR(_name, _mode, _show, _store)
-
 /**
  * channel_attr_show - show function of channel object
  * @kobj: pointer to its kobject
@@ -352,14 +348,14 @@ create_show_channel_attribute(size_of_stream_buffer);
 create_show_channel_attribute(size_of_packet_buffer);
 create_show_channel_attribute(channel_starving);
 
-static ssize_t show_set_number_of_buffers(struct most_c_obj *c,
+static ssize_t set_number_of_buffers_show(struct most_c_obj *c,
  struct most_c_attr *attr,
  char *buf)
 {
return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.num_buffers);
 }
 
-static ssize_t store_set_number_of_buffers(struct most_c_obj *c,
+static ssize_t set_number_of_buffers_store(struct most_c_obj *c,
   struct most_c_attr *attr,
   const char *buf,
   size_t count)
@@ -371,14 +367,14 @@ static ssize_t store_set_number_of_buffers(struct 
most_c_obj *c,
return count;
 }
 
-static ssize_t show_set_buffer_size(struct most_c_obj *c,
+static ssize_t set_buffer_size_show(struct most_c_obj *c,
struct most_c_attr *attr,
char *buf)
 {
return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.buffer_size);
 }
 
-static ssize_t store_set_buffer_size(struct most_c_obj *c,
+static ssize_t set_buffer_size_store(struct most_c_obj *c,
 struct most_c_attr *attr,
 const char *buf,
 size_t count)
@@ -390,7 +386,7 @@ static ssize_t store_set_buffer_size(struct most_c_obj *c,
return count;
 }
 
-static ssize_t show_set_direction(struct most_c_obj *c,
+static ssize_t set_direction_show(struct most_c_obj *c,
  struct most_c_attr *attr,
  char *buf)
 {
@@ -401,7 +397,7 @@ static ssize_t show_set_direction(struct most_c_obj *c,
return snprintf(buf, PAGE_SIZE, "unconfigured\n");
 }
 
-static ssize_t store_set_direction(struct most_c_obj *c,
+static ssize_t set_direction_store(struct most_c_obj *c,
   struct most_c_attr *attr,
   const char *buf,
   size_t count)
@@ -421,7 +417,7 @@ static ssize_t store_set_direction(struct most_c_obj *c,
return count;
 }
 
-static ssize_t show_set_datatype(struct most_c_obj *c,
+static ssize_t set_datatype_show(struct most_c_obj *c,
 struct most_c_attr *attr,
 char *buf)
 {
@@ -434,7 +430,7 @@ static ssize_t show_set_datatype(struct most_c_obj *c,
return snprintf(buf, PAGE_SIZE, "unconfigured\n");
 }
 
-static ssize_t store_set_datatype(struct most_c_obj *c,
+static ssize_t set_datatype_store(struct most_c_obj *c,
  struct most_c_attr *attr,
  const char *buf,
  size_t count)
@@ -455,14 +451,14 @@ static ssize_t store_set_datatype(struct most_c_obj *c,
return count;
 }
 
-static ssize_t show_set_subbuffer_size(struct most_c_obj *c,
+static ssize_t set_subbuffer_size_show(struct most_c_obj *c,
   struct most_c_attr *attr,
   char *buf)
 {
return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.subbuffer_size);
 }
 
-static ssize_t store_set_subbuffer_size(struct most_c_obj *c,
+static ssize_t set_subbuffer_size_store(struct most_c_obj *c,
struct most_c_attr *attr,
const char *buf,
size_t count)
@@ -474,14 +470,14 @@ static ssize_t store_set_subbuffer_size(struct most_c_obj 
*c,
return count;
 }
 
-static ssize_t show_set_packets_per_xact(struct 

[PATCH 06/13] staging: most: use __ATTR_RO in create_show_channel_attribute

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces the use of a macro MOST_CHNL_ATTR in the macro
create_show_channel_attribute with a macro __ATTR_RO.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index 826984c2424d..9cbd893989b3 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -252,7 +252,7 @@ static void most_channel_release(struct kobject *kobj)
kfree(c);
 }
 
-static ssize_t show_available_directions(struct most_c_obj *c,
+static ssize_t available_directions_show(struct most_c_obj *c,
 struct most_c_attr *attr,
 char *buf)
 {
@@ -267,7 +267,7 @@ static ssize_t show_available_directions(struct most_c_obj 
*c,
return strlen(buf);
 }
 
-static ssize_t show_available_datatypes(struct most_c_obj *c,
+static ssize_t available_datatypes_show(struct most_c_obj *c,
struct most_c_attr *attr,
char *buf)
 {
@@ -286,10 +286,9 @@ static ssize_t show_available_datatypes(struct most_c_obj 
*c,
return strlen(buf);
 }
 
-static
-ssize_t show_number_of_packet_buffers(struct most_c_obj *c,
- struct most_c_attr *attr,
- char *buf)
+static ssize_t number_of_packet_buffers_show(struct most_c_obj *c,
+struct most_c_attr *attr,
+char *buf)
 {
unsigned int i = c->channel_id;
 
@@ -297,10 +296,9 @@ ssize_t show_number_of_packet_buffers(struct most_c_obj *c,
c->iface->channel_vector[i].num_buffers_packet);
 }
 
-static
-ssize_t show_number_of_stream_buffers(struct most_c_obj *c,
- struct most_c_attr *attr,
- char *buf)
+static ssize_t number_of_stream_buffers_show(struct most_c_obj *c,
+struct most_c_attr *attr,
+char *buf)
 {
unsigned int i = c->channel_id;
 
@@ -308,10 +306,9 @@ ssize_t show_number_of_stream_buffers(struct most_c_obj *c,
c->iface->channel_vector[i].num_buffers_streaming);
 }
 
-static
-ssize_t show_size_of_packet_buffer(struct most_c_obj *c,
-  struct most_c_attr *attr,
-  char *buf)
+static ssize_t size_of_packet_buffer_show(struct most_c_obj *c,
+ struct most_c_attr *attr,
+ char *buf)
 {
unsigned int i = c->channel_id;
 
@@ -319,10 +316,9 @@ ssize_t show_size_of_packet_buffer(struct most_c_obj *c,
c->iface->channel_vector[i].buffer_size_packet);
 }
 
-static
-ssize_t show_size_of_stream_buffer(struct most_c_obj *c,
-  struct most_c_attr *attr,
-  char *buf)
+static ssize_t size_of_stream_buffer_show(struct most_c_obj *c,
+ struct most_c_attr *attr,
+ char *buf)
 {
unsigned int i = c->channel_id;
 
@@ -330,7 +326,7 @@ ssize_t show_size_of_stream_buffer(struct most_c_obj *c,
c->iface->channel_vector[i].buffer_size_streaming);
 }
 
-static ssize_t show_channel_starving(struct most_c_obj *c,
+static ssize_t channel_starving_show(struct most_c_obj *c,
 struct most_c_attr *attr,
 char *buf)
 {
@@ -338,7 +334,7 @@ static ssize_t show_channel_starving(struct most_c_obj *c,
 }
 
 #define create_show_channel_attribute(val) \
-   static MOST_CHNL_ATTR(val, 0444, show_##val, NULL)
+   static struct most_c_attr most_chnl_attr_##val = __ATTR_RO(val)
 
 create_show_channel_attribute(available_directions);
 create_show_channel_attribute(available_datatypes);
-- 
2.11.0

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

[PATCH 02/13] staging: most: fix comment of the function remove_link_store

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces the name store_remove_link by the remove_link_store
in the comment for the corresponding function.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index a4beb32a8bbf..fb5e8f2ecb74 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1017,7 +1017,7 @@ static struct most_aim_attribute most_aim_attr_add_link =
__ATTR_RW(add_link);
 
 /**
- * store_remove_link - store function for remove_link attribute
+ * remove_link_store - store function for remove_link attribute
  * @aim_obj: pointer to AIM object
  * @attr: its attributes
  * @buf: buffer
-- 
2.11.0

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

[PATCH 01/13] staging: most: fix comment of the function add_link_store

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces the name store_add_link by the add_link_store in the
comment for the corresponding function.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index 191404bc5906..a4beb32a8bbf 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -943,7 +943,7 @@ most_c_obj *get_channel_by_name(char *mdev, char *mdev_ch)
 }
 
 /**
- * store_add_link - store() function for add_link attribute
+ * add_link_store - store() function for add_link attribute
  * @aim_obj: pointer to AIM object
  * @attr: its attributes
  * @buf: buffer
-- 
2.11.0

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

[PATCH 03/13] staging: most: use __ATTR_RO for the attribute interface

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces the use of a macro create_inst_attribute for the
attribute interface with a macro __ATTR_RO.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index fb5e8f2ecb74..a9fd896c9a9e 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -660,7 +660,7 @@ static ssize_t show_description(struct most_inst_obj 
*instance_obj,
instance_obj->iface->description);
 }
 
-static ssize_t show_interface(struct most_inst_obj *instance_obj,
+static ssize_t interface_show(struct most_inst_obj *instance_obj,
  struct most_inst_attribute *attr,
  char *buf)
 {
@@ -691,7 +691,9 @@ static ssize_t show_interface(struct most_inst_obj 
*instance_obj,
static MOST_INST_ATTR(value, 0444, show_##value, NULL)
 
 create_inst_attribute(description);
-create_inst_attribute(interface);
+
+static struct most_inst_attribute most_inst_attr_interface =
+   __ATTR_RO(interface);
 
 static struct attribute *most_inst_def_attrs[] = {
_inst_attr_description.attr,
-- 
2.11.0

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

[PATCH 04/13] staging: most: use __ATTR_RO for the attribute value

[Christian Gromm]

From: Andrey Shvetsov 

This patch replaces the use of a macro create_inst_attribute for the
attribute value with a macro __ATTR_RO.

Additionally, this patch removes not anymore used macros MOST_INST_ATTR
and create_inst_attribute.

Signed-off-by: Andrey Shvetsov 
Signed-off-by: Christian Gromm 
---
 drivers/staging/most/mostcore/core.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c 
b/drivers/staging/most/mostcore/core.c
index a9fd896c9a9e..f3a2dc4f4b47 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -562,9 +562,6 @@ create_most_c_obj(const char *name, struct kobject *parent)
 /*  ___   ___
  *  ___I N S T A N C E___
  */
-#define MOST_INST_ATTR(_name, _mode, _show, _store) \
-   struct most_inst_attribute most_inst_attr_##_name = \
-   __ATTR(_name, _mode, _show, _store)
 
 static struct list_head instance_list;
 
@@ -652,7 +649,7 @@ static void most_inst_release(struct kobject *kobj)
kfree(inst);
 }
 
-static ssize_t show_description(struct most_inst_obj *instance_obj,
+static ssize_t description_show(struct most_inst_obj *instance_obj,
struct most_inst_attribute *attr,
char *buf)
 {
@@ -687,10 +684,8 @@ static ssize_t interface_show(struct most_inst_obj 
*instance_obj,
return snprintf(buf, PAGE_SIZE, "unknown\n");
 }
 
-#define create_inst_attribute(value) \
-   static MOST_INST_ATTR(value, 0444, show_##value, NULL)
-
-create_inst_attribute(description);
+static struct most_inst_attribute most_inst_attr_description =
+   __ATTR_RO(description);
 
 static struct most_inst_attribute most_inst_attr_interface =
__ATTR_RO(interface);
-- 
2.11.0

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

[PATCH] Staging: nvec: Remove FSF's mailing address

[Riku Salminen]

Removed Free Software Foundation's address from the copyright notice
and replaced it with a link to http://www.gnu.org/licenses

Signed-off-by: Riku Salminen 
---
 drivers/staging/nvec/nvec-keytable.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/nvec/nvec-keytable.h 
b/drivers/staging/nvec/nvec-keytable.h
index 1dc22cb8812a..7008c96bdbbe 100644
--- a/drivers/staging/nvec/nvec-keytable.h
+++ b/drivers/staging/nvec/nvec-keytable.h
@@ -17,8 +17,7 @@
  * more details.
  *
  * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ * with this program; if not, see http://www.gnu.org/licenses
  */
 
 static unsigned short code_tab_102us[] = {
-- 
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/1] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Greg KH]

On Fri, Mar 31, 2017 at 03:01:12AM -0700, Chewie Lin wrote:
> fix a checkpatch warning:
> WARNING: Prefer using "%s", __func__ to embedded function names
> 
> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(>usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

Your patch does not match your description at all :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: bcm2835-audio: remove unnecessary log messages

[Aishwarya Pant]

Remove unnecessary log messages in the driver which were just tracking
function entry and exits.

Signed-off-by: Aishwarya Pant 
---

There are many functions here like:
bcm2835_audio_setup(..)
bcm2835_audio_flush_playback_buffers(..)
bcm2835_audio_flush_buffers(..)
which are not doing any work. Should they be removed or kept here as
placeholder?

 .../vc04_services/bcm2835-audio/bcm2835-ctl.c  |  2 --
 .../vc04_services/bcm2835-audio/bcm2835-pcm.c  | 19 
 .../vc04_services/bcm2835-audio/bcm2835-vchiq.c| 36 --
 3 files changed, 57 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
index 1fae169..f484bb0 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
@@ -41,7 +41,6 @@
 static int snd_bcm2835_ctl_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
 {
-   audio_info(" ... IN\n");
if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) {
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
uinfo->count = 1;
@@ -58,7 +57,6 @@ static int snd_bcm2835_ctl_info(struct snd_kcontrol *kcontrol,
uinfo->value.integer.min = 0;
uinfo->value.integer.max = AUDIO_DEST_MAX - 1;
}
-   audio_info(" ... OUT\n");
return 0;
 }
 
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
index 8bd69b9..e8cf0b9 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
@@ -65,7 +65,6 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream 
*alsa_stream)
unsigned int consumed = 0;
int new_period = 0;
 
-   audio_info(" .. IN\n");
 
audio_info("alsa_stream=%p substream=%p\n", alsa_stream,
alsa_stream ? alsa_stream->substream : 0);
@@ -100,7 +99,6 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream 
*alsa_stream)
} else {
audio_warning(" unexpected NULL substream\n");
}
-   audio_info(" .. OUT\n");
 }
 
 /* open callback */
@@ -113,7 +111,6 @@ static int snd_bcm2835_playback_open_generic(
int idx;
int err;
 
-   audio_info(" .. IN (%d)\n", substream->number);
 
if (mutex_lock_interruptible(>audio_mutex)) {
audio_error("Interrupted whilst waiting for lock\n");
@@ -187,7 +184,6 @@ static int snd_bcm2835_playback_open_generic(
 out:
mutex_unlock(>audio_mutex);
 
-   audio_info(" .. OUT =%d\n", err);
 
return err;
 }
@@ -211,7 +207,6 @@ static int snd_bcm2835_playback_close(struct 
snd_pcm_substream *substream)
struct snd_pcm_runtime *runtime;
struct bcm2835_alsa_stream *alsa_stream;
 
-   audio_info(" .. IN\n");
 
chip = snd_pcm_substream_chip(substream);
if (mutex_lock_interruptible(>audio_mutex)) {
@@ -252,7 +247,6 @@ static int snd_bcm2835_playback_close(struct 
snd_pcm_substream *substream)
chip->opened &= ~(1 << substream->number);
 
mutex_unlock(>audio_mutex);
-   audio_info(" .. OUT\n");
 
return 0;
 }
@@ -265,7 +259,6 @@ static int snd_bcm2835_pcm_hw_params(struct 
snd_pcm_substream *substream,
struct bcm2835_alsa_stream *alsa_stream = runtime->private_data;
int err;
 
-   audio_info(" .. IN\n");
 
err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
if (err < 0) {
@@ -277,7 +270,6 @@ static int snd_bcm2835_pcm_hw_params(struct 
snd_pcm_substream *substream,
alsa_stream->channels = params_channels(params);
alsa_stream->params_rate = params_rate(params);
alsa_stream->pcm_format_width = 
snd_pcm_format_width(params_format(params));
-   audio_info(" .. OUT\n");
 
return err;
 }
@@ -285,7 +277,6 @@ static int snd_bcm2835_pcm_hw_params(struct 
snd_pcm_substream *substream,
 /* hw_free callback */
 static int snd_bcm2835_pcm_hw_free(struct snd_pcm_substream *substream)
 {
-   audio_info(" .. IN\n");
return snd_pcm_lib_free_pages(substream);
 }
 
@@ -298,7 +289,6 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream 
*substream)
int channels;
int err;
 
-   audio_info(" .. IN\n");
 
if (mutex_lock_interruptible(>audio_mutex))
return -EINTR;
@@ -339,7 +329,6 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream 
*substream)
alsa_stream->pos, runtime->frame_bits);
 
mutex_unlock(>audio_mutex);
-   audio_info(" .. OUT\n");
return 0;
 }
 
@@ -376,7 +365,6 @@ static int snd_bcm2835_pcm_trigger(struct snd_pcm_substream 
*substream, int cmd)
struct bcm2835_alsa_stream *alsa_stream = runtime->private_data;
   

Re: [PATCH RFC] remove custom Michael MIC implementation

[Wolfram Sang]

Hi,

> The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> a SDW-823 and should use the ks7010 driver.

Sorry, likely not. It is an early SDW-821 and has a MediaTek chipset for
which no driver is known:
https://wikidevi.com/wiki/Spectec_SDW-821_%28MediaTek%29

For SDW-821 (SD size) and KS7010, you'd need a "S2Y-WLAN-11G-K" (but I
have never seen one yet):
https://wikidevi.com/wiki/Spectec_SDW-821_%28KeyStream%29

One can find an SDW-823 (microSD) "S2Y-MWLAN-11B-G" once in a while:
https://wikidevi.com/wiki/Spectec_SDW-823

> > Without the CFG80211 conversion, replacing the Michael custom
> > implementation with the in-kernel one makes the driver a tad better and
> > is good exercise. However, it will sadly not help to get the driver out
> > of staging.
> 
> I'll drop it then. Could you please tell me, is there any thing else
> more I need to do to let LKML know that this RFC is dropped? Or is
> this reply enough. I don't want to use any ones time unnecessarily.

That should do.

> Let's go for a CFG80211 driver and get out of staging :) So next step
> is I guess study the ath6kl driver, learn how CFG80211 is done and
> implement that interface in ks7010? Oh, and test that it works.

Yes, have a look around and check if you like that task. I might have a
spare SDW-823 lying around if you are up to it. But check first, it is
not a trivial task. On the pro side, there are tons of interesting
things about WiFI and kernel development to learn on the way :)

Regards,

   Wolfram



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

Re: [PATCH 1/1] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Kim Phillips]

On Fri, 31 Mar 2017 03:01:12 -0700
Chewie Lin  wrote:

> fix a checkpatch warning:
> WARNING: Prefer using "%s", __func__ to embedded function names

__func__, so:

> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

"%s=%d\n", __func__ " fail status", status);

?

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

Re: [PATCH RFC] remove custom Michael MIC implementation

[Tobin C. Harding]

On Fri, Mar 31, 2017 at 09:58:51AM +0200, Wolfram Sang wrote:
> 
> > The code is untested, I have hardware in the mail.
> 
> Cool!

The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
a SDW-823 and should use the ks7010 driver. I am going to attempt to
get it running on a Raspberry Pi B+. I ordered the wrong size break
out board originally so waiting on the new one now.

> 
> > If any one is interested and has any comments I would really like to
> > hear them. I am open to all suggestions (even down to trivial coding
> > style issues).
> 
> I'll just repeat that the key move to get this driver out of staging is
> to get away from the WEXT interface to CFG80211. Otherwise no chance
> that wireless maintainers will even look at it. This is a huge change
> but once it is done, features like Michael MIC come with it for free
> (from what I recall, I am not a wireless expert myself).

That would explain why I could not find more than the Orinoco driver
using the Michael MIC module directly.

> Without the CFG80211 conversion, replacing the Michael custom
> implementation with the in-kernel one makes the driver a tad better and
> is good exercise. However, it will sadly not help to get the driver out
> of staging.

I'll drop it then. Could you please tell me, is there any thing else
more I need to do to let LKML know that this RFC is dropped? Or is
this reply enough. I don't want to use any ones time unnecessarily.

> 
> But if you want a clean WEXT driver first, this is a step in the right
> direction.
> 

Let's go for a CFG80211 driver and get out of staging :) So next step
is I guess study the ath6kl driver, learn how CFG80211 is done and
implement that interface in ks7010? Oh, and test that it works.

thanks,
Tobin.




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

[PATCH v2] iio: gyro: adis16060: Change the name of function.

[simran singhal]

Change the name of function from adis16060_spi_write_than_read()
to adis16060_spi_write_then_read(). change "than" to "then" as
its time depended.

Signed-off-by: simran singhal 
---

 v2:
   -Change the subject.
   -Add signed-off-by.

 drivers/staging/iio/gyro/adis16060_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
b/drivers/staging/iio/gyro/adis16060_core.c
index 8115962..9675245 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,7 +40,7 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+static int adis16060_spi_write_then_read(struct iio_dev *indio_dev,
 u8 conf, u16 *val)
 {
int ret;
@@ -81,7 +81,7 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 
switch (mask) {
case IIO_CHAN_INFO_RAW:
-   ret = adis16060_spi_write_than_read(indio_dev,
+   ret = adis16060_spi_write_then_read(indio_dev,
chan->address, );
if (ret < 0)
return ret;
-- 
2.7.4

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

[PATCH 1/1] drivers/staging/vt6656/main_usb.c: checkpatch warning

[Chewie Lin]

fix a checkpatch warning:
WARNING: Prefer using "%s", __func__ to embedded function names

Signed-off-by: Chewie Lin 
---
 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9..2d9e7af 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
status = usb_reset_device(priv->usb);
if (status)
dev_warn(>usb->dev,
-"usb_device_reset fail status=%d\n", status);
+"%s=%d\n", "usb_device_reset fail status", status);
 }
 
 static void vnt_free_int_bufs(struct vnt_private *priv)
-- 
2.10.0

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

[PATCH] Eudyptula challenge

[Chewie Lin]

Hi greg k-h and forest:

Sorry about all the spam I've been sending earlier. One more try.
I'm submitting this patch as part of Eudyptula challenge to fix a 
coding style problem.  Thanks for taking time on this trivial patch.

sl424

Chewie Lin (1):
  fixed a checkpatch warning

 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.10.0

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

Re: [PATCH RFC] staging: ks7010: remove custom Michael MIC implementation

[Tobin C. Harding]

On Thu, Mar 30, 2017 at 10:16:05PM -0700, Joe Perches wrote:
> On Fri, 2017-03-31 at 15:47 +1100, Tobin C. Harding wrote:
> > ks7010 currently uses a custom implementation of the Michael MIC
> > algorithm. The kernel has an implementation of this algorithm
> > already, we should use it.
> 
> ok, trivia:
> 
> Do please run your patch through checkpatch and fix a few style nits.
> 
> $ ./scripts/checkpatch.pl ~/1.mbox --strict --terse | cut -f2- -d":"
> 161: WARNING: line over 80 characters
> 170: WARNING: Missing a blank line after declarations
> 205: WARNING: line over 80 characters
> 229: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> 263: WARNING: Prefer using "%s", __func__ to embedded function names
> 264: ERROR: code indent should use tabs where possible
> 264: WARNING: quoted string split across lines
> 272: WARNING: Prefer using "%s", __func__ to embedded function names
> 273: ERROR: code indent should use tabs where possible
> 273: WARNING: quoted string split across lines
> 325: WARNING: Prefer pr_warn(... to pr_warning(...
>  2 errors, 9 warnings, 0 checks, 262 lines checked

Face palm :)

> 
> and
> 
> > diff --git a/drivers/staging/ks7010/mic.c b/drivers/staging/ks7010/mic.c
> []
> > +int ks_wlan_mic(struct crypto_shash *tfm_michael, u8 *key,
> > +   u8 priority, u8 *data, size_t data_len, u8 *mic)
> > +{
> > +   SHASH_DESC_ON_STACK(desc, tfm_michael);
> > +   u8 hdr[ETH_HLEN + 2]; /* 16 bytes */
> 
> It might be better to declare a struct for this
> 
> > +   hdr[ETH_ALEN * 2] = priority;
> > +   hdr[ETH_ALEN * 2 + 1] = 0;
> > +   hdr[ETH_ALEN * 2 + 2] = 0;
> > +   hdr[ETH_ALEN * 2 + 3] = 0;
> 
> And use struct members here.
> 

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

Re: [PATCH] drivers: fixed a checkpatch warning

[Greg KH]

On Fri, Mar 31, 2017 at 01:40:29AM -0700, Chewie Lin wrote:
> fixed a coding style error/warning.

Worst changelog and subject: line ever :)

Please go read the links that my patchbot sent you last time you
submitted this change.  You didn't do so for some odd reason :(

thanks,

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

[PATCH] drivers: fixed a checkpatch warning

[Chewie Lin]

fixed a coding style error/warning.

Signed-off-by: Chewie Lin 
---
 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9..2d9e7af 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
status = usb_reset_device(priv->usb);
if (status)
dev_warn(>usb->dev,
-"usb_device_reset fail status=%d\n", status);
+"%s=%d\n", "usb_device_reset fail status", status);
 }
 
 static void vnt_free_int_bufs(struct vnt_private *priv)
-- 
2.10.0

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

[PATCH] Eudyptula challenge

[Chewie Lin]

Hi,

I'm submitting this patch as part of Eudyptula challenge to fix a 
coding style problem.  Thanks for taking time on this trivial patch.

sl424

Chewie Lin (1):
  fixed a checkpatch warning

 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.10.0

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

Re: [PATCH RFC] remove custom Michael MIC implementation

[Wolfram Sang]

> The code is untested, I have hardware in the mail.

Cool!

> If any one is interested and has any comments I would really like to
> hear them. I am open to all suggestions (even down to trivial coding
> style issues).

I'll just repeat that the key move to get this driver out of staging is
to get away from the WEXT interface to CFG80211. Otherwise no chance
that wireless maintainers will even look at it. This is a huge change
but once it is done, features like Michael MIC come with it for free
(from what I recall, I am not a wireless expert myself).

Without the CFG80211 conversion, replacing the Michael custom
implementation with the in-kernel one makes the driver a tad better and
is good exercise. However, it will sadly not help to get the driver out
of staging.

But if you want a clean WEXT driver first, this is a step in the right
direction.



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

Re: [REGRESSION][Stable][v3.12.y][v4.4.y][v4.9.y][v4.10.y][v4.11-rc1] scsi: storvsc: properly set residual data length on errors

[gre...@linuxfoundation.org]

Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Mar 30, 2017 at 09:51:53AM -0400, Cathy Avery wrote:
> Hi,
> 
> So which commit is moving forward and which one is not?
> 
> f1c635b439a5c01776fe3a25b1e2dc546ea82e6f or
> 40630f462824ee24bc00d692865c86c3828094e0?
> 
> We have backported 40630f462824ee24bc00d692865c86c3828094e0 and I am unclear
> if this is a regression and must be removed or it is a regression but is
> fixed
> by f1c635b439a5c01776fe3a25b1e2dc546ea82e6f and can remain.

Well, I'm not doing anything here until a hyperv maintainer tells me
(and stable@) what patches to backport to what kernels...

thanks,

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

Re: [PATCH] iio: gyro: adis16060: Change the function's name

[Greg Kroah-Hartman]

On Fri, Mar 31, 2017 at 12:32:26AM +0530, simran singhal wrote:
> Change the name of function from adis16060_spi_write_than_read()
> to adis16060_spi_write_then_read(). change "than" to "then" as
> its time depended.
> ---
>  drivers/staging/iio/gyro/adis16060_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

No signed-off-by:?

Always run your patches through checkpatch.pl to see if you did
something wrong.

thanks,

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

Re: [PATCH] staging: unisys: visorbus: fix kernel BUG discovered by day0 testing

[Greg KH]

On Thu, Mar 30, 2017 at 05:43:17PM -0400, David Kershner wrote:
> Fixes: 5b6f9b95f7ae ("staging: unisys: visorbus: get rid of create_bus_type.")
> 
> Kernel day0 testing robot reported a kernel BUG at drivers/base/driver.c!
> with the following call stack:
> 
> [   14.963563] [ cut here ]
> [   14.967298] kernel BUG at drivers/base/driver.c:153!
> [   14.970948] invalid opcode:  [#1] SMP
> [   14.974013] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 4.11.0-rc4-00790-g0789e2c #1
> [   14.978221] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.9.3-20161025_171302-gandalf 04/01/2014
> [   14.983417] task: 88001ea46040 task.stack: c9008000
> [   14.987315] RIP: 0010:driver_register+0xa1/0xd0
> [   14.990044] RSP: :c900be60 EFLAGS: 00010246
> [   14.993039] RAX:  RBX: 831d4c20 RCX: 
> 
> [   14.997040] RDX: 004d RSI: 831d47c0 RDI: 
> 831d4c20
> [   15.001511] RBP: c900be78 R08: c900be78 R09: 
> c900be7c
> [   15.006163] R10:  R11: 0001 R12: 
> 
> [   15.010068] R13:  R14: 832f3923 R15: 
> 
> [   15.013715] FS:  () GS:88001fa0() 
> knlGS:
> [   15.017460] CS:  0010 DS:  ES:  CR0: 80050033
> [   15.021268] CR2:  CR3: 03009000 CR4: 
> 06b0
> [   15.025633] Call Trace:
> [   15.028069]  ? visorbus_register_visor_driver+0x3f/0x60
> [   15.031065]  ? init_unisys+0x3a/0x90
> [   15.033562]  ? device_resume_response+0x50/0x50
> [   15.036083]  visorinput_init+0x10/0x20
> [   15.038937]  do_one_initcall+0x9a/0x164
> [   15.041838]  ? set_debug_rodata+0x12/0x12
> [   15.045333]  kernel_init_freeable+0x11e/0x1a1
> [   15.048369]  ? rest_init+0x80/0x80
> [   15.050813]  kernel_init+0x9/0x100
> [   15.053353]  ret_from_fork+0x2c/0x40
> [   15.056009] Code: ff 85 c0 41 89 c4 75 13 48 8b 7b 70 31 f6 e8 97 16 be ff 
> 44 89 e0 5b 41 5c 5d c3 48 89 df e8 57 e1 ff ff 44 89 e0 5b 41 5c 5d c3 <0f> 
> 0b 48 8b 33 48 c7 c7 a0 dd d5 82 e8 ec f0 6f ff 48 8b 73 08
> [   15.065144] RIP: driver_register+0xa1/0xd0 RSP: c900be60
> [   15.068360] ---[ end trace 7d13369c38d80a8f ]---
> 
> This bug will occur if the visorbus driver is built-in to the kernel, and
> the resulting kernel is run in an environment where visorbus devices are
> NOT supported, and an attempt is made to load any of the drivers: visorhba,
> visornic, or visorinput.
> 
> This reverts commit 5b6f9b95f7ae ("staging: unisys: visorbus: get rid of
> create_bus_type.")
> 
> Signed-off-by: David Kershner 
> Reviewed-by: Tim Sell 
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 32 
> ++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index 4348072..9f698ab 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -29,6 +29,7 @@
>  #define CURRENT_FILE_PC VISOR_BUS_PC_visorbus_main_c
>  #define POLLJIFFIES_NORMALCHANNEL 10
>  
> +static int busreg_rc = -ENODEV; /* stores the result from bus registration */
>  static struct dentry *visorbus_debugfs_dir;
>  
>  /*
> @@ -959,6 +960,9 @@ static void bus_device_info_init(
>   */
>  int visorbus_register_visor_driver(struct visor_driver *drv)
>  {
> + if (busreg_rc < 0)
> + return -ENODEV; /*can't register on a nonexistent bus*/

Why no extra ' ' at the beginning and end of the comment?

> +
>   drv->driver.name = drv->name;
>   drv->driver.bus = _type;
>   drv->driver.probe = visordriver_probe_device;
> @@ -1067,6 +1071,29 @@ int visorbus_register_visor_driver(struct visor_driver 
> *drv)
>  }
>  
>  /*
> + * create_bus_type() - create and register the one-and-only one instance of
> + * the visor bus type (visorbus_type)
> + * Return: 0 for success, otherwise negative errno value returned by
> + * bus_register() indicating the reason for failure
> + */
> +static int
> +create_bus_type(void)
> +{
> + busreg_rc = bus_register(_type);
> + return busreg_rc;
> +}
> +
> +/*
> + * remove_bus_type() - remove the one-and-only one instance of the visor bus
> + * type (visorbus_type)
> + */
> +static void
> +remove_bus_type(void)
> +{
> + bus_unregister(_type);
> +}

Why are these two separate functions?  Just include the logic where the
functions were called, a one-line function that is only called one place
is not needed.

Also, why not set "busreg_rc" when you unregister the bus?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org