Re: [PATCH v2 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959

[Geert Uytterhoeven]

Hi Andy,

On Mon, Feb 12, 2024 at 6:16 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> On Mon, Feb 12, 2024 at 07:14:53PM +0200, Andy Shevchenko wrote:
> > On Mon, Feb 12, 2024 at 07:01:47PM +0200, Andy Shevchenko wrote:
> > > Add initial device tree documentation for Maxim MAX6958/6959.
> >
> > Oh, this is an old version :-(
>
> Here is a new one:
>
> From d8c826e06cf9237cd5fc6b2bb0b1cac5aff4fd8a Mon Sep 17 00:00:00 2001
> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Date: Thu, 8 Feb 2024 17:23:38 +0200
> Subject: [PATCH 1/1] dt-bindings: auxdisplay: Add Maxim MAX6958/6959
>
> Add initial device tree documentation for Maxim MAX6958/6959.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/auxdisplay/maxim,max6959.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/auxdisplay/maxim,max6959.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAX6958/6959 7-segment LED display controller with keyscan
> +
> +maintainers:
> +  - Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> +
> +description:
> +  The Maxim MAX6958/6959 7-segment LED display controller provides
> +  an I2C interface to up to four 7-segment LED digits. The MAX6959
> +  in comparison to MAX6958 has the debounce and interrupt support.

IUIC, the primary differentiating factor is that the MAX6959 adds input
and GPIO support?  Debounce and interrupt support are merely features
of input support.

> +  Type of the chip can be autodetected via specific register read,
> +  and hence the features may be enabled in the driver at run-time.

I don't think you need to read that register, as the users of the
features (keypad mapping, interrupts property, ...) also need to be
described in DTS (once supported).

> +  Given hardware is simple and does not provide any additional pins,
> +  such as reset or enable.

Does this matter? I.e. is it important to say this in the bindings?

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