* Re: interface to trigger a bus recovery sequence
From: Wolfram Sang @ 2016-12-18 23:14 UTC (permalink / raw)
To: Jorge Ramirez; +Cc: linux-i2c
In-Reply-To: <9d29f4a6-50ae-2400-4a4d-79ba7ed48a53@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 399 bytes --]
> I was looking into the i2c bus recovery sequence and I was wondering why
> there doesn't seem to be an interface (ioctl/sysfs) that allows a user to
> trigger a recovery sequence.
I can't think of a reason why this should be possible. The I2C bus
driver should figure out when things go wrong and do the apropriate
actions. Also, the I2C specs have a clear definition when to use bus
recovery.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* interface to trigger a bus recovery sequence
From: Jorge Ramirez @ 2016-12-18 17:56 UTC (permalink / raw)
To: linux-i2c
Hi all,
I was looking into the i2c bus recovery sequence and I was wondering why
there doesn't seem to be an interface (ioctl/sysfs) that allows a user
to trigger a recovery sequence.
Is this intentional or would such an interface - if it were to be
implemented- be welcome?
many thanks in advance.
jorge
^ permalink raw reply
* Re: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
From: Wolfram Sang @ 2016-12-18 7:56 UTC (permalink / raw)
To: Peter Rosin; +Cc: linux-kernel, Vadim Pasternak, Michael Shych, linux-i2c
In-Reply-To: <1482006551-23108-1-git-send-email-peda@axentia.se>
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
On Sat, Dec 17, 2016 at 09:29:11PM +0100, Peter Rosin wrote:
> smbus functions return -ve on error, 0 on success. However,
> __i2c_transfer() have a different return signature - -ve on error, or
> number of buffers transferred (which may be zero or greater.)
>
> The upshot of this is that the sense of the text is reversed when using
> the mux on a bus supporting the master_xfer method: we cache the value
> and never retry if we fail to transfer any buffers, but if we succeed,
> we clear the cached value
>
> Fix this by making mlxcpld_mux_reg_write() return a -ve error code for
> all failure cases, just as was done in commit 7f638c1cb0a1 ("i2c: mux:
> pca954x: fix i2c mux selection caching")
>
> This also aligns the implementations of these two muxes in this area.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Fixed the commit message and applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
From: Vadim Pasternak @ 2016-12-17 20:56 UTC (permalink / raw)
To: Peter Rosin, linux-kernel@vger.kernel.org
Cc: Michael Shych, Wolfram Sang, linux-i2c@vger.kernel.org
In-Reply-To: <1482006551-23108-1-git-send-email-peda@axentia.se>
> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Saturday, December 17, 2016 10:29 PM
> To: linux-kernel@vger.kernel.org
> Cc: Peter Rosin <peda@axentia.se>; Vadim Pasternak
> <vadimp@mellanox.com>; Michael Shych <michaelsh@mellanox.com>;
> Wolfram Sang <wsa@the-dreams.de>; linux-i2c@vger.kernel.org
> Subject: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
>
> smbus functions return -ve on error, 0 on success. However,
> __i2c_transfer() have a different return signature - -ve on error, or number
> of buffers transferred (which may be zero or greater.)
>
> The upshot of this is that the sense of the text is reversed when using the
> mux on a bus supporting the master_xfer method: we cache the value and
> never retry if we fail to transfer any buffers, but if we succeed, we clear the
> cached value
>
> Fix this by making mlxcpld_mux_reg_write() return a -ve error code for all
> failure cases, just as was done in commit 7f638c1cb0a1 ("i2c: mux:
> pca954x: fix i2c mux selection caching")
>
> This also aligns the implementations of these two muxes in this area.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
Acked-by: Vadim Pasternak <vadimp@mellanox.com>
>
> Hi!
>
> I audited the other i2c muxes for similar problems after the recent regression
> report for the pca954x driver [1], and found only this.
>
> I obviously don't have the hardware to test this patch.
>
> Anyway, this driver is probably lower profile than the pca954x, but this still
> needs fixing in my opinion...
>
> Cheers,
> peda
>
> [1] http://www.spinics.net/lists/linux-i2c/msg27586.html
>
> drivers/i2c/muxes/i2c-mux-mlxcpld.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-
> mux-mlxcpld.c
> index 3ab654bbfab5..b7ca249ec9c3 100644
> --- a/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> +++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> @@ -95,6 +95,7 @@ static int mlxcpld_mux_reg_write(struct i2c_adapter
> *adap,
> struct i2c_client *client, u8 val) {
> struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client-
> >dev);
> + int ret = -ENODEV;
>
> if (adap->algo->master_xfer) {
> struct i2c_msg msg;
> @@ -104,17 +105,21 @@ static int mlxcpld_mux_reg_write(struct
> i2c_adapter *adap,
> msg.flags = 0;
> msg.len = 2;
> msg.buf = msgbuf;
> - return __i2c_transfer(adap, &msg, 1);
> + ret = __i2c_transfer(adap, &msg, 1);
> +
> + if (ret >= 0 && ret != 1)
> + ret = -EREMOTEIO;
> } else if (adap->algo->smbus_xfer) {
> union i2c_smbus_data data;
>
> data.byte = val;
> - return adap->algo->smbus_xfer(adap, client->addr,
> - client->flags, I2C_SMBUS_WRITE,
> - pdata->sel_reg_addr,
> - I2C_SMBUS_BYTE_DATA, &data);
> - } else
> - return -ENODEV;
> + ret = adap->algo->smbus_xfer(adap, client->addr,
> + client->flags, I2C_SMBUS_WRITE,
> + pdata->sel_reg_addr,
> + I2C_SMBUS_BYTE_DATA, &data);
> + }
> +
> + return ret;
> }
>
> static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
> @@ -127,10 +132,7 @@ static int mlxcpld_mux_select_chan(struct
> i2c_mux_core *muxc, u32 chan)
> /* Only select the channel if its different from the last channel */
> if (data->last_chan != regval) {
> err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
> - if (err)
> - data->last_chan = 0;
> - else
> - data->last_chan = regval;
> + data->last_chan = err < 0 ? 0 : regval;
> }
>
> return err;
> --
> 2.1.4
^ permalink raw reply
* Re: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-17 20:34 UTC (permalink / raw)
To: linux-kernel; +Cc: Vadim Pasternak, Michael Shych, Wolfram Sang, linux-i2c
In-Reply-To: <1482006551-23108-1-git-send-email-peda@axentia.se>
On 2016-12-17 21:29, Peter Rosin wrote:
> smbus functions return -ve on error, 0 on success. However,
> __i2c_transfer() have a different return signature - -ve on error, or
> number of buffers transferred (which may be zero or greater.)
>
> The upshot of this is that the sense of the text is reversed when using
s/text/test/
> the mux on a bus supporting the master_xfer method: we cache the value
> and never retry if we fail to transfer any buffers, but if we succeed,
> we clear the cached value
missing terminating period
>
> Fix this by making mlxcpld_mux_reg_write() return a -ve error code for
> all failure cases, just as was done in commit 7f638c1cb0a1 ("i2c: mux:
> pca954x: fix i2c mux selection caching")
>
> This also aligns the implementations of these two muxes in this area.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
Sorry for the poor proof-reading skills.
Cheers,
peda
^ permalink raw reply
* [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-17 20:29 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Vadim Pasternak, Michael Shych, Wolfram Sang,
linux-i2c
smbus functions return -ve on error, 0 on success. However,
__i2c_transfer() have a different return signature - -ve on error, or
number of buffers transferred (which may be zero or greater.)
The upshot of this is that the sense of the text is reversed when using
the mux on a bus supporting the master_xfer method: we cache the value
and never retry if we fail to transfer any buffers, but if we succeed,
we clear the cached value
Fix this by making mlxcpld_mux_reg_write() return a -ve error code for
all failure cases, just as was done in commit 7f638c1cb0a1 ("i2c: mux:
pca954x: fix i2c mux selection caching")
This also aligns the implementations of these two muxes in this area.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Hi!
I audited the other i2c muxes for similar problems after the recent regression
report for the pca954x driver [1], and found only this.
I obviously don't have the hardware to test this patch.
Anyway, this driver is probably lower profile than the pca954x, but this
still needs fixing in my opinion...
Cheers,
peda
[1] http://www.spinics.net/lists/linux-i2c/msg27586.html
drivers/i2c/muxes/i2c-mux-mlxcpld.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
index 3ab654bbfab5..b7ca249ec9c3 100644
--- a/drivers/i2c/muxes/i2c-mux-mlxcpld.c
+++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
@@ -95,6 +95,7 @@ static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
struct i2c_client *client, u8 val)
{
struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+ int ret = -ENODEV;
if (adap->algo->master_xfer) {
struct i2c_msg msg;
@@ -104,17 +105,21 @@ static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
msg.flags = 0;
msg.len = 2;
msg.buf = msgbuf;
- return __i2c_transfer(adap, &msg, 1);
+ ret = __i2c_transfer(adap, &msg, 1);
+
+ if (ret >= 0 && ret != 1)
+ ret = -EREMOTEIO;
} else if (adap->algo->smbus_xfer) {
union i2c_smbus_data data;
data.byte = val;
- return adap->algo->smbus_xfer(adap, client->addr,
- client->flags, I2C_SMBUS_WRITE,
- pdata->sel_reg_addr,
- I2C_SMBUS_BYTE_DATA, &data);
- } else
- return -ENODEV;
+ ret = adap->algo->smbus_xfer(adap, client->addr,
+ client->flags, I2C_SMBUS_WRITE,
+ pdata->sel_reg_addr,
+ I2C_SMBUS_BYTE_DATA, &data);
+ }
+
+ return ret;
}
static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
@@ -127,10 +132,7 @@ static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
- if (err)
- data->last_chan = 0;
- else
- data->last_chan = regval;
+ data->last_chan = err < 0 ? 0 : regval;
}
return err;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH V5] i2c: designware: fix wrong Tx/Rx FIFO for ACPI
From: Wolfram Sang @ 2016-12-17 18:38 UTC (permalink / raw)
To: Tin Huynh
Cc: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, linux-i2c,
linux-kernel, linux-acpi, Loc Ho, Thang Nguyen, Phong Vo, patches
In-Reply-To: <1481707438-32240-1-git-send-email-tnhuynh@apm.com>
[-- Attachment #1: Type: text/plain, Size: 377 bytes --]
On Wed, Dec 14, 2016 at 04:23:58PM +0700, Tin Huynh wrote:
> ACPI always sets Tx/Rx FIFO to 32. This configuration will
> cause problem if the IP core supports a FIFO size of less than 32.
> The driver should read the FIFO size from the IP and select the smaller
> one of the two.
>
> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V1] i2c: xgene: Fix missing code of DTB support
From: Wolfram Sang @ 2016-12-17 18:36 UTC (permalink / raw)
To: Tin Huynh
Cc: linux-i2c, linux-kernel, Loc Ho, Thang Nguyen, Phong Vo, Duc Dang,
Dien Tien Hoang Nguyen, patches
In-Reply-To: <1481699846-10620-1-git-send-email-tnhuynh@apm.com>
[-- Attachment #1: Type: text/plain, Size: 311 bytes --]
On Wed, Dec 14, 2016 at 02:17:26PM +0700, Tin Huynh wrote:
> In DTB case, i2c-core doesn't create slave device which is installed
> on i2c-xgene bus because of missing code in this driver.
> This patch fixes this issue.
>
> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Wolfram Sang @ 2016-12-17 18:30 UTC (permalink / raw)
To: Russell King; +Cc: Peter Rosin, linux-i2c
In-Reply-To: <E1cIDpI-0005y4-SW@rmk-PC.armlinux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 857 bytes --]
On Sat, Dec 17, 2016 at 12:10:56PM +0000, Russell King wrote:
> smbus functions return -ve on error, 0 on success. However,
> __i2c_transfer() have a different return signature - -ve on error, or
> number of buffers transferred (which may be zero or greater.)
>
> The upshot of this is that the sense of the test is reversed when using
> the mux on a bus supporting the master_xfer method: we cache the value
> and never retry if we fail to transfer any buffers, but if we succeed,
> we clear the cached value.
>
> Fix this by making pca954x_reg_write() return a negative error code for
> all failure cases.
>
> Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
> Acked-by: Peter Rosin <peda@axentia.se>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] i2c: octeon: thunderx: Limit register access retries
From: Wolfram Sang @ 2016-12-17 18:29 UTC (permalink / raw)
To: Jan Glauber
Cc: Wolfram Sang, Paul Burton, Steven J . Hill, linux-i2c, linux-mips,
David Daney
In-Reply-To: <20161209093158.3161-4-jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 432 bytes --]
On Fri, Dec 09, 2016 at 10:31:57AM +0100, Jan Glauber wrote:
> Do not infinitely retry register readq and writeq operations
> in order to not lock up the CPU in case the TWSI gets stuck.
>
> Return -EIO in case of a failed data read. For all other
> cases just return so subsequent operations will fail
> and trigger the recovery.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] power: supply: axp288_fuel_gauge: Drop platform_data dependency
From: Sebastian Reichel @ 2016-12-17 15:00 UTC (permalink / raw)
To: Hans de Goede
Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-pm, linux-i2c
In-Reply-To: <20161214163853.454-4-hdegoede@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]
Hi,
On Wed, Dec 14, 2016 at 05:38:53PM +0100, Hans de Goede wrote:
> When the axp288_faul_gauge driver was originally merged, it was
> merged with a dependency on some other driver providing platform
> data for it.
>
> However the battery-data-framework which should provide that data
> never got merged, resulting in x86 tablets / laptops with an axp288
> having no working battery monitor, as before this commit the driver
> would simply return -ENODEV if there is no platform data.
>
> This commit removes the dependency on the platform_data instead
> checking that the firmware has initialized the fuel-gauge and
> reading the info back from the pmic.
>
> What is missing from the read-back info is the table to map raw adc
> values to temperature, so this commit drops the temperature and
> temperature limits properties. The min voltage, charge design and
> model name info is also missing. Note that none of these are really
> important for userspace to have.
>
> All other functionality is preserved and actually made available
> by this commit.
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=88471
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Looks fine to me, thanks for the cleanup. I will wait
a bit with merging it to give others a chance to comment.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] power: supply: axp288_fuel_gauge: Read 12 bit values 2 registers at a time
From: Sebastian Reichel @ 2016-12-17 14:58 UTC (permalink / raw)
To: Hans de Goede
Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-pm, linux-i2c
In-Reply-To: <20161214163853.454-3-hdegoede@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 653 bytes --]
Hi,
On Wed, Dec 14, 2016 at 05:38:52PM +0100, Hans de Goede wrote:
> In order for the MSB -> LSB latching to work correctly we must read the
> 2 8 bit registers of a 12 bit value in one consecutive read.
>
> This fixes voltage_ocv reporting inconsistent values on my tablet.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Thanks for your patch. We are currently in the merge
window and your patch will appear in linux-next once
4.10-rc1 has been tagged by Linus Torvalds.
Until then I queued it into this branch:
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/log/?h=for-next-next
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] power: supply: axp288_fuel_gauge: Read 15 bit values 2 registers at a time
From: Sebastian Reichel @ 2016-12-17 14:58 UTC (permalink / raw)
To: Hans de Goede
Cc: Chen-Yu Tsai, russianneuromancer @ ya . ru, linux-pm, linux-i2c
In-Reply-To: <20161214163853.454-2-hdegoede@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
Hi,
On Wed, Dec 14, 2016 at 05:38:51PM +0100, Hans de Goede wrote:
> In order for the MSB -> LSB latching to work correctly we must read the
> 2 8 bit registers of a 15 bit value in one consecutive read.
>
> This fixes charge_full reporting 3498768 on some reads and 3354624 one
> other reads on my tablet (for the 3354624 value the raw LSB is 0x00).
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Thanks for your patch. We are currently in the merge
window and your patch will appear in linux-next once
4.10-rc1 has been tagged by Linus Torvalds.
Until then I queued it into this branch:
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/log/?h=for-next-next
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Russell King @ 2016-12-17 12:10 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Peter Rosin, linux-i2c
smbus functions return -ve on error, 0 on success. However,
__i2c_transfer() have a different return signature - -ve on error, or
number of buffers transferred (which may be zero or greater.)
The upshot of this is that the sense of the test is reversed when using
the mux on a bus supporting the master_xfer method: we cache the value
and never retry if we fail to transfer any buffers, but if we succeed,
we clear the cached value.
Fix this by making pca954x_reg_write() return a negative error code for
all failure cases.
Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
Acked-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36d2837..9c4ac26c014e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
buf[0] = val;
msg.buf = buf;
ret = __i2c_transfer(adap, &msg, 1);
+
+ if (ret >= 0 && ret != 1)
+ ret = -EREMOTEIO;
} else {
union i2c_smbus_data data;
ret = adap->algo->smbus_xfer(adap, client->addr,
@@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret < 0 ? 0 : regval;
}
return ret;
--
2.7.4
^ permalink raw reply related
* Re: i2c: xiic: Strange clk_prepare_enable() in xiic_i2c_remove()
From: Shubhrajyoti Datta @ 2016-12-17 7:06 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Shubhrajyoti Datta, Rob Herring, Wolfram Sang, linux-i2c,
linux-kernel, ldv-project
In-Reply-To: <1481925285-594-1-git-send-email-khoroshilov@ispras.ru>
On Sat, Dec 17, 2016 at 3:24 AM, Alexey Khoroshilov
<khoroshilov@ispras.ru> wrote:
> Dear Shubhrajyoti,
>
> Looking at 36ecbcab84d0 ("i2c: xiic: Implement power management")
> it is not clear why clk_prepare_enable(i2c->clk) is required in
> xiic_i2c_remove()?
834 ret = clk_prepare_enable(i2c->clk);
835 if (ret) {
836 dev_err(&pdev->dev, "Unable to enable clock.\n");
837 return ret;
838 }
839 xiic_deinit(i2c);
840 clk_disable_unprepare(i2c->clk);
so it is enabled and disabled after xiic_deinit.
>
> It is enabled in xiic_i2c_probe() and disabled/enabled in
> cdns_i2c_runtime_suspend()/cdns_i2c_runtime_resume().
>
> Could you please clarify the point.
>
> --
> Alexey Khoroshilov
> Linux Verification Center, ISPRAS
> web: http://linuxtesting.org
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-17 6:50 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Vivien Didelot, linux-i2c, linux-arm-kernel, Wolfram Sang
In-Reply-To: <20161216232355.GO14217@n2100.armlinux.org.uk>
On 2016-12-17 00:23, Russell King - ARM Linux wrote:
> On Fri, Dec 16, 2016 at 10:20:35PM +0100, Peter Rosin wrote:
>> On 2016-12-16 21:06, Russell King wrote:
>>> smbus functions return -ve on error, 0 on success. However,
>>> __i2c_transfer() have a different return signature - -ve on error, or
>>> number of buffers transferred (which may be zero or greater.)
>>>
>>> The upshot of this is that the sense of the test is reversed when using
>>> the mux on a bus supporting the master_xfer method: we cache the value
>>> and never retry if we fail to transfer any buffers, but if we succeed,
>>> we clear the cached value.
>>
>> Ouch! Thanks for catching this.
>>
>>> Fix this.
>>
>> But lets fix the corner case of __i2c_transfer returning 0 instead of
>> the expected 1 as well (not sure if that's even possible, but lets close
>> the possibility just in case), so I'd prefer if you could fix
>> pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
>> instead, and -EREMOTEIO on other non-negative return values. Thanks!
>
> So you want something like this instead?
Yes, but I was originally thinking that the second hunk was no
longer needed...
Either way,
Acked-by: Peter Rosin <peda@axentia.se>
Cheers,
peda
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8bc3d36d2837..9c4ac26c014e 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
> buf[0] = val;
> msg.buf = buf;
> ret = __i2c_transfer(adap, &msg, 1);
> +
> + if (ret >= 0 && ret != 1)
> + ret = -EREMOTEIO;
> } else {
> union i2c_smbus_data data;
> ret = adap->algo->smbus_xfer(adap, client->addr,
> @@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> /* Only select the channel if its different from the last channel */
> if (data->last_chan != regval) {
> ret = pca954x_reg_write(muxc->parent, client, regval);
> - data->last_chan = ret ? 0 : regval;
> + data->last_chan = ret < 0 ? 0 : regval;
> }
>
> return ret;
>
>
^ permalink raw reply
* Re: [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Russell King - ARM Linux @ 2016-12-16 23:23 UTC (permalink / raw)
To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-arm-kernel
In-Reply-To: <251c31e3-141c-8884-d56a-fa539714d1ff@axentia.se>
On Fri, Dec 16, 2016 at 10:20:35PM +0100, Peter Rosin wrote:
> On 2016-12-16 21:06, Russell King wrote:
> > smbus functions return -ve on error, 0 on success. However,
> > __i2c_transfer() have a different return signature - -ve on error, or
> > number of buffers transferred (which may be zero or greater.)
> >
> > The upshot of this is that the sense of the test is reversed when using
> > the mux on a bus supporting the master_xfer method: we cache the value
> > and never retry if we fail to transfer any buffers, but if we succeed,
> > we clear the cached value.
>
> Ouch! Thanks for catching this.
>
> > Fix this.
>
> But lets fix the corner case of __i2c_transfer returning 0 instead of
> the expected 1 as well (not sure if that's even possible, but lets close
> the possibility just in case), so I'd prefer if you could fix
> pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
> instead, and -EREMOTEIO on other non-negative return values. Thanks!
So you want something like this instead?
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36d2837..9c4ac26c014e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
buf[0] = val;
msg.buf = buf;
ret = __i2c_transfer(adap, &msg, 1);
+
+ if (ret >= 0 && ret != 1)
+ ret = -EREMOTEIO;
} else {
union i2c_smbus_data data;
ret = adap->algo->smbus_xfer(adap, client->addr,
@@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret < 0 ? 0 : regval;
}
return ret;
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply related
* i2c: xiic: Strange clk_prepare_enable() in xiic_i2c_remove()
From: Alexey Khoroshilov @ 2016-12-16 21:54 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: Alexey Khoroshilov, Rob Herring, Wolfram Sang, linux-i2c,
linux-kernel, ldv-project
Dear Shubhrajyoti,
Looking at 36ecbcab84d0 ("i2c: xiic: Implement power management")
it is not clear why clk_prepare_enable(i2c->clk) is required in
xiic_i2c_remove()?
It is enabled in xiic_i2c_probe() and disabled/enabled in
cdns_i2c_runtime_suspend()/cdns_i2c_runtime_resume().
Could you please clarify the point.
--
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
^ permalink raw reply
* Re: [PATCH] i2c: i2c-mux-pca954x: only reset last channel on error
From: Peter Rosin @ 2016-12-16 21:30 UTC (permalink / raw)
To: Vivien Didelot
Cc: Wolfram Sang, linux-i2c, linux-kernel, kernel, Russell King
In-Reply-To: <20161216210843.26853-1-vivien.didelot@savoirfairelinux.com>
On 2016-12-16 22:08, Vivien Didelot wrote:
> The current code is selecting the mux channel before each operation and
> does not benefit from the cached value (data->last_chan).
>
> That is because pca954x_select_chan() considers any non-zero values from
> pca954x_reg_write() as an error. But this function (via __i2c_transfer)
> returns either a negative error code or the positive number of messages
> executed.
>
> Only check "ret" against negative values to restore the caching.
>
> Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Hi Vivien,
Thanks for the report, and sorry for breaking things. However, your
patch got beaten by an hour by Russel King who sent approximately
the same patch. I had already responded to his message when I saw
yours, so I did not Cc you.
Cheers,
peda
^ permalink raw reply
* Re: [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-16 21:20 UTC (permalink / raw)
To: Russell King; +Cc: Wolfram Sang, linux-i2c, linux-arm-kernel
In-Reply-To: <E1cHylf-00071o-KZ@rmk-PC.armlinux.org.uk>
On 2016-12-16 21:06, Russell King wrote:
> smbus functions return -ve on error, 0 on success. However,
> __i2c_transfer() have a different return signature - -ve on error, or
> number of buffers transferred (which may be zero or greater.)
>
> The upshot of this is that the sense of the test is reversed when using
> the mux on a bus supporting the master_xfer method: we cache the value
> and never retry if we fail to transfer any buffers, but if we succeed,
> we clear the cached value.
Ouch! Thanks for catching this.
> Fix this.
But lets fix the corner case of __i2c_transfer returning 0 instead of
the expected 1 as well (not sure if that's even possible, but lets close
the possibility just in case), so I'd prefer if you could fix
pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
instead, and -EREMOTEIO on other non-negative return values. Thanks!
Cheers,
peda
> Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8bc3d36d2837..b6d62ecbd5b6 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -179,7 +179,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> /* Only select the channel if its different from the last channel */
> if (data->last_chan != regval) {
> ret = pca954x_reg_write(muxc->parent, client, regval);
> - data->last_chan = ret ? 0 : regval;
> + data->last_chan = ret >= 0 ? regval : 0;
> }
>
> return ret;
>
^ permalink raw reply
* [PATCH] i2c: i2c-mux-pca954x: only reset last channel on error
From: Vivien Didelot @ 2016-12-16 21:08 UTC (permalink / raw)
To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-kernel, kernel, Vivien Didelot
The current code is selecting the mux channel before each operation and
does not benefit from the cached value (data->last_chan).
That is because pca954x_select_chan() considers any non-zero values from
pca954x_reg_write() as an error. But this function (via __i2c_transfer)
returns either a negative error code or the positive number of messages
executed.
Only check "ret" against negative values to restore the caching.
Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36..0469f1e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -179,7 +179,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret < 0 ? 0 : regval;
}
return ret;
--
2.10.2
^ permalink raw reply related
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Russell King @ 2016-12-16 20:06 UTC (permalink / raw)
To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-arm-kernel
smbus functions return -ve on error, 0 on success. However,
__i2c_transfer() have a different return signature - -ve on error, or
number of buffers transferred (which may be zero or greater.)
The upshot of this is that the sense of the test is reversed when using
the mux on a bus supporting the master_xfer method: we cache the value
and never retry if we fail to transfer any buffers, but if we succeed,
we clear the cached value.
Fix this.
Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36d2837..b6d62ecbd5b6 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -179,7 +179,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret >= 0 ? regval : 0;
}
return ret;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4] i2c: designware: Cleaning and comment style fixes.
From: Jarkko Nikula @ 2016-12-16 10:48 UTC (permalink / raw)
To: Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <928f25ad1fb39ec458758070d100852f7e47f1bb.1481881774.git.lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On 12/16/2016 11:54 AM, Luis Oliveira wrote:
> The purpose of this commit is to fix some comments and styling issues in the
> existing code due to the need of reuse this code. What is being made here is:
>
./scripts/checkpatch.pl complains about this. Following in ~/.vimrc will
help if vim is your editor for git commit:
autocmd Filetype gitcommit set textwidth=72
Although not that big issue.
Acked-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4] i2c: designware: Cleaning and comment style fixes.
From: Luis Oliveira @ 2016-12-16 9:54 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
The purpose of this commit is to fix some comments and styling issues in the
existing code due to the need of reuse this code. What is being made here is:
- Sorted the headers files
- Corrected some comments style
- Reverse tree in the variables declaration
- Add/remove empty lines and tabs where needed
- Fix of a misspelled word
The value of this, besides the rules of coding style, is because I
will use this code after and it will make my future patch a lot bigger and
complicated to review. The work here won't bring any additional work to
backported fixes because is just style and reordering.
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
Changes V3->V4: (Peter Rosin)
- Remove the unnecessary TAB as suggested
drivers/i2c/busses/i2c-designware-core.c | 36 ++++++++++++++---------------
drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++-----------
2 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 6d81c56184d3..9d724a6a7ec8 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -21,17 +21,17 @@
* ----------------------------------------------------------------------------
*
*/
+#include <linux/delay.h>
#include <linux/export.h>
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/io.h>
-#include <linux/pm_runtime.h>
-#include <linux/delay.h>
#include <linux/module.h>
-#include "i2c-designware-core.h"
+#include <linux/pm_runtime.h>
+#include "i2c-designware-core.h"
/*
* Registers offset
*/
@@ -98,7 +98,7 @@
#define DW_IC_ERR_TX_ABRT 0x1
-#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
+#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
#define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH (BIT(2) | BIT(3))
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
@@ -113,10 +113,10 @@
#define TIMEOUT 20 /* ms */
/*
- * hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
+ * Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
*
- * only expected abort codes are listed here
- * refer to the datasheet for the full list
+ * Only expected abort codes are listed here,
+ * refer to the datasheet for the full list.
*/
#define ABRT_7B_ADDR_NOACK 0
#define ABRT_10ADDR1_NOACK 1
@@ -338,14 +338,14 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
- /* Configure register endianess access */
+ /* Configure register endianness access */
dev->accessor_flags |= ACCESS_SWAP;
} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
/* Configure register access mode 16bit */
dev->accessor_flags |= ACCESS_16BIT;
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
- dev_err(dev->dev, "Unknown Synopsys component type: "
- "0x%08x\n", reg);
+ dev_err(dev->dev,
+ "Unknown Synopsys component type: 0x%08x\n", reg);
i2c_dw_release_lock(dev);
return -ENODEV;
}
@@ -355,7 +355,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);
- /* set standard and fast speed deviders for high/low periods */
+ /* Set standard and fast speed deviders for high/low periods */
sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
@@ -444,7 +444,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
dw_writel(dev, 0, DW_IC_RX_TL);
- /* configure the i2c master */
+ /* Configure the I2C master */
dw_writel(dev, dev->master_cfg , DW_IC_CON);
i2c_dw_release_lock(dev);
@@ -480,7 +480,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);
- /* if the slave address is ten bit address, enable 10BITADDR */
+ /* If the slave address is ten bit address, enable 10BITADDR */
if (dev->dynamic_tar_update_enabled) {
/*
* If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
@@ -505,7 +505,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
*/
dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);
- /* enforce disabled interrupts (due to HW issues) */
+ /* Enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);
/* Enable the adapter */
@@ -539,7 +539,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u32 flags = msgs[dev->msg_write_idx].flags;
/*
- * if target address has changed, we need to
+ * If target address has changed, we need to
* reprogram the target address in the i2c
* adapter when we are done with this transfer
*/
@@ -601,7 +601,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
- /* avoid rx buffer overrun */
+ /* Avoid rx buffer overrun */
if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
@@ -905,7 +905,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
/*
* Anytime TX_ABRT is set, the contents of the tx/rx
- * buffers are flushed. Make sure to skip them.
+ * buffers are flushed. Make sure to skip them.
*/
dw_writel(dev, 0, DW_IC_INTR_MASK);
goto tx_aborted;
@@ -927,7 +927,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(&dev->cmd_complete);
else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
- /* workaround to trigger pending interrupt */
+ /* Workaround to trigger pending interrupt */
stat = dw_readl(dev, DW_IC_INTR_MASK);
i2c_dw_disable_int(dev);
dw_writel(dev, stat, DW_IC_INTR_MASK);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 08153ea4d848..886e39753fc2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -21,26 +21,27 @@
* ----------------------------------------------------------------------------
*
*/
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dmi.h>
-#include <linux/i2c.h>
-#include <linux/clk.h>
-#include <linux/clk-provider.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
-#include <linux/io.h>
+#include <linux/sched.h>
#include <linux/slab.h>
-#include <linux/acpi.h>
-#include <linux/platform_data/i2c-designware.h>
+
#include "i2c-designware-core.h"
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
@@ -153,11 +154,11 @@ static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
- struct dw_i2c_dev *dev;
struct i2c_adapter *adap;
+ struct dw_i2c_dev *dev;
+ u32 acpi_speed, ht = 0;
struct resource *mem;
int irq, r;
- u32 acpi_speed, ht = 0;
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -354,7 +355,7 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
#define DW_I2C_DEV_PMOPS NULL
#endif
-/* work with hotplug and coldplug */
+/* Work with hotplug and coldplug */
MODULE_ALIAS("platform:i2c_designware");
static struct platform_driver dw_i2c_driver = {
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2] i2c: designware: add reset interface
From: zhangfei @ 2016-12-16 3:01 UTC (permalink / raw)
To: Ramiro Oliveira, Andy Shevchenko, Wolfram Sang,
mika.westerberg@linux.intel.com, jarkko.nikula@linux.intel.com,
Philipp Zabel
Cc: linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org
In-Reply-To: <fd0afd5f-6ee4-a06f-2d03-444f71662c1b@linaro.org>
Hi, Philipp
On 2016年12月16日 10:45, zhangfei wrote:
>
>
> On 2016年12月15日 23:30, Ramiro Oliveira wrote:
>> Hi Andy and Zhangfei
>>
>> On 12/15/2016 12:33 PM, Andy Shevchenko wrote:
>>> On Thu, 2016-12-15 at 16:59 +0800, Zhangfei Gao wrote:
>>>> Some platforms like hi3660 need do reset first to allow accessing
>>>> registers
>>> Patch itself looks good, but would be nice to have it tested.
>>>
>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>
>> I tested the patch and it's working for the ARC architecture.
>>
>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>> ---
>>>> drivers/i2c/busses/i2c-designware-core.h | 1 +
>>>> drivers/i2c/busses/i2c-designware-platdrv.c | 28
>>>> ++++++++++++++++++++++++----
>>>> 2 files changed, 25 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/i2c/busses/i2c-designware-core.h
>>>> b/drivers/i2c/busses/i2c-designware-core.h
>>>> index 0d44d2a..94b14fa 100644
>>>> --- a/drivers/i2c/busses/i2c-designware-core.h
>>>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>>>> @@ -80,6 +80,7 @@ struct dw_i2c_dev {
>>>> void __iomem *base;
>>>> struct completion cmd_complete;
>>>> struct clk *clk;
>>>> + struct reset_control *rst;
>>>> u32 (*get_clk_rate_khz) (struct
>>>> dw_i2c_dev *dev);
>>>> struct dw_pci_controller *controller;
>>>> int cmd_err;
>>>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> b/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> index 0b42a12..e9016ae 100644
>>>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> @@ -38,6 +38,7 @@
>>>> #include <linux/pm_runtime.h>
>>>> #include <linux/property.h>
>>>> #include <linux/io.h>
>>>> +#include <linux/reset.h>
>>>> #include <linux/slab.h>
>>>> #include <linux/acpi.h>
>>>> #include <linux/platform_data/i2c-designware.h>
>>>> @@ -176,6 +177,14 @@ static int dw_i2c_plat_probe(struct
>>>> platform_device *pdev)
>>>> dev->irq = irq;
>>>> platform_set_drvdata(pdev, dev);
>>>> + dev->rst = devm_reset_control_get_optional(&pdev->dev, NULL);
>> devm_reset_control_get_optional() is deprecated as explained in
>> linux/reset.h,
>> you should use devm_reset_control_get_optional_exclusive() or
>> devm_reset_control_get_optional_shared() instead, as applicable.
>>
>> I submitted a similar patch earlier today and I made the same mistake.
>
> Thanks Ramiro for the info
> Will use devm_reset_control_get_optional_exclusive instead.
> But should the interface be as simple as possible?
>
> Thanks
Sorry, a bit confused.
Is that mean we should not use devm_reset_control_get_optional()
While driver should decide whether use
devm_reset_control_get_optional_exclusive() or
devm_reset_control_get_optional_shared()
What if different platform has different requirement?
Looks the difference between _exclusive and _shared is refcount,
How about handle this inside, and not decided by interface?
Thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox