RE: [PATCH v4 1/3] watchdog: add rza_wdt driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thursday, March 02, 2017, Guenter Roeck wrote:
> > > > > The rate check should probably be here to avoid situations where
> > > > > rate < 16384.
> > > >
> > > > Do I need that if it's technically not possible to have a 'rate'
> > > > less
> > > than 25MHz?
> > > >
> > > > These watchdogs HW are always feed directly from the peripheral
> > > > clock and there is no such thing as a 16kHz peripheral block an
> > > > any Renesas
> > > SoC.
> > > >
> > > Following that line of argument, can clk_get_rate() ever return 0 ?
> >
> > In the DT binding, it says that a clock source is required to be present.
> >
> > If the user leaves out the "clocks =", then devm_clk_get will fail.
> >
> > If the user puts in some crazy value for "clocks = ", then maybe you
> > could get
> > 0 (assuming there is a valid clock node they made by themselves
> > somewhere that runs at 0Hz).
> > But in that extreme case, I think they deserve to have it crash and
> > burn because who knows what they are doing.
> >
> 
> But then there could also be a clock source with a rate of less than 16
> kHz, as wrong as it may be ?
> 
> Anyway, I disagree about the crash and burn. It isn't as if this would be
> really fatal except for the watchdog driver. Bad data in devicetree should
> not result in a system crash.

OK. I will put the check in. Something like:

	rate = clk_get_rate(priv->clk); 
	if (rate < 16384) {
		dev_err(&pdev->dev, "invalid clock specified\n");
		return -ENOENT;
	}


> > > That is the point of devm_ functions. It also means that you won't
> > > need a remove function if you also use devm_watchdog_register_device().
> >
> > OK.
> > I see that only 1 driver is using devm_watchdog_register_device
> > (wdat_wdt.c), so maybe that is a new method.
> >
> Yes, it is quite new. Still, you are a bit behind. I count 19 users in the
> mainline kernel.

OK, I see now.


Thank you,
Chris
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux