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

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

[Daniel Mack]

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@gmail.com>
---
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.
+ - 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
+
+
+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;
+	if (!of_property_read_u32(np, "st,click-latency", &tmp))
+		pdata->click_latency = tmp;
+	if (!of_property_read_u32(np, "st,click-window", &tmp))
+		pdata->click_window = tmp;
+
+	if (of_get_property(np, "st,irq1-disable", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
+	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
+	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
+	if (of_get_property(np, "st,irq1-data-ready", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
+	if (of_get_property(np, "st,irq1-click", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
+
+	if (of_get_property(np, "st,irq2-disable", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
+	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
+	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
+	if (of_get_property(np, "st,irq2-data-ready", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
+	if (of_get_property(np, "st,irq2-click", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
+
+	if (of_get_property(np, "st,irq-open-drain", NULL))
+		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
+	if (of_get_property(np, "st,irq-active-low", NULL))
+		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
+
+	if (!of_property_read_u32(np, "st,wu-duration-1", &tmp))
+		pdata->duration1 = tmp;
+	if (!of_property_read_u32(np, "st,wu-duration-2", &tmp))
+		pdata->duration2 = tmp;
+
+	if (of_get_property(np, "st,wakeup-x-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
+	if (of_get_property(np, "st,wakeup-x-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
+	if (of_get_property(np, "st,wakeup-y-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
+	if (of_get_property(np, "st,wakeup-y-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
+	if (of_get_property(np, "st,wakeup-z-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
+	if (of_get_property(np, "st,wakeup-z-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
+
+	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &tmp)) {
+		switch (tmp) {
+		case 1:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
+			break;
+		case 2:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
+			break;
+		case 4:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
+			break;
+		case 8:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
+			break;
+		}
+	}
+
+	if (of_get_property(np, "st,hipass1-disable", NULL))
+		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
+	if (of_get_property(np, "st,hipass2-disable", NULL))
+		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
+
+	if (of_get_property(np, "st,axis-x", NULL))
+		pdata->axis_x = tmp;
+	if (of_get_property(np, "st,axis-y", NULL))
+		pdata->axis_y = tmp;
+	if (of_get_property(np, "st,axis-z", NULL))
+		pdata->axis_z = tmp;
+
+	if (of_get_property(np, "st,default-rate", NULL))
+		pdata->default_rate = tmp;
+
+	lis3->pdata = pdata;
+
+	return 0;
+}
+
+#else
+static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
+{
+	return 0;
+}
+#endif
+
 /*
  * Initialise the accelerometer and the various subsystems.
  * Should be rather independent of the bus system.
@@ -922,6 +1055,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
 	irq_handler_t thread_fn;
 	int irq_flags = 0;
 
+	err = lis3lv02d_init_dt(lis3);
+	if (err < 0)
+		return err;
+
 	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
 
 	switch (lis3->whoami) {
diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
index 2b1482a..a296f1d 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d.h
+++ b/drivers/misc/lis3lv02d/lis3lv02d.h
@@ -282,6 +282,10 @@ struct lis3lv02d {
 
 	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
 	struct mutex		mutex;     /* Serialize poll and selftest */
+
+#ifdef CONFIG_OF
+	struct device_node	*of_node;
+#endif
 };
 
 int lis3lv02d_init_device(struct lis3lv02d *lis3);
-- 
1.7.11.2

[PATCH v3 2/2] lis3-spi: add DT matching table passthru code

[Daniel Mack]

If probed from a device tree, this driver now passes the node
information to the generic part, so the runtime information can be
derived.

Successfully tested on a PXA3xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---

No changes from v1 and v2

 drivers/misc/lis3lv02d/lis3lv02d_spi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/misc/lis3lv02d/lis3lv02d_spi.c b/drivers/misc/lis3lv02d/lis3lv02d_spi.c
index 80880e9..8616054 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d_spi.c
+++ b/drivers/misc/lis3lv02d/lis3lv02d_spi.c
@@ -17,6 +17,7 @@
 #include <linux/workqueue.h>
 #include <linux/spi/spi.h>
 #include <linux/pm.h>
+#include <linux/of_platform.h>
 
 #include "lis3lv02d.h"
 
@@ -58,6 +59,12 @@ static int lis3_spi_init(struct lis3lv02d *lis3)
 static union axis_conversion lis3lv02d_axis_normal =
 	{ .as_array = { 1, 2, 3 } };
 
+static struct of_device_id lis302dl_spi_dt_ids[] = {
+	{ .compatible = "st,lis302dl-spi" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, lis302dl_spi_dt_ids);
+
 static int __devinit lis302dl_spi_probe(struct spi_device *spi)
 {
 	int ret;
@@ -75,6 +82,12 @@ static int __devinit lis302dl_spi_probe(struct spi_device *spi)
 	lis3_dev.irq		= spi->irq;
 	lis3_dev.ac		= lis3lv02d_axis_normal;
 	lis3_dev.pdata		= spi->dev.platform_data;
+
+#ifdef CONFIG_OF
+	if (of_match_device(lis302dl_spi_dt_ids, &spi->dev))
+		lis3_dev.of_node = spi->dev.of_node;
+#endif
+
 	spi_set_drvdata(spi, &lis3_dev);
 
 	return lis3lv02d_init_device(&lis3_dev);
@@ -121,6 +134,7 @@ static struct spi_driver lis302dl_spi_driver = {
 		.name   = DRV_NAME,
 		.owner  = THIS_MODULE,
 		.pm	= &lis3lv02d_spi_pm,
+		.of_match_table = of_match_ptr(lis302dl_spi_dt_ids),
 	},
 	.probe	= lis302dl_spi_probe,
 	.remove	= __devexit_p(lis302dl_spi_remove),
-- 
1.7.11.2

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

[Daniel Mack]

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@gmail.com>
> ---
> 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.
> + - 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
> +
> +
> +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;
> +	if (!of_property_read_u32(np, "st,click-latency", &tmp))
> +		pdata->click_latency = tmp;
> +	if (!of_property_read_u32(np, "st,click-window", &tmp))
> +		pdata->click_window = tmp;
> +
> +	if (of_get_property(np, "st,irq1-disable", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
> +	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
> +	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
> +	if (of_get_property(np, "st,irq1-data-ready", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
> +	if (of_get_property(np, "st,irq1-click", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
> +
> +	if (of_get_property(np, "st,irq2-disable", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
> +	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
> +	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
> +	if (of_get_property(np, "st,irq2-data-ready", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
> +	if (of_get_property(np, "st,irq2-click", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
> +
> +	if (of_get_property(np, "st,irq-open-drain", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
> +	if (of_get_property(np, "st,irq-active-low", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
> +
> +	if (!of_property_read_u32(np, "st,wu-duration-1", &tmp))
> +		pdata->duration1 = tmp;
> +	if (!of_property_read_u32(np, "st,wu-duration-2", &tmp))
> +		pdata->duration2 = tmp;
> +
> +	if (of_get_property(np, "st,wakeup-x-lo", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
> +	if (of_get_property(np, "st,wakeup-x-hi", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
> +	if (of_get_property(np, "st,wakeup-y-lo", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
> +	if (of_get_property(np, "st,wakeup-y-hi", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
> +	if (of_get_property(np, "st,wakeup-z-lo", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
> +	if (of_get_property(np, "st,wakeup-z-hi", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
> +
> +	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &tmp)) {
> +		switch (tmp) {
> +		case 1:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
> +			break;
> +		case 2:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
> +			break;
> +		case 4:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
> +			break;
> +		case 8:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
> +			break;
> +		}
> +	}
> +
> +	if (of_get_property(np, "st,hipass1-disable", NULL))
> +		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
> +	if (of_get_property(np, "st,hipass2-disable", NULL))
> +		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
> +
> +	if (of_get_property(np, "st,axis-x", NULL))
> +		pdata->axis_x = tmp;
> +	if (of_get_property(np, "st,axis-y", NULL))
> +		pdata->axis_y = tmp;
> +	if (of_get_property(np, "st,axis-z", NULL))
> +		pdata->axis_z = tmp;
> +
> +	if (of_get_property(np, "st,default-rate", NULL))
> +		pdata->default_rate = tmp;
> +
> +	lis3->pdata = pdata;
> +
> +	return 0;
> +}
> +
> +#else
> +static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * Initialise the accelerometer and the various subsystems.
>   * Should be rather independent of the bus system.
> @@ -922,6 +1055,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
>  	irq_handler_t thread_fn;
>  	int irq_flags = 0;
>  
> +	err = lis3lv02d_init_dt(lis3);
> +	if (err < 0)
> +		return err;
> +
>  	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
>  
>  	switch (lis3->whoami) {
> diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
> index 2b1482a..a296f1d 100644
> --- a/drivers/misc/lis3lv02d/lis3lv02d.h
> +++ b/drivers/misc/lis3lv02d/lis3lv02d.h
> @@ -282,6 +282,10 @@ struct lis3lv02d {
>  
>  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
>  	struct mutex		mutex;     /* Serialize poll and selftest */
> +
> +#ifdef CONFIG_OF
> +	struct device_node	*of_node;
> +#endif
>  };
>  
>  int lis3lv02d_init_device(struct lis3lv02d *lis3);
> 

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

[Daniel Mack]

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.
> + - 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
> +
> +
> +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;
> +	if (!of_property_read_u32(np, "st,click-latency", &tmp))
> +		pdata->click_latency = tmp;
> +	if (!of_property_read_u32(np, "st,click-window", &tmp))
> +		pdata->click_window = tmp;
> +
> +	if (of_get_property(np, "st,irq1-disable", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
> +	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
> +	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
> +	if (of_get_property(np, "st,irq1-data-ready", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
> +	if (of_get_property(np, "st,irq1-click", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
> +
> +	if (of_get_property(np, "st,irq2-disable", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
> +	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
> +	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
> +	if (of_get_property(np, "st,irq2-data-ready", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
> +	if (of_get_property(np, "st,irq2-click", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
> +
> +	if (of_get_property(np, "st,irq-open-drain", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
> +	if (of_get_property(np, "st,irq-active-low", NULL))
> +		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
> +
> +	if (!of_property_read_u32(np, "st,wu-duration-1", &tmp))
> +		pdata->duration1 = tmp;
> +	if (!of_property_read_u32(np, "st,wu-duration-2", &tmp))
> +		pdata->duration2 = tmp;
> +
> +	if (of_get_property(np, "st,wakeup-x-lo", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
> +	if (of_get_property(np, "st,wakeup-x-hi", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
> +	if (of_get_property(np, "st,wakeup-y-lo", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
> +	if (of_get_property(np, "st,wakeup-y-hi", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
> +	if (of_get_property(np, "st,wakeup-z-lo", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
> +	if (of_get_property(np, "st,wakeup-z-hi", NULL))
> +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
> +
> +	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &tmp)) {
> +		switch (tmp) {
> +		case 1:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
> +			break;
> +		case 2:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
> +			break;
> +		case 4:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
> +			break;
> +		case 8:
> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
> +			break;
> +		}
> +	}
> +
> +	if (of_get_property(np, "st,hipass1-disable", NULL))
> +		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
> +	if (of_get_property(np, "st,hipass2-disable", NULL))
> +		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
> +
> +	if (of_get_property(np, "st,axis-x", NULL))
> +		pdata->axis_x = tmp;
> +	if (of_get_property(np, "st,axis-y", NULL))
> +		pdata->axis_y = tmp;
> +	if (of_get_property(np, "st,axis-z", NULL))
> +		pdata->axis_z = tmp;
> +
> +	if (of_get_property(np, "st,default-rate", NULL))
> +		pdata->default_rate = tmp;
> +
> +	lis3->pdata = pdata;
> +
> +	return 0;
> +}
> +
> +#else
> +static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * Initialise the accelerometer and the various subsystems.
>   * Should be rather independent of the bus system.
> @@ -922,6 +1055,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
>  	irq_handler_t thread_fn;
>  	int irq_flags = 0;
>  
> +	err = lis3lv02d_init_dt(lis3);
> +	if (err < 0)
> +		return err;
> +
>  	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
>  
>  	switch (lis3->whoami) {
> diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
> index 2b1482a..a296f1d 100644
> --- a/drivers/misc/lis3lv02d/lis3lv02d.h
> +++ b/drivers/misc/lis3lv02d/lis3lv02d.h
> @@ -282,6 +282,10 @@ struct lis3lv02d {
>  
>  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
>  	struct mutex		mutex;     /* Serialize poll and selftest */
> +
> +#ifdef CONFIG_OF
> +	struct device_node	*of_node;
> +#endif
>  };
>  
>  int lis3lv02d_init_device(struct lis3lv02d *lis3);
> 

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

[Rob Herring]

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@gmail.com>
>> ---
>> 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...

>> +
>> +
>> +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);

The value is unchanged on error.

Rob

>> +	if (!of_property_read_u32(np, "st,click-latency", &tmp))
>> +		pdata->click_latency = tmp;
>> +	if (!of_property_read_u32(np, "st,click-window", &tmp))
>> +		pdata->click_window = tmp;
>> +
>> +	if (of_get_property(np, "st,irq1-disable", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
>> +	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
>> +	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
>> +	if (of_get_property(np, "st,irq1-data-ready", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
>> +	if (of_get_property(np, "st,irq1-click", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
>> +
>> +	if (of_get_property(np, "st,irq2-disable", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
>> +	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
>> +	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
>> +	if (of_get_property(np, "st,irq2-data-ready", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
>> +	if (of_get_property(np, "st,irq2-click", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
>> +
>> +	if (of_get_property(np, "st,irq-open-drain", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
>> +	if (of_get_property(np, "st,irq-active-low", NULL))
>> +		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
>> +
>> +	if (!of_property_read_u32(np, "st,wu-duration-1", &tmp))
>> +		pdata->duration1 = tmp;
>> +	if (!of_property_read_u32(np, "st,wu-duration-2", &tmp))
>> +		pdata->duration2 = tmp;
>> +
>> +	if (of_get_property(np, "st,wakeup-x-lo", NULL))
>> +		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
>> +	if (of_get_property(np, "st,wakeup-x-hi", NULL))
>> +		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
>> +	if (of_get_property(np, "st,wakeup-y-lo", NULL))
>> +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
>> +	if (of_get_property(np, "st,wakeup-y-hi", NULL))
>> +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
>> +	if (of_get_property(np, "st,wakeup-z-lo", NULL))
>> +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
>> +	if (of_get_property(np, "st,wakeup-z-hi", NULL))
>> +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
>> +
>> +	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &tmp)) {
>> +		switch (tmp) {
>> +		case 1:
>> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
>> +			break;
>> +		case 2:
>> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
>> +			break;
>> +		case 4:
>> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
>> +			break;
>> +		case 8:
>> +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (of_get_property(np, "st,hipass1-disable", NULL))
>> +		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
>> +	if (of_get_property(np, "st,hipass2-disable", NULL))
>> +		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
>> +
>> +	if (of_get_property(np, "st,axis-x", NULL))
>> +		pdata->axis_x = tmp;
>> +	if (of_get_property(np, "st,axis-y", NULL))
>> +		pdata->axis_y = tmp;
>> +	if (of_get_property(np, "st,axis-z", NULL))
>> +		pdata->axis_z = tmp;
>> +
>> +	if (of_get_property(np, "st,default-rate", NULL))
>> +		pdata->default_rate = tmp;
>> +
>> +	lis3->pdata = pdata;
>> +
>> +	return 0;
>> +}
>> +
>> +#else
>> +static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>>  /*
>>   * Initialise the accelerometer and the various subsystems.
>>   * Should be rather independent of the bus system.
>> @@ -922,6 +1055,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
>>  	irq_handler_t thread_fn;
>>  	int irq_flags = 0;
>>  
>> +	err = lis3lv02d_init_dt(lis3);
>> +	if (err < 0)
>> +		return err;
>> +
>>  	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
>>  
>>  	switch (lis3->whoami) {
>> diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
>> index 2b1482a..a296f1d 100644
>> --- a/drivers/misc/lis3lv02d/lis3lv02d.h
>> +++ b/drivers/misc/lis3lv02d/lis3lv02d.h
>> @@ -282,6 +282,10 @@ struct lis3lv02d {
>>  
>>  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
>>  	struct mutex		mutex;     /* Serialize poll and selftest */
>> +
>> +#ifdef CONFIG_OF
>> +	struct device_node	*of_node;
>> +#endif
>>  };
>>  
>>  int lis3lv02d_init_device(struct lis3lv02d *lis3);
>>
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

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

[Daniel Mack]

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@gmail.com>
>>> ---
>>> 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

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

[Daniel Mack]

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

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

[AnilKumar, Chimata]

On Sun, Aug 05, 2012 at 21:48:42, 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@gmail.com>
> > ---
> > 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.
> > + - 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

Some more parameters missing, what about st_min_limits and st_max_limits
required for selftest.

> > +
> > +
> > +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;
> > +	};

Why can't we add these flags in driver itself like below?
Is these parameters varies from SoC to SoC or accelerometer
- to - accelerometer?

#ifdef CONFIG_OF
static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
        .click_flags    = LIS3_CLICK_SINGLE_X |
                          LIS3_CLICK_SINGLE_Y |
                          LIS3_CLICK_SINGLE_Z,
        .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
        .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
                          LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
                          LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
};

static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
        .click_flags    = LIS3_CLICK_SINGLE_X |
                          LIS3_CLICK_SINGLE_Y |
                          LIS3_CLICK_SINGLE_Z,
        .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
        .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
                          LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
                          LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
};

static const struct of_device_id lis3_of_match[] = {
       {
               .compatible = "st,lis302dl-spi",
               .data = &lis302dl_spi_pdata,
       },
       {
               .compatible = "st,lis331dlh-i2c",
               .data = &lis331dlh_i2c_pdata,
       },
       { },
};
MODULE_DEVICE_TABLE(of, lis3_of_match);
#endif

Ignore if parameters between SoC - SoC are different. In
probe we can add these flags to pdata.

> > +
> > 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;

Can you use some better name than tmp, if require?

> > +
> > +	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;
> > +	if (!of_property_read_u32(np, "st,click-latency", &tmp))
> > +		pdata->click_latency = tmp;
> > +	if (!of_property_read_u32(np, "st,click-window", &tmp))
> > +		pdata->click_window = tmp;
> > +
> > +	if (of_get_property(np, "st,irq1-disable", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
> > +	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
> > +	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
> > +	if (of_get_property(np, "st,irq1-data-ready", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
> > +	if (of_get_property(np, "st,irq1-click", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
> > +
> > +	if (of_get_property(np, "st,irq2-disable", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
> > +	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
> > +	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
> > +	if (of_get_property(np, "st,irq2-data-ready", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
> > +	if (of_get_property(np, "st,irq2-click", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
> > +
> > +	if (of_get_property(np, "st,irq-open-drain", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
> > +	if (of_get_property(np, "st,irq-active-low", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
> > +
> > +	if (!of_property_read_u32(np, "st,wu-duration-1", &tmp))
> > +		pdata->duration1 = tmp;
> > +	if (!of_property_read_u32(np, "st,wu-duration-2", &tmp))
> > +		pdata->duration2 = tmp;
> > +
> > +	if (of_get_property(np, "st,wakeup-x-lo", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
> > +	if (of_get_property(np, "st,wakeup-x-hi", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
> > +	if (of_get_property(np, "st,wakeup-y-lo", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
> > +	if (of_get_property(np, "st,wakeup-y-hi", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
> > +	if (of_get_property(np, "st,wakeup-z-lo", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
> > +	if (of_get_property(np, "st,wakeup-z-hi", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
> > +
> > +	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &tmp)) {
> > +		switch (tmp) {
> > +		case 1:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
> > +			break;
> > +		case 2:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
> > +			break;
> > +		case 4:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
> > +			break;
> > +		case 8:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (of_get_property(np, "st,hipass1-disable", NULL))
> > +		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
> > +	if (of_get_property(np, "st,hipass2-disable", NULL))
> > +		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
> > +
> > +	if (of_get_property(np, "st,axis-x", NULL))

&tmp missed here.

> > +		pdata->axis_x = tmp;
> > +	if (of_get_property(np, "st,axis-y", NULL))

&tmp missed here.
> > +		pdata->axis_y = tmp;
> > +	if (of_get_property(np, "st,axis-z", NULL))
> > +		pdata->axis_z = tmp;
> > +
> > +	if (of_get_property(np, "st,default-rate", NULL))
> > +		pdata->default_rate = tmp;
> > +
> > +	lis3->pdata = pdata;
> > +
> > +	return 0;
> > +}
> > +
> > +#else
> > +static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > +
> >  /*
> >   * Initialise the accelerometer and the various subsystems.
> >   * Should be rather independent of the bus system.
> > @@ -922,6 +1055,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
> >  	irq_handler_t thread_fn;
> >  	int irq_flags = 0;
> >  
> > +	err = lis3lv02d_init_dt(lis3);
> > +	if (err < 0)
> > +		return err;
> > +
> >  	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
> >  
> >  	switch (lis3->whoami) {
> > diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
> > index 2b1482a..a296f1d 100644
> > --- a/drivers/misc/lis3lv02d/lis3lv02d.h
> > +++ b/drivers/misc/lis3lv02d/lis3lv02d.h
> > @@ -282,6 +282,10 @@ struct lis3lv02d {
> >  
> >  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
> >  	struct mutex		mutex;     /* Serialize poll and selftest */
> > +
> > +#ifdef CONFIG_OF
> > +	struct device_node	*of_node;
> > +#endif
> >  };
> >  
> >  int lis3lv02d_init_device(struct lis3lv02d *lis3);
> > 
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

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

[AnilKumar, Chimata]

On Sun, Aug 05, 2012 at 21:48:42, 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.
> > + - 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

Some more parameters missing, what about st_min_limits and st_max_limits
required for selftest.

> > +
> > +
> > +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;
> > +	};

Why can't we add these flags in driver itself like below?
Is these parameters varies from SoC to SoC or accelerometer
- to - accelerometer?

#ifdef CONFIG_OF
static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
        .click_flags    = LIS3_CLICK_SINGLE_X |
                          LIS3_CLICK_SINGLE_Y |
                          LIS3_CLICK_SINGLE_Z,
        .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
        .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
                          LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
                          LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
};

static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
        .click_flags    = LIS3_CLICK_SINGLE_X |
                          LIS3_CLICK_SINGLE_Y |
                          LIS3_CLICK_SINGLE_Z,
        .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
        .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
                          LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
                          LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
};

static const struct of_device_id lis3_of_match[] = {
       {
               .compatible = "st,lis302dl-spi",
               .data = &lis302dl_spi_pdata,
       },
       {
               .compatible = "st,lis331dlh-i2c",
               .data = &lis331dlh_i2c_pdata,
       },
       { },
};
MODULE_DEVICE_TABLE(of, lis3_of_match);
#endif

Ignore if parameters between SoC - SoC are different. In
probe we can add these flags to pdata.

> > +
> > 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;

Can you use some better name than tmp, if require?

> > +
> > +	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;
> > +	if (!of_property_read_u32(np, "st,click-latency", &tmp))
> > +		pdata->click_latency = tmp;
> > +	if (!of_property_read_u32(np, "st,click-window", &tmp))
> > +		pdata->click_window = tmp;
> > +
> > +	if (of_get_property(np, "st,irq1-disable", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
> > +	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
> > +	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
> > +	if (of_get_property(np, "st,irq1-data-ready", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
> > +	if (of_get_property(np, "st,irq1-click", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
> > +
> > +	if (of_get_property(np, "st,irq2-disable", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
> > +	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
> > +	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
> > +	if (of_get_property(np, "st,irq2-data-ready", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
> > +	if (of_get_property(np, "st,irq2-click", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
> > +
> > +	if (of_get_property(np, "st,irq-open-drain", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
> > +	if (of_get_property(np, "st,irq-active-low", NULL))
> > +		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
> > +
> > +	if (!of_property_read_u32(np, "st,wu-duration-1", &tmp))
> > +		pdata->duration1 = tmp;
> > +	if (!of_property_read_u32(np, "st,wu-duration-2", &tmp))
> > +		pdata->duration2 = tmp;
> > +
> > +	if (of_get_property(np, "st,wakeup-x-lo", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
> > +	if (of_get_property(np, "st,wakeup-x-hi", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
> > +	if (of_get_property(np, "st,wakeup-y-lo", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
> > +	if (of_get_property(np, "st,wakeup-y-hi", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
> > +	if (of_get_property(np, "st,wakeup-z-lo", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
> > +	if (of_get_property(np, "st,wakeup-z-hi", NULL))
> > +		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
> > +
> > +	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &tmp)) {
> > +		switch (tmp) {
> > +		case 1:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
> > +			break;
> > +		case 2:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
> > +			break;
> > +		case 4:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
> > +			break;
> > +		case 8:
> > +			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (of_get_property(np, "st,hipass1-disable", NULL))
> > +		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
> > +	if (of_get_property(np, "st,hipass2-disable", NULL))
> > +		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
> > +
> > +	if (of_get_property(np, "st,axis-x", NULL))

&tmp missed here.

> > +		pdata->axis_x = tmp;
> > +	if (of_get_property(np, "st,axis-y", NULL))

&tmp missed here.
> > +		pdata->axis_y = tmp;
> > +	if (of_get_property(np, "st,axis-z", NULL))
> > +		pdata->axis_z = tmp;
> > +
> > +	if (of_get_property(np, "st,default-rate", NULL))
> > +		pdata->default_rate = tmp;
> > +
> > +	lis3->pdata = pdata;
> > +
> > +	return 0;
> > +}
> > +
> > +#else
> > +static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > +
> >  /*
> >   * Initialise the accelerometer and the various subsystems.
> >   * Should be rather independent of the bus system.
> > @@ -922,6 +1055,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
> >  	irq_handler_t thread_fn;
> >  	int irq_flags = 0;
> >  
> > +	err = lis3lv02d_init_dt(lis3);
> > +	if (err < 0)
> > +		return err;
> > +
> >  	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
> >  
> >  	switch (lis3->whoami) {
> > diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
> > index 2b1482a..a296f1d 100644
> > --- a/drivers/misc/lis3lv02d/lis3lv02d.h
> > +++ b/drivers/misc/lis3lv02d/lis3lv02d.h
> > @@ -282,6 +282,10 @@ struct lis3lv02d {
> >  
> >  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
> >  	struct mutex		mutex;     /* Serialize poll and selftest */
> > +
> > +#ifdef CONFIG_OF
> > +	struct device_node	*of_node;
> > +#endif
> >  };
> >  
> >  int lis3lv02d_init_device(struct lis3lv02d *lis3);
> > 
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

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

[Daniel Mack]

[-- Attachment #1: Type: text/plain, Size: 7438 bytes --]

Hi,

thanks for your review.

On 06.08.2012 12:45, AnilKumar, Chimata wrote:
> On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
>> 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@gmail.com>
>>> ---
>>> 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.
>>> + - 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
> 
> Some more parameters missing, what about st_min_limits and st_max_limits
> required for selftest.

Right. Added them now.

>>> +
>>> +
>>> +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;
>>> +	};
> 
> Why can't we add these flags in driver itself like below?
> Is these parameters varies from SoC to SoC or accelerometer
> - to - accelerometer?

I don't understand, sorry. The point here is that the driver that is
probed for device initialization are the PCI/I2C/SPI drivers. The
generic part is not something the device tree knows about.

Hence I put the generic parsing of common DT bindings to the generic
part of the driver, and made the SPI driver just pass through the
of_node pointer.

> #ifdef CONFIG_OF
> static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
>         .click_flags    = LIS3_CLICK_SINGLE_X |
>                           LIS3_CLICK_SINGLE_Y |
>                           LIS3_CLICK_SINGLE_Z,
>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> };
> 
> static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
>         .click_flags    = LIS3_CLICK_SINGLE_X |
>                           LIS3_CLICK_SINGLE_Y |
>                           LIS3_CLICK_SINGLE_Z,
>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> };
> 
> static const struct of_device_id lis3_of_match[] = {
>        {
>                .compatible = "st,lis302dl-spi",
>                .data = &lis302dl_spi_pdata,
>        },
>        {
>                .compatible = "st,lis331dlh-i2c",
>                .data = &lis331dlh_i2c_pdata,
>        },
>        { },
> };
> MODULE_DEVICE_TABLE(of, lis3_of_match);
> #endif
> 
> Ignore if parameters between SoC - SoC are different. In
> probe we can add these flags to pdata.

No. We want to expose all hardware features to DT so users can configure
the device at wish. We can't ignore that SoCs want different device configs.

>>> +
>>> 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;
> 
> Can you use some better name than tmp, if require?

Yes, done.

[...]

>>> +	if (of_get_property(np, "st,axis-x", NULL))
> 
> &tmp missed here.
> 
>>> +		pdata->axis_x = tmp;
>>> +	if (of_get_property(np, "st,axis-y", NULL))
> 
> &tmp missed here.

True, thanks.


I fixed all these issues now and attached a v4.


Best regards,
Daniel


[-- Attachment #2: 0001-lis3-add-generic-DT-matching-code.patch --]
[-- Type: text/x-patch, Size: 10195 bytes --]

>From 0926551982cb647f39cea4f4e904f4db2f435bd4 Mon Sep 17 00:00:00 2001
From: Daniel Mack <zonque@gmail.com>
Date: Mon, 23 Jul 2012 15:27:19 +0200
Subject: [PATCH v4 1/2] lis3: add generic DT matching code

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@gmail.com>
---
changes from v3:
 - fix 3 missing &tmp references in 3 calls
 - renamed 'tmp' to 'val'
 - added 'st,{min,max}-limit-{x,y,z}'

Changes from v2:
 - kzalloc braino

Changes from v1:
 - some typos in properties fixed

 Documentation/devicetree/bindings/misc/lis302.txt |  76 +++++++++++
 drivers/misc/lis3lv02d/lis3lv02d.c                | 152 ++++++++++++++++++++++
 drivers/misc/lis3lv02d/lis3lv02d.h                |   4 +
 3 files changed, 232 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..e18af9d
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/lis302.txt
@@ -0,0 +1,76 @@
+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.
+ - 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
+ - st,{min,max}-limit-{x,y,z}	set the min/max limits for x/y/z axis
+				(used by self-test)
+
+
+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..6292e8c 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,153 @@ 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 val;
+
+	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", &val))
+		pdata->click_thresh_x = val;
+	if (!of_property_read_u32(np, "st,click-threshold-y", &val))
+		pdata->click_thresh_y = val;
+	if (!of_property_read_u32(np, "st,click-threshold-z", &val))
+		pdata->click_thresh_z = val;
+
+	if (!of_property_read_u32(np, "st,click-time-limit", &val))
+		pdata->click_time_limit = val;
+	if (!of_property_read_u32(np, "st,click-latency", &val))
+		pdata->click_latency = val;
+	if (!of_property_read_u32(np, "st,click-window", &val))
+		pdata->click_window = val;
+
+	if (of_get_property(np, "st,irq1-disable", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
+	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
+	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
+	if (of_get_property(np, "st,irq1-data-ready", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
+	if (of_get_property(np, "st,irq1-click", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
+
+	if (of_get_property(np, "st,irq2-disable", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
+	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
+	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
+	if (of_get_property(np, "st,irq2-data-ready", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
+	if (of_get_property(np, "st,irq2-click", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
+
+	if (of_get_property(np, "st,irq-open-drain", NULL))
+		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
+	if (of_get_property(np, "st,irq-active-low", NULL))
+		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
+
+	if (!of_property_read_u32(np, "st,wu-duration-1", &val))
+		pdata->duration1 = val;
+	if (!of_property_read_u32(np, "st,wu-duration-2", &val))
+		pdata->duration2 = val;
+
+	if (of_get_property(np, "st,wakeup-x-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
+	if (of_get_property(np, "st,wakeup-x-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
+	if (of_get_property(np, "st,wakeup-y-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
+	if (of_get_property(np, "st,wakeup-y-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
+	if (of_get_property(np, "st,wakeup-z-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
+	if (of_get_property(np, "st,wakeup-z-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
+
+	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &val)) {
+		switch (val) {
+		case 1:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
+			break;
+		case 2:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
+			break;
+		case 4:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
+			break;
+		case 8:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
+			break;
+		}
+	}
+
+	if (of_get_property(np, "st,hipass1-disable", NULL))
+		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
+	if (of_get_property(np, "st,hipass2-disable", NULL))
+		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
+
+	if (of_get_property(np, "st,axis-x", &val))
+		pdata->axis_x = val;
+	if (of_get_property(np, "st,axis-y", &val))
+		pdata->axis_y = val;
+	if (of_get_property(np, "st,axis-z", &val))
+		pdata->axis_z = val;
+
+	if (of_get_property(np, "st,default-rate", NULL))
+		pdata->default_rate = val;
+
+	if (of_get_property(np, "st,min-limit-x", &val))
+		pdata->st_min_limits[0] = val;
+	if (of_get_property(np, "st,min-limit-y", &val))
+		pdata->st_min_limits[1] = val;
+	if (of_get_property(np, "st,min-limit-z", &val))
+		pdata->st_min_limits[2] = val;
+
+	if (of_get_property(np, "st,max-limit-x", &val))
+		pdata->st_max_limits[0] = val;
+	if (of_get_property(np, "st,max-limit-y", &val))
+		pdata->st_max_limits[1] = val;
+	if (of_get_property(np, "st,max-limit-z", &val))
+		pdata->st_max_limits[2] = val;
+
+
+	lis3->pdata = pdata;
+
+	return 0;
+}
+
+#else
+static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
+{
+	return 0;
+}
+#endif
+
 /*
  * Initialise the accelerometer and the various subsystems.
  * Should be rather independent of the bus system.
@@ -922,6 +1070,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
 	irq_handler_t thread_fn;
 	int irq_flags = 0;
 
+	err = lis3lv02d_init_dt(lis3);
+	if (err < 0)
+		return err;
+
 	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
 
 	switch (lis3->whoami) {
diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
index 2b1482a..a296f1d 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d.h
+++ b/drivers/misc/lis3lv02d/lis3lv02d.h
@@ -282,6 +282,10 @@ struct lis3lv02d {
 
 	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
 	struct mutex		mutex;     /* Serialize poll and selftest */
+
+#ifdef CONFIG_OF
+	struct device_node	*of_node;
+#endif
 };
 
 int lis3lv02d_init_device(struct lis3lv02d *lis3);
-- 
1.7.11.2

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

[Daniel Mack]

[-- Attachment #1: Type: text/plain, Size: 7438 bytes --]

Hi,

thanks for your review.

On 06.08.2012 12:45, AnilKumar, Chimata wrote:
> On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
>> 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@gmail.com>
>>> ---
>>> 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.
>>> + - 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
> 
> Some more parameters missing, what about st_min_limits and st_max_limits
> required for selftest.

Right. Added them now.

>>> +
>>> +
>>> +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;
>>> +	};
> 
> Why can't we add these flags in driver itself like below?
> Is these parameters varies from SoC to SoC or accelerometer
> - to - accelerometer?

I don't understand, sorry. The point here is that the driver that is
probed for device initialization are the PCI/I2C/SPI drivers. The
generic part is not something the device tree knows about.

Hence I put the generic parsing of common DT bindings to the generic
part of the driver, and made the SPI driver just pass through the
of_node pointer.

> #ifdef CONFIG_OF
> static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
>         .click_flags    = LIS3_CLICK_SINGLE_X |
>                           LIS3_CLICK_SINGLE_Y |
>                           LIS3_CLICK_SINGLE_Z,
>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> };
> 
> static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
>         .click_flags    = LIS3_CLICK_SINGLE_X |
>                           LIS3_CLICK_SINGLE_Y |
>                           LIS3_CLICK_SINGLE_Z,
>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> };
> 
> static const struct of_device_id lis3_of_match[] = {
>        {
>                .compatible = "st,lis302dl-spi",
>                .data = &lis302dl_spi_pdata,
>        },
>        {
>                .compatible = "st,lis331dlh-i2c",
>                .data = &lis331dlh_i2c_pdata,
>        },
>        { },
> };
> MODULE_DEVICE_TABLE(of, lis3_of_match);
> #endif
> 
> Ignore if parameters between SoC - SoC are different. In
> probe we can add these flags to pdata.

No. We want to expose all hardware features to DT so users can configure
the device at wish. We can't ignore that SoCs want different device configs.

>>> +
>>> 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;
> 
> Can you use some better name than tmp, if require?

Yes, done.

[...]

>>> +	if (of_get_property(np, "st,axis-x", NULL))
> 
> &tmp missed here.
> 
>>> +		pdata->axis_x = tmp;
>>> +	if (of_get_property(np, "st,axis-y", NULL))
> 
> &tmp missed here.

True, thanks.


I fixed all these issues now and attached a v4.


Best regards,
Daniel


[-- Attachment #2: 0001-lis3-add-generic-DT-matching-code.patch --]
[-- Type: text/x-patch, Size: 10195 bytes --]

>From 0926551982cb647f39cea4f4e904f4db2f435bd4 Mon Sep 17 00:00:00 2001
From: Daniel Mack <zonque@gmail.com>
Date: Mon, 23 Jul 2012 15:27:19 +0200
Subject: [PATCH v4 1/2] lis3: add generic DT matching code

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@gmail.com>
---
changes from v3:
 - fix 3 missing &tmp references in 3 calls
 - renamed 'tmp' to 'val'
 - added 'st,{min,max}-limit-{x,y,z}'

Changes from v2:
 - kzalloc braino

Changes from v1:
 - some typos in properties fixed

 Documentation/devicetree/bindings/misc/lis302.txt |  76 +++++++++++
 drivers/misc/lis3lv02d/lis3lv02d.c                | 152 ++++++++++++++++++++++
 drivers/misc/lis3lv02d/lis3lv02d.h                |   4 +
 3 files changed, 232 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..e18af9d
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/lis302.txt
@@ -0,0 +1,76 @@
+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.
+ - 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
+ - st,{min,max}-limit-{x,y,z}	set the min/max limits for x/y/z axis
+				(used by self-test)
+
+
+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..6292e8c 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,153 @@ 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 val;
+
+	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", &val))
+		pdata->click_thresh_x = val;
+	if (!of_property_read_u32(np, "st,click-threshold-y", &val))
+		pdata->click_thresh_y = val;
+	if (!of_property_read_u32(np, "st,click-threshold-z", &val))
+		pdata->click_thresh_z = val;
+
+	if (!of_property_read_u32(np, "st,click-time-limit", &val))
+		pdata->click_time_limit = val;
+	if (!of_property_read_u32(np, "st,click-latency", &val))
+		pdata->click_latency = val;
+	if (!of_property_read_u32(np, "st,click-window", &val))
+		pdata->click_window = val;
+
+	if (of_get_property(np, "st,irq1-disable", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_DISABLE;
+	if (of_get_property(np, "st,irq1-ff-wu-1", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_1;
+	if (of_get_property(np, "st,irq1-ff-wu-2", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_FF_WU_2;
+	if (of_get_property(np, "st,irq1-data-ready", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_DATA_READY;
+	if (of_get_property(np, "st,irq1-click", NULL))
+		pdata->irq_cfg |= LIS3_IRQ1_CLICK;
+
+	if (of_get_property(np, "st,irq2-disable", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_DISABLE;
+	if (of_get_property(np, "st,irq2-ff-wu-1", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_1;
+	if (of_get_property(np, "st,irq2-ff-wu-2", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_FF_WU_2;
+	if (of_get_property(np, "st,irq2-data-ready", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_DATA_READY;
+	if (of_get_property(np, "st,irq2-click", NULL))
+		pdata->irq_cfg |= LIS3_IRQ2_CLICK;
+
+	if (of_get_property(np, "st,irq-open-drain", NULL))
+		pdata->irq_cfg |= LIS3_IRQ_OPEN_DRAIN;
+	if (of_get_property(np, "st,irq-active-low", NULL))
+		pdata->irq_cfg |= LIS3_IRQ_ACTIVE_LOW;
+
+	if (!of_property_read_u32(np, "st,wu-duration-1", &val))
+		pdata->duration1 = val;
+	if (!of_property_read_u32(np, "st,wu-duration-2", &val))
+		pdata->duration2 = val;
+
+	if (of_get_property(np, "st,wakeup-x-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_X_LO;
+	if (of_get_property(np, "st,wakeup-x-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_X_HI;
+	if (of_get_property(np, "st,wakeup-y-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Y_LO;
+	if (of_get_property(np, "st,wakeup-y-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Y_HI;
+	if (of_get_property(np, "st,wakeup-z-lo", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Z_LO;
+	if (of_get_property(np, "st,wakeup-z-hi", NULL))
+		pdata->wakeup_flags |= LIS3_WAKEUP_Z_HI;
+
+	if (!of_property_read_u32(np, "st,highpass-cutoff-hz", &val)) {
+		switch (val) {
+		case 1:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_1HZ;
+			break;
+		case 2:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_2HZ;
+			break;
+		case 4:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_4HZ;
+			break;
+		case 8:
+			pdata->hipass_ctrl = LIS3_HIPASS_CUTFF_8HZ;
+			break;
+		}
+	}
+
+	if (of_get_property(np, "st,hipass1-disable", NULL))
+		pdata->hipass_ctrl |= LIS3_HIPASS1_DISABLE;
+	if (of_get_property(np, "st,hipass2-disable", NULL))
+		pdata->hipass_ctrl |= LIS3_HIPASS2_DISABLE;
+
+	if (of_get_property(np, "st,axis-x", &val))
+		pdata->axis_x = val;
+	if (of_get_property(np, "st,axis-y", &val))
+		pdata->axis_y = val;
+	if (of_get_property(np, "st,axis-z", &val))
+		pdata->axis_z = val;
+
+	if (of_get_property(np, "st,default-rate", NULL))
+		pdata->default_rate = val;
+
+	if (of_get_property(np, "st,min-limit-x", &val))
+		pdata->st_min_limits[0] = val;
+	if (of_get_property(np, "st,min-limit-y", &val))
+		pdata->st_min_limits[1] = val;
+	if (of_get_property(np, "st,min-limit-z", &val))
+		pdata->st_min_limits[2] = val;
+
+	if (of_get_property(np, "st,max-limit-x", &val))
+		pdata->st_max_limits[0] = val;
+	if (of_get_property(np, "st,max-limit-y", &val))
+		pdata->st_max_limits[1] = val;
+	if (of_get_property(np, "st,max-limit-z", &val))
+		pdata->st_max_limits[2] = val;
+
+
+	lis3->pdata = pdata;
+
+	return 0;
+}
+
+#else
+static int lis3lv02d_init_dt(struct lis3lv02d *lis3)
+{
+	return 0;
+}
+#endif
+
 /*
  * Initialise the accelerometer and the various subsystems.
  * Should be rather independent of the bus system.
@@ -922,6 +1070,10 @@ int lis3lv02d_init_device(struct lis3lv02d *lis3)
 	irq_handler_t thread_fn;
 	int irq_flags = 0;
 
+	err = lis3lv02d_init_dt(lis3);
+	if (err < 0)
+		return err;
+
 	lis3->whoami = lis3lv02d_read_8(lis3, WHO_AM_I);
 
 	switch (lis3->whoami) {
diff --git a/drivers/misc/lis3lv02d/lis3lv02d.h b/drivers/misc/lis3lv02d/lis3lv02d.h
index 2b1482a..a296f1d 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d.h
+++ b/drivers/misc/lis3lv02d/lis3lv02d.h
@@ -282,6 +282,10 @@ struct lis3lv02d {
 
 	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
 	struct mutex		mutex;     /* Serialize poll and selftest */
+
+#ifdef CONFIG_OF
+	struct device_node	*of_node;
+#endif
 };
 
 int lis3lv02d_init_device(struct lis3lv02d *lis3);
-- 
1.7.11.2

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

[AnilKumar, Chimata]

Hi Mack,

No attachments please.

On Wed, Aug 08, 2012 at 00:19:01, Daniel Mack wrote:
> Hi,
> 
> thanks for your review.
> 
> On 06.08.2012 12:45, AnilKumar, Chimata wrote:
> > On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
> >> 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@gmail.com>
> >>> ---
> >>> 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.
> >>> + - 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
> > 
> > Some more parameters missing, what about st_min_limits and st_max_limits
> > required for selftest.
> 
> Right. Added them now.
> 
> >>> +
> >>> +
> >>> +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;
> >>> +	};
> > 
> > Why can't we add these flags in driver itself like below?
> > Is these parameters varies from SoC to SoC or accelerometer
> > - to - accelerometer?
> 
> I don't understand, sorry. The point here is that the driver that is
> probed for device initialization are the PCI/I2C/SPI drivers. The

Look at the below example it has different drivers (SPI, I2C).
Compatible name changes form acc-acc so that we can use different
parameters corresponding to accelerometer, if it is acce specific.

Like
.compatible = "st,lis302dl-spi",
.compatible = "st,lis331dlh-i2c",
.compatible = "st,xx-spi",
.compatible = "st,xx-i2c",

If we do like this we can reduce lot of DT reads in driver.

> generic part is not something the device tree knows about.
> 
> Hence I put the generic parsing of common DT bindings to the generic
> part of the driver, and made the SPI driver just pass through the
> of_node pointer.
> 
> > #ifdef CONFIG_OF
> > static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
> >         .click_flags    = LIS3_CLICK_SINGLE_X |
> >                           LIS3_CLICK_SINGLE_Y |
> >                           LIS3_CLICK_SINGLE_Z,
> >         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
> >         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
> >                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
> >                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> > };
> > 
> > static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
> >         .click_flags    = LIS3_CLICK_SINGLE_X |
> >                           LIS3_CLICK_SINGLE_Y |
> >                           LIS3_CLICK_SINGLE_Z,
> >         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
> >         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
> >                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
> >                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> > };
> > 
> > static const struct of_device_id lis3_of_match[] = {
> >        {
> >                .compatible = "st,lis302dl-spi",
> >                .data = &lis302dl_spi_pdata,
> >        },
> >        {
> >                .compatible = "st,lis331dlh-i2c",
> >                .data = &lis331dlh_i2c_pdata,
> >        },
> >        { },
> > };
> > MODULE_DEVICE_TABLE(of, lis3_of_match);
> > #endif
> > 
> > Ignore if parameters between SoC - SoC are different. In
> > probe we can add these flags to pdata.
> 
> No. We want to expose all hardware features to DT so users can configure
> the device at wish. We can't ignore that SoCs want different device configs.

Is it require is my question, how many SoCs take these different
parameters from platform data.

Regards
AnilKumar

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

[AnilKumar, Chimata]

Hi Mack,

No attachments please.

On Wed, Aug 08, 2012 at 00:19:01, Daniel Mack wrote:
> Hi,
> 
> thanks for your review.
> 
> On 06.08.2012 12:45, AnilKumar, Chimata wrote:
> > On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
> >> 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.
> >>> + - 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
> > 
> > Some more parameters missing, what about st_min_limits and st_max_limits
> > required for selftest.
> 
> Right. Added them now.
> 
> >>> +
> >>> +
> >>> +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;
> >>> +	};
> > 
> > Why can't we add these flags in driver itself like below?
> > Is these parameters varies from SoC to SoC or accelerometer
> > - to - accelerometer?
> 
> I don't understand, sorry. The point here is that the driver that is
> probed for device initialization are the PCI/I2C/SPI drivers. The

Look at the below example it has different drivers (SPI, I2C).
Compatible name changes form acc-acc so that we can use different
parameters corresponding to accelerometer, if it is acce specific.

Like
.compatible = "st,lis302dl-spi",
.compatible = "st,lis331dlh-i2c",
.compatible = "st,xx-spi",
.compatible = "st,xx-i2c",

If we do like this we can reduce lot of DT reads in driver.

> generic part is not something the device tree knows about.
> 
> Hence I put the generic parsing of common DT bindings to the generic
> part of the driver, and made the SPI driver just pass through the
> of_node pointer.
> 
> > #ifdef CONFIG_OF
> > static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
> >         .click_flags    = LIS3_CLICK_SINGLE_X |
> >                           LIS3_CLICK_SINGLE_Y |
> >                           LIS3_CLICK_SINGLE_Z,
> >         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
> >         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
> >                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
> >                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> > };
> > 
> > static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
> >         .click_flags    = LIS3_CLICK_SINGLE_X |
> >                           LIS3_CLICK_SINGLE_Y |
> >                           LIS3_CLICK_SINGLE_Z,
> >         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
> >         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
> >                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
> >                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
> > };
> > 
> > static const struct of_device_id lis3_of_match[] = {
> >        {
> >                .compatible = "st,lis302dl-spi",
> >                .data = &lis302dl_spi_pdata,
> >        },
> >        {
> >                .compatible = "st,lis331dlh-i2c",
> >                .data = &lis331dlh_i2c_pdata,
> >        },
> >        { },
> > };
> > MODULE_DEVICE_TABLE(of, lis3_of_match);
> > #endif
> > 
> > Ignore if parameters between SoC - SoC are different. In
> > probe we can add these flags to pdata.
> 
> No. We want to expose all hardware features to DT so users can configure
> the device at wish. We can't ignore that SoCs want different device configs.

Is it require is my question, how many SoCs take these different
parameters from platform data.

Regards
AnilKumar

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

[Daniel Mack]

On 08.08.2012 07:19, AnilKumar, Chimata wrote:
> Hi Mack,

Call me Daniel :)

> On Wed, Aug 08, 2012 at 00:19:01, Daniel Mack wrote:
>> On 06.08.2012 12:45, AnilKumar, Chimata wrote:
>>> On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
>>>> 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@gmail.com>
>>>>> ---
>>>>> 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
>>>>>

[...]

>>>>> +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;
>>>>> +	};
>>>
>>> Why can't we add these flags in driver itself like below?
>>> Is these parameters varies from SoC to SoC or accelerometer
>>> - to - accelerometer?
>>
>> I don't understand, sorry. The point here is that the driver that is
>> probed for device initialization are the PCI/I2C/SPI drivers. The
> 
> Look at the below example it has different drivers (SPI, I2C).
> Compatible name changes form acc-acc so that we can use different
> parameters corresponding to accelerometer, if it is acce specific.
> 
> Like
> .compatible = "st,lis302dl-spi",
> .compatible = "st,lis331dlh-i2c",
> .compatible = "st,xx-spi",
> .compatible = "st,xx-i2c",
> 
> If we do like this we can reduce lot of DT reads in driver.

No, we can't. Look, this chip has a huge number of registers that can be
set in order to accomplish a variety of different functions. It can be
used as a free-fall detector or as a full-fledged accelerometer, with
different settings per axis. None of them is specific to the bus this
chip is connected through.

Of course we want to leave it to the board vendor what functions to use,
and hence all of them are exposed via DT. The whole idea of DT is to
describe the hardware and the desired behaviour of each component as
exact as possible, so vendors don't have to hack along in the source
code but can boot a generic kernel.

[...]

>>> #ifdef CONFIG_OF
>>> static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
>>>         .click_flags    = LIS3_CLICK_SINGLE_X |
>>>                           LIS3_CLICK_SINGLE_Y |
>>>                           LIS3_CLICK_SINGLE_Z,
>>>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>>>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>>>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>>>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
>>> };
>>>
>>> static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
>>>         .click_flags    = LIS3_CLICK_SINGLE_X |
>>>                           LIS3_CLICK_SINGLE_Y |
>>>                           LIS3_CLICK_SINGLE_Z,
>>>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>>>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>>>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>>>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
>>> };
>>>
>>> static const struct of_device_id lis3_of_match[] = {
>>>        {
>>>                .compatible = "st,lis302dl-spi",
>>>                .data = &lis302dl_spi_pdata,
>>>        },
>>>        {
>>>                .compatible = "st,lis331dlh-i2c",
>>>                .data = &lis331dlh_i2c_pdata,
>>>        },
>>>        { },
>>> };
>>> MODULE_DEVICE_TABLE(of, lis3_of_match);
>>> #endif
>>>
>>> Ignore if parameters between SoC - SoC are different. In
>>> probe we can add these flags to pdata.
>>
>> No. We want to expose all hardware features to DT so users can configure
>> the device at wish. We can't ignore that SoCs want different device configs.
> 
> Is it require is my question, how many SoCs take these different
> parameters from platform data.

We can't know, and it doesn't matter. The goal here is not to provide a
way to boot the boards that are currently in mainline via DT with
minimal effort, but to have DT bindings for this driver that reflect the
features of this piece of hardware, so anything is possible.


Regards,
Daniel

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

[Daniel Mack]

On 08.08.2012 07:19, AnilKumar, Chimata wrote:
> Hi Mack,

Call me Daniel :)

> On Wed, Aug 08, 2012 at 00:19:01, Daniel Mack wrote:
>> On 06.08.2012 12:45, AnilKumar, Chimata wrote:
>>> On Sun, Aug 05, 2012 at 21:48:42, Daniel Mack wrote:
>>>> 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@gmail.com>
>>>>> ---
>>>>> 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
>>>>>

[...]

>>>>> +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;
>>>>> +	};
>>>
>>> Why can't we add these flags in driver itself like below?
>>> Is these parameters varies from SoC to SoC or accelerometer
>>> - to - accelerometer?
>>
>> I don't understand, sorry. The point here is that the driver that is
>> probed for device initialization are the PCI/I2C/SPI drivers. The
> 
> Look at the below example it has different drivers (SPI, I2C).
> Compatible name changes form acc-acc so that we can use different
> parameters corresponding to accelerometer, if it is acce specific.
> 
> Like
> .compatible = "st,lis302dl-spi",
> .compatible = "st,lis331dlh-i2c",
> .compatible = "st,xx-spi",
> .compatible = "st,xx-i2c",
> 
> If we do like this we can reduce lot of DT reads in driver.

No, we can't. Look, this chip has a huge number of registers that can be
set in order to accomplish a variety of different functions. It can be
used as a free-fall detector or as a full-fledged accelerometer, with
different settings per axis. None of them is specific to the bus this
chip is connected through.

Of course we want to leave it to the board vendor what functions to use,
and hence all of them are exposed via DT. The whole idea of DT is to
describe the hardware and the desired behaviour of each component as
exact as possible, so vendors don't have to hack along in the source
code but can boot a generic kernel.

[...]

>>> #ifdef CONFIG_OF
>>> static struct lis3lv02d_platform_data lis302dl_spi_pdata = {
>>>         .click_flags    = LIS3_CLICK_SINGLE_X |
>>>                           LIS3_CLICK_SINGLE_Y |
>>>                           LIS3_CLICK_SINGLE_Z,
>>>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>>>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>>>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>>>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
>>> };
>>>
>>> static struct lis3lv02d_platform_data lis331dlh_i2c_pdata = {
>>>         .click_flags    = LIS3_CLICK_SINGLE_X |
>>>                           LIS3_CLICK_SINGLE_Y |
>>>                           LIS3_CLICK_SINGLE_Z,
>>>         .irq_cfg        = LIS3_IRQ1_CLICK | LIS3_IRQ2_CLICK,
>>>         .wakeup_flags   = LIS3_WAKEUP_X_LO | LIS3_WAKEUP_X_HI |
>>>                           LIS3_WAKEUP_Y_LO | LIS3_WAKEUP_Y_HI |
>>>                           LIS3_WAKEUP_Z_LO | LIS3_WAKEUP_Z_HI,
>>> };
>>>
>>> static const struct of_device_id lis3_of_match[] = {
>>>        {
>>>                .compatible = "st,lis302dl-spi",
>>>                .data = &lis302dl_spi_pdata,
>>>        },
>>>        {
>>>                .compatible = "st,lis331dlh-i2c",
>>>                .data = &lis331dlh_i2c_pdata,
>>>        },
>>>        { },
>>> };
>>> MODULE_DEVICE_TABLE(of, lis3_of_match);
>>> #endif
>>>
>>> Ignore if parameters between SoC - SoC are different. In
>>> probe we can add these flags to pdata.
>>
>> No. We want to expose all hardware features to DT so users can configure
>> the device at wish. We can't ignore that SoCs want different device configs.
> 
> Is it require is my question, how many SoCs take these different
> parameters from platform data.

We can't know, and it doesn't matter. The goal here is not to provide a
way to boot the boards that are currently in mainline via DT with
minimal effort, but to have DT bindings for this driver that reflect the
features of this piece of hardware, so anything is possible.


Regards,
Daniel

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

[Éric Piel]

On 07-08-12 20:49, Daniel Mack wrote:
:
>
> I fixed all these issues now and attached a v4.
>
Sorry for the late reply, I had read the v3 but didn't find time to send 
comments. They are all addressed in v4.

For both [PATCH v4 1/2] and  [PATCH v3 2/2], here is my:
Reviewed-by: Éric Piel <eric.piel@tremplin-utc.net>


Cheers,
Éric

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

[Éric Piel]

On 07-08-12 20:49, Daniel Mack wrote:
:
>
> I fixed all these issues now and attached a v4.
>
Sorry for the late reply, I had read the v3 but didn't find time to send 
comments. They are all addressed in v4.

For both [PATCH v4 1/2] and  [PATCH v3 2/2], here is my:
Reviewed-by: Éric Piel <eric.piel@tremplin-utc.net>


Cheers,
Éric

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

[Daniel Mack]

On 15.08.2012 09:13, Éric Piel wrote:
> On 07-08-12 20:49, Daniel Mack wrote:
> :
>>
>> I fixed all these issues now and attached a v4.
>>
> Sorry for the late reply, I had read the v3 but didn't find time to send 
> comments. They are all addressed in v4.
> 
> For both [PATCH v4 1/2] and  [PATCH v3 2/2], here is my:
> Reviewed-by: Éric Piel <eric.piel@tremplin-utc.net>

Thanks!


Daniel

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

[Daniel Mack]

On 15.08.2012 09:13, Éric Piel wrote:
> On 07-08-12 20:49, Daniel Mack wrote:
> :
>>
>> I fixed all these issues now and attached a v4.
>>
> Sorry for the late reply, I had read the v3 but didn't find time to send 
> comments. They are all addressed in v4.
> 
> For both [PATCH v4 1/2] and  [PATCH v3 2/2], here is my:
> Reviewed-by: Éric Piel <eric.piel@tremplin-utc.net>

Thanks!


Daniel

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss