Messages in this thread | | | From | Prabhakar Mahadev Lad <> | Subject | RE: [PATCH 2/2] media: i2c: ov5645: Switch to assigned-clock-rates | Date | Tue, 10 Mar 2020 08:57:42 +0000 |
| |
Hi Sakari,
Thank you for the review.
> -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: 10 March 2020 08:50 > To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>; Rob Herring > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Shawn > Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com>; NXP Linux Team <linux-imx@nxp.com>; Kieran > Bingham <kieran.bingham+renesas@ideasonboard.com>; linux- > media@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Lad > Prabhakar <prabhakar.csengg@gmail.com> > Subject: Re: [PATCH 2/2] media: i2c: ov5645: Switch to assigned-clock-rates > > Hi Prabhakar, > > On Mon, Mar 09, 2020 at 11:46:13AM +0000, Lad Prabhakar wrote: > > This patch switches to assigned-clock-rates for specifying the clock rate. > > The clk-conf.c internally handles setting the clock rate, as a result > > setting the clk rate from the driver is dropped. > > > > Correspondingly imx6qdl-wandboard.dtsi which references to ov5645 has > > been updated to use assigned-clock-rates in the same patch to avoid > > bisect failures. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev- > lad.rj@bp.renesas.com> > > --- > > arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 3 ++- > > drivers/media/i2c/ov5645.c | 9 ++------- > > 2 files changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi > > b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi > > index c070893..71f5f75 100644 > > --- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi > > +++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi > > @@ -126,7 +126,8 @@ > > reg = <0x3c>; > > clocks = <&clks IMX6QDL_CLK_CKO2>; > > clock-names = "xclk"; > > -clock-frequency = <24000000>; > > +assigned-clocks = <&clks IMX6QDL_CLK_CKO2>; > > +assigned-clock-rates = <24000000>; > > vdddo-supply = <®_1p8v>; > > vdda-supply = <®_2p8v>; > > vddd-supply = <®_1p5v>; > > Shouldn't this be a separate patch? > I did think about it, but realized it might break bisect, but with below changes this can be a separate patch now.
> > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c > > index a6c17d1..2aa2677 100644 > > --- a/drivers/media/i2c/ov5645.c > > +++ b/drivers/media/i2c/ov5645.c > > @@ -1094,7 +1094,8 @@ static int ov5645_probe(struct i2c_client *client) > > return PTR_ERR(ov5645->xclk); > > } > > > > -ret = of_property_read_u32(dev->of_node, "clock-frequency", > &xclk_freq); > > +ret = of_property_read_u32(dev->of_node, "assigned-clock-rates", > > + &xclk_freq); > > I think you'd still need to check for clock-frequency to be compatible with > existing DT binaries. > Makes sense, I will add the check for it and if clock-frequency is specified Ill let the driver set the clock.
Cheers, --Prabhakar
> > if (ret) { > > dev_err(dev, "could not get xclk frequency\n"); > > return ret; > > @@ -1107,12 +1108,6 @@ static int ov5645_probe(struct i2c_client *client) > > return -EINVAL; > > } > > > > -ret = clk_set_rate(ov5645->xclk, xclk_freq); > > -if (ret) { > > -dev_err(dev, "could not set xclk frequency\n"); > > -return ret; > > -} > > - > > for (i = 0; i < OV5645_NUM_SUPPLIES; i++) > > ov5645->supplies[i].supply = ov5645_supply_name[i]; > > > > -- > Regards, > > Sakari Ailus
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
| |