Re: [PATCH v3 1/2] lis3: add generic DT matching code

[Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 06.08.2012 06:52, Rob Herring wrote:
> On 08/05/2012 11:18 AM, Daniel Mack wrote:
>> Ping, anyone?
>>
>> On 30.07.2012 09:36, Daniel Mack wrote:
>>> This patch adds logic to parse lis3 properties from a device tree node
>>> and store them in a freshly allocated lis3lv02d_platform_data.
>>>
>>> Note that the actual match tables are left out here. This part should
>>> happen in the drivers that bind to the individual busses (SPI/I2C/PCI).
>>>
>>> Also adds some DT bindinds documentation.
>>>
>>> Signed-off-by: Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> Changes from v2:
>>>  - kzalloc braino
>>>
>>> Changes from v1:
>>>  - some typos in properties fixed
>>>
>>>
>>>  Documentation/devicetree/bindings/misc/lis302.txt |  74 ++++++++++++
>>>  drivers/misc/lis3lv02d/lis3lv02d.c                | 137 ++++++++++++++++++++++
>>>  drivers/misc/lis3lv02d/lis3lv02d.h                |   4 +
>>>  3 files changed, 215 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/misc/lis302.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/lis302.txt b/Documentation/devicetree/bindings/misc/lis302.txt
>>> new file mode 100644
>>> index 0000000..66230fd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/lis302.txt
>>> @@ -0,0 +1,74 @@
>>> +LIS302 accelerometer devicetree bindings
>>> +
>>> +This device is matched via its bus drivers, and has a number of properties
>>> +that apply in on the generic device (independent from the bus).
>>> +
>>> +
>>> +Required properties for the SPI bindings:
>>> + - compatible: 		should be set to "st,lis3lv02d_spi"
>>> + - reg:			the chipselect index
>>> + - spi-max-frequency:	maximal bus speed, should be set to 1000000 unless
>>> +			constrained by external circuitry
>>> + - interrupts:		the interrupt generated by the device
>>> +
>>> +
>>> +Optional properties for all bus drivers:
>>> +
>>> + - st,click-single-{x,y,z}:	if present, tells the device to issue an
>>> +				interrupt on single click events on the
>>> +				x/y/z axis.
>>> + - st,click-double-{x,y,z}:	if present, tells the device to issue an
>>> +				interrupt on double click events on the
>>> +				x/y/z axis.
>>> + - st,click-thresh-{x,y,z}:	set the x/y/z axis threshold
>>> + - st,click-click-time-limit:	click time limit, from 0 to 127.5msec
>>> +				with step of 0.5 msec
>>> + - st,click-latency:		click latency, from 0 to 255 msec with
>>> +				step of 1 msec.
>>> + - st,click-window:		click window, from 0 to 255 msec with
>>> +				step of 1 msec.
> 
> These should append "-msec" to define what units they are in.
> 
>>> + - st,irq{1,2}-disable:		disable IRQ 1/2
>>> + - st,irq{1,2}-ff-wu-1:		raise IRQ 1/2 on FF_WU_1 condition
>>> + - st,irq{1,2}-ff-wu-2:		raise IRQ 1/2 on FF_WU_2 condition
>>> + - st,irq{1,2}-data-ready:	raise IRQ 1/2 on data ready contition
>>> + - st,irq{1,2}-click:		raise IRQ 1/2 on click condition
>>> + - st,irq-open-drain:		consider IRQ lines open-drain
>>> + - st,irq-active-low:		make IRQ lines active low
>>> + - st,wu-duration-1:		duration register for Free-Fall/Wake-Up
>>> +				interrupt 1
>>> + - st,wu-duration-2:		duration register for Free-Fall/Wake-Up
>>> +				interrupt 2
>>> + - st,wakeup-{x,y,z}-{lo,hi}:	set wakeup condition on x/y/z axis for
>>> +				upper/lower limit
>>> + - st,highpass-cutoff-hz=:	1, 2, 4 or 8 for 1Hz, 2Hz, 4Hz or 8Hz of
>>> +				highpass cut-off frequency
>>> + - st,hipass{1,2}-disable:	disable highpass 1/2.
>>> + - st,default-rate=:		set the default rate
>>> + - st,axis-{x,y,z}=:		set the axis to map to the three coordinates
> 
> Wow, that's a lot of properties...

Yes, but that reflects the hardware's capabilities :)

>>> +
>>> +
>>> +Example for a SPI device node:
>>> +
>>> +	lis302@0 {
>>> +		compatible = "st,lis302dl-spi";
>>> +		reg = <0>;
>>> +		spi-max-frequency = <1000000>;
>>> +		interrupt-parent = <&gpio>;
>>> +		interrupts = <104 0>;
>>> +
>>> +		st,click-single-x;
>>> +		st,click-single-y;
>>> +		st,click-single-z;
>>> +		st,click-thresh-x = <10>;
>>> +		st,click-thresh-y = <10>;
>>> +		st,click-thresh-z = <10>;
>>> +		st,irq1-click;
>>> +		st,irq2-click;
>>> +		st,wakeup-x-lo;
>>> +		st,wakeup-x-hi;
>>> +		st,wakeup-y-lo;
>>> +		st,wakeup-y-hi;
>>> +		st,wakeup-z-lo;
>>> +		st,wakeup-z-hi;
>>> +	};
>>> +
>>> diff --git a/drivers/misc/lis3lv02d/lis3lv02d.c b/drivers/misc/lis3lv02d/lis3lv02d.c
>>> index a981e2a..1411fdc 100644
>>> --- a/drivers/misc/lis3lv02d/lis3lv02d.c
>>> +++ b/drivers/misc/lis3lv02d/lis3lv02d.c
>>> @@ -39,6 +39,7 @@
>>>  #include <linux/miscdevice.h>
>>>  #include <linux/pm_runtime.h>
>>>  #include <linux/atomic.h>
>>> +#include <linux/of_device.h>
>>>  #include "lis3lv02d.h"
>>>  
>>>  #define DRIVER_NAME     "lis3lv02d"
>>> @@ -912,6 +913,138 @@ static void lis3lv02d_8b_configure(struct lis3lv02d *lis3,
>>>  	}
>>>  }
>>>  
>>> +#ifdef CONFIG_OF
>>> +static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
>>> +{
>>> +	struct lis3lv02d_platform_data *pdata;
>>> +	struct device_node *np = lis3->of_node;
>>> +	u32 tmp;
>>> +
>>> +	if (!lis3->of_node)
>>> +		return 0;
>>> +
>>> +	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
>>> +	if (!pdata)
>>> +		return -ENOMEM;
>>> +
>>> +	if (of_get_property(np, "st,click-single-x", NULL))
>>> +		pdata->click_flags |= LIS3_CLICK_SINGLE_X;
>>> +	if (of_get_property(np, "st,click-double-x", NULL))
>>> +		pdata->click_flags |= LIS3_CLICK_DOUBLE_X;
>>> +
>>> +	if (of_get_property(np, "st,click-single-y", NULL))
>>> +		pdata->click_flags |= LIS3_CLICK_SINGLE_Y;
>>> +	if (of_get_property(np, "st,click-double-y", NULL))
>>> +		pdata->click_flags |= LIS3_CLICK_DOUBLE_Y;
>>> +
>>> +	if (of_get_property(np, "st,click-single-z", NULL))
>>> +		pdata->click_flags |= LIS3_CLICK_SINGLE_Z;
>>> +	if (of_get_property(np, "st,click-double-z", NULL))
>>> +		pdata->click_flags |= LIS3_CLICK_DOUBLE_Z;
>>> +
>>> +	if (!of_property_read_u32(np, "st,click-threshold-x", &tmp))
>>> +		pdata->click_thresh_x = tmp;
>>> +	if (!of_property_read_u32(np, "st,click-threshold-y", &tmp))
>>> +		pdata->click_thresh_y = tmp;
>>> +	if (!of_property_read_u32(np, "st,click-threshold-z", &tmp))
>>> +		pdata->click_thresh_z = tmp;
>>> +
>>> +	if (!of_property_read_u32(np, "st,click-time-limit", &tmp))
>>> +		pdata->click_time_limit = tmp;
> 
> These can be just:
> 
> 	of_property_read_u32(np, "st,click-time-limit", &pdata->click_time_limit);

Yes, but pdata->click_time_limit isn't an u32, which causes a build
warning. We might consider introducing helpers to solve this more
generically.


Daniel