diff mbox series

[v6.1] media: dt-bindings: Add OV5670

Message ID 20230128112736.8000-1-jacopo.mondi@ideasonboard.com
State Accepted
Commit 3e4ab2342fc26800e8d40a82a6ad5dcedb419924
Headers show
Series [v6.1] media: dt-bindings: Add OV5670 | expand

Commit Message

Jacopo Mondi Jan. 28, 2023, 11:27 a.m. UTC
Add the bindings documentation for Omnivision OV5670 image sensor.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
v6->6.1
- Use additionalProperties: false for endpoint properties from
  video-interfaces.yaml
- List 'remote-endpoint' among the accepted endpoint properties
  now that we use additionalProperties: false
---
 .../bindings/media/i2c/ovti,ov5670.yaml       | 93 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml

--
2.39.0

Comments

Krzysztof Kozlowski Jan. 29, 2023, 11:40 a.m. UTC | #1
On 28/01/2023 12:27, Jacopo Mondi wrote:
> Add the bindings documentation for Omnivision OV5670 image sensor.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> v6->6.1
> - Use additionalProperties: false for endpoint properties from
>   video-interfaces.yaml
> - List 'remote-endpoint' among the accepted endpoint properties
>   now that we use additionalProperties: false

b4 diff '20230128112736.8000-1-jacopo.mondi@ideasonboard.com'
Could not create fake-am range for lower series v1

Can you send patches in a way it does not break out workflows? Why
making our review process more difficult?

Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 29, 2023, 11:40 a.m. UTC | #2
On 28/01/2023 12:27, Jacopo Mondi wrote:
> Add the bindings documentation for Omnivision OV5670 image sensor.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> v6->6.1
> - Use additionalProperties: false for endpoint properties from
>   video-interfaces.yaml
> - List 'remote-endpoint' among the accepted endpoint properties
>   now that we use additionalProperties: false
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Jacopo Mondi Jan. 29, 2023, 12:11 p.m. UTC | #3
On Sun, Jan 29, 2023 at 12:40:03PM +0100, Krzysztof Kozlowski wrote:
> On 28/01/2023 12:27, Jacopo Mondi wrote:
> > Add the bindings documentation for Omnivision OV5670 image sensor.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> > v6->6.1
> > - Use additionalProperties: false for endpoint properties from
> >   video-interfaces.yaml
> > - List 'remote-endpoint' among the accepted endpoint properties
> >   now that we use additionalProperties: false
>
> b4 diff '20230128112736.8000-1-jacopo.mondi@ideasonboard.com'
> Could not create fake-am range for lower series v1
>
> Can you send patches in a way it does not break out workflows? Why
> making our review process more difficult?

Because it's a nit on a 10 patches series with no other changes
requested ?

What is difficult exactly ?

I see several patches in linux-media being sent inline to a previous
version for small fixes if the only required changed is a nit like
this one.


> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Jan. 29, 2023, 12:31 p.m. UTC | #4
On 29/01/2023 13:11, Jacopo Mondi wrote:
> On Sun, Jan 29, 2023 at 12:40:03PM +0100, Krzysztof Kozlowski wrote:
>> On 28/01/2023 12:27, Jacopo Mondi wrote:
>>> Add the bindings documentation for Omnivision OV5670 image sensor.
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>> ---
>>> v6->6.1
>>> - Use additionalProperties: false for endpoint properties from
>>>   video-interfaces.yaml
>>> - List 'remote-endpoint' among the accepted endpoint properties
>>>   now that we use additionalProperties: false
>>
>> b4 diff '20230128112736.8000-1-jacopo.mondi@ideasonboard.com'
>> Could not create fake-am range for lower series v1
>>
>> Can you send patches in a way it does not break out workflows? Why
>> making our review process more difficult?
> 
> Because it's a nit on a 10 patches series with no other changes
> requested ?
> 
> What is difficult exactly ?

I wrote above what's difficult.

> 
> I see several patches in linux-media being sent inline to a previous
> version for small fixes if the only required changed is a nit like
> this one.

If you sent it as separate v7 would be fine, but:
1. Threading is wrong - it's buried in other patch.
2. Version is wrong - you did there changes, not nits. There are no
point versions...

Best regards,
Krzysztof
Rob Herring Jan. 30, 2023, 3:58 p.m. UTC | #5
On Sun, Jan 29, 2023 at 01:11:32PM +0100, Jacopo Mondi wrote:
> On Sun, Jan 29, 2023 at 12:40:03PM +0100, Krzysztof Kozlowski wrote:
> > On 28/01/2023 12:27, Jacopo Mondi wrote:
> > > Add the bindings documentation for Omnivision OV5670 image sensor.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > > v6->6.1
> > > - Use additionalProperties: false for endpoint properties from
> > >   video-interfaces.yaml
> > > - List 'remote-endpoint' among the accepted endpoint properties
> > >   now that we use additionalProperties: false
> >
> > b4 diff '20230128112736.8000-1-jacopo.mondi@ideasonboard.com'
> > Could not create fake-am range for lower series v1
> >
> > Can you send patches in a way it does not break out workflows? Why
> > making our review process more difficult?
> 
> Because it's a nit on a 10 patches series with no other changes
> requested ?

So? Think of patch series as an 'email transport' for your git branches. 
If you rebase your branch, that's a whole new branch to send.

> What is difficult exactly ?

In addition to 'b4 diff', if a maintainer is applying this series, for a 
v7 they just do:

b4 shazam msgid-of-v7

For v6.1, they do:

b4 shazam msgid-of-v6
git rebase -i ...
<stop on patch 1>
git reset --hard HEAD^
b4 shazam msgid-of-v6.1
git rebase --continue

Which one makes the maintainer's life easier?

If it's a CI job trying to apply and test this, there's no way it's 
going to do the second case.

Rob
Jacopo Mondi Jan. 30, 2023, 4:11 p.m. UTC | #6
On Mon, Jan 30, 2023 at 09:58:40AM -0600, Rob Herring wrote:
> On Sun, Jan 29, 2023 at 01:11:32PM +0100, Jacopo Mondi wrote:
> > On Sun, Jan 29, 2023 at 12:40:03PM +0100, Krzysztof Kozlowski wrote:
> > > On 28/01/2023 12:27, Jacopo Mondi wrote:
> > > > Add the bindings documentation for Omnivision OV5670 image sensor.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > > ---
> > > > v6->6.1
> > > > - Use additionalProperties: false for endpoint properties from
> > > >   video-interfaces.yaml
> > > > - List 'remote-endpoint' among the accepted endpoint properties
> > > >   now that we use additionalProperties: false
> > >
> > > b4 diff '20230128112736.8000-1-jacopo.mondi@ideasonboard.com'
> > > Could not create fake-am range for lower series v1
> > >
> > > Can you send patches in a way it does not break out workflows? Why
> > > making our review process more difficult?
> >
> > Because it's a nit on a 10 patches series with no other changes
> > requested ?
>
> So? Think of patch series as an 'email transport' for your git branches.
> If you rebase your branch, that's a whole new branch to send.
>

So if a series has a single comment and could be then collected as it
is but one patch I saw it happening multiple times on the ML and I
thought it was an accepted practice.


> > What is difficult exactly ?
>
> In addition to 'b4 diff', if a maintainer is applying this series, for a
> v7 they just do:
>
> b4 shazam msgid-of-v7
>
> For v6.1, they do:
>
> b4 shazam msgid-of-v6
> git rebase -i ...
> <stop on patch 1>
> git reset --hard HEAD^
> b4 shazam msgid-of-v6.1
> git rebase --continue
>
> Which one makes the maintainer's life easier?
>

With b4 it now certainly makes a difference.

As I save patches from my mail client and apply them manually I never
really considered picking one patch over the other from the same
thread "more difficult". I should have noticed when Krzysztof
mentioned b4 in his first reply.

> If it's a CI job trying to apply and test this, there's no way it's
> going to do the second case.
>

That's another point yes.

Got your message, I'll stop :)

Don't think a v7 is needed for this on though (if not other
comments ofc)

> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml
new file mode 100644
index 000000000000..6e089fe1d613
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml
@@ -0,0 +1,93 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov5670.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Omnivision OV5670 5 Megapixels raw image sensor
+
+maintainers:
+  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
+
+description: |-
+  The OV5670 is a 5 Megapixels raw image sensor which provides images in 10-bits
+  RAW BGGR Bayer format on a 2 data lanes MIPI CSI-2 serial interface and is
+  controlled through an I2C compatible control bus.
+
+properties:
+  compatible:
+    const: ovti,ov5670
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: System clock. From 6 to 27 MHz.
+    maxItems: 1
+
+  powerdown-gpios:
+    description: Reference to the GPIO connected to the PWDNB pin. Active low.
+
+  reset-gpios:
+    description: Reference to the GPIO connected to the XSHUTDOWN pin. Active low.
+    maxItems: 1
+
+  avdd-supply:
+    description: Analog circuit power. Typically 2.8V.
+
+  dvdd-supply:
+    description: Digital circuit power. Typically 1.2V.
+
+  dovdd-supply:
+    description: Digital I/O circuit power. Typically 2.8V or 1.8V.
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        additionalProperties: false
+
+        properties:
+          data-lanes:
+            minItems: 1
+            maxItems: 2
+            items:
+              enum: [1, 2]
+
+          clock-noncontinuous: true
+          remote-endpoint: true
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ov5670: sensor@36 {
+            compatible = "ovti,ov5670";
+            reg = <0x36>;
+
+            clocks = <&sensor_xclk>;
+
+            port {
+                ov5670_ep: endpoint {
+                    remote-endpoint = <&csi_ep>;
+                    data-lanes = <1 2>;
+                    clock-noncontinuous;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index f61eb221415b..38d8d1d5d536 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15468,6 +15468,7 @@  M:	Chiranjeevi Rapolu <chiranjeevi.rapolu@intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/ovti,ov5670.yaml
 F:	drivers/media/i2c/ov5670.c

 OMNIVISION OV5675 SENSOR DRIVER