On Fri, Aug 24, 2018 at 4:07 PM dbasehore . <dbaseh...@chromium.org> wrote: > > > > On Fri, Aug 24, 2018 at 1:49 AM Andi Shyti <a...@etezian.org> wrote: >> >> Hi Derek, >> >> > > > On Thu, Aug 23, 2018 at 04:10:13PM -0700, Derek Basehore wrote: >> > > > > We only need to wait 10ms instead of 30ms before starting fastboot or >> > > > > sending IAP on the touchscreen. Also, instead of delaying everytime >> > > > > sw_reset is called, this delays 10ms in the function that starts >> > > > > fastboot. There's also an explicit 20ms delay before sending IAP when >> > > > > updating the firmware, so no additional delay is needed there. This >> > > > > change also has the benefit of not delaying when wakeup is enabled >> > > > > during suspend. This is because sw_reset is called, yet fastboot >> > > > > isn't. >> >> ... >> >> > > > > - /* >> > > > > - * We should wait at least 10 msec (but no more than 40) >> > > > > before >> > > > > - * sending fastboot or IAP command to the device. >> > > > > - */ >> > > > > - msleep(30); >> > > > > - >> >> moving from 30 to 0 is a bit alarming... what does the datasheet >> say? > > > We're moving from 30msec to 10msec. We used to do this in an older kernel. > >> >> >> Sometimes delays are implicit in the system where you are testing >> the driver, so that without any msleep it might work in your >> system but it might not on others. >> >> > + /* >> > + * We should wait at least 10 msec (but no more than 40) >> > before >> > + * sending IAP command to the device. >> > + */ >> > msleep(20); >> >> I agree though that it's not nice to wait twice here (even though >> as Dmitry says it doesn't hurt so much). Wouldn't it make more >> sense to remove this msleep instead? >> This way... > > > We still need this if the delay is removed from the sw_reset function. We > have a total of 20msecs delayed between sw_reset and sending IAP in our old > kernel which should be well tested. There seems to be no reason why the > delays increased. > >> >> >> > + /* >> > + * We should wait at least 10 msec (but no more than 40) before >> > sending >> > + * fastboot command to the device. >> > + */ >> > + usleep_range(10 * 1000, 11 * 1000); >> > + >> > error = elants_i2c_send(client, boot_cmd, sizeof(boot_cmd)); >> >> ... you do not need to add an extra sleep here. > > > I added it here since I removed the 30msec sleep in sw_reset. This prevents > us from sleeping on resume when the touch controller is already on for wake > up capabilities. This is the same delay that we have in our old 3.8 kernel, > so it's pretty well tested and shipped in production. > >> >> >> Andi
Resending since it seems my plain text setting was removed...
