diff mbox series

usb: renesas_usbhs: Fix unused variable warning

Message ID 20211221171532.29881-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State New, archived
Headers show
Series usb: renesas_usbhs: Fix unused variable warning | expand

Commit Message

Prabhakar Dec. 21, 2021, 5:15 p.m. UTC
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 <lkp@intel.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/usb/renesas_usbhs/mod.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Greg KH Dec. 22, 2021, 6:10 a.m. UTC | #1
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 <lkp@intel.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  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
Lad, Prabhakar Dec. 22, 2021, 9:01 a.m. UTC | #2
Hi Greg,

Thank you for the review.

On Wed, Dec 22, 2021 at 6:10 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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 <lkp@intel.com>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  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
Greg KH Dec. 22, 2021, 9:25 a.m. UTC | #3
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
> <gregkh@linuxfoundation.org> 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 <lkp@intel.com>
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  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
diff mbox series

Patch

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);