[PATCH 0/6] i2c: designware: Code duplication removal and cleanups

[PATCH 0/6] i2c: designware: Code duplication removal and cleanups

[Jarkko Nikula]

It bothered to me to see "static struct i2c_algorithm i2c_dw_algo {}"
defined twice both in i2c-designware-pcidrv.c and
i2c-designware-platdrv.c and so many exported i2c-designware-core
functions.

It turned out some of them became unused or are local and there were
also duplicated probe code that I moved to new common i2c_dw_probe().

Object sizes below before and after this set from CONFIG_X86_64=y build.

   text	   data	    bss	    dec	    hex	filename
   6439	   1096	      0	   7535	   1d6f	drivers/i2c/busses/i2c-designware-core.ko
   5123	   1588	     16	   6727	   1a47	drivers/i2c/busses/i2c-designware-pci.ko
   5274	   1096	     16	   6386	   18f2	drivers/i2c/busses/i2c-designware-platform.ko
  16836	   3780	     32	  20648	   50a8	(TOTALS)

   text	   data	    bss	    dec	    hex	filename
   7225	   1120	     16	   8361	   20a9	drivers/i2c/busses/i2c-designware-core.ko
   4281	   1524	      0	   5805	   16ad	drivers/i2c/busses/i2c-designware-pci.ko
   4337	   1072	      0	   5409	   1521	drivers/i2c/busses/i2c-designware-platform.ko
  15843	   3716	     16	  19575	   4c77	(TOTALS)

Jarkko Nikula (6):
  i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()
  i2c: designware: Disable interrupts before requesting PCI device
    interrupt
  i2c: designware: Remove unused functions
  i2c: designware: Make dw_readl() and dw_writel() static
  i2c: designware: Rename platform driver probe and PM functions
  i2c: designware: Move common probe code into i2c_dw_probe()

 drivers/i2c/busses/i2c-designware-core.c    | 73 ++++++++++++++++++-----------
 drivers/i2c/busses/i2c-designware-core.h    | 10 +---
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 31 ++----------
 drivers/i2c/busses/i2c-designware-platdrv.c | 52 ++++++--------------
 4 files changed, 63 insertions(+), 103 deletions(-)

-- 
2.5.0

[PATCH 1/6] i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()

[Jarkko Nikula]

There is no need to clear interrupts in i2c_dw_pci_probe() since only place
where interrupts are unmasked is i2c_dw_xfer_init() and there interrupts
are always cleared after commit 2a2d95e9d6d2 ("i2c: designware: always
clear interrupts before enabling them").

This allows to cleanup the code and replace i2c_dw_clear_int() in
i2c_dw_xfer_init() by direct register read as there are no other callers.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c   | 8 +-------
 drivers/i2c/busses/i2c-designware-core.h   | 1 -
 drivers/i2c/busses/i2c-designware-pcidrv.c | 1 -
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 7441cdc1b34a..dec0af7d0471 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -438,7 +438,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	__i2c_dw_enable(dev, true);
 
 	/* Clear and enable interrupts */
-	i2c_dw_clear_int(dev);
+	dw_readl(dev, DW_IC_CLR_INTR);
 	dw_writel(dev, DW_IC_INTR_DEFAULT_MASK, DW_IC_INTR_MASK);
 }
 
@@ -839,12 +839,6 @@ void i2c_dw_disable(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_disable);
 
-void i2c_dw_clear_int(struct dw_i2c_dev *dev)
-{
-	dw_readl(dev, DW_IC_CLR_INTR);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_clear_int);
-
 void i2c_dw_disable_int(struct dw_i2c_dev *dev)
 {
 	dw_writel(dev, 0, DW_IC_INTR_MASK);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9630222abf32..f44aeeeb91c6 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -122,7 +122,6 @@ extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
 extern void i2c_dw_enable(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_is_enabled(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
-extern void i2c_dw_clear_int(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
 
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index df23e8c30e6f..e477dddcdae5 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -268,7 +268,6 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	}
 
 	i2c_dw_disable_int(dev);
-	i2c_dw_clear_int(dev);
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
-- 
2.5.0

[PATCH 2/6] i2c: designware: Disable interrupts before requesting PCI device interrupt

[Jarkko Nikula]

Device must not generate interrupts before registering the interrupt
handler so move i2c_dw_disable_int() before requesting it.

There are no known issues with this. The code has been here since commit
fe20ff5c7e9c ("i2c-designware: Add support for Designware core behind PCI
devices.").

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index e477dddcdae5..d9e8937f062f 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -260,6 +260,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci");
 
+	i2c_dw_disable_int(dev);
 	r = devm_request_irq(&pdev->dev, pdev->irq, i2c_dw_isr,
 			IRQF_SHARED | IRQF_COND_SUSPEND, adap->name, dev);
 	if (r) {
@@ -267,7 +268,6 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 		return r;
 	}
 
-	i2c_dw_disable_int(dev);
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
-- 
2.5.0

[PATCH 3/6] i2c: designware: Remove unused functions

[Jarkko Nikula]

i2c_dw_is_enabled() became unused by the commit be58eda775c8
("i2c: designware-pci: Cleanup driver power management") and
i2c_dw_enable() by the commit 3a48d1c08fe0 ("i2c: prevent spurious
interrupt on Designware controllers").

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c | 13 -------------
 drivers/i2c/busses/i2c-designware-core.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index dec0af7d0471..c9e93a76a455 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -815,19 +815,6 @@ tx_aborted:
 }
 EXPORT_SYMBOL_GPL(i2c_dw_isr);
 
-void i2c_dw_enable(struct dw_i2c_dev *dev)
-{
-       /* Enable the adapter */
-	__i2c_dw_enable(dev, true);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_enable);
-
-u32 i2c_dw_is_enabled(struct dw_i2c_dev *dev)
-{
-	return dw_readl(dev, DW_IC_ENABLE);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_is_enabled);
-
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
 	/* Disable controller */
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index f44aeeeb91c6..10c79038834d 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -119,8 +119,6 @@ extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 		int num);
 extern u32 i2c_dw_func(struct i2c_adapter *adap);
 extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
-extern void i2c_dw_enable(struct dw_i2c_dev *dev);
-extern u32 i2c_dw_is_enabled(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
-- 
2.5.0

[PATCH 4/6] i2c: designware: Make dw_readl() and dw_writel() static

[Jarkko Nikula]

dw_readl() and dw_writel() are not used outside of i2c-designware-core and
they are not exported so make them static and remove their forward
declaration.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c | 4 ++--
 drivers/i2c/busses/i2c-designware-core.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index c9e93a76a455..52272a01c679 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -165,7 +165,7 @@ static char *abort_sources[] = {
 		"lost arbitration",
 };
 
-u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 {
 	u32 value;
 
@@ -181,7 +181,7 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 		return value;
 }
 
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
 {
 	if (dev->accessor_flags & ACCESS_SWAP)
 		b = swab32(b);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 10c79038834d..0e73b8672402 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -112,8 +112,6 @@ struct dw_i2c_dev {
 #define ACCESS_SWAP		0x00000001
 #define ACCESS_16BIT		0x00000002
 
-extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
-extern void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
 extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 		int num);
-- 
2.5.0

[PATCH 5/6] i2c: designware: Rename platform driver probe and PM functions

[Jarkko Nikula]

Make it easier to distinguish between i2c-designware-platdrv and
i2c-designware-core functions and to be consistent with
i2c-designware-pcidrv.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3dd2de31a2f8..17167cd4812d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -133,7 +133,7 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
 static inline void dw_i2c_acpi_unconfigure(struct platform_device *pdev) { }
 #endif
 
-static int dw_i2c_probe(struct platform_device *pdev)
+static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
@@ -271,7 +271,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int dw_i2c_remove(struct platform_device *pdev)
+static int dw_i2c_plat_remove(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 
@@ -300,12 +300,12 @@ MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-static int dw_i2c_prepare(struct device *dev)
+static int dw_i2c_plat_prepare(struct device *dev)
 {
 	return pm_runtime_suspended(dev);
 }
 
-static void dw_i2c_complete(struct device *dev)
+static void dw_i2c_plat_complete(struct device *dev)
 {
 	if (dev->power.direct_complete)
 		pm_request_resume(dev);
@@ -316,7 +316,7 @@ static void dw_i2c_complete(struct device *dev)
 #endif
 
 #ifdef CONFIG_PM
-static int dw_i2c_suspend(struct device *dev)
+static int dw_i2c_plat_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
@@ -327,7 +327,7 @@ static int dw_i2c_suspend(struct device *dev)
 	return 0;
 }
 
-static int dw_i2c_resume(struct device *dev)
+static int dw_i2c_plat_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
@@ -341,10 +341,10 @@ static int dw_i2c_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
-	.prepare = dw_i2c_prepare,
-	.complete = dw_i2c_complete,
-	SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_suspend, dw_i2c_resume)
-	SET_RUNTIME_PM_OPS(dw_i2c_suspend, dw_i2c_resume, NULL)
+	.prepare = dw_i2c_plat_prepare,
+	.complete = dw_i2c_plat_complete,
+	SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
+	SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
 };
 
 #define DW_I2C_DEV_PMOPS (&dw_i2c_dev_pm_ops)
@@ -356,8 +356,8 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 MODULE_ALIAS("platform:i2c_designware");
 
 static struct platform_driver dw_i2c_driver = {
-	.probe = dw_i2c_probe,
-	.remove = dw_i2c_remove,
+	.probe = dw_i2c_plat_probe,
+	.remove = dw_i2c_plat_remove,
 	.driver		= {
 		.name	= "i2c_designware",
 		.of_match_table = of_match_ptr(dw_i2c_of_match),
-- 
2.5.0

[PATCH 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

There is some code duplication in i2c-designware-platdrv and
i2c-designware-pcidrv probe functions. What is even worse that duplication
requires i2c_dw_xfer(), i2c_dw_func() and i2c_dw_isr() i2c-designware-core
functions to be exported.

Therefore move common code into new i2c_dw_probe() and make functions above
local to i2c-designware-core.

While merging the code patch does following functional changes:

- I2C Adapter name will be "i2c-designware". Previously adapter name was
  "Synopsys DesignWare I2C adapter" for platform and ACPI devices and
  "i2c-designware-pci" for PCI devices.
- Interrupt name too will be "i2c-designware". Previous it was platform
  device name, ACPI device name or "i2c-designware-pci".
- Error code from devm_request_irq() and i2c_add_numbered_adapter() will be
  printed in case of error.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c    | 48 +++++++++++++++++++++++++----
 drivers/i2c/busses/i2c-designware-core.h    |  5 +--
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 30 ++----------------
 drivers/i2c/busses/i2c-designware-platdrv.c | 28 ++---------------
 4 files changed, 48 insertions(+), 63 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 52272a01c679..501c8ac0cf14 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -618,7 +618,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 /*
  * Prepare controller for a transaction and call i2c_dw_xfer_msg
  */
-int
+static int
 i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -702,14 +702,17 @@ done_nolock:
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_xfer);
 
-u32 i2c_dw_func(struct i2c_adapter *adap)
+static u32 i2c_dw_func(struct i2c_adapter *adap)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	return dev->functionality;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_func);
+
+static struct i2c_algorithm i2c_dw_algo = {
+	.master_xfer	= i2c_dw_xfer,
+	.functionality	= i2c_dw_func,
+};
 
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
@@ -770,7 +773,7 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
  * Interrupt service routine. This gets called whenever an I2C interrupt
  * occurs.
  */
-irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 {
 	struct dw_i2c_dev *dev = dev_id;
 	u32 stat, enabled;
@@ -813,7 +816,6 @@ tx_aborted:
 
 	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_isr);
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
@@ -838,5 +840,39 @@ u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
 
+int i2c_dw_probe(struct dw_i2c_dev *dev)
+{
+	struct i2c_adapter *adap = &dev->adapter;
+	int r;
+
+	init_completion(&dev->cmd_complete);
+	mutex_init(&dev->lock);
+
+	r = i2c_dw_init(dev);
+	if (r)
+		return r;
+
+	snprintf(adap->name, sizeof(adap->name), "i2c-designware");
+	adap->algo = &i2c_dw_algo;
+	adap->dev.parent = dev->dev;
+	i2c_set_adapdata(adap, dev);
+
+	i2c_dw_disable_int(dev);
+	r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
+			     adap->name, dev);
+	if (r) {
+		dev_err(dev->dev, "failure requesting irq %i: %d\n",
+			dev->irq, r);
+		return r;
+	}
+
+	r = i2c_add_numbered_adapter(adap);
+	if (r)
+		dev_err(dev->dev, "failure adding adapter: %d\n", r);
+
+	return r;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_probe);
+
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 0e73b8672402..1d50898e7b24 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -113,13 +113,10 @@ struct dw_i2c_dev {
 #define ACCESS_16BIT		0x00000002
 
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
-extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
-		int num);
-extern u32 i2c_dw_func(struct i2c_adapter *adap);
-extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
+extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 
 #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
 extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d9e8937f062f..169be1e26241 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -158,11 +158,6 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 	},
 };
 
-static struct i2c_algorithm i2c_dw_algo = {
-	.master_xfer	= i2c_dw_xfer,
-	.functionality	= i2c_dw_func,
-};
-
 #ifdef CONFIG_PM
 static int i2c_dw_pci_suspend(struct device *dev)
 {
@@ -222,13 +217,12 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	if (!dev)
 		return -ENOMEM;
 
-	init_completion(&dev->cmd_complete);
-	mutex_init(&dev->lock);
 	dev->clk = NULL;
 	dev->controller = controller;
 	dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
 	dev->base = pcim_iomap_table(pdev)[0];
 	dev->dev = &pdev->dev;
+	dev->irq = pdev->irq;
 	dev->functionality = controller->functionality |
 				DW_DEFAULT_FUNCTIONALITY;
 
@@ -246,33 +240,15 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	dev->tx_fifo_depth = controller->tx_fifo_depth;
 	dev->rx_fifo_depth = controller->rx_fifo_depth;
-	r = i2c_dw_init(dev);
-	if (r)
-		return r;
 
 	adap = &dev->adapter;
-	i2c_set_adapdata(adap, dev);
 	adap->owner = THIS_MODULE;
 	adap->class = 0;
-	adap->algo = &i2c_dw_algo;
-	adap->dev.parent = &pdev->dev;
 	adap->nr = controller->bus_num;
 
-	snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci");
-
-	i2c_dw_disable_int(dev);
-	r = devm_request_irq(&pdev->dev, pdev->irq, i2c_dw_isr,
-			IRQF_SHARED | IRQF_COND_SUSPEND, adap->name, dev);
-	if (r) {
-		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		return r;
-	}
-
-	r = i2c_add_numbered_adapter(adap);
-	if (r) {
-		dev_err(&pdev->dev, "failure adding adapter\n");
+	r = i2c_dw_probe(dev);
+	if (r)
 		return r;
-	}
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
 	pm_runtime_use_autosuspend(&pdev->dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 17167cd4812d..adcf4c3a81f0 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -41,10 +41,6 @@
 #include <linux/platform_data/i2c-designware.h>
 #include "i2c-designware-core.h"
 
-static struct i2c_algorithm i2c_dw_algo = {
-	.master_xfer	= i2c_dw_xfer,
-	.functionality	= i2c_dw_func,
-};
 static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 {
 	return clk_get_rate(dev->clk)/1000;
@@ -155,8 +151,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->base))
 		return PTR_ERR(dev->base);
 
-	init_completion(&dev->cmd_complete);
-	mutex_init(&dev->lock);
 	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
@@ -231,33 +225,15 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
 		dev->adapter.nr = pdev->id;
 	}
-	r = i2c_dw_init(dev);
-	if (r)
-		return r;
-
-	i2c_dw_disable_int(dev);
-	r = devm_request_irq(&pdev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
-			pdev->name, dev);
-	if (r) {
-		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		return r;
-	}
 
 	adap = &dev->adapter;
-	i2c_set_adapdata(adap, dev);
 	adap->owner = THIS_MODULE;
 	adap->class = I2C_CLASS_DEPRECATED;
-	strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
-			sizeof(adap->name));
-	adap->algo = &i2c_dw_algo;
-	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
-	r = i2c_add_numbered_adapter(adap);
-	if (r) {
-		dev_err(&pdev->dev, "failure adding adapter\n");
+	r = i2c_dw_probe(dev);
+	if (r)
 		return r;
-	}
 
 	if (dev->pm_runtime_disabled) {
 		pm_runtime_forbid(&pdev->dev);
-- 
2.5.0

Re: [PATCH 1/6] i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()

[Wolfram Sang]

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

On Mon, Aug 31, 2015 at 05:31:28PM +0300, Jarkko Nikula wrote:
> There is no need to clear interrupts in i2c_dw_pci_probe() since only place
> where interrupts are unmasked is i2c_dw_xfer_init() and there interrupts
> are always cleared after commit 2a2d95e9d6d2 ("i2c: designware: always
> clear interrupts before enabling them").

Still, their status might be unclear when we init because firmware might
have used it before. It should be easy to reintroduce this in patch 6,
or?


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

Re: [PATCH 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Wolfram Sang]

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


> - I2C Adapter name will be "i2c-designware". Previously adapter name was
>   "Synopsys DesignWare I2C adapter" for platform and ACPI devices and
>   "i2c-designware-pci" for PCI devices.

I have a small tendency to make it "Synopsys DesignWare I2C adapter" for
all cases. Not much, though.

> - Interrupt name too will be "i2c-designware". Previous it was platform
>   device name, ACPI device name or "i2c-designware-pci".

The device name makes them easier to distinguish in case you have
multiple instantiations.

Rest of the series looks good to me, thanks!


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

Re: [PATCH 1/6] i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()

[Jarkko Nikula]

On 10/10/2015 10:53 AM, Wolfram Sang wrote:
> On Mon, Aug 31, 2015 at 05:31:28PM +0300, Jarkko Nikula wrote:
>> There is no need to clear interrupts in i2c_dw_pci_probe() since only place
>> where interrupts are unmasked is i2c_dw_xfer_init() and there interrupts
>> are always cleared after commit 2a2d95e9d6d2 ("i2c: designware: always
>> clear interrupts before enabling them").
>
> Still, their status might be unclear when we init because firmware might
> have used it before. It should be easy to reintroduce this in patch 6,
> or?
>
Yes and then it would be also uniform across different enumeration 
methods if we ever need to add it. Now this clearing was done only for 
PCI devices. Although I don't think there is any need to add clearing to 
probe path as clearing happens beginning of transmit before enabling the 
interrupts.

-- 
Jarkko

Re: [PATCH 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

On 10/10/2015 10:57 AM, Wolfram Sang wrote:
>
>> - I2C Adapter name will be "i2c-designware". Previously adapter name was
>>    "Synopsys DesignWare I2C adapter" for platform and ACPI devices and
>>    "i2c-designware-pci" for PCI devices.
>
> I have a small tendency to make it "Synopsys DesignWare I2C adapter" for
> all cases. Not much, though.
>
Ok, will change.

>> - Interrupt name too will be "i2c-designware". Previous it was platform
>>    device name, ACPI device name or "i2c-designware-pci".
>
> The device name makes them easier to distinguish in case you have
> multiple instantiations.
>
Good point. Bug reports definitely will benefit from it given that we 
have platforms where there are differences between i2c-designware 
instances. Same platform can have different instances enumerated from 
PCI or ACPI and those are registered as platform devices in 
drivers/mfd/intel-lpss.c.

-- 
Jarkko

[PATCH v2 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

There is some code duplication in i2c-designware-platdrv and
i2c-designware-pcidrv probe functions. What is even worse that duplication
requires i2c_dw_xfer(), i2c_dw_func() and i2c_dw_isr() i2c-designware-core
functions to be exported.

Therefore move common code into new i2c_dw_probe() and make functions above
local to i2c-designware-core.

While merging the code patch does following functional changes:

- I2C Adapter name will be "Synopsys DesignWare I2C adapter". Previously it
  was used for platform and ACPI devices but PCI device used
  "i2c-designware-pci".
- Using device name for interrupt name. Previous it was platform device name,
  ACPI device name or "i2c-designware-pci".
- Error code from devm_request_irq() and i2c_add_numbered_adapter() will be
  printed in case of error.

Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
v2:
- Adapter name changed to "Synopsys DesignWare I2C adapter"
- Using device name for interrupt name
---
 drivers/i2c/busses/i2c-designware-core.c    | 49 +++++++++++++++++++++++++----
 drivers/i2c/busses/i2c-designware-core.h    |  5 +--
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 30 ++----------------
 drivers/i2c/busses/i2c-designware-platdrv.c | 28 ++---------------
 4 files changed, 49 insertions(+), 63 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 52272a01c679..8c48b27ba059 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -618,7 +618,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 /*
  * Prepare controller for a transaction and call i2c_dw_xfer_msg
  */
-int
+static int
 i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -702,14 +702,17 @@ done_nolock:
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_xfer);
 
-u32 i2c_dw_func(struct i2c_adapter *adap)
+static u32 i2c_dw_func(struct i2c_adapter *adap)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	return dev->functionality;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_func);
+
+static struct i2c_algorithm i2c_dw_algo = {
+	.master_xfer	= i2c_dw_xfer,
+	.functionality	= i2c_dw_func,
+};
 
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
@@ -770,7 +773,7 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
  * Interrupt service routine. This gets called whenever an I2C interrupt
  * occurs.
  */
-irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 {
 	struct dw_i2c_dev *dev = dev_id;
 	u32 stat, enabled;
@@ -813,7 +816,6 @@ tx_aborted:
 
 	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_isr);
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
@@ -838,5 +840,40 @@ u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
 
+int i2c_dw_probe(struct dw_i2c_dev *dev)
+{
+	struct i2c_adapter *adap = &dev->adapter;
+	int r;
+
+	init_completion(&dev->cmd_complete);
+	mutex_init(&dev->lock);
+
+	r = i2c_dw_init(dev);
+	if (r)
+		return r;
+
+	snprintf(adap->name, sizeof(adap->name),
+		 "Synopsys DesignWare I2C adapter");
+	adap->algo = &i2c_dw_algo;
+	adap->dev.parent = dev->dev;
+	i2c_set_adapdata(adap, dev);
+
+	i2c_dw_disable_int(dev);
+	r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
+			     dev_name(dev->dev), dev);
+	if (r) {
+		dev_err(dev->dev, "failure requesting irq %i: %d\n",
+			dev->irq, r);
+		return r;
+	}
+
+	r = i2c_add_numbered_adapter(adap);
+	if (r)
+		dev_err(dev->dev, "failure adding adapter: %d\n", r);
+
+	return r;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_probe);
+
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 0e73b8672402..1d50898e7b24 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -113,13 +113,10 @@ struct dw_i2c_dev {
 #define ACCESS_16BIT		0x00000002
 
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
-extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
-		int num);
-extern u32 i2c_dw_func(struct i2c_adapter *adap);
-extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
+extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 
 #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
 extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d9e8937f062f..169be1e26241 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -158,11 +158,6 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 	},
 };
 
-static struct i2c_algorithm i2c_dw_algo = {
-	.master_xfer	= i2c_dw_xfer,
-	.functionality	= i2c_dw_func,
-};
-
 #ifdef CONFIG_PM
 static int i2c_dw_pci_suspend(struct device *dev)
 {
@@ -222,13 +217,12 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	if (!dev)
 		return -ENOMEM;
 
-	init_completion(&dev->cmd_complete);
-	mutex_init(&dev->lock);
 	dev->clk = NULL;
 	dev->controller = controller;
 	dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
 	dev->base = pcim_iomap_table(pdev)[0];
 	dev->dev = &pdev->dev;
+	dev->irq = pdev->irq;
 	dev->functionality = controller->functionality |
 				DW_DEFAULT_FUNCTIONALITY;
 
@@ -246,33 +240,15 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	dev->tx_fifo_depth = controller->tx_fifo_depth;
 	dev->rx_fifo_depth = controller->rx_fifo_depth;
-	r = i2c_dw_init(dev);
-	if (r)
-		return r;
 
 	adap = &dev->adapter;
-	i2c_set_adapdata(adap, dev);
 	adap->owner = THIS_MODULE;
 	adap->class = 0;
-	adap->algo = &i2c_dw_algo;
-	adap->dev.parent = &pdev->dev;
 	adap->nr = controller->bus_num;
 
-	snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci");
-
-	i2c_dw_disable_int(dev);
-	r = devm_request_irq(&pdev->dev, pdev->irq, i2c_dw_isr,
-			IRQF_SHARED | IRQF_COND_SUSPEND, adap->name, dev);
-	if (r) {
-		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		return r;
-	}
-
-	r = i2c_add_numbered_adapter(adap);
-	if (r) {
-		dev_err(&pdev->dev, "failure adding adapter\n");
+	r = i2c_dw_probe(dev);
+	if (r)
 		return r;
-	}
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
 	pm_runtime_use_autosuspend(&pdev->dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 17167cd4812d..adcf4c3a81f0 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -41,10 +41,6 @@
 #include <linux/platform_data/i2c-designware.h>
 #include "i2c-designware-core.h"
 
-static struct i2c_algorithm i2c_dw_algo = {
-	.master_xfer	= i2c_dw_xfer,
-	.functionality	= i2c_dw_func,
-};
 static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 {
 	return clk_get_rate(dev->clk)/1000;
@@ -155,8 +151,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->base))
 		return PTR_ERR(dev->base);
 
-	init_completion(&dev->cmd_complete);
-	mutex_init(&dev->lock);
 	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
@@ -231,33 +225,15 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
 		dev->adapter.nr = pdev->id;
 	}
-	r = i2c_dw_init(dev);
-	if (r)
-		return r;
-
-	i2c_dw_disable_int(dev);
-	r = devm_request_irq(&pdev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
-			pdev->name, dev);
-	if (r) {
-		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
-		return r;
-	}
 
 	adap = &dev->adapter;
-	i2c_set_adapdata(adap, dev);
 	adap->owner = THIS_MODULE;
 	adap->class = I2C_CLASS_DEPRECATED;
-	strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
-			sizeof(adap->name));
-	adap->algo = &i2c_dw_algo;
-	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
-	r = i2c_add_numbered_adapter(adap);
-	if (r) {
-		dev_err(&pdev->dev, "failure adding adapter\n");
+	r = i2c_dw_probe(dev);
+	if (r)
 		return r;
-	}
 
 	if (dev->pm_runtime_disabled) {
 		pm_runtime_forbid(&pdev->dev);
-- 
2.6.1

Re: [PATCH 1/6] i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()

[Wolfram Sang]

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


> Although I don't think there is any need to add clearing to probe path as
> clearing happens beginning of transmit before enabling the interrupts.

Well, in case the firmware left interrupts dangling, they would still be
dangling unless someone starts some communication. Chances are small,
but better safe than sorry? Could be an incremental patch on top of your
V2 if you are up to it.

Thanks,

   Wolfram


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

Re: [PATCH 0/6] i2c: designware: Code duplication removal and cleanups

[Wolfram Sang]

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

On Mon, Aug 31, 2015 at 05:31:27PM +0300, Jarkko Nikula wrote:
> It bothered to me to see "static struct i2c_algorithm i2c_dw_algo {}"
> defined twice both in i2c-designware-pcidrv.c and
> i2c-designware-platdrv.c and so many exported i2c-designware-core
> functions.
> 
> It turned out some of them became unused or are local and there were
> also duplicated probe code that I moved to new common i2c_dw_probe().
> 
> Object sizes below before and after this set from CONFIG_X86_64=y build.
> 
>    text	   data	    bss	    dec	    hex	filename
>    6439	   1096	      0	   7535	   1d6f	drivers/i2c/busses/i2c-designware-core.ko
>    5123	   1588	     16	   6727	   1a47	drivers/i2c/busses/i2c-designware-pci.ko
>    5274	   1096	     16	   6386	   18f2	drivers/i2c/busses/i2c-designware-platform.ko
>   16836	   3780	     32	  20648	   50a8	(TOTALS)
> 
>    text	   data	    bss	    dec	    hex	filename
>    7225	   1120	     16	   8361	   20a9	drivers/i2c/busses/i2c-designware-core.ko
>    4281	   1524	      0	   5805	   16ad	drivers/i2c/busses/i2c-designware-pci.ko
>    4337	   1072	      0	   5409	   1521	drivers/i2c/busses/i2c-designware-platform.ko
>   15843	   3716	     16	  19575	   4c77	(TOTALS)
> 
> Jarkko Nikula (6):
>   i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()
>   i2c: designware: Disable interrupts before requesting PCI device
>     interrupt
>   i2c: designware: Remove unused functions
>   i2c: designware: Make dw_readl() and dw_writel() static
>   i2c: designware: Rename platform driver probe and PM functions
>   i2c: designware: Move common probe code into i2c_dw_probe()

Applied to for-next (with V2 of patch 6), thanks!

Jarkko, would you be interested in maintaining the designware driver?
For any non-trivial patch to this driver, I'd need assistance.

Thanks,

   Wolfram


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

Re: [PATCH 0/6] i2c: designware: Code duplication removal and cleanups

[Jarkko Nikula]

Hi

On 10/15/2015 03:18 PM, Wolfram Sang wrote:
> Jarkko, would you be interested in maintaining the designware driver?
> For any non-trivial patch to this driver, I'd need assistance.
>
Yes I can. I think you could add also Mika and Andy as all of us are 
actively supporting it.

-- 
Jarkko

Re: [PATCH v2 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Wolfram Sang]

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

On Mon, Oct 12, 2015 at 04:55:35PM +0300, Jarkko Nikula wrote:
> There is some code duplication in i2c-designware-platdrv and
> i2c-designware-pcidrv probe functions. What is even worse that duplication
> requires i2c_dw_xfer(), i2c_dw_func() and i2c_dw_isr() i2c-designware-core
> functions to be exported.
> 
> Therefore move common code into new i2c_dw_probe() and make functions above
> local to i2c-designware-core.
> 
> While merging the code patch does following functional changes:
> 
> - I2C Adapter name will be "Synopsys DesignWare I2C adapter". Previously it
>   was used for platform and ACPI devices but PCI device used
>   "i2c-designware-pci".
> - Using device name for interrupt name. Previous it was platform device name,
>   ACPI device name or "i2c-designware-pci".
> - Error code from devm_request_irq() and i2c_add_numbered_adapter() will be
>   printed in case of error.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

There was a merge conflict with a bugfix from i2c/for-current. I think
it is okay, but you may want to double check my i2c/for-next.

What about this irq-clearing-in-probe thingie on top of this series? :)


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

Re: [PATCH v2 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

Hi

On 10/20/2015 07:32 PM, Wolfram Sang wrote:
> There was a merge conflict with a bugfix from i2c/for-current. I think
> it is okay, but you may want to double check my i2c/for-next.
>
Looks like pm_runtime_disable() got dropped from your 36d48fb5766a 
("i2c: designware-platdrv: enable RuntimePM before registering to the 
core") while handling the merge conflict. I'll send a fix.

> What about this irq-clearing-in-probe thingie on top of this series? :)
>
I'll have a look at it. What's not entirely clear to me would it be 
no-op or not. HW is actually disabled after i2c_dw_init() which is 
called before requesting the interrupt but is not clear to me from the 
spec does HW clear interrupts while it goes idle.

So as a result I'd expect either a explicit interrupt clearing patch (to 
be more robust against potential unmasking changes) or a comment in 
__i2c_dw_enable() :-)

-- 
Jarkko