Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754046AbcDJOMj (ORCPT ); Sun, 10 Apr 2016 10:12:39 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:44137 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753226AbcDJOMg (ORCPT ); Sun, 10 Apr 2016 10:12:36 -0400 Subject: Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core To: Peter Rosin , linux-kernel@vger.kernel.org References: <1459673574-11440-1-git-send-email-peda@lysator.liu.se> <1459673574-11440-9-git-send-email-peda@lysator.liu.se> <5700F594.9090105@kernel.org> <570103AE.1020707@lysator.liu.se> Cc: Peter Rosin , Wolfram Sang , Jonathan Corbet , Peter Korsgaard , Guenter Roeck , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Antti Palosaari , Mauro Carvalho Chehab , Rob Herring , Frank Rowand , Grant Likely , Andrew Morton , Greg Kroah-Hartman , "David S. Miller" , Kalle Valo , Joe Perches , Jiri Slaby , Daniel Baluta , Adriana Reus , Lucas De Marchi , Matt Ranostay , Krzysztof Kozlowski , Terry Heo , Hans Verkuil , Arnd Bergmann , Tommi Rantala , linux-i2c@vger.kernel.org, linux-doc@vger.kernel.org, linux-iio@vger.kernel.org, linux-media@vger.kernel.org, devicetree@vger.kernel.org From: Jonathan Cameron Message-ID: <570A5F4D.7040104@kernel.org> Date: Sun, 10 Apr 2016 15:12:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <570103AE.1020707@lysator.liu.se> 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: 7051 Lines: 187 On 03/04/16 12:51, Peter Rosin wrote: > On 2016-04-03 12:51, Jonathan Cameron wrote: >> On 03/04/16 09:52, Peter Rosin wrote: >>> From: 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 >> I'm mostly fine with this (though one unrelated change seems to have snuck >> in). However, I'm not set up to test it - hence other than fixing the change >> you can have my ack, but ideal would be a tested by from someone with >> relevant hardware... However, it looks to be a fairly mechanical change so >> if no one is currently setup to test it, then don't let it hold up the >> series too long! >> >> Acked-by: Jonathan Cameron > > Thanks for your acks! > >> Jonathan >>> --- >>> 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 | 32 +++++++++++++----------------- >>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++- >>> 4 files changed, 17 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 >>> #include >>> #include >>> -#include >>> #include >>> #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..0d429d788106 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 >>> #include >>> #include >>> -#include >>> #include >>> #include >>> #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); > > Here, the existing code uses drv_get_drvdata to get from i2c_client to iio_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); > > ...and here too... > >>> struct inv_mpu6050_state *st = iio_priv(indio_dev); >>> >>> @@ -136,16 +133,15 @@ 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_one_adapter(client->adapter, &client->dev, 0, 0, >>> + 0, 0, 0, >>> + inv_mpu6050_select_bypass, >>> + inv_mpu6050_deselect_bypass); >>> + if (IS_ERR(st->muxc)) { >>> + result = PTR_ERR(st->muxc); >>> goto out_unreg_device; >>> } >>> + st->muxc->priv = client; >>> >>> result = inv_mpu_acpi_create_mux_client(client); >>> if (result) >>> @@ -154,7 +150,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 +158,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); >> Why this change? Seems unrelated. > > ...which is why I made this change. Maybe a bad call, but the inconsistency > disturbed me and I was changing the function anyway. I could split it out > to its own commit I suppose, or should I just not bother at all? Funny thing is I'd say the i2c_get_clientdata option is the better of the two! I don't really care though either way. J > > Cheers, > Peter > >>> 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 >>> +#include >>> #include >>> #include >>> #include >>> @@ -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; >>> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >