Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Mon, 8 Feb 2021 11:18:01 +0100 | Subject | Re: [PATCH] staging: emxx_udc: Fix incorrectly defined global |
| |
Hi Kumar,
CC Nishad, Magnus, linux-renesas-soc,
On Sun, Feb 7, 2021 at 8:40 AM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > On Sun, Feb 07, 2021 at 12:04:41PM IST, Stephen Rothwell wrote: > > Given that drivers/staging/emxx_udc/emxx_udc.h is only included by > > drivers/staging/emxx_udc/emxx_udc.c, shouldn't these variables just be > > declared static in emxx_udc.c and removed from emxx_udc.h? > > > > Either would be correct. I went this way because it was originally trying to > (incorrectly) define a global variable instead. I guess they can be static now > and when more users are added, the linkage can be adjusted as needed. > > Here's another version of the patch: > > -- > From 5835ad916ceeba620c85bc32f52ecd69f21f8dab Mon Sep 17 00:00:00 2001 > From: Kumar Kartikeya Dwivedi <memxor@gmail.com> > Date: Sun, 7 Feb 2021 12:53:39 +0530 > Subject: [PATCH] staging: emxx_udc: Make incorrectly defined global static > > The global gpio_desc pointer and int vbus_irq were defined in the header, > instead put the definitions in the translation unit and make them static as > there's only a single consumer in emxx_udc.c. > > This fixes the following sparse warnings for this driver: > drivers/staging/emxx_udc/emxx_udc.c: note: in included file: > drivers/staging/emxx_udc/emxx_udc.h:23:18: warning: symbol 'vbus_gpio' was not > declared. Should it be static? > drivers/staging/emxx_udc/emxx_udc.h:24:5: warning: symbol 'vbus_irq' was not > declared. Should it be static? > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Thanks for your patch!
> --- a/drivers/staging/emxx_udc/emxx_udc.c > +++ b/drivers/staging/emxx_udc/emxx_udc.c > @@ -34,6 +34,9 @@ > #define DRIVER_DESC "EMXX UDC driver" > #define DMA_ADDR_INVALID (~(dma_addr_t)0) > > +static struct gpio_desc *vbus_gpio; > +static int vbus_irq;
While I agree these must be static, I expect the driver to be still broken, as vbus_gpio is never set?
Commit 48101806c52203f6 ("Staging: emxx_udc: Switch to the gpio descriptor interface") introduced both variables, which was presumably never tested.
Magnus: perhaps we should just remove this driver?
> + > static const char driver_name[] = "emxx_udc"; > static const char driver_desc[] = DRIVER_DESC; > > diff --git a/drivers/staging/emxx_udc/emxx_udc.h b/drivers/staging/emxx_udc/emxx_udc.h > index bca614d69..c9e37a1b8 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.h > +++ b/drivers/staging/emxx_udc/emxx_udc.h > @@ -20,8 +20,6 @@ > /* below hacked up for staging integration */ > #define GPIO_VBUS 0 /* GPIO_P153 on KZM9D */ > #define INT_VBUS 0 /* IRQ for GPIO_P153 */ > -struct gpio_desc *vbus_gpio; > -int vbus_irq;
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |