lkml.org 
[lkml]   [2022]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/2] leds: pca955x: Add HW blink support
From

On 4/8/22 06:19, Andy Shevchenko wrote:
> On Thu, Apr 7, 2022 at 10:43 PM Eddie James <eajames@linux.ibm.com> wrote:
>> Support blinking using the PCA955x chip. Use PWM0 for blinking
>> instead of LED_HALF brightness. Since there is only one frequency
>> and brightness register for any blinking LED, track the blink state
>> of each LED and only support one HW blinking frequency. If another
>> frequency is requested, fallback to software blinking.
> ...
>
>> +#define PCA955X_BLINK_DEFAULT 1000
> What's the unit of this number?


milliseconds, I'll change the name to reflect that.


>
> ...
>
>> * Write to frequency prescaler register, used to program the
>> - * period of the PWM output. period = (PSCx + 1) / 38
>> + * period of the PWM output. period = (PSCx + 1) / <38 or 44, chip dependent>
> Using <> in formulas a bit confusing, what about
>
> * period of the PWM output. period = (PSCx + 1) / coeff
> * where for ... chips coeff = 38, for ... chips coeff = 44.
>
> ?


Ack.


>
> ...
>
>> + dev_err(&pca955x->client->dev, "%s: reg 0x%x, err %d\n",
>> + __func__, n, ret);
> Can be indented better. But I would rather see regmap, where this kind
> of debugging is for free and already present in the regmap core/.


Agree, but perhaps for a future enhancement?


>
> ...
>
>> +static u8 pca955x_period_to_psc(struct pca955x *pca955x, unsigned long p)
>> +{
>> + p *= (unsigned long)pca955x->chipdef->blink_div;
> Why casting?


Ack, and all the rest. Will get a v2 up.


Thanks Andy.

Eddie

\
 
 \ /
  Last update: 2022-04-11 18:33    [W:0.037 / U:0.528 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site