From mboxrd@z Thu Jan 1 00:00:00 1970 From: walter harms Date: Thu, 05 Dec 2019 08:37:55 +0000 Subject: Re: [PATCH] pwm: sun4i: Narrow scope of local variable Message-Id: <5DE8C1E3.4080204@bfs.de> List-Id: References: <20191002101624.gljyf7g4nia2rcbx@pengutronix.de> <20191205072404.6858-1-u.kleine-koenig@pengutronix.de> In-Reply-To: <20191205072404.6858-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= Cc: linux-pwm@vger.kernel.org, kernel-janitors@vger.kernel.org, Thierry Reding , Dan Carpenter , kernel@pengutronix.de, Colin King , linux-arm-kernel@lists.infradead.org Am 05.12.2019 08:24, schrieb Uwe Kleine-König: > The variable pval is only used in a single block in the function > sun4i_pwm_calculate(). So declare it in a more local scope to simplify > the function for humans and compilers. > > While the diffstat for this patch is negative for this patch I still > thing the advantage of having a narrower scope is beneficial. > > Signed-off-by: Uwe Kleine-König > --- > Hello, > > for the patch that became > > 1b98ad3b3be9 ("pwm: sun4i: Drop redundant assignment to variable pval") > > (and which yielded the situation that pval is only used in this single > block) I suggested to do this change. This was ignored however by both > Colin and Thierry without comment. So I suggest the change here > separately. > > Best regards > Uwe > > drivers/pwm/pwm-sun4i.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c > index 581d23287333..8919e6ab7577 100644 > --- a/drivers/pwm/pwm-sun4i.c > +++ b/drivers/pwm/pwm-sun4i.c > @@ -149,7 +149,7 @@ static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm, > u32 *dty, u32 *prd, unsigned int *prsclr) > { > u64 clk_rate, div = 0; > - unsigned int pval, prescaler = 0; > + unsigned int prescaler = 0; > > clk_rate = clk_get_rate(sun4i_pwm->clk); > > @@ -170,6 +170,8 @@ static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm, > if (prescaler = 0) { > /* Go up from the first divider */ > for (prescaler = 0; prescaler < PWM_PRESCAL_MASK; prescaler++) { > + 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]. unsigned int pval = prescaler_table[prescaler]; if ( ! pval ) continue; 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++]; .... jm2c, wh From mboxrd@z Thu Jan 1 00:00:00 1970 From: walter harms Subject: Re: [PATCH] pwm: sun4i: Narrow scope of local variable Date: Thu, 05 Dec 2019 09:37:55 +0100 Message-ID: <5DE8C1E3.4080204@bfs.de> References: <20191002101624.gljyf7g4nia2rcbx@pengutronix.de> <20191205072404.6858-1-u.kleine-koenig@pengutronix.de> Reply-To: wharms@bfs.de Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20191205072404.6858-1-u.kleine-koenig@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= Cc: linux-pwm@vger.kernel.org, kernel-janitors@vger.kernel.org, Thierry Reding , Dan Carpenter , kernel@pengutronix.de, Colin King , linux-arm-kernel@lists.infradead.org List-Id: linux-pwm@vger.kernel.org CgpBbSAwNS4xMi4yMDE5IDA4OjI0LCBzY2hyaWViIFV3ZSBLbGVpbmUtS8O2bmlnOgo+IFRoZSB2 YXJpYWJsZSBwdmFsIGlzIG9ubHkgdXNlZCBpbiBhIHNpbmdsZSBibG9jayBpbiB0aGUgZnVuY3Rp b24KPiBzdW40aV9wd21fY2FsY3VsYXRlKCkuIFNvIGRlY2xhcmUgaXQgaW4gYSBtb3JlIGxvY2Fs IHNjb3BlIHRvIHNpbXBsaWZ5Cj4gdGhlIGZ1bmN0aW9uIGZvciBodW1hbnMgYW5kIGNvbXBpbGVy cy4KPiAKPiBXaGlsZSB0aGUgZGlmZnN0YXQgZm9yIHRoaXMgcGF0Y2ggaXMgbmVnYXRpdmUgZm9y IHRoaXMgcGF0Y2ggSSBzdGlsbAo+IHRoaW5nIHRoZSBhZHZhbnRhZ2Ugb2YgaGF2aW5nIGEgbmFy cm93ZXIgc2NvcGUgaXMgYmVuZWZpY2lhbC4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBVd2UgS2xlaW5l LUvDtm5pZyA8dS5rbGVpbmUta29lbmlnQHBlbmd1dHJvbml4LmRlPgo+IC0tLQo+IEhlbGxvLAo+ IAo+IGZvciB0aGUgcGF0Y2ggdGhhdCBiZWNhbWUKPiAKPiAJMWI5OGFkM2IzYmU5ICgicHdtOiBz dW40aTogRHJvcCByZWR1bmRhbnQgYXNzaWdubWVudCB0byB2YXJpYWJsZSBwdmFsIikKPiAKPiAo YW5kIHdoaWNoIHlpZWxkZWQgdGhlIHNpdHVhdGlvbiB0aGF0IHB2YWwgaXMgb25seSB1c2VkIGlu IHRoaXMgc2luZ2xlCj4gYmxvY2spIEkgc3VnZ2VzdGVkIHRvIGRvIHRoaXMgY2hhbmdlLiBUaGlz IHdhcyBpZ25vcmVkIGhvd2V2ZXIgYnkgYm90aAo+IENvbGluIGFuZCBUaGllcnJ5IHdpdGhvdXQg Y29tbWVudC4gU28gSSBzdWdnZXN0IHRoZSBjaGFuZ2UgaGVyZQo+IHNlcGFyYXRlbHkuCj4gCj4g QmVzdCByZWdhcmRzCj4gVXdlCj4gCj4gIGRyaXZlcnMvcHdtL3B3bS1zdW40aS5jIHwgNCArKyst Cj4gIDEgZmlsZSBjaGFuZ2VkLCAzIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiAKPiBk aWZmIC0tZ2l0IGEvZHJpdmVycy9wd20vcHdtLXN1bjRpLmMgYi9kcml2ZXJzL3B3bS9wd20tc3Vu NGkuYwo+IGluZGV4IDU4MWQyMzI4NzMzMy4uODkxOWU2YWI3NTc3IDEwMDY0NAo+IC0tLSBhL2Ry aXZlcnMvcHdtL3B3bS1zdW40aS5jCj4gKysrIGIvZHJpdmVycy9wd20vcHdtLXN1bjRpLmMKPiBA QCAtMTQ5LDcgKzE0OSw3IEBAIHN0YXRpYyBpbnQgc3VuNGlfcHdtX2NhbGN1bGF0ZShzdHJ1Y3Qg c3VuNGlfcHdtX2NoaXAgKnN1bjRpX3B3bSwKPiAgCQkJICAgICAgIHUzMiAqZHR5LCB1MzIgKnBy ZCwgdW5zaWduZWQgaW50ICpwcnNjbHIpCj4gIHsKPiAgCXU2NCBjbGtfcmF0ZSwgZGl2ID0gMDsK PiAtCXVuc2lnbmVkIGludCBwdmFsLCBwcmVzY2FsZXIgPSAwOwo+ICsJdW5zaWduZWQgaW50IHBy ZXNjYWxlciA9IDA7Cj4gIAo+ICAJY2xrX3JhdGUgPSBjbGtfZ2V0X3JhdGUoc3VuNGlfcHdtLT5j bGspOwo+ICAKPiBAQCAtMTcwLDYgKzE3MCw4IEBAIHN0YXRpYyBpbnQgc3VuNGlfcHdtX2NhbGN1 bGF0ZShzdHJ1Y3Qgc3VuNGlfcHdtX2NoaXAgKnN1bjRpX3B3bSwKPiAgCWlmIChwcmVzY2FsZXIg PT0gMCkgewo+ICAJCS8qIEdvIHVwIGZyb20gdGhlIGZpcnN0IGRpdmlkZXIgKi8KPiAgCQlmb3Ig KHByZXNjYWxlciA9IDA7IHByZXNjYWxlciA8IFBXTV9QUkVTQ0FMX01BU0s7IHByZXNjYWxlcisr KSB7Cj4gKwkJCXVuc2lnbmVkIGludCBwdmFsOwo+ICsKPiAgCQkJaWYgKCFwcmVzY2FsZXJfdGFi bGVbcHJlc2NhbGVyXSkKPiAgCQkJCWNvbnRpbnVlOwo+ICAJCQlwdmFsID0gcHJlc2NhbGVyX3Rh YmxlW3ByZXNjYWxlcl07CgoKbml0IHBpY2tpbmc6CkRvaW5nIHRoZSBhc3NpZ25tZW50IGZpcnN0 IHdvdWxkIHJlbW92ZSB0aGUgb25seSB1c2UKb2YgcHJlc2NhbGVyX3RhYmxlW3ByZXNjYWxlcl0u Cgp1bnNpZ25lZCBpbnQgcHZhbCA9IHByZXNjYWxlcl90YWJsZVtwcmVzY2FsZXJdOwppZiAoICEg cHZhbCApCiAgY29udGludWU7CgppZiB5b3UgZmVlbCBhZHZlbnR1cmVzIHlvdSBjb3VsZCBhbHNv IHJlcGxhY2UgdGhlIGZvcigpIGZvciBhIHdoaWxlKCkKc2luY2Ugd2Uga25vdyB0aGF0IHByZXNj YWxlciA9PSAwLgoKd2hpbGUgKCBwcmVzY2FsZXIgPCBQV01fUFJFU0NBTF9NQVNLICkKewp1bnNp Z25lZCBpbnQgcHZhbCA9IHByZXNjYWxlcl90YWJsZVtwcmVzY2FsZXIrK107Ci4uLi4KCgpqbTJj LAoKIHdoCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwps aW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJh ZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51 eC1hcm0ta2VybmVsCg==