Re: [PATCH v2 1/5] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
From: Rob Herring
Date: Wed Oct 26 2022 - 12:57:00 EST
On Sat, Oct 15, 2022 at 12:54 AM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> On Fri, Oct 14, 2022 at 04:40:29PM -0500, Rob Herring wrote:
> > On Fri, Oct 14, 2022 at 10:27:53PM +0100, Lad, Prabhakar wrote:
> > > On Fri, Oct 14, 2022 at 10:05 PM Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> > > > On Fri, Oct 14, 2022 at 1:35 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > > > >
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > > >
> > > > > Convert the simple OV5645 Device Tree binding to json-schema.
> > > > >
> > > > > The previous binding marked the below properties as required which was a
> > > > > driver requirement and not the device requirement so just drop them from
> > > > > the required list during the conversion.
> > > > > - clock-frequency
> > > > > - enable-gpios
> > > > > - reset-gpios
> > > > >
> > > > > Also drop the "clock-names" property as we have a single clock source for
> > > > > the sensor and the driver has been updated to drop the clk referencing by
> > > > > name.
> > > >
> > > > Driver requirements are the ABI!
> > > >
> > > > This breaks a kernel without the driver change and a DTB that has
> > > > dropped the properties.
> > > >
> > > I already have a patch for the driver [0] which I missed to include
> > > along with the series.
> >
> > You completely miss the point. Read the first sentence again. Changing
> > driver requirements changes the ABI.
> >
> > This breaks the ABI. The driver patch does not help that.
>
> I'm not following you here. If the DT binding makes a mandatory property
> optional, it doesn't break any existing platform. The only thing that
> would not work is a new DT that doesn't contain the now optional
> property combined with an older driver that makes it required. That's
> not a regression, as it would be a *new* DT.
>
> > > > Also, with 'clock-names' dropped, you've just introduced a bunch of
> > > > warnings on other people's platforms. Are you going to 'fix' all of
> > > > them?
> > > >
> > > Yes I will fix them, once the patch driver patch [0] is merged in.
> >
> > Why? You are just making extra work. We have enough warnings as-is to
> > fix.
>
> I agree that a DT binding change should patch all in-tree DTS to avoid
> introducing new warnings.
That is not what I was saying. Why not just keep 'clock-names' and go
spend the DTS fixing time fixing some other warnings that we already
have. Also, there is no requirement that converting bindings also fix
DTS files. The only wish is that any warnings we do see are ones
deemed needing to be fixed in the DTS file.
Anyways, there's patches now for the new warnings, so nevermind on this one.
Rob