Re: [PATCH v5 1/3] auxdisplay: Add 7-segment LED display driver

From: Geert Uytterhoeven
Date: Thu Mar 07 2024 - 08:13:46 EST


Hi Chris,

On Thu, Mar 7, 2024 at 12:50 AM Chris Packham
<chris.packham@xxxxxxxxxxxxxxxxxxx> wrote:
> Add a driver for a 7-segment LED display. At the moment only one
> character is supported but it should be possible to expand this to
> support more characters and/or 14-segment displays in the future.
>
> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> ---
>
> Notes:
> Changes in v5:
> - Add depends on GPIOLIB || COMPILE_TEST
> - Validate that we get enough GPIOs

Thanks for the update!

> --- /dev/null
> +++ b/drivers/auxdisplay/seg-led-gpio.c

> +static void seg_led_update(struct work_struct *work)
> +{
> + struct seg_led_priv *priv = container_of(work, struct seg_led_priv, work.work);
> + struct linedisp *linedisp = &priv->linedisp;
> + struct linedisp_map *map = linedisp->map;
> + DECLARE_BITMAP(values, 8) = { 0 };
> +
> + bitmap_set_value8(values, map_to_seg7(&map->map.seg7, linedisp->buf[0]), 0);
> +
> + gpiod_set_array_value_cansleep(priv->segment_gpios->ndescs, priv->segment_gpios->desc,
> + priv->segment_gpios->info, values);

This may still cause an out-of-bounds access of values if ndescs > 8.

> +}

> +static int seg_led_probe(struct platform_device *pdev)
> +{
> + struct seg_led_priv *priv;
> + struct device *dev = &pdev->dev;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + priv->segment_gpios = devm_gpiod_get_array(dev, "segment", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->segment_gpios))
> + return PTR_ERR(priv->segment_gpios);
> +
> + if (priv->segment_gpios->ndescs < 7)

|| priv->segment_gpios->ndescs > 8

> + return -EINVAL;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds