Messages in this thread | ![/](/images/icornerl.gif) | | From | Geert Uytterhoeven <> | Date | Thu, 15 Feb 2024 12:01:49 +0100 | Subject | Re: [PATCH v2 15/15] auxdisplay: Add driver for MAX695x 7-segment LED controllers |
| |
Hi Andy,
On Mon, Feb 12, 2024 at 6:04 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Add initial driver for the MAX6958 and MAX6959 7-segment LED > controllers. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks for your patch!
> --- a/drivers/auxdisplay/Kconfig > +++ b/drivers/auxdisplay/Kconfig > @@ -187,6 +187,20 @@ config HT16K33 > Say yes here to add support for Holtek HT16K33, RAM mapping 16*8 > LED controller driver with keyscan. > > +config MAX6959 > + tristate "Maxim MAX6958/6959 7-segment LED controller with keyscan"
I'd drop the "with keyscan" for now...
> + depends on I2C > + select REGMAP_I2C > + select LINEDISP > + help > + If you say yes here you get support for the following Maxim chips > + (I2C 7-segment LED display controller with keyscan): > + - MAX6958 > + - MAX6959 (debounce support)
s/debounce/input/
> + > + This driver can also be built as a module. If so, the module > + will be called max6959.
> --- /dev/null > +++ b/drivers/auxdisplay/max6959.c
> +/* Defines */ > +#define MIN_BRIGHTNESS 0x01 > +#define MAX_BRIGHTNESS 0x40
Unused? (for now, until you add LED brightness support)
> + > +struct max6959_priv { > + struct linedisp linedisp; > + > + struct delayed_work work; > +
IMHO these blank lines don't add any value.
> + struct regmap *regmap; > +};
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
| ![\](/images/icornerr.gif) |