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