2021-12-21 17:15:53

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH] usb: renesas_usbhs: Fix unused variable warning

This patch fixes the below warning:

drivers/usb/renesas_usbhs/mod.c: In function 'usbhs_status_get_each_irq':
drivers/usb/renesas_usbhs/mod.c:195:13: warning: variable 'intenb0'
set but not used [-Wunused-but-set-variable]
195 | u16 intenb0, intenb1;
|

Fixes: 33e4245ee919 ("usb: renesas_usbhs: Use platform_get_irq() to get the interrupt")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/usb/renesas_usbhs/mod.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
index f2ea3e1412d2..3919e350b487 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -192,13 +192,12 @@ static int usbhs_status_get_each_irq(struct usbhs_priv *priv,
struct usbhs_irq_state *state)
{
struct usbhs_mod *mod = usbhs_mod_get_current(priv);
- u16 intenb0, intenb1;
unsigned long flags;
+ u16 intenb1;

/******************** spin lock ********************/
usbhs_lock(priv, flags);
state->intsts0 = usbhs_read(priv, INTSTS0);
- intenb0 = usbhs_read(priv, INTENB0);

if (usbhs_mod_is_host(priv)) {
state->intsts1 = usbhs_read(priv, INTSTS1);
--
2.17.1



2021-12-22 06:10:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: renesas_usbhs: Fix unused variable warning

On Tue, Dec 21, 2021 at 05:15:32PM +0000, Lad Prabhakar wrote:
> This patch fixes the below warning:
>
> drivers/usb/renesas_usbhs/mod.c: In function 'usbhs_status_get_each_irq':
> drivers/usb/renesas_usbhs/mod.c:195:13: warning: variable 'intenb0'
> set but not used [-Wunused-but-set-variable]
> 195 | u16 intenb0, intenb1;
> |
>
> Fixes: 33e4245ee919 ("usb: renesas_usbhs: Use platform_get_irq() to get the interrupt")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/usb/renesas_usbhs/mod.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
> index f2ea3e1412d2..3919e350b487 100644
> --- a/drivers/usb/renesas_usbhs/mod.c
> +++ b/drivers/usb/renesas_usbhs/mod.c
> @@ -192,13 +192,12 @@ static int usbhs_status_get_each_irq(struct usbhs_priv *priv,
> struct usbhs_irq_state *state)
> {
> struct usbhs_mod *mod = usbhs_mod_get_current(priv);
> - u16 intenb0, intenb1;
> unsigned long flags;
> + u16 intenb1;
>
> /******************** spin lock ********************/
> usbhs_lock(priv, flags);
> state->intsts0 = usbhs_read(priv, INTSTS0);
> - intenb0 = usbhs_read(priv, INTENB0);

Did you just break the hardware? Reading is often times needed and
clang has no idea about hardware issues. We need proof in the changlog
that this really is safe to do.

How did you test your change?

thanks,

greg k-h

2021-12-22 09:01:36

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] usb: renesas_usbhs: Fix unused variable warning

Hi Greg,

Thank you for the review.

On Wed, Dec 22, 2021 at 6:10 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Tue, Dec 21, 2021 at 05:15:32PM +0000, Lad Prabhakar wrote:
> > This patch fixes the below warning:
> >
> > drivers/usb/renesas_usbhs/mod.c: In function 'usbhs_status_get_each_irq':
> > drivers/usb/renesas_usbhs/mod.c:195:13: warning: variable 'intenb0'
> > set but not used [-Wunused-but-set-variable]
> > 195 | u16 intenb0, intenb1;
> > |
> >
> > Fixes: 33e4245ee919 ("usb: renesas_usbhs: Use platform_get_irq() to get the interrupt")
> > Reported-by: kernel test robot <[email protected]>
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > drivers/usb/renesas_usbhs/mod.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
> > index f2ea3e1412d2..3919e350b487 100644
> > --- a/drivers/usb/renesas_usbhs/mod.c
> > +++ b/drivers/usb/renesas_usbhs/mod.c
> > @@ -192,13 +192,12 @@ static int usbhs_status_get_each_irq(struct usbhs_priv *priv,
> > struct usbhs_irq_state *state)
> > {
> > struct usbhs_mod *mod = usbhs_mod_get_current(priv);
> > - u16 intenb0, intenb1;
> > unsigned long flags;
> > + u16 intenb1;
> >
> > /******************** spin lock ********************/
> > usbhs_lock(priv, flags);
> > state->intsts0 = usbhs_read(priv, INTSTS0);
> > - intenb0 = usbhs_read(priv, INTENB0);
>
> Did you just break the hardware? Reading is often times needed and
> clang has no idea about hardware issues. We need proof in the changlog
> that this really is safe to do.
>
I introduced this warning in commit 33e4245ee919 ("usb: renesas_usbhs:
Use platform_get_irq() to get the interrupt'') where
IORESOURCE_IRQ_SHAREABLE flag handling was dropped and I missed to
remove this change. As a result I included a fixes tag for this
commit. Let me know if you want me to update the changelog.

> How did you test your change?
>
I haven't but can test it.

Cheers,
Prabhakar

2021-12-22 09:25:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: renesas_usbhs: Fix unused variable warning

On Wed, Dec 22, 2021 at 09:01:03AM +0000, Lad, Prabhakar wrote:
> Hi Greg,
>
> Thank you for the review.
>
> On Wed, Dec 22, 2021 at 6:10 AM Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > On Tue, Dec 21, 2021 at 05:15:32PM +0000, Lad Prabhakar wrote:
> > > This patch fixes the below warning:
> > >
> > > drivers/usb/renesas_usbhs/mod.c: In function 'usbhs_status_get_each_irq':
> > > drivers/usb/renesas_usbhs/mod.c:195:13: warning: variable 'intenb0'
> > > set but not used [-Wunused-but-set-variable]
> > > 195 | u16 intenb0, intenb1;
> > > |
> > >
> > > Fixes: 33e4245ee919 ("usb: renesas_usbhs: Use platform_get_irq() to get the interrupt")
> > > Reported-by: kernel test robot <[email protected]>
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > ---
> > > drivers/usb/renesas_usbhs/mod.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
> > > index f2ea3e1412d2..3919e350b487 100644
> > > --- a/drivers/usb/renesas_usbhs/mod.c
> > > +++ b/drivers/usb/renesas_usbhs/mod.c
> > > @@ -192,13 +192,12 @@ static int usbhs_status_get_each_irq(struct usbhs_priv *priv,
> > > struct usbhs_irq_state *state)
> > > {
> > > struct usbhs_mod *mod = usbhs_mod_get_current(priv);
> > > - u16 intenb0, intenb1;
> > > unsigned long flags;
> > > + u16 intenb1;
> > >
> > > /******************** spin lock ********************/
> > > usbhs_lock(priv, flags);
> > > state->intsts0 = usbhs_read(priv, INTSTS0);
> > > - intenb0 = usbhs_read(priv, INTENB0);
> >
> > Did you just break the hardware? Reading is often times needed and
> > clang has no idea about hardware issues. We need proof in the changlog
> > that this really is safe to do.
> >
> I introduced this warning in commit 33e4245ee919 ("usb: renesas_usbhs:
> Use platform_get_irq() to get the interrupt'') where
> IORESOURCE_IRQ_SHAREABLE flag handling was dropped and I missed to
> remove this change. As a result I included a fixes tag for this
> commit. Let me know if you want me to update the changelog.

Yes please explain why this change is ok, not just that it is a random
compiler warning cleanup.

> > How did you test your change?
> >
> I haven't but can test it.

Please do.

thanks,

greg k-h