Add initial device tree documentation for Maxim MAX6958/6959.
Signed-off-by: Andy Shevchenko <[email protected]>
---
.../bindings/auxdisplay/maxim,max6959.yaml | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/auxdisplay/maxim,max6959.yaml
diff --git a/Documentation/devicetree/bindings/auxdisplay/maxim,max6959.yaml b/Documentation/devicetree/bindings/auxdisplay/maxim,max6959.yaml
new file mode 100644
index 000000000000..49ce26176797
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/maxim,max6959.yaml
@@ -0,0 +1,35 @@
+# 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 <[email protected]>
+
+properties:
+ compatible:
+ const: maxim,max6959
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ max6959: max6959@38 {
+ compatible = "maxim,max6959";
+ reg = <0x38>;
+ };
+ };
--
2.43.0.rc1.1.gbec44491f096
On Thu, Feb 08, 2024 at 06:58:57PM +0200, Andy Shevchenko wrote:
> Add initial device tree documentation for Maxim MAX6958/6959.
Why "initial"? Are there elements this display that you've not
documented yet?
> +title: MAX6958/6959 7-segment LED display controller with keyscan
> +properties:
> + compatible:
> + const: maxim,max6959
Where's the max6958's compatible? I don't see it in your driver either.
It seems that the max6959 has some interrupt capabilities that are not
available on the max6958, so a dedicated compatible seems suitable to
me.
Cheers,
Conor.
On Thu, Feb 08, 2024 at 05:50:51PM +0000, Conor Dooley wrote:
> On Thu, Feb 08, 2024 at 06:58:57PM +0200, Andy Shevchenko wrote:
> > Add initial device tree documentation for Maxim MAX6958/6959.
>
> Why "initial"? Are there elements this display that you've not
> documented yet?
s/documented/implemented/
There are features of the hardware that may need additional properties.
> > +title: MAX6958/6959 7-segment LED display controller with keyscan
>
> > +properties:
> > + compatible:
> > + const: maxim,max6959
>
> Where's the max6958's compatible? I don't see it in your driver either.
For now, see above, there is no need. Moreover, there is no need at all
as we have autodetection mechanism. I don't see why we should have two
compatible strings just for the sake of having them.
> It seems that the max6959 has some interrupt capabilities that are not
> available on the max6958, so a dedicated compatible seems suitable to
> me.
So, please clarify the DT's p.o.v. on the hardware that can be autodetected.
Do we need to still have different compatible strings? What for? I don't
see the need, sorry for my (silly) questions.
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 08, 2024 at 08:16:56PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 08, 2024 at 05:50:51PM +0000, Conor Dooley wrote:
> > On Thu, Feb 08, 2024 at 06:58:57PM +0200, Andy Shevchenko wrote:
> > > Add initial device tree documentation for Maxim MAX6958/6959.
> >
> > Why "initial"? Are there elements this display that you've not
> > documented yet?
>
> s/documented/implemented/
Oh no, I meant documented. Whether or not you implement support in the
driver doesn't really matter, but you should endeavour to document as
much of the hardware as possible. Certainly simple things like
interrupts.
> There are features of the hardware that may need additional properties.
>
> > > +title: MAX6958/6959 7-segment LED display controller with keyscan
> >
> > > +properties:
> > > + compatible:
> > > + const: maxim,max6959
> >
> > Where's the max6958's compatible? I don't see it in your driver either.
>
> For now, see above, there is no need.
I don't know what I am supposed to see above.
> Moreover, there is no need at all
> as we have autodetection mechanism. I don't see why we should have two
> compatible strings just for the sake of having them.
>
> > It seems that the max6959 has some interrupt capabilities that are not
> > available on the max6958, so a dedicated compatible seems suitable to
> > me.
>
> So, please clarify the DT's p.o.v. on the hardware that can be autodetected.
> Do we need to still have different compatible strings? What for? I don't
> see the need, sorry for my (silly) questions.
If there's an auto-detection mechanism, which is a valid justification,
you need to put it in the commit message. If you don't it just looks
like you're missing a compatible.
The advantage of a dedicated compatible is restricting the properties
that would only apply to either the 6958 or 6959 to just that device.
You've only partially documented these devices though, so it is not
clear how much of a divergence there'd actually be.
cheers,
Conor
On 08/02/2024 18:50, Conor Dooley wrote:
> On Thu, Feb 08, 2024 at 06:58:57PM +0200, Andy Shevchenko wrote:
>> Add initial device tree documentation for Maxim MAX6958/6959.
>
> Why "initial"? Are there elements this display that you've not
> documented yet?
>
Why there are multiple versions of this patchset? This leads to
different reviewers commenting in different patchsets?
Andy,
please implement feedback given to all your patchsets...
Best regards,
Krzysztof