Message-ID: <20250916215756.GA4190660-robh@kernel.org>
Date: Tue, 16 Sep 2025 16:57:56 -0500
From: Rob Herring <robh@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Krzysztof Kozlowski <krzk@...nel.org>,
	Guoniu Zhou <guoniu.zhou@....com>,
	Rui Miguel Silva <rmfrfs@...il.com>,
	Martin Kepplinger <martink@...teo.de>,
	Purism Kernel Team <kernel@...i.sm>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Philipp Zabel <p.zabel@...gutronix.de>, linux-media@...r.kernel.org,
	devicetree@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/4] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add
 i.MX8ULP compatible string

On Thu, Sep 04, 2025 at 10:49:48AM -0400, Frank Li wrote:
> On Wed, Sep 03, 2025 at 09:21:43PM +0200, Laurent Pinchart wrote:
> > On Tue, Sep 02, 2025 at 05:53:39PM +0200, Krzysztof Kozlowski wrote:
> > > On 02/09/2025 14:35, Laurent Pinchart wrote:
> > > > On Tue, Sep 02, 2025 at 02:26:53PM +0200, Krzysztof Kozlowski wrote:
> > > >> On 02/09/2025 10:35, Laurent Pinchart wrote:
> > > >>>>>>          compatible:
> > > >>>>>>            contains:
> > > >>>>>>              enum:
> > > >>>>>> -              - fsl,imx8qxp-mipi-csi2
> > > >>>>>> +              - fsl,imx8ulp-mipi-csi2
> > > >>>>>> +    then:
> > > >>>>>> +      properties:
> > > >>>>>> +        reg:
> > > >>>>>> +          minItems: 2
> > > >>>>>> +        resets:
> > > >>>>>> +          minItems: 2
> > > >>>>>> +          maxItems: 2
> > > >>>>>> +        clocks:
> > > >>>>>> +          minItems: 4
> > > >>>>>> +        clock-names:
> > > >>>>>> +          minItems: 4
> > > >>>>>
> > > >>>>> But according to this, the ULP version requires more clocks than the QXP
> > > >>>>> version.
> > > >>>>
> > > >>>> If only clock number difference, generally, it is still compatible and can
> > > >>>> be fallback, especialy driver use devm_bulk_clk_get_all().
> > > >>>
> > > >>> That's a driver-specific implementation decision, so I don't think it
> > > >>> should be taken into account to decide on compatibility.
> > > >>
> > > >> The clock inputs do not restrict compatibility. If Linux can use
> > > >> fallback to bind and operate properly, then it's a strong indication
> > > >> devices are compatible.
> > > >>
> > > >> Imagine exactly the same registers, so same programming interface, but
> > > >> one device takes one more clock which just needs to be enabled through
> > > >> its lifetime. Such devices are fully compatible, even though clock
> > > >> inputs differ.
> > > >
> > > > That's only the case if someone enables the clock, isn't it ? From a DT
> > > > binding point of view, how can we know that the extra clock will be
> > >
> > > We talk about software using the binding in this particular case. Can
> > > the software use fallback? Yes, it can.
> >
> > The Linux kernel driver, in its current implementation, can, yes. No
> > disagreement about that.
> >
> > > > enabled by a component separate from the driver (in this case by the
> > > > fact that the devm_bulk_clk_get_all() function gets all clocks) ?
> > >
> > > If you go that way, only 100% identical devices are compatible.
> > >
> > > >> I also wanted to express exactly that case on my slides from OSSE -
> > > >> slide 28:
> > > >> https://osseu2025.sched.com/event/25Vsl/dts-101-from-roots-to-trees-aka-devicetree-for-beginners-krzysztof-kozlowski-linaro
> > > >
> > > > Quoting that slide, you wrote
> > > >
> > > > "Two devices are compatible when the new device works with Linux drivers
> > > > bound via fallback (old) compatible".
> > > >
> > > > That is clearly the case here for the existing *Linux* driver. But what
> > > > if the driver called devm_bulkd_clk_get() with a device-specific list of
> > > > clocks ? Or what if the same DT bindings are used on an OS that has no
> > > > clk_get_all() equivalent ? This is my concern with declaring those two
> > > > devices as compatible: they may be from the point of view of the current
> > > > implementation of the corresponding Linux kernel driver, but DT bindings
> > > > are not Linux-specific.
> > >
> > > It seems you think of compatibility as new device is compatible with old
> > > kernel, e.g. one not requesting that clock. We don't talk about such case.
> >
> > No no, I'm considering compatibility in the same sense as you. Sorry if
> > that wasn't clear.
> >
> > > > Or do DT bindings assume that drivers have to always enable all clocks
> > > > declared in DT, even if they don't know what those clocks are ? That
> > > > seems error-prone, in quite a few cases drivers need to handle separate
> > > > clocks in a device-specific way, with for instance a particular
> > > > ordering, preventing them from using devm_bulk_clk_get_all(). If all
> > > > drivers are required to manage all clocks declared in DT, this would get
> > > > messy quite quickly.
> > >
> > > I don't really want to dive into such specifics, because it is
> > > impossible to create a generic rule of out.
> >
> > We're on the same page there :-)
> >
> > Compatible strings model compatibility with software. As DT bindings are
> > not OS-specific, they should be designed based on the concept of a
> > driver, and not on a particular driver implementation. As a conceptual
> > generic driver can't be precisely defined, we will always have edge
> > cases.
> >
> > In this specific case, I think that devm_bulk_clk_get_all() is too much
> > of a Linux-specific concept to consider that devices with different
> > clocks are compatible. Even considering Linux only, a driver that needs
> > to handle at least one of the clocks in a particular way (for instance
> > to guarantee a device-specific clock sequencing requirement, or to
> > retrieve or set the frequency of a particular clock) will need to get
> > clocks by their names, making fully generic handling of all clocks not
> > possible.
> 
> New added clocks is simple clock, needn't specific handler. Only need
> enable at runtime resume.
> 
> Back compatible is hard to decouple with driver's implement 100%.
> 
> Compatible string C1 have clock A, B, C
> Compatible string C2 have clock A, B, C, D, E, F
> 
> A, B, C is common for both C1 and C2, which need special handle.
> D, E, F is simple enable at probe or runtime resume.

I think it would only be backwards compatible if clocks D, E, and F were 
entirely optional and could be left unmanaged.

> 
> Can C1 be back compatible C2 (assume all the same except only D, E, F
> clock difference)? It is always depend on drivers' implemment.
> 
> Add back compatible have NOT bad impact for drivers and bindings. Although
> back compatible "C1", "C2", driver still use can use C1 firstly to do
> special process.
> 
> 
> > For such drivers, difference in clocks will preclude
> > considering two devices as compatible.
> >
> > As this is somewhat of an edge case someone will need to make a
> > decision, and I won't fight tooth and nail over it.
> 
> Agree. Need a guide line. My opinion is
> 
> back compatible if there are no new drvdata (pltdata) in drivers.
> Needn't back compatible if need add new item in drvdata(pltdata) in drivers.

That's a good indication, but not 100%.

If the chip overall needs kernel changes anyways, then backwards 
compatibility for 1 block doesn't really matter so much other than 1 
less patch. 

Rob

Powered by blists - more mailing lists