[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <DB7PR04MB519538CC79789CB6DF65B9B2E2890@DB7PR04MB5195.eurprd04.prod.outlook.com>
Date: Thu, 19 Sep 2019 03:53:33 +0000
From: Wen He <wen.he_1@....com>
To: Stephen Boyd <sboyd@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
"linux-devel@...ux.nxdi.nxp.com" <linux-devel@...ux.nxdi.nxp.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Mark Rutland <mark.rutland@....com>,
Michael Turquette <mturquette@...libre.com>,
Rob Herring <robh+dt@...nel.org>
CC: Leo Li <leoyang.li@....com>,
"liviu.dudau@....com" <liviu.dudau@....com>
Subject: RE: [EXT] Re: [v4 2/2] clk: ls1028a: Add clock driver for Display
output interface
> -----Original Message-----
> From: Stephen Boyd <sboyd@...nel.org>
> Sent: 2019年9月19日 1:01
> To: devicetree@...r.kernel.org; linux-clk@...r.kernel.org;
> linux-devel@...ux.nxdi.nxp.com; linux-kernel@...r.kernel.org; Mark Rutland
> <mark.rutland@....com>; Michael Turquette <mturquette@...libre.com>;
> Rob Herring <robh+dt@...nel.org>; Wen He <wen.he_1@....com>
> Cc: Leo Li <leoyang.li@....com>; liviu.dudau@....com
> Subject: RE: [EXT] Re: [v4 2/2] clk: ls1028a: Add clock driver for Display output
> interface
>
>
> Quoting Wen He (2019-09-18 02:20:26)
> > > -----Original Message-----
> > > From: Stephen Boyd <sboyd@...nel.org> Quoting Wen He (2019-08-29
> > > 03:59:19)
> > > > diff --git a/drivers/clk/clk-plldig.c b/drivers/clk/clk-plldig.c
> > > > new file mode 100644 index 000000000000..d3239bcf59de
> > > > --- /dev/null
> > > > +++ b/drivers/clk/clk-plldig.c
> > > > @@ -0,0 +1,298 @@
> [...]
> >
> > >
> > > > +
> > > > +/* Maximum of the divider */
> > > > +#define MAX_RFDPHI1 63
> > > > +
> > > > +/* Best value of multiplication factor divider */
> > > > +#define PLLDIG_DEFAULE_MULT 44
> > > > +
> > > > +/*
> > > > + * Clock configuration relationship between the PHI1
> > > > +frequency(fpll_phi) and
> > > > + * the output frequency of the PLL is determined by the PLLDV,
> > > > +according to
> > > > + * the following equation:
> > > > + * fpll_phi = (pll_ref * mfd) / div_rfdphi1 */ struct
> > > > +plldig_phi1_param {
> > > > + unsigned long rate;
> > > > + unsigned int rfdphi1;
> > > > + unsigned int mfd;
> > > > +};
> > > > +
> > > > +enum plldig_phi1_freq_range {
> > > > + PHI1_MIN = 27000000U,
> > > > + PHI1_MAX = 600000000U
> > > > +};
> > >
> > > Please just inline these values in the one place they're used.
> > >
> > > > +
> > > > +struct clk_plldig {
> > > > + struct clk_hw hw;
> > > > + void __iomem *regs;
> > > > + struct device *dev;
> > >
> > > Please remove this, it is unused.
> >
> > It is used for probe.
>
> Use a local variable and don't store it away forever in the struct.
>
Understand, will remove it.
> > >
> > > > +
> > > > + val = readl(data->regs + PLLDIG_REG_PLLDV);
> > > > + val = phi1_param.mfd;
> > > > + rfdphi1 = phi1_param.rfdphi1;
> > > > + val |= rfdphi1;
> > > > +
> > > > + writel(val, data->regs + PLLDIG_REG_PLLDV);
> > > > +
> > > > + /* delay 200us make sure that old lock state is cleared */
> > > > + udelay(200);
> > > > +
> > > > + /* Wait until PLL is locked or timeout (maximum 1000 usecs) */
> > > > + ret = readl_poll_timeout_atomic(data->regs +
> > > > + PLLDIG_REG_PLLSR,
> > > cond,
> > > > + cond &
> PLLDIG_LOCK_MASK,
> > > 0,
> > > > + USEC_PER_MSEC);
> > > > +
> > > > + return ret;
> > >
> > > Just return readl_poll_timeout_atomic(...) here.
> >
> > Maybe use below code will to best describes.
> >
> > If (ret)
> > return -ETIMEOUT;
> >
> > return 0;
>
> No, just return readl_poll_timeout_atomic().
Understand, I will send next version patch for this.
Thanks && Best Regards,
Wen
Powered by blists - more mailing lists