| Message ID | 20230530173000.3060865-20-dave.stevenson@raspberrypi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | imx258 improvements series | expand |
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@raspberrypi.com> > --- > .../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. > 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. Cheers, Conor.
Hi Conor Thanks for the incredibly speedy review. On Tue, 30 May 2023 at 18:39, Conor Dooley <conor@kernel.org> 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@raspberrypi.com> > > --- > > .../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.
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@kernel.org> 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@raspberrypi.com> > > > --- > > > .../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.
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. properties: compatible: - const: sony,imx258 + oneOf: + - enum: + - sony,imx258 + - sony,imx258-pdaf assigned-clocks: true assigned-clock-parents: true
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@raspberrypi.com> --- .../devicetree/bindings/media/i2c/sony,imx258.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)