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

Krzysztof Kozlowski posted 1 patch 2 years ago
There is a newer version of this series
.../devicetree/bindings/media/i2c/ov8856.yaml | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
[PATCH v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver
Posted by Krzysztof Kozlowski 2 years ago
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.
-            maxItems: 2
             items:
-              enum: [ 360000000, 180000000 ]
+              enum: [ 1440000000, 720000000, 360000000, 180000000 ]
 
         required:
           - link-frequencies
-- 
2.34.1
Re: [PATCH v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver
Posted by Sakari Ailus 2 years ago
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

-- 
Regards,

Sakari Ailus
Re: [PATCH v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver
Posted by Krzysztof Kozlowski 2 years ago
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
Re: [PATCH v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver
Posted by Sakari Ailus 2 years ago
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.

-- 
Sakari Ailus
Re: [PATCH v2] media: dt-bindings: ov8856: decouple lanes and link frequency from driver
Posted by Conor Dooley 2 years ago
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
>