Hi Conor
Thanks for the incredibly speedy review.
On Tue, 30 May 2023 at 18:39, Conor Dooley <conor@xxxxxxxxxx> wrote:
>
> Hey Dave,
>
> On Tue, May 30, 2023 at 06:29:58PM +0100, Dave Stevenson wrote:
> > There are a number of variants of the imx258 modules that can not
> > be differentiated at runtime, so add compatible strings for them.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > ---
> > .../devicetree/bindings/media/i2c/sony,imx258.yaml | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > index bee61a443b23..3415b26b5991 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > @@ -14,10 +14,15 @@ description: |-
> > type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > is programmable through I2C interface. Image data is sent through MIPI
> > CSI-2.
> > + There are a number of variants of the sensor which cannot be detected at
> > + runtime, so multiple compatible strings are required to differentiate these.
>
> This is implied by having several compatibles.
I'm happy to drop it, just that I've seen a number of media bindings
that had debate on why extra compatible strings were required.
> > properties:
> > compatible:
> > - const: sony,imx258
> > + oneOf:
> > + - enum:
> > + - sony,imx258
> > + - sony,imx258-pdaf
>
> Why not just
> properties:
> compatible:
> enum:
> ?
> I don't see other patches anding more complex compatibles (or they've
> not arrived yet) so it doesn't appear to be avoiding churn.
I'll freely admit that DT bindings are a black art to me, so I was
following sony,imx290.yaml [1].
properties:
compatible:
oneOf:
- enum:
- sony,imx290lqr # Colour
- sony,imx290llr # Monochrome
- sony,imx327lqr # Colour
- const: sony,imx290
deprecated: true
Looking again at that case, I assume the oneOf is selecting between
the enum and the const? Seeing as we don't have the const, I guess we
can drop the "oneOf:"
Thanks for your help.
Dave
[1] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/i2c/sony%2Cimx290.yaml#L27-L34
> Cheers,
> Conor.
