* Re: [PATCH v1 1/2] iio: imu: inv_mpu6050: Add i2c mux for by pass
[not found] ` <1416333184-1367-2-git-send-email-srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2014-11-22 11:58 ` Jonathan Cameron
[not found] ` <54707A80.90809-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2014-11-22 11:58 UTC (permalink / raw)
To: Srinivas Pandruvada, knaack.h-Mmb7MZpHnFY
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linux I2C
On 18/11/14 17:53, Srinivas Pandruvada wrote:
> This chip has a mode in which this chipset can be i2c master. But
> the current upstream driver doesn't support such mode as there is
> some limited support of clients, which can be connected.
>
> To attach such clients to main i2c bus chip has to be set up in
> bypass mode. Creates an i2c mux, which will enable/disable this mode.
> This was discussed for a while in mailing list, this was the decision.
>
> This change also provides an API, which allows clients to be created for
> this mux adapter.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Still wants to go to Wolfram and linux-i2c, given we are adding an i2c mux
deep in an IIO driver.
Whilst Wolfram was happy (iirc) with the approach he might want to take
a look at the implementation (and I'd rather have his ack before taking this).
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/Kconfig | 1 +
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 168 +++++++++++++++++++++++++++--
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 9 ++
> 3 files changed, 172 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig b/drivers/iio/imu/inv_mpu6050/Kconfig
> index 2d0608b..48fbc0b 100644
> --- a/drivers/iio/imu/inv_mpu6050/Kconfig
> +++ b/drivers/iio/imu/inv_mpu6050/Kconfig
> @@ -7,6 +7,7 @@ config INV_MPU6050_IIO
> depends on I2C && SYSFS
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> + select I2C_MUX
> help
> This driver supports the Invensense MPU6050 devices.
> This driver can also support MPU6500 in MPU6050 compatibility mode
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index b75519d..4fce0d1 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -23,6 +23,7 @@
> #include <linux/kfifo.h>
> #include <linux/spinlock.h>
> #include <linux/iio/iio.h>
> +#include <linux/i2c-mux.h>
> #include "inv_mpu_iio.h"
>
> /*
> @@ -52,6 +53,7 @@ static const struct inv_mpu6050_reg_map reg_set_6050 = {
> .int_enable = INV_MPU6050_REG_INT_ENABLE,
> .pwr_mgmt_1 = INV_MPU6050_REG_PWR_MGMT_1,
> .pwr_mgmt_2 = INV_MPU6050_REG_PWR_MGMT_2,
> + .int_pin_cfg = INV_MPU6050_REG_INT_PIN_CFG,
> };
>
> static const struct inv_mpu6050_chip_config chip_config_6050 = {
> @@ -72,11 +74,150 @@ static const struct inv_mpu6050_hw hw_info[INV_NUM_PARTS] = {
> },
> };
>
> +static struct i2c_adapter *inv_mux_adapter;
> +
> int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 d)
> {
> return i2c_smbus_write_i2c_block_data(st->client, reg, 1, &d);
> }
>
> +/*
> + * 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 inv_mpu6050_state *st,
> + u8 reg, u8 d)
> +{
> + int ret;
> + u8 buf[2];
> + struct i2c_msg msg[1] = {
> + {
> + .addr = st->client->addr,
> + .flags = 0,
> + .len = sizeof(buf),
> + .buf = buf,
> + }
> + };
> +
> + buf[0] = reg;
> + buf[1] = d;
> + ret = __i2c_transfer(st->client->adapter, msg, 1);
> + if (ret != 1)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
> + u32 chan_id)
> +{
> + struct iio_dev *indio_dev = mux_priv;
> + 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(st, st->reg->pwr_mgmt_1,
> + 0);
> + if (ret)
> + goto write_error;
> +
> + msleep(INV_MPU6050_REG_UP_TIME);
> + }
> + if (!ret) {
> + st->powerup_count++;
> + ret = inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
> + st->client->irq |
> + INV_MPU6050_BIT_BYPASS_EN);
> + }
> +write_error:
> + mutex_unlock(&indio_dev->mlock);
> +
> + return ret;
> +}
> +
> +static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
> + void *mux_priv, u32 chan_id)
> +{
> + struct iio_dev *indio_dev = mux_priv;
> + 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(st, st->reg->int_pin_cfg,
> + st->client->irq);
> + st->powerup_count--;
> + if (!st->powerup_count)
> + inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
> + INV_MPU6050_BIT_SLEEP);
> + mutex_unlock(&indio_dev->mlock);
> +
> + return 0;
> +}
> +
> +static int inv_mpu6050_create_mux(struct iio_dev *indio_dev)
> +{
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct i2c_client *client = st->client;
> +
> + st->mux_adapter = i2c_add_mux_adapter(client->adapter,
> + &client->dev,
> + indio_dev,
> + 0, 0, 0,
> + inv_mpu6050_select_bypass,
> + inv_mpu6050_deselect_bypass);
> + if (st->mux_adapter == NULL) {
> + dev_err(&st->client->dev, "Mux create Failed\n");
> + return -ENODEV;
> + }
> + inv_mux_adapter = st->mux_adapter;
> +
> + return 0;
> +}
> +
> +static void inv_mpu6050_destroy_mux(struct iio_dev *indio_dev)
> +{
> + struct inv_mpu6050_state *mpu_st = iio_priv(indio_dev);
> +
> + if (mpu_st->mux_adapter)
> + i2c_del_mux_adapter(mpu_st->mux_adapter);
> + inv_mux_adapter = NULL;
> +}
> +
> +struct i2c_client *inv_mpu6050_add_mux_client(char *name, unsigned short addr,
> + int irq, void *plat_data)
> +{
> + struct i2c_client *client;
> + struct i2c_board_info info;
> +
> + if (!inv_mux_adapter)
> + return NULL;
> +
> + memset(&info, 0, sizeof(struct i2c_board_info));
> + strlcpy(info.type, name, sizeof(info.type));
> + info.addr = addr;
> + info.irq = irq;
> + info.platform_data = plat_data;
> + client = i2c_new_device(inv_mux_adapter, &info);
> + if (!client) {
> + dev_err(&inv_mux_adapter->dev,
> + "failed to create mux client %d\n", addr);
> + return NULL;
> + }
> +
> + return client;
> +}
> +EXPORT_SYMBOL_GPL(inv_mpu6050_add_mux_client);
> +
> +void inv_mpu6050_del_mux_client(struct i2c_client *client)
> +{
> + i2c_unregister_device(client);
> +}
> +EXPORT_SYMBOL_GPL(inv_mpu6050_del_mux_client);
> +
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
> {
> u8 d, mgmt_1;
> @@ -133,13 +274,22 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>
> int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
> {
> - int result;
> + int result = 0;
> +
> + if (power_on) {
> + /* Already under indio-dev->mlock mutex */
> + if (!st->powerup_count)
> + result = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1,
> + 0);
> + if (!result)
> + st->powerup_count++;
> + } else {
> + st->powerup_count--;
> + if (!st->powerup_count)
> + result = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1,
> + INV_MPU6050_BIT_SLEEP);
> + }
>
> - if (power_on)
> - result = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1, 0);
> - else
> - result = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1,
> - INV_MPU6050_BIT_SLEEP);
> if (result)
> return result;
>
> @@ -673,6 +823,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>
> st = iio_priv(indio_dev);
> st->client = client;
> + st->powerup_count = 0;
> pdata = dev_get_platdata(&client->dev);
> if (pdata)
> st->plat_data = *pdata;
> @@ -720,6 +871,10 @@ static int inv_mpu_probe(struct i2c_client *client,
> goto out_remove_trigger;
> }
>
> + result = inv_mpu6050_create_mux(indio_dev);
> + if (result)
> + goto out_remove_trigger;
> +
> return 0;
>
> out_remove_trigger:
> @@ -734,6 +889,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>
> + inv_mpu6050_destroy_mux(indio_dev);
> iio_device_unregister(indio_dev);
> inv_mpu6050_remove_trigger(st);
> iio_triggered_buffer_cleanup(indio_dev);
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e779931..6f19b1e 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -54,6 +54,7 @@ struct inv_mpu6050_reg_map {
> u8 int_enable;
> u8 pwr_mgmt_1;
> u8 pwr_mgmt_2;
> + u8 int_pin_cfg;
> };
>
> /*device enum */
> @@ -119,6 +120,8 @@ struct inv_mpu6050_state {
> enum inv_devices chip_type;
> spinlock_t time_stamp_lock;
> struct i2c_client *client;
> + struct i2c_adapter *mux_adapter;
> + unsigned int powerup_count;
> struct inv_mpu6050_platform_data plat_data;
> DECLARE_KFIFO(timestamps, long long, TIMESTAMP_FIFO_SIZE);
> };
> @@ -179,6 +182,9 @@ struct inv_mpu6050_state {
> /* 6 + 6 round up and plus 8 */
> #define INV_MPU6050_OUTPUT_DATA_SIZE 24
>
> +#define INV_MPU6050_REG_INT_PIN_CFG 0x37
> +#define INV_MPU6050_BIT_BYPASS_EN 0x2
> +
> /* init parameters */
> #define INV_MPU6050_INIT_FIFO_RATE 50
> #define INV_MPU6050_TIME_STAMP_TOR 5
> @@ -245,3 +251,6 @@ int inv_reset_fifo(struct iio_dev *indio_dev);
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
> int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
> int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
> +struct i2c_client *inv_mpu6050_add_mux_client(char *name, unsigned short addr,
> + int irq, void *plat_data);
> +void inv_mpu6050_del_mux_client(struct i2c_client *client);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/2] iio: imu: inv_mpu6050: Add i2c mux for by pass
[not found] ` <54707A80.90809-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2014-11-22 13:08 ` Wolfram Sang
2014-11-22 20:20 ` Srinivas Pandruvada
0 siblings, 1 reply; 3+ messages in thread
From: Wolfram Sang @ 2014-11-22 13:08 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Srinivas Pandruvada, knaack.h-Mmb7MZpHnFY,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linux I2C
[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]
On Sat, Nov 22, 2014 at 11:58:56AM +0000, Jonathan Cameron wrote:
> On 18/11/14 17:53, Srinivas Pandruvada wrote:
> > This chip has a mode in which this chipset can be i2c master. But
I don't think it is a master...
> > the current upstream driver doesn't support such mode as there is
> > some limited support of clients, which can be connected.
> >
> > To attach such clients to main i2c bus chip has to be set up in
> > bypass mode. Creates an i2c mux, which will enable/disable this mode.
> > This was discussed for a while in mailing list, this was the decision.
... but more a by-pass? What is the advantage of putting slaves behind
the by-pass instead of directly connecting it to the bus?
> > This change also provides an API, which allows clients to be created for
> > this mux adapter.
> >
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Still wants to go to Wolfram and linux-i2c, given we are adding an i2c mux
> deep in an IIO driver.
>
> Whilst Wolfram was happy (iirc) with the approach he might want to take
> a look at the implementation (and I'd rather have his ack before taking this).
Thanks! Please notice my new email address.
> > +static struct i2c_adapter *inv_mux_adapter;
static??? And if I have multiple mpu6050 on the bus?
...
> > +struct i2c_client *inv_mpu6050_add_mux_client(char *name, unsigned short addr,
> > + int irq, void *plat_data)
> > +{
Huh? Why do we need that? Why can't we use the instantiation methods we
already have?
Rest looks okay from a first glimpse.
Wolfram
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/2] iio: imu: inv_mpu6050: Add i2c mux for by pass
2014-11-22 13:08 ` Wolfram Sang
@ 2014-11-22 20:20 ` Srinivas Pandruvada
0 siblings, 0 replies; 3+ messages in thread
From: Srinivas Pandruvada @ 2014-11-22 20:20 UTC (permalink / raw)
To: Wolfram Sang
Cc: Jonathan Cameron, knaack.h-Mmb7MZpHnFY,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Linux I2C
On Sat, 2014-11-22 at 14:08 +0100, Wolfram Sang wrote:
> On Sat, Nov 22, 2014 at 11:58:56AM +0000, Jonathan Cameron wrote:
> > On 18/11/14 17:53, Srinivas Pandruvada wrote:
> > > This chip has a mode in which this chipset can be i2c master. But
>
> I don't think it is a master...
>
Let me explain. The INV MPU65XX chipset provides an accelerometer and
gyroscope. But it allows one more sensor like AK8975 can be connected
directly to it. A INV MPU65XX device driver can access this AK8975 via
INV MPU65XX through its register interface. In this mode the AK8975
chipset is controlled by INV MPU chipset. So outside driver don't need
to worry about the chipset connected to it. The INV MPU chipset take
care of i2c communication to AK8975. You can't access AK8975 from main
processor's i2c bus. There is an analog switch in the MPU INV device
disconnects this client chipset from main processor i2c bus.
They provide a mode in which you can ask to connect the client chipset
like AK8975 to main processor i2c bus. This is called bypass mode. Once
you set MPU in bypass mode then AK8975 can be communicated like any i2c
client device from main processor i2c bus. Since MPU INV can go to sleep
mode deactivating analog switch, we have to make sure that each
transaction with AK8975 enable bypass mode.
> > > the current upstream driver doesn't support such mode as there is
> > > some limited support of clients, which can be connected.
> > >
> > > To attach such clients to main i2c bus chip has to be set up in
> > > bypass mode. Creates an i2c mux, which will enable/disable this mode.
> > > This was discussed for a while in mailing list, this was the decision.
>
> ... but more a by-pass? What is the advantage of putting slaves behind
> the by-pass instead of directly connecting it to the bus?
bypass by directly connecting to processor i2c bus.
>
> > > This change also provides an API, which allows clients to be created for
> > > this mux adapter.
> > >
> > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Still wants to go to Wolfram and linux-i2c, given we are adding an i2c mux
> > deep in an IIO driver.
> >
> > Whilst Wolfram was happy (iirc) with the approach he might want to take
> > a look at the implementation (and I'd rather have his ack before taking this).
>
> Thanks! Please notice my new email address.
>
> > > +static struct i2c_adapter *inv_mux_adapter;
>
> static??? And if I have multiple mpu6050 on the bus?
>
I am not sure that you can connect two MPU devices as i2c address is
fixed at 0x68. But I can make part of mpu device instance structure
removing static.
> ...
>
> > > +struct i2c_client *inv_mpu6050_add_mux_client(char *name, unsigned short addr,
> > > + int irq, void *plat_data)
> > > +{
>
> Huh? Why do we need that? Why can't we use the instantiation methods we
> already have?
>
Unfortunately no. These are Windows 8 tablets with assumption that INV
MPU6X driver is controlling AK8975, without any special driver for
AK8975 (Linux drivers doesn't have this capability). So the ACPI config
data will not have any entry for AK8975 as a device with i2c address. We
have to parse INV MPU6X ACPI configuration's private data in a DMI
specific x86 platform driver and find out AK8975 address and create a
client device for this i2c-mux. This configuration doesn't follow any
standard ACPI device with i2c configuration (Here only INV MPU6X is
shown as a i2c device in ACPI).
Thanks,
Srinivas
> Rest looks okay from a first glimpse.
>
> Wolfram
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-22 20:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1416333184-1367-1-git-send-email-srinivas.pandruvada@linux.intel.com>
[not found] ` <1416333184-1367-2-git-send-email-srinivas.pandruvada@linux.intel.com>
[not found] ` <1416333184-1367-2-git-send-email-srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2014-11-22 11:58 ` [PATCH v1 1/2] iio: imu: inv_mpu6050: Add i2c mux for by pass Jonathan Cameron
[not found] ` <54707A80.90809-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-11-22 13:08 ` Wolfram Sang
2014-11-22 20:20 ` Srinivas Pandruvada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).