Hi Biju,
On Tue, Nov 9, 2021 at 9:22 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [RFC 4/4] watchdog: Add Watchdog Timer driver for RZ/G2L
> > On Mon, Nov 8, 2021 at 7:38 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > > On 11/4/21 9:08 AM, Biju Das wrote:
> > > > Add Watchdog Timer driver for RZ/G2L SoC.
> > > >
> > > > WDT IP block supports normal watchdog timer function and reset
> > > > request function due to CPU parity error.
> > > >
> > > > This driver currently supports normal watchdog timer function and
> > > > later will add support for reset request function due to CPU parity
> > > > error.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > > > --- /dev/null
> > > > +++ b/drivers/watchdog/rzg2l_wdt.c
> >
> > > > +struct rzg2l_wdt_priv {
> > > > + void __iomem *base;
> > > > + struct watchdog_device wdev;
> > > > + struct reset_control *rstc;
> > > > + unsigned long osc_clk_rate;
> > > > + unsigned long pclk_rate;
> > >
> > > pclk_rate is only used in the probe function and thus not needed here.
> >
> > Indeed...
> OK.
> >
> > > > +static int rzg2l_wdt_probe(struct platform_device *pdev) {
> > > > + struct device *dev = &pdev->dev;
> > > > + struct rzg2l_wdt_priv *priv;
> > > > + struct clk *wdt_clk;
> > > > + int ret;
> > > > +
> > > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > > + if (!priv)
> > > > + return -ENOMEM;
> > > > +
> > > > + priv->base = devm_platform_ioremap_resource(pdev, 0);
> > > > + if (IS_ERR(priv->base))
> > > > + return PTR_ERR(priv->base);
> > > > +
> > > > + /* Get watchdog main clock */
> > > > + wdt_clk = devm_clk_get(&pdev->dev, "oscclk");
> > > > + if (IS_ERR(wdt_clk))
> > > > + return dev_err_probe(&pdev->dev, PTR_ERR(wdt_clk), "no
> > > > + oscclk");
> > > > +
> > > > + priv->osc_clk_rate = clk_get_rate(wdt_clk);
> > > > + if (!priv->osc_clk_rate)
> > > > + return dev_err_probe(&pdev->dev, -EINVAL, "oscclk rate
> > > > + is 0");
> > > > +
> > > > + /* Get Peripheral clock */
> > > > + wdt_clk = devm_clk_get(&pdev->dev, "pclk");
> > > > + if (IS_ERR(wdt_clk))
> > > > + return dev_err_probe(&pdev->dev, PTR_ERR(wdt_clk), "no
> > > > + pclk");
> > > > +
> > > > + priv->pclk_rate = clk_get_rate(wdt_clk);
> > > > + if (!priv->pclk_rate)
> > > > + return dev_err_probe(&pdev->dev, -EINVAL, "pclk rate
> > > > + is 0");
> >
> > ... and this can't really happen, can it?
>
> But I need pclk frequency for delay calculation. That is the reason I am doing a get. Probably after
> Getting the rate, I should do a "put". So that Run time PM will be in full control for the clocks.
> Same for oscillator clk. Is it ok?
Oops, I missed that. Please ignore my comment, I'll grab a coffee
soon ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
