Re: [PATCH v2 2/2] leds: pca955x: Add HW blink support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/8/22 06:19, Andy Shevchenko wrote:
On Thu, Apr 7, 2022 at 10:43 PM Eddie James <eajames@xxxxxxxxxxxxx> 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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux