Re: [PATCH 2/4] watchdog: loongson1: Add Loongson-7A watchdog support

["Rong Zhang" <rongrong@oss.cipunited.com>]

Hi Guenter,

On Mon, 2026-11-09 at 23:47 -0800, Guenter Roeck wrote:
> On 11/9/26 23:02, Rong Zhang wrote:
> > The Loongson-7A watchdog is similar to Loongson 2K0300, except that it
> > has no clock controller.
> > 
> > Retrieve clock frequency from OF to make it work.
> > 
> > Tested on Loongson-LS3A4000-7A1000-NUC-SE and Lenovo KaiTian M530Z.
> > 
> > Signed-off-by: Rong Zhang <rongrong@oss.cipunited.com>
> > ---
> >   drivers/watchdog/loongson1_wdt.c | 29 +++++++++++++++++++++++++----
> >   1 file changed, 25 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/watchdog/loongson1_wdt.c b/drivers/watchdog/loongson1_wdt.c
> > index 2811e7eab631..57b9e699c131 100644
> > --- a/drivers/watchdog/loongson1_wdt.c
> > +++ b/drivers/watchdog/loongson1_wdt.c
> > @@ -31,6 +31,7 @@ struct ls1x_wdt_pdata {
> >   	u32 timer_offset;
> >   	u32 set_offset;
> >   	u32 wdt_en_bit;
> > +	bool no_clk;
> >   };
> >   
> >   static const struct ls1x_wdt_pdata ls1b_wdt_pdata = {
> > @@ -45,6 +46,13 @@ static const struct ls1x_wdt_pdata ls2k0300_wdt_pdata = {
> >   	.wdt_en_bit = BIT(1),
> >   };
> >   
> > +static const struct ls1x_wdt_pdata ls7a_wdt_pdata = {
> > +	.timer_offset = 0x8,
> > +	.set_offset = 0x4,
> > +	.wdt_en_bit = BIT(1),
> > +	.no_clk = true,
> > +};
> > +
> >   struct ls1x_wdt_drvdata {
> >   	void __iomem *base;
> >   	struct clk *clk;
> > @@ -129,6 +137,8 @@ static int ls1x_wdt_probe(struct platform_device *pdev)
> >   	struct ls1x_wdt_drvdata *drvdata;
> >   	struct watchdog_device *ls1x_wdt;
> >   	unsigned long clk_rate;
> > +	u32 freq;
> > +	int ret;
> >   
> >   	drvdata = devm_kzalloc_obj(dev, *drvdata);
> >   	if (!drvdata)
> > @@ -136,16 +146,26 @@ static int ls1x_wdt_probe(struct platform_device *pdev)
> >   	platform_set_drvdata(pdev, drvdata);
> >   
> >   	drvdata->pdata = of_device_get_match_data(dev);
> > +	if (!drvdata->pdata)
> > +		return -ENODEV;
> >   
> >   	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
> >   	if (IS_ERR(drvdata->base))
> >   		return PTR_ERR(drvdata->base);
> >   
> > -	drvdata->clk = devm_clk_get_enabled(dev, NULL);
> > -	if (IS_ERR(drvdata->clk))
> > -		return PTR_ERR(drvdata->clk);
> > +	if (drvdata->pdata->no_clk) {
> > +		ret = of_property_read_u32(dev->of_node, "clock-frequency", &freq);
> > +		if (ret)
> > +			return ret;
> > +
> > +		clk_rate = freq;
> > +	} else {
> > +		drvdata->clk = devm_clk_get_enabled(dev, NULL);
> > +		if (IS_ERR(drvdata->clk))
> > +			return PTR_ERR(drvdata->clk);
> >   
> 
> Wouldn't it be much easier to just create a fixed rate clock and use it
> (as suggested by Sashiko in the devicetree patch) ?

Makes sense. I will take that approach (and fix other concerns found by
Sashiko) when I resubmit this.

Thanks,
Rong

> 
> Guenter
> 
> > -	clk_rate = clk_get_rate(drvdata->clk);
> > +		clk_rate = clk_get_rate(drvdata->clk);
> > +	}
> >   	if (!clk_rate)
> >   		return -EINVAL;
> >   	drvdata->clk_rate = clk_rate;
> > @@ -191,6 +211,7 @@ static const struct of_device_id ls1x_wdt_dt_ids[] = {
> >   	{ .compatible = "loongson,ls1b-wdt", .data = &ls1b_wdt_pdata },
> >   	{ .compatible = "loongson,ls1c-wdt", .data = &ls1b_wdt_pdata },
> >   	{ .compatible = "loongson,ls2k0300-wdt", .data = &ls2k0300_wdt_pdata },
> > +	{ .compatible = "loongson,ls7a-wdt", .data = &ls7a_wdt_pdata },
> >   	{ /* sentinel */ }
> >   };
> >   MODULE_DEVICE_TABLE(of, ls1x_wdt_dt_ids);
> >