diff for duplicates of <1531918626.16896.22.camel@toradex.com> diff --git a/a/1.txt b/N1/1.txt index 235de06..8079f75 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,85 +1,136 @@ -T24gV2VkLCAyMDE4LTA3LTE4IGF0IDExOjEyICswMTAwLCBEYW5pZWwgVGhvbXBzb24gd3JvdGU6 -DQo+IE9uIFdlZCwgSnVsIDE4LCAyMDE4IGF0IDEwOjUzOjM1QU0gKzAxMDAsIExlZSBKb25lcyB3 -cm90ZToNCj4gPiBPbiBXZWQsIDE4IEp1bCAyMDE4LCBNYXJjZWwgWmlzd2lsZXIgd3JvdGU6DQo+ -ID4gDQo+ID4gPiBPbiBXZWQsIDIwMTgtMDctMTggYXQgMDk6MDkgKzAxMDAsIExlZSBKb25lcyB3 -cm90ZToNCj4gPiA+ID4gT24gTW9uLCAxNiBKdWwgMjAxOCwgRGFuaWVsIFRob21wc29uIHdyb3Rl -Og0KPiA+ID4gPiANCj4gPiA+ID4gPiBDdXJyZW50bHksIGlmIHRoZSBEVCBkb2VzIG5vdCBkZWZp -bmUgbnVtLWludGVycG9sYXRlZC1zdGVwcw0KPiA+ID4gPiA+IHRoZW4NCj4gPiA+ID4gPiBudW1f -c3RlcHMgaXMgdW5kZWZpbmVkIGFuZCB0aGUgaW50ZXJwb2xhdGlvbiBjb2RlIHdpbGwgZGVwbG95 -DQo+ID4gPiA+ID4gcmFuZG9tbHkuDQo+ID4gPiA+ID4gRml4IHRoaXMuDQo+ID4gPiA+ID4gDQo+ -ID4gPiA+ID4gRml4ZXM6IDU3M2ZlNmQxYzI1YyAoImJhY2tsaWdodDogcHdtX2JsOiBMaW5lYXIg -aW50ZXJwb2xhdGlvbg0KPiA+ID4gPiA+IGJldHdlZW4NCj4gPiA+ID4gPiBicmlnaHRuZXNzLWxl -dmVscyIpDQo+ID4gPiA+ID4gUmVwb3J0ZWQtYnk6IE1hcmNlbCBaaXN3aWxlciA8bWFyY2VsLnpp -c3dpbGVyQHRvcmFkZXguY29tPg0KPiA+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IERhbmllbCBUaG9t -cHNvbiA8ZGFuaWVsLnRob21wc29uQGxpbmFyby5vcmc+DQo+ID4gPiA+ID4gU2lnbmVkLW9mZi1i -eTogTWFyY2VsIFppc3dpbGVyIDxtYXJjZWwuemlzd2lsZXJAdG9yYWRleC5jb20+DQo+ID4gPiA+ -IA0KPiA+ID4gPiBUaGlzIGxpbmUgaXMgY29uZnVzaW5nLiAgRGlkIHlvdSBndXlzIGF1dGhvciB0 -aGlzIHBhdGNoDQo+ID4gPiA+IHRvZ2V0aGVyPw0KPiA+ID4gDQo+ID4gPiBZZXMsIEkgcmVwb3J0 -ZWQgaXQgYW5kIHdlIGNhbWUgdG8gYSBjb25jbHVzaW9uIHRvZ2V0aGVyLg0KPiA+IA0KPiA+IEl0 -IHNvdW5kcyBsaWtlIHlvdSBuZWVkIHRvIGhhdmUgYWxsIG9mIHRoZSB0YWdzIChleGNlcHQgdGhp -cyBvbmUpLg0KPiA+IDopDQo+ID4gDQo+ID4gIFJlcG9ydGVkLWJ5OiAgZm9yIHJlcG9ydGluZyB0 -aGUgaXNzdWUNCj4gPiAgU3VnZ2VzdGVkLWJ5OiBmb3Igc3VnZ2VzdGluZyBhIHJlc29sdXRpb24N -Cj4gPiAgQWNrZWQtYnk6ICAgICBmb3IgcmV2aWV3aW5nIGl0DQo+ID4gIFRlc3RlZC1ieTogICAg -Zm9yIHRlc3RpbmcgaXQNCj4gPiANCj4gPiBTaWduZWQtb2ZmLWJ5IHVzdWFsbHkgbWVhbnMgeW91 -IGVpdGhlciB3cm90ZSBhIHNpZ25pZmljYW50IGFtb3VudA0KPiA+IG9mDQo+ID4gdGhlIGRpZmZz -dGF0IG9yIHlvdSB3ZXJlIHBhcnQgb2YgdGhlIHN1Ym1pc3Npb24gcGF0aC4NCj4gDQo+IEhlIGRp -ZCBbSSBkb24ndCBvYmplY3QgdG8gYnV0IHdvdWxkbid0IGhhdmUgdXNlZCB0aGUgZXh0cmEgYnJh -Y2tldHMNCj4geW91DQo+IGJyb3VnaHQgdXAgOy0pIF0uDQoNClllcywgSSB0YWtlIGFsbCB0aGUg -YmxhbWUgZm9yIHRoZSBleHRyYSBicmFja2V0cy4gUmVnYXJkbGVzcyBvZiBoYXZpbmcNCmEgbWFz -dGVycyBpbiBDUyBvciBub3QgSSBzdGlsbCBwcmVmZXIgdG9vIG1hbnkgdGhlbiB0b28gZmV3IG9m -IHRoZW0gKDstDQpwKS4NCg0KPiA+ID4gPiBNeSBndWVzcyBpcyB0aGF0IHRoaXMgbGluZSBzaG91 -bGQgYmUgZHJvcHBlZCBhbmQgdGhlIFJCIGFuZCBUQg0KPiA+ID4gPiB0YWdzDQo+ID4gPiA+IHNo -b3VsZCByZW1haW4/ICBJZiBpdCB3YXMgcmV2aWV3ZWQgdG9vLCBwZXJoYXBzIGFuIEFCIHRvbz8N -Cj4gPiA+IA0KPiA+ID4gSSdtIE9LIGVpdGhlciB3YXkgYW5kIGRvIG5vdCBuZWVkIGFueSBleHBs -aWNpdCBhdXRob3JzaGlwIHRvIGJlDQo+ID4gPiBleHByZXNzZWQgZm9yIG1lLg0KPiA+IA0KPiA+ -IEluIHRoaXMgaW5zdGFuY2UgSSBzdWdnZXN0IGtlZXBpbmcgUmVwb3J0ZWQtYnkgYW5kIFRlc3Rl -ZC1ieS4NCj4gPiANCj4gPiA+ID4gPiBUZXN0ZWQtYnk6IE1hcmNlbCBaaXN3aWxlciA8bWFyY2Vs -Lnppc3dpbGVyQHRvcmFkZXguY29tPg0KPiA+ID4gPiA+IC0tLQ0KPiA+ID4gPiA+ICBkcml2ZXJz -L3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYyB8IDE3ICsrKysrKysrLS0tLS0tLS0tDQo+ID4gPiA+ -ID4gIDEgZmlsZSBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyksIDkgZGVsZXRpb25zKC0pDQo+ID4g -PiA+ID4gDQo+ID4gPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3 -bV9ibC5jDQo+ID4gPiA+ID4gYi9kcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYw0KPiA+ -ID4gPiA+IGluZGV4IDllZTRjMWI3MzViMi4uZTNjMjJiNzlmYmNkIDEwMDY0NA0KPiA+ID4gPiA+ -IC0tLSBhL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3bV9ibC5jDQo+ID4gPiA+ID4gKysrIGIv -ZHJpdmVycy92aWRlby9iYWNrbGlnaHQvcHdtX2JsLmMNCj4gPiA+ID4gPiBAQCAtMjk5LDE1ICsy -OTksMTQgQEAgc3RhdGljIGludA0KPiA+ID4gPiA+IHB3bV9iYWNrbGlnaHRfcGFyc2VfZHQoc3Ry -dWN0DQo+ID4gPiA+ID4gZGV2aWNlICpkZXYsDQo+ID4gPiA+ID4gIAkJICogaW50ZXJwb2xhdGlv -biBiZXR3ZWVuIGVhY2ggb2YgdGhlIHZhbHVlcw0KPiA+ID4gPiA+IG9mDQo+ID4gPiA+ID4gYnJp -Z2h0bmVzcyBsZXZlbHMNCj4gPiA+ID4gPiAgCQkgKiBhbmQgY3JlYXRlcyBhIG5ldyBwcmUtY29t -cHV0ZWQgdGFibGUuDQo+ID4gPiA+ID4gIAkJICovDQo+ID4gPiA+ID4gLQkJb2ZfcHJvcGVydHlf -cmVhZF91MzIobm9kZSwgIm51bS0NCj4gPiA+ID4gPiBpbnRlcnBvbGF0ZWQtDQo+ID4gPiA+ID4g -c3RlcHMiLA0KPiA+ID4gPiA+IC0JCQkJICAgICAmbnVtX3N0ZXBzKTsNCj4gPiA+ID4gPiAtDQo+ -ID4gPiA+ID4gLQkJLyoNCj4gPiA+ID4gPiAtCQkgKiBNYWtlIHN1cmUgdGhhdCB0aGVyZSBpcyBh -dCBsZWFzdCB0d28NCj4gPiA+ID4gPiBlbnRyaWVzIGluDQo+ID4gPiA+ID4gdGhlDQo+ID4gPiA+ -ID4gLQkJICogYnJpZ2h0bmVzcy1sZXZlbHMgdGFibGUsIG90aGVyd2lzZSB3ZQ0KPiA+ID4gPiA+ -IGNhbid0DQo+ID4gPiA+ID4gaW50ZXJwb2xhdGUNCj4gPiA+ID4gPiAtCQkgKiBiZXR3ZWVuIHR3 -byBwb2ludHMuDQo+ID4gPiA+ID4gLQkJICovDQo+ID4gPiA+ID4gLQkJaWYgKG51bV9zdGVwcykg -ew0KPiA+ID4gPiA+ICsJCWlmICgob2ZfcHJvcGVydHlfcmVhZF91MzIobm9kZSwgIm51bS0NCj4g -PiA+ID4gPiBpbnRlcnBvbGF0ZWQtDQo+ID4gPiA+ID4gc3RlcHMiLA0KPiA+ID4gPiA+ICsJCQkJ -CSAgJm51bV9zdGVwcykgPT0gMCkNCj4gPiA+ID4gPiAmJg0KPiA+ID4gPiA+IG51bV9zdGVwcykg -ew0KPiA+ID4gPiANCj4gPiA+ID4gVGhpcyBpcyBwcmV0dHkgdWdseSwgYW5kIGlzbid0IGl0IHN1 -ZmZlcmluZyBmcm9tIG92ZXItDQo+ID4gPiA+IGJyYWNrZXRpbmc/ICBNeQ0KPiA+ID4gPiBzdWdn -ZXN0aW9uIHdvdWxkIGJlIHRvIGJyZWFrIG91dCB0aGUgaW52b2NhdGlvbiBvZg0KPiA+ID4gPiBv -Zl9wcm9wZXJ0eV9yZWFkX3UzMigpIGZyb20gdGhlIGlmIGFuZCB0ZXN0IG9ubHkgdGhlIHJlc3Vs -dC4NCj4gPiA+ID4gDQo+ID4gPiA+IAkJb2ZfcHJvcGVydHlfcmVhZF91MzIobm9kZSwgIm51bS1p -bnRlcnBvbGF0ZWQtDQo+ID4gPiA+IHN0ZXBzIiwgDQo+ID4gPiA+ICZudW1fc3RlcHMpOw0KPiA+ -ID4gDQo+ID4gPiB5b3UgbWVhbjoNCj4gPiA+IA0KPiA+ID4gCQlyZXQgPSBvZl9wcm9wZXJ0eV9y -ZWFkX3UzMihub2RlLCAibnVtLWludGVycG9sYXRlZC0NCj4gPiA+IHN0ZXBzIiwgJm51bV9zdGVw -cyk7DQo+ID4gPiANCj4gPiA+ID4gCQlpZiAoIXJldCAmJiBudW1fc3RlcHMpIHsNCj4gPiA+ID4g -DQo+ID4gPiA+IEkgaGF2ZW4ndCBjaGVja2VkIHRoZSB1bmRlcmxpbmcgY29kZSwgYnV0IGlzIGl0 -IGV2ZW4gZmVhc2libGUNCj4gPiA+ID4gZm9yDQo+ID4gPiA+IG9mX3Byb3BlcnR5X3JlYWRfdTMy -KCkgdG8gbm90IHN1Y2NlZWQgQU5EIGZvciBudW1fc3RlcHMgdG8gYmUNCj4gPiA+ID4gc2V0Pw0K -PiA+ID4gPiANCj4gPiA+ID4gSWYgbm90LCB0aGUgY2hlY2sgZm9yICFyZXQgaWYgc3VwZXJmbHVv -dXMgYW5kIHlvdSBjYW4gZHJvcCBpdC4NCj4gPiA+IA0KPiA+ID4gTm8sIHRoZW4gd2UgYXJlIGJh -Y2sgdG8gdGhlIGluaXRpYWwgaXNzdWUgb2YgbnVtX3N0ZXBzDQo+ID4gPiBwb3RlbnRpYWxseSBu -b3QNCj4gPiA+IGJlaW5nIGluaXRpYWxpc2VkLiBXZSByZWFsbHkgd2FudCBib3RoIG9mX3Byb3Bl -cnR5X3JlYWRfdTMyKCkgdG8NCj4gPiA+IHN1Y2NlZWQgQU5EIG51bV9zdGVwcyB0byBhY3R1YWxs -eSBiZSBzZXQuDQo+ID4gDQo+ID4gSSBhbHNvIHRoaW5rIG51bV9zdGVwcyBzaG91bGQgYmUgcHJl -LWluaXRpYWxpc2VkLg0KDQpZZXMsIEkgZ3Vlc3MgaXQgZGVmaW5pdGVseSBkb2VzIG5vdCBodXJ0 -Lg0KDQo+ID4gVGhlbiBpdCB3aWxsIG9ubHkgYmUgc2V0IGlmIG9mX3Byb3BlcnR5X3JlYWRfdTMy -KCkgc3VjY2VlZHMuDQoNClllcywgYnV0IHdlIHN0aWxsIG5lZWQgdG8gY2hlY2sgZm9yIGJvdGgs -IHRoZSBmdW5jdGlvbiBub3QgZmFpbGluZyBhbmQNCm51bV9zdGVwcyB0byBhY3R1YWxseSBiZSBu -b24gemVyby4NCg0KPiA+IC0tIA0KPiA+IExlZSBKb25lcyBb5p2O55C85pavXQ0KPiA+IExpbmFy -byBTZXJ2aWNlcyBUZWNobmljYWwgTGVhZA0KPiA+IExpbmFyby5vcmcg4pSCIE9wZW4gc291cmNl -IHNvZnR3YXJlIGZvciBBUk0gU29Dcw0KPiA+IEZvbGxvdyBMaW5hcm86IEZhY2Vib29rIHwgVHdp -dHRlciB8IEJsb2c +On Wed, 2018-07-18 at 11:12 +0100, Daniel Thompson wrote: +> On Wed, Jul 18, 2018 at 10:53:35AM +0100, Lee Jones wrote: +> > On Wed, 18 Jul 2018, Marcel Ziswiler wrote: +> > +> > > On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote: +> > > > On Mon, 16 Jul 2018, Daniel Thompson wrote: +> > > > +> > > > > Currently, if the DT does not define num-interpolated-steps +> > > > > then +> > > > > num_steps is undefined and the interpolation code will deploy +> > > > > randomly. +> > > > > Fix this. +> > > > > +> > > > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation +> > > > > between +> > > > > brightness-levels") +> > > > > Reported-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> +> > > > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> +> > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> +> > > > +> > > > This line is confusing. Did you guys author this patch +> > > > together? +> > > +> > > Yes, I reported it and we came to a conclusion together. +> > +> > It sounds like you need to have all of the tags (except this one). +> > :) +> > +> > Reported-by: for reporting the issue +> > Suggested-by: for suggesting a resolution +> > Acked-by: for reviewing it +> > Tested-by: for testing it +> > +> > Signed-off-by usually means you either wrote a significant amount +> > of +> > the diffstat or you were part of the submission path. +> +> He did [I don't object to but wouldn't have used the extra brackets +> you +> brought up ;-) ]. + +Yes, I take all the blame for the extra brackets. Regardless of having +a masters in CS or not I still prefer too many then too few of them (;- +p). + +> > > > My guess is that this line should be dropped and the RB and TB +> > > > tags +> > > > should remain? If it was reviewed too, perhaps an AB too? +> > > +> > > I'm OK either way and do not need any explicit authorship to be +> > > expressed for me. +> > +> > In this instance I suggest keeping Reported-by and Tested-by. +> > +> > > > > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> +> > > > > --- +> > > > > drivers/video/backlight/pwm_bl.c | 17 ++++++++--------- +> > > > > 1 file changed, 8 insertions(+), 9 deletions(-) +> > > > > +> > > > > diff --git a/drivers/video/backlight/pwm_bl.c +> > > > > b/drivers/video/backlight/pwm_bl.c +> > > > > index 9ee4c1b735b2..e3c22b79fbcd 100644 +> > > > > --- a/drivers/video/backlight/pwm_bl.c +> > > > > +++ b/drivers/video/backlight/pwm_bl.c +> > > > > @@ -299,15 +299,14 @@ static int +> > > > > pwm_backlight_parse_dt(struct +> > > > > device *dev, +> > > > > * interpolation between each of the values +> > > > > of +> > > > > brightness levels +> > > > > * and creates a new pre-computed table. +> > > > > */ +> > > > > - of_property_read_u32(node, "num- +> > > > > interpolated- +> > > > > steps", +> > > > > - &num_steps); +> > > > > - +> > > > > - /* +> > > > > - * Make sure that there is at least two +> > > > > entries in +> > > > > the +> > > > > - * brightness-levels table, otherwise we +> > > > > can't +> > > > > interpolate +> > > > > - * between two points. +> > > > > - */ +> > > > > - if (num_steps) { +> > > > > + if ((of_property_read_u32(node, "num- +> > > > > interpolated- +> > > > > steps", +> > > > > + &num_steps) == 0) +> > > > > && +> > > > > num_steps) { +> > > > +> > > > This is pretty ugly, and isn't it suffering from over- +> > > > bracketing? My +> > > > suggestion would be to break out the invocation of +> > > > of_property_read_u32() from the if and test only the result. +> > > > +> > > > of_property_read_u32(node, "num-interpolated- +> > > > steps", +> > > > &num_steps); +> > > +> > > you mean: +> > > +> > > ret = of_property_read_u32(node, "num-interpolated- +> > > steps", &num_steps); +> > > +> > > > if (!ret && num_steps) { +> > > > +> > > > I haven't checked the underling code, but is it even feasible +> > > > for +> > > > of_property_read_u32() to not succeed AND for num_steps to be +> > > > set? +> > > > +> > > > If not, the check for !ret if superfluous and you can drop it. +> > > +> > > No, then we are back to the initial issue of num_steps +> > > potentially not +> > > being initialised. We really want both of_property_read_u32() to +> > > succeed AND num_steps to actually be set. +> > +> > I also think num_steps should be pre-initialised. + +Yes, I guess it definitely does not hurt. + +> > Then it will only be set if of_property_read_u32() succeeds. + +Yes, but we still need to check for both, the function not failing and +num_steps to actually be non zero. + +> > -- +> > Lee Jones [李琼斯] +> > Linaro Services Technical Lead +> > Linaro.org │ Open source software for ARM SoCs +> > Follow Linaro: Facebook | Twitter | Blog diff --git a/a/content_digest b/N1/content_digest index ec0e70f..5e45775 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -18,90 +18,141 @@ " patches@linaro.org <patches@linaro.org>\0" "\00:1\0" "b\0" - "T24gV2VkLCAyMDE4LTA3LTE4IGF0IDExOjEyICswMTAwLCBEYW5pZWwgVGhvbXBzb24gd3JvdGU6\n" - "DQo+IE9uIFdlZCwgSnVsIDE4LCAyMDE4IGF0IDEwOjUzOjM1QU0gKzAxMDAsIExlZSBKb25lcyB3\n" - "cm90ZToNCj4gPiBPbiBXZWQsIDE4IEp1bCAyMDE4LCBNYXJjZWwgWmlzd2lsZXIgd3JvdGU6DQo+\n" - "ID4gDQo+ID4gPiBPbiBXZWQsIDIwMTgtMDctMTggYXQgMDk6MDkgKzAxMDAsIExlZSBKb25lcyB3\n" - "cm90ZToNCj4gPiA+ID4gT24gTW9uLCAxNiBKdWwgMjAxOCwgRGFuaWVsIFRob21wc29uIHdyb3Rl\n" - "Og0KPiA+ID4gPiANCj4gPiA+ID4gPiBDdXJyZW50bHksIGlmIHRoZSBEVCBkb2VzIG5vdCBkZWZp\n" - "bmUgbnVtLWludGVycG9sYXRlZC1zdGVwcw0KPiA+ID4gPiA+IHRoZW4NCj4gPiA+ID4gPiBudW1f\n" - "c3RlcHMgaXMgdW5kZWZpbmVkIGFuZCB0aGUgaW50ZXJwb2xhdGlvbiBjb2RlIHdpbGwgZGVwbG95\n" - "DQo+ID4gPiA+ID4gcmFuZG9tbHkuDQo+ID4gPiA+ID4gRml4IHRoaXMuDQo+ID4gPiA+ID4gDQo+\n" - "ID4gPiA+ID4gRml4ZXM6IDU3M2ZlNmQxYzI1YyAoImJhY2tsaWdodDogcHdtX2JsOiBMaW5lYXIg\n" - "aW50ZXJwb2xhdGlvbg0KPiA+ID4gPiA+IGJldHdlZW4NCj4gPiA+ID4gPiBicmlnaHRuZXNzLWxl\n" - "dmVscyIpDQo+ID4gPiA+ID4gUmVwb3J0ZWQtYnk6IE1hcmNlbCBaaXN3aWxlciA8bWFyY2VsLnpp\n" - "c3dpbGVyQHRvcmFkZXguY29tPg0KPiA+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IERhbmllbCBUaG9t\n" - "cHNvbiA8ZGFuaWVsLnRob21wc29uQGxpbmFyby5vcmc+DQo+ID4gPiA+ID4gU2lnbmVkLW9mZi1i\n" - "eTogTWFyY2VsIFppc3dpbGVyIDxtYXJjZWwuemlzd2lsZXJAdG9yYWRleC5jb20+DQo+ID4gPiA+\n" - "IA0KPiA+ID4gPiBUaGlzIGxpbmUgaXMgY29uZnVzaW5nLiAgRGlkIHlvdSBndXlzIGF1dGhvciB0\n" - "aGlzIHBhdGNoDQo+ID4gPiA+IHRvZ2V0aGVyPw0KPiA+ID4gDQo+ID4gPiBZZXMsIEkgcmVwb3J0\n" - "ZWQgaXQgYW5kIHdlIGNhbWUgdG8gYSBjb25jbHVzaW9uIHRvZ2V0aGVyLg0KPiA+IA0KPiA+IEl0\n" - "IHNvdW5kcyBsaWtlIHlvdSBuZWVkIHRvIGhhdmUgYWxsIG9mIHRoZSB0YWdzIChleGNlcHQgdGhp\n" - "cyBvbmUpLg0KPiA+IDopDQo+ID4gDQo+ID4gIFJlcG9ydGVkLWJ5OiAgZm9yIHJlcG9ydGluZyB0\n" - "aGUgaXNzdWUNCj4gPiAgU3VnZ2VzdGVkLWJ5OiBmb3Igc3VnZ2VzdGluZyBhIHJlc29sdXRpb24N\n" - "Cj4gPiAgQWNrZWQtYnk6ICAgICBmb3IgcmV2aWV3aW5nIGl0DQo+ID4gIFRlc3RlZC1ieTogICAg\n" - "Zm9yIHRlc3RpbmcgaXQNCj4gPiANCj4gPiBTaWduZWQtb2ZmLWJ5IHVzdWFsbHkgbWVhbnMgeW91\n" - "IGVpdGhlciB3cm90ZSBhIHNpZ25pZmljYW50IGFtb3VudA0KPiA+IG9mDQo+ID4gdGhlIGRpZmZz\n" - "dGF0IG9yIHlvdSB3ZXJlIHBhcnQgb2YgdGhlIHN1Ym1pc3Npb24gcGF0aC4NCj4gDQo+IEhlIGRp\n" - "ZCBbSSBkb24ndCBvYmplY3QgdG8gYnV0IHdvdWxkbid0IGhhdmUgdXNlZCB0aGUgZXh0cmEgYnJh\n" - "Y2tldHMNCj4geW91DQo+IGJyb3VnaHQgdXAgOy0pIF0uDQoNClllcywgSSB0YWtlIGFsbCB0aGUg\n" - "YmxhbWUgZm9yIHRoZSBleHRyYSBicmFja2V0cy4gUmVnYXJkbGVzcyBvZiBoYXZpbmcNCmEgbWFz\n" - "dGVycyBpbiBDUyBvciBub3QgSSBzdGlsbCBwcmVmZXIgdG9vIG1hbnkgdGhlbiB0b28gZmV3IG9m\n" - "IHRoZW0gKDstDQpwKS4NCg0KPiA+ID4gPiBNeSBndWVzcyBpcyB0aGF0IHRoaXMgbGluZSBzaG91\n" - "bGQgYmUgZHJvcHBlZCBhbmQgdGhlIFJCIGFuZCBUQg0KPiA+ID4gPiB0YWdzDQo+ID4gPiA+IHNo\n" - "b3VsZCByZW1haW4/ICBJZiBpdCB3YXMgcmV2aWV3ZWQgdG9vLCBwZXJoYXBzIGFuIEFCIHRvbz8N\n" - "Cj4gPiA+IA0KPiA+ID4gSSdtIE9LIGVpdGhlciB3YXkgYW5kIGRvIG5vdCBuZWVkIGFueSBleHBs\n" - "aWNpdCBhdXRob3JzaGlwIHRvIGJlDQo+ID4gPiBleHByZXNzZWQgZm9yIG1lLg0KPiA+IA0KPiA+\n" - "IEluIHRoaXMgaW5zdGFuY2UgSSBzdWdnZXN0IGtlZXBpbmcgUmVwb3J0ZWQtYnkgYW5kIFRlc3Rl\n" - "ZC1ieS4NCj4gPiANCj4gPiA+ID4gPiBUZXN0ZWQtYnk6IE1hcmNlbCBaaXN3aWxlciA8bWFyY2Vs\n" - "Lnppc3dpbGVyQHRvcmFkZXguY29tPg0KPiA+ID4gPiA+IC0tLQ0KPiA+ID4gPiA+ICBkcml2ZXJz\n" - "L3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYyB8IDE3ICsrKysrKysrLS0tLS0tLS0tDQo+ID4gPiA+\n" - "ID4gIDEgZmlsZSBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyksIDkgZGVsZXRpb25zKC0pDQo+ID4g\n" - "PiA+ID4gDQo+ID4gPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3\n" - "bV9ibC5jDQo+ID4gPiA+ID4gYi9kcml2ZXJzL3ZpZGVvL2JhY2tsaWdodC9wd21fYmwuYw0KPiA+\n" - "ID4gPiA+IGluZGV4IDllZTRjMWI3MzViMi4uZTNjMjJiNzlmYmNkIDEwMDY0NA0KPiA+ID4gPiA+\n" - "IC0tLSBhL2RyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3B3bV9ibC5jDQo+ID4gPiA+ID4gKysrIGIv\n" - "ZHJpdmVycy92aWRlby9iYWNrbGlnaHQvcHdtX2JsLmMNCj4gPiA+ID4gPiBAQCAtMjk5LDE1ICsy\n" - "OTksMTQgQEAgc3RhdGljIGludA0KPiA+ID4gPiA+IHB3bV9iYWNrbGlnaHRfcGFyc2VfZHQoc3Ry\n" - "dWN0DQo+ID4gPiA+ID4gZGV2aWNlICpkZXYsDQo+ID4gPiA+ID4gIAkJICogaW50ZXJwb2xhdGlv\n" - "biBiZXR3ZWVuIGVhY2ggb2YgdGhlIHZhbHVlcw0KPiA+ID4gPiA+IG9mDQo+ID4gPiA+ID4gYnJp\n" - "Z2h0bmVzcyBsZXZlbHMNCj4gPiA+ID4gPiAgCQkgKiBhbmQgY3JlYXRlcyBhIG5ldyBwcmUtY29t\n" - "cHV0ZWQgdGFibGUuDQo+ID4gPiA+ID4gIAkJICovDQo+ID4gPiA+ID4gLQkJb2ZfcHJvcGVydHlf\n" - "cmVhZF91MzIobm9kZSwgIm51bS0NCj4gPiA+ID4gPiBpbnRlcnBvbGF0ZWQtDQo+ID4gPiA+ID4g\n" - "c3RlcHMiLA0KPiA+ID4gPiA+IC0JCQkJICAgICAmbnVtX3N0ZXBzKTsNCj4gPiA+ID4gPiAtDQo+\n" - "ID4gPiA+ID4gLQkJLyoNCj4gPiA+ID4gPiAtCQkgKiBNYWtlIHN1cmUgdGhhdCB0aGVyZSBpcyBh\n" - "dCBsZWFzdCB0d28NCj4gPiA+ID4gPiBlbnRyaWVzIGluDQo+ID4gPiA+ID4gdGhlDQo+ID4gPiA+\n" - "ID4gLQkJICogYnJpZ2h0bmVzcy1sZXZlbHMgdGFibGUsIG90aGVyd2lzZSB3ZQ0KPiA+ID4gPiA+\n" - "IGNhbid0DQo+ID4gPiA+ID4gaW50ZXJwb2xhdGUNCj4gPiA+ID4gPiAtCQkgKiBiZXR3ZWVuIHR3\n" - "byBwb2ludHMuDQo+ID4gPiA+ID4gLQkJICovDQo+ID4gPiA+ID4gLQkJaWYgKG51bV9zdGVwcykg\n" - "ew0KPiA+ID4gPiA+ICsJCWlmICgob2ZfcHJvcGVydHlfcmVhZF91MzIobm9kZSwgIm51bS0NCj4g\n" - "PiA+ID4gPiBpbnRlcnBvbGF0ZWQtDQo+ID4gPiA+ID4gc3RlcHMiLA0KPiA+ID4gPiA+ICsJCQkJ\n" - "CSAgJm51bV9zdGVwcykgPT0gMCkNCj4gPiA+ID4gPiAmJg0KPiA+ID4gPiA+IG51bV9zdGVwcykg\n" - "ew0KPiA+ID4gPiANCj4gPiA+ID4gVGhpcyBpcyBwcmV0dHkgdWdseSwgYW5kIGlzbid0IGl0IHN1\n" - "ZmZlcmluZyBmcm9tIG92ZXItDQo+ID4gPiA+IGJyYWNrZXRpbmc/ICBNeQ0KPiA+ID4gPiBzdWdn\n" - "ZXN0aW9uIHdvdWxkIGJlIHRvIGJyZWFrIG91dCB0aGUgaW52b2NhdGlvbiBvZg0KPiA+ID4gPiBv\n" - "Zl9wcm9wZXJ0eV9yZWFkX3UzMigpIGZyb20gdGhlIGlmIGFuZCB0ZXN0IG9ubHkgdGhlIHJlc3Vs\n" - "dC4NCj4gPiA+ID4gDQo+ID4gPiA+IAkJb2ZfcHJvcGVydHlfcmVhZF91MzIobm9kZSwgIm51bS1p\n" - "bnRlcnBvbGF0ZWQtDQo+ID4gPiA+IHN0ZXBzIiwgDQo+ID4gPiA+ICZudW1fc3RlcHMpOw0KPiA+\n" - "ID4gDQo+ID4gPiB5b3UgbWVhbjoNCj4gPiA+IA0KPiA+ID4gCQlyZXQgPSBvZl9wcm9wZXJ0eV9y\n" - "ZWFkX3UzMihub2RlLCAibnVtLWludGVycG9sYXRlZC0NCj4gPiA+IHN0ZXBzIiwgJm51bV9zdGVw\n" - "cyk7DQo+ID4gPiANCj4gPiA+ID4gCQlpZiAoIXJldCAmJiBudW1fc3RlcHMpIHsNCj4gPiA+ID4g\n" - "DQo+ID4gPiA+IEkgaGF2ZW4ndCBjaGVja2VkIHRoZSB1bmRlcmxpbmcgY29kZSwgYnV0IGlzIGl0\n" - "IGV2ZW4gZmVhc2libGUNCj4gPiA+ID4gZm9yDQo+ID4gPiA+IG9mX3Byb3BlcnR5X3JlYWRfdTMy\n" - "KCkgdG8gbm90IHN1Y2NlZWQgQU5EIGZvciBudW1fc3RlcHMgdG8gYmUNCj4gPiA+ID4gc2V0Pw0K\n" - "PiA+ID4gPiANCj4gPiA+ID4gSWYgbm90LCB0aGUgY2hlY2sgZm9yICFyZXQgaWYgc3VwZXJmbHVv\n" - "dXMgYW5kIHlvdSBjYW4gZHJvcCBpdC4NCj4gPiA+IA0KPiA+ID4gTm8sIHRoZW4gd2UgYXJlIGJh\n" - "Y2sgdG8gdGhlIGluaXRpYWwgaXNzdWUgb2YgbnVtX3N0ZXBzDQo+ID4gPiBwb3RlbnRpYWxseSBu\n" - "b3QNCj4gPiA+IGJlaW5nIGluaXRpYWxpc2VkLiBXZSByZWFsbHkgd2FudCBib3RoIG9mX3Byb3Bl\n" - "cnR5X3JlYWRfdTMyKCkgdG8NCj4gPiA+IHN1Y2NlZWQgQU5EIG51bV9zdGVwcyB0byBhY3R1YWxs\n" - "eSBiZSBzZXQuDQo+ID4gDQo+ID4gSSBhbHNvIHRoaW5rIG51bV9zdGVwcyBzaG91bGQgYmUgcHJl\n" - "LWluaXRpYWxpc2VkLg0KDQpZZXMsIEkgZ3Vlc3MgaXQgZGVmaW5pdGVseSBkb2VzIG5vdCBodXJ0\n" - "Lg0KDQo+ID4gVGhlbiBpdCB3aWxsIG9ubHkgYmUgc2V0IGlmIG9mX3Byb3BlcnR5X3JlYWRfdTMy\n" - "KCkgc3VjY2VlZHMuDQoNClllcywgYnV0IHdlIHN0aWxsIG5lZWQgdG8gY2hlY2sgZm9yIGJvdGgs\n" - "IHRoZSBmdW5jdGlvbiBub3QgZmFpbGluZyBhbmQNCm51bV9zdGVwcyB0byBhY3R1YWxseSBiZSBu\n" - "b24gemVyby4NCg0KPiA+IC0tIA0KPiA+IExlZSBKb25lcyBb5p2O55C85pavXQ0KPiA+IExpbmFy\n" - "byBTZXJ2aWNlcyBUZWNobmljYWwgTGVhZA0KPiA+IExpbmFyby5vcmcg4pSCIE9wZW4gc291cmNl\n" - "IHNvZnR3YXJlIGZvciBBUk0gU29Dcw0KPiA+IEZvbGxvdyBMaW5hcm86IEZhY2Vib29rIHwgVHdp\n" - dHRlciB8IEJsb2c + "On Wed, 2018-07-18 at 11:12 +0100, Daniel Thompson wrote:\n" + "> On Wed, Jul 18, 2018 at 10:53:35AM +0100, Lee Jones wrote:\n" + "> > On Wed, 18 Jul 2018, Marcel Ziswiler wrote:\n" + "> > \n" + "> > > On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:\n" + "> > > > On Mon, 16 Jul 2018, Daniel Thompson wrote:\n" + "> > > > \n" + "> > > > > Currently, if the DT does not define num-interpolated-steps\n" + "> > > > > then\n" + "> > > > > num_steps is undefined and the interpolation code will deploy\n" + "> > > > > randomly.\n" + "> > > > > Fix this.\n" + "> > > > > \n" + "> > > > > Fixes: 573fe6d1c25c (\"backlight: pwm_bl: Linear interpolation\n" + "> > > > > between\n" + "> > > > > brightness-levels\")\n" + "> > > > > Reported-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>\n" + "> > > > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>\n" + "> > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>\n" + "> > > > \n" + "> > > > This line is confusing. Did you guys author this patch\n" + "> > > > together?\n" + "> > > \n" + "> > > Yes, I reported it and we came to a conclusion together.\n" + "> > \n" + "> > It sounds like you need to have all of the tags (except this one).\n" + "> > :)\n" + "> > \n" + "> > Reported-by: for reporting the issue\n" + "> > Suggested-by: for suggesting a resolution\n" + "> > Acked-by: for reviewing it\n" + "> > Tested-by: for testing it\n" + "> > \n" + "> > Signed-off-by usually means you either wrote a significant amount\n" + "> > of\n" + "> > the diffstat or you were part of the submission path.\n" + "> \n" + "> He did [I don't object to but wouldn't have used the extra brackets\n" + "> you\n" + "> brought up ;-) ].\n" + "\n" + "Yes, I take all the blame for the extra brackets. Regardless of having\n" + "a masters in CS or not I still prefer too many then too few of them (;-\n" + "p).\n" + "\n" + "> > > > My guess is that this line should be dropped and the RB and TB\n" + "> > > > tags\n" + "> > > > should remain? If it was reviewed too, perhaps an AB too?\n" + "> > > \n" + "> > > I'm OK either way and do not need any explicit authorship to be\n" + "> > > expressed for me.\n" + "> > \n" + "> > In this instance I suggest keeping Reported-by and Tested-by.\n" + "> > \n" + "> > > > > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>\n" + "> > > > > ---\n" + "> > > > > drivers/video/backlight/pwm_bl.c | 17 ++++++++---------\n" + "> > > > > 1 file changed, 8 insertions(+), 9 deletions(-)\n" + "> > > > > \n" + "> > > > > diff --git a/drivers/video/backlight/pwm_bl.c\n" + "> > > > > b/drivers/video/backlight/pwm_bl.c\n" + "> > > > > index 9ee4c1b735b2..e3c22b79fbcd 100644\n" + "> > > > > --- a/drivers/video/backlight/pwm_bl.c\n" + "> > > > > +++ b/drivers/video/backlight/pwm_bl.c\n" + "> > > > > @@ -299,15 +299,14 @@ static int\n" + "> > > > > pwm_backlight_parse_dt(struct\n" + "> > > > > device *dev,\n" + "> > > > > \t\t * interpolation between each of the values\n" + "> > > > > of\n" + "> > > > > brightness levels\n" + "> > > > > \t\t * and creates a new pre-computed table.\n" + "> > > > > \t\t */\n" + "> > > > > -\t\tof_property_read_u32(node, \"num-\n" + "> > > > > interpolated-\n" + "> > > > > steps\",\n" + "> > > > > -\t\t\t\t &num_steps);\n" + "> > > > > -\n" + "> > > > > -\t\t/*\n" + "> > > > > -\t\t * Make sure that there is at least two\n" + "> > > > > entries in\n" + "> > > > > the\n" + "> > > > > -\t\t * brightness-levels table, otherwise we\n" + "> > > > > can't\n" + "> > > > > interpolate\n" + "> > > > > -\t\t * between two points.\n" + "> > > > > -\t\t */\n" + "> > > > > -\t\tif (num_steps) {\n" + "> > > > > +\t\tif ((of_property_read_u32(node, \"num-\n" + "> > > > > interpolated-\n" + "> > > > > steps\",\n" + "> > > > > +\t\t\t\t\t &num_steps) == 0)\n" + "> > > > > &&\n" + "> > > > > num_steps) {\n" + "> > > > \n" + "> > > > This is pretty ugly, and isn't it suffering from over-\n" + "> > > > bracketing? My\n" + "> > > > suggestion would be to break out the invocation of\n" + "> > > > of_property_read_u32() from the if and test only the result.\n" + "> > > > \n" + "> > > > \t\tof_property_read_u32(node, \"num-interpolated-\n" + "> > > > steps\", \n" + "> > > > &num_steps);\n" + "> > > \n" + "> > > you mean:\n" + "> > > \n" + "> > > \t\tret = of_property_read_u32(node, \"num-interpolated-\n" + "> > > steps\", &num_steps);\n" + "> > > \n" + "> > > > \t\tif (!ret && num_steps) {\n" + "> > > > \n" + "> > > > I haven't checked the underling code, but is it even feasible\n" + "> > > > for\n" + "> > > > of_property_read_u32() to not succeed AND for num_steps to be\n" + "> > > > set?\n" + "> > > > \n" + "> > > > If not, the check for !ret if superfluous and you can drop it.\n" + "> > > \n" + "> > > No, then we are back to the initial issue of num_steps\n" + "> > > potentially not\n" + "> > > being initialised. We really want both of_property_read_u32() to\n" + "> > > succeed AND num_steps to actually be set.\n" + "> > \n" + "> > I also think num_steps should be pre-initialised.\n" + "\n" + "Yes, I guess it definitely does not hurt.\n" + "\n" + "> > Then it will only be set if of_property_read_u32() succeeds.\n" + "\n" + "Yes, but we still need to check for both, the function not failing and\n" + "num_steps to actually be non zero.\n" + "\n" + "> > -- \n" + "> > Lee Jones [\346\235\216\347\220\274\346\226\257]\n" + "> > Linaro Services Technical Lead\n" + "> > Linaro.org \342\224\202 Open source software for ARM SoCs\n" + > > Follow Linaro: Facebook | Twitter | Blog -3f53660e72c8fe99de2f57349f2fe90b8decc2c0e36930c0e66519b9d86cdbb4 +f38142681e555148560a54cb81e8e62bc4c6e50fad31e587d6693be94d88ba10
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.