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

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
To: Maxime Ripard <maxime@cerno.tech>,
	Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Fabio Estevam <festevam@gmail.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Magnus Damm <magnus.damm@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	linux-media <linux-media@vger.kernel.org>,
	LAK <linux-arm-kernel@lists.infradead.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH v3 1/4] media: dt-bindings: media: i2c: Switch to assigned-clock-rates
Date: Thu, 19 Mar 2020 13:05:37 +0000	[thread overview]
Message-ID: <CA+V-a8vH+wJoC1SL0nuk8ypAH3Mosd+K1dLNDA5wwgP4W6A=CQ@mail.gmail.com> (raw)
In-Reply-To: <20200319124452.3yfcvq754vi4q2rv@gilmour.lan>

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