2016-04-20 13:15:31

by Crestez Dan Leonard

[permalink] [raw]
Subject: [PATCH 0/5] iio: inv_mpu6050: Cleanup chip types and add mpu9150

This adds support for mpu9150 explictly. This device includes a MPU60X0 and
AK8975 inside the same package. It also cleans the chip_type <=> hw_info
mappings which were just a mess.

Crestez Dan Leonard (5):
iio: inv_mpu6050: Cleanup hw_info mapping
iio: inv_mpu6050: Remove inv_mpu6050_hw.num_reg
iio: inv_mpu6050: Check WHO_AM_I register on probe
iio: inv_mpu6050: Add spi_device_id for INV_MPU6500
iio: inv_mpu6050: Add explicit support for MPU9150

drivers/iio/imu/inv_mpu6050/Kconfig | 10 ++++----
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 37 +++++++++++++++++++++++++++---
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 3 ++-
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 12 ++++++++--
drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 20 ++++++++++++----
5 files changed, 66 insertions(+), 16 deletions(-)

--
2.5.5


2016-04-20 13:15:34

by Crestez Dan Leonard

[permalink] [raw]
Subject: [PATCH 2/5] iio: inv_mpu6050: Remove inv_mpu6050_hw.num_reg

This field was unused and incorrect for mpu6500.

Signed-off-by: Crestez Dan Leonard <[email protected]>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 3 ---
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 --
2 files changed, 5 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 52e62b3..faccabafc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -91,19 +91,16 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
/* Indexed by enum inv_devices */
static const struct inv_mpu6050_hw hw_info[] = {
{
- .num_reg = 117,
.name = "MPU6050",
.reg = &reg_set_6050,
.config = &chip_config_6050,
},
{
- .num_reg = 117,
.name = "MPU6500",
.reg = &reg_set_6500,
.config = &chip_config_6050,
},
{
- .num_reg = 117,
.name = "MPU6000",
.reg = &reg_set_6050,
.config = &chip_config_6050,
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e302a49..c66dbfc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -93,13 +93,11 @@ struct inv_mpu6050_chip_config {

/**
* struct inv_mpu6050_hw - Other important hardware information.
- * @num_reg: Number of registers on device.
* @name: name of the chip.
* @reg: register map of the chip.
* @config: configuration of the chip.
*/
struct inv_mpu6050_hw {
- u8 num_reg;
u8 *name;
const struct inv_mpu6050_reg_map *reg;
const struct inv_mpu6050_chip_config *config;
--
2.5.5

2016-04-20 13:15:42

by Crestez Dan Leonard

[permalink] [raw]
Subject: [PATCH 5/5] iio: inv_mpu6050: Add explicit support for MPU9150

This device is a package containing a MPU6050-like sensor and an AK8975
magnetometer. The magnetometer component is supported by the existing
ak8975 driver.

This patch also rephrases the Kconfig descriptions.

Signed-off-by: Crestez Dan Leonard <[email protected]>
---
drivers/iio/imu/inv_mpu6050/Kconfig | 10 ++++------
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 ++++++
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 1 +
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 ++
drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 1 +
5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig b/drivers/iio/imu/inv_mpu6050/Kconfig
index a7f557a..c05d474 100644
--- a/drivers/iio/imu/inv_mpu6050/Kconfig
+++ b/drivers/iio/imu/inv_mpu6050/Kconfig
@@ -14,10 +14,8 @@ config INV_MPU6050_I2C
select I2C_MUX
select REGMAP_I2C
help
- This driver supports the Invensense MPU6050 devices.
- This driver can also support MPU6500 in MPU6050 compatibility mode
- and also in MPU6500 mode with some limitations.
- It is a gyroscope/accelerometer combo device.
+ This driver supports the Invensense MPU6050/6500/9150 motion tracking
+ devices over I2C.
This driver can be built as a module. The module will be called
inv-mpu6050-i2c.

@@ -27,7 +25,7 @@ config INV_MPU6050_SPI
select INV_MPU6050_IIO
select REGMAP_SPI
help
- This driver supports the Invensense MPU6050 devices.
- It is a gyroscope/accelerometer combo device.
+ This driver supports the Invensense MPU6000/6500/9150 motion tracking
+ devices over SPI.
This driver can be built as a module. The module will be called
inv-mpu6050-spi.
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 273b7fa7..3a82a49 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -108,6 +108,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
.reg = &reg_set_6050,
.config = &chip_config_6050,
},
+ {
+ .whoami = INV_MPU9150_WHOAMI_VALUE,
+ .name = "MPU9150",
+ .reg = &reg_set_6050,
+ .config = &chip_config_6050,
+ },
};

int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index bb1a7b1..1a424a6 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -202,6 +202,7 @@ static int inv_mpu_remove(struct i2c_client *client)
static const struct i2c_device_id inv_mpu_id[] = {
{"mpu6050", INV_MPU6050},
{"mpu6500", INV_MPU6500},
+ {"mpu9150", INV_MPU9150},
{}
};

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 564cabd..38d6a09 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -68,6 +68,7 @@ enum inv_devices {
INV_MPU6050,
INV_MPU6500,
INV_MPU6000,
+ INV_MPU9150,
INV_NUM_PARTS
};

@@ -220,6 +221,7 @@ struct inv_mpu6050_state {
#define INV_MPU6000_WHOAMI_VALUE 0x68
#define INV_MPU6050_WHOAMI_VALUE 0x68
#define INV_MPU6500_WHOAMI_VALUE 0x70
+#define INV_MPU9150_WHOAMI_VALUE 0x68

/* scan element definition */
enum inv_mpu6050_scan {
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index a0f8df2..190a4a5 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -81,6 +81,7 @@ static int inv_mpu_remove(struct spi_device *spi)
static const struct spi_device_id inv_mpu_id[] = {
{"mpu6000", INV_MPU6000},
{"mpu6500", INV_MPU6500},
+ {"mpu9150", INV_MPU9150},
{}
};

--
2.5.5

2016-04-20 13:15:40

by Crestez Dan Leonard

[permalink] [raw]
Subject: [PATCH 4/5] iio: inv_mpu6050: Add spi_device_id for INV_MPU6500

Signed-off-by: Crestez Dan Leonard <[email protected]>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index 3972a46..a0f8df2 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -80,6 +80,7 @@ static int inv_mpu_remove(struct spi_device *spi)
*/
static const struct spi_device_id inv_mpu_id[] = {
{"mpu6000", INV_MPU6000},
+ {"mpu6500", INV_MPU6500},
{}
};

--
2.5.5

2016-04-20 13:15:38

by Crestez Dan Leonard

[permalink] [raw]
Subject: [PATCH 3/5] iio: inv_mpu6050: Check WHO_AM_I register on probe

This can be used to distinguish mpu6500. This is a warning rather than
an error because the differences are mostly irrelevant and it's nice to
avoid breaking users with slightly incorrect ACPI/DT.

Signed-off-by: Crestez Dan Leonard <[email protected]>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 15 +++++++++++++++
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 8 ++++++++
2 files changed, 23 insertions(+)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index faccabafc..273b7fa7 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -91,16 +91,19 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
/* Indexed by enum inv_devices */
static const struct inv_mpu6050_hw hw_info[] = {
{
+ .whoami = INV_MPU6050_WHOAMI_VALUE,
.name = "MPU6050",
.reg = &reg_set_6050,
.config = &chip_config_6050,
},
{
+ .whoami = INV_MPU6500_WHOAMI_VALUE,
.name = "MPU6500",
.reg = &reg_set_6500,
.config = &chip_config_6050,
},
{
+ .whoami = INV_MPU6000_WHOAMI_VALUE,
.name = "MPU6000",
.reg = &reg_set_6050,
.config = &chip_config_6050,
@@ -730,6 +733,7 @@ static const struct iio_info mpu_info = {
static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
{
int result;
+ unsigned int regval;

st->hw = &hw_info[st->chip_type];
st->reg = hw_info[st->chip_type].reg;
@@ -740,6 +744,17 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
if (result)
return result;
msleep(INV_MPU6050_POWER_UP_TIME);
+
+ /* check chip self-identification */
+ result = regmap_read(st->map, INV_MPU6050_REG_WHOAMI, &regval);
+ if (result)
+ return result;
+ if (regval != st->hw->whoami) {
+ dev_warn(regmap_get_device(st->map),
+ "whoami mismatch got %#02x expected %#02hhx for %s\n",
+ regval, st->hw->whoami, st->hw->name);
+ }
+
/*
* toggle power state. After reset, the sleep bit could be on
* or off depending on the OTP settings. Toggling power would
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index c66dbfc..564cabd 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -93,11 +93,13 @@ struct inv_mpu6050_chip_config {

/**
* struct inv_mpu6050_hw - Other important hardware information.
+ * @whoami: Self identification byte from WHO_AM_I register
* @name: name of the chip.
* @reg: register map of the chip.
* @config: configuration of the chip.
*/
struct inv_mpu6050_hw {
+ u8 whoami;
u8 *name;
const struct inv_mpu6050_reg_map *reg;
const struct inv_mpu6050_chip_config *config;
@@ -213,6 +215,12 @@ struct inv_mpu6050_state {
#define INV_MPU6050_MIN_FIFO_RATE 4
#define INV_MPU6050_ONE_K_HZ 1000

+#define INV_MPU6050_REG_WHOAMI 117
+
+#define INV_MPU6000_WHOAMI_VALUE 0x68
+#define INV_MPU6050_WHOAMI_VALUE 0x68
+#define INV_MPU6500_WHOAMI_VALUE 0x70
+
/* scan element definition */
enum inv_mpu6050_scan {
INV_MPU6050_SCAN_ACCL_X,
--
2.5.5

2016-04-20 13:16:50

by Crestez Dan Leonard

[permalink] [raw]
Subject: [PATCH 1/5] iio: inv_mpu6050: Cleanup hw_info mapping

The hw_info array was indexed by enum inv_devices chip_type despite the
fact that the enumeration had more members than the array and was
ordered differently.

The patch cleans this up and adds explicit chip_types to i2c/spi/acpi
IDs. It also adds some stricter checks inside the driver core.

This happened to work so far because the differences between the
supported models are very minor.

Signed-off-by: Crestez Dan Leonard <[email protected]>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 15 ++++++++++++++-
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 18 ++++++++++++++----
3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index d192953..52e62b3 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -88,16 +88,23 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
.accl_fs = INV_MPU6050_FS_02G,
};

+/* Indexed by enum inv_devices */
static const struct inv_mpu6050_hw hw_info[] = {
{
.num_reg = 117,
+ .name = "MPU6050",
+ .reg = &reg_set_6050,
+ .config = &chip_config_6050,
+ },
+ {
+ .num_reg = 117,
.name = "MPU6500",
.reg = &reg_set_6500,
.config = &chip_config_6050,
},
{
.num_reg = 117,
- .name = "MPU6050",
+ .name = "MPU6000",
.reg = &reg_set_6050,
.config = &chip_config_6050,
},
@@ -774,6 +781,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
if (!indio_dev)
return -ENOMEM;

+ BUILD_BUG_ON(ARRAY_SIZE(hw_info) != INV_NUM_PARTS);
+ if (chip_type < 0 || chip_type >= INV_NUM_PARTS) {
+ dev_err(dev, "Bad invensense chip_type=%d name=%s\n",
+ chip_type, name);
+ return -ENODEV;
+ }
st = iio_priv(indio_dev);
st->chip_type = chip_type;
st->powerup_count = 0;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 5ee4e0d..bb1a7b1 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -208,7 +208,7 @@ static const struct i2c_device_id inv_mpu_id[] = {
MODULE_DEVICE_TABLE(i2c, inv_mpu_id);

static const struct acpi_device_id inv_acpi_match[] = {
- {"INVN6500", 0},
+ {"INVN6500", INV_MPU6500},
{ },
};

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index 7bcb8d8..3972a46 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -44,9 +44,19 @@ static int inv_mpu_i2c_disable(struct iio_dev *indio_dev)
static int inv_mpu_probe(struct spi_device *spi)
{
struct regmap *regmap;
- const struct spi_device_id *id = spi_get_device_id(spi);
- const char *name = id ? id->name : NULL;
- const int chip_type = id ? id->driver_data : 0;
+ const struct spi_device_id *spi_id;
+ const struct acpi_device_id *acpi_id;
+ const char *name = NULL;
+ enum inv_devices chip_type;
+
+ if ((spi_id = spi_get_device_id(spi))) {
+ chip_type = (enum inv_devices)spi_id->driver_data;
+ name = spi_id->name;
+ } else if ((acpi_id = acpi_match_device(spi->dev.driver->acpi_match_table, &spi->dev))) {
+ chip_type = (enum inv_devices)acpi_id->driver_data;
+ } else {
+ return -ENODEV;
+ }

regmap = devm_regmap_init_spi(spi, &inv_mpu_regmap_config);
if (IS_ERR(regmap)) {
@@ -76,7 +86,7 @@ static const struct spi_device_id inv_mpu_id[] = {
MODULE_DEVICE_TABLE(spi, inv_mpu_id);

static const struct acpi_device_id inv_acpi_match[] = {
- {"INVN6000", 0},
+ {"INVN6000", INV_MPU6000},
{ },
};
MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
--
2.5.5

2016-04-24 11:10:25

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/5] iio: inv_mpu6050: Cleanup hw_info mapping

On 20/04/16 14:15, Crestez Dan Leonard wrote:
> The hw_info array was indexed by enum inv_devices chip_type despite the
> fact that the enumeration had more members than the array and was
> ordered differently.
>
> The patch cleans this up and adds explicit chip_types to i2c/spi/acpi
> IDs. It also adds some stricter checks inside the driver core.
>
> This happened to work so far because the differences between the
> supported models are very minor.
>
> Signed-off-by: Crestez Dan Leonard <[email protected]>h
Ideally I'd like an Ack / review from Ge on these.
The same is true for the whole series.

Looks good to me though!

Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 15 ++++++++++++++-
> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 2 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 18 ++++++++++++++----
> 3 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index d192953..52e62b3 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -88,16 +88,23 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
> .accl_fs = INV_MPU6050_FS_02G,
> };
>
> +/* Indexed by enum inv_devices */
> static const struct inv_mpu6050_hw hw_info[] = {
> {
> .num_reg = 117,
> + .name = "MPU6050",
> + .reg = &reg_set_6050,
> + .config = &chip_config_6050,
> + },
> + {
> + .num_reg = 117,
> .name = "MPU6500",
> .reg = &reg_set_6500,
> .config = &chip_config_6050,
> },
> {
> .num_reg = 117,
> - .name = "MPU6050",
> + .name = "MPU6000",
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> },
> @@ -774,6 +781,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> if (!indio_dev)
> return -ENOMEM;
>
> + BUILD_BUG_ON(ARRAY_SIZE(hw_info) != INV_NUM_PARTS);
> + if (chip_type < 0 || chip_type >= INV_NUM_PARTS) {
> + dev_err(dev, "Bad invensense chip_type=%d name=%s\n",
> + chip_type, name);
> + return -ENODEV;
> + }
> st = iio_priv(indio_dev);
> st->chip_type = chip_type;
> st->powerup_count = 0;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 5ee4e0d..bb1a7b1 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -208,7 +208,7 @@ static const struct i2c_device_id inv_mpu_id[] = {
> MODULE_DEVICE_TABLE(i2c, inv_mpu_id);
>
> static const struct acpi_device_id inv_acpi_match[] = {
> - {"INVN6500", 0},
> + {"INVN6500", INV_MPU6500},
> { },
> };
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> index 7bcb8d8..3972a46 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> @@ -44,9 +44,19 @@ static int inv_mpu_i2c_disable(struct iio_dev *indio_dev)
> static int inv_mpu_probe(struct spi_device *spi)
> {
> struct regmap *regmap;
> - const struct spi_device_id *id = spi_get_device_id(spi);
> - const char *name = id ? id->name : NULL;
> - const int chip_type = id ? id->driver_data : 0;
> + const struct spi_device_id *spi_id;
> + const struct acpi_device_id *acpi_id;
> + const char *name = NULL;
> + enum inv_devices chip_type;
> +
> + if ((spi_id = spi_get_device_id(spi))) {
> + chip_type = (enum inv_devices)spi_id->driver_data;
> + name = spi_id->name;
> + } else if ((acpi_id = acpi_match_device(spi->dev.driver->acpi_match_table, &spi->dev))) {
> + chip_type = (enum inv_devices)acpi_id->driver_data;
> + } else {
> + return -ENODEV;
> + }
>
> regmap = devm_regmap_init_spi(spi, &inv_mpu_regmap_config);
> if (IS_ERR(regmap)) {
> @@ -76,7 +86,7 @@ static const struct spi_device_id inv_mpu_id[] = {
> MODULE_DEVICE_TABLE(spi, inv_mpu_id);
>
> static const struct acpi_device_id inv_acpi_match[] = {
> - {"INVN6000", 0},
> + {"INVN6000", INV_MPU6000},
> { },
> };
> MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
>

2016-04-24 11:12:09

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/5] iio: inv_mpu6050: Remove inv_mpu6050_hw.num_reg

On 20/04/16 14:15, Crestez Dan Leonard wrote:
> This field was unused and incorrect for mpu6500.
>
> Signed-off-by: Crestez Dan Leonard <[email protected]>.
This one I think I can safely take :)

Good spot
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 3 ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 --
> 2 files changed, 5 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 52e62b3..faccabafc 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -91,19 +91,16 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
> /* Indexed by enum inv_devices */
> static const struct inv_mpu6050_hw hw_info[] = {
> {
> - .num_reg = 117,
> .name = "MPU6050",
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> },
> {
> - .num_reg = 117,
> .name = "MPU6500",
> .reg = &reg_set_6500,
> .config = &chip_config_6050,
> },
> {
> - .num_reg = 117,
> .name = "MPU6000",
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e302a49..c66dbfc 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -93,13 +93,11 @@ struct inv_mpu6050_chip_config {
>
> /**
> * struct inv_mpu6050_hw - Other important hardware information.
> - * @num_reg: Number of registers on device.
> * @name: name of the chip.
> * @reg: register map of the chip.
> * @config: configuration of the chip.
> */
> struct inv_mpu6050_hw {
> - u8 num_reg;
> u8 *name;
> const struct inv_mpu6050_reg_map *reg;
> const struct inv_mpu6050_chip_config *config;
>

2016-04-24 11:14:24

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/5] iio: inv_mpu6050: Check WHO_AM_I register on probe

On 20/04/16 14:15, Crestez Dan Leonard wrote:
> This can be used to distinguish mpu6500. This is a warning rather than
> an error because the differences are mostly irrelevant and it's nice to
> avoid breaking users with slightly incorrect ACPI/DT.
>
> Signed-off-by: Crestez Dan Leonard <[email protected]>
Would we be better off fixing their configuration though by using the right part
if we can identify it? So if wrong, maybe we should search the info table to
figure out what it is? I'm not certain on this though as then we are trying to
deal with unknown future cases - maybe what you have here is the best balance.

Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 15 +++++++++++++++
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 8 ++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index faccabafc..273b7fa7 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -91,16 +91,19 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
> /* Indexed by enum inv_devices */
> static const struct inv_mpu6050_hw hw_info[] = {
> {
> + .whoami = INV_MPU6050_WHOAMI_VALUE,
> .name = "MPU6050",
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> },
> {
> + .whoami = INV_MPU6500_WHOAMI_VALUE,
> .name = "MPU6500",
> .reg = &reg_set_6500,
> .config = &chip_config_6050,
> },
> {
> + .whoami = INV_MPU6000_WHOAMI_VALUE,
> .name = "MPU6000",
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> @@ -730,6 +733,7 @@ static const struct iio_info mpu_info = {
> static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
> {
> int result;
> + unsigned int regval;
>
> st->hw = &hw_info[st->chip_type];
> st->reg = hw_info[st->chip_type].reg;
> @@ -740,6 +744,17 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
> if (result)
> return result;
> msleep(INV_MPU6050_POWER_UP_TIME);
> +
> + /* check chip self-identification */
> + result = regmap_read(st->map, INV_MPU6050_REG_WHOAMI, &regval);
> + if (result)
> + return result;
> + if (regval != st->hw->whoami) {
> + dev_warn(regmap_get_device(st->map),
> + "whoami mismatch got %#02x expected %#02hhx for %s\n",
> + regval, st->hw->whoami, st->hw->name);
> + }
> +
> /*
> * toggle power state. After reset, the sleep bit could be on
> * or off depending on the OTP settings. Toggling power would
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index c66dbfc..564cabd 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -93,11 +93,13 @@ struct inv_mpu6050_chip_config {
>
> /**
> * struct inv_mpu6050_hw - Other important hardware information.
> + * @whoami: Self identification byte from WHO_AM_I register
> * @name: name of the chip.
> * @reg: register map of the chip.
> * @config: configuration of the chip.
> */
> struct inv_mpu6050_hw {
> + u8 whoami;
> u8 *name;
> const struct inv_mpu6050_reg_map *reg;
> const struct inv_mpu6050_chip_config *config;
> @@ -213,6 +215,12 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_MIN_FIFO_RATE 4
> #define INV_MPU6050_ONE_K_HZ 1000
>
> +#define INV_MPU6050_REG_WHOAMI 117
> +
> +#define INV_MPU6000_WHOAMI_VALUE 0x68
> +#define INV_MPU6050_WHOAMI_VALUE 0x68
> +#define INV_MPU6500_WHOAMI_VALUE 0x70
> +
> /* scan element definition */
> enum inv_mpu6050_scan {
> INV_MPU6050_SCAN_ACCL_X,
>

2016-04-24 11:16:23

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 5/5] iio: inv_mpu6050: Add explicit support for MPU9150

On 20/04/16 14:15, Crestez Dan Leonard wrote:
> This device is a package containing a MPU6050-like sensor and an AK8975
> magnetometer. The magnetometer component is supported by the existing
> ak8975 driver.
>
> This patch also rephrases the Kconfig descriptions.
>
> Signed-off-by: Crestez Dan Leonard <[email protected]>
Looks good. Will just wait for Ge to hopefully have time to take a look
at the whole series. For future reference, it's always worth making sure
you have cc'd the original author of a driver unless you know someone
else is definitely maintaining it.

Ge was reply to emails recently so I guess he is still very much active!

Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/Kconfig | 10 ++++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 ++++++
> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 1 +
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 ++
> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 1 +
> 5 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig b/drivers/iio/imu/inv_mpu6050/Kconfig
> index a7f557a..c05d474 100644
> --- a/drivers/iio/imu/inv_mpu6050/Kconfig
> +++ b/drivers/iio/imu/inv_mpu6050/Kconfig
> @@ -14,10 +14,8 @@ config INV_MPU6050_I2C
> select I2C_MUX
> select REGMAP_I2C
> help
> - This driver supports the Invensense MPU6050 devices.
> - This driver can also support MPU6500 in MPU6050 compatibility mode
> - and also in MPU6500 mode with some limitations.
> - It is a gyroscope/accelerometer combo device.
> + This driver supports the Invensense MPU6050/6500/9150 motion tracking
> + devices over I2C.
> This driver can be built as a module. The module will be called
> inv-mpu6050-i2c.
>
> @@ -27,7 +25,7 @@ config INV_MPU6050_SPI
> select INV_MPU6050_IIO
> select REGMAP_SPI
> help
> - This driver supports the Invensense MPU6050 devices.
> - It is a gyroscope/accelerometer combo device.
> + This driver supports the Invensense MPU6000/6500/9150 motion tracking
> + devices over SPI.
> This driver can be built as a module. The module will be called
> inv-mpu6050-spi.
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 273b7fa7..3a82a49 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -108,6 +108,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> },
> + {
> + .whoami = INV_MPU9150_WHOAMI_VALUE,
> + .name = "MPU9150",
> + .reg = &reg_set_6050,
> + .config = &chip_config_6050,
> + },
> };
>
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index bb1a7b1..1a424a6 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -202,6 +202,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> static const struct i2c_device_id inv_mpu_id[] = {
> {"mpu6050", INV_MPU6050},
> {"mpu6500", INV_MPU6500},
> + {"mpu9150", INV_MPU9150},
> {}
> };
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 564cabd..38d6a09 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -68,6 +68,7 @@ enum inv_devices {
> INV_MPU6050,
> INV_MPU6500,
> INV_MPU6000,
> + INV_MPU9150,
> INV_NUM_PARTS
> };
>
> @@ -220,6 +221,7 @@ struct inv_mpu6050_state {
> #define INV_MPU6000_WHOAMI_VALUE 0x68
> #define INV_MPU6050_WHOAMI_VALUE 0x68
> #define INV_MPU6500_WHOAMI_VALUE 0x70
> +#define INV_MPU9150_WHOAMI_VALUE 0x68
>
> /* scan element definition */
> enum inv_mpu6050_scan {
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> index a0f8df2..190a4a5 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> @@ -81,6 +81,7 @@ static int inv_mpu_remove(struct spi_device *spi)
> static const struct spi_device_id inv_mpu_id[] = {
> {"mpu6000", INV_MPU6000},
> {"mpu6500", INV_MPU6500},
> + {"mpu9150", INV_MPU9150},
> {}
> };
>
>

2016-04-25 11:18:05

by Crestez Dan Leonard

[permalink] [raw]
Subject: Re: [PATCH 3/5] iio: inv_mpu6050: Check WHO_AM_I register on probe

On 04/24/2016 02:14 PM, Jonathan Cameron wrote:
> On 20/04/16 14:15, Crestez Dan Leonard wrote:
>> This can be used to distinguish mpu6500. This is a warning rather than
>> an error because the differences are mostly irrelevant and it's nice to
>> avoid breaking users with slightly incorrect ACPI/DT.
>>
>> Signed-off-by: Crestez Dan Leonard <[email protected]>
> Would we be better off fixing their configuration though by using the right part
> if we can identify it? So if wrong, maybe we should search the info table to
> figure out what it is? I'm not certain on this though as then we are trying to
> deal with unknown future cases - maybe what you have here is the best balance.

I'm not sure about that. One issue is that 6000/6050/9150 have the same
WHOAMI value and can't be distinguished this way. They also seem to
identical interfaces. Models MPU6500 and MPU9250 report different WHOAMI
values.

Changing chip_type based on the WHOAMI would require some additional
refactoring. Placing that in a separate patch might be worthwhile anyway.

>> +#define INV_MPU6050_REG_WHOAMI 117
>> +
>> +#define INV_MPU6000_WHOAMI_VALUE 0x68
>> +#define INV_MPU6050_WHOAMI_VALUE 0x68
>> +#define INV_MPU6500_WHOAMI_VALUE 0x70

2016-04-25 18:09:25

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/5] iio: inv_mpu6050: Check WHO_AM_I register on probe

On 25/04/16 12:17, Crestez Dan Leonard wrote:
> On 04/24/2016 02:14 PM, Jonathan Cameron wrote:
>> On 20/04/16 14:15, Crestez Dan Leonard wrote:
>>> This can be used to distinguish mpu6500. This is a warning rather than
>>> an error because the differences are mostly irrelevant and it's nice to
>>> avoid breaking users with slightly incorrect ACPI/DT.
>>>
>>> Signed-off-by: Crestez Dan Leonard <[email protected]>
>> Would we be better off fixing their configuration though by using the right part
>> if we can identify it? So if wrong, maybe we should search the info table to
>> figure out what it is? I'm not certain on this though as then we are trying to
>> deal with unknown future cases - maybe what you have here is the best balance.
>
> I'm not sure about that. One issue is that 6000/6050/9150 have the same
> WHOAMI value and can't be distinguished this way. They also seem to
> identical interfaces. Models MPU6500 and MPU9250 report different WHOAMI
> values.
>
> Changing chip_type based on the WHOAMI would require some additional
> refactoring. Placing that in a separate patch might be worthwhile anyway.
>
Agreed.
>>> +#define INV_MPU6050_REG_WHOAMI 117
>>> +
>>> +#define INV_MPU6000_WHOAMI_VALUE 0x68
>>> +#define INV_MPU6050_WHOAMI_VALUE 0x68
>>> +#define INV_MPU6500_WHOAMI_VALUE 0x70

2016-04-25 18:28:38

by Ge Gao

[permalink] [raw]
Subject: RE: [PATCH 5/5] iio: inv_mpu6050: Add explicit support for MPU9150

This looks good to me.

Acked-by: Ge Gao <[email protected] >



-----Original Message-----
From: Jonathan Cameron [mailto:[email protected]]
Sent: Sunday, April 24, 2016 4:16 AM
To: Crestez Dan Leonard; [email protected]
Cc: [email protected]; Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald-Stadler; Daniel Baluta; Ge Gao
Subject: Re: [PATCH 5/5] iio: inv_mpu6050: Add explicit support for MPU9150

On 20/04/16 14:15, Crestez Dan Leonard wrote:
> This device is a package containing a MPU6050-like sensor and an
> AK8975 magnetometer. The magnetometer component is supported by the
> existing
> ak8975 driver.
>
> This patch also rephrases the Kconfig descriptions.
>
> Signed-off-by: Crestez Dan Leonard <[email protected]>
Looks good. Will just wait for Ge to hopefully have time to take a look at the whole series. For future reference, it's always worth making sure you have cc'd the original author of a driver unless you know someone else is definitely maintaining it.

Ge was reply to emails recently so I guess he is still very much active!

Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/Kconfig | 10 ++++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 ++++++
> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 1 +
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 ++
> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 1 +
> 5 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig
> b/drivers/iio/imu/inv_mpu6050/Kconfig
> index a7f557a..c05d474 100644
> --- a/drivers/iio/imu/inv_mpu6050/Kconfig
> +++ b/drivers/iio/imu/inv_mpu6050/Kconfig
> @@ -14,10 +14,8 @@ config INV_MPU6050_I2C
> select I2C_MUX
> select REGMAP_I2C
> help
> - This driver supports the Invensense MPU6050 devices.
> - This driver can also support MPU6500 in MPU6050 compatibility mode
> - and also in MPU6500 mode with some limitations.
> - It is a gyroscope/accelerometer combo device.
> + This driver supports the Invensense MPU6050/6500/9150 motion tracking
> + devices over I2C.
> This driver can be built as a module. The module will be called
> inv-mpu6050-i2c.
>
> @@ -27,7 +25,7 @@ config INV_MPU6050_SPI
> select INV_MPU6050_IIO
> select REGMAP_SPI
> help
> - This driver supports the Invensense MPU6050 devices.
> - It is a gyroscope/accelerometer combo device.
> + This driver supports the Invensense MPU6000/6500/9150 motion tracking
> + devices over SPI.
> This driver can be built as a module. The module will be called
> inv-mpu6050-spi.
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 273b7fa7..3a82a49 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -108,6 +108,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .reg = &reg_set_6050,
> .config = &chip_config_6050,
> },
> + {
> + .whoami = INV_MPU9150_WHOAMI_VALUE,
> + .name = "MPU9150",
> + .reg = &reg_set_6050,
> + .config = &chip_config_6050,
> + },
> };
>
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
> u32 mask) diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index bb1a7b1..1a424a6 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -202,6 +202,7 @@ static int inv_mpu_remove(struct i2c_client
> *client) static const struct i2c_device_id inv_mpu_id[] = {
> {"mpu6050", INV_MPU6050},
> {"mpu6500", INV_MPU6500},
> + {"mpu9150", INV_MPU9150},
> {}
> };
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 564cabd..38d6a09 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -68,6 +68,7 @@ enum inv_devices {
> INV_MPU6050,
> INV_MPU6500,
> INV_MPU6000,
> + INV_MPU9150,
> INV_NUM_PARTS
> };
>
> @@ -220,6 +221,7 @@ struct inv_mpu6050_state {
> #define INV_MPU6000_WHOAMI_VALUE 0x68
> #define INV_MPU6050_WHOAMI_VALUE 0x68
> #define INV_MPU6500_WHOAMI_VALUE 0x70
> +#define INV_MPU9150_WHOAMI_VALUE 0x68
>
> /* scan element definition */
> enum inv_mpu6050_scan {
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> index a0f8df2..190a4a5 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> @@ -81,6 +81,7 @@ static int inv_mpu_remove(struct spi_device *spi)
> static const struct spi_device_id inv_mpu_id[] = {
> {"mpu6000", INV_MPU6000},
> {"mpu6500", INV_MPU6500},
> + {"mpu9150", INV_MPU9150},
> {}
> };
>
>

2016-04-25 18:41:28

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 1/5] iio: inv_mpu6050: Cleanup hw_info mapping

On 24/04/16 12:10, Jonathan Cameron wrote:
> On 20/04/16 14:15, Crestez Dan Leonard wrote:
>> The hw_info array was indexed by enum inv_devices chip_type despite the
>> fact that the enumeration had more members than the array and was
>> ordered differently.
>>
>> The patch cleans this up and adds explicit chip_types to i2c/spi/acpi
>> IDs. It also adds some stricter checks inside the driver core.
>>
>> This happened to work so far because the differences between the
>> supported models are very minor.
>>
>> Signed-off-by: Crestez Dan Leonard <[email protected]>h
> Ideally I'd like an Ack / review from Ge on these.
> The same is true for the whole series.
Applied.
>
> Looks good to me though!
>
> Jonathan
>> ---
>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 15 ++++++++++++++-
>> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 2 +-
>> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 18 ++++++++++++++----
>> 3 files changed, 29 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index d192953..52e62b3 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -88,16 +88,23 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
>> .accl_fs = INV_MPU6050_FS_02G,
>> };
>>
>> +/* Indexed by enum inv_devices */
>> static const struct inv_mpu6050_hw hw_info[] = {
>> {
>> .num_reg = 117,
>> + .name = "MPU6050",
>> + .reg = &reg_set_6050,
>> + .config = &chip_config_6050,
>> + },
>> + {
>> + .num_reg = 117,
>> .name = "MPU6500",
>> .reg = &reg_set_6500,
>> .config = &chip_config_6050,
>> },
>> {
>> .num_reg = 117,
>> - .name = "MPU6050",
>> + .name = "MPU6000",
>> .reg = &reg_set_6050,
>> .config = &chip_config_6050,
>> },
>> @@ -774,6 +781,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>> if (!indio_dev)
>> return -ENOMEM;
>>
>> + BUILD_BUG_ON(ARRAY_SIZE(hw_info) != INV_NUM_PARTS);
>> + if (chip_type < 0 || chip_type >= INV_NUM_PARTS) {
>> + dev_err(dev, "Bad invensense chip_type=%d name=%s\n",
>> + chip_type, name);
>> + return -ENODEV;
>> + }
>> st = iio_priv(indio_dev);
>> st->chip_type = chip_type;
>> st->powerup_count = 0;
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> index 5ee4e0d..bb1a7b1 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> @@ -208,7 +208,7 @@ static const struct i2c_device_id inv_mpu_id[] = {
>> MODULE_DEVICE_TABLE(i2c, inv_mpu_id);
>>
>> static const struct acpi_device_id inv_acpi_match[] = {
>> - {"INVN6500", 0},
>> + {"INVN6500", INV_MPU6500},
>> { },
>> };
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> index 7bcb8d8..3972a46 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> @@ -44,9 +44,19 @@ static int inv_mpu_i2c_disable(struct iio_dev *indio_dev)
>> static int inv_mpu_probe(struct spi_device *spi)
>> {
>> struct regmap *regmap;
>> - const struct spi_device_id *id = spi_get_device_id(spi);
>> - const char *name = id ? id->name : NULL;
>> - const int chip_type = id ? id->driver_data : 0;
>> + const struct spi_device_id *spi_id;
>> + const struct acpi_device_id *acpi_id;
>> + const char *name = NULL;
>> + enum inv_devices chip_type;
>> +
>> + if ((spi_id = spi_get_device_id(spi))) {
>> + chip_type = (enum inv_devices)spi_id->driver_data;
>> + name = spi_id->name;
>> + } else if ((acpi_id = acpi_match_device(spi->dev.driver->acpi_match_table, &spi->dev))) {
>> + chip_type = (enum inv_devices)acpi_id->driver_data;
>> + } else {
>> + return -ENODEV;
>> + }
>>
>> regmap = devm_regmap_init_spi(spi, &inv_mpu_regmap_config);
>> if (IS_ERR(regmap)) {
>> @@ -76,7 +86,7 @@ static const struct spi_device_id inv_mpu_id[] = {
>> MODULE_DEVICE_TABLE(spi, inv_mpu_id);
>>
>> static const struct acpi_device_id inv_acpi_match[] = {
>> - {"INVN6000", 0},
>> + {"INVN6000", INV_MPU6000},
>> { },
>> };
>> MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2016-04-25 18:43:08

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 2/5] iio: inv_mpu6050: Remove inv_mpu6050_hw.num_reg

On 24/04/16 12:12, Jonathan Cameron wrote:
> On 20/04/16 14:15, Crestez Dan Leonard wrote:
>> This field was unused and incorrect for mpu6500.
>>
>> Signed-off-by: Crestez Dan Leonard <[email protected]>.
> This one I think I can safely take :)
>
> Good spot
Applied.

Thanks,
>> ---
>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 3 ---
>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 --
>> 2 files changed, 5 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index 52e62b3..faccabafc 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -91,19 +91,16 @@ static const struct inv_mpu6050_chip_config chip_config_6050 = {
>> /* Indexed by enum inv_devices */
>> static const struct inv_mpu6050_hw hw_info[] = {
>> {
>> - .num_reg = 117,
>> .name = "MPU6050",
>> .reg = &reg_set_6050,
>> .config = &chip_config_6050,
>> },
>> {
>> - .num_reg = 117,
>> .name = "MPU6500",
>> .reg = &reg_set_6500,
>> .config = &chip_config_6050,
>> },
>> {
>> - .num_reg = 117,
>> .name = "MPU6000",
>> .reg = &reg_set_6050,
>> .config = &chip_config_6050,
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> index e302a49..c66dbfc 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> @@ -93,13 +93,11 @@ struct inv_mpu6050_chip_config {
>>
>> /**
>> * struct inv_mpu6050_hw - Other important hardware information.
>> - * @num_reg: Number of registers on device.
>> * @name: name of the chip.
>> * @reg: register map of the chip.
>> * @config: configuration of the chip.
>> */
>> struct inv_mpu6050_hw {
>> - u8 num_reg;
>> u8 *name;
>> const struct inv_mpu6050_reg_map *reg;
>> const struct inv_mpu6050_chip_config *config;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2016-04-25 19:21:03

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 5/5] iio: inv_mpu6050: Add explicit support for MPU9150

On 25/04/16 18:56, Ge Gao wrote:
> This looks good to me.
>
> Acked-by: Ge Gao <[email protected] >
Applied - I wasn't entirely certain whether you meant it as such,
but I've added the ack to all the patches in this series.

Thanks,

Jonathan
>
>
>
> -----Original Message-----
> From: Jonathan Cameron [mailto:[email protected]]
> Sent: Sunday, April 24, 2016 4:16 AM
> To: Crestez Dan Leonard; [email protected]
> Cc: [email protected]; Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald-Stadler; Daniel Baluta; Ge Gao
> Subject: Re: [PATCH 5/5] iio: inv_mpu6050: Add explicit support for MPU9150
>
> On 20/04/16 14:15, Crestez Dan Leonard wrote:
>> This device is a package containing a MPU6050-like sensor and an
>> AK8975 magnetometer. The magnetometer component is supported by the
>> existing
>> ak8975 driver.
>>
>> This patch also rephrases the Kconfig descriptions.
>>
>> Signed-off-by: Crestez Dan Leonard <[email protected]>
> Looks good. Will just wait for Ge to hopefully have time to take a look at the whole series. For future reference, it's always worth making sure you have cc'd the original author of a driver unless you know someone else is definitely maintaining it.
>
> Ge was reply to emails recently so I guess he is still very much active!
>
> Jonathan
>> ---
>> drivers/iio/imu/inv_mpu6050/Kconfig | 10 ++++------
>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 ++++++
>> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 1 +
>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 ++
>> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 1 +
>> 5 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig
>> b/drivers/iio/imu/inv_mpu6050/Kconfig
>> index a7f557a..c05d474 100644
>> --- a/drivers/iio/imu/inv_mpu6050/Kconfig
>> +++ b/drivers/iio/imu/inv_mpu6050/Kconfig
>> @@ -14,10 +14,8 @@ config INV_MPU6050_I2C
>> select I2C_MUX
>> select REGMAP_I2C
>> help
>> - This driver supports the Invensense MPU6050 devices.
>> - This driver can also support MPU6500 in MPU6050 compatibility mode
>> - and also in MPU6500 mode with some limitations.
>> - It is a gyroscope/accelerometer combo device.
>> + This driver supports the Invensense MPU6050/6500/9150 motion tracking
>> + devices over I2C.
>> This driver can be built as a module. The module will be called
>> inv-mpu6050-i2c.
>>
>> @@ -27,7 +25,7 @@ config INV_MPU6050_SPI
>> select INV_MPU6050_IIO
>> select REGMAP_SPI
>> help
>> - This driver supports the Invensense MPU6050 devices.
>> - It is a gyroscope/accelerometer combo device.
>> + This driver supports the Invensense MPU6000/6500/9150 motion tracking
>> + devices over SPI.
>> This driver can be built as a module. The module will be called
>> inv-mpu6050-spi.
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index 273b7fa7..3a82a49 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -108,6 +108,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
>> .reg = &reg_set_6050,
>> .config = &chip_config_6050,
>> },
>> + {
>> + .whoami = INV_MPU9150_WHOAMI_VALUE,
>> + .name = "MPU9150",
>> + .reg = &reg_set_6050,
>> + .config = &chip_config_6050,
>> + },
>> };
>>
>> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
>> u32 mask) diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> index bb1a7b1..1a424a6 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> @@ -202,6 +202,7 @@ static int inv_mpu_remove(struct i2c_client
>> *client) static const struct i2c_device_id inv_mpu_id[] = {
>> {"mpu6050", INV_MPU6050},
>> {"mpu6500", INV_MPU6500},
>> + {"mpu9150", INV_MPU9150},
>> {}
>> };
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> index 564cabd..38d6a09 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> @@ -68,6 +68,7 @@ enum inv_devices {
>> INV_MPU6050,
>> INV_MPU6500,
>> INV_MPU6000,
>> + INV_MPU9150,
>> INV_NUM_PARTS
>> };
>>
>> @@ -220,6 +221,7 @@ struct inv_mpu6050_state {
>> #define INV_MPU6000_WHOAMI_VALUE 0x68
>> #define INV_MPU6050_WHOAMI_VALUE 0x68
>> #define INV_MPU6500_WHOAMI_VALUE 0x70
>> +#define INV_MPU9150_WHOAMI_VALUE 0x68
>>
>> /* scan element definition */
>> enum inv_mpu6050_scan {
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> index a0f8df2..190a4a5 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
>> @@ -81,6 +81,7 @@ static int inv_mpu_remove(struct spi_device *spi)
>> static const struct spi_device_id inv_mpu_id[] = {
>> {"mpu6000", INV_MPU6000},
>> {"mpu6500", INV_MPU6500},
>> + {"mpu9150", INV_MPU9150},
>> {}
>> };
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>