Re: [PATCH v4 3/3] leds: tlc5925: Add support for non blocking operations

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

 



On Tue, Jun 14, 2022 at 4:27 PM Jean-Jacques Hiblot
<jjhiblot@xxxxxxxxxxxxxxx> wrote:
>
> Settings multiple LEDs in a row can be a slow operation because of the
> time required to acquire the bus and prepare the transfer.
> And, in most cases, it is not required that the operation is synchronous.
>
> Implementing the non-blocking brightness_set() for such cases.
> A work queue is used to perform the actual SPI transfer.
>
> The blocking method is still available in case someone needs to perform
> this operation synchronously (ie by calling led_set_brightness_sync()).
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>
> ---
>  drivers/leds/leds-tlc5925.c | 39 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/leds-tlc5925.c b/drivers/leds/leds-tlc5925.c
> index eeab1138ba2c..2c083b04d7f6 100644
> --- a/drivers/leds/leds-tlc5925.c
> +++ b/drivers/leds/leds-tlc5925.c
> @@ -14,6 +14,7 @@
>  #include <linux/leds.h>
>  #include <linux/module.h>
>  #include <linux/property.h>
> +#include <linux/workqueue.h>
>  #include <linux/spi/spi.h>
>
>  struct single_led_priv {
> @@ -24,10 +25,24 @@ struct single_led_priv {
>  struct tlc5925_leds_priv {
>         int max_num_leds;
>         unsigned long *state;
> +       struct spi_device *spi;
> +       struct work_struct xmit_work;
>         struct single_led_priv leds[];
>  };
>
> -static int tlc5925_brightness_set_blocking(struct led_classdev *cdev,
> +static int xmit(struct tlc5925_leds_priv *priv)
> +{
> +       return spi_write(priv->spi, priv->state, BITS_TO_BYTES(priv->max_num_leds));
> +}
> +
> +static void xmit_work(struct work_struct *ws)
> +{
> +       struct tlc5925_leds_priv *priv =
> +               container_of(ws, struct tlc5925_leds_priv, xmit_work);

+ Blank line.

> +       xmit(priv);
> +};
> +
> +static void tlc5925_brightness_set(struct led_classdev *cdev,
>                                             enum led_brightness brightness)
>  {
>         struct spi_device *spi = to_spi_device(cdev->dev->parent);
> @@ -40,9 +55,25 @@ static int tlc5925_brightness_set_blocking(struct led_classdev *cdev,
>         // assign_bit() is atomic, no need for lock
>         assign_bit(index, priv->state, !!brightness);
>
> -       return spi_write(spi, priv->state, BITS_TO_BYTES(priv->max_num_leds));
> +       schedule_work(&priv->xmit_work);
>  }
>
> +static int tlc5925_brightness_set_blocking(struct led_classdev *cdev,
> +                                           enum led_brightness brightness)
> +{
> +       struct spi_device *spi = to_spi_device(cdev->dev->parent);
> +       struct tlc5925_leds_priv *priv = spi_get_drvdata(spi);

> +       struct single_led_priv *led = container_of(cdev,
> +                                                  struct single_led_priv,
> +                                                  cdev);

Better:

       struct single_led_priv *led =
                        container_of(cdev, struct single_led_priv, cdev);

...

> +       // assign_bit() is atomic, no need for lock

The useless comment.

-- 
With Best Regards,
Andy Shevchenko



[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