Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1420303

Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver.

From Linus Walleij <linus.walleij@linaro.org>
Newsgroups linux.kernel
Subject Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver.
Date 2016-06-12 19:40 +0200
Message-ID <rJhwB-36U-5@gated-at.bofh.it> (permalink)
References <rI48p-4md-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Jun 9, 2016 at 11:00 AM, Kevin Tsai <capellamicro@gmail.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

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. Kevin Tsai <capellamicro@gmail.com> - 2016-06-09 11:10 +0200
  Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. Anatol Pomazau <anatol@google.com> - 2016-06-09 16:10 +0200
    Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. Jonathan Cameron <jic23@kernel.org> - 2016-06-11 13:20 +0200
  Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. "Andrew F. Davis" <afd@ti.com> - 2016-06-09 18:00 +0200
  Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. Rob Herring <robh@kernel.org> - 2016-06-10 18:30 +0200
  Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. Jonathan Cameron <jic23@kernel.org> - 2016-06-11 13:40 +0200
  Re: [PATCH V2 1/1] iio: light: Added CM36672 Proximity Sensor Driver. Linus Walleij <linus.walleij@linaro.org> - 2016-06-12 19:40 +0200

csiph-web