[PATCH v4 0/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[PATCH v4 0/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[Robert Shearman]

From: Robert Shearman <robert.shearman@att.com>

The behaviour, by default, to not deselect after each transfer is
unsafe when there is a device with an address that conflicts with
another device on another pca954x mux on the same parent bus, and it
may not be convenient to use devicetree to set the deselect mux,
e.g. when running on x86_64 when ACPI is used to discover most of the
device hierarchy.

Therefore, provide the ability to set the idle state behaviour using a
new sysfs file, idle_state as a complement to the method of
instantiating the device via sysfs.

Changes in v4:
 - also remove variables that are redundant as a result of platform
   data removal
 - improve ABI file description field
Changes in v3:
 - remove use of platform data to simplify the idle state by not
   having per-channel idle states.
 - correct some comments
 - fix a style issue regarding use of READ_ONCE
Changes in v2:
 - change from exposing deselect mask to idle state with more options
   and less implementation specific
 - fix style issues
 - don't fail the create of the mux device if the device attribute
   file in sysfs failed to create

Robert Shearman (3):
  i2c: mux: pca954x: remove support for unused platform data
  i2c: mux: pca9541: remove support for unused platform data
  i2c: mux: pca954x: allow management of device idle state via sysfs

 .../ABI/testing/sysfs-bus-i2c-devices-pca954x |  20 ++++
 drivers/i2c/muxes/i2c-mux-pca9541.c           |   8 +-
 drivers/i2c/muxes/i2c-mux-pca954x.c           | 106 +++++++++++++-----
 include/linux/platform_data/pca954x.h         |  48 --------
 4 files changed, 100 insertions(+), 82 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x
 delete mode 100644 include/linux/platform_data/pca954x.h

-- 
2.20.1

[PATCH v4 1/3] i2c: mux: pca954x: remove support for unused platform data

[Robert Shearman]

From: Robert Shearman <robert.shearman@att.com>

There are no in-tree users of the pca954x platform data and the
per-channel deselect configuration complicates efforts to export the
configuration to user-space in a way that could be applied to other
muxes. Therefore, remove support for the pca954x platform data.

Signed-off-by: Robert Shearman <robert.shearman@att.com>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index bfabf985e830..e32fef560684 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -46,7 +46,6 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
-#include <linux/platform_data/pca954x.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -348,14 +347,13 @@ static int pca954x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adap = client->adapter;
-	struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct device *dev = &client->dev;
 	struct device_node *np = dev->of_node;
 	bool idle_disconnect_dt;
 	struct gpio_desc *gpio;
-	int num, force, class;
 	struct i2c_mux_core *muxc;
 	struct pca954x *data;
+	int num;
 	int ret;
 
 	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
@@ -422,24 +420,9 @@ static int pca954x_probe(struct i2c_client *client,
 
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
-		bool idle_disconnect_pd = false;
-
-		force = 0;			  /* dynamic adap number */
-		class = 0;			  /* no class by default */
-		if (pdata) {
-			if (num < pdata->num_modes) {
-				/* force static number */
-				force = pdata->modes[num].adap_id;
-				class = pdata->modes[num].class;
-			} else
-				/* discard unconfigured channels */
-				break;
-			idle_disconnect_pd = pdata->modes[num].deselect_on_exit;
-		}
-		data->deselect |= (idle_disconnect_pd ||
-				   idle_disconnect_dt) << num;
+		data->deselect |= idle_disconnect_dt << num;
 
-		ret = i2c_mux_add_adapter(muxc, force, num, class);
+		ret = i2c_mux_add_adapter(muxc, 0, num, 0);
 		if (ret)
 			goto fail_cleanup;
 	}
-- 
2.20.1

[PATCH v4 2/3] i2c: mux: pca9541: remove support for unused platform data

[Robert Shearman]

From: Robert Shearman <robert.shearman@att.com>

There are no in-tree users of the platform data, so remove it to
simplify the code slightly.

Remove the now unused pca954x.h platform data header.

Signed-off-by: Robert Shearman <robert.shearman@att.com>
---
 drivers/i2c/muxes/i2c-mux-pca9541.c   |  8 +----
 include/linux/platform_data/pca954x.h | 48 ---------------------------
 2 files changed, 1 insertion(+), 55 deletions(-)
 delete mode 100644 include/linux/platform_data/pca954x.h

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 9e75d6b9140b..50e1fb4aedf5 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -22,7 +22,6 @@
 #include <linux/i2c-mux.h>
 #include <linux/jiffies.h>
 #include <linux/module.h>
-#include <linux/platform_data/pca954x.h>
 #include <linux/slab.h>
 
 /*
@@ -287,10 +286,8 @@ static int pca9541_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adap = client->adapter;
-	struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct i2c_mux_core *muxc;
 	struct pca9541 *data;
-	int force;
 	int ret;
 
 	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
@@ -306,9 +303,6 @@ static int pca9541_probe(struct i2c_client *client,
 
 	/* Create mux adapter */
 
-	force = 0;
-	if (pdata)
-		force = pdata->modes[0].adap_id;
 	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
 			     I2C_MUX_ARBITRATOR,
 			     pca9541_select_chan, pca9541_release_chan);
@@ -320,7 +314,7 @@ static int pca9541_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, muxc);
 
-	ret = i2c_mux_add_adapter(muxc, force, 0, 0);
+	ret = i2c_mux_add_adapter(muxc, 0, 0, 0);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/platform_data/pca954x.h b/include/linux/platform_data/pca954x.h
deleted file mode 100644
index 1712677d5904..000000000000
--- a/include/linux/platform_data/pca954x.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- *
- * pca954x.h - I2C multiplexer/switch support
- *
- * Copyright (c) 2008-2009 Rodolfo Giometti <giometti@linux.it>
- * Copyright (c) 2008-2009 Eurotech S.p.A. <info@eurotech.it>
- * Michael Lawnick <michael.lawnick.ext@nsn.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-
-#ifndef _LINUX_I2C_PCA954X_H
-#define _LINUX_I2C_PCA954X_H
-
-/* Platform data for the PCA954x I2C multiplexers */
-
-/* Per channel initialisation data:
- * @adap_id: bus number for the adapter. 0 = don't care
- * @deselect_on_exit: set this entry to 1, if your H/W needs deselection
- *                    of this channel after transaction.
- *
- */
-struct pca954x_platform_mode {
-	int		adap_id;
-	unsigned int	deselect_on_exit:1;
-	unsigned int	class;
-};
-
-/* Per mux/switch data, used with i2c_register_board_info */
-struct pca954x_platform_data {
-	struct pca954x_platform_mode *modes;
-	int num_modes;
-};
-
-#endif /* _LINUX_I2C_PCA954X_H */
-- 
2.20.1

[PATCH v4 3/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[Robert Shearman]

From: Robert Shearman <robert.shearman@att.com>

The behaviour, by default, to not deselect after each transfer is
unsafe when there is a device with an address that conflicts with
another device on another mux on the same parent bus, and it
may not be convenient to use devicetree to set the deselect mux,
e.g. when running on x86_64 when ACPI is used to discover most of the
device hierarchy.

Therefore, provide the ability to set the idle state behaviour using a
new sysfs file, idle_state as a complement to the method of
instantiating the device via sysfs. The possible behaviours are
disconnect, i.e. to deselect all channels from the mux, as-is (the
default), i.e. leave the last channel selected, and set a
predetermined channel.

The current behaviour of leaving the channel as-is after each
transaction is preserved.

Signed-off-by: Robert Shearman <robert.shearman@att.com>
---
 .../ABI/testing/sysfs-bus-i2c-devices-pca954x | 20 +++++
 drivers/i2c/muxes/i2c-mux-pca954x.c           | 85 +++++++++++++++++--
 2 files changed, 97 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x b/Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x
new file mode 100644
index 000000000000..0b0de8cd0d13
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x
@@ -0,0 +1,20 @@
+What:		/sys/bus/i2c/.../idle_state
+Date:		January 2019
+KernelVersion:	5.2
+Contact:	Robert Shearman <robert.shearman@att.com>
+Description:
+		Value that exists only for mux devices that can be
+		written to control the behaviour of the multiplexer on
+		idle. Possible values:
+		-2 - disconnect on idle, i.e. deselect the last used
+		     channel, which is useful when there is a device
+		     with an address that conflicts with another
+		     device on another mux on the same parent bus.
+		-1 - leave the mux as-is, which is the most optimal
+		     setting in terms of I2C operations and is the
+		     default mode.
+		0..<nchans> - set the mux to a predetermined channel,
+		     which is useful if there is one channel that is
+		     used almost always, and you want to reduce the
+		     latency for normal operations after rare
+		     transactions on other channels
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index e32fef560684..923aa3a5a3dc 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -49,6 +49,7 @@
 #include <linux/pm.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <dt-bindings/mux/mux.h>
 
 #define PCA954X_MAX_NCHANS 8
 
@@ -84,7 +85,9 @@ struct pca954x {
 	const struct chip_desc *chip;
 
 	u8 last_chan;		/* last register value */
-	u8 deselect;
+	/* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
+	s8 idle_state;
+
 	struct i2c_client *client;
 
 	struct irq_domain *irq;
@@ -253,15 +256,71 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
+	s8 idle_state;
+
+	idle_state = READ_ONCE(data->idle_state);
+	if (idle_state >= 0)
+		/* Set the mux back to a predetermined channel */
+		return pca954x_select_chan(muxc, idle_state);
+
+	if (idle_state == MUX_IDLE_DISCONNECT) {
+		/* Deselect active channel */
+		data->last_chan = 0;
+		return pca954x_reg_write(muxc->parent, client,
+					 data->last_chan);
+	}
 
-	if (!(data->deselect & (1 << chan)))
-		return 0;
+	/* otherwise leave as-is */
 
-	/* Deselect active channel */
-	data->last_chan = 0;
-	return pca954x_reg_write(muxc->parent, client, data->last_chan);
+	return 0;
+}
+
+static ssize_t idle_state_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+
+	return sprintf(buf, "%d\n", READ_ONCE(data->idle_state));
+}
+
+static ssize_t idle_state_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int val;
+	int ret;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val != MUX_IDLE_AS_IS && val != MUX_IDLE_DISCONNECT &&
+	    (val < 0 || val >= data->chip->nchans))
+		return -EINVAL;
+
+	i2c_lock_bus(muxc->parent, I2C_LOCK_SEGMENT);
+
+	WRITE_ONCE(data->idle_state, val);
+	/*
+	 * Set the mux into a state consistent with the new
+	 * idle_state.
+	 */
+	if (data->last_chan || val != MUX_IDLE_DISCONNECT)
+		ret = pca954x_deselect_mux(muxc, 0);
+
+	i2c_unlock_bus(muxc->parent, I2C_LOCK_SEGMENT);
+
+	return ret < 0 ? ret : count;
 }
 
+static DEVICE_ATTR_RW(idle_state);
+
 static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
 {
 	struct pca954x *data = dev_id;
@@ -328,8 +387,11 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 static void pca954x_cleanup(struct i2c_mux_core *muxc)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
 	int c, irq;
 
+	device_remove_file(&client->dev, &dev_attr_idle_state);
+
 	if (data->irq) {
 		for (c = 0; c < data->chip->nchans; c++) {
 			irq = irq_find_mapping(data->irq, c);
@@ -410,9 +472,12 @@ static int pca954x_probe(struct i2c_client *client,
 	}
 
 	data->last_chan = 0;		   /* force the first selection */
+	data->idle_state = MUX_IDLE_AS_IS;
 
 	idle_disconnect_dt = np &&
 		of_property_read_bool(np, "i2c-mux-idle-disconnect");
+	if (idle_disconnect_dt)
+		data->idle_state = MUX_IDLE_DISCONNECT;
 
 	ret = pca954x_irq_setup(muxc);
 	if (ret)
@@ -420,8 +485,6 @@ static int pca954x_probe(struct i2c_client *client,
 
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
-		data->deselect |= idle_disconnect_dt << num;
-
 		ret = i2c_mux_add_adapter(muxc, 0, num, 0);
 		if (ret)
 			goto fail_cleanup;
@@ -436,6 +499,12 @@ static int pca954x_probe(struct i2c_client *client,
 			goto fail_cleanup;
 	}
 
+	/*
+	 * The attr probably isn't going to be needed in most cases,
+	 * so don't fail completely on error.
+	 */
+	device_create_file(dev, &dev_attr_idle_state);
+
 	dev_info(dev, "registered %d multiplexed busses for I2C %s %s\n",
 		 num, data->chip->muxtype == pca954x_ismux
 				? "mux" : "switch", client->name);
-- 
2.20.1

Re: [PATCH v4 3/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[Peter Rosin]

Hi Greg,

I was wondering if you think the below ABI addition looks sane to you?
Or, should I ask someone else for a tag?

Cheers,
Peter

On 2019-02-28 12:43, Robert Shearman wrote:
> From: Robert Shearman <robert.shearman@att.com>
> 
> The behaviour, by default, to not deselect after each transfer is
> unsafe when there is a device with an address that conflicts with
> another device on another mux on the same parent bus, and it
> may not be convenient to use devicetree to set the deselect mux,
> e.g. when running on x86_64 when ACPI is used to discover most of the
> device hierarchy.
> 
> Therefore, provide the ability to set the idle state behaviour using a
> new sysfs file, idle_state as a complement to the method of
> instantiating the device via sysfs. The possible behaviours are
> disconnect, i.e. to deselect all channels from the mux, as-is (the
> default), i.e. leave the last channel selected, and set a
> predetermined channel.
> 
> The current behaviour of leaving the channel as-is after each
> transaction is preserved.
> 
> Signed-off-by: Robert Shearman <robert.shearman@att.com>
> ---
>  .../ABI/testing/sysfs-bus-i2c-devices-pca954x | 20 +++++
>  drivers/i2c/muxes/i2c-mux-pca954x.c           | 85 +++++++++++++++++--
>  2 files changed, 97 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x b/Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x
> new file mode 100644
> index 000000000000..0b0de8cd0d13
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-pca954x
> @@ -0,0 +1,20 @@
> +What:		/sys/bus/i2c/.../idle_state
> +Date:		January 2019
> +KernelVersion:	5.2
> +Contact:	Robert Shearman <robert.shearman@att.com>
> +Description:
> +		Value that exists only for mux devices that can be
> +		written to control the behaviour of the multiplexer on
> +		idle. Possible values:
> +		-2 - disconnect on idle, i.e. deselect the last used
> +		     channel, which is useful when there is a device
> +		     with an address that conflicts with another
> +		     device on another mux on the same parent bus.
> +		-1 - leave the mux as-is, which is the most optimal
> +		     setting in terms of I2C operations and is the
> +		     default mode.
> +		0..<nchans> - set the mux to a predetermined channel,
> +		     which is useful if there is one channel that is
> +		     used almost always, and you want to reduce the
> +		     latency for normal operations after rare
> +		     transactions on other channels
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index e32fef560684..923aa3a5a3dc 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -49,6 +49,7 @@
>  #include <linux/pm.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <dt-bindings/mux/mux.h>
>  
>  #define PCA954X_MAX_NCHANS 8
>  
> @@ -84,7 +85,9 @@ struct pca954x {
>  	const struct chip_desc *chip;
>  
>  	u8 last_chan;		/* last register value */
> -	u8 deselect;
> +	/* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
> +	s8 idle_state;
> +
>  	struct i2c_client *client;
>  
>  	struct irq_domain *irq;
> @@ -253,15 +256,71 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>  {
>  	struct pca954x *data = i2c_mux_priv(muxc);
>  	struct i2c_client *client = data->client;
> +	s8 idle_state;
> +
> +	idle_state = READ_ONCE(data->idle_state);
> +	if (idle_state >= 0)
> +		/* Set the mux back to a predetermined channel */
> +		return pca954x_select_chan(muxc, idle_state);
> +
> +	if (idle_state == MUX_IDLE_DISCONNECT) {
> +		/* Deselect active channel */
> +		data->last_chan = 0;
> +		return pca954x_reg_write(muxc->parent, client,
> +					 data->last_chan);
> +	}
>  
> -	if (!(data->deselect & (1 << chan)))
> -		return 0;
> +	/* otherwise leave as-is */
>  
> -	/* Deselect active channel */
> -	data->last_chan = 0;
> -	return pca954x_reg_write(muxc->parent, client, data->last_chan);
> +	return 0;
> +}
> +
> +static ssize_t idle_state_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +	struct pca954x *data = i2c_mux_priv(muxc);
> +
> +	return sprintf(buf, "%d\n", READ_ONCE(data->idle_state));
> +}
> +
> +static ssize_t idle_state_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +	struct pca954x *data = i2c_mux_priv(muxc);
> +	int val;
> +	int ret;
> +
> +	ret = kstrtoint(buf, 0, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (val != MUX_IDLE_AS_IS && val != MUX_IDLE_DISCONNECT &&
> +	    (val < 0 || val >= data->chip->nchans))
> +		return -EINVAL;
> +
> +	i2c_lock_bus(muxc->parent, I2C_LOCK_SEGMENT);
> +
> +	WRITE_ONCE(data->idle_state, val);
> +	/*
> +	 * Set the mux into a state consistent with the new
> +	 * idle_state.
> +	 */
> +	if (data->last_chan || val != MUX_IDLE_DISCONNECT)
> +		ret = pca954x_deselect_mux(muxc, 0);
> +
> +	i2c_unlock_bus(muxc->parent, I2C_LOCK_SEGMENT);
> +
> +	return ret < 0 ? ret : count;
>  }
>  
> +static DEVICE_ATTR_RW(idle_state);
> +
>  static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
>  {
>  	struct pca954x *data = dev_id;
> @@ -328,8 +387,11 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
>  static void pca954x_cleanup(struct i2c_mux_core *muxc)
>  {
>  	struct pca954x *data = i2c_mux_priv(muxc);
> +	struct i2c_client *client = data->client;
>  	int c, irq;
>  
> +	device_remove_file(&client->dev, &dev_attr_idle_state);
> +
>  	if (data->irq) {
>  		for (c = 0; c < data->chip->nchans; c++) {
>  			irq = irq_find_mapping(data->irq, c);
> @@ -410,9 +472,12 @@ static int pca954x_probe(struct i2c_client *client,
>  	}
>  
>  	data->last_chan = 0;		   /* force the first selection */
> +	data->idle_state = MUX_IDLE_AS_IS;
>  
>  	idle_disconnect_dt = np &&
>  		of_property_read_bool(np, "i2c-mux-idle-disconnect");
> +	if (idle_disconnect_dt)
> +		data->idle_state = MUX_IDLE_DISCONNECT;
>  
>  	ret = pca954x_irq_setup(muxc);
>  	if (ret)
> @@ -420,8 +485,6 @@ static int pca954x_probe(struct i2c_client *client,
>  
>  	/* Now create an adapter for each channel */
>  	for (num = 0; num < data->chip->nchans; num++) {
> -		data->deselect |= idle_disconnect_dt << num;
> -
>  		ret = i2c_mux_add_adapter(muxc, 0, num, 0);
>  		if (ret)
>  			goto fail_cleanup;
> @@ -436,6 +499,12 @@ static int pca954x_probe(struct i2c_client *client,
>  			goto fail_cleanup;
>  	}
>  
> +	/*
> +	 * The attr probably isn't going to be needed in most cases,
> +	 * so don't fail completely on error.
> +	 */
> +	device_create_file(dev, &dev_attr_idle_state);
> +
>  	dev_info(dev, "registered %d multiplexed busses for I2C %s %s\n",
>  		 num, data->chip->muxtype == pca954x_ismux
>  				? "mux" : "switch", client->name);
> 

Re: [PATCH v4 3/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[Greg Kroah-Hartman]

On Wed, Mar 13, 2019 at 07:04:06AM +0000, Peter Rosin wrote:
> Hi Greg,
> 
> I was wondering if you think the below ABI addition looks sane to you?

I am not the i2c maintainer :)

> Or, should I ask someone else for a tag?

It's the middle of the merge window.  None of us can do much other than
focus on that until 5.1-rc1 is out.

thanks,

greg k-h

Re: [PATCH v4 3/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[Wolfram Sang]

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

Hi Greg,

> > I was wondering if you think the below ABI addition looks sane to you?
> I am not the i2c maintainer :)

Peter is the i2c-mux maintainer, so I trust him very much on the I2C
side of things. We just wondered if you'd notice a general flaw when
exposing such an interface to userspace.

> > Or, should I ask someone else for a tag?
> 
> It's the middle of the merge window.  None of us can do much other than
> focus on that until 5.1-rc1 is out.

Sure, thanks anyway for the response.

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 3/3] i2c: mux: pca954x: allow management of device idle state via sysfs

[Greg Kroah-Hartman]

On Wed, Mar 13, 2019 at 05:36:10PM +0100, Wolfram Sang wrote:
> Hi Greg,
> 
> > > I was wondering if you think the below ABI addition looks sane to you?
> > I am not the i2c maintainer :)
> 
> Peter is the i2c-mux maintainer, so I trust him very much on the I2C
> side of things. We just wondered if you'd notice a general flaw when
> exposing such an interface to userspace.

Ah, I see.  No, looks fine to me, it's a single value, well documented,
should be just fine.

thanks,

greg k-h