Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753491AbcDCLvU (ORCPT ); Sun, 3 Apr 2016 07:51:20 -0400 Received: from mail.lysator.liu.se ([130.236.254.3]:39193 "EHLO mail.lysator.liu.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752446AbcDCLvR (ORCPT ); Sun, 3 Apr 2016 07:51:17 -0400 Message-ID: <570103AE.1020707@lysator.liu.se> Date: Sun, 03 Apr 2016 13:51:10 +0200 From: Peter Rosin User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Jonathan Cameron , linux-kernel@vger.kernel.org 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 Subject: Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core 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> In-Reply-To: <5700F594.9090105@kernel.org> 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: 6495 Lines: 176 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? 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; >> >