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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 8 Feb 2024 20:44:11 +0000
From: Conor Dooley <conor@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Miguel Ojeda <ojeda@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Robin van der Gracht <robin@...tonic.nl>,
	Paul Burton <paulburton@...nel.org>,
	Geert Uytterhoeven <geert+renesas@...der.be>
Subject: Re: [PATCH v1 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959

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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)