| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 14 Oct 2016 08:36:23 +0200
From: Jacek Anaszewski <j.anaszewski@...sung.com>
To: Matt Ranostay <mranostay@...il.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Matt Ranostay <matt@...ostay.consulting>,
Tony Lindgren <tony@...mide.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <Mark.Rutland@....com>
Subject: Re: [PATCH] leds: leds-pca963x: workaround group blink scaling issue
On 10/13/2016 04:20 PM, Matt Ranostay wrote:
> On Thu, Oct 13, 2016 at 4:05 PM, Jacek Anaszewski
> <j.anaszewski@...sung.com> wrote:
>> Hi Matt,
>>
>> On 10/13/2016 03:16 PM, Matt Ranostay wrote:
>>>
>>> PCA9632TK part seems to incorrectly blink at ~1.3x of the programmed
>>> rate. This patchset add a nxp,period-scale devicetree property to
>>> adjust for this misconfiguration.
>>>
>>> Cc: Tony Lindgren <tony@...mide.com>
>>> Cc: Jacek Anaszewski <j.anaszewski@...sung.com>
>>> Signed-off-by: Matt Ranostay <matt@...ostay.consulting>
>>> ---
>>> Documentation/devicetree/bindings/leds/pca963x.txt | 3 +++
>>> drivers/leds/leds-pca963x.c | 18
>>> +++++++++++++++---
>>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/pca963x.txt
>>> b/Documentation/devicetree/bindings/leds/pca963x.txt
>>> index dafbe9931c2b..dfbdb123a9bf 100644
>>> --- a/Documentation/devicetree/bindings/leds/pca963x.txt
>>> +++ b/Documentation/devicetree/bindings/leds/pca963x.txt
>>> @@ -7,6 +7,9 @@ Optional properties:
>>> - nxp,totem-pole : use totem pole (push-pull) instead of open-drain
>>> (pca9632 defaults
>>> to open-drain, newer chips to totem pole)
>>> - nxp,hw-blink : use hardware blinking instead of software blinking
>>> +- nxp,period-scale : In some configurations, the chip blinks faster than
>>> expected.
>>> + This parameter provides a scaling ratio (fixed point,
>>> decimal divided
>>> + by 1000) to compensate, e.g. 1300=1.3x and 750=0.75x.
>>
>>
>> Why DT property? Is it somehow dependent on the board configuration?
>> How this period-scale value is calculated? Is it inferred empirically?
>>
>
> We empirically discovered and verified this with an logic analyzer on
> multiple batches of this part.
> Reason for the DT entry is we aren't 100% sure that it is always going
> to be the same with different board revs.
>
> Could be that parts clock acts differently with supply voltage. This
> has been calculated by setting it an expected value, and measuring the
> actual result with the logic analyzer.
I'd like to have DT maintainer's ack for this.
Cc Rob and Mark.
>>> Each led is represented as a sub-node of the nxp,pca963x device.
>>>
>>> diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
>>> index 407eba11e187..b6ce1f2ec33e 100644
>>> --- a/drivers/leds/leds-pca963x.c
>>> +++ b/drivers/leds/leds-pca963x.c
>>> @@ -59,6 +59,7 @@ struct pca963x_chipdef {
>>> u8 grpfreq;
>>> u8 ledout_base;
>>> int n_leds;
>>> + unsigned int scaling;
>>> };
>>>
>>> static struct pca963x_chipdef pca963x_chipdefs[] = {
>>> @@ -189,6 +190,14 @@ static int pca963x_led_set(struct led_classdev
>>> *led_cdev,
>>> return pca963x_brightness(pca963x, value);
>>> }
>>>
>>> +static unsigned int pca963x_period_scale(struct pca963x_led *pca963x,
>>> + unsigned int val)
>>> +{
>>> + unsigned int scaling = pca963x->chip->chipdef->scaling;
>>> +
>>> + return scaling ? DIV_ROUND_CLOSEST(val * scaling, 1000) : val;
>>> +}
>>> +
>>> static int pca963x_blink_set(struct led_classdev *led_cdev,
>>> unsigned long *delay_on, unsigned long *delay_off)
>>> {
>>> @@ -207,14 +216,14 @@ static int pca963x_blink_set(struct led_classdev
>>> *led_cdev,
>>> time_off = 500;
>>> }
>>>
>>> - period = time_on + time_off;
>>> + period = pca963x_period_scale(pca963x, time_on + time_off);
>>>
>>> /* If period not supported by hardware, default to someting sane.
>>> */
>>> if ((period < PCA963X_BLINK_PERIOD_MIN) ||
>>> (period > PCA963X_BLINK_PERIOD_MAX)) {
>>> time_on = 500;
>>> time_off = 500;
>>> - period = time_on + time_off;
>>> + period = pca963x_period_scale(pca963x, 1000);
>>> }
>>>
>>> /*
>>> @@ -222,7 +231,7 @@ static int pca963x_blink_set(struct led_classdev
>>> *led_cdev,
>>> * (time_on / period) = (GDC / 256) ->
>>> * GDC = ((time_on * 256) / period)
>>> */
>>> - gdc = (time_on * 256) / period;
>>> + gdc = (pca963x_period_scale(pca963x, time_on) * 256) / period;
>>>
>>> /*
>>> * From manual: period = ((GFRQ + 1) / 24) in seconds.
>>> @@ -294,6 +303,9 @@ pca963x_dt_init(struct i2c_client *client, struct
>>> pca963x_chipdef *chip)
>>> else
>>> pdata->blink_type = PCA963X_SW_BLINK;
>>>
>>> + if (of_property_read_u32(np, "nxp,period-scale", &chip->scaling))
>>> + chip->scaling = 1000;
>>> +
>>> return pdata;
>>> }
>>>
>>>
>>
>>
>> --
>> Best regards,
>> Jacek Anaszewski
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
--
Best regards,
Jacek Anaszewski
Powered by blists - more mailing lists