> -----Original Message----- > From: Kalle Jokiniemi [mailto:kalle.jokiniemi@xxxxxxxxx] > Sent: Tuesday, October 27, 2009 7:02 AM > To: Sonasath, Moiz > Cc: khilman@xxxxxxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; Jarkko > Nikula; Paul Walmsley; Menon, Nishanth; Pandita, Vikram; jouni.hogander > Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint > in i2c > > 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. > Yes Kalle, you are right! Not having different RX/TX wake-up latencies will absolutely work fine with the in-place code as we have both the RX/TX thresholds same. But, I think in future we might have to play around with different RX/TX thresholds and so from a conceptually right and generic point of view it makes sense to incur the cost of the added complexity. The patch V4 looks perfect to me :) - Moiz > - 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
