> -----Original Message----- > From: ext Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] > Sent: 20. marraskuuta 2009 18:28 > To: Jokiniemi Kalle (Nokia-D/Tampere); Tony Lindgren > Cc: linux-omap@xxxxxxxxxxxxxxx; m-sonasath@xxxxxx; Hogander > Jouni (Nokia-D/Tampere) > Subject: Re: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up > latency constraint in i2c > > <kalle.jokiniemi@xxxxxxxxx> writes: > > > Could you please take this patch in? Currently i2c is not > working very > > well with cpuidle and off mode, and the patch is needed. > > > > I would go with V4 of the patch. > > Kalle, > > Last I saw with this thread, it didn't look like there was resolution. OK. Let's resolve it then :) Moiz, could you please comment on my last reply in the old thread: --------- (sent on 29th of October) ---------------- > >>Well, I don't completely agree with the necessity of preparing for > >>different rx/tx thresholds. For this to make sense, the i2c-omap > >>driver should first separate in it's code the use of rx and tx > >>thresholds. If someone is planning to do that, he/she should anyway > >>update the usage of fifo_size throughout the code, > including the wake > >>up latency setting. > >> > >>Anyways, attached a patch that separates the mpu wake up > latencies for > >>rx and tx. In case that is needed. Though I'm not for it, since it > >>adds unneeded complexity. > >> --------- (sent on 29th of October) ---------------- Attached also the alternative patch I was referring to. > If things are resolved, this shouldn't go upstream via PM > branch, since it doesn't seem to have any dependencies on the > PM branch. Ah, the pm interfaces are in mainline now. > > It should go upstream either via the I2C list, or via Tony. > > If it goes via i2c list, I can add it to PM branch while > waiting for it to merge. Tony, would you take this? If not, what is the address for i2c mailing list, and who maintains i2c? - Kalle > > Kevin > > > >>-----Original Message----- > >>From: linux-omap-owner@xxxxxxxxxxxxxxx > >>[mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of ext Kalle > >>Jokiniemi > >>Sent: 29. lokakuuta 2009 10:55 > >>To: Sonasath, Moiz > >>Cc: khilman@xxxxxxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx > >>Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency > >>constraint in i2c > >> > >>OK, let's try this once more, since my mail did not seem to go to > >>linux-omap. > >> > >>Sorry for the spam. > >> > >>See my comments below: > >> > >>On Fri, 2009-10-23 at 18:53 +0300, Sonasath, Moiz wrote: > >>> Hello Jokiniemi! > >>> > >>> > -----Original Message----- > >>> > From: Kalle Jokiniemi [mailto:kalle.jokiniemi@xxxxxxxxx] > >>> > Sent: Wednesday, October 21, 2009 6:51 AM > >>> > To: khilman@xxxxxxxxxxxxxxxxxxx > >>> > Cc: linux-omap@xxxxxxxxxxxxxxx; Kalle Jokiniemi; Sonasath, Moiz; > >>> Jarkko > >>> > Nikula; Paul Walmsley; Menon, Nishanth > >>> > Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency > >>> constraint in > >>> > i2c > >>> > > >>> > While waiting for completion of the i2c transfer, the MPU > >>could hit > >>> > OFF mode and cause several msecs of delay that made i2c > transfers > >>> > fail more often. The extra delays and subsequent > re-trys cause i2c > >>> > clocks to be active more often. This has also an negative > >>effect on > >>> > power consumption. > >>> > > >>> > Created a mechanism for passing and using the > constraint setting > >>> > function in driver code. The used mpu wake up latency > constraints > >>> > are now set individually per bus, and they are > calculated based on > >>> > clock rate and fifo size. > >>> > > >>> > Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley, and > >>Nishanth > >>> > Menon for tuning out the details of this patch. > >>> > > >>> > Cc: Moiz Sonasath <m-sonasath@xxxxxx> > >>> > Cc: Jarkko Nikula <jhnikula@xxxxxxxxx> > >>> > Cc: Paul Walmsley <paul@xxxxxxxxx> > >>> > Cc: Nishanth Menon <nm@xxxxxx> > >>> > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx> > >>> > --- > >>> > > >>> > >>> > >>> > dev->speed = speed; > >>> > dev->idle = 1; > >>> > @@ -911,6 +923,11 @@ omap_i2c_probe(struct > platform_device *pdev) > >>> > */ > >>> > dev->fifo_size = (dev->fifo_size / 2); > >>> > dev->b_hw = 1; /* Enable hardware fixes */ > >>> > + > >>> > + /* calculate wakeup latency constraint > for MPU */ > >>> > + if (dev->set_mpu_wkup_lat != NULL) > >>> > + dev->latency = (1000000 * > dev->fifo_size) / > >>> > + (1000 * speed / 8); > >>> > } > >>> > >>> IMHO, here instead of using 'dev->fifo_size' for calculating the > >>> 'dev->latency', we need to use: > >>> > >>> 1. For RX case, to avoid Reciver overrun: > >>> if (msg->flags & I2C_M_RD) > >>> Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in > >>calculating > >>> dev->latency > >>> > >>> Because conceptually, RDR/RRDY interrupts are generated when > >>RTRSH is > >>> reached, so we want the MPU to be wake up within the time it > >>takes to > >>> fill the FIFO from RTRSH to FIFO Depth (FIFO full). > >>> > >>> 2. For TX case, to avoid Transmitter Underflow: > >>> if (!(msg->flags & I2C_M_RD)) > >>> Use (FIFO THRSH)bytes in calculating dev->latency > >>> > >>> Because conceptually, XDR/XRDY interrupts are generated when > >>XTRSH is > >>> reached, so we want the MPU to be wake up within the time it > >>takes to > >>> drain the FIFO from XTRSH to Zero (FIFO empty). > >>> > >>> Using, dev->fifo_size instead, works in the present code > because we > >>> have a RTRSH/XTRSH = dev->fifo_size/2 = 4 bytes And > therefore, (FIFO > >>> Depth)bytes - (FIFO THRSH)bytes = 8 - 4 = 4 bytes > >>> > >>> But, to make it more generic in future and to make it > independent of > >>> any changes in the RTRSH/XTRSH values or FIFO depths in > future, we > >>> should use a generic code here. > >> > >>Well, I don't completely agree with the necessity of preparing for > >>different rx/tx thresholds. For this to make sense, the i2c-omap > >>driver should first separate in it's code the use of rx and tx > >>thresholds. If someone is planning to do that, he/she should anyway > >>update the usage of fifo_size throughout the code, > including the wake > >>up latency setting. > >> > >>Anyways, attached a patch that separates the mpu wake up > latencies for > >>rx and tx. In case that is needed. Though I'm not for it, since it > >>adds unneeded complexity. > >> > >>- Kalle > >> > >>> > >>> > > >>> > /* reset ASAP, clearing any IRQs */ diff --git > >>> > a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h new > >>file mode > >>> > 100644 index 0000000..1362fba > >>> > --- /dev/null > >>> > +++ b/include/linux/i2c-omap.h > >>> > @@ -0,0 +1,9 @@ > >>> > +#ifndef __I2C_OMAP_H__ > >>> > +#define __I2C_OMAP_H__ > >>> > + > >>> > +struct omap_i2c_bus_platform_data { > >>> > + u32 clkrate; > >>> > + void (*set_mpu_wkup_lat)(struct device *dev, > >>int set); > >>> > +}; > >>> > + > >>> > +#endif > >>> > -- > >>> > 1.5.4.3 > >>> > >>> Regards > >>> Moiz Sonasath > >>> > >> >
Attachment:
0001-OMAP-I2C-Add-mpu-wake-up-latency-constraint-in-i2c.patch
Description: 0001-OMAP-I2C-Add-mpu-wake-up-latency-constraint-in-i2c.patch
