Re: [PATCH 02/17] dt-bindings: auxdisplay: ht16k33: Document Adafruit segment displays

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

 



Hi Rob,

On 2021-05-18 16:35, Geert Uytterhoeven wrote:
Hoi Robin,

On Tue, Mar 23, 2021 at 10:12 AM robin <robin@xxxxxxxxxxx> wrote:
On 2021-03-22 15:48, Geert Uytterhoeven wrote:
> The Holtek HT16K33 LED controller is not only used for driving
> dot-matrix displays, but also for driving segment displays.
>
> Document compatible values for the Adafruit 7-segment[1] and
> 14-segment[2] FeatherWing expansion boards with red displays.
> According
> to the schematics, all other Adafruit 7-segment and 14-segment display
> backpack and FeatherWing expansion boards (including bare boards and
> boards fitted with displays) are compatible with these two boards.
> Add a "color" property to support the different color variants.
>
> [1] https://www.adafruit.com/product/3108
> [2] https://www.adafruit.com/product/3130
>
> Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>

> --- a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
> +++ b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
> @@ -14,14 +14,23 @@ allOf:
>
>  properties:
>    compatible:
> -    const: holtek,ht16k33
> +    oneOf:
> +      - items:
> +          - const: adafruit,3108  # 0.56" 4-Digit 7-Segment
> FeatherWing Display (Red)
> +          - const: holtek,ht16k33
> +
> +      - items:
> +          - const: adafruit,3130  # 0.54" Quad Alphanumeric
> FeatherWing Display (Red)
> +          - const: holtek,ht16k33
> +
> +      - const: holtek,ht16k33     # Generic 16*8 LED controller with
> dot-matrix display
>
>    reg:
>      maxItems: 1
>
>    refresh-rate-hz:
>      maxItems: 1
> -    description: Display update interval in Hertz
> +    description: Display update interval in Hertz for dot-matrix
> displays

The above should be included in patch 16

I disagree: bindings are independent from the driver implementation.

>    interrupts:
>      maxItems: 1
> @@ -41,10 +50,17 @@ properties:
>      default: 16
>      description: Initial brightness level
>
> +  color: true
> +    description:
> +      Color of the display.  Use one of the LED_COLOR_ID_* prefixed
> definitions
> +      from the header include/dt-bindings/leds/common.h.  The default
> is red.
> +    minimum: 0
> +    maximum: 9
> +    default: 1
> +

The above should be included in patch 17

Same here.

My thought was that one without the other makes no sense. But if it's common
practice to create a separate patch for device tree bindings (it's a patch-set
after all) than that's fine with me.

@Rob what do you think?

Best regards,
Met vriendelijke groet,

Robin van der Gracht



[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux