RE: [PATCH] i2c-at91: fix data-loss issue
Adrian Yanes wrote on 2012-04-21:
> On the other hand we found that the Underrun Error (UNRE) is not handled
> in the driver. When we send data up > 2-4 bytes (quite random to say
> when it really fails) and we add some dev_dbg() to print
> dev->transfer_status we get 141 (==UNRE). According with the datasheet,
> after the first UNRE received "this action automatically generated a
> STOP bit in Master Mode. Reset by read in TWI_SR when TXCOMP is set."
>
> We thought that one possibility for this was that the board was too slow
> to process the requests or that other interrupts were interfering.
> Disabling the interrupts inside of the twi interrupt handler did not
> help either.
>
> The datasheet does not mention any method to implement some mechanism to
> avoid the UNRE telling the hardware to wait a bit longer for the next
> byte. Thus, one way will be to restart the transfer with the remaining
> bytes (maybe only applicable to at91rm9200) or just to return some
> error/message to userland informing that could not send all the data.
The latter is probably the easiest and most transparent solution.
There is no UNRE on G45, it just pauses the clock on an underrun
condition.
So in case UNRE is set, EIO should be returned similar to the already
handled OVRE:
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index a6f9e73..a84e19b 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -238,6 +238,11 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
dev_err(dev->dev, "overrun while reading\n");
return -EIO;
}
+ if (dev->transfer_status & AT91_TWI_UNRE && dev->is_rm9200) {
+ dev_err(dev->dev, "underrun while writing\n");
+ return -EIO;
+ }
+
dev_dbg(dev->dev, "transfer complete\n");
return 0;
N�r��yb�X��ǧv�^�){.n�+{��g"��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: [PATCH 1/2] i2c-eg20t: add helper function for xfer check
On Sat, Apr 21, 2012 at 5:39 AM, Wolfram Sang wrote: > On Thu, Apr 19, 2012 at 03:38:04PM +0900, Tomoya MORINAGA wrote: >> Currently, there are the same code for xfer complete check processing. >> So, I add helper function pch_i2c_wait_for_check_xfer. >> >> Reported-by: Wolfram Sang >> Signed-off-by: Tomoya MORINAGA > > Both applied to next, thanks for doing this! ~120 lines saved, that's > about 10%. And also way better maintainable, yay. Thank you for your applied. > > My checkers found some other issues, for which I will send patches in a > minute. Please have a look and test. OK. I'll do until this week end and report to you. thanks, -- ROHM Co., Ltd. tomoya -- 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] i2c: designware: Add clk_{un}prepare() support
On Sun, Apr 22, 2012 at 11:08:55PM +0530, viresh kumar wrote:
> On 4/22/12, Wolfram Sang wrote:
> > On Tue, Apr 17, 2012 at 05:04:31PM +0530, Viresh Kumar wrote:
> >> clk_{un}prepare is mandatory for platforms using common clock framework.
> >> Since
> >> this driver is used by SPEAr platform, which supports common clock
> >> framework,
> >> add clk_{un}prepare() support for designware i2c.
> >>
> >> Signed-off-by: Viresh Kumar
> >
> > ? Can't apply, which version is this against? Looks good otherwise.
>
> Because following is waiting in you todo list and must be applied
> before this one: :)
Okay, thanks, both applied to next. If you mention dependencies somewhere
when submitting, that might speed things, a tiny bit at least.
> What are your plans for following patchset:
> i2c/adapter: Add bus recovery infrastructure
My plans are to get them into 3.5.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 1/2] i2c/busses: Add PM support
On Fri, Feb 24, 2012 at 05:01:15PM +0530, Viresh Kumar wrote: > From: Deepak Sikri > > This patch adds in support for standby/S2R/hybernate for i2c-designware > driver. > > Signed-off-by: Deepak Sikri > Signed-off-by: Rajeev Kumar Applied to next, thanks! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH V2] i2c: designware: Add clk_{un}prepare() support
On 4/22/12, Wolfram Sang wrote:
> On Tue, Apr 17, 2012 at 05:04:31PM +0530, Viresh Kumar wrote:
>> clk_{un}prepare is mandatory for platforms using common clock framework.
>> Since
>> this driver is used by SPEAr platform, which supports common clock
>> framework,
>> add clk_{un}prepare() support for designware i2c.
>>
>> Signed-off-by: Viresh Kumar
>
> ? Can't apply, which version is this against? Looks good otherwise.
Because following is waiting in you todo list and must be applied
before this one: :)
Author: Deepak Sikri
Date: Mon Sep 26 18:42:10 2011 +0530
i2c/busses: Add PM support
This patch adds in support for standby/S2R/hybernate for
i2c-designware driver.
Signed-off-by: Deepak Sikri
Signed-off-by: Rajeev Kumar
---
drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++
1 files changed, 27 insertions(+), 0 deletions(-)
Actually suspend/resume of above are also fixed by this patch.
What are your plans for following patchset:
i2c/adapter: Add bus recovery infrastructure
Would be good if you can apply that or ask for another version if you see some
shortcomings. Otherwise this kind of issues will happen again.
Please allocate some time for this, it is really important for other bus
drivers. All controllers/slaves have that bug.
They will be fixed only once this is applied. :)
--
viresh
--
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] i2c: designware: Add clk_{un}prepare() support
On Tue, Apr 17, 2012 at 05:04:31PM +0530, Viresh Kumar wrote:
> clk_{un}prepare is mandatory for platforms using common clock framework. Since
> this driver is used by SPEAr platform, which supports common clock framework,
> add clk_{un}prepare() support for designware i2c.
>
> Signed-off-by: Viresh Kumar
? Can't apply, which version is this against? Looks good otherwise.
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 3/5] i2c: remove ixp2000 driver
On Tue, Apr 03, 2012 at 08:34:01PM -0500, Rob Herring wrote: > From: Rob Herring > > The platform is removed, so there are no users of this driver. > > Signed-off-by: Rob Herring Thanks, applied to next. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.
On Thu, Apr 12, 2012 at 02:14:23PM -0700, David Daney wrote:
> From: David Daney
>
> For 'normal' i2c bus drivers, we can call of_i2c_register_devices()
> and have the device tree framework automatically populate the bus with
> the devices specified in the device tree.
>
> This patch adds a common code to the i2c mux framework to have the mux
> sub-busses be populated by the of_i2c_register_devices() too. If the
> mux device has an of_node, we populate the sub-bus' of_node so that
> the subsequent call to of_i2c_register_devices() will find the
> corresponding devices.
>
> It seemed better to put this logic in i2c_add_mux_adapter() rather
> than the individual mux drivers, as they will all probably want to do
> the same thing.
Both patches looking mostly good, two things here:
> + /*
> + * Try to get populate the mux adapter's of_node, expands to
"get populate"? I'd think you mean "populate" only, but am not sure
enough to fix it myself.
> + * nothing if !CONFIG_OF.
> + */
> + if (mux_dev->of_node) {
> + struct device_node *child;
> + u32 reg;
> + int ret;
We have a "ret" already in this function.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
