devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
       [not found] <20220630074525.481790-1-tommaso.merciai@amarulasolutions.com>
@ 2022-06-30  7:45 ` Tommaso Merciai
  2022-06-30  8:07   ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2022-06-30  7:45 UTC (permalink / raw)
  To: tommaso.merciai
  Cc: linuxfancy, linux-amarula, quentin.schulz, Krzysztof Kozlowski,
	Sakari Ailus, Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

Add documentation of device tree in YAML schema for the OV5693
CMOS image sensor from Omnivision

Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
---
Changes since v1:
 - Fix allOf position as suggested by Krzysztof
 - Remove port description as suggested by Krzysztof
 - Fix EOF as suggested by Krzysztof

Changes since v2:
 - Fix commit body as suggested by Krzysztof

Changes since v3:
 - Add reviewed-by tags, ssuggested by Jacopo, Krzysztof

 .../bindings/media/i2c/ovti,ov5693.yaml       | 106 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 107 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
new file mode 100644
index 000000000000..b83c9fc04023
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2022 Amarulasolutions
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Omnivision OV5693 CMOS Sensor
+
+maintainers:
+  - Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
+
+description: |
+  The Omnivision OV5693 is a high performance, 1/4-inch, 5 megapixel, CMOS
+  image sensor that delivers 2592x1944 at 30fps. It provides full-frame,
+  sub-sampled, and windowed 10-bit MIPI images in various formats via the
+  Serial Camera Control Bus (SCCB) interface.
+
+  OV5693 is controlled via I2C and two-wire Serial Camera Control Bus (SCCB).
+  The sensor output is available via CSI-2 serial data output (up to 2-lane).
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: ovti,ov5693
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description:
+      System input clock (aka XVCLK). From 6 to 27 MHz.
+    maxItems: 1
+
+  dovdd-supply:
+    description:
+      Digital I/O voltage supply, 1.8V.
+
+  avdd-supply:
+    description:
+      Analog voltage supply, 2.8V.
+
+  dvdd-supply:
+    description:
+      Digital core voltage supply, 1.2V.
+
+  reset-gpios:
+    description:
+      The phandle and specifier for the GPIO that controls sensor reset.
+      This corresponds to the hardware pin XSHUTDN which is physically
+      active low.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - dovdd-supply
+  - avdd-supply
+  - dvdd-supply
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/px30-cru.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/pinctrl/rockchip.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ov5693: camera@36 {
+            compatible = "ovti,ov5693";
+            reg = <0x36>;
+
+            reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&cif_clkout_m0>;
+
+            clocks = <&cru SCLK_CIF_OUT>;
+            assigned-clocks = <&cru SCLK_CIF_OUT>;
+            assigned-clock-rates = <19200000>;
+
+            avdd-supply = <&vcc_1v8>;
+            dvdd-supply = <&vcc_1v2>;
+            dovdd-supply = <&vcc_2v8>;
+
+            rotation = <90>;
+            orientation = <0>;
+
+            port {
+                ucam_out: endpoint {
+                    remote-endpoint = <&mipi_in_ucam>;
+                    data-lanes = <1 2>;
+                    link-frequencies = /bits/ 64 <450000000>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 1fc9ead83d2a..844307cb20c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14719,6 +14719,7 @@ M:	Daniel Scally <djrscally@gmail.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
 F:	drivers/media/i2c/ov5693.c
 
 OMNIVISION OV5695 SENSOR DRIVER
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30  7:45 ` [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding Tommaso Merciai
@ 2022-06-30  8:07   ` Krzysztof Kozlowski
  2022-06-30  9:02     ` Tommaso Merciai
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-06-30  8:07 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: linuxfancy, linux-amarula, quentin.schulz, Sakari Ailus,
	Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

On 30/06/2022 09:45, Tommaso Merciai wrote:
> Add documentation of device tree in YAML schema for the OV5693
> CMOS image sensor from Omnivision
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>

How Sakari's tag appeared here? There was no email from him.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30  8:07   ` Krzysztof Kozlowski
@ 2022-06-30  9:02     ` Tommaso Merciai
  2022-06-30  9:12       ` Sakari Ailus
  0 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2022-06-30  9:02 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linuxfancy, linux-amarula, quentin.schulz, Sakari Ailus,
	Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> On 30/06/2022 09:45, Tommaso Merciai wrote:
> > Add documentation of device tree in YAML schema for the OV5693
> > CMOS image sensor from Omnivision
> > 
> > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> 
> How Sakari's tag appeared here? There was no email from him.

Sakari made me some review on v2, but I think he forgot to add the mailing
list in cc. ( I suppose :) )

Let me know if I need to remove this.

Regards,
Tommaso


> 
> Best regards,
> Krzysztof

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30  9:02     ` Tommaso Merciai
@ 2022-06-30  9:12       ` Sakari Ailus
  2022-06-30  9:16         ` Tommaso Merciai
  0 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2022-06-30  9:12 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: Krzysztof Kozlowski, linuxfancy, linux-amarula, quentin.schulz,
	Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
> On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> > On 30/06/2022 09:45, Tommaso Merciai wrote:
> > > Add documentation of device tree in YAML schema for the OV5693
> > > CMOS image sensor from Omnivision
> > > 
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> > 
> > How Sakari's tag appeared here? There was no email from him.
> 
> Sakari made me some review on v2, but I think he forgot to add the mailing
> list in cc. ( I suppose :) )
> 
> Let me know if I need to remove this.

You're only supposed to put these tags into patches if you get them in
written form as part of the review, signalling acceptance of the patch in
various forms. Just commenting a patch does not imply this.

Please also see Documentation/process/submitting-patches.rst for more
information on how to use the tags.

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30  9:12       ` Sakari Ailus
@ 2022-06-30  9:16         ` Tommaso Merciai
  2022-06-30  9:50           ` Sakari Ailus
  0 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2022-06-30  9:16 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Krzysztof Kozlowski, linuxfancy, linux-amarula, quentin.schulz,
	Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

Hi Sakari,

On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote:
> On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
> > On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> > > On 30/06/2022 09:45, Tommaso Merciai wrote:
> > > > Add documentation of device tree in YAML schema for the OV5693
> > > > CMOS image sensor from Omnivision
> > > > 
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > 
> > > How Sakari's tag appeared here? There was no email from him.
> > 
> > Sakari made me some review on v2, but I think he forgot to add the mailing
> > list in cc. ( I suppose :) )
> > 
> > Let me know if I need to remove this.
> 
> You're only supposed to put these tags into patches if you get them in
> written form as part of the review, signalling acceptance of the patch in
> various forms. Just commenting a patch does not imply this.
> 
> Please also see Documentation/process/submitting-patches.rst for more
> information on how to use the tags.

Thanks for sharing this. My bad.
I remove your tags.

Regards,
Tommaso

> 
> -- 
> Sakari Ailus

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30  9:16         ` Tommaso Merciai
@ 2022-06-30  9:50           ` Sakari Ailus
  2022-06-30 10:09             ` Tommaso Merciai
  0 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2022-06-30  9:50 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: Krzysztof Kozlowski, linuxfancy, linux-amarula, quentin.schulz,
	Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

Hi Tommaso,

On Thu, Jun 30, 2022 at 11:16:13AM +0200, Tommaso Merciai wrote:
> Hi Sakari,
> 
> On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote:
> > On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
> > > On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> > > > On 30/06/2022 09:45, Tommaso Merciai wrote:
> > > > > Add documentation of device tree in YAML schema for the OV5693
> > > > > CMOS image sensor from Omnivision
> > > > > 
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > > 
> > > > How Sakari's tag appeared here? There was no email from him.
> > > 
> > > Sakari made me some review on v2, but I think he forgot to add the mailing
> > > list in cc. ( I suppose :) )
> > > 
> > > Let me know if I need to remove this.
> > 
> > You're only supposed to put these tags into patches if you get them in
> > written form as part of the review, signalling acceptance of the patch in
> > various forms. Just commenting a patch does not imply this.
> > 
> > Please also see Documentation/process/submitting-patches.rst for more
> > information on how to use the tags.
> 
> Thanks for sharing this. My bad.
> I remove your tags.

The patches themselves seem fine. I'd just drop the 4th patch or at least
come up with a better name for ov5693_hwcfg() --- you're acquiring
resources there, and that generally fits well for probe. The code is fine
already.

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30  9:50           ` Sakari Ailus
@ 2022-06-30 10:09             ` Tommaso Merciai
  2022-06-30 10:15               ` Daniel Scally
  0 siblings, 1 reply; 10+ messages in thread
From: Tommaso Merciai @ 2022-06-30 10:09 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Krzysztof Kozlowski, linuxfancy, linux-amarula, quentin.schulz,
	Jacopo Mondi, Daniel Scally, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

Hi Sakari,

On Thu, Jun 30, 2022 at 12:50:05PM +0300, Sakari Ailus wrote:
> Hi Tommaso,
> 
> On Thu, Jun 30, 2022 at 11:16:13AM +0200, Tommaso Merciai wrote:
> > Hi Sakari,
> > 
> > On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote:
> > > On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
> > > > On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> > > > > On 30/06/2022 09:45, Tommaso Merciai wrote:
> > > > > > Add documentation of device tree in YAML schema for the OV5693
> > > > > > CMOS image sensor from Omnivision
> > > > > > 
> > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > > Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > > > 
> > > > > How Sakari's tag appeared here? There was no email from him.
> > > > 
> > > > Sakari made me some review on v2, but I think he forgot to add the mailing
> > > > list in cc. ( I suppose :) )
> > > > 
> > > > Let me know if I need to remove this.
> > > 
> > > You're only supposed to put these tags into patches if you get them in
> > > written form as part of the review, signalling acceptance of the patch in
> > > various forms. Just commenting a patch does not imply this.
> > > 
> > > Please also see Documentation/process/submitting-patches.rst for more
> > > information on how to use the tags.
> > 
> > Thanks for sharing this. My bad.
> > I remove your tags.
> 
> The patches themselves seem fine. I'd just drop the 4th patch or at least
> come up with a better name for ov5693_hwcfg() --- you're acquiring
> resources there, and that generally fits well for probe. The code is fine
> already.

Then we don't need v5 with your reviewed tags removed?

I think the patch4 is needed to add dts support properly.
Also this contains devm_clk_get_optional fix suggested by Jacopo and
support for ACPI-based platforms that specify the clock frequency by
using the "clock-frequency" property instead of specifying a clock
provider reference.

Some suggestion on the function name?

Thanks,
Tommaso

> 
> -- 
> Sakari Ailus

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30 10:09             ` Tommaso Merciai
@ 2022-06-30 10:15               ` Daniel Scally
  2022-06-30 11:21                 ` Sakari Ailus
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Scally @ 2022-06-30 10:15 UTC (permalink / raw)
  To: Tommaso Merciai, Sakari Ailus
  Cc: Krzysztof Kozlowski, linuxfancy, linux-amarula, quentin.schulz,
	Jacopo Mondi, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

Hello

On 30/06/2022 11:09, Tommaso Merciai wrote:
> Hi Sakari,
>
> On Thu, Jun 30, 2022 at 12:50:05PM +0300, Sakari Ailus wrote:
>> Hi Tommaso,
>>
>> On Thu, Jun 30, 2022 at 11:16:13AM +0200, Tommaso Merciai wrote:
>>> Hi Sakari,
>>>
>>> On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote:
>>>> On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
>>>>> On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
>>>>>> On 30/06/2022 09:45, Tommaso Merciai wrote:
>>>>>>> Add documentation of device tree in YAML schema for the OV5693
>>>>>>> CMOS image sensor from Omnivision
>>>>>>>
>>>>>>> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
>>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>>> Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
>>>>>> How Sakari's tag appeared here? There was no email from him.
>>>>> Sakari made me some review on v2, but I think he forgot to add the mailing
>>>>> list in cc. ( I suppose :) )
>>>>>
>>>>> Let me know if I need to remove this.
>>>> You're only supposed to put these tags into patches if you get them in
>>>> written form as part of the review, signalling acceptance of the patch in
>>>> various forms. Just commenting a patch does not imply this.
>>>>
>>>> Please also see Documentation/process/submitting-patches.rst for more
>>>> information on how to use the tags.
>>> Thanks for sharing this. My bad.
>>> I remove your tags.
>> The patches themselves seem fine. I'd just drop the 4th patch or at least
>> come up with a better name for ov5693_hwcfg() --- you're acquiring
>> resources there, and that generally fits well for probe. The code is fine
>> already.
> Then we don't need v5 with your reviewed tags removed?
>
> I think the patch4 is needed to add dts support properly.
> Also this contains devm_clk_get_optional fix suggested by Jacopo and
> support for ACPI-based platforms that specify the clock frequency by
> using the "clock-frequency" property instead of specifying a clock
> provider reference.


I agree patch 4 in some form is needed - I didn't do the clock handling
particularly well in this driver, and though it's ostensibly an ACPI
driver it wouldn't actually work with a "normal" ACPI, but just with the
cio2-bridge-repaired style. So the changes to the clock handling logic
are welcome and needed I think. whether it needs to go into a separate
function I don't particularly mind either way.

>
> Some suggestion on the function name?
>
> Thanks,
> Tommaso
>
>> -- 
>> Sakari Ailus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30 10:15               ` Daniel Scally
@ 2022-06-30 11:21                 ` Sakari Ailus
  2022-06-30 13:56                   ` Tommaso Merciai
  0 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2022-06-30 11:21 UTC (permalink / raw)
  To: Daniel Scally
  Cc: Tommaso Merciai, Krzysztof Kozlowski, linuxfancy, linux-amarula,
	quentin.schulz, Jacopo Mondi, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

On Thu, Jun 30, 2022 at 11:15:40AM +0100, Daniel Scally wrote:
> Hello
> 
> On 30/06/2022 11:09, Tommaso Merciai wrote:
> > Hi Sakari,
> >
> > On Thu, Jun 30, 2022 at 12:50:05PM +0300, Sakari Ailus wrote:
> >> Hi Tommaso,
> >>
> >> On Thu, Jun 30, 2022 at 11:16:13AM +0200, Tommaso Merciai wrote:
> >>> Hi Sakari,
> >>>
> >>> On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote:
> >>>> On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
> >>>>> On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> >>>>>> On 30/06/2022 09:45, Tommaso Merciai wrote:
> >>>>>>> Add documentation of device tree in YAML schema for the OV5693
> >>>>>>> CMOS image sensor from Omnivision
> >>>>>>>
> >>>>>>> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> >>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>>>>> Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> >>>>>> How Sakari's tag appeared here? There was no email from him.
> >>>>> Sakari made me some review on v2, but I think he forgot to add the mailing
> >>>>> list in cc. ( I suppose :) )
> >>>>>
> >>>>> Let me know if I need to remove this.
> >>>> You're only supposed to put these tags into patches if you get them in
> >>>> written form as part of the review, signalling acceptance of the patch in
> >>>> various forms. Just commenting a patch does not imply this.
> >>>>
> >>>> Please also see Documentation/process/submitting-patches.rst for more
> >>>> information on how to use the tags.
> >>> Thanks for sharing this. My bad.
> >>> I remove your tags.
> >> The patches themselves seem fine. I'd just drop the 4th patch or at least
> >> come up with a better name for ov5693_hwcfg() --- you're acquiring
> >> resources there, and that generally fits well for probe. The code is fine
> >> already.
> > Then we don't need v5 with your reviewed tags removed?
> >
> > I think the patch4 is needed to add dts support properly.
> > Also this contains devm_clk_get_optional fix suggested by Jacopo and
> > support for ACPI-based platforms that specify the clock frequency by
> > using the "clock-frequency" property instead of specifying a clock
> > provider reference.
> 
> 
> I agree patch 4 in some form is needed - I didn't do the clock handling
> particularly well in this driver, and though it's ostensibly an ACPI
> driver it wouldn't actually work with a "normal" ACPI, but just with the
> cio2-bridge-repaired style. So the changes to the clock handling logic
> are welcome and needed I think. whether it needs to go into a separate
> function I don't particularly mind either way.

Yes, the clock handling needs to be changed. But I'd keep it in probe.

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding
  2022-06-30 11:21                 ` Sakari Ailus
@ 2022-06-30 13:56                   ` Tommaso Merciai
  0 siblings, 0 replies; 10+ messages in thread
From: Tommaso Merciai @ 2022-06-30 13:56 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Daniel Scally, Krzysztof Kozlowski, linuxfancy, linux-amarula,
	quentin.schulz, Jacopo Mondi, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, linux-media, devicetree, linux-kernel

Hi Sakari,

On Thu, Jun 30, 2022 at 02:21:04PM +0300, Sakari Ailus wrote:
> On Thu, Jun 30, 2022 at 11:15:40AM +0100, Daniel Scally wrote:
> > Hello
> > 
> > On 30/06/2022 11:09, Tommaso Merciai wrote:
> > > Hi Sakari,
> > >
> > > On Thu, Jun 30, 2022 at 12:50:05PM +0300, Sakari Ailus wrote:
> > >> Hi Tommaso,
> > >>
> > >> On Thu, Jun 30, 2022 at 11:16:13AM +0200, Tommaso Merciai wrote:
> > >>> Hi Sakari,
> > >>>
> > >>> On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote:
> > >>>> On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote:
> > >>>>> On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote:
> > >>>>>> On 30/06/2022 09:45, Tommaso Merciai wrote:
> > >>>>>>> Add documentation of device tree in YAML schema for the OV5693
> > >>>>>>> CMOS image sensor from Omnivision
> > >>>>>>>
> > >>>>>>> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > >>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > >>>>>>> Reviewed-by: Sakari Ailus <sakari.ailus@iki.fi>
> > >>>>>> How Sakari's tag appeared here? There was no email from him.
> > >>>>> Sakari made me some review on v2, but I think he forgot to add the mailing
> > >>>>> list in cc. ( I suppose :) )
> > >>>>>
> > >>>>> Let me know if I need to remove this.
> > >>>> You're only supposed to put these tags into patches if you get them in
> > >>>> written form as part of the review, signalling acceptance of the patch in
> > >>>> various forms. Just commenting a patch does not imply this.
> > >>>>
> > >>>> Please also see Documentation/process/submitting-patches.rst for more
> > >>>> information on how to use the tags.
> > >>> Thanks for sharing this. My bad.
> > >>> I remove your tags.
> > >> The patches themselves seem fine. I'd just drop the 4th patch or at least
> > >> come up with a better name for ov5693_hwcfg() --- you're acquiring
> > >> resources there, and that generally fits well for probe. The code is fine
> > >> already.
> > > Then we don't need v5 with your reviewed tags removed?
> > >
> > > I think the patch4 is needed to add dts support properly.
> > > Also this contains devm_clk_get_optional fix suggested by Jacopo and
> > > support for ACPI-based platforms that specify the clock frequency by
> > > using the "clock-frequency" property instead of specifying a clock
> > > provider reference.
> > 
> > 
> > I agree patch 4 in some form is needed - I didn't do the clock handling
> > particularly well in this driver, and though it's ostensibly an ACPI
> > driver it wouldn't actually work with a "normal" ACPI, but just with the
> > cio2-bridge-repaired style. So the changes to the clock handling logic
> > are welcome and needed I think. whether it needs to go into a separate
> > function I don't particularly mind either way.
> 
> Yes, the clock handling needs to be changed. But I'd keep it in probe.

Fixed in v5, as you suggest.
Thanks all for your time.

Regards,
Tommaso

> 
> -- 
> Sakari Ailus

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-06-30 14:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220630074525.481790-1-tommaso.merciai@amarulasolutions.com>
2022-06-30  7:45 ` [PATCH v4 5/6] media: dt-bindings: ov5693: document YAML binding Tommaso Merciai
2022-06-30  8:07   ` Krzysztof Kozlowski
2022-06-30  9:02     ` Tommaso Merciai
2022-06-30  9:12       ` Sakari Ailus
2022-06-30  9:16         ` Tommaso Merciai
2022-06-30  9:50           ` Sakari Ailus
2022-06-30 10:09             ` Tommaso Merciai
2022-06-30 10:15               ` Daniel Scally
2022-06-30 11:21                 ` Sakari Ailus
2022-06-30 13:56                   ` Tommaso Merciai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).