2021-01-27 23:52:47

by Lee Jones

[permalink] [raw]
Subject: [PATCH 01/12] thunderbolt: dma_port: Remove unused variable 'ret'

Fixes the following W=1 kernel build warning(s):

drivers/thunderbolt/dma_port.c: In function ‘dma_port_flash_write_block’:
drivers/thunderbolt/dma_port.c:331:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]

Cc: Andreas Noever <[email protected]>
Cc: Michael Jamet <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Yehezkel Bernat <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
drivers/thunderbolt/dma_port.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/dma_port.c b/drivers/thunderbolt/dma_port.c
index 847dd07a7b172..5aced91e17dc4 100644
--- a/drivers/thunderbolt/dma_port.c
+++ b/drivers/thunderbolt/dma_port.c
@@ -328,13 +328,12 @@ static int dma_port_flash_write_block(struct tb_dma_port *dma, u32 address,
{
struct tb_switch *sw = dma->sw;
u32 in, dwaddress, dwords;
- int ret;

dwords = size / 4;

/* Write the block to MAIL_DATA registers */
- ret = dma_port_write(sw->tb->ctl, buf, tb_route(sw), dma->port,
- dma->base + MAIL_DATA, dwords, DMA_PORT_TIMEOUT);
+ dma_port_write(sw->tb->ctl, buf, tb_route(sw), dma->port,
+ dma->base + MAIL_DATA, dwords, DMA_PORT_TIMEOUT);

in = MAIL_IN_CMD_FLASH_WRITE << MAIL_IN_CMD_SHIFT;

--
2.25.1


2021-01-28 00:07:33

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH 01/12] thunderbolt: dma_port: Remove unused variable 'ret'

Hi,

On Wed, Jan 27, 2021 at 04:19:16PM +0000, Lee Jones wrote:
> On Wed, 27 Jan 2021, Andy Shevchenko wrote:
>
> > On Wednesday, January 27, 2021, Lee Jones <[email protected]> wrote:
> >
> > > Fixes the following W=1 kernel build warning(s):
> > >
> > > drivers/thunderbolt/dma_port.c: In function ‘dma_port_flash_write_block’:
> > > drivers/thunderbolt/dma_port.c:331:6: warning: variable ‘ret’ set but
> > > not used [-Wunused-but-set-variable]
> > >
> > >
> > Is it scripted somehow?
>
> A script opens up the file on the warning line.
>
> The patch is hand-written.
>
> > Because I am not sure we are okay to simply drop the assignment.
>
> I've been careful not to change the semantics of the code.
>
> The return value has never been checked since the driver's inception 4
> years ago.
>
> However, if this is an oversight and the intention was to check the
> value and error-out during a failure condition, I can make that
> happen.
>
> I would need a nod from the author before I make such a change.

It's actually an oversight from my side. It should do something like:

if (ret)
return ret:

there. Feel free to fix it up :)

Thanks!

2021-01-28 00:07:46

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 01/12] thunderbolt: dma_port: Remove unused variable 'ret'

On Wed, 27 Jan 2021, Andy Shevchenko wrote:

> On Wednesday, January 27, 2021, Lee Jones <[email protected]> wrote:
>
> > Fixes the following W=1 kernel build warning(s):
> >
> > drivers/thunderbolt/dma_port.c: In function ‘dma_port_flash_write_block’:
> > drivers/thunderbolt/dma_port.c:331:6: warning: variable ‘ret’ set but
> > not used [-Wunused-but-set-variable]
> >
> >
> Is it scripted somehow?

A script opens up the file on the warning line.

The patch is hand-written.

> Because I am not sure we are okay to simply drop the assignment.

I've been careful not to change the semantics of the code.

The return value has never been checked since the driver's inception 4
years ago.

However, if this is an oversight and the intention was to check the
value and error-out during a failure condition, I can make that
happen.

I would need a nod from the author before I make such a change.

> > Cc: Andreas Noever <[email protected]>
> > Cc: Michael Jamet <[email protected]>
> > Cc: Mika Westerberg <[email protected]>
> > Cc: Yehezkel Bernat <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/thunderbolt/dma_port.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/thunderbolt/dma_port.c b/drivers/thunderbolt/dma_
> > port.c
> > index 847dd07a7b172..5aced91e17dc4 100644
> > --- a/drivers/thunderbolt/dma_port.c
> > +++ b/drivers/thunderbolt/dma_port.c
> > @@ -328,13 +328,12 @@ static int dma_port_flash_write_block(struct
> > tb_dma_port *dma, u32 address,
> > {
> > struct tb_switch *sw = dma->sw;
> > u32 in, dwaddress, dwords;
> > - int ret;
> >
> > dwords = size / 4;
> >
> > /* Write the block to MAIL_DATA registers */
> > - ret = dma_port_write(sw->tb->ctl, buf, tb_route(sw), dma->port,
> > - dma->base + MAIL_DATA, dwords,
> > DMA_PORT_TIMEOUT);
> > + dma_port_write(sw->tb->ctl, buf, tb_route(sw), dma->port,
> > + dma->base + MAIL_DATA, dwords, DMA_PORT_TIMEOUT);
> >
> > in = MAIL_IN_CMD_FLASH_WRITE << MAIL_IN_CMD_SHIFT;
> >
> >
> >
>

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

2021-01-28 08:54:54

by Lee Jones

[permalink] [raw]
Subject: [PATCH V2 01/12] thunderbolt: dma_port: Check 'dma_port_flash_write_block()'s return value

... and take the error path if it fails.

Fixes the following W=1 kernel build warning(s):

drivers/thunderbolt/dma_port.c: In function ‘dma_port_flash_write_block’:
drivers/thunderbolt/dma_port.c:331:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]

Cc: Andreas Noever <[email protected]>
Cc: Michael Jamet <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Yehezkel Bernat <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---

v2: Different approach: actually handle the error.

drivers/thunderbolt/dma_port.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/thunderbolt/dma_port.c b/drivers/thunderbolt/dma_port.c
index 847dd07a7b172..7288aaf01ae6a 100644
--- a/drivers/thunderbolt/dma_port.c
+++ b/drivers/thunderbolt/dma_port.c
@@ -335,6 +335,8 @@ static int dma_port_flash_write_block(struct tb_dma_port *dma, u32 address,
/* Write the block to MAIL_DATA registers */
ret = dma_port_write(sw->tb->ctl, buf, tb_route(sw), dma->port,
dma->base + MAIL_DATA, dwords, DMA_PORT_TIMEOUT);
+ if (ret)
+ return ret;

in = MAIL_IN_CMD_FLASH_WRITE << MAIL_IN_CMD_SHIFT;

2021-01-28 11:05:15

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH V2 01/12] thunderbolt: dma_port: Check 'dma_port_flash_write_block()'s return value

On Thu, Jan 28, 2021 at 08:52:33AM +0000, Lee Jones wrote:
> ... and take the error path if it fails.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/thunderbolt/dma_port.c: In function ‘dma_port_flash_write_block’:
> drivers/thunderbolt/dma_port.c:331:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>
> Cc: Andreas Noever <[email protected]>
> Cc: Michael Jamet <[email protected]>
> Cc: Mika Westerberg <[email protected]>
> Cc: Yehezkel Bernat <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>

Applied, thanks!