linux-kernel - Re: [PATCH v3 1/2] ARM: at91/dt: shdwc binding: add new shutdown controller documentation

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] [day] [month] [year] [list]

Date:	Mon, 7 Mar 2016 15:31:47 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	<robh+dt@...nel.org>, <mark.rutland@....com>,
	<ijc+devicetree@...lion.org.uk>, Pawel Moll <pawel.moll@....com>
CC:	<linux-arm-kernel@...ts.infradead.org>, <sre@...nel.org>,
	<linux-kernel@...r.kernel.org>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	<linux@...ck-us.net>,
	"Jean-Christophe PLAGNIOL-VILLARD" <plagnioj@...osoft.com>
Subject: Re: [PATCH v3 1/2] ARM: at91/dt: shdwc binding: add new shutdown
 controller documentation

Le 03/10/2015 19:02, Alexandre Belloni a écrit :
> On 30/09/2015 at 18:22:00 +0200, Nicolas Ferre wrote :
>> +SHDWC Shutdown Controller (Alternative)
>> +
>> +1) shdwc node
>> +
>> +required properties:
>> +- compatible: should be "atmel,sama5d2-shdwc".
>> +- reg: should contain registers location and length
>> +- clocks: phandle to input clock.
>> +- #address-cells: should be one. The cell is the wake-up input index.
>> +- #size-cells: should be zero.
>> +
>> +optional properties:
>> +
>> +- atmel,wakeup-debouncer: minimum wake-up inputs debouncer period in
> 
> Shouldn't that property be called atmel,wakeup-debouncer-ms ?

Well, as it's microseconds, it should be "-us", according to the file:
Documentation/devicetree/bindings/property-units.txt.

Reworking on this I realised that a kind "generic" property exists for
this type: "debounce-delay-ms", without a vendor prefix.

So, for the DT people, what do you think if I change my private property
to this one that seems to be used in a bunch of bindings already in its
milliseconds variant:
gpio-matrix-keypad.txt, tc3589x.txt, extcon-palmas.txt or with a vendor
prefix in nvidia,tegra20-kbc.txt or lpc32xx-key.txt

and if I diversify it to a "debounce-delay-us" variant and use it in
this driver?

>> +
>> +2) input nodes
>> +
>> +Wake-up input nodes are usually described in the "board" part of the Device
>> +Tree. Note also that input 0 is linked to the wake-up pin and is frequently
>> +used.
>> +
>> +Required properties:
>> +- reg: should contain the wake-up input index [0 - 15].
>> +
>> +Optional properties:
>> +- atmel,wakeup-type: string, operation mode of the input described by the child
>> +  node. Supported values are: "high" or "low".
>> +
> 
> Maybe we could avoid parsing string and use an integer with a few defines

Yes, even a boolean should do the trick. What about:

- atmel,wakeup-active-high: boolean, the corresponding wake-up
  input described by the child, forces the wake-up of the core power
  supply on a high level. The default is to be active low.

Thanks for your insights. Best regards,
-- 
Nicolas Ferre