2020-03-13 21:13:43

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Use assigned-clock-rates to specify the clock rate. Also mark
clock-frequency property as deprecated.

Signed-off-by: Lad Prabhakar <[email protected]>
---
Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
index 72ad992..e62fe82 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
@@ -8,7 +8,7 @@ Required Properties:
- compatible: Value should be "ovti,ov5645".
- clocks: Reference to the xclk clock.
- clock-names: Should be "xclk".
-- clock-frequency: Frequency of the xclk clock.
+- clock-frequency (deprecated): Frequency of the xclk clock.
- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
to the hardware pin PWDNB which is physically active low.
- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
@@ -37,7 +37,8 @@ Example:

clocks = <&clks 200>;
clock-names = "xclk";
- clock-frequency = <24000000>;
+ assigned-clocks = <&clks 200>;
+ assigned-clock-rates = <24000000>;

vdddo-supply = <&camera_dovdd_1v8>;
vdda-supply = <&camera_avdd_2v8>;
--
2.7.4


2020-03-13 21:22:47

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Prabhakar,

Thank you for the patch.

On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> Use assigned-clock-rates to specify the clock rate. Also mark
> clock-frequency property as deprecated.

I would phrase it the other way around, this patch mainly deprecates
clock-frequency, and as a side effect recommends usage of
assigned-clock-rates.

"Deprecate usage of the clock-frequency propertly. The preferred method
to set clock rates is to use assigned-clock-rates."

> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> index 72ad992..e62fe82 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> @@ -8,7 +8,7 @@ Required Properties:
> - compatible: Value should be "ovti,ov5645".
> - clocks: Reference to the xclk clock.
> - clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock.
> +- clock-frequency (deprecated): Frequency of the xclk clock.

I would drop this completely. Drivers need to ensure backward
compatibility, but DT bindings should only document the latest version,
the history is available in git.

Reviewed-by: Laurent Pinchart <[email protected]>

While at it, can I enlist you to convert these bindings to yaml ? :-)

> - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> to the hardware pin PWDNB which is physically active low.
> - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> @@ -37,7 +37,8 @@ Example:
>
> clocks = <&clks 200>;
> clock-names = "xclk";
> - clock-frequency = <24000000>;
> + assigned-clocks = <&clks 200>;
> + assigned-clock-rates = <24000000>;
>
> vdddo-supply = <&camera_dovdd_1v8>;
> vdda-supply = <&camera_avdd_2v8>;

--
Regards,

Laurent Pinchart

2020-03-13 21:26:27

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: RE: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Laurent,

Thank you for the quick review.

> -----Original Message-----
> From: Laurent Pinchart <[email protected]>
> Sent: 13 March 2020 21:20
> To: Prabhakar Mahadev Lad <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>; Shawn Guo
> <[email protected]>; Sascha Hauer <[email protected]>;
> Pengutronix Kernel Team <[email protected]>; Rob Herring
> <[email protected]>; Mark Rutland <[email protected]>; Sakari
> Ailus <[email protected]>; NXP Linux Team <[email protected]>;
> Magnus Damm <[email protected]>; Ezequiel Garcia
> <[email protected]>; Geert Uytterhoeven <[email protected]>;
> [email protected]; [email protected]; linux-renesas-
> [email protected]; Fabio Estevam <[email protected]>; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to
> assigned-clock-rates
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > Use assigned-clock-rates to specify the clock rate. Also mark
> > clock-frequency property as deprecated.
>
> I would phrase it the other way around, this patch mainly deprecates clock-
> frequency, and as a side effect recommends usage of assigned-clock-rates.
>
> "Deprecate usage of the clock-frequency propertly. The preferred method
> to set clock rates is to use assigned-clock-rates."
>
Agreed will do that.

> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-
> [email protected]>
> > ---
> > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > index 72ad992..e62fe82 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > @@ -8,7 +8,7 @@ Required Properties:
> > - compatible: Value should be "ovti,ov5645".
> > - clocks: Reference to the xclk clock.
> > - clock-names: Should be "xclk".
> > -- clock-frequency: Frequency of the xclk clock.
> > +- clock-frequency (deprecated): Frequency of the xclk clock.
>
> I would drop this completely. Drivers need to ensure backward compatibility,
> but DT bindings should only document the latest version, the history is
> available in git.
>
Sure will drop it.

> Reviewed-by: Laurent Pinchart <[email protected]>
>
> While at it, can I enlist you to convert these bindings to yaml ? :-)
>
Sure will do the honours ????, will make sure yaml patch is ontop of this patch too.

Cheers,
--Prabhakar

> > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This
> corresponds
> > to the hardware pin PWDNB which is physically active low.
> > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This
> > corresponds to @@ -37,7 +37,8 @@ Example:
> >
> > clocks = <&clks 200>;
> > clock-names = "xclk";
> > -clock-frequency = <24000000>;
> > +assigned-clocks = <&clks 200>;
> > +assigned-clock-rates = <24000000>;
> >
> > vdddo-supply = <&camera_dovdd_1v8>;
> > vdda-supply = <&camera_avdd_2v8>;
>
> --
> Regards,
>
> Laurent Pinchart


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

2020-03-13 21:28:13

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Prabhakar,

On Fri, Mar 13, 2020 at 09:25:01PM +0000, Prabhakar Mahadev Lad wrote:
> On Sent: 13 March 2020 21:20, Laurent Pinchart wrote:
> > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > clock-frequency property as deprecated.
> >
> > I would phrase it the other way around, this patch mainly deprecates clock-
> > frequency, and as a side effect recommends usage of assigned-clock-rates.
> >
> > "Deprecate usage of the clock-frequency propertly. The preferred method
> > to set clock rates is to use assigned-clock-rates."
>
> Agreed will do that.
>
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > index 72ad992..e62fe82 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > @@ -8,7 +8,7 @@ Required Properties:
> > > - compatible: Value should be "ovti,ov5645".
> > > - clocks: Reference to the xclk clock.
> > > - clock-names: Should be "xclk".
> > > -- clock-frequency: Frequency of the xclk clock.
> > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> >
> > I would drop this completely. Drivers need to ensure backward compatibility,
> > but DT bindings should only document the latest version, the history is
> > available in git.
> >
> Sure will drop it.
>
> > Reviewed-by: Laurent Pinchart <[email protected]>
> >
> > While at it, can I enlist you to convert these bindings to yaml ? :-)
> >
> Sure will do the honours ????, will make sure yaml patch is ontop of this patch too.

Thank you :-)

> > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > to the hardware pin PWDNB which is physically active low.
> > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > @@ -37,7 +37,8 @@ Example:
> > >
> > > clocks = <&clks 200>;
> > > clock-names = "xclk";
> > > -clock-frequency = <24000000>;
> > > +assigned-clocks = <&clks 200>;
> > > +assigned-clock-rates = <24000000>;
> > >
> > > vdddo-supply = <&camera_dovdd_1v8>;
> > > vdda-supply = <&camera_avdd_2v8>;

--
Regards,

Laurent Pinchart

2020-03-18 20:14:15

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Laurent,

On Fri, Mar 13, 2020 at 9:27 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Mar 13, 2020 at 09:25:01PM +0000, Prabhakar Mahadev Lad wrote:
> > On Sent: 13 March 2020 21:20, Laurent Pinchart wrote:
> > > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > > clock-frequency property as deprecated.
> > >
> > > I would phrase it the other way around, this patch mainly deprecates clock-
> > > frequency, and as a side effect recommends usage of assigned-clock-rates.
> > >
> > > "Deprecate usage of the clock-frequency propertly. The preferred method
> > > to set clock rates is to use assigned-clock-rates."
> >
> > Agreed will do that.
> >
> > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > index 72ad992..e62fe82 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > @@ -8,7 +8,7 @@ Required Properties:
> > > > - compatible: Value should be "ovti,ov5645".
> > > > - clocks: Reference to the xclk clock.
> > > > - clock-names: Should be "xclk".
> > > > -- clock-frequency: Frequency of the xclk clock.
> > > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > >
> > > I would drop this completely. Drivers need to ensure backward compatibility,
> > > but DT bindings should only document the latest version, the history is
> > > available in git.
> > >
> > Sure will drop it.
> >
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> > >
> > > While at it, can I enlist you to convert these bindings to yaml ? :-)
> > >
> > Sure will do the honours , will make sure yaml patch is ontop of this patch too.
>
Shall I enlist you as the maintainer in the json-schema ?
dt_binding_check says 'maintainers' is a required property.

Cheers,
--Prabhakar Lad

> Thank you :-)
>
> > > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > > to the hardware pin PWDNB which is physically active low.
> > > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > > @@ -37,7 +37,8 @@ Example:
> > > >
> > > > clocks = <&clks 200>;
> > > > clock-names = "xclk";
> > > > -clock-frequency = <24000000>;
> > > > +assigned-clocks = <&clks 200>;
> > > > +assigned-clock-rates = <24000000>;
> > > >
> > > > vdddo-supply = <&camera_dovdd_1v8>;
> > > > vdda-supply = <&camera_avdd_2v8>;
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2020-03-18 22:34:56

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Prabhakar,

On Wed, Mar 18, 2020 at 08:13:00PM +0000, Lad, Prabhakar wrote:
> On Fri, Mar 13, 2020 at 9:27 PM Laurent Pinchart wrote:
> > On Fri, Mar 13, 2020 at 09:25:01PM +0000, Prabhakar Mahadev Lad wrote:
> >> On Sent: 13 March 2020 21:20, Laurent Pinchart wrote:
> >>> On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> >>>> Use assigned-clock-rates to specify the clock rate. Also mark
> >>>> clock-frequency property as deprecated.
> >>>
> >>> I would phrase it the other way around, this patch mainly deprecates clock-
> >>> frequency, and as a side effect recommends usage of assigned-clock-rates.
> >>>
> >>> "Deprecate usage of the clock-frequency propertly. The preferred method
> >>> to set clock rates is to use assigned-clock-rates."
> >>
> >> Agreed will do that.
> >>
> >>>> Signed-off-by: Lad Prabhakar <[email protected]>
> >>>> ---
> >>>> Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> >>>> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >>>> b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >>>> index 72ad992..e62fe82 100644
> >>>> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >>>> @@ -8,7 +8,7 @@ Required Properties:
> >>>> - compatible: Value should be "ovti,ov5645".
> >>>> - clocks: Reference to the xclk clock.
> >>>> - clock-names: Should be "xclk".
> >>>> -- clock-frequency: Frequency of the xclk clock.
> >>>> +- clock-frequency (deprecated): Frequency of the xclk clock.
> >>>
> >>> I would drop this completely. Drivers need to ensure backward compatibility,
> >>> but DT bindings should only document the latest version, the history is
> >>> available in git.
> >>
> >> Sure will drop it.
> >>
> >>> Reviewed-by: Laurent Pinchart <[email protected]>
> >>>
> >>> While at it, can I enlist you to convert these bindings to yaml ? :-)
> >>>
> >> Sure will do the honours , will make sure yaml patch is ontop of this patch too.
>
> Shall I enlist you as the maintainer in the json-schema ?
> dt_binding_check says 'maintainers' is a required property.

Do you want to be the new maintainer ? :-) Sakari is maintaining sensor
drivers (in his spare time though) so maybe he could be listed in the DT
bindings too if he wants. Otherwise, I could do it.

> > Thank you :-)
> >
> >>>> - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> >>>> to the hardware pin PWDNB which is physically active low.
> >>>> - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> >>>> @@ -37,7 +37,8 @@ Example:
> >>>>
> >>>> clocks = <&clks 200>;
> >>>> clock-names = "xclk";
> >>>> -clock-frequency = <24000000>;
> >>>> +assigned-clocks = <&clks 200>;
> >>>> +assigned-clock-rates = <24000000>;
> >>>>
> >>>> vdddo-supply = <&camera_dovdd_1v8>;
> >>>> vdda-supply = <&camera_avdd_2v8>;

--
Regards,

Laurent Pinchart

2020-03-18 22:39:09

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Laurent,

On Wed, Mar 18, 2020 at 10:33 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Wed, Mar 18, 2020 at 08:13:00PM +0000, Lad, Prabhakar wrote:
> > On Fri, Mar 13, 2020 at 9:27 PM Laurent Pinchart wrote:
> > > On Fri, Mar 13, 2020 at 09:25:01PM +0000, Prabhakar Mahadev Lad wrote:
> > >> On Sent: 13 March 2020 21:20, Laurent Pinchart wrote:
> > >>> On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > >>>> Use assigned-clock-rates to specify the clock rate. Also mark
> > >>>> clock-frequency property as deprecated.
> > >>>
> > >>> I would phrase it the other way around, this patch mainly deprecates clock-
> > >>> frequency, and as a side effect recommends usage of assigned-clock-rates.
> > >>>
> > >>> "Deprecate usage of the clock-frequency propertly. The preferred method
> > >>> to set clock rates is to use assigned-clock-rates."
> > >>
> > >> Agreed will do that.
> > >>
> > >>>> Signed-off-by: Lad Prabhakar <[email protected]>
> > >>>> ---
> > >>>> Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > >>>> 1 file changed, 3 insertions(+), 2 deletions(-)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > >>>> b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > >>>> index 72ad992..e62fe82 100644
> > >>>> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > >>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > >>>> @@ -8,7 +8,7 @@ Required Properties:
> > >>>> - compatible: Value should be "ovti,ov5645".
> > >>>> - clocks: Reference to the xclk clock.
> > >>>> - clock-names: Should be "xclk".
> > >>>> -- clock-frequency: Frequency of the xclk clock.
> > >>>> +- clock-frequency (deprecated): Frequency of the xclk clock.
> > >>>
> > >>> I would drop this completely. Drivers need to ensure backward compatibility,
> > >>> but DT bindings should only document the latest version, the history is
> > >>> available in git.
> > >>
> > >> Sure will drop it.
> > >>
> > >>> Reviewed-by: Laurent Pinchart <[email protected]>
> > >>>
> > >>> While at it, can I enlist you to convert these bindings to yaml ? :-)
> > >>>
> > >> Sure will do the honours , will make sure yaml patch is ontop of this patch too.
> >
> > Shall I enlist you as the maintainer in the json-schema ?
> > dt_binding_check says 'maintainers' is a required property.
>
> Do you want to be the new maintainer ? :-) Sakari is maintaining sensor
> drivers (in his spare time though) so maybe he could be listed in the DT
> bindings too if he wants. Otherwise, I could do it.
>
OK I will add myself and Sakari for now and post a v4.

Cheers,
--Prabhakar

> > > Thank you :-)
> > >
> > >>>> - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > >>>> to the hardware pin PWDNB which is physically active low.
> > >>>> - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > >>>> @@ -37,7 +37,8 @@ Example:
> > >>>>
> > >>>> clocks = <&clks 200>;
> > >>>> clock-names = "xclk";
> > >>>> -clock-frequency = <24000000>;
> > >>>> +assigned-clocks = <&clks 200>;
> > >>>> +assigned-clock-rates = <24000000>;
> > >>>>
> > >>>> vdddo-supply = <&camera_dovdd_1v8>;
> > >>>> vdda-supply = <&camera_avdd_2v8>;
>
> --
> Regards,
>
> Laurent Pinchart

2020-03-19 12:45:32

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi,

On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> Use assigned-clock-rates to specify the clock rate. Also mark
> clock-frequency property as deprecated.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> index 72ad992..e62fe82 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> @@ -8,7 +8,7 @@ Required Properties:
> - compatible: Value should be "ovti,ov5645".
> - clocks: Reference to the xclk clock.
> - clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock.
> +- clock-frequency (deprecated): Frequency of the xclk clock.
> - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> to the hardware pin PWDNB which is physically active low.
> - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> @@ -37,7 +37,8 @@ Example:
>
> clocks = <&clks 200>;
> clock-names = "xclk";
> - clock-frequency = <24000000>;
> + assigned-clocks = <&clks 200>;
> + assigned-clock-rates = <24000000>;
>
> vdddo-supply = <&camera_dovdd_1v8>;
> vdda-supply = <&camera_avdd_2v8>;

clock-frequency is quite different from assigned-clock-rates though,
semantically speaking. clock-frequency is only about what the clock
frequency is, while assigned-clock-rates will change the rate as well,
and you have no idea how long it will last.

If you want to retrieve that through the clock framework, then just
making clock-frequency optional is enough and falling back to
clk_get_rate on the clocks property already provided is enough.

Maxime


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

2020-03-19 13:05:48

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Maxime,

On Thu, Mar 19, 2020 at 01:44:52PM +0100, Maxime Ripard wrote:
> On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > Use assigned-clock-rates to specify the clock rate. Also mark
> > clock-frequency property as deprecated.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > index 72ad992..e62fe82 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > @@ -8,7 +8,7 @@ Required Properties:
> > - compatible: Value should be "ovti,ov5645".
> > - clocks: Reference to the xclk clock.
> > - clock-names: Should be "xclk".
> > -- clock-frequency: Frequency of the xclk clock.
> > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > to the hardware pin PWDNB which is physically active low.
> > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > @@ -37,7 +37,8 @@ Example:
> >
> > clocks = <&clks 200>;
> > clock-names = "xclk";
> > - clock-frequency = <24000000>;
> > + assigned-clocks = <&clks 200>;
> > + assigned-clock-rates = <24000000>;
> >
> > vdddo-supply = <&camera_dovdd_1v8>;
> > vdda-supply = <&camera_avdd_2v8>;
>
> clock-frequency is quite different from assigned-clock-rates though,
> semantically speaking. clock-frequency is only about what the clock
> frequency is, while assigned-clock-rates will change the rate as well,
> and you have no idea how long it will last.

The driver currently reads the clock-frequency property and then calls
clk_set_rate(). I agree tht assigned-clock-rates isn't a panacea, but I
think it's less of a hack than what we currently have.

As discussed on IRC, maybe the best option in this specific case is to
drop clock-frequency and assigned-clok-rates, and call clk_set_rate()
with a hardcoded frequency of 24MHz in the driver, as that's the only
frequency the driver supports.

> If you want to retrieve that through the clock framework, then just
> making clock-frequency optional is enough and falling back to
> clk_get_rate on the clocks property already provided is enough.

--
Regards,

Laurent Pinchart

2020-03-19 13:19:07

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Laurent,

On Thu, Mar 19, 2020 at 1:04 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Maxime,
>
> On Thu, Mar 19, 2020 at 01:44:52PM +0100, Maxime Ripard wrote:
> > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > clock-frequency property as deprecated.
> > >
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > index 72ad992..e62fe82 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > @@ -8,7 +8,7 @@ Required Properties:
> > > - compatible: Value should be "ovti,ov5645".
> > > - clocks: Reference to the xclk clock.
> > > - clock-names: Should be "xclk".
> > > -- clock-frequency: Frequency of the xclk clock.
> > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > to the hardware pin PWDNB which is physically active low.
> > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > @@ -37,7 +37,8 @@ Example:
> > >
> > > clocks = <&clks 200>;
> > > clock-names = "xclk";
> > > - clock-frequency = <24000000>;
> > > + assigned-clocks = <&clks 200>;
> > > + assigned-clock-rates = <24000000>;
> > >
> > > vdddo-supply = <&camera_dovdd_1v8>;
> > > vdda-supply = <&camera_avdd_2v8>;
> >
> > clock-frequency is quite different from assigned-clock-rates though,
> > semantically speaking. clock-frequency is only about what the clock
> > frequency is, while assigned-clock-rates will change the rate as well,
> > and you have no idea how long it will last.
>
> The driver currently reads the clock-frequency property and then calls
> clk_set_rate(). I agree tht assigned-clock-rates isn't a panacea, but I
> think it's less of a hack than what we currently have.
>
> As discussed on IRC, maybe the best option in this specific case is to
> drop clock-frequency and assigned-clok-rates, and call clk_set_rate()
> with a hardcoded frequency of 24MHz in the driver, as that's the only
> frequency the driver supports.
>
Does this mean any driver which has a fixed clock requirement shouldn't be a
DT property and should be just handled by the drivers internally ?

Cheers,
--Prabhakar

> > If you want to retrieve that through the clock framework, then just
> > making clock-frequency optional is enough and falling back to
> > clk_get_rate on the clocks property already provided is enough.
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2020-03-19 13:38:59

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Maxime,

Thank you for the review.

On Thu, Mar 19, 2020 at 12:45 PM Maxime Ripard <[email protected]> wrote:
>
> Hi,
>
> On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > Use assigned-clock-rates to specify the clock rate. Also mark
> > clock-frequency property as deprecated.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > index 72ad992..e62fe82 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > @@ -8,7 +8,7 @@ Required Properties:
> > - compatible: Value should be "ovti,ov5645".
> > - clocks: Reference to the xclk clock.
> > - clock-names: Should be "xclk".
> > -- clock-frequency: Frequency of the xclk clock.
> > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > to the hardware pin PWDNB which is physically active low.
> > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > @@ -37,7 +37,8 @@ Example:
> >
> > clocks = <&clks 200>;
> > clock-names = "xclk";
> > - clock-frequency = <24000000>;
> > + assigned-clocks = <&clks 200>;
> > + assigned-clock-rates = <24000000>;
> >
> > vdddo-supply = <&camera_dovdd_1v8>;
> > vdda-supply = <&camera_avdd_2v8>;
>
> clock-frequency is quite different from assigned-clock-rates though,
> semantically speaking. clock-frequency is only about what the clock
> frequency is, while assigned-clock-rates will change the rate as well,
> and you have no idea how long it will last.
>
Agreed clock-frequency tells whats the clock frequency, wrt ov5645 driver
this property was read and and the clock rate was changed accordingly as per
the value being passed. So switching to assigned-clock-rates does bypass
of clock rate being set in the ov5645 driver [1] as the framework does it.

> If you want to retrieve that through the clock framework, then just
> making clock-frequency optional is enough and falling back to
> clk_get_rate on the clocks property already provided is enough.
>
As done in patch [1] ?

Fyi I have posted a v4 [2] to ML.

[1] https://patchwork.linuxtv.org/patch/62378/
[2] https://patchwork.linuxtv.org/project/linux-media/list/?series=1990

Cheers,
--Prabhakar

> Maxime
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2020-03-24 15:41:20

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

On Thu, Mar 19, 2020 at 01:17:51PM +0000, Lad, Prabhakar wrote:
> Hi Laurent,
>
> On Thu, Mar 19, 2020 at 1:04 PM Laurent Pinchart
> <[email protected]> wrote:
> >
> > Hi Maxime,
> >
> > On Thu, Mar 19, 2020 at 01:44:52PM +0100, Maxime Ripard wrote:
> > > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > > clock-frequency property as deprecated.
> > > >
> > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > index 72ad992..e62fe82 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > @@ -8,7 +8,7 @@ Required Properties:
> > > > - compatible: Value should be "ovti,ov5645".
> > > > - clocks: Reference to the xclk clock.
> > > > - clock-names: Should be "xclk".
> > > > -- clock-frequency: Frequency of the xclk clock.
> > > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > > to the hardware pin PWDNB which is physically active low.
> > > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > > @@ -37,7 +37,8 @@ Example:
> > > >
> > > > clocks = <&clks 200>;
> > > > clock-names = "xclk";
> > > > - clock-frequency = <24000000>;
> > > > + assigned-clocks = <&clks 200>;
> > > > + assigned-clock-rates = <24000000>;
> > > >
> > > > vdddo-supply = <&camera_dovdd_1v8>;
> > > > vdda-supply = <&camera_avdd_2v8>;
> > >
> > > clock-frequency is quite different from assigned-clock-rates though,
> > > semantically speaking. clock-frequency is only about what the clock
> > > frequency is, while assigned-clock-rates will change the rate as well,
> > > and you have no idea how long it will last.
> >
> > The driver currently reads the clock-frequency property and then calls
> > clk_set_rate(). I agree tht assigned-clock-rates isn't a panacea, but I
> > think it's less of a hack than what we currently have.
> >
> > As discussed on IRC, maybe the best option in this specific case is to
> > drop clock-frequency and assigned-clok-rates, and call clk_set_rate()
> > with a hardcoded frequency of 24MHz in the driver, as that's the only
> > frequency the driver supports.
> >
> Does this mean any driver which has a fixed clock requirement shouldn't be a
> DT property and should be just handled by the drivers internally ?

It's hard to give a generic policy, but here, the hardware is pretty
flexible since it can deal with anything between 6MHz to 50-something
MHz, it's the driver that chooses to enforce a 24MHz and be pedantic
about it, so it's up to the driver to enforce that policy, not to the
DT since it's essentially a software limitation, not a hardware one.

Maxime


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

2020-03-24 16:07:53

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Maxime,

On Tue, Mar 24, 2020 at 3:40 PM Maxime Ripard <[email protected]> wrote:
>
> On Thu, Mar 19, 2020 at 01:17:51PM +0000, Lad, Prabhakar wrote:
> > Hi Laurent,
> >
> > On Thu, Mar 19, 2020 at 1:04 PM Laurent Pinchart
> > <[email protected]> wrote:
> > >
> > > Hi Maxime,
> > >
> > > On Thu, Mar 19, 2020 at 01:44:52PM +0100, Maxime Ripard wrote:
> > > > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > > > clock-frequency property as deprecated.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > index 72ad992..e62fe82 100644
> > > > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > @@ -8,7 +8,7 @@ Required Properties:
> > > > > - compatible: Value should be "ovti,ov5645".
> > > > > - clocks: Reference to the xclk clock.
> > > > > - clock-names: Should be "xclk".
> > > > > -- clock-frequency: Frequency of the xclk clock.
> > > > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > > > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > > > to the hardware pin PWDNB which is physically active low.
> > > > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > > > @@ -37,7 +37,8 @@ Example:
> > > > >
> > > > > clocks = <&clks 200>;
> > > > > clock-names = "xclk";
> > > > > - clock-frequency = <24000000>;
> > > > > + assigned-clocks = <&clks 200>;
> > > > > + assigned-clock-rates = <24000000>;
> > > > >
> > > > > vdddo-supply = <&camera_dovdd_1v8>;
> > > > > vdda-supply = <&camera_avdd_2v8>;
> > > >
> > > > clock-frequency is quite different from assigned-clock-rates though,
> > > > semantically speaking. clock-frequency is only about what the clock
> > > > frequency is, while assigned-clock-rates will change the rate as well,
> > > > and you have no idea how long it will last.
> > >
> > > The driver currently reads the clock-frequency property and then calls
> > > clk_set_rate(). I agree tht assigned-clock-rates isn't a panacea, but I
> > > think it's less of a hack than what we currently have.
> > >
> > > As discussed on IRC, maybe the best option in this specific case is to
> > > drop clock-frequency and assigned-clok-rates, and call clk_set_rate()
> > > with a hardcoded frequency of 24MHz in the driver, as that's the only
> > > frequency the driver supports.
> > >
> > Does this mean any driver which has a fixed clock requirement shouldn't be a
> > DT property and should be just handled by the drivers internally ?
>
> It's hard to give a generic policy, but here, the hardware is pretty
> flexible since it can deal with anything between 6MHz to 50-something
> MHz, it's the driver that chooses to enforce a 24MHz and be pedantic
> about it, so it's up to the driver to enforce that policy, not to the
> DT since it's essentially a software limitation, not a hardware one.
>
Thank you for the clarification, Ill drop patches 1-4 from this series.

Cheers,
--Prabhakar

> Maxime
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2020-03-24 16:15:00

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Prabhakar,

On Tue, Mar 24, 2020 at 04:04:43PM +0000, Lad, Prabhakar wrote:
> On Tue, Mar 24, 2020 at 3:40 PM Maxime Ripard <[email protected]> wrote:
> > On Thu, Mar 19, 2020 at 01:17:51PM +0000, Lad, Prabhakar wrote:
> > > On Thu, Mar 19, 2020 at 1:04 PM Laurent Pinchart wrote:
> > > > On Thu, Mar 19, 2020 at 01:44:52PM +0100, Maxime Ripard wrote:
> > > > > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > > > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > > > > clock-frequency property as deprecated.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > > index 72ad992..e62fe82 100644
> > > > > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > > @@ -8,7 +8,7 @@ Required Properties:
> > > > > > - compatible: Value should be "ovti,ov5645".
> > > > > > - clocks: Reference to the xclk clock.
> > > > > > - clock-names: Should be "xclk".
> > > > > > -- clock-frequency: Frequency of the xclk clock.
> > > > > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > > > > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > > > > to the hardware pin PWDNB which is physically active low.
> > > > > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > > > > @@ -37,7 +37,8 @@ Example:
> > > > > >
> > > > > > clocks = <&clks 200>;
> > > > > > clock-names = "xclk";
> > > > > > - clock-frequency = <24000000>;
> > > > > > + assigned-clocks = <&clks 200>;
> > > > > > + assigned-clock-rates = <24000000>;
> > > > > >
> > > > > > vdddo-supply = <&camera_dovdd_1v8>;
> > > > > > vdda-supply = <&camera_avdd_2v8>;
> > > > >
> > > > > clock-frequency is quite different from assigned-clock-rates though,
> > > > > semantically speaking. clock-frequency is only about what the clock
> > > > > frequency is, while assigned-clock-rates will change the rate as well,
> > > > > and you have no idea how long it will last.
> > > >
> > > > The driver currently reads the clock-frequency property and then calls
> > > > clk_set_rate(). I agree tht assigned-clock-rates isn't a panacea, but I
> > > > think it's less of a hack than what we currently have.
> > > >
> > > > As discussed on IRC, maybe the best option in this specific case is to
> > > > drop clock-frequency and assigned-clok-rates, and call clk_set_rate()
> > > > with a hardcoded frequency of 24MHz in the driver, as that's the only
> > > > frequency the driver supports.
> > > >
> > > Does this mean any driver which has a fixed clock requirement shouldn't be a
> > > DT property and should be just handled by the drivers internally ?
> >
> > It's hard to give a generic policy, but here, the hardware is pretty
> > flexible since it can deal with anything between 6MHz to 50-something
> > MHz, it's the driver that chooses to enforce a 24MHz and be pedantic
> > about it, so it's up to the driver to enforce that policy, not to the
> > DT since it's essentially a software limitation, not a hardware one.
>
> Thank you for the clarification, Ill drop patches 1-4 from this series.

That's the whole series... :-) I think you should keep patch 1/4 but
just remove the clock-frequency from the bindings, then remove it from
the DT files, and patch the driver to set the clock rate to 24MHz
unconditionally in patch 4/4.

--
Regards,

Laurent Pinchart

2020-03-25 21:53:11

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates

Hi Laurent,

On Tue, Mar 24, 2020 at 4:12 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Tue, Mar 24, 2020 at 04:04:43PM +0000, Lad, Prabhakar wrote:
> > On Tue, Mar 24, 2020 at 3:40 PM Maxime Ripard <[email protected]> wrote:
> > > On Thu, Mar 19, 2020 at 01:17:51PM +0000, Lad, Prabhakar wrote:
> > > > On Thu, Mar 19, 2020 at 1:04 PM Laurent Pinchart wrote:
> > > > > On Thu, Mar 19, 2020 at 01:44:52PM +0100, Maxime Ripard wrote:
> > > > > > On Fri, Mar 13, 2020 at 09:12:31PM +0000, Lad Prabhakar wrote:
> > > > > > > Use assigned-clock-rates to specify the clock rate. Also mark
> > > > > > > clock-frequency property as deprecated.
> > > > > > >
> > > > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > > > > ---
> > > > > > > Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
> > > > > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > > > index 72ad992..e62fe82 100644
> > > > > > > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > > > > > > @@ -8,7 +8,7 @@ Required Properties:
> > > > > > > - compatible: Value should be "ovti,ov5645".
> > > > > > > - clocks: Reference to the xclk clock.
> > > > > > > - clock-names: Should be "xclk".
> > > > > > > -- clock-frequency: Frequency of the xclk clock.
> > > > > > > +- clock-frequency (deprecated): Frequency of the xclk clock.
> > > > > > > - enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > > > > > > to the hardware pin PWDNB which is physically active low.
> > > > > > > - reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > > > > > > @@ -37,7 +37,8 @@ Example:
> > > > > > >
> > > > > > > clocks = <&clks 200>;
> > > > > > > clock-names = "xclk";
> > > > > > > - clock-frequency = <24000000>;
> > > > > > > + assigned-clocks = <&clks 200>;
> > > > > > > + assigned-clock-rates = <24000000>;
> > > > > > >
> > > > > > > vdddo-supply = <&camera_dovdd_1v8>;
> > > > > > > vdda-supply = <&camera_avdd_2v8>;
> > > > > >
> > > > > > clock-frequency is quite different from assigned-clock-rates though,
> > > > > > semantically speaking. clock-frequency is only about what the clock
> > > > > > frequency is, while assigned-clock-rates will change the rate as well,
> > > > > > and you have no idea how long it will last.
> > > > >
> > > > > The driver currently reads the clock-frequency property and then calls
> > > > > clk_set_rate(). I agree tht assigned-clock-rates isn't a panacea, but I
> > > > > think it's less of a hack than what we currently have.
> > > > >
> > > > > As discussed on IRC, maybe the best option in this specific case is to
> > > > > drop clock-frequency and assigned-clok-rates, and call clk_set_rate()
> > > > > with a hardcoded frequency of 24MHz in the driver, as that's the only
> > > > > frequency the driver supports.
> > > > >
> > > > Does this mean any driver which has a fixed clock requirement shouldn't be a
> > > > DT property and should be just handled by the drivers internally ?
> > >
> > > It's hard to give a generic policy, but here, the hardware is pretty
> > > flexible since it can deal with anything between 6MHz to 50-something
> > > MHz, it's the driver that chooses to enforce a 24MHz and be pedantic
> > > about it, so it's up to the driver to enforce that policy, not to the
> > > DT since it's essentially a software limitation, not a hardware one.
> >
> > Thank you for the clarification, Ill drop patches 1-4 from this series.
>
> That's the whole series... :-) I think you should keep patch 1/4 but
> just remove the clock-frequency from the bindings, then remove it from
> the DT files, and patch the driver to set the clock rate to 24MHz
> unconditionally in patch 4/4.
>
My bad I was referring to v4 series patch 5/5 which converts dt
bindings to json schema.
I'll shall post a v5 as suggested above.

Cheers,
--Prabhakar

> --
> Regards,
>
> Laurent Pinchart