Re: [PATCH] i2c: davinci: Increase module clock frequency
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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