All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
  2015-12-11 12:02 ` Xiangliang Yu
  (?)
@ 2015-12-11  8:42 ` Jarkko Nikula
  -1 siblings, 0 replies; 6+ messages in thread
From: Jarkko Nikula @ 2015-12-11  8:42 UTC (permalink / raw)
  To: Xiangliang Yu, andriy.shevchenko, mika.westerberg, wsa,
	linux-i2c, linux-kernel
  Cc: SPG_Linux_Kernel

On 12/11/2015 02:02 PM, Xiangliang Yu wrote:
> Because of some hardware limitation, AMD I2C controller can't
> trigger pending interrupt if interrupt status has been changed
> after clearing interrupt status bits. Then, I2C will lost
> interrupt and IO timeout.
>
> According to hardware design, this patch implements a workaround
> to disable i2c controller interrupt and re-enable i2c interrupt
> before exiting ISR.
>
> To reduce the performance impacts on other vendors, use unlikely
> function to check flag in ISR.
> ---
> Changes in v2:
>      - pass flags with ->driver_data
>      - unmask interrupt right after masking
>
> Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
> ---
>   drivers/i2c/busses/i2c-designware-core.c    | 6 ++++++
>   drivers/i2c/busses/i2c-designware-core.h    | 1 +
>   drivers/i2c/busses/i2c-designware-platdrv.c | 7 ++++++-
>   3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index 8c48b27..de7fbbb 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -813,6 +813,12 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
>   tx_aborted:
>   	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 */
> +		stat = dw_readl(dev, DW_IC_INTR_MASK);
> +		i2c_dw_disable_int(dev);
> +		dw_writel(dev, stat, DW_IC_INTR_MASK);
> +	}
>
Looks ok to me.

Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
@ 2015-12-11 12:02 ` Xiangliang Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Xiangliang Yu @ 2015-12-11 12:02 UTC (permalink / raw)
  To: andriy.shevchenko, jarkko.nikula, mika.westerberg, wsa,
	linux-i2c, linux-kernel
  Cc: SPG_Linux_Kernel, Xiangliang Yu

Because of some hardware limitation, AMD I2C controller can't
trigger pending interrupt if interrupt status has been changed
after clearing interrupt status bits. Then, I2C will lost
interrupt and IO timeout.

According to hardware design, this patch implements a workaround
to disable i2c controller interrupt and re-enable i2c interrupt
before exiting ISR.

To reduce the performance impacts on other vendors, use unlikely
function to check flag in ISR.
---
Changes in v2:
    - pass flags with ->driver_data
    - unmask interrupt right after masking

Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
---
 drivers/i2c/busses/i2c-designware-core.c    | 6 ++++++
 drivers/i2c/busses/i2c-designware-core.h    | 1 +
 drivers/i2c/busses/i2c-designware-platdrv.c | 7 ++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..de7fbbb 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -813,6 +813,12 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 tx_aborted:
 	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 */
+		stat = dw_readl(dev, DW_IC_INTR_MASK);
+		i2c_dw_disable_int(dev);
+		dw_writel(dev, stat, DW_IC_INTR_MASK);
+	}
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 1d50898..9ffb63a 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -111,6 +111,7 @@ struct dw_i2c_dev {
 
 #define ACCESS_SWAP		0x00000001
 #define ACCESS_16BIT		0x00000002
+#define ACCESS_INTR_MASK	0x00000004
 
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..f03ea71 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -93,6 +93,7 @@ static void dw_i2c_acpi_params(struct platform_device *pdev, char method[],
 static int dw_i2c_acpi_configure(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+	const struct acpi_device_id *id;
 
 	dev->adapter.nr = -1;
 	dev->tx_fifo_depth = 32;
@@ -106,6 +107,10 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 	dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt,
 			   &dev->sda_hold_time);
 
+	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+	if (id && id->driver_data)
+		dev->accessor_flags |= (u32)id->driver_data;
+
 	return 0;
 }
 
@@ -116,7 +121,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "INT3433", 0 },
 	{ "80860F41", 0 },
 	{ "808622C1", 0 },
-	{ "AMD0010", 0 },
+	{ "AMD0010", ACCESS_INTR_MASK },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
@ 2015-12-11 12:02 ` Xiangliang Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Xiangliang Yu @ 2015-12-11 12:02 UTC (permalink / raw)
  To: andriy.shevchenko, jarkko.nikula, mika.westerberg, wsa,
	linux-i2c, linux-kernel
  Cc: SPG_Linux_Kernel, Xiangliang Yu

Because of some hardware limitation, AMD I2C controller can't
trigger pending interrupt if interrupt status has been changed
after clearing interrupt status bits. Then, I2C will lost
interrupt and IO timeout.

According to hardware design, this patch implements a workaround
to disable i2c controller interrupt and re-enable i2c interrupt
before exiting ISR.

To reduce the performance impacts on other vendors, use unlikely
function to check flag in ISR.
---
Changes in v2:
    - pass flags with ->driver_data
    - unmask interrupt right after masking

Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
---
 drivers/i2c/busses/i2c-designware-core.c    | 6 ++++++
 drivers/i2c/busses/i2c-designware-core.h    | 1 +
 drivers/i2c/busses/i2c-designware-platdrv.c | 7 ++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..de7fbbb 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -813,6 +813,12 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 tx_aborted:
 	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 */
+		stat = dw_readl(dev, DW_IC_INTR_MASK);
+		i2c_dw_disable_int(dev);
+		dw_writel(dev, stat, DW_IC_INTR_MASK);
+	}
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 1d50898..9ffb63a 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -111,6 +111,7 @@ struct dw_i2c_dev {
 
 #define ACCESS_SWAP		0x00000001
 #define ACCESS_16BIT		0x00000002
+#define ACCESS_INTR_MASK	0x00000004
 
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..f03ea71 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -93,6 +93,7 @@ static void dw_i2c_acpi_params(struct platform_device *pdev, char method[],
 static int dw_i2c_acpi_configure(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+	const struct acpi_device_id *id;
 
 	dev->adapter.nr = -1;
 	dev->tx_fifo_depth = 32;
@@ -106,6 +107,10 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 	dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt,
 			   &dev->sda_hold_time);
 
+	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+	if (id && id->driver_data)
+		dev->accessor_flags |= (u32)id->driver_data;
+
 	return 0;
 }
 
@@ -116,7 +121,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "INT3433", 0 },
 	{ "80860F41", 0 },
 	{ "808622C1", 0 },
-	{ "AMD0010", 0 },
+	{ "AMD0010", ACCESS_INTR_MASK },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
  2015-12-11 12:02 ` Xiangliang Yu
  (?)
  (?)
@ 2015-12-12 17:03 ` Wolfram Sang
  2015-12-13 12:01     ` Yu, Xiangliang
  -1 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2015-12-12 17:03 UTC (permalink / raw)
  To: Xiangliang Yu
  Cc: andriy.shevchenko, jarkko.nikula, mika.westerberg, linux-i2c,
	linux-kernel, SPG_Linux_Kernel

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

On Fri, Dec 11, 2015 at 08:02:53PM +0800, Xiangliang Yu wrote:
> Because of some hardware limitation, AMD I2C controller can't
> trigger pending interrupt if interrupt status has been changed
> after clearing interrupt status bits. Then, I2C will lost
> interrupt and IO timeout.
> 
> According to hardware design, this patch implements a workaround
> to disable i2c controller interrupt and re-enable i2c interrupt
> before exiting ISR.
> 
> To reduce the performance impacts on other vendors, use unlikely
> function to check flag in ISR.
> ---

Don't manually add "---". This breaks a lot of workflow scripts.
"Patchwork" missed your Signed-off, for example!

> Changes in v2:
>     - pass flags with ->driver_data
>     - unmask interrupt right after masking

This paragraph...

> 
> Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
> ---

... needs to go here.

However, I fixed it this time and applied to for-current, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
  2015-12-12 17:03 ` Wolfram Sang
@ 2015-12-13 12:01     ` Yu, Xiangliang
  0 siblings, 0 replies; 6+ messages in thread
From: Yu, Xiangliang @ 2015-12-13 12:01 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: andriy.shevchenko, jarkko.nikula, mika.westerberg, linux-i2c,
	linux-kernel, SPG_Linux_Kernel

> -----Original Message-----
> From: Wolfram Sang [mailto:wsa@the-dreams.de]
> Sent: Sunday, December 13, 2015 1:03 AM
> To: Yu, Xiangliang
> Cc: andriy.shevchenko@linux.intel.com; jarkko.nikula@linux.intel.com;
> mika.westerberg@linux.intel.com; linux-i2c@vger.kernel.org; linux-
> kernel@vger.kernel.org; SPG_Linux_Kernel
> Subject: Re: [PATCH v2] I2C: designware: fix IO timeout issue for AMD
> controller
> 
> On Fri, Dec 11, 2015 at 08:02:53PM +0800, Xiangliang Yu wrote:
> > Because of some hardware limitation, AMD I2C controller can't trigger
> > pending interrupt if interrupt status has been changed after clearing
> > interrupt status bits. Then, I2C will lost interrupt and IO timeout.
> >
> > According to hardware design, this patch implements a workaround to
> > disable i2c controller interrupt and re-enable i2c interrupt before
> > exiting ISR.
> >
> > To reduce the performance impacts on other vendors, use unlikely
> > function to check flag in ISR.
> > ---
> 
> Don't manually add "---". This breaks a lot of workflow scripts.
> "Patchwork" missed your Signed-off, for example!
Sorry for my mistake. 

> > Changes in v2:
> >     - pass flags with ->driver_data
> >     - unmask interrupt right after masking
> 
> This paragraph...
>
> >
> > Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
> > ---
> 
> ... needs to go here.
> 
> However, I fixed it this time and applied to for-current, thanks!

Thank you very much!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
@ 2015-12-13 12:01     ` Yu, Xiangliang
  0 siblings, 0 replies; 6+ messages in thread
From: Yu, Xiangliang @ 2015-12-13 12:01 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: andriy.shevchenko, jarkko.nikula, mika.westerberg, linux-i2c,
	linux-kernel, SPG_Linux_Kernel

> -----Original Message-----
> From: Wolfram Sang [mailto:wsa@the-dreams.de]
> Sent: Sunday, December 13, 2015 1:03 AM
> To: Yu, Xiangliang
> Cc: andriy.shevchenko@linux.intel.com; jarkko.nikula@linux.intel.com;
> mika.westerberg@linux.intel.com; linux-i2c@vger.kernel.org; linux-
> kernel@vger.kernel.org; SPG_Linux_Kernel
> Subject: Re: [PATCH v2] I2C: designware: fix IO timeout issue for AMD
> controller
> 
> On Fri, Dec 11, 2015 at 08:02:53PM +0800, Xiangliang Yu wrote:
> > Because of some hardware limitation, AMD I2C controller can't trigger
> > pending interrupt if interrupt status has been changed after clearing
> > interrupt status bits. Then, I2C will lost interrupt and IO timeout.
> >
> > According to hardware design, this patch implements a workaround to
> > disable i2c controller interrupt and re-enable i2c interrupt before
> > exiting ISR.
> >
> > To reduce the performance impacts on other vendors, use unlikely
> > function to check flag in ISR.
> > ---
> 
> Don't manually add "---". This breaks a lot of workflow scripts.
> "Patchwork" missed your Signed-off, for example!
Sorry for my mistake. 

> > Changes in v2:
> >     - pass flags with ->driver_data
> >     - unmask interrupt right after masking
> 
> This paragraph...
>
> >
> > Signed-off-by: Xiangliang Yu <Xiangliang.Yu@amd.com>
> > ---
> 
> ... needs to go here.
> 
> However, I fixed it this time and applied to for-current, thanks!

Thank you very much!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-12-13 12:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-11 12:02 [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller Xiangliang Yu
2015-12-11 12:02 ` Xiangliang Yu
2015-12-11  8:42 ` Jarkko Nikula
2015-12-12 17:03 ` Wolfram Sang
2015-12-13 12:01   ` Yu, Xiangliang
2015-12-13 12:01     ` Yu, Xiangliang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.