| Message ID | 20201124084955.30270-1-min.guo@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] usb: musb: remove unused variable 'devctl' | expand |
Hello! On 24.11.2020 11:49, min.guo@mediatek.com wrote: > From: Min Guo <min.guo@mediatek.com> > > Remove unused 'devctl' variable to fix compile warnings: > > drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq': > drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set > but not used [-Wunused-but-set-variable] > > Signed-off-by: Min Guo <min.guo@mediatek.com> > --- > changes in v3 > suggested by Greg Kroah-Hartman: > Add a comment. > > changes in v2 > suggested by Alan Stern: > Add void before musb_read to indicate that the register MUSB_DEVCTL > was intended to be read and discarded. > --- > drivers/usb/musb/musbhsdma.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c > index 0aacfc8be5a1..2a345b4ad015 100644 > --- a/drivers/usb/musb/musbhsdma.c > +++ b/drivers/usb/musb/musbhsdma.c [...] > @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) > < musb_channel->len) ? > "=> reconfig 0" : "=> complete"); > > - devctl = musb_readb(mbase, MUSB_DEVCTL); > + /* > + * Some hardware may need to read the > + * MUSB_DEVCTL register once to take effect. > + */ > + (void)musb_readb(mbase, MUSB_DEVCTL); Hm, forcibly reading DevCtl in the DMA driver... sounds quite nonsensically. Lemme take a look... [...] MBR, Sergei
On 11/24/20 12:13 PM, Sergei Shtylyov wrote: [...] >> From: Min Guo <min.guo@mediatek.com> >> >> Remove unused 'devctl' variable to fix compile warnings: >> >> drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq': >> drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set >> but not used [-Wunused-but-set-variable] >> >> Signed-off-by: Min Guo <min.guo@mediatek.com> >> --- >> changes in v3 >> suggested by Greg Kroah-Hartman: >> Add a comment. >> >> changes in v2 >> suggested by Alan Stern: >> Add void before musb_read to indicate that the register MUSB_DEVCTL >> was intended to be read and discarded. >> --- >> drivers/usb/musb/musbhsdma.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c >> index 0aacfc8be5a1..2a345b4ad015 100644 >> --- a/drivers/usb/musb/musbhsdma.c >> +++ b/drivers/usb/musb/musbhsdma.c > [...] >> @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) >> < musb_channel->len) ? >> "=> reconfig 0" : "=> complete"); >> - devctl = musb_readb(mbase, MUSB_DEVCTL); >> + /* >> + * Some hardware may need to read the >> + * MUSB_DEVCTL register once to take effect. >> + */ >> + (void)musb_readb(mbase, MUSB_DEVCTL); > > Hm, forcibly reading DevCtl in the DMA driver... sounds quite nonsensically. Lemme take a look... Indeed, prior to commit c418fd6c01fbc5516a2cd1eaf1df1ec86869028a ("usb: gadget: musb: fix short isoc packets with inventra dma") the DevCtl read was done in order to check the DevCtl.HostMode bit -- this test was removed but the DevCtl read was left intact... so my vote goes for deleting both the read and the variable... > [...] MBR, Sergei
On Tue, Nov 24, 2020 at 12:13:42PM +0300, Sergei Shtylyov wrote: > Hello! > > On 24.11.2020 11:49, min.guo@mediatek.com wrote: > > > From: Min Guo <min.guo@mediatek.com> > > > > Remove unused 'devctl' variable to fix compile warnings: > > > > drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq': > > drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set > > but not used [-Wunused-but-set-variable] > > > > Signed-off-by: Min Guo <min.guo@mediatek.com> > > --- > > changes in v3 > > suggested by Greg Kroah-Hartman: > > Add a comment. > > > > changes in v2 > > suggested by Alan Stern: > > Add void before musb_read to indicate that the register MUSB_DEVCTL > > was intended to be read and discarded. > > --- > > drivers/usb/musb/musbhsdma.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c > > index 0aacfc8be5a1..2a345b4ad015 100644 > > --- a/drivers/usb/musb/musbhsdma.c > > +++ b/drivers/usb/musb/musbhsdma.c > [...] > > @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) > > < musb_channel->len) ? > > "=> reconfig 0" : "=> complete"); > > - devctl = musb_readb(mbase, MUSB_DEVCTL); > > + /* > > + * Some hardware may need to read the > > + * MUSB_DEVCTL register once to take effect. > > + */ > > + (void)musb_readb(mbase, MUSB_DEVCTL); > > Hm, forcibly reading DevCtl in the DMA driver... sounds quite > nonsensically. Lemme take a look... What happened to your look? thanks, greg k-h
On 12/10/20 6:01 PM, Greg Kroah-Hartman wrote: [...] >>> From: Min Guo <min.guo@mediatek.com> >>> >>> Remove unused 'devctl' variable to fix compile warnings: >>> >>> drivers/usb/musb/musbhsdma.c: In function 'dma_controller_irq': >>> drivers/usb/musb/musbhsdma.c:324:8: warning: variable 'devctl' set >>> but not used [-Wunused-but-set-variable] >>> >>> Signed-off-by: Min Guo <min.guo@mediatek.com> >>> --- >>> changes in v3 >>> suggested by Greg Kroah-Hartman: >>> Add a comment. >>> >>> changes in v2 >>> suggested by Alan Stern: >>> Add void before musb_read to indicate that the register MUSB_DEVCTL >>> was intended to be read and discarded. >>> --- >>> drivers/usb/musb/musbhsdma.c | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c >>> index 0aacfc8be5a1..2a345b4ad015 100644 >>> --- a/drivers/usb/musb/musbhsdma.c >>> +++ b/drivers/usb/musb/musbhsdma.c >> [...] >>> @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) >>> < musb_channel->len) ? >>> "=> reconfig 0" : "=> complete"); >>> - devctl = musb_readb(mbase, MUSB_DEVCTL); >>> + /* >>> + * Some hardware may need to read the >>> + * MUSB_DEVCTL register once to take effect. >>> + */ >>> + (void)musb_readb(mbase, MUSB_DEVCTL); >> >> Hm, forcibly reading DevCtl in the DMA driver... sounds quite >> nonsensically. Lemme take a look... > > What happened to your look? I thought I posted it the same day... Indeed, here it is, archived: https://marc.info/?l=linux-usb&m=160621841910477 > thanks, > > greg k-h MBR, Sergei
diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c index 0aacfc8be5a1..2a345b4ad015 100644 --- a/drivers/usb/musb/musbhsdma.c +++ b/drivers/usb/musb/musbhsdma.c @@ -321,8 +321,6 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) musb_channel->channel.status = MUSB_DMA_STATUS_BUS_ABORT; } else { - u8 devctl; - addr = musb_read_hsdma_addr(mbase, bchannel); channel->actual_len = addr @@ -336,7 +334,11 @@ irqreturn_t dma_controller_irq(int irq, void *private_data) < musb_channel->len) ? "=> reconfig 0" : "=> complete"); - devctl = musb_readb(mbase, MUSB_DEVCTL); + /* + * Some hardware may need to read the + * MUSB_DEVCTL register once to take effect. + */ + (void)musb_readb(mbase, MUSB_DEVCTL); channel->status = MUSB_DMA_STATUS_FREE;