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
