[v3,1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates
diff mbox series

Message ID 1584133954-6953-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com
State New
Delegated to: Kieran Bingham
Headers show
Series
  • ov5645: Switch to assigned-clock-rates
Related show

Commit Message

Lad Prabhakar March 13, 2020, 9:12 p.m. UTC
Use assigned-clock-rates to specify the clock rate. Also mark
clock-frequency property as deprecated.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 Documentation/devicetree/bindings/media/i2c/ov5645.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart March 13, 2020, 9:20 p.m. UTC | #1
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  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 <laurent.pinchart@ideasonboard.com>

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>;
Lad Prabhakar March 13, 2020, 9:25 p.m. UTC | #2
Hi Laurent,

Thank you for the quick review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 13 March 2020 21:20
> To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Rob Herring
> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Sakari
> Ailus <sakari.ailus@linux.intel.com>; NXP Linux Team <linux-imx@nxp.com>;
> Magnus Damm <magnus.damm@gmail.com>; Ezequiel Garcia
> <ezequiel@collabora.com>; Geert Uytterhoeven <geert@linux-m68k.org>;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Fabio Estevam <festevam@gmail.com>; linux-
> media@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> 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-
> lad.rj@bp.renesas.com>
> > ---
> >  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 <laurent.pinchart@ideasonboard.com>
>
> While at it, can I enlist you to convert these bindings to yaml ? :-)
>
Sure will do the honours 
Laurent Pinchart March 13, 2020, 9:27 p.m. UTC | #3
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  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 <laurent.pinchart@ideasonboard.com>
> >
> > While at it, can I enlist you to convert these bindings to yaml ? :-)
> >
> Sure will do the honours 
Lad, Prabhakar March 18, 2020, 8:13 p.m. UTC | #4
Hi Laurent,

On Fri, Mar 13, 2020 at 9:27 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > >  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 <laurent.pinchart@ideasonboard.com>
> > >
> > > 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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Laurent Pinchart March 18, 2020, 10:33 p.m. UTC | #5
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>> ---
> >>>>  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 <laurent.pinchart@ideasonboard.com>
> >>>
> >>> 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>;
Lad, Prabhakar March 18, 2020, 10:37 p.m. UTC | #6
Hi Laurent,

On Wed, Mar 18, 2020 at 10:33 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >>>> ---
> > >>>>  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 <laurent.pinchart@ideasonboard.com>
> > >>>
> > >>> 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
Maxime Ripard March 19, 2020, 12:44 p.m. UTC | #7
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  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
Laurent Pinchart March 19, 2020, 1:03 p.m. UTC | #8
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  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.
Lad, Prabhakar March 19, 2020, 1:05 p.m. UTC | #9
Hi Maxime,

Thank you for the review.

On Thu, Mar 19, 2020 at 12:45 PM Maxime Ripard <maxime@cerno.tech> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Lad, Prabhakar March 19, 2020, 1:17 p.m. UTC | #10
Hi Laurent,

On Thu, Mar 19, 2020 at 1:04 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Maxime Ripard March 24, 2020, 3:40 p.m. UTC | #11
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
> <laurent.pinchart@ideasonboard.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > >  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
Lad, Prabhakar March 24, 2020, 4:04 p.m. UTC | #12
Hi Maxime,

On Tue, Mar 24, 2020 at 3:40 PM Maxime Ripard <maxime@cerno.tech> 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
> > <laurent.pinchart@ideasonboard.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > ---
> > > > >  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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Laurent Pinchart March 24, 2020, 4:12 p.m. UTC | #13
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 <maxime@cerno.tech> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > ---
> > > > > >  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.
Lad, Prabhakar March 25, 2020, 9:52 p.m. UTC | #14
Hi Laurent,

On Tue, Mar 24, 2020 at 4:12 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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 <maxime@cerno.tech> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > > ---
> > > > > > >  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

Patch
diff mbox series

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>;