linux-kernel - Re: [PATCH v4] watchdog: Add MOXA ART watchdog driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201307201840.52455.arnd@arndb.de>
Date:	Sat, 20 Jul 2013 18:40:52 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Jonas Jensen <jonas.jensen@...il.com>
Cc:	linux-watchdog@...r.kernel.org, wim@...ana.be,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	arm@...nel.org, linux@...ck-us.net
Subject: Re: [PATCH v4] watchdog: Add MOXA ART watchdog driver

On Friday 19 July 2013, Jonas Jensen wrote:
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..5507f2b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Should be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for the MOXA ART core clock "coreclk"
> +
> +Example:
> +
> +       watchdog: watchdog@...00000 {
> +               compatible = "moxa,moxart-watchdog";
> +               reg = <0x98500000 0x10>;
> +               clocks = <&coreclk>;
> +       };

I think the property descriptions need to be clarified here:

* I think "should" makes no sense for the "compatible" property since it is
  required here, you probably mean "must", see http://www.ietf.org/rfc/rfc2119.txt. 

* for the clock, make sure that the description is from the point of view of
  the device you are describing, not from the perspective of the clock controller.
  The device should not make any assumptions about who provides a clock, only
  what it is used for, and it sounds like "coreclk" is an SoC-wide identifier,
  not the name of the clock input of the watchdog device. This is not as important
  as you don't define a "clock-names" property here, butsomething to keep in
  mind anyway.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/