[PATCH 1/2] Input: adxl34x - add OF support

[PATCH 1/2] Input: adxl34x - add OF support

[Walter Lozano]

This patch adds the missing support for OF to the adxl34x digital
accelerometer. This is a basic version which supports the main
optional parameters. This implementation copies the default values
to the adxl34x_platform_data structure and overrides the values
that are passed from the device tree.

Signed-off-by: Walter Lozano <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/input/misc/adxl34x.c |  108 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
index 2b2d02f..b3e06a3 100644
--- a/drivers/input/misc/adxl34x.c
+++ b/drivers/input/misc/adxl34x.c
@@ -16,6 +16,8 @@
 #include <linux/workqueue.h>
 #include <linux/input/adxl34x.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include "adxl34x.h"
 
@@ -688,13 +690,113 @@ static void adxl34x_input_close(struct input_dev *input)
 	mutex_unlock(&ac->mutex);
 }
 
+void adxl34x_parse_dt(struct device *dev, struct adxl34x_platform_data *pdata){
+	if (!dev->of_node)
+		return;
+
+	pdata =  kzalloc(sizeof(*pdata), GFP_KERNEL);
+
+	memcpy(pdata, &adxl34x_default_init, sizeof(struct adxl34x_platform_data));
+
+	of_property_read_u8(dev->of_node, "adi,tap-axis-control",
+		 &pdata->tap_axis_control);
+
+	of_property_read_u8(dev->of_node, "adi,tap-threshold",
+		 &pdata->tap_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,tap-duration",
+		 &pdata->tap_duration);
+
+	of_property_read_u8(dev->of_node, "adi,tap-latency",
+		 &pdata->tap_latency);
+
+	of_property_read_u8(dev->of_node, "adi,tap-window",
+		 &pdata->tap_window);
+
+	of_property_read_u8(dev->of_node, "adi,act-axis-control",
+		 &pdata->act_axis_control);
+
+	of_property_read_u8(dev->of_node, "adi,activity-threshold",
+		 &pdata->activity_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,inactivity-threshold",
+		 &pdata->inactivity_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,inactivity-time",
+		 &pdata->inactivity_time);
+
+	of_property_read_u8(dev->of_node, "adi,free-fall-threshold",
+		 &pdata->free_fall_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,free-fall-time",
+		 &pdata->free_fall_time);
+
+	of_property_read_u8(dev->of_node, "adi,data-rate",
+		 &pdata->data_rate);
+
+	of_property_read_u8(dev->of_node, "adi,data-range",
+		 &pdata->data_range);
+
+	of_property_read_u8(dev->of_node, "adi,low-power-mode",
+		 &pdata->low_power_mode);
+
+	of_property_read_u8(dev->of_node, "adi,power-mode",
+		 &pdata->power_mode);
+
+	of_property_read_u8(dev->of_node, "adi,fifo-mode",
+		 &pdata->fifo_mode);
+
+	of_property_read_u8(dev->of_node, "adi,watermark",
+		 &pdata->watermark);
+
+	of_property_read_u8(dev->of_node, "adi,use-int2",
+		 &pdata->use_int2);
+
+	of_property_read_u8(dev->of_node, "adi,orientation-enable",
+		 &pdata->orientation_enable);
+
+	of_property_read_u8(dev->of_node, "adi,deadzone-angle",
+		 &pdata->deadzone_angle);
+
+	of_property_read_u8(dev->of_node, "adi,divisor-length",
+		 &pdata->divisor_length);
+
+	of_property_read_u32(dev->of_node, "adi,ev-type",
+		 &pdata->ev_type);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-x",
+		 &pdata->ev_code_x);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-y",
+		 &pdata->ev_code_y);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-z",
+		 &pdata->ev_code_z);
+
+	of_property_read_u32_array(dev->of_node, "adi,ev-code-tap",
+		 pdata->ev_code_tap, 3);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-ff",
+		 &pdata->ev_code_ff);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-act-inactivity",
+		 &pdata->ev_code_act_inactivity);
+
+	of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-2d",
+		 pdata->ev_codes_orient_2d, 4);
+
+	of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-3d",
+		 pdata->ev_codes_orient_3d, 6);
+
+}
+
 struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 			      bool fifo_delay_default,
 			      const struct adxl34x_bus_ops *bops)
 {
 	struct adxl34x *ac;
 	struct input_dev *input_dev;
-	const struct adxl34x_platform_data *pdata;
+	struct adxl34x_platform_data *pdata;
 	int err, range, i;
 	unsigned char revid;
 
@@ -714,6 +816,10 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 	ac->fifo_delay = fifo_delay_default;
 
 	pdata = dev_get_platdata(dev);
+
+	if (!pdata && dev->of_node)
+		adxl34x_parse_dt(dev, pdata);
+
 	if (!pdata) {
 		dev_dbg(dev,
 			"No platform data: Using default initialization\n");
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] Input: adxl34x - add device tree documentation

[Walter Lozano]

This patch adds a basic documentation regarding the device tree
bindings of adxl34x digital accelerometer.

Signed-off-by: Walter Lozano <walter@vanguardiasur.com.ar>
---
 .../devicetree/bindings/input/adxl34x.txt          |   53 ++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/adxl34x.txt

diff --git a/Documentation/devicetree/bindings/input/adxl34x.txt b/Documentation/devicetree/bindings/input/adxl34x.txt
new file mode 100644
index 0000000..0774f7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adxl34x.txt
@@ -0,0 +1,53 @@
+Device Tree bindings for adxl34x digital accelerometer, both spi and i2c.
+
+Depending on the interface this node should be a child of a spi or i2c bus.
+
+Required properties:
+- compatible: "adxl34x"
+- reg: SPI/I2C device address
+- interrupt-parent: a phandle for the interrupt controller
+- interrupts: interrupt to which the chip is connected
+
+Optional parameters:
+Please refer to include/linux/input/adxl34x.h for specific info. If the value
+is not specified the driver will use its default.
+
+- adi,tap-axis-control: u8 default 0
+- adi,tap-threshold: u8 default 35
+- adi,tap-duration: u8 default 3
+- adi,tap-latency: u8 default 20
+- adi,tap-window: u8 default 20
+- adi,act-axis-control: u8 default 0xFF
+- adi,activity-threshold: u8 default 6
+- adi,inactivity-threshold: u8 default 4
+- adi,inactivity-time: u8 default 3
+- adi,free-fall-threshold: u8 default 8
+- adi,free-fall-time: u8 default 20
+- adi,data-rate: u8 default 8
+- adi,data-range: u8 default 4
+- adi,low-power-mode: u8 defaut 0
+- adi,power-mode: u8 default 0x30
+- adi,fifo-mode: u8 default 2
+- adi,watermark: u8 default 0
+- adi,use-int2: u8
+- adi,orientation-enable: u8 default 0
+- adi,deadzone-angle: u8 default 0
+- divisor-length: u8 default 0
+
+Example
+
+&i2c1 {
+	status = "okay";
+
+	accel1: accelerometer@53 {
+		compatible = "adxl34x";
+		reg = <0x53>;
+
+		interrupt-parent = <&portc>;
+		interrupts = <3 2>;
+
+		adi,inactivity-time = /bits/ 8 <6>;
+		adi,data-range = /bits/ 8 <1>;
+	};
+};
+
-- 
1.7.10.4

Re: [PATCH 1/2] Input: adxl34x - add OF support

[Dmitry Torokhov]

Hi Walter,

On Tue, Jan 06, 2015 at 11:58:56PM -0300, Walter Lozano wrote:
> This patch adds the missing support for OF to the adxl34x digital
> accelerometer. This is a basic version which supports the main
> optional parameters. This implementation copies the default values
> to the adxl34x_platform_data structure and overrides the values
> that are passed from the device tree.
> 
> Signed-off-by: Walter Lozano <walter@vanguardiasur.com.ar>
> ---
>  drivers/input/misc/adxl34x.c |  108 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
> index 2b2d02f..b3e06a3 100644
> --- a/drivers/input/misc/adxl34x.c
> +++ b/drivers/input/misc/adxl34x.c
> @@ -16,6 +16,8 @@
>  #include <linux/workqueue.h>
>  #include <linux/input/adxl34x.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  #include "adxl34x.h"
>  
> @@ -688,13 +690,113 @@ static void adxl34x_input_close(struct input_dev *input)
>  	mutex_unlock(&ac->mutex);
>  }
>  
> +void adxl34x_parse_dt(struct device *dev, struct adxl34x_platform_data *pdata){
> +	if (!dev->of_node)
> +		return;
> +
> +	pdata =  kzalloc(sizeof(*pdata), GFP_KERNEL);
> +
> +	memcpy(pdata, &adxl34x_default_init, sizeof(struct adxl34x_platform_data));
> +
> +	of_property_read_u8(dev->of_node, "adi,tap-axis-control",
> +		 &pdata->tap_axis_control);
> +
> +	of_property_read_u8(dev->of_node, "adi,tap-threshold",
> +		 &pdata->tap_threshold);
> +
> +	of_property_read_u8(dev->of_node, "adi,tap-duration",
> +		 &pdata->tap_duration);
> +
> +	of_property_read_u8(dev->of_node, "adi,tap-latency",
> +		 &pdata->tap_latency);
> +
> +	of_property_read_u8(dev->of_node, "adi,tap-window",
> +		 &pdata->tap_window);
> +
> +	of_property_read_u8(dev->of_node, "adi,act-axis-control",
> +		 &pdata->act_axis_control);
> +
> +	of_property_read_u8(dev->of_node, "adi,activity-threshold",
> +		 &pdata->activity_threshold);
> +
> +	of_property_read_u8(dev->of_node, "adi,inactivity-threshold",
> +		 &pdata->inactivity_threshold);
> +
> +	of_property_read_u8(dev->of_node, "adi,inactivity-time",
> +		 &pdata->inactivity_time);
> +
> +	of_property_read_u8(dev->of_node, "adi,free-fall-threshold",
> +		 &pdata->free_fall_threshold);
> +
> +	of_property_read_u8(dev->of_node, "adi,free-fall-time",
> +		 &pdata->free_fall_time);
> +
> +	of_property_read_u8(dev->of_node, "adi,data-rate",
> +		 &pdata->data_rate);
> +
> +	of_property_read_u8(dev->of_node, "adi,data-range",
> +		 &pdata->data_range);
> +
> +	of_property_read_u8(dev->of_node, "adi,low-power-mode",
> +		 &pdata->low_power_mode);
> +
> +	of_property_read_u8(dev->of_node, "adi,power-mode",
> +		 &pdata->power_mode);
> +
> +	of_property_read_u8(dev->of_node, "adi,fifo-mode",
> +		 &pdata->fifo_mode);
> +
> +	of_property_read_u8(dev->of_node, "adi,watermark",
> +		 &pdata->watermark);
> +
> +	of_property_read_u8(dev->of_node, "adi,use-int2",
> +		 &pdata->use_int2);
> +
> +	of_property_read_u8(dev->of_node, "adi,orientation-enable",
> +		 &pdata->orientation_enable);
> +
> +	of_property_read_u8(dev->of_node, "adi,deadzone-angle",
> +		 &pdata->deadzone_angle);
> +
> +	of_property_read_u8(dev->of_node, "adi,divisor-length",
> +		 &pdata->divisor_length);
> +
> +	of_property_read_u32(dev->of_node, "adi,ev-type",
> +		 &pdata->ev_type);

All these ev* properties are Linux-specific so should have linux prefix
and not adi.

> +
> +	of_property_read_u32(dev->of_node, "adi,ev-code-x",
> +		 &pdata->ev_code_x);
> +
> +	of_property_read_u32(dev->of_node, "adi,ev-code-y",
> +		 &pdata->ev_code_y);
> +
> +	of_property_read_u32(dev->of_node, "adi,ev-code-z",
> +		 &pdata->ev_code_z);
> +
> +	of_property_read_u32_array(dev->of_node, "adi,ev-code-tap",
> +		 pdata->ev_code_tap, 3);
> +
> +	of_property_read_u32(dev->of_node, "adi,ev-code-ff",
> +		 &pdata->ev_code_ff);
> +
> +	of_property_read_u32(dev->of_node, "adi,ev-code-act-inactivity",
> +		 &pdata->ev_code_act_inactivity);
> +
> +	of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-2d",
> +		 pdata->ev_codes_orient_2d, 4);
> +
> +	of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-3d",
> +		 pdata->ev_codes_orient_3d, 6);
> +
> +}
> +
>  struct adxl34x *adxl34x_probe(struct device *dev, int irq,
>  			      bool fifo_delay_default,
>  			      const struct adxl34x_bus_ops *bops)
>  {
>  	struct adxl34x *ac;
>  	struct input_dev *input_dev;
> -	const struct adxl34x_platform_data *pdata;
> +	struct adxl34x_platform_data *pdata;
>  	int err, range, i;
>  	unsigned char revid;
>  
> @@ -714,6 +816,10 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
>  	ac->fifo_delay = fifo_delay_default;
>  
>  	pdata = dev_get_platdata(dev);
> +
> +	if (!pdata && dev->of_node)
> +		adxl34x_parse_dt(dev, pdata);
> +
>  	if (!pdata) {

Umm, what's changing data pointer? Was this tested?

>  		dev_dbg(dev,
>  			"No platform data: Using default initialization\n");
> -- 
> 1.7.10.4
> 

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] Input: adxl34x - add device tree documentation

[Geert Uytterhoeven]

On Wed, Jan 7, 2015 at 3:58 AM, Walter Lozano
<walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> diff --git a/Documentation/devicetree/bindings/input/adxl34x.txt b/Documentation/devicetree/bindings/input/adxl34x.txt
> new file mode 100644
> index 0000000..0774f7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/adxl34x.txt
> @@ -0,0 +1,53 @@
> +Device Tree bindings for adxl34x digital accelerometer, both spi and i2c.
> +
> +Depending on the interface this node should be a child of a spi or i2c bus.
> +
> +Required properties:
> +- compatible: "adxl34x"

"adi,adxl34x"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Input: adxl34x - add OF support

[Walter Lozano]

Hi Dimitry,

First of all, thanks for your feedback

On Wed, Jan 7, 2015 at 5:01 AM, Dmitry Torokhov
<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Walter,
>
> On Tue, Jan 06, 2015 at 11:58:56PM -0300, Walter Lozano wrote:
>> This patch adds the missing support for OF to the adxl34x digital
>> accelerometer. This is a basic version which supports the main
>> optional parameters. This implementation copies the default values
>> to the adxl34x_platform_data structure and overrides the values
>> that are passed from the device tree.
>>
>> Signed-off-by: Walter Lozano <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
>> ---
>>  drivers/input/misc/adxl34x.c |  108 +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 107 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
>> index 2b2d02f..b3e06a3 100644
>> --- a/drivers/input/misc/adxl34x.c
>> +++ b/drivers/input/misc/adxl34x.c
>> @@ -16,6 +16,8 @@
>>  #include <linux/workqueue.h>
>>  #include <linux/input/adxl34x.h>
>>  #include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>>
>>  #include "adxl34x.h"
>>
>> @@ -688,13 +690,113 @@ static void adxl34x_input_close(struct input_dev *input)
>>       mutex_unlock(&ac->mutex);
>>  }
>>
>> +void adxl34x_parse_dt(struct device *dev, struct adxl34x_platform_data *pdata){
>> +     if (!dev->of_node)
>> +             return;
>> +
>> +     pdata =  kzalloc(sizeof(*pdata), GFP_KERNEL);
>> +
>> +     memcpy(pdata, &adxl34x_default_init, sizeof(struct adxl34x_platform_data));
>> +
>> +     of_property_read_u8(dev->of_node, "adi,tap-axis-control",
>> +              &pdata->tap_axis_control);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,tap-threshold",
>> +              &pdata->tap_threshold);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,tap-duration",
>> +              &pdata->tap_duration);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,tap-latency",
>> +              &pdata->tap_latency);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,tap-window",
>> +              &pdata->tap_window);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,act-axis-control",
>> +              &pdata->act_axis_control);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,activity-threshold",
>> +              &pdata->activity_threshold);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,inactivity-threshold",
>> +              &pdata->inactivity_threshold);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,inactivity-time",
>> +              &pdata->inactivity_time);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,free-fall-threshold",
>> +              &pdata->free_fall_threshold);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,free-fall-time",
>> +              &pdata->free_fall_time);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,data-rate",
>> +              &pdata->data_rate);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,data-range",
>> +              &pdata->data_range);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,low-power-mode",
>> +              &pdata->low_power_mode);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,power-mode",
>> +              &pdata->power_mode);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,fifo-mode",
>> +              &pdata->fifo_mode);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,watermark",
>> +              &pdata->watermark);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,use-int2",
>> +              &pdata->use_int2);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,orientation-enable",
>> +              &pdata->orientation_enable);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,deadzone-angle",
>> +              &pdata->deadzone_angle);
>> +
>> +     of_property_read_u8(dev->of_node, "adi,divisor-length",
>> +              &pdata->divisor_length);
>> +
>> +     of_property_read_u32(dev->of_node, "adi,ev-type",
>> +              &pdata->ev_type);
>
> All these ev* properties are Linux-specific so should have linux prefix
> and not adi.

Yes, I have many doubts about these parameters. First I'm not sure if anybody
will need to change them as the accelerometer reports absolute values.
I was thinking
about omitting them but I would like someone else opinion.

The default values for these parameters are

    .ev_type = EV_ABS,
    .ev_code_x = ABS_X, /* EV_REL */
    .ev_code_y = ABS_Y, /* EV_REL */
    .ev_code_z = ABS_Z, /* EV_REL */

    .ev_code_tap = {BTN_TOUCH, BTN_TOUCH, BTN_TOUCH}, /* EV_KEY {x,y,z} */

In case if it is worthy should I add nodes for each different event?

For example

       axisx{
            label = "Axis X";
            linux,code = <0>;
        }

>> +
>> +     of_property_read_u32(dev->of_node, "adi,ev-code-x",
>> +              &pdata->ev_code_x);
>> +
>> +     of_property_read_u32(dev->of_node, "adi,ev-code-y",
>> +              &pdata->ev_code_y);
>> +
>> +     of_property_read_u32(dev->of_node, "adi,ev-code-z",
>> +              &pdata->ev_code_z);
>> +
>> +     of_property_read_u32_array(dev->of_node, "adi,ev-code-tap",
>> +              pdata->ev_code_tap, 3);
>> +
>> +     of_property_read_u32(dev->of_node, "adi,ev-code-ff",
>> +              &pdata->ev_code_ff);
>> +
>> +     of_property_read_u32(dev->of_node, "adi,ev-code-act-inactivity",
>> +              &pdata->ev_code_act_inactivity);
>> +
>> +     of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-2d",
>> +              pdata->ev_codes_orient_2d, 4);
>> +
>> +     of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-3d",
>> +              pdata->ev_codes_orient_3d, 6);
>> +
>> +}
>> +
>>  struct adxl34x *adxl34x_probe(struct device *dev, int irq,
>>                             bool fifo_delay_default,
>>                             const struct adxl34x_bus_ops *bops)
>>  {
>>       struct adxl34x *ac;
>>       struct input_dev *input_dev;
>> -     const struct adxl34x_platform_data *pdata;
>> +     struct adxl34x_platform_data *pdata;
>>       int err, range, i;
>>       unsigned char revid;
>>
>> @@ -714,6 +816,10 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
>>       ac->fifo_delay = fifo_delay_default;
>>
>>       pdata = dev_get_platdata(dev);
>> +
>> +     if (!pdata && dev->of_node)
>> +             adxl34x_parse_dt(dev, pdata);
>> +
>>       if (!pdata) {
>
> Umm, what's changing data pointer? Was this tested?
>

You are right. I've tested a previous version, but then I make some changes as
I have some doubts about how is the better way to handle the compatibility
with board files.

My initial intention was to keep the original code as much as I can, so in case
of using board files the pdata is set to adxl34x_default_init, but if
DT is used,
memory is allocated and the default parameters are copied. Then in the remove,
only if DT is used free the memory (not included in this patch).

So as I wasn't quite sure if this was the best approach I send this patch to get
some comments. Probably I should send a RFC...

>>               dev_dbg(dev,
>>                       "No platform data: Using default initialization\n");
>> --
>> 1.7.10.4
>>

Thanks again for your time and comments.

Regards,

Walter

-- 
Walter Lozano, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Input: adxl34x - add OF support

[Dmitry Torokhov]

On Wed, Jan 07, 2015 at 01:25:54PM -0300, Walter Lozano wrote:
> Hi Dimitry,
> 
> First of all, thanks for your feedback
> 
> On Wed, Jan 7, 2015 at 5:01 AM, Dmitry Torokhov
> <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Hi Walter,
> >
> > On Tue, Jan 06, 2015 at 11:58:56PM -0300, Walter Lozano wrote:
> >> This patch adds the missing support for OF to the adxl34x digital
> >> accelerometer. This is a basic version which supports the main
> >> optional parameters. This implementation copies the default values
> >> to the adxl34x_platform_data structure and overrides the values
> >> that are passed from the device tree.
> >>
> >> Signed-off-by: Walter Lozano <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
> >> ---
> >>  drivers/input/misc/adxl34x.c |  108 +++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 107 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
> >> index 2b2d02f..b3e06a3 100644
> >> --- a/drivers/input/misc/adxl34x.c
> >> +++ b/drivers/input/misc/adxl34x.c
> >> @@ -16,6 +16,8 @@
> >>  #include <linux/workqueue.h>
> >>  #include <linux/input/adxl34x.h>
> >>  #include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_device.h>
> >>
> >>  #include "adxl34x.h"
> >>
> >> @@ -688,13 +690,113 @@ static void adxl34x_input_close(struct input_dev *input)
> >>       mutex_unlock(&ac->mutex);
> >>  }
> >>
> >> +void adxl34x_parse_dt(struct device *dev, struct adxl34x_platform_data *pdata){
> >> +     if (!dev->of_node)
> >> +             return;
> >> +
> >> +     pdata =  kzalloc(sizeof(*pdata), GFP_KERNEL);
> >> +
> >> +     memcpy(pdata, &adxl34x_default_init, sizeof(struct adxl34x_platform_data));
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,tap-axis-control",
> >> +              &pdata->tap_axis_control);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,tap-threshold",
> >> +              &pdata->tap_threshold);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,tap-duration",
> >> +              &pdata->tap_duration);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,tap-latency",
> >> +              &pdata->tap_latency);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,tap-window",
> >> +              &pdata->tap_window);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,act-axis-control",
> >> +              &pdata->act_axis_control);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,activity-threshold",
> >> +              &pdata->activity_threshold);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,inactivity-threshold",
> >> +              &pdata->inactivity_threshold);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,inactivity-time",
> >> +              &pdata->inactivity_time);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,free-fall-threshold",
> >> +              &pdata->free_fall_threshold);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,free-fall-time",
> >> +              &pdata->free_fall_time);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,data-rate",
> >> +              &pdata->data_rate);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,data-range",
> >> +              &pdata->data_range);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,low-power-mode",
> >> +              &pdata->low_power_mode);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,power-mode",
> >> +              &pdata->power_mode);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,fifo-mode",
> >> +              &pdata->fifo_mode);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,watermark",
> >> +              &pdata->watermark);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,use-int2",
> >> +              &pdata->use_int2);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,orientation-enable",
> >> +              &pdata->orientation_enable);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,deadzone-angle",
> >> +              &pdata->deadzone_angle);
> >> +
> >> +     of_property_read_u8(dev->of_node, "adi,divisor-length",
> >> +              &pdata->divisor_length);
> >> +
> >> +     of_property_read_u32(dev->of_node, "adi,ev-type",
> >> +              &pdata->ev_type);
> >
> > All these ev* properties are Linux-specific so should have linux prefix
> > and not adi.
> 
> Yes, I have many doubts about these parameters. First I'm not sure if anybody
> will need to change them as the accelerometer reports absolute values.
> I was thinking
> about omitting them but I would like someone else opinion.

I wonder if someone will mount the chip "weirdly" thy might want to
remap the axes.

> 
> The default values for these parameters are
> 
>     .ev_type = EV_ABS,
>     .ev_code_x = ABS_X, /* EV_REL */
>     .ev_code_y = ABS_Y, /* EV_REL */
>     .ev_code_z = ABS_Z, /* EV_REL */
> 
>     .ev_code_tap = {BTN_TOUCH, BTN_TOUCH, BTN_TOUCH}, /* EV_KEY {x,y,z} */
> 
> In case if it is worthy should I add nodes for each different event?
> 
> For example
> 
>        axisx{
>             label = "Axis X";
>             linux,code = <0>;
>         }
> 

I'll defer to device tree guys on this.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Input: adxl34x - add device tree documentation

[Walter Lozano]

On Wed, Jan 7, 2015 at 6:42 AM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Wed, Jan 7, 2015 at 3:58 AM, Walter Lozano
> <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
>> diff --git a/Documentation/devicetree/bindings/input/adxl34x.txt b/Documentation/devicetree/bindings/input/adxl34x.txt
>> new file mode 100644
>> index 0000000..0774f7e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/adxl34x.txt
>> @@ -0,0 +1,53 @@
>> +Device Tree bindings for adxl34x digital accelerometer, both spi and i2c.
>> +
>> +Depending on the interface this node should be a child of a spi or i2c bus.
>> +
>> +Required properties:
>> +- compatible: "adxl34x"
>
> "adi,adxl34x"?

Great, thanks for your feedback.

Regards,

Walter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Input: adxl34x - add device tree documentation

[Geert Uytterhoeven]

On Thu, Jan 8, 2015 at 6:40 PM, Walter Lozano
<walter@vanguardiasur.com.ar> wrote:
> On Wed, Jan 7, 2015 at 6:42 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Wed, Jan 7, 2015 at 3:58 AM, Walter Lozano
>> <walter@vanguardiasur.com.ar> wrote:
>>> diff --git a/Documentation/devicetree/bindings/input/adxl34x.txt b/Documentation/devicetree/bindings/input/adxl34x.txt
>>> new file mode 100644
>>> index 0000000..0774f7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/adxl34x.txt
>>> @@ -0,0 +1,53 @@
>>> +Device Tree bindings for adxl34x digital accelerometer, both spi and i2c.
>>> +
>>> +Depending on the interface this node should be a child of a spi or i2c bus.
>>> +
>>> +Required properties:
>>> +- compatible: "adxl34x"
>>
>> "adi,adxl34x"?
>
> Great, thanks for your feedback.

BTW, did you see "[PATCH] input: adxl34x: Add OF match support"
(http://www.spinics.net/lists/linux-i2c/msg18107.html)?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] Input: adxl34x - add device tree documentation

[Walter Lozano]

On Thu, Jan 8, 2015 at 3:51 PM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Thu, Jan 8, 2015 at 6:40 PM, Walter Lozano
> <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
>> On Wed, Jan 7, 2015 at 6:42 AM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>>> On Wed, Jan 7, 2015 at 3:58 AM, Walter Lozano
>>> <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
>>>> diff --git a/Documentation/devicetree/bindings/input/adxl34x.txt b/Documentation/devicetree/bindings/input/adxl34x.txt
>>>> new file mode 100644
>>>> index 0000000..0774f7e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/input/adxl34x.txt
>>>> @@ -0,0 +1,53 @@
>>>> +Device Tree bindings for adxl34x digital accelerometer, both spi and i2c.
>>>> +
>>>> +Depending on the interface this node should be a child of a spi or i2c bus.
>>>> +
>>>> +Required properties:
>>>> +- compatible: "adxl34x"
>>>
>>> "adi,adxl34x"?
>>
>> Great, thanks for your feedback.
>
> BTW, did you see "[PATCH] input: adxl34x: Add OF match support"
> (http://www.spinics.net/lists/linux-i2c/msg18107.html)?

Yes, I have already seen it. Thanks for you time.

Regards,

Walter

-- 
Walter Lozano, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html