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

2016-01-05 Thread David E. Box 
Hi

Sorry I missed this discussion. I believe the following code in
i2c_dw_eval_lock_support() should make it so that it doesn't matter how
IOSF_MBI is built:

   if (!iosf_mbi_available())
   return -EPROBE_DEFER;

I added this to address i2c_designware probing before iosf_mbi. It worked but
I do not recall if IOSF_MBI=m was the problem scenario. If so you can just
change it to:

   depends in I2C_DESIGNWARE_PLATFORM && IOSF_MBI

Give me a few days to confirm on my Baytrail device. 

David

On Thu, Dec 10, 2015 at 01:48:44PM +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)
>   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: linux-next: Tree for Feb 17 (i2c-designware-baytrail.c)

2015-02-17 Thread David E. Box 
Hi Wolfram,

On Tue, Feb 17, 2015 at 08:11:58PM +0100, Wolfram Sang wrote:
 Randy,
 
 thanks for the report!
 
  I suppose someone could make that:
  
  depends on I2C_DESIGNWARE_PLATFORM  IOSF_MBI=y  ACPI
  
  or even make I2C_DESIGNWARE_BAYTRAIL be a tristate symbol.
 
 I'll wait a day for input from David what he prefers, because I don't
 own/know this platform. If there is no response, I'll apply the first
 solution.
 

This is fine with me. I own IOSF_MBI as well. Thanks.

Dave


--
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 v1 4/5] i2c-designware-baytrail: cross-check lock functions

2015-02-11 Thread David E. Box 
On Tue, Feb 10, 2015 at 07:06:09PM +0200, Andy Shevchenko wrote:
 It seems the idea behind the cross-check is to prevent acquire semaphore when
 there is no release callback and vice versa. Thus, patch fixes a typo.
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com

Acked-by: David E. Box david.e@linux.intel.com

 ---
  drivers/i2c/busses/i2c-designware-baytrail.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c 
 b/drivers/i2c/busses/i2c-designware-baytrail.c
 index d334744..036d9bdc0 100644
 --- a/drivers/i2c/busses/i2c-designware-baytrail.c
 +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
 @@ -71,7 +71,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
   if (!dev || !dev-dev)
   return -ENODEV;
  
 - if (!dev-acquire_lock)
 + if (!dev-release_lock)
   return 0;
  
   /* host driver writes to side band semaphore register */
 -- 
 2.1.4
 
--
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 v1 1/5] i2c-designware-baytrail: describe magic numbers

2015-02-11 Thread David E. Box 
On Tue, Feb 10, 2015 at 07:06:06PM +0200, Andy Shevchenko wrote:
 The patch converts hardcoded numerical constants to a named ones.
 
 While here, align the variable name in get_sem() and reset_semaphore().
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com

Acked-by: David E. Box david.e@linux.intel.com
--
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 v1 3/5] i2c-designware-baytrail: fix sparse warnings

2015-02-11 Thread David E. Box 
On Tue, Feb 10, 2015 at 07:06:08PM +0200, Andy Shevchenko wrote:
 There is no need to export functions that are used as the callbacks in the
 struct dw_i2c_dev. Otherwise we get the following warnings:
 
 drivers/i2c/busses/i2c-designware-baytrail.c:63:5: warning: symbol 
 'baytrail_i2c_acquire' was not declared. Should it be static?
 drivers/i2c/busses/i2c-designware-baytrail.c:114:6: warning: symbol 
 'baytrail_i2c_release' was not declared. Should it be static?
 
 While here, do few indentation fixes, remove i2c_dw_eval_lock_support() from
 functions exported to the modules and redundant assignment of local sem
 variable.
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com

Acked-by: David E. Box david.e@linux.intel.com

Yep. These remained from original versions of the patch that did use them as
callbacks in struct dw_i2c_dev. Thanks.

--
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 v1 2/5] i2c-designware-baytrail: fix typo in error path

2015-02-11 Thread David E. Box 
On Tue, Feb 10, 2015 at 07:06:07PM +0200, Andy Shevchenko wrote:
 It seems we have same message for different return values in get_sem() and
 baytrail_i2c_acquire(). I suspect this is just a typo, so this patch fixes it.
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com

Acked-by: David E. Box david.e@linux.intel.com

Thanks for spotting this. Timeouts are infrequent and I didn't notice the
error here.

Dave

 ---
  drivers/i2c/busses/i2c-designware-baytrail.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c 
 b/drivers/i2c/busses/i2c-designware-baytrail.c
 index e9cb355..9b67655 100644
 --- a/drivers/i2c/busses/i2c-designware-baytrail.c
 +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
 @@ -99,8 +99,8 @@ int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
   reset_semaphore(dev-dev);
  
   ret = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
 - PUNIT_SEMAPHORE, sem);
 - if (!ret)
 + PUNIT_SEMAPHORE, sem);
 + if (ret)
   dev_err(dev-dev, iosf failed to read punit semaphore\n);
   else
   dev_err(dev-dev, PUNIT SEM: %d\n, sem);
 -- 
 2.1.4
 
--
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 v1 5/5] i2c-designware-baytrail: baytrail_i2c_acquire() might sleep

2015-02-11 Thread David E. Box 
On Tue, Feb 10, 2015 at 07:06:10PM +0200, Andy Shevchenko wrote:
 This patch marks baytrail_i2c_acquire() that it might sleep. Also it chages
 while-loop to do-while and, though it is matter of taste, gives a chance to
 check one more time before report a timeout.
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com

Acked-by: David E. Box david.e@linux.intel.com

 ---
  drivers/i2c/busses/i2c-designware-baytrail.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c 
 b/drivers/i2c/busses/i2c-designware-baytrail.c
 index 036d9bdc0..7d7ae97 100644
 --- a/drivers/i2c/busses/i2c-designware-baytrail.c
 +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
 @@ -68,6 +68,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
   int ret;
   unsigned long start, end;
  
 + might_sleep();
 +
   if (!dev || !dev-dev)
   return -ENODEV;
  
 @@ -85,7 +87,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
   /* host driver waits for bit 0 to be set in semaphore register */
   start = jiffies;
   end = start + msecs_to_jiffies(SEMAPHORE_TIMEOUT);
 - while (!time_after(jiffies, end)) {
 + do {
   ret = get_sem(dev-dev, sem);
   if (!ret  sem) {
   acquired = jiffies;
 @@ -95,7 +97,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
   }
  
   usleep_range(1000, 2000);
 - }
 + } while (time_before(jiffies, end));
  
   dev_err(dev-dev, punit semaphore timed out, resetting\n);
   reset_semaphore(dev-dev);
 -- 
 2.1.4
 
--
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 v1 2/5] i2c-designware-baytrail: fix typo in error path

2015-02-11 Thread David E. Box 
On Wed, Feb 11, 2015 at 06:59:51PM +0200, Andy Shevchenko wrote:
 On Wed, 2015-02-11 at 17:46 +0100, Wolfram Sang wrote:
   + PUNIT_SEMAPHORE, sem);
   + if (ret)
 dev_err(dev-dev, iosf failed to read punit semaphore\n);
 else
 dev_err(dev-dev, PUNIT SEM: %d\n, sem);
  
  Shouldn't the latter be a dev_dbg?
 
 For me it seems not. Here is error patch and we have already in error
 recovery, so, intention to see if the semaphore becomes alive after
 reset. Am I right, David?

Yes. We've timed out by this section of code so we want to verify the
semaphore was reset. Failure to read the semaphore though is a separate
error as well.

Dave

 
 
 -- 
 Andy Shevchenko andriy.shevche...@intel.com
 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

Re: [PATCH] i2c: designware-baytrail: use proper Kconfig dependencies

2015-01-26 Thread David E. Box 
On Mon, Jan 26, 2015 at 05:49:34PM +0100, Wolfram Sang wrote:
 IOSF_MBI depends on PCI, so we should not select it but depend on it.
 This ensures also we compile on X86 only, other archs will break because
 of an arch specific include. Also depend on ACPI since this driver uses
 it.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de

Acked-by: David E. Box david.e@linux.intel.com

 ---
  drivers/i2c/busses/Kconfig | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
 index 83062b2c0e6b..442408d2e7b7 100644
 --- a/drivers/i2c/busses/Kconfig
 +++ b/drivers/i2c/busses/Kconfig
 @@ -467,8 +467,7 @@ config I2C_DESIGNWARE_PCI
  
  config I2C_DESIGNWARE_BAYTRAIL
   bool Intel Baytrail I2C semaphore support
 - depends on I2C_DESIGNWARE_PLATFORM
 - select IOSF_MBI
 + depends on I2C_DESIGNWARE_PLATFORM  IOSF_MBI  ACPI
   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.1.3
 
--
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: designware-baytrail: use proper Kconfig dependencies

2015-01-26 Thread David E. Box 
On Mon, Jan 26, 2015 at 05:49:34PM +0100, Wolfram Sang wrote:
 IOSF_MBI depends on PCI, so we should not select it but depend on it.
 This ensures also we compile on X86 only, other archs will break because
 of an arch specific include. Also depend on ACPI since this driver uses
 it.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de

Signed-off-by: David E. Box david.e@linux.intel.com

 ---
  drivers/i2c/busses/Kconfig | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
 
 diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
 index 83062b2c0e6b..442408d2e7b7 100644
 --- a/drivers/i2c/busses/Kconfig
 +++ b/drivers/i2c/busses/Kconfig
 @@ -467,8 +467,7 @@ config I2C_DESIGNWARE_PCI
  
  config I2C_DESIGNWARE_BAYTRAIL
   bool Intel Baytrail I2C semaphore support
 - depends on I2C_DESIGNWARE_PLATFORM
 - select IOSF_MBI
 + depends on I2C_DESIGNWARE_PLATFORM  IOSF_MBI  ACPI
   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
 -- 

Dave
--
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 V4 2/2] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2015-01-22 Thread David E. Box 
Hi Mika,

On Thu, Jan 22, 2015 at 04:28:19PM +0200, Mika Westerberg wrote:
 On Thu, Jan 15, 2015 at 01:12:17AM -0800, David E. Box wrote:
  This patch implements an I2C bus sharing mechanism between the host and 
  platform
  hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 
  PMIC.
  
  On these platforms access to the PMIC must be shared with platform 
  hardware. The
  hardware unit assumes full control of the I2C bus and the host must request
  access through a special semaphore. Hardware control of the bus also makes 
  it
  necessary to disable runtime pm to avoid interfering with hardware 
  transactions.
  
  Signed-off-by: David E. Box david.e@linux.intel.com
 
 Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com
 
 One comment, though:
 
  ---
   drivers/i2c/busses/Kconfig   |  11 ++
   drivers/i2c/busses/Makefile  |   1 +
   drivers/i2c/busses/i2c-designware-baytrail.c | 160 
  +++
   drivers/i2c/busses/i2c-designware-core.h |   6 +
   drivers/i2c/busses/i2c-designware-platdrv.c  |  20 +++-
   5 files changed, 193 insertions(+), 5 deletions(-)
   create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c
  
  diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
  index 917c358..9a83c46 100644
  --- a/drivers/i2c/busses/Kconfig
  +++ b/drivers/i2c/busses/Kconfig
  @@ -464,6 +464,17 @@ config I2C_DESIGNWARE_PCI
This driver can also be built as a module.  If so, the module
will be called i2c-designware-pci.
   
  +config I2C_DESIGNWARE_BAYTRAIL
  +   bool Intel Baytrail I2C semaphore support
 
 It would be nice if it was possible to compile this as a module.

In the makefile the driver is now part of the composite object for
I2C_DESIGNWARE_PLATFORM. So if selected it will compile as the platform driver
is compiled. This makes the most sense since the linking doesn't work right if
both drivers aren't built the same. This is really an extension to the platform
driver anyway and not standalone code.

Dave
--
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 V4 0/2] i2c-designware: Add Intel Baytrail pmic i2c bus support

2015-01-15 Thread David E. Box 
V4: - Added bus lock to i2c_dw_init() as suggested by Jarrko Nikula
  jarkko.nik...@linux.intel.com. Addresses infrequent hang that was
  seen occuring during probe.
- Added label in i2c_dw_xfer() to bypass unnecessary lock release when
  acquire fails. Suggested by Mika.
- Changed make to build driver as composite addition to
  i2c-designware-platform. Driver can be module or built-in following
  the platform driver selection. Tested as both.
- Added EPROBE_DEFER return to baytrail definition of i2c_dw_eval_lock()
  to defer probe if iosf_mbi driver (required for acquring bus lock) is
  not available.

V3: - Split lock support and driver into separate patches
- Change module build to bool. Platforms running without this driver
  cannot perform critical functions such as charging. Furthermore 
attempts
  by other drivers to access the i2c bus without a lock will hang the
  platform.
- Replaced has_hw_lock flag with acquire/release function pointers.
- Replaced acquire/release ifdef code with single
  i2c_dw_eval_lock_support() test for cleaner (if still undesirable)
  compile time scalability. Future Intel platforms will however continue
  to use the Baytrail driver.

V2: - Moved semaphore detection out of dw platform driver
- Replaced function pointers with defined acquire/release functions in
  dw core. This helps eliminate the ifdefery in the dw platform driver.
- Use new has_hw_lock flag to check if the lock exists on a given bus.
- Use new pm_runtime_disabled flag to conditionally turnoff runtime pm
  in the dw platform driver.

David E. Box (2):
  i2c-designware: Add i2c bus locking support
  i2c-designware: Add Intel Baytrail PMIC I2C bus support

 drivers/i2c/busses/Kconfig   |  11 ++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-designware-baytrail.c | 160 +++
 drivers/i2c/busses/i2c-designware-core.c |  26 +
 drivers/i2c/busses/i2c-designware-core.h |  12 ++
 drivers/i2c/busses/i2c-designware-platdrv.c  |  20 +++-
 6 files changed, 225 insertions(+), 5 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c

-- 
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 V4 2/2] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2015-01-15 Thread David E. Box 
This patch implements an I2C bus sharing mechanism between the host and platform
hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 PMIC.

On these platforms access to the PMIC must be shared with platform hardware. The
hardware unit assumes full control of the I2C bus and the host must request
access through a special semaphore. Hardware control of the bus also makes it
necessary to disable runtime pm to avoid interfering with hardware transactions.

Signed-off-by: David E. Box david.e@linux.intel.com
---
 drivers/i2c/busses/Kconfig   |  11 ++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-designware-baytrail.c | 160 +++
 drivers/i2c/busses/i2c-designware-core.h |   6 +
 drivers/i2c/busses/i2c-designware-platdrv.c  |  20 +++-
 5 files changed, 193 insertions(+), 5 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 917c358..9a83c46 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -464,6 +464,17 @@ config I2C_DESIGNWARE_PCI
  This driver can also be built as a module.  If so, the module
  will be called i2c-designware-pci.
 
+config I2C_DESIGNWARE_BAYTRAIL
+   bool Intel Baytrail I2C semaphore support
+   depends on I2C_DESIGNWARE_PLATFORM
+   select IOSF_MBI
+   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
+ the host to request uninterrupted access to the PMIC's I2C bus from
+ the platform firmware controlling it. You should say Y if running on
+ a BayTrail system using the AXP288.
+
 config I2C_EFM32
tristate EFM32 I2C controller
depends on ARCH_EFM32 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 78d56c5..15b2965 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
 obj-$(CONFIG_I2C_DESIGNWARE_CORE)  += i2c-designware-core.o
 obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM)  += i2c-designware-platform.o
 i2c-designware-platform-objs := i2c-designware-platdrv.o
+i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += 
i2c-designware-baytrail.o
 obj-$(CONFIG_I2C_DESIGNWARE_PCI)   += i2c-designware-pci.o
 i2c-designware-pci-objs := i2c-designware-pcidrv.o
 obj-$(CONFIG_I2C_EFM32)+= i2c-efm32.o
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c 
b/drivers/i2c/busses/i2c-designware-baytrail.c
new file mode 100644
index 000..5f1ff4c
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -0,0 +1,160 @@
+/*
+ * Intel BayTrail PMIC I2C bus semaphore implementaion
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#include linux/module.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/acpi.h
+#include linux/i2c.h
+#include linux/interrupt.h
+#include asm/iosf_mbi.h
+#include i2c-designware-core.h
+
+#define SEMAPHORE_TIMEOUT  100
+#define PUNIT_SEMAPHORE0x7
+
+static unsigned long acquired;
+
+static int get_sem(struct device *dev, u32 *sem)
+{
+   u32 reg_val;
+   int ret;
+
+   ret = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ, PUNIT_SEMAPHORE,
+   reg_val);
+   if (ret) {
+   dev_err(dev, iosf failed to read punit semaphore\n);
+   return ret;
+   }
+
+   *sem = reg_val  0x1;
+
+   return 0;
+}
+
+static void reset_semaphore(struct device *dev)
+{
+   u32 data;
+
+   if (iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
+   PUNIT_SEMAPHORE, data)) {
+   dev_err(dev, iosf failed to reset punit semaphore during 
read\n);
+   return;
+   }
+
+   data = data  0xfffe;
+   if (iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
+PUNIT_SEMAPHORE, data))
+   dev_err(dev, iosf failed to reset punit semaphore during 
write\n);
+}
+
+int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
+{
+   u32 sem = 0;
+   int ret;
+   unsigned long start, end;
+
+   if (!dev || !dev-dev)
+   return -ENODEV;
+
+   if (!dev-acquire_lock)
+   return 0;
+
+   /* host driver writes 0x2 to side band semaphore register

[PATCH V4 1/2] i2c-designware: Add i2c bus locking support

2015-01-15 Thread David E. Box 
Adds support for acquiring and releasing a hardware bus lock in the i2c
designware core transfer function. This is needed for i2c bus controllers
that are shared with but not controlled by the kernel.

Signed-off-by: David E. Box david.e@linux.intel.com
---
 drivers/i2c/busses/i2c-designware-core.c | 26 ++
 drivers/i2c/busses/i2c-designware-core.h |  6 ++
 2 files changed, 32 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 3c20e4b..3d3ac4d 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -289,6 +289,15 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
u32 hcnt, lcnt;
u32 reg;
u32 sda_falling_time, scl_falling_time;
+   int ret;
+
+   if (dev-acquire_lock) {
+   ret = dev-acquire_lock(dev);
+   if (ret) {
+   dev_err(dev-dev, couldn't acquire bus ownership\n);
+   return ret;
+   }
+   }
 
input_clock_khz = dev-get_clk_rate_khz(dev);
 
@@ -302,6 +311,8 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
dev_err(dev-dev, Unknown Synopsys component type: 
0x%08x\n, reg);
+   if (dev-release_lock)
+   dev-release_lock(dev);
return -ENODEV;
}
 
@@ -368,6 +379,9 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
 
/* configure the i2c master */
dw_writel(dev, dev-master_cfg , DW_IC_CON);
+
+   if (dev-release_lock)
+   dev-release_lock(dev);
return 0;
 }
 EXPORT_SYMBOL_GPL(i2c_dw_init);
@@ -631,6 +645,14 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
dev-abort_source = 0;
dev-rx_outstanding = 0;
 
+   if (dev-acquire_lock) {
+   ret = dev-acquire_lock(dev);
+   if (ret) {
+   dev_err(dev-dev, couldn't acquire bus ownership\n);
+   goto done_nolock;
+   }
+   }
+
ret = i2c_dw_wait_bus_not_busy(dev);
if (ret  0)
goto done;
@@ -676,6 +698,10 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
ret = -EIO;
 
 done:
+   if (dev-release_lock)
+   dev-release_lock(dev);
+
+done_nolock:
pm_runtime_mark_last_busy(dev-dev);
pm_runtime_put_autosuspend(dev-dev);
mutex_unlock(dev-lock);
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index d66b6cb..a472c91 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -65,6 +65,9 @@
  * @ss_lcnt: standard speed LCNT value
  * @fs_hcnt: fast speed HCNT value
  * @fs_lcnt: fast speed LCNT value
+ * @acquire_lock: function to acquire a hardware lock on the bus
+ * @release_lock: function to release a hardware lock on the bus
+ * @pm_runtime_disabled: true if pm runtime is disabled
  *
  * HCNT and LCNT parameters can be used if the platform knows more accurate
  * values than the one computed based only on the input clock frequency.
@@ -105,6 +108,9 @@ struct dw_i2c_dev {
u16 ss_lcnt;
u16 fs_hcnt;
u16 fs_lcnt;
+   int (*acquire_lock)(struct dw_i2c_dev *dev);
+   void(*release_lock)(struct dw_i2c_dev *dev);
+   boolpm_runtime_disabled;
 };
 
 #define ACCESS_SWAP0x0001
-- 
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

Re: [PATCH V3 1/2] i2c-designware: Add i2c bus locking support

2015-01-14 Thread David E. Box 
Hi,

Expect something tonight should the latest tests run okay. I needed to include
an EPROBE_DEFER to address the unavailibity of the pci driver needed in order to
request the lock during probe of the i2c device. This due to the lock now being
requested during probe because of the hang.

Dave

On Tue, Jan 13, 2015 at 10:48:33AM +0100, Wolfram Sang wrote:
 Hi Dave,
 
  Timely reply. Around i2c_dw_init(), yes. I just discovered this as the 
  source
  of a recent hang that's occuring in the loop in __i2c_dw_enable().
  The hange occurs very infrequently and only, so far, when not built in. A
  block around i2c_dw_disable_int() would make sense as well as a precaution.
 
 Any news on this or on a V4 of this series?
 
 Thanks,
 
Wolfram
 


--
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 1/2] i2c-designware: Add i2c bus locking support

2014-12-04 Thread David E. Box 
On Thu, Dec 04, 2014 at 09:59:11AM +0200, Jarkko Nikula wrote:
 Hi
 
 On 12/02/2014 02:09 AM, David E. Box wrote:
 Adds support for acquiring and releasing a hardware bus lock in the i2c
 designware core transfer function. This is needed for i2c bus controllers
 that are shared with but not controlled by the kernel.
 
 Signed-off-by: David E. Box david.e@linux.intel.com
 ---
   drivers/i2c/busses/i2c-designware-core.c| 11 +++
   drivers/i2c/busses/i2c-designware-core.h|  6 ++
   drivers/i2c/busses/i2c-designware-platdrv.c | 18 +-
   3 files changed, 30 insertions(+), 5 deletions(-)
 
 ...
 diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
 b/drivers/i2c/busses/i2c-designware-platdrv.c
 index a743115..afdff3b 100644
 --- a/drivers/i2c/busses/i2c-designware-platdrv.c
 +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
 @@ -261,10 +261,16 @@ static int dw_i2c_probe(struct platform_device *pdev)
  return r;
  }
 -pm_runtime_set_autosuspend_delay(pdev-dev, 1000);
 -pm_runtime_use_autosuspend(pdev-dev);
 -pm_runtime_set_active(pdev-dev);
 -pm_runtime_enable(pdev-dev);
 +i2c_dw_eval_lock_support(dev);
 i2c_dw_eval_lock_support() is added in the next patch.
 +
 +if (dev-pm_runtime_disabled) {
 +pm_runtime_forbid(pdev-dev);
 +} else {
 +pm_runtime_set_autosuspend_delay(pdev-dev, 1000);
 +pm_runtime_use_autosuspend(pdev);
 +pm_runtime_set_active(pdev-dev);
 +pm_runtime_enable(pdev-dev);
 +}
  return 0;
   }
 @@ -314,7 +320,9 @@ static int dw_i2c_resume(struct device *dev)
  struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
  clk_prepare_enable(i_dev-clk);
 -i2c_dw_init(i_dev);
 +
 +if (!i_dev-pm_runtime_disabled)
 +i2c_dw_init(i_dev);
 Should there be similar conditional call or locking around
 i2c_dw_init() and i2c_dw_disable_int() also in dw_i2c_probe()?
 

Timely reply. Around i2c_dw_init(), yes. I just discovered this as the source
of a recent hang that's occuring in the loop in __i2c_dw_enable().
The hange occurs very infrequently and only, so far, when not built in. A
block around i2c_dw_disable_int() would make sense as well as a precaution.

Dave
--
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 1/2] i2c-designware: Add i2c bus locking support

2014-12-04 Thread David E. Box 
On Wed, Dec 03, 2014 at 06:01:25PM +0200, Mika Westerberg wrote:
 On Mon, Dec 01, 2014 at 04:09:32PM -0800, David E. Box wrote:
  Adds support for acquiring and releasing a hardware bus lock in the i2c
  designware core transfer function. This is needed for i2c bus controllers
  that are shared with but not controlled by the kernel.
  
  Signed-off-by: David E. Box david.e@linux.intel.com
  ---
   drivers/i2c/busses/i2c-designware-core.c| 11 +++
   drivers/i2c/busses/i2c-designware-core.h|  6 ++
   drivers/i2c/busses/i2c-designware-platdrv.c | 18 +-
   3 files changed, 30 insertions(+), 5 deletions(-)
  
  diff --git a/drivers/i2c/busses/i2c-designware-core.c 
  b/drivers/i2c/busses/i2c-designware-core.c
  index 3c20e4b..377deeb 100644
  --- a/drivers/i2c/busses/i2c-designware-core.c
  +++ b/drivers/i2c/busses/i2c-designware-core.c
  @@ -631,6 +631,14 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
  msgs[], int num)
  dev-abort_source = 0;
  dev-rx_outstanding = 0;
   
  +   if (dev-acquire_lock) {
  +   ret = dev-acquire_lock(dev);
  +   if (ret) {
  +   dev_err(dev-dev, couldn't acquire bus ownership\n);
  +   goto done;
 
 I wonder what happens now since you failed to acquire the lock...
 
  +   }
  +   }
  +
  ret = i2c_dw_wait_bus_not_busy(dev);
  if (ret  0)
  goto done;
  @@ -676,6 +684,9 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
  msgs[], int num)
  ret = -EIO;
   
   done:
  +   if (dev-release_lock)
  +   dev-release_lock(dev);
 
 ... but here you unconditionally release it?
 

Releasing the lock unconditionally isn't a problem, but it is an unnecessary
path should we fail to acquire the lock. I'll add another label to skip it.

Dave


--
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] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2014-12-04 Thread David E. Box 
On Wed, Dec 03, 2014 at 06:10:46PM +0200, Mika Westerberg wrote:
 On Mon, Dec 01, 2014 at 04:09:33PM -0800, David E. Box wrote:
  This patch implements an I2C bus sharing mechanism between the host and 
  platform
  hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 
  PMIC.
  
  On these platforms access to the PMIC must be shared with platform 
  hardware. The
  hardware unit assumes full control of the I2C bus and the host must request
  access through a special semaphore. Hardware control of the bus also makes 
  it
  necessary to disable runtime pm to avoid interfering with hardware 
  transactions.
  
  Signed-off-by: David E. Box david.e@linux.intel.com
  ---
   drivers/i2c/busses/Kconfig   |  12 +++
   drivers/i2c/busses/Makefile  |   1 +
   drivers/i2c/busses/i2c-designware-baytrail.c | 155 
  +++
   drivers/i2c/busses/i2c-designware-core.h |   6 ++
   4 files changed, 174 insertions(+)
   create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c
  
  diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
  index 917c358..d2bfd88 100644
  --- a/drivers/i2c/busses/Kconfig
  +++ b/drivers/i2c/busses/Kconfig
  @@ -464,6 +464,18 @@ config I2C_DESIGNWARE_PCI
This driver can also be built as a module.  If so, the module
will be called i2c-designware-pci.
   
  +config I2C_DESIGNWARE_BAYTRAIL
  +   bool Intel Baytrail I2C semaphore support
  +   depends on I2C_DESIGNWARE_PLATFORM=y
 
 Hmm, is there something preventing to compile this as module?
 

There were load order issues. This driver is really a support module for the
platform driver. I think Wolfram suggested this earlier but I didn't realize it
until now. The proper solution is to link it conditionally with
i2c-designware-platform.

Dave
--
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 V3 0/2] i2c-designware: Baytrail bus locking driver

2014-12-01 Thread David E. Box 
Select Intel Baytrail platforms support PMIC's whose i2c bus may be controlled
exclusively by platform hardware. This patch set adds support for i2c bus
locking to the designware core and provides a driver module for managing
the lock on these platforms. Since the lock on these systems isn't enumerable
outside of the i2c platform driver, the locking functions are assigned at
compile time.

V2: Moved semaphore detection out of dw platform driver
Replaced function pointers with defined acquire/release functions in dw
core. This helps elliminate the ifdefery in the dw platform 
driver.
Use new has_hw_lock flag to check if the lock exists on a given bus.
Use new pm_runtime_disabled flag to conditionally turnoff runtime pm
in the dw platform driver.

V3: Split lock support and driver into separate patches
Change module build to bool. Platforms running without this driver 
cannot
perform critical functions such as charging. Futhermore 
attempts by
other drivers to access the i2c bus without a lock will hang the
platform.
Replaced has_hw_lock flag with acquire/release function pointers.
Replaced acquire/release ifdef code with single 
i2c_dw_eval_lock_support()
test for cleaner (if still undesireable) compile time 
scalability.
Future Intel platforms will however continue to use the Baytrail
driver.

David E. Box (2):
  i2c-designware: Add i2c bus locking support
  i2c-designware: Add Intel Baytrail PMIC I2C bus support

 drivers/i2c/busses/Kconfig   |  12 +++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-designware-baytrail.c | 155 +++
 drivers/i2c/busses/i2c-designware-core.c |  11 ++
 drivers/i2c/busses/i2c-designware-core.h |  12 +++
 drivers/i2c/busses/i2c-designware-platdrv.c  |  18 +++-
 6 files changed, 204 insertions(+), 5 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c

-- 
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 V3 2/2] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2014-12-01 Thread David E. Box 
This patch implements an I2C bus sharing mechanism between the host and platform
hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 PMIC.

On these platforms access to the PMIC must be shared with platform hardware. The
hardware unit assumes full control of the I2C bus and the host must request
access through a special semaphore. Hardware control of the bus also makes it
necessary to disable runtime pm to avoid interfering with hardware transactions.

Signed-off-by: David E. Box david.e@linux.intel.com
---
 drivers/i2c/busses/Kconfig   |  12 +++
 drivers/i2c/busses/Makefile  |   1 +
 drivers/i2c/busses/i2c-designware-baytrail.c | 155 +++
 drivers/i2c/busses/i2c-designware-core.h |   6 ++
 4 files changed, 174 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-designware-baytrail.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 917c358..d2bfd88 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -464,6 +464,18 @@ config I2C_DESIGNWARE_PCI
  This driver can also be built as a module.  If so, the module
  will be called i2c-designware-pci.
 
+config I2C_DESIGNWARE_BAYTRAIL
+   bool Intel Baytrail I2C semaphore support
+   depends on I2C_DESIGNWARE_PLATFORM=y
+   select IOSF_MBI
+   help
+ This driver enables host access to the PMIC I2C bus on select Intel
+ BayTrail platforms using the X-Powers AXP288 PMIC. This driver is
+ required for host access to the PMIC on these platforms. You should
+ probably say Y if you have a BayTrail system, unless you know it uses
+ a different PMIC. Otherwise critical PMIC functions, like charging,
+ may not operate.
+
 config I2C_EFM32
tristate EFM32 I2C controller
depends on ARCH_EFM32 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 78d56c5..48fc23b 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += 
i2c-designware-platform.o
 i2c-designware-platform-objs := i2c-designware-platdrv.o
 obj-$(CONFIG_I2C_DESIGNWARE_PCI)   += i2c-designware-pci.o
 i2c-designware-pci-objs := i2c-designware-pcidrv.o
+obj-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL)  += i2c-designware-baytrail.o
 obj-$(CONFIG_I2C_EFM32)+= i2c-efm32.o
 obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
 obj-$(CONFIG_I2C_EXYNOS5)  += i2c-exynos5.o
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c 
b/drivers/i2c/busses/i2c-designware-baytrail.c
new file mode 100644
index 000..ce80241
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -0,0 +1,155 @@
+/*
+ * Intel BayTrail PMIC I2C bus semaphore implementaion
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#include linux/module.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/acpi.h
+#include linux/i2c.h
+#include linux/interrupt.h
+#include asm/iosf_mbi.h
+#include i2c-designware-core.h
+
+#define SEMAPHORE_TIMEOUT  100
+#define PUNIT_SEMAPHORE0x7
+
+static unsigned long acquired;
+
+static int get_sem(struct device *dev, u32 *sem)
+{
+   u32 reg_val;
+   int ret;
+
+   ret = iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ, PUNIT_SEMAPHORE,
+   reg_val);
+   if (ret) {
+   dev_err(dev, iosf failed to read punit semaphore\n);
+   return ret;
+   }
+
+   *sem = reg_val  0x1;
+
+   return 0;
+}
+
+static void reset_semaphore(struct device *dev)
+{
+   u32 data;
+
+   if (iosf_mbi_read(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_READ,
+   PUNIT_SEMAPHORE, data)) {
+   dev_err(dev, iosf failed to reset punit semaphore during 
read\n);
+   return;
+   }
+
+   data = data  0xfffe;
+   if (iosf_mbi_write(BT_MBI_UNIT_PMC, BT_MBI_BUNIT_WRITE,
+PUNIT_SEMAPHORE, data))
+   dev_err(dev, iosf failed to reset punit semaphore during 
write\n);
+}
+
+int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
+{
+   u32 sem = 0;
+   int ret;
+   unsigned long start, end;
+
+   if (!dev || !dev-dev)
+   return -ENODEV;
+
+   if (!dev-acquire_lock)
+   return 0;
+
+   /* host driver writes 0x2 to side band semaphore register */
+   ret = iosf_mbi_write(BT_MBI_UNIT_PMC

[PATCH V3 1/2] i2c-designware: Add i2c bus locking support

2014-12-01 Thread David E. Box 
Adds support for acquiring and releasing a hardware bus lock in the i2c
designware core transfer function. This is needed for i2c bus controllers
that are shared with but not controlled by the kernel.

Signed-off-by: David E. Box david.e@linux.intel.com
---
 drivers/i2c/busses/i2c-designware-core.c| 11 +++
 drivers/i2c/busses/i2c-designware-core.h|  6 ++
 drivers/i2c/busses/i2c-designware-platdrv.c | 18 +-
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 3c20e4b..377deeb 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -631,6 +631,14 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
dev-abort_source = 0;
dev-rx_outstanding = 0;
 
+   if (dev-acquire_lock) {
+   ret = dev-acquire_lock(dev);
+   if (ret) {
+   dev_err(dev-dev, couldn't acquire bus ownership\n);
+   goto done;
+   }
+   }
+
ret = i2c_dw_wait_bus_not_busy(dev);
if (ret  0)
goto done;
@@ -676,6 +684,9 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
ret = -EIO;
 
 done:
+   if (dev-release_lock)
+   dev-release_lock(dev);
+
pm_runtime_mark_last_busy(dev-dev);
pm_runtime_put_autosuspend(dev-dev);
mutex_unlock(dev-lock);
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index d66b6cb..a472c91 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -65,6 +65,9 @@
  * @ss_lcnt: standard speed LCNT value
  * @fs_hcnt: fast speed HCNT value
  * @fs_lcnt: fast speed LCNT value
+ * @acquire_lock: function to acquire a hardware lock on the bus
+ * @release_lock: function to release a hardware lock on the bus
+ * @pm_runtime_disabled: true if pm runtime is disabled
  *
  * HCNT and LCNT parameters can be used if the platform knows more accurate
  * values than the one computed based only on the input clock frequency.
@@ -105,6 +108,9 @@ struct dw_i2c_dev {
u16 ss_lcnt;
u16 fs_hcnt;
u16 fs_lcnt;
+   int (*acquire_lock)(struct dw_i2c_dev *dev);
+   void(*release_lock)(struct dw_i2c_dev *dev);
+   boolpm_runtime_disabled;
 };
 
 #define ACCESS_SWAP0x0001
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index a743115..afdff3b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -261,10 +261,16 @@ static int dw_i2c_probe(struct platform_device *pdev)
return r;
}
 
-   pm_runtime_set_autosuspend_delay(pdev-dev, 1000);
-   pm_runtime_use_autosuspend(pdev-dev);
-   pm_runtime_set_active(pdev-dev);
-   pm_runtime_enable(pdev-dev);
+   i2c_dw_eval_lock_support(dev);
+
+   if (dev-pm_runtime_disabled) {
+   pm_runtime_forbid(pdev-dev);
+   } else {
+   pm_runtime_set_autosuspend_delay(pdev-dev, 1000);
+   pm_runtime_use_autosuspend(pdev-dev);
+   pm_runtime_set_active(pdev-dev);
+   pm_runtime_enable(pdev-dev);
+   }
 
return 0;
 }
@@ -314,7 +320,9 @@ static int dw_i2c_resume(struct device *dev)
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
 
clk_prepare_enable(i_dev-clk);
-   i2c_dw_init(i_dev);
+
+   if (!i_dev-pm_runtime_disabled)
+   i2c_dw_init(i_dev);
 
return 0;
 }
-- 
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

Re: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2014-11-11 Thread David E. Box 
On Tue, Nov 11, 2014 at 12:32:56PM +0100, Wolfram Sang wrote:
 On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote:
  This patch implements an I2C bus sharing mechanism between the host and 
  platform
  hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 
  PMIC.
  
  On these platforms access to the PMIC must be shared with platform 
  hardware. The
  hardware unit assumes full control of the I2C bus and the host must request
  access through a special semaphore. Hardware control of the bus also makes 
  it
  necessary to disable runtime pm to avoid interfering with hardware 
  transactions.
 
 Can we foresee that other platforms will have similar mechanisms in the
 future?


Maybe one other platform. Unlikely there'll be any others. Okay on your
comments below.

Dave

  +config I2C_BAYTRAIL_SEM
 
 I2C_DESIGNWARE_BAYTRAIL_SEM
 
  +   tristate Intel Baytrail I2C semaphore support
  +   depends on I2C_DESIGNWARE_PLATFORM
  +   select I2C_DESIGNWARE_CORE
 
 This select is already covered by I2C_DESIGNWARE_PLATFORM.
 
  +   select IOSF_MBI
  +   help
  + This driver enables host access to the PMIC I2C bus on select Intel
  + BayTrail platforms using the X-Powers AXP288 PMIC. This driver is
  + required for host access to the PMIC on these platforms. You should
  + probably say Y if you have a BayTrail system, unless you know it uses
  + a different PMIC. Otherwises critical PMIC functions, like charging,
  + may not operate.
  +
  + This driver should be built as a m if I2C_DESIGNWARE_PLATFORM=m,
  + and as y if I2C_DESIGNWARE_PLATFORM=y.
 
 That shouldn't be the user's task to ensure. Please enforce this in the
 makefile. Check Documentation/kbuid/makefiles.txt, Section 3.3.
 
  --- /dev/null
  +++ b/drivers/i2c/busses/i2c-baytrail-sem.c
  @@ -0,0 +1,157 @@
  +/*
  + * Intel BayTrail PMIC I2C bus semaphore implementaion
  + * Copyright (c) 2014, Intel Corporation.
 
 Mika, can you have a look at the ACPI part here?
 
  diff --git a/drivers/i2c/busses/i2c-designware-core.h 
  b/drivers/i2c/busses/i2c-designware-core.h
  index d66b6cb..13e0809 100644
  --- a/drivers/i2c/busses/i2c-designware-core.h
  +++ b/drivers/i2c/busses/i2c-designware-core.h
  @@ -65,6 +65,8 @@
* @ss_lcnt: standard speed LCNT value
* @fs_hcnt: fast speed HCNT value
* @fs_lcnt: fast speed LCNT value
  + * has_hw_lock: true if bus access requires hardware lock
  + * pm_runtime_disabled: true if pm runtime is disabled
 
 Look closely. There is a difference to the entries above.
 
  @@ -123,3 +127,18 @@ 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);
  +
  +#if IS_ENABLED(CONFIG_I2C_BAYTRAIL_SEM)
  +extern int baytrail_i2c_acquire(struct dw_i2c_dev *dev);
  +extern void baytrail_i2c_release(struct dw_i2c_dev *dev);
  +extern void baytrail_evaluate_sem(struct dw_i2c_dev *dev);
  +#define i2c_dw_acquire_ownership(dev) baytrail_i2c_acquire(dev)
  +#define i2c_dw_release_ownership(dev) baytrail_i2c_release(dev)
  +#define i2c_dw_eval_lock(dev) baytrail_evaluate_sem(dev)
 i2c_dw_test_ownership_support()?
 
 That doesn't scale in case other platformts will need this. I could
 imagine a struct i2c_dw_ownership_ops() (or whatever name) which gets
 populated according to the matched device.
 
 Thanks,
 
Wolfram
 


--
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] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2014-09-23 Thread David E. Box 
This patch implements an I2C bus sharing mechanism between the host and platform
hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 PMIC.

On these platforms access to the PMIC must be shared with platform hardware. The
hardware unit assumes full control of the I2C bus and the host must request
access through a special semaphore. Hardware control of the bus also makes it
necessary to disable runtime pm to avoid interfering with hardware transactions.

Signed-off-by: David E. Box david.e@linux.intel.com
---

V2: Moved semaphore detection out of dw platform driver
Replaced function pointers with defined acquire/release functions in dw
core. This helps elliminate the ifdefery in the dw platform 
driver.
Use new has_hw_lock flag to check if the lock exists on a given bus.
Use new pm_runtime_disabled flag to conditionally turnoff runtime pm
in the dw platform driver.

 drivers/i2c/busses/Kconfig  |  16 +++
 drivers/i2c/busses/Makefile |   1 +
 drivers/i2c/busses/i2c-baytrail-sem.c   | 157 
 drivers/i2c/busses/i2c-designware-core.c|   7 ++
 drivers/i2c/busses/i2c-designware-core.h|  19 
 drivers/i2c/busses/i2c-designware-platdrv.c |  18 +++-
 6 files changed, 213 insertions(+), 5 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-baytrail-sem.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ac87fa..036f16f 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -441,6 +441,22 @@ config I2C_DESIGNWARE_PCI
  This driver can also be built as a module.  If so, the module
  will be called i2c-designware-pci.
 
+config I2C_BAYTRAIL_SEM
+   tristate Intel Baytrail I2C semaphore support
+   depends on I2C_DESIGNWARE_PLATFORM
+   select I2C_DESIGNWARE_CORE
+   select IOSF_MBI
+   help
+ This driver enables host access to the PMIC I2C bus on select Intel
+ BayTrail platforms using the X-Powers AXP288 PMIC. This driver is
+ required for host access to the PMIC on these platforms. You should
+ probably say Y if you have a BayTrail system, unless you know it uses
+ a different PMIC. Otherwises critical PMIC functions, like charging,
+ may not operate.
+
+ This driver should be built as a m if I2C_DESIGNWARE_PLATFORM=m,
+ and as y if I2C_DESIGNWARE_PLATFORM=y.
+
 config I2C_EFM32
tristate EFM32 I2C controller
depends on ARCH_EFM32 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49bf07e..6f143b4 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += 
i2c-designware-platform.o
 i2c-designware-platform-objs := i2c-designware-platdrv.o
 obj-$(CONFIG_I2C_DESIGNWARE_PCI)   += i2c-designware-pci.o
 i2c-designware-pci-objs := i2c-designware-pcidrv.o
+obj-$(CONFIG_I2C_BAYTRAIL_SEM) += i2c-baytrail-sem.o
 obj-$(CONFIG_I2C_EFM32)+= i2c-efm32.o
 obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
 obj-$(CONFIG_I2C_EXYNOS5)  += i2c-exynos5.o
diff --git a/drivers/i2c/busses/i2c-baytrail-sem.c 
b/drivers/i2c/busses/i2c-baytrail-sem.c
new file mode 100644
index 000..389aa23
--- /dev/null
+++ b/drivers/i2c/busses/i2c-baytrail-sem.c
@@ -0,0 +1,157 @@
+/*
+ * Intel BayTrail PMIC I2C bus semaphore implementaion
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#include linux/module.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/acpi.h
+#include linux/i2c.h
+#include linux/interrupt.h
+#include asm/iosf_mbi.h
+#include i2c-designware-core.h
+
+#define SEMAPHORE_TIMEOUT  100
+#define PUNIT_SEMAPHORE0x7
+
+static unsigned long acquired;
+
+void baytrail_evaluate_sem(struct dw_i2c_dev *dev)
+{
+   acpi_status status;
+   unsigned long long shared_host = 0;
+   acpi_handle handle;
+
+   if (!dev || !dev-dev) {
+   pr_err(%s:%d: device is NULL\n, __func__, __LINE__);
+   return;
+   }
+
+   handle = ACPI_HANDLE(dev-dev);
+   if (!handle)
+   return;
+
+   status = acpi_evaluate_integer(handle, _SEM, NULL, shared_host);
+
+   if (shared_host) {
+   dev_info(dev-dev, I2C bus managed by PUNIT\n);
+   dev-has_hw_lock = true;
+   dev-pm_runtime_disabled = true

Re: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support

2014-09-23 Thread David E. Box 
Hi Maxime,

On Tue, Sep 23, 2014 at 09:00:57PM +0200, Maxime Ripard wrote:
 Hi David,
 
 On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote:
  This patch implements an I2C bus sharing mechanism between the host and 
  platform
  hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 
  PMIC.
  
  On these platforms access to the PMIC must be shared with platform 
  hardware. The
  hardware unit assumes full control of the I2C bus and the host must request
  access through a special semaphore. Hardware control of the bus also makes 
  it
  necessary to disable runtime pm to avoid interfering with hardware 
  transactions.
  
  Signed-off-by: David E. Box david.e@linux.intel.com
 
 Sorry for stepping in like this without really knowing your platform,
 but wouldn't using the hwspinlock framework make more sense than
 hardcoding your own internal functions here?

I looked into this but didn't see a clear way on our platform to identify the
semaphore seperately from doing it in the designware platform driver. The way
we can find it now is through evaluating an ACPI _SEM object on every i2c device
that gets probed by the dw driver since at probe time we can get the acpi 
handle.
Without this handle however there isn't a clear way of evaluating the _SEM
object which would be needed to register a hwspinlock in separate code.

Plus it would still require changes to the designware i2c core, though 
admittedly
having a generic hwspinlock pointer added to the struct is cleaner.

Dave
--
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-designware: Intel BayTrail PMIC I2C bus support

2014-09-15 Thread David E. Box 
Hi Maxime,

On Mon, Sep 15, 2014 at 08:57:38AM +0200, Maxime Coquelin wrote:

 +err = dev-acquire_ownership(dev-dev);
 Have you considered using hwspinlocks instead?

No, I've not used it before but it looks applicable here. I'll take a look.

 @@ -212,6 +259,25 @@ static int dw_i2c_probe(struct platform_device *pdev)
  adap-dev.parent = pdev-dev;
  adap-dev.of_node = pdev-dev.of_node;
 
 +#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER)
 +if (dev-shared_host)
 +adap-algo = i2c_sc_algo;
 +
 +r = i2c_add_numbered_adapter(adap);
 +if (r) {
 +dev_err(pdev-dev, failure adding adapter\n);
 +return r;
 +}
 +
 +if (dev-shared_host)
 +pm_runtime_forbid(pdev-dev);
 +else {
 +pm_runtime_set_autosuspend_delay(pdev-dev, 1000);
 +pm_runtime_use_autosuspend(pdev-dev);
 +pm_runtime_set_active(pdev-dev);
 +pm_runtime_enable(pdev-dev);
 +}
 +#else
 Why do you put all this under config flags?

So that this additional code only compiles for this very specific
implementation.

 @@ -268,7 +334,11 @@ static int dw_i2c_resume(struct device *dev)
  struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
 
  clk_prepare_enable(i_dev-clk);
 -i2c_dw_init(i_dev);
 +
 +#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER)
 +if (!i_dev-shared_host)
 +#endif
 Putting this under config flag should not be needed.
 
 And even not under config flags, why don't you re-initialize your
 device in case of resume?

Because the device is already being managed by hardware, not the OS.

David Box
--
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] i2c-designware: Intel BayTrail PMIC I2C bus support

2014-09-12 Thread David E. Box 
This patch implements an I2C bus sharing mechanism between the host and platform
hardware on select Intel BayTrail SoC platforms using the XPower AXP288 PMIC.

On these platforms access to the PMIC must be shared with platform hardware. The
hardware unit assumes full control of the I2C bus and the host must request
access through a special semaphore. Hardware control of the bus also makes it
necessary to disable runtime pm to avoid interfering with hardware transactions.

Signed-off-by: David E. Box david.e@linux.intel.com
---
 drivers/i2c/busses/Kconfig  |  10 +++
 drivers/i2c/busses/Makefile |   1 +
 drivers/i2c/busses/i2c-designware-core.h|  14 
 drivers/i2c/busses/i2c-designware-platdrv.c |  78 +++--
 drivers/i2c/busses/i2c-shared-controller.c  | 101 
 5 files changed, 200 insertions(+), 4 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-shared-controller.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ac87fa..672ef23 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -441,6 +441,16 @@ config I2C_DESIGNWARE_PCI
  This driver can also be built as a module.  If so, the module
  will be called i2c-designware-pci.
 
+config I2C_SHARED_CONTROLLER
+   tristate Intel Baytrail PMIC shared I2C bus support
+   depends on ACPI
+   select IOSF_MBI
+   select I2C_DESIGNWARE_CORE
+   help
+ This driver enables shared access to the PMIC I2C bus on select Intel
+ BayTrail platforms using the XPower AXP288 PMIC. This driver is
+ required for host access to the PMIC on these platforms.
+
 config I2C_EFM32
tristate EFM32 I2C controller
depends on ARCH_EFM32 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49bf07e..33d62d1 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
 obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM)  += i2c-designware-platform.o
 i2c-designware-platform-objs := i2c-designware-platdrv.o
 obj-$(CONFIG_I2C_DESIGNWARE_PCI)   += i2c-designware-pci.o
+obj-$(CONFIG_I2C_SHARED_CONTROLLER)+= i2c-shared-controller.o
 i2c-designware-pci-objs := i2c-designware-pcidrv.o
 obj-$(CONFIG_I2C_EFM32)+= i2c-efm32.o
 obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index d66b6cb..a2b72f4 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -65,6 +65,10 @@
  * @ss_lcnt: standard speed LCNT value
  * @fs_hcnt: fast speed HCNT value
  * @fs_lcnt: fast speed LCNT value
+ * @shared_host: if host must share access to adapter with other
+ * firmware/hardware units
+ * @acquire_ownership: function to acquire exclusive use of the controller
+ * @release_ownership: function to release exclusive use of the controller
  *
  * HCNT and LCNT parameters can be used if the platform knows more accurate
  * values than the one computed based only on the input clock frequency.
@@ -105,6 +109,11 @@ struct dw_i2c_dev {
u16 ss_lcnt;
u16 fs_hcnt;
u16 fs_lcnt;
+#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER)
+   int shared_host;
+   int (*acquire_ownership)(struct device *dev);
+   int (*release_ownership)(struct device *dev);
+#endif
 };
 
 #define ACCESS_SWAP0x0001
@@ -123,3 +132,8 @@ 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);
+
+#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER)
+extern int i2c_acquire_ownership(struct device *dev);
+extern int i2c_release_ownership(struct device *dev);
+#endif
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index f9b1dec..f86c285 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -52,6 +52,32 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
return clk_get_rate(dev-clk)/1000;
 }
 
+#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER)
+int i2c_shared_controller_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+   int num)
+{
+   struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+   int err;
+
+   if (dev-shared_host) {
+   err = dev-acquire_ownership(dev-dev);
+   if (!err) {
+   err = i2c_dw_xfer(adap, msgs, num);
+   dev-release_ownership(dev-dev);
+   } else
+   dev_WARN(dev-dev, couldnt acquire