diff mbox series

[v3,2/5] dt-bindings: auxdisplay: Add Titan Micro Electronics TM1628

Message ID 2b7f0011-8872-70fa-b9b7-50b7817e7084@gmail.com (mailing list archive)
State Superseded
Headers show
Series auxdisplay: Add support for the Titanmec TM1628 7 segment display controller | expand

Commit Message

Heiner Kallweit Feb. 23, 2022, 5:59 p.m. UTC 
Add a YAML schema binding for TM1628 auxdisplay
(7/11-segment LED) controller.

This patch is partially based on previous work from
Andreas Färber <afaerber@suse.de>.

Co-Developed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Co-Developed-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v3:
- fix remaining YAML issues
- use Co-Developed-by
---
 .../bindings/auxdisplay/titanmec,tm1628.yaml  | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml

Comments

Rob Herring Feb. 24, 2022, 8:22 p.m. UTC | #1 
On Wed, Feb 23, 2022 at 06:59:31PM +0100, Heiner Kallweit wrote:
> Add a YAML schema binding for TM1628 auxdisplay
> (7/11-segment LED) controller.
> 
> This patch is partially based on previous work from
> Andreas Färber <afaerber@suse.de>.
> 
> Co-Developed-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Co-Developed-by: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v3:
> - fix remaining YAML issues
> - use Co-Developed-by
> ---
>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 92 +++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> 
> diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> new file mode 100644
> index 000000000..2a1ef692c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Titan Micro Electronics TM1628 LED controller
> +
> +maintainers:
> +  - Andreas Färber <afaerber@suse.de>
> +  - Heiner Kallweit <hkallweit1@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: titanmec,tm1628
> +
> +  reg:
> +    maxItems: 1
> +

> +  grid:
> +    description:
> +      Mapping of display digit position to grid number.
> +      This implicitly defines the display size.
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 1
> +    maxItems: 7
> +
> +  segment-mapping:
> +    description:
> +      Mapping of 7 segment display segments A-G to bit numbers 1-12.
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 7
> +    maxItems: 7

Are these properties useful for any 7 segment display or specific to 
this controller?

The commit msg mentions 11 segment display. Does this need to be?:

oneOf:
  - minItems: 7
    maxItems: 7
  - minItems: 11
    maxItems: 11


> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +
> +patternProperties:
> +  "^.*@[1-7],([1-9]|1[0-6])$":
> +    type: object
> +    $ref: /schemas/leds/common.yaml#
> +    unevaluatedProperties: false
> +    description: |
> +      Properties for a single LED.
> +
> +    properties:
> +      reg:
> +        description: |
> +          1-based grid number, followed by 1-based segment bit number.
> +        maxItems: 1
> +
> +    required:
> +      - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led-controller@0 {
> +            compatible = "titanmec,tm1628";
> +            reg = <0>;
> +            spi-3-wire;
> +            spi-lsb-first;
> +            spi-max-frequency = <500000>;
> +            grid = /bits/ 8 <4 3 2 1>;
> +            segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
> +            #address-cells = <2>;
> +            #size-cells = <0>;
> +
> +            alarmn@5,4 {
> +                reg = <5 4>;
> +                function = LED_FUNCTION_ALARM;
> +            };
> +        };
> +    };
> +...
> -- 
> 2.35.1
> 
> 
>
Heiner Kallweit Feb. 24, 2022, 8:55 p.m. UTC | #2 
On 24.02.2022 21:22, Rob Herring wrote:
> On Wed, Feb 23, 2022 at 06:59:31PM +0100, Heiner Kallweit wrote:
>> Add a YAML schema binding for TM1628 auxdisplay
>> (7/11-segment LED) controller.
>>
>> This patch is partially based on previous work from
>> Andreas Färber <afaerber@suse.de>.
>>
>> Co-Developed-by: Andreas Färber <afaerber@suse.de>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Co-Developed-by: Heiner Kallweit <hkallweit1@gmail.com>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v3:
>> - fix remaining YAML issues
>> - use Co-Developed-by
>> ---
>>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 92 +++++++++++++++++++
>>  1 file changed, 92 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>> new file mode 100644
>> index 000000000..2a1ef692c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>> @@ -0,0 +1,92 @@
>> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Titan Micro Electronics TM1628 LED controller
>> +
>> +maintainers:
>> +  - Andreas Färber <afaerber@suse.de>
>> +  - Heiner Kallweit <hkallweit1@gmail.com>
>> +
>> +allOf:
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: titanmec,tm1628
>> +
>> +  reg:
>> +    maxItems: 1
>> +
> 
>> +  grid:
>> +    description:
>> +      Mapping of display digit position to grid number.
>> +      This implicitly defines the display size.
>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>> +    minItems: 1
>> +    maxItems: 7
>> +
>> +  segment-mapping:
>> +    description:
>> +      Mapping of 7 segment display segments A-G to bit numbers 1-12.
>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>> +    minItems: 7
>> +    maxItems: 7
> 
> Are these properties useful for any 7 segment display or specific to 
> this controller?
> 
Both are controller-specific. E.g. the functionally similar driver
ht16k33 uses different properties.

> The commit msg mentions 11 segment display. Does this need to be?:
> 
> oneOf:
>   - minItems: 7
>     maxItems: 7
>   - minItems: 11
>     maxItems: 11
> 
The controller would be able to drive 11 segments, but the driver
supports 7 segments only (at least for now). Therefore a 11 segment
display can be used, but only the 7 segment part will be active.
All devices with this controller I've seen and heard of have
7 segment displays.

> 
>> +
>> +  "#address-cells":
>> +    const: 2
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +patternProperties:
>> +  "^.*@[1-7],([1-9]|1[0-6])$":
>> +    type: object
>> +    $ref: /schemas/leds/common.yaml#
>> +    unevaluatedProperties: false
>> +    description: |
>> +      Properties for a single LED.
>> +
>> +    properties:
>> +      reg:
>> +        description: |
>> +          1-based grid number, followed by 1-based segment bit number.
>> +        maxItems: 1
>> +
>> +    required:
>> +      - reg
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/leds/common.h>
>> +
>> +    spi {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        led-controller@0 {
>> +            compatible = "titanmec,tm1628";
>> +            reg = <0>;
>> +            spi-3-wire;
>> +            spi-lsb-first;
>> +            spi-max-frequency = <500000>;
>> +            grid = /bits/ 8 <4 3 2 1>;
>> +            segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
>> +            #address-cells = <2>;
>> +            #size-cells = <0>;
>> +
>> +            alarmn@5,4 {
>> +                reg = <5 4>;
>> +                function = LED_FUNCTION_ALARM;
>> +            };
>> +        };
>> +    };
>> +...
>> -- 
>> 2.35.1
>>
>>
>>
Rob Herring Feb. 24, 2022, 9:30 p.m. UTC | #3 
On Thu, Feb 24, 2022 at 2:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 24.02.2022 21:22, Rob Herring wrote:
> > On Wed, Feb 23, 2022 at 06:59:31PM +0100, Heiner Kallweit wrote:
> >> Add a YAML schema binding for TM1628 auxdisplay
> >> (7/11-segment LED) controller.
> >>
> >> This patch is partially based on previous work from
> >> Andreas Färber <afaerber@suse.de>.
> >>
> >> Co-Developed-by: Andreas Färber <afaerber@suse.de>
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> Co-Developed-by: Heiner Kallweit <hkallweit1@gmail.com>
> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> >> ---
> >> v3:
> >> - fix remaining YAML issues
> >> - use Co-Developed-by
> >> ---
> >>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 92 +++++++++++++++++++
> >>  1 file changed, 92 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> >> new file mode 100644
> >> index 000000000..2a1ef692c
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
> >> @@ -0,0 +1,92 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Titan Micro Electronics TM1628 LED controller
> >> +
> >> +maintainers:
> >> +  - Andreas Färber <afaerber@suse.de>
> >> +  - Heiner Kallweit <hkallweit1@gmail.com>
> >> +
> >> +allOf:
> >> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: titanmec,tm1628
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >
> >> +  grid:
> >> +    description:
> >> +      Mapping of display digit position to grid number.
> >> +      This implicitly defines the display size.
> >> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> >> +    minItems: 1
> >> +    maxItems: 7
> >> +
> >> +  segment-mapping:
> >> +    description:
> >> +      Mapping of 7 segment display segments A-G to bit numbers 1-12.
> >> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> >> +    minItems: 7
> >> +    maxItems: 7
> >
> > Are these properties useful for any 7 segment display or specific to
> > this controller?
> >
> Both are controller-specific. E.g. the functionally similar driver
> ht16k33 uses different properties.

Then they both need a vendor prefix.

> > The commit msg mentions 11 segment display. Does this need to be?:
> >
> > oneOf:
> >   - minItems: 7
> >     maxItems: 7
> >   - minItems: 11
> >     maxItems: 11
> >
> The controller would be able to drive 11 segments, but the driver
> supports 7 segments only (at least for now). Therefore a 11 segment
> display can be used, but only the 7 segment part will be active.
> All devices with this controller I've seen and heard of have
> 7 segment displays.

Bindings should be complete, not just what a particular version of a
driver supports. However, if all known h/w is just 7 seg, then it is
fine.

Rob
Heiner Kallweit Feb. 24, 2022, 10:07 p.m. UTC | #4 
On 24.02.2022 22:30, Rob Herring wrote:
> On Thu, Feb 24, 2022 at 2:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 24.02.2022 21:22, Rob Herring wrote:
>>> On Wed, Feb 23, 2022 at 06:59:31PM +0100, Heiner Kallweit wrote:
>>>> Add a YAML schema binding for TM1628 auxdisplay
>>>> (7/11-segment LED) controller.
>>>>
>>>> This patch is partially based on previous work from
>>>> Andreas Färber <afaerber@suse.de>.
>>>>
>>>> Co-Developed-by: Andreas Färber <afaerber@suse.de>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> Co-Developed-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>> v3:
>>>> - fix remaining YAML issues
>>>> - use Co-Developed-by
>>>> ---
>>>>  .../bindings/auxdisplay/titanmec,tm1628.yaml  | 92 +++++++++++++++++++
>>>>  1 file changed, 92 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>>> new file mode 100644
>>>> index 000000000..2a1ef692c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
>>>> @@ -0,0 +1,92 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Titan Micro Electronics TM1628 LED controller
>>>> +
>>>> +maintainers:
>>>> +  - Andreas Färber <afaerber@suse.de>
>>>> +  - Heiner Kallweit <hkallweit1@gmail.com>
>>>> +
>>>> +allOf:
>>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: titanmec,tm1628
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>
>>>> +  grid:
>>>> +    description:
>>>> +      Mapping of display digit position to grid number.
>>>> +      This implicitly defines the display size.
>>>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>>>> +    minItems: 1
>>>> +    maxItems: 7
>>>> +
>>>> +  segment-mapping:
>>>> +    description:
>>>> +      Mapping of 7 segment display segments A-G to bit numbers 1-12.
>>>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>>>> +    minItems: 7
>>>> +    maxItems: 7
>>>
>>> Are these properties useful for any 7 segment display or specific to
>>> this controller?
>>>
>> Both are controller-specific. E.g. the functionally similar driver
>> ht16k33 uses different properties.
> 
> Then they both need a vendor prefix.
> 
OK, will add the prefix. Thanks for the hint.

>>> The commit msg mentions 11 segment display. Does this need to be?:
>>>
>>> oneOf:
>>>   - minItems: 7
>>>     maxItems: 7
>>>   - minItems: 11
>>>     maxItems: 11
>>>
>> The controller would be able to drive 11 segments, but the driver
>> supports 7 segments only (at least for now). Therefore a 11 segment
>> display can be used, but only the 7 segment part will be active.
>> All devices with this controller I've seen and heard of have
>> 7 segment displays.
> 
> Bindings should be complete, not just what a particular version of a
> driver supports. However, if all known h/w is just 7 seg, then it is
> fine.
> 
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
new file mode 100644
index 000000000..2a1ef692c
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/titanmec,tm1628.yaml
@@ -0,0 +1,92 @@ 
+# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/titanmec,tm1628.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Titan Micro Electronics TM1628 LED controller
+
+maintainers:
+  - Andreas Färber <afaerber@suse.de>
+  - Heiner Kallweit <hkallweit1@gmail.com>
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: titanmec,tm1628
+
+  reg:
+    maxItems: 1
+
+  grid:
+    description:
+      Mapping of display digit position to grid number.
+      This implicitly defines the display size.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 1
+    maxItems: 7
+
+  segment-mapping:
+    description:
+      Mapping of 7 segment display segments A-G to bit numbers 1-12.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 7
+    maxItems: 7
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^.*@[1-7],([1-9]|1[0-6])$":
+    type: object
+    $ref: /schemas/leds/common.yaml#
+    unevaluatedProperties: false
+    description: |
+      Properties for a single LED.
+
+    properties:
+      reg:
+        description: |
+          1-based grid number, followed by 1-based segment bit number.
+        maxItems: 1
+
+    required:
+      - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@0 {
+            compatible = "titanmec,tm1628";
+            reg = <0>;
+            spi-3-wire;
+            spi-lsb-first;
+            spi-max-frequency = <500000>;
+            grid = /bits/ 8 <4 3 2 1>;
+            segment-mapping = /bits/ 8 <4 5 6 1 2 3 7>;
+            #address-cells = <2>;
+            #size-cells = <0>;
+
+            alarmn@5,4 {
+                reg = <5 4>;
+                function = LED_FUNCTION_ALARM;
+            };
+        };
+    };
+...