2023-12-07 14:25:35

by Krzysztof Kozlowski

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

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 <[email protected]>

---

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


2023-12-07 17:16:10

by Conor Dooley

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

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 <[email protected]>
>
> ---
>
> Changes in v2:
> 1. Rework approach: decouple bindings from driver instead of fixing
> DTS example (Sakari)

Acked-by: Conor Dooley <[email protected]>

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
>


Attachments:
(No filename) (2.27 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-08 18:07:31

by Sakari Ailus

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

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 <[email protected]>
>
> ---
>
> 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

2023-12-08 19:37:32

by Krzysztof Kozlowski

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

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 <[email protected]>
>>
>> ---
>>
>> 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

2023-12-08 19:48:06

by Sakari Ailus

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

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 <[email protected]>
> >>
> >> ---
> >>
> >> 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