* [PATCH v2 0/2] i2c: octeon: thunderx: Recovery fixes and improvements
From: Jan Glauber @ 2016-09-23 9:40 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov, Jan Glauber
Hi Wolfram,
here are the remaining two patches with iopoll.h usage and a timed wait
before entering the recovery on a failed bus check.
thanks,
Jan
---------------------
Jan Glauber (2):
i2c: octeon: thunderx: Check bus state before starting a transaction
i2c: octeon: thunderx: Limit register access retries
drivers/i2c/busses/i2c-octeon-core.c | 33 ++++++++++++++++++++++++++++++++-
drivers/i2c/busses/i2c-octeon-core.h | 27 ++++++++++++++++-----------
2 files changed, 48 insertions(+), 12 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH v2 2/2] i2c: octeon: thunderx: Limit register access retries
From: Jan Glauber @ 2016-09-23 9:40 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov, Jan Glauber
In-Reply-To: <cover.1474565039.git.jglauber@cavium.com>
Do not infinitely retry register readq and writeq operations
in order to not lock up the CPU in case the TWSI gets stuck.
Return -ETIMEDOUT 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>
---
drivers/i2c/busses/i2c-octeon-core.c | 4 +++-
drivers/i2c/busses/i2c-octeon-core.h | 27 ++++++++++++++++-----------
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index f526511..1dfc4b8 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -381,7 +381,9 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,
if (result)
return result;
- data[i] = octeon_i2c_data_read(i2c);
+ data[i] = octeon_i2c_data_read(i2c, &result);
+ if (result)
+ return result;
if (recv_len && i == 0) {
if (data[i] > I2C_SMBUS_BLOCK_MAX + 1)
return -EPROTO;
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 87151ea..1db7c83 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -5,6 +5,7 @@
#include <linux/i2c.h>
#include <linux/i2c-smbus.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/pci.h>
@@ -144,9 +145,9 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
u64 tmp;
__raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI(i2c));
- do {
- tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
- } while ((tmp & SW_TWSI_V) != 0);
+
+ readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp, tmp & SW_TWSI_V,
+ I2C_OCTEON_EVENT_WAIT, i2c->adap.timeout);
}
#define octeon_i2c_ctl_write(i2c, val) \
@@ -163,24 +164,28 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
*
* The I2C core registers are accessed indirectly via the SW_TWSI CSR.
*/
-static inline u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)
+static inline int octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg,
+ int *error)
{
u64 tmp;
+ int ret;
__raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI(i2c));
- do {
- tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
- } while ((tmp & SW_TWSI_V) != 0);
+ ret = readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp,
+ tmp & SW_TWSI_V, I2C_OCTEON_EVENT_WAIT,
+ i2c->adap.timeout);
+ if (error)
+ *error = ret;
return tmp & 0xFF;
}
#define octeon_i2c_ctl_read(i2c) \
- octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL)
-#define octeon_i2c_data_read(i2c) \
- octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA)
+ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL, NULL)
+#define octeon_i2c_data_read(i2c, error) \
+ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA, error)
#define octeon_i2c_stat_read(i2c) \
- octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT)
+ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT, NULL)
/**
* octeon_i2c_read_int - read the TWSI_INT register
--
1.9.1
^ permalink raw reply related
* [PATCH] i2c: axxia: disable clks in case of failure in probe
From: Alexey Khoroshilov @ 2016-09-23 9:15 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Alexey Khoroshilov, linux-i2c, linux-kernel, ldv-project
In-Reply-To: <20160922175703.GE1861@katana>
axxia_i2c_probe() does not disable clock in case of failure
in i2c_add_adapter(). Also it ignores returned value from
clk_prepare_enable().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/i2c/busses/i2c-axxia.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-axxia.c b/drivers/i2c/busses/i2c-axxia.c
index d3bcaf4..4351a93 100644
--- a/drivers/i2c/busses/i2c-axxia.c
+++ b/drivers/i2c/busses/i2c-axxia.c
@@ -545,7 +545,11 @@ static int axxia_i2c_probe(struct platform_device *pdev)
return ret;
}
- clk_prepare_enable(idev->i2c_clk);
+ ret = clk_prepare_enable(idev->i2c_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable clock\n");
+ return ret;
+ }
i2c_set_adapdata(&idev->adapter, idev);
strlcpy(idev->adapter.name, pdev->name, sizeof(idev->adapter.name));
@@ -558,7 +562,13 @@ static int axxia_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, idev);
- return i2c_add_adapter(&idev->adapter);
+ ret = i2c_add_adapter(&idev->adapter);
+ if (ret) {
+ clk_disable_unprepare(idev->i2c_clk);
+ return ret;
+ }
+
+ return 0;
}
static int axxia_i2c_remove(struct platform_device *pdev)
--
2.7.4
^ permalink raw reply related
* [PATCH v2] i2c: i801: Add support for Kaby Lake PCH-H
From: Mika Westerberg @ 2016-09-23 8:56 UTC (permalink / raw)
To: Wolfram Sang, Jean Delvare; +Cc: linux-i2c, Andy Shevchenko, Mika Westerberg
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Kaby Lake PCH-H has the same legacy SMBus host controller than Intel
Sunrisepoint PCH. It also has same iTCO watchdog on the bus.
Add Kaby Lake PCH-H PCI ID to the list of supported devices.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes from v1:
* Call it Kaby Lake PCH-H instead of Kabypoint
* Add the device to the list of supported devices in comment on top of
the file.
drivers/i2c/busses/i2c-i801.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5ef9b733d153..4785e102e31c 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -64,6 +64,7 @@
* Broxton (SOC) 0x5ad4 32 hard yes yes yes
* Lewisburg (PCH) 0xa1a3 32 hard yes yes yes
* Lewisburg Supersku (PCH) 0xa223 32 hard yes yes yes
+ * Kaby Lake PCH-H (PCH) 0xa2a3 32 hard yes yes yes
*
* Features supported by this driver:
* Software PEC no
@@ -226,6 +227,7 @@
#define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS 0xa123
#define PCI_DEVICE_ID_INTEL_LEWISBURG_SMBUS 0xa1a3
#define PCI_DEVICE_ID_INTEL_LEWISBURG_SSKU_SMBUS 0xa223
+#define PCI_DEVICE_ID_INTEL_KABYLAKE_PCH_H_SMBUS 0xa2a3
struct i801_mux_config {
char *gpio_chip;
@@ -1006,6 +1008,7 @@ static const struct pci_device_id i801_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BROXTON_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LEWISBURG_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LEWISBURG_SSKU_SMBUS) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_KABYLAKE_PCH_H_SMBUS) },
{ 0, }
};
@@ -1482,6 +1485,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
case PCI_DEVICE_ID_INTEL_LEWISBURG_SMBUS:
case PCI_DEVICE_ID_INTEL_LEWISBURG_SSKU_SMBUS:
case PCI_DEVICE_ID_INTEL_DNV_SMBUS:
+ case PCI_DEVICE_ID_INTEL_KABYLAKE_PCH_H_SMBUS:
priv->features |= FEATURE_I2C_BLOCK_READ;
priv->features |= FEATURE_IRQ;
priv->features |= FEATURE_SMBUS_PEC;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning
From: Linus Walleij @ 2016-09-23 8:10 UTC (permalink / raw)
To: Wolfram Sang
Cc: Bartosz Golaszewski, Alexandre Courbot, Andy Shevchenko,
Vignesh R, Yong Li, Geert Uytterhoeven, Peter Zijlstra,
Ingo Molnar, Peter Rosin, linux-i2c, linux-gpio, LKML
In-Reply-To: <20160921054521.GB1484@katana>
On Wed, Sep 21, 2016 at 7:45 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> In order to get rid of the warning, retrieve the adapter nesting depth
>> and use it as lockdep subclass for chip->i2c_lock.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Linus, we'd like that in 4.9. Can I get your ack for the gpio part?
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Sorry for not attending quick enough, I was down in the
block layer.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
From: Joel Stanley @ 2016-09-23 6:02 UTC (permalink / raw)
To: Brendan Higgins
Cc: Rob Herring, Wolfram Sang, Mark Rutland, linux-i2c, devicetree,
linux-kernel, OpenBMC Maillist, Jeremy Kerr
In-Reply-To: <CAFd5g46j02sb2w_DwaXAgnfcO=ONmJGZwqdRneNd0QHq-t53rA@mail.gmail.com>
Hi Brendan,
On Wed, Sep 21, 2016 at 3:35 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> First off, someone pointed out to me that the mapping that I used between
> addresses and bus numbers is not actually valid for busses 8-14.
>
> This could be fixed by checking the offset, but I am wondering if that is
> the right way to do it. It seems like this is not completely trivial so
> maybe this should be specified in the device tree? If that is the case,
> should I do this as another reg entry or go back to the way I was doing it
> before?
I don't see an alternative way to derive these numbers. As you
mention, the block of SRAM in the middle of the IP screwing up the
linear mapping.
I suggest going back to what you have in v2, with a mention in the
commit message as to why the bus property is necessary.
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH v1 1/1] i2c: i801: Add support for Kaby Lake PCH-H
From: Wolfram Sang @ 2016-09-22 18:15 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-i2c, Mika Westerberg
In-Reply-To: <1469457828-9871-1-git-send-email-andriy.shevchenko@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On Mon, Jul 25, 2016 at 05:43:48PM +0300, Andy Shevchenko wrote:
> Intel Kaby Lake PCH-H has the same legacy SMBus host controller than Intel
> Sunrisepoint PCH. It also has same iTCO watchdog on the bus.
>
> Add Kaby Lake PCH-H PCI ID to the list of supported devices.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I guess Jean also wants the table at the top of the file to be extended
;)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: qup: skip qup_i2c_suspend if the device is already runtime suspended
From: Wolfram Sang @ 2016-09-22 18:10 UTC (permalink / raw)
To: Andy Gross
Cc: Sudeep Holla, linux-kernel, linux-i2c, linux-soc, linux-arm-msm,
David Brown
In-Reply-To: <20160914173307.GA5431@hector.attlocal.net>
[-- Attachment #1: Type: text/plain, Size: 169 bytes --]
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Reviewed-by: Andy Gross <andy.gross@linro.org>
I took the liberty to make "linaro" out of "linro".
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: qup: skip qup_i2c_suspend if the device is already runtime suspended
From: Wolfram Sang @ 2016-09-22 18:09 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-kernel, linux-i2c, linux-soc, linux-arm-msm, Andy Gross,
David Brown
In-Reply-To: <1472124219-1627-1-git-send-email-sudeep.holla@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
On Thu, Aug 25, 2016 at 12:23:39PM +0100, Sudeep Holla wrote:
> If the i2c device is already runtime suspended, if qup_i2c_suspend is
> executed during suspend-to-idle or suspend-to-ram it will result in the
> following splat:
>
> WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 clk_core_unprepare+0x80/0x90
> Modules linked in:
>
> CPU: 3 PID: 1593 Comm: bash Tainted: G W 4.8.0-rc3 #14
> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> PC is at clk_core_unprepare+0x80/0x90
> LR is at clk_unprepare+0x28/0x40
> pc : [<ffff0000086eecf0>] lr : [<ffff0000086f0c58>] pstate: 60000145
> Call trace:
> clk_core_unprepare+0x80/0x90
> qup_i2c_disable_clocks+0x2c/0x68
> qup_i2c_suspend+0x10/0x20
> platform_pm_suspend+0x24/0x68
> dpm_run_callback.isra.7+0x1c/0x70
> __device_suspend+0xf4/0x298
> dpm_suspend+0x10c/0x228
> dpm_suspend_start+0x68/0x78
> suspend_devices_and_enter+0xb8/0x460
> pm_suspend+0x1ec/0x240
> state_store+0x84/0xf8
> kobj_attr_store+0x14/0x28
> sysfs_kf_write+0x48/0x58
> kernfs_fop_write+0x15c/0x1f8
> __vfs_write+0x1c/0x100
> vfs_write+0x9c/0x1b8
> SyS_write+0x44/0xa0
> el0_svc_naked+0x24/0x28
>
> This patch fixes the issue by executing qup_i2c_pm_suspend_runtime
> conditionally in qup_i2c_suspend.
>
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: David Brown <david.brown@linaro.org>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: axxia: disable clks in case of failure in probe
From: Wolfram Sang @ 2016-09-22 17:57 UTC (permalink / raw)
To: Alexey Khoroshilov; +Cc: linux-i2c, linux-kernel, ldv-project
In-Reply-To: <1474057854-22545-1-git-send-email-khoroshilov@ispras.ru>
[-- Attachment #1: Type: text/plain, Size: 429 bytes --]
On Fri, Sep 16, 2016 at 11:30:54PM +0300, Alexey Khoroshilov wrote:
> axxia_i2c_probe() does not disable clock in case of failure
> in i2c_add_adapter(). Also it ignores returned value from
> clk_prepare_enable().
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
The driver has changed. Can you rebase against i2c-next or linux-next?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] Fix warnings for i2c-rk3x.c
From: Wolfram Sang @ 2016-09-22 17:52 UTC (permalink / raw)
To: David Wu
Cc: heiko, dianders, huangtao, wxt, linux-arm-kernel, linux-rockchip,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <1474543764-51135-1-git-send-email-david.wu@rock-chips.com>
[-- Attachment #1: Type: text/plain, Size: 208 bytes --]
On Thu, Sep 22, 2016 at 07:29:22PM +0800, David Wu wrote:
> David Wu (2):
> i2c: rk3x: Fix sparse warning
> i2c: rk3x: fix variable 'min_total_ns' unused warning
>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] i2c / ACPI: Do not touch an I2C device if it belongs to another adapter
From: Wolfram Sang @ 2016-09-22 17:46 UTC (permalink / raw)
To: Mika Westerberg
Cc: Nicolai Stange, Octavian Purdila, Rafael J . Wysocki,
Jarkko Nikula, linux-i2c, linux-kernel
In-Reply-To: <20160920135925.45450-1-mika.westerberg@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
On Tue, Sep 20, 2016 at 04:59:25PM +0300, Mika Westerberg wrote:
> When enumerating I2C devices connected to an I2C adapter we scan the whole
> namespace (as it is possible to have devices anywhere in that namespace,
> not just below the I2C adapter device) and add each found device to the I2C
> bus in question.
>
> Now after commit 525e6fabeae2 ("i2c / ACPI: add support for ACPI
> reconfigure notifications") checking of the adapter handle to the one found
> in the I2cSerialBus() resource was moved to happen after resources of the
> I2C device has been parsed. This means that if the I2cSerialBus() resource
> points to an adapter that does not exists in the system we still parse
> those resources. This is problematic in particular because
> acpi_dev_resource_interrupt() tries to configure GSI if the device also has
> an Interrupt() resource. Failing to do that results errrors like this to be
> printed on the console:
>
> [ 10.409490] ERROR: Unable to locate IOAPIC for GSI 37
>
> To fix this we pass the I2C adapter to i2c_acpi_get_info() and make sure
> the handle matches the one in the I2cSerialBus() resource before doing
> anything else to the device.
>
> Reported-and-tested-by: Nicolai Stange <nicstange@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries
From: Jan Glauber @ 2016-09-22 16:40 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Bazhenov, Dmitry
In-Reply-To: <20160921210335.GG1513@katana>
On Wed, Sep 21, 2016 at 11:03:35PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 08:51:06AM +0200, 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>
>
> I didn't really check, but have you considered using
> readq_poll_timeout() from iopoll.h?
>
Indeed, readq_poll_timeout() fits quite well here. It will lose some cycles
on mips but I'm not convinced that matters with i2c.
That would be the first user of readq_poll_timeout() in the kernel :)
--Jan
^ permalink raw reply
* Re: [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction
From: Jan Glauber @ 2016-09-22 16:08 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Bazhenov, Dmitry
In-Reply-To: <20160921205540.GC1513@katana>
On Wed, Sep 21, 2016 at 10:55:41PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 08:51:05AM +0200, Jan Glauber wrote:
> > Add an additional status check before starting a transaction and,
> > if required, trigger the recovery if the check fails.
> >
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>
> Won't this break multi-master setups?
>
I'm afraid yes. I don't have a multi-master setup, but agree that we
should not break it.
How about re-checking if the bus is idle until a timeout
(i2c->adap.timeout ?) happens and only then recover?
Additionaly we can check for arbitration loss as Dmitry did in his
original patch.
--Jan
^ permalink raw reply
* [PATCH 2/2] i2c: rk3x: Fix variable 'min_total_ns' unused warning
From: David Wu @ 2016-09-22 11:29 UTC (permalink / raw)
To: heiko, wsa
Cc: dianders, huangtao, wxt, linux-arm-kernel, linux-rockchip,
linux-i2c, devicetree, linux-kernel, David Wu
In-Reply-To: <1474543764-51135-1-git-send-email-david.wu@rock-chips.com>
This patch fixs the following warning:
drivers/i2c/busses/i2c-rk3x.c: In function 'rk3x_i2c_v1_calc_timings':
drivers/i2c/busses/i2c-rk3x.c:745:41: warning: variable 'min_total_ns' set but not used [-Wunused-but-set-variable]
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
drivers/i2c/busses/i2c-rk3x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 965477e..50702c7 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -742,7 +742,7 @@ static int rk3x_i2c_v1_calc_timings(unsigned long clk_rate,
struct i2c_timings *t,
struct rk3x_i2c_calced_timings *t_calc)
{
- unsigned long min_low_ns, min_high_ns, min_total_ns;
+ unsigned long min_low_ns, min_high_ns;
unsigned long min_setup_start_ns, min_setup_data_ns;
unsigned long min_setup_stop_ns, max_hold_data_ns;
@@ -793,7 +793,6 @@ static int rk3x_i2c_v1_calc_timings(unsigned long clk_rate,
/* These are the min dividers needed for min hold times. */
min_div_for_hold = (min_low_div + min_high_div);
- min_total_ns = min_low_ns + min_high_ns;
/*
* This is the maximum divider so we don't go over the maximum.
--
1.9.1
^ permalink raw reply related
* [PATCH 0/2] Fix warnings for i2c-rk3x.c
From: David Wu @ 2016-09-22 11:29 UTC (permalink / raw)
To: heiko, wsa
Cc: dianders, huangtao, wxt, linux-arm-kernel, linux-rockchip,
linux-i2c, devicetree, linux-kernel, David Wu
David Wu (2):
i2c: rk3x: Fix sparse warning
i2c: rk3x: fix variable 'min_total_ns' unused warning
drivers/i2c/busses/i2c-rk3x.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 1/2] i2c: rk3x: Fix sparse warning
From: David Wu @ 2016-09-22 11:29 UTC (permalink / raw)
To: heiko, wsa
Cc: dianders, huangtao, wxt, linux-arm-kernel, linux-rockchip,
linux-i2c, devicetree, linux-kernel, David Wu
In-Reply-To: <1474543764-51135-1-git-send-email-david.wu@rock-chips.com>
This patch fixes the following sparse warning:
drivers/i2c/busses/i2c-rk3x.c:888:17: warning: cast truncates bits from constant value (ffffffffff00 becomes ffffff00)
Signed-off-by: David Wu <david.wu@rock-chips.com>
---
drivers/i2c/busses/i2c-rk3x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index f2d0148..965477e 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -58,7 +58,7 @@ enum {
#define REG_CON_LASTACK BIT(5) /* 1: send NACK after last received byte */
#define REG_CON_ACTACK BIT(6) /* 1: stop if NACK is received */
-#define REG_CON_TUNING_MASK GENMASK(15, 8)
+#define REG_CON_TUNING_MASK GENMASK_ULL(15, 8)
#define REG_CON_SDA_CFG(cfg) ((cfg) << 8)
#define REG_CON_STA_CFG(cfg) ((cfg) << 12)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2] i2c / ACPI: Do not touch an I2C device if it belongs to another adapter
From: Mika Westerberg @ 2016-09-22 9:25 UTC (permalink / raw)
To: Wolfram Sang
Cc: Nicolai Stange, Octavian Purdila, Rafael J . Wysocki,
Jarkko Nikula, linux-i2c, linux-kernel
In-Reply-To: <20160922085924.GB1433@katana>
On Thu, Sep 22, 2016 at 10:59:24AM +0200, Wolfram Sang wrote:
>
> > > Huh? It doesn't apply on top of rc7 here? What did you base it on?
> >
> > It is based on linux-next as it is on top of Jarkko's I2C ACPI namespace
> > cleanup patches. I'm wondering if I make an updated patch on top of
> > v4.8-rc7 does it conflict with the I2C stuff in linux-next? What's your
> > preference?
>
> I see. I'll add it to for-next, then. If someone wants it backported, it
> needs to be rewritten and re-tested.
OK, thanks.
^ permalink raw reply
* Re: [PATCH v2] i2c / ACPI: Do not touch an I2C device if it belongs to another adapter
From: Wolfram Sang @ 2016-09-22 8:59 UTC (permalink / raw)
To: Mika Westerberg
Cc: Nicolai Stange, Octavian Purdila, Rafael J . Wysocki,
Jarkko Nikula, linux-i2c, linux-kernel
In-Reply-To: <20160922084930.GI1218@lahna.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
> > Huh? It doesn't apply on top of rc7 here? What did you base it on?
>
> It is based on linux-next as it is on top of Jarkko's I2C ACPI namespace
> cleanup patches. I'm wondering if I make an updated patch on top of
> v4.8-rc7 does it conflict with the I2C stuff in linux-next? What's your
> preference?
I see. I'll add it to for-next, then. If someone wants it backported, it
needs to be rewritten and re-tested.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] i2c / ACPI: Do not touch an I2C device if it belongs to another adapter
From: Mika Westerberg @ 2016-09-22 8:49 UTC (permalink / raw)
To: Wolfram Sang
Cc: Nicolai Stange, Octavian Purdila, Rafael J . Wysocki,
Jarkko Nikula, linux-i2c, linux-kernel
In-Reply-To: <20160921161435.GA1432@katana>
On Wed, Sep 21, 2016 at 06:14:38PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 11:45:02AM +0300, Mika Westerberg wrote:
> > On Wed, Sep 21, 2016 at 07:48:35AM +0200, Wolfram Sang wrote:
> > > On Tue, Sep 20, 2016 at 04:59:25PM +0300, Mika Westerberg wrote:
> > > > When enumerating I2C devices connected to an I2C adapter we scan the whole
> > > > namespace (as it is possible to have devices anywhere in that namespace,
> > > > not just below the I2C adapter device) and add each found device to the I2C
> > > > bus in question.
> > > >
> > > > Now after commit 525e6fabeae2 ("i2c / ACPI: add support for ACPI
> > > > reconfigure notifications") checking of the adapter handle to the one found
> > > > in the I2cSerialBus() resource was moved to happen after resources of the
> > > > I2C device has been parsed. This means that if the I2cSerialBus() resource
> > > > points to an adapter that does not exists in the system we still parse
> > > > those resources. This is problematic in particular because
> > > > acpi_dev_resource_interrupt() tries to configure GSI if the device also has
> > > > an Interrupt() resource. Failing to do that results errrors like this to be
> > > > printed on the console:
> > > >
> > > > [ 10.409490] ERROR: Unable to locate IOAPIC for GSI 37
> > > >
> > > > To fix this we pass the I2C adapter to i2c_acpi_get_info() and make sure
> > > > the handle matches the one in the I2cSerialBus() resource before doing
> > > > anything else to the device.
> > > >
> > > > Reported-and-tested-by: Nicolai Stange <nicstange@gmail.com>
> > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > >
> > > Considering this for for-current. So shall we add:
> > >
> > > Fixes: 525e6fabeae2 ("i2c / ACPI: add support for ACPI reconfigure notifications")
> > >
> > > ?
> >
> > Yes please :)
>
> Huh? It doesn't apply on top of rc7 here? What did you base it on?
It is based on linux-next as it is on top of Jarkko's I2C ACPI namespace
cleanup patches. I'm wondering if I make an updated patch on top of
v4.8-rc7 does it conflict with the I2C stuff in linux-next? What's your
preference?
^ permalink raw reply
* Re: [PATCH] i2c-eg20t: fix race between i2c init and interrupt enable
From: Wolfram Sang @ 2016-09-22 6:02 UTC (permalink / raw)
To: Yadi; +Cc: jdelvare, linux-i2c
In-Reply-To: <57E32E06.8080803@windriver.com>
[-- Attachment #1: Type: text/plain, Size: 354 bytes --]
> >Applied to for-next, thanks!
> >
> >Please make sure "V3" also appears in the patch subject since patch
> >management tools pick this up. "-v <nr>" in recent git versions makes
> >this super easy.
>
> Got it, I will resend the patch with V3 tag.
No need, it is already applied.
for-current carries patches for 4.8, for-next for 4.9.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/6] I2C: i2c-smbus: add device tree support
From: Phil Reid @ 2016-09-22 2:24 UTC (permalink / raw)
To: Mark Rutland
Cc: wsa, robh+dt, sre, andrea.merello, karl-heinz, arnd, linux-i2c,
devicetree, linux-pm
In-Reply-To: <20160921113405.GE18176@leverpostej>
G'day Mark
On 21/09/2016 19:34, Mark Rutland wrote:
> On Wed, Sep 21, 2016 at 04:41:09PM +0800, Phil Reid wrote:
>> From: Andrea Merello <andrea.merello@gmail.com>
>>
>> According to Documentation/i2c/smbus-protocol, a smbus controller driver
>> that wants to hook-in smbus extensions support, can call
>> i2c_setup_smbus_alert(). There are very few drivers that are currently
>> doing this.
>>
>> However the i2c-smbus module can also work with any
>> smbus-extensions-unaware I2C controller, as long as we provide an extra
>> IRQ line connected to the I2C bus ALARM signal.
>>
>> This patch makes it possible to go this way via DT. Note that the DT node
>> will indeed describe a (little) piece of HW, that is the routing of the
>> ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).
>
> Which piece of hardware actually generates this IRQ? The I2C controller?
> A slave SMBus device? Or something else?
>
> I'm not at all familiar with I2C or SMBus, and a quick scan of
> Documentation/i2c/smbus-protocol, has left me none-the-wiser on that
> front.
The originating source is the smbus device. SMBALERT is an active low interrupt
with the addition that the SMBUS device can be polled by the controller to determine
which device is asserting by performing a read of i2c address 0xc.
Asserting devices will return their address.
See the datahsheet of the LTC1760: http://www.linear.com/docs/1958 Page 22
The i2c parport driver looks to be only controller setup for this at the moment.
However that has limitations to only poll the primary i2c segment.
If muxes are involved then each muxed bus needs to be queried.
My hardware has:
/- ltc1760
i2c_cntlr - mux +
\- ltc1760
The alert signal are routed to the mux (pca9543) irq inputs and then muxes combined
irq output to an fpga pin on an ALTERA SOC which I can route to a GPIO / IRQ
I've modified the pca954x driver to generate separate interrupts for each segment.
Submitted an RFC PATCH on this a while ago, but it needs a lot of cleaning up before
submission.
So I can create an virtual SMB_ALERT device per segment and it knows to just poll that segment.
>
>> Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
>> with address 0x0C (that is the alarm response address), and IMHO this is
>> quite consistent with usage in the DT as a I2C child node.
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>> Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
>> drivers/i2c/i2c-smbus.c | 20 +++++++++++++++-----
>> 2 files changed, 35 insertions(+), 5 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>> new file mode 100644
>> index 0000000..da83127
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>> @@ -0,0 +1,20 @@
>> +* i2c-smbus extensions
>> +
>> +Required Properties:
>> + - compatible: Must contain "smbus_alert"
>
> Nit: s/_/-/ in compatible strings please.
>
>> + - interrupts: The irq line for smbus ALERT signal
>> + - reg: I2C slave address. Set to 0x0C (alert response address).
>> +
>> +Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
>> +a smbus controller directly support smbus extensions (and its driver supports
>> +this), there is no need to add anything special to the DT. Otherwise, for using
>> +i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
>> +route the smbus ALARM signal to an extra IRQ line, thus you need to describe
>> +this in the DT.
>
> Bindings shouldn't mention driver details (e.g. the i2c-smbus module
> behaviour). It feels like we're creating a virtual device for the sake
> of a driver, rather than accurately capturing the hardware.
>
> So as-is, I don't think this is the right way to describe this.
>
Yes that seemed to be the discussion last time.
It's like a shared IRQ but has the extra feature of the polling to determine what device is active.
For my use case the polling is not necessary as there's only one smbalert device per segment.
So an irq would work, however that didn't seem to have much support either.
I don't know what direction to go, to get this functionality into the upstream source.
--
Regards
Phil Reid
^ permalink raw reply
* Re: [PATCH] i2c-eg20t: fix race between i2c init and interrupt enable
From: Yadi @ 2016-09-22 1:19 UTC (permalink / raw)
To: Wolfram Sang; +Cc: jdelvare, linux-i2c
In-Reply-To: <20160921161846.GC1432@katana>
On 2016年09月22日 00:18, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 06:16:09PM +0200, Wolfram Sang wrote:
>> On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote:
>>> From: "Yadi.hu" <yadi.hu@windriver.com>
>>>
>>> the eg20t driver call request_irq() function before the pch_base_address,
>>> base address of i2c controller's register, is assigned an effective value.
>>>
>>> there is one possible scenario that an interrupt which isn't inside eg20t
>>> arrives immediately after request_irq() is executed when i2c controller
>>> shares an interrupt number with others. since the interrupt handler
>>> pch_i2c_handler() has already active as shared action, it will be called
>>> and read its own register to determine if this interrupt is from itself.
>>>
>>> At that moment, since base address of i2c registers is not remapped
>>> in kernel space yet,so the INT handler will access an illegal address
>>> and then a error occurs.
>>>
>>> Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
>> Applied to for-next, thanks!
> I meant: applied to for-current!
one quick and stupid question, what's different between for-current and
for-next branch?
Yadi
>
^ permalink raw reply
* [PATCH v3] i2c-eg20t: fix race between i2c init and interrupt enable
From: Yadi Hu @ 2016-09-22 1:09 UTC (permalink / raw)
To: yadi.hu, wsa, jdelvare; +Cc: linux-i2c
In-Reply-To: <1474506584-25153-1-git-send-email-yadi.hu@windriver.com>
From: "Yadi.hu" <yadi.hu@windriver.com>
the eg20t driver call request_irq() function before the pch_base_address,
base address of i2c controller's register, is assigned an effective value.
there is one possible scenario that an interrupt which isn't inside eg20t
arrives immediately after request_irq() is executed when i2c controller
shares an interrupt number with others. since the interrupt handler
pch_i2c_handler() has already active as shared action, it will be called
and read its own register to determine if this interrupt is from itself.
At that moment, since base address of i2c registers is not remapped
in kernel space yet,so the INT handler will access an illegal address
and then a error occurs.
Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
---
drivers/i2c/busses/i2c-eg20t.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index 137125b..5ce71ce 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -773,13 +773,6 @@ static int pch_i2c_probe(struct pci_dev *pdev,
/* Set the number of I2C channel instance */
adap_info->ch_num = id->driver_data;
- ret = request_irq(pdev->irq, pch_i2c_handler, IRQF_SHARED,
- KBUILD_MODNAME, adap_info);
- if (ret) {
- pch_pci_err(pdev, "request_irq FAILED\n");
- goto err_request_irq;
- }
-
for (i = 0; i < adap_info->ch_num; i++) {
pch_adap = &adap_info->pch_data[i].pch_adapter;
adap_info->pch_i2c_suspended = false;
@@ -797,6 +790,17 @@ static int pch_i2c_probe(struct pci_dev *pdev,
pch_adap->dev.of_node = pdev->dev.of_node;
pch_adap->dev.parent = &pdev->dev;
+ }
+
+ ret = request_irq(pdev->irq, pch_i2c_handler, IRQF_SHARED,
+ KBUILD_MODNAME, adap_info);
+ if (ret) {
+ pch_pci_err(pdev, "request_irq FAILED\n");
+ goto err_request_irq;
+ }
+
+ for (i = 0; i < adap_info->ch_num; i++) {
+ pch_adap = &adap_info->pch_data[i].pch_adapter;
pch_i2c_init(&adap_info->pch_data[i]);
--
2.9.3
^ permalink raw reply related
* RESEND:v3 i2c-eg20t:fix race between i2c init and interrupt enable
From: Yadi Hu @ 2016-09-22 1:09 UTC (permalink / raw)
To: yadi.hu, wsa, jdelvare; +Cc: linux-i2c
the eg20t driver call request_irq() function before the pch_base_address,
base address of i2c controller's register, isassigned an effective value.
there is one possible scenario that an interrupt which isn't inside eg20t
arrives immediately after request_irq() is executed when i2c controller
shares an interrupt number with others. since the interrupt handler
pch_i2c_handler() has already active as shared action, it will be called
and read its own register to determine if this interrupt is from itself.
At that moment, since base address of i2c registers is not remapped
in kernel space yet,so the INT handler will access an illegal address
and then a error occurs.
Bad IO access at port 0x18 (return inl(port))
Call Trace:
[<c102c733>] warn_slowpath_common+0x73/0xa0
[<c13109f5>] ? bad_io_access+0x45/0x50
[<c13109f5>] ? bad_io_access+0x45/0x50
[<c102c804>] warn_slowpath_fmt+0x34/0x40
[<c13109f5>] bad_io_access+0x45/0x50
[<c1310b62>] ioread32+0x22/0x40
[<c14c60db>] pch_i2c_handler+0x3b/0x120
[<c1092f34>] handle_irq_event_percpu+0x64/0x330
[<c109323b>] handle_irq_event+0x3b/0x60
[<c1095b50>] ? unmask_irq+0x30/0x30
[<c1095ba0>] handle_fasteoi_irq+0x50/0xe0
<IRQ> [<c16e7e78>] ? do_IRQ+0x48/0xc0
[<c16e7f6f>] ? smp_apic_timer_interrupt+0x7f/0x17a
[<c16e7da9>] ? common_interrupt+0x29/0x30
[<c1008ebb>] ? mwait_idle+0x8b/0x2c0
[<c1009e8e>] ? cpu_idle+0x9e/0xc0
[<c16be408>] ? rest_init+0x6c/0x74
[<c19f770a>] ? start_kernel+0x311/0x318
[<c19f7231>] ? repair_env_string+0x51/0x51
[<c19f7078>] ? i386_start_kernel+0x78/0x7d
---[ end trace eb3a1028f468a140 ]---
i2c_eg20t 0000:05:0c.2: pch_i2c_handler :I2C-3 mode(0) is not supported
Yadi Hu (1)
i2c-eg20t: fix race between i2c init and interrupt enable
drivers/i2c/busses/i2c-eg20t.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
^ 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