Re: [PATCH] i2c: davinci: Increase module clock frequency

2015-11-19 Thread santosh shilimkar

On 11/19/2015 4:21 AM, Alexander Sverdlin wrote:

I2C controller used in Keystone SoC has an undocumented peculiarity which
results in SDA-SCL margins being dependent on module clock. Driving high
capacity bus near its limits can result in STOP condition sometimes being
understood as REPEATED-START by slaves (or NACK instead of ACK, etc...).
Driving the module with higher clocks increases the margin between SDA and SCL
transitions, making the operations with higher bus rates more robust. Therefore,
target the module clock to 12MHz instead of 7MHz, still staying within
the specification limits.

Before the change STOP timing looked like this on 400kHz:

SDA   --+  +
  \/
   \  /
++
(1)
SCL   --+  +
  \/
   \  /
++
(2)

While only point (1) signals STOP, point (2) could be incorrectly recognized as
repeated-START (almost no margin between SDA and SCL transitions).

After the change there is at least 600ns margin measured between SCL fall and
SDA fall during STOP generation:

SDA   --+  +
  \/
   \  /
++

SCL   --+  +
  \/
   \  /
++
->||<- 600ns
 ->|   |<- tSUSTO

So called tSUSTO (setup time for STOP condition) is still slightly higher than
600ns, so no problem here.

Signed-off-by: Alexander Sverdlin 
---

Nice text artwork.
Acked-by: Santosh Shilimkar 
--
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 0/2] i2c: davinci: Couple of patches to make i2c usable on Keystone SOCs

2013-08-09 Thread Santosh Shilimkar
Wolfram, Sekhar, 

On Wednesday 24 July 2013 08:28 PM, Santosh Shilimkar wrote:
> Keystone SOCs uses the same I2C IP as available on DaVinci SOCs. These
> couple of patches makes it possible to select the I2C_DAVINCI on
> Keystone SOCs.
> 
> Cc: Sekhar Nori 
> Cc: Wolfram Sang 
> 
> Santosh Shilimkar (2):
>   i2c: davinci: remove useless mach/hardware include
>   i2c: davinci: Allow i2c driver available for keystone platforms
> 
Who is queuing up these patches ?

regards,
Santosh

--
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 1/2] i2c: davinci: remove useless mach/hardware include

2013-07-25 Thread Santosh Shilimkar
On Thursday 25 July 2013 05:40 AM, Sekhar Nori wrote:
> On Thursday 25 July 2013 05:58 AM, Santosh Shilimkar wrote:
>> This driver no longer uses definitions from mach/hardware.h.
>> On the other hand, including this header breaks this driver
>> on non-davinci platforms which don't have such a header.
>>
>> Fix it.
>>
>> Cc: Sekhar Nori 
>> Cc: Wolfram Sang 
>>
>> Signed-off-by: Santosh Shilimkar 
> 
> Acked-by: Sekhar Nori 
> 
> One minor nit:
> 
>> ---
>>  drivers/i2c/busses/i2c-davinci.c |1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-davinci.c 
>> b/drivers/i2c/busses/i2c-davinci.c
>> index fa55605..ddaa2a3 100644
>> --- a/drivers/i2c/busses/i2c-davinci.c
>> +++ b/drivers/i2c/busses/i2c-davinci.c
>> @@ -41,7 +41,6 @@
>>  #include 
>>  #include 
>>  
> 
> You can now remove this empty line too.
> 
Yes. Updated patch below with your ack added.

Regards,
Santosh

>From d7ae1c488ebae19028f33abff76c4d684271a7fb Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar 
Date: Tue, 23 Jul 2013 15:40:46 -0400
Subject: [PATCH 1/2] i2c: davinci: remove useless mach/hardware include

This driver no longer uses definitions from mach/hardware.h.
On the other hand, including this header breaks this driver
on non-davinci platforms which don't have such a header.

Fix it.


Cc: Wolfram Sang 

Acked-by: Sekhar Nori 
Signed-off-by: Santosh Shilimkar 
---
 drivers/i2c/busses/i2c-davinci.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index fa55605..5898df3 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -40,8 +40,6 @@
 #include 
 #include 
 #include 
-
-#include 
 #include 
 
 /* - global defines --- */
-- 
1.7.9.5


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


[PATCH 2/2] i2c: davinci: Allow i2c driver available for keystone platforms

2013-07-24 Thread Santosh Shilimkar
Keystone SOCs uses the same I2C IP as available on DaVinci SOCs.
Update the config so that ARCH_KEYSTONE can use it.

Cc: Sekhar Nori 
Cc: Wolfram Sang 

Signed-off-by: Santosh Shilimkar 
---
 drivers/i2c/busses/Kconfig |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index dc6dea6..fcdd321 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -385,7 +385,7 @@ config I2C_CPM
 
 config I2C_DAVINCI
tristate "DaVinci I2C driver"
-   depends on ARCH_DAVINCI
+   depends on ARCH_DAVINCI || ARCH_KEYSTONE
help
  Support for TI DaVinci I2C controller driver.
 
-- 
1.7.9.5

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


[PATCH 1/2] i2c: davinci: remove useless mach/hardware include

2013-07-24 Thread Santosh Shilimkar
This driver no longer uses definitions from mach/hardware.h.
On the other hand, including this header breaks this driver
on non-davinci platforms which don't have such a header.

Fix it.

Cc: Sekhar Nori 
Cc: Wolfram Sang 

Signed-off-by: Santosh Shilimkar 
---
 drivers/i2c/busses/i2c-davinci.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index fa55605..ddaa2a3 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -41,7 +41,6 @@
 #include 
 #include 
 
-#include 
 #include 
 
 /* - global defines --- */
-- 
1.7.9.5

--
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 0/2] i2c: davinci: Couple of patches to make i2c usable on Keystone SOCs

2013-07-24 Thread Santosh Shilimkar
Keystone SOCs uses the same I2C IP as available on DaVinci SOCs. These
couple of patches makes it possible to select the I2C_DAVINCI on
Keystone SOCs.

Cc: Sekhar Nori 
Cc: Wolfram Sang 

Santosh Shilimkar (2):
  i2c: davinci: remove useless mach/hardware include
  i2c: davinci: Allow i2c driver available for keystone platforms

 drivers/i2c/busses/Kconfig   |2 +-
 drivers/i2c/busses/i2c-davinci.c |1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
1.7.9.5

--
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: omap: ensure writes to dev->buf_len are ordered

2012-10-27 Thread Santosh Shilimkar

On Saturday 27 October 2012 09:29 PM, Paul Walmsley wrote:

On Sat, 27 Oct 2012, Santosh Shilimkar wrote:


Another alternative, which I will recommend to just make use of the
read*/wrire* instead __raw versions. The barriers are taken care
already and driver point of view, it is transparent.


Those barriers will disappear if CONFIG_ARM_DMA_MEM_BUFFERABLE is set to
N, so that's probably not the right thing to do in this case.  The barrier
here isn't DMA-related, it's needed due to the design of the driver.


Good point.


In fact the wmb() is probably overkill, since only a compiler reordering
barrier is needed.  It can probably just be barrier().


I agree. Just barrier() is enough to avoid compiler re-ordering.

Regards
Santosh

--
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: omap: ensure writes to dev->buf_len are ordered

2012-10-27 Thread Santosh Shilimkar

On Saturday 27 October 2012 04:31 AM, Paul Walmsley wrote:

Hi Felipe

just two quick comments

On Thu, 25 Oct 2012, Felipe Balbi wrote:


if we allow compiler reorder our writes, we could
fall into a situation where dev->buf_len is reset
for no apparent reason.

This bug was found with a simple script which would
transfer data to an i2c client from 1 to 1024 bytes
(a simple for loop), when we got to transfer sizes
bigger than the fifo size, dev->buf_len was reset
to zero before we had an oportunity to handle XDR
Interrupt. Because dev->buf_len was zero, we entered
omap_i2c_transmit_data() to transfer zero bytes,
which would mean we would just silently exit
omap_i2c_transmit_data() without actually writing
anything to DATA register. That would cause XDR
IRQ to trigger forever and we would never transfer
the remaining bytes.

After adding the memory barrier, we also drop resetting
dev->buf_len to zero in omap_i2c_xfer_msg() because
both omap_i2c_transmit_data() and omap_i2c_receive_data()
will act until dev->buf_len reaches zero, rendering the
other write in omap_i2c_xfer_msg() redundant.

This patch has been tested with pandaboard for a few
iterations of the script mentioned above.

Signed-off-by: Felipe Balbi 
---

This bug has been there forever, but it's quite annoying.
I think it deserves being pushed upstream during this -rc
cycle, but if Wolfram decides to wait until v3.8, I don't
mind.

  drivers/i2c/busses/i2c-omap.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..1ec4e6e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -521,6 +521,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
/* REVISIT: Could the STB bit of I2C_CON be used with probing? */
dev->buf = msg->buf;
dev->buf_len = msg->len;
+   wmb();

omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len);



Would suggest moving the wmb() immediately before the point at which the
interrupt can occur.  Looks to me that's when the OMAP_I2C_CON_REG write
occurs.

Also would suggest adding a comment to clarify what the wmb() is intended
to do.  Maybe something like 'Prevent the compiler from moving earlier
changes to dev->buf and dev->buf_len after the write to CON_REG.  This
write enables interrupts and those variables are used in the interrupt
handler'.


Another alternative, which I will recommend to just make use of the
read*/wrire* instead __raw versions. The barriers are taken care
already and driver point of view, it is transparent.

-->
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..0cd6365 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -265,13 +265,13 @@ static const u8 reg_map_ip_v2[] = {
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
  int reg, u16 val)
 {
-   __raw_writew(val, i2c_dev->base +
+   writew(val, i2c_dev->base +
(i2c_dev->regs[reg] << i2c_dev->reg_shift));
 }

 static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
 {
-   return __raw_readw(i2c_dev->base +
+   return readw(i2c_dev->base +
(i2c_dev->regs[reg] << i2c_dev->reg_shift));
 }

Patch might be damaged because of copy paste.

Regards
Santosh
--
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 4/7] i2c: omap: in case of VERSION_2 read IRQSTATUS_RAW but write to IRQSTATUS

2012-10-25 Thread Santosh Shilimkar

On Thursday 25 October 2012 06:22 PM, Felipe Balbi wrote:

Hi,

On Thu, Oct 25, 2012 at 06:23:57PM +0530, Santosh Shilimkar wrote:

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

on OMAP4+ we want to read IRQSTATUS_RAW register,
instead of IRQSTATUS. The reason being that IRQSTATUS
will only contain the bits which were enabled on
IRQENABLE_SET and that will break when we need to
poll for a certain bit which wasn't enabled as an
IRQ source.

One such case is after we finish converting to
deferred stop bit, we will have to poll for ARDY
bit before returning control for the client driver
in order to prevent us from trying to start a
transfer on a bus which is already busy.

Note, however, that omap-i2c.c needs a big rework
on register definition and register access. Such
work will be done in a separate series of patches.

Cc: Benoit Cousson 
Signed-off-by: Felipe Balbi 
---
  drivers/i2c/busses/i2c-omap.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index b004126..20f9ad6 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -271,8 +271,18 @@ static inline void omap_i2c_write_reg(struct omap_i2c_dev 
*i2c_dev,

  static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
  {
-   return __raw_readw(i2c_dev->base +
+   /* if we are OMAP_I2C_IP_VERSION_2, we need to read from
+* IRQSTATUS_RAW, but write to IRQSTATUS
+*/
+   if ((i2c_dev->dtrev == OMAP_I2C_IP_VERSION_2) &&
+   (reg == OMAP_I2C_STAT_REG)) {

Doing this check on every I2C register read seems to
expensive to me. Can you not sort this in init with some offset
which can be 0 or non zero ?  Sorry in case this is already dicussed.


could be. I didn't go that route because I'm planning a complete rewrite
of all register accesses. The way it's done today is completely broken
and already expensive (with reg_shift and different map tables and so
on).

If it's really a big of a deal, I can try to find another way, maybe
just adding omap_i2c_read_stat() and limit the version check just to
I2C_STAT reads would do it for now...


Its a hot path since you read many I2C register reads, so getting
rid of that additional check will be good.
--
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 5/7] i2c: omap: wait for transfer completion before sending STP bit

2012-10-25 Thread Santosh Shilimkar

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

Later patches will come adding support for
reporting amount of bytes transferred so that
client drivers can count how many bytes are
left to transfer.

This is useful mostly in case of NACKs when
client driver wants to know exactly which
byte got NACKed so it doesn't have to resend
all bytes again.

In order to make that work with OMAP's I2C
controller, we need to prevent sending STP
bit until message is transferred. The reason
behind that is because OMAP_I2C_CNT_REG gets
reset to original value after STP bit is
shifted through I2C_SDA line and that would
prevent us from reading the correct number of
bytes left to transfer.

The full programming model suggested by IP
owner was the following:

- start I2C transfer (without STP bit)
- upon completion or NACK, read I2C_CNT register
- write STP bit to I2C_CON register
- wait for ARDY bit

With this patch we're implementing all steps
except step #2 which will come in a later
patch adding such support.


Will this not break the bisect since CNT and
NACK, completion is added in later patch


Signed-off-by: Felipe Balbi 
---

Apart from above, rest of the change follow
the change log and looks fine tome. The
change is quite drastic so hopefully it
has gone through wider testing.

Regards
santosh


--
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 4/7] i2c: omap: in case of VERSION_2 read IRQSTATUS_RAW but write to IRQSTATUS

2012-10-25 Thread Santosh Shilimkar

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

on OMAP4+ we want to read IRQSTATUS_RAW register,
instead of IRQSTATUS. The reason being that IRQSTATUS
will only contain the bits which were enabled on
IRQENABLE_SET and that will break when we need to
poll for a certain bit which wasn't enabled as an
IRQ source.

One such case is after we finish converting to
deferred stop bit, we will have to poll for ARDY
bit before returning control for the client driver
in order to prevent us from trying to start a
transfer on a bus which is already busy.

Note, however, that omap-i2c.c needs a big rework
on register definition and register access. Such
work will be done in a separate series of patches.

Cc: Benoit Cousson 
Signed-off-by: Felipe Balbi 
---
  drivers/i2c/busses/i2c-omap.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index b004126..20f9ad6 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -271,8 +271,18 @@ static inline void omap_i2c_write_reg(struct omap_i2c_dev 
*i2c_dev,

  static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
  {
-   return __raw_readw(i2c_dev->base +
+   /* if we are OMAP_I2C_IP_VERSION_2, we need to read from
+* IRQSTATUS_RAW, but write to IRQSTATUS
+*/
+   if ((i2c_dev->dtrev == OMAP_I2C_IP_VERSION_2) &&
+   (reg == OMAP_I2C_STAT_REG)) {

Doing this check on every I2C register read seems to
expensive to me. Can you not sort this in init with some offset
which can be 0 or non zero ?  Sorry in case this is already dicussed.

regards
santosh
--
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 3/7] i2c: omap: also complete() when stat becomes zero

2012-10-25 Thread Santosh Shilimkar

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

In case we loop on IRQ handler until stat is
finally zero, we would end up in a situation
where all I2C transfers would misteriously
timeout because we were not calling complete()
in that situation.

Fix the issue by moving omap_i2c_complete_cmd()
call inside the 'out' label.

Signed-off-by: Felipe Balbi 
---

Looks fine. Have you hit this issue in any corner case ?

Regards
santosh

--
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 2/7] i2c: omap: reorder exit path of omap_i2c_xfer_msg()

2012-10-25 Thread Santosh Shilimkar

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

just a cleanup patch trying to make exit path
more straightforward. No changes otherwise.

Signed-off-by: Felipe Balbi 
---
  drivers/i2c/busses/i2c-omap.c | 26 +-
  1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index c07d9c4..bea0277 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -505,6 +505,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
  {
struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
unsigned long timeout;
+   int ret;

You might want to initialize the error value to avoid return 0. Compiler
might be already cribbing for it


u16 w;

dev_dbg(dev->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
@@ -582,31 +583,38 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
dev->buf_len = 0;
if (timeout == 0) {
dev_err(dev->dev, "controller timed out\n");
-   omap_i2c_init(dev);
-   return -ETIMEDOUT;
+   ret = -ETIMEDOUT;
+   goto err_i2c_init;
}

-   if (likely(!dev->cmd_err))
-   return 0;
-

Have you have drooped this check completely ?


/* We have an error */
if (dev->cmd_err & (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
OMAP_I2C_STAT_XUDF)) {
-   omap_i2c_init(dev);
-   return -EIO;
+   ret = -EIO;
+   goto err_i2c_init;
}

if (dev->cmd_err & OMAP_I2C_STAT_NACK) {
if (msg->flags & I2C_M_IGNORE_NAK)
return 0;
+
if (stop) {
w = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
w |= OMAP_I2C_CON_STP;
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
}
-   return -EREMOTEIO;
+
+   ret = -EREMOTEIO;
+   goto err;
}
-   return -EIO;
+
+   return 0;

With initialized value you can use
return ret;

Regards
Santosh
--
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 1/7] i2c: omap: no need to access platform_device

2012-10-25 Thread Santosh Shilimkar

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

PM callbacks pass our device pointer as argument
and we don't need to access the platform_device
just to dereference that down to dev->drvdata.

instead, just use dev_get_drvdata() directly.

Signed-off-by: Felipe Balbi 
---

Acked-by: Santosh Shilimkar 
--
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: [PATCHV2 5/5] OMAP: I2C: Restore only if context is lost

2011-07-21 Thread Santosh Shilimkar

On 7/21/2011 4:39 PM, Shubhrajyoti D wrote:

Currently restore is done always.
Adding conditional restore. The restore is done only if the context is lost.


Minor comment. You may want to say 'i2c register restore'
instead of just 'restore'


Signed-off-by: Shubhrajyoti D

o.w
Acked-by: Santosh Shilimkar 

--
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: [PATCHV2 4/5] OMAP: I2C: Remove the SYSC register definition

2011-07-21 Thread Santosh Shilimkar

On 7/21/2011 4:39 PM, Shubhrajyoti D wrote:

The SYSC register should not accessed in the driver removing the
define from the driver.

Signed-off-by: Shubhrajyoti D


Looks good.
Acked-by: Santosh Shilimkar 

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

2011-07-21 Thread Santosh Shilimkar

On 7/21/2011 4:39 PM, Shubhrajyoti D wrote:

The reset in the driver at init is not needed
anymore as the hwmod framework takes care of
reseting it.

Signed-off-by: Shubhrajyoti D
---
  drivers/i2c/busses/i2c-omap.c |   73 
  1 files changed, 22 insertions(+), 51 deletions(-)


Excellent cleanup. Some minor coments



diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 8f87a37..8dc54d5 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -155,9 +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)
@@ -182,6 +179,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;
@@ -332,7 +330,6 @@ 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 platform_device *pdev;
@@ -341,53 +338,16 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
pdev = to_platform_device(dev->dev);
pdata = pdev->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,
-   dev->syscstate);
-   /*
-* Enabling all wakup sources to stop I2C freezing on
-* WFI instruction.
-* REVISIT: Some wkup sources might not be needed.
-*/
-   dev->westate = OMAP_I2C_WE_ALL;
-   if (dev->rev<  OMAP_I2C_REV_ON_3530_4430)
-   omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
-   dev->westate);
-   }
+   if (dev->rev>= OMAP_I2C_REV_ON_3430) {

Space please if (dev->rev >= OMAP_I2C_REV_ON_3430) {

+   /*
+* Enabling all wakup sources to stop I2C freezing on
+* WFI instruction.
+* REVISIT: Some wkup sources might not be needed.
+*/

Surely not related to your patch. But the 'REVISIT:' caught
my attention. Is the comment still valid.


+   dev->westate = OMAP_I2C_WE_ALL;
+   if (dev->rev<  OMAP_I2C_REV_ON_3530_4430)

Space if (dev->rev <  OMAP_I2C_REV_ON_3530_4430)

+   omap_i2c_write_reg(dev, OMAP_I2C_WE_RE

Re: [PATCHV2 2/5] OMAP: I2C: Reset support

2011-07-21 Thread Santosh Shilimkar

On 7/21/2011 4:39 PM, Shubhrajyoti D wrote:

Under some error conditions the i2c  driver may do a reset
adding support in the platform.

Signed-off-by: Shubhrajyoti D
---

As commented on 1/5, this one should be folded
with 1/5 unless and until you have some valid reason.

Regards
Santosh
--
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: [PATCHV2 1/5] OMAP: I2C: Add a device reset field to platform data

2011-07-21 Thread Santosh Shilimkar

Shubro,

On 7/21/2011 4:39 PM, Shubhrajyoti D wrote:

Under some conditions the driver may want to do a reset
of the device. Adding a reset field  to the platform
data.

Signed-off-by: Shubhrajyoti D
---
  include/linux/i2c-omap.h |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

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);

Any reason you haven't clubbed this with the omap i2c reset
implementation patch ?
Since i2c-omap.h isn't a generic header file and specific
to omap i2c drive, you could combine 1/5 and 2/5


--
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] omap: i2c: Add i2c support on omap4 platform

2010-02-19 Thread Santosh Shilimkar
This patch is rebased version of earlier post to add I2C
driver support to OMAP4 platform. On OMAP4, all
I2C register address offsets are changed from OMAP1/2/3 I2C.
In order to not have #ifdef's at various places in code,
as well as to support multi-OMAP build, an array is created
to hold the register addresses with it's offset.

This patch was submitted, reviewed and acked on mailing list
already. For more details refer below link
http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg02281.html

This updated verion has a depedancy on "Add support for 16-bit registers"
posted on linux-omap. Below is the patch-works link for the same

http://patchwork.kernel.org/patch/72295/

Signed-off-by: Syed Rafiuddin 
Signed-off-by: Santosh Shilimkar 
Acked-by: Kevin Hilman 
Reviewed-by: Paul Walmsley 
Reviewed-by: Tony Lindgren 
Cc: Ben Dooks 
Cc: Cory Maccarrone 
---
 drivers/i2c/busses/i2c-omap.c |  146 -
 1 files changed, 114 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 9c3ce4d..7c15496 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -44,29 +44,37 @@
 /* I2C controller revisions present on specific hardware */
 #define OMAP_I2C_REV_ON_2430   0x36
 #define OMAP_I2C_REV_ON_3430   0x3C
+#define OMAP_I2C_REV_ON_4430   0x40
 
 /* timeout waiting for the controller to respond */
 #define OMAP_I2C_TIMEOUT (msecs_to_jiffies(1000))
 
-#define OMAP_I2C_REV_REG   0x00
-#define OMAP_I2C_IE_REG0x01
-#define OMAP_I2C_STAT_REG  0x02
-#define OMAP_I2C_IV_REG0x03
 /* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */
-#define OMAP_I2C_WE_REG0x03
-#define OMAP_I2C_SYSS_REG  0x04
-#define OMAP_I2C_BUF_REG   0x05
-#define OMAP_I2C_CNT_REG   0x06
-#define OMAP_I2C_DATA_REG  0x07
-#define OMAP_I2C_SYSC_REG  0x08
-#define OMAP_I2C_CON_REG   0x09
-#define OMAP_I2C_OA_REG0x0a
-#define OMAP_I2C_SA_REG0x0b
-#define OMAP_I2C_PSC_REG   0x0c
-#define OMAP_I2C_SCLL_REG  0x0d
-#define OMAP_I2C_SCLH_REG  0x0e
-#define OMAP_I2C_SYSTEST_REG   0x0f
-#define OMAP_I2C_BUFSTAT_REG   0x10
+enum {
+   OMAP_I2C_REV_REG = 0,
+   OMAP_I2C_IE_REG,
+   OMAP_I2C_STAT_REG,
+   OMAP_I2C_IV_REG,
+   OMAP_I2C_WE_REG,
+   OMAP_I2C_SYSS_REG,
+   OMAP_I2C_BUF_REG,
+   OMAP_I2C_CNT_REG,
+   OMAP_I2C_DATA_REG,
+   OMAP_I2C_SYSC_REG,
+   OMAP_I2C_CON_REG,
+   OMAP_I2C_OA_REG,
+   OMAP_I2C_SA_REG,
+   OMAP_I2C_PSC_REG,
+   OMAP_I2C_SCLL_REG,
+   OMAP_I2C_SCLH_REG,
+   OMAP_I2C_SYSTEST_REG,
+   OMAP_I2C_BUFSTAT_REG,
+   OMAP_I2C_REVNB_LO,
+   OMAP_I2C_REVNB_HI,
+   OMAP_I2C_IRQSTATUS_RAW,
+   OMAP_I2C_IRQENABLE_SET,
+   OMAP_I2C_IRQENABLE_CLR,
+};
 
 /* I2C Interrupt Enable Register (OMAP_I2C_IE): */
 #define OMAP_I2C_IE_XDR(1 << 14)   /* TX Buffer drain int 
enable */
@@ -169,6 +177,7 @@ struct omap_i2c_dev {
u32 speed;  /* Speed of bus in Khz */
u16 cmd_err;
u8  *buf;
+   u8  *regs;
size_t  buf_len;
struct i2c_adapter  adapter;
u8  fifo_size;  /* use as flag and value
@@ -187,15 +196,64 @@ struct omap_i2c_dev {
u16 westate;
 };
 
+const static u8 reg_map[] = {
+   [OMAP_I2C_REV_REG] = 0x00,
+   [OMAP_I2C_IE_REG] = 0x01,
+   [OMAP_I2C_STAT_REG] = 0x02,
+   [OMAP_I2C_IV_REG] = 0x03,
+   [OMAP_I2C_WE_REG] = 0x03,
+   [OMAP_I2C_SYSS_REG] = 0x04,
+   [OMAP_I2C_BUF_REG] = 0x05,
+   [OMAP_I2C_CNT_REG] = 0x06,
+   [OMAP_I2C_DATA_REG] = 0x07,
+   [OMAP_I2C_SYSC_REG] = 0x08,
+   [OMAP_I2C_CON_REG] = 0x09,
+   [OMAP_I2C_OA_REG] = 0x0a,
+   [OMAP_I2C_SA_REG] = 0x0b,
+   [OMAP_I2C_PSC_REG] = 0x0c,
+   [OMAP_I2C_SCLL_REG] = 0x0d,
+   [OMAP_I2C_SCLH_REG] = 0x0e,
+   [OMAP_I2C_SYSTEST_REG] = 0x0f,
+   [OMAP_I2C_BUFSTAT_REG] = 0x10,
+};
+
+const static u8 omap4_reg_map[] = {
+   [OMAP_I2C_REV_REG] = 0x04,
+   [OMAP_I2C_IE_REG] = 0x2c,
+   [OMAP_I2C_STAT_REG] = 0x28,
+   [OMAP_I2C_IV_REG] = 0x34,
+   [OMAP_I2C_WE_REG] = 0x34,
+   [OMAP_I2C_SYSS_REG] = 0x90,
+   [OMAP_I2C_BUF_REG] = 0x94,
+   [OMAP_I2C_CNT_REG] = 0x98,
+   [OMAP_I2C_DATA_REG] = 0x9c,
+   [OMAP_I2C_SYSC_REG] = 0x20,
+   [OMAP_I2C_CON_REG] = 0xa4,
+   [OMAP_I2C_OA_REG] = 0xa8,
+   [OMAP_I2C_SA_REG] = 0xac,
+   [OMAP_I2C_PSC_REG] = 0xb0,
+   [OMAP_I2C_SCLL_REG] = 0xb4,
+   [OMAP_I2C_SCLH_REG] = 0xb8,
+   [OM