| Message ID | 1484238105-10785-1-git-send-email-hofrat@osadl.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 01/12/2017 05:21 PM, Nicholas Mc Guire wrote: > the delays here are in the 10 to 20ms range so msleep() will do - no > need to burden the highres timer subsystem. > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > --- > > Problem found by coccinelle script > > While msleep(10) has a worst case uncertainty of 10ms (on HZ=100 systems) > this seems ok here as the delays are not called frequently (init and > reset functions) By the same logic, this is not much of a burden on the high-res timer subsys though. > and the uncertainty of 10ms fits the permitted range of > the original usleep_ranges(). Either way this patch is fine with me. Regards, Hans > > Patch was compile tested with: x86_64_defconfig + > CONFIG_TOUCHSCREEN_SILEAD=m > > Patch is against 4.10-rc3 (localversion-next is next-20170112) > > drivers/input/touchscreen/silead.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c > index 404830a..3aa885c 100644 > --- a/drivers/input/touchscreen/silead.c > +++ b/drivers/input/touchscreen/silead.c > @@ -58,8 +58,7 @@ > #define SILEAD_POINT_X_MSB_OFF 0x03 > #define SILEAD_TOUCH_ID_MASK 0xF0 > > -#define SILEAD_CMD_SLEEP_MIN 10000 > -#define SILEAD_CMD_SLEEP_MAX 20000 > +#define SILEAD_CMD_SLEEP_MIN 10 /* 10+ ms */ > #define SILEAD_POWER_SLEEP 20 > #define SILEAD_STARTUP_SLEEP 30 > > @@ -190,25 +189,25 @@ static int silead_ts_init(struct i2c_client *client) > SILEAD_CMD_RESET); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > error = i2c_smbus_write_byte_data(client, SILEAD_REG_TOUCH_NR, > data->max_fingers); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > error = i2c_smbus_write_byte_data(client, SILEAD_REG_CLOCK, > SILEAD_CLOCK); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > error = i2c_smbus_write_byte_data(client, SILEAD_REG_RESET, > SILEAD_CMD_START); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > return 0; > > @@ -225,19 +224,19 @@ static int silead_ts_reset(struct i2c_client *client) > SILEAD_CMD_RESET); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > error = i2c_smbus_write_byte_data(client, SILEAD_REG_CLOCK, > SILEAD_CLOCK); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > error = i2c_smbus_write_byte_data(client, SILEAD_REG_POWER, > SILEAD_CMD_START); > if (error) > goto i2c_write_err; > - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); > + msleep(SILEAD_CMD_SLEEP_MIN); > > return 0; > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 12, 2017 at 9:46 AM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 01/12/2017 05:21 PM, Nicholas Mc Guire wrote: >> >> the delays here are in the 10 to 20ms range so msleep() will do - no >> need to burden the highres timer subsystem. >> >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> >> --- >> >> Problem found by coccinelle script >> >> While msleep(10) has a worst case uncertainty of 10ms (on HZ=100 systems) >> this seems ok here as the delays are not called frequently (init and >> reset functions) > > > By the same logic, this is not much of a burden on the high-res timer > subsys though. > >> and the uncertainty of 10ms fits the permitted range of >> the original usleep_ranges(). > > > Either way this patch is fine with me. I'd rather not because next will come a checkpatch warrior and I will have to convince them why msleep is OK here. And another one, and another one... :( Thanks.
On Thu, Jan 12, 2017 at 10:10:44AM -0800, Dmitry Torokhov wrote: > On Thu, Jan 12, 2017 at 9:46 AM, Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > > > On 01/12/2017 05:21 PM, Nicholas Mc Guire wrote: > >> > >> the delays here are in the 10 to 20ms range so msleep() will do - no > >> need to burden the highres timer subsystem. > >> > >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > >> --- > >> > >> Problem found by coccinelle script > >> > >> While msleep(10) has a worst case uncertainty of 10ms (on HZ=100 systems) > >> this seems ok here as the delays are not called frequently (init and > >> reset functions) > > > > > > By the same logic, this is not much of a burden on the high-res timer > > subsys though. > > > >> and the uncertainty of 10ms fits the permitted range of > >> the original usleep_ranges(). > > > > > > Either way this patch is fine with me. > > I'd rather not because next will come a checkpatch warrior and I will > have to convince them why msleep is OK here. And another one, and > another one... :( > there is no checkpatch warning here - checkpatch only throws warnings of range < 20ms if hardcoded but this is in a #define so its fine with respect to checkpatch. But if there are concerns with this - thats fine - its most likely not critical - the goal is to have a consistent usage of highres timers - including limiting there use to the cases where its really needed. thx! hofrat -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c index 404830a..3aa885c 100644 --- a/drivers/input/touchscreen/silead.c +++ b/drivers/input/touchscreen/silead.c @@ -58,8 +58,7 @@ #define SILEAD_POINT_X_MSB_OFF 0x03 #define SILEAD_TOUCH_ID_MASK 0xF0 -#define SILEAD_CMD_SLEEP_MIN 10000 -#define SILEAD_CMD_SLEEP_MAX 20000 +#define SILEAD_CMD_SLEEP_MIN 10 /* 10+ ms */ #define SILEAD_POWER_SLEEP 20 #define SILEAD_STARTUP_SLEEP 30 @@ -190,25 +189,25 @@ static int silead_ts_init(struct i2c_client *client) SILEAD_CMD_RESET); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); error = i2c_smbus_write_byte_data(client, SILEAD_REG_TOUCH_NR, data->max_fingers); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); error = i2c_smbus_write_byte_data(client, SILEAD_REG_CLOCK, SILEAD_CLOCK); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); error = i2c_smbus_write_byte_data(client, SILEAD_REG_RESET, SILEAD_CMD_START); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); return 0; @@ -225,19 +224,19 @@ static int silead_ts_reset(struct i2c_client *client) SILEAD_CMD_RESET); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); error = i2c_smbus_write_byte_data(client, SILEAD_REG_CLOCK, SILEAD_CLOCK); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); error = i2c_smbus_write_byte_data(client, SILEAD_REG_POWER, SILEAD_CMD_START); if (error) goto i2c_write_err; - usleep_range(SILEAD_CMD_SLEEP_MIN, SILEAD_CMD_SLEEP_MAX); + msleep(SILEAD_CMD_SLEEP_MIN); return 0;
the delays here are in the 10 to 20ms range so msleep() will do - no need to burden the highres timer subsystem. Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> --- Problem found by coccinelle script While msleep(10) has a worst case uncertainty of 10ms (on HZ=100 systems) this seems ok here as the delays are not called frequently (init and reset functions) and the uncertainty of 10ms fits the permitted range of the original usleep_ranges(). Patch was compile tested with: x86_64_defconfig + CONFIG_TOUCHSCREEN_SILEAD=m Patch is against 4.10-rc3 (localversion-next is next-20170112) drivers/input/touchscreen/silead.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)