Received-SPF: pass (google.com: domain of linux-kernel+bounces-58673-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 8 Feb 2024 20:44:11 +0000
From: Conor Dooley <conor@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Miguel Ojeda <ojeda@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Robin van der Gracht <robin@protonic.nl>,
	Paul Burton <paulburton@kernel.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>
Subject: Re: [PATCH v1 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959
Message-ID: <20240208-morbidly-submerge-fcafe85bffc9@spud>
References: <20240208165937.2221193-1-andriy.shevchenko@linux.intel.com>
 <20240208165937.2221193-15-andriy.shevchenko@linux.intel.com>
 <20240208-chute-bacteria-75425bd34dc9@spud>
 <ZcUamOqKUuA-ahRY@smile.fi.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="sxJRa8heXZCzXt88"
Content-Disposition: inline
In-Reply-To: <ZcUamOqKUuA-ahRY@smile.fi.intel.com>


--sxJRa8heXZCzXt88
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
> >=20
> > Why "initial"? Are there elements this display that you've not
> > documented yet?=20
>=20
> 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.
>=20
> > > +title: MAX6958/6959 7-segment LED display controller with keyscan
> >=20
> > > +properties:
> > > +  compatible:
> > > +    const: maxim,max6959
> >=20
> > Where's the max6958's compatible? I don't see it in your driver either.
>=20
> For now, see above, there is no need.=20

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.
>=20
> > It seems that the max6959 has some interrupt capabilities that are not
> > available on the max6958, so a dedicated compatible seems suitable to
> > me.
>=20
> So, please clarify the DT's p.o.v. on the hardware that can be autodetect=
ed.
> 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

--sxJRa8heXZCzXt88
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZcU9GwAKCRB4tDGHoIJi
0hs+AQDCko8IQ57nsKd0xBsoOxTDO7hnzRgkrEyfG9eQFeWRpQEA78y+k53qmS8I
JdySkG1FA4lAUXaiepc4F+RFVXbhYgo=
=HrI2
-----END PGP SIGNATURE-----

--sxJRa8heXZCzXt88--