| Message ID | 20231205084835.15871-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | media: dt-bindings: ov8856: add missing second link frequency in example | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 8 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Hi Krzysztof, Thanks for the patch. On Tue, Dec 05, 2023 at 09:48:35AM +0100, Krzysztof Kozlowski wrote: > Bindings and Linux driver require two link frequencies, so correct the > 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> > --- > Documentation/devicetree/bindings/media/i2c/ov8856.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > index 57f5e48fd8e0..bd1a55d767e7 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > @@ -126,7 +126,7 @@ examples: > wcam_out: endpoint { > remote-endpoint = <&mipi_in_wcam>; > data-lanes = <1 2 3 4>; > - link-frequencies = /bits/ 64 <360000000>; > + link-frequencies = /bits/ 64 <360000000 180000000>; There indeed seems to be a problem with the example as far as the bindings are concerned but the primary issue seems to be in the bindings. Both of these frequencies have significance from driver point of view only while the device itself supports a (wider) range. How about removing maxItems and items from the bindings instead? > }; > }; > };
On Tue, Dec 05, 2023 at 11:01:41AM +0000, Sakari Ailus wrote: > Hi Krzysztof, > > Thanks for the patch. > > On Tue, Dec 05, 2023 at 09:48:35AM +0100, Krzysztof Kozlowski wrote: > > Bindings and Linux driver require two link frequencies, so correct the > > 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> > > --- > > Documentation/devicetree/bindings/media/i2c/ov8856.yaml | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > > index 57f5e48fd8e0..bd1a55d767e7 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml > > @@ -126,7 +126,7 @@ examples: > > wcam_out: endpoint { > > remote-endpoint = <&mipi_in_wcam>; > > data-lanes = <1 2 3 4>; > > - link-frequencies = /bits/ 64 <360000000>; > > + link-frequencies = /bits/ 64 <360000000 180000000>; > > There indeed seems to be a problem with the example as far as the bindings > are concerned but the primary issue seems to be in the bindings. Both of > these frequencies have significance from driver point of view only while > the device itself supports a (wider) range. > > How about removing maxItems and items from the bindings instead? There's also a similar issue with lanes: 1, 2 and 4 are supported.
On 05/12/2023 12:01, Sakari Ailus wrote: > Hi Krzysztof, > > Thanks for the patch. > > On Tue, Dec 05, 2023 at 09:48:35AM +0100, Krzysztof Kozlowski wrote: >> Bindings and Linux driver require two link frequencies, so correct the >> 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> >> --- >> Documentation/devicetree/bindings/media/i2c/ov8856.yaml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >> index 57f5e48fd8e0..bd1a55d767e7 100644 >> --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >> +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >> @@ -126,7 +126,7 @@ examples: >> wcam_out: endpoint { >> remote-endpoint = <&mipi_in_wcam>; >> data-lanes = <1 2 3 4>; >> - link-frequencies = /bits/ 64 <360000000>; >> + link-frequencies = /bits/ 64 <360000000 180000000>; > > There indeed seems to be a problem with the example as far as the bindings > are concerned but the primary issue seems to be in the bindings. Both of > these frequencies have significance from driver point of view only while > the device itself supports a (wider) range. > > How about removing maxItems and items from the bindings instead? Yeah, it makes sense. Best regards, Krzysztof
On 05/12/2023 12:43, Sakari Ailus wrote: > On Tue, Dec 05, 2023 at 11:01:41AM +0000, Sakari Ailus wrote: >> Hi Krzysztof, >> >> Thanks for the patch. >> >> On Tue, Dec 05, 2023 at 09:48:35AM +0100, Krzysztof Kozlowski wrote: >>> Bindings and Linux driver require two link frequencies, so correct the >>> 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> >>> --- >>> Documentation/devicetree/bindings/media/i2c/ov8856.yaml | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >>> index 57f5e48fd8e0..bd1a55d767e7 100644 >>> --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >>> +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml >>> @@ -126,7 +126,7 @@ examples: >>> wcam_out: endpoint { >>> remote-endpoint = <&mipi_in_wcam>; >>> data-lanes = <1 2 3 4>; >>> - link-frequencies = /bits/ 64 <360000000>; >>> + link-frequencies = /bits/ 64 <360000000 180000000>; >> >> There indeed seems to be a problem with the example as far as the bindings >> are concerned but the primary issue seems to be in the bindings. Both of >> these frequencies have significance from driver point of view only while >> the device itself supports a (wider) range. >> >> How about removing maxItems and items from the bindings instead? > > There's also a similar issue with lanes: 1, 2 and 4 are supported. ack Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml index 57f5e48fd8e0..bd1a55d767e7 100644 --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml @@ -126,7 +126,7 @@ examples: wcam_out: endpoint { remote-endpoint = <&mipi_in_wcam>; data-lanes = <1 2 3 4>; - link-frequencies = /bits/ 64 <360000000>; + link-frequencies = /bits/ 64 <360000000 180000000>; }; }; };
Bindings and Linux driver require two link frequencies, so correct the 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> --- Documentation/devicetree/bindings/media/i2c/ov8856.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)