On Tue, May 30, 2023 at 06:48:44PM +0100, Dave Stevenson wrote: > Thanks for the incredibly speedy review. Just happened to change mailboxes right as it arrived ;) > 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. If there were no non-detectable differences, then there should be a fallback compatible i.e. compatible = "sony,imx666-foo", "sony,imx666"; Maybe Laurent will come in here and scream at me, but I don't think the pattern should be propagated. > > > 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? Bingo! > Seeing as we don't have the const, I guess we > can drop the "oneOf:" Cheers, Conor.
Attachment:
signature.asc
Description: PGP signature
