linux-kernel - Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor 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] [day] [month] [year] [list]

Date:	Sun, 12 Jun 2016 19:30:04 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Kevin Tsai <capellamicro@...il.com>
Cc:	Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Jonathan Cameron <jic23@...nel.org>,
	Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Guenter Roeck <linux@...ck-us.net>,
	Jiri Slaby <jslaby@...e.cz>,
	Daniel Baluta <daniel.baluta@...el.com>,
	Adriana Reus <adriana.reus@...el.com>,
	Andreas Dannenberg <dannenberg@...com>,
	Matt Ranostay <mranostay@...il.com>,
	Crestez Dan Leonard <leonard.crestez@...el.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	Anatol Pomazau <anatol@...gle.com>
Subject: Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver.

On Thu, Jun 9, 2016 at 11:00 AM, Kevin Tsai <capellamicro@...il.com> wrote:

> V2:
> Thanks commends from Peter Meerwald-Stadler, Jonathan Cameron, and Linux
> Walleij.

Haha that again, everybody does that.

> Updated for the following:
>  - Remove unused defines.
>  - Rewrite event registration.
>  - Remove direct register values in the device tree.
>  - Rewrite by regmap API.
>  - Remove irq module parameter.
>  - Correct cm36672_remove().

Awesome. Minor comments still:

> +Required properties:
> +- compatible: must be capella,cm36672"
> +- reg: the I2C slave address, usually 0x60
> +- interrupts: interrupt mapping for GPIO IRQ

Don't say GPIO IRQ. Some chips have hardwired external interrupt lines
rather than GPIOs and those should work just as fine. Does it support
level, edge etc IRQs?

> +Optional properties:
> +- interrupt-parent: interrupt controller
> +- interrupts: interrupt mapping for GPIO IRQ

That is mentioned again, I guess it is not optional and should be removed
from here?

> +- cm36672,prx_led_current: LED current, must be one of the following values:
> +    - 0: 50mA
> +    - 1: 75mA
> +    - 2: 100mA
> +    - 3: 120mA
> +    - 4: 140mA
> +    - 5: 160mA
> +    - 6: 180mA
> +    - 7: 200mA

Do not specify enumerators. Specify milliamps in the device tree
and have the code translate it to enumerators. DTS should be
human-readable and easy to understand without reading tables.

Just use vendor prefix and something you seen before like:
capella,driver-strength = <160>;
For 160mA

> +- cm36672,prx_hd: width of proximity sensor output data, must be one of the
> +  following values:
> +    - 0: 12-bit
> +    - 1: 16-bit

Use
capella,bus-width = <12>;
The SD cards have this pattern for bus widths.

Yours,
Linus Walleij