| Message ID | 20240827131841.629920-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: staging: max96712: Add support for MAX96724 | expand |
On 27/08/2024 15:18, Niklas Söderlund wrote: > The MAX96712 and MAX96724 are almost identical and can be supported by > the same driver, add a compatible for MAX96724. The driver statement in this context is meaningless. You did not make them compatible so what does it matter? > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > Acked-by: Conor Dooley <conor.dooley@microchip.com> > --- > * Changes since v1 > - Group in series together with driver change. > --- > .../devicetree/bindings/media/i2c/maxim,max96712.yaml | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml > index 6c72e77b927c..26f85151afbd 100644 > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml > @@ -25,7 +25,10 @@ description: | > > properties: > compatible: > - const: maxim,max96712 > + items: > + - enum: > + - maxim,max96712 > + - maxim,max96724 Driver change tells these are compatible and version is detectable. Express it in the binding with fallback or explain in commit msg why they are not compatible. Best regards, Krzysztof
Hi Krzysztof, Thanks for your feedback. On 2024-08-27 16:53:20 +0200, Krzysztof Kozlowski wrote: > On 27/08/2024 15:18, Niklas Söderlund wrote: > > The MAX96712 and MAX96724 are almost identical and can be supported by > > the same driver, add a compatible for MAX96724. > > The driver statement in this context is meaningless. You did not make > them compatible so what does it matter? Agreed, this commit message can be improved. > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > Acked-by: Conor Dooley <conor.dooley@microchip.com> > > --- > > * Changes since v1 > > - Group in series together with driver change. > > --- > > .../devicetree/bindings/media/i2c/maxim,max96712.yaml | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml > > index 6c72e77b927c..26f85151afbd 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml > > @@ -25,7 +25,10 @@ description: | > > > > properties: > > compatible: > > - const: maxim,max96712 > > + items: > > + - enum: > > + - maxim,max96712 > > + - maxim,max96724 > > Driver change tells these are compatible and version is detectable. > Express it in the binding with fallback or explain in commit msg why > they are not compatible. It is, and as you point out the commit message can be improved. It should have made it clear that what is supported by the staging driver (test pattern generator) is similar for the two devices, the full device have larger differences. Before this driver can move out of staging the full GMSL side of things needs to be added and there they differ more and will need different bindings as one device have more PHYs then the other for example. Run time detecting is not enough to be able to verify that in DTS files. I will fix this for the next version. > > > Best regards, > Krzysztof >
On 27/08/2024 20:03, Niklas Söderlund wrote: > Hi Krzysztof, > > Thanks for your feedback. > > On 2024-08-27 16:53:20 +0200, Krzysztof Kozlowski wrote: >> On 27/08/2024 15:18, Niklas Söderlund wrote: >>> The MAX96712 and MAX96724 are almost identical and can be supported by >>> the same driver, add a compatible for MAX96724. >> >> The driver statement in this context is meaningless. You did not make >> them compatible so what does it matter? > > Agreed, this commit message can be improved. > >> >>> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> >>> Acked-by: Conor Dooley <conor.dooley@microchip.com> >>> --- >>> * Changes since v1 >>> - Group in series together with driver change. >>> --- >>> .../devicetree/bindings/media/i2c/maxim,max96712.yaml | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml >>> index 6c72e77b927c..26f85151afbd 100644 >>> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml >>> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml >>> @@ -25,7 +25,10 @@ description: | >>> >>> properties: >>> compatible: >>> - const: maxim,max96712 >>> + items: >>> + - enum: >>> + - maxim,max96712 >>> + - maxim,max96724 >> >> Driver change tells these are compatible and version is detectable. >> Express it in the binding with fallback or explain in commit msg why >> they are not compatible. > > It is, and as you point out the commit message can be improved. It > should have made it clear that what is supported by the staging driver > (test pattern generator) is similar for the two devices, the full device > have larger differences. OK. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml index 6c72e77b927c..26f85151afbd 100644 --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml @@ -25,7 +25,10 @@ description: | properties: compatible: - const: maxim,max96712 + items: + - enum: + - maxim,max96712 + - maxim,max96724 reg: description: I2C device address