public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 04/12] I2C re-init for every cmd
@ 2008-09-01 13:38 Rajendra Nayak
  2008-09-11 10:04 ` Högander Jouni
  0 siblings, 1 reply; 12+ messages in thread
From: Rajendra Nayak @ 2008-09-01 13:38 UTC (permalink / raw)
  To: linux-omap

This patch does i2c init/re-init for every transfer

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/i2c/busses/i2c-omap.c |    2 ++
 1 files changed, 2 insertions(+)

Index: linux-omap-2.6/drivers/i2c/busses/i2c-omap.c
===================================================================
--- linux-omap-2.6.orig/drivers/i2c/busses/i2c-omap.c	2008-09-01
18:11:28.000000000 +0530
+++ linux-omap-2.6/drivers/i2c/busses/i2c-omap.c	2008-09-01 18:11:52.000000000
+0530
@@ -496,6 +496,8 @@ omap_i2c_xfer(struct i2c_adapter *adap,

 	omap_i2c_unidle(dev);

+	omap_i2c_init(dev);
+
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-01 13:38 [PATCH 04/12] I2C re-init for every cmd Rajendra Nayak
@ 2008-09-11 10:04 ` Högander Jouni
  2008-09-11 10:33   ` shekhar, chandra
  0 siblings, 1 reply; 12+ messages in thread
From: Högander Jouni @ 2008-09-11 10:04 UTC (permalink / raw)
  To: ext Rajendra Nayak; +Cc: linux-omap

"ext Rajendra Nayak" <rnayak@ti.com> writes:

> This patch does i2c init/re-init for every transfer
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  drivers/i2c/busses/i2c-omap.c |    2 ++
>  1 files changed, 2 insertions(+)
>
> Index: linux-omap-2.6/drivers/i2c/busses/i2c-omap.c
> ===================================================================
> --- linux-omap-2.6.orig/drivers/i2c/busses/i2c-omap.c	2008-09-01
> 18:11:28.000000000 +0530
> +++ linux-omap-2.6/drivers/i2c/busses/i2c-omap.c	2008-09-01 18:11:52.000000000
> +0530
> @@ -496,6 +496,8 @@ omap_i2c_xfer(struct i2c_adapter *adap,
>
>  	omap_i2c_unidle(dev);
>
> +	omap_i2c_init(dev);
> +
>  	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>  		goto out;

This is causing unacceptable delays on i2c transfers. This is because
occasionally reset loop in init function enters msleep. Is it
necessary to reset i2c controller after off-mode?

Any opinions on this:

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3778735..4a27035 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -126,6 +126,14 @@
 /* I2C System Configuration Register (OMAP_I2C_SYSC): */
 #define OMAP_I2C_SYSC_SRST             (1 << 1)        /* Soft Reset */
 
+struct omap3_i2c_regs {
+       u16 sysc;
+       u16 psc;
+       u16 scll;
+       u16 sclh;
+       u16 buf;
+};
+
 struct omap_i2c_dev {
        struct device           *dev;
        void __iomem            *base;          /* virtual */
@@ -147,6 +155,9 @@ struct omap_i2c_dev {
        unsigned                b_hw:1;         /* bad h/w fixes */
        unsigned                idle:1;
        u16                     iestate;        /* Saved interrupt register */
+#ifdef CONFIG_ARCH_OMAP34XX
+       struct omap3_i2c_regs   context;
+#endif
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -160,6 +171,26 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
        return __raw_readw(i2c_dev->base + reg);
 }
 
+#ifdef CONFIG_ARCH_OMAP34XX
+void omap3_i2c_save_context(struct omap_i2c_dev *dev)
+{
+       dev->context.sysc = omap_i2c_read_reg(dev, OMAP_I2C_SYSC_REG);
+       dev->context.psc = omap_i2c_read_reg(dev, OMAP_I2C_PSC_REG);
+       dev->context.scll = omap_i2c_read_reg(dev, OMAP_I2C_SCLL_REG);
+       dev->context.sclh = omap_i2c_read_reg(dev, OMAP_I2C_SCLH_REG);
+       dev->context.buf = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG);
+}
+
+void omap3_i2c_restore_context(struct omap_i2c_dev *dev)
+{
+       omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, dev->context.sysc);
+       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->context.psc);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->context.scll);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->context.sclh);
+       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->context.buf);
+}
+#endif
+
 static int __init omap_i2c_get_clocks(struct omap_i2c_dev *dev)
 {
        if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
@@ -210,6 +241,10 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
                clk_enable(dev->iclk);
        clk_enable(dev->fclk);
        dev->idle = 0;
+
+       if (cpu_is_omap34xx())
+               omap3_i2c_restore_context(dev);
+
        if (dev->iestate)
                omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }
@@ -344,6 +379,10 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
                                (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
+
+       if (cpu_is_omap34xx())
+               omap3_i2c_save_context(dev);
+
        return 0;
 }
 
@@ -496,8 +535,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
        omap_i2c_unidle(dev);
 
-       omap_i2c_init(dev);
-
        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
                goto out;

-- 
Jouni Högander

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-11 10:04 ` Högander Jouni
@ 2008-09-11 10:33   ` shekhar, chandra
  2008-09-11 11:01     ` Högander Jouni
  0 siblings, 1 reply; 12+ messages in thread
From: shekhar, chandra @ 2008-09-11 10:33 UTC (permalink / raw)
  To: "Högander" Jouni, ext Rajendra Nayak; +Cc: linux-omap

Hi,

Few comments inlined.

Regards,
Chandra
----- Original Message ----- 
From: ""Högander" Jouni" <jouni.hogander@nokia.com>
To: "ext Rajendra Nayak" <rnayak@ti.com>
Cc: <linux-omap@vger.kernel.org>
Sent: Thursday, September 11, 2008 3:34 PM
Subject: Re: [PATCH 04/12] I2C re-init for every cmd


"ext Rajendra Nayak" <rnayak@ti.com> writes:

> This patch does i2c init/re-init for every transfer
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  drivers/i2c/busses/i2c-omap.c |    2 ++
>  1 files changed, 2 insertions(+)
>
> Index: linux-omap-2.6/drivers/i2c/busses/i2c-omap.c
> ===================================================================
> --- linux-omap-2.6.orig/drivers/i2c/busses/i2c-omap.c 2008-09-01
> 18:11:28.000000000 +0530
> +++ linux-omap-2.6/drivers/i2c/busses/i2c-omap.c 2008-09-01 18:11:52.000000000
> +0530
> @@ -496,6 +496,8 @@ omap_i2c_xfer(struct i2c_adapter *adap,
>
>  omap_i2c_unidle(dev);
>
> + omap_i2c_init(dev);
> +
>  if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>  goto out;

This is causing unacceptable delays on i2c transfers. This is because
occasionally reset loop in init function enters msleep. Is it
necessary to reset i2c controller after off-mode?

Any opinions on this:

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3778735..4a27035 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -126,6 +126,14 @@
 /* I2C System Configuration Register (OMAP_I2C_SYSC): */
 #define OMAP_I2C_SYSC_SRST             (1 << 1)        /* Soft Reset */

+struct omap3_i2c_regs {
+       u16 sysc;
+       u16 psc;
+       u16 scll;
+       u16 sclh;
+       u16 buf;
+};
+



We can add this as a part of controller structure itself instaed of creating a 
new structure. we will store this
psc, scll and others as a part of controller and just restore it. That way we 
can do away with  omap3_i2c_save_context every time in clk disable.





 struct omap_i2c_dev {
        struct device           *dev;
        void __iomem            *base;          /* virtual */
@@ -147,6 +155,9 @@ struct omap_i2c_dev {
        unsigned                b_hw:1;         /* bad h/w fixes */
        unsigned                idle:1;
        u16                     iestate;        /* Saved interrupt register */
+#ifdef CONFIG_ARCH_OMAP34XX
+       struct omap3_i2c_regs   context;
+#endif



I guess this should work for all the platforms. at least for omap2/3 it should 
work w/o any hiccups.
I2C_BUF is the only register which will require cpu check..so while restoring we 
can put that check for 2430/34xx.


 };



 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -160,6 +171,26 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev 
*i2c_dev, int reg)
        return __raw_readw(i2c_dev->base + reg);
 }

+#ifdef CONFIG_ARCH_OMAP34XX
+void omap3_i2c_save_context(struct omap_i2c_dev *dev)
+{
+       dev->context.sysc = omap_i2c_read_reg(dev, OMAP_I2C_SYSC_REG);
+       dev->context.psc = omap_i2c_read_reg(dev, OMAP_I2C_PSC_REG);
+       dev->context.scll = omap_i2c_read_reg(dev, OMAP_I2C_SCLL_REG);
+       dev->context.sclh = omap_i2c_read_reg(dev, OMAP_I2C_SCLH_REG);
+       dev->context.buf = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG);
+}
+


+void omap3_i2c_restore_context(struct omap_i2c_dev *dev)
+{
+       omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, dev->context.sysc);
+       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->context.psc);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->context.scll);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->context.sclh);
+       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->context.buf);
+}
+#endif
+

 static int __init omap_i2c_get_clocks(struct omap_i2c_dev *dev)
 {
        if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
@@ -210,6 +241,10 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
                clk_enable(dev->iclk);
        clk_enable(dev->fclk);
        dev->idle = 0;
+
+       if (cpu_is_omap34xx())
+               omap3_i2c_restore_context(dev);
+
        if (dev->iestate)
                omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }
@@ -344,6 +379,10 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
                                (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
+
+       if (cpu_is_omap34xx())
+               omap3_i2c_save_context(dev);
+



this will become redundant as data will be part of controller structure.


        return 0;
 }

@@ -496,8 +535,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)

        omap_i2c_unidle(dev);

-       omap_i2c_init(dev);
-


instead of calling restore from clk_enable we can replace omap_i2c_init by 
omap3_i2c_restore_context. that way clock enable code will be clean.



        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
                goto out;

-- 
Jouni Högander

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

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-11 10:33   ` shekhar, chandra
@ 2008-09-11 11:01     ` Högander Jouni
  0 siblings, 0 replies; 12+ messages in thread
From: Högander Jouni @ 2008-09-11 11:01 UTC (permalink / raw)
  To: ext shekhar, chandra; +Cc: ext Rajendra Nayak, linux-omap

"ext shekhar, chandra" <x0044955@ti.com> writes:

> Hi,
>
> Few comments inlined.
>
> Regards,
> Chandra
> ----- Original Message ----- 
> From: ""Högander" Jouni" <jouni.hogander@nokia.com>
> To: "ext Rajendra Nayak" <rnayak@ti.com>
> Cc: <linux-omap@vger.kernel.org>
> Sent: Thursday, September 11, 2008 3:34 PM
> Subject: Re: [PATCH 04/12] I2C re-init for every cmd
>
>
> "ext Rajendra Nayak" <rnayak@ti.com> writes:
>
>> This patch does i2c init/re-init for every transfer
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> ---
>>  drivers/i2c/busses/i2c-omap.c |    2 ++
>>  1 files changed, 2 insertions(+)
>>
>> Index: linux-omap-2.6/drivers/i2c/busses/i2c-omap.c
>> ===================================================================
>> --- linux-omap-2.6.orig/drivers/i2c/busses/i2c-omap.c 2008-09-01
>> 18:11:28.000000000 +0530
>> +++ linux-omap-2.6/drivers/i2c/busses/i2c-omap.c 2008-09-01 18:11:52.000000000
>> +0530
>> @@ -496,6 +496,8 @@ omap_i2c_xfer(struct i2c_adapter *adap,
>>
>>  omap_i2c_unidle(dev);
>>
>> + omap_i2c_init(dev);
>> +
>>  if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>>  goto out;
>
> This is causing unacceptable delays on i2c transfers. This is because
> occasionally reset loop in init function enters msleep. Is it
> necessary to reset i2c controller after off-mode?
>
> Any opinions on this:
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 3778735..4a27035 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -126,6 +126,14 @@
> /* I2C System Configuration Register (OMAP_I2C_SYSC): */
> #define OMAP_I2C_SYSC_SRST             (1 << 1)        /* Soft Reset */
>
> +struct omap3_i2c_regs {
> +       u16 sysc;
> +       u16 psc;
> +       u16 scll;
> +       u16 sclh;
> +       u16 buf;
> +};
> +
>
>
>
> We can add this as a part of controller structure itself instaed of
> creating a new structure. we will store this
> psc, scll and others as a part of controller and just restore it. That
> way we can do away with  omap3_i2c_save_context every time in clk
> disable.

My patch is neither saving ctx on every clk disable. Just after init.

>
>
>
>
>
> struct omap_i2c_dev {
>        struct device           *dev;
>        void __iomem            *base;          /* virtual */
> @@ -147,6 +155,9 @@ struct omap_i2c_dev {
>        unsigned                b_hw:1;         /* bad h/w fixes */
>        unsigned                idle:1;
>        u16                     iestate;        /* Saved interrupt register */
> +#ifdef CONFIG_ARCH_OMAP34XX
> +       struct omap3_i2c_regs   context;
> +#endif
>
>
>
> I guess this should work for all the platforms. at least for omap2/3
> it should work w/o any hiccups.
> I2C_BUF is the only register which will require cpu check..so while
> restoring we can put that check for 2430/34xx.

Can we use off mode with omap2? Generally, should we build in
save/restore code for other than omap3?

>
>
> };
>
>
>
> static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
> @@ -160,6 +171,26 @@ static inline u16 omap_i2c_read_reg(struct
> omap_i2c_dev *i2c_dev, int reg)
>        return __raw_readw(i2c_dev->base + reg);
> }
>
> +#ifdef CONFIG_ARCH_OMAP34XX
> +void omap3_i2c_save_context(struct omap_i2c_dev *dev)
> +{
> +       dev->context.sysc = omap_i2c_read_reg(dev, OMAP_I2C_SYSC_REG);
> +       dev->context.psc = omap_i2c_read_reg(dev, OMAP_I2C_PSC_REG);
> +       dev->context.scll = omap_i2c_read_reg(dev, OMAP_I2C_SCLL_REG);
> +       dev->context.sclh = omap_i2c_read_reg(dev, OMAP_I2C_SCLH_REG);
> +       dev->context.buf = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG);
> +}
> +
>
>
> +void omap3_i2c_restore_context(struct omap_i2c_dev *dev)
> +{
> +       omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, dev->context.sysc);
> +       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->context.psc);
> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->context.scll);
> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->context.sclh);
> +       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->context.buf);
> +}
> +#endif
> +
>
> static int __init omap_i2c_get_clocks(struct omap_i2c_dev *dev)
> {
>        if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
> @@ -210,6 +241,10 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
>                clk_enable(dev->iclk);
>        clk_enable(dev->fclk);
>        dev->idle = 0;
> +
> +       if (cpu_is_omap34xx())
> +               omap3_i2c_restore_context(dev);
> +
>        if (dev->iestate)
>                omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> }
> @@ -344,6 +379,10 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
>                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
>                                (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
> +
> +       if (cpu_is_omap34xx())
> +               omap3_i2c_save_context(dev);
> +
>
>
>
> this will become redundant as data will be part of controller
> structure.
>
>
>        return 0;
> }
>
> @@ -496,8 +535,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct
> i2c_msg msgs[], int num)
>
>        omap_i2c_unidle(dev);
>
> -       omap_i2c_init(dev);
> -
>
>
> instead of calling restore from clk_enable we can replace
> omap_i2c_init by omap3_i2c_restore_context. that way clock enable code
> will be clean.

Well, I though unidle could be ok as ie reg is already restored there?

>
>
>
>        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>                goto out;
>
> -- 
> Jouni Högander
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

-- 
Jouni Högander

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
@ 2008-09-11 15:55 chandra shekhar
  2008-09-12  7:48 ` Högander Jouni
  0 siblings, 1 reply; 12+ messages in thread
From: chandra shekhar @ 2008-09-11 15:55 UTC (permalink / raw)
  To: jouni.hogander; +Cc: linux-omap

> Hi,
>
> Few comments inlined.
>
> Regards,
> Chandra
> ----- Original Message -----
> From: ""Högander" Jouni" <jouni.hogander@nokia.com>
> To: "ext Rajendra Nayak" <rnayak@ti.com>
> Cc: <linux-omap@vger.kernel.org>
> Sent: Thursday, September 11, 2008 3:34 PM
> Subject: Re: [PATCH 04/12] I2C re-init for every cmd
>
>
> "ext Rajendra Nayak" <rnayak@ti.com> writes:
>
>> This patch does i2c init/re-init for every transfer
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> ---
>>  drivers/i2c/busses/i2c-omap.c |    2 ++
>>  1 files changed, 2 insertions(+)
>>
>> Index: linux-omap-2.6/drivers/i2c/busses/i2c-omap.c
>> ===================================================================
>> --- linux-omap-2.6.orig/drivers/i2c/busses/i2c-omap.c 2008-09-01
>> 18:11:28.000000000 +0530
>> +++ linux-omap-2.6/drivers/i2c/busses/i2c-omap.c 2008-09-01 18:11:52.000000000
>> +0530
>> @@ -496,6 +496,8 @@ omap_i2c_xfer(struct i2c_adapter *adap,
>>
>>  omap_i2c_unidle(dev);
>>
>> + omap_i2c_init(dev);
>> +
>>  if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>>  goto out;
>
> This is causing unacceptable delays on i2c transfers. This is because
> occasionally reset loop in init function enters msleep. Is it
> necessary to reset i2c controller after off-mode?
>
> Any opinions on this:
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 3778735..4a27035 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -126,6 +126,14 @@
> /* I2C System Configuration Register (OMAP_I2C_SYSC): */
> #define OMAP_I2C_SYSC_SRST             (1 << 1)        /* Soft Reset */
>
> +struct omap3_i2c_regs {
> +       u16 sysc;
> +       u16 psc;
> +       u16 scll;
> +       u16 sclh;
> +       u16 buf;
> +};
> +
>
>
>
> We can add this as a part of controller structure itself instaed of
> creating a new structure. we will store this
> psc, scll and others as a part of controller and just restore it. That
> way we can do away with  omap3_i2c_save_context every time in clk
> disable.

>My patch is neither saving ctx on every clk disable. Just after init.
>
>
>
>
>
> struct omap_i2c_dev {
>        struct device           *dev;
>        void __iomem            *base;          /* virtual */
> @@ -147,6 +155,9 @@ struct omap_i2c_dev {
>        unsigned                b_hw:1;         /* bad h/w fixes */
>        unsigned                idle:1;
>        u16                     iestate;        /* Saved interrupt register */
> +#ifdef CONFIG_ARCH_OMAP34XX
> +       struct omap3_i2c_regs   context;
> +#endif
>
>
>
> I guess this should work for all the platforms. at least for omap2/3
> it should work w/o any hiccups.
> I2C_BUF is the only register which will require cpu check..so while
> restoring we can put that check for 2430/34xx.

>Can we use off mode with omap2? Generally, should we build in
>save/restore code for other than omap3?

agreed..but i guess it will not compile for 2430 and prev. platforms.
omap3_i2c_save_context and restore definition is under macro
while call is not. let me know if i am missing something.

also is SYSC register restore needed??

>
>
> };
>
>
>
> static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
> @@ -160,6 +171,26 @@ static inline u16 omap_i2c_read_reg(struct
> omap_i2c_dev *i2c_dev, int reg)
>        return __raw_readw(i2c_dev->base + reg);
> }
>
> +#ifdef CONFIG_ARCH_OMAP34XX
> +void omap3_i2c_save_context(struct omap_i2c_dev *dev)
> +{
> +       dev->context.sysc = omap_i2c_read_reg(dev, OMAP_I2C_SYSC_REG);
> +       dev->context.psc = omap_i2c_read_reg(dev, OMAP_I2C_PSC_REG);
> +       dev->context.scll = omap_i2c_read_reg(dev, OMAP_I2C_SCLL_REG);
> +       dev->context.sclh = omap_i2c_read_reg(dev, OMAP_I2C_SCLH_REG);
> +       dev->context.buf = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG);
> +}
> +
>
>
> +void omap3_i2c_restore_context(struct omap_i2c_dev *dev)
> +{
> +       omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, dev->context.sysc);
> +       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->context.psc);
> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->context.scll);
> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->context.sclh);
> +       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->context.buf);
> +}
> +#endif
> +
>
> static int __init omap_i2c_get_clocks(struct omap_i2c_dev *dev)
> {
>        if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
> @@ -210,6 +241,10 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
>                clk_enable(dev->iclk);
>        clk_enable(dev->fclk);
>        dev->idle = 0;
> +
> +       if (cpu_is_omap34xx())
> +               omap3_i2c_restore_context(dev);
> +
>        if (dev->iestate)
>                omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> }
> @@ -344,6 +379,10 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
>                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
>                                (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
> +
> +       if (cpu_is_omap34xx())
> +               omap3_i2c_save_context(dev);
> +
>
>
>
> this will become redundant as data will be part of controller
> structure.
>
>
>        return 0;
> }
>
> @@ -496,8 +535,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct
> i2c_msg msgs[], int num)
>
>        omap_i2c_unidle(dev);
>
> -       omap_i2c_init(dev);
> -
>
>
> instead of calling restore from clk_enable we can replace
> omap_i2c_init by omap3_i2c_restore_context. that way clock enable code
> will be clean.

Well, I though unidle could be ok as ie reg is already restored there?

>
>
>
>        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>                goto out;
>
> --
> Jouni Högander
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-11 15:55 [PATCH 04/12] I2C re-init for every cmd chandra shekhar
@ 2008-09-12  7:48 ` Högander Jouni
  2008-09-15  6:57   ` shekhar, chandra
  2008-09-16  9:25   ` Kevin Hilman
  0 siblings, 2 replies; 12+ messages in thread
From: Högander Jouni @ 2008-09-12  7:48 UTC (permalink / raw)
  To: ext chandra shekhar; +Cc: linux-omap

"ext chandra shekhar" <x0044955@ti.com> writes:

> agreed..but i guess it will not compile for 2430 and prev. platforms.
> omap3_i2c_save_context and restore definition is under macro
> while call is not. let me know if i am missing something.
>
> also is SYSC register restore needed??

Here is the second version. Does it look any better to you?

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3778735..0a11621 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -147,6 +147,12 @@ struct omap_i2c_dev {
        unsigned                b_hw:1;         /* bad h/w fixes */
        unsigned                idle:1;
        u16                     iestate;        /* Saved interrupt register */
+#ifdef CONFIG_ARCH_OMAP34XX
+       u16                     pscstate;
+       u16                     scllstate;
+       u16                     sclhstate;
+       u16                     bufstate;
+#endif
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -209,16 +215,21 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
        if (dev->iclk != NULL)
                clk_enable(dev->iclk);
        clk_enable(dev->fclk);
+
+#ifdef CONFIG_ARCH_OMAP34XX
+       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
+       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
+#endif
        dev->idle = 0;
-       if (dev->iestate)
-               omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }
 
 static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
        u16 iv;
 
-       dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
        omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
        if (dev->rev1)
                iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
@@ -238,7 +249,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)
 {
-       u16 psc = 0, scll = 0, sclh = 0;
+       u16 psc = 0, scll = 0, sclh = 0, buf = 0;
        u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
        unsigned long fclk_rate = 12000000;
        unsigned long timeout;
@@ -327,23 +338,30 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
        omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
        omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
 
-       if (dev->fifo_size)
-               /* Note: setup required fifo size - 1 */
-               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
-                                       (dev->fifo_size - 1) << 8 | /* RTRSH */
-                                       OMAP_I2C_BUF_RXFIF_CLR |
-                                       (dev->fifo_size - 1) | /* XTRSH */
-                                       OMAP_I2C_BUF_TXFIF_CLR);
+       if (dev->fifo_size) {
+               /* Note: setup required fifo size - 1. RTRSH and XTRSH */
+               buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR |
+                       (dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
+               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
+       }
 
        /* Take the I2C module out of reset: */
        omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
 
        /* Enable interrupts */
-       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
-                       (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
+       dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
-                               (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
+                                           (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0);
+       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+
+#ifdef CONFIG_ARCH_OMAP34XX
+       dev->pscstate = psc;
+       dev->scllstate = scll;
+       dev->sclhstate = sclh;
+       dev->bufstate = buf;
+#endif
+
        return 0;
 }
 
@@ -496,8 +514,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
        omap_i2c_unidle(dev);
 
-       omap_i2c_init(dev);
-
        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
                goto out;

-- 
Jouni Högander

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-12  7:48 ` Högander Jouni
@ 2008-09-15  6:57   ` shekhar, chandra
  2008-09-16  9:25   ` Kevin Hilman
  1 sibling, 0 replies; 12+ messages in thread
From: shekhar, chandra @ 2008-09-15  6:57 UTC (permalink / raw)
  To: "Högander" Jouni; +Cc: linux-omap

Hi,

This patch looks ok.
i would have preferred runtime check for 34xx. It will be a bit overhead for 
previous platforms.
but will be good for  multi-omap support.

----- Original Message ----- 
From: ""Högander" Jouni" <jouni.hogander@nokia.com>
To: "ext chandra shekhar" <x0044955@ti.com>
Cc: <linux-omap@vger.kernel.org>
Sent: Friday, September 12, 2008 1:18 PM
Subject: Re: [PATCH 04/12] I2C re-init for every cmd


"ext chandra shekhar" <x0044955@ti.com> writes:

> agreed..but i guess it will not compile for 2430 and prev. platforms.
> omap3_i2c_save_context and restore definition is under macro
> while call is not. let me know if i am missing something.
>
> also is SYSC register restore needed??

Here is the second version. Does it look any better to you?

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3778735..0a11621 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -147,6 +147,12 @@ struct omap_i2c_dev {
        unsigned                b_hw:1;         /* bad h/w fixes */
        unsigned                idle:1;
        u16                     iestate;        /* Saved interrupt register */
+#ifdef CONFIG_ARCH_OMAP34XX
+       u16                     pscstate;
+       u16                     scllstate;
+       u16                     sclhstate;
+       u16                     bufstate;
+#endif
 };

 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -209,16 +215,21 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
        if (dev->iclk != NULL)
                clk_enable(dev->iclk);
        clk_enable(dev->fclk);
+
+#ifdef CONFIG_ARCH_OMAP34XX
+       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
+       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
+       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
+#endif
        dev->idle = 0;
-       if (dev->iestate)
-               omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }

 static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
        u16 iv;

-       dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
        omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
        if (dev->rev1)
                iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
@@ -238,7 +249,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)

 static int omap_i2c_init(struct omap_i2c_dev *dev)
 {
-       u16 psc = 0, scll = 0, sclh = 0;
+       u16 psc = 0, scll = 0, sclh = 0, buf = 0;
        u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
        unsigned long fclk_rate = 12000000;
        unsigned long timeout;
@@ -327,23 +338,30 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
        omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
        omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);

-       if (dev->fifo_size)
-               /* Note: setup required fifo size - 1 */
-               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
-                                       (dev->fifo_size - 1) << 8 | /* RTRSH */
-                                       OMAP_I2C_BUF_RXFIF_CLR |
-                                       (dev->fifo_size - 1) | /* XTRSH */
-                                       OMAP_I2C_BUF_TXFIF_CLR);
+       if (dev->fifo_size) {
+               /* Note: setup required fifo size - 1. RTRSH and XTRSH */
+               buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR |
+                       (dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
+               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
+       }

        /* Take the I2C module out of reset: */
        omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);

        /* Enable interrupts */
-       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
-                       (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
+       dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
-                               (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
+                                           (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) 
: 0);
+       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+
+#ifdef CONFIG_ARCH_OMAP34XX
+       dev->pscstate = psc;
+       dev->scllstate = scll;
+       dev->sclhstate = sclh;
+       dev->bufstate = buf;
+#endif
+
        return 0;
 }

@@ -496,8 +514,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)

        omap_i2c_unidle(dev);

-       omap_i2c_init(dev);
-
        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
                goto out;

-- 
Jouni Högander


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-12  7:48 ` Högander Jouni
  2008-09-15  6:57   ` shekhar, chandra
@ 2008-09-16  9:25   ` Kevin Hilman
  2008-09-16 10:13     ` Högander Jouni
  1 sibling, 1 reply; 12+ messages in thread
From: Kevin Hilman @ 2008-09-16  9:25 UTC (permalink / raw)
  To: Högander Jouni; +Cc: ext chandra shekhar, linux-omap

jouni.hogander@nokia.com (Högander Jouni) writes:

> "ext chandra shekhar" <x0044955@ti.com> writes:
>
>> agreed..but i guess it will not compile for 2430 and prev. platforms.
>> omap3_i2c_save_context and restore definition is under macro
>> while call is not. let me know if i am missing something.
>>
>> also is SYSC register restore needed??
>
> Here is the second version. Does it look any better to you?
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 3778735..0a11621 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -147,6 +147,12 @@ struct omap_i2c_dev {
>         unsigned                b_hw:1;         /* bad h/w fixes */
>         unsigned                idle:1;
>         u16                     iestate;        /* Saved interrupt register */
> +#ifdef CONFIG_ARCH_OMAP34XX
> +       u16                     pscstate;
> +       u16                     scllstate;
> +       u16                     sclhstate;
> +       u16                     bufstate;
> +#endif
>  };
>  
>  static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
> @@ -209,16 +215,21 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
>         if (dev->iclk != NULL)
>                 clk_enable(dev->iclk);
>         clk_enable(dev->fclk);
> +
> +#ifdef CONFIG_ARCH_OMAP34XX
> +       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
> +       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
> +#endif

The above should also be inside an if cpu_is_omap34xx() block so
it works correctly on multi-omap kernels.

Kevin

>         dev->idle = 0;
> -       if (dev->iestate)
> -               omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> +       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>  }
>  
>  static void omap_i2c_idle(struct omap_i2c_dev *dev)
>  {
>         u16 iv;
>  
> -       dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
>         omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
>         if (dev->rev1)
>                 iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
> @@ -238,7 +249,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
>  
>  static int omap_i2c_init(struct omap_i2c_dev *dev)
>  {
> -       u16 psc = 0, scll = 0, sclh = 0;
> +       u16 psc = 0, scll = 0, sclh = 0, buf = 0;
>         u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
>         unsigned long fclk_rate = 12000000;
>         unsigned long timeout;
> @@ -327,23 +338,30 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>         omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
>         omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
>  
> -       if (dev->fifo_size)
> -               /* Note: setup required fifo size - 1 */
> -               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
> -                                       (dev->fifo_size - 1) << 8 | /* RTRSH */
> -                                       OMAP_I2C_BUF_RXFIF_CLR |
> -                                       (dev->fifo_size - 1) | /* XTRSH */
> -                                       OMAP_I2C_BUF_TXFIF_CLR);
> +       if (dev->fifo_size) {
> +               /* Note: setup required fifo size - 1. RTRSH and XTRSH */
> +               buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR |
> +                       (dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
> +               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
> +       }
>  
>         /* Take the I2C module out of reset: */
>         omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
>  
>         /* Enable interrupts */
> -       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
> -                       (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
> +       dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
>                         OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
>                         OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
> -                               (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
> +                                           (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0);
> +       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> +
> +#ifdef CONFIG_ARCH_OMAP34XX
> +       dev->pscstate = psc;
> +       dev->scllstate = scll;
> +       dev->sclhstate = sclh;
> +       dev->bufstate = buf;
> +#endif
> +
>         return 0;
>  }
>  
> @@ -496,8 +514,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  
>         omap_i2c_unidle(dev);
>  
> -       omap_i2c_init(dev);
> -
>         if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>                 goto out;
>
> -- 
> Jouni Högander
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 04/12] I2C re-init for every cmd
  2008-09-16  9:25   ` Kevin Hilman
@ 2008-09-16 10:13     ` Högander Jouni
  2008-09-17  9:45       ` [PATCH PM-0] OMAP3: I2C: Implement i2c save/restore Jouni Hogander
  0 siblings, 1 reply; 12+ messages in thread
From: Högander Jouni @ 2008-09-16 10:13 UTC (permalink / raw)
  To: ext Kevin Hilman; +Cc: ext chandra shekhar, linux-omap

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

"ext Kevin Hilman" <khilman@deeprootsystems.com> writes:

> jouni.hogander@nokia.com (Högander Jouni) writes:
>
>> @@ -209,16 +215,21 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
>>         if (dev->iclk != NULL)
>>                 clk_enable(dev->iclk);
>>         clk_enable(dev->fclk);
>> +
>> +#ifdef CONFIG_ARCH_OMAP34XX
>> +       omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
>> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
>> +       omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
>> +       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
>> +#endif
>
> The above should also be inside an if cpu_is_omap34xx() block so
> it works correctly on multi-omap kernels.

Here you are:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-OMAP3-I2C-Implement-i2c-save-restore.patch --]
[-- Type: text/x-diff, Size: 3723 bytes --]

>From 24db406dfb2ba6c9069b70c2efd1134ea29c23eb Mon Sep 17 00:00:00 2001
From: Jouni Hogander <jouni.hogander@nokia.com>
Date: Tue, 16 Sep 2008 13:03:24 +0300
Subject: [RFC] OMAP3: I2C: Implement i2c save/restore


Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 drivers/i2c/busses/i2c-omap.c |   52 ++++++++++++++++++++++++++++------------
 1 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3778735..6f6cfcc 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -147,6 +147,12 @@ struct omap_i2c_dev {
 	unsigned		b_hw:1;		/* bad h/w fixes */
 	unsigned		idle:1;
 	u16			iestate;	/* Saved interrupt register */
+#ifdef CONFIG_ARCH_OMAP34XX
+	u16			pscstate;
+	u16			scllstate;
+	u16			sclhstate;
+	u16			bufstate;
+#endif
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -209,16 +215,23 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
+
+#ifdef CONFIG_ARCH_OMAP34XX
+	if (cpu_is_omap34xx()) {
+		omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
+		omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
+		omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
+		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
+	}
+#endif
 	dev->idle = 0;
-	if (dev->iestate)
-		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }
 
 static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
 	u16 iv;
 
-	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
 	if (dev->rev1)
 		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
@@ -238,7 +251,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)
 {
-	u16 psc = 0, scll = 0, sclh = 0;
+	u16 psc = 0, scll = 0, sclh = 0, buf = 0;
 	u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
 	unsigned long fclk_rate = 12000000;
 	unsigned long timeout;
@@ -327,23 +340,32 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 	omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
 	omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
 
-	if (dev->fifo_size)
-		/* Note: setup required fifo size - 1 */
-		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
-					(dev->fifo_size - 1) << 8 | /* RTRSH */
-					OMAP_I2C_BUF_RXFIF_CLR |
-					(dev->fifo_size - 1) | /* XTRSH */
-					OMAP_I2C_BUF_TXFIF_CLR);
+	if (dev->fifo_size) {
+		/* Note: setup required fifo size - 1. RTRSH and XTRSH */
+		buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR |
+			(dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
+		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
+	}
 
 	/* Take the I2C module out of reset: */
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
 
 	/* Enable interrupts */
-	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
-			(OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
+	dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
 			OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
 			OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
-				(OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
+				(OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+
+#ifdef CONFIG_ARCH_OMAP34XX
+	if (cpu_is_omap34xx()) {
+		dev->pscstate = psc;
+		dev->scllstate = scll;
+		dev->sclhstate = sclh;
+		dev->bufstate = buf;
+	}
+#endif
+
 	return 0;
 }
 
@@ -496,8 +518,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	omap_i2c_unidle(dev);
 
-	omap_i2c_init(dev);
-
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;
 
-- 
1.5.5


[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


-- 
Jouni Högander

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH PM-0] OMAP3: I2C: Implement i2c save/restore
  2008-09-16 10:13     ` Högander Jouni
@ 2008-09-17  9:45       ` Jouni Hogander
  2008-09-17 11:18         ` Kevin Hilman
  0 siblings, 1 reply; 12+ messages in thread
From: Jouni Hogander @ 2008-09-17  9:45 UTC (permalink / raw)
  To: linux-omap

Save and restore needed registers instead of re-init.

Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
---
 drivers/i2c/busses/i2c-omap.c |   47 +++++++++++++++++++++++++++--------------
 1 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a661ed3..211d7b5 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -147,6 +147,10 @@ struct omap_i2c_dev {
 	unsigned		b_hw:1;		/* bad h/w fixes */
 	unsigned		idle:1;
 	u16			iestate;	/* Saved interrupt register */
+	u16			pscstate;
+	u16			scllstate;
+	u16			sclhstate;
+	u16			bufstate;
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -209,16 +213,22 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
+
+	if (cpu_is_omap34xx()) {
+		omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
+		omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
+		omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
+		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
+	}
+
 	dev->idle = 0;
-	if (dev->iestate)
-		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }
 
 static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
 	u16 iv;
 
-	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
 	if (dev->rev1)
 		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
@@ -238,7 +248,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)
 {
-	u16 psc = 0, scll = 0, sclh = 0;
+	u16 psc = 0, scll = 0, sclh = 0, buf = 0;
 	u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
 	unsigned long fclk_rate = 12000000;
 	unsigned long timeout;
@@ -327,23 +337,30 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 	omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
 	omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
 
-	if (dev->fifo_size)
-		/* Note: setup required fifo size - 1 */
-		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
-					(dev->fifo_size - 1) << 8 | /* RTRSH */
-					OMAP_I2C_BUF_RXFIF_CLR |
-					(dev->fifo_size - 1) | /* XTRSH */
-					OMAP_I2C_BUF_TXFIF_CLR);
+	if (dev->fifo_size) {
+		/* Note: setup required fifo size - 1. RTRSH and XTRSH */
+		buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR |
+			(dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
+		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
+	}
 
 	/* Take the I2C module out of reset: */
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
 
 	/* Enable interrupts */
-	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
-			(OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
+	dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
 			OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
 			OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
-				(OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
+				(OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+
+	if (cpu_is_omap34xx()) {
+		dev->pscstate = psc;
+		dev->scllstate = scll;
+		dev->sclhstate = sclh;
+		dev->bufstate = buf;
+	}
+
 	return 0;
 }
 
@@ -503,8 +520,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	omap_i2c_unidle(dev);
 
-	omap_i2c_init(dev);
-
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;
 
-- 
1.5.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH PM-0] OMAP3: I2C: Implement i2c save/restore
  2008-09-17  9:45       ` [PATCH PM-0] OMAP3: I2C: Implement i2c save/restore Jouni Hogander
@ 2008-09-17 11:18         ` Kevin Hilman
  2008-09-17 11:25           ` Felipe Balbi
  0 siblings, 1 reply; 12+ messages in thread
From: Kevin Hilman @ 2008-09-17 11:18 UTC (permalink / raw)
  To: Jouni Hogander; +Cc: linux-omap

Jouni Hogander <jouni.hogander@nokia.com> writes:

> Save and restore needed registers instead of re-init.
>
> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>

Ack.  Pushing to pm-0 today.

Kevin

> ---
>  drivers/i2c/busses/i2c-omap.c |   47 +++++++++++++++++++++++++++--------------
>  1 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index a661ed3..211d7b5 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -147,6 +147,10 @@ struct omap_i2c_dev {
>  	unsigned		b_hw:1;		/* bad h/w fixes */
>  	unsigned		idle:1;
>  	u16			iestate;	/* Saved interrupt register */
> +	u16			pscstate;
> +	u16			scllstate;
> +	u16			sclhstate;
> +	u16			bufstate;
>  };
>  
>  static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
> @@ -209,16 +213,22 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
>  	if (dev->iclk != NULL)
>  		clk_enable(dev->iclk);
>  	clk_enable(dev->fclk);
> +
> +	if (cpu_is_omap34xx()) {
> +		omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
> +		omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
> +		omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
> +		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
> +	}
> +
>  	dev->idle = 0;
> -	if (dev->iestate)
> -		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> +	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>  }
>  
>  static void omap_i2c_idle(struct omap_i2c_dev *dev)
>  {
>  	u16 iv;
>  
> -	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
>  	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
>  	if (dev->rev1)
>  		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
> @@ -238,7 +248,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
>  
>  static int omap_i2c_init(struct omap_i2c_dev *dev)
>  {
> -	u16 psc = 0, scll = 0, sclh = 0;
> +	u16 psc = 0, scll = 0, sclh = 0, buf = 0;
>  	u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
>  	unsigned long fclk_rate = 12000000;
>  	unsigned long timeout;
> @@ -327,23 +337,30 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>  	omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
>  	omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
>  
> -	if (dev->fifo_size)
> -		/* Note: setup required fifo size - 1 */
> -		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
> -					(dev->fifo_size - 1) << 8 | /* RTRSH */
> -					OMAP_I2C_BUF_RXFIF_CLR |
> -					(dev->fifo_size - 1) | /* XTRSH */
> -					OMAP_I2C_BUF_TXFIF_CLR);
> +	if (dev->fifo_size) {
> +		/* Note: setup required fifo size - 1. RTRSH and XTRSH */
> +		buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR |
> +			(dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
> +		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
> +	}
>  
>  	/* Take the I2C module out of reset: */
>  	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
>  
>  	/* Enable interrupts */
> -	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
> -			(OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
> +	dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
>  			OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
>  			OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
> -				(OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0));
> +				(OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0);
> +	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> +
> +	if (cpu_is_omap34xx()) {
> +		dev->pscstate = psc;
> +		dev->scllstate = scll;
> +		dev->sclhstate = sclh;
> +		dev->bufstate = buf;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -503,8 +520,6 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  
>  	omap_i2c_unidle(dev);
>  
> -	omap_i2c_init(dev);
> -
>  	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
>  		goto out;
>  
> -- 
> 1.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH PM-0] OMAP3: I2C: Implement i2c save/restore
  2008-09-17 11:18         ` Kevin Hilman
@ 2008-09-17 11:25           ` Felipe Balbi
  0 siblings, 0 replies; 12+ messages in thread
From: Felipe Balbi @ 2008-09-17 11:25 UTC (permalink / raw)
  To: ext Kevin Hilman; +Cc: Jouni Hogander, linux-omap

On Wed, Sep 17, 2008 at 02:18:57PM +0300, ext Kevin Hilman wrote:
> Jouni Hogander <jouni.hogander@nokia.com> writes:
> 
> > Save and restore needed registers instead of re-init.
> >
> > Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
> 
> Ack.  Pushing to pm-0 today.

About that, pm-0 only appears in source.mvista.com. Anyway of pushing it
to kernel.org as well ??

-- 
balbi

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-09-17 11:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-11 15:55 [PATCH 04/12] I2C re-init for every cmd chandra shekhar
2008-09-12  7:48 ` Högander Jouni
2008-09-15  6:57   ` shekhar, chandra
2008-09-16  9:25   ` Kevin Hilman
2008-09-16 10:13     ` Högander Jouni
2008-09-17  9:45       ` [PATCH PM-0] OMAP3: I2C: Implement i2c save/restore Jouni Hogander
2008-09-17 11:18         ` Kevin Hilman
2008-09-17 11:25           ` Felipe Balbi
  -- strict thread matches above, loose matches on Subject: below --
2008-09-01 13:38 [PATCH 04/12] I2C re-init for every cmd Rajendra Nayak
2008-09-11 10:04 ` Högander Jouni
2008-09-11 10:33   ` shekhar, chandra
2008-09-11 11:01     ` Högander Jouni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox