Re: [PATCH v2] i2c: omap: re-factor omap_i2c_init function
On Thu, Oct 25, 2012 at 12:06 PM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Oct 25, 2012 at 12:06:51PM +0530, Shubhrajyoti D wrote:
>> re-factor omap_i2c_init() so that we can re-use it for resume.
>> While at it also remove the bufstate variable as we write it
>> in omap_i2c_resize_fifo for every transfer.
>>
>> Signed-off-by: Shubhrajyoti D
>> ---
>> v2 - add the iestate 0 check back.
>>- Remove a stray change.
>> - Applies on top of Felipe's patches.
>> http://www.spinics.net/lists/linux-omap/msg79995.html
>>
>>
>> Tested with Terro sys fix + Felipe's stop sched_clock() during suspend
>> on omap3636 beagle both idle and suspend.
>>
>> Functional testing on omap4sdp.
>>
>> drivers/i2c/busses/i2c-omap.c | 71
>> ++--
>> 1 files changed, 32 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
>> index 5e5cefb..3d400b1 100644
>> --- a/drivers/i2c/busses/i2c-omap.c
>> +++ b/drivers/i2c/busses/i2c-omap.c
>> @@ -209,7 +209,6 @@ struct omap_i2c_dev {
>> u16 pscstate;
>> u16 scllstate;
>> u16 sclhstate;
>> - u16 bufstate;
>> u16 syscstate;
>> u16 westate;
>> u16 errata;
>> @@ -285,9 +284,31 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev
>> *i2c_dev, int reg)
>> }
>> }
>>
>> +static void __omap_i2c_init(struct omap_i2c_dev *dev)
>> +{
>> +
>> + omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
>> + /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
>> + omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
>> +
>> + /* SCL low and high time values */
>> + omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
>> + omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
>> + if (dev->rev >= OMAP_I2C_REV_ON_3430_3530)
>> + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate);
>> + /* Take the I2C module out of reset: */
>> + omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
>
> a few blank lines in this function wouldn't hurt and they would help
> with readability.
Will add .
>
>> + /*
>> + * Don't write to this register if the IE state is 0 as it can
>> + * cause deadlock.
>> + */
>> + if (dev->iestate)
>> + omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>> +}
>> +
>> static int omap_i2c_init(struct omap_i2c_dev *dev)
>> {
>> - u16 psc = 0, scll = 0, sclh = 0, buf = 0;
>> + u16 psc = 0, scll = 0, sclh = 0;
>> u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
>> unsigned long fclk_rate = 1200;
>> unsigned long timeout;
>> @@ -337,11 +358,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>>* REVISIT: Some wkup sources might not be needed.
>>*/
>> dev->westate = OMAP_I2C_WE_ALL;
>> - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
>> - dev->westate);
>
> remove the comment too since now that's done by some other function ?
>
>> }
>> }
>> - omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
>>
>> if (dev->flags & OMAP_I2C_FLAG_ALWAYS_ARMXOR_CLK) {
>> /*
>> @@ -426,28 +444,18 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>> sclh = fclk_rate / (dev->speed * 2) - 7 + psc;
>> }
>>
>> - /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
>> - omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, psc);
>> -
>> - /* SCL low and high time values */
>> - omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
>> - omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
>> -
>> - /* Take the I2C module out of reset: */
>> - omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
>> -
>> /* Enable interrupts */
>> dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
>> OMAP_I2C_IE_NACK | OMAP_I2C_IE_AL) |
>> ((dev->fifo_size) ? (OMAP_I2C_IE_RDR |
>> OMAP_I2C_IE_XDR) : 0);
>> - omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>> - if (dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
>> - dev->pscstate = psc;
>> - dev->scllstate = scll;
>> - dev->sclhstate = sclh;
>> - dev->bufstate = buf;
>> - }
>> +
>> + dev->pscstate = psc;
>> + dev->scllstate = scll;
>> + dev->sclhstate = sclh;
>> +
>> + __omap_i2c_init(dev);
>> +
>> return 0;
>> }
>>
>> @@ -1268,23 +1276,8 @@ static int omap_i2c_runtime_resume(struct device *dev)
>> {
>> struct omap_i2c_dev *_dev = dev_get_drvdata(dev);
>>
>> - if (_dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
>> - oma
Re: [PATCH v2] i2c: omap: re-factor omap_i2c_init function
Hi,
On Thu, Oct 25, 2012 at 12:06:51PM +0530, Shubhrajyoti D wrote:
> re-factor omap_i2c_init() so that we can re-use it for resume.
> While at it also remove the bufstate variable as we write it
> in omap_i2c_resize_fifo for every transfer.
>
> Signed-off-by: Shubhrajyoti D
> ---
> v2 - add the iestate 0 check back.
>- Remove a stray change.
> - Applies on top of Felipe's patches.
> http://www.spinics.net/lists/linux-omap/msg79995.html
>
>
> Tested with Terro sys fix + Felipe's stop sched_clock() during suspend
> on omap3636 beagle both idle and suspend.
>
> Functional testing on omap4sdp.
>
> drivers/i2c/busses/i2c-omap.c | 71 ++--
> 1 files changed, 32 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 5e5cefb..3d400b1 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -209,7 +209,6 @@ struct omap_i2c_dev {
> u16 pscstate;
> u16 scllstate;
> u16 sclhstate;
> - u16 bufstate;
> u16 syscstate;
> u16 westate;
> u16 errata;
> @@ -285,9 +284,31 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev
> *i2c_dev, int reg)
> }
> }
>
> +static void __omap_i2c_init(struct omap_i2c_dev *dev)
> +{
> +
> + omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
> + /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
> + omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
> +
> + /* SCL low and high time values */
> + omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
> + omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
> + if (dev->rev >= OMAP_I2C_REV_ON_3430_3530)
> + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate);
> + /* Take the I2C module out of reset: */
> + omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
a few blank lines in this function wouldn't hurt and they would help
with readability.
> + /*
> + * Don't write to this register if the IE state is 0 as it can
> + * cause deadlock.
> + */
> + if (dev->iestate)
> + omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> +}
> +
> static int omap_i2c_init(struct omap_i2c_dev *dev)
> {
> - u16 psc = 0, scll = 0, sclh = 0, buf = 0;
> + u16 psc = 0, scll = 0, sclh = 0;
> u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
> unsigned long fclk_rate = 1200;
> unsigned long timeout;
> @@ -337,11 +358,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>* REVISIT: Some wkup sources might not be needed.
>*/
> dev->westate = OMAP_I2C_WE_ALL;
> - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
> - dev->westate);
remove the comment too since now that's done by some other function ?
> }
> }
> - omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
>
> if (dev->flags & OMAP_I2C_FLAG_ALWAYS_ARMXOR_CLK) {
> /*
> @@ -426,28 +444,18 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
> sclh = fclk_rate / (dev->speed * 2) - 7 + psc;
> }
>
> - /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
> - omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, psc);
> -
> - /* SCL low and high time values */
> - omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
> - omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
> -
> - /* Take the I2C module out of reset: */
> - omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
> -
> /* Enable interrupts */
> dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
> OMAP_I2C_IE_NACK | OMAP_I2C_IE_AL) |
> ((dev->fifo_size) ? (OMAP_I2C_IE_RDR |
> OMAP_I2C_IE_XDR) : 0);
> - omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
> - if (dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
> - dev->pscstate = psc;
> - dev->scllstate = scll;
> - dev->sclhstate = sclh;
> - dev->bufstate = buf;
> - }
> +
> + dev->pscstate = psc;
> + dev->scllstate = scll;
> + dev->sclhstate = sclh;
> +
> + __omap_i2c_init(dev);
> +
> return 0;
> }
>
> @@ -1268,23 +1276,8 @@ static int omap_i2c_runtime_resume(struct device *dev)
> {
> struct omap_i2c_dev *_dev = dev_get_drvdata(dev);
>
> - if (_dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
> - omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, 0);
> - omap_i2c_write_reg(_dev, OMAP_I2C_PSC_REG, _dev->pscstate);
> - omap_i2c_write_reg(_dev, OMAP_I2C_SCLL_REG, _dev->sclls
[PATCH v2] i2c: omap: re-factor omap_i2c_init function
re-factor omap_i2c_init() so that we can re-use it for resume.
While at it also remove the bufstate variable as we write it
in omap_i2c_resize_fifo for every transfer.
Signed-off-by: Shubhrajyoti D
---
v2 - add the iestate 0 check back.
- Remove a stray change.
- Applies on top of Felipe's patches.
http://www.spinics.net/lists/linux-omap/msg79995.html
Tested with Terro sys fix + Felipe's stop sched_clock() during suspend
on omap3636 beagle both idle and suspend.
Functional testing on omap4sdp.
drivers/i2c/busses/i2c-omap.c | 71 ++--
1 files changed, 32 insertions(+), 39 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 5e5cefb..3d400b1 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -209,7 +209,6 @@ struct omap_i2c_dev {
u16 pscstate;
u16 scllstate;
u16 sclhstate;
- u16 bufstate;
u16 syscstate;
u16 westate;
u16 errata;
@@ -285,9 +284,31 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev
*i2c_dev, int reg)
}
}
+static void __omap_i2c_init(struct omap_i2c_dev *dev)
+{
+
+ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
+ /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
+ omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
+
+ /* SCL low and high time values */
+ omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
+ if (dev->rev >= OMAP_I2C_REV_ON_3430_3530)
+ omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate);
+ /* Take the I2C module out of reset: */
+ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
+ /*
+* Don't write to this register if the IE state is 0 as it can
+* cause deadlock.
+*/
+ if (dev->iestate)
+ omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+}
+
static int omap_i2c_init(struct omap_i2c_dev *dev)
{
- u16 psc = 0, scll = 0, sclh = 0, buf = 0;
+ u16 psc = 0, scll = 0, sclh = 0;
u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
unsigned long fclk_rate = 1200;
unsigned long timeout;
@@ -337,11 +358,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
* REVISIT: Some wkup sources might not be needed.
*/
dev->westate = OMAP_I2C_WE_ALL;
- omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
- dev->westate);
}
}
- omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
if (dev->flags & OMAP_I2C_FLAG_ALWAYS_ARMXOR_CLK) {
/*
@@ -426,28 +444,18 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
sclh = fclk_rate / (dev->speed * 2) - 7 + psc;
}
- /* Setup clock prescaler to obtain approx 12MHz I2C module clock: */
- omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, psc);
-
- /* SCL low and high time values */
- omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
- omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
-
- /* Take the I2C module out of reset: */
- omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
-
/* Enable interrupts */
dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
OMAP_I2C_IE_NACK | OMAP_I2C_IE_AL) |
((dev->fifo_size) ? (OMAP_I2C_IE_RDR |
OMAP_I2C_IE_XDR) : 0);
- omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
- if (dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
- dev->pscstate = psc;
- dev->scllstate = scll;
- dev->sclhstate = sclh;
- dev->bufstate = buf;
- }
+
+ dev->pscstate = psc;
+ dev->scllstate = scll;
+ dev->sclhstate = sclh;
+
+ __omap_i2c_init(dev);
+
return 0;
}
@@ -1268,23 +1276,8 @@ static int omap_i2c_runtime_resume(struct device *dev)
{
struct omap_i2c_dev *_dev = dev_get_drvdata(dev);
- if (_dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
- omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, 0);
- omap_i2c_write_reg(_dev, OMAP_I2C_PSC_REG, _dev->pscstate);
- 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);
- omap_i2c_write_reg(_dev, OMAP_I2C_SYSC_REG, _dev->syscstate);
- omap_i2c_write_reg(_dev, OMAP_I2C_WE_REG, _dev->westate);
- omap_
Re: [PATCH] i2c: omap: re-factor omap_i2c_init function
On Tue, Oct 23, 2012 at 11:27 PM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Oct 23, 2012 at 11:26:15PM +0530, Shubhrajyoti Datta wrote:
>> >> @@ -1268,23 +1271,8 @@ static int omap_i2c_runtime_resume(struct device
>> >> *dev)
>> >> {
>> >> struct omap_i2c_dev *_dev = dev_get_drvdata(dev);
>> >>
>> >> - if (_dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
>> >> - omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, 0);
>> >> - omap_i2c_write_reg(_dev, OMAP_I2C_PSC_REG, _dev->pscstate);
>> >> - 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);
>> >> - omap_i2c_write_reg(_dev, OMAP_I2C_SYSC_REG,
>> >> _dev->syscstate);
>> >> - omap_i2c_write_reg(_dev, OMAP_I2C_WE_REG, _dev->westate);
>> >> - omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
>> >> - }
>> >> -
>> >> - /*
>> >> - * Don't write to this register if the IE state is 0 as it can
>> >> - * cause deadlock.
>> >> - */
>> >> - if (_dev->iestate)
>> >> - omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, _dev->iestate);
>> >
>> > this part is not on __omap_i2c_init() so you're potentially causing a
>> > regression here.
>>
>> iestate is set at init so cannot be zero. so the check was removed.
>
Hmm thinking a little more, there is a case that I missed the resume
handler may get called before
the omap_i2c_init will restore the check and send another version.
--
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: re-factor omap_i2c_init function
On 10/24/2012 12:41 AM, Felipe Balbi wrote: >return 0; >} another thing, the few places in omap_i2c_xfer_msg() which are currently calling omap_i2c_init() should also be converted to call the newly added __omap_i2c_init(). We don't need to recalculate any of those clock dividers and whatnot. Yes in fact omap_i2c_init() can be reset - calculate - and __omap_i2c_init. Then in those places the recalculate can be optimised. -- 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] Support Elan Touchscreen eKTF product.
Hi Dmitry, Thanks for review. > -Original Message- > From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] > Sent: Thursday, October 25, 2012 2:13 AM > To: Scott Liu > Cc: linux-in...@vger.kernel.org; linux-i2c@vger.kernel.org; linux-ker...@vger.kernel.org; > Benjamin Tissoires; Jesse; Vincent Wang; Paul > Subject: Re: [PATCH v2] Support Elan Touchscreen eKTF product. > > Hi Scott, > > On Wed, Oct 24, 2012 at 09:41:43AM +0800, Scott Liu wrote: > > This patch is for Elan eKTF Touchscreen product, I2C adpater module. > > > > Signed-off-by: Scott Liu > > --- > > > > Hi, > > v2 revision I have fixed some bug as your advise. > > 1. To target the mainline > > 2. No Android dependency > > 3. reuse those duplication code from Henrik's patchset. > > (input_mt_sync_frame() / input_mt_get_slot_by_key()) > > Just a quick run through the code, so: > > - please remove polling support, it is not useful in production; OK. > - why do you need a separate probe work instead of doing what you > need in elants_probe() will fix. > - it is not a good idea to register input device first and then > allocating memory for MT handling. Ooop...will fix. > - I do not understand why kfifo is needed The firmware and the host would conflict by read command and finger report simultaneously. So I'm simply using kfifo in IRQ thread function. * read command: writing 4 bytes commands and the device asserts GPIO interrupt and then response 4 bytes data. There was an error if we do not use kfifo: With heavy loading by finger report / read command, the driver may get finger report as response data. So, do you understand my meaning? > - please remove the rest of the custom threads OK > - you do not need to call input_mt_destroy_slots() explicitly OK > - use request_firmware() instead of special character device to upload > firmware. OK, but I'll remove firmware update function at this patch first. > - please use standard kernel-doc markup. > - consider what attributes are there only for debugging and move them to > debugfs. OK. > - I find the use of enums in this driver quite unconventional, just > standard #defines would probably be more straightforward. OK. Thanks, Scott > > Thanks. > > -- > Dmitry -- 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] Support Elan Touchscreen eKTF product.
Hi Scott, On Wed, Oct 24, 2012 at 09:41:43AM +0800, Scott Liu wrote: > This patch is for Elan eKTF Touchscreen product, I2C adpater module. > > Signed-off-by: Scott Liu > --- > > Hi, > v2 revision I have fixed some bug as your advise. > 1. To target the mainline > 2. No Android dependency > 3. reuse those duplication code from Henrik's patchset. > (input_mt_sync_frame() / input_mt_get_slot_by_key()) Just a quick run through the code, so: - please remove polling support, it is not useful in production; - why do you need a separate probe work instead of doing what you need in elants_probe() - it is not a good idea to register input device first and then allocating memory for MT handling. - I do not understand why kfifo is needed - please remove the rest of the custom threads - you do not need to call input_mt_destroy_slots() explicitly - use request_firmware() instead of special character device to upload firmware. - please use standard kernel-doc markup. - consider what attributes are there only for debugging and move them to debugfs. - I find the use of enums in this driver quite unconventional, just standard #defines would probably be more straightforward. Thanks. -- Dmitry -- 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: mux: Add dt support to i2c-mux-gpio driver
> "MR" == Maxime Ripard writes:
Hi,
MR> Allow the i2c-mux-gpio to be used by a device tree enabled device. The
MR> bindings are inspired by the one found in the i2c-mux-pinctrl driver.
MR> Signed-off-by: Maxime Ripard
MR> Reviewed-by: Stephen Warren
MR> ---
MR> .../devicetree/bindings/i2c/i2c-mux-gpio.txt | 81 +++
MR> drivers/i2c/muxes/i2c-mux-gpio.c | 146
+++-
MR> 2 files changed, 196 insertions(+), 31 deletions(-)
MR> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> new file mode 100644
MR> index 000..d61726f
MR> --- /dev/null
MR> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> @@ -0,0 +1,81 @@
MR> +GPIO-based I2C Bus Mux
MR> +
MR> +This binding describes an I2C bus multiplexer that uses GPIOs to
MR> +route the I2C signals.
MR> +
MR> + +-+ +-+
MR> + | dev | | dev |
MR> ++++-+ +-+
MR> +| SoC| ||
MR> +|| /++
MR> +| +--+ | +--+child bus A, on GPIO value set to 0
MR> +| | I2C |-|--| Mux |
MR> +| +--+ | +--+---+child bus B, on GPIO value set to 1
MR> +|| |\--+++
MR> +| +--+ | | |||
MR> +| | GPIO |-|-++-+ +-+ +-+
MR> +| +--+ | | dev | | dev | | dev |
MR> +++ +-+ +-+ +-+
MR> +
MR> +Required properties:
MR> +- compatible: i2c-mux-gpio
MR> +- i2c-parent: The phandle of the I2C bus that this multiplexer's
master-side
MR> + port is connected to.
MR> +- mux-gpios: list of gpios used to control the muxer
MR> +* Standard I2C mux properties. See mux.txt in this directory.
MR> +* I2C child bus nodes. See mux.txt in this directory.
MR> +
MR> +Optional properties:
MR> +- idle-state: value to set to the muxer when idle. When no value is
s/to set to the muxer/to set the muxer to/
MR> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
MR> @@ -16,6 +16,8 @@
MR> #include
MR> #include
MR> #include
MR> +#include
MR> +#include
MR> struct gpiomux {
MR> struct i2c_adapter *parent;
MR> @@ -57,29 +59,111 @@ static int __devinit match_gpio_chip_by_label(struct
gpio_chip *chip,
MR> return !strcmp(chip->label, data);
MR> }
MR> +#ifdef CONFIG_OF
MR> +static int __devinit i2c_mux_gpio_probe_dt(struct gpiomux *mux,
MR> + struct platform_device *pdev)
MR> +{
MR> + struct device_node *np = pdev->dev.of_node;
MR> + struct device_node *adapter_np, *child;
MR> + struct i2c_adapter *adapter;
MR> + unsigned *values, *gpios;
MR> + int i = 0;
MR> +
MR> + if (!np)
MR> + return 0;
This should be -ENODEV, otherwise we end up using a zeroed out struct
i2c_mux_gpio_platform_data in case i2c_mux_gpio is used with the
platform bus but the platform forgets to pass the pdata.
With those two minor fixes:
Acked-by: Peter Korsgaard
--
Sorry about disclaimer - It's out of my control.
Bye, Peter Korsgaard
DISCLAIMER:
Unless indicated otherwise, the information contained in this message is
privileged and confidential, and is intended only for the use of the
addressee(s) named above and others who have been specifically authorized to
receive it. If you are not the intended recipient, you are hereby notified that
any dissemination, distribution or copying of this message and/or attachments
is strictly prohibited. The company accepts no liability for any damage caused
by any virus transmitted by this email. Furthermore, the company does not
warrant a proper and complete transmission of this information, nor does it
accept liability for any delays. If you have received this message in error,
please contact the sender and delete the message. Thank you.
--
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 3/8] i2c: omap: fix error checking
Hi
On 10/22/2012 11:46 AM, Felipe Balbi wrote:
> It's impossible to have Arbitration Lost,
> Read Overflow, and Tranmist Underflow all
> asserted at the same time.
>
> Those error conditions are mutually exclusive
> so what the code should be doing, instead, is
> check each error flag separataly.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/i2c/busses/i2c-omap.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index bea0277..e0eab38 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -587,9 +587,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> goto err_i2c_init;
> }
>
> - /* We have an error */
> - if (dev->cmd_err & (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
> - OMAP_I2C_STAT_XUDF)) {
> + if ((dev->cmd_err & OMAP_I2C_STAT_AL)
> + || (dev->cmd_err & OMAP_I2C_STAT_ROVR)
> + || (dev->cmd_err & OMAP_I2C_STAT_XUDF)) {
Sorry, what is the difference? I didn't understand the optimisation and why now
is more clear. Can you just add a comment?
Michael
> ret = -EIO;
> goto err_i2c_init;
> }
>
--
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: mux: Add dt support to i2c-mux-gpio driver
Allow the i2c-mux-gpio to be used by a device tree enabled device. The
bindings are inspired by the one found in the i2c-mux-pinctrl driver.
Signed-off-by: Maxime Ripard
Reviewed-by: Stephen Warren
---
.../devicetree/bindings/i2c/i2c-mux-gpio.txt | 81 +++
drivers/i2c/muxes/i2c-mux-gpio.c | 146 +++-
2 files changed, 196 insertions(+), 31 deletions(-)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
new file mode 100644
index 000..d61726f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
@@ -0,0 +1,81 @@
+GPIO-based I2C Bus Mux
+
+This binding describes an I2C bus multiplexer that uses GPIOs to
+route the I2C signals.
+
+ +-+ +-+
+ | dev | | dev |
++++-+ +-+
+| SoC| ||
+|| /++
+| +--+ | +--+child bus A, on GPIO value set to 0
+| | I2C |-|--| Mux |
+| +--+ | +--+---+child bus B, on GPIO value set to 1
+|| |\--+++
+| +--+ | | |||
+| | GPIO |-|-++-+ +-+ +-+
+| +--+ | | dev | | dev | | dev |
+++ +-+ +-+ +-+
+
+Required properties:
+- compatible: i2c-mux-gpio
+- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
+ port is connected to.
+- mux-gpios: list of gpios used to control the muxer
+* Standard I2C mux properties. See mux.txt in this directory.
+* I2C child bus nodes. See mux.txt in this directory.
+
+Optional properties:
+- idle-state: value to set to the muxer when idle. When no value is
+ given, it defaults to the last value used.
+
+For each i2c child node, an I2C child bus will be created. They will
+be numbered based on their order in the device tree.
+
+Whenever an access is made to a device on a child bus, the value set
+in the revelant node's reg property will be output using the list of
+GPIOs, the first in the list holding the least-significant value.
+
+If an idle state is defined, using the idle-state (optional) property,
+whenever an access is not being made to a device on a child bus, the
+GPIOs will be set according to the idle value.
+
+If an idle state is not defined, the most recently used value will be
+left programmed into hardware whenever no access is being made to a
+device on a child bus.
+
+Example:
+ i2cmux {
+ compatible = "i2c-mux-gpio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
+ i2c-parent = <&i2c1>;
+
+ i2c@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ssd1307: oled@3c {
+ compatible = "solomon,ssd1307fb-i2c";
+ reg = <0x3c>;
+ pwms = <&pwm 4 3000>;
+ reset-gpios = <&gpio2 7 1>;
+ reset-active-low;
+ };
+ };
+
+ i2c@3 {
+ reg = <3>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pca9555: pca9555@20 {
+ compatible = "nxp,pca9555";
+ gpio-controller;
+ #gpio-cells = <2>;
+ reg = <0x20>;
+ };
+ };
+ };
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 566a675..5f20f54 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -16,6 +16,8 @@
#include
#include
#include
+#include
+#include
struct gpiomux {
struct i2c_adapter *parent;
@@ -57,29 +59,111 @@ static int __devinit match_gpio_chip_by_label(struct
gpio_chip *chip,
return !strcmp(chip->label, data);
}
+#ifdef CONFIG_OF
+static int __devinit i2c_mux_gpio_probe_dt(struct gpiomux *mux,
+ struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *adapter_np, *child;
+ struct i2c_adapter *adapter;
+ unsigned *values, *gpios;
+ int i = 0;
+
+ if (!np)
+ return 0;
+
+ adapter_np = of_parse_phandle(np, "i2c-parent", 0);
+ if (!adapter_np) {
+ dev_err(&pdev->dev, "Cannot parse i2c-parent\n");
+ re
[PATCH 2/2] ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049
This will allow to add the 3 Nuvoton NAU7802 ADCs and the NXP PCA9555
GPIO expander eventually.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/imx28-cfa10049.dts | 24
1 file changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts
b/arch/arm/boot/dts/imx28-cfa10049.dts
index 97ee098..2cda823 100644
--- a/arch/arm/boot/dts/imx28-cfa10049.dts
+++ b/arch/arm/boot/dts/imx28-cfa10049.dts
@@ -76,6 +76,30 @@
status = "okay";
};
+ i2cmux {
+ compatible = "i2c-mux-gpio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
+ i2c-parent = <&i2c1>;
+
+ i2c@0 {
+ reg = <0>;
+ };
+
+ i2c@1 {
+ reg = <1>;
+ };
+
+ i2c@2 {
+ reg = <2>;
+ };
+
+ i2c@3 {
+ reg = <3>;
+ };
+ };
+
usbphy1: usbphy@8007e000 {
status = "okay";
};
--
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
[PATCHv6 0/2] ARM: I2C: Add device tree bindings to i2c-mux-gpio
Hi everyone, This patchset adds the device tree entry to the CFA-10049 board of its i2c muxer. This muxer controls sub-buses that contains three Nuvoton NAU7802 ADCs and a NXP PCA955 GPIO expander. Support for these will be added eventually. Thanks, Maxime Changes from v5: - Fix few errors in the dt bindings documentation - Removed the change of the data variable to a pointer in the gpiomux structure Maxime Ripard (2): i2c: mux: Add dt support to i2c-mux-gpio driver ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049 .../devicetree/bindings/i2c/i2c-mux-gpio.txt | 81 +++ arch/arm/boot/dts/imx28-cfa10049.dts | 24 drivers/i2c/muxes/i2c-mux-gpio.c | 146 +++- 3 files changed, 220 insertions(+), 31 deletions(-) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt -- 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 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
> "Andreas" == Andreas Larsson writes: >> Are all platforms using i2c-ocores guaranteed to provide ioread32be / >> iowrite32be or should we stick an #ifdef CONFIG_SPARC around it? Andreas> As far as I can see, after digging around, the only platforms that Andreas> have ioread/write32, but not ioread/write32be are frv and mn10300. Do Andreas> you know if those platforms are using i2c-ocores? Not to my knowledge, no. In that case: Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- 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: mux: Add dt support to i2c-mux-gpio driver
> "MR" == Maxime Ripard writes: Hi, MR> +* Standard I2C mux properties. See mux.txt in this directory. MR> +* I2C child bus nodes. See mux.txt in this directory. MR> + MR> +Optional properties: MR> +- idle-state: value to set to the muxer when idle. When no value is >> >> How about 'bitmask defining mux state when idle' instead? MR> Since the array documented as using the bitmasks in the platform_data MR> and described as an array of bitmasks is called "values", and the file MR> mux.txt talks about "numbers" to put into reg, won't this be confusing MR> to have three names for the exact same thing? Yeah, the mess is less than perfect. To my defense I did use bitmask in the platform data documentation: @values: Array of bitmasks of GPIO settings (low/high) for each position @idle: Bitmask to write to MUX when idle or GPIO_I2CMUX_NO_IDLE if not used But ok, I don't feel strongly about it. >> Why this change? I don't see why it is needed and the patch would be a >> lot easier to review without all the s/.data/->data/ noise. MR> Ah yes, since mux is already allocated using kcalloc, we don't need to MR> do it for data as well. I will remove it. Ok, great. -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- 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 5/5] i2c: i2c-sh_mobile: fix spurious transfer request timed out
Ensure that any of preceding register write operations to the I2C hardware block reached the module, and the write data is reflected in the registers, before leaving the interrupt handler. Otherwise, we'll suffer from spurious WAIT interrupts that lead to 'Transfer request timed out' message, and the transaction failed. Tracked-down-by: Teppei Kamijou Signed-off-by: Shinya Kuribayashi --- drivers/i2c/busses/i2c-sh_mobile.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c index 4c28358..9411c1b 100644 --- a/drivers/i2c/busses/i2c-sh_mobile.c +++ b/drivers/i2c/busses/i2c-sh_mobile.c @@ -469,6 +469,9 @@ static irqreturn_t sh_mobile_i2c_isr(int irq, void *dev_id) wake_up(&pd->wait); } + /* defeat write posting to avoid spurious WAIT interrupts */ + iic_rd(pd, ICSR); + return IRQ_HANDLED; } -- 1.7.12.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] i2c: i2c-sh_mobile: support I2C hardware block with a faster operating clock
On newer SH-/R-Mobile SoCs, a clock supply to the I2C hardware block,
which is used to generate the SCL clock output, is getting faster than
before, while on the other hand, the SCL clock control registers, ICCH
and ICCL, stay unchanged in 9-bit-wide (8+1).
On such silicons, the internal SCL clock counter gets incremented every
2 clocks of the operating clock.
This patch makes it configurable through platform data.
Signed-off-by: Shinya Kuribayashi
---
drivers/i2c/busses/i2c-sh_mobile.c | 5 +
include/linux/i2c/i2c-sh_mobile.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
b/drivers/i2c/busses/i2c-sh_mobile.c
index 4dc0cc3..4c28358 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -120,6 +120,7 @@ struct sh_mobile_i2c_data {
void __iomem *reg;
struct i2c_adapter adap;
unsigned long bus_speed;
+ unsigned int clks_per_count;
struct clk *clk;
u_int8_t icic;
u_int8_t flags;
@@ -231,6 +232,7 @@ static void sh_mobile_i2c_init(struct sh_mobile_i2c_data
*pd)
/* Get clock rate after clock is enabled */
clk_enable(pd->clk);
i2c_clk_khz = clk_get_rate(pd->clk) / 1000;
+ i2c_clk_khz /= pd->clks_per_count;
if (pd->bus_speed == STANDARD_MODE) {
tLOW= 47; /* tLOW = 4.7 us */
@@ -658,6 +660,9 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
pd->bus_speed = STANDARD_MODE;
if (pdata && pdata->bus_speed)
pd->bus_speed = pdata->bus_speed;
+ pd->clks_per_count = 1;
+ if (pdata && pdata->clks_per_count)
+ pd->clks_per_count = pdata->clks_per_count;
/* The IIC blocks on SH-Mobile ARM processors
* come with two new bits in ICIC.
diff --git a/include/linux/i2c/i2c-sh_mobile.h
b/include/linux/i2c/i2c-sh_mobile.h
index beda708..06e3089 100644
--- a/include/linux/i2c/i2c-sh_mobile.h
+++ b/include/linux/i2c/i2c-sh_mobile.h
@@ -5,6 +5,7 @@
struct i2c_sh_mobile_platform_data {
unsigned long bus_speed;
+ unsigned int clks_per_count;
};
#endif /* __I2C_SH_MOBILE_H__ */
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] i2c: i2c-sh_mobile: fix ICCH to avoid violation of the tHD;STA timing spec
The optimized ICCH/ICCL values in the previous commit afterward turned
out to violate tHD;STA timing spec. We had to take into account the
fall time of SDA signal (tf) at START condition. Fix it accordingly.
With this change, sh_mobile_i2c_icch() is virtually identical to
sh_mobile_i2c_iccl(), but they're providing good descriptions of
SH-/R-Mobile I2C hardware spec, and I'd leave them as separated.
Signed-off-by: Shinya Kuribayashi
---
drivers/i2c/busses/i2c-sh_mobile.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
b/drivers/i2c/busses/i2c-sh_mobile.c
index 1ad80c9..4dc0cc3 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -203,18 +203,23 @@ static u32 sh_mobile_i2c_iccl(unsigned long count_khz,
u32 tLOW, u32 tf, int off
return (((count_khz * (tLOW + tf)) + 5000) / 1) + offset;
}
-static u32 sh_mobile_i2c_icch(unsigned long count_khz, u32 tHIGH, int offset)
+static u32 sh_mobile_i2c_icch(unsigned long count_khz, u32 tHIGH, u32 tf, int
offset)
{
/*
* Conditional expression:
-* ICCH >= COUNT_CLK * tHIGH
+* ICCH >= COUNT_CLK * (tHIGH + tf)
*
* SH-Mobile IIC hardware is aware of SCL transition period 'tr',
* and can ignore it. SH-Mobile IIC controller starts counting
* the HIGH period of the SCL signal (tHIGH) after the SCL input
* voltage increases at VIH.
+*
+* Afterward it turned out calculating ICCH using only tHIGH spec
+* will result in violation of the tHD;STA timing spec. We need
+* to take into account the fall time of SDA signal (tf) at START
+* condition, in order to meet both tHIGH and tHD;STA specs.
*/
- return ((count_khz * tHIGH) + 5000) / 1 + offset;
+ return (((count_khz * (tHIGH + tf)) + 5000) / 1) + offset;
}
static void sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
@@ -250,7 +255,7 @@ static void sh_mobile_i2c_init(struct sh_mobile_i2c_data
*pd)
else
pd->icic &= ~ICIC_ICCLB8;
- pd->icch = sh_mobile_i2c_icch(i2c_clk_khz, tHIGH, offset);
+ pd->icch = sh_mobile_i2c_icch(i2c_clk_khz, tHIGH, tf, offset);
/* one more bit of ICCH in ICIC */
if ((pd->icch > 0xff) && (pd->flags & IIC_FLAG_HAS_ICIC67))
pd->icic |= ICIC_ICCHB8;
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] i2c: i2c-sh_mobile: optimize ICCH/ICCL values according to I2C bus speed
ICCH/ICCL values is supposed to be calculated/optimized to strictly meet
the timing specs required by the I2C standard. The resulting I2C bus
speed does not matter at all, if it's less than 100 or 400 kHz.
Also fix a typo in the comment, print icch/iccl values at probe, etc.
Signed-off-by: Shinya Kuribayashi
---
drivers/i2c/busses/i2c-sh_mobile.c | 121 ++---
1 file changed, 72 insertions(+), 49 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
b/drivers/i2c/busses/i2c-sh_mobile.c
index 309d0d5..1ad80c9 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -122,9 +122,9 @@ struct sh_mobile_i2c_data {
unsigned long bus_speed;
struct clk *clk;
u_int8_t icic;
- u_int8_t iccl;
- u_int8_t icch;
u_int8_t flags;
+ u_int16_t iccl;
+ u_int16_t icch;
spinlock_t lock;
wait_queue_head_t wait;
@@ -135,7 +135,8 @@ struct sh_mobile_i2c_data {
#define IIC_FLAG_HAS_ICIC67(1 << 0)
-#define NORMAL_SPEED 10 /* FAST_SPEED 40 */
+#define STANDARD_MODE 10
+#define FAST_MODE 40
/* Register offsets */
#define ICDR 0x00
@@ -187,55 +188,76 @@ static void iic_set_clr(struct sh_mobile_i2c_data *pd,
int offs,
iic_wr(pd, offs, (iic_rd(pd, offs) | set) & ~clr);
}
+static u32 sh_mobile_i2c_iccl(unsigned long count_khz, u32 tLOW, u32 tf, int
offset)
+{
+ /*
+* Conditional expression:
+* ICCL >= COUNT_CLK * (tLOW + tf)
+*
+* SH-Mobile IIC hardware starts counting the LOW period of
+* the SCL signal (tLOW) as soon as it pulls the SCL line.
+* In order to meet the tLOW timing spec, we need to take into
+* account the fall time of SCL signal (tf). Default tf value
+* should be 0.3 us, for safety.
+*/
+ return (((count_khz * (tLOW + tf)) + 5000) / 1) + offset;
+}
+
+static u32 sh_mobile_i2c_icch(unsigned long count_khz, u32 tHIGH, int offset)
+{
+ /*
+* Conditional expression:
+* ICCH >= COUNT_CLK * tHIGH
+*
+* SH-Mobile IIC hardware is aware of SCL transition period 'tr',
+* and can ignore it. SH-Mobile IIC controller starts counting
+* the HIGH period of the SCL signal (tHIGH) after the SCL input
+* voltage increases at VIH.
+*/
+ return ((count_khz * tHIGH) + 5000) / 1 + offset;
+}
+
static void sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
{
- unsigned long i2c_clk;
- u_int32_t num;
- u_int32_t denom;
- u_int32_t tmp;
+ unsigned long i2c_clk_khz;
+ u32 tHIGH, tLOW, tf;
+ int offset;
/* Get clock rate after clock is enabled */
clk_enable(pd->clk);
- i2c_clk = clk_get_rate(pd->clk);
-
- /* Calculate the value for iccl. From the data sheet:
-* iccl = (p clock / transfer rate) * (L / (L + H))
-* where L and H are the SCL low/high ratio (5/4 in this case).
-* We also round off the result.
-*/
- num = i2c_clk * 5;
- denom = pd->bus_speed * 9;
- tmp = num * 10 / denom;
- if (tmp % 10 >= 5)
- pd->iccl = (u_int8_t)((num/denom) + 1);
- else
- pd->iccl = (u_int8_t)(num/denom);
-
- /* one more bit of ICCL in ICIC */
- if (pd->flags & IIC_FLAG_HAS_ICIC67) {
- if ((num/denom) > 0xff)
- pd->icic |= ICIC_ICCLB8;
- else
- pd->icic &= ~ICIC_ICCLB8;
+ i2c_clk_khz = clk_get_rate(pd->clk) / 1000;
+
+ if (pd->bus_speed == STANDARD_MODE) {
+ tLOW= 47; /* tLOW = 4.7 us */
+ tHIGH = 40; /* tHD;STA = tHIGH = 4.0 us */
+ tf = 3;/* tf = 0.3 us */
+ offset = 0;/* No offset */
+ } else if (pd->bus_speed == FAST_MODE) {
+ tLOW= 13; /* tLOW = 1.3 us */
+ tHIGH = 6;/* tHD;STA = tHIGH = 0.6 us */
+ tf = 3;/* tf = 0.3 us */
+ offset = 0;/* No offset */
+ } else {
+ dev_err(pd->dev, "unrecognized bus speed %lu Hz\n",
+ pd->bus_speed);
+ goto out;
}
- /* Calculate the value for icch. From the data sheet:
- icch = (p clock / transfer rate) * (H / (L + H)) */
- num = i2c_clk * 4;
- tmp = num * 10 / denom;
- if (tmp % 10 >= 5)
- pd->icch = (u_int8_t)((num/denom) + 1);
+ pd->iccl = sh_mobile_i2c_iccl(i2c_clk_khz, tLOW, tf, offset);
+ /* one more bit of ICCL in ICIC */
+ if ((pd->iccl > 0xff) && (pd->flags & IIC_FLAG_HAS_ICIC67))
+ pd->icic |= ICIC_ICCLB8;
else
- pd->icch = (u_int8_t)(num/denom);
+ pd->icic &= ~ICIC_ICCLB8;
+ pd->icch = sh_mobile_i2c_ic
[PATCH 1/5] i2c: i2c-sh_mobile: calculate clock parameters at driver probing time
Currently SCL clock parameters (ICCH/ICCL) are calculated in
activate_ch(), which gets called every time sh_mobile_i2c_xfer() is
processed, while each I2C bus speed is system-defined and in general
those parameters do not have to be updated over I2C transactions.
The only reason I could see having it transaction-time is to adjust
ICCH/ICCL values according to the operating frequency of the I2C
hardware block, in the face of DFS (Dynamic Frequency Scaling).
However, this won't be necessary.
The operating frequency of the I2C hardware block can change _even_
in the middle of I2C transactions. There is no way to prevent it
from happening, and I2C hardware block can work with such dynamic
frequency change, of course.
Another is that ICCH/ICCL clock parameters optimized for the faster
operating frequency, can also be applied to the slower operating
frequency, as long as slave devices work. However, the converse is
not true. It would violate SCL timing specs of the I2C standard.
What we can do now is to calculate the ICCH/ICCL clock parameters
according to the fastest operating clock of the I2C hardware block.
And if that's the case, that calculation should be done just once
at driver-module-init time.
This patch moves ICCH/ICCL calculating part from activate_ch() into
sh_mobile_i2c_init(), and call it from sh_mobile_i2c_probe().
Note that sh_mobile_i2c_init() just prepares clock parameters using
the clock rate and platform data provided, but does _not_ make any
hardware I/O accesses. We don't have to care about run-time PM
maintenance here.
Signed-off-by: Shinya Kuribayashi
---
drivers/i2c/busses/i2c-sh_mobile.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
b/drivers/i2c/busses/i2c-sh_mobile.c
index 8110ca4..309d0d5 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -187,18 +187,15 @@ static void iic_set_clr(struct sh_mobile_i2c_data *pd,
int offs,
iic_wr(pd, offs, (iic_rd(pd, offs) | set) & ~clr);
}
-static void activate_ch(struct sh_mobile_i2c_data *pd)
+static void sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
{
unsigned long i2c_clk;
u_int32_t num;
u_int32_t denom;
u_int32_t tmp;
- /* Wake up device and enable clock */
- pm_runtime_get_sync(pd->dev);
- clk_enable(pd->clk);
-
/* Get clock rate after clock is enabled */
+ clk_enable(pd->clk);
i2c_clk = clk_get_rate(pd->clk);
/* Calculate the value for iccl. From the data sheet:
@@ -239,6 +236,15 @@ static void activate_ch(struct sh_mobile_i2c_data *pd)
pd->icic &= ~ICIC_ICCHB8;
}
+ clk_disable(pd->clk);
+}
+
+static void activate_ch(struct sh_mobile_i2c_data *pd)
+{
+ /* Wake up device and enable clock */
+ pm_runtime_get_sync(pd->dev);
+ clk_enable(pd->clk);
+
/* Enable channel and configure rx ack */
iic_set_clr(pd, ICCR, ICCR_ICE, 0);
@@ -632,6 +638,8 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
if (size > 0x17)
pd->flags |= IIC_FLAG_HAS_ICIC67;
+ sh_mobile_i2c_init(pd);
+
/* Enable Runtime PM for this device.
*
* Also tell the Runtime PM core to ignore children
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] i2c-sh_mobile non-urgent changes
Hello, This is the first batch to fix the i2c-sh_mobile driver. As a first step, this comprises of the SCL optimization work and a simple fix to annoying spurious WAIT interrupt issue; in other words, this does not change tx/rx data handling logic. This driver has an architectural problem with send/receive procedures and needs a fundamental overhaul. I've been working on splitting into logical chunks, but not yet completed. Patches are prepared against the vanilla v3.6. I don't have a chance to give it a try with v3.6+ kernels myself, but tend to be optimistic about that. I have been cooking these patches over a year, and they work perfectly fine with older kernels ..v3.4 so far. Shinya Kuribayashi (5): i2c: i2c-sh_mobile: calculate clock parameters at driver probing time i2c: i2c-sh_mobile: optimize ICCH/ICCL values according to I2C bus speed i2c: i2c-sh_mobile: fix ICCH to avoid violation of the tHD;STA timing spec i2c: i2c-sh_mobile: support I2C hardware block with a faster operating clock i2c: i2c-sh_mobile: fix spurious transfer request timed out drivers/i2c/busses/i2c-sh_mobile.c | 150 - include/linux/i2c/i2c-sh_mobile.h | 1 + 2 files changed, 98 insertions(+), 53 deletions(-) -- Shinya Kuribayashi Renesas Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
On 10/23/2012 10:24 PM, Peter Korsgaard wrote:
"Andreas" == Andreas Larsson writes:
[...]
Andreas> +/* Read and write functions for the GRLIB port of the controller.
Registers are
Andreas> + * 32-bit big endian and the PRELOW and PREHIGH registers are
merged into one
Andreas> + * register. The subsequent registers has their offset decreased
accordingly. */
Andreas> +static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
Andreas> +{
Andreas> + u32 rd;
Andreas> + int rreg = reg;
Andreas> + if (reg != OCI2C_PRELOW)
Andreas> + rreg--;
Andreas> + rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
Andreas> + if (reg == OCI2C_PREHIGH)
Andreas> + return (u8)rd >> 8;
Andreas> + else
Andreas> + return (u8)rd;
Andreas> +}
Andreas> +
Andreas> +static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8
value)
Andreas> +{
Andreas> + u32 curr, wr;
Andreas> + int rreg = reg;
Andreas> + if (reg != OCI2C_PRELOW)
Andreas> + rreg--;
Andreas> + if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
Andreas> + curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
Andreas> + if (reg == OCI2C_PRELOW)
Andreas> + wr = (curr & 0xff00) | value;
Andreas> + else
Andreas> + wr = (((u32)value) << 8) | (curr & 0xff);
Andreas> + } else {
Andreas> + wr = value;
Andreas> + }
Andreas> + iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
Are all platforms using i2c-ocores guaranteed to provide ioread32be /
iowrite32be or should we stick an #ifdef CONFIG_SPARC around it?
As far as I can see, after digging around, the only platforms that have
ioread/write32, but not ioread/write32be are frv and mn10300. Do you
know if those platforms are using i2c-ocores?
Cheers,
Andreas Larsson
--
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: i2c-ocores: Add irq support for sparc
On 10/23/2012 10:13 PM, Peter Korsgaard wrote: "Andreas" == Andreas Larsson writes: Andreas> There are no platform resources of type IORESOURCE_IRQ on Andreas> sparc, so the irq number is acquired in a different manner for Andreas> sparc. The general case uses platform_get_irq, that internally Andreas> still uses platform_get_resource. I have no idea why sparc is being odd in this regard, but assuming this is how it's done, I'm fine with this change. A quick grep doesn't find any other drivers doing this though: git grep -l archdata.irqs drivers | xargs grep platform_get_irq Acked-by: Peter Korsgaard Other drivers that work both on sparc and on other platforms usually use irq_of_parse_and_map on a corresponding device_node. For non-sparc architectures irq_of_parse_and_map sets up mappings that needs to be teared down on module exit. Sparc however has its own version of irq_of_parse_and_map that just returns the irq number using archdata.irq[]. I am trying to get through a patch platform_get_irq to work for sparc as well. If that eventually goes through, the CONFIG_SPARC stuff can then be removed cleanly from this driver withouth having to mess with irq_of_parse_and_map and tearing mappings down. Another solution is to use irq_of_parse_and_map for the of-case if no irq was found using platform_get_irq. But that would make for more rearrangements and add the need for irq_dispose_mapping to be added on module exit as well (even though the disposing would do nothing for sparc). Cheers, Andreas Larsson -- 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: mux: Add dt support to i2c-mux-gpio driver
Hi Peter,
Le 23/10/2012 21:51, Peter Korsgaard a écrit :
>> "MR" == Maxime Ripard writes:
>
> Hi Maxime,
>
> I'm fine with the idea, but a few comments:
>
>
> MR> Allow the i2c-mux-gpio to be used by a device tree enabled device. The
> MR> bindings are inspired by the one found in the i2c-mux-pinctrl driver.
>
> MR> Signed-off-by: Maxime Ripard
> MR> ---
> MR> .../devicetree/bindings/i2c/i2c-mux-gpio.txt | 81 ++
> MR> drivers/i2c/muxes/i2c-mux-gpio.c | 169
> +++-
> MR> 2 files changed, 211 insertions(+), 39 deletions(-)
> MR> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
>
> MR> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
> b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
> MR> new file mode 100644
> MR> index 000..335fc4e
> MR> --- /dev/null
> MR> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
> MR> @@ -0,0 +1,81 @@
> MR> +GPIO-based I2C Bus Mux
> MR> +
> MR> +This binding describes an I2C bus multiplexer that uses GPIOs to
> MR> +route the I2C signals.
> MR> +
> MR> + +-+ +-+
> MR> + | dev | | dev |
> MR> ++++-+ +-+
> MR> +| SoC| ||
> MR> +|| /++
> MR> +| +--+ | +--+child bus A, on GPIO value set to 0
> MR> +| | I2C |-|--| Mux |
> MR> +| +--+ | +--+---+child bus B, on GPIO value set to 1
> MR> +|| |\--+++
> MR> +| +--+ | | |||
> MR> +| | GPIO |-|-++-+ +-+ +-+
> MR> +| +--+ | | dev | | dev | | dev |
> MR> +++ +-+ +-+ +-+
> MR> +
> MR> +Required properties:
> MR> +- compatible: i2c-mux-gpio
> MR> +- i2c-parent: The phandle of the I2C bus that this multiplexer's
> master-side
> MR> + port is connected to.
> MR> +- mux-gpios: list of gpios to use to control the muxer
>
> s/to use to/used to/
>
>
> MR> +* Standard I2C mux properties. See mux.txt in this directory.
> MR> +* I2C child bus nodes. See mux.txt in this directory.
> MR> +
> MR> +Optional properties:
> MR> +- idle-state: value to set to the muxer when idle. When no value is
>
> How about 'bitmask defining mux state when idle' instead?
Since the array documented as using the bitmasks in the platform_data
and described as an array of bitmasks is called "values", and the file
mux.txt talks about "numbers" to put into reg, won't this be confusing
to have three names for the exact same thing?
> MR> + given, it defaults to the last value used.
> MR> +
> MR> +For each i2c child node, an I2C child bus will be created. They will
> MR> +be numbered based on the reg property of each node.
>
> As far as I can see they are dynamically assigned numbers in the order
> they are listed in the dt.
Ah, yes.
> MR> +
> MR> +Whenever an access is made to a device on a child bus, the value set
> MR> +in the revelant node's reg property will be output using the list of
> MR> +GPIOs, the first in the list holding the most-significant value.
>
> Isn't it the other way around, E.G. first gpio in mux-gpios controlled
> by LSB of reg property, next gpio by lsb+1, ..?
True indeed.
> MR> +
> MR> +If an idle state is defined, using the idle-state (optional) property,
> MR> +whenever an access is not being made to a device on a child bus, the
> MR> +idle value will be programmed into the GPIOs.
>
> s/idle value will be programmed into the GPIOS/GPIOS set according to
> the idle value bitmask/
Once again, I'm really not fond of the term "bitmask".
[..]
> MR> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c
> b/drivers/i2c/muxes/i2c-mux-gpio.c
> MR> index 566a675..7ebef01 100644
> MR> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> MR> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> MR> @@ -16,11 +16,13 @@
> MR> #include
> MR> #include
> MR> #include
> MR> +#include
> MR> +#include
>
> MR> struct gpiomux {
> MR> struct i2c_adapter *parent;
> MR> struct i2c_adapter **adap; /* child busses */
> MR> - struct i2c_mux_gpio_platform_data data;
> MR> + struct i2c_mux_gpio_platform_data *data;
>
>
> Why this change? I don't see why it is needed and the patch would be a
> lot easier to review without all the s/.data/->data/ noise.
Ah yes, since mux is already allocated using kcalloc, we don't need to
do it for data as well. I will remove it.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-in
