diff mbox series

[v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver

Message ID 20231207142356.100453-1-krzysztof.kozlowski@linaro.org
State New
Headers show
Series [v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver | expand

Commit Message

Krzysztof Kozlowski Dec. 7, 2023, 2:23 p.m. UTC
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(-)

Comments

Conor Dooley Dec. 7, 2023, 5:15 p.m. UTC | #1
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
>
Sakari Ailus Dec. 8, 2023, 6:07 p.m. UTC | #2
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
Krzysztof Kozlowski Dec. 8, 2023, 7:37 p.m. UTC | #3
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
Sakari Ailus Dec. 8, 2023, 7:47 p.m. UTC | #4
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 mbox series

Patch

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