Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752635AbcDXLKZ (ORCPT ); Sun, 24 Apr 2016 07:10:25 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:35680 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751770AbcDXLKX (ORCPT ); Sun, 24 Apr 2016 07:10:23 -0400 Subject: Re: [PATCH 1/5] iio: inv_mpu6050: Cleanup hw_info mapping To: Crestez Dan Leonard , linux-iio@vger.kernel.org References: Cc: linux-kernel@vger.kernel.org, Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Daniel Baluta , Ge Gao From: Jonathan Cameron Message-ID: Date: Sun, 24 Apr 2016 12:10:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4079 Lines: 116 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 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 = ®_set_6050, > + .config = &chip_config_6050, > + }, > + { > + .num_reg = 117, > .name = "MPU6500", > .reg = ®_set_6500, > .config = &chip_config_6050, > }, > { > .num_reg = 117, > - .name = "MPU6050", > + .name = "MPU6000", > .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); >