All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: walter harms <wharms@bfs.de>
Cc: linux-pwm@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	kernel@pengutronix.de, Colin King <colin.king@canonical.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] pwm: sun4i: Narrow scope of local variable
Date: Tue, 10 Dec 2019 10:12:29 +0000	[thread overview]
Message-ID: <20191210101229.rvqelc2hanisd2cp@pengutronix.de> (raw)
In-Reply-To: <5DE8C1E3.4080204@bfs.de>

Hello Walter,

On Thu, Dec 05, 2019 at 09:37:55AM +0100, walter harms wrote:
> Am 05.12.2019 08:24, schrieb Uwe Kleine-König:
> > +			unsigned int pval;
> > +
> >  			if (!prescaler_table[prescaler])
> >  				continue;
> >  			pval = prescaler_table[prescaler];
> 
> 
> nit picking:
> Doing the assignment first would remove the only use
> of prescaler_table[prescaler].

nit picking: it would be reduced to a single use?!

> unsigned int pval = prescaler_table[prescaler];
> if ( ! pval )
>   continue;

Right, will send a v2 with that.

> if you feel adventures you could also replace the for() for a while()
> since we know that prescaler = 0.
> 
> while ( prescaler < PWM_PRESCAL_MASK )
> {
> unsigned int pval = prescaler_table[prescaler++];
> ....

That however has some side effects as prescaler is used after leaving
the loop.

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: walter harms <wharms@bfs.de>
Cc: linux-pwm@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	kernel@pengutronix.de, Colin King <colin.king@canonical.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] pwm: sun4i: Narrow scope of local variable
Date: Tue, 10 Dec 2019 11:12:29 +0100	[thread overview]
Message-ID: <20191210101229.rvqelc2hanisd2cp@pengutronix.de> (raw)
In-Reply-To: <5DE8C1E3.4080204@bfs.de>

Hello Walter,

On Thu, Dec 05, 2019 at 09:37:55AM +0100, walter harms wrote:
> Am 05.12.2019 08:24, schrieb Uwe Kleine-König:
> > +			unsigned int pval;
> > +
> >  			if (!prescaler_table[prescaler])
> >  				continue;
> >  			pval = prescaler_table[prescaler];
> 
> 
> nit picking:
> Doing the assignment first would remove the only use
> of prescaler_table[prescaler].

nit picking: it would be reduced to a single use?!

> unsigned int pval = prescaler_table[prescaler];
> if ( ! pval )
>   continue;

Right, will send a v2 with that.

> if you feel adventures you could also replace the for() for a while()
> since we know that prescaler == 0.
> 
> while ( prescaler < PWM_PRESCAL_MASK )
> {
> unsigned int pval = prescaler_table[prescaler++];
> ....

That however has some side effects as prescaler is used after leaving
the loop.

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: walter harms <wharms@bfs.de>
Cc: linux-pwm@vger.kernel.org, kernel-janitors@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	kernel@pengutronix.de, Colin King <colin.king@canonical.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] pwm: sun4i: Narrow scope of local variable
Date: Tue, 10 Dec 2019 11:12:29 +0100	[thread overview]
Message-ID: <20191210101229.rvqelc2hanisd2cp@pengutronix.de> (raw)
In-Reply-To: <5DE8C1E3.4080204@bfs.de>

Hello Walter,

On Thu, Dec 05, 2019 at 09:37:55AM +0100, walter harms wrote:
> Am 05.12.2019 08:24, schrieb Uwe Kleine-König:
> > +			unsigned int pval;
> > +
> >  			if (!prescaler_table[prescaler])
> >  				continue;
> >  			pval = prescaler_table[prescaler];
> 
> 
> nit picking:
> Doing the assignment first would remove the only use
> of prescaler_table[prescaler].

nit picking: it would be reduced to a single use?!

> unsigned int pval = prescaler_table[prescaler];
> if ( ! pval )
>   continue;

Right, will send a v2 with that.

> if you feel adventures you could also replace the for() for a while()
> since we know that prescaler == 0.
> 
> while ( prescaler < PWM_PRESCAL_MASK )
> {
> unsigned int pval = prescaler_table[prescaler++];
> ....

That however has some side effects as prescaler is used after leaving
the loop.

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-12-10 10:12 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 10:08 [PATCH] pwm: sun4i: redundant assignment to variable pval Colin King
2019-10-02 10:08 ` Colin King
2019-10-02 10:08 ` Colin King
2019-10-02 10:16 ` Uwe Kleine-König
2019-10-02 10:16   ` Uwe Kleine-König
2019-10-02 10:16   ` Uwe Kleine-König
2019-12-05  7:24   ` [PATCH] pwm: sun4i: Narrow scope of local variable Uwe Kleine-König
2019-12-05  7:24     ` Uwe Kleine-König
2019-12-05  8:37     ` walter harms
2019-12-05  8:37       ` walter harms
2019-12-10 10:12       ` Uwe Kleine-König [this message]
2019-12-10 10:12         ` Uwe Kleine-König
2019-12-10 10:12         ` Uwe Kleine-König
2019-12-10 10:24         ` [PATCH v2] " Uwe Kleine-König
2019-12-10 10:24           ` Uwe Kleine-König
2019-10-02 10:39 ` [PATCH] pwm: sun4i: redundant assignment to variable pval Thierry Reding
2019-10-02 10:39   ` Thierry Reding
2019-10-02 10:39   ` Thierry Reding
2019-10-02 13:25 ` Dan Carpenter
2019-10-02 13:25   ` Dan Carpenter
2019-10-02 13:25   ` Dan Carpenter
2019-10-02 13:25   ` Dan Carpenter
2019-10-02 13:28   ` Colin Ian King
2019-10-02 13:28     ` Colin Ian King
2019-10-02 13:28     ` Colin Ian King
2019-10-02 13:28     ` Colin Ian King
2019-10-02 13:29   ` Dan Carpenter
2019-10-02 13:29     ` Dan Carpenter
2019-10-02 13:29     ` Dan Carpenter
2019-10-02 13:29     ` Dan Carpenter
2019-10-02 13:30     ` Colin Ian King
2019-10-02 13:30       ` Colin Ian King
2019-10-02 13:30       ` Colin Ian King
2019-10-02 13:30       ` Colin Ian King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191210101229.rvqelc2hanisd2cp@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=colin.king@canonical.com \
    --cc=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=wharms@bfs.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.