| Message ID | 20231207142356.100453-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 31 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, Dec 07, 2023 at 03:23:56PM +0100, Krzysztof Kozlowski wrote: > The data lanes and link frequency were set to match exiting Linux driver > limitations, however bindings should be independent of chosen Linux > driver support. > > Decouple these properties from the driver to match what is actually > supported by the hardware. > > This also fixes DTS example: > > ov8856.example.dtb: camera@10: port:endpoint:link-frequencies:0: [360000000] is too short > > Fixes: 066a94e28a23 ("media: dt-bindings: media: Use graph and video-interfaces schemas") > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Changes in v2: > 1. Rework approach: decouple bindings from driver instead of fixing > DTS example (Sakari) Acked-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor. > --- > .../devicetree/bindings/media/i2c/ov8856.yaml | 21 +++++++++++-------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > index 57f5e48fd8e0..71102a71cf81 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > @@ -67,19 +67,22 @@ properties: > > properties: > data-lanes: > - description: |- > - The driver only supports four-lane operation. > - items: > - - const: 1 > - - const: 2 > - - const: 3 > - - const: 4 > + oneOf: > + - items: > + - const: 1 > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > > link-frequencies: > description: Frequencies listed are driver, not h/w limitations. > - maxItems: 2 > items: > - enum: [ 360000000, 180000000 ] > + enum: [ 1440000000, 720000000, 360000000, 180000000 ] > > required: > - link-frequencies > -- > 2.34.1 >
Hi Krzysztof, Thanks for the update. On Thu, Dec 07, 2023 at 03:23:56PM +0100, Krzysztof Kozlowski wrote: > The data lanes and link frequency were set to match exiting Linux driver > limitations, however bindings should be independent of chosen Linux > driver support. > > Decouple these properties from the driver to match what is actually > supported by the hardware. > > This also fixes DTS example: > > ov8856.example.dtb: camera@10: port:endpoint:link-frequencies:0: [360000000] is too short > > Fixes: 066a94e28a23 ("media: dt-bindings: media: Use graph and video-interfaces schemas") > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Changes in v2: > 1. Rework approach: decouple bindings from driver instead of fixing > DTS example (Sakari) > --- > .../devicetree/bindings/media/i2c/ov8856.yaml | 21 +++++++++++-------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > index 57f5e48fd8e0..71102a71cf81 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > @@ -67,19 +67,22 @@ properties: > > properties: > data-lanes: > - description: |- > - The driver only supports four-lane operation. > - items: > - - const: 1 > - - const: 2 > - - const: 3 > - - const: 4 > + oneOf: > + - items: > + - const: 1 > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > > link-frequencies: > description: Frequencies listed are driver, not h/w limitations. This should be dropped, too. > - maxItems: 2 > items: > - enum: [ 360000000, 180000000 ] > + enum: [ 1440000000, 720000000, 360000000, 180000000 ] These frequencies are listed in the datasheet but they're just an example---the sensor hardware isn't limited to these, the resulting frequency on the CSI-2 bus is simply up to the external clock frequency and PLL configuration. I'd remove the values here altogether. > > required: > - link-frequencies
On 08/12/2023 19:07, Sakari Ailus wrote: > Hi Krzysztof, > > Thanks for the update. > > On Thu, Dec 07, 2023 at 03:23:56PM +0100, Krzysztof Kozlowski wrote: >> The data lanes and link frequency were set to match exiting Linux driver >> limitations, however bindings should be independent of chosen Linux >> driver support. >> >> Decouple these properties from the driver to match what is actually >> supported by the hardware. >> >> This also fixes DTS example: >> >> ov8856.example.dtb: camera@10: port:endpoint:link-frequencies:0: [360000000] is too short >> >> Fixes: 066a94e28a23 ("media: dt-bindings: media: Use graph and video-interfaces schemas") >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> --- >> >> Changes in v2: >> 1. Rework approach: decouple bindings from driver instead of fixing >> DTS example (Sakari) >> --- >> .../devicetree/bindings/media/i2c/ov8856.yaml | 21 +++++++++++-------- >> 1 file changed, 12 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >> index 57f5e48fd8e0..71102a71cf81 100644 >> --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >> +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >> @@ -67,19 +67,22 @@ properties: >> >> properties: >> data-lanes: >> - description: |- >> - The driver only supports four-lane operation. >> - items: >> - - const: 1 >> - - const: 2 >> - - const: 3 >> - - const: 4 >> + oneOf: >> + - items: >> + - const: 1 >> + - items: >> + - const: 1 >> + - const: 2 >> + - items: >> + - const: 1 >> + - const: 2 >> + - const: 3 >> + - const: 4 >> >> link-frequencies: >> description: Frequencies listed are driver, not h/w limitations. > > This should be dropped, too. Ack, I forgot. > >> - maxItems: 2 >> items: >> - enum: [ 360000000, 180000000 ] >> + enum: [ 1440000000, 720000000, 360000000, 180000000 ] > > These frequencies are listed in the datasheet but they're just an > example---the sensor hardware isn't limited to these, the resulting > frequency on the CSI-2 bus is simply up to the external clock frequency and > PLL configuration. I'd remove the values here altogether. Hm, are you sure? Isn't it quite difficult to program device to any frequency? But if that's not the case here, I can drop it. Best regards, Krzysztof
On Fri, Dec 08, 2023 at 08:37:10PM +0100, Krzysztof Kozlowski wrote: > On 08/12/2023 19:07, Sakari Ailus wrote: > > Hi Krzysztof, > > > > Thanks for the update. > > > > On Thu, Dec 07, 2023 at 03:23:56PM +0100, Krzysztof Kozlowski wrote: > >> The data lanes and link frequency were set to match exiting Linux driver > >> limitations, however bindings should be independent of chosen Linux > >> driver support. > >> > >> Decouple these properties from the driver to match what is actually > >> supported by the hardware. > >> > >> This also fixes DTS example: > >> > >> ov8856.example.dtb: camera@10: port:endpoint:link-frequencies:0: [360000000] is too short > >> > >> Fixes: 066a94e28a23 ("media: dt-bindings: media: Use graph and video-interfaces schemas") > >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> > >> --- > >> > >> Changes in v2: > >> 1. Rework approach: decouple bindings from driver instead of fixing > >> DTS example (Sakari) > >> --- > >> .../devicetree/bindings/media/i2c/ov8856.yaml | 21 +++++++++++-------- > >> 1 file changed, 12 insertions(+), 9 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > >> index 57f5e48fd8e0..71102a71cf81 100644 > >> --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > >> +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > >> @@ -67,19 +67,22 @@ properties: > >> > >> properties: > >> data-lanes: > >> - description: |- > >> - The driver only supports four-lane operation. > >> - items: > >> - - const: 1 > >> - - const: 2 > >> - - const: 3 > >> - - const: 4 > >> + oneOf: > >> + - items: > >> + - const: 1 > >> + - items: > >> + - const: 1 > >> + - const: 2 > >> + - items: > >> + - const: 1 > >> + - const: 2 > >> + - const: 3 > >> + - const: 4 > >> > >> link-frequencies: > >> description: Frequencies listed are driver, not h/w limitations. > > > > This should be dropped, too. > > Ack, I forgot. > > > > >> - maxItems: 2 > >> items: > >> - enum: [ 360000000, 180000000 ] > >> + enum: [ 1440000000, 720000000, 360000000, 180000000 ] > > > > These frequencies are listed in the datasheet but they're just an > > example---the sensor hardware isn't limited to these, the resulting > > frequency on the CSI-2 bus is simply up to the external clock frequency and > > PLL configuration. I'd remove the values here altogether. > > Hm, are you sure? Isn't it quite difficult to program device to any > frequency? But if that's not the case here, I can drop it. The driver doesn't currently do that, no, but that doesn't mean the hardware wouldn't support that. There are a few sensor drivers that calculate the PLL configuration, such as ccs and ov5640. I'd drop it as it's indeed a driver, not a device limitation.
diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml index 57f5e48fd8e0..71102a71cf81 100644 --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml @@ -67,19 +67,22 @@ properties: properties: data-lanes: - description: |- - The driver only supports four-lane operation. - items: - - const: 1 - - const: 2 - - const: 3 - - const: 4 + oneOf: + - items: + - const: 1 + - items: + - const: 1 + - const: 2 + - items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 link-frequencies: description: Frequencies listed are driver, not h/w limitations. - maxItems: 2 items: - enum: [ 360000000, 180000000 ] + enum: [ 1440000000, 720000000, 360000000, 180000000 ] required: - link-frequencies
The data lanes and link frequency were set to match exiting Linux driver limitations, however bindings should be independent of chosen Linux driver support. Decouple these properties from the driver to match what is actually supported by the hardware. This also fixes DTS example: ov8856.example.dtb: camera@10: port:endpoint:link-frequencies:0: [360000000] is too short Fixes: 066a94e28a23 ("media: dt-bindings: media: Use graph and video-interfaces schemas") Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Changes in v2: 1. Rework approach: decouple bindings from driver instead of fixing DTS example (Sakari) --- .../devicetree/bindings/media/i2c/ov8856.yaml | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-)