Re: [PATCH 2/2] DT: Add binding documentation for CM36651 proximity/light sensor

[Beomho Seo <beomho.seo@samsung.com>]

Thank you for review.
I revised on your advice.
I will send v3 patch in the course of today.

reply about your question inline

> On Mon, Sep 09, 2013 at 07:10:41AM +0100, Beomho Seo wrote:
>> This patch add device tree binding documentation for CM36651
>> proximity/light sensor.
>>
>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>> ---
>>  .../devicetree/bindings/iio/light/cm36651.txt      |   24
>> ++++++++++++++++++++
>>  .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>>  2 files changed, 25 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/light/cm36651.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/cm36651.txt
>> b/Documentation/devicetree/bindings/iio/light/cm36651.txt
>> new file mode 100644
>> index 0000000..3a6ba38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/light/cm36651.txt
>> @@ -0,0 +1,24 @@
>> +* Capella CM36651 I2C Proximity and Color Light sensor
>> +
>> +Required properties:
>> +- compatible: must be "capella,cm36651"
>> +- reg: the I2C address of the device
>> +- interrupts: interrupt to which the chip is connected
> 
> Nit: I'd prefer something like the below:
> 
> - interrupts: interrupt-specifier for the sole interrupt generated by
>               the device.
> 
>> +- vled-supply: regulator for the sensor device, as covered
>> +	       in ../../regulator/regulator.txt
> 
> I assume by the name that this is the regulator for the LED that's used
> alongside the sensor, rather than for the sensor itself?
> 
> Is the LED part of the CM36651 unit, or is it external? I couldn't find
> any documentation for the unit.
> 

vled_suppy is the regulator for the IR LED. In order to cm36651 manual, IR_LED part of the cm36651 unit.

>> +
>> +Example:
>> +
>> +	i2c_cm36651: i2c-gpio {
>> +		/* ... */
>> +
>> +		cm36651@18 {
>> +			compatible = "capella,cm36651";
>> +			reg = <0x18>;
>> +			interrupt-parent = <&gpx0>;
>> +			interrupts = <2 0>;
>> +			vled-supply = <&ps_als_reg>;
>> +		};
>> +
>> +		/* ... */
>> +	};
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index ec4d713..044796d 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -17,6 +17,7 @@ brcm	Broadcom Corporation
>>  cavium	Cavium, Inc.
>>  chrp	Common Hardware Reference Platform
>>  cirrus	Cirrus Logic, Inc.
>> +capella	Capella Microsystems, Inc
> 
> The vendor prefix looks OK to me. However, it should apper before cavium
> to maintain the ordering of the file.
> 
> Thanks,
> Mark.
> 
>>  cortina	Cortina Systems, Inc.
>>  dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
>>  davicom	DAVICOM Semiconductor, Inc.
>> -- 
>> 1.7.9.5
>>
>> -- 
>> Beomho Seo, Assistant Engineer
>> System S/W Lab., S/W Platform Team, Software Center
>> Samsung Electronics
>>
> 


-- 
Best Regards,

Beomho Seo, Assistant Engineer
System S/W Lab., S/W Platform Team, Software Center
Samsung Electronics