[PATCH v2] I2C: designware: fix IO timeout issue for AMD controller

2015-12-10 Thread 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 
---
 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_SWAP0x0001
 #define ACCESS_16BIT   0x0002
+#define ACCESS_INTR_MASK   0x0004
 
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Patch V10] i2c: imx: add runtime pm support to improve the performance

2015-12-10 Thread Gao Pan 
In our former i2c driver, i2c clk is enabled and disabled in
xfer function, which contributes to power saving. However,
the clk enable process brings a busy wait delay until the core
is stable. As a result, the performance is sacrificed.

To weigh the power consumption and i2c bus performance, runtime
pm is the good solution for it. The clk is enabled when a i2c
transfer starts, and disabled after a specifically defined delay.

If CONFIG_PM is disabled the net result of this patch is that the
clock is never disabled.

Without the patch the test case (many eeprom reads) executes with approx:
real 1m7.735s
user 0m0.488s
sys 0m20.040s

With the patch the same test case (many eeprom reads) executes with approx:
real 0m54.241s
user 0m0.440s
sys 0m5.920s

Signed-off-by: Fugang Duan 
Signed-off-by: Gao Pan 
Acked-by: Uwe Kleine-König 
---
V2:
As Uwe Kleine-König's suggestion, the version do below changes:
 -call clk_prepare_enable in probe to avoid never enabling clock
  if CONFIG_PM is disabled
 -enable clock before request IRQ in probe
 -remove the pm staff in i2c_imx_isr

V3:
 -pm_runtime_get_sync returns < 0 as error

V4:
 -add pm_runtime_set_active before pm_runtime_enable
 -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend
  in probe
 -add error disposal when i2c_add_numbered_adapter fails

V5:
 -clean up and disable runtime PM when i2c_add_numbered_adapter fails
 -use pm_runtime_get and pm_runtime_put_autosuspend in probe

V6:
 -disable the clock manually and set the state to suspended explicitly with
  pm_runtime_set_suspended

V7:
 -manually disabling the clock and use pm_runtime_put_noidle in the remove 
callback

V8:
 -move out label after the two runtime pm calls in i2c_imx_xfer

V9:
 -add comment in the log what is the result when CONFIG_PM is disabled
 -do the rpm_disable cleanup in reverse order

V10:
 -based on for-next, commit b31f032e9bd738dbaa07c157c354597007dc5ab4

 drivers/i2c/busses/i2c-imx.c | 90 ++--
 1 file changed, 78 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d4d8536..baae9cc 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -55,6 +55,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /** Defines 

 
***/
@@ -120,6 +121,8 @@
 #define I2CR_IEN_OPCODE_0  0x0
 #define I2CR_IEN_OPCODE_1  I2CR_IEN
 
+#define I2C_PM_TIMEOUT 10 /* ms */
+
 /** Variables 
**
 
***/
 
@@ -527,9 +530,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 
i2c_imx_set_clk(i2c_imx);
 
-   result = clk_prepare_enable(i2c_imx->clk);
-   if (result)
-   return result;
imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
/* Enable I2C controller */
imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, 
IMX_I2C_I2SR);
@@ -582,7 +582,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
/* Disable I2C controller */
temp = i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
-   clk_disable_unprepare(i2c_imx->clk);
 }
 
 static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
@@ -901,6 +900,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 
dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
+   result = pm_runtime_get_sync(i2c_imx->adapter.dev.parent);
+   if (result < 0)
+   goto out;
+
/* Start I2C transfer */
result = i2c_imx_start(i2c_imx);
if (result) {
@@ -964,6 +967,10 @@ fail0:
/* Stop I2C transfer */
i2c_imx_stop(i2c_imx);
 
+   pm_runtime_mark_last_busy(i2c_imx->adapter.dev.parent);
+   pm_runtime_put_autosuspend(i2c_imx->adapter.dev.parent);
+
+out:
dev_dbg(&i2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
(result < 0) ? "error" : "success msg",
(result < 0) ? result : num);
@@ -1083,7 +1090,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 
ret = clk_prepare_enable(i2c_imx->clk);
if (ret) {
-   dev_err(&pdev->dev, "can't enable I2C clock\n");
+   dev_err(&pdev->dev, "can't enable I2C clock, ret=%d\n", ret);
return ret;
}
 
@@ -1107,6 +1114,18 @@ static int i2c_imx_probe(struct platform_device *pdev)
/* Set up adapter data */
i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
 
+   /* Set up platform driver data */
+   platform_set_drvdata(pdev, i2c_imx);
+
+   pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
+   pm_runtime_use_autosuspend(&pdev->dev);
+   pm_runt

Re: [PATCH v2 1/2] acpi:apd: Add APM X-Gene ACPI I2C device support

2015-12-10 Thread Ken Xue 
On Thu, 2015-12-10 at 14:19 -0700, Loc Ho wrote:
> Add APM X-Gene ACPI I2C device support by hooks into existent
> ACPI apd driver. To fully enable support, require another
> patch to add the X-Gene ACPI node into the DW I2C driver.
> 
> Signed-off-by: Loc Ho 
> ---
>  drivers/acpi/acpi_apd.c |   16 +++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> index a450e7a..d507cf6 100644
> --- a/drivers/acpi/acpi_apd.c
> +++ b/drivers/acpi/acpi_apd.c
> @@ -51,7 +51,7 @@ struct apd_private_data {
>   const struct apd_device_desc *dev_desc;
>  };
>  
> -#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
> +#if defined(CONFIG_X86_AMD_PLATFORM_DEVICE) || defined(CONFIG_ARM64)
>  #define APD_ADDR(desc)   ((unsigned long)&desc)
>  
>  static int acpi_apd_setup(struct apd_private_data *pdata)
> @@ -71,6 +71,7 @@ static int acpi_apd_setup(struct apd_private_data *pdata)
>   return 0;
>  }
>  
> +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
>  static struct apd_device_desc cz_i2c_desc = {
>   .setup = acpi_apd_setup,
>   .fixed_clk_rate = 13300,
> @@ -80,6 +81,14 @@ static struct apd_device_desc cz_uart_desc = {
>   .setup = acpi_apd_setup,
>   .fixed_clk_rate = 4800,
>  };
> +#endif
> +
> +#ifdef CONFIG_ARM64
> +static struct apd_device_desc xgene_i2c_desc = {
> + .setup = acpi_apd_setup,
> + .fixed_clk_rate = 1,
> +};
> +#endif
>  
>  #else
>  
> @@ -132,9 +141,14 @@ static int acpi_apd_create_device(struct acpi_device 
> *adev,
>  
>  static const struct acpi_device_id acpi_apd_device_ids[] = {
>   /* Generic apd devices */
> +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
>   { "AMD0010", APD_ADDR(cz_i2c_desc) },
>   { "AMD0020", APD_ADDR(cz_uart_desc) },
>   { "AMD0030", },
> +#endif
> +#ifdef CONFIG_ARM64
> + { "APMC0D0F", APD_ADDR(xgene_i2c_desc) },
> +#endif
>   { }
>  };
>  

Reviewed-by: Ken Xue 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] i2c:dw: Add APM X-Gene ACPI I2C device support

2015-12-10 Thread Loc Ho 
Enable APM X-Gene ACPI I2C device support by adding the
corresponding ACPI ID. The platform ACPI APD corresponding
change is required to provide the proper clock frequency input.

Signed-off-by: Loc Ho 
Acked-by: Mika Westerberg 
---
 drivers/i2c/busses/i2c-designware-platdrv.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..423371d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -117,6 +117,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "80860F41", 0 },
{ "808622C1", 0 },
{ "AMD0010", 0 },
+   { "APMC0D0F", 0 },
{ }
 };
 MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] acpi:apd: Add APM X-Gene ACPI I2C device support

2015-12-10 Thread Loc Ho 
Add APM X-Gene ACPI I2C device support by hooks into existent
ACPI apd driver. To fully enable support, require another
patch to add the X-Gene ACPI node into the DW I2C driver.

Signed-off-by: Loc Ho 
---
 drivers/acpi/acpi_apd.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
index a450e7a..d507cf6 100644
--- a/drivers/acpi/acpi_apd.c
+++ b/drivers/acpi/acpi_apd.c
@@ -51,7 +51,7 @@ struct apd_private_data {
const struct apd_device_desc *dev_desc;
 };
 
-#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
+#if defined(CONFIG_X86_AMD_PLATFORM_DEVICE) || defined(CONFIG_ARM64)
 #define APD_ADDR(desc) ((unsigned long)&desc)
 
 static int acpi_apd_setup(struct apd_private_data *pdata)
@@ -71,6 +71,7 @@ static int acpi_apd_setup(struct apd_private_data *pdata)
return 0;
 }
 
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
 static struct apd_device_desc cz_i2c_desc = {
.setup = acpi_apd_setup,
.fixed_clk_rate = 13300,
@@ -80,6 +81,14 @@ static struct apd_device_desc cz_uart_desc = {
.setup = acpi_apd_setup,
.fixed_clk_rate = 4800,
 };
+#endif
+
+#ifdef CONFIG_ARM64
+static struct apd_device_desc xgene_i2c_desc = {
+   .setup = acpi_apd_setup,
+   .fixed_clk_rate = 1,
+};
+#endif
 
 #else
 
@@ -132,9 +141,14 @@ static int acpi_apd_create_device(struct acpi_device *adev,
 
 static const struct acpi_device_id acpi_apd_device_ids[] = {
/* Generic apd devices */
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
{ "AMD0010", APD_ADDR(cz_i2c_desc) },
{ "AMD0020", APD_ADDR(cz_uart_desc) },
{ "AMD0030", },
+#endif
+#ifdef CONFIG_ARM64
+   { "APMC0D0F", APD_ADDR(xgene_i2c_desc) },
+#endif
{ }
 };
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/2] i2c:dw: Add APM X-Gene ACPI I2C device support

2015-12-10 Thread Loc Ho 
Add APM X-Gene ACPI I2C device support. These patches follow
the same implementation as AMD I2C driver - changes in ACPI APD
and Designware I2C drivers.

v2:
* Spilt the acpi_apd change with defines for AMD and X-Gene I2C's

Signed-off-by: Loc Ho 
---
Loc Ho (2):
  acpi:apd: Add APM X-Gene ACPI I2C device support
  i2c:dw: Add APM X-Gene ACPI I2C device support

 drivers/acpi/acpi_apd.c |   16 +++-
 drivers/i2c/busses/i2c-designware-platdrv.c |1 +
 2 files changed, 16 insertions(+), 1 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] ACPI / gpio: Add irq_type when a gpio is used as an interrupt

2015-12-10 Thread Linus Walleij 
On Mon, Nov 30, 2015 at 11:47 PM, Christophe Ricard
 wrote:

> When a gpio is used as an interrupt, the irq_type was not available for
> device driver. It is not align with devicetree probing.
>
> Signed-off-by: Christophe Ricard 

Acked-by: Linus Walleij 

Rafael you can merge this into the ACPI tree.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: allow building emev2 without slave mode again

2015-12-10 Thread Wolfram Sang 
> Alternatively, the  inline could return an error, and both bus
> drivers check for the error before using 'value'.

I'll try to check these options tomorrow.



signature.asc
Description: Digital signature

Re: [PATCH] i2c: allow building emev2 without slave mode again

2015-12-10 Thread Arnd Bergmann 
On Thursday 10 December 2015 22:54:25 kbuild test robot wrote:
> 
>In file included from arch/x86/include/asm/realmode.h:5:0,
> from arch/x86/include/asm/acpi.h:33,
> from arch/x86/include/asm/fixmap.h:19,
> from arch/x86/include/asm/apic.h:12,
> from arch/x86/include/asm/smp.h:12,
> from arch/x86/include/asm/mmzone_64.h:10,
> from arch/x86/include/asm/mmzone.h:4,
> from include/linux/mmzone.h:856,
> from include/linux/gfp.h:5,
> from include/linux/device.h:29,
> from drivers/i2c/busses/i2c-emev2.c:15:
>drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_irq_handler':
> >> arch/x86/include/asm/io.h:53:3: warning: 'value' may be used uninitialized 
> >> in this function [-Wmaybe-uninitialized]
> { asm volatile("mov" size " %0,%1": :reg (val), \
>   ^
>drivers/i2c/busses/i2c-emev2.c:232:13: note: 'value' was declared here
>  u8 status, value;
> ^

The warning was indeed introduced by my change, but I think there
was a preexisting issue:

The slave_cb callback function is supposed to set the 'value'
here, but it might return an error not assign the pointer, which
is something that both the rcar and the emev2 drivers do not handle
correctly.

It might be best to change the callback to return 'void' and not
allow it to fail. At least the eeprom slave cannot fail anyway,
and it is the only implementation we have at the moment.
The inline function would then have to be changed to initialize
the 'value'.

Alternatively, the  inline could return an error, and both bus
drivers check for the error before using 'value'.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: allow building emev2 without slave mode again

2015-12-10 Thread kbuild test robot 
Hi Arnd,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on next-20151210]
[cannot apply to v4.4-rc4]

url:
https://github.com/0day-ci/linux/commits/Arnd-Bergmann/i2c-allow-building-emev2-without-slave-mode-again/20151210-211642
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
config: x86_64-randconfig-r0-12102217 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/realmode.h:5:0,
from arch/x86/include/asm/acpi.h:33,
from arch/x86/include/asm/fixmap.h:19,
from arch/x86/include/asm/apic.h:12,
from arch/x86/include/asm/smp.h:12,
from arch/x86/include/asm/mmzone_64.h:10,
from arch/x86/include/asm/mmzone.h:4,
from include/linux/mmzone.h:856,
from include/linux/gfp.h:5,
from include/linux/device.h:29,
from drivers/i2c/busses/i2c-emev2.c:15:
   drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_irq_handler':
>> arch/x86/include/asm/io.h:53:3: warning: 'value' may be used uninitialized 
>> in this function [-Wmaybe-uninitialized]
{ asm volatile("mov" size " %0,%1": :reg (val), \
  ^
   drivers/i2c/busses/i2c-emev2.c:232:13: note: 'value' was declared here
 u8 status, value;
^

vim +/value +53 arch/x86/include/asm/io.h

b310f381d include/asm-x86/io.h  venkatesh.pallip...@intel.com 2008-03-18  
37  #define ARCH_HAS_IOREMAP_WC
d838270e2 arch/x86/include/asm/io.h Toshi Kani2015-06-04  
38  #define ARCH_HAS_IOREMAP_WT
b310f381d include/asm-x86/io.h  venkatesh.pallip...@intel.com 2008-03-18  
39  
1c5b9069e arch/x86/include/asm/io.h Brian Gerst   2010-02-05  
40  #include 
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
41  #include 
976e8f677 arch/x86/include/asm/io.h Jeremy Fitzhardinge   2009-02-06  
42  #include 
5b7c73e00 arch/x86/include/asm/io.h Mark Salter   2014-04-07  
43  #include 
d6472302f arch/x86/include/asm/io.h Stephen Rothwell  2015-06-02  
44  #include 
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
45  
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
46  #define build_mmio_read(name, size, type, reg, barrier) \
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
47  static inline type name(const volatile void __iomem *addr) \
1c5b0eb66 include/asm-x86/io.h  Mikael Pettersson 2008-08-13  
48  { type ret; asm volatile("mov" size " %1,%0":reg (ret) \
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
49  :"m" (*(volatile type __force *)addr) barrier); return ret; }
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
50  
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
51  #define build_mmio_write(name, size, type, reg, barrier) \
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
52  static inline void name(type val, volatile void __iomem *addr) \
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27 
@53  { asm volatile("mov" size " %0,%1": :reg (val), \
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
54  "m" (*(volatile type __force *)addr) barrier); }
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
55  
1c5b0eb66 include/asm-x86/io.h  Mikael Pettersson 2008-08-13  
56  build_mmio_read(readb, "b", unsigned char, "=q", :"memory")
1c5b0eb66 include/asm-x86/io.h  Mikael Pettersson 2008-08-13  
57  build_mmio_read(readw, "w", unsigned short, "=r", :"memory")
1c5b0eb66 include/asm-x86/io.h  Mikael Pettersson 2008-08-13  
58  build_mmio_read(readl, "l", unsigned int, "=r", :"memory")
c1f64a580 include/asm-x86/io.h  Linus Torvalds2008-05-27  
59  
1c5b0eb66 include/asm-x86/io.h  Mikael Pettersson 2008-08-13  
60  build_mmio_read(__readb, "b", unsigned char, "=q", )
1c5b0eb66 include/asm-x86/io.h  Mikael Pettersson 2008-08-13  
61  build_mmio_read(__readw, "w", unsigned short, "=r", )

:: The code at line 53 was first introduced by commit
:: c1f64a58003fd2efaa725a857e269a15

Re: [PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m

2015-12-10 Thread Jarkko Nikula 

On 12/10/2015 02:59 PM, Andy Shevchenko wrote:

On Thu, 2015-12-10 at 13:48 +0200, Jarkko Nikula wrote:

I believe i2c-designware-baytrail.c doesn't have strict dependency
that
Intel SoC IOSF Sideband support must be always built-in in order to
be
able to compile support for Intel Baytrail I2C bus sharing HW
semaphore.

Redefine build dependencies so that CONFIG_IOSF_MBI=y is required
only
when CONFIG_I2C_DESIGNWARE_PLATFORM is built-in.

Signed-off-by: Jarkko Nikula 
---
Hi David. Can you ack/nak this patch as I'm not fully familiar with
this
HW semaphore can there be problems when IOSF_MBI is built as a
module.




At least I'm getting similar sensible looking "punit semaphore
acquired/held for x ms" debug messages when I modprobe/rmmod
i2c_designware_platform independently is the CONFIG_IOSF_MBI=y or =m.
---
  drivers/i2c/busses/Kconfig | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 69c46fe13777..76f4d024def0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -490,7 +490,9 @@ config I2C_DESIGNWARE_PCI

  config I2C_DESIGNWARE_BAYTRAIL
bool "Intel Baytrail I2C semaphore support"
-   depends on I2C_DESIGNWARE_PLATFORM && IOSF_MBI=y && ACPI
+   depends on ACPI
+   depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
+  (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)


Would it be more readable in the following way

depends on ACPI
depends on I2C_DESIGNWARE_PLATFORM
depends on IOSF_MBI if I2C_DESIGNWARE_PLATFORM=m
depends on IOSF_MBI=y if I2C_DESIGNWARE_PLATFORM=y

For my eyes it looks a bit more complex but I think it's matter of 
taste. However, the syntax you are proposing is not supported for 
"depends on" like it is for "select" statements.


--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: allow building emev2 without slave mode again

2015-12-10 Thread Arnd Bergmann 
On Thursday 10 December 2015 14:34:46 Wolfram Sang wrote:
> On Thu, Dec 10, 2015 at 02:14:49PM +0100, Arnd Bergmann wrote:
> > The emev2 driver stopped compiling in today's linux-next kernel:
> > 
> > drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_slave_irq':
> > drivers/i2c/busses/i2c-emev2.c:233:23: error: storage size of 'event' isn't 
> > known
> > drivers/i2c/busses/i2c-emev2.c:250:3: error: implicit declaration of 
> > function 'i2c_slave_event' [-Werror=implicit-function-declaration]
> > drivers/i2c/busses/i2c-emev2.c:250:32: error: 'I2C_SLAVE_STOP' undeclared 
> > (first use in this function)
> > 
> > It works again if we enable CONFIG_I2C_SLAVE, but it seems wrong
> > to add a dependency on that symbol:
> > 
> > * The symbol is user-selectable, but only one or two (including this
> >   one) bus drivers actually implement it, and it makes no sense
> >   if you don't have one of them.
> > 
> > * The other driver (R-Car) uses 'select I2C_SLAVE', which seems
> >   reasonable in principle, but we should not do that on user
> >   visible symbols.
> > 
> > * I2C slave mode could be implemented in a lot of other drivers
> >   as an optional feature, but we shouldn't require enabling it
> >   if we don't use it.
> > 
> > This changes the two drivers that provide I2C slave mode so they
> > can again build if the slave mode being disabled. To do this, I
> > move the definition of i2c_slave_event() and enum i2c_slave_event
> > out of the #ifdef and instead make the assignment of the reg_slave
> > and unreg_slave pointers optional in the bus drivers. The functions
> > implementing the feature are unused in that case, so they get
> > marked as __maybe_unused in order to still give compile-time
> > coverage.
> 
> Thanks a lot! Making this clear and consistent was on my todo-list,
> unfortunately below some other items.
> 
> Both drivers have quite orthogonal slave_irq routines. What do you think
> about grouping this and the reg/unreg-calls together and compile them
> conditionally on I2C_SLAVE? I think the code savings are worth it.

Ah, I had missed that part, good point. The change below should take
care of leaving out the extra code here. Do you want to just fold that
in?

Arnd

diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c
index 75d6095c5fe1..b01c9f30c545 100644
--- a/drivers/i2c/busses/i2c-emev2.c
+++ b/drivers/i2c/busses/i2c-emev2.c
@@ -303,7 +303,7 @@ static irqreturn_t em_i2c_irq_handler(int this_irq, void 
*dev_id)
 {
struct em_i2c_device *priv = dev_id;
 
-   if (em_i2c_slave_irq(priv))
+   if (IS_ENABLED(CONFIG_I2C_SLAVE) && em_i2c_slave_irq(priv))
return IRQ_HANDLED;
 
complete(&priv->msg_done);
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index e67824adeba0..a1ea66d6267e 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -428,7 +428,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
/* Only handle interrupts that are currently enabled */
msr &= rcar_i2c_read(priv, ICMIER);
if (!msr) {
-   if (rcar_i2c_slave_irq(priv))
+   if (IS_ENABLED(CONFIG_I2C_SLAVE) && rcar_i2c_slave_irq(priv))
return IRQ_HANDLED;
 
return IRQ_NONE;

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: allow building emev2 without slave mode again

2015-12-10 Thread Wolfram Sang 
On Thu, Dec 10, 2015 at 02:14:49PM +0100, Arnd Bergmann wrote:
> The emev2 driver stopped compiling in today's linux-next kernel:
> 
> drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_slave_irq':
> drivers/i2c/busses/i2c-emev2.c:233:23: error: storage size of 'event' isn't 
> known
> drivers/i2c/busses/i2c-emev2.c:250:3: error: implicit declaration of function 
> 'i2c_slave_event' [-Werror=implicit-function-declaration]
> drivers/i2c/busses/i2c-emev2.c:250:32: error: 'I2C_SLAVE_STOP' undeclared 
> (first use in this function)
> 
> It works again if we enable CONFIG_I2C_SLAVE, but it seems wrong
> to add a dependency on that symbol:
> 
> * The symbol is user-selectable, but only one or two (including this
>   one) bus drivers actually implement it, and it makes no sense
>   if you don't have one of them.
> 
> * The other driver (R-Car) uses 'select I2C_SLAVE', which seems
>   reasonable in principle, but we should not do that on user
>   visible symbols.
> 
> * I2C slave mode could be implemented in a lot of other drivers
>   as an optional feature, but we shouldn't require enabling it
>   if we don't use it.
> 
> This changes the two drivers that provide I2C slave mode so they
> can again build if the slave mode being disabled. To do this, I
> move the definition of i2c_slave_event() and enum i2c_slave_event
> out of the #ifdef and instead make the assignment of the reg_slave
> and unreg_slave pointers optional in the bus drivers. The functions
> implementing the feature are unused in that case, so they get
> marked as __maybe_unused in order to still give compile-time
> coverage.

Thanks a lot! Making this clear and consistent was on my todo-list,
unfortunately below some other items.

Both drivers have quite orthogonal slave_irq routines. What do you think
about grouping this and the reg/unreg-calls together and compile them
conditionally on I2C_SLAVE? I think the code savings are worth it.



signature.asc
Description: Digital signature

[PATCH] i2c: allow building emev2 without slave mode again

2015-12-10 Thread Arnd Bergmann 
The emev2 driver stopped compiling in today's linux-next kernel:

drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_slave_irq':
drivers/i2c/busses/i2c-emev2.c:233:23: error: storage size of 'event' isn't 
known
drivers/i2c/busses/i2c-emev2.c:250:3: error: implicit declaration of function 
'i2c_slave_event' [-Werror=implicit-function-declaration]
drivers/i2c/busses/i2c-emev2.c:250:32: error: 'I2C_SLAVE_STOP' undeclared 
(first use in this function)

It works again if we enable CONFIG_I2C_SLAVE, but it seems wrong
to add a dependency on that symbol:

* The symbol is user-selectable, but only one or two (including this
  one) bus drivers actually implement it, and it makes no sense
  if you don't have one of them.

* The other driver (R-Car) uses 'select I2C_SLAVE', which seems
  reasonable in principle, but we should not do that on user
  visible symbols.

* I2C slave mode could be implemented in a lot of other drivers
  as an optional feature, but we shouldn't require enabling it
  if we don't use it.

This changes the two drivers that provide I2C slave mode so they
can again build if the slave mode being disabled. To do this, I
move the definition of i2c_slave_event() and enum i2c_slave_event
out of the #ifdef and instead make the assignment of the reg_slave
and unreg_slave pointers optional in the bus drivers. The functions
implementing the feature are unused in that case, so they get
marked as __maybe_unused in order to still give compile-time
coverage.

Signed-off-by: Arnd Bergmann 
Fixes: c31d0a00021d ("i2c: emev2: add slave support")

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 69c46fe13777..1c8d53f34dd3 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -985,7 +985,6 @@ config I2C_XLP9XX
 config I2C_RCAR
tristate "Renesas R-Car I2C Controller"
depends on ARCH_SHMOBILE || COMPILE_TEST
-   select I2C_SLAVE
help
  If you say yes to this option, support will be included for the
  R-Car I2C controller.
diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c
index 96bb4e749012..75d6095c5fe1 100644
--- a/drivers/i2c/busses/i2c-emev2.c
+++ b/drivers/i2c/busses/i2c-emev2.c
@@ -316,7 +316,7 @@ static u32 em_i2c_func(struct i2c_adapter *adap)
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SLAVE;
 }
 
-static int em_i2c_reg_slave(struct i2c_client *slave)
+static int __maybe_unused em_i2c_reg_slave(struct i2c_client *slave)
 {
struct em_i2c_device *priv = i2c_get_adapdata(slave->adapter);
 
@@ -334,7 +334,7 @@ static int em_i2c_reg_slave(struct i2c_client *slave)
return 0;
 }
 
-static int em_i2c_unreg_slave(struct i2c_client *slave)
+static int __maybe_unused em_i2c_unreg_slave(struct i2c_client *slave)
 {
struct em_i2c_device *priv = i2c_get_adapdata(slave->adapter);
 
@@ -350,8 +350,10 @@ static int em_i2c_unreg_slave(struct i2c_client *slave)
 static struct i2c_algorithm em_i2c_algo = {
.master_xfer = em_i2c_xfer,
.functionality = em_i2c_func,
+#ifdef CONFIG_I2C_SLAVE
.reg_slave  = em_i2c_reg_slave,
.unreg_slave= em_i2c_unreg_slave,
+#endif
 };
 
 static int em_i2c_probe(struct platform_device *pdev)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 3ed1f0aa5eeb..e67824adeba0 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -521,7 +521,7 @@ out:
return ret;
 }
 
-static int rcar_reg_slave(struct i2c_client *slave)
+static int __maybe_unused rcar_reg_slave(struct i2c_client *slave)
 {
struct rcar_i2c_priv *priv = i2c_get_adapdata(slave->adapter);
 
@@ -542,7 +542,7 @@ static int rcar_reg_slave(struct i2c_client *slave)
return 0;
 }
 
-static int rcar_unreg_slave(struct i2c_client *slave)
+static int __maybe_unused rcar_unreg_slave(struct i2c_client *slave)
 {
struct rcar_i2c_priv *priv = i2c_get_adapdata(slave->adapter);
 
@@ -568,8 +568,10 @@ static u32 rcar_i2c_func(struct i2c_adapter *adap)
 static const struct i2c_algorithm rcar_i2c_algo = {
.master_xfer= rcar_i2c_master_xfer,
.functionality  = rcar_i2c_func,
+#ifdef CONFIG_I2C_SLAVE
.reg_slave  = rcar_reg_slave,
.unreg_slave= rcar_unreg_slave,
+#endif
 };
 
 static const struct of_device_id rcar_i2c_dt_ids[] = {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 51028f351d13..69871e5ee44a 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -254,7 +254,6 @@ static inline void i2c_set_clientdata(struct i2c_client 
*dev, void *data)
 
 /* I2C slave support */
 
-#if IS_ENABLED(CONFIG_I2C_SLAVE)
 enum i2c_slave_event {
I2C_SLAVE_READ_REQUESTED,
I2C_SLAVE_WRITE_REQUESTED,
@@ -269,9 +268,12 @@ extern int i2c_slave_unregister(struct i2c_client *client);
 static inline int i2c_slave_event(struct i2c_client *client,
  enum i2c_slave_event event, u8 *val)
 {
+#if

Re: [PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m

2015-12-10 Thread Andy Shevchenko 
On Thu, 2015-12-10 at 13:48 +0200, Jarkko Nikula wrote:
> I believe i2c-designware-baytrail.c doesn't have strict dependency
> that
> Intel SoC IOSF Sideband support must be always built-in in order to
> be
> able to compile support for Intel Baytrail I2C bus sharing HW
> semaphore.
> 
> Redefine build dependencies so that CONFIG_IOSF_MBI=y is required
> only
> when CONFIG_I2C_DESIGNWARE_PLATFORM is built-in.
> 
> Signed-off-by: Jarkko Nikula 
> ---
> Hi David. Can you ack/nak this patch as I'm not fully familiar with
> this
> HW semaphore can there be problems when IOSF_MBI is built as a
> module.


> At least I'm getting similar sensible looking "punit semaphore
> acquired/held for x ms" debug messages when I modprobe/rmmod
> i2c_designware_platform independently is the CONFIG_IOSF_MBI=y or =m.
> ---
>  drivers/i2c/busses/Kconfig | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 69c46fe13777..76f4d024def0 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -490,7 +490,9 @@ config I2C_DESIGNWARE_PCI
>  
>  config I2C_DESIGNWARE_BAYTRAIL
>   bool "Intel Baytrail I2C semaphore support"
> - depends on I2C_DESIGNWARE_PLATFORM && IOSF_MBI=y && ACPI
> + depends on ACPI
> + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
> +    (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)

Would it be more readable in the following way

depends on ACPI
depends on I2C_DESIGNWARE_PLATFORM
depends on IOSF_MBI if I2C_DESIGNWARE_PLATFORM=m
depends on IOSF_MBI=y if I2C_DESIGNWARE_PLATFORM=y

>   help
>     This driver enables managed host access to the PMIC I2C
> bus on select
>     Intel BayTrail platforms using the X-Powers AXP288 PMIC.
> It allows

-- 
Andy Shevchenko 
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] i2c: designware: Keep pm_runtime_enable/_disable calls in sync

2015-12-10 Thread Jarkko Nikula 
On an hardware shared I2C bus (certain Intel Baytrail SoC platforms) the
runtime PM disable depth keeps increasing over repeated modprobe/rmmod
cycle because pm_runtime_disable() is called without checking should it
be disabled already because of bus sharing.

This hasn't made any other harm than dev->power.disable_depth keeps
increasing but keep it sync by calling pm_runtime_disable() only when
runtime PM is not disabled.

Signed-off-by: Jarkko Nikula 
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579ecb5a4..1308666b054b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -240,12 +240,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
}
 
r = i2c_dw_probe(dev);
-   if (r) {
+   if (r && !dev->pm_runtime_disabled)
pm_runtime_disable(&pdev->dev);
-   return r;
-   }
 
-   return 0;
+   return r;
 }
 
 static int dw_i2c_plat_remove(struct platform_device *pdev)
@@ -260,7 +258,8 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
 
pm_runtime_dont_use_autosuspend(&pdev->dev);
pm_runtime_put_sync(&pdev->dev);
-   pm_runtime_disable(&pdev->dev);
+   if (!dev->pm_runtime_disabled)
+   pm_runtime_disable(&pdev->dev);
 
return 0;
 }
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m

2015-12-10 Thread Jarkko Nikula 
I believe i2c-designware-baytrail.c doesn't have strict dependency that
Intel SoC IOSF Sideband support must be always built-in in order to be
able to compile support for Intel Baytrail I2C bus sharing HW semaphore.

Redefine build dependencies so that CONFIG_IOSF_MBI=y is required only
when CONFIG_I2C_DESIGNWARE_PLATFORM is built-in.

Signed-off-by: Jarkko Nikula 
---
Hi David. Can you ack/nak this patch as I'm not fully familiar with this
HW semaphore can there be problems when IOSF_MBI is built as a module.
At least I'm getting similar sensible looking "punit semaphore
acquired/held for x ms" debug messages when I modprobe/rmmod
i2c_designware_platform independently is the CONFIG_IOSF_MBI=y or =m.
---
 drivers/i2c/busses/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 69c46fe13777..76f4d024def0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -490,7 +490,9 @@ config I2C_DESIGNWARE_PCI
 
 config I2C_DESIGNWARE_BAYTRAIL
bool "Intel Baytrail I2C semaphore support"
-   depends on I2C_DESIGNWARE_PLATFORM && IOSF_MBI=y && ACPI
+   depends on ACPI
+   depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
+  (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
help
  This driver enables managed host access to the PMIC I2C bus on select
  Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: rk3x: Add two new features for rk3399

2015-12-10 Thread Heiko Stübner 
Hi David,

Am Mittwoch, 9. Dezember 2015, 17:11:46 schrieb David Wu:
> 1. support highspeed.
> 2. check i2c bus idle status.

Listing two separate changes in one patch is a big indicator that it should be 
split up into two patches. Also please be more verbose (aka explain more) what 
patches do - and especially why it's needed.


>From what I've seen below, my personal favorite would probably be:

patch1: introduce ops struct and move the current calc_divs to it
patch2: introduce v1, highspeed with that new calc_divs
patch3: introduce the idle status-check


> Change-Id: I9c22e752af621c0f8dbcbd399c86b34fd810ec38

no change-ids etc from external revision control systems please


> Signed-off-by: David Wu 
> ---
>  drivers/i2c/busses/i2c-rk3x.c | 336
> -- 1 file changed, 320
> insertions(+), 16 deletions(-)
>  mode change 100644 => 100755 drivers/i2c/busses/i2c-rk3x.c
> 
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> old mode 100644
> new mode 100755
> index c1935eb..b1aa702
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
> 
>  /* Register Map */
> @@ -37,6 +38,7 @@
>  #define REG_IEN0x18 /* interrupt enable */
>  #define REG_IPD0x1c /* interrupt pending */
>  #define REG_FCNT   0x20 /* finished count */
> +#define I2C_ST 0x220 /* i2c pin status */

while the registers are called I2C_* in the TRMs, please keep with the current 
notation in the driver ... so REG_ST here


> 
>  /* Data buffer offsets */
>  #define TXBUFFER_BASE 0x100
> @@ -58,6 +60,12 @@ enum {
>  #define REG_CON_LASTACK   BIT(5) /* 1: send NACK after last received byte
> */ #define REG_CON_ACTACKBIT(6) /* 1: stop if NACK is received */
> 
> +#define VERSION_MASK   0x
> +#define VERSION_SHIFT  16
> +
> +#define RK3X_I2C_V00x0
> +#define RK3X_I2C_V10x1
> +
>  /* REG_MRXADDR bits */
>  #define REG_MRXADDR_VALID(x) BIT(24 + (x)) /* [x*8+7:x*8] of MRX[R]ADDR
> valid */
> 
> @@ -71,6 +79,13 @@ enum {
>  #define REG_INT_NAKRCVBIT(6) /* NACK received */
>  #define REG_INT_ALL   0x7f
> 
> +enum {
> + I2C_IDLE = 0,
> + I2C_SDA_LOW,
> + I2C_SCL_LOW,
> + BOTH_LOW,
> +};
> +
>  /* Constants */
>  #define WAIT_TIMEOUT  1000 /* ms */
>  #define DEFAULT_SCL_RATE  (100 * 1000) /* Hz */
> @@ -90,10 +105,23 @@ struct rk3x_i2c_soc_data {
>   int grf_offset;
>  };
> 
> +struct rk3x_i2c_ops {
> + int (*check_idle)(void __iomem *);
> + int (*calc_divs)(unsigned long,
> +  unsigned long,
> +  unsigned long,
> +  unsigned long,
> +  unsigned long,
> +  unsigned long *,
> +  unsigned long *,
> +  unsigned int *);
> +};
> +
>  struct rk3x_i2c {
>   struct i2c_adapter adap;
>   struct device *dev;
>   struct rk3x_i2c_soc_data *soc_data;
> + struct rk3x_i2c_ops ops;
> 
>   /* Hardware resources */
>   void __iomem *regs;
> @@ -116,6 +144,7 @@ struct rk3x_i2c {
>   u8 addr;
>   unsigned int mode;
>   bool is_last_msg;
> + unsigned int time_con;
> 
>   /* I2C state machine */
>   enum rk3x_i2c_state state;
> @@ -151,7 +180,8 @@ static void rk3x_i2c_start(struct rk3x_i2c *i2c)
>   i2c_writel(i2c, REG_INT_START, REG_IEN);
> 
>   /* enable adapter with correct mode, send START condition */
> - val = REG_CON_EN | REG_CON_MOD(i2c->mode) | REG_CON_START;
> + val = i2c->time_con | REG_CON_EN | REG_CON_MOD(i2c->mode)
> + | REG_CON_START;
> 
>   /* if we want to react to NACK, set ACTACK bit */
>   if (!(i2c->msg->flags & I2C_M_IGNORE_NAK))
> @@ -443,16 +473,19 @@ out:
>   * @sda_fall_ns: How many ns it takes for SDA to fall.
>   * @div_low: Divider output for low
>   * @div_high: Divider output for high
> + * @con: version0 is not used
>   *
>   * Returns: 0 on success, -EINVAL if the goal SCL rate is too slow. In that
> case * a best-effort divider value is returned in divs. If the target rate
> is * too high, we silently use the highest possible rate.
>   */
> -static int rk3x_i2c_calc_divs(unsigned long clk_rate, unsigned long
> scl_rate, - unsigned long scl_rise_ns,
> -   unsigned long scl_fall_ns,
> -   unsigned long sda_fall_ns,
> -   unsigned long *div_low, unsigned long *div_high)
> +static int rk3x_i2c_v0_calc_divs(unsigned long clk_rate, unsigned long
> scl_rate, +unsigned long scl_rise_ns,
> +  unsigned long scl_fall_ns,
> +  unsigned long sda_fall_ns,
> +  unsigned long *div_low,
> +  unsigned long *div_high,
> +  unsigned