* [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).