Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

From: Sakari Ailus <sakari.ailus@iki.fi>
To: Rob Herring <robh@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Daniel Scally <djrscally@gmail.com>,
	Dafna Hirschfeld <dafna@fastmail.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Tommaso Merciai <tommaso.merciai@amarulasolutions.com>,
	Helen Koike <helen.koike@collabora.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding
Date: Tue, 1 Aug 2023 14:06:01 +0000	[thread overview]
Message-ID: <ZMkRSfAwcLQIKkHL@valkosipuli.retiisi.eu> (raw)
In-Reply-To: <CAL_JsqK4A9vpUiP3sCz9+hAHitcsB9NYMMy05n4=1hn7NvjOtg@mail.gmail.com>

Hi Rob,

On Tue, Aug 01, 2023 at 07:45:08AM -0600, Rob Herring wrote:
> On Mon, Jul 31, 2023 at 5:21 AM Sakari Ailus <sakari.ailus@iki.fi> wrote:
> >
> > Hi Rob,
> >
> > On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
> > > The OV5695 binding is almost the same as the OV5693 binding. The only
> > > difference is 'clock-names' is defined for OV5695. However, the lack of
> > > clock-names is an omission as the Linux OV5693 driver expects the same
> > > 'xvclk' clock name.
> > >
> > > 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
> > > from being required. Expressing it conditionally would be ugly. It
> > > shouldn't really be required either as the driver only supports 1
> > > frequency anyways.
> >
> > The correct way to address this would appear to be to add link-frequencies
> > for both of these devices. I think I've seen one or two sensors of this
> > class (raw, CSI-2/parallel, external clock etc.) with link frequencies
> > documented as "fixed" --- which is probably a documentation issue more than
> > anything else.
> 
> link-frequencies is already supported. It's just a question of being
> required or not. Adding a property as required is an ABI break (if the
> OS starts requiring the property).

Currently the ov5693 driver requires a link-frequencies property, the
ov5695 driver doesn't care.

In this case the backwards compatible way would be to make it optional for
ov5695 and if it exists, then allow only those frequencies. It's a fairly
trivial driver, it only supports a single link frequency at the moment (as
well as a single external clock frequency).

At least link-frequencies should continue to be required for ov5693.

-- 
Regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel