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

From: AnilKumar, Chimata
Date: Mon Aug 06 2012 - 06:50:50 EST


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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxx
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/