Re: [PATCH] drivers: gpio: gpio-adp5588: Fix sleep-in-atomic-context bug

[Michael Hennerich]

On 29.08.2018 10:55, Linus Walleij wrote:

On Mon, Aug 13, 2018 at 3:53 PM  wrote:


From: Michael Hennerich 

This fixes:
[BUG] gpio: gpio-adp5588: A possible sleep-in-atomic-context bug
   in adp5588_gpio_write()
[BUG] gpio: gpio-adp5588: A possible sleep-in-atomic-context bug
   in adp5588_gpio_direction_input()

Reported-by: Jia-Ju Bai 
Signed-off-by: Michael Hennerich 


Thanks Michael, excellent and prompt fix. Patch applied
for fixes (v4.19-rcs).

Should we even tag it for stable?


Thanks, Linus - Yes that would make sense.

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Stefan Steyerl, Thomas Edward Cribben, Eileen Wynne

Re: [PATCH] drivers: gpio: gpio-adp5588: Fix sleep-in-atomic-context bug

[Michael Hennerich]

On 29.08.2018 10:55, Linus Walleij wrote:

On Mon, Aug 13, 2018 at 3:53 PM  wrote:


From: Michael Hennerich 

This fixes:
[BUG] gpio: gpio-adp5588: A possible sleep-in-atomic-context bug
   in adp5588_gpio_write()
[BUG] gpio: gpio-adp5588: A possible sleep-in-atomic-context bug
   in adp5588_gpio_direction_input()

Reported-by: Jia-Ju Bai 
Signed-off-by: Michael Hennerich 


Thanks Michael, excellent and prompt fix. Patch applied
for fixes (v4.19-rcs).

Should we even tag it for stable?


Thanks, Linus - Yes that would make sense.

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Stefan Steyerl, Thomas Edward Cribben, Eileen Wynne

Re: [BUG] gpio: gpio-adp5588: A possible sleep-in-atomic-context bug in adp5588_gpio_direction_input()

[Michael Hennerich]

On 11.08.2018 04:03, Jia-Ju Bai wrote:

The driver may sleep with holding a spinlock.

The function call paths (from bottom to top) in Linux-4.16 are:

[FUNC] mutex_lock_nested
drivers/gpio/gpio-adp5588.c, 113: mutex_lock_nested in 
adp5588_gpio_direction_input
drivers/gpio/gpio-adp5588.c, 224: adp5588_gpio_direction_input in 
adp5588_irq_set_type
kernel/irq/manage.c, 686: [FUNC_PTR]adp5588_irq_set_type in 
__irq_set_trigger

kernel/irq/manage.c, 1350: __irq_set_trigger in __setup_irq
kernel/irq/manage.c, 1238: _raw_spin_lock_irqsave in __setup_irq

Note that [FUNC_PTR] means a function pointer call is used.

I do not find a good way to fix, so I only report.
This is found by my static analysis tool (DSAC).



Confirmed - adp5588_irq_set_type calls are atomic context and hence must 
not sleep. I'll move this stuff to irq_bus_sync_unlock.


Patch will be provided.

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [BUG] gpio: gpio-adp5588: A possible sleep-in-atomic-context bug in adp5588_gpio_direction_input()

[Michael Hennerich]

On 11.08.2018 04:03, Jia-Ju Bai wrote:

The driver may sleep with holding a spinlock.

The function call paths (from bottom to top) in Linux-4.16 are:

[FUNC] mutex_lock_nested
drivers/gpio/gpio-adp5588.c, 113: mutex_lock_nested in 
adp5588_gpio_direction_input
drivers/gpio/gpio-adp5588.c, 224: adp5588_gpio_direction_input in 
adp5588_irq_set_type
kernel/irq/manage.c, 686: [FUNC_PTR]adp5588_irq_set_type in 
__irq_set_trigger

kernel/irq/manage.c, 1350: __irq_set_trigger in __setup_irq
kernel/irq/manage.c, 1238: _raw_spin_lock_irqsave in __setup_irq

Note that [FUNC_PTR] means a function pointer call is used.

I do not find a good way to fix, so I only report.
This is found by my static analysis tool (DSAC).



Confirmed - adp5588_irq_set_type calls are atomic context and hence must 
not sleep. I'll move this stuff to irq_bus_sync_unlock.


Patch will be provided.

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2 09/14] staging: iio: ad7746: Add remove()

[Michael Hennerich]

On 18.04.2018 21:25, Hernán Gonzalez wrote:

On Sun, Apr 15, 2018 at 12:31 PM, Jonathan Cameron  wrote:

On Fri, 13 Apr 2018 13:36:46 -0300
Hernán Gonzalez  wrote:


This allows the driver to be probed and removed as a module powering it
down on remove().

Signed-off-by: Hernán Gonzalez 
---
  drivers/staging/iio/cdc/ad7746.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index c29a221..05506bf9 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -775,6 +775,31 @@ static int ad7746_probe(struct i2c_client *client,
   return 0;
  }

+static int ad7746_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct ad7746_chip_info *chip = iio_priv(indio_dev);
+ unsigned char regval;
+ int ret;
+
+ mutex_lock(>lock);
+
+ regval = chip->config | AD7746_CONF_MODE_PWRDN;
+ ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);

As this is a one off operation, perhaps it would be better to factor
it out to a utility function then use devm_add_action_or_reset in
the probe?

Also, I am nervous about this change as there doesn't seem to be
path in probe by which this is deliberately reversed?
It seems to be 'accidentally' handled by the read_raw write to the
CFG register.

The data sheet doesn't really mention much about this register
at all.  It may have special requirements to exit from power down - I have
no idea, but if it is costless, why do we bother with idle mode?

Perhaps Michael can confirm if this is safe to do or not.




I guess it'll be better to just drop this until Michael answers. I've
been trying to get a hold of the hw but with no success (or I have to
pay 3 times its cost in shipping), will keep searching though.



It's some time since I last worked with the device. I think it would be 
safe to restore the power on default in the configuration register upon 
probe. Which would be idle state. Besides a unspecified delay, I don't 
think there is anything else to handle here. Due to fact it's not 
specified it might not be required at all.
If your planning to do further cleanup on this driver, I ship you an 
eval board free of charge. Feel free to contact me off-list.






+
+ mutex_unlock(>lock);
+
+ if (ret < 0) {
+ dev_warn(>dev, "Could NOT Power Down!\n");
+ goto out;
+ }
+
+ iio_device_unregister(indio_dev);

You can't safely do this against the devm_iio_device_register.


+
+out:
+ return ret;
+}
+
  static const struct i2c_device_id ad7746_id[] = {
   { "ad7745", 7745 },
   { "ad7746", 7746 },
@@ -799,6 +824,7 @@ static struct i2c_driver ad7746_driver = {
   .of_match_table = of_match_ptr(ad7746_of_match),
   },
   .probe = ad7746_probe,
+ .remove = ad7746_remove,
   .id_table = ad7746_id,
  };
  module_i2c_driver(ad7746_driver);







--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2 09/14] staging: iio: ad7746: Add remove()

[Michael Hennerich]

On 18.04.2018 21:25, Hernán Gonzalez wrote:

On Sun, Apr 15, 2018 at 12:31 PM, Jonathan Cameron  wrote:

On Fri, 13 Apr 2018 13:36:46 -0300
Hernán Gonzalez  wrote:


This allows the driver to be probed and removed as a module powering it
down on remove().

Signed-off-by: Hernán Gonzalez 
---
  drivers/staging/iio/cdc/ad7746.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index c29a221..05506bf9 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -775,6 +775,31 @@ static int ad7746_probe(struct i2c_client *client,
   return 0;
  }

+static int ad7746_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct ad7746_chip_info *chip = iio_priv(indio_dev);
+ unsigned char regval;
+ int ret;
+
+ mutex_lock(>lock);
+
+ regval = chip->config | AD7746_CONF_MODE_PWRDN;
+ ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);

As this is a one off operation, perhaps it would be better to factor
it out to a utility function then use devm_add_action_or_reset in
the probe?

Also, I am nervous about this change as there doesn't seem to be
path in probe by which this is deliberately reversed?
It seems to be 'accidentally' handled by the read_raw write to the
CFG register.

The data sheet doesn't really mention much about this register
at all.  It may have special requirements to exit from power down - I have
no idea, but if it is costless, why do we bother with idle mode?

Perhaps Michael can confirm if this is safe to do or not.




I guess it'll be better to just drop this until Michael answers. I've
been trying to get a hold of the hw but with no success (or I have to
pay 3 times its cost in shipping), will keep searching though.



It's some time since I last worked with the device. I think it would be 
safe to restore the power on default in the configuration register upon 
probe. Which would be idle state. Besides a unspecified delay, I don't 
think there is anything else to handle here. Due to fact it's not 
specified it might not be required at all.
If your planning to do further cleanup on this driver, I ship you an 
eval board free of charge. Feel free to contact me off-list.






+
+ mutex_unlock(>lock);
+
+ if (ret < 0) {
+ dev_warn(>dev, "Could NOT Power Down!\n");
+ goto out;
+ }
+
+ iio_device_unregister(indio_dev);

You can't safely do this against the devm_iio_device_register.


+
+out:
+ return ret;
+}
+
  static const struct i2c_device_id ad7746_id[] = {
   { "ad7745", 7745 },
   { "ad7746", 7746 },
@@ -799,6 +824,7 @@ static struct i2c_driver ad7746_driver = {
   .of_match_table = of_match_ptr(ad7746_of_match),
   },
   .probe = ad7746_probe,
+ .remove = ad7746_remove,
   .id_table = ad7746_id,
  };
  module_i2c_driver(ad7746_driver);







--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/4] i2c: mux: ltc4306: switch to using .probe_new

[Michael Hennerich]

On 17.04.2018 16:32, Peter Rosin wrote:

Use the new probe style for i2c drivers.

Signed-off-by: Peter Rosin <p...@axentia.se>


Thanks Peter,

Don't know why i2c needs a new probe style to save one function parameter...
But this patch looks otherwise good to me.

Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/i2c/muxes/i2c-mux-ltc4306.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c 
b/drivers/i2c/muxes/i2c-mux-ltc4306.c
index 311b1cced0c0..a9af93259b19 100644
--- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
+++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
@@ -206,8 +206,7 @@ static const struct of_device_id ltc4306_of_match[] = {
  };
  MODULE_DEVICE_TABLE(of, ltc4306_of_match);
  
-static int ltc4306_probe(struct i2c_client *client,

-const struct i2c_device_id *id)
+static int ltc4306_probe(struct i2c_client *client)
  {
struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
const struct chip_desc *chip;
@@ -221,7 +220,7 @@ static int ltc4306_probe(struct i2c_client *client,
chip = of_device_get_match_data(>dev);
  
  	if (!chip)

-   chip = [id->driver_data];
+   chip = [i2c_match_id(ltc4306_id, client)->driver_data];
  
  	idle_disc = device_property_read_bool(>dev,

  "i2c-mux-idle-disconnect");
@@ -310,7 +309,7 @@ static struct i2c_driver ltc4306_driver = {
.name   = "ltc4306",
.of_match_table = of_match_ptr(ltc4306_of_match),
},
-   .probe  = ltc4306_probe,
+   .probe_new  = ltc4306_probe,
.remove = ltc4306_remove,
.id_table   = ltc4306_id,
  };




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/4] i2c: mux: ltc4306: switch to using .probe_new

[Michael Hennerich]

On 17.04.2018 16:32, Peter Rosin wrote:

Use the new probe style for i2c drivers.

Signed-off-by: Peter Rosin 


Thanks Peter,

Don't know why i2c needs a new probe style to save one function parameter...
But this patch looks otherwise good to me.

Acked-by: Michael Hennerich 


---
  drivers/i2c/muxes/i2c-mux-ltc4306.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c 
b/drivers/i2c/muxes/i2c-mux-ltc4306.c
index 311b1cced0c0..a9af93259b19 100644
--- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
+++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
@@ -206,8 +206,7 @@ static const struct of_device_id ltc4306_of_match[] = {
  };
  MODULE_DEVICE_TABLE(of, ltc4306_of_match);
  
-static int ltc4306_probe(struct i2c_client *client,

-const struct i2c_device_id *id)
+static int ltc4306_probe(struct i2c_client *client)
  {
struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
const struct chip_desc *chip;
@@ -221,7 +220,7 @@ static int ltc4306_probe(struct i2c_client *client,
chip = of_device_get_match_data(>dev);
  
  	if (!chip)

-   chip = [id->driver_data];
+   chip = [i2c_match_id(ltc4306_id, client)->driver_data];
  
  	idle_disc = device_property_read_bool(>dev,

  "i2c-mux-idle-disconnect");
@@ -310,7 +309,7 @@ static struct i2c_driver ltc4306_driver = {
.name   = "ltc4306",
.of_match_table = of_match_ptr(ltc4306_of_match),
},
-   .probe  = ltc4306_probe,
+   .probe_new  = ltc4306_probe,
.remove = ltc4306_remove,
.id_table   = ltc4306_id,
  };




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 5/5] backlight: adp8860: document sysfs attributes

[Michael Hennerich]

On 26.01.2018 15:55, Aishwarya Pant wrote:

Add documentation for sysfs interface of adp8860 series backlight
devices by reading through code and git commits.

Signed-off-by: Aishwarya Pant <aishp...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  .../ABI/testing/sysfs-class-backlight-adp8860  | 54 ++
  1 file changed, 54 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-class-backlight-adp8860

diff --git a/Documentation/ABI/testing/sysfs-class-backlight-adp8860 
b/Documentation/ABI/testing/sysfs-class-backlight-adp8860
new file mode 100644
index ..54d61c788b1b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-backlight-adp8860
@@ -0,0 +1,54 @@
+sysfs interface for analog devices adp8860 backlight driver
+---
+
+The backlight brightness control operates at three different levels for the
+adp8860, adp8861 and adp8863 devices: daylight (level 1), office (level 2) and
+dark (level 3). By default the brightness operates at the daylight brightness
+level.
+
+What:  /sys/class/backlight//ambient_light_level
+Date:  Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich <michael.henner...@analog.com>
+Description:
+   (RO) 13-bit conversion value for the first light sensor—high
+   byte (Bit 12 to Bit 8). The value is updated every 80 ms (when
+   the light sensor is enabled).
+
+
+What:  /sys/class/backlight//ambient_light_zone
+Date:  Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich <michael.henner...@analog.com>
+Description:
+   (RW) Read or write the specific level at which the backlight
+   operates. Value "0" enables automatic ambient light sensing, and
+   values "1", "2" or "3" set the control to daylight, office or
+   dark respectively.
+
+
+What:  /sys/class/backlight//l1_daylight_max
+What:  /sys/class/backlight//l2_office_max
+What:  /sys/class/backlight//l3_dark_max
+Date:      Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich <michael.henner...@analog.com>
+Description:
+   (RW) Maximum current setting for the backlight when brightness
+   is at one of the three levels (daylight, office or dark). This
+   is an input code between 0 and 127, which is transformed to a
+   value between 0 mA and 30 mA using linear or non-linear
+   algorithms.
+
+
+What:  /sys/class/backlight//l1_daylight_dim
+What:  /sys/class/backlight//l2_office_dim
+What:  /sys/class/backlight//l3_dark_dim
+Date:      Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich <michael.henner...@analog.com>
+Description:
+   (RW) Dim current setting for the backlight when brightness is at
+   one of the three levels (daylight, office or dark). This is an
+   input code between 0 and 127, which is transformed to a value
+   between 0 mA and 30 mA using linear or non-linear algorithms.

Re: [PATCH 5/5] backlight: adp8860: document sysfs attributes

[Michael Hennerich]

On 26.01.2018 15:55, Aishwarya Pant wrote:

Add documentation for sysfs interface of adp8860 series backlight
devices by reading through code and git commits.

Signed-off-by: Aishwarya Pant 


Acked-by: Michael Hennerich 


---
  .../ABI/testing/sysfs-class-backlight-adp8860  | 54 ++
  1 file changed, 54 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-class-backlight-adp8860

diff --git a/Documentation/ABI/testing/sysfs-class-backlight-adp8860 
b/Documentation/ABI/testing/sysfs-class-backlight-adp8860
new file mode 100644
index ..54d61c788b1b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-backlight-adp8860
@@ -0,0 +1,54 @@
+sysfs interface for analog devices adp8860 backlight driver
+---
+
+The backlight brightness control operates at three different levels for the
+adp8860, adp8861 and adp8863 devices: daylight (level 1), office (level 2) and
+dark (level 3). By default the brightness operates at the daylight brightness
+level.
+
+What:  /sys/class/backlight//ambient_light_level
+Date:  Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich 
+Description:
+   (RO) 13-bit conversion value for the first light sensor—high
+   byte (Bit 12 to Bit 8). The value is updated every 80 ms (when
+   the light sensor is enabled).
+
+
+What:  /sys/class/backlight//ambient_light_zone
+Date:  Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich 
+Description:
+   (RW) Read or write the specific level at which the backlight
+   operates. Value "0" enables automatic ambient light sensing, and
+   values "1", "2" or "3" set the control to daylight, office or
+   dark respectively.
+
+
+What:  /sys/class/backlight//l1_daylight_max
+What:  /sys/class/backlight//l2_office_max
+What:  /sys/class/backlight//l3_dark_max
+Date:  Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich 
+Description:
+   (RW) Maximum current setting for the backlight when brightness
+   is at one of the three levels (daylight, office or dark). This
+   is an input code between 0 and 127, which is transformed to a
+   value between 0 mA and 30 mA using linear or non-linear
+   algorithms.
+
+
+What:  /sys/class/backlight//l1_daylight_dim
+What:  /sys/class/backlight//l2_office_dim
+What:  /sys/class/backlight//l3_dark_dim
+Date:  Apr, 2010
+KernelVersion: v2.6.35
+Contact:   Michael Hennerich 
+Description:
+   (RW) Dim current setting for the backlight when brightness is at
+   one of the three levels (daylight, office or dark). This is an
+   input code between 0 and 127, which is transformed to a value
+   between 0 mA and 30 mA using linear or non-linear algorithms.

Re: [PATCH 4/5] backlight: adp5520: document sysfs attributes

[Michael Hennerich]

On 26.01.2018 15:54, Aishwarya Pant wrote:

Add documentation for sysfs interface of adp5520/adp5501 analog devices
backlight driver by reading code and looking through git commit logs.

Signed-off-by: Aishwarya Pant <aishp...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  .../ABI/testing/sysfs-class-backlight-adp5520  | 31 ++
  1 file changed, 31 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-class-backlight-adp5520

diff --git a/Documentation/ABI/testing/sysfs-class-backlight-adp5520 
b/Documentation/ABI/testing/sysfs-class-backlight-adp5520
new file mode 100644
index ..34b6ebafa210
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-backlight-adp5520
@@ -0,0 +1,31 @@
+sysfs interface for analog devices adp5520(01) backlight driver
+---
+
+The backlight brightness control operates at three different levels for the
+adp5520 and adp5501 devices: daylight (level 1), office (level 2) and dark
+(level 3). By default the brightness operates at the daylight brightness level.
+
+What:  /sys/class/backlight//daylight_max
+What:  /sys/class/backlight//office_max
+What:  /sys/class/backlight//dark_max
+Date:  Sep, 2009
+KernelVersion: v2.6.32
+Contact:   Michael Hennerich <michael.henner...@analog.com>
+Description:
+   (RW) Maximum current setting for the backlight when brightness
+   is at one of the three levels (daylight, office or dark). This
+   is an input code between 0 and 127, which is transformed to a
+   value between 0 mA and 30 mA using linear or non-linear
+   algorithms.
+
+What:  /sys/class/backlight//daylight_dim
+What:  /sys/class/backlight//office_dim
+What:  /sys/class/backlight//dark_dim
+Date:  Sep, 2009
+KernelVersion: v2.6.32
+Contact:   Michael Hennerich <michael.henner...@analog.com>
+Description:
+   (RW) Dim current setting for the backlight when brightness is at
+   one of the three levels (daylight, office or dark). This is an
+   input code between 0 and 127, which is transformed to a value
+   between 0 mA and 30 mA using linear or non-linear algorithms.

Re: [PATCH 4/5] backlight: adp5520: document sysfs attributes

[Michael Hennerich]

On 26.01.2018 15:54, Aishwarya Pant wrote:

Add documentation for sysfs interface of adp5520/adp5501 analog devices
backlight driver by reading code and looking through git commit logs.

Signed-off-by: Aishwarya Pant 


Acked-by: Michael Hennerich 


---
  .../ABI/testing/sysfs-class-backlight-adp5520  | 31 ++
  1 file changed, 31 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-class-backlight-adp5520

diff --git a/Documentation/ABI/testing/sysfs-class-backlight-adp5520 
b/Documentation/ABI/testing/sysfs-class-backlight-adp5520
new file mode 100644
index ..34b6ebafa210
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-backlight-adp5520
@@ -0,0 +1,31 @@
+sysfs interface for analog devices adp5520(01) backlight driver
+---
+
+The backlight brightness control operates at three different levels for the
+adp5520 and adp5501 devices: daylight (level 1), office (level 2) and dark
+(level 3). By default the brightness operates at the daylight brightness level.
+
+What:  /sys/class/backlight//daylight_max
+What:  /sys/class/backlight//office_max
+What:  /sys/class/backlight//dark_max
+Date:  Sep, 2009
+KernelVersion: v2.6.32
+Contact:   Michael Hennerich 
+Description:
+   (RW) Maximum current setting for the backlight when brightness
+   is at one of the three levels (daylight, office or dark). This
+   is an input code between 0 and 127, which is transformed to a
+   value between 0 mA and 30 mA using linear or non-linear
+   algorithms.
+
+What:  /sys/class/backlight//daylight_dim
+What:  /sys/class/backlight//office_dim
+What:  /sys/class/backlight//dark_dim
+Date:  Sep, 2009
+KernelVersion: v2.6.32
+Contact:   Michael Hennerich 
+Description:
+   (RW) Dim current setting for the backlight when brightness is at
+   one of the three levels (daylight, office or dark). This is an
+   input code between 0 and 127, which is transformed to a value
+   between 0 mA and 30 mA using linear or non-linear algorithms.

Re: [PATCH] Input: adp5520-keys: Delete an error message for a failed memory allocation in adp5520_keys_probe()

[Michael Hennerich]

On 27.01.2018 15:20, SF Markus Elfring wrote:

From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 27 Jan 2018 15:15:52 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/input/keyboard/adp5520-keys.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/adp5520-keys.c 
b/drivers/input/keyboard/adp5520-keys.c
index f0b9b37bde58..72e01cb77881 100644
--- a/drivers/input/keyboard/adp5520-keys.c
+++ b/drivers/input/keyboard/adp5520-keys.c
@@ -91,10 +91,8 @@ static int adp5520_keys_probe(struct platform_device *pdev)
return -EINVAL;
  
  	dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);

-   if (!dev) {
-   dev_err(>dev, "failed to alloc memory\n");
+   if (!dev)
return -ENOMEM;
-   }
  
  	input = devm_input_allocate_device(>dev);

if (!input)

Re: [PATCH] Input: adp5520-keys: Delete an error message for a failed memory allocation in adp5520_keys_probe()

[Michael Hennerich]

On 27.01.2018 15:20, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Sat, 27 Jan 2018 15:15:52 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 


Acked-by: Michael Hennerich 


---
  drivers/input/keyboard/adp5520-keys.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/adp5520-keys.c 
b/drivers/input/keyboard/adp5520-keys.c
index f0b9b37bde58..72e01cb77881 100644
--- a/drivers/input/keyboard/adp5520-keys.c
+++ b/drivers/input/keyboard/adp5520-keys.c
@@ -91,10 +91,8 @@ static int adp5520_keys_probe(struct platform_device *pdev)
return -EINVAL;
  
  	dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);

-   if (!dev) {
-   dev_err(>dev, "failed to alloc memory\n");
+   if (!dev)
return -ENOMEM;
-   }
  
  	input = devm_input_allocate_device(>dev);

if (!input)

Re: [PATCH] staging: iio: ad7192: Use the dedicated reset function

[Michael Hennerich]

On 14.09.2017 15:50, Stefan Popa wrote:

SPI host drivers can use DMA to transfer data, so the buffer should be properly 
allocated.
Keeping it on the stack could cause an undefined behavior.

The dedicated reset function solves this issue.

Signed-off-by: Stefan Popa <stefan.p...@analog.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>

Well done!



---
  drivers/staging/iio/adc/ad7192.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d11c6de..6150d27 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
unsigned long long scale_uv;
int i, ret, id;
-   u8 ones[6];
  
  	/* reset the serial interface */

-   memset(, 0xFF, 6);
-   ret = spi_write(st->sd.spi, , 6);
+   ret = ad_sd_reset(>sd, 48);
if (ret < 0)
goto out;
usleep_range(500, 1000); /* Wait for at least 500us */




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] staging: iio: ad7192: Use the dedicated reset function

[Michael Hennerich]

On 14.09.2017 15:50, Stefan Popa wrote:

SPI host drivers can use DMA to transfer data, so the buffer should be properly 
allocated.
Keeping it on the stack could cause an undefined behavior.

The dedicated reset function solves this issue.

Signed-off-by: Stefan Popa 


Acked-by: Michael Hennerich 

Well done!



---
  drivers/staging/iio/adc/ad7192.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d11c6de..6150d27 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
unsigned long long scale_uv;
int i, ret, id;
-   u8 ones[6];
  
  	/* reset the serial interface */

-   memset(, 0xFF, 6);
-   ret = spi_write(st->sd.spi, , 6);
+   ret = ad_sd_reset(>sd, 48);
if (ret < 0)
goto out;
usleep_range(500, 1000); /* Wait for at least 500us */




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/2] video: adp8860: move header file out of I2C realm

[Michael Hennerich]

On 07.06.2017 12:57, Daniel Thompson wrote:

On 21/05/17 23:09, Wolfram Sang wrote:

include/linux/i2c is not for client devices. Move the header file to a
more appropriate location.

Signed-off-by: Wolfram Sang <w...@the-dreams.de>


Acked-by: Daniel Thompson <daniel.thomp...@linaro.org>


Acked-by: Michael Hennerich <michael.henner...@analog.com>




---
  arch/blackfin/mach-bf537/boards/stamp.c| 2 +-
  drivers/video/backlight/adp8860_bl.c   | 2 +-
  include/linux/{i2c => platform_data}/adp8860.h | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename include/linux/{i2c => platform_data}/adp8860.h (100%)

diff --git a/arch/blackfin/mach-bf537/boards/stamp.c 
b/arch/blackfin/mach-bf537/boards/stamp.c

index 24985e658c19cd..7db90c72bd8ddc 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -2047,7 +2047,7 @@ static struct adp8870_backlight_platform_data 
adp8870_pdata = {

  #endif
  #if IS_ENABLED(CONFIG_BACKLIGHT_ADP8860)
-#include 
+#include 
  static struct led_info adp8860_leds[] = {
  {
  .name = "adp8860-led7",
diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c

index 510e559c060e59..e7315bf14d6015 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -18,7 +18,7 @@
  #include 
  #include 
-#include 
+#include 
  #define ADP8860_EXT_FEATURES
  #define ADP8860_USE_LEDS
diff --git a/include/linux/i2c/adp8860.h 
b/include/linux/platform_data/adp8860.h

similarity index 100%
rename from include/linux/i2c/adp8860.h
rename to include/linux/platform_data/adp8860.h







--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/2] video: adp8860: move header file out of I2C realm

[Michael Hennerich]

On 07.06.2017 12:57, Daniel Thompson wrote:

On 21/05/17 23:09, Wolfram Sang wrote:

include/linux/i2c is not for client devices. Move the header file to a
more appropriate location.

Signed-off-by: Wolfram Sang 


Acked-by: Daniel Thompson 


Acked-by: Michael Hennerich 




---
  arch/blackfin/mach-bf537/boards/stamp.c| 2 +-
  drivers/video/backlight/adp8860_bl.c   | 2 +-
  include/linux/{i2c => platform_data}/adp8860.h | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename include/linux/{i2c => platform_data}/adp8860.h (100%)

diff --git a/arch/blackfin/mach-bf537/boards/stamp.c 
b/arch/blackfin/mach-bf537/boards/stamp.c

index 24985e658c19cd..7db90c72bd8ddc 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -2047,7 +2047,7 @@ static struct adp8870_backlight_platform_data 
adp8870_pdata = {

  #endif
  #if IS_ENABLED(CONFIG_BACKLIGHT_ADP8860)
-#include 
+#include 
  static struct led_info adp8860_leds[] = {
  {
  .name = "adp8860-led7",
diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c

index 510e559c060e59..e7315bf14d6015 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -18,7 +18,7 @@
  #include 
  #include 
-#include 
+#include 
  #define ADP8860_EXT_FEATURES
  #define ADP8860_USE_LEDS
diff --git a/include/linux/i2c/adp8860.h 
b/include/linux/platform_data/adp8860.h

similarity index 100%
rename from include/linux/i2c/adp8860.h
rename to include/linux/platform_data/adp8860.h







--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v5 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 12.04.2017 18:53, Peter Rosin wrote:

On 2017-04-12 10:26, Linus Walleij wrote:

On Tue, Apr 11, 2017 at 2:16 PM,  <michael.henner...@analog.com> wrote:


From: Michael Hennerich <michael.henner...@analog.com>

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.


Great, thanks for your contribution Michael! Both patches pushed to the
for-next branch of the i2c-mux tree.


Signed-off-by: Michael Hennerich <michael.henner...@analog.com>

(...)

Changes since v4:


Reviewed-by: Linus Walleij <linus.wall...@linaro.org>

Please go ahead and merge to the i2c tree if you like.


Yup, thanks for the review! (But not merged directly to the i2c tree,
although they will get there eventually...)

Cheers,
peda




Peter and Linus,

Thanks for your precious time - really appreciated!

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v5 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 12.04.2017 18:53, Peter Rosin wrote:

On 2017-04-12 10:26, Linus Walleij wrote:

On Tue, Apr 11, 2017 at 2:16 PM,   wrote:


From: Michael Hennerich 

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.


Great, thanks for your contribution Michael! Both patches pushed to the
for-next branch of the i2c-mux tree.


Signed-off-by: Michael Hennerich 

(...)

Changes since v4:


Reviewed-by: Linus Walleij 

Please go ahead and merge to the i2c tree if you like.


Yup, thanks for the review! (But not merged directly to the i2c tree,
although they will get there eventually...)

Cheers,
peda




Peter and Linus,

Thanks for your precious time - really appreciated!

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v4 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 06.04.2017 22:03, Peter Rosin wrote:

On 2017-04-06 13:31, Michael Hennerich wrote:

On 06.04.2017 10:39, Peter Rosin wrote:


Hi Peter,

again - thanks for your review.
Comments below.


+static const struct regmap_config ltc4306_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = LTC_REG_SWITCH,
+   .volatile_reg = ltc4306_is_volatile_reg,
+   .cache_type = REGCACHE_RBTREE,


Did you consider REGCACHE_FLAT? There are very few registers and no hole
in the map, and maintaining a tree seems like total overkill.


There is no reason to use REGCACHE_FLAT, in our case it will be a single
node.


Ok, so that makes me wonder what need REGCACHE_FLAT satisfies? To me,
a flat regmap seems like a perfect fit here. Oh well...


https://lkml.org/lkml/2012/12/19/172

It's not worth arguing - if you prefer FLAT - then it's FLAT
While it's still round :-)



+static int ltc4306_gpio_init(struct ltc4306 *data)
+{
+   struct device *dev = regmap_get_device(data->regmap);
+
+   if (!data->chip->num_gpios)
+   return 0;
+
+   data->gpiochip.label = dev_name(dev);
+   data->gpiochip.base = -1;
+   data->gpiochip.ngpio = data->chip->num_gpios;
+   data->gpiochip.parent = dev;
+   data->gpiochip.can_sleep = true;
+   data->gpiochip.direction_input = ltc4306_gpio_direction_input;
+   data->gpiochip.direction_output = ltc4306_gpio_direction_output;


I'm missing a get_direction op?


No - its purely optional - the vast majority of gpiochips don't
implement it.

linux/drivers/gpio$ grep -lr --include \*.c get_direction | wc
  36  36 523
linux/drivers/gpio$ grep -Lr --include \*.c get_direction | wc
 101 1011461


Ok, but while optional, why not provide it? The implementation would
be about as simple as ltc4306_gpio_get, no?


ok - convinced me.


I'll send version 5 shortly.


--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v4 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 06.04.2017 22:03, Peter Rosin wrote:

On 2017-04-06 13:31, Michael Hennerich wrote:

On 06.04.2017 10:39, Peter Rosin wrote:


Hi Peter,

again - thanks for your review.
Comments below.


+static const struct regmap_config ltc4306_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = LTC_REG_SWITCH,
+   .volatile_reg = ltc4306_is_volatile_reg,
+   .cache_type = REGCACHE_RBTREE,


Did you consider REGCACHE_FLAT? There are very few registers and no hole
in the map, and maintaining a tree seems like total overkill.


There is no reason to use REGCACHE_FLAT, in our case it will be a single
node.


Ok, so that makes me wonder what need REGCACHE_FLAT satisfies? To me,
a flat regmap seems like a perfect fit here. Oh well...


https://lkml.org/lkml/2012/12/19/172

It's not worth arguing - if you prefer FLAT - then it's FLAT
While it's still round :-)



+static int ltc4306_gpio_init(struct ltc4306 *data)
+{
+   struct device *dev = regmap_get_device(data->regmap);
+
+   if (!data->chip->num_gpios)
+   return 0;
+
+   data->gpiochip.label = dev_name(dev);
+   data->gpiochip.base = -1;
+   data->gpiochip.ngpio = data->chip->num_gpios;
+   data->gpiochip.parent = dev;
+   data->gpiochip.can_sleep = true;
+   data->gpiochip.direction_input = ltc4306_gpio_direction_input;
+   data->gpiochip.direction_output = ltc4306_gpio_direction_output;


I'm missing a get_direction op?


No - its purely optional - the vast majority of gpiochips don't
implement it.

linux/drivers/gpio$ grep -lr --include \*.c get_direction | wc
  36  36 523
linux/drivers/gpio$ grep -Lr --include \*.c get_direction | wc
 101 1011461


Ok, but while optional, why not provide it? The implementation would
be about as simple as ltc4306_gpio_get, no?


ok - convinced me.


I'll send version 5 shortly.


--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v4 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 10.04.2017 22:04, Linus Walleij wrote:

On Wed, Apr 5, 2017 at 3:07 PM,  <michael.henner...@analog.com> wrote:


From: Michael Hennerich <michael.henner...@analog.com>

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.

Signed-off-by: Michael Hennerich <michael.henner...@analog.com>


Okay!


Hi Linus,

Thanks for your review.
Comments below.




+#include 
+#include 
+#include 
+#include 


Why are you including all these?
Normally a GPIO driver should just include



Well - this driver is also a gpio consumer.
But right I can drop gpio.h, and while gpio/driver.h also includes 
device.h - we don't need it here as well.





+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 



+static int ltc4306_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+   struct ltc4306 *data = gpiochip_get_data(chip);
+   unsigned int val;
+   int ret;
+
+   ret = regmap_read(data->regmap, LTC_REG_CONFIG, );
+   if (ret < 0)
+   return ret;
+
+   return (val & BIT(1 - offset));


Do this:

return !!(val & BIT(1 - offset));

So you clamp the return value to [0,1]


That's what I had in a previous version of the patch.
Then I noticed gpiolib is also doing this.
Anyways I'll add it back.




+static int ltc4306_gpio_set_config(struct gpio_chip *chip,
+  unsigned int offset, unsigned long config)
+{
+   struct ltc4306 *data = gpiochip_get_data(chip);
+   unsigned int val;
+
+   switch (pinconf_to_config_param(config)) {
+   case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+   val = 0;
+   break;
+   case PIN_CONFIG_DRIVE_PUSH_PULL:
+   val = BIT(4 - offset);
+   break;
+   default:
+   return -ENOTSUPP;
+   }
+
+   return regmap_update_bits(data->regmap, LTC_REG_MODE,
+ BIT(4 - offset), val);
+}


Nice!


+   data->gpiochip.label = dev_name(dev);
+   data->gpiochip.base = -1;
+   data->gpiochip.ngpio = data->chip->num_gpios;
+   data->gpiochip.parent = dev;
+   data->gpiochip.can_sleep = true;
+   data->gpiochip.direction_input = ltc4306_gpio_direction_input;
+   data->gpiochip.direction_output = ltc4306_gpio_direction_output;
+   data->gpiochip.get = ltc4306_gpio_get;
+   data->gpiochip.set = ltc4306_gpio_set;
+   data->gpiochip.set_config = ltc4306_gpio_set_config;
+   data->gpiochip.owner = THIS_MODULE;


Please implement .get_direction().
This is very helpful to userspace, have you tested to use tools/gpio/*
from the kernel? Like lsgpio?


Ok - convinced me.



Yours,
Linus Walleij



--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v4 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 10.04.2017 22:04, Linus Walleij wrote:

On Wed, Apr 5, 2017 at 3:07 PM,   wrote:


From: Michael Hennerich 

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.

Signed-off-by: Michael Hennerich 


Okay!


Hi Linus,

Thanks for your review.
Comments below.




+#include 
+#include 
+#include 
+#include 


Why are you including all these?
Normally a GPIO driver should just include



Well - this driver is also a gpio consumer.
But right I can drop gpio.h, and while gpio/driver.h also includes 
device.h - we don't need it here as well.





+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 



+static int ltc4306_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+   struct ltc4306 *data = gpiochip_get_data(chip);
+   unsigned int val;
+   int ret;
+
+   ret = regmap_read(data->regmap, LTC_REG_CONFIG, );
+   if (ret < 0)
+   return ret;
+
+   return (val & BIT(1 - offset));


Do this:

return !!(val & BIT(1 - offset));

So you clamp the return value to [0,1]


That's what I had in a previous version of the patch.
Then I noticed gpiolib is also doing this.
Anyways I'll add it back.




+static int ltc4306_gpio_set_config(struct gpio_chip *chip,
+  unsigned int offset, unsigned long config)
+{
+   struct ltc4306 *data = gpiochip_get_data(chip);
+   unsigned int val;
+
+   switch (pinconf_to_config_param(config)) {
+   case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+   val = 0;
+   break;
+   case PIN_CONFIG_DRIVE_PUSH_PULL:
+   val = BIT(4 - offset);
+   break;
+   default:
+   return -ENOTSUPP;
+   }
+
+   return regmap_update_bits(data->regmap, LTC_REG_MODE,
+ BIT(4 - offset), val);
+}


Nice!


+   data->gpiochip.label = dev_name(dev);
+   data->gpiochip.base = -1;
+   data->gpiochip.ngpio = data->chip->num_gpios;
+   data->gpiochip.parent = dev;
+   data->gpiochip.can_sleep = true;
+   data->gpiochip.direction_input = ltc4306_gpio_direction_input;
+   data->gpiochip.direction_output = ltc4306_gpio_direction_output;
+   data->gpiochip.get = ltc4306_gpio_get;
+   data->gpiochip.set = ltc4306_gpio_set;
+   data->gpiochip.set_config = ltc4306_gpio_set_config;
+   data->gpiochip.owner = THIS_MODULE;


Please implement .get_direction().
This is very helpful to userspace, have you tested to use tools/gpio/*
from the kernel? Like lsgpio?


Ok - convinced me.



Yours,
Linus Walleij



--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v4 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 06.04.2017 10:39, Peter Rosin wrote:

Hi Michael,

I would still like to hear from someone with more gpio experience.


I'll ping Linus.



Anyway, from my point of view, there's just a few minor things left,
with comments inline as usual.

Thanks for you patience!


Thanks for review.



Cheers,
peda

On 2017-04-05 15:07, michael.henner...@analog.com wrote:

From: Michael Hennerich <michael.henner...@analog.com>

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.

Signed-off-by: Michael Hennerich <michael.henner...@analog.com>

---

Changes since v1:

 - Sort makefile entries
 - Sort driver includes
 - Use proper defines
 - Miscellaneous coding style fixups
 - Rename mux select callback
 - Revise i2c-mux-idle-disconnect handling
 - Add ENABLE GPIO handling on error and device removal.
 - Remove surplus of_match_device call.

Changes since v2:

 - Stop double error reporting (i2c_mux_add_adapter)
 - Change subject
 - Split dt bindings to separate patch

Changes since v3:

 - Change subject and add spaces
 - Convert to I2C_MUX_LOCKED
 - Convert to regmap
 - Remove local register cache
 - Restore previous ENABLE GPIO handling
 - Initially pulse ENABLE low
 - Eliminate i2c client struct in driver state structure
 - Simplify error return path
 - Misc minor cleanups
---
 MAINTAINERS |   8 +
 drivers/i2c/muxes/Kconfig   |  11 ++
 drivers/i2c/muxes/Makefile  |   1 +
 drivers/i2c/muxes/i2c-mux-ltc4306.c | 310 
 4 files changed, 330 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-ltc4306.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..9a27a19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7698,6 +7698,14 @@ S:   Maintained
 F: Documentation/hwmon/ltc4261
 F: drivers/hwmon/ltc4261.c

+LTC4306 I2C MULTIPLEXER DRIVER
+M: Michael Hennerich <michael.henner...@analog.com>
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-mux-ltc4306.c
+F: Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
+
 LTP (Linux Test Project)
 M: Mike Frysinger <vap...@gentoo.org>
 M: Cyril Hrubis <chru...@suse.cz>
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 10b3d17..41153b4 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -30,6 +30,17 @@ config I2C_MUX_GPIO
  This driver can also be built as a module.  If so, the module
  will be called i2c-mux-gpio.

+config I2C_MUX_LTC4306
+   tristate "LTC LTC4306/5 I2C multiplexer"
+   select GPIOLIB
+   select REGMAP_I2C
+   help
+ If you say yes here you get support for the LTC LTC4306 or LTC4305


This reads a bit funny, and I think you should just spell out the
first LTC? But perhaps not in the tristate above though, depending
on how long it gets?


Ok - I rename LTC -> Analog Devices




+ I2C mux/switch devices.
+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-mux-ltc4306.
+
 config I2C_MUX_PCA9541
tristate "NXP PCA9541 I2C Master Selector"
help
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 9948fa4..ff7618c 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE)+= 
i2c-arb-gpio-challenge.o
 obj-$(CONFIG_I2C_DEMUX_PINCTRL)+= i2c-demux-pinctrl.o

 obj-$(CONFIG_I2C_MUX_GPIO) += i2c-mux-gpio.o
+obj-$(CONFIG_I2C_MUX_LTC4306)  += i2c-mux-ltc4306.o
 obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
 obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
 obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c 
b/drivers/i2c/muxes/i2c-mux-ltc4306.c
new file mode 100644
index 000..7d34434
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
@@ -0,0 +1,310 @@
+/*
+ * Linear Technology LTC4306 and LTC4305 I2C multiplexer/switch
+ *
+ * Copyright (C) 2017 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * Based on: i2c-mux-pca954x.c
+ *
+ * Datasheet: http://cds.linear.com/docs/en/datasheet/4306.pdf
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define LTC4305_MAX_NCHANS 2
+#define LTC4306_MAX_NCHANS 4
+
+#define LTC_REG_STATUS 0x0
+#define LTC_REG_CONFIG 0x1
+#define LTC_REG_MODE   0x2
+#define LTC_REG_SWITCH 0x3
+
+#define LTC_DOWNSTREAM_ACCL_EN BIT(6)
+#define LTC_UPSTREAM_ACCL_EN   BIT(7)
+
+#define LTC_GPIO_ALL_INPUT 0xC0
+#define

Re: [PATCH v4 2/2] i2c: mux: ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 06.04.2017 10:39, Peter Rosin wrote:

Hi Michael,

I would still like to hear from someone with more gpio experience.


I'll ping Linus.



Anyway, from my point of view, there's just a few minor things left,
with comments inline as usual.

Thanks for you patience!


Thanks for review.



Cheers,
peda

On 2017-04-05 15:07, michael.henner...@analog.com wrote:

From: Michael Hennerich 

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.

Signed-off-by: Michael Hennerich 

---

Changes since v1:

 - Sort makefile entries
 - Sort driver includes
 - Use proper defines
 - Miscellaneous coding style fixups
 - Rename mux select callback
 - Revise i2c-mux-idle-disconnect handling
 - Add ENABLE GPIO handling on error and device removal.
 - Remove surplus of_match_device call.

Changes since v2:

 - Stop double error reporting (i2c_mux_add_adapter)
 - Change subject
 - Split dt bindings to separate patch

Changes since v3:

 - Change subject and add spaces
 - Convert to I2C_MUX_LOCKED
 - Convert to regmap
 - Remove local register cache
 - Restore previous ENABLE GPIO handling
 - Initially pulse ENABLE low
 - Eliminate i2c client struct in driver state structure
 - Simplify error return path
 - Misc minor cleanups
---
 MAINTAINERS |   8 +
 drivers/i2c/muxes/Kconfig   |  11 ++
 drivers/i2c/muxes/Makefile  |   1 +
 drivers/i2c/muxes/i2c-mux-ltc4306.c | 310 
 4 files changed, 330 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-ltc4306.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..9a27a19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7698,6 +7698,14 @@ S:   Maintained
 F: Documentation/hwmon/ltc4261
 F: drivers/hwmon/ltc4261.c

+LTC4306 I2C MULTIPLEXER DRIVER
+M: Michael Hennerich 
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-mux-ltc4306.c
+F: Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
+
 LTP (Linux Test Project)
 M: Mike Frysinger 
 M: Cyril Hrubis 
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 10b3d17..41153b4 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -30,6 +30,17 @@ config I2C_MUX_GPIO
  This driver can also be built as a module.  If so, the module
  will be called i2c-mux-gpio.

+config I2C_MUX_LTC4306
+   tristate "LTC LTC4306/5 I2C multiplexer"
+   select GPIOLIB
+   select REGMAP_I2C
+   help
+ If you say yes here you get support for the LTC LTC4306 or LTC4305


This reads a bit funny, and I think you should just spell out the
first LTC? But perhaps not in the tristate above though, depending
on how long it gets?


Ok - I rename LTC -> Analog Devices




+ I2C mux/switch devices.
+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-mux-ltc4306.
+
 config I2C_MUX_PCA9541
tristate "NXP PCA9541 I2C Master Selector"
help
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 9948fa4..ff7618c 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE)+= 
i2c-arb-gpio-challenge.o
 obj-$(CONFIG_I2C_DEMUX_PINCTRL)+= i2c-demux-pinctrl.o

 obj-$(CONFIG_I2C_MUX_GPIO) += i2c-mux-gpio.o
+obj-$(CONFIG_I2C_MUX_LTC4306)  += i2c-mux-ltc4306.o
 obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
 obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
 obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c 
b/drivers/i2c/muxes/i2c-mux-ltc4306.c
new file mode 100644
index 000..7d34434
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
@@ -0,0 +1,310 @@
+/*
+ * Linear Technology LTC4306 and LTC4305 I2C multiplexer/switch
+ *
+ * Copyright (C) 2017 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * Based on: i2c-mux-pca954x.c
+ *
+ * Datasheet: http://cds.linear.com/docs/en/datasheet/4306.pdf
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define LTC4305_MAX_NCHANS 2
+#define LTC4306_MAX_NCHANS 4
+
+#define LTC_REG_STATUS 0x0
+#define LTC_REG_CONFIG 0x1
+#define LTC_REG_MODE   0x2
+#define LTC_REG_SWITCH 0x3
+
+#define LTC_DOWNSTREAM_ACCL_EN BIT(6)
+#define LTC_UPSTREAM_ACCL_EN   BIT(7)
+
+#define LTC_GPIO_ALL_INPUT 0xC0
+#define LTC_SWITCH_MASK0xF0
+
+enum ltc_type {
+   ltc_4305,
+   ltc_4306,
+};
+
+struct chip_desc {
+   u8 nchans;
+   u8 num_gpios

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 04.04.2017 11:28, Peter Rosin wrote:

*snip* *snip*


+static int ltc4306_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+   struct ltc4306 *data = gpiochip_get_data(chip);
+   int ret = 0;
+
+   if (gpiochip_line_is_open_drain(chip, offset) ||
+   (data->regs[LTC_REG_MODE] & BIT(7 - offset))) {


I wonder about this open-coded register cache. So, gpio people, is there
a guarantee from gpiolib that only one gpio_chip operation is in flight
concurrently? Because I don't see any evidence of that. With that in
mind, I think some locking is needed?


I thought there is a per chip mutex in the gpiolib. But I can't find
anything like this either. Since these two gpios can be used from
different internal or external users. The locking seem to be needed.

This gets us back to the regmap option. I did a quick grep, and 9 out of
205 drivers using regmap i2c, also use i2c_smbus... concurrently.

grep -Rl regmap_init_i2c ./drivers  | xargs grep -l i2c_smbus_ | grep "\.c"

Mostly to work around non uniform transfer layouts.


I see three options.

1. Go with regmap and convert to mux-locked. Then the unlocked i2c-xfer
becomes an ordinary i2c-xfer (or smbus, whatever). This will result in
the cleanest code.


ok - you convinced me.



2. Go with regmap and stay parent-locked. Then hook into the regmap
locking as is done in one of the drivers that have worked around similar
problems with regmap and parent-locked i2c-mux interactions:

drivers/media/dvb-frontends/rtl2830.c
drivers/media/dvb-frontends/m88ds3103.c

This will probably work, but you'd need to add a number of extra helper
functions.

3. Exclude register 3 from regmap and only use regmap for the other
registers. This will be a bit ugly and ad-hoc, will need clear comments
on what is going on and why it is safe etc. And I want to see it before
I accept it. And it might not be my call to begin with, because TBH, it
sounds a bit disgusting...


I'll check with Mark Brown on this topic.


Ok, might be a good idea...


+
+add_adapter_failed:
+   i2c_mux_del_adapters(muxc);
+gpio_default:
+   gpiod_direction_input(data->en_gpio);


This was actually not what I had in mind when I asked about it in v1, and
this looks a bit strange. You have no way of knowing if the pin was
configured as input when probe was called, and I don't see code like this
all over the place. Maybe it's is ok to not disable the chip over
suspend/resume, I was just asking because it looked a bit strange to grab
a pin and then forget about it. Now that I think about it some more, it's
probably ok to do just that since it is perhaps not possible to make the
chip draw less power by deasserting enable, but what do I know?


GPIOs are assumed by default inputs. So if you want to undo the actions
in probe. The logical consequence is to move them back to inputs, and
let the external PULL-UP or PULL-DOWN on the ENABLE decide what happens.
I would also prefer to leave it enabled, so that the GPIOs can retain


My point is that I do not see any probe functions undoing gpio configs.
Why bother in this case? Or are other probe functions really doing this?
I actually didn't check, but I haven't stumbled over it previously and
at least think I would have noticed...


it's last state. Well I think the device draws a bit less power when
disabled. But we don't support runtime PM anyways.


It might not be safe to reset the gpio pins over a suspend/resume depending
on what they are used for, so it is probably a bad idea to go there. Sorry
for bringing the whole issue up and muddying the waters...


I restore the original implementation and also pulse the ENABLE low so 
we're always doing a clean reset.


I'll send a new patch shortly.

Thanks!

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 04.04.2017 11:28, Peter Rosin wrote:

*snip* *snip*


+static int ltc4306_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+   struct ltc4306 *data = gpiochip_get_data(chip);
+   int ret = 0;
+
+   if (gpiochip_line_is_open_drain(chip, offset) ||
+   (data->regs[LTC_REG_MODE] & BIT(7 - offset))) {


I wonder about this open-coded register cache. So, gpio people, is there
a guarantee from gpiolib that only one gpio_chip operation is in flight
concurrently? Because I don't see any evidence of that. With that in
mind, I think some locking is needed?


I thought there is a per chip mutex in the gpiolib. But I can't find
anything like this either. Since these two gpios can be used from
different internal or external users. The locking seem to be needed.

This gets us back to the regmap option. I did a quick grep, and 9 out of
205 drivers using regmap i2c, also use i2c_smbus... concurrently.

grep -Rl regmap_init_i2c ./drivers  | xargs grep -l i2c_smbus_ | grep "\.c"

Mostly to work around non uniform transfer layouts.


I see three options.

1. Go with regmap and convert to mux-locked. Then the unlocked i2c-xfer
becomes an ordinary i2c-xfer (or smbus, whatever). This will result in
the cleanest code.


ok - you convinced me.



2. Go with regmap and stay parent-locked. Then hook into the regmap
locking as is done in one of the drivers that have worked around similar
problems with regmap and parent-locked i2c-mux interactions:

drivers/media/dvb-frontends/rtl2830.c
drivers/media/dvb-frontends/m88ds3103.c

This will probably work, but you'd need to add a number of extra helper
functions.

3. Exclude register 3 from regmap and only use regmap for the other
registers. This will be a bit ugly and ad-hoc, will need clear comments
on what is going on and why it is safe etc. And I want to see it before
I accept it. And it might not be my call to begin with, because TBH, it
sounds a bit disgusting...


I'll check with Mark Brown on this topic.


Ok, might be a good idea...


+
+add_adapter_failed:
+   i2c_mux_del_adapters(muxc);
+gpio_default:
+   gpiod_direction_input(data->en_gpio);


This was actually not what I had in mind when I asked about it in v1, and
this looks a bit strange. You have no way of knowing if the pin was
configured as input when probe was called, and I don't see code like this
all over the place. Maybe it's is ok to not disable the chip over
suspend/resume, I was just asking because it looked a bit strange to grab
a pin and then forget about it. Now that I think about it some more, it's
probably ok to do just that since it is perhaps not possible to make the
chip draw less power by deasserting enable, but what do I know?


GPIOs are assumed by default inputs. So if you want to undo the actions
in probe. The logical consequence is to move them back to inputs, and
let the external PULL-UP or PULL-DOWN on the ENABLE decide what happens.
I would also prefer to leave it enabled, so that the GPIOs can retain


My point is that I do not see any probe functions undoing gpio configs.
Why bother in this case? Or are other probe functions really doing this?
I actually didn't check, but I haven't stumbled over it previously and
at least think I would have noticed...


it's last state. Well I think the device draws a bit less power when
disabled. But we don't support runtime PM anyways.


It might not be safe to reset the gpio pins over a suspend/resume depending
on what they are used for, so it is probably a bad idea to go there. Sorry
for bringing the whole issue up and muddying the waters...


I restore the original implementation and also pulse the ENABLE low so 
we're always doing a clean reset.


I'll send a new patch shortly.

Thanks!

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: question about concurrent regmap i2c usage

[Michael Hennerich]

On 04.04.2017 14:26, Mark Brown wrote:

On Tue, Apr 04, 2017 at 10:28:23AM +0200, Michael Hennerich wrote:


A question came up, regarding whether it's recommended to mix regmap_i2c and
plain i2c_smbus or direct adapter transfers.



In this specific case for the i2c MUX portion we need to avoid double locks,
and therefore use un-locked direct adapter transfers.



The same time we also implement a small gpiochip, where we would like to use
regmap to avoid some boilerplate code for the register cache.


That should work fine providing nothing tries to change cached registers
underneath the regmap.  regmap won't be able to tell anything's changed
otherwise.


Hi Mark,

Thanks for confirmation!

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: question about concurrent regmap i2c usage

[Michael Hennerich]

On 04.04.2017 14:26, Mark Brown wrote:

On Tue, Apr 04, 2017 at 10:28:23AM +0200, Michael Hennerich wrote:


A question came up, regarding whether it's recommended to mix regmap_i2c and
plain i2c_smbus or direct adapter transfers.



In this specific case for the i2c MUX portion we need to avoid double locks,
and therefore use un-locked direct adapter transfers.



The same time we also implement a small gpiochip, where we would like to use
regmap to avoid some boilerplate code for the register cache.


That should work fine providing nothing tries to change cached registers
underneath the regmap.  regmap won't be able to tell anything's changed
otherwise.


Hi Mark,

Thanks for confirmation!

--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

question about concurrent regmap i2c usage

[Michael Hennerich]

Hi Mark,

During the discussion here: https://lkml.org/lkml/2017/4/4/76

A question came up, regarding whether it's recommended to mix regmap_i2c 
and plain i2c_smbus or direct adapter transfers.


In this specific case for the i2c MUX portion we need to avoid double 
locks, and therefore use un-locked direct adapter transfers.


The same time we also implement a small gpiochip, where we would like to 
use regmap to avoid some boilerplate code for the register cache.



What is your thought on this?


--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

question about concurrent regmap i2c usage

[Michael Hennerich]

Hi Mark,

During the discussion here: https://lkml.org/lkml/2017/4/4/76

A question came up, regarding whether it's recommended to mix regmap_i2c 
and plain i2c_smbus or direct adapter transfers.


In this specific case for the i2c MUX portion we need to avoid double 
locks, and therefore use un-locked direct adapter transfers.


The same time we also implement a small gpiochip, where we would like to 
use regmap to avoid some boilerplate code for the register cache.



What is your thought on this?


--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 31.03.2017 17:29, Peter Rosin wrote:

Hi!

Sorry for my incremental reviewing...

There's a question for the gpio people below that I would like
to have some confirmation on. Thanks!


Hi Peter,

Please find comments in-line.




On 2017-03-29 12:15, michael.henner...@analog.com wrote:

From: Michael Hennerich <michael.henner...@analog.com>

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.

Signed-off-by: Michael Hennerich <michael.henner...@analog.com>

---

Changes since v1:

 - Sort makefile entries
 - Sort driver includes
 - Use proper defines
 - Miscellaneous coding style fixups
 - Rename mux select callback
 - Revise i2c-mux-idle-disconnect handling
 - Add ENABLE GPIO handling on error and device removal.
 - Remove surplus of_match_device call.
---
 .../devicetree/bindings/i2c/i2c-mux-ltc4306.txt|  61 
 MAINTAINERS|   8 +
 drivers/i2c/muxes/Kconfig  |  10 +
 drivers/i2c/muxes/Makefile |   1 +
 drivers/i2c/muxes/i2c-mux-ltc4306.c| 367 +
 5 files changed, 447 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-ltc4306.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt 
b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
new file mode 100644
index 000..1e98c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
@@ -0,0 +1,61 @@
+* Linear Technology / Analog Devices I2C bus switch
+
+Required Properties:
+
+  - compatible: Must contain one of the following.
+"lltc,ltc4305", "lltc,ltc4306"
+  - reg: The I2C address of the device.
+
+  The following required properties are defined externally:
+
+  - Standard I2C mux properties. See i2c-mux.txt in this directory.
+  - I2C child bus nodes. See i2c-mux.txt in this directory.
+
+Optional Properties:
+
+  - enable-gpios: Reference to the GPIO connected to the enable input.
+  - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
+children in idle state. This is necessary for example, if there are several
+multiplexers on the bus and the devices behind them use same I2C addresses.
+  - gpio-controller: Marks the device node as a GPIO Controller.
+  - #gpio-cells: Should be two.  The first cell is the pin number and
+   the second cell is used to specify flags.
+   See ../gpio/gpio.txt for more information.
+  - ltc,downstream-accelerators-enable: Enables the rise time accelerators
+   on the downstream port.
+  - ltc,upstream-accelerators-enable: Enables the rise time accelerators
+   on the upstream port.
+
+Example:
+
+   ltc4306: i2c-mux@4a {
+   compatible = "lltc,ltc4306";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x4a>;
+
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   i2c@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+
+   i2c@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..9a27a19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7698,6 +7698,14 @@ S:   Maintained
 F: Documentation/hwmon/ltc4261
 F: drivers/hwmon/ltc4261.c

+LTC4306 I2C MULTIPLEXER DRIVER
+M: Michael Hennerich <michael.henner...@analog.com>
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-mux-ltc4306.c
+F: Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
+
 LTP (Linux Test Project)
 M: Mike Frysinger <vap...@gentoo.org>
 M: Cyril Hrubis <chru...@suse.cz>
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 10b3d17..f501b3b 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -30,6 +30,16 @@ config I2C_MUX_GPIO
  This driver can also be built as a module.  I

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 31.03.2017 17:29, Peter Rosin wrote:

Hi!

Sorry for my incremental reviewing...

There's a question for the gpio people below that I would like
to have some confirmation on. Thanks!


Hi Peter,

Please find comments in-line.




On 2017-03-29 12:15, michael.henner...@analog.com wrote:

From: Michael Hennerich 

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.

Signed-off-by: Michael Hennerich 

---

Changes since v1:

 - Sort makefile entries
 - Sort driver includes
 - Use proper defines
 - Miscellaneous coding style fixups
 - Rename mux select callback
 - Revise i2c-mux-idle-disconnect handling
 - Add ENABLE GPIO handling on error and device removal.
 - Remove surplus of_match_device call.
---
 .../devicetree/bindings/i2c/i2c-mux-ltc4306.txt|  61 
 MAINTAINERS|   8 +
 drivers/i2c/muxes/Kconfig  |  10 +
 drivers/i2c/muxes/Makefile |   1 +
 drivers/i2c/muxes/i2c-mux-ltc4306.c| 367 +
 5 files changed, 447 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-ltc4306.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt 
b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
new file mode 100644
index 000..1e98c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
@@ -0,0 +1,61 @@
+* Linear Technology / Analog Devices I2C bus switch
+
+Required Properties:
+
+  - compatible: Must contain one of the following.
+"lltc,ltc4305", "lltc,ltc4306"
+  - reg: The I2C address of the device.
+
+  The following required properties are defined externally:
+
+  - Standard I2C mux properties. See i2c-mux.txt in this directory.
+  - I2C child bus nodes. See i2c-mux.txt in this directory.
+
+Optional Properties:
+
+  - enable-gpios: Reference to the GPIO connected to the enable input.
+  - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
+children in idle state. This is necessary for example, if there are several
+multiplexers on the bus and the devices behind them use same I2C addresses.
+  - gpio-controller: Marks the device node as a GPIO Controller.
+  - #gpio-cells: Should be two.  The first cell is the pin number and
+   the second cell is used to specify flags.
+   See ../gpio/gpio.txt for more information.
+  - ltc,downstream-accelerators-enable: Enables the rise time accelerators
+   on the downstream port.
+  - ltc,upstream-accelerators-enable: Enables the rise time accelerators
+   on the upstream port.
+
+Example:
+
+   ltc4306: i2c-mux@4a {
+   compatible = "lltc,ltc4306";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x4a>;
+
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   i2c@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+
+   i2c@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..9a27a19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7698,6 +7698,14 @@ S:   Maintained
 F: Documentation/hwmon/ltc4261
 F: drivers/hwmon/ltc4261.c

+LTC4306 I2C MULTIPLEXER DRIVER
+M: Michael Hennerich 
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-mux-ltc4306.c
+F: Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
+
 LTP (Linux Test Project)
 M: Mike Frysinger 
 M: Cyril Hrubis 
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 10b3d17..f501b3b 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -30,6 +30,16 @@ config I2C_MUX_GPIO
  This driver can also be built as a module.  If so, the module
  will be called i2c-mux-gpio.

+config I2C_MUX_LTC4306
+   tristate "LTC LTC4306/5 I2C multiplexer"
+   selec

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 03.04.2017 16:20, Peter Rosin wrote:

On 2017-04-03 15:36, Michael Hennerich wrote:

On 03.04.2017 14:03, Peter Rosin wrote:

On 2017-03-31 17:29, Peter Rosin wrote:

Hi!

Sorry for my incremental reviewing...



Another incremental...


On 2017-03-29 12:15, michael.henner...@analog.com wrote:

+
+   /* Now create an adapter for each channel */
+   for (num = 0; num < data->chip->nchans; num++) {
+   ret = i2c_mux_add_adapter(muxc, 0, num, 0);
+   if (ret) {
+   dev_err(>dev,
+   "failed to register multiplexed adapter %d\n",
+   num);


Just a heads up, I submitted a series to remove a bunch of dev_err calls
when i2c_mux_add_adapter fails. See https://lkml.org/lkml/2017/4/3/115

You can remove this one as well.

And please use a subject of the form:
i2c: mux: ltc4306: 

ok - no problem.


You managed to drop the spaces after the new colons in the subject.

And maybe there is a problem, because I don't see any reaction to any of
the review comments I made in https://lkml.org/lkml/2017/3/31/525

Was that on purpose? Sure, the gpio "jury" is still out on the bigger
question so maybe you're waiting for that, but there were a few nitpicks
as well. Anyway, sorry again for failing to compile all comments up front.


Hi Peter,

sorry - this was not on purpose. I simply missed your second to last 
incremental review. I fix the subject now finally.


Thanks for your patience.




I sent out a new patch. Per Rob's request, I split out the dt-bindings
into a separate patch.


Thanks. I think(?) it is customary to have the bindings first, and then
implement that "specification" in followup patches. No big deal though...

Cheers,
peda





--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 03.04.2017 16:20, Peter Rosin wrote:

On 2017-04-03 15:36, Michael Hennerich wrote:

On 03.04.2017 14:03, Peter Rosin wrote:

On 2017-03-31 17:29, Peter Rosin wrote:

Hi!

Sorry for my incremental reviewing...



Another incremental...


On 2017-03-29 12:15, michael.henner...@analog.com wrote:

+
+   /* Now create an adapter for each channel */
+   for (num = 0; num < data->chip->nchans; num++) {
+   ret = i2c_mux_add_adapter(muxc, 0, num, 0);
+   if (ret) {
+   dev_err(>dev,
+   "failed to register multiplexed adapter %d\n",
+   num);


Just a heads up, I submitted a series to remove a bunch of dev_err calls
when i2c_mux_add_adapter fails. See https://lkml.org/lkml/2017/4/3/115

You can remove this one as well.

And please use a subject of the form:
i2c: mux: ltc4306: 

ok - no problem.


You managed to drop the spaces after the new colons in the subject.

And maybe there is a problem, because I don't see any reaction to any of
the review comments I made in https://lkml.org/lkml/2017/3/31/525

Was that on purpose? Sure, the gpio "jury" is still out on the bigger
question so maybe you're waiting for that, but there were a few nitpicks
as well. Anyway, sorry again for failing to compile all comments up front.


Hi Peter,

sorry - this was not on purpose. I simply missed your second to last 
incremental review. I fix the subject now finally.


Thanks for your patience.




I sent out a new patch. Per Rob's request, I split out the dt-bindings
into a separate patch.


Thanks. I think(?) it is customary to have the bindings first, and then
implement that "specification" in followup patches. No big deal though...

Cheers,
peda





--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 03.04.2017 14:03, Peter Rosin wrote:

On 2017-03-31 17:29, Peter Rosin wrote:

Hi!

Sorry for my incremental reviewing...



Another incremental...


On 2017-03-29 12:15, michael.henner...@analog.com wrote:

+
+   /* Now create an adapter for each channel */
+   for (num = 0; num < data->chip->nchans; num++) {
+   ret = i2c_mux_add_adapter(muxc, 0, num, 0);
+   if (ret) {
+   dev_err(>dev,
+   "failed to register multiplexed adapter %d\n",
+   num);


Just a heads up, I submitted a series to remove a bunch of dev_err calls
when i2c_mux_add_adapter fails. See https://lkml.org/lkml/2017/4/3/115

You can remove this one as well.

And please use a subject of the form:
i2c: mux: ltc4306: 

Cheers,
peda


ok - no problem.
I sent out a new patch. Per Rob's request, I split out the dt-bindings 
into a separate patch.


--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 03.04.2017 14:03, Peter Rosin wrote:

On 2017-03-31 17:29, Peter Rosin wrote:

Hi!

Sorry for my incremental reviewing...



Another incremental...


On 2017-03-29 12:15, michael.henner...@analog.com wrote:

+
+   /* Now create an adapter for each channel */
+   for (num = 0; num < data->chip->nchans; num++) {
+   ret = i2c_mux_add_adapter(muxc, 0, num, 0);
+   if (ret) {
+   dev_err(>dev,
+   "failed to register multiplexed adapter %d\n",
+   num);


Just a heads up, I submitted a series to remove a bunch of dev_err calls
when i2c_mux_add_adapter fails. See https://lkml.org/lkml/2017/4/3/115

You can remove this one as well.

And please use a subject of the form:
i2c: mux: ltc4306: 

Cheers,
peda


ok - no problem.
I sent out a new patch. Per Rob's request, I split out the dt-bindings 
into a separate patch.


--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC

[Michael Hennerich]

On 23.03.2017 12:05, Lars-Peter Clausen wrote:

Sorry - I missed some of this review feedback ...


+
+static int ltc2497_wait_conv(struct ltc2497_st *st)
+{
+   s64 time_elapsed;
+
+   time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
+
+   if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
+   /* delay if conversion time not passed
+* since last read or write
+*/
+   msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);


Considering how long this sleeps msleep_interruptible() might be the better
choice.


Wondering what should be the outcome of this?
We can't simply replace it. Actually I've seen cases here in drivers/iio 
where the delay is essential, but the return value of 
msleep_interruptible() is not being checked.

Thus causing a malicious access, in case a signal is received.

We must delay here. If we switch to msleep_interruptible() the only 
reason for this would be to cancel the read and return -EINTR to the user.


Also there is another msleep below which would also need this kind of 
handling.





+   return 0;
+   }
+
+   if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
+   /* We're in automatic mode -
+* so the last reading is stil not outdated
+*/
+   return 0;
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
+{
+   struct i2c_client *client = st->client;
+   __be32 buf = 0;


transfer buffers must not be on the stack to avoid issues if the controller
should use DMA.


+   int ret;
+
+   ret = ltc2497_wait_conv(st);
+   if (ret < 0 || st->addr_prev != address) {
+   ret = i2c_smbus_write_byte(st->client, 0xA0 | address);
+   if (ret < 0)
+   return ret;
+   st->addr_prev = address;
+   msleep(LTC2497_CONVERSION_TIME_MS);
+   }
+   ret = i2c_master_recv(client, (char *), 3);
+   if (ret < 0)  {
+   dev_err(>dev, "i2c_master_recv failed\n");
+   return ret;
+   }
+   st->time_prev = ktime_get();
+   *val = (be32_to_cpu(buf) >> 14) - (1 << 17);
+
+   return ret;
+}

[...]




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC

[Michael Hennerich]

On 23.03.2017 12:05, Lars-Peter Clausen wrote:

Sorry - I missed some of this review feedback ...


+
+static int ltc2497_wait_conv(struct ltc2497_st *st)
+{
+   s64 time_elapsed;
+
+   time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
+
+   if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
+   /* delay if conversion time not passed
+* since last read or write
+*/
+   msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);


Considering how long this sleeps msleep_interruptible() might be the better
choice.


Wondering what should be the outcome of this?
We can't simply replace it. Actually I've seen cases here in drivers/iio 
where the delay is essential, but the return value of 
msleep_interruptible() is not being checked.

Thus causing a malicious access, in case a signal is received.

We must delay here. If we switch to msleep_interruptible() the only 
reason for this would be to cancel the read and return -EINTR to the user.


Also there is another msleep below which would also need this kind of 
handling.





+   return 0;
+   }
+
+   if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
+   /* We're in automatic mode -
+* so the last reading is stil not outdated
+*/
+   return 0;
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
+{
+   struct i2c_client *client = st->client;
+   __be32 buf = 0;


transfer buffers must not be on the stack to avoid issues if the controller
should use DMA.


+   int ret;
+
+   ret = ltc2497_wait_conv(st);
+   if (ret < 0 || st->addr_prev != address) {
+   ret = i2c_smbus_write_byte(st->client, 0xA0 | address);
+   if (ret < 0)
+   return ret;
+   st->addr_prev = address;
+   msleep(LTC2497_CONVERSION_TIME_MS);
+   }
+   ret = i2c_master_recv(client, (char *), 3);
+   if (ret < 0)  {
+   dev_err(>dev, "i2c_master_recv failed\n");
+   return ret;
+   }
+   st->time_prev = ktime_get();
+   *val = (be32_to_cpu(buf) >> 14) - (1 << 17);
+
+   return ret;
+}

[...]




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 24.03.2017 09:01, Peter Rosin wrote:

On 2017-03-23 15:22, michael.henner...@analog.com wrote:

From: Michael Hennerich <michael.henner...@analog.com>

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.


Hi Peter,

Thanks for your review.



Hmmm, I'm not sure if it is ok to implement a gpio provider outside of
drivers/gpio like this? Should it be done with MFD, or is that just
adding needless complexity?


Its not uncommon to have these small gpiochips inside the driver which 
implements the primary function. Linus is on the To list to add his review.




I also wonder if you have thought about implementing support for
alerts? And do you need recovery if things get stuck? I see timeout
bits and failure to connect bits in the register map...


The irq_chip handling the ALERTs will be added in future. However I 
don't think the chip internal timeout handling will be exposed. I would 
assume the parent adapter handles timeouts on the connected downstream 
ports.






Signed-off-by: Michael Hennerich <michael.henner...@analog.com>
---
 .../devicetree/bindings/i2c/i2c-mux-ltc4306.txt|  61 
 MAINTAINERS|   8 +
 drivers/i2c/muxes/Kconfig  |  10 +
 drivers/i2c/muxes/Makefile |   1 +
 drivers/i2c/muxes/i2c-mux-ltc4306.c| 365 +
 5 files changed, 445 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-ltc4306.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt 
b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
new file mode 100644
index 000..1e98c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
@@ -0,0 +1,61 @@
+* Linear Technology / Analog Devices I2C bus switch
+
+Required Properties:
+
+  - compatible: Must contain one of the following.
+"lltc,ltc4305", "lltc,ltc4306"
+  - reg: The I2C address of the device.
+
+  The following required properties are defined externally:
+
+  - Standard I2C mux properties. See i2c-mux.txt in this directory.
+  - I2C child bus nodes. See i2c-mux.txt in this directory.
+
+Optional Properties:
+
+  - enable-gpios: Reference to the GPIO connected to the enable input.
+  - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
+children in idle state. This is necessary for example, if there are several
+multiplexers on the bus and the devices behind them use same I2C addresses.
+  - gpio-controller: Marks the device node as a GPIO Controller.
+  - #gpio-cells: Should be two.  The first cell is the pin number and
+   the second cell is used to specify flags.
+   See ../gpio/gpio.txt for more information.
+  - ltc,downstream-accelerators-enable: Enables the rise time accelerators
+   on the downstream port.
+  - ltc,upstream-accelerators-enable: Enables the rise time accelerators
+   on the upstream port.
+
+Example:
+
+   ltc4306: i2c-mux@4a {
+   compatible = "lltc,ltc4306";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x4a>;
+
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   i2c@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+
+   i2c@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..9a27a19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7698,6 +7698,14 @@ S:   Maintained
 F: Documentation/hwmon/ltc4261
 F: drivers/hwmon/ltc4261.c

+LTC4306 I2C MULTIPLEXER DRIVER
+M: Michael Hennerich <michael.henner...@analog.com>
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-mux-ltc4306.c
+F: Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
+
 LTP (Linux Test Project)
 M: Mike Frysinger <vap...@gentoo.org>
 M: Cyril Hrubis 

Re: [PATCH] i2c/muxes/i2c-mux-ltc4306: LTC4306 and LTC4305 I2C multiplexer/switch

[Michael Hennerich]

On 24.03.2017 09:01, Peter Rosin wrote:

On 2017-03-23 15:22, michael.henner...@analog.com wrote:

From: Michael Hennerich 

This patch adds support for the Analog Devices / Linear Technology
LTC4306 and LTC4305 4/2 Channel I2C Bus Multiplexer/Switches.
The LTC4306 optionally provides two general purpose input/output pins
(GPIOs) that can be configured as logic inputs, opendrain outputs or
push-pull outputs via the generic GPIOLIB framework.


Hi Peter,

Thanks for your review.



Hmmm, I'm not sure if it is ok to implement a gpio provider outside of
drivers/gpio like this? Should it be done with MFD, or is that just
adding needless complexity?


Its not uncommon to have these small gpiochips inside the driver which 
implements the primary function. Linus is on the To list to add his review.




I also wonder if you have thought about implementing support for
alerts? And do you need recovery if things get stuck? I see timeout
bits and failure to connect bits in the register map...


The irq_chip handling the ALERTs will be added in future. However I 
don't think the chip internal timeout handling will be exposed. I would 
assume the parent adapter handles timeouts on the connected downstream 
ports.






Signed-off-by: Michael Hennerich 
---
 .../devicetree/bindings/i2c/i2c-mux-ltc4306.txt|  61 
 MAINTAINERS|   8 +
 drivers/i2c/muxes/Kconfig  |  10 +
 drivers/i2c/muxes/Makefile |   1 +
 drivers/i2c/muxes/i2c-mux-ltc4306.c| 365 +
 5 files changed, 445 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-ltc4306.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt 
b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
new file mode 100644
index 000..1e98c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
@@ -0,0 +1,61 @@
+* Linear Technology / Analog Devices I2C bus switch
+
+Required Properties:
+
+  - compatible: Must contain one of the following.
+"lltc,ltc4305", "lltc,ltc4306"
+  - reg: The I2C address of the device.
+
+  The following required properties are defined externally:
+
+  - Standard I2C mux properties. See i2c-mux.txt in this directory.
+  - I2C child bus nodes. See i2c-mux.txt in this directory.
+
+Optional Properties:
+
+  - enable-gpios: Reference to the GPIO connected to the enable input.
+  - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
+children in idle state. This is necessary for example, if there are several
+multiplexers on the bus and the devices behind them use same I2C addresses.
+  - gpio-controller: Marks the device node as a GPIO Controller.
+  - #gpio-cells: Should be two.  The first cell is the pin number and
+   the second cell is used to specify flags.
+   See ../gpio/gpio.txt for more information.
+  - ltc,downstream-accelerators-enable: Enables the rise time accelerators
+   on the downstream port.
+  - ltc,upstream-accelerators-enable: Enables the rise time accelerators
+   on the upstream port.
+
+Example:
+
+   ltc4306: i2c-mux@4a {
+   compatible = "lltc,ltc4306";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x4a>;
+
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   i2c@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+
+   i2c@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   eeprom@50 {
+   compatible = "at,24c02";
+   reg = <0x50>;
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..9a27a19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7698,6 +7698,14 @@ S:   Maintained
 F: Documentation/hwmon/ltc4261
 F: drivers/hwmon/ltc4261.c

+LTC4306 I2C MULTIPLEXER DRIVER
+M: Michael Hennerich 
+W: http://ez.analog.com/community/linux-device-drivers
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-mux-ltc4306.c
+F: Documentation/devicetree/bindings/i2c/i2c-mux-ltc4306.txt
+
 LTP (Linux Test Project)
 M: Mike Frysinger 
 M: Cyril Hrubis 
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 10b3d17..f501b3b 100644
--- a/drivers/i2c/muxes/Kconfig

Re: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC

[Michael Hennerich]

On 23.03.2017 12:28, Peter Meerwald-Stadler wrote:

On Thu, 23 Mar 2017, michael.henner...@analog.com wrote:


From: Michael Hennerich <michael.henner...@analog.com>


comments below


Hi Peter,

Thanks for the review -
comments inline.




Signed-off-by: Michael Hennerich <michael.henner...@analog.com>
---
 .../devicetree/bindings/iio/adc/ltc2497.txt|  13 +
 MAINTAINERS|   1 +
 drivers/iio/adc/Kconfig|  11 +
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/ltc2497.c  | 263 +
 5 files changed, 289 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt
 create mode 100644 drivers/iio/adc/ltc2497.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt 
b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
new file mode 100644
index 000..c2829c19
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
@@ -0,0 +1,13 @@
+* Linear Technology / Analog Devices LTC2497 ADC
+
+Required properties:
+ - compatible: Should be "lltc,ltc2497"
+ - reg: Should contain the ADC I2C address
+ - vref-supply: The regulator supply for ADC reference voltage
+
+Example:
+   ltc2497: adc@76 {
+   compatible = "lltc,ltc2497";
+   reg = <0x76>;
+   vref-supply = <_reg>;
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index a7d6f9a..173043c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -813,6 +813,7 @@ W:  http://wiki.analog.com/
 W: http://ez.analog.com/community/linux-device-drivers
 S: Supported
 F: drivers/iio/*/ad*
+F: drivers/iio/adc/ltc2497*
 X: drivers/iio/*/adjd*
 F: drivers/staging/iio/*/ad*
 F: drivers/staging/iio/trigger/iio-trig-bfin-timer.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 2268a6f..141cf4a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -326,6 +326,17 @@ config LTC2485
  To compile this driver as a module, choose M here: the module will be
  called ltc2485.

+config LTC2497
+   tristate "Linear Technology LTC2497 ADC driver"
+   depends on I2C
+   help
+ Say yes here to build support for Linear Technology LTC2497
+ 16-Bit 8-/16-Channel Delta Sigma ADC.
+ Provides direct access via sysfs


the line "Provides..." should be removed I think, it confuses me at least


Sure I can remove - originally added to make checkpatch happy.




+
+ To compile this driver as a module, choose M here: the module will be
+ called ltc2497.
+
 config MAX1027
tristate "Maxim max1027 ADC driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 73dbe39..9d626b5 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
+obj-$(CONFIG_LTC2497) += ltc2497.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1363) += max1363.o
diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
new file mode 100644
index 000..0452715
--- /dev/null
+++ b/drivers/iio/adc/ltc2497.c
@@ -0,0 +1,263 @@
+/*
+ * ltc2497.c - Driver for Linear Technology LTC2497 ADC
+ *
+ * Copyright (C) 2017 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define LTC2497_ENABLE 0xA0
+#define LTC2497_SGL(1 << 4)
+#define LTC2497_DIFF   (0 << 4)
+#define LTC2497_SIGN   (1 << 3)
+#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE
+#define LTC2497_CONVERSION_TIME_MS 150ULL


is the ULL really needed?


Needed? - maybe not - but probably not wrong ktime_ms_delta() returns 
64bit and we later do some comparisons...





+struct ltc2497_st {
+   struct i2c_client *client;
+   struct regulator *ref;
+   ktime_t time_prev;
+   u8 addr_prev;
+};
+
+static int ltc2497_wait_conv(struct ltc2497_st *st)
+{
+   s64 time_elapsed;
+
+   time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
+
+   if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
+   /* delay if conversion time not passed
+* since last read or write
+*/
+   msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);
+   return 0;
+   }
+
+   if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
+   /* We're in automatic mode -
+* so the last reading is stil not outdated

Re: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC

[Michael Hennerich]

On 23.03.2017 12:28, Peter Meerwald-Stadler wrote:

On Thu, 23 Mar 2017, michael.henner...@analog.com wrote:


From: Michael Hennerich 


comments below


Hi Peter,

Thanks for the review -
comments inline.




Signed-off-by: Michael Hennerich 
---
 .../devicetree/bindings/iio/adc/ltc2497.txt|  13 +
 MAINTAINERS|   1 +
 drivers/iio/adc/Kconfig|  11 +
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/ltc2497.c  | 263 +
 5 files changed, 289 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt
 create mode 100644 drivers/iio/adc/ltc2497.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt 
b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
new file mode 100644
index 000..c2829c19
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
@@ -0,0 +1,13 @@
+* Linear Technology / Analog Devices LTC2497 ADC
+
+Required properties:
+ - compatible: Should be "lltc,ltc2497"
+ - reg: Should contain the ADC I2C address
+ - vref-supply: The regulator supply for ADC reference voltage
+
+Example:
+   ltc2497: adc@76 {
+   compatible = "lltc,ltc2497";
+   reg = <0x76>;
+   vref-supply = <_reg>;
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index a7d6f9a..173043c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -813,6 +813,7 @@ W:  http://wiki.analog.com/
 W: http://ez.analog.com/community/linux-device-drivers
 S: Supported
 F: drivers/iio/*/ad*
+F: drivers/iio/adc/ltc2497*
 X: drivers/iio/*/adjd*
 F: drivers/staging/iio/*/ad*
 F: drivers/staging/iio/trigger/iio-trig-bfin-timer.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 2268a6f..141cf4a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -326,6 +326,17 @@ config LTC2485
  To compile this driver as a module, choose M here: the module will be
  called ltc2485.

+config LTC2497
+   tristate "Linear Technology LTC2497 ADC driver"
+   depends on I2C
+   help
+ Say yes here to build support for Linear Technology LTC2497
+ 16-Bit 8-/16-Channel Delta Sigma ADC.
+ Provides direct access via sysfs


the line "Provides..." should be removed I think, it confuses me at least


Sure I can remove - originally added to make checkpatch happy.




+
+ To compile this driver as a module, choose M here: the module will be
+ called ltc2497.
+
 config MAX1027
tristate "Maxim max1027 ADC driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 73dbe39..9d626b5 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
+obj-$(CONFIG_LTC2497) += ltc2497.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1363) += max1363.o
diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
new file mode 100644
index 000..0452715
--- /dev/null
+++ b/drivers/iio/adc/ltc2497.c
@@ -0,0 +1,263 @@
+/*
+ * ltc2497.c - Driver for Linear Technology LTC2497 ADC
+ *
+ * Copyright (C) 2017 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define LTC2497_ENABLE 0xA0
+#define LTC2497_SGL(1 << 4)
+#define LTC2497_DIFF   (0 << 4)
+#define LTC2497_SIGN   (1 << 3)
+#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE
+#define LTC2497_CONVERSION_TIME_MS 150ULL


is the ULL really needed?


Needed? - maybe not - but probably not wrong ktime_ms_delta() returns 
64bit and we later do some comparisons...





+struct ltc2497_st {
+   struct i2c_client *client;
+   struct regulator *ref;
+   ktime_t time_prev;
+   u8 addr_prev;
+};
+
+static int ltc2497_wait_conv(struct ltc2497_st *st)
+{
+   s64 time_elapsed;
+
+   time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
+
+   if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
+   /* delay if conversion time not passed
+* since last read or write
+*/
+   msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);
+   return 0;
+   }
+
+   if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
+   /* We're in automatic mode -
+* so the last reading is stil not outdated
+*/
+   return 0;
+   }
+
+   ret

Re: [PATCH 2/4] Input: ad7879 - return plain error code from ad7879_probe()

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

With the switch to devm, there is no need for ad7879_probe() to return the
touchscreen structure, we can use plain error code. This also fixes issue
introduced with devm concersion, where we returned 0 on success (which
worked OK since IS_ERR(0) would not trigger, but was not correct
regardless).

Fixes: 381f688eee3d ("Input: ad7879 - use more devm interfaces")
Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
 drivers/input/touchscreen/ad7879-i2c.c |  9 ++---
 drivers/input/touchscreen/ad7879-spi.c |  7 +--
 drivers/input/touchscreen/ad7879.c | 28 ++--
 drivers/input/touchscreen/ad7879.h |  5 ++---
 4 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879-i2c.c 
b/drivers/input/touchscreen/ad7879-i2c.c
index a282d1c9e2c6..49b902b10c5f 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -27,7 +27,6 @@ static const struct regmap_config ad7879_i2c_regmap_config = {
 static int ad7879_i2c_probe(struct i2c_client *client,
  const struct i2c_device_id *id)
 {
-   struct ad7879 *ts;
struct regmap *regmap;

if (!i2c_check_functionality(client->adapter,
@@ -40,12 +39,8 @@ static int ad7879_i2c_probe(struct i2c_client *client,
if (IS_ERR(regmap))
return PTR_ERR(regmap);

-   ts = ad7879_probe(>dev, regmap, client->irq,
- BUS_I2C, AD7879_DEVID);
-   if (IS_ERR(ts))
-   return PTR_ERR(ts);
-
-   return 0;
+   return ad7879_probe(>dev, regmap, client->irq,
+   BUS_I2C, AD7879_DEVID);
 }

 static const struct i2c_device_id ad7879_id[] = {
diff --git a/drivers/input/touchscreen/ad7879-spi.c 
b/drivers/input/touchscreen/ad7879-spi.c
index 59486ccba37d..3457a5626d75 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -32,7 +32,6 @@ static const struct regmap_config ad7879_spi_regmap_config = {

 static int ad7879_spi_probe(struct spi_device *spi)
 {
-   struct ad7879 *ts;
struct regmap *regmap;

/* don't exceed max specified SPI CLK frequency */
@@ -45,11 +44,7 @@ static int ad7879_spi_probe(struct spi_device *spi)
if (IS_ERR(regmap))
return PTR_ERR(regmap);

-   ts = ad7879_probe(>dev, regmap, spi->irq, BUS_SPI, AD7879_DEVID);
-   if (IS_ERR(ts))
-   return PTR_ERR(ts);
-
-   return 0;
+   return ad7879_probe(>dev, regmap, spi->irq, BUS_SPI, AD7879_DEVID);
 }

 #ifdef CONFIG_OF
diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 52daaa4edc67..7118f611e222 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -531,8 +531,8 @@ static void ad7879_cleanup_sysfs(void *_ts)
sysfs_remove_group(>dev->kobj, _attr_group);
 }

-struct ad7879 *ad7879_probe(struct device *dev, struct regmap *regmap,
-   int irq, u16 bustype, u8 devid)
+int ad7879_probe(struct device *dev, struct regmap *regmap,
+int irq, u16 bustype, u8 devid)
 {
struct ad7879_platform_data *pdata = dev_get_platdata(dev);
struct ad7879 *ts;
@@ -542,12 +542,12 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,

if (irq <= 0) {
dev_err(dev, "No IRQ specified\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}

ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
if (!ts)
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;

if (pdata) {
/* Platform data use swapped axis (backward compatibility) */
@@ -564,13 +564,13 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,
ad7879_parse_dt(dev, ts);
} else {
dev_err(dev, "No platform data\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}

input_dev = devm_input_allocate_device(dev);
if (!input_dev) {
dev_err(dev, "Failed to allocate input device\n");
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;
}

ts->dev = dev;
@@ -618,14 +618,14 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,
touchscreen_parse_properties(input_dev, false, NULL);
if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
dev_err(dev, "Touchscreen pressure is not specified\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}
}

err = ad7879_w

Re: [PATCH 2/4] Input: ad7879 - return plain error code from ad7879_probe()

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

With the switch to devm, there is no need for ad7879_probe() to return the
touchscreen structure, we can use plain error code. This also fixes issue
introduced with devm concersion, where we returned 0 on success (which
worked OK since IS_ERR(0) would not trigger, but was not correct
regardless).

Fixes: 381f688eee3d ("Input: ad7879 - use more devm interfaces")
Signed-off-by: Dmitry Torokhov 


Acked-by: Michael Hennerich 


---
 drivers/input/touchscreen/ad7879-i2c.c |  9 ++---
 drivers/input/touchscreen/ad7879-spi.c |  7 +--
 drivers/input/touchscreen/ad7879.c | 28 ++--
 drivers/input/touchscreen/ad7879.h |  5 ++---
 4 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879-i2c.c 
b/drivers/input/touchscreen/ad7879-i2c.c
index a282d1c9e2c6..49b902b10c5f 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -27,7 +27,6 @@ static const struct regmap_config ad7879_i2c_regmap_config = {
 static int ad7879_i2c_probe(struct i2c_client *client,
  const struct i2c_device_id *id)
 {
-   struct ad7879 *ts;
struct regmap *regmap;

if (!i2c_check_functionality(client->adapter,
@@ -40,12 +39,8 @@ static int ad7879_i2c_probe(struct i2c_client *client,
if (IS_ERR(regmap))
return PTR_ERR(regmap);

-   ts = ad7879_probe(>dev, regmap, client->irq,
- BUS_I2C, AD7879_DEVID);
-   if (IS_ERR(ts))
-   return PTR_ERR(ts);
-
-   return 0;
+   return ad7879_probe(>dev, regmap, client->irq,
+   BUS_I2C, AD7879_DEVID);
 }

 static const struct i2c_device_id ad7879_id[] = {
diff --git a/drivers/input/touchscreen/ad7879-spi.c 
b/drivers/input/touchscreen/ad7879-spi.c
index 59486ccba37d..3457a5626d75 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -32,7 +32,6 @@ static const struct regmap_config ad7879_spi_regmap_config = {

 static int ad7879_spi_probe(struct spi_device *spi)
 {
-   struct ad7879 *ts;
struct regmap *regmap;

/* don't exceed max specified SPI CLK frequency */
@@ -45,11 +44,7 @@ static int ad7879_spi_probe(struct spi_device *spi)
if (IS_ERR(regmap))
return PTR_ERR(regmap);

-   ts = ad7879_probe(>dev, regmap, spi->irq, BUS_SPI, AD7879_DEVID);
-   if (IS_ERR(ts))
-   return PTR_ERR(ts);
-
-   return 0;
+   return ad7879_probe(>dev, regmap, spi->irq, BUS_SPI, AD7879_DEVID);
 }

 #ifdef CONFIG_OF
diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 52daaa4edc67..7118f611e222 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -531,8 +531,8 @@ static void ad7879_cleanup_sysfs(void *_ts)
sysfs_remove_group(>dev->kobj, _attr_group);
 }

-struct ad7879 *ad7879_probe(struct device *dev, struct regmap *regmap,
-   int irq, u16 bustype, u8 devid)
+int ad7879_probe(struct device *dev, struct regmap *regmap,
+int irq, u16 bustype, u8 devid)
 {
struct ad7879_platform_data *pdata = dev_get_platdata(dev);
struct ad7879 *ts;
@@ -542,12 +542,12 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,

if (irq <= 0) {
dev_err(dev, "No IRQ specified\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}

ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
if (!ts)
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;

if (pdata) {
/* Platform data use swapped axis (backward compatibility) */
@@ -564,13 +564,13 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,
ad7879_parse_dt(dev, ts);
} else {
dev_err(dev, "No platform data\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}

input_dev = devm_input_allocate_device(dev);
if (!input_dev) {
dev_err(dev, "Failed to allocate input device\n");
-   return ERR_PTR(-ENOMEM);
+   return -ENOMEM;
}

ts->dev = dev;
@@ -618,14 +618,14 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,
touchscreen_parse_properties(input_dev, false, NULL);
if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
dev_err(dev, "Touchscreen pressure is not specified\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}
}

err = ad7879_write(ts, AD7879_REG_CTRL2, AD7879_RESET);
if (err < 0) {
 

Re: [PATCH 3/4] Input: ad7879 - try parsing properties on non-DT systems

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

We have switched the driver to use generic device properties API, so there
is no need to check for presence of DT node before trying parse properties.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
 drivers/input/touchscreen/ad7879.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 7118f611e222..c415614ada68 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -560,11 +560,10 @@ int ad7879_probe(struct device *dev, struct regmap 
*regmap,
ts->averaging = pdata->averaging;
ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
ts->median = pdata->median;
-   } else if (dev->of_node) {
-   ad7879_parse_dt(dev, ts);
} else {
-   dev_err(dev, "No platform data\n");
-   return -EINVAL;
+   err = ad7879_parse_dt(dev, ts);
+   if (err)
+   return err;
}

input_dev = devm_input_allocate_device(dev);




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 3/4] Input: ad7879 - try parsing properties on non-DT systems

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

We have switched the driver to use generic device properties API, so there
is no need to check for presence of DT node before trying parse properties.

Signed-off-by: Dmitry Torokhov 


Acked-by: Michael Hennerich 


---
 drivers/input/touchscreen/ad7879.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 7118f611e222..c415614ada68 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -560,11 +560,10 @@ int ad7879_probe(struct device *dev, struct regmap 
*regmap,
ts->averaging = pdata->averaging;
ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
ts->median = pdata->median;
-   } else if (dev->of_node) {
-   ad7879_parse_dt(dev, ts);
} else {
-   dev_err(dev, "No platform data\n");
-   return -EINVAL;
+   err = ad7879_parse_dt(dev, ts);
+   if (err)
+   return err;
}

input_dev = devm_input_allocate_device(dev);




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/4] Input: ad7879 - make sure we set up drvdata

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

The conversion to devm accidentally removed setting up of I2C client data
upon successful probe of the touchscreen. Let's move this setting into the
core, so we do not forger about it again.

Fixes: 381f688eee3d ("Input: ad7879 - use more devm interfaces")
Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
 drivers/input/touchscreen/ad7879-spi.c | 2 --
 drivers/input/touchscreen/ad7879.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879-spi.c 
b/drivers/input/touchscreen/ad7879-spi.c
index c73798297b98..59486ccba37d 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -49,8 +49,6 @@ static int ad7879_spi_probe(struct spi_device *spi)
if (IS_ERR(ts))
return PTR_ERR(ts);

-   spi_set_drvdata(spi, ts);
-
return 0;
 }

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 1bd870277e1a..52daaa4edc67 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -680,6 +680,8 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,
if (err)
return ERR_PTR(err);

+   dev_set_drvdata(dev, ts);
+
return 0;
 }
 EXPORT_SYMBOL(ad7879_probe);




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/4] Input: ad7879 - make sure we set up drvdata

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

The conversion to devm accidentally removed setting up of I2C client data
upon successful probe of the touchscreen. Let's move this setting into the
core, so we do not forger about it again.

Fixes: 381f688eee3d ("Input: ad7879 - use more devm interfaces")
Signed-off-by: Dmitry Torokhov 


Acked-by: Michael Hennerich 


---
 drivers/input/touchscreen/ad7879-spi.c | 2 --
 drivers/input/touchscreen/ad7879.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879-spi.c 
b/drivers/input/touchscreen/ad7879-spi.c
index c73798297b98..59486ccba37d 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -49,8 +49,6 @@ static int ad7879_spi_probe(struct spi_device *spi)
if (IS_ERR(ts))
return PTR_ERR(ts);

-   spi_set_drvdata(spi, ts);
-
return 0;
 }

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 1bd870277e1a..52daaa4edc67 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -680,6 +680,8 @@ struct ad7879 *ad7879_probe(struct device *dev, struct 
regmap *regmap,
if (err)
return ERR_PTR(err);

+   dev_set_drvdata(dev, ts);
+
return 0;
 }
 EXPORT_SYMBOL(ad7879_probe);




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 4/4] Input: ad7879 - do not manipulate capability bits directly

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

Instead of manipulating capabilities bits of input device directly, let's
use input_set_capability() API.

Also, stop setting ABS_X/Y bits explicitly as input_set_abs_params() does
this for us.

Signed-off-by: Dmitry Torokhov <dmitry.torok...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
 drivers/input/touchscreen/ad7879.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index c415614ada68..196028c45210 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -590,13 +590,7 @@ int ad7879_probe(struct device *dev, struct regmap *regmap,

input_set_drvdata(input_dev, ts);

-   __set_bit(EV_ABS, input_dev->evbit);
-   __set_bit(ABS_X, input_dev->absbit);
-   __set_bit(ABS_Y, input_dev->absbit);
-   __set_bit(ABS_PRESSURE, input_dev->absbit);
-
-   __set_bit(EV_KEY, input_dev->evbit);
-   __set_bit(BTN_TOUCH, input_dev->keybit);
+   input_set_capability(input_dev, EV_KEY, BTN_TOUCH);

if (pdata) {
input_set_abs_params(input_dev, ABS_X,
@@ -614,6 +608,7 @@ int ad7879_probe(struct device *dev, struct regmap *regmap,
} else {
input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+   input_set_capability(input_dev, EV_ABS, ABS_PRESSURE);
touchscreen_parse_properties(input_dev, false, NULL);
if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
dev_err(dev, "Touchscreen pressure is not specified\n");




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 4/4] Input: ad7879 - do not manipulate capability bits directly

[Michael Hennerich]

On 28.02.2017 23:08, Dmitry Torokhov wrote:

Instead of manipulating capabilities bits of input device directly, let's
use input_set_capability() API.

Also, stop setting ABS_X/Y bits explicitly as input_set_abs_params() does
this for us.

Signed-off-by: Dmitry Torokhov 


Acked-by: Michael Hennerich 


---
 drivers/input/touchscreen/ad7879.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index c415614ada68..196028c45210 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -590,13 +590,7 @@ int ad7879_probe(struct device *dev, struct regmap *regmap,

input_set_drvdata(input_dev, ts);

-   __set_bit(EV_ABS, input_dev->evbit);
-   __set_bit(ABS_X, input_dev->absbit);
-   __set_bit(ABS_Y, input_dev->absbit);
-   __set_bit(ABS_PRESSURE, input_dev->absbit);
-
-   __set_bit(EV_KEY, input_dev->evbit);
-   __set_bit(BTN_TOUCH, input_dev->keybit);
+   input_set_capability(input_dev, EV_KEY, BTN_TOUCH);

if (pdata) {
input_set_abs_params(input_dev, ABS_X,
@@ -614,6 +608,7 @@ int ad7879_probe(struct device *dev, struct regmap *regmap,
} else {
input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+   input_set_capability(input_dev, EV_ABS, ABS_PRESSURE);
touchscreen_parse_properties(input_dev, false, NULL);
if (!input_abs_get_max(input_dev, ABS_PRESSURE)) {
dev_err(dev, "Touchscreen pressure is not specified\n");




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [RFC/RFT PATCH 1/3] Input: ad7879 - convert to use regmap

[Michael Hennerich]

On 18.02.2017 00:31, Dmitry Torokhov wrote:

Instead of rolling our own infrastructure to provide uniform access to I2C
and SPI buses, let's switch to using regmap.

Signed-off-by: Dmitry Torokhov 
---

Michael,

I am a bit (actually a lot) confused how this driver was working with SPI
and Blackfin, which is, as far as I understand, little-endian, because from
my reading of the spec the data on wire is big-endian. If this is not
correct we need to change spi regmap config to be:

static const struct regmap_config ad7879_spi_regmap_config = {
.reg_bits = 16,
.val_bits = 16,
.max_register = 15,
.read_flag_mask = AD7879_CMD_MAGIC | AD7879_CMD_READ,
.write_flag_mask = AD7879_CMD_MAGIC,
.reg_format_endian_default = REGMAP_ENDIAN_LITTLE,
.val_format_endian_default = REGMAP_ENDIAN_LITTLE,
};

Also, the original ad7879_spi_xfer was funky to tell the least. We
allocated n+2 spi_transfer structures, then smashed the beginning of the
first one with u16 command, then basically "restored" it???

Anyway, if you could try it out (and fix ;) ), that would be great.


Hi Dimitry,

Thanks for doing this.
Overall looks great, but i definitely need to test this.
Your right this xfer functions looks awkward.
I'll review and get back to you.



--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [RFC/RFT PATCH 1/3] Input: ad7879 - convert to use regmap

[Michael Hennerich]

On 18.02.2017 00:31, Dmitry Torokhov wrote:

Instead of rolling our own infrastructure to provide uniform access to I2C
and SPI buses, let's switch to using regmap.

Signed-off-by: Dmitry Torokhov 
---

Michael,

I am a bit (actually a lot) confused how this driver was working with SPI
and Blackfin, which is, as far as I understand, little-endian, because from
my reading of the spec the data on wire is big-endian. If this is not
correct we need to change spi regmap config to be:

static const struct regmap_config ad7879_spi_regmap_config = {
.reg_bits = 16,
.val_bits = 16,
.max_register = 15,
.read_flag_mask = AD7879_CMD_MAGIC | AD7879_CMD_READ,
.write_flag_mask = AD7879_CMD_MAGIC,
.reg_format_endian_default = REGMAP_ENDIAN_LITTLE,
.val_format_endian_default = REGMAP_ENDIAN_LITTLE,
};

Also, the original ad7879_spi_xfer was funky to tell the least. We
allocated n+2 spi_transfer structures, then smashed the beginning of the
first one with u16 command, then basically "restored" it???

Anyway, if you could try it out (and fix ;) ), that would be great.


Hi Dimitry,

Thanks for doing this.
Overall looks great, but i definitely need to test this.
Your right this xfer functions looks awkward.
I'll review and get back to you.



--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] backlight: adp5520: fix error handling in adp5520_bl_probe()

[Michael Hennerich]

On 09.07.2016 00:19, Alexey Khoroshilov wrote:

If adp5520_bl_setup() fails, sysfs group left unremoved.

By the way, fix overcomplicated assignement of error code.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshi...@ispras.ru>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/video/backlight/adp5520_bl.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/adp5520_bl.c 
b/drivers/video/backlight/adp5520_bl.c
index dd88ba1d71ce..35373e2065b2 100644
--- a/drivers/video/backlight/adp5520_bl.c
+++ b/drivers/video/backlight/adp5520_bl.c
@@ -332,10 +332,18 @@ static int adp5520_bl_probe(struct platform_device *pdev)
}

platform_set_drvdata(pdev, bl);
-   ret |= adp5520_bl_setup(bl);
+   ret = adp5520_bl_setup(bl);
+   if (ret) {
+   dev_err(>dev, "failed to setup\n");
+   if (data->pdata->en_ambl_sens)
+   sysfs_remove_group(>dev.kobj,
+   _bl_attr_group);
+   return ret;
+   }
+
backlight_update_status(bl);

-   return ret;
+   return 0;
  }

  static int adp5520_bl_remove(struct platform_device *pdev)




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] backlight: adp5520: fix error handling in adp5520_bl_probe()

[Michael Hennerich]

On 09.07.2016 00:19, Alexey Khoroshilov wrote:

If adp5520_bl_setup() fails, sysfs group left unremoved.

By the way, fix overcomplicated assignement of error code.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 


Acked-by: Michael Hennerich 


---
  drivers/video/backlight/adp5520_bl.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/adp5520_bl.c 
b/drivers/video/backlight/adp5520_bl.c
index dd88ba1d71ce..35373e2065b2 100644
--- a/drivers/video/backlight/adp5520_bl.c
+++ b/drivers/video/backlight/adp5520_bl.c
@@ -332,10 +332,18 @@ static int adp5520_bl_probe(struct platform_device *pdev)
}

platform_set_drvdata(pdev, bl);
-   ret |= adp5520_bl_setup(bl);
+   ret = adp5520_bl_setup(bl);
+   if (ret) {
+   dev_err(>dev, "failed to setup\n");
+   if (data->pdata->en_ambl_sens)
+   sysfs_remove_group(>dev.kobj,
+   _bl_attr_group);
+   return ret;
+   }
+
backlight_update_status(bl);

-   return ret;
+   return 0;
  }

  static int adp5520_bl_remove(struct platform_device *pdev)




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] iio: dac: fix off-by-one comparison and out-of-bounds write

[Michael Hennerich]

On 06/02/2016 12:06 PM, Colin King wrote:

From: Colin Ian King <colin.k...@canonical.com>

The check on reg is off-by-one, it should be >= rather than >. Fix
this to stop an out-of-bounds write to st->channel_modes[reg].

Signed-off-by: Colin Ian King <colin.k...@canonical.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/iio/dac/ad5592r-base.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index 948f600..69bde59 100644
--- a/drivers/iio/dac/ad5592r-base.c
+++ b/drivers/iio/dac/ad5592r-base.c
@@ -525,7 +525,7 @@ static int ad5592r_alloc_channels(struct ad5592r_state *st)

device_for_each_child_node(st->dev, child) {
ret = fwnode_property_read_u32(child, "reg", );
-   if (ret || reg > ARRAY_SIZE(st->channel_modes))
+   if (ret || reg >= ARRAY_SIZE(st->channel_modes))
continue;

ret = fwnode_property_read_u32(child, "adi,mode", );




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH] iio: dac: fix off-by-one comparison and out-of-bounds write

[Michael Hennerich]

On 06/02/2016 12:06 PM, Colin King wrote:

From: Colin Ian King 

The check on reg is off-by-one, it should be >= rather than >. Fix
this to stop an out-of-bounds write to st->channel_modes[reg].

Signed-off-by: Colin Ian King 


Acked-by: Michael Hennerich 


---
  drivers/iio/dac/ad5592r-base.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index 948f600..69bde59 100644
--- a/drivers/iio/dac/ad5592r-base.c
+++ b/drivers/iio/dac/ad5592r-base.c
@@ -525,7 +525,7 @@ static int ad5592r_alloc_channels(struct ad5592r_state *st)

device_for_each_child_node(st->dev, child) {
ret = fwnode_property_read_u32(child, "reg", );
-   if (ret || reg > ARRAY_SIZE(st->channel_modes))
+   if (ret || reg >= ARRAY_SIZE(st->channel_modes))
continue;

ret = fwnode_property_read_u32(child, "adi,mode", );




--
Greetings,
Michael

--
Analog Devices GmbH  Otl-Aicher Strasse 60-64  80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH 1/1] net: ieee802154/adf7242: syntax error ifdef DEBUG

[Michael Hennerich]

On 05/17/2016 11:58 PM, Heinrich Schuchardt wrote:

If DEBUG is defined, a superfluous closing brace
is introduced.

Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/net/ieee802154/adf7242.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index b82e39d..9fa7ac9 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -915,7 +915,6 @@ static void adf7242_debug(u8 irq1)
(stat & 0xf) == RC_STATUS_PHY_RDY ? "RC_STATUS_PHY_RDY" : "",
(stat & 0xf) == RC_STATUS_RX ? "RC_STATUS_RX" : "",
(stat & 0xf) == RC_STATUS_TX ? "RC_STATUS_TX" : "");
-   }
  #endif
  }





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 1/1] net: ieee802154/adf7242: syntax error ifdef DEBUG

[Michael Hennerich]

On 05/17/2016 11:58 PM, Heinrich Schuchardt wrote:

If DEBUG is defined, a superfluous closing brace
is introduced.

Signed-off-by: Heinrich Schuchardt 


Acked-by: Michael Hennerich 


---
  drivers/net/ieee802154/adf7242.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index b82e39d..9fa7ac9 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -915,7 +915,6 @@ static void adf7242_debug(u8 irq1)
(stat & 0xf) == RC_STATUS_PHY_RDY ? "RC_STATUS_PHY_RDY" : "",
(stat & 0xf) == RC_STATUS_RX ? "RC_STATUS_RX" : "",
(stat & 0xf) == RC_STATUS_TX ? "RC_STATUS_TX" : "");
-   }
  #endif
  }





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] ieee802154/adf7242: fix memory leak of firmware

[Michael Hennerich]

On 04/07/2016 01:16 PM, Sudip Mukherjee wrote:

If the firmware upload or the firmware verification fails then we
printed the error message and exited but we missed releasing the
firmware.

Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/net/ieee802154/adf7242.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index 89154c0..91d4531 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -1030,6 +1030,7 @@ static int adf7242_hw_init(struct adf7242_local *lp)
if (ret) {
dev_err(>spi->dev,
"upload firmware failed with %d\n", ret);
+   release_firmware(fw);
return ret;
}

@@ -1037,6 +1038,7 @@ static int adf7242_hw_init(struct adf7242_local *lp)
if (ret) {
dev_err(>spi->dev,
"verify firmware failed with %d\n", ret);
+   release_firmware(fw);
return ret;
}





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] ieee802154/adf7242: fix memory leak of firmware

[Michael Hennerich]

On 04/07/2016 01:16 PM, Sudip Mukherjee wrote:

If the firmware upload or the firmware verification fails then we
printed the error message and exited but we missed releasing the
firmware.

Signed-off-by: Sudip Mukherjee 


Acked-by: Michael Hennerich 


---
  drivers/net/ieee802154/adf7242.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index 89154c0..91d4531 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -1030,6 +1030,7 @@ static int adf7242_hw_init(struct adf7242_local *lp)
if (ret) {
dev_err(>spi->dev,
"upload firmware failed with %d\n", ret);
+   release_firmware(fw);
return ret;
}

@@ -1037,6 +1038,7 @@ static int adf7242_hw_init(struct adf7242_local *lp)
if (ret) {
dev_err(>spi->dev,
"verify firmware failed with %d\n", ret);
+   release_firmware(fw);
return ret;
}





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 06/61] gpio: adp5520: Use devm_gpiochip_add_data() for gpio registration

[Michael Hennerich]

On 02/22/2016 03:07 PM, Laxman Dewangan wrote:

Use devm_gpiochip_add_data() for GPIO registration and remove the
call for gpiochip_remove() from error path.

Also remove the need of driver callback .remove.

Signed-off-by: Laxman Dewangan <ldewan...@nvidia.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


Cc: Michael Hennerich <michael.henner...@analog.com>
---
  drivers/gpio/gpio-adp5520.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-adp5520.c b/drivers/gpio/gpio-adp5520.c
index 4fa7ff1..abf1996 100644
--- a/drivers/gpio/gpio-adp5520.c
+++ b/drivers/gpio/gpio-adp5520.c
@@ -153,7 +153,7 @@ static int adp5520_gpio_probe(struct platform_device *pdev)
goto err;
}

-   ret = gpiochip_add_data(>gpio_chip, dev);
+   ret = devm_gpiochip_add_data(>dev, >gpio_chip, dev);
if (ret)
goto err;

@@ -164,22 +164,11 @@ err:
return ret;
  }

-static int adp5520_gpio_remove(struct platform_device *pdev)
-{
-   struct adp5520_gpio *dev;
-
-   dev = platform_get_drvdata(pdev);
-   gpiochip_remove(>gpio_chip);
-
-   return 0;
-}
-
  static struct platform_driver adp5520_gpio_driver = {
.driver = {
.name   = "adp5520-gpio",
},
.probe  = adp5520_gpio_probe,
-   .remove = adp5520_gpio_remove,
  };

  module_platform_driver(adp5520_gpio_driver);




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 06/61] gpio: adp5520: Use devm_gpiochip_add_data() for gpio registration

[Michael Hennerich]

On 02/22/2016 03:07 PM, Laxman Dewangan wrote:

Use devm_gpiochip_add_data() for GPIO registration and remove the
call for gpiochip_remove() from error path.

Also remove the need of driver callback .remove.

Signed-off-by: Laxman Dewangan 


Acked-by: Michael Hennerich 


Cc: Michael Hennerich 
---
  drivers/gpio/gpio-adp5520.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-adp5520.c b/drivers/gpio/gpio-adp5520.c
index 4fa7ff1..abf1996 100644
--- a/drivers/gpio/gpio-adp5520.c
+++ b/drivers/gpio/gpio-adp5520.c
@@ -153,7 +153,7 @@ static int adp5520_gpio_probe(struct platform_device *pdev)
goto err;
}

-   ret = gpiochip_add_data(>gpio_chip, dev);
+   ret = devm_gpiochip_add_data(>dev, >gpio_chip, dev);
if (ret)
goto err;

@@ -164,22 +164,11 @@ err:
return ret;
  }

-static int adp5520_gpio_remove(struct platform_device *pdev)
-{
-   struct adp5520_gpio *dev;
-
-   dev = platform_get_drvdata(pdev);
-   gpiochip_remove(>gpio_chip);
-
-   return 0;
-}
-
  static struct platform_driver adp5520_gpio_driver = {
.driver = {
.name   = "adp5520-gpio",
},
.probe  = adp5520_gpio_probe,
-   .remove = adp5520_gpio_remove,
  };

  module_platform_driver(adp5520_gpio_driver);




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 07/61] gpio: adp5588: Use devm_gpiochip_add_data() for gpio registration

[Michael Hennerich]

On 02/22/2016 03:07 PM, Laxman Dewangan wrote:

Use devm_gpiochip_add_data() for GPIO registration and remove the
call for gpiochip_remove() from remove callback.

Signed-off-by: Laxman Dewangan <ldewan...@nvidia.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


Cc: Michael Hennerich <michael.henner...@analog.com>
---
  drivers/gpio/gpio-adp5588.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
index 19a0eba..c0f718b 100644
--- a/drivers/gpio/gpio-adp5588.c
+++ b/drivers/gpio/gpio-adp5588.c
@@ -414,7 +414,7 @@ static int adp5588_gpio_probe(struct i2c_client *client,
}
}

-   ret = gpiochip_add_data(>gpio_chip, dev);
+   ret = devm_gpiochip_add_data(>dev, >gpio_chip, dev);
if (ret)
goto err_irq;

@@ -457,8 +457,6 @@ static int adp5588_gpio_remove(struct i2c_client *client)
if (dev->irq_base)
free_irq(dev->client->irq, dev);

-   gpiochip_remove(>gpio_chip);
-
return 0;
  }





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 07/61] gpio: adp5588: Use devm_gpiochip_add_data() for gpio registration

[Michael Hennerich]

On 02/22/2016 03:07 PM, Laxman Dewangan wrote:

Use devm_gpiochip_add_data() for GPIO registration and remove the
call for gpiochip_remove() from remove callback.

Signed-off-by: Laxman Dewangan 


Acked-by: Michael Hennerich 


Cc: Michael Hennerich 
---
  drivers/gpio/gpio-adp5588.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
index 19a0eba..c0f718b 100644
--- a/drivers/gpio/gpio-adp5588.c
+++ b/drivers/gpio/gpio-adp5588.c
@@ -414,7 +414,7 @@ static int adp5588_gpio_probe(struct i2c_client *client,
}
}

-   ret = gpiochip_add_data(>gpio_chip, dev);
+   ret = devm_gpiochip_add_data(>dev, >gpio_chip, dev);
if (ret)
goto err_irq;

@@ -457,8 +457,6 @@ static int adp5588_gpio_remove(struct i2c_client *client)
if (dev->irq_base)
free_irq(dev->client->irq, dev);

-   gpiochip_remove(>gpio_chip);
-
return 0;
  }





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] misc: ad525x_dpot: Fix the enabling of the "otpXen" attributes

[Michael Hennerich]

On 11/19/2015 09:22 AM, Michael Hennerich wrote:

On 11/18/2015 05:16 PM, Dan Bogdan Nechita wrote:

Currently writing the attributes with "echo" will result in comparing:
"enabled\n" with "enabled\0" and attribute is always set to false.

Use the sysfs_streq() instead because it treats both NUL and
new-line-then-NUL as equivalent string terminations.

Signed-off-by: Dan Bogdan Nechita <dan.bogdan.nech...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>



Looks like to patch got lost?





---
  drivers/misc/ad525x_dpot.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index 15e8807..4230e6a 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -452,7 +452,7 @@ static ssize_t sysfs_set_reg(struct device *dev,
  int err;

  if (reg & DPOT_ADDR_OTP_EN) {
-if (!strncmp(buf, "enabled", sizeof("enabled")))
+if (sysfs_streq(buf, "enabled"))
  set_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);
  else
  clear_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);







--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] misc: ad525x_dpot: Fix the enabling of the "otpXen" attributes

[Michael Hennerich]

On 11/19/2015 09:22 AM, Michael Hennerich wrote:

On 11/18/2015 05:16 PM, Dan Bogdan Nechita wrote:

Currently writing the attributes with "echo" will result in comparing:
"enabled\n" with "enabled\0" and attribute is always set to false.

Use the sysfs_streq() instead because it treats both NUL and
new-line-then-NUL as equivalent string terminations.

Signed-off-by: Dan Bogdan Nechita 


Acked-by: Michael Hennerich 



Looks like to patch got lost?





---
  drivers/misc/ad525x_dpot.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index 15e8807..4230e6a 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -452,7 +452,7 @@ static ssize_t sysfs_set_reg(struct device *dev,
  int err;

  if (reg & DPOT_ADDR_OTP_EN) {
-if (!strncmp(buf, "enabled", sizeof("enabled")))
+if (sysfs_streq(buf, "enabled"))
  set_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);
  else
  clear_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);







--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] regulator: ad5398: Fix return value of ad5398_write_reg

[Michael Hennerich]

On 02/11/2016 11:17 AM, Axel Lin wrote:

i2c_master_send() returns the number of bytes written on success.
So current code returns 2 if ad5398_write_reg() success.
This return value is propagated to .set_current_limit, .enable and .disable
callbacks of regulator_ops. This can be a problem, for example, if the
users test if the return value of regulator_set_current_limit() is 0.
Fix it by making ad5398_write_reg() return 0 on success.

Signed-off-by: Axel Lin 


Acked-by: Michael Hennerich 


---
  drivers/regulator/ad5398.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/ad5398.c b/drivers/regulator/ad5398.c
index ea50a88..8b0f788 100644
--- a/drivers/regulator/ad5398.c
+++ b/drivers/regulator/ad5398.c
@@ -58,10 +58,12 @@ static int ad5398_write_reg(struct i2c_client *client, 
const unsigned short data

val = cpu_to_be16(data);
ret = i2c_master_send(client, (char *), 2);
-   if (ret < 0)
+   if (ret != 2) {
dev_err(>dev, "I2C write error\n");
+   return ret < 0 ? ret : -EIO;
+   }

-   return ret;
+   return 0;
  }

  static int ad5398_get_current_limit(struct regulator_dev *rdev)




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] regulator: ad5398: Fix return value of ad5398_write_reg

[Michael Hennerich]

On 02/11/2016 11:17 AM, Axel Lin wrote:

i2c_master_send() returns the number of bytes written on success.
So current code returns 2 if ad5398_write_reg() success.
This return value is propagated to .set_current_limit, .enable and .disable
callbacks of regulator_ops. This can be a problem, for example, if the
users test if the return value of regulator_set_current_limit() is 0.
Fix it by making ad5398_write_reg() return 0 on success.

Signed-off-by: Axel Lin <axel@ingics.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/regulator/ad5398.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/ad5398.c b/drivers/regulator/ad5398.c
index ea50a88..8b0f788 100644
--- a/drivers/regulator/ad5398.c
+++ b/drivers/regulator/ad5398.c
@@ -58,10 +58,12 @@ static int ad5398_write_reg(struct i2c_client *client, 
const unsigned short data

val = cpu_to_be16(data);
ret = i2c_master_send(client, (char *), 2);
-   if (ret < 0)
+   if (ret != 2) {
dev_err(>dev, "I2C write error\n");
+   return ret < 0 ? ret : -EIO;
+   }

-   return ret;
+   return 0;
  }

  static int ad5398_get_current_limit(struct regulator_dev *rdev)




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support

[Michael Hennerich]

On 02/03/2016 12:20 AM, Stefan Agner wrote:

Add device tree support for the I2C and SPI variant of AD7879(-1).
This allows to specify the touchscreen controller as a I2C client
node or SPI slave device. Most of the options available in platform
data are also available as device tree properties, the only exception
being GPIO capabilities, which can not be activated through device
tree currently.

Acked-by: Rob Herring 
Signed-off-by: Stefan Agner 


Acked-by: Michael Hennerich 


---
Changes since v2:
- do not free driver data and irq on remove (since we are using devm now)
Changes since v1:
- Move device tree parsing to main driver file ad7879.c
- Use common touchscreen_parse_properties for common properties
- Use device_property_* API
- Use struct ad7879 directly to store parsed values
- Support SPI variant through device tree too (untested)
- Add vendor prefix to vendor specific dt properties

  .../bindings/input/touchscreen/ad7879.txt  |  53 
  drivers/input/touchscreen/ad7879-i2c.c |  10 ++
  drivers/input/touchscreen/ad7879-spi.c |  10 ++
  drivers/input/touchscreen/ad7879.c | 145 +
  4 files changed, 161 insertions(+), 57 deletions(-)
  create mode 100644 
Documentation/devicetree/bindings/input/touchscreen/ad7879.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt 
b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
new file mode 100644
index 000..e3f22d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
@@ -0,0 +1,53 @@
+* Analog Devices AD7879(-1)/AD7889(-1) touchscreen interface (SPI/I2C)
+
+Required properties:
+- compatible   : for SPI slave, use "adi,ad7879"
+ for I2C slave, use "adi,ad7879-1"
+- reg  : SPI chipselect/I2C slave address
+ See spi-bus.txt for more SPI slave properties
+- interrupt-parent : the phandle for the interrupt controller
+- interrupts   : touch controller interrupt
+- touchscreen-max-pressure : maximum reported pressure
+- adi,resistance-plate-x   : total resistance of X-plate (for pressure
+ calculation)
+Optional properties:
+- touchscreen-swapped-x-y  : X and Y axis are swapped (boolean)
+- adi,first-conversion-delay   : 0-12: In 128us steps (starting with 128us)
+ 13  : 2.560ms
+ 14  : 3.584ms
+ 15  : 4.096ms
+ This property has to be a '/bits/ 8' value
+- adi,acquisition-time : 0: 2us
+ 1: 4us
+ 2: 8us
+ 3: 16us
+ This property has to be a '/bits/ 8' value
+- adi,median-filter-size   : 0: disabled
+ 1: 4 measurements
+ 2: 8 measurements
+ 3: 16 measurements
+ This property has to be a '/bits/ 8' value
+- adi,averaging: 0: 2 middle values (1 if median 
disabled)
+ 1: 4 middle values
+ 2: 8 middle values
+ 3: 16 values
+ This property has to be a '/bits/ 8' value
+- adi,conversion-interval: : 0: convert one time only
+ 1-255: 515us + val * 35us (up to 9.440ms)
+ This property has to be a '/bits/ 8' value
+
+Example:
+
+   ad7879@2c {
+   compatible = "adi,ad7879-1";
+   reg = <0x2c>;
+   interrupt-parent = <>;
+   interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+   touchscreen-max-pressure = <4096>;
+   adi,resistance-plate-x = <120>;
+   adi,first-conversion-delay = /bits/ 8 <3>;
+   adi,acquisition-time = /bits/ 8 <1>;
+   adi,median-filter-size = /bits/ 8 <2>;
+   adi,averaging = /bits/ 8 <1>;
+   adi,conversion-interval = /bits/ 8 <255>;
+   };
diff --git a/drivers/input/touchscreen/ad7879-i2c.c 
b/drivers/input/touchscreen/ad7879-i2c.c
index d66962c..58f72e0 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -10,6 +10,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 

  #include "ad7879.h"
@@ -91,10 +92,19 @@ static const struct i2c_device_id ad7879_id[] = {
  };
  MODULE_DEVICE_TABLE(i2c, ad7879_id);

+#ifdef CONFIG_OF
+static const struct of_device_id ad7879_i2c_dt_ids[] = {
+   { .compatible = "adi,ad7879-1", },
+

Re: [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment

[Michael Hennerich]

On 02/03/2016 12:20 AM, Stefan Agner wrote:

The X/Y position measurements read from the controller are interpreted
wrong. The first measurement X+ contains the Y position, and the second
measurement Y+ the X position (see also Table 11 Register Table in the
data sheet).

The problem is already known and a swap option has been introduced:
commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")

However, the meaning of the new boolean is inverted since the underlying
values are already swapped. Let ts->swap_xy set to true actually be the
swapped configuration of the two axis.

Signed-off-by: Stefan Agner 


Acked-by: Michael Hennerich 


---
Changes since v2:
- (none)
Changes since v1:
- Keep axis swapped by default when using platform data

  drivers/input/touchscreen/ad7879.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 93b8ea2..abd0220 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -94,8 +94,8 @@
  #define AD7879_TEMP_BIT   (1<<1)

  enum {
-   AD7879_SEQ_XPOS  = 0,
-   AD7879_SEQ_YPOS  = 1,
+   AD7879_SEQ_YPOS  = 0,
+   AD7879_SEQ_XPOS  = 1,
AD7879_SEQ_Z1= 2,
AD7879_SEQ_Z2= 3,
AD7879_NR_SENSE  = 4,
@@ -517,7 +517,9 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, 
unsigned int irq,
ts->dev = dev;
ts->input = input_dev;
ts->irq = irq;
-   ts->swap_xy = pdata->swap_xy;
+
+   /* Use swapped axis by default (backward compatibility) */
+   ts->swap_xy = !pdata->swap_xy;

setup_timer(>timer, ad7879_timer, (unsigned long) ts);





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH v3 2/3] input: touchscreen: ad7879: fix default x/y axis assignment

[Michael Hennerich]

On 02/03/2016 12:20 AM, Stefan Agner wrote:

The X/Y position measurements read from the controller are interpreted
wrong. The first measurement X+ contains the Y position, and the second
measurement Y+ the X position (see also Table 11 Register Table in the
data sheet).

The problem is already known and a swap option has been introduced:
commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")

However, the meaning of the new boolean is inverted since the underlying
values are already swapped. Let ts->swap_xy set to true actually be the
swapped configuration of the two axis.

Signed-off-by: Stefan Agner <ste...@agner.ch>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
Changes since v2:
- (none)
Changes since v1:
- Keep axis swapped by default when using platform data

  drivers/input/touchscreen/ad7879.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index 93b8ea2..abd0220 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -94,8 +94,8 @@
  #define AD7879_TEMP_BIT   (1<<1)

  enum {
-   AD7879_SEQ_XPOS  = 0,
-   AD7879_SEQ_YPOS  = 1,
+   AD7879_SEQ_YPOS  = 0,
+   AD7879_SEQ_XPOS  = 1,
AD7879_SEQ_Z1= 2,
AD7879_SEQ_Z2= 3,
AD7879_NR_SENSE  = 4,
@@ -517,7 +517,9 @@ struct ad7879 *ad7879_probe(struct device *dev, u8 devid, 
unsigned int irq,
ts->dev = dev;
ts->input = input_dev;
ts->irq = irq;
-   ts->swap_xy = pdata->swap_xy;
+
+   /* Use swapped axis by default (backward compatibility) */
+   ts->swap_xy = !pdata->swap_xy;

setup_timer(>timer, ad7879_timer, (unsigned long) ts);





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH v3 3/3] input: touchscreen: ad7879: add device tree support

[Michael Hennerich]

On 02/03/2016 12:20 AM, Stefan Agner wrote:

Add device tree support for the I2C and SPI variant of AD7879(-1).
This allows to specify the touchscreen controller as a I2C client
node or SPI slave device. Most of the options available in platform
data are also available as device tree properties, the only exception
being GPIO capabilities, which can not be activated through device
tree currently.

Acked-by: Rob Herring <r...@kernel.org>
Signed-off-by: Stefan Agner <ste...@agner.ch>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
Changes since v2:
- do not free driver data and irq on remove (since we are using devm now)
Changes since v1:
- Move device tree parsing to main driver file ad7879.c
- Use common touchscreen_parse_properties for common properties
- Use device_property_* API
- Use struct ad7879 directly to store parsed values
- Support SPI variant through device tree too (untested)
- Add vendor prefix to vendor specific dt properties

  .../bindings/input/touchscreen/ad7879.txt  |  53 
  drivers/input/touchscreen/ad7879-i2c.c |  10 ++
  drivers/input/touchscreen/ad7879-spi.c |  10 ++
  drivers/input/touchscreen/ad7879.c | 145 +
  4 files changed, 161 insertions(+), 57 deletions(-)
  create mode 100644 
Documentation/devicetree/bindings/input/touchscreen/ad7879.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt 
b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
new file mode 100644
index 000..e3f22d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879.txt
@@ -0,0 +1,53 @@
+* Analog Devices AD7879(-1)/AD7889(-1) touchscreen interface (SPI/I2C)
+
+Required properties:
+- compatible   : for SPI slave, use "adi,ad7879"
+ for I2C slave, use "adi,ad7879-1"
+- reg  : SPI chipselect/I2C slave address
+ See spi-bus.txt for more SPI slave properties
+- interrupt-parent : the phandle for the interrupt controller
+- interrupts   : touch controller interrupt
+- touchscreen-max-pressure : maximum reported pressure
+- adi,resistance-plate-x   : total resistance of X-plate (for pressure
+ calculation)
+Optional properties:
+- touchscreen-swapped-x-y  : X and Y axis are swapped (boolean)
+- adi,first-conversion-delay   : 0-12: In 128us steps (starting with 128us)
+ 13  : 2.560ms
+ 14  : 3.584ms
+ 15  : 4.096ms
+ This property has to be a '/bits/ 8' value
+- adi,acquisition-time : 0: 2us
+ 1: 4us
+ 2: 8us
+ 3: 16us
+ This property has to be a '/bits/ 8' value
+- adi,median-filter-size   : 0: disabled
+ 1: 4 measurements
+ 2: 8 measurements
+ 3: 16 measurements
+ This property has to be a '/bits/ 8' value
+- adi,averaging: 0: 2 middle values (1 if median 
disabled)
+ 1: 4 middle values
+ 2: 8 middle values
+ 3: 16 values
+ This property has to be a '/bits/ 8' value
+- adi,conversion-interval: : 0: convert one time only
+ 1-255: 515us + val * 35us (up to 9.440ms)
+ This property has to be a '/bits/ 8' value
+
+Example:
+
+   ad7879@2c {
+   compatible = "adi,ad7879-1";
+   reg = <0x2c>;
+   interrupt-parent = <>;
+   interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+   touchscreen-max-pressure = <4096>;
+   adi,resistance-plate-x = <120>;
+   adi,first-conversion-delay = /bits/ 8 <3>;
+   adi,acquisition-time = /bits/ 8 <1>;
+   adi,median-filter-size = /bits/ 8 <2>;
+   adi,averaging = /bits/ 8 <1>;
+   adi,conversion-interval = /bits/ 8 <255>;
+   };
diff --git a/drivers/input/touchscreen/ad7879-i2c.c 
b/drivers/input/touchscreen/ad7879-i2c.c
index d66962c..58f72e0 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -10,6 +10,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 

  #include "ad7879.h"
@@ -91,10 +92,19 @@ static const struct i2c_device_id ad7879_id[] = {
  };
  MODULE_DEVICE_TABLE(i2c, ad7879_id);

+#ifdef CONFIG_OF
+static const struc

Re: [PATCH 2/3] input: touchscreen: ad7879: fix default x/y axis assignment

[Michael Hennerich]

On 01/26/2016 04:04 AM, Stefan Agner wrote:

The measurements read from the controller which are temporary stored
in conversion_data, are interpreted wrong. The first measurement X+
contains the Y position, and the second measurement Y+ the X position
(see also Table 11 Register Table in the data sheet).

The problem is already known and a swap option has been introduced:
commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")

However, with that the meaning of the new boolean is inverted since
the underlying values are already swapped. With this change, a true
in swap_xy actually swaps the two axis.

Signed-off-by: Stefan Agner 
---
Hi Michael,

It seems that swap_xy is not used in any board which is in mainline,
hence swap_xy is always false. Therefore, up until now all boards
actually used swapped axis. However, I doubt that the blackfin boards
really have those axis swapped, it is probably more likely that the
userspace calibration took care of it.

However, if they are really swapped, we should set the swap_xy flag
to 1 for those board...

Do you happen to now what is the case with those boards?




Hi Stefan,

I would be hesitant to invert the default behaviour of the driver.
Too many people in the field already using it as it is.

A XY swap can have multiple reasons.

Lot's of small VGA/QVGA TFTs have the option to switch the scan 
direction from Landscape to Portrait. In addition you can also rotate 
and flip or mirror using VDMA options. So it really depends on the use 
case, how the touch panel is mounted to the screen or how it is wired.


Regards,
Michael


--
Stefan

  drivers/input/touchscreen/ad7879.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index a73934b..e290e7b 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -94,8 +94,8 @@
  #define AD7879_TEMP_BIT   (1<<1)

  enum {
-   AD7879_SEQ_XPOS  = 0,
-   AD7879_SEQ_YPOS  = 1,
+   AD7879_SEQ_YPOS  = 0,
+   AD7879_SEQ_XPOS  = 1,
AD7879_SEQ_Z1= 2,
AD7879_SEQ_Z2= 3,
AD7879_NR_SENSE  = 4,




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH 3/3] input: touchscreen: ad7879: add device tree support

[Michael Hennerich]

On 01/26/2016 04:04 AM, Stefan Agner wrote:

Add device tree support for the I2C variant of AD7879 (AD7879-1). This
allows to specify the touchscreen controller as a I2C client node.
Most of the options available as platform data are also available as
device tree properties. Exporting the GPIO is currently not possible
through device tree.

Signed-off-by: Stefan Agner 



Hi Stefan,

Thanks for the patch -
There is something similar in our tree but I forgot to send it mainline 
a long time ago.


https://github.com/analogdevicesinc/linux/commit/69b16d4b616a4bbe9001d3f67d3ff54f3deb85ce

There are some build issues can you have a look?

I also don't understand why "exporting the GPIO is not possible through 
device tree"?


Can you explain?

Regards,
Michael



---
  .../bindings/input/touchscreen/ad7879-i2c.txt  | 47 
  drivers/input/touchscreen/ad7879-i2c.c | 63 +-
  drivers/input/touchscreen/ad7879-spi.c |  3 +-
  drivers/input/touchscreen/ad7879.c |  2 +-
  drivers/input/touchscreen/ad7879.h |  1 +
  5 files changed, 113 insertions(+), 3 deletions(-)
  create mode 100644 
Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt 
b/Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt
new file mode 100644
index 000..bf169a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt
@@ -0,0 +1,47 @@
+* Analog Devices AD7879-1/AD7889-1 touchscreen interface (I2C)
+
+Required properties:
+- compatible: must be "adi,ad7879-1"
+- reg: i2c slave address
+- interrupt-parent: the phandle for the interrupt controller
+- interrupts: touch controller interrupt
+- resistance-plate-x   : total resistance of X-plate (for pressure
+ calculation)
+- touchscreen-max-pressure : maximum reported pressure
+- touchscreen-swapped-x-y  : X and Y axis are swapped (boolean)
+ Swapping is done after inverting the axis
+Optional properties:
+- first-conversion-delay   : 0-12 in 128us steps (starting with 128us)
+ 13: 2.560ms
+ 14: 3.584ms
+ 15: 4.096ms
+- acquisition-time : 0: 2us
+ 1: 4us
+ 2: 8us
+ 3: 16us
+- median-filter-size   : 0: disabled
+ 1: 4 measurements
+ 2: 8 measurements
+ 3: 16 measurements
+- averaging: 0: 2 middle values (1 if median disabled)
+ 1: 4 middle values
+ 2: 8 middle values
+ 3: 16 values
+- conversion-interval: : 0: convert one time only
+ 1-255: 515us + val * 35us (up to 9.440ms)
+
+Example:
+
+   ad7879@2c {
+   compatible = "adi,ad7879-1";
+   reg = <0x2c>;
+   interrupt-parent = <>;
+   interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+   resistance-plate-x = <120>;
+   touchscreen-max-pressure = <4096>;
+   first-conversion-delay = /bits/ 8 <3>;
+   acquisition-time = /bits/ 8 <1>;
+   median-filter-size = /bits/ 8 <2>;
+   averaging = /bits/ 8 <1>;
+   conversion-interval = /bits/ 8 <255>;
+   };
diff --git a/drivers/input/touchscreen/ad7879-i2c.c 
b/drivers/input/touchscreen/ad7879-i2c.c
index d66962c..08a2c9a 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -11,6 +11,7 @@
  #include 
  #include 
  #include 
+#include 

  #include "ad7879.h"

@@ -54,9 +55,50 @@ static const struct ad7879_bus_ops ad7879_i2c_bus_ops = {
.write  = ad7879_i2c_write,
  };

+static struct ad7879_platform_data *ad7879_parse_dt(struct device *dev)
+{
+   struct ad7879_platform_data *pdata;
+   struct device_node *np = dev->of_node;
+   int err;
+   u32 tmp;
+
+   if (!np)
+   return NULL;
+
+   pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+
+   if (!pdata)
+   return ERR_PTR(-ENOMEM);
+
+   err = of_property_read_u32(np, "resistance-plate-x", );
+   if (err) {
+   dev_err(dev, "failed to get resistance-plate-x property\n");
+   return ERR_PTR(err);
+   }
+   pdata->x_plate_ohms = (u16)tmp;
+
+   err = of_property_read_u32(np, "touchscreen-max-pressure", );
+   if (err) {
+   dev_err(dev, "failed to get touchscreen-max-pressure 
property\n");
+   return ERR_PTR(err);
+   }
+   pdata->pressure_min = (u16)tmp;
+
+   

Re: [PATCH 3/3] input: touchscreen: ad7879: add device tree support

[Michael Hennerich]

On 01/26/2016 04:04 AM, Stefan Agner wrote:

Add device tree support for the I2C variant of AD7879 (AD7879-1). This
allows to specify the touchscreen controller as a I2C client node.
Most of the options available as platform data are also available as
device tree properties. Exporting the GPIO is currently not possible
through device tree.

Signed-off-by: Stefan Agner 



Hi Stefan,

Thanks for the patch -
There is something similar in our tree but I forgot to send it mainline 
a long time ago.


https://github.com/analogdevicesinc/linux/commit/69b16d4b616a4bbe9001d3f67d3ff54f3deb85ce

There are some build issues can you have a look?

I also don't understand why "exporting the GPIO is not possible through 
device tree"?


Can you explain?

Regards,
Michael



---
  .../bindings/input/touchscreen/ad7879-i2c.txt  | 47 
  drivers/input/touchscreen/ad7879-i2c.c | 63 +-
  drivers/input/touchscreen/ad7879-spi.c |  3 +-
  drivers/input/touchscreen/ad7879.c |  2 +-
  drivers/input/touchscreen/ad7879.h |  1 +
  5 files changed, 113 insertions(+), 3 deletions(-)
  create mode 100644 
Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt 
b/Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt
new file mode 100644
index 000..bf169a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ad7879-i2c.txt
@@ -0,0 +1,47 @@
+* Analog Devices AD7879-1/AD7889-1 touchscreen interface (I2C)
+
+Required properties:
+- compatible: must be "adi,ad7879-1"
+- reg: i2c slave address
+- interrupt-parent: the phandle for the interrupt controller
+- interrupts: touch controller interrupt
+- resistance-plate-x   : total resistance of X-plate (for pressure
+ calculation)
+- touchscreen-max-pressure : maximum reported pressure
+- touchscreen-swapped-x-y  : X and Y axis are swapped (boolean)
+ Swapping is done after inverting the axis
+Optional properties:
+- first-conversion-delay   : 0-12 in 128us steps (starting with 128us)
+ 13: 2.560ms
+ 14: 3.584ms
+ 15: 4.096ms
+- acquisition-time : 0: 2us
+ 1: 4us
+ 2: 8us
+ 3: 16us
+- median-filter-size   : 0: disabled
+ 1: 4 measurements
+ 2: 8 measurements
+ 3: 16 measurements
+- averaging: 0: 2 middle values (1 if median disabled)
+ 1: 4 middle values
+ 2: 8 middle values
+ 3: 16 values
+- conversion-interval: : 0: convert one time only
+ 1-255: 515us + val * 35us (up to 9.440ms)
+
+Example:
+
+   ad7879@2c {
+   compatible = "adi,ad7879-1";
+   reg = <0x2c>;
+   interrupt-parent = <>;
+   interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+   resistance-plate-x = <120>;
+   touchscreen-max-pressure = <4096>;
+   first-conversion-delay = /bits/ 8 <3>;
+   acquisition-time = /bits/ 8 <1>;
+   median-filter-size = /bits/ 8 <2>;
+   averaging = /bits/ 8 <1>;
+   conversion-interval = /bits/ 8 <255>;
+   };
diff --git a/drivers/input/touchscreen/ad7879-i2c.c 
b/drivers/input/touchscreen/ad7879-i2c.c
index d66962c..08a2c9a 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -11,6 +11,7 @@
  #include 
  #include 
  #include 
+#include 

  #include "ad7879.h"

@@ -54,9 +55,50 @@ static const struct ad7879_bus_ops ad7879_i2c_bus_ops = {
.write  = ad7879_i2c_write,
  };

+static struct ad7879_platform_data *ad7879_parse_dt(struct device *dev)
+{
+   struct ad7879_platform_data *pdata;
+   struct device_node *np = dev->of_node;
+   int err;
+   u32 tmp;
+
+   if (!np)
+   return NULL;
+
+   pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+
+   if (!pdata)
+   return ERR_PTR(-ENOMEM);
+
+   err = of_property_read_u32(np, "resistance-plate-x", );
+   if (err) {
+   dev_err(dev, "failed to get resistance-plate-x property\n");
+   return ERR_PTR(err);
+   }
+   pdata->x_plate_ohms = (u16)tmp;
+
+   err = of_property_read_u32(np, "touchscreen-max-pressure", );
+   if (err) {
+   dev_err(dev, "failed to get touchscreen-max-pressure 
property\n");
+   return ERR_PTR(err);
+   }
+   pdata->pressure_min = 

Re: [PATCH 2/3] input: touchscreen: ad7879: fix default x/y axis assignment

[Michael Hennerich]

On 01/26/2016 04:04 AM, Stefan Agner wrote:

The measurements read from the controller which are temporary stored
in conversion_data, are interpreted wrong. The first measurement X+
contains the Y position, and the second measurement Y+ the X position
(see also Table 11 Register Table in the data sheet).

The problem is already known and a swap option has been introduced:
commit 6680884a4420 ("Input: ad7879 - add option to correct xy axis")

However, with that the meaning of the new boolean is inverted since
the underlying values are already swapped. With this change, a true
in swap_xy actually swaps the two axis.

Signed-off-by: Stefan Agner 
---
Hi Michael,

It seems that swap_xy is not used in any board which is in mainline,
hence swap_xy is always false. Therefore, up until now all boards
actually used swapped axis. However, I doubt that the blackfin boards
really have those axis swapped, it is probably more likely that the
userspace calibration took care of it.

However, if they are really swapped, we should set the swap_xy flag
to 1 for those board...

Do you happen to now what is the case with those boards?




Hi Stefan,

I would be hesitant to invert the default behaviour of the driver.
Too many people in the field already using it as it is.

A XY swap can have multiple reasons.

Lot's of small VGA/QVGA TFTs have the option to switch the scan 
direction from Landscape to Portrait. In addition you can also rotate 
and flip or mirror using VDMA options. So it really depends on the use 
case, how the touch panel is mounted to the screen or how it is wired.


Regards,
Michael


--
Stefan

  drivers/input/touchscreen/ad7879.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ad7879.c 
b/drivers/input/touchscreen/ad7879.c
index a73934b..e290e7b 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -94,8 +94,8 @@
  #define AD7879_TEMP_BIT   (1<<1)

  enum {
-   AD7879_SEQ_XPOS  = 0,
-   AD7879_SEQ_YPOS  = 1,
+   AD7879_SEQ_YPOS  = 0,
+   AD7879_SEQ_XPOS  = 1,
AD7879_SEQ_Z1= 2,
AD7879_SEQ_Z2= 3,
AD7879_NR_SENSE  = 4,




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

Re: [PATCH] backlight: adp8860: fix another uninitialized variable use

[Michael Hennerich]

On 11/30/2015 12:24 PM, Arnd Bergmann wrote:

A recent patch I did fixed two potential uses of uninitialized
variables in the adp8870 and adp8860 drivers. Unfortunately,
I missed another one:

drivers/video/backlight/adp8860_bl.c: In function 
'adp8860_bl_ambient_light_level_show':
drivers/video/backlight/adp8860_bl.c:570:11: warning: 'reg_val' may be used 
uninitialized in this function

This does the same change as before in one additional function,
and also changes the check for the return value in a way that
avoids another false positive warning with a similar message.

Signed-off-by: Arnd Bergmann 


Acked-by: Michael Hennerich 


Fixes: 6be3a5a9cd91 ("backlight: adp88x0: Fix uninitialized variable use")
---
Sorry for missing this third hunk the first time around.

diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c
index f0d4c0324580..510e559c060e 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -566,11 +566,13 @@ static ssize_t adp8860_bl_ambient_light_level_show(struct 
device *dev,

mutex_lock(>lock);
error = adp8860_read(data->client, ADP8860_PH1LEVL, _val);
-   ret_val = reg_val;
-   error |= adp8860_read(data->client, ADP8860_PH1LEVH, _val);
+   if (!error) {
+   ret_val = reg_val;
+   error = adp8860_read(data->client, ADP8860_PH1LEVH, _val);
+   }
mutex_unlock(>lock);

-   if (error < 0)
+   if (error)
return error;

/* Return 13-bit conversion value for the first light sensor */




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] backlight: adp8860: fix another uninitialized variable use

[Michael Hennerich]

On 11/30/2015 12:24 PM, Arnd Bergmann wrote:

A recent patch I did fixed two potential uses of uninitialized
variables in the adp8870 and adp8860 drivers. Unfortunately,
I missed another one:

drivers/video/backlight/adp8860_bl.c: In function 
'adp8860_bl_ambient_light_level_show':
drivers/video/backlight/adp8860_bl.c:570:11: warning: 'reg_val' may be used 
uninitialized in this function

This does the same change as before in one additional function,
and also changes the check for the return value in a way that
avoids another false positive warning with a similar message.

Signed-off-by: Arnd Bergmann <a...@arndb.de>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


Fixes: 6be3a5a9cd91 ("backlight: adp88x0: Fix uninitialized variable use")
---
Sorry for missing this third hunk the first time around.

diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c
index f0d4c0324580..510e559c060e 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -566,11 +566,13 @@ static ssize_t adp8860_bl_ambient_light_level_show(struct 
device *dev,

mutex_lock(>lock);
error = adp8860_read(data->client, ADP8860_PH1LEVL, _val);
-   ret_val = reg_val;
-   error |= adp8860_read(data->client, ADP8860_PH1LEVH, _val);
+   if (!error) {
+   ret_val = reg_val;
+   error = adp8860_read(data->client, ADP8860_PH1LEVH, _val);
+   }
mutex_unlock(>lock);

-   if (error < 0)
+   if (error)
return error;

/* Return 13-bit conversion value for the first light sensor */




--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] backlight: adp88x0: fix uninitialized variable use

[Michael Hennerich]

On 11/23/2015 02:44 PM, Arnd Bergmann wrote:

gcc correctly warns about both the adp8860 and adp8870 backlight
drivers using an uninitialized variable in their error handling
path:

drivers/video/backlight/adp8870_bl.c: In function 
'adp8870_bl_ambient_light_zone_store':
drivers/video/backlight/adp8870_bl.c:811:11: warning: 'reg_val' may be used 
uninitialized in this function

This changes the code to only write back the data if it was
correctly read to start with.

As a side-note, the drivers are mostly identical, so I think they
should really be merged into one file to avoid having to fix every
bug twice.

Signed-off-by: Arnd Bergmann 


Acked-by: Michael Hennerich 



diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c
index 98ffe71e8af2..f0d4c0324580 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -621,10 +621,12 @@ static ssize_t adp8860_bl_ambient_light_zone_store(struct 
device *dev,

/* Set user supplied ambient light zone */
mutex_lock(>lock);
-   adp8860_read(data->client, ADP8860_CFGR, _val);
-   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
-   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
-   adp8860_write(data->client, ADP8860_CFGR, reg_val);
+   ret = adp8860_read(data->client, ADP8860_CFGR, _val);
+   if (!ret) {
+   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
+   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
+   adp8860_write(data->client, ADP8860_CFGR, reg_val);
+   }
mutex_unlock(>lock);
}

diff --git a/drivers/video/backlight/adp8870_bl.c 
b/drivers/video/backlight/adp8870_bl.c
index 9d738352d7d4..21acac90fd77 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -807,10 +807,12 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct 
device *dev,

/* Set user supplied ambient light zone */
mutex_lock(>lock);
-   adp8870_read(data->client, ADP8870_CFGR, _val);
-   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
-   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
-   adp8870_write(data->client, ADP8870_CFGR, reg_val);
+   ret = adp8870_read(data->client, ADP8870_CFGR, _val);
+   if (!ret) {
+   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
+   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
+   adp8870_write(data->client, ADP8870_CFGR, reg_val);
+   }
mutex_unlock(>lock);
}





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] backlight: adp88x0: fix uninitialized variable use

[Michael Hennerich]

On 11/23/2015 02:44 PM, Arnd Bergmann wrote:

gcc correctly warns about both the adp8860 and adp8870 backlight
drivers using an uninitialized variable in their error handling
path:

drivers/video/backlight/adp8870_bl.c: In function 
'adp8870_bl_ambient_light_zone_store':
drivers/video/backlight/adp8870_bl.c:811:11: warning: 'reg_val' may be used 
uninitialized in this function

This changes the code to only write back the data if it was
correctly read to start with.

As a side-note, the drivers are mostly identical, so I think they
should really be merged into one file to avoid having to fix every
bug twice.

Signed-off-by: Arnd Bergmann <a...@arndb.de>


Acked-by: Michael Hennerich <michael.henner...@analog.com>



diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c
index 98ffe71e8af2..f0d4c0324580 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -621,10 +621,12 @@ static ssize_t adp8860_bl_ambient_light_zone_store(struct 
device *dev,

/* Set user supplied ambient light zone */
mutex_lock(>lock);
-   adp8860_read(data->client, ADP8860_CFGR, _val);
-   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
-   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
-   adp8860_write(data->client, ADP8860_CFGR, reg_val);
+   ret = adp8860_read(data->client, ADP8860_CFGR, _val);
+   if (!ret) {
+   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
+   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
+   adp8860_write(data->client, ADP8860_CFGR, reg_val);
+   }
mutex_unlock(>lock);
}

diff --git a/drivers/video/backlight/adp8870_bl.c 
b/drivers/video/backlight/adp8870_bl.c
index 9d738352d7d4..21acac90fd77 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -807,10 +807,12 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct 
device *dev,

/* Set user supplied ambient light zone */
mutex_lock(>lock);
-   adp8870_read(data->client, ADP8870_CFGR, _val);
-   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
-   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
-   adp8870_write(data->client, ADP8870_CFGR, reg_val);
+   ret = adp8870_read(data->client, ADP8870_CFGR, _val);
+   if (!ret) {
+   reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
+   reg_val |= (val - 1) << CFGR_BLV_SHIFT;
+   adp8870_write(data->client, ADP8870_CFGR, reg_val);
+   }
mutex_unlock(>lock);
}





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] misc: ad525x_dpot: Fix the enabling of the "otpXen" attributes

[Michael Hennerich]

On 11/18/2015 05:16 PM, Dan Bogdan Nechita wrote:

Currently writing the attributes with "echo" will result in comparing:
"enabled\n" with "enabled\0" and attribute is always set to false.

Use the sysfs_streq() instead because it treats both NUL and
new-line-then-NUL as equivalent string terminations.

Signed-off-by: Dan Bogdan Nechita 


Acked-by: Michael Hennerich 


---
  drivers/misc/ad525x_dpot.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index 15e8807..4230e6a 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -452,7 +452,7 @@ static ssize_t sysfs_set_reg(struct device *dev,
int err;

if (reg & DPOT_ADDR_OTP_EN) {
-   if (!strncmp(buf, "enabled", sizeof("enabled")))
+   if (sysfs_streq(buf, "enabled"))
set_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);
else
clear_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);




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

Re: [PATCH] misc: ad525x_dpot: Fix the enabling of the "otpXen" attributes

[Michael Hennerich]

On 11/18/2015 05:16 PM, Dan Bogdan Nechita wrote:

Currently writing the attributes with "echo" will result in comparing:
"enabled\n" with "enabled\0" and attribute is always set to false.

Use the sysfs_streq() instead because it treats both NUL and
new-line-then-NUL as equivalent string terminations.

Signed-off-by: Dan Bogdan Nechita <dan.bogdan.nech...@gmail.com>


Acked-by: Michael Hennerich <michael.henner...@analog.com>


---
  drivers/misc/ad525x_dpot.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index 15e8807..4230e6a 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -452,7 +452,7 @@ static ssize_t sysfs_set_reg(struct device *dev,
int err;

if (reg & DPOT_ADDR_OTP_EN) {
-   if (!strncmp(buf, "enabled", sizeof("enabled")))
+   if (sysfs_streq(buf, "enabled"))
set_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);
else
clear_bit(DPOT_RDAC_MASK & reg, data->otp_en_mask);




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

Re: [PATCH/RFC 09/51] leds: adp5520: Remove work queue

[Michael Hennerich]

On 07/17/2015 10:46 AM, Jacek Anaszewski wrote:

From: Andrew Lunn 

Now the core implements the work queue, remove it from the driver.

Signed-off-by: Andrew Lunn 


Acked-by: Michael Hennerich 


Cc: Michael Hennerich 
---
  drivers/leds/leds-adp5520.c |   22 +++---
  1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-adp5520.c b/drivers/leds/leds-adp5520.c
index 07e66ca..6378150 100644
--- a/drivers/leds/leds-adp5520.c
+++ b/drivers/leds/leds-adp5520.c
@@ -17,34 +17,24 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  
  struct adp5520_led {

struct led_classdev cdev;
-   struct work_struct  work;
struct device   *master;
-   enum led_brightness new_brightness;
int id;
int flags;
  };
  
-static void adp5520_led_work(struct work_struct *work)

-{
-   struct adp5520_led *led = container_of(work, struct adp5520_led, work);
-   adp5520_write(led->master, ADP5520_LED1_CURRENT + led->id - 1,
-led->new_brightness >> 2);
-}
-
  static void adp5520_led_set(struct led_classdev *led_cdev,
   enum led_brightness value)
  {
struct adp5520_led *led;
  
  	led = container_of(led_cdev, struct adp5520_led, cdev);

-   led->new_brightness = value;
-   schedule_work(>work);
+   adp5520_write(led->master, ADP5520_LED1_CURRENT + led->id - 1,
+value >> 2);
  }
  
  static int adp5520_led_setup(struct adp5520_led *led)

@@ -146,9 +136,6 @@ static int adp5520_led_probe(struct platform_device *pdev)
led_dat->id = led_dat->flags & ADP5520_FLAG_LED_MASK;
  
  		led_dat->master = pdev->dev.parent;

-   led_dat->new_brightness = LED_OFF;
-
-   INIT_WORK(_dat->work, adp5520_led_work);
  
  		ret = led_classdev_register(led_dat->master, _dat->cdev);

if (ret) {
@@ -170,10 +157,8 @@ static int adp5520_led_probe(struct platform_device *pdev)
  
  err:

if (i > 0) {
-   for (i = i - 1; i >= 0; i--) {
+   for (i = i - 1; i >= 0; i--)
led_classdev_unregister([i].cdev);
-   cancel_work_sync([i].work);
-   }
}
  
  	return ret;

@@ -192,7 +177,6 @@ static int adp5520_led_remove(struct platform_device *pdev)
  
  	for (i = 0; i < pdata->num_leds; i++) {

led_classdev_unregister([i].cdev);
-   cancel_work_sync([i].work);
}
  
  	return 0;



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

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

Re: [PATCH/RFC 09/51] leds: adp5520: Remove work queue

[Michael Hennerich]

On 07/17/2015 10:46 AM, Jacek Anaszewski wrote:

From: Andrew Lunn and...@lunn.ch

Now the core implements the work queue, remove it from the driver.

Signed-off-by: Andrew Lunn and...@lunn.ch


Acked-by: Michael Hennerich michael.henner...@analog.com


Cc: Michael Hennerich michael.henner...@analog.com
---
  drivers/leds/leds-adp5520.c |   22 +++---
  1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-adp5520.c b/drivers/leds/leds-adp5520.c
index 07e66ca..6378150 100644
--- a/drivers/leds/leds-adp5520.c
+++ b/drivers/leds/leds-adp5520.c
@@ -17,34 +17,24 @@
  #include linux/kernel.h
  #include linux/platform_device.h
  #include linux/leds.h
-#include linux/workqueue.h
  #include linux/mfd/adp5520.h
  #include linux/slab.h
  
  struct adp5520_led {

struct led_classdev cdev;
-   struct work_struct  work;
struct device   *master;
-   enum led_brightness new_brightness;
int id;
int flags;
  };
  
-static void adp5520_led_work(struct work_struct *work)

-{
-   struct adp5520_led *led = container_of(work, struct adp5520_led, work);
-   adp5520_write(led-master, ADP5520_LED1_CURRENT + led-id - 1,
-led-new_brightness  2);
-}
-
  static void adp5520_led_set(struct led_classdev *led_cdev,
   enum led_brightness value)
  {
struct adp5520_led *led;
  
  	led = container_of(led_cdev, struct adp5520_led, cdev);

-   led-new_brightness = value;
-   schedule_work(led-work);
+   adp5520_write(led-master, ADP5520_LED1_CURRENT + led-id - 1,
+value  2);
  }
  
  static int adp5520_led_setup(struct adp5520_led *led)

@@ -146,9 +136,6 @@ static int adp5520_led_probe(struct platform_device *pdev)
led_dat-id = led_dat-flags  ADP5520_FLAG_LED_MASK;
  
  		led_dat-master = pdev-dev.parent;

-   led_dat-new_brightness = LED_OFF;
-
-   INIT_WORK(led_dat-work, adp5520_led_work);
  
  		ret = led_classdev_register(led_dat-master, led_dat-cdev);

if (ret) {
@@ -170,10 +157,8 @@ static int adp5520_led_probe(struct platform_device *pdev)
  
  err:

if (i  0) {
-   for (i = i - 1; i = 0; i--) {
+   for (i = i - 1; i = 0; i--)
led_classdev_unregister(led[i].cdev);
-   cancel_work_sync(led[i].work);
-   }
}
  
  	return ret;

@@ -192,7 +177,6 @@ static int adp5520_led_remove(struct platform_device *pdev)
  
  	for (i = 0; i  pdata-num_leds; i++) {

led_classdev_unregister(led[i].cdev);
-   cancel_work_sync(led[i].work);
}
  
  	return 0;



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

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

Re: [PATCH] Input: adp5588-keys: cancel workqueue in failure path

[Michael Hennerich]

On 10/07/2014 09:30 AM, Pramod Gurav wrote:

This change introduces a label to call cancel_delayed_work_sync in
failure path.

Cc: Michael Hennerich 
Cc: Dmitry Torokhov 
Cc: linux-in...@vger.kernel.org
Signed-off-by: Pramod Gurav 


The interrupt triggers the work queue. adp5588_setup() enable the HW 
interrupt.

Only if the device was configured before, without being resetb -
The earliest point in time where a problem could happen - is after the 
request_irq().


Anyways patch below doesn't harm and fixes a potential problem.

Acked-by: Michael Hennerich 


---
  drivers/input/keyboard/adp5588-keys.c |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c 
b/drivers/input/keyboard/adp5588-keys.c
index 5ef7fcf..b494062 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -559,7 +559,7 @@ static int adp5588_probe(struct i2c_client *client,
error = input_register_device(input);
if (error) {
dev_err(>dev, "unable to register input device\n");
-   goto err_free_mem;
+   goto err_delayed_work;
}
  
  	error = request_irq(client->irq, adp5588_irq,

@@ -592,6 +592,8 @@ static int adp5588_probe(struct i2c_client *client,
   err_unreg_dev:
input_unregister_device(input);
input = NULL;
+ err_delayed_work:
+   cancel_delayed_work_sync(>work);
   err_free_mem:
input_free_device(input);
kfree(kpad);



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

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

Re: [PATCH] Input: adp5588-keys: cancel workqueue in failure path

[Michael Hennerich]

On 10/07/2014 09:30 AM, Pramod Gurav wrote:

This change introduces a label to call cancel_delayed_work_sync in
failure path.

Cc: Michael Hennerich michael.henner...@analog.com
Cc: Dmitry Torokhov dmitry.torok...@gmail.com
Cc: linux-in...@vger.kernel.org
Signed-off-by: Pramod Gurav pramod.gu...@smartplayin.com


The interrupt triggers the work queue. adp5588_setup() enable the HW 
interrupt.

Only if the device was configured before, without being resetb -
The earliest point in time where a problem could happen - is after the 
request_irq().


Anyways patch below doesn't harm and fixes a potential problem.

Acked-by: Michael Hennerich michael.henner...@analog.com


---
  drivers/input/keyboard/adp5588-keys.c |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c 
b/drivers/input/keyboard/adp5588-keys.c
index 5ef7fcf..b494062 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -559,7 +559,7 @@ static int adp5588_probe(struct i2c_client *client,
error = input_register_device(input);
if (error) {
dev_err(client-dev, unable to register input device\n);
-   goto err_free_mem;
+   goto err_delayed_work;
}
  
  	error = request_irq(client-irq, adp5588_irq,

@@ -592,6 +592,8 @@ static int adp5588_probe(struct i2c_client *client,
   err_unreg_dev:
input_unregister_device(input);
input = NULL;
+ err_delayed_work:
+   cancel_delayed_work_sync(kpad-work);
   err_free_mem:
input_free_device(input);
kfree(kpad);



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif

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

Re: [PATCH 03/12] backlight: adp8870: Remove unnecessary OOM messages

[Michael Hennerich]

On 02/06/2014 09:17 AM, Jingoo Han wrote:

The site-specific OOM messages are unnecessary, because they
duplicate the MM subsystem generic OOM message.

Signed-off-by: Jingoo Han 

Acked-by: Michael Hennerich 

---
  drivers/video/backlight/adp8870_bl.c |4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/video/backlight/adp8870_bl.c 
b/drivers/video/backlight/adp8870_bl.c
index 6370720..251af4d 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -246,10 +246,8 @@ static int adp8870_led_probe(struct i2c_client *client)
  
  	led = devm_kzalloc(>dev, pdata->num_leds * sizeof(*led),

GFP_KERNEL);
-   if (led == NULL) {
-   dev_err(>dev, "failed to alloc memory\n");
+   if (led == NULL)
return -ENOMEM;
-   }
  
  	ret = adp8870_write(client, ADP8870_ISCLAW, pdata->led_fade_law);

if (ret)



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH 03/12] backlight: adp8870: Remove unnecessary OOM messages

[Michael Hennerich]

On 02/06/2014 09:17 AM, Jingoo Han wrote:

The site-specific OOM messages are unnecessary, because they
duplicate the MM subsystem generic OOM message.

Signed-off-by: Jingoo Han jg1@samsung.com

Acked-by: Michael Hennerich michael.henner...@analog.com

---
  drivers/video/backlight/adp8870_bl.c |4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/video/backlight/adp8870_bl.c 
b/drivers/video/backlight/adp8870_bl.c
index 6370720..251af4d 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -246,10 +246,8 @@ static int adp8870_led_probe(struct i2c_client *client)
  
  	led = devm_kzalloc(client-dev, pdata-num_leds * sizeof(*led),

GFP_KERNEL);
-   if (led == NULL) {
-   dev_err(client-dev, failed to alloc memory\n);
+   if (led == NULL)
return -ENOMEM;
-   }
  
  	ret = adp8870_write(client, ADP8870_ISCLAW, pdata-led_fade_law);

if (ret)



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH 4/5] drivers: misc: Mark functions as static in ad525x_dpot.c

[Michael Hennerich]

On 12/13/2013 07:57 AM, Rashika Kheria wrote:

This patch marks the function ad_dpot_add_files() and
ad_dpot_remove_files() as static in ad525x_dpot.c because they are not
used outside this file.

Thus, it also eliminates the following warnings in ad525x_dpot.c:
drivers/misc/ad525x_dpot.c:644:5: warning: no previous prototype for 
‘ad_dpot_add_files’ [-Wmissing-prototypes]
drivers/misc/ad525x_dpot.c:669:13: warning: no previous prototype for 
‘ad_dpot_remove_files’ [-Wmissing-prototypes]

Signed-off-by: Rashika Kheria 

Acked-by: Michael Hennerich 

---
  drivers/misc/ad525x_dpot.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index 0daadcf..d3eee11 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -641,7 +641,7 @@ static const struct attribute_group ad525x_group_commands = 
{
.attrs = ad525x_attributes_commands,
  };
  
-int ad_dpot_add_files(struct device *dev,

+static int ad_dpot_add_files(struct device *dev,
unsigned features, unsigned rdac)
  {
int err = sysfs_create_file(>kobj,
@@ -666,7 +666,7 @@ int ad_dpot_add_files(struct device *dev,
return err;
  }
  
-inline void ad_dpot_remove_files(struct device *dev,

+static inline void ad_dpot_remove_files(struct device *dev,
unsigned features, unsigned rdac)
  {
sysfs_remove_file(>kobj,



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH 4/5] drivers: misc: Mark functions as static in ad525x_dpot.c

[Michael Hennerich]

On 12/13/2013 07:57 AM, Rashika Kheria wrote:

This patch marks the function ad_dpot_add_files() and
ad_dpot_remove_files() as static in ad525x_dpot.c because they are not
used outside this file.

Thus, it also eliminates the following warnings in ad525x_dpot.c:
drivers/misc/ad525x_dpot.c:644:5: warning: no previous prototype for 
‘ad_dpot_add_files’ [-Wmissing-prototypes]
drivers/misc/ad525x_dpot.c:669:13: warning: no previous prototype for 
‘ad_dpot_remove_files’ [-Wmissing-prototypes]

Signed-off-by: Rashika Kheria rashika.khe...@gmail.com

Acked-by: Michael Hennerich michael.henner...@analog.com

---
  drivers/misc/ad525x_dpot.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/ad525x_dpot.c b/drivers/misc/ad525x_dpot.c
index 0daadcf..d3eee11 100644
--- a/drivers/misc/ad525x_dpot.c
+++ b/drivers/misc/ad525x_dpot.c
@@ -641,7 +641,7 @@ static const struct attribute_group ad525x_group_commands = 
{
.attrs = ad525x_attributes_commands,
  };
  
-int ad_dpot_add_files(struct device *dev,

+static int ad_dpot_add_files(struct device *dev,
unsigned features, unsigned rdac)
  {
int err = sysfs_create_file(dev-kobj,
@@ -666,7 +666,7 @@ int ad_dpot_add_files(struct device *dev,
return err;
  }
  
-inline void ad_dpot_remove_files(struct device *dev,

+static inline void ad_dpot_remove_files(struct device *dev,
unsigned features, unsigned rdac)
  {
sysfs_remove_file(dev-kobj,



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH] backlight: adp8860/adp8870: Fix resume

[Michael Hennerich]

On 02/19/2013 02:02 PM, Lars-Peter Clausen wrote:

Clearing the NSTBY bit in the control register also automatically clears the
BLEN bit. So we need to make sure to set it again during resume, otherwise the
backlight will stay off.

Cc: sta...@vger.kernel.org
Signed-off-by: Lars-Peter Clausen 

Acked-by: Michael Hennerich 

---
  drivers/video/backlight/adp8860_bl.c | 2 +-
  drivers/video/backlight/adp8870_bl.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c
index 6bb72c0..a77c9ca 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -783,7 +783,7 @@ static int adp8860_i2c_suspend(struct i2c_client *client, 
pm_message_t message)
  
  static int adp8860_i2c_resume(struct i2c_client *client)

  {
-   adp8860_set_bits(client, ADP8860_MDCR, NSTBY);
+   adp8860_set_bits(client, ADP8860_MDCR, NSTBY | BLEN);
  
  	return 0;

  }
diff --git a/drivers/video/backlight/adp8870_bl.c 
b/drivers/video/backlight/adp8870_bl.c
index 63c882b..712c25a 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -957,7 +957,7 @@ static int adp8870_i2c_suspend(struct i2c_client *client, 
pm_message_t message)
  
  static int adp8870_i2c_resume(struct i2c_client *client)

  {
-   adp8870_set_bits(client, ADP8870_MDCR, NSTBY);
+   adp8870_set_bits(client, ADP8870_MDCR, NSTBY | BLEN);
  
  	return 0;

  }



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH] mfd:adp5520: Restore mode bits on resume

[Michael Hennerich]

On 02/19/2013 11:51 AM, Lars-Peter Clausen wrote:

The adp5520 unfortunately also clears the BL_EN bit when the nSTNDBY bit is
cleared. So we need to make sure to restore it during resume if it was set
before suspend.

Cc: sta...@vger.kernel.org
Signed-off-by: Lars-Peter Clausen 

Acked-by: Michael Hennerich 

---
  drivers/mfd/adp5520.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
index 210dd03..6b40e0c 100644
--- a/drivers/mfd/adp5520.c
+++ b/drivers/mfd/adp5520.c
@@ -36,6 +36,7 @@ struct adp5520_chip {
struct blocking_notifier_head notifier_list;
int irq;
unsigned long id;
+   uint8_t mode;
  };
  
  static int __adp5520_read(struct i2c_client *client,

@@ -326,7 +327,10 @@ static int adp5520_suspend(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct adp5520_chip *chip = dev_get_drvdata(>dev);
  
-	adp5520_clr_bits(chip->dev, ADP5520_MODE_STATUS, ADP5520_nSTNBY);

+   adp5520_read(chip->dev, ADP5520_MODE_STATUS, >mode);
+   /* All other bits are W1C */
+   chip->mode &= ADP5520_BL_EN | ADP5520_DIM_EN | ADP5520_nSTNBY;
+   adp5520_write(chip->dev, ADP5520_MODE_STATUS, 0);
return 0;
  }
  
@@ -335,7 +339,7 @@ static int adp5520_resume(struct device *dev)

struct i2c_client *client = to_i2c_client(dev);
struct adp5520_chip *chip = dev_get_drvdata(>dev);
  
-	adp5520_set_bits(chip->dev, ADP5520_MODE_STATUS, ADP5520_nSTNBY);

+   adp5520_write(chip->dev, ADP5520_MODE_STATUS, chip->mode);
return 0;
  }
  #endif



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH] backlight: adp8860/adp8870: Fix resume

[Michael Hennerich]

On 02/19/2013 02:02 PM, Lars-Peter Clausen wrote:

Clearing the NSTBY bit in the control register also automatically clears the
BLEN bit. So we need to make sure to set it again during resume, otherwise the
backlight will stay off.

Cc: sta...@vger.kernel.org
Signed-off-by: Lars-Peter Clausen l...@metafoo.de

Acked-by: Michael Hennerich michael.henner...@analog.com

---
  drivers/video/backlight/adp8860_bl.c | 2 +-
  drivers/video/backlight/adp8870_bl.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/adp8860_bl.c 
b/drivers/video/backlight/adp8860_bl.c
index 6bb72c0..a77c9ca 100644
--- a/drivers/video/backlight/adp8860_bl.c
+++ b/drivers/video/backlight/adp8860_bl.c
@@ -783,7 +783,7 @@ static int adp8860_i2c_suspend(struct i2c_client *client, 
pm_message_t message)
  
  static int adp8860_i2c_resume(struct i2c_client *client)

  {
-   adp8860_set_bits(client, ADP8860_MDCR, NSTBY);
+   adp8860_set_bits(client, ADP8860_MDCR, NSTBY | BLEN);
  
  	return 0;

  }
diff --git a/drivers/video/backlight/adp8870_bl.c 
b/drivers/video/backlight/adp8870_bl.c
index 63c882b..712c25a 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -957,7 +957,7 @@ static int adp8870_i2c_suspend(struct i2c_client *client, 
pm_message_t message)
  
  static int adp8870_i2c_resume(struct i2c_client *client)

  {
-   adp8870_set_bits(client, ADP8870_MDCR, NSTBY);
+   adp8870_set_bits(client, ADP8870_MDCR, NSTBY | BLEN);
  
  	return 0;

  }



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH] mfd:adp5520: Restore mode bits on resume

[Michael Hennerich]

On 02/19/2013 11:51 AM, Lars-Peter Clausen wrote:

The adp5520 unfortunately also clears the BL_EN bit when the nSTNDBY bit is
cleared. So we need to make sure to restore it during resume if it was set
before suspend.

Cc: sta...@vger.kernel.org
Signed-off-by: Lars-Peter Clausen l...@metafoo.de

Acked-by: Michael Hennerich michael.henner...@analog.com

---
  drivers/mfd/adp5520.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
index 210dd03..6b40e0c 100644
--- a/drivers/mfd/adp5520.c
+++ b/drivers/mfd/adp5520.c
@@ -36,6 +36,7 @@ struct adp5520_chip {
struct blocking_notifier_head notifier_list;
int irq;
unsigned long id;
+   uint8_t mode;
  };
  
  static int __adp5520_read(struct i2c_client *client,

@@ -326,7 +327,10 @@ static int adp5520_suspend(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct adp5520_chip *chip = dev_get_drvdata(client-dev);
  
-	adp5520_clr_bits(chip-dev, ADP5520_MODE_STATUS, ADP5520_nSTNBY);

+   adp5520_read(chip-dev, ADP5520_MODE_STATUS, chip-mode);
+   /* All other bits are W1C */
+   chip-mode = ADP5520_BL_EN | ADP5520_DIM_EN | ADP5520_nSTNBY;
+   adp5520_write(chip-dev, ADP5520_MODE_STATUS, 0);
return 0;
  }
  
@@ -335,7 +339,7 @@ static int adp5520_resume(struct device *dev)

struct i2c_client *client = to_i2c_client(dev);
struct adp5520_chip *chip = dev_get_drvdata(client-dev);
  
-	adp5520_set_bits(chip-dev, ADP5520_MODE_STATUS, ADP5520_nSTNBY);

+   adp5520_write(chip-dev, ADP5520_MODE_STATUS, chip-mode);
return 0;
  }
  #endif



--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH] mfd: use module_i2c_driver to simplify the code

[Michael Hennerich]

On 10/08/2012 04:09 PM, Wei Yongjun wrote:

From: Wei Yongjun 

Use the module_i2c_driver() macro to make the code smaller
and a bit simpler.

dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun 

Acked-by: Michael Hennerich 

---
  drivers/mfd/adp5520.c | 12 +---
  1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
index ea8b947..94e7a6d 100644
--- a/drivers/mfd/adp5520.c
+++ b/drivers/mfd/adp5520.c
@@ -360,17 +360,7 @@ static struct i2c_driver adp5520_driver = {
.id_table   = adp5520_id,
  };
  
-static int __init adp5520_init(void)

-{
-   return i2c_add_driver(_driver);
-}
-module_init(adp5520_init);
-
-static void __exit adp5520_exit(void)
-{
-   i2c_del_driver(_driver);
-}
-module_exit(adp5520_exit);
+module_i2c_driver(adp5520_driver);
  
  MODULE_AUTHOR("Michael Hennerich ");

  MODULE_DESCRIPTION("ADP5520(01) PMIC-MFD Driver");





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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

Re: [PATCH] mfd: use module_i2c_driver to simplify the code

[Michael Hennerich]

On 10/08/2012 04:09 PM, Wei Yongjun wrote:

From: Wei Yongjun yongjun_...@trendmicro.com.cn

Use the module_i2c_driver() macro to make the code smaller
and a bit simpler.

dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn

Acked-by: Michael Hennerich michael.henner...@analog.com

---
  drivers/mfd/adp5520.c | 12 +---
  1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
index ea8b947..94e7a6d 100644
--- a/drivers/mfd/adp5520.c
+++ b/drivers/mfd/adp5520.c
@@ -360,17 +360,7 @@ static struct i2c_driver adp5520_driver = {
.id_table   = adp5520_id,
  };
  
-static int __init adp5520_init(void)

-{
-   return i2c_add_driver(adp5520_driver);
-}
-module_init(adp5520_init);
-
-static void __exit adp5520_exit(void)
-{
-   i2c_del_driver(adp5520_driver);
-}
-module_exit(adp5520_exit);
+module_i2c_driver(adp5520_driver);
  
  MODULE_AUTHOR(Michael Hennerich henner...@blackfin.uclinux.org);

  MODULE_DESCRIPTION(ADP5520(01) PMIC-MFD Driver);





--
Greetings,
Michael

--
Analog Devices GmbH  Wilhelm-Wagenfeld-Str. 6  80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif


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