* [PATCH] pwm: sun4i: Initialize variables before use
@ 2020-01-20 14:32 Thierry Reding
2020-01-20 20:09 ` Uwe Kleine-König
0 siblings, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2020-01-20 14:32 UTC (permalink / raw)
To: Thierry Reding
Cc: Uwe Kleine-König, Maxime Ripard, Jernej Skrabec,
Clément Péron, linux-pwm
GCC can't always determine that the duty, period and prescaler values
are initialized when returning from sun4i_pwm_calculate(), so help out a
little by initializing them to 0.
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
drivers/pwm/pwm-sun4i.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 0decc7cde133..3e3efa6c768f 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -234,9 +234,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
{
struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
struct pwm_state cstate;
- u32 ctrl, duty, period, val;
+ u32 ctrl, duty = 0, period = 0, val;
int ret;
- unsigned int delay_us, prescaler;
+ unsigned int delay_us, prescaler = 0;
unsigned long now;
bool bypass;
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] pwm: sun4i: Initialize variables before use
2020-01-20 14:32 [PATCH] pwm: sun4i: Initialize variables before use Thierry Reding
@ 2020-01-20 20:09 ` Uwe Kleine-König
2020-01-20 20:23 ` Clément Péron
2020-01-21 13:50 ` Thierry Reding
0 siblings, 2 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2020-01-20 20:09 UTC (permalink / raw)
To: Thierry Reding
Cc: Maxime Ripard, Jernej Skrabec, Clément Péron, linux-pwm
Hello Thierry,
On Mon, Jan 20, 2020 at 03:32:06PM +0100, Thierry Reding wrote:
> GCC can't always determine that the duty, period and prescaler values
> are initialized when returning from sun4i_pwm_calculate(), so help out a
> little by initializing them to 0.
Is it worth mentioning the gcc version you're using?
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> drivers/pwm/pwm-sun4i.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 0decc7cde133..3e3efa6c768f 100644
I don't find this object (0decc7cde133) in my tree or next. Which
version is this?
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -234,9 +234,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> {
> struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> struct pwm_state cstate;
> - u32 ctrl, duty, period, val;
> + u32 ctrl, duty = 0, period = 0, val;
+ u32 ctrl, uninitialized_var(duty), uninitialized_var(period), val;
should fix the warnings, too, and additionally explicitly documents that
it's just the compiler that doesn't see there is no problem.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pwm: sun4i: Initialize variables before use
2020-01-20 20:09 ` Uwe Kleine-König
@ 2020-01-20 20:23 ` Clément Péron
2020-01-21 13:50 ` Thierry Reding
1 sibling, 0 replies; 5+ messages in thread
From: Clément Péron @ 2020-01-20 20:23 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Thierry Reding, Maxime Ripard, Jernej Skrabec, Linux PWM List
Hi Uwe, Thierry
On Mon, 20 Jan 2020 at 21:09, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello Thierry,
>
> On Mon, Jan 20, 2020 at 03:32:06PM +0100, Thierry Reding wrote:
> > GCC can't always determine that the duty, period and prescaler values
> > are initialized when returning from sun4i_pwm_calculate(), so help out a
> > little by initializing them to 0.
>
> Is it worth mentioning the gcc version you're using?
This issue has been trig by kbuild test robot.
I planned to submit a patch for it as it's due to my modification but
forget to submit it...
Original report
[linux-next:master 6586/9861] drivers/pwm/pwm-sun4i.c:57:34: warning:
'prescaler' may be used uninitialized in this function
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master
head: de970dffa7d19eae1d703c3534825308ef8d5dec
commit: 9f28e95b5286fce63a3d0d90dc7ca43eca8dda58 [6586/9861] pwm:
sun4i: Add support to output source clock directly
config: microblaze-randconfig-a001-20200118 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 9f28e95b5286fce63a3d0d90dc7ca43eca8dda58
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=microblaze
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>
> > Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> > ---
> > drivers/pwm/pwm-sun4i.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 0decc7cde133..3e3efa6c768f 100644
>
> I don't find this object (0decc7cde133) in my tree or next. Which
> version is this?
>
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -234,9 +234,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > {
> > struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > struct pwm_state cstate;
> > - u32 ctrl, duty, period, val;
> > + u32 ctrl, duty = 0, period = 0, val;
>
> + u32 ctrl, uninitialized_var(duty), uninitialized_var(period), val;
>
> should fix the warnings, too, and additionally explicitly documents that
> it's just the compiler that doesn't see there is no problem.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pwm: sun4i: Initialize variables before use
2020-01-20 20:09 ` Uwe Kleine-König
2020-01-20 20:23 ` Clément Péron
@ 2020-01-21 13:50 ` Thierry Reding
2020-01-21 14:31 ` Uwe Kleine-König
1 sibling, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2020-01-21 13:50 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Maxime Ripard, Jernej Skrabec, Clément Péron, linux-pwm
[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]
On Mon, Jan 20, 2020 at 09:09:17PM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
>
> On Mon, Jan 20, 2020 at 03:32:06PM +0100, Thierry Reding wrote:
> > GCC can't always determine that the duty, period and prescaler values
> > are initialized when returning from sun4i_pwm_calculate(), so help out a
> > little by initializing them to 0.
>
> Is it worth mentioning the gcc version you're using?
I could, but what good is that going to be? I don't think this is
something that's limited to one specific version but I don't know
exactly which ones are impacted. Stating just one specific version
isn't all that useful in that case.
> > Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> > ---
> > drivers/pwm/pwm-sun4i.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 0decc7cde133..3e3efa6c768f 100644
>
> I don't find this object (0decc7cde133) in my tree or next. Which
> version is this?
I made this on top of my local pwm/for-next when I was build-testing,
which I usually do before pushing, so it's not surprising that you
don't have this in your tree.
>
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -234,9 +234,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > {
> > struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > struct pwm_state cstate;
> > - u32 ctrl, duty, period, val;
> > + u32 ctrl, duty = 0, period = 0, val;
>
> + u32 ctrl, uninitialized_var(duty), uninitialized_var(period), val;
>
> should fix the warnings, too, and additionally explicitly documents that
> it's just the compiler that doesn't see there is no problem.
I haven't convinced myself fully yet that there really isn't a problem.
I'm fairly sure it's safe, but always initializing to 0 doesn't hurt.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pwm: sun4i: Initialize variables before use
2020-01-21 13:50 ` Thierry Reding
@ 2020-01-21 14:31 ` Uwe Kleine-König
0 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2020-01-21 14:31 UTC (permalink / raw)
To: Thierry Reding
Cc: Maxime Ripard, Jernej Skrabec, Clément Péron,
linux-pwm, kernel
Hello Thierry,
On Tue, Jan 21, 2020 at 02:50:11PM +0100, Thierry Reding wrote:
> On Mon, Jan 20, 2020 at 09:09:17PM +0100, Uwe Kleine-König wrote:
> > On Mon, Jan 20, 2020 at 03:32:06PM +0100, Thierry Reding wrote:
> > > GCC can't always determine that the duty, period and prescaler values
> > > are initialized when returning from sun4i_pwm_calculate(), so help out a
> > > little by initializing them to 0.
> >
> > Is it worth mentioning the gcc version you're using?
>
> I could, but what good is that going to be? I don't think this is
> something that's limited to one specific version but I don't know
> exactly which ones are impacted. Stating just one specific version
> isn't all that useful in that case.
I think something like:
gcc 4.6 reports [...], newer gccs are fine.
is useful. If you read that in a few years, it helps you either to
reproduce the problem or determine it is not important any more.
> > > Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> > > ---
> > > drivers/pwm/pwm-sun4i.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > index 0decc7cde133..3e3efa6c768f 100644
> >
> > I don't find this object (0decc7cde133) in my tree or next. Which
> > version is this?
>
> I made this on top of my local pwm/for-next when I was build-testing,
> which I usually do before pushing, so it's not surprising that you
> don't have this in your tree.
The reason I asked this (and also the gcc version) is to reproduce the
issue and work with it. With your reply I can only say that I expect
that uninitialized_var fixes the problem in a better way. If I knew the
exact circumstances, I could test them and claim that it indeed fixes it
(or see it doesn't and don't take your time with non-sense reviews).
> > > --- a/drivers/pwm/pwm-sun4i.c
> > > +++ b/drivers/pwm/pwm-sun4i.c
> > > @@ -234,9 +234,9 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > > {
> > > struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > > struct pwm_state cstate;
> > > - u32 ctrl, duty, period, val;
> > > + u32 ctrl, duty = 0, period = 0, val;
> >
> > + u32 ctrl, uninitialized_var(duty), uninitialized_var(period), val;
> >
> > should fix the warnings, too, and additionally explicitly documents that
> > it's just the compiler that doesn't see there is no problem.
>
> I haven't convinced myself fully yet that there really isn't a problem.
> I'm fairly sure it's safe, but always initializing to 0 doesn't hurt.
Yes, I agree that initializing the variable fixes the warning. Still
using uninitialized_var is the better way (assuming it indeed fixes the
problem, see above).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-21 14:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 14:32 [PATCH] pwm: sun4i: Initialize variables before use Thierry Reding
2020-01-20 20:09 ` Uwe Kleine-König
2020-01-20 20:23 ` Clément Péron
2020-01-21 13:50 ` Thierry Reding
2020-01-21 14:31 ` Uwe Kleine-König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.