From: "Souptick Joarder (HPE)" <[email protected]>
Kernel test robot throws below warning ->
drivers/pcmcia/omap_cf.c:127:7: warning: variable 'control'
set but not used [-Wunused-but-set-variable]
Removed unused variable control.
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Souptick Joarder (HPE) <[email protected]>
---
drivers/pcmcia/omap_cf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/pcmcia/omap_cf.c b/drivers/pcmcia/omap_cf.c
index 1972a8f6fa8e..c96943805f77 100644
--- a/drivers/pcmcia/omap_cf.c
+++ b/drivers/pcmcia/omap_cf.c
@@ -124,7 +124,6 @@ static int omap_cf_get_status(struct pcmcia_socket *s, u_int *sp)
static int
omap_cf_set_socket(struct pcmcia_socket *sock, struct socket_state_t *s)
{
- u16 control;
/* REVISIT some non-OSK boards may support power switching */
switch (s->Vcc) {
@@ -135,7 +134,6 @@ omap_cf_set_socket(struct pcmcia_socket *sock, struct socket_state_t *s)
return -EINVAL;
}
- control = omap_readw(CF_CONTROL);
if (s->flags & SS_RESET)
omap_writew(CF_CONTROL_RESET, CF_CONTROL);
else
--
2.25.1
Am Mon, Jul 25, 2022 at 06:59:55AM +0530 schrieb Souptick Joarder:
> From: "Souptick Joarder (HPE)" <[email protected]>
>
> Kernel test robot throws below warning ->
> drivers/pcmcia/omap_cf.c:127:7: warning: variable 'control'
> set but not used [-Wunused-but-set-variable]
>
> Removed unused variable control.
From a code-generation standpoint, this is obiously correct, and probably
the compiler removes that omap_readw() already. But, to be honest, I'm not
perfectly sure on what the device expects -- and whether it's required to
write back the control register (or parts of it). Does anyone still have the
hardware (or specs)? If not, I'm tempted to apply this patch on the basis
that the compiler might remove that omap_readw() anyway.
Thanks,
Dominik
On Thu, Sep 22, 2022, at 4:39 PM, Dominik Brodowski wrote:
> Am Mon, Jul 25, 2022 at 06:59:55AM +0530 schrieb Souptick Joarder:
>> From: "Souptick Joarder (HPE)" <[email protected]>
>>
>> Kernel test robot throws below warning ->
>> drivers/pcmcia/omap_cf.c:127:7: warning: variable 'control'
>> set but not used [-Wunused-but-set-variable]
>>
>> Removed unused variable control.
>
> From a code-generation standpoint, this is obiously correct, and probably
> the compiler removes that omap_readw() already.
omap_readw() is an 'extern' function, so it does not get removed.
Even if it was inline, it's still "and volatile" read that gets
left in because it may (and probably does) have side-effects.
> But, to be honest, I'm not
> perfectly sure on what the device expects -- and whether it's required to
> write back the control register (or parts of it). Does anyone still have the
> hardware (or specs)? If not, I'm tempted to apply this patch on the basis
> that the compiler might remove that omap_readw() anyway.
I think assigning to an unused variable was at some point needed to
avoid a compiler warning because otherwise omap_readw() was just a
pointer dereference. The safe way to transform the code should be to
drop the variable but leave the function call.
Note that the driver is only used on the 'osk' reference board,
not on any devices that were in mass-production.
Arnd
Am Fri, Sep 23, 2022 at 08:58:47AM +0200 schrieb Arnd Bergmann:
> On Thu, Sep 22, 2022, at 4:39 PM, Dominik Brodowski wrote:
> > Am Mon, Jul 25, 2022 at 06:59:55AM +0530 schrieb Souptick Joarder:
> >> From: "Souptick Joarder (HPE)" <[email protected]>
> >>
> >> Kernel test robot throws below warning ->
> >> drivers/pcmcia/omap_cf.c:127:7: warning: variable 'control'
> >> set but not used [-Wunused-but-set-variable]
> >>
> >> Removed unused variable control.
> >
> > From a code-generation standpoint, this is obiously correct, and probably
> > the compiler removes that omap_readw() already.
>
> omap_readw() is an 'extern' function, so it does not get removed.
>
> Even if it was inline, it's still "and volatile" read that gets
> left in because it may (and probably does) have side-effects.
>
> > But, to be honest, I'm not
> > perfectly sure on what the device expects -- and whether it's required to
> > write back the control register (or parts of it). Does anyone still have the
> > hardware (or specs)? If not, I'm tempted to apply this patch on the basis
> > that the compiler might remove that omap_readw() anyway.
>
> I think assigning to an unused variable was at some point needed to
> avoid a compiler warning because otherwise omap_readw() was just a
> pointer dereference. The safe way to transform the code should be to
> drop the variable but leave the function call.
>
> Note that the driver is only used on the 'osk' reference board,
> not on any devices that were in mass-production.
Thanks for the evaluation, Arnd! So here's what I intend to push upstream:
From: "Souptick Joarder (HPE)" <[email protected]>
Kernel test robot throws below warning ->
drivers/pcmcia/omap_cf.c:127:7: warning: variable 'control'
set but not used [-Wunused-but-set-variable]
Removed unused variable control.
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Souptick Joarder (HPE) <[email protected]>
[[email protected]: retain omap_readw() call, as it might be required]
Signed-off-by: Dominik Brodowski <[email protected]>
diff --git a/drivers/pcmcia/omap_cf.c b/drivers/pcmcia/omap_cf.c
index 1972a8f6fa8e..d3f827d4224a 100644
--- a/drivers/pcmcia/omap_cf.c
+++ b/drivers/pcmcia/omap_cf.c
@@ -124,8 +124,6 @@ static int omap_cf_get_status(struct pcmcia_socket *s, u_int *sp)
static int
omap_cf_set_socket(struct pcmcia_socket *sock, struct socket_state_t *s)
{
- u16 control;
-
/* REVISIT some non-OSK boards may support power switching */
switch (s->Vcc) {
case 0:
@@ -135,7 +133,7 @@ omap_cf_set_socket(struct pcmcia_socket *sock, struct socket_state_t *s)
return -EINVAL;
}
- control = omap_readw(CF_CONTROL);
+ omap_readw(CF_CONTROL);
if (s->flags & SS_RESET)
omap_writew(CF_CONTROL_RESET, CF_CONTROL);
else