Re: [PATCHv5 1/3] OMAP: I2C: Reset support

2011-08-04 Thread Shubhrajyoti 

On Wednesday 03 August 2011 04:53 AM, Kevin Hilman wrote:

Shubhrajyoti Dshubhrajy...@ti.com  writes:


  Under some error conditions the i2c  driver may do a reset.

   ^^
minor: extra whitespace


  Adding a reset field and support in the platform.

s/platform/device-specific code/


Yes will fix these.

Signed-off-by: Shubhrajyoti Dshubhrajy...@ti.com

That being said, omap_device_shutdown() should already do a clean
shutdown + reset for you, and i2c-omap.h already has a pointer for
-device_shutdown() so a new device_reset() should not be needed.

IOW, simply adding pdata-device_shutdown = omap_device_shutdown()
should work.
i2C has a special reset sequence ie Disable - reset - enable - Poll 
on reset done.

This function is implemented in omap_i2c_reset. The

omap_hwmod_reset calls -  _reset

which calls ocp_softrest  or custom reset if it is present.The latter is 
true for I2C.


However I see that

omap_device_shutdown -  _assert_hardreset However for I2c there doesnt seem to 
be a HW reset line.


Am I missing something?


Kevin


---
  arch/arm/plat-omap/i2c.c |   18 ++
  include/linux/i2c-omap.h |1 +
  2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
index 2388b8e..be36cac 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -146,6 +146,22 @@ static struct omap_device_pm_latency omap_i2c_latency[] = {
.flags  = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
},
  };
+/**
+ * omap2_i2c_reset - reset the omap i2c module.
+ * @dev: struct device*
+ */
+
+static int omap2_i2c_reset(struct device *dev)
+{
+   int r = 0;
+   struct platform_device *pdev = to_platform_device(dev);
+   struct omap_device *odev = to_omap_device(pdev);
+   struct omap_hwmod *oh;
+
+   oh = odev-hwmods[0];
+   r = omap_hwmod_reset(oh);
+   return r;
+}

  static inline int omap2_i2c_add_bus(int bus_id)
  {
@@ -187,6 +203,8 @@ static inline int omap2_i2c_add_bus(int bus_id)
 */
if (cpu_is_omap34xx())
pdata-set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat;
+
+   pdata-device_reset = omap2_i2c_reset;
od = omap_device_build(name, bus_id, oh, pdata,
sizeof(struct omap_i2c_bus_platform_data),
omap_i2c_latency, ARRAY_SIZE(omap_i2c_latency), 0);
diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
index 98ae49b..8aa91b6 100644
--- a/include/linux/i2c-omap.h
+++ b/include/linux/i2c-omap.h
@@ -38,6 +38,7 @@ struct omap_i2c_bus_platform_data {
int (*device_enable) (struct platform_device *pdev);
int (*device_shutdown) (struct platform_device *pdev);
int (*device_idle) (struct platform_device *pdev);
+   int (*device_reset) (struct device *dev);
  };

  #endif


--
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: add OF binding support

2011-08-04 Thread Ben Dooks 
On Wed, Aug 03, 2011 at 03:04:23PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com
 
 Add of_match_table and DT style i2c registration to designware i2c
 driver.
 
 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c  |   13 
  2 files changed, 36 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt
 
 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 + i2c@f {
 + #address-cells = 1;
 + #size-cells = 0;
 + compatible = snps,designware-i2c;
 + reg = 0xf 0x1000;
 + interrupts = 11;
 + clock-frequency = 40;
 + };
 +

looks good to me.

 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..2911a49 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -37,6 +37,7 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/of_i2c.h
  
  /*
   * Registers offset
 @@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct 
 platform_device *pdev)
   adap-algo = i2c_dw_algo;
   adap-dev.parent = pdev-dev;
  
 +#ifdef CONFIG_OF
 + r = i2c_add_adapter(adap);
 +#else
   adap-nr = pdev-id;
   r = i2c_add_numbered_adapter(adap);
 +#endif

I would say that doing the #ifdef CONFIG_OF is dangerous here when we
are in a mixed OF/platform enviromnent as we're depending on compile
time selection.

I'm also wondering whether we have an of helper macro which takes
a pdev and gives you an adapter number either given on pdev-id or
-1 for the case when we're using the OF bindings.

It might be worth talking to Grant about setting pdev-id to -1 if we
are using an OF device.

   if (r) {
   dev_err(pdev-dev, failure adding adapter\n);
   goto err_free_irq;
   }
 + of_i2c_register_devices(adap);

If we did that, we could add a of_i2c_register_adapter() call which
would take the platform device and then do the of_i2c_register_devices()
and do these steps.

   return 0;
  
 @@ -819,6 +825,12 @@ static int __devexit dw_i2c_remove(struct 
 platform_device *pdev)
   return 0;
  }
  
 +static const struct of_device_id dw_i2c_of_match[] = {
 + { .compatible = snps,designware-i2c, },
 + {},
 +};
 +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 +
  /* work with hotplug and coldplug */
  MODULE_ALIAS(platform:i2c_designware);
  
 @@ -827,6 +839,7 @@ static struct platform_driver dw_i2c_driver = {
   .driver = {
   .name   = i2c_designware,
   .owner  = THIS_MODULE,
 + .of_match_table = dw_i2c_of_match,

If my patch for of_match_ptr() is accepted, it will be needed here
otherwise you will need to do something about remvoing the of table
above if not config of.

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.
--
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] Revert i2c-omap: fix static suspend vs. runtime suspend

2011-08-04 Thread Kevin Hilman 
Felipe Balbi ba...@ti.com writes:

 On Wed, Aug 03, 2011 at 10:59:39AM -0700, Kevin Hilman wrote:
 This reverts commit adf6e07922255937c8bfeea777d19502b4c9a2be.
 
 Remove system PM methods which can race with runtime PM methods.
 
 Also, as of v3.1, the PM domain level code for OMAP handles device
 power state transistions automatically for devices, so calling them
 from

 from where ? I didn't quite get this ;-)

heh, looks like i got distracted before finishing the changelog.   Will
respin.

Thanks,

Kevin
--
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] Revert i2c-omap: fix static suspend vs. runtime suspend

2011-08-04 Thread Kevin Hilman 
This reverts commit adf6e07922255937c8bfeea777d19502b4c9a2be.

Remove system PM methods which can race with runtime PM methods.

Also, as of v3.1, the PM domain level code for OMAP handles device
power state transistions automatically for devices, so drivers no
longer need to specifically call the bus/pm_domain methods themselves.

Signed-off-by: Kevin Hilman khil...@ti.com
---
v2: updated changelog to remove cliff-hanger ending

 drivers/i2c/busses/i2c-omap.c |   29 -
 1 files changed, 0 insertions(+), 29 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 84df53f..e854be0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1148,41 +1148,12 @@ omap_i2c_remove(struct platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_SUSPEND
-static int omap_i2c_suspend(struct device *dev)
-{
-   if (!pm_runtime_suspended(dev))
-   if (dev-bus  dev-bus-pm  dev-bus-pm-runtime_suspend)
-   dev-bus-pm-runtime_suspend(dev);
-
-   return 0;
-}
-
-static int omap_i2c_resume(struct device *dev)
-{
-   if (!pm_runtime_suspended(dev))
-   if (dev-bus  dev-bus-pm  dev-bus-pm-runtime_resume)
-   dev-bus-pm-runtime_resume(dev);
-
-   return 0;
-}
-
-static struct dev_pm_ops omap_i2c_pm_ops = {
-   .suspend = omap_i2c_suspend,
-   .resume = omap_i2c_resume,
-};
-#define OMAP_I2C_PM_OPS (omap_i2c_pm_ops)
-#else
-#define OMAP_I2C_PM_OPS NULL
-#endif
-
 static struct platform_driver omap_i2c_driver = {
.probe  = omap_i2c_probe,
.remove = omap_i2c_remove,
.driver = {
.name   = omap_i2c,
.owner  = THIS_MODULE,
-   .pm = OMAP_I2C_PM_OPS,
},
 };
 
-- 
1.7.6

--
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 2/2] I2C: OMAP: remove dev-idle, use usage counting provided by runtime PM

2011-08-04 Thread Kevin Hilman 
Felipe Balbi ba...@ti.com writes:

 Hi,

 On Wed, Aug 03, 2011 at 11:09:20AM -0700, Kevin Hilman wrote:
 Current usage of runtime PM is not quite correct.  The actual
 idle/unidle of the I2C hardware should not happen until the runtime PM
 callbacks are called.  Therefore, change omap_i2c_[un]idle() functions
 to only be called from the runtime PM callbacks (when usage count
 transitions to/from zero.)
 
 Also, the runtime PM core does usage counting and replaces
 functionality currently managed by the dev-idle flag.  Remove usage
 of dev-idle in favor of using runtime PM, and checking status using
 pm_runtime_suspended().
 
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  drivers/i2c/busses/i2c-omap.c |   58 
 ++--
  1 files changed, 38 insertions(+), 20 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
 index 12d0cbc..1b5325b 100644
 --- a/drivers/i2c/busses/i2c-omap.c
 +++ b/drivers/i2c/busses/i2c-omap.c
 @@ -194,7 +194,6 @@ struct omap_i2c_dev {
   */
  u8  rev;
  unsignedb_hw:1; /* bad h/w fixes */
 -unsignedidle:1;
  u16 iestate;/* Saved interrupt register */
  u16 pscstate;
  u16 scllstate;
 @@ -269,12 +268,8 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
  {
  struct omap_i2c_bus_platform_data *pdata;
  
 -WARN_ON(!dev-idle);
 -
  pdata = dev-dev-platform_data;
  
 -pm_runtime_get_sync(dev-dev);
 -
  if (pdata-flags  OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
  omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
  omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev-pscstate);
 @@ -285,7 +280,6 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
  omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate);
  omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
  }
 -dev-idle = 0;
  
  /*
   * Don't write to this register if the IE state is 0 as it can
 @@ -300,8 +294,6 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
  struct omap_i2c_bus_platform_data *pdata;
  u16 iv;
  
 -WARN_ON(dev-idle);
 -
  pdata = dev-dev-platform_data;
  
  dev-iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 @@ -315,12 +307,9 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
  } else {
  omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev-iestate);
  
 -/* Flush posted write before the dev-idle store occurs */
 +/* Flush posted write */
  omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
  }
 -dev-idle = 1;
 -
 -pm_runtime_put_sync(dev-dev);
  }
  
  static int omap_i2c_init(struct omap_i2c_dev *dev)
 @@ -644,7 +633,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
 msgs[], int num)
  int i;
  int r;
  
 -omap_i2c_unidle(dev);
 +pm_runtime_get_sync(dev-dev);
  
  r = omap_i2c_wait_for_bb(dev);
  if (r  0)
 @@ -667,7 +656,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
 msgs[], int num)
  
  omap_i2c_wait_for_bb(dev);
  out:
 -omap_i2c_idle(dev);
 +pm_runtime_put_sync(dev-dev);

 I wonder if these pm_runtime_put need to be synchronous ? Could we just
 call pm_runtime_put() instead ? Ditto to all other.

Yes, will use asynchronous versions.

 @@ -1140,6 +1128,36 @@ omap_i2c_remove(struct platform_device *pdev)
  return 0;
  }
  
 +#ifdef CONFIG_PM_RUNTIME
 +static int omap_i2c_runtime_suspend(struct device *dev)
 +{
 +struct platform_device *pdev = to_platform_device(dev);
 +struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);

 what happened to dev_get_drvdata(dev) ??


Yes, that would work too since:

static inline void *platform_get_drvdata(const struct platform_device *pdev)
{
return dev_get_drvdata(pdev-dev);
}

but IMO, readability is better if the driver does platform_set_drvdata()
and then the corresponding platform_get_drvdata()

 +omap_i2c_idle(_dev);
 +
 +return 0;
 +}
 +
 +static int omap_i2c_runtime_resume(struct device *dev)
 +{
 +struct platform_device *pdev = to_platform_device(dev);
 +struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);

 ditto

 +omap_i2c_unidle(_dev);
 +
 +return 0;
 +}
 +
 +static struct dev_pm_ops omap_i2c_pm_ops = {
 +.runtime_suspend = omap_i2c_runtime_suspend,
 +.runtime_resume = omap_i2c_runtime_resume,
 +};
 +#define OMAP_I2C_PM_OPS (omap_i2c_pm_ops)
 +#else
 +#define OMAP_I2C_PM_OPS NULL
 +#endif

 OMAP_I2C_PM_OPS isn't used anywhere ??

doh

thanks for catching

Kevin
--
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] I2C: OMAP: remove unneccesary use of pdev

2011-08-04 Thread Kevin Hilman 
A pointer to the struct device associated with the i2c device is
already kept in the struct omap_i2c_dev, so use omap_i2c_device to
find the pointer to struct device.

Reviewed-by: Felipe Balbi ba...@ti.com
Signed-off-by: Kevin Hilman khil...@ti.com
---
 drivers/i2c/busses/i2c-omap.c |   22 +++---
 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e854be0..12d0cbc 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -267,15 +267,13 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev 
*i2c_dev, int reg)
 
 static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
-   struct platform_device *pdev;
struct omap_i2c_bus_platform_data *pdata;
 
WARN_ON(!dev-idle);
 
-   pdev = to_platform_device(dev-dev);
-   pdata = pdev-dev.platform_data;
+   pdata = dev-dev-platform_data;
 
-   pm_runtime_get_sync(pdev-dev);
+   pm_runtime_get_sync(dev-dev);
 
if (pdata-flags  OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
@@ -299,14 +297,12 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 
 static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
-   struct platform_device *pdev;
struct omap_i2c_bus_platform_data *pdata;
u16 iv;
 
WARN_ON(dev-idle);
 
-   pdev = to_platform_device(dev-dev);
-   pdata = pdev-dev.platform_data;
+   pdata = dev-dev-platform_data;
 
dev-iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
if (pdata-rev == OMAP_I2C_IP_VERSION_2)
@@ -324,7 +320,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
}
dev-idle = 1;
 
-   pm_runtime_put_sync(pdev-dev);
+   pm_runtime_put_sync(dev-dev);
 }
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)
@@ -335,11 +331,9 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
unsigned long timeout;
unsigned long internal_clk = 0;
struct clk *fclk;
-   struct platform_device *pdev;
struct omap_i2c_bus_platform_data *pdata;
 
-   pdev = to_platform_device(dev-dev);
-   pdata = pdev-dev.platform_data;
+   pdata = dev-dev-platform_data;
 
if (dev-rev = OMAP_I2C_OMAP1_REV_2) {
/* Disable I2C controller before soft reset */
@@ -821,11 +815,9 @@ omap_i2c_isr(int this_irq, void *dev_id)
u16 bits;
u16 stat, w;
int err, count = 0;
-   struct platform_device *pdev;
struct omap_i2c_bus_platform_data *pdata;
 
-   pdev = to_platform_device(dev-dev);
-   pdata = pdev-dev.platform_data;
+   pdata = dev-dev-platform_data;
 
if (dev-idle)
return IRQ_NONE;
@@ -1047,7 +1039,7 @@ omap_i2c_probe(struct platform_device *pdev)
else
dev-regs = (u8 *)reg_map_ip_v1;
 
-   pm_runtime_enable(pdev-dev);
+   pm_runtime_enable(dev-dev);
omap_i2c_unidle(dev);
 
dev-rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG)  0xff;
-- 
1.7.6

--
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: OMAP: remove dev-idle, use usage counting provided by runtime PM

2011-08-04 Thread Kevin Hilman 
Current usage of runtime PM is not quite correct.  The actual
idle/unidle of the I2C hardware should not happen until the runtime PM
callbacks are called.  Therefore, change omap_i2c_[un]idle() functions
to only be called from the runtime PM callbacks (when usage count
transitions to/from zero.)

Also, the runtime PM core does usage counting and replaces
functionality currently managed by the dev-idle flag.  Remove usage
of dev-idle in favor of using runtime PM, and checking status using
pm_runtime_suspended().

Signed-off-by: Kevin Hilman khil...@ti.com
---
 drivers/i2c/busses/i2c-omap.c |   59 +++--
 1 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 12d0cbc..1c75d13 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -194,7 +194,6 @@ struct omap_i2c_dev {
 */
u8  rev;
unsignedb_hw:1; /* bad h/w fixes */
-   unsignedidle:1;
u16 iestate;/* Saved interrupt register */
u16 pscstate;
u16 scllstate;
@@ -269,12 +268,8 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
struct omap_i2c_bus_platform_data *pdata;
 
-   WARN_ON(!dev-idle);
-
pdata = dev-dev-platform_data;
 
-   pm_runtime_get_sync(dev-dev);
-
if (pdata-flags  OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev-pscstate);
@@ -285,7 +280,6 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate);
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
}
-   dev-idle = 0;
 
/*
 * Don't write to this register if the IE state is 0 as it can
@@ -300,8 +294,6 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
struct omap_i2c_bus_platform_data *pdata;
u16 iv;
 
-   WARN_ON(dev-idle);
-
pdata = dev-dev-platform_data;
 
dev-iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
@@ -315,12 +307,9 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
} else {
omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev-iestate);
 
-   /* Flush posted write before the dev-idle store occurs */
+   /* Flush posted write */
omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
}
-   dev-idle = 1;
-
-   pm_runtime_put_sync(dev-dev);
 }
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)
@@ -644,7 +633,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
int i;
int r;
 
-   omap_i2c_unidle(dev);
+   pm_runtime_get_sync(dev-dev);
 
r = omap_i2c_wait_for_bb(dev);
if (r  0)
@@ -667,7 +656,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
 
omap_i2c_wait_for_bb(dev);
 out:
-   omap_i2c_idle(dev);
+   pm_runtime_put(dev-dev);
return r;
 }
 
@@ -727,7 +716,7 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
struct omap_i2c_dev *dev = dev_id;
u16 iv, w;
 
-   if (dev-idle)
+   if (pm_runtime_suspended(dev-dev))
return IRQ_NONE;
 
iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
@@ -819,7 +808,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
 
pdata = dev-dev-platform_data;
 
-   if (dev-idle)
+   if (pm_runtime_suspended(dev-dev))
return IRQ_NONE;
 
bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
@@ -1021,7 +1010,6 @@ omap_i2c_probe(struct platform_device *pdev)
}
 
dev-speed = speed;
-   dev-idle = 1;
dev-dev = pdev-dev;
dev-irq = irq-start;
dev-base = ioremap(mem-start, resource_size(mem));
@@ -1040,7 +1028,7 @@ omap_i2c_probe(struct platform_device *pdev)
dev-regs = (u8 *)reg_map_ip_v1;
 
pm_runtime_enable(dev-dev);
-   omap_i2c_unidle(dev);
+   pm_runtime_get_sync(dev-dev);
 
dev-rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG)  0xff;
 
@@ -1087,7 +1075,7 @@ omap_i2c_probe(struct platform_device *pdev)
dev_info(dev-dev, bus %d rev%d.%d.%d at %d kHz\n, pdev-id,
 pdata-rev, dev-rev  4, dev-rev  0xf, dev-speed);
 
-   omap_i2c_idle(dev);
+   pm_runtime_put(dev-dev);
 
adap = dev-adapter;
i2c_set_adapdata(adap, dev);
@@ -,7 +1099,7 @@ err_free_irq:
free_irq(dev-irq, dev);
 err_unuse_clocks:
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
-   omap_i2c_idle(dev);
+   pm_runtime_put(dev-dev);
iounmap(dev-base);
 err_free_mem:
platform_set_drvdata(pdev, NULL);
@@ -1140,12 +1128,43 @@

[PATCH v2 0/2] I2C: OMAP: misc. PM-related cleanups

2011-08-04 Thread Kevin Hilman 
Do some PM-related cleanup on this driver and make the runtime PM
usage more standard.

Series applies on Andy's I2C cleanup series (for_3.1/i2c-andy-2 branch
in my tree[1]) plus the misc. i2c fixes queued for v3.1-rc
(for_3.1/i2c-fixes branch) and are available in the
for_3.2/i2c-cleanup branch of my tree.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

Kevin Hilman (2):
  I2C: OMAP: remove unneccesary use of pdev
  I2C: OMAP: remove dev-idle, use usage counting provided by runtime
PM

 drivers/i2c/busses/i2c-omap.c |   77 +++-
 1 files changed, 44 insertions(+), 33 deletions(-)

-- 
1.7.6

--
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: [PATCHv5 1/3] OMAP: I2C: Reset support

2011-08-04 Thread Kevin Hilman 
Shubhrajyoti shubhrajy...@ti.com writes:

 On Wednesday 03 August 2011 04:53 AM, Kevin Hilman wrote:
 Shubhrajyoti Dshubhrajy...@ti.com  writes:

   Under some error conditions the i2c  driver may do a reset.
^^
 minor: extra whitespace

   Adding a reset field and support in the platform.
 s/platform/device-specific code/

 Yes will fix these.
 Signed-off-by: Shubhrajyoti Dshubhrajy...@ti.com
 That being said, omap_device_shutdown() should already do a clean
 shutdown + reset for you, and i2c-omap.h already has a pointer for
 -device_shutdown() so a new device_reset() should not be needed.

 IOW, simply adding pdata-device_shutdown = omap_device_shutdown()
 should work.
 i2C has a special reset sequence ie Disable - reset - enable - Poll
 on reset done.
 This function is implemented in omap_i2c_reset. The

 omap_hwmod_reset calls -  _reset

 which calls ocp_softrest  or custom reset if it is present.The latter
 is true for I2C.

 However I see that

 omap_device_shutdown -  _assert_hardreset However for I2c there doesnt seem 
 to be a HW reset line.


 Am I missing something?


No, my fault.

I thought omap_device_shutdown() also did soft reset, but I see that's
not the case.  You can ignore my comments about using
omap_device_shutdown.

Kevin
--
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 3/2] I2C: OMAP: remove unused function pointers from pdata

2011-08-04 Thread Kevin Hilman 
Now that this driver is using runtime PM, there is no longer a need
for the idle/enable/shutdown function pointers in pdata.

Signed-off-by: Kevin Hilman khil...@ti.com
---
Here's one more for the i2c-cleanup series

 include/linux/i2c-omap.h |3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
index 56a9924..92a0dc7 100644
--- a/include/linux/i2c-omap.h
+++ b/include/linux/i2c-omap.h
@@ -35,9 +35,6 @@ struct omap_i2c_bus_platform_data {
u32 rev;
u32 flags;
void(*set_mpu_wkup_lat)(struct device *dev, long set);
-   int (*device_enable) (struct platform_device *pdev);
-   int (*device_shutdown) (struct platform_device *pdev);
-   int (*device_idle) (struct platform_device *pdev);
 };
 
 #endif
-- 
1.7.6

--
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 2/2] I2C: OMAP: remove dev-idle, use usage counting provided by runtime PM

2011-08-04 Thread Felipe Balbi 
Hi,

On Thu, Aug 04, 2011 at 07:53:37AM -0700, Kevin Hilman wrote:
  @@ -1140,6 +1128,36 @@ omap_i2c_remove(struct platform_device *pdev)
 return 0;
   }
   
  +#ifdef CONFIG_PM_RUNTIME
  +static int omap_i2c_runtime_suspend(struct device *dev)
  +{
  +  struct platform_device *pdev = to_platform_device(dev);
  +  struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
 
  what happened to dev_get_drvdata(dev) ??
 
 
 Yes, that would work too since:
 
 static inline void *platform_get_drvdata(const struct platform_device *pdev)
 {
   return dev_get_drvdata(pdev-dev);
 }
 
 but IMO, readability is better if the driver does platform_set_drvdata()
 and then the corresponding platform_get_drvdata()

fair enough, but if you already have the dev pointer, what's the gain in
doing a container_of() just to go back to the dev pointer again ?

IMHO, there's really no need for that and just calling dev_get_drvdata()
straight is enough. All in all, platform_get_[sg]et_drvdata() are just
wrappers for dev_[sg]et_drvdata(). The whole point, was to avoid
dev_[sg]et_drvdata(pdev-dev). It's the same with drivername_writel()
calls we see on all drivers. Just a wrapper, but you can use
__raw_writel() directly if you wish.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] i2c-designware: add OF binding support

2011-08-04 Thread Rob Herring 
Ben,

On 08/04/2011 04:12 AM, Ben Dooks wrote:
 On Wed, Aug 03, 2011 at 03:04:23PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 Add of_match_table and DT style i2c registration to designware i2c
 driver.

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c  |   13 
  2 files changed, 36 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 +i2c@f {
 +#address-cells = 1;
 +#size-cells = 0;
 +compatible = snps,designware-i2c;
 +reg = 0xf 0x1000;
 +interrupts = 11;
 +clock-frequency = 40;
 +};
 +
 
 looks good to me.
 
 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..2911a49 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -37,6 +37,7 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/of_i2c.h
  
  /*
   * Registers offset
 @@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct 
 platform_device *pdev)
  adap-algo = i2c_dw_algo;
  adap-dev.parent = pdev-dev;
  
 +#ifdef CONFIG_OF
 +r = i2c_add_adapter(adap);
 +#else
  adap-nr = pdev-id;
  r = i2c_add_numbered_adapter(adap);
 +#endif
 
 I would say that doing the #ifdef CONFIG_OF is dangerous here when we
 are in a mixed OF/platform enviromnent as we're depending on compile
 time selection.
 
 I'm also wondering whether we have an of helper macro which takes
 a pdev and gives you an adapter number either given on pdev-id or
 -1 for the case when we're using the OF bindings.
 
 It might be worth talking to Grant about setting pdev-id to -1 if we
 are using an OF device.
 

As Grant said, that's already done and this hunk is not needed.

  if (r) {
  dev_err(pdev-dev, failure adding adapter\n);
  goto err_free_irq;
  }
 +of_i2c_register_devices(adap);
 
 If we did that, we could add a of_i2c_register_adapter() call which
 would take the platform device and then do the of_i2c_register_devices()
 and do these steps.
 

Better yet, how about putting of_i2c_register_devices into
i2c_register_adapter? Everywhere that calls of_i2c_register_devices is
preceded by a call to i2c_add_numbered_adapter or i2c_add_adapter. It
seems logical to put it with i2c_scan_static_board_info. I'll prepare a
patch to add that and remove all the other callers unless you think
that's a bad idea.

  return 0;
  
 @@ -819,6 +825,12 @@ static int __devexit dw_i2c_remove(struct 
 platform_device *pdev)
  return 0;
  }
  
 +static const struct of_device_id dw_i2c_of_match[] = {
 +{ .compatible = snps,designware-i2c, },
 +{},
 +};
 +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 +
  /* work with hotplug and coldplug */
  MODULE_ALIAS(platform:i2c_designware);
  
 @@ -827,6 +839,7 @@ static struct platform_driver dw_i2c_driver = {
  .driver = {
  .name   = i2c_designware,
  .owner  = THIS_MODULE,
 +.of_match_table = dw_i2c_of_match,
 
 If my patch for of_match_ptr() is accepted, it will be needed here
 otherwise you will need to do something about remvoing the of table
 above if not config of.

There's really not much harm with having the table. If you match the
device in the non-DT way, the table is not used. Drivers should never
directly access the table either, but use the helpers to get their data.

Rob


--
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: [PATCHv5 2/3] OMAP: I2C: Remove the reset in the init path

2011-08-04 Thread Shubhrajyoti 

On Wednesday 03 August 2011 04:57 AM, Kevin Hilman wrote:

Shubhrajyoti Dshubhrajy...@ti.com  writes:


-  The reset in the driver at init is not needed anymore as the
hwmod framework takes care of reseting it.
-  Reset is removed from omap_i2c_init, which was called
not only during probe, but also after time out and error handling.
device_reset were added in those places to effect the reset.

Not specifically related to this patch, as the reset behavior was
already there, but do you know why the reset is needed after timeout and
error handling?

IOW, was the reset truly required in those cases, or was just the
re-init necessary?

To me doing a full reset of the IP in those cases sounds like a hack for
a buggy driver.
My idea, there have been errata workaround that recommend reset in case 
of a

lockup that may happen after a warmreset.


-  Earlier the hwmod SYSC settings were over-written in the driver.
Removing the same and letting the hwmod take care of the settings.
-  Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
-  Clean up the SYSCONFIG SYSC bit defination macros.
-  Fix the typos in wakeup.

Signed-off-by: Shubhrajyoti Dshubhrajy...@ti.com
---
  drivers/i2c/busses/i2c-omap.c |   83 +++--
  1 files changed, 22 insertions(+), 61 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4e3256f..d8f4470 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -155,19 +155,6 @@ enum {
  #define OMAP_I2C_SYSTEST_SDA_O(1  0)  /* SDA line drive out 
*/
  #endif

-/* OCP_SYSSTATUS bit definitions */
-#define SYSS_RESETDONE_MASK(1  0)
-
-/* OCP_SYSCONFIG bit definitions */
-#define SYSC_CLOCKACTIVITY_MASK(0x3  8)
-#define SYSC_SIDLEMODE_MASK(0x3  3)
-#define SYSC_ENAWAKEUP_MASK(1  2)
-#define SYSC_SOFTRESET_MASK(1  1)
-#define SYSC_AUTOIDLE_MASK (1  0)
-
-#define SYSC_IDLEMODE_SMART0x2
-#define SYSC_CLOCKACTIVITY_FCLK0x2
-
  /* Errata definitions */
  #define I2C_OMAP_ERRATA_I207  (1  0)
  #define I2C_OMAP3_1P153   (1  1)
@@ -182,6 +169,7 @@ struct omap_i2c_dev {
u32 latency;/* maximum mpu wkup latency */
void(*set_mpu_wkup_lat)(struct device *dev,
long latency);
+   int (*device_reset)(struct device *dev);
u32 speed;  /* Speed of bus in Khz */
u16 cmd_err;
u8  *buf;
@@ -317,60 +305,22 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
u16 psc = 0, scll = 0, sclh = 0, buf = 0;
u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
unsigned long fclk_rate = 1200;
-   unsigned long timeout;
unsigned long internal_clk = 0;
struct clk *fclk;
struct omap_i2c_bus_platform_data *pdata;

pdata = dev-dev-platform_data;

-   if (dev-rev= OMAP_I2C_OMAP1_REV_2) {
-   /* Disable I2C controller before soft reset */
-   omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
-   omap_i2c_read_reg(dev, OMAP_I2C_CON_REG)
-   ~(OMAP_I2C_CON_EN));
-
-   omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, SYSC_SOFTRESET_MASK);
-   /* For some reason we need to set the EN bit before the
-* reset done bit gets set. */
-   timeout = jiffies + OMAP_I2C_TIMEOUT;
-   omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
-   while (!(omap_i2c_read_reg(dev, OMAP_I2C_SYSS_REG)
-SYSS_RESETDONE_MASK)) {
-   if (time_after(jiffies, timeout)) {
-   dev_warn(dev-dev, timeout waiting 
-   for controller reset\n);
-   return -ETIMEDOUT;
-   }
-   msleep(1);
-   }
-
-   /* SYSC register is cleared by the reset; rewrite it */
-   if (dev-rev == OMAP_I2C_REV_ON_2430) {
-
-   omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
-  SYSC_AUTOIDLE_MASK);
-
-   } else if (dev-rev= OMAP_I2C_REV_ON_3430) {
-   dev-syscstate = SYSC_AUTOIDLE_MASK;
-   dev-syscstate |= SYSC_ENAWAKEUP_MASK;
-   dev-syscstate |= (SYSC_IDLEMODE_SMART
- __ffs(SYSC_SIDLEMODE_MASK));
-   dev-syscstate |= (SYSC_CLOCKACTIVITY_FCLK
- __ffs(SYSC_CLOCKACTIVITY_MASK));
-
-   omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
-

Re: [PATCH] i2c: imx: choose the better clock divider

2011-08-04 Thread Ben Dooks 
On Tue, Jun 14, 2011 at 03:17:05PM +0800, Eric Miao wrote:
 The original algorithm doesn't perform very well in some cases, e.g.
 
   When the source clock of the I2C controller is 66MHz, and the
   requested rate is 100KHz, it gives a divider of 768 instead of
   the better 640.
 
 Choose a better clock divider so the final clock rate is closer to
 the requested one by comparing the rate distances calculated by
 two adjacent dividers.
 
 Cc: Richard Zhao richard.z...@linaro.org
 Signed-off-by: Eric Miao eric.m...@linaro.org
 ---
  drivers/i2c/busses/i2c-imx.c |   46 -
  1 files changed, 31 insertions(+), 15 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
 index 4c2a62b..cd640ff 100644
 --- a/drivers/i2c/busses/i2c-imx.c
 +++ b/drivers/i2c/busses/i2c-imx.c
 @@ -112,6 +112,8 @@ static u16 __initdata i2c_clk_div[50][2] = {
   { 3072, 0x1E }, { 3840, 0x1F }
  };
  
 +#define I2C_CLK_DIV_NUM  ARRAY_SIZE(i2c_clk_div)
 +
  struct imx_i2c_struct {
   struct i2c_adapter  adapter;
   struct resource *res;
 @@ -240,22 +242,37 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
   clk_disable(i2c_imx-clk);
  }
  
 -static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 - unsigned int rate)
 +/* find the index into i2c_clk_div[] array, compare with the two
 + * dividers found, return the one with smaller error
 + */
 +static int find_div(unsigned long i2c_clk_rate, unsigned long rate)
  {
 - unsigned int i2c_clk_rate;
 - unsigned int div;
 + unsigned long div, delta_l, delta_h;
   int i;
  
 - /* Divider value calculation */
 - i2c_clk_rate = clk_get_rate(i2c_imx-clk);
   div = (i2c_clk_rate + rate - 1) / rate;
 +
   if (div  i2c_clk_div[0][0])
 - i = 0;
 - else if (div  i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1][0])
 - i = ARRAY_SIZE(i2c_clk_div) - 1;
 - else
 - for (i = 0; i2c_clk_div[i][0]  div; i++);
 + return 0;
 +
 + if (div  i2c_clk_div[I2C_CLK_DIV_NUM - 1][0])
 + return I2C_CLK_DIV_NUM;
 +
 + for (i = 0; i  I2C_CLK_DIV_NUM; i++)
 + if (i2c_clk_div[i][0]  div)
 + break;
 +
 + delta_h = (i2c_clk_rate / i2c_clk_div[i - 1][0]) - rate;
 + delta_l = rate - (i2c_clk_rate / i2c_clk_div[i][0]);

hmm, what happens for the case of i being zero?

 +
 + return (delta_l  delta_h) ? i : i - 1;
 +}
 +
 +static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 +unsigned int rate)
 +{
 + unsigned long i2c_clk_rate = clk_get_rate(i2c_imx-clk);
 + int i = find_div(i2c_clk_rate, rate);
  
   /* Store divider value */
   i2c_imx-ifdr = i2c_clk_div[i][1];
 @@ -271,10 +288,9 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct 
 *i2c_imx,
  
   /* dev_dbg() can't be used, because adapter is not yet registered */
  #ifdef CONFIG_I2C_DEBUG_BUS
 - printk(KERN_DEBUG I2C: %s I2C_CLK=%d, REQ DIV=%d\n,
 - __func__, i2c_clk_rate, div);
 - printk(KERN_DEBUG I2C: %s IFDR[IC]=0x%x, REAL DIV=%d\n,
 - __func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
 + pr_debug(%s I2C_CLK=%ld, REQ RATE=%d, DIV=%d, IFDR[IC]=0x%x\n,
 + __func__, i2c_clk_rate, rate,
 + i2c_clk_div[i][0], i2c_clk_div[i][1]);
  #endif
  }
  
 -- 
 1.7.4.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

-- 
Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

--
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] i2c: Support for Netlogic XLR/XLS on-chip I2C controller.

2011-08-04 Thread Ganesan Ramalingam 
Add support for on chip I2C bus controller on Netlogic XLR/XLS family
of MIPS64 SoCs. The changes are:

Kconfig/Makefile: add CONFIG_I2C_XLR option
busses/i2c-xlr.c : driver for Netlogic XLR/XLS on-chip I2C controller

Signed-off-by: Ganesan Ramalingam ganes...@netlogicmicro.com
Signed-off-by: Jayachandran C jayachandr...@netlogicmicro.com
---
[Posting this again, comments and suggestions welcome]

Changes since v2:
 - Remove platform related code changes to a different patch that will be 
   submitted later thru linux-mips
Changes since v1:   
   
 - Use master_xfer in place of smbus_xfer   
   
 - Fixes for the comments from Ben Dooks and Mark Brown 
   

 drivers/i2c/busses/Kconfig   |   11 ++
 drivers/i2c/busses/Makefile  |1 +
 drivers/i2c/busses/i2c-xlr.c |  303 ++
 3 files changed, 315 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-xlr.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 646068e..4ac63f5 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -309,6 +309,17 @@ config I2C_AU1550
  This driver can also be built as a module.  If so, the module
  will be called i2c-au1550.
 
+config I2C_XLR
+   tristate XLR I2C support
+   depends on CPU_XLR
+   help
+ This driver enables support for the on-chip I2C interface of
+ the Netlogic XLR/XLS MIPS processors.
+
+ Say yes to this option if you have a Netlogic XLR/XLS based
+ board and you need to access the I2C devices (typically the
+ RTC, sensors, EEPROM) connected to this interface.
+
 config I2C_BLACKFIN_TWI
tristate Blackfin TWI I2C support
depends on BLACKFIN
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index e6cf294..14e7a76 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_I2C_TEGRA)   += i2c-tegra.o
 obj-$(CONFIG_I2C_VERSATILE)+= i2c-versatile.o
 obj-$(CONFIG_I2C_OCTEON)   += i2c-octeon.o
 obj-$(CONFIG_I2C_XILINX)   += i2c-xiic.o
+obj-$(CONFIG_I2C_XLR)   += i2c-xlr.o
 obj-$(CONFIG_I2C_EG20T) += i2c-eg20t.o
 
 # External I2C/SMBus adapter drivers
diff --git a/drivers/i2c/busses/i2c-xlr.c b/drivers/i2c/busses/i2c-xlr.c
new file mode 100644
index 000..d433b2f
--- /dev/null
+++ b/drivers/i2c/busses/i2c-xlr.c
@@ -0,0 +1,303 @@
+/*
+ * Copyright 2011, Netlogic Microsystems Inc.
+ * Copyright 2004, Matt Porter mpor...@kernel.crashing.org
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed as is without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/init.h
+#include linux/ioport.h
+#include linux/delay.h
+#include linux/errno.h
+#include linux/i2c.h
+#include linux/io.h
+#include linux/platform_device.h
+#include asm/netlogic/xlr/iomap.h
+
+/* XLR I2C REGISTERS */
+#define XLR_I2C_CFG0x00
+#define XLR_I2C_CLKDIV 0x01
+#define XLR_I2C_DEVADDR0x02
+#define XLR_I2C_ADDR   0x03
+#define XLR_I2C_DATAOUT0x04
+#define XLR_I2C_DATAIN 0x05
+#define XLR_I2C_STATUS 0x06
+#define XLR_I2C_STARTXFR   0x07
+#define XLR_I2C_BYTECNT0x08
+#define XLR_I2C_HDSTATIM   0x09
+
+/* XLR I2C REGISTERS FLAGS */
+#define XLR_I2C_BUS_BUSY   0x01
+#define XLR_I2C_SDOEMPTY   0x02
+#define XLR_I2C_RXRDY  0x04
+#define XLR_I2C_ACK_ERR0x08
+#define XLR_I2C_ARB_STARTERR   0x30
+
+/* Register Programming Values!! Change as required */
+#define XLR_I2C_CFG_ADDR   0xF8/* 8-Bit dev Addr + POR Values */
+#define XLR_I2C_CFG_NOADDR 0xFA/* 8-Bit reg Addr + POR Values */
+#define XLR_I2C_STARTXFR_ND0x02/* No data , only addr */
+#define XLR_I2C_STARTXFR_RD0x01/* Read */
+#define XLR_I2C_STARTXFR_WR0x00/* Write */
+#define XLR_I2C_CLKDIV_DEF 0x14A   /* 0x0052 */
+#define XLR_I2C_HDSTATIM_DEF   0x107   /* 0x */
+
+#define XLR_I2C_IO_SIZE0x1000
+
+struct xlr_i2c_private {
+   struct i2c_adapter adap;
+   u32 __iomem *iobase;
+};
+
+static int xlr_i2c_tx(struct xlr_i2c_private *priv,  u16 len,
+   u8 *buf, u16 addr)
+{
+   u32 i2c_status = 0x00;
+   u8 nb;
+   int pos, timeout = 0;
+   struct i2c_adapter *adap = priv-adap;
+   u8 offset = buf[0];
+
+   netlogic_write_reg(priv-iobase, XLR_I2C_ADDR, offset);
+   netlogic_write_reg(priv-iobase, XLR_I2C_DEVADDR, addr);
+   netlogic_write_reg(priv-iobase, XLR_I2C_CFG,

Re: [PATCH 2/2] I2C: OMAP: remove dev-idle, use usage counting provided by runtime PM

2011-08-04 Thread Kevin Hilman 
Felipe Balbi ba...@ti.com writes:

 Hi,

 On Thu, Aug 04, 2011 at 07:53:37AM -0700, Kevin Hilman wrote:
  @@ -1140,6 +1128,36 @@ omap_i2c_remove(struct platform_device *pdev)
return 0;
   }
   
  +#ifdef CONFIG_PM_RUNTIME
  +static int omap_i2c_runtime_suspend(struct device *dev)
  +{
  + struct platform_device *pdev = to_platform_device(dev);
  + struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
 
  what happened to dev_get_drvdata(dev) ??
 
 
 Yes, that would work too since:
 
 static inline void *platform_get_drvdata(const struct platform_device *pdev)
 {
  return dev_get_drvdata(pdev-dev);
 }
 
 but IMO, readability is better if the driver does platform_set_drvdata()
 and then the corresponding platform_get_drvdata()

 fair enough, but if you already have the dev pointer, what's the gain in
 doing a container_of() just to go back to the dev pointer again ?

There is no gain, but there is no loss either.  The compiler is smart
enough that the resulting assembly is the same.

 IMHO, there's really no need for that and just calling dev_get_drvdata()
 straight is enough. All in all, platform_get_[sg]et_drvdata() are just
 wrappers for dev_[sg]et_drvdata(). The whole point, was to avoid
 dev_[sg]et_drvdata(pdev-dev).

Well, whether or not to use dev_[gs]et_* or platform_[gs]et_* is not
relevant to $SUBJECT patch. 

The current driver does platform_set_drvdata(), so I used
platform_get_drvdata() for consistency and readability.  If I were to
use dev_get* then I would want the correpsonding set changed to
dev_set_* also for consistency.

If someone wants to change both the sets and gets to the dev_ versions,
that's fine with me, but it should be separate patch.

Kevin
--
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: add OF binding support

2011-08-04 Thread Rob Herring 
On 08/04/2011 10:45 AM, Rob Herring wrote:
 Ben,
 
 On 08/04/2011 04:12 AM, Ben Dooks wrote:
 On Wed, Aug 03, 2011 at 03:04:23PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 Add of_match_table and DT style i2c registration to designware i2c
 driver.

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c  |   13 
  2 files changed, 36 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 +   i2c@f {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +   compatible = snps,designware-i2c;
 +   reg = 0xf 0x1000;
 +   interrupts = 11;
 +   clock-frequency = 40;
 +   };
 +

 looks good to me.

 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..2911a49 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -37,6 +37,7 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/of_i2c.h
  
  /*
   * Registers offset
 @@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct 
 platform_device *pdev)
 adap-algo = i2c_dw_algo;
 adap-dev.parent = pdev-dev;
  
 +#ifdef CONFIG_OF
 +   r = i2c_add_adapter(adap);
 +#else
 adap-nr = pdev-id;
 r = i2c_add_numbered_adapter(adap);
 +#endif

 I would say that doing the #ifdef CONFIG_OF is dangerous here when we
 are in a mixed OF/platform enviromnent as we're depending on compile
 time selection.

 I'm also wondering whether we have an of helper macro which takes
 a pdev and gives you an adapter number either given on pdev-id or
 -1 for the case when we're using the OF bindings.

 It might be worth talking to Grant about setting pdev-id to -1 if we
 are using an OF device.

 
 As Grant said, that's already done and this hunk is not needed.
 
 if (r) {
 dev_err(pdev-dev, failure adding adapter\n);
 goto err_free_irq;
 }
 +   of_i2c_register_devices(adap);

 If we did that, we could add a of_i2c_register_adapter() call which
 would take the platform device and then do the of_i2c_register_devices()
 and do these steps.

 
 Better yet, how about putting of_i2c_register_devices into
 i2c_register_adapter? Everywhere that calls of_i2c_register_devices is
 preceded by a call to i2c_add_numbered_adapter or i2c_add_adapter. It
 seems logical to put it with i2c_scan_static_board_info. I'll prepare a
 patch to add that and remove all the other callers unless you think
 that's a bad idea.
 

Nevermind. That would be undoing this commit:

of/i2c: Fix module load order issue caused by of_i2c.c

Commit 959e85f7, i2c: add OF-style registration and binding caused a
module dependency loop where of_i2c.c calls functions in i2c-core, and
i2c-core calls of_i2c_register_devices() in of_i2c.  This means that
when i2c support is built as a module when CONFIG_OF is set, then
neither i2c_core nor of_i2c are able to be loaded.

This patch fixes the problem by moving the of_i2c_register_devices()
calls back into the device drivers.  Device drivers already
specifically request the core code to parse the device tree for
devices anyway by setting the of_node pointer, so it isn't a big
deal to also call the registration function.  The drivers just become
slightly more verbose.

Signed-off-by: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Jean Delvare kh...@linux-fr.org

Rob

 return 0;
  
 @@ -819,6 +825,12 @@ static int __devexit dw_i2c_remove(struct 
 platform_device *pdev)
 return 0;
  }
  
 +static const struct of_device_id dw_i2c_of_match[] = {
 +   { .compatible = snps,designware-i2c, },
 +   {},
 +};
 +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 +
  /* work with hotplug and coldplug */
  MODULE_ALIAS(platform:i2c_designware);
  
 @@ -827,6 +839,7 @@ static struct platform_driver dw_i2c_driver = {
 .driver = {
 .name   = i2c_designware,
 .owner  = THIS_MODULE,
 +   .of_match_table = dw_i2c_of_match,

Re: [PATCH] i2c-designware: add OF binding support

2011-08-04 Thread Grant Likely 
On Thu, Aug 4, 2011 at 10:52 PM, Rob Herring robherri...@gmail.com wrote:
 On 08/04/2011 10:45 AM, Rob Herring wrote:
 Ben,

 On 08/04/2011 04:12 AM, Ben Dooks wrote:
 On Wed, Aug 03, 2011 at 03:04:23PM -0500, Rob Herring wrote:
 From: Rob Herring rob.herr...@calxeda.com

 Add of_match_table and DT style i2c registration to designware i2c
 driver.

 Signed-off-by: Rob Herring rob.herr...@calxeda.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: devicetree-disc...@lists.ozlabs.org
 Cc: Ben Dooks ben-li...@fluff.org
 Cc: linux-i2c@vger.kernel.org
 ---
  Documentation/devicetree/bindings/i2c/dw-i2c.txt |   23 
 ++
  drivers/i2c/busses/i2c-designware.c              |   13 
  2 files changed, 36 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/i2c/dw-i2c.txt

 diff --git a/Documentation/devicetree/bindings/i2c/dw-i2c.txt 
 b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 new file mode 100644
 index 000..cbcb404
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/dw-i2c.txt
 @@ -0,0 +1,23 @@
 +* Synopsys DesignWare I2C
 +
 +Required properties :
 +
 + - compatible : should be snps,designware-i2c
 + - reg : Offset and length of the register set for the device
 + - interrupts : IRQ where IRQ is the interrupt number.
 +
 +Recommended properties :
 +
 + - clock-frequency : desired I2C bus clock frequency in Hz.
 +
 +Example :
 +
 +   i2c@f {
 +           #address-cells = 1;
 +           #size-cells = 0;
 +           compatible = snps,designware-i2c;
 +           reg = 0xf 0x1000;
 +           interrupts = 11;
 +           clock-frequency = 40;
 +   };
 +

 looks good to me.

 diff --git a/drivers/i2c/busses/i2c-designware.c 
 b/drivers/i2c/busses/i2c-designware.c
 index b7a51c4..2911a49 100644
 --- a/drivers/i2c/busses/i2c-designware.c
 +++ b/drivers/i2c/busses/i2c-designware.c
 @@ -37,6 +37,7 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/of_i2c.h

  /*
   * Registers offset
 @@ -770,12 +771,17 @@ static int __devinit dw_i2c_probe(struct 
 platform_device *pdev)
     adap-algo = i2c_dw_algo;
     adap-dev.parent = pdev-dev;

 +#ifdef CONFIG_OF
 +   r = i2c_add_adapter(adap);
 +#else
     adap-nr = pdev-id;
     r = i2c_add_numbered_adapter(adap);
 +#endif

 I would say that doing the #ifdef CONFIG_OF is dangerous here when we
 are in a mixed OF/platform enviromnent as we're depending on compile
 time selection.

 I'm also wondering whether we have an of helper macro which takes
 a pdev and gives you an adapter number either given on pdev-id or
 -1 for the case when we're using the OF bindings.

 It might be worth talking to Grant about setting pdev-id to -1 if we
 are using an OF device.


 As Grant said, that's already done and this hunk is not needed.

     if (r) {
             dev_err(pdev-dev, failure adding adapter\n);
             goto err_free_irq;
     }
 +   of_i2c_register_devices(adap);

 If we did that, we could add a of_i2c_register_adapter() call which
 would take the platform device and then do the of_i2c_register_devices()
 and do these steps.


 Better yet, how about putting of_i2c_register_devices into
 i2c_register_adapter? Everywhere that calls of_i2c_register_devices is
 preceded by a call to i2c_add_numbered_adapter or i2c_add_adapter. It
 seems logical to put it with i2c_scan_static_board_info. I'll prepare a
 patch to add that and remove all the other callers unless you think
 that's a bad idea.


 Nevermind. That would be undoing this commit:

    of/i2c: Fix module load order issue caused by of_i2c.c

The other alternative would be to move the i2c dt parsing code into
drivers/i2c.  I'm already planning to do that for spi and gpio dt
parsing code.

g.
--
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