* [PATCH v7 01/24] i2c-mux: add common data for every i2c-mux instance
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 02/24] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core Peter Rosin
` (25 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
All i2c-muxes have a parent adapter and one or many child
adapters. A mux also has some means of selection. Previously,
this was stored per child adapter, but it is only needed
to keep track of this per mux.
Add an i2c mux core, that keeps track of this consistently.
Also add some glue for users of the old interface, which will
create one implicit mux core per child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/i2c-mux.c | 175 ++++++++++++++++++++++++++++++++++++------------
include/linux/i2c-mux.h | 34 ++++++++++
2 files changed, 168 insertions(+), 41 deletions(-)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index d4022878b2f0..5ce1b0704cb5 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -28,33 +28,34 @@
#include <linux/slab.h>
/* multiplexer per channel data */
+struct i2c_mux_priv_old {
+ void *mux_priv;
+ int (*select)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
+ int (*deselect)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
+};
+
struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
-
- struct i2c_adapter *parent;
- struct device *mux_dev;
- void *mux_priv;
+ struct i2c_mux_core *muxc;
u32 chan_id;
-
- int (*select)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
- int (*deselect)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
};
static int i2c_mux_master_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[], int num)
{
struct i2c_mux_priv *priv = adap->algo_data;
- struct i2c_adapter *parent = priv->parent;
+ struct i2c_mux_core *muxc = priv->muxc;
+ struct i2c_adapter *parent = muxc->parent;
int ret;
/* Switch to the right mux port and perform the transfer. */
- ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+ ret = muxc->select(muxc, priv->chan_id);
if (ret >= 0)
ret = __i2c_transfer(parent, msgs, num);
- if (priv->deselect)
- priv->deselect(parent, priv->mux_priv, priv->chan_id);
+ if (muxc->deselect)
+ muxc->deselect(muxc, priv->chan_id);
return ret;
}
@@ -65,17 +66,18 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
int size, union i2c_smbus_data *data)
{
struct i2c_mux_priv *priv = adap->algo_data;
- struct i2c_adapter *parent = priv->parent;
+ struct i2c_mux_core *muxc = priv->muxc;
+ struct i2c_adapter *parent = muxc->parent;
int ret;
/* Select the right mux port and perform the transfer. */
- ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+ ret = muxc->select(muxc, priv->chan_id);
if (ret >= 0)
ret = parent->algo->smbus_xfer(parent, addr, flags,
read_write, command, size, data);
- if (priv->deselect)
- priv->deselect(parent, priv->mux_priv, priv->chan_id);
+ if (muxc->deselect)
+ muxc->deselect(muxc, priv->chan_id);
return ret;
}
@@ -84,7 +86,7 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
static u32 i2c_mux_functionality(struct i2c_adapter *adap)
{
struct i2c_mux_priv *priv = adap->algo_data;
- struct i2c_adapter *parent = priv->parent;
+ struct i2c_adapter *parent = priv->muxc->parent;
return parent->algo->functionality(parent);
}
@@ -102,6 +104,20 @@ static unsigned int i2c_mux_parent_classes(struct i2c_adapter *parent)
return class;
}
+static int i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct i2c_mux_priv_old *priv = i2c_mux_priv(muxc);
+
+ return priv->select(muxc->parent, priv->mux_priv, chan);
+}
+
+static int i2c_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct i2c_mux_priv_old *priv = i2c_mux_priv(muxc);
+
+ return priv->deselect(muxc->parent, priv->mux_priv, chan);
+}
+
struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
struct device *mux_dev,
void *mux_priv, u32 force_nr, u32 chan_id,
@@ -111,21 +127,77 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
int (*deselect) (struct i2c_adapter *,
void *, u32))
{
+ struct i2c_mux_core *muxc;
+ struct i2c_mux_priv_old *priv;
+ int ret;
+
+ muxc = i2c_mux_alloc(parent, mux_dev, 1, sizeof(*priv), 0,
+ i2c_mux_select, i2c_mux_deselect);
+ if (!muxc)
+ return NULL;
+
+ priv = i2c_mux_priv(muxc);
+ priv->select = select;
+ priv->deselect = deselect;
+ priv->mux_priv = mux_priv;
+
+ ret = i2c_mux_add_adapter(muxc, force_nr, chan_id, class);
+ if (ret) {
+ devm_kfree(mux_dev, muxc);
+ return NULL;
+ }
+
+ return muxc->adapter[0];
+}
+EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
+
+struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
+ struct device *dev, int max_adapters,
+ int sizeof_priv, u32 flags,
+ int (*select)(struct i2c_mux_core *, u32),
+ int (*deselect)(struct i2c_mux_core *, u32))
+{
+ struct i2c_mux_core *muxc;
+
+ muxc = devm_kzalloc(dev, sizeof(*muxc)
+ + max_adapters * sizeof(muxc->adapter[0])
+ + sizeof_priv, GFP_KERNEL);
+ if (!muxc)
+ return NULL;
+ if (sizeof_priv)
+ muxc->priv = &muxc->adapter[max_adapters];
+
+ muxc->parent = parent;
+ muxc->dev = dev;
+ muxc->select = select;
+ muxc->deselect = deselect;
+ muxc->max_adapters = max_adapters;
+
+ return muxc;
+}
+EXPORT_SYMBOL_GPL(i2c_mux_alloc);
+
+int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
+ u32 force_nr, u32 chan_id,
+ unsigned int class)
+{
+ struct i2c_adapter *parent = muxc->parent;
struct i2c_mux_priv *priv;
char symlink_name[20];
int ret;
- priv = kzalloc(sizeof(struct i2c_mux_priv), GFP_KERNEL);
+ if (muxc->num_adapters >= muxc->max_adapters) {
+ dev_err(muxc->dev, "No room for more i2c-mux adapters\n");
+ return -EINVAL;
+ }
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
- return NULL;
+ return -ENOMEM;
/* Set up private adapter data */
- priv->parent = parent;
- priv->mux_dev = mux_dev;
- priv->mux_priv = mux_priv;
+ priv->muxc = muxc;
priv->chan_id = chan_id;
- priv->select = select;
- priv->deselect = deselect;
/* Need to do algo dynamically because we don't know ahead
* of time what sort of physical adapter we'll be dealing with.
@@ -159,11 +231,11 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
* Try to populate the mux adapter's of_node, expands to
* nothing if !CONFIG_OF.
*/
- if (mux_dev->of_node) {
+ if (muxc->dev->of_node) {
struct device_node *child;
u32 reg;
- for_each_child_of_node(mux_dev->of_node, child) {
+ for_each_child_of_node(muxc->dev->of_node, child) {
ret = of_property_read_u32(child, "reg", ®);
if (ret)
continue;
@@ -177,8 +249,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
/*
* Associate the mux channel with an ACPI node.
*/
- if (has_acpi_companion(mux_dev))
- acpi_preset_companion(&priv->adap.dev, ACPI_COMPANION(mux_dev),
+ if (has_acpi_companion(muxc->dev))
+ acpi_preset_companion(&priv->adap.dev,
+ ACPI_COMPANION(muxc->dev),
chan_id);
if (force_nr) {
@@ -192,33 +265,53 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
"failed to add mux-adapter (error=%d)\n",
ret);
kfree(priv);
- return NULL;
+ return ret;
}
- WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj, "mux_device"),
- "can't create symlink to mux device\n");
+ WARN(sysfs_create_link(&priv->adap.dev.kobj, &muxc->dev->kobj,
+ "mux_device"),
+ "can't create symlink to mux device\n");
snprintf(symlink_name, sizeof(symlink_name), "channel-%u", chan_id);
- WARN(sysfs_create_link(&mux_dev->kobj, &priv->adap.dev.kobj, symlink_name),
- "can't create symlink for channel %u\n", chan_id);
+ WARN(sysfs_create_link(&muxc->dev->kobj, &priv->adap.dev.kobj,
+ symlink_name),
+ "can't create symlink for channel %u\n", chan_id);
dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
i2c_adapter_id(&priv->adap));
- return &priv->adap;
+ muxc->adapter[muxc->num_adapters++] = &priv->adap;
+ return 0;
}
-EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
+EXPORT_SYMBOL_GPL(i2c_mux_add_adapter);
-void i2c_del_mux_adapter(struct i2c_adapter *adap)
+void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
{
- struct i2c_mux_priv *priv = adap->algo_data;
char symlink_name[20];
- snprintf(symlink_name, sizeof(symlink_name), "channel-%u", priv->chan_id);
- sysfs_remove_link(&priv->mux_dev->kobj, symlink_name);
+ while (muxc->num_adapters) {
+ struct i2c_adapter *adap = muxc->adapter[--muxc->num_adapters];
+ struct i2c_mux_priv *priv = adap->algo_data;
+
+ muxc->adapter[muxc->num_adapters] = NULL;
+
+ snprintf(symlink_name, sizeof(symlink_name),
+ "channel-%u", priv->chan_id);
+ sysfs_remove_link(&muxc->dev->kobj, symlink_name);
+
+ sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
+ i2c_del_adapter(adap);
+ kfree(priv);
+ }
+}
+EXPORT_SYMBOL_GPL(i2c_mux_del_adapters);
+
+void i2c_del_mux_adapter(struct i2c_adapter *adap)
+{
+ struct i2c_mux_priv *priv = adap->algo_data;
+ struct i2c_mux_core *muxc = priv->muxc;
- sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
- i2c_del_adapter(adap);
- kfree(priv);
+ i2c_mux_del_adapters(muxc);
+ devm_kfree(muxc->dev, muxc);
}
EXPORT_SYMBOL_GPL(i2c_del_mux_adapter);
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index b5f9a007a3ab..71ac1b3f4f68 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -27,6 +27,31 @@
#ifdef __KERNEL__
+struct i2c_mux_core {
+ struct i2c_adapter *parent;
+ struct device *dev;
+
+ void *priv;
+
+ int (*select)(struct i2c_mux_core *, u32 chan_id);
+ int (*deselect)(struct i2c_mux_core *, u32 chan_id);
+
+ int num_adapters;
+ int max_adapters;
+ struct i2c_adapter *adapter[0];
+};
+
+struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
+ struct device *dev, int max_adapters,
+ int sizeof_priv, u32 flags,
+ int (*select)(struct i2c_mux_core *, u32),
+ int (*deselect)(struct i2c_mux_core *, u32));
+
+static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
+{
+ return muxc->priv;
+}
+
/*
* Called to create a i2c bus on a multiplexed bus segment.
* The mux_dev and chan_id parameters are passed to the select
@@ -41,8 +66,17 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
void *mux_dev, u32 chan_id),
int (*deselect) (struct i2c_adapter *,
void *mux_dev, u32 chan_id));
+/*
+ * Called to create an i2c bus on a multiplexed bus segment.
+ * The chan_id parameter is passed to the select and deselect
+ * callback functions to perform hardware-specific mux control.
+ */
+int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
+ u32 force_nr, u32 chan_id,
+ unsigned int class);
void i2c_del_mux_adapter(struct i2c_adapter *adap);
+void i2c_mux_del_adapters(struct i2c_mux_core *muxc);
#endif /* __KERNEL__ */
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 02/24] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
2016-04-20 15:17 ` [PATCH v7 01/24] i2c-mux: add common data for every i2c-mux instance Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 03/24] i2c: i2c-mux-pinctrl: " Peter Rosin
` (24 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 55 ++++++++++++++++------------------------
1 file changed, 22 insertions(+), 33 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index b8e11c16d98c..f6270ee934f9 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -18,8 +18,6 @@
#include <linux/of_gpio.h>
struct gpiomux {
- struct i2c_adapter *parent;
- struct i2c_adapter **adap; /* child busses */
struct i2c_mux_gpio_platform_data data;
unsigned gpio_base;
};
@@ -33,18 +31,18 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
val & (1 << i));
}
-static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
+static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
{
- struct gpiomux *mux = data;
+ struct gpiomux *mux = i2c_mux_priv(muxc);
i2c_mux_gpio_set(mux, chan);
return 0;
}
-static int i2c_mux_gpio_deselect(struct i2c_adapter *adap, void *data, u32 chan)
+static int i2c_mux_gpio_deselect(struct i2c_mux_core *muxc, u32 chan)
{
- struct gpiomux *mux = data;
+ struct gpiomux *mux = i2c_mux_priv(muxc);
i2c_mux_gpio_set(mux, mux->data.idle);
@@ -136,19 +134,15 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
static int i2c_mux_gpio_probe(struct platform_device *pdev)
{
+ struct i2c_mux_core *muxc;
struct gpiomux *mux;
struct i2c_adapter *parent;
- int (*deselect) (struct i2c_adapter *, void *, u32);
unsigned initial_state, gpio_base;
int i, ret;
mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
- if (!mux) {
- dev_err(&pdev->dev, "Cannot allocate gpiomux structure");
+ if (!mux)
return -ENOMEM;
- }
-
- platform_set_drvdata(pdev, mux);
if (!dev_get_platdata(&pdev->dev)) {
ret = i2c_mux_gpio_probe_dt(mux, pdev);
@@ -180,24 +174,23 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
if (!parent)
return -EPROBE_DEFER;
- mux->parent = parent;
- mux->gpio_base = gpio_base;
-
- mux->adap = devm_kzalloc(&pdev->dev,
- sizeof(*mux->adap) * mux->data.n_values,
- GFP_KERNEL);
- if (!mux->adap) {
- dev_err(&pdev->dev, "Cannot allocate i2c_adapter structure");
+ muxc = i2c_mux_alloc(parent, &pdev->dev, mux->data.n_values, 0, 0,
+ i2c_mux_gpio_select, NULL);
+ if (!muxc) {
ret = -ENOMEM;
goto alloc_failed;
}
+ muxc->priv = mux;
+
+ platform_set_drvdata(pdev, muxc);
+
+ mux->gpio_base = gpio_base;
if (mux->data.idle != I2C_MUX_GPIO_NO_IDLE) {
initial_state = mux->data.idle;
- deselect = i2c_mux_gpio_deselect;
+ muxc->deselect = i2c_mux_gpio_deselect;
} else {
initial_state = mux->data.values[0];
- deselect = NULL;
}
for (i = 0; i < mux->data.n_gpios; i++) {
@@ -223,11 +216,8 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
u32 nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
unsigned int class = mux->data.classes ? mux->data.classes[i] : 0;
- mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
- mux->data.values[i], class,
- i2c_mux_gpio_select, deselect);
- if (!mux->adap[i]) {
- ret = -ENODEV;
+ ret = i2c_mux_add_adapter(muxc, nr, mux->data.values[i], class);
+ if (ret) {
dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
goto add_adapter_failed;
}
@@ -239,8 +229,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
return 0;
add_adapter_failed:
- for (; i > 0; i--)
- i2c_del_mux_adapter(mux->adap[i - 1]);
+ i2c_mux_del_adapters(muxc);
i = mux->data.n_gpios;
err_request_gpio:
for (; i > 0; i--)
@@ -253,16 +242,16 @@ alloc_failed:
static int i2c_mux_gpio_remove(struct platform_device *pdev)
{
- struct gpiomux *mux = platform_get_drvdata(pdev);
+ struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
+ struct gpiomux *mux = i2c_mux_priv(muxc);
int i;
- for (i = 0; i < mux->data.n_values; i++)
- i2c_del_mux_adapter(mux->adap[i]);
+ i2c_mux_del_adapters(muxc);
for (i = 0; i < mux->data.n_gpios; i++)
gpio_free(mux->gpio_base + mux->data.gpios[i]);
- i2c_put_adapter(mux->parent);
+ i2c_put_adapter(muxc->parent);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 03/24] i2c: i2c-mux-pinctrl: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
2016-04-20 15:17 ` [PATCH v7 01/24] i2c-mux: add common data for every i2c-mux instance Peter Rosin
2016-04-20 15:17 ` [PATCH v7 02/24] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 04/24] i2c: i2c-arb-gpio-challenge: " Peter Rosin
` (23 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-pinctrl.c | 83 ++++++++++++++-----------------------
1 file changed, 30 insertions(+), 53 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index b5a982ba8898..1b8dc711815e 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -26,34 +26,29 @@
#include <linux/of.h>
struct i2c_mux_pinctrl {
- struct device *dev;
struct i2c_mux_pinctrl_platform_data *pdata;
struct pinctrl *pinctrl;
struct pinctrl_state **states;
struct pinctrl_state *state_idle;
- struct i2c_adapter *parent;
- struct i2c_adapter **busses;
};
-static int i2c_mux_pinctrl_select(struct i2c_adapter *adap, void *data,
- u32 chan)
+static int i2c_mux_pinctrl_select(struct i2c_mux_core *muxc, u32 chan)
{
- struct i2c_mux_pinctrl *mux = data;
+ struct i2c_mux_pinctrl *mux = i2c_mux_priv(muxc);
return pinctrl_select_state(mux->pinctrl, mux->states[chan]);
}
-static int i2c_mux_pinctrl_deselect(struct i2c_adapter *adap, void *data,
- u32 chan)
+static int i2c_mux_pinctrl_deselect(struct i2c_mux_core *muxc, u32 chan)
{
- struct i2c_mux_pinctrl *mux = data;
+ struct i2c_mux_pinctrl *mux = i2c_mux_priv(muxc);
return pinctrl_select_state(mux->pinctrl, mux->state_idle);
}
#ifdef CONFIG_OF
static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
- struct platform_device *pdev)
+ struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
int num_names, i, ret;
@@ -64,15 +59,12 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
return 0;
mux->pdata = devm_kzalloc(&pdev->dev, sizeof(*mux->pdata), GFP_KERNEL);
- if (!mux->pdata) {
- dev_err(mux->dev,
- "Cannot allocate i2c_mux_pinctrl_platform_data\n");
+ if (!mux->pdata)
return -ENOMEM;
- }
num_names = of_property_count_strings(np, "pinctrl-names");
if (num_names < 0) {
- dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
+ dev_err(&pdev->dev, "Cannot parse pinctrl-names: %d\n",
num_names);
return num_names;
}
@@ -80,23 +72,22 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
sizeof(*mux->pdata->pinctrl_states) * num_names,
GFP_KERNEL);
- if (!mux->pdata->pinctrl_states) {
- dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
+ if (!mux->pdata->pinctrl_states)
return -ENOMEM;
- }
for (i = 0; i < num_names; i++) {
ret = of_property_read_string_index(np, "pinctrl-names", i,
&mux->pdata->pinctrl_states[mux->pdata->bus_count]);
if (ret < 0) {
- dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
+ dev_err(&pdev->dev, "Cannot parse pinctrl-names: %d\n",
ret);
return ret;
}
if (!strcmp(mux->pdata->pinctrl_states[mux->pdata->bus_count],
"idle")) {
if (i != num_names - 1) {
- dev_err(mux->dev, "idle state must be last\n");
+ dev_err(&pdev->dev,
+ "idle state must be last\n");
return -EINVAL;
}
mux->pdata->pinctrl_state_idle = "idle";
@@ -107,13 +98,13 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
adapter_np = of_parse_phandle(np, "i2c-parent", 0);
if (!adapter_np) {
- dev_err(mux->dev, "Cannot parse i2c-parent\n");
+ dev_err(&pdev->dev, "Cannot parse i2c-parent\n");
return -ENODEV;
}
adapter = of_find_i2c_adapter_by_node(adapter_np);
of_node_put(adapter_np);
if (!adapter) {
- dev_err(mux->dev, "Cannot find parent bus\n");
+ dev_err(&pdev->dev, "Cannot find parent bus\n");
return -EPROBE_DEFER;
}
mux->pdata->parent_bus_num = i2c_adapter_id(adapter);
@@ -131,19 +122,15 @@ static inline int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
{
+ struct i2c_mux_core *muxc;
struct i2c_mux_pinctrl *mux;
- int (*deselect)(struct i2c_adapter *, void *, u32);
int i, ret;
mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
if (!mux) {
- dev_err(&pdev->dev, "Cannot allocate i2c_mux_pinctrl\n");
ret = -ENOMEM;
goto err;
}
- platform_set_drvdata(pdev, mux);
-
- mux->dev = &pdev->dev;
mux->pdata = dev_get_platdata(&pdev->dev);
if (!mux->pdata) {
@@ -166,14 +153,15 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
goto err;
}
- mux->busses = devm_kzalloc(&pdev->dev,
- sizeof(*mux->busses) * mux->pdata->bus_count,
- GFP_KERNEL);
- if (!mux->busses) {
- dev_err(&pdev->dev, "Cannot allocate busses\n");
+ muxc = i2c_mux_alloc(NULL, &pdev->dev, mux->pdata->bus_count, 0, 0,
+ i2c_mux_pinctrl_select, NULL);
+ if (!muxc) {
ret = -ENOMEM;
goto err;
}
+ muxc->priv = mux;
+
+ platform_set_drvdata(pdev, muxc);
mux->pinctrl = devm_pinctrl_get(&pdev->dev);
if (IS_ERR(mux->pinctrl)) {
@@ -203,13 +191,11 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
goto err;
}
- deselect = i2c_mux_pinctrl_deselect;
- } else {
- deselect = NULL;
+ muxc->deselect = i2c_mux_pinctrl_deselect;
}
- mux->parent = i2c_get_adapter(mux->pdata->parent_bus_num);
- if (!mux->parent) {
+ muxc->parent = i2c_get_adapter(mux->pdata->parent_bus_num);
+ if (!muxc->parent) {
dev_err(&pdev->dev, "Parent adapter (%d) not found\n",
mux->pdata->parent_bus_num);
ret = -EPROBE_DEFER;
@@ -220,12 +206,8 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
u32 bus = mux->pdata->base_bus_num ?
(mux->pdata->base_bus_num + i) : 0;
- mux->busses[i] = i2c_add_mux_adapter(mux->parent, &pdev->dev,
- mux, bus, i, 0,
- i2c_mux_pinctrl_select,
- deselect);
- if (!mux->busses[i]) {
- ret = -ENODEV;
+ ret = i2c_mux_add_adapter(muxc, bus, i, 0);
+ if (ret) {
dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
goto err_del_adapter;
}
@@ -234,23 +216,18 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
return 0;
err_del_adapter:
- for (; i > 0; i--)
- i2c_del_mux_adapter(mux->busses[i - 1]);
- i2c_put_adapter(mux->parent);
+ i2c_mux_del_adapters(muxc);
+ i2c_put_adapter(muxc->parent);
err:
return ret;
}
static int i2c_mux_pinctrl_remove(struct platform_device *pdev)
{
- struct i2c_mux_pinctrl *mux = platform_get_drvdata(pdev);
- int i;
-
- for (i = 0; i < mux->pdata->bus_count; i++)
- i2c_del_mux_adapter(mux->busses[i]);
-
- i2c_put_adapter(mux->parent);
+ struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
+ i2c_mux_del_adapters(muxc);
+ i2c_put_adapter(muxc->parent);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 04/24] i2c: i2c-arb-gpio-challenge: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (2 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 03/24] i2c: i2c-mux-pinctrl: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 05/24] i2c: i2c-mux-pca9541: " Peter Rosin
` (22 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 47 +++++++++++++-----------------
1 file changed, 20 insertions(+), 27 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index 402e3a6c671a..a90bbc4037dd 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -28,8 +28,6 @@
/**
* struct i2c_arbitrator_data - Driver data for I2C arbitrator
*
- * @parent: Parent adapter
- * @child: Child bus
* @our_gpio: GPIO we'll use to claim.
* @our_gpio_release: 0 if active high; 1 if active low; AKA if the GPIO ==
* this then consider it released.
@@ -42,8 +40,6 @@
*/
struct i2c_arbitrator_data {
- struct i2c_adapter *parent;
- struct i2c_adapter *child;
int our_gpio;
int our_gpio_release;
int their_gpio;
@@ -59,9 +55,9 @@ struct i2c_arbitrator_data {
*
* Use the GPIO-based signalling protocol; return -EBUSY if we fail.
*/
-static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan)
+static int i2c_arbitrator_select(struct i2c_mux_core *muxc, u32 chan)
{
- const struct i2c_arbitrator_data *arb = data;
+ const struct i2c_arbitrator_data *arb = i2c_mux_priv(muxc);
unsigned long stop_retry, stop_time;
/* Start a round of trying to claim the bus */
@@ -93,7 +89,7 @@ static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan)
/* Give up, release our claim */
gpio_set_value(arb->our_gpio, arb->our_gpio_release);
udelay(arb->slew_delay_us);
- dev_err(&adap->dev, "Could not claim bus, timeout\n");
+ dev_err(muxc->dev, "Could not claim bus, timeout\n");
return -EBUSY;
}
@@ -102,10 +98,9 @@ static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan)
*
* Release the I2C bus using the GPIO-based signalling protocol.
*/
-static int i2c_arbitrator_deselect(struct i2c_adapter *adap, void *data,
- u32 chan)
+static int i2c_arbitrator_deselect(struct i2c_mux_core *muxc, u32 chan)
{
- const struct i2c_arbitrator_data *arb = data;
+ const struct i2c_arbitrator_data *arb = i2c_mux_priv(muxc);
/* Release the bus and wait for the other master to notice */
gpio_set_value(arb->our_gpio, arb->our_gpio_release);
@@ -119,6 +114,7 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct device_node *parent_np;
+ struct i2c_mux_core *muxc;
struct i2c_arbitrator_data *arb;
enum of_gpio_flags gpio_flags;
unsigned long out_init;
@@ -134,12 +130,13 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
return -EINVAL;
}
- arb = devm_kzalloc(dev, sizeof(*arb), GFP_KERNEL);
- if (!arb) {
- dev_err(dev, "Cannot allocate i2c_arbitrator_data\n");
+ muxc = i2c_mux_alloc(NULL, dev, 1, sizeof(*arb), 0,
+ i2c_arbitrator_select, i2c_arbitrator_deselect);
+ if (!muxc)
return -ENOMEM;
- }
- platform_set_drvdata(pdev, arb);
+ arb = i2c_mux_priv(muxc);
+
+ platform_set_drvdata(pdev, muxc);
/* Request GPIOs */
ret = of_get_named_gpio_flags(np, "our-claim-gpio", 0, &gpio_flags);
@@ -196,21 +193,18 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
dev_err(dev, "Cannot parse i2c-parent\n");
return -EINVAL;
}
- arb->parent = of_get_i2c_adapter_by_node(parent_np);
+ muxc->parent = of_get_i2c_adapter_by_node(parent_np);
of_node_put(parent_np);
- if (!arb->parent) {
+ if (!muxc->parent) {
dev_err(dev, "Cannot find parent bus\n");
return -EPROBE_DEFER;
}
/* Actually add the mux adapter */
- arb->child = i2c_add_mux_adapter(arb->parent, dev, arb, 0, 0, 0,
- i2c_arbitrator_select,
- i2c_arbitrator_deselect);
- if (!arb->child) {
+ ret = i2c_mux_add_adapter(muxc, 0, 0, 0);
+ if (ret) {
dev_err(dev, "Failed to add adapter\n");
- ret = -ENODEV;
- i2c_put_adapter(arb->parent);
+ i2c_put_adapter(muxc->parent);
}
return ret;
@@ -218,11 +212,10 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
static int i2c_arbitrator_remove(struct platform_device *pdev)
{
- struct i2c_arbitrator_data *arb = platform_get_drvdata(pdev);
-
- i2c_del_mux_adapter(arb->child);
- i2c_put_adapter(arb->parent);
+ struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
+ i2c_mux_del_adapters(muxc);
+ i2c_put_adapter(muxc->parent);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 05/24] i2c: i2c-mux-pca9541: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (3 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 04/24] i2c: i2c-arb-gpio-challenge: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 06/24] i2c: i2c-mux-pca954x: " Peter Rosin
` (21 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-pca9541.c | 58 +++++++++++++++++--------------------
1 file changed, 27 insertions(+), 31 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index d0ba424adebc..3cb8af635db5 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -73,7 +73,7 @@
#define SELECT_DELAY_LONG 1000
struct pca9541 {
- struct i2c_adapter *mux_adap;
+ struct i2c_client *client;
unsigned long select_timeout;
unsigned long arb_timeout;
};
@@ -217,7 +217,8 @@ static const u8 pca9541_control[16] = {
*/
static int pca9541_arbitrate(struct i2c_client *client)
{
- struct pca9541 *data = i2c_get_clientdata(client);
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+ struct pca9541 *data = i2c_mux_priv(muxc);
int reg;
reg = pca9541_reg_read(client, PCA9541_CONTROL);
@@ -285,9 +286,10 @@ static int pca9541_arbitrate(struct i2c_client *client)
return 0;
}
-static int pca9541_select_chan(struct i2c_adapter *adap, void *client, u32 chan)
+static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
{
- struct pca9541 *data = i2c_get_clientdata(client);
+ struct pca9541 *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
int ret;
unsigned long timeout = jiffies + ARB2_TIMEOUT;
/* give up after this time */
@@ -309,9 +311,11 @@ static int pca9541_select_chan(struct i2c_adapter *adap, void *client, u32 chan)
return -ETIMEDOUT;
}
-static int pca9541_release_chan(struct i2c_adapter *adap,
- void *client, u32 chan)
+static int pca9541_release_chan(struct i2c_mux_core *muxc, u32 chan)
{
+ struct pca9541 *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+
pca9541_release_bus(client);
return 0;
}
@@ -324,20 +328,13 @@ static int pca9541_probe(struct i2c_client *client,
{
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 = -ENODEV;
+ int ret;
if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
- goto err;
-
- data = kzalloc(sizeof(struct pca9541), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto err;
- }
-
- i2c_set_clientdata(client, data);
+ return -ENODEV;
/*
* I2C accesses are unprotected here.
@@ -352,34 +349,33 @@ static int pca9541_probe(struct i2c_client *client,
force = 0;
if (pdata)
force = pdata->modes[0].adap_id;
- data->mux_adap = i2c_add_mux_adapter(adap, &client->dev, client,
- force, 0, 0,
- pca9541_select_chan,
- pca9541_release_chan);
+ muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data), 0,
+ pca9541_select_chan, pca9541_release_chan);
+ if (!muxc)
+ return -ENOMEM;
- if (data->mux_adap == NULL) {
+ data = i2c_mux_priv(muxc);
+ data->client = client;
+
+ i2c_set_clientdata(client, muxc);
+
+ ret = i2c_mux_add_adapter(muxc, force, 0, 0);
+ if (ret) {
dev_err(&client->dev, "failed to register master selector\n");
- goto exit_free;
+ return ret;
}
dev_info(&client->dev, "registered master selector for I2C %s\n",
client->name);
return 0;
-
-exit_free:
- kfree(data);
-err:
- return ret;
}
static int pca9541_remove(struct i2c_client *client)
{
- struct pca9541 *data = i2c_get_clientdata(client);
-
- i2c_del_mux_adapter(data->mux_adap);
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
- kfree(data);
+ i2c_mux_del_adapters(muxc);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 06/24] i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (4 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 05/24] i2c: i2c-mux-pca9541: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 07/24] i2c: i2c-mux-reg: " Peter Rosin
` (20 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Add a mask to handle the case where not all child adapters should
cause a mux deselect to happen, now that there is a common deselect op
for all child adapters.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 61 ++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 31 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index acfcef3d4068..528e755c468f 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -60,9 +60,10 @@ enum pca_type {
struct pca954x {
enum pca_type type;
- struct i2c_adapter *virt_adaps[PCA954X_MAX_NCHANS];
u8 last_chan; /* last register value */
+ u8 deselect;
+ struct i2c_client *client;
};
struct chip_desc {
@@ -146,10 +147,10 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
return ret;
}
-static int pca954x_select_chan(struct i2c_adapter *adap,
- void *client, u32 chan)
+static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
{
- struct pca954x *data = i2c_get_clientdata(client);
+ struct pca954x *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
const struct chip_desc *chip = &chips[data->type];
u8 regval;
int ret = 0;
@@ -162,21 +163,24 @@ static int pca954x_select_chan(struct i2c_adapter *adap,
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
- ret = pca954x_reg_write(adap, client, regval);
+ ret = pca954x_reg_write(muxc->parent, client, regval);
data->last_chan = regval;
}
return ret;
}
-static int pca954x_deselect_mux(struct i2c_adapter *adap,
- void *client, u32 chan)
+static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
{
- struct pca954x *data = i2c_get_clientdata(client);
+ struct pca954x *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+
+ if (!(data->deselect & (1 << chan)))
+ return 0;
/* Deselect active channel */
data->last_chan = 0;
- return pca954x_reg_write(adap, client, data->last_chan);
+ return pca954x_reg_write(muxc->parent, client, data->last_chan);
}
/*
@@ -191,17 +195,22 @@ static int pca954x_probe(struct i2c_client *client,
bool idle_disconnect_dt;
struct gpio_desc *gpio;
int num, force, class;
+ struct i2c_mux_core *muxc;
struct pca954x *data;
int ret;
if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
return -ENODEV;
- data = devm_kzalloc(&client->dev, sizeof(struct pca954x), GFP_KERNEL);
- if (!data)
+ muxc = i2c_mux_alloc(adap, &client->dev,
+ PCA954X_MAX_NCHANS, sizeof(*data), 0,
+ pca954x_select_chan, pca954x_deselect_mux);
+ if (!muxc)
return -ENOMEM;
+ data = i2c_mux_priv(muxc);
- i2c_set_clientdata(client, data);
+ i2c_set_clientdata(client, muxc);
+ data->client = client;
/* Get the mux out of reset if a reset GPIO is specified. */
gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_LOW);
@@ -238,16 +247,13 @@ static int pca954x_probe(struct i2c_client *client,
/* discard unconfigured channels */
break;
idle_disconnect_pd = pdata->modes[num].deselect_on_exit;
+ data->deselect |= (idle_disconnect_pd
+ || idle_disconnect_dt) << num;
}
- data->virt_adaps[num] =
- i2c_add_mux_adapter(adap, &client->dev, client,
- force, num, class, pca954x_select_chan,
- (idle_disconnect_pd || idle_disconnect_dt)
- ? pca954x_deselect_mux : NULL);
+ ret = i2c_mux_add_adapter(muxc, force, num, class);
- if (data->virt_adaps[num] == NULL) {
- ret = -ENODEV;
+ if (ret) {
dev_err(&client->dev,
"failed to register multiplexed adapter"
" %d as bus %d\n", num, force);
@@ -263,23 +269,15 @@ static int pca954x_probe(struct i2c_client *client,
return 0;
virt_reg_failed:
- for (num--; num >= 0; num--)
- i2c_del_mux_adapter(data->virt_adaps[num]);
+ i2c_mux_del_adapters(muxc);
return ret;
}
static int pca954x_remove(struct i2c_client *client)
{
- struct pca954x *data = i2c_get_clientdata(client);
- const struct chip_desc *chip = &chips[data->type];
- int i;
-
- for (i = 0; i < chip->nchans; ++i)
- if (data->virt_adaps[i]) {
- i2c_del_mux_adapter(data->virt_adaps[i]);
- data->virt_adaps[i] = NULL;
- }
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+ i2c_mux_del_adapters(muxc);
return 0;
}
@@ -287,7 +285,8 @@ static int pca954x_remove(struct i2c_client *client)
static int pca954x_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
- struct pca954x *data = i2c_get_clientdata(client);
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+ struct pca954x *data = i2c_mux_priv(muxc);
data->last_chan = 0;
return i2c_smbus_write_byte(client, 0);
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 07/24] i2c: i2c-mux-reg: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (5 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 06/24] i2c: i2c-mux-pca954x: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 08/24] iio: imu: inv_mpu6050: " Peter Rosin
` (19 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-reg.c | 69 +++++++++++++++--------------------------
1 file changed, 25 insertions(+), 44 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-reg.c b/drivers/i2c/muxes/i2c-mux-reg.c
index 5fbd5bd0878f..6773cadf7c9f 100644
--- a/drivers/i2c/muxes/i2c-mux-reg.c
+++ b/drivers/i2c/muxes/i2c-mux-reg.c
@@ -21,8 +21,6 @@
#include <linux/slab.h>
struct regmux {
- struct i2c_adapter *parent;
- struct i2c_adapter **adap; /* child busses */
struct i2c_mux_reg_platform_data data;
};
@@ -64,18 +62,16 @@ static int i2c_mux_reg_set(const struct regmux *mux, unsigned int chan_id)
return 0;
}
-static int i2c_mux_reg_select(struct i2c_adapter *adap, void *data,
- unsigned int chan)
+static int i2c_mux_reg_select(struct i2c_mux_core *muxc, u32 chan)
{
- struct regmux *mux = data;
+ struct regmux *mux = i2c_mux_priv(muxc);
return i2c_mux_reg_set(mux, chan);
}
-static int i2c_mux_reg_deselect(struct i2c_adapter *adap, void *data,
- unsigned int chan)
+static int i2c_mux_reg_deselect(struct i2c_mux_core *muxc, u32 chan)
{
- struct regmux *mux = data;
+ struct regmux *mux = i2c_mux_priv(muxc);
if (mux->data.idle_in_use)
return i2c_mux_reg_set(mux, mux->data.idle);
@@ -85,7 +81,7 @@ static int i2c_mux_reg_deselect(struct i2c_adapter *adap, void *data,
#ifdef CONFIG_OF
static int i2c_mux_reg_probe_dt(struct regmux *mux,
- struct platform_device *pdev)
+ struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct device_node *adapter_np, *child;
@@ -107,7 +103,6 @@ static int i2c_mux_reg_probe_dt(struct regmux *mux,
if (!adapter)
return -EPROBE_DEFER;
- mux->parent = adapter;
mux->data.parent = i2c_adapter_id(adapter);
put_device(&adapter->dev);
@@ -161,7 +156,7 @@ static int i2c_mux_reg_probe_dt(struct regmux *mux,
}
#else
static int i2c_mux_reg_probe_dt(struct regmux *mux,
- struct platform_device *pdev)
+ struct platform_device *pdev)
{
return 0;
}
@@ -169,10 +164,10 @@ static int i2c_mux_reg_probe_dt(struct regmux *mux,
static int i2c_mux_reg_probe(struct platform_device *pdev)
{
+ struct i2c_mux_core *muxc;
struct regmux *mux;
struct i2c_adapter *parent;
struct resource *res;
- int (*deselect)(struct i2c_adapter *, void *, u32);
unsigned int class;
int i, ret, nr;
@@ -180,17 +175,9 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
if (!mux)
return -ENOMEM;
- platform_set_drvdata(pdev, mux);
-
if (dev_get_platdata(&pdev->dev)) {
memcpy(&mux->data, dev_get_platdata(&pdev->dev),
sizeof(mux->data));
-
- parent = i2c_get_adapter(mux->data.parent);
- if (!parent)
- return -EPROBE_DEFER;
-
- mux->parent = parent;
} else {
ret = i2c_mux_reg_probe_dt(mux, pdev);
if (ret < 0) {
@@ -199,6 +186,10 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
}
}
+ parent = i2c_get_adapter(mux->data.parent);
+ if (!parent)
+ return -EPROBE_DEFER;
+
if (!mux->data.reg) {
dev_info(&pdev->dev,
"Register not set, using platform resource\n");
@@ -215,55 +206,45 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
return -EINVAL;
}
- mux->adap = devm_kzalloc(&pdev->dev,
- sizeof(*mux->adap) * mux->data.n_values,
- GFP_KERNEL);
- if (!mux->adap) {
- dev_err(&pdev->dev, "Cannot allocate i2c_adapter structure");
+ muxc = i2c_mux_alloc(parent, &pdev->dev, mux->data.n_values, 0, 0,
+ i2c_mux_reg_select, NULL);
+ if (!muxc)
return -ENOMEM;
- }
+ muxc->priv = mux;
+
+ platform_set_drvdata(pdev, muxc);
if (mux->data.idle_in_use)
- deselect = i2c_mux_reg_deselect;
- else
- deselect = NULL;
+ muxc->deselect = i2c_mux_reg_deselect;
for (i = 0; i < mux->data.n_values; i++) {
nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
class = mux->data.classes ? mux->data.classes[i] : 0;
- mux->adap[i] = i2c_add_mux_adapter(mux->parent, &pdev->dev, mux,
- nr, mux->data.values[i],
- class, i2c_mux_reg_select,
- deselect);
- if (!mux->adap[i]) {
- ret = -ENODEV;
+ ret = i2c_mux_add_adapter(muxc, nr, mux->data.values[i], class);
+ if (ret) {
dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
goto add_adapter_failed;
}
}
dev_dbg(&pdev->dev, "%d port mux on %s adapter\n",
- mux->data.n_values, mux->parent->name);
+ mux->data.n_values, muxc->parent->name);
return 0;
add_adapter_failed:
- for (; i > 0; i--)
- i2c_del_mux_adapter(mux->adap[i - 1]);
+ i2c_mux_del_adapters(muxc);
return ret;
}
static int i2c_mux_reg_remove(struct platform_device *pdev)
{
- struct regmux *mux = platform_get_drvdata(pdev);
- int i;
-
- for (i = 0; i < mux->data.n_values; i++)
- i2c_del_mux_adapter(mux->adap[i]);
+ struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
- i2c_put_adapter(mux->parent);
+ i2c_mux_del_adapters(muxc);
+ i2c_put_adapter(muxc->parent);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (6 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 07/24] i2c: i2c-mux-reg: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-22 11:37 ` [PATCH v8 " Peter Rosin
2016-04-20 15:17 ` [PATCH v7 09/24] [media] m88ds3103: " Peter Rosin
` (18 subsequent siblings)
26 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 35 +++++++++++++++---------------
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++-
4 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index 2771106fd650..f62b8bd9ad7e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -183,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
} else
return 0; /* no secondary addr, which is OK */
}
- st->mux_client = i2c_new_device(st->mux_adapter, &info);
+ st->mux_client = i2c_new_device(st->muxc->adapter[0], &info);
if (!st->mux_client)
return -ENODEV;
}
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index d192953e9a38..0c2bded2b5b7 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -23,7 +23,6 @@
#include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <linux/iio/iio.h>
-#include <linux/i2c-mux.h>
#include <linux/acpi.h>
#include "inv_mpu_iio.h"
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index f581256d9d4c..664a45082d39 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -15,7 +15,6 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/i2c.h>
-#include <linux/i2c-mux.h>
#include <linux/iio/iio.h>
#include <linux/module.h>
#include "inv_mpu_iio.h"
@@ -52,10 +51,9 @@ static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
return 0;
}
-static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
- u32 chan_id)
+static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
int ret = 0;
@@ -84,10 +82,9 @@ write_error:
return ret;
}
-static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
- void *mux_priv, u32 chan_id)
+static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
@@ -136,16 +133,18 @@ static int inv_mpu_probe(struct i2c_client *client,
return result;
st = iio_priv(dev_get_drvdata(&client->dev));
- st->mux_adapter = i2c_add_mux_adapter(client->adapter,
- &client->dev,
- client,
- 0, 0, 0,
- inv_mpu6050_select_bypass,
- inv_mpu6050_deselect_bypass);
- if (!st->mux_adapter) {
- result = -ENODEV;
+ st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
+ 1, 0, 0,
+ inv_mpu6050_select_bypass,
+ inv_mpu6050_deselect_bypass);
+ if (!st->muxc) {
+ result = -ENOMEM;
goto out_unreg_device;
}
+ st->muxc->priv = dev_get_drvdata(&client->dev);
+ result = i2c_mux_add_adapter(st->muxc, 0, 0, 0);
+ if (result)
+ goto out_unreg_device;
result = inv_mpu_acpi_create_mux_client(client);
if (result)
@@ -154,7 +153,7 @@ static int inv_mpu_probe(struct i2c_client *client,
return 0;
out_del_mux:
- i2c_del_mux_adapter(st->mux_adapter);
+ i2c_mux_del_adapters(st->muxc);
out_unreg_device:
inv_mpu_core_remove(&client->dev);
return result;
@@ -162,11 +161,11 @@ out_unreg_device:
static int inv_mpu_remove(struct i2c_client *client)
{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
inv_mpu_acpi_delete_mux_client(client);
- i2c_del_mux_adapter(st->mux_adapter);
+ i2c_mux_del_adapters(st->muxc);
return inv_mpu_core_remove(&client->dev);
}
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e302a49703bf..bb3cef6d7059 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -11,6 +11,7 @@
* GNU General Public License for more details.
*/
#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
#include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <linux/iio/iio.h>
@@ -127,7 +128,7 @@ struct inv_mpu6050_state {
const struct inv_mpu6050_hw *hw;
enum inv_devices chip_type;
spinlock_t time_stamp_lock;
- struct i2c_adapter *mux_adapter;
+ struct i2c_mux_core *muxc;
struct i2c_client *mux_client;
unsigned int powerup_count;
struct inv_mpu6050_platform_data plat_data;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v8 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core
2016-04-20 15:17 ` [PATCH v7 08/24] iio: imu: inv_mpu6050: " Peter Rosin
@ 2016-04-22 11:37 ` Peter Rosin
0 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-22 11:37 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 33 +++++++++++++++---------------
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++-
4 files changed, 19 insertions(+), 20 deletions(-)
v8 compared to v7:
- Do not change i2c_get_clientdata() into dev_get_drvdata()
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index 2771106fd650..f62b8bd9ad7e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -183,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
} else
return 0; /* no secondary addr, which is OK */
}
- st->mux_client = i2c_new_device(st->mux_adapter, &info);
+ st->mux_client = i2c_new_device(st->muxc->adapter[0], &info);
if (!st->mux_client)
return -ENODEV;
}
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index d192953e9a38..0c2bded2b5b7 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -23,7 +23,6 @@
#include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <linux/iio/iio.h>
-#include <linux/i2c-mux.h>
#include <linux/acpi.h>
#include "inv_mpu_iio.h"
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index f581256d9d4c..3a078df84224 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -15,7 +15,6 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/i2c.h>
-#include <linux/i2c-mux.h>
#include <linux/iio/iio.h>
#include <linux/module.h>
#include "inv_mpu_iio.h"
@@ -52,10 +51,9 @@ static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
return 0;
}
-static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
- u32 chan_id)
+static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
int ret = 0;
@@ -84,10 +82,9 @@ write_error:
return ret;
}
-static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
- void *mux_priv, u32 chan_id)
+static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
@@ -136,16 +133,18 @@ static int inv_mpu_probe(struct i2c_client *client,
return result;
st = iio_priv(dev_get_drvdata(&client->dev));
- st->mux_adapter = i2c_add_mux_adapter(client->adapter,
- &client->dev,
- client,
- 0, 0, 0,
- inv_mpu6050_select_bypass,
- inv_mpu6050_deselect_bypass);
- if (!st->mux_adapter) {
- result = -ENODEV;
+ st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
+ 1, 0, 0,
+ inv_mpu6050_select_bypass,
+ inv_mpu6050_deselect_bypass);
+ if (!st->muxc) {
+ result = -ENOMEM;
goto out_unreg_device;
}
+ st->muxc->priv = dev_get_drvdata(&client->dev);
+ result = i2c_mux_add_adapter(st->muxc, 0, 0, 0);
+ if (result)
+ goto out_unreg_device;
result = inv_mpu_acpi_create_mux_client(client);
if (result)
@@ -154,7 +153,7 @@ static int inv_mpu_probe(struct i2c_client *client,
return 0;
out_del_mux:
- i2c_del_mux_adapter(st->mux_adapter);
+ i2c_mux_del_adapters(st->muxc);
out_unreg_device:
inv_mpu_core_remove(&client->dev);
return result;
@@ -166,7 +165,7 @@ static int inv_mpu_remove(struct i2c_client *client)
struct inv_mpu6050_state *st = iio_priv(indio_dev);
inv_mpu_acpi_delete_mux_client(client);
- i2c_del_mux_adapter(st->mux_adapter);
+ i2c_mux_del_adapters(st->muxc);
return inv_mpu_core_remove(&client->dev);
}
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e302a49703bf..bb3cef6d7059 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -11,6 +11,7 @@
* GNU General Public License for more details.
*/
#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
#include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <linux/iio/iio.h>
@@ -127,7 +128,7 @@ struct inv_mpu6050_state {
const struct inv_mpu6050_hw *hw;
enum inv_devices chip_type;
spinlock_t time_stamp_lock;
- struct i2c_adapter *mux_adapter;
+ struct i2c_mux_core *muxc;
struct i2c_client *mux_client;
unsigned int powerup_count;
struct inv_mpu6050_platform_data plat_data;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v7 09/24] [media] m88ds3103: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (7 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 08/24] iio: imu: inv_mpu6050: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 10/24] [media] rtl2830: " Peter Rosin
` (17 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select op to be in terms of the i2c mux core instead
of the child adapter.
Reviewed-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/dvb-frontends/m88ds3103.c | 19 +++++++++++--------
drivers/media/dvb-frontends/m88ds3103_priv.h | 2 +-
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index 76883600ec6f..5557ef8fc704 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -1251,9 +1251,9 @@ static void m88ds3103_release(struct dvb_frontend *fe)
i2c_unregister_device(client);
}
-static int m88ds3103_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
+static int m88ds3103_select(struct i2c_mux_core *muxc, u32 chan)
{
- struct m88ds3103_dev *dev = mux_priv;
+ struct m88ds3103_dev *dev = i2c_mux_priv(muxc);
struct i2c_client *client = dev->client;
int ret;
struct i2c_msg msg = {
@@ -1374,7 +1374,7 @@ static struct i2c_adapter *m88ds3103_get_i2c_adapter(struct i2c_client *client)
dev_dbg(&client->dev, "\n");
- return dev->i2c_adapter;
+ return dev->muxc->adapter[0];
}
static int m88ds3103_probe(struct i2c_client *client,
@@ -1467,13 +1467,16 @@ static int m88ds3103_probe(struct i2c_client *client,
goto err_kfree;
/* create mux i2c adapter for tuner */
- dev->i2c_adapter = i2c_add_mux_adapter(client->adapter, &client->dev,
- dev, 0, 0, 0, m88ds3103_select,
- NULL);
- if (dev->i2c_adapter == NULL) {
+ dev->muxc = i2c_mux_alloc(client->adapter, &client->dev, 1, 0, 0,
+ m88ds3103_select, NULL);
+ if (!dev->muxc) {
ret = -ENOMEM;
goto err_kfree;
}
+ dev->muxc->priv = dev;
+ ret = i2c_mux_add_adapter(dev->muxc, 0, 0, 0);
+ if (ret)
+ goto err_kfree;
/* create dvb_frontend */
memcpy(&dev->fe.ops, &m88ds3103_ops, sizeof(struct dvb_frontend_ops));
@@ -1502,7 +1505,7 @@ static int m88ds3103_remove(struct i2c_client *client)
dev_dbg(&client->dev, "\n");
- i2c_del_mux_adapter(dev->i2c_adapter);
+ i2c_mux_del_adapters(dev->muxc);
kfree(dev);
return 0;
diff --git a/drivers/media/dvb-frontends/m88ds3103_priv.h b/drivers/media/dvb-frontends/m88ds3103_priv.h
index eee8c22c51ec..c5b4e177c6ea 100644
--- a/drivers/media/dvb-frontends/m88ds3103_priv.h
+++ b/drivers/media/dvb-frontends/m88ds3103_priv.h
@@ -42,7 +42,7 @@ struct m88ds3103_dev {
enum fe_status fe_status;
u32 dvbv3_ber; /* for old DVBv3 API read_ber */
bool warm; /* FW running */
- struct i2c_adapter *i2c_adapter;
+ struct i2c_mux_core *muxc;
/* auto detect chip id to do different config */
u8 chip_id;
/* main mclk is calculated for M88RS6000 dynamically */
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 10/24] [media] rtl2830: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (8 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 09/24] [media] m88ds3103: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 11/24] [media] rtl2832: " Peter Rosin
` (16 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select op to be in terms of the i2c mux core instead
of the child adapter.
Reviewed-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/dvb-frontends/rtl2830.c | 20 ++++++++++++--------
drivers/media/dvb-frontends/rtl2830_priv.h | 2 +-
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
index 3f96429af0e5..d25d1e0cd4ca 100644
--- a/drivers/media/dvb-frontends/rtl2830.c
+++ b/drivers/media/dvb-frontends/rtl2830.c
@@ -677,9 +677,9 @@ err:
* adapter lock is already taken by tuner driver.
* Gate is closed automatically after single I2C transfer.
*/
-static int rtl2830_select(struct i2c_adapter *adap, void *mux_priv, u32 chan_id)
+static int rtl2830_select(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct rtl2830_dev *dev = i2c_get_clientdata(client);
int ret;
@@ -712,7 +712,7 @@ static struct i2c_adapter *rtl2830_get_i2c_adapter(struct i2c_client *client)
dev_dbg(&client->dev, "\n");
- return dev->adapter;
+ return dev->muxc->adapter[0];
}
/*
@@ -865,12 +865,16 @@ static int rtl2830_probe(struct i2c_client *client,
goto err_regmap_exit;
/* create muxed i2c adapter for tuner */
- dev->adapter = i2c_add_mux_adapter(client->adapter, &client->dev,
- client, 0, 0, 0, rtl2830_select, NULL);
- if (dev->adapter == NULL) {
- ret = -ENODEV;
+ dev->muxc = i2c_mux_alloc(client->adapter, &client->dev, 1, 0, 0,
+ rtl2830_select, NULL);
+ if (!dev->muxc) {
+ ret = -ENOMEM;
goto err_regmap_exit;
}
+ dev->muxc->priv = client;
+ ret = i2c_mux_add_adapter(dev->muxc, 0, 0, 0);
+ if (ret)
+ goto err_regmap_exit;
/* create dvb frontend */
memcpy(&dev->fe.ops, &rtl2830_ops, sizeof(dev->fe.ops));
@@ -903,7 +907,7 @@ static int rtl2830_remove(struct i2c_client *client)
/* stop statistics polling */
cancel_delayed_work_sync(&dev->stat_work);
- i2c_del_mux_adapter(dev->adapter);
+ i2c_mux_del_adapters(dev->muxc);
regmap_exit(dev->regmap);
kfree(dev);
diff --git a/drivers/media/dvb-frontends/rtl2830_priv.h b/drivers/media/dvb-frontends/rtl2830_priv.h
index cf793f39a09b..da4909543da2 100644
--- a/drivers/media/dvb-frontends/rtl2830_priv.h
+++ b/drivers/media/dvb-frontends/rtl2830_priv.h
@@ -29,7 +29,7 @@ struct rtl2830_dev {
struct rtl2830_platform_data *pdata;
struct i2c_client *client;
struct regmap *regmap;
- struct i2c_adapter *adapter;
+ struct i2c_mux_core *muxc;
struct dvb_frontend fe;
bool sleeping;
unsigned long filters;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 11/24] [media] rtl2832: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (9 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 10/24] [media] rtl2830: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 12/24] [media] si2168: " Peter Rosin
` (15 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Reviewed-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/dvb-frontends/rtl2832.c | 25 ++++++++++++++-----------
drivers/media/dvb-frontends/rtl2832_priv.h | 2 +-
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 7c96f7679669..1b23788797b5 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -847,9 +847,9 @@ err:
dev_dbg(&client->dev, "failed=%d\n", ret);
}
-static int rtl2832_select(struct i2c_adapter *adap, void *mux_priv, u32 chan_id)
+static int rtl2832_select(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct rtl2832_dev *dev = mux_priv;
+ struct rtl2832_dev *dev = i2c_mux_priv(muxc);
struct i2c_client *client = dev->client;
int ret;
@@ -870,10 +870,9 @@ err:
return ret;
}
-static int rtl2832_deselect(struct i2c_adapter *adap, void *mux_priv,
- u32 chan_id)
+static int rtl2832_deselect(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct rtl2832_dev *dev = mux_priv;
+ struct rtl2832_dev *dev = i2c_mux_priv(muxc);
schedule_delayed_work(&dev->i2c_gate_work, usecs_to_jiffies(100));
return 0;
@@ -1059,7 +1058,7 @@ static struct i2c_adapter *rtl2832_get_i2c_adapter(struct i2c_client *client)
struct rtl2832_dev *dev = i2c_get_clientdata(client);
dev_dbg(&client->dev, "\n");
- return dev->i2c_adapter_tuner;
+ return dev->muxc->adapter[0];
}
static int rtl2832_slave_ts_ctrl(struct i2c_client *client, bool enable)
@@ -1242,12 +1241,16 @@ static int rtl2832_probe(struct i2c_client *client,
goto err_regmap_exit;
/* create muxed i2c adapter for demod tuner bus */
- dev->i2c_adapter_tuner = i2c_add_mux_adapter(i2c, &i2c->dev, dev,
- 0, 0, 0, rtl2832_select, rtl2832_deselect);
- if (dev->i2c_adapter_tuner == NULL) {
- ret = -ENODEV;
+ dev->muxc = i2c_mux_alloc(i2c, &i2c->dev, 1, 0, 0,
+ rtl2832_select, rtl2832_deselect);
+ if (!dev->muxc) {
+ ret = -ENOMEM;
goto err_regmap_exit;
}
+ dev->muxc->priv = dev;
+ ret = i2c_mux_add_adapter(dev->muxc, 0, 0, 0);
+ if (ret)
+ goto err_regmap_exit;
/* create dvb_frontend */
memcpy(&dev->fe.ops, &rtl2832_ops, sizeof(struct dvb_frontend_ops));
@@ -1282,7 +1285,7 @@ static int rtl2832_remove(struct i2c_client *client)
cancel_delayed_work_sync(&dev->i2c_gate_work);
- i2c_del_mux_adapter(dev->i2c_adapter_tuner);
+ i2c_mux_del_adapters(dev->muxc);
regmap_exit(dev->regmap);
diff --git a/drivers/media/dvb-frontends/rtl2832_priv.h b/drivers/media/dvb-frontends/rtl2832_priv.h
index 6b875f462f8b..d8f97d14f6fd 100644
--- a/drivers/media/dvb-frontends/rtl2832_priv.h
+++ b/drivers/media/dvb-frontends/rtl2832_priv.h
@@ -36,7 +36,7 @@ struct rtl2832_dev {
struct mutex regmap_mutex;
struct regmap_config regmap_config;
struct regmap *regmap;
- struct i2c_adapter *i2c_adapter_tuner;
+ struct i2c_mux_core *muxc;
struct dvb_frontend fe;
enum fe_status fe_status;
u64 post_bit_error_prev; /* for old DVBv3 read_ber() calculation */
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 12/24] [media] si2168: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (10 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 11/24] [media] rtl2832: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 13/24] [media] cx231xx: " Peter Rosin
` (14 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Reviewed-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/dvb-frontends/si2168.c | 25 +++++++++++++++----------
drivers/media/dvb-frontends/si2168_priv.h | 2 +-
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 821a8f481507..5583827c386e 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -615,9 +615,9 @@ static int si2168_get_tune_settings(struct dvb_frontend *fe,
* We must use unlocked I2C I/O because I2C adapter lock is already taken
* by the caller (usually tuner driver).
*/
-static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
+static int si2168_select(struct i2c_mux_core *muxc, u32 chan)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
int ret;
struct si2168_cmd cmd;
@@ -635,9 +635,9 @@ err:
return ret;
}
-static int si2168_deselect(struct i2c_adapter *adap, void *mux_priv, u32 chan)
+static int si2168_deselect(struct i2c_mux_core *muxc, u32 chan)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
int ret;
struct si2168_cmd cmd;
@@ -709,17 +709,22 @@ static int si2168_probe(struct i2c_client *client,
}
/* create mux i2c adapter for tuner */
- dev->adapter = i2c_add_mux_adapter(client->adapter, &client->dev,
- client, 0, 0, 0, si2168_select, si2168_deselect);
- if (dev->adapter == NULL) {
- ret = -ENODEV;
+ dev->muxc = i2c_mux_alloc(client->adapter, &client->dev,
+ 1, 0, 0,
+ si2168_select, si2168_deselect);
+ if (!dev->muxc) {
+ ret = -ENOMEM;
goto err_kfree;
}
+ dev->muxc->priv = client;
+ ret = i2c_mux_add_adapter(dev->muxc, 0, 0, 0);
+ if (ret)
+ goto err_kfree;
/* create dvb_frontend */
memcpy(&dev->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
dev->fe.demodulator_priv = client;
- *config->i2c_adapter = dev->adapter;
+ *config->i2c_adapter = dev->muxc->adapter[0];
*config->fe = &dev->fe;
dev->ts_mode = config->ts_mode;
dev->ts_clock_inv = config->ts_clock_inv;
@@ -743,7 +748,7 @@ static int si2168_remove(struct i2c_client *client)
dev_dbg(&client->dev, "\n");
- i2c_del_mux_adapter(dev->adapter);
+ i2c_mux_del_adapters(dev->muxc);
dev->fe.ops.release = NULL;
dev->fe.demodulator_priv = NULL;
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index c07e6fe2cb10..165bf1412063 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -29,7 +29,7 @@
/* state struct */
struct si2168_dev {
- struct i2c_adapter *adapter;
+ struct i2c_mux_core *muxc;
struct dvb_frontend fe;
enum fe_delivery_system delivery_system;
enum fe_status fe_status;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 13/24] [media] cx231xx: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (11 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 12/24] [media] si2168: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 14/24] of/unittest: " Peter Rosin
` (13 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select op to be in terms of the i2c mux core instead
of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/usb/cx231xx/cx231xx-core.c | 6 ++--
drivers/media/usb/cx231xx/cx231xx-i2c.c | 47 ++++++++++++++++----------------
drivers/media/usb/cx231xx/cx231xx.h | 4 ++-
3 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
index f497888d94bf..f7aac2abd783 100644
--- a/drivers/media/usb/cx231xx/cx231xx-core.c
+++ b/drivers/media/usb/cx231xx/cx231xx-core.c
@@ -1304,6 +1304,9 @@ int cx231xx_dev_init(struct cx231xx *dev)
cx231xx_i2c_register(&dev->i2c_bus[1]);
cx231xx_i2c_register(&dev->i2c_bus[2]);
+ errCode = cx231xx_i2c_mux_create(dev);
+ if (errCode < 0)
+ return errCode;
cx231xx_i2c_mux_register(dev, 0);
cx231xx_i2c_mux_register(dev, 1);
@@ -1426,8 +1429,7 @@ EXPORT_SYMBOL_GPL(cx231xx_dev_init);
void cx231xx_dev_uninit(struct cx231xx *dev)
{
/* Un Initialize I2C bus */
- cx231xx_i2c_mux_unregister(dev, 1);
- cx231xx_i2c_mux_unregister(dev, 0);
+ cx231xx_i2c_mux_unregister(dev);
cx231xx_i2c_unregister(&dev->i2c_bus[2]);
cx231xx_i2c_unregister(&dev->i2c_bus[1]);
cx231xx_i2c_unregister(&dev->i2c_bus[0]);
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index a29c345b027d..473cd3433fe5 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -557,40 +557,41 @@ int cx231xx_i2c_unregister(struct cx231xx_i2c *bus)
* cx231xx_i2c_mux_select()
* switch i2c master number 1 between port1 and port3
*/
-static int cx231xx_i2c_mux_select(struct i2c_adapter *adap,
- void *mux_priv, u32 chan_id)
+static int cx231xx_i2c_mux_select(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct cx231xx *dev = mux_priv;
+ struct cx231xx *dev = i2c_mux_priv(muxc);
return cx231xx_enable_i2c_port_3(dev, chan_id);
}
+int cx231xx_i2c_mux_create(struct cx231xx *dev)
+{
+ dev->muxc = i2c_mux_alloc(&dev->i2c_bus[1].i2c_adap, dev->dev, 2, 0, 0,
+ cx231xx_i2c_mux_select, NULL);
+ if (!dev->muxc)
+ return -ENOMEM;
+ dev->muxc->priv = dev;
+ return 0;
+}
+
int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no)
{
- struct i2c_adapter *i2c_parent = &dev->i2c_bus[1].i2c_adap;
- /* what is the correct mux_dev? */
- struct device *mux_dev = dev->dev;
-
- dev->i2c_mux_adap[mux_no] = i2c_add_mux_adapter(i2c_parent,
- mux_dev,
- dev /* mux_priv */,
- 0,
- mux_no /* chan_id */,
- 0 /* class */,
- &cx231xx_i2c_mux_select,
- NULL);
-
- if (!dev->i2c_mux_adap[mux_no])
+ int rc;
+
+ rc = i2c_mux_add_adapter(dev->muxc,
+ 0,
+ mux_no /* chan_id */,
+ 0 /* class */);
+ if (rc)
dev_warn(dev->dev,
"i2c mux %d register FAILED\n", mux_no);
- return 0;
+ return rc;
}
-void cx231xx_i2c_mux_unregister(struct cx231xx *dev, int mux_no)
+void cx231xx_i2c_mux_unregister(struct cx231xx *dev)
{
- i2c_del_mux_adapter(dev->i2c_mux_adap[mux_no]);
- dev->i2c_mux_adap[mux_no] = NULL;
+ i2c_mux_del_adapters(dev->muxc);
}
struct i2c_adapter *cx231xx_get_i2c_adap(struct cx231xx *dev, int i2c_port)
@@ -603,9 +604,9 @@ struct i2c_adapter *cx231xx_get_i2c_adap(struct cx231xx *dev, int i2c_port)
case I2C_2:
return &dev->i2c_bus[2].i2c_adap;
case I2C_1_MUX_1:
- return dev->i2c_mux_adap[0];
+ return dev->muxc->adapter[0];
case I2C_1_MUX_3:
- return dev->i2c_mux_adap[1];
+ return dev->muxc->adapter[1];
default:
return NULL;
}
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 69f6d20870f5..90c867683076 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -624,6 +624,7 @@ struct cx231xx {
/* I2C adapters: Master 1 & 2 (External) & Master 3 (Internal only) */
struct cx231xx_i2c i2c_bus[3];
+ struct i2c_mux_core *muxc;
struct i2c_adapter *i2c_mux_adap[2];
unsigned int xc_fw_load_done:1;
@@ -760,8 +761,9 @@ int cx231xx_reset_analog_tuner(struct cx231xx *dev);
void cx231xx_do_i2c_scan(struct cx231xx *dev, int i2c_port);
int cx231xx_i2c_register(struct cx231xx_i2c *bus);
int cx231xx_i2c_unregister(struct cx231xx_i2c *bus);
+int cx231xx_i2c_mux_create(struct cx231xx *dev);
int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no);
-void cx231xx_i2c_mux_unregister(struct cx231xx *dev, int mux_no);
+void cx231xx_i2c_mux_unregister(struct cx231xx *dev);
struct i2c_adapter *cx231xx_get_i2c_adap(struct cx231xx *dev, int i2c_port);
/* Internal block control functions */
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 14/24] of/unittest: convert to use an explicit i2c mux core
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (12 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 13/24] [media] cx231xx: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 15/24] i2c-mux: drop old unused i2c-mux api Peter Rosin
` (12 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select op to be in terms of the i2c mux core instead
of the child adapter.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/of/unittest.c | 37 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index e986e6ee52e0..c1ebbfb79453 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1692,13 +1692,7 @@ static struct i2c_driver unittest_i2c_dev_driver = {
#if IS_BUILTIN(CONFIG_I2C_MUX)
-struct unittest_i2c_mux_data {
- int nchans;
- struct i2c_adapter *adap[];
-};
-
-static int unittest_i2c_mux_select_chan(struct i2c_adapter *adap,
- void *client, u32 chan)
+static int unittest_i2c_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
{
return 0;
}
@@ -1706,11 +1700,11 @@ static int unittest_i2c_mux_select_chan(struct i2c_adapter *adap,
static int unittest_i2c_mux_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- int ret, i, nchans, size;
+ int ret, i, nchans;
struct device *dev = &client->dev;
struct i2c_adapter *adap = to_i2c_adapter(dev->parent);
struct device_node *np = client->dev.of_node, *child;
- struct unittest_i2c_mux_data *stm;
+ struct i2c_mux_core *muxc;
u32 reg, max_reg;
dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name);
@@ -1734,25 +1728,20 @@ static int unittest_i2c_mux_probe(struct i2c_client *client,
return -EINVAL;
}
- size = offsetof(struct unittest_i2c_mux_data, adap[nchans]);
- stm = devm_kzalloc(dev, size, GFP_KERNEL);
- if (!stm) {
- dev_err(dev, "Out of memory\n");
+ muxc = i2c_mux_alloc(adap, dev, nchans, 0, 0,
+ unittest_i2c_mux_select_chan, NULL);
+ if (!muxc)
return -ENOMEM;
- }
- stm->nchans = nchans;
for (i = 0; i < nchans; i++) {
- stm->adap[i] = i2c_add_mux_adapter(adap, dev, client,
- 0, i, 0, unittest_i2c_mux_select_chan, NULL);
- if (!stm->adap[i]) {
+ ret = i2c_mux_add_adapter(muxc, 0, i, 0);
+ if (ret) {
dev_err(dev, "Failed to register mux #%d\n", i);
- for (i--; i >= 0; i--)
- i2c_del_mux_adapter(stm->adap[i]);
+ i2c_mux_del_adapters(muxc);
return -ENODEV;
}
}
- i2c_set_clientdata(client, stm);
+ i2c_set_clientdata(client, muxc);
return 0;
};
@@ -1761,12 +1750,10 @@ static int unittest_i2c_mux_remove(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct device_node *np = client->dev.of_node;
- struct unittest_i2c_mux_data *stm = i2c_get_clientdata(client);
- int i;
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name);
- for (i = stm->nchans - 1; i >= 0; i--)
- i2c_del_mux_adapter(stm->adap[i]);
+ i2c_mux_del_adapters(muxc);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 15/24] i2c-mux: drop old unused i2c-mux api
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (13 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 14/24] of/unittest: " Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking Peter Rosin
` (11 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
All i2c mux users are using an explicit i2c mux core, drop support
for implicit i2c mux cores.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/i2c-mux.c | 63 -------------------------------------------------
include/linux/i2c-mux.h | 15 ------------
2 files changed, 78 deletions(-)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 5ce1b0704cb5..25e9336b0e6e 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -28,12 +28,6 @@
#include <linux/slab.h>
/* multiplexer per channel data */
-struct i2c_mux_priv_old {
- void *mux_priv;
- int (*select)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
- int (*deselect)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
-};
-
struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
@@ -104,53 +98,6 @@ static unsigned int i2c_mux_parent_classes(struct i2c_adapter *parent)
return class;
}
-static int i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
-{
- struct i2c_mux_priv_old *priv = i2c_mux_priv(muxc);
-
- return priv->select(muxc->parent, priv->mux_priv, chan);
-}
-
-static int i2c_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
-{
- struct i2c_mux_priv_old *priv = i2c_mux_priv(muxc);
-
- return priv->deselect(muxc->parent, priv->mux_priv, chan);
-}
-
-struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
- struct device *mux_dev,
- void *mux_priv, u32 force_nr, u32 chan_id,
- unsigned int class,
- int (*select) (struct i2c_adapter *,
- void *, u32),
- int (*deselect) (struct i2c_adapter *,
- void *, u32))
-{
- struct i2c_mux_core *muxc;
- struct i2c_mux_priv_old *priv;
- int ret;
-
- muxc = i2c_mux_alloc(parent, mux_dev, 1, sizeof(*priv), 0,
- i2c_mux_select, i2c_mux_deselect);
- if (!muxc)
- return NULL;
-
- priv = i2c_mux_priv(muxc);
- priv->select = select;
- priv->deselect = deselect;
- priv->mux_priv = mux_priv;
-
- ret = i2c_mux_add_adapter(muxc, force_nr, chan_id, class);
- if (ret) {
- devm_kfree(mux_dev, muxc);
- return NULL;
- }
-
- return muxc->adapter[0];
-}
-EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
-
struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
struct device *dev, int max_adapters,
int sizeof_priv, u32 flags,
@@ -305,16 +252,6 @@ void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
}
EXPORT_SYMBOL_GPL(i2c_mux_del_adapters);
-void i2c_del_mux_adapter(struct i2c_adapter *adap)
-{
- struct i2c_mux_priv *priv = adap->algo_data;
- struct i2c_mux_core *muxc = priv->muxc;
-
- i2c_mux_del_adapters(muxc);
- devm_kfree(muxc->dev, muxc);
-}
-EXPORT_SYMBOL_GPL(i2c_del_mux_adapter);
-
MODULE_AUTHOR("Rodolfo Giometti <giometti@linux.it>");
MODULE_DESCRIPTION("I2C driver for multiplexed I2C busses");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 71ac1b3f4f68..2fa93fe1345e 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -53,20 +53,6 @@ static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
}
/*
- * Called to create a i2c bus on a multiplexed bus segment.
- * The mux_dev and chan_id parameters are passed to the select
- * and deselect callback functions to perform hardware-specific
- * mux control.
- */
-struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
- struct device *mux_dev,
- void *mux_priv, u32 force_nr, u32 chan_id,
- unsigned int class,
- int (*select) (struct i2c_adapter *,
- void *mux_dev, u32 chan_id),
- int (*deselect) (struct i2c_adapter *,
- void *mux_dev, u32 chan_id));
-/*
* Called to create an i2c bus on a multiplexed bus segment.
* The chan_id parameter is passed to the select and deselect
* callback functions to perform hardware-specific mux control.
@@ -75,7 +61,6 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
u32 force_nr, u32 chan_id,
unsigned int class);
-void i2c_del_mux_adapter(struct i2c_adapter *adap);
void i2c_mux_del_adapters(struct i2c_mux_core *muxc);
#endif /* __KERNEL__ */
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (14 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 15/24] i2c-mux: drop old unused i2c-mux api Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-28 20:50 ` Wolfram Sang
2016-05-03 21:38 ` Wolfram Sang
2016-04-20 15:17 ` [PATCH v7 17/24] i2c: muxes always lock the parent adapter Peter Rosin
` (10 subsequent siblings)
26 siblings, 2 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
unlock_bus ops in the adapter. These funcs/ops take an additional flags
argument that indicates for what purpose the adapter is locked.
There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
both implemented the same. For now. Locking the adapter means that the
whole bus is locked, locking the segment means that only the current bus
segment is locked (i.e. i2c traffic on the parent side of mux is still
allowed even if the child side of the mux is locked.
Also support a trylock_bus op (but no function to call it, as it is not
expected to be needed outside of the i2c core).
Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).
Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/i2c-core.c | 46 ++++++++++++++++++++++++++++------------------
include/linux/i2c.h | 28 ++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 0f2f8484e8ec..21f46d011c33 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -960,10 +960,12 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
}
/**
- * i2c_lock_adapter - Get exclusive access to an I2C bus segment
+ * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment
* @adapter: Target I2C bus segment
+ * @flags: I2C_LOCK_ADAPTER locks the root i2c adapter, I2C_LOCK_SEGMENT
+ * locks only this branch in the adapter tree
*/
-void i2c_lock_adapter(struct i2c_adapter *adapter)
+static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
@@ -972,27 +974,30 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
else
rt_mutex_lock(&adapter->bus_lock);
}
-EXPORT_SYMBOL_GPL(i2c_lock_adapter);
/**
- * i2c_trylock_adapter - Try to get exclusive access to an I2C bus segment
+ * i2c_adapter_trylock_bus - Try to get exclusive access to an I2C bus segment
* @adapter: Target I2C bus segment
+ * @flags: I2C_LOCK_ADAPTER trylocks the root i2c adapter, I2C_LOCK_SEGMENT
+ * trylocks only this branch in the adapter tree
*/
-static int i2c_trylock_adapter(struct i2c_adapter *adapter)
+static int i2c_adapter_trylock_bus(struct i2c_adapter *adapter, int flags)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
if (parent)
- return i2c_trylock_adapter(parent);
+ return parent->trylock_bus(parent, flags);
else
return rt_mutex_trylock(&adapter->bus_lock);
}
/**
- * i2c_unlock_adapter - Release exclusive access to an I2C bus segment
+ * i2c_adapter_unlock_bus - Release exclusive access to an I2C bus segment
* @adapter: Target I2C bus segment
+ * @flags: I2C_LOCK_ADAPTER unlocks the root i2c adapter, I2C_LOCK_SEGMENT
+ * unlocks only this branch in the adapter tree
*/
-void i2c_unlock_adapter(struct i2c_adapter *adapter)
+static void i2c_adapter_unlock_bus(struct i2c_adapter *adapter, int flags)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
@@ -1001,7 +1006,6 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
else
rt_mutex_unlock(&adapter->bus_lock);
}
-EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
static void i2c_dev_set_name(struct i2c_adapter *adap,
struct i2c_client *client)
@@ -1547,6 +1551,12 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
return -EINVAL;
}
+ if (!adap->lock_bus) {
+ adap->lock_bus = i2c_adapter_lock_bus;
+ adap->trylock_bus = i2c_adapter_trylock_bus;
+ adap->unlock_bus = i2c_adapter_unlock_bus;
+ }
+
rt_mutex_init(&adap->bus_lock);
mutex_init(&adap->userspace_clients_lock);
INIT_LIST_HEAD(&adap->userspace_clients);
@@ -2315,16 +2325,16 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
#endif
if (in_atomic() || irqs_disabled()) {
- ret = i2c_trylock_adapter(adap);
+ ret = adap->trylock_bus(adap, I2C_LOCK_SEGMENT);
if (!ret)
/* I2C activity is ongoing. */
return -EAGAIN;
} else {
- i2c_lock_adapter(adap);
+ i2c_lock_bus(adap, I2C_LOCK_SEGMENT);
}
ret = __i2c_transfer(adap, msgs, num);
- i2c_unlock_adapter(adap);
+ i2c_unlock_bus(adap, I2C_LOCK_SEGMENT);
return ret;
} else {
@@ -3099,7 +3109,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
if (adapter->algo->smbus_xfer) {
- i2c_lock_adapter(adapter);
+ i2c_lock_bus(adapter, I2C_LOCK_SEGMENT);
/* Retry automatically on arbitration loss */
orig_jiffies = jiffies;
@@ -3113,7 +3123,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
orig_jiffies + adapter->timeout))
break;
}
- i2c_unlock_adapter(adapter);
+ i2c_unlock_bus(adapter, I2C_LOCK_SEGMENT);
if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
goto trace;
@@ -3224,9 +3234,9 @@ int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
client->slave_cb = slave_cb;
- i2c_lock_adapter(client->adapter);
+ i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
ret = client->adapter->algo->reg_slave(client);
- i2c_unlock_adapter(client->adapter);
+ i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
if (ret) {
client->slave_cb = NULL;
@@ -3246,9 +3256,9 @@ int i2c_slave_unregister(struct i2c_client *client)
return -EOPNOTSUPP;
}
- i2c_lock_adapter(client->adapter);
+ i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
ret = client->adapter->algo->unreg_slave(client);
- i2c_unlock_adapter(client->adapter);
+ i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
if (ret == 0)
client->slave_cb = NULL;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 200cf13b00f6..c5f79fec1bfb 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -538,6 +538,10 @@ struct i2c_adapter {
struct i2c_bus_recovery_info *bus_recovery_info;
const struct i2c_adapter_quirks *quirks;
+
+ void (*lock_bus)(struct i2c_adapter *, int flags);
+ int (*trylock_bus)(struct i2c_adapter *, int flags);
+ void (*unlock_bus)(struct i2c_adapter *, int flags);
};
#define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
@@ -567,8 +571,28 @@ i2c_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
int i2c_for_each_dev(void *data, int (*fn)(struct device *, void *));
/* Adapter locking functions, exported for shared pin cases */
-void i2c_lock_adapter(struct i2c_adapter *);
-void i2c_unlock_adapter(struct i2c_adapter *);
+#define I2C_LOCK_ADAPTER 0x01
+#define I2C_LOCK_SEGMENT 0x02
+static inline void
+i2c_lock_bus(struct i2c_adapter *adapter, int flags)
+{
+ adapter->lock_bus(adapter, flags);
+}
+static inline void
+i2c_unlock_bus(struct i2c_adapter *adapter, int flags)
+{
+ adapter->unlock_bus(adapter, flags);
+}
+static inline void
+i2c_lock_adapter(struct i2c_adapter *adapter)
+{
+ i2c_lock_bus(adapter, I2C_LOCK_ADAPTER);
+}
+static inline void
+i2c_unlock_adapter(struct i2c_adapter *adapter)
+{
+ i2c_unlock_bus(adapter, I2C_LOCK_ADAPTER);
+}
/*flags for the client struct: */
#define I2C_CLIENT_PEC 0x04 /* Use Packet Error Checking */
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-20 15:17 ` [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking Peter Rosin
@ 2016-04-28 20:50 ` Wolfram Sang
2016-04-28 21:08 ` Peter Rosin
2016-05-03 21:38 ` Wolfram Sang
1 sibling, 1 reply; 47+ messages in thread
From: Wolfram Sang @ 2016-04-28 20:50 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]
On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote:
> Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
> unlock_bus ops in the adapter. These funcs/ops take an additional flags
> argument that indicates for what purpose the adapter is locked.
>
> There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
> both implemented the same. For now. Locking the adapter means that the
> whole bus is locked, locking the segment means that only the current bus
> segment is locked (i.e. i2c traffic on the parent side of mux is still
> allowed even if the child side of the mux is locked.
>
> Also support a trylock_bus op (but no function to call it, as it is not
> expected to be needed outside of the i2c core).
>
> Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
> scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).
>
> Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Letting you know that I start reviewing the 2nd part of your series. Did
the first glimpse today. Will hopefully do the in-depth part this
weekend. One thing already:
> +static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
Shouldn't flags be unsigned?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-28 20:50 ` Wolfram Sang
@ 2016-04-28 21:08 ` Peter Rosin
2016-04-29 7:16 ` Wolfram Sang
0 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-04-28 21:08 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
On 2016-04-28 22:50, Wolfram Sang wrote:
> On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote:
>> Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
>> unlock_bus ops in the adapter. These funcs/ops take an additional flags
>> argument that indicates for what purpose the adapter is locked.
>>
>> There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
>> both implemented the same. For now. Locking the adapter means that the
>> whole bus is locked, locking the segment means that only the current bus
>> segment is locked (i.e. i2c traffic on the parent side of mux is still
>> allowed even if the child side of the mux is locked.
>>
>> Also support a trylock_bus op (but no function to call it, as it is not
>> expected to be needed outside of the i2c core).
>>
>> Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
>> scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).
>>
>> Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> Letting you know that I start reviewing the 2nd part of your series. Did
> the first glimpse today. Will hopefully do the in-depth part this
> weekend. One thing already:
>
>> +static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
> Shouldn't flags be unsigned?
>
Yes, obviously... I'll make that change locally and wait for the rest.
Cheers,
Peter
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-28 21:08 ` Peter Rosin
@ 2016-04-29 7:16 ` Wolfram Sang
2016-04-29 9:16 ` Peter Rosin
0 siblings, 1 reply; 47+ messages in thread
From: Wolfram Sang @ 2016-04-29 7:16 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 336 bytes --]
> Yes, obviously... I'll make that change locally and wait for the rest.
Another nit: You could use '--strict' with checkpatch and see if you
want to fix the issues reported. I am not keen on those (except for
'space around operators'), it's a matter of taste I guess, but maybe you
like some of the suggestions.
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-29 7:16 ` Wolfram Sang
@ 2016-04-29 9:16 ` Peter Rosin
2016-05-02 19:20 ` Antti Palosaari
2016-05-03 21:39 ` Wolfram Sang
0 siblings, 2 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-29 9:16 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
On 2016-04-29 09:16, Wolfram Sang wrote:
>> Yes, obviously... I'll make that change locally and wait for the rest.
> Another nit: You could use '--strict' with checkpatch and see if you
> want to fix the issues reported. I am not keen on those (except for
> 'space around operators'), it's a matter of taste I guess, but maybe you
> like some of the suggestions.
>
Yes, they look like reasonable complaints.
So, I fixed all of them locally except the complaint about lack of comment
on the new struct mutex member in struct si2168_dev (patch 21/24),
because that patch is Anttis and he's the maintainer of that driver...
Antti, if you want that fixed as part of this series, send a suitable comment
for the mutex this way and I'll incorporate it.
Cheers,
Peter
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-29 9:16 ` Peter Rosin
@ 2016-05-02 19:20 ` Antti Palosaari
2016-05-03 21:39 ` Wolfram Sang
1 sibling, 0 replies; 47+ messages in thread
From: Antti Palosaari @ 2016-05-02 19:20 UTC (permalink / raw)
To: Peter Rosin, Wolfram Sang
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Mauro Carvalho Chehab, Rob Herring, Frank Rowand,
Grant Likely, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Kalle Valo, Jiri Slaby, Daniel Baluta, Lucas De Marchi,
Adriana Reus, Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil,
Terry Heo, Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard,
linux-i2c, linux-doc, linux-iio, linux-media, devicetree,
Peter Rosin
On 04/29/2016 12:16 PM, Peter Rosin wrote:
> On 2016-04-29 09:16, Wolfram Sang wrote:
>>> Yes, obviously... I'll make that change locally and wait for the rest.
>> Another nit: You could use '--strict' with checkpatch and see if you
>> want to fix the issues reported. I am not keen on those (except for
>> 'space around operators'), it's a matter of taste I guess, but maybe you
>> like some of the suggestions.
>>
> Yes, they look like reasonable complaints.
>
> So, I fixed all of them locally except the complaint about lack of comment
> on the new struct mutex member in struct si2168_dev (patch 21/24),
> because that patch is Anttis and he's the maintainer of that driver...
>
> Antti, if you want that fixed as part of this series, send a suitable comment
> for the mutex this way and I'll incorporate it.
Ah, I never ran checkpatch with --strict option...
CHECK: struct mutex definition without comment
#202: FILE: drivers/media/dvb-frontends/si2168_priv.h:32:
+ struct mutex i2c_mutex;
If you wish you could add some comment for it, but for me it is still
pretty much self explaining. It is lock to protect firmware command
execution. Command is executed always with I2C write and then poll reply
using I2C read until it timeouts or answers with "ready" status.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-29 9:16 ` Peter Rosin
2016-05-02 19:20 ` Antti Palosaari
@ 2016-05-03 21:39 ` Wolfram Sang
2016-05-04 14:10 ` Peter Rosin
1 sibling, 1 reply; 47+ messages in thread
From: Wolfram Sang @ 2016-05-03 21:39 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
> Yes, they look like reasonable complaints.
Thanks for fixing them. I just sent out my latest comments and when you
fix those and send V8, I'll apply that right away. I think we are safe
to fix the rest incrementally if needed. Note that I didn't review the
IIO and media patches, I trust the reviewers on those.
Thanks for your work on this! I need a break now, this is
mind-boggling...
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-05-03 21:39 ` Wolfram Sang
@ 2016-05-04 14:10 ` Peter Rosin
2016-05-04 16:38 ` Wolfram Sang
0 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-05-04 14:10 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
On 2016-05-03 23:39, Wolfram Sang wrote:
>> Yes, they look like reasonable complaints.
>
> Thanks for fixing them. I just sent out my latest comments and when you
> fix those and send V8, I'll apply that right away. I think we are safe
> to fix the rest incrementally if needed. Note that I didn't review the
Sounds like a plan.
> IIO and media patches, I trust the reviewers on those.
>
> Thanks for your work on this! I need a break now, this is
> mind-boggling...
And thanks for reviewing it!
A question on best practices here... I already did a v8 (but only as
a branch) so I think this will be v9, bit that's a minor detail. The
real question is what I should do about patches 1-15 that are already
in next? Send them too? If not, should I send 16-24 with the same old
patch numbers or should they be numbered 1-9 now? And should such a
shortened series be rebased on 1-15 in next?
Or does it not really matter?
Cheers,
Peter
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-05-04 14:10 ` Peter Rosin
@ 2016-05-04 16:38 ` Wolfram Sang
0 siblings, 0 replies; 47+ messages in thread
From: Wolfram Sang @ 2016-05-04 16:38 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 563 bytes --]
> A question on best practices here... I already did a v8 (but only as
> a branch) so I think this will be v9, bit that's a minor detail. The
> real question is what I should do about patches 1-15 that are already
> in next? Send them too? If not, should I send 16-24 with the same old
> patch numbers or should they be numbered 1-9 now? And should such a
> shortened series be rebased on 1-15 in next?
>
> Or does it not really matter?
Easiest for me is:
Send as v9, only the patches not yet applied, numbered from 1-9, based
on my for-next.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-04-20 15:17 ` [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking Peter Rosin
2016-04-28 20:50 ` Wolfram Sang
@ 2016-05-03 21:38 ` Wolfram Sang
2016-05-04 10:01 ` Peter Rosin
1 sibling, 1 reply; 47+ messages in thread
From: Wolfram Sang @ 2016-05-03 21:38 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote:
> Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
> unlock_bus ops in the adapter. These funcs/ops take an additional flags
> argument that indicates for what purpose the adapter is locked.
>
> There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
> both implemented the same. For now. Locking the adapter means that the
> whole bus is locked, locking the segment means that only the current bus
> segment is locked (i.e. i2c traffic on the parent side of mux is still
> allowed even if the child side of the mux is locked.
>
> Also support a trylock_bus op (but no function to call it, as it is not
> expected to be needed outside of the i2c core).
>
> Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
> scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).
>
> Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.
Can you explain a little why it is SEGMENT and not ADAPTER here? That
probably makes it easier to get into this patch.
And to double check my understanding: I was surprised to not see any
i2c_lock_adapter() or I2C_LOCK_ADAPTER in action. This is because muxes
call I2C_LOCK_SEGMENT on their parent which in case of the parent being
the root adapter is essentially the same as I2C_LOCK_ADAPTER. Correct?
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> drivers/i2c/i2c-core.c | 46 ++++++++++++++++++++++++++++------------------
> include/linux/i2c.h | 28 ++++++++++++++++++++++++++--
> 2 files changed, 54 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 0f2f8484e8ec..21f46d011c33 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -960,10 +960,12 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
> }
>
> /**
> - * i2c_lock_adapter - Get exclusive access to an I2C bus segment
> + * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment
> * @adapter: Target I2C bus segment
> + * @flags: I2C_LOCK_ADAPTER locks the root i2c adapter, I2C_LOCK_SEGMENT
> + * locks only this branch in the adapter tree
> */
I think this kerneldoc should be moved to i2c_lock_adapter and/or
i2c_lock_bus() which are now in i2c.h. This is what users will use, not
this static, adapter-specific implementation. I think it is enough to
have a comment here explaining what is special in handling adapters.
Thanks,
Wolfram
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-05-03 21:38 ` Wolfram Sang
@ 2016-05-04 10:01 ` Peter Rosin
2016-05-04 12:07 ` Wolfram Sang
0 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-05-04 10:01 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
Hi!
On 2016-05-03 23:38, Wolfram Sang wrote:
> On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote:
>> Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
>> unlock_bus ops in the adapter. These funcs/ops take an additional flags
>> argument that indicates for what purpose the adapter is locked.
>>
>> There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
>> both implemented the same. For now. Locking the adapter means that the
>> whole bus is locked, locking the segment means that only the current bus
>> segment is locked (i.e. i2c traffic on the parent side of mux is still
>> allowed even if the child side of the mux is locked.
>>
>> Also support a trylock_bus op (but no function to call it, as it is not
>> expected to be needed outside of the i2c core).
>>
>> Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
>> scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).
>>
>> Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.
>
> Can you explain a little why it is SEGMENT and not ADAPTER here? That
> probably makes it easier to get into this patch.
>
> And to double check my understanding: I was surprised to not see any
> i2c_lock_adapter() or I2C_LOCK_ADAPTER in action. This is because muxes
> call I2C_LOCK_SEGMENT on their parent which in case of the parent being
> the root adapter is essentially the same as I2C_LOCK_ADAPTER. Correct?
Correct. Locking the ADAPTER and the SEGMENT is the same thing for
the root adapter and for traditional muxes (i.e. not mux-locked).
Traditional muxes simply feed the locking request upwards to the
root adapter. The new mux-locked muxes behave the same for ADAPTER
locking; all locks all the way up to the root adapter are grabbed
instantly. If you instead lock SEGMENT on a mux-locked mux, only
the new mux lock in the parent adapter is grabbed right away, but
when the mux then fires off accesses on its parent adapter, that
triggers SEGMENT locks one level up in the tree and the process
recurses.
So, SEGMENT locking is the normal thing that happens when e.g. normal
i2c_transfer calls are made. ADAPTER locking is used for transactions.
The patch enables muxes to use more relaxed locking compared to
locking the ADAPTER for its transations.
The naming can probably be improved. SEGMENT made more sense when
it did not lock all mux accesses one level up (that changed in v6,
but I didn't change the I2C_LOCK_SEGMENT name at that time), so it
is kind of outdated. I2C_LOCK_ROOT_ADAPTER and I2C_LOCK_MUXES instead
of I2C_LOCK_ADAPTER and L2C_LOCK_SEGMENT perhaps?
But I don't really like I2C_LOCK_MUXES as I find it a bit too specific,
and people not thinking about i2c muxes at all (most people I gather,
ignorance is bliss) will not think that it is something for them...
It is also the case that the two flags are mutually exclusive, and
at this point there is no valid uses of using neither flag, nor for
using both. It is a binary decision and one flag would technically be
enough. So, not setting e.g. I2C_LOCK_ADAPTER could in theory imply
I2C_LOCK_SEGMENT. I did it as two separate flags since there might
be a third (or fourth even) option in the future (I don't see what
it would be though, I have no crystal ball...)
So maybe there should be only one flag, e.g. I2C_LOCK_ROOT_ADAPTER?
I.e. perhaps leave the future for later?
Hmmm, I just now realized that you were not really suggesting any
changes other than to the commit message. Oh well, I can perhaps
rephrase some of the above in the commit message if you think that
we should not unnecessarily touch the code at this point...
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>> drivers/i2c/i2c-core.c | 46 ++++++++++++++++++++++++++++------------------
>> include/linux/i2c.h | 28 ++++++++++++++++++++++++++--
>> 2 files changed, 54 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 0f2f8484e8ec..21f46d011c33 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -960,10 +960,12 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
>> }
>>
>> /**
>> - * i2c_lock_adapter - Get exclusive access to an I2C bus segment
>> + * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment
>> * @adapter: Target I2C bus segment
>> + * @flags: I2C_LOCK_ADAPTER locks the root i2c adapter, I2C_LOCK_SEGMENT
>> + * locks only this branch in the adapter tree
>> */
>
> I think this kerneldoc should be moved to i2c_lock_adapter and/or
> i2c_lock_bus() which are now in i2c.h. This is what users will use, not
> this static, adapter-specific implementation. I think it is enough to
> have a comment here explaining what is special in handling adapters.
Yes, I was not really satisfied with having documentation on static
functions. But if I move it, there is no natural home for the current
i2c_trylock_adapter docs, and I'd hate killing documentation that
still applies. Do you have a suggestion? Maybe keep that one doc at
the static i2c_trylock_adapter for now and move it to ->trylock_bus
when someone decides to write kerneldoc for struct i2c_adapter?
Cheers,
Peter
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
2016-05-04 10:01 ` Peter Rosin
@ 2016-05-04 12:07 ` Wolfram Sang
0 siblings, 0 replies; 47+ messages in thread
From: Wolfram Sang @ 2016-05-04 12:07 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]
Hi Peter,
thanks for the detailed explanation!
> So maybe there should be only one flag, e.g. I2C_LOCK_ROOT_ADAPTER?
> I.e. perhaps leave the future for later?
I think this makes the current code easier understandable at this point,
but I'll leave the decision to you. I am fine with both. Maybe a few
words of explanation would be good if you want to keep both flags.
> Hmmm, I just now realized that you were not really suggesting any
> changes other than to the commit message. Oh well, I can perhaps
> rephrase some of the above in the commit message if you think that
> we should not unnecessarily touch the code at this point...
Yes, updated commit description is enough for me now. If you want to
change to one flag, we should do it incrementally. I think I can apply
this as a fixup until around rc3 I'd say.
> > I think this kerneldoc should be moved to i2c_lock_adapter and/or
> > i2c_lock_bus() which are now in i2c.h. This is what users will use, not
> > this static, adapter-specific implementation. I think it is enough to
> > have a comment here explaining what is special in handling adapters.
>
> Yes, I was not really satisfied with having documentation on static
> functions. But if I move it, there is no natural home for the current
> i2c_trylock_adapter docs, and I'd hate killing documentation that
> still applies. Do you have a suggestion? Maybe keep that one doc at
> the static i2c_trylock_adapter for now and move it to ->trylock_bus
> when someone decides to write kerneldoc for struct i2c_adapter?
Well, because I think redundancy is acceptable when it comes to
documentation, how about keeping the chunks you already have and copy an
adapted one over to the functions in i2c.h?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v7 17/24] i2c: muxes always lock the parent adapter
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (15 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:17 ` [PATCH v7 18/24] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing Peter Rosin
` (9 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Instead of checking for i2c parent adapters for every lock/unlock, simply
override the locking for muxes to always lock/unlock the parent adapter
directly.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/i2c-core.c | 21 +++------------------
drivers/i2c/i2c-mux.c | 27 +++++++++++++++++++++++++++
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 21f46d011c33..5314434c2b5d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -967,12 +967,7 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
*/
static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
{
- struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
-
- if (parent)
- i2c_lock_adapter(parent);
- else
- rt_mutex_lock(&adapter->bus_lock);
+ rt_mutex_lock(&adapter->bus_lock);
}
/**
@@ -983,12 +978,7 @@ static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
*/
static int i2c_adapter_trylock_bus(struct i2c_adapter *adapter, int flags)
{
- struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
-
- if (parent)
- return parent->trylock_bus(parent, flags);
- else
- return rt_mutex_trylock(&adapter->bus_lock);
+ return rt_mutex_trylock(&adapter->bus_lock);
}
/**
@@ -999,12 +989,7 @@ static int i2c_adapter_trylock_bus(struct i2c_adapter *adapter, int flags)
*/
static void i2c_adapter_unlock_bus(struct i2c_adapter *adapter, int flags)
{
- struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
-
- if (parent)
- i2c_unlock_adapter(parent);
- else
- rt_mutex_unlock(&adapter->bus_lock);
+ rt_mutex_unlock(&adapter->bus_lock);
}
static void i2c_dev_set_name(struct i2c_adapter *adap,
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 25e9336b0e6e..dc958eebdbb4 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -98,6 +98,30 @@ static unsigned int i2c_mux_parent_classes(struct i2c_adapter *parent)
return class;
}
+static void i2c_parent_lock_bus(struct i2c_adapter *adapter, int flags)
+{
+ struct i2c_mux_priv *priv = adapter->algo_data;
+ struct i2c_adapter *parent = priv->muxc->parent;
+
+ parent->lock_bus(parent, flags);
+}
+
+static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
+{
+ struct i2c_mux_priv *priv = adapter->algo_data;
+ struct i2c_adapter *parent = priv->muxc->parent;
+
+ return parent->trylock_bus(parent, flags);
+}
+
+static void i2c_parent_unlock_bus(struct i2c_adapter *adapter, int flags)
+{
+ struct i2c_mux_priv *priv = adapter->algo_data;
+ struct i2c_adapter *parent = priv->muxc->parent;
+
+ parent->unlock_bus(parent, flags);
+}
+
struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
struct device *dev, int max_adapters,
int sizeof_priv, u32 flags,
@@ -165,6 +189,9 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
priv->adap.retries = parent->retries;
priv->adap.timeout = parent->timeout;
priv->adap.quirks = parent->quirks;
+ priv->adap.lock_bus = i2c_parent_lock_bus;
+ priv->adap.trylock_bus = i2c_parent_trylock_bus;
+ priv->adap.unlock_bus = i2c_parent_unlock_bus;
/* Sanity check on class */
if (i2c_mux_parent_classes(parent) & class)
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 18/24] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (16 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 17/24] i2c: muxes always lock the parent adapter Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-05-03 21:38 ` Wolfram Sang
2016-04-20 15:17 ` [PATCH v7 19/24] i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes Peter Rosin
` (8 subsequent siblings)
26 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
With a i2c topology like the following
GPIO ---| ------ BAT1
| v /
I2C -----+----------+---- MUX
| \
EEPROM ------ BAT2
there is a locking problem with the GPIO controller since it is a client
on the same i2c bus that it muxes. Transfers to the mux clients (e.g. BAT1)
will lock the whole i2c bus prior to attempting to switch the mux to the
correct i2c segment. In the above case, the GPIO device is an I/O expander
with an i2c interface, and since the GPIO subsystem knows nothing (and
rightfully so) about the lockless needs of the i2c mux code, this results
in a deadlock when the GPIO driver issues i2c transfers to modify the
mux.
So, observing that while it is needed to have the i2c bus locked during the
actual MUX update in order to avoid random garbage on the slave side, it
is not strictly a must to have it locked over the whole sequence of a full
select-transfer-deselect mux client operation. The mux itself needs to be
locked, so transfers to clients behind the mux are serialized, and the mux
needs to be stable during all i2c traffic (otherwise individual mux slave
segments might see garbage, or worse).
Introduce this new locking concept as "mux-locked" muxes, and call the
pre-existing mux locking scheme "parent-locked".
Modify the i2c mux locking so that muxes that are "mux-locked" locks only
the muxes on the parent adapter instead of the whole i2c bus when there is
a transfer to the slave side of the mux. This lock serializes transfers to
the slave side of the muxes on the parent adapter.
Add code to i2c-mux-gpio and i2c-mux-pinctrl that checks if all involved
gpio/pinctrl devices have a parent that is an i2c adapter in the same
adapter tree that is muxed, and request a "mux-locked mux" if that is the
case.
Modify the select-transfer-deselect code for "mux-locked" muxes so
that each of the select-transfer-deselect ops locks the mux parent
adapter individually.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/i2c-core.c | 1 +
drivers/i2c/i2c-mux.c | 152 +++++++++++++++++++++++++++++++++---
drivers/i2c/muxes/i2c-mux-gpio.c | 18 +++++
drivers/i2c/muxes/i2c-mux-pinctrl.c | 38 +++++++++
include/linux/i2c-mux.h | 6 ++
include/linux/i2c.h | 1 +
6 files changed, 203 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5314434c2b5d..53514ea314cd 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1543,6 +1543,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
}
rt_mutex_init(&adap->bus_lock);
+ rt_mutex_init(&adap->mux_lock);
mutex_init(&adap->userspace_clients_lock);
INIT_LIST_HEAD(&adap->userspace_clients);
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index dc958eebdbb4..364319c3c95f 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -35,6 +35,25 @@ struct i2c_mux_priv {
u32 chan_id;
};
+static int __i2c_mux_master_xfer(struct i2c_adapter *adap,
+ struct i2c_msg msgs[], int num)
+{
+ struct i2c_mux_priv *priv = adap->algo_data;
+ struct i2c_mux_core *muxc = priv->muxc;
+ struct i2c_adapter *parent = muxc->parent;
+ int ret;
+
+ /* Switch to the right mux port and perform the transfer. */
+
+ ret = muxc->select(muxc, priv->chan_id);
+ if (ret >= 0)
+ ret = __i2c_transfer(parent, msgs, num);
+ if (muxc->deselect)
+ muxc->deselect(muxc, priv->chan_id);
+
+ return ret;
+}
+
static int i2c_mux_master_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[], int num)
{
@@ -47,7 +66,29 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
ret = muxc->select(muxc, priv->chan_id);
if (ret >= 0)
- ret = __i2c_transfer(parent, msgs, num);
+ ret = i2c_transfer(parent, msgs, num);
+ if (muxc->deselect)
+ muxc->deselect(muxc, priv->chan_id);
+
+ return ret;
+}
+
+static int __i2c_mux_smbus_xfer(struct i2c_adapter *adap,
+ u16 addr, unsigned short flags,
+ char read_write, u8 command,
+ int size, union i2c_smbus_data *data)
+{
+ struct i2c_mux_priv *priv = adap->algo_data;
+ struct i2c_mux_core *muxc = priv->muxc;
+ struct i2c_adapter *parent = muxc->parent;
+ int ret;
+
+ /* Select the right mux port and perform the transfer. */
+
+ ret = muxc->select(muxc, priv->chan_id);
+ if (ret >= 0)
+ ret = parent->algo->smbus_xfer(parent, addr, flags,
+ read_write, command, size, data);
if (muxc->deselect)
muxc->deselect(muxc, priv->chan_id);
@@ -68,8 +109,8 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
ret = muxc->select(muxc, priv->chan_id);
if (ret >= 0)
- ret = parent->algo->smbus_xfer(parent, addr, flags,
- read_write, command, size, data);
+ ret = i2c_smbus_xfer(parent, addr, flags,
+ read_write, command, size, data);
if (muxc->deselect)
muxc->deselect(muxc, priv->chan_id);
@@ -98,12 +139,49 @@ static unsigned int i2c_mux_parent_classes(struct i2c_adapter *parent)
return class;
}
+static void i2c_mux_lock_bus(struct i2c_adapter *adapter, int flags)
+{
+ struct i2c_mux_priv *priv = adapter->algo_data;
+ struct i2c_adapter *parent = priv->muxc->parent;
+
+ rt_mutex_lock(&parent->mux_lock);
+ if (!(flags & I2C_LOCK_ADAPTER))
+ return;
+ i2c_lock_bus(parent, flags);
+}
+
+static int i2c_mux_trylock_bus(struct i2c_adapter *adapter, int flags)
+{
+ struct i2c_mux_priv *priv = adapter->algo_data;
+ struct i2c_adapter *parent = priv->muxc->parent;
+
+ if (!rt_mutex_trylock(&parent->mux_lock))
+ return 0;
+ if (!(flags & I2C_LOCK_ADAPTER))
+ return 1;
+ if (parent->trylock_bus(parent, flags))
+ return 1;
+ rt_mutex_unlock(&parent->mux_lock);
+ return 0;
+}
+
+static void i2c_mux_unlock_bus(struct i2c_adapter *adapter, int flags)
+{
+ struct i2c_mux_priv *priv = adapter->algo_data;
+ struct i2c_adapter *parent = priv->muxc->parent;
+
+ if (flags & I2C_LOCK_ADAPTER)
+ i2c_unlock_bus(parent, flags);
+ rt_mutex_unlock(&parent->mux_lock);
+}
+
static void i2c_parent_lock_bus(struct i2c_adapter *adapter, int flags)
{
struct i2c_mux_priv *priv = adapter->algo_data;
struct i2c_adapter *parent = priv->muxc->parent;
- parent->lock_bus(parent, flags);
+ rt_mutex_lock(&parent->mux_lock);
+ i2c_lock_bus(parent, flags);
}
static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
@@ -111,7 +189,12 @@ static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
struct i2c_mux_priv *priv = adapter->algo_data;
struct i2c_adapter *parent = priv->muxc->parent;
- return parent->trylock_bus(parent, flags);
+ if (!rt_mutex_trylock(&parent->mux_lock))
+ return 0;
+ if (parent->trylock_bus(parent, flags))
+ return 1;
+ rt_mutex_unlock(&parent->mux_lock);
+ return 0;
}
static void i2c_parent_unlock_bus(struct i2c_adapter *adapter, int flags)
@@ -119,8 +202,35 @@ static void i2c_parent_unlock_bus(struct i2c_adapter *adapter, int flags)
struct i2c_mux_priv *priv = adapter->algo_data;
struct i2c_adapter *parent = priv->muxc->parent;
- parent->unlock_bus(parent, flags);
+ i2c_unlock_bus(parent, flags);
+ rt_mutex_unlock(&parent->mux_lock);
+}
+
+struct i2c_adapter *i2c_root_adapter(struct device *dev)
+{
+ struct device *i2c;
+ struct i2c_adapter *i2c_root;
+
+ /*
+ * Walk up the device tree to find an i2c adapter, indicating
+ * that this is an i2c client device. Check all ancestors to
+ * handle mfd devices etc.
+ */
+ for (i2c = dev; i2c; i2c = i2c->parent) {
+ if (i2c->type == &i2c_adapter_type)
+ break;
+ }
+ if (!i2c)
+ return NULL;
+
+ /* Continue up the tree to find the root i2c adapter */
+ i2c_root = to_i2c_adapter(i2c);
+ while (i2c_parent_is_i2c_adapter(i2c_root))
+ i2c_root = i2c_parent_is_i2c_adapter(i2c_root);
+
+ return i2c_root;
}
+EXPORT_SYMBOL_GPL(i2c_root_adapter);
struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
struct device *dev, int max_adapters,
@@ -140,6 +250,8 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
muxc->parent = parent;
muxc->dev = dev;
+ if (flags & I2C_MUX_LOCKED)
+ muxc->mux_locked = 1;
muxc->select = select;
muxc->deselect = deselect;
muxc->max_adapters = max_adapters;
@@ -173,10 +285,18 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
/* Need to do algo dynamically because we don't know ahead
* of time what sort of physical adapter we'll be dealing with.
*/
- if (parent->algo->master_xfer)
- priv->algo.master_xfer = i2c_mux_master_xfer;
- if (parent->algo->smbus_xfer)
- priv->algo.smbus_xfer = i2c_mux_smbus_xfer;
+ if (parent->algo->master_xfer) {
+ if (muxc->mux_locked)
+ priv->algo.master_xfer = i2c_mux_master_xfer;
+ else
+ priv->algo.master_xfer = __i2c_mux_master_xfer;
+ }
+ if (parent->algo->smbus_xfer) {
+ if (muxc->mux_locked)
+ priv->algo.smbus_xfer = i2c_mux_smbus_xfer;
+ else
+ priv->algo.smbus_xfer = __i2c_mux_smbus_xfer;
+ }
priv->algo.functionality = i2c_mux_functionality;
/* Now fill out new adapter structure */
@@ -189,9 +309,15 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
priv->adap.retries = parent->retries;
priv->adap.timeout = parent->timeout;
priv->adap.quirks = parent->quirks;
- priv->adap.lock_bus = i2c_parent_lock_bus;
- priv->adap.trylock_bus = i2c_parent_trylock_bus;
- priv->adap.unlock_bus = i2c_parent_unlock_bus;
+ if (muxc->mux_locked) {
+ priv->adap.lock_bus = i2c_mux_lock_bus;
+ priv->adap.trylock_bus = i2c_mux_trylock_bus;
+ priv->adap.unlock_bus = i2c_mux_unlock_bus;
+ } else {
+ priv->adap.lock_bus = i2c_parent_lock_bus;
+ priv->adap.trylock_bus = i2c_parent_trylock_bus;
+ priv->adap.unlock_bus = i2c_parent_unlock_bus;
+ }
/* Sanity check on class */
if (i2c_mux_parent_classes(parent) & class)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index f6270ee934f9..e5cf26eefa97 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/gpio.h>
+#include "../../gpio/gpiolib.h"
#include <linux/of_gpio.h>
struct gpiomux {
@@ -137,6 +138,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
struct i2c_mux_core *muxc;
struct gpiomux *mux;
struct i2c_adapter *parent;
+ struct i2c_adapter *root;
unsigned initial_state, gpio_base;
int i, ret;
@@ -184,6 +186,9 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, muxc);
+ root = i2c_root_adapter(&parent->dev);
+
+ muxc->mux_locked = true;
mux->gpio_base = gpio_base;
if (mux->data.idle != I2C_MUX_GPIO_NO_IDLE) {
@@ -194,6 +199,9 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
}
for (i = 0; i < mux->data.n_gpios; i++) {
+ struct device *gpio_dev;
+ struct gpio_desc *gpio_desc;
+
ret = gpio_request(gpio_base + mux->data.gpios[i], "i2c-mux-gpio");
if (ret) {
dev_err(&pdev->dev, "Failed to request GPIO %d\n",
@@ -210,8 +218,18 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
i++; /* gpio_request above succeeded, so must free */
goto err_request_gpio;
}
+
+ if (!muxc->mux_locked)
+ continue;
+
+ gpio_desc = gpio_to_desc(gpio_base + mux->data.gpios[i]);
+ gpio_dev = &gpio_desc->gdev->dev;
+ muxc->mux_locked = i2c_root_adapter(gpio_dev) == root;
}
+ if (muxc->mux_locked)
+ dev_info(&pdev->dev, "mux-locked i2c mux\n");
+
for (i = 0; i < mux->data.n_values; i++) {
u32 nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
unsigned int class = mux->data.classes ? mux->data.classes[i] : 0;
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index 1b8dc711815e..bfc9d3989a9b 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -24,6 +24,7 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include "../../pinctrl/core.h"
struct i2c_mux_pinctrl {
struct i2c_mux_pinctrl_platform_data *pdata;
@@ -120,10 +121,31 @@ static inline int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
}
#endif
+static struct i2c_adapter *i2c_mux_pinctrl_root_adapter(
+ struct pinctrl_state *state)
+{
+ struct i2c_adapter *root = NULL;
+ struct pinctrl_setting *setting;
+ struct i2c_adapter *pin_root;
+
+ list_for_each_entry(setting, &state->settings, node) {
+ pin_root = i2c_root_adapter(setting->pctldev->dev);
+ if (!pin_root)
+ return NULL;
+ if (!root)
+ root = pin_root;
+ else if (root != pin_root)
+ return NULL;
+ }
+
+ return root;
+}
+
static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
{
struct i2c_mux_core *muxc;
struct i2c_mux_pinctrl *mux;
+ struct i2c_adapter *root;
int i, ret;
mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
@@ -202,6 +224,22 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
goto err;
}
+ root = i2c_root_adapter(&muxc->parent->dev);
+
+ muxc->mux_locked = true;
+ for (i = 0; i < mux->pdata->bus_count; i++) {
+ if (root != i2c_mux_pinctrl_root_adapter(mux->states[i])) {
+ muxc->mux_locked = false;
+ break;
+ }
+ }
+ if (muxc->mux_locked && mux->pdata->pinctrl_state_idle
+ && root != i2c_mux_pinctrl_root_adapter(mux->state_idle))
+ muxc->mux_locked = false;
+
+ if (muxc->mux_locked)
+ dev_info(&pdev->dev, "mux-locked i2c mux\n");
+
for (i = 0; i < mux->pdata->bus_count; i++) {
u32 bus = mux->pdata->base_bus_num ?
(mux->pdata->base_bus_num + i) : 0;
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 2fa93fe1345e..56a136927321 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -30,6 +30,7 @@
struct i2c_mux_core {
struct i2c_adapter *parent;
struct device *dev;
+ bool mux_locked;
void *priv;
@@ -47,11 +48,16 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
int (*select)(struct i2c_mux_core *, u32),
int (*deselect)(struct i2c_mux_core *, u32));
+/* flags for i2c_mux_alloc */
+#define I2C_MUX_LOCKED (1<<0)
+
static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
{
return muxc->priv;
}
+struct i2c_adapter *i2c_root_adapter(struct device *dev);
+
/*
* Called to create an i2c bus on a multiplexed bus segment.
* The chan_id parameter is passed to the select and deselect
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index c5f79fec1bfb..e682d41fa4a6 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -524,6 +524,7 @@ struct i2c_adapter {
/* data fields that are valid for all devices */
struct rt_mutex bus_lock;
+ struct rt_mutex mux_lock;
int timeout; /* in jiffies */
int retries;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v7 18/24] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing
2016-04-20 15:17 ` [PATCH v7 18/24] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing Peter Rosin
@ 2016-05-03 21:38 ` Wolfram Sang
0 siblings, 0 replies; 47+ messages in thread
From: Wolfram Sang @ 2016-05-03 21:38 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
> +static int i2c_mux_trylock_bus(struct i2c_adapter *adapter, int flags)
> +{
> + struct i2c_mux_priv *priv = adapter->algo_data;
> + struct i2c_adapter *parent = priv->muxc->parent;
> +
> + if (!rt_mutex_trylock(&parent->mux_lock))
> + return 0;
> + if (!(flags & I2C_LOCK_ADAPTER))
> + return 1;
> + if (parent->trylock_bus(parent, flags))
> + return 1;
> + rt_mutex_unlock(&parent->mux_lock);
> + return 0;
> +}
This function needs a few short comments why we can leave in this or
that state. Not everyone knows the exit values of trylock by heart and
then it can look a little confusing.
> static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
> @@ -111,7 +189,12 @@ static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
> struct i2c_mux_priv *priv = adapter->algo_data;
> struct i2c_adapter *parent = priv->muxc->parent;
>
> - return parent->trylock_bus(parent, flags);
> + if (!rt_mutex_trylock(&parent->mux_lock))
> + return 0;
> + if (parent->trylock_bus(parent, flags))
> + return 1;
> + rt_mutex_unlock(&parent->mux_lock);
> + return 0;
> }
Same comment as i2c_mux_trylock_bus.
> struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
> struct device *dev, int max_adapters,
> @@ -140,6 +250,8 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
>
> muxc->parent = parent;
> muxc->dev = dev;
> + if (flags & I2C_MUX_LOCKED)
> + muxc->mux_locked = 1;
s/1/true/;
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v7 19/24] i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (17 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 18/24] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing Peter Rosin
@ 2016-04-20 15:17 ` Peter Rosin
2016-04-20 15:18 ` [PATCH v7 20/24] iio: imu: inv_mpu6050: change the i2c gate to be mux-locked Peter Rosin
` (7 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:17 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/i2c/i2c-topology | 370 +++++++++++++++++++++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 371 insertions(+)
create mode 100644 Documentation/i2c/i2c-topology
diff --git a/Documentation/i2c/i2c-topology b/Documentation/i2c/i2c-topology
new file mode 100644
index 000000000000..27bfd682808d
--- /dev/null
+++ b/Documentation/i2c/i2c-topology
@@ -0,0 +1,370 @@
+I2C topology
+============
+
+There are a couple of reasons for building more complex i2c topologies
+than a straight-forward i2c bus with one adapter and one or more devices.
+
+1. A mux may be needed on the bus to prevent address collisions.
+
+2. The bus may be accessible from some external bus master, and arbitration
+ may be needed to determine if it is ok to access the bus.
+
+3. A device (particularly RF tuners) may want to avoid the digital noise
+ from the i2c bus, at least most of the time, and sits behind a gate
+ that has to be operated before the device can be accessed.
+
+Etc
+
+These constructs are represented as i2c adapter trees by Linux, where
+each adapter has a parent adapter (except the root adapter) and zero or
+more child adapters. The root adapter is the actual adapter that issues
+i2c transfers, and all adapters with a parent are part of an "i2c-mux"
+object (quoted, since it can also be an arbitrator or a gate).
+
+Depending of the particular mux driver, something happens when there is
+an i2c transfer on one of its child adapters. The mux driver can
+obviously operate a mux, but it can also do arbitration with an external
+bus master or open a gate. The mux driver has two operations for this,
+select and deselect. select is called before the transfer and (the
+optional) deselect is called after the transfer.
+
+
+Locking
+=======
+
+There are two variants of locking available to i2c muxes, they can be
+mux-locked or parent-locked muxes. As is evident from below, it can be
+useful to know if a mux is mux-locked or if it is parent-locked. The
+following list was correct at the time of writing:
+
+In drivers/i2c/muxes/
+i2c-arb-gpio-challenge Parent-locked
+i2c-mux-gpio Normally parent-locked, mux-locked iff
+ all involved gpio pins are controlled by the
+ same i2c root adapter that they mux.
+i2c-mux-pca9541 Parent-locked
+i2c-mux-pca954x Parent-locked
+i2c-mux-pinctrl Normally parent-locked, mux-locked iff
+ all involved pinctrl devices are controlled
+ by the same i2c root adapter that they mux.
+i2c-mux-reg Parent-locked
+
+In drivers/iio/
+imu/inv_mpu6050/ Parent-locked
+
+In drivers/media/
+dvb-frontends/m88ds3103 Parent-locked
+dvb-frontends/rtl2830 Parent-locked
+dvb-frontends/rtl2832 Parent-locked
+dvb-frontends/si2168 Parent-locked
+usb/cx231xx/ Parent-locked
+
+
+Mux-locked muxes
+----------------
+
+Mux-locked muxes does not lock the entire parent adapter during the
+full select-transfer-deselect transaction, only the muxes on the parent
+adapter are locked. Mux-locked muxes are mostly interesting if the
+select and/or deselect operations must use i2c transfers to complete
+their tasks. Since the parent adapter is not fully locked during the
+full transaction, unrelated i2c transfers may interleave the different
+stages of the transaction. This has the benefit that the mux driver
+may be easier and cleaner to implement, but it has some caveats.
+
+ML1. If you build a topology with a mux-locked mux being the parent
+ of a parent-locked mux, this might break the expectation from the
+ parent-locked mux that the root adapter is locked during the
+ transaction.
+
+ML2. It is not safe to build arbitrary topologies with two (or more)
+ mux-locked muxes that are not siblings, when there are address
+ collisions between the devices on the child adapters of these
+ non-sibling muxes.
+
+ I.e. the select-transfer-deselect transaction targeting e.g. device
+ address 0x42 behind mux-one may be interleaved with a similar
+ operation targeting device address 0x42 behind mux-two. The
+ intension with such a topology would in this hypothetical example
+ be that mux-one and mux-two should not be selected simultaneously,
+ but mux-locked muxes do not guarantee that in all topologies.
+
+ML3. A mux-locked mux cannot be used by a driver for auto-closing
+ gates/muxes, i.e. something that closes automatically after a given
+ number (one, in most cases) of i2c transfers. Unrelated i2c transfers
+ may creep in and close prematurely.
+
+ML4. If any non-i2c operation in the mux driver changes the i2c mux state,
+ the driver has to lock the root adapter during that operation.
+ Otherwise garbage may appear on the bus as seen from devices
+ behind the mux, when an unrelated i2c transfer is in flight during
+ the non-i2c mux-changing operation.
+
+
+Mux-locked Example
+------------------
+
+ .----------. .--------.
+ .--------. | mux- |-----| dev D1 |
+ | root |--+--| locked | '--------'
+ '--------' | | mux M1 |--. .--------.
+ | '----------' '--| dev D2 |
+ | .--------. '--------'
+ '--| dev D3 |
+ '--------'
+
+When there is an access to D1, this happens:
+
+ 1. Someone issues an i2c-transfer to D1.
+ 2. M1 locks muxes on its parent (the root adapter in this case).
+ 3. M1 calls ->select to ready the mux.
+ 4. M1 (presumably) does some i2c-transfers as part of its select.
+ These transfers are normal i2c-transfers that locks the parent
+ adapter.
+ 5. M1 feeds the i2c-transfer from step 1 to its parent adapter as a
+ normal i2c-transfer that locks the parent adapter.
+ 6. M1 calls ->deselect, if it has one.
+ 7. Same rules as in step 4, but for ->deselect.
+ 8. M1 unlocks muxes on its parent.
+
+This means that accesses to D2 are lockout out for the full duration
+of the entire operation. But accesses to D3 are possibly interleaved
+at any point.
+
+
+Parent-locked muxes
+-------------------
+
+Parent-locked muxes lock the parent adapter during the full select-
+transfer-deselect transaction. The implication is that the mux driver
+has to ensure that any and all i2c transfers through that parent
+adapter during the transaction are unlocked i2c transfers (using e.g.
+__i2c_transfer), or a deadlock will follow. There are a couple of
+caveats.
+
+PL1. If you build a topology with a parent-locked mux being the child
+ of another mux, this might break a possible assumption from the
+ child mux that the root adapter is unused between its select op
+ and the actual transfer (e.g. if the child mux is auto-closing
+ and the parent mux issus i2c-transfers as part of its select).
+ This is especially the case if the parent mux is mux-locked, but
+ it may also happen if the parent mux is parent-locked.
+
+PL2. If select/deselect calls out to other subsystems such as gpio,
+ pinctrl, regmap or iio, it is essential that any i2c transfers
+ caused by these subsystems are unlocked. This can be convoluted to
+ accomplish, maybe even impossible if an acceptably clean solution
+ is sought.
+
+
+Parent-locked Example
+---------------------
+
+ .----------. .--------.
+ .--------. | parent- |-----| dev D1 |
+ | root |--+--| locked | '--------'
+ '--------' | | mux M1 |--. .--------.
+ | '----------' '--| dev D2 |
+ | .--------. '--------'
+ '--| dev D3 |
+ '--------'
+
+When there is an access to D1, this happens:
+
+ 1. Someone issues an i2c-transfer to D1.
+ 2. M1 locks muxes on its parent (the root adapter in this case).
+ 3. M1 locks its parent adapter.
+ 4. M1 calls ->select to ready the mux.
+ 5. If M1 does any i2c-transfers (on this root adapter) as part of
+ its select, those transfers must be unlocked i2c-transfers so
+ that they do not deadlock the root adapter.
+ 6. M1 feeds the i2c-transfer from step 1 to the root adapter as an
+ unlocked i2c-transfer, so that it does not deadlock the parent
+ adapter.
+ 7. M1 calls ->deselect, if it has one.
+ 8. Same rules as in step 5, but for ->deselect.
+ 9. M1 unlocks its parent adapter.
+10. M1 unlocks muxes on its parent.
+
+
+This means that accesses to both D2 and D3 are locked out for the full
+duration of the entire operation.
+
+
+Complex Examples
+================
+
+Parent-locked mux as parent of parent-locked mux
+------------------------------------------------
+
+This is a useful topology, but it can be bad.
+
+ .----------. .----------. .--------.
+ .--------. | parent- |-----| parent- |-----| dev D1 |
+ | root |--+--| locked | | locked | '--------'
+ '--------' | | mux M1 |--. | mux M2 |--. .--------.
+ | '----------' | '----------' '--| dev D2 |
+ | .--------. | .--------. '--------'
+ '--| dev D4 | '--| dev D3 |
+ '--------' '--------'
+
+When any device is accessed, all other devices are locked out for
+the full duration of the operation (both muxes lock their parent,
+and specifically when M2 requests its parent to lock, M1 passes
+the buck to the root adapter).
+
+This topology is bad if M2 is an auto-closing mux and M1->select
+issues any unlocked i2c transfers on the root adapter that may leak
+through and be seen by the M2 adapter, thus closing M2 prematurely.
+
+
+Mux-locked mux as parent of mux-locked mux
+------------------------------------------
+
+This is a good topology.
+
+ .----------. .----------. .--------.
+ .--------. | mux- |-----| mux- |-----| dev D1 |
+ | root |--+--| locked | | locked | '--------'
+ '--------' | | mux M1 |--. | mux M2 |--. .--------.
+ | '----------' | '----------' '--| dev D2 |
+ | .--------. | .--------. '--------'
+ '--| dev D4 | '--| dev D3 |
+ '--------' '--------'
+
+When device D1 is accessed, accesses to D2 are locked out for the
+full duration of the operation (muxes on the top child adapter of M1
+are locked). But accesses to D3 and D4 are possibly interleaved at
+any point. Accesses to D3 locks out D1 and D2, but accesses to D4
+are still possibly interleaved.
+
+
+Mux-locked mux as parent of parent-locked mux
+---------------------------------------------
+
+This is probably a bad topology.
+
+ .----------. .----------. .--------.
+ .--------. | mux- |-----| parent- |-----| dev D1 |
+ | root |--+--| locked | | locked | '--------'
+ '--------' | | mux M1 |--. | mux M2 |--. .--------.
+ | '----------' | '----------' '--| dev D2 |
+ | .--------. | .--------. '--------'
+ '--| dev D4 | '--| dev D3 |
+ '--------' '--------'
+
+When device D1 is accessed, accesses to D2 and D3 are locked out
+for the full duration of the operation (M1 locks child muxes on the
+root adapter). But accesses to D4 are possibly interleaved at any
+point.
+
+This kind of topology is generally not suitable and should probably
+be avoided. The reason is that M2 probably assumes that there will
+be no i2c transfers during its calls to ->select and ->deselect, and
+if there are, any such transfers might appear on the slave side of M2
+as partial i2c transfers, i.e. garbage or worse. This might cause
+device lockups and/or other problems.
+
+The topology is especially troublesome if M2 is an auto-closing
+mux. In that case, any interleaved accesses to D4 might close M2
+prematurely, as might any i2c-transfers part of M1->select.
+
+But if M2 is not making the above stated assumption, and if M2 is not
+auto-closing, the topology is fine.
+
+
+Parent-locked mux as parent of mux-locked mux
+---------------------------------------------
+
+This is a good topology.
+
+ .----------. .----------. .--------.
+ .--------. | parent- |-----| mux- |-----| dev D1 |
+ | root |--+--| locked | | locked | '--------'
+ '--------' | | mux M1 |--. | mux M2 |--. .--------.
+ | '----------' | '----------' '--| dev D2 |
+ | .--------. | .--------. '--------'
+ '--| dev D4 | '--| dev D3 |
+ '--------' '--------'
+
+When D1 is accessed, accesses to D2 are locked out for the full
+duration of the operation (muxes on the top child adapter of M1
+are locked). Accesses to D3 and D4 are possibly interleaved at
+any point, just as is expected for mux-locked muxes.
+
+When D3 or D4 are accessed, everything else is locked out. For D3
+accesses, M1 locks the root adapter. For D4 accesses, the root
+adapter is locked directly.
+
+
+Two mux-locked sibling muxes
+----------------------------
+
+This is a good topology.
+
+ .--------.
+ .----------. .--| dev D1 |
+ | mux- |--' '--------'
+ .--| locked | .--------.
+ | | mux M1 |-----| dev D2 |
+ | '----------' '--------'
+ | .----------. .--------.
+ .--------. | | mux- |-----| dev D3 |
+ | root |--+--| locked | '--------'
+ '--------' | | mux M2 |--. .--------.
+ | '----------' '--| dev D4 |
+ | .--------. '--------'
+ '--| dev D5 |
+ '--------'
+
+When D1 is accessed, accesses to D2, D3 and D4 are locked out. But
+accesses to D5 may be interleaved at any time.
+
+
+Two parent-locked sibling muxes
+-------------------------------
+
+This is a good topology.
+
+ .--------.
+ .----------. .--| dev D1 |
+ | parent- |--' '--------'
+ .--| locked | .--------.
+ | | mux M1 |-----| dev D2 |
+ | '----------' '--------'
+ | .----------. .--------.
+ .--------. | | parent- |-----| dev D3 |
+ | root |--+--| locked | '--------'
+ '--------' | | mux M2 |--. .--------.
+ | '----------' '--| dev D4 |
+ | .--------. '--------'
+ '--| dev D5 |
+ '--------'
+
+When any device is accessed, accesses to all other devices are locked
+out.
+
+
+Mux-locked and parent-locked sibling muxes
+------------------------------------------
+
+This is a good topology.
+
+ .--------.
+ .----------. .--| dev D1 |
+ | mux- |--' '--------'
+ .--| locked | .--------.
+ | | mux M1 |-----| dev D2 |
+ | '----------' '--------'
+ | .----------. .--------.
+ .--------. | | parent- |-----| dev D3 |
+ | root |--+--| locked | '--------'
+ '--------' | | mux M2 |--. .--------.
+ | '----------' '--| dev D4 |
+ | .--------. '--------'
+ '--| dev D5 |
+ '--------'
+
+When D1 or D2 are accessed, accesses to D3 and D4 are locked out while
+accesses to D5 may interleave. When D3 or D4 are accessed, accesses to
+all other devices are locked out.
diff --git a/MAINTAINERS b/MAINTAINERS
index 03e00c7c88eb..d17afeb81246 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5274,6 +5274,7 @@ I2C MUXES
M: Peter Rosin <peda@axentia.se>
L: linux-i2c@vger.kernel.org
S: Maintained
+F: Documentation/i2c/i2c-topology
F: Documentation/i2c/muxes/
F: Documentation/devicetree/bindings/i2c/i2c-mux*
F: drivers/i2c/i2c-mux.c
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 20/24] iio: imu: inv_mpu6050: change the i2c gate to be mux-locked
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (18 preceding siblings ...)
2016-04-20 15:17 ` [PATCH v7 19/24] i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes Peter Rosin
@ 2016-04-20 15:18 ` Peter Rosin
2016-04-20 15:18 ` [PATCH v7 21/24] [media] si2168: " Peter Rosin
` (6 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
The root i2c adapter lock is then no longer held by the i2c mux during
accesses behind the i2c gate, and such accesses need to take that lock
just like any other ordinary i2c accesses do.
So, declare the i2c gate mux-locked, and zap the code that makes the
unlocked i2c accesses and just use ordinary regmap_write accesses.
This also happens to fix the deadlock described in
http://patchwork.ozlabs.org/patch/584776/ authored by
Adriana Reus <adriana.reus@intel.com> and submitted by
Daniel Baluta <daniel.baluta@intel.com>
----------8<----------
iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock
This deadlock occurs if the accel/gyro and the sensor on the auxiliary
I2C (in my setup it's an ak8975) are working at the same time.
Scenario:
T1 T2
==== ====
inv_mpu6050_read_fifo aux sensor op (eg. ak8975_read_raw)
| |
mutex_lock(&indio_dev->mlock) i2c_transfer
| |
i2c transaction i2c adapter lock
| |
i2c adapter lock i2c_mux_master_xfer
|
inv_mpu6050_select_bypass
|
mutex_lock(&indio_dev->mlock)
When we operate on an mpu sensor the order of locking is mpu lock
followed by the i2c adapter lock. However, when we operate the auxiliary
sensor the order of locking is the other way around.
...
----------8<----------
The reason this patch fixes the deadlock is that T2 does not grab the
i2c adapter lock until the very end (and grabs the newfangled i2c mux
lock where it previously grabbed the i2c adapter lock).
Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Daniel Baluta <daniel.baluta@intel.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/i2c/i2c-topology | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 52 ++++++-------------------------
2 files changed, 11 insertions(+), 43 deletions(-)
diff --git a/Documentation/i2c/i2c-topology b/Documentation/i2c/i2c-topology
index 27bfd682808d..69b008518454 100644
--- a/Documentation/i2c/i2c-topology
+++ b/Documentation/i2c/i2c-topology
@@ -50,7 +50,7 @@ i2c-mux-pinctrl Normally parent-locked, mux-locked iff
i2c-mux-reg Parent-locked
In drivers/iio/
-imu/inv_mpu6050/ Parent-locked
+imu/inv_mpu6050/ Mux-locked
In drivers/media/
dvb-frontends/m88ds3103 Parent-locked
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 664a45082d39..2c4188db8634 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -24,45 +24,16 @@ static const struct regmap_config inv_mpu_regmap_config = {
.val_bits = 8,
};
-/*
- * The i2c read/write needs to happen in unlocked mode. As the parent
- * adapter is common. If we use locked versions, it will fail as
- * the mux adapter will lock the parent i2c adapter, while calling
- * select/deselect functions.
- */
-static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
- u8 reg, u8 d)
-{
- int ret;
- u8 buf[2] = {reg, d};
- struct i2c_msg msg[1] = {
- {
- .addr = client->addr,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
- };
-
- ret = __i2c_transfer(client->adapter, msg, 1);
- if (ret != 1)
- return ret;
-
- return 0;
-}
-
static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = i2c_mux_priv(muxc);
- struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
+ struct iio_dev *indio_dev = i2c_mux_priv(muxc);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
int ret = 0;
/* Use the same mutex which was used everywhere to protect power-op */
mutex_lock(&indio_dev->mlock);
if (!st->powerup_count) {
- ret = inv_mpu6050_write_reg_unlocked(client,
- st->reg->pwr_mgmt_1, 0);
+ ret = regmap_write(st->map, st->reg->pwr_mgmt_1, 0);
if (ret)
goto write_error;
@@ -71,10 +42,9 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
}
if (!ret) {
st->powerup_count++;
- ret = inv_mpu6050_write_reg_unlocked(client,
- st->reg->int_pin_cfg,
- INV_MPU6050_INT_PIN_CFG |
- INV_MPU6050_BIT_BYPASS_EN);
+ ret = regmap_write(st->map, st->reg->int_pin_cfg,
+ INV_MPU6050_INT_PIN_CFG |
+ INV_MPU6050_BIT_BYPASS_EN);
}
write_error:
mutex_unlock(&indio_dev->mlock);
@@ -84,18 +54,16 @@ write_error:
static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = i2c_mux_priv(muxc);
- struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
+ struct iio_dev *indio_dev = i2c_mux_priv(muxc);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
mutex_lock(&indio_dev->mlock);
/* It doesn't really mattter, if any of the calls fails */
- inv_mpu6050_write_reg_unlocked(client, st->reg->int_pin_cfg,
- INV_MPU6050_INT_PIN_CFG);
+ regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
st->powerup_count--;
if (!st->powerup_count)
- inv_mpu6050_write_reg_unlocked(client, st->reg->pwr_mgmt_1,
- INV_MPU6050_BIT_SLEEP);
+ regmap_write(st->map, st->reg->pwr_mgmt_1,
+ INV_MPU6050_BIT_SLEEP);
mutex_unlock(&indio_dev->mlock);
return 0;
@@ -134,7 +102,7 @@ static int inv_mpu_probe(struct i2c_client *client,
st = iio_priv(dev_get_drvdata(&client->dev));
st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
- 1, 0, 0,
+ 1, 0, I2C_MUX_LOCKED,
inv_mpu6050_select_bypass,
inv_mpu6050_deselect_bypass);
if (!st->muxc) {
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 21/24] [media] si2168: change the i2c gate to be mux-locked
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (19 preceding siblings ...)
2016-04-20 15:18 ` [PATCH v7 20/24] iio: imu: inv_mpu6050: change the i2c gate to be mux-locked Peter Rosin
@ 2016-04-20 15:18 ` Peter Rosin
2016-04-20 15:18 ` [PATCH v7 22/24] [media] rtl2832: " Peter Rosin
` (5 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Antti Palosaari, Peter Rosin, Wolfram Sang, Jonathan Corbet,
Peter Korsgaard, Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
From: Antti Palosaari <crope@iki.fi>
The root i2c adapter lock is then no longer held by the i2c mux during
accesses behind the i2c gate, and such accesses need to take that lock
just like any other ordinary i2c accesses do.
So, declare the i2c gate mux-locked, and zap the code that makes the
i2c accesses unlocked. But add a mutex so that firmware commands are
still serialized.
Signed-off-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/i2c/i2c-topology | 2 +-
drivers/media/dvb-frontends/si2168.c | 83 ++++++++-----------------------
drivers/media/dvb-frontends/si2168_priv.h | 1 +
3 files changed, 22 insertions(+), 64 deletions(-)
diff --git a/Documentation/i2c/i2c-topology b/Documentation/i2c/i2c-topology
index 69b008518454..5e40802f0be2 100644
--- a/Documentation/i2c/i2c-topology
+++ b/Documentation/i2c/i2c-topology
@@ -56,7 +56,7 @@ In drivers/media/
dvb-frontends/m88ds3103 Parent-locked
dvb-frontends/rtl2830 Parent-locked
dvb-frontends/rtl2832 Parent-locked
-dvb-frontends/si2168 Parent-locked
+dvb-frontends/si2168 Mux-locked
usb/cx231xx/ Parent-locked
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 5583827c386e..108a069fa1ae 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -18,53 +18,23 @@
static const struct dvb_frontend_ops si2168_ops;
-/* Own I2C adapter locking is needed because of I2C gate logic. */
-static int si2168_i2c_master_send_unlocked(const struct i2c_client *client,
- const char *buf, int count)
-{
- int ret;
- struct i2c_msg msg = {
- .addr = client->addr,
- .flags = 0,
- .len = count,
- .buf = (char *)buf,
- };
-
- ret = __i2c_transfer(client->adapter, &msg, 1);
- return (ret == 1) ? count : ret;
-}
-
-static int si2168_i2c_master_recv_unlocked(const struct i2c_client *client,
- char *buf, int count)
-{
- int ret;
- struct i2c_msg msg = {
- .addr = client->addr,
- .flags = I2C_M_RD,
- .len = count,
- .buf = buf,
- };
-
- ret = __i2c_transfer(client->adapter, &msg, 1);
- return (ret == 1) ? count : ret;
-}
-
/* execute firmware command */
-static int si2168_cmd_execute_unlocked(struct i2c_client *client,
- struct si2168_cmd *cmd)
+static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
{
+ struct si2168_dev *dev = i2c_get_clientdata(client);
int ret;
unsigned long timeout;
+ mutex_lock(&dev->i2c_mutex);
+
if (cmd->wlen) {
/* write cmd and args for firmware */
- ret = si2168_i2c_master_send_unlocked(client, cmd->args,
- cmd->wlen);
+ ret = i2c_master_send(client, cmd->args, cmd->wlen);
if (ret < 0) {
- goto err;
+ goto err_mutex_unlock;
} else if (ret != cmd->wlen) {
ret = -EREMOTEIO;
- goto err;
+ goto err_mutex_unlock;
}
}
@@ -73,13 +43,12 @@ static int si2168_cmd_execute_unlocked(struct i2c_client *client,
#define TIMEOUT 70
timeout = jiffies + msecs_to_jiffies(TIMEOUT);
while (!time_after(jiffies, timeout)) {
- ret = si2168_i2c_master_recv_unlocked(client, cmd->args,
- cmd->rlen);
+ ret = i2c_master_recv(client, cmd->args, cmd->rlen);
if (ret < 0) {
- goto err;
+ goto err_mutex_unlock;
} else if (ret != cmd->rlen) {
ret = -EREMOTEIO;
- goto err;
+ goto err_mutex_unlock;
}
/* firmware ready? */
@@ -94,32 +63,23 @@ static int si2168_cmd_execute_unlocked(struct i2c_client *client,
/* error bit set? */
if ((cmd->args[0] >> 6) & 0x01) {
ret = -EREMOTEIO;
- goto err;
+ goto err_mutex_unlock;
}
if (!((cmd->args[0] >> 7) & 0x01)) {
ret = -ETIMEDOUT;
- goto err;
+ goto err_mutex_unlock;
}
}
+ mutex_unlock(&dev->i2c_mutex);
return 0;
-err:
+err_mutex_unlock:
+ mutex_unlock(&dev->i2c_mutex);
dev_dbg(&client->dev, "failed=%d\n", ret);
return ret;
}
-static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
-{
- int ret;
-
- i2c_lock_adapter(client->adapter);
- ret = si2168_cmd_execute_unlocked(client, cmd);
- i2c_unlock_adapter(client->adapter);
-
- return ret;
-}
-
static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
{
struct i2c_client *client = fe->demodulator_priv;
@@ -610,11 +570,6 @@ static int si2168_get_tune_settings(struct dvb_frontend *fe,
return 0;
}
-/*
- * I2C gate logic
- * We must use unlocked I2C I/O because I2C adapter lock is already taken
- * by the caller (usually tuner driver).
- */
static int si2168_select(struct i2c_mux_core *muxc, u32 chan)
{
struct i2c_client *client = i2c_mux_priv(muxc);
@@ -625,7 +580,7 @@ static int si2168_select(struct i2c_mux_core *muxc, u32 chan)
memcpy(cmd.args, "\xc0\x0d\x01", 3);
cmd.wlen = 3;
cmd.rlen = 0;
- ret = si2168_cmd_execute_unlocked(client, &cmd);
+ ret = si2168_cmd_execute(client, &cmd);
if (ret)
goto err;
@@ -645,7 +600,7 @@ static int si2168_deselect(struct i2c_mux_core *muxc, u32 chan)
memcpy(cmd.args, "\xc0\x0d\x00", 3);
cmd.wlen = 3;
cmd.rlen = 0;
- ret = si2168_cmd_execute_unlocked(client, &cmd);
+ ret = si2168_cmd_execute(client, &cmd);
if (ret)
goto err;
@@ -708,9 +663,11 @@ static int si2168_probe(struct i2c_client *client,
goto err;
}
+ mutex_init(&dev->i2c_mutex);
+
/* create mux i2c adapter for tuner */
dev->muxc = i2c_mux_alloc(client->adapter, &client->dev,
- 1, 0, 0,
+ 1, 0, I2C_MUX_LOCKED,
si2168_select, si2168_deselect);
if (!dev->muxc) {
ret = -ENOMEM;
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 165bf1412063..8a1f36d2014d 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -29,6 +29,7 @@
/* state struct */
struct si2168_dev {
+ struct mutex i2c_mutex;
struct i2c_mux_core *muxc;
struct dvb_frontend fe;
enum fe_delivery_system delivery_system;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 22/24] [media] rtl2832: change the i2c gate to be mux-locked
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (20 preceding siblings ...)
2016-04-20 15:18 ` [PATCH v7 21/24] [media] si2168: " Peter Rosin
@ 2016-04-20 15:18 ` Peter Rosin
2016-04-28 21:47 ` Wolfram Sang
2016-04-20 15:18 ` [PATCH v7 23/24] [media] rtl2832_sdr: get rid of empty regmap wrappers Peter Rosin
` (4 subsequent siblings)
26 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
The root i2c adapter lock is then no longer held by the i2c mux during
accesses behind the i2c gate, and such accesses need to take that lock
just like any other ordinary i2c accesses do.
So, declare the i2c gate mux-locked, and zap the regmap overrides
that makes the i2c accesses unlocked and use plain old regmap
accesses. This also removes the need for the regmap wrappers used by
rtl2832_sdr, so deconvolute the code further and provide the regmap
handle directly instead of the wrapper functions.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/i2c/i2c-topology | 2 +-
drivers/media/dvb-frontends/rtl2832.c | 190 ++++--------------------------
drivers/media/dvb-frontends/rtl2832.h | 4 +-
drivers/media/dvb-frontends/rtl2832_sdr.c | 13 +-
drivers/media/dvb-frontends/rtl2832_sdr.h | 5 +-
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 5 +-
6 files changed, 37 insertions(+), 182 deletions(-)
diff --git a/Documentation/i2c/i2c-topology b/Documentation/i2c/i2c-topology
index 5e40802f0be2..e0aefeece551 100644
--- a/Documentation/i2c/i2c-topology
+++ b/Documentation/i2c/i2c-topology
@@ -55,7 +55,7 @@ imu/inv_mpu6050/ Mux-locked
In drivers/media/
dvb-frontends/m88ds3103 Parent-locked
dvb-frontends/rtl2830 Parent-locked
-dvb-frontends/rtl2832 Parent-locked
+dvb-frontends/rtl2832 Mux-locked
dvb-frontends/si2168 Mux-locked
usb/cx231xx/ Parent-locked
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 1b23788797b5..957523f07f61 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -153,43 +153,6 @@ static const struct rtl2832_reg_entry registers[] = {
[DVBT_REG_4MSEL] = {0x013, 0, 0},
};
-/* Our regmap is bypassing I2C adapter lock, thus we do it! */
-static int rtl2832_bulk_write(struct i2c_client *client, unsigned int reg,
- const void *val, size_t val_count)
-{
- struct rtl2832_dev *dev = i2c_get_clientdata(client);
- int ret;
-
- i2c_lock_adapter(client->adapter);
- ret = regmap_bulk_write(dev->regmap, reg, val, val_count);
- i2c_unlock_adapter(client->adapter);
- return ret;
-}
-
-static int rtl2832_update_bits(struct i2c_client *client, unsigned int reg,
- unsigned int mask, unsigned int val)
-{
- struct rtl2832_dev *dev = i2c_get_clientdata(client);
- int ret;
-
- i2c_lock_adapter(client->adapter);
- ret = regmap_update_bits(dev->regmap, reg, mask, val);
- i2c_unlock_adapter(client->adapter);
- return ret;
-}
-
-static int rtl2832_bulk_read(struct i2c_client *client, unsigned int reg,
- void *val, size_t val_count)
-{
- struct rtl2832_dev *dev = i2c_get_clientdata(client);
- int ret;
-
- i2c_lock_adapter(client->adapter);
- ret = regmap_bulk_read(dev->regmap, reg, val, val_count);
- i2c_unlock_adapter(client->adapter);
- return ret;
-}
-
static int rtl2832_rd_demod_reg(struct rtl2832_dev *dev, int reg, u32 *val)
{
struct i2c_client *client = dev->client;
@@ -204,7 +167,7 @@ static int rtl2832_rd_demod_reg(struct rtl2832_dev *dev, int reg, u32 *val)
len = (msb >> 3) + 1;
mask = REG_MASK(msb - lsb);
- ret = rtl2832_bulk_read(client, reg_start_addr, reading, len);
+ ret = regmap_bulk_read(dev->regmap, reg_start_addr, reading, len);
if (ret)
goto err;
@@ -234,7 +197,7 @@ static int rtl2832_wr_demod_reg(struct rtl2832_dev *dev, int reg, u32 val)
len = (msb >> 3) + 1;
mask = REG_MASK(msb - lsb);
- ret = rtl2832_bulk_read(client, reg_start_addr, reading, len);
+ ret = regmap_bulk_read(dev->regmap, reg_start_addr, reading, len);
if (ret)
goto err;
@@ -248,7 +211,7 @@ static int rtl2832_wr_demod_reg(struct rtl2832_dev *dev, int reg, u32 val)
for (i = 0; i < len; i++)
writing[i] = (writing_tmp >> ((len - 1 - i) * 8)) & 0xff;
- ret = rtl2832_bulk_write(client, reg_start_addr, writing, len);
+ ret = regmap_bulk_write(dev->regmap, reg_start_addr, writing, len);
if (ret)
goto err;
@@ -525,7 +488,8 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)
}
for (j = 0; j < sizeof(bw_params[0]); j++) {
- ret = rtl2832_bulk_write(client, 0x11c + j, &bw_params[i][j], 1);
+ ret = regmap_bulk_write(dev->regmap,
+ 0x11c + j, &bw_params[i][j], 1);
if (ret)
goto err;
}
@@ -581,11 +545,11 @@ static int rtl2832_get_frontend(struct dvb_frontend *fe,
if (dev->sleeping)
return 0;
- ret = rtl2832_bulk_read(client, 0x33c, buf, 2);
+ ret = regmap_bulk_read(dev->regmap, 0x33c, buf, 2);
if (ret)
goto err;
- ret = rtl2832_bulk_read(client, 0x351, &buf[2], 1);
+ ret = regmap_bulk_read(dev->regmap, 0x351, &buf[2], 1);
if (ret)
goto err;
@@ -716,7 +680,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, enum fe_status *status)
/* signal strength */
if (dev->fe_status & FE_HAS_SIGNAL) {
/* read digital AGC */
- ret = rtl2832_bulk_read(client, 0x305, &u8tmp, 1);
+ ret = regmap_bulk_read(dev->regmap, 0x305, &u8tmp, 1);
if (ret)
goto err;
@@ -742,7 +706,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, enum fe_status *status)
{87659938, 87659938, 87885178, 88241743},
};
- ret = rtl2832_bulk_read(client, 0x33c, &u8tmp, 1);
+ ret = regmap_bulk_read(dev->regmap, 0x33c, &u8tmp, 1);
if (ret)
goto err;
@@ -754,7 +718,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, enum fe_status *status)
if (hierarchy > HIERARCHY_NUM - 1)
goto err;
- ret = rtl2832_bulk_read(client, 0x40c, buf, 2);
+ ret = regmap_bulk_read(dev->regmap, 0x40c, buf, 2);
if (ret)
goto err;
@@ -775,7 +739,7 @@ static int rtl2832_read_status(struct dvb_frontend *fe, enum fe_status *status)
/* BER */
if (dev->fe_status & FE_HAS_LOCK) {
- ret = rtl2832_bulk_read(client, 0x34e, buf, 2);
+ ret = regmap_bulk_read(dev->regmap, 0x34e, buf, 2);
if (ret)
goto err;
@@ -825,8 +789,6 @@ static int rtl2832_read_ber(struct dvb_frontend *fe, u32 *ber)
/*
* I2C gate/mux/repeater logic
- * We must use unlocked __i2c_transfer() here (through regmap) because of I2C
- * adapter lock is already taken by tuner driver.
* There is delay mechanism to avoid unneeded I2C gate open / close. Gate close
* is delayed here a little bit in order to see if there is sequence of I2C
* messages sent to same I2C bus.
@@ -838,7 +800,7 @@ static void rtl2832_i2c_gate_work(struct work_struct *work)
int ret;
/* close gate */
- ret = rtl2832_update_bits(dev->client, 0x101, 0x08, 0x00);
+ ret = regmap_update_bits(dev->regmap, 0x101, 0x08, 0x00);
if (ret)
goto err;
@@ -856,10 +818,7 @@ static int rtl2832_select(struct i2c_mux_core *muxc, u32 chan_id)
/* terminate possible gate closing */
cancel_delayed_work(&dev->i2c_gate_work);
- /*
- * I2C adapter lock is already taken and due to that we will use
- * regmap_update_bits() which does not lock again I2C adapter.
- */
+ /* open gate */
ret = regmap_update_bits(dev->regmap, 0x101, 0x08, 0x08);
if (ret)
goto err;
@@ -932,94 +891,6 @@ static bool rtl2832_volatile_reg(struct device *dev, unsigned int reg)
}
/*
- * We implement own I2C access routines for regmap in order to get manual access
- * to I2C adapter lock, which is needed for I2C mux adapter.
- */
-static int rtl2832_regmap_read(void *context, const void *reg_buf,
- size_t reg_size, void *val_buf, size_t val_size)
-{
- struct i2c_client *client = context;
- int ret;
- struct i2c_msg msg[2] = {
- {
- .addr = client->addr,
- .flags = 0,
- .len = reg_size,
- .buf = (u8 *)reg_buf,
- }, {
- .addr = client->addr,
- .flags = I2C_M_RD,
- .len = val_size,
- .buf = val_buf,
- }
- };
-
- ret = __i2c_transfer(client->adapter, msg, 2);
- if (ret != 2) {
- dev_warn(&client->dev, "i2c reg read failed %d reg %02x\n",
- ret, *(u8 *)reg_buf);
- if (ret >= 0)
- ret = -EREMOTEIO;
- return ret;
- }
- return 0;
-}
-
-static int rtl2832_regmap_write(void *context, const void *data, size_t count)
-{
- struct i2c_client *client = context;
- int ret;
- struct i2c_msg msg[1] = {
- {
- .addr = client->addr,
- .flags = 0,
- .len = count,
- .buf = (u8 *)data,
- }
- };
-
- ret = __i2c_transfer(client->adapter, msg, 1);
- if (ret != 1) {
- dev_warn(&client->dev, "i2c reg write failed %d reg %02x\n",
- ret, *(u8 *)data);
- if (ret >= 0)
- ret = -EREMOTEIO;
- return ret;
- }
- return 0;
-}
-
-static int rtl2832_regmap_gather_write(void *context, const void *reg,
- size_t reg_len, const void *val,
- size_t val_len)
-{
- struct i2c_client *client = context;
- int ret;
- u8 buf[256];
- struct i2c_msg msg[1] = {
- {
- .addr = client->addr,
- .flags = 0,
- .len = 1 + val_len,
- .buf = buf,
- }
- };
-
- buf[0] = *(u8 const *)reg;
- memcpy(&buf[1], val, val_len);
-
- ret = __i2c_transfer(client->adapter, msg, 1);
- if (ret != 1) {
- dev_warn(&client->dev, "i2c reg write failed %d reg %02x\n",
- ret, *(u8 const *)reg);
- if (ret >= 0)
- ret = -EREMOTEIO;
- return ret;
- }
- return 0;
-}
-
-/*
* FIXME: Hack. Implement own regmap locking in order to silence lockdep
* recursive lock warning. That happens when regmap I2C client calls I2C mux
* adapter, which leads demod I2C repeater enable via demod regmap. Operation
@@ -1072,29 +943,29 @@ static int rtl2832_slave_ts_ctrl(struct i2c_client *client, bool enable)
ret = rtl2832_wr_demod_reg(dev, DVBT_SOFT_RST, 0x0);
if (ret)
goto err;
- ret = rtl2832_bulk_write(client, 0x10c, "\x5f\xff", 2);
+ ret = regmap_bulk_write(dev->regmap, 0x10c, "\x5f\xff", 2);
if (ret)
goto err;
ret = rtl2832_wr_demod_reg(dev, DVBT_PIP_ON, 0x1);
if (ret)
goto err;
- ret = rtl2832_bulk_write(client, 0x0bc, "\x18", 1);
+ ret = regmap_bulk_write(dev->regmap, 0x0bc, "\x18", 1);
if (ret)
goto err;
- ret = rtl2832_bulk_write(client, 0x192, "\x7f\xf7\xff", 3);
+ ret = regmap_bulk_write(dev->regmap, 0x192, "\x7f\xf7\xff", 3);
if (ret)
goto err;
} else {
- ret = rtl2832_bulk_write(client, 0x192, "\x00\x0f\xff", 3);
+ ret = regmap_bulk_write(dev->regmap, 0x192, "\x00\x0f\xff", 3);
if (ret)
goto err;
- ret = rtl2832_bulk_write(client, 0x0bc, "\x08", 1);
+ ret = regmap_bulk_write(dev->regmap, 0x0bc, "\x08", 1);
if (ret)
goto err;
ret = rtl2832_wr_demod_reg(dev, DVBT_PIP_ON, 0x0);
if (ret)
goto err;
- ret = rtl2832_bulk_write(client, 0x10c, "\x00\x00", 2);
+ ret = regmap_bulk_write(dev->regmap, 0x10c, "\x00\x00", 2);
if (ret)
goto err;
ret = rtl2832_wr_demod_reg(dev, DVBT_SOFT_RST, 0x1);
@@ -1123,7 +994,7 @@ static int rtl2832_pid_filter_ctrl(struct dvb_frontend *fe, int onoff)
else
u8tmp = 0x00;
- ret = rtl2832_update_bits(client, 0x061, 0xc0, u8tmp);
+ ret = regmap_update_bits(dev->regmap, 0x061, 0xc0, u8tmp);
if (ret)
goto err;
@@ -1158,14 +1029,14 @@ static int rtl2832_pid_filter(struct dvb_frontend *fe, u8 index, u16 pid,
buf[1] = (dev->filters >> 8) & 0xff;
buf[2] = (dev->filters >> 16) & 0xff;
buf[3] = (dev->filters >> 24) & 0xff;
- ret = rtl2832_bulk_write(client, 0x062, buf, 4);
+ ret = regmap_bulk_write(dev->regmap, 0x062, buf, 4);
if (ret)
goto err;
/* add PID */
buf[0] = (pid >> 8) & 0xff;
buf[1] = (pid >> 0) & 0xff;
- ret = rtl2832_bulk_write(client, 0x066 + 2 * index, buf, 2);
+ ret = regmap_bulk_write(dev->regmap, 0x066 + 2 * index, buf, 2);
if (ret)
goto err;
@@ -1183,12 +1054,6 @@ static int rtl2832_probe(struct i2c_client *client,
struct rtl2832_dev *dev;
int ret;
u8 tmp;
- static const struct regmap_bus regmap_bus = {
- .read = rtl2832_regmap_read,
- .write = rtl2832_regmap_write,
- .gather_write = rtl2832_regmap_gather_write,
- .val_format_endian_default = REGMAP_ENDIAN_NATIVE,
- };
static const struct regmap_range_cfg regmap_range_cfg[] = {
{
.selector_reg = 0x00,
@@ -1228,20 +1093,19 @@ static int rtl2832_probe(struct i2c_client *client,
dev->regmap_config.ranges = regmap_range_cfg,
dev->regmap_config.num_ranges = ARRAY_SIZE(regmap_range_cfg),
dev->regmap_config.cache_type = REGCACHE_NONE,
- dev->regmap = regmap_init(&client->dev, ®map_bus, client,
- &dev->regmap_config);
+ dev->regmap = regmap_init_i2c(client, &dev->regmap_config);
if (IS_ERR(dev->regmap)) {
ret = PTR_ERR(dev->regmap);
goto err_kfree;
}
/* check if the demod is there */
- ret = rtl2832_bulk_read(client, 0x000, &tmp, 1);
+ ret = regmap_bulk_read(dev->regmap, 0x000, &tmp, 1);
if (ret)
goto err_regmap_exit;
/* create muxed i2c adapter for demod tuner bus */
- dev->muxc = i2c_mux_alloc(i2c, &i2c->dev, 1, 0, 0,
+ dev->muxc = i2c_mux_alloc(i2c, &i2c->dev, 1, 0, I2C_MUX_LOCKED,
rtl2832_select, rtl2832_deselect);
if (!dev->muxc) {
ret = -ENOMEM;
@@ -1262,9 +1126,7 @@ static int rtl2832_probe(struct i2c_client *client,
pdata->slave_ts_ctrl = rtl2832_slave_ts_ctrl;
pdata->pid_filter = rtl2832_pid_filter;
pdata->pid_filter_ctrl = rtl2832_pid_filter_ctrl;
- pdata->bulk_read = rtl2832_bulk_read;
- pdata->bulk_write = rtl2832_bulk_write;
- pdata->update_bits = rtl2832_update_bits;
+ pdata->regmap = dev->regmap;
dev_info(&client->dev, "Realtek RTL2832 successfully attached\n");
return 0;
diff --git a/drivers/media/dvb-frontends/rtl2832.h b/drivers/media/dvb-frontends/rtl2832.h
index 6390af64cf45..03c0de039fa9 100644
--- a/drivers/media/dvb-frontends/rtl2832.h
+++ b/drivers/media/dvb-frontends/rtl2832.h
@@ -57,9 +57,7 @@ struct rtl2832_platform_data {
int (*pid_filter)(struct dvb_frontend *, u8, u16, int);
int (*pid_filter_ctrl)(struct dvb_frontend *, int);
/* private: Register access for SDR module use only */
- int (*bulk_read)(struct i2c_client *, unsigned int, void *, size_t);
- int (*bulk_write)(struct i2c_client *, unsigned int, const void *, size_t);
- int (*update_bits)(struct i2c_client *, unsigned int, unsigned int, unsigned int);
+ struct regmap *regmap;
};
#endif /* RTL2832_H */
diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
index b860f02a4e55..6a6b1debe277 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -35,6 +35,7 @@
#include <linux/platform_device.h>
#include <linux/jiffies.h>
#include <linux/math64.h>
+#include <linux/regmap.h>
static bool rtl2832_sdr_emulated_fmt;
module_param_named(emulated_formats, rtl2832_sdr_emulated_fmt, bool, 0644);
@@ -169,9 +170,9 @@ static int rtl2832_sdr_wr_regs(struct rtl2832_sdr_dev *dev, u16 reg,
{
struct platform_device *pdev = dev->pdev;
struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
- struct i2c_client *client = pdata->i2c_client;
+ struct regmap *regmap = pdata->regmap;
- return pdata->bulk_write(client, reg, val, len);
+ return regmap_bulk_write(regmap, reg, val, len);
}
#if 0
@@ -181,9 +182,9 @@ static int rtl2832_sdr_rd_regs(struct rtl2832_sdr_dev *dev, u16 reg, u8 *val,
{
struct platform_device *pdev = dev->pdev;
struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
- struct i2c_client *client = pdata->i2c_client;
+ struct regmap *regmap = pdata->regmap;
- return pdata->bulk_read(client, reg, val, len);
+ return regmap_bulk_read(regmap, reg, val, len);
}
#endif
@@ -199,9 +200,9 @@ static int rtl2832_sdr_wr_reg_mask(struct rtl2832_sdr_dev *dev, u16 reg,
{
struct platform_device *pdev = dev->pdev;
struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
- struct i2c_client *client = pdata->i2c_client;
+ struct regmap *regmap = pdata->regmap;
- return pdata->update_bits(client, reg, mask, val);
+ return regmap_update_bits(regmap, reg, mask, val);
}
/* Private functions */
diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.h b/drivers/media/dvb-frontends/rtl2832_sdr.h
index 342ea84860df..d8fc7e7212e3 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.h
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.h
@@ -56,10 +56,7 @@ struct rtl2832_sdr_platform_data {
#define RTL2832_SDR_TUNER_R828D 0x2b
u8 tuner;
- struct i2c_client *i2c_client;
- int (*bulk_read)(struct i2c_client *, unsigned int, void *, size_t);
- int (*bulk_write)(struct i2c_client *, unsigned int, const void *, size_t);
- int (*update_bits)(struct i2c_client *, unsigned int, unsigned int, unsigned int);
+ struct regmap *regmap;
struct dvb_frontend *dvb_frontend;
struct v4l2_subdev *v4l2_subdev;
struct dvb_usb_device *dvb_usb_device;
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index fa72642d41f3..eb7af8cb8aca 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1333,10 +1333,7 @@ static int rtl2832u_tuner_attach(struct dvb_usb_adapter *adap)
case TUNER_RTL2832_R828D:
pdata.clk = dev->rtl2832_platform_data.clk;
pdata.tuner = dev->tuner;
- pdata.i2c_client = dev->i2c_client_demod;
- pdata.bulk_read = dev->rtl2832_platform_data.bulk_read;
- pdata.bulk_write = dev->rtl2832_platform_data.bulk_write;
- pdata.update_bits = dev->rtl2832_platform_data.update_bits;
+ pdata.regmap = dev->rtl2832_platform_data.regmap;
pdata.dvb_frontend = adap->fe[0];
pdata.dvb_usb_device = d;
pdata.v4l2_subdev = subdev;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v7 22/24] [media] rtl2832: change the i2c gate to be mux-locked
2016-04-20 15:18 ` [PATCH v7 22/24] [media] rtl2832: " Peter Rosin
@ 2016-04-28 21:47 ` Wolfram Sang
2016-04-29 4:41 ` Peter Rosin
0 siblings, 1 reply; 47+ messages in thread
From: Wolfram Sang @ 2016-04-28 21:47 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 717 bytes --]
On Wed, Apr 20, 2016 at 05:18:02PM +0200, Peter Rosin wrote:
> The root i2c adapter lock is then no longer held by the i2c mux during
> accesses behind the i2c gate, and such accesses need to take that lock
> just like any other ordinary i2c accesses do.
>
> So, declare the i2c gate mux-locked, and zap the regmap overrides
> that makes the i2c accesses unlocked and use plain old regmap
> accesses. This also removes the need for the regmap wrappers used by
> rtl2832_sdr, so deconvolute the code further and provide the regmap
> handle directly instead of the wrapper functions.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Antti, I'd need some tag from you since this is not the i2c realm.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 22/24] [media] rtl2832: change the i2c gate to be mux-locked
2016-04-28 21:47 ` Wolfram Sang
@ 2016-04-29 4:41 ` Peter Rosin
2016-04-29 6:57 ` Wolfram Sang
0 siblings, 1 reply; 47+ messages in thread
From: Peter Rosin @ 2016-04-29 4:41 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
On 2016-04-28 23:47, Wolfram Sang wrote:
> On Wed, Apr 20, 2016 at 05:18:02PM +0200, Peter Rosin wrote:
>> The root i2c adapter lock is then no longer held by the i2c mux during
>> accesses behind the i2c gate, and such accesses need to take that lock
>> just like any other ordinary i2c accesses do.
>>
>> So, declare the i2c gate mux-locked, and zap the regmap overrides
>> that makes the i2c accesses unlocked and use plain old regmap
>> accesses. This also removes the need for the regmap wrappers used by
>> rtl2832_sdr, so deconvolute the code further and provide the regmap
>> handle directly instead of the wrapper functions.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> Antti, I'd need some tag from you since this is not the i2c realm.
>
Antti sent this:
https://lkml.org/lkml/2016/4/20/828
and I added a Tested-by in v8
https://github.com/peda-r/i2c-mux/commit/c023584d34db7aacbc59f28386378131cfa970d2
but the patch was never sent as an email, only as part of a pull request for
https://github.com/peda-r/i2c-mux/commits/mux-core-and-locking-8
So, I think all is ok, or do you need more than Tested-by?
Cheers,
Peter
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v7 22/24] [media] rtl2832: change the i2c gate to be mux-locked
2016-04-29 4:41 ` Peter Rosin
@ 2016-04-29 6:57 ` Wolfram Sang
0 siblings, 0 replies; 47+ messages in thread
From: Wolfram Sang @ 2016-04-29 6:57 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 93 bytes --]
> So, I think all is ok, or do you need more than Tested-by?
I think this will do. Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v7 23/24] [media] rtl2832_sdr: get rid of empty regmap wrappers
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (21 preceding siblings ...)
2016-04-20 15:18 ` [PATCH v7 22/24] [media] rtl2832: " Peter Rosin
@ 2016-04-20 15:18 ` Peter Rosin
2016-04-20 15:18 ` [PATCH v7 24/24] [media] rtl2832: regmap is aware of lockdep, drop local locking hack Peter Rosin
` (3 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Reviewed-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/dvb-frontends/rtl2832_sdr.c | 302 +++++++++++++-----------------
1 file changed, 132 insertions(+), 170 deletions(-)
diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
index 6a6b1debe277..47a480a7d46c 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -120,6 +120,7 @@ struct rtl2832_sdr_dev {
unsigned long flags;
struct platform_device *pdev;
+ struct regmap *regmap;
struct video_device vdev;
struct v4l2_device v4l2_dev;
@@ -164,47 +165,6 @@ struct rtl2832_sdr_dev {
unsigned long jiffies_next;
};
-/* write multiple registers */
-static int rtl2832_sdr_wr_regs(struct rtl2832_sdr_dev *dev, u16 reg,
- const u8 *val, int len)
-{
- struct platform_device *pdev = dev->pdev;
- struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
- struct regmap *regmap = pdata->regmap;
-
- return regmap_bulk_write(regmap, reg, val, len);
-}
-
-#if 0
-/* read multiple registers */
-static int rtl2832_sdr_rd_regs(struct rtl2832_sdr_dev *dev, u16 reg, u8 *val,
- int len)
-{
- struct platform_device *pdev = dev->pdev;
- struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
- struct regmap *regmap = pdata->regmap;
-
- return regmap_bulk_read(regmap, reg, val, len);
-}
-#endif
-
-/* write single register */
-static int rtl2832_sdr_wr_reg(struct rtl2832_sdr_dev *dev, u16 reg, u8 val)
-{
- return rtl2832_sdr_wr_regs(dev, reg, &val, 1);
-}
-
-/* write single register with mask */
-static int rtl2832_sdr_wr_reg_mask(struct rtl2832_sdr_dev *dev, u16 reg,
- u8 val, u8 mask)
-{
- struct platform_device *pdev = dev->pdev;
- struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
- struct regmap *regmap = pdata->regmap;
-
- return regmap_update_bits(regmap, reg, mask, val);
-}
-
/* Private functions */
static struct rtl2832_sdr_frame_buf *rtl2832_sdr_get_next_fill_buf(
struct rtl2832_sdr_dev *dev)
@@ -559,11 +519,11 @@ static int rtl2832_sdr_set_adc(struct rtl2832_sdr_dev *dev)
f_sr = dev->f_adc;
- ret = rtl2832_sdr_wr_regs(dev, 0x13e, "\x00\x00", 2);
+ ret = regmap_bulk_write(dev->regmap, 0x13e, "\x00\x00", 2);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_regs(dev, 0x115, "\x00\x00\x00\x00", 4);
+ ret = regmap_bulk_write(dev->regmap, 0x115, "\x00\x00\x00\x00", 4);
if (ret)
goto err;
@@ -589,7 +549,7 @@ static int rtl2832_sdr_set_adc(struct rtl2832_sdr_dev *dev)
buf[1] = (u32tmp >> 8) & 0xff;
buf[2] = (u32tmp >> 0) & 0xff;
- ret = rtl2832_sdr_wr_regs(dev, 0x119, buf, 3);
+ ret = regmap_bulk_write(dev->regmap, 0x119, buf, 3);
if (ret)
goto err;
@@ -603,15 +563,15 @@ static int rtl2832_sdr_set_adc(struct rtl2832_sdr_dev *dev)
u8tmp2 = 0xcd; /* enable ADC I, ADC Q */
}
- ret = rtl2832_sdr_wr_reg(dev, 0x1b1, u8tmp1);
+ ret = regmap_write(dev->regmap, 0x1b1, u8tmp1);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_reg(dev, 0x008, u8tmp2);
+ ret = regmap_write(dev->regmap, 0x008, u8tmp2);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_reg(dev, 0x006, 0x80);
+ ret = regmap_write(dev->regmap, 0x006, 0x80);
if (ret)
goto err;
@@ -622,168 +582,169 @@ static int rtl2832_sdr_set_adc(struct rtl2832_sdr_dev *dev)
buf[1] = (u32tmp >> 16) & 0xff;
buf[2] = (u32tmp >> 8) & 0xff;
buf[3] = (u32tmp >> 0) & 0xff;
- ret = rtl2832_sdr_wr_regs(dev, 0x19f, buf, 4);
+ ret = regmap_bulk_write(dev->regmap, 0x19f, buf, 4);
if (ret)
goto err;
/* low-pass filter */
- ret = rtl2832_sdr_wr_regs(dev, 0x11c,
- "\xca\xdc\xd7\xd8\xe0\xf2\x0e\x35\x06\x50\x9c\x0d\x71\x11\x14\x71\x74\x19\x41\xa5",
- 20);
+ ret = regmap_bulk_write(dev->regmap, 0x11c,
+ "\xca\xdc\xd7\xd8\xe0\xf2\x0e\x35\x06\x50\x9c\x0d\x71\x11\x14\x71\x74\x19\x41\xa5",
+ 20);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_regs(dev, 0x017, "\x11\x10", 2);
+ ret = regmap_bulk_write(dev->regmap, 0x017, "\x11\x10", 2);
if (ret)
goto err;
/* mode */
- ret = rtl2832_sdr_wr_regs(dev, 0x019, "\x05", 1);
+ ret = regmap_write(dev->regmap, 0x019, 0x05);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_regs(dev, 0x01a, "\x1b\x16\x0d\x06\x01\xff", 6);
+ ret = regmap_bulk_write(dev->regmap, 0x01a,
+ "\x1b\x16\x0d\x06\x01\xff", 6);
if (ret)
goto err;
/* FSM */
- ret = rtl2832_sdr_wr_regs(dev, 0x192, "\x00\xf0\x0f", 3);
+ ret = regmap_bulk_write(dev->regmap, 0x192, "\x00\xf0\x0f", 3);
if (ret)
goto err;
/* PID filter */
- ret = rtl2832_sdr_wr_regs(dev, 0x061, "\x60", 1);
+ ret = regmap_write(dev->regmap, 0x061, 0x60);
if (ret)
goto err;
/* used RF tuner based settings */
switch (pdata->tuner) {
case RTL2832_SDR_TUNER_E4000:
- ret = rtl2832_sdr_wr_regs(dev, 0x112, "\x5a", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x102, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x103, "\x5a", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c7, "\x30", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x104, "\xd0", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x105, "\xbe", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c8, "\x18", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x106, "\x35", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c9, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ca, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cb, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x107, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cd, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ce, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x108, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x109, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10a, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10b, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x011, "\xd4", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1e5, "\xf0", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1d9, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1db, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1dd, "\x14", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1de, "\xec", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1d8, "\x0c", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1e6, "\x02", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1d7, "\x09", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00d, "\x83", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x010, "\x49", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00d, "\x87", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00d, "\x85", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x013, "\x02", 1);
+ ret = regmap_write(dev->regmap, 0x112, 0x5a);
+ ret = regmap_write(dev->regmap, 0x102, 0x40);
+ ret = regmap_write(dev->regmap, 0x103, 0x5a);
+ ret = regmap_write(dev->regmap, 0x1c7, 0x30);
+ ret = regmap_write(dev->regmap, 0x104, 0xd0);
+ ret = regmap_write(dev->regmap, 0x105, 0xbe);
+ ret = regmap_write(dev->regmap, 0x1c8, 0x18);
+ ret = regmap_write(dev->regmap, 0x106, 0x35);
+ ret = regmap_write(dev->regmap, 0x1c9, 0x21);
+ ret = regmap_write(dev->regmap, 0x1ca, 0x21);
+ ret = regmap_write(dev->regmap, 0x1cb, 0x00);
+ ret = regmap_write(dev->regmap, 0x107, 0x40);
+ ret = regmap_write(dev->regmap, 0x1cd, 0x10);
+ ret = regmap_write(dev->regmap, 0x1ce, 0x10);
+ ret = regmap_write(dev->regmap, 0x108, 0x80);
+ ret = regmap_write(dev->regmap, 0x109, 0x7f);
+ ret = regmap_write(dev->regmap, 0x10a, 0x80);
+ ret = regmap_write(dev->regmap, 0x10b, 0x7f);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_write(dev->regmap, 0x011, 0xd4);
+ ret = regmap_write(dev->regmap, 0x1e5, 0xf0);
+ ret = regmap_write(dev->regmap, 0x1d9, 0x00);
+ ret = regmap_write(dev->regmap, 0x1db, 0x00);
+ ret = regmap_write(dev->regmap, 0x1dd, 0x14);
+ ret = regmap_write(dev->regmap, 0x1de, 0xec);
+ ret = regmap_write(dev->regmap, 0x1d8, 0x0c);
+ ret = regmap_write(dev->regmap, 0x1e6, 0x02);
+ ret = regmap_write(dev->regmap, 0x1d7, 0x09);
+ ret = regmap_write(dev->regmap, 0x00d, 0x83);
+ ret = regmap_write(dev->regmap, 0x010, 0x49);
+ ret = regmap_write(dev->regmap, 0x00d, 0x87);
+ ret = regmap_write(dev->regmap, 0x00d, 0x85);
+ ret = regmap_write(dev->regmap, 0x013, 0x02);
break;
case RTL2832_SDR_TUNER_FC0012:
case RTL2832_SDR_TUNER_FC0013:
- ret = rtl2832_sdr_wr_regs(dev, 0x112, "\x5a", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x102, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x103, "\x5a", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c7, "\x2c", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x104, "\xcc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x105, "\xbe", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c8, "\x16", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x106, "\x35", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c9, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ca, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cb, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x107, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cd, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ce, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x108, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x109, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10a, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10b, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x011, "\xe9\xbf", 2);
- ret = rtl2832_sdr_wr_regs(dev, 0x1e5, "\xf0", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1d9, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1db, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1dd, "\x11", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1de, "\xef", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1d8, "\x0c", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1e6, "\x02", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1d7, "\x09", 1);
+ ret = regmap_write(dev->regmap, 0x112, 0x5a);
+ ret = regmap_write(dev->regmap, 0x102, 0x40);
+ ret = regmap_write(dev->regmap, 0x103, 0x5a);
+ ret = regmap_write(dev->regmap, 0x1c7, 0x2c);
+ ret = regmap_write(dev->regmap, 0x104, 0xcc);
+ ret = regmap_write(dev->regmap, 0x105, 0xbe);
+ ret = regmap_write(dev->regmap, 0x1c8, 0x16);
+ ret = regmap_write(dev->regmap, 0x106, 0x35);
+ ret = regmap_write(dev->regmap, 0x1c9, 0x21);
+ ret = regmap_write(dev->regmap, 0x1ca, 0x21);
+ ret = regmap_write(dev->regmap, 0x1cb, 0x00);
+ ret = regmap_write(dev->regmap, 0x107, 0x40);
+ ret = regmap_write(dev->regmap, 0x1cd, 0x10);
+ ret = regmap_write(dev->regmap, 0x1ce, 0x10);
+ ret = regmap_write(dev->regmap, 0x108, 0x80);
+ ret = regmap_write(dev->regmap, 0x109, 0x7f);
+ ret = regmap_write(dev->regmap, 0x10a, 0x80);
+ ret = regmap_write(dev->regmap, 0x10b, 0x7f);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_bulk_write(dev->regmap, 0x011, "\xe9\xbf", 2);
+ ret = regmap_write(dev->regmap, 0x1e5, 0xf0);
+ ret = regmap_write(dev->regmap, 0x1d9, 0x00);
+ ret = regmap_write(dev->regmap, 0x1db, 0x00);
+ ret = regmap_write(dev->regmap, 0x1dd, 0x11);
+ ret = regmap_write(dev->regmap, 0x1de, 0xef);
+ ret = regmap_write(dev->regmap, 0x1d8, 0x0c);
+ ret = regmap_write(dev->regmap, 0x1e6, 0x02);
+ ret = regmap_write(dev->regmap, 0x1d7, 0x09);
break;
case RTL2832_SDR_TUNER_R820T:
case RTL2832_SDR_TUNER_R828D:
- ret = rtl2832_sdr_wr_regs(dev, 0x112, "\x5a", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x102, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x115, "\x01", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x103, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c7, "\x24", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x104, "\xcc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x105, "\xbe", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c8, "\x14", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x106, "\x35", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c9, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ca, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cb, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x107, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cd, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ce, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x108, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x109, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10a, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10b, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x011, "\xf4", 1);
+ ret = regmap_write(dev->regmap, 0x112, 0x5a);
+ ret = regmap_write(dev->regmap, 0x102, 0x40);
+ ret = regmap_write(dev->regmap, 0x115, 0x01);
+ ret = regmap_write(dev->regmap, 0x103, 0x80);
+ ret = regmap_write(dev->regmap, 0x1c7, 0x24);
+ ret = regmap_write(dev->regmap, 0x104, 0xcc);
+ ret = regmap_write(dev->regmap, 0x105, 0xbe);
+ ret = regmap_write(dev->regmap, 0x1c8, 0x14);
+ ret = regmap_write(dev->regmap, 0x106, 0x35);
+ ret = regmap_write(dev->regmap, 0x1c9, 0x21);
+ ret = regmap_write(dev->regmap, 0x1ca, 0x21);
+ ret = regmap_write(dev->regmap, 0x1cb, 0x00);
+ ret = regmap_write(dev->regmap, 0x107, 0x40);
+ ret = regmap_write(dev->regmap, 0x1cd, 0x10);
+ ret = regmap_write(dev->regmap, 0x1ce, 0x10);
+ ret = regmap_write(dev->regmap, 0x108, 0x80);
+ ret = regmap_write(dev->regmap, 0x109, 0x7f);
+ ret = regmap_write(dev->regmap, 0x10a, 0x80);
+ ret = regmap_write(dev->regmap, 0x10b, 0x7f);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_write(dev->regmap, 0x011, 0xf4);
break;
case RTL2832_SDR_TUNER_FC2580:
- ret = rtl2832_sdr_wr_regs(dev, 0x112, "\x39", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x102, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x103, "\x5a", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c7, "\x2c", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x104, "\xcc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x105, "\xbe", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c8, "\x16", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x106, "\x35", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1c9, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ca, "\x21", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cb, "\x00", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x107, "\x40", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1cd, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x1ce, "\x10", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x108, "\x80", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x109, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10a, "\x9c", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x10b, "\x7f", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x00e, "\xfc", 1);
- ret = rtl2832_sdr_wr_regs(dev, 0x011, "\xe9\xf4", 2);
+ ret = regmap_write(dev->regmap, 0x112, 0x39);
+ ret = regmap_write(dev->regmap, 0x102, 0x40);
+ ret = regmap_write(dev->regmap, 0x103, 0x5a);
+ ret = regmap_write(dev->regmap, 0x1c7, 0x2c);
+ ret = regmap_write(dev->regmap, 0x104, 0xcc);
+ ret = regmap_write(dev->regmap, 0x105, 0xbe);
+ ret = regmap_write(dev->regmap, 0x1c8, 0x16);
+ ret = regmap_write(dev->regmap, 0x106, 0x35);
+ ret = regmap_write(dev->regmap, 0x1c9, 0x21);
+ ret = regmap_write(dev->regmap, 0x1ca, 0x21);
+ ret = regmap_write(dev->regmap, 0x1cb, 0x00);
+ ret = regmap_write(dev->regmap, 0x107, 0x40);
+ ret = regmap_write(dev->regmap, 0x1cd, 0x10);
+ ret = regmap_write(dev->regmap, 0x1ce, 0x10);
+ ret = regmap_write(dev->regmap, 0x108, 0x80);
+ ret = regmap_write(dev->regmap, 0x109, 0x7f);
+ ret = regmap_write(dev->regmap, 0x10a, 0x9c);
+ ret = regmap_write(dev->regmap, 0x10b, 0x7f);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_write(dev->regmap, 0x00e, 0xfc);
+ ret = regmap_bulk_write(dev->regmap, 0x011, "\xe9\xf4", 2);
break;
default:
dev_notice(&pdev->dev, "Unsupported tuner\n");
}
/* software reset */
- ret = rtl2832_sdr_wr_reg_mask(dev, 0x101, 0x04, 0x04);
+ ret = regmap_update_bits(dev->regmap, 0x101, 0x04, 0x04);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_reg_mask(dev, 0x101, 0x00, 0x04);
+ ret = regmap_update_bits(dev->regmap, 0x101, 0x04, 0x00);
if (ret)
goto err;
err:
@@ -798,29 +759,29 @@ static void rtl2832_sdr_unset_adc(struct rtl2832_sdr_dev *dev)
dev_dbg(&pdev->dev, "\n");
/* PID filter */
- ret = rtl2832_sdr_wr_regs(dev, 0x061, "\xe0", 1);
+ ret = regmap_write(dev->regmap, 0x061, 0xe0);
if (ret)
goto err;
/* mode */
- ret = rtl2832_sdr_wr_regs(dev, 0x019, "\x20", 1);
+ ret = regmap_write(dev->regmap, 0x019, 0x20);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_regs(dev, 0x017, "\x11\x10", 2);
+ ret = regmap_bulk_write(dev->regmap, 0x017, "\x11\x10", 2);
if (ret)
goto err;
/* FSM */
- ret = rtl2832_sdr_wr_regs(dev, 0x192, "\x00\x0f\xff", 3);
+ ret = regmap_bulk_write(dev->regmap, 0x192, "\x00\x0f\xff", 3);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_regs(dev, 0x13e, "\x40\x00", 2);
+ ret = regmap_bulk_write(dev->regmap, 0x13e, "\x40\x00", 2);
if (ret)
goto err;
- ret = rtl2832_sdr_wr_regs(dev, 0x115, "\x06\x3f\xce\xcc", 4);
+ ret = regmap_bulk_write(dev->regmap, 0x115, "\x06\x3f\xce\xcc", 4);
if (ret)
goto err;
err:
@@ -1400,6 +1361,7 @@ static int rtl2832_sdr_probe(struct platform_device *pdev)
subdev = pdata->v4l2_subdev;
dev->v4l2_subdev = pdata->v4l2_subdev;
dev->pdev = pdev;
+ dev->regmap = pdata->regmap;
dev->udev = pdata->dvb_usb_device->udev;
dev->f_adc = bands_adc[0].rangelow;
dev->f_tuner = bands_fm[0].rangelow;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v7 24/24] [media] rtl2832: regmap is aware of lockdep, drop local locking hack
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (22 preceding siblings ...)
2016-04-20 15:18 ` [PATCH v7 23/24] [media] rtl2832_sdr: get rid of empty regmap wrappers Peter Rosin
@ 2016-04-20 15:18 ` Peter Rosin
2016-04-20 20:52 ` [PATCH v7 00/24] i2c mux cleanup and locking update Wolfram Sang
` (2 subsequent siblings)
26 siblings, 0 replies; 47+ messages in thread
From: Peter Rosin @ 2016-04-20 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Jonathan Corbet, Peter Korsgaard,
Guenter Roeck, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Antti Palosaari,
Mauro Carvalho Chehab, Rob Herring, Frank Rowand, Grant Likely,
Andrew Morton, David S. Miller, Greg Kroah-Hartman, Kalle Valo,
Jiri Slaby, Daniel Baluta, Lucas De Marchi, Adriana Reus,
Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil, Terry Heo,
Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard, linux-i2c,
linux-doc, linux-iio, linux-media, devicetree, Peter Rosin
Reviewed-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/media/dvb-frontends/rtl2832.c | 30 ------------------------------
drivers/media/dvb-frontends/rtl2832_priv.h | 1 -
2 files changed, 31 deletions(-)
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 957523f07f61..bfb6beedd40b 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -890,32 +890,6 @@ static bool rtl2832_volatile_reg(struct device *dev, unsigned int reg)
return false;
}
-/*
- * FIXME: Hack. Implement own regmap locking in order to silence lockdep
- * recursive lock warning. That happens when regmap I2C client calls I2C mux
- * adapter, which leads demod I2C repeater enable via demod regmap. Operation
- * takes two regmap locks recursively - but those are different regmap instances
- * in a two different I2C drivers, so it is not deadlock. Proper fix is to make
- * regmap aware of lockdep.
- */
-static void rtl2832_regmap_lock(void *__dev)
-{
- struct rtl2832_dev *dev = __dev;
- struct i2c_client *client = dev->client;
-
- dev_dbg(&client->dev, "\n");
- mutex_lock(&dev->regmap_mutex);
-}
-
-static void rtl2832_regmap_unlock(void *__dev)
-{
- struct rtl2832_dev *dev = __dev;
- struct i2c_client *client = dev->client;
-
- dev_dbg(&client->dev, "\n");
- mutex_unlock(&dev->regmap_mutex);
-}
-
static struct dvb_frontend *rtl2832_get_dvb_frontend(struct i2c_client *client)
{
struct rtl2832_dev *dev = i2c_get_clientdata(client);
@@ -1082,12 +1056,8 @@ static int rtl2832_probe(struct i2c_client *client,
dev->sleeping = true;
INIT_DELAYED_WORK(&dev->i2c_gate_work, rtl2832_i2c_gate_work);
/* create regmap */
- mutex_init(&dev->regmap_mutex);
dev->regmap_config.reg_bits = 8,
dev->regmap_config.val_bits = 8,
- dev->regmap_config.lock = rtl2832_regmap_lock,
- dev->regmap_config.unlock = rtl2832_regmap_unlock,
- dev->regmap_config.lock_arg = dev,
dev->regmap_config.volatile_reg = rtl2832_volatile_reg,
dev->regmap_config.max_register = 5 * 0x100,
dev->regmap_config.ranges = regmap_range_cfg,
diff --git a/drivers/media/dvb-frontends/rtl2832_priv.h b/drivers/media/dvb-frontends/rtl2832_priv.h
index d8f97d14f6fd..c1a8a69e9015 100644
--- a/drivers/media/dvb-frontends/rtl2832_priv.h
+++ b/drivers/media/dvb-frontends/rtl2832_priv.h
@@ -33,7 +33,6 @@
struct rtl2832_dev {
struct rtl2832_platform_data *pdata;
struct i2c_client *client;
- struct mutex regmap_mutex;
struct regmap_config regmap_config;
struct regmap *regmap;
struct i2c_mux_core *muxc;
--
2.1.4
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v7 00/24] i2c mux cleanup and locking update
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (23 preceding siblings ...)
2016-04-20 15:18 ` [PATCH v7 24/24] [media] rtl2832: regmap is aware of lockdep, drop local locking hack Peter Rosin
@ 2016-04-20 20:52 ` Wolfram Sang
2016-04-21 1:10 ` Antti Palosaari
2016-04-21 12:33 ` Crestez Dan Leonard
26 siblings, 0 replies; 47+ messages in thread
From: Wolfram Sang @ 2016-04-20 20:52 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, Crestez Dan Leonard, linux-i2c, linux-doc,
linux-iio, linux-media, devicetree, Peter Rosin
[-- Attachment #1: Type: text/plain, Size: 460 bytes --]
This was the diff of v6:
> 32 files changed, 1277 insertions(+), 915 deletions(-)
This is v7:
> 32 files changed, 1225 insertions(+), 916 deletions(-)
So, we gained a little overall. And while the individual drivers have a
few lines more now, I still think it is more readable.
So, thanks for doing that!
I'll give people some time for testing. I'll have a further look, too.
Hopefully, I can pick up patches 1-14 by the end of the week.
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v7 00/24] i2c mux cleanup and locking update
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (24 preceding siblings ...)
2016-04-20 20:52 ` [PATCH v7 00/24] i2c mux cleanup and locking update Wolfram Sang
@ 2016-04-21 1:10 ` Antti Palosaari
2016-04-21 12:33 ` Crestez Dan Leonard
26 siblings, 0 replies; 47+ messages in thread
From: Antti Palosaari @ 2016-04-21 1:10 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Wolfram Sang, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Mauro Carvalho Chehab, Rob Herring, Frank Rowand,
Grant Likely, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Kalle Valo, Jiri Slaby, Daniel Baluta, Lucas De Marchi,
Adriana Reus, Matt Ranostay, Krzysztof Kozlowski, Hans Verkuil,
Terry Heo, Arnd Bergmann, Tommi Rantala, Crestez Dan Leonard,
linux-i2c, linux-doc, linux-iio, linux-media, devicetree,
Peter Rosin
On 04/20/2016 06:17 PM, Peter Rosin wrote:
Retested all the previously tested + now I tested also cx231xx with
Hauppauge 930C HD device having eeprom other mux port and demod on the
other port.
> [media] si2168: change the i2c gate to be mux-locked
> [media] m88ds3103: convert to use an explicit i2c mux core
> [media] rtl2830: convert to use an explicit i2c mux core
> [media] rtl2832: convert to use an explicit i2c mux core
> [media] si2168: convert to use an explicit i2c mux core
> [media] cx231xx: convert to use an explicit i2c mux core
> [media] rtl2832: change the i2c gate to be mux-locked
> [media] rtl2832_sdr: get rid of empty regmap wrappers
> [media] rtl2832: regmap is aware of lockdep, drop local locking hack
I really hope that this whole patch series will arrive asap to mainline.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v7 00/24] i2c mux cleanup and locking update
2016-04-20 15:17 [PATCH v7 00/24] i2c mux cleanup and locking update Peter Rosin
` (25 preceding siblings ...)
2016-04-21 1:10 ` Antti Palosaari
@ 2016-04-21 12:33 ` Crestez Dan Leonard
26 siblings, 0 replies; 47+ messages in thread
From: Crestez Dan Leonard @ 2016-04-21 12:33 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Wolfram Sang, Jonathan Corbet, Peter Korsgaard, Guenter Roeck,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Antti Palosaari, Mauro Carvalho Chehab,
Rob Herring, Frank Rowand, Grant Likely, Andrew Morton,
David S. Miller, Greg Kroah-Hartman, Kalle Valo, Jiri Slaby,
Daniel Baluta, Lucas De Marchi, Adriana Reus, Matt Ranostay,
Krzysztof Kozlowski, Hans Verkuil, Terry Heo, Arnd Bergmann,
Tommi Rantala, linux-i2c, linux-doc, linux-iio, linux-media,
devicetree, Peter Rosin
On 04/20/2016 06:17 PM, Peter Rosin wrote:
> v7 compared to v6:
> - Removed i2c_mux_reserve_adapters, and all realloc attempts in
> i2c_mux_add_adapter. Supply a maximum number of adapters in i2c_mux_alloc
> instead.
> - Removed i2c_mux_one_adapter since it is was hard to use correctly, which
> was evident from the crash in the mpu6050 driver (on a mpu9150 chip) reported
> by Crestez Dan Leonard. Also, it didn't make things all that much simpler
> anyway (even if used correctly).
> - Rename i2c_mux_core:adapters into i2c_mux_core:num_adapters.
> - Some grammar and spelling fixes.
I tested this new version on mpu9150 and there are no more obvious
deadlocks or crashes. The magnetometer and accel/gyro can be used at the
same time without issues.
^ permalink raw reply [flat|nested] 47+ messages in thread