Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On Thu, 05 Nov 2020, Jiri Slaby wrote:
> On 05. 11. 20, 9:36, Lee Jones wrote:
> > On Thu, 05 Nov 2020, Jiri Slaby wrote:
> >
> > > On 05. 11. 20, 8:04, Christophe Leroy wrote:
> > > >
> > > >
> > > > Le 04/11/2020 à 20:35, Lee Jones a écrit :
> > > > > Fixes the following W=1 kernel build warning(s):
> > > > >
> > > > > drivers/tty/serial/pmac_zilog.h:365:58: warning: variable
> > > > > ‘garbage’ set but not used [-Wunused-but-set-variable]
> > > >
> > > > Explain how you are fixing this warning.
> > > >
> > > > Setting __always_unused is usually not the good solution for fixing
> > > > this warning, but here I guess this is likely the good solution. But it
> > > > should be explained why.
> >
> > There are normally 3 ways to fix this warning;
> >
> > - Start using/checking the variable/result
> > - Remove the variable
> > - Mark it as __{always,maybe}_unused
> >
> > The later just tells the compiler that not checking the resultant
> > value is intentional. There are some functions (as Jiri mentions
> > below) which are marked as '__must_check' which *require* a dummy
> > (garbage) variable to be used.
> >
> > > Or, why is the "garbage =" needed in the first place? read_zsdata is not
> > > defined with __warn_unused_result__.
> >
> > I used '__always_used' here for fear of breaking something.
> >
> > However, if it's safe to remove it, then all the better.
>
> Yes please -- this "garbage" is one of the examples of volatile misuses. If
> readb didn't work on volatile pointer, marking the return variable as
> volatile wouldn't save it.
>
> > > And even if it was, would (void)!read_zsdata(port) fix it?
> >
> > That's hideous. :D
>
> Sure, marking reads as must_check would be insane.
>
> > *Much* better to just use '__always_used' in that use-case.
>
> Then using a dummy variable to fool must_check must mean must_check is used
> incorrectly, no :)? But there are always exceptions…
Agreed on all points.
Will fix.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On 05. 11. 20, 9:36, Lee Jones wrote:
On Thu, 05 Nov 2020, Jiri Slaby wrote:
On 05. 11. 20, 8:04, Christophe Leroy wrote:
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable
‘garbage’ set but not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing
this warning, but here I guess this is likely the good solution. But it
should be explained why.
There are normally 3 ways to fix this warning;
- Start using/checking the variable/result
- Remove the variable
- Mark it as __{always,maybe}_unused
The later just tells the compiler that not checking the resultant
value is intentional. There are some functions (as Jiri mentions
below) which are marked as '__must_check' which *require* a dummy
(garbage) variable to be used.
Or, why is the "garbage =" needed in the first place? read_zsdata is not
defined with __warn_unused_result__.
I used '__always_used' here for fear of breaking something.
However, if it's safe to remove it, then all the better.
Yes please -- this "garbage" is one of the examples of volatile misuses.
If readb didn't work on volatile pointer, marking the return variable as
volatile wouldn't save it.
And even if it was, would (void)!read_zsdata(port) fix it?
That's hideous. :D
Sure, marking reads as must_check would be insane.
*Much* better to just use '__always_used' in that use-case.
Then using a dummy variable to fool must_check must mean must_check is
used incorrectly, no :)? But there are always exceptions…
thanks,
--
js
suse labs
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On Thu, 05 Nov 2020, Jiri Slaby wrote:
> On 05. 11. 20, 8:04, Christophe Leroy wrote:
> >
> >
> > Le 04/11/2020 à 20:35, Lee Jones a écrit :
> > > Fixes the following W=1 kernel build warning(s):
> > >
> > > drivers/tty/serial/pmac_zilog.h:365:58: warning: variable
> > > ‘garbage’ set but not used [-Wunused-but-set-variable]
> >
> > Explain how you are fixing this warning.
> >
> > Setting __always_unused is usually not the good solution for fixing
> > this warning, but here I guess this is likely the good solution. But it
> > should be explained why.
There are normally 3 ways to fix this warning;
- Start using/checking the variable/result
- Remove the variable
- Mark it as __{always,maybe}_unused
The later just tells the compiler that not checking the resultant
value is intentional. There are some functions (as Jiri mentions
below) which are marked as '__must_check' which *require* a dummy
(garbage) variable to be used.
> Or, why is the "garbage =" needed in the first place? read_zsdata is not
> defined with __warn_unused_result__.
I used '__always_used' here for fear of breaking something.
However, if it's safe to remove it, then all the better.
> And even if it was, would (void)!read_zsdata(port) fix it?
That's hideous. :D
*Much* better to just use '__always_used' in that use-case.
> > > Cc: Greg Kroah-Hartman
> > > Cc: Jiri Slaby
> > > Cc: Michael Ellerman
> > > Cc: Benjamin Herrenschmidt
> > > Cc: Paul Mackerras
> > > Cc: linux-ser...@vger.kernel.org
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones
> > > ---
> > > drivers/tty/serial/pmac_zilog.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/pmac_zilog.h
> > > b/drivers/tty/serial/pmac_zilog.h
> > > index bb874e76810e0..968aec7c1cf82 100644
> > > --- a/drivers/tty/serial/pmac_zilog.h
> > > +++ b/drivers/tty/serial/pmac_zilog.h
> > > @@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port
> > > *port)
> > > /* Misc macros */
> > > #define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES))
> > > -#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
> > > +#define ZS_CLEARFIFO(port) do { volatile unsigned char
> > > __always_unused garbage; \
> > > garbage = read_zsdata(port); \
> > > garbage = read_zsdata(port); \
> > > garbage = read_zsdata(port); \
> > >
>
> thanks,
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
On 05. 11. 20, 8:04, Christophe Leroy wrote:
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’
set but not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing
this warning, but here I guess this is likely the good solution. But it
should be explained why.
Or, why is the "garbage =" needed in the first place? read_zsdata is not
defined with __warn_unused_result__. And even if it was, would
(void)!read_zsdata(port) fix it?
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linux-ser...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/serial/pmac_zilog.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.h
b/drivers/tty/serial/pmac_zilog.h
index bb874e76810e0..968aec7c1cf82 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port
*port)
/* Misc macros */
#define ZS_CLEARERR(port) (write_zsreg(port, 0, ERR_RES))
-#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
+#define ZS_CLEARFIFO(port) do { volatile unsigned char
__always_unused garbage; \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
thanks,
--
js
Re: [PATCH 34/36] tty: serial: pmac_zilog: Make disposable variable __always_unused
Le 04/11/2020 à 20:35, Lee Jones a écrit :
Fixes the following W=1 kernel build warning(s):
drivers/tty/serial/pmac_zilog.h:365:58: warning: variable ‘garbage’ set but
not used [-Wunused-but-set-variable]
Explain how you are fixing this warning.
Setting __always_unused is usually not the good solution for fixing this warning, but here I guess
this is likely the good solution. But it should be explained why.
Christophe
Cc: Greg Kroah-Hartman
Cc: Jiri Slaby
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linux-ser...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones
---
drivers/tty/serial/pmac_zilog.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.h b/drivers/tty/serial/pmac_zilog.h
index bb874e76810e0..968aec7c1cf82 100644
--- a/drivers/tty/serial/pmac_zilog.h
+++ b/drivers/tty/serial/pmac_zilog.h
@@ -362,7 +362,7 @@ static inline void zssync(struct uart_pmac_port *port)
/* Misc macros */
#define ZS_CLEARERR(port)(write_zsreg(port, 0, ERR_RES))
-#define ZS_CLEARFIFO(port) do { volatile unsigned char garbage; \
+#define ZS_CLEARFIFO(port) do { volatile unsigned char __always_unused
garbage; \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
garbage = read_zsdata(port); \
