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

2017-04-11 Thread 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

2017-04-11 Thread 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

2017-04-11 Thread 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

2017-04-11 Thread 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

2017-04-10 Thread Linus Walleij
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!

> +#include 
> +#include 
> +#include 
> +#include 

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


> +#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]

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

Yours,
Linus Walleij


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

2017-04-10 Thread Linus Walleij
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!

> +#include 
> +#include 
> +#include 
> +#include 

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


> +#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]

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

Yours,
Linus Walleij


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

2017-04-06 Thread Peter Rosin
On 2017-04-06 13:31, Michael Hennerich wrote:
> 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!

s/you/your/

> 
> 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: 

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

2017-04-06 Thread Peter Rosin
On 2017-04-06 13:31, Michael Hennerich wrote:
> 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!

s/you/your/

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

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

2017-04-06 Thread 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 

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

2017-04-06 Thread 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;
+};
+
+struct ltc4306 

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

2017-04-06 Thread Peter Rosin
Hi Michael,

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

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

Thanks for you patience!

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?

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

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

2017-04-06 Thread Peter Rosin
Hi Michael,

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

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

Thanks for you patience!

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?

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

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

2017-04-05 Thread michael.hennerich
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
+ 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;
+};
+
+struct ltc4306 {
+   struct regmap *regmap;
+   struct gpio_chip gpiochip;
+   const struct chip_desc *chip;
+};
+
+static const struct chip_desc chips[] = {
+   [ltc_4305] = {
+   .nchans = LTC4305_MAX_NCHANS,
+   },
+   [ltc_4306] = {
+   .nchans = LTC4306_MAX_NCHANS,
+   .num_gpios = 2,
+   },
+};
+
+static bool ltc4306_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+   

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

2017-04-05 Thread michael.hennerich
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
+ 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;
+};
+
+struct ltc4306 {
+   struct regmap *regmap;
+   struct gpio_chip gpiochip;
+   const struct chip_desc *chip;
+};
+
+static const struct chip_desc chips[] = {
+   [ltc_4305] = {
+   .nchans = LTC4305_MAX_NCHANS,
+   },
+   [ltc_4306] = {
+   .nchans = LTC4306_MAX_NCHANS,
+   .num_gpios = 2,
+   },
+};
+
+static bool ltc4306_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+   return (reg == LTC_REG_CONFIG) ? true : false;
+}
+
+static const struct regmap_config ltc4306_regmap_config = {
+