Message ID | 20210305020310.550527-1-caleb@connolly.tech (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | input: s6sy761: fix coordinate read bit shift | expand |
Hi Caleb, > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 Please clean up the commit message. > The touch coordinates are read by shifting a value left by 3, > this is incorrect and effectively causes the coordinates to > be half of the correct value. > > Shift by 4 bits instead to report the correct value. > > This matches downstream examples, and has been confirmed on my > device (OnePlus 7 Pro). The real reason is that from the register we get: byte 3 byte 2 byte 1 +--------+--------+ +-----------------+ +-----------------+ | | | | | | | | X[3:0] | Y[3:0] | | Y[11:4] | | X[11:4] | | | | | | | | +--------+--------+ +-----------------+ +-----------------+ and the 12 bit values have to fit in a 16bit variable. The upper 8 bits (in event[2] and event[1] need to be shifted left by '4' and not by '3' in order to leave space to the lower 4 bits (in event[3]). > > Signed-off-by: Caleb Connolly <caleb@connolly.tech> > --- > drivers/input/touchscreen/s6sy761.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c > index b63d7fdf0cd2..85a1f465c097 100644 > --- a/drivers/input/touchscreen/s6sy761.c > +++ b/drivers/input/touchscreen/s6sy761.c > @@ -145,8 +145,8 @@ static void s6sy761_report_coordinates(struct s6sy761_data *sdata, > u8 major = event[4]; > u8 minor = event[5]; > u8 z = event[6] & S6SY761_MASK_Z; > - u16 x = (event[1] << 3) | ((event[3] & S6SY761_MASK_X) >> 4); > - u16 y = (event[2] << 3) | (event[3] & S6SY761_MASK_Y); > + u16 x = (event[1] << 4) | ((event[3] & S6SY761_MASK_X) >> 4); > + u16 y = (event[2] << 4) | (event[3] & S6SY761_MASK_Y); the devil knows how that '3' has ended up there :) Thanks for catching it! Reviewed-by: Andi Shyti <andi@etezian.org> Andi
Hi Andi, On 05/03/2021 11:55 am, Andi Shyti wrote: > Hi Caleb, > >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA256 > Please clean up the commit message. oops! Sorry > >> The touch coordinates are read by shifting a value left by 3, >> this is incorrect and effectively causes the coordinates to >> be half of the correct value. >> >> Shift by 4 bits instead to report the correct value. >> >> This matches downstream examples, and has been confirmed on my >> device (OnePlus 7 Pro). > The real reason is that from the register we get: > > byte 3 byte 2 byte 1 > +--------+--------+ +-----------------+ +-----------------+ > | | | | | | | > | X[3:0] | Y[3:0] | | Y[11:4] | | X[11:4] | > | | | | | | | > +--------+--------+ +-----------------+ +-----------------+ > > and the 12 bit values have to fit in a 16bit variable. > > The upper 8 bits (in event[2] and event[1] need to be shifted > left by '4' and not by '3' in order to leave space to the lower > 4 bits (in event[3]). Thanks for clarifying, sorry for my rather naive commit message. >> Signed-off-by: Caleb Connolly <caleb@connolly.tech> >> --- >> drivers/input/touchscreen/s6sy761.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c >> index b63d7fdf0cd2..85a1f465c097 100644 >> --- a/drivers/input/touchscreen/s6sy761.c >> +++ b/drivers/input/touchscreen/s6sy761.c >> @@ -145,8 +145,8 @@ static void s6sy761_report_coordinates(struct s6sy761_data *sdata, >> u8 major = event[4]; >> u8 minor = event[5]; >> u8 z = event[6] & S6SY761_MASK_Z; >> - u16 x = (event[1] << 3) | ((event[3] & S6SY761_MASK_X) >> 4); >> - u16 y = (event[2] << 3) | (event[3] & S6SY761_MASK_Y); >> + u16 x = (event[1] << 4) | ((event[3] & S6SY761_MASK_X) >> 4); >> + u16 y = (event[2] << 4) | (event[3] & S6SY761_MASK_Y); > the devil knows how that '3' has ended up there :) > > Thanks for catching it! > > Reviewed-by: Andi Shyti <andi@etezian.org> Regards, Caleb > > Andi
diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c index b63d7fdf0cd2..85a1f465c097 100644 --- a/drivers/input/touchscreen/s6sy761.c +++ b/drivers/input/touchscreen/s6sy761.c @@ -145,8 +145,8 @@ static void s6sy761_report_coordinates(struct s6sy761_data *sdata, u8 major = event[4]; u8 minor = event[5]; u8 z = event[6] & S6SY761_MASK_Z; - u16 x = (event[1] << 3) | ((event[3] & S6SY761_MASK_X) >> 4); - u16 y = (event[2] << 3) | (event[3] & S6SY761_MASK_Y); + u16 x = (event[1] << 4) | ((event[3] & S6SY761_MASK_X) >> 4); + u16 y = (event[2] << 4) | (event[3] & S6SY761_MASK_Y); input_mt_slot(sdata->input, tid);
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 The touch coordinates are read by shifting a value left by 3, this is incorrect and effectively causes the coordinates to be half of the correct value. Shift by 4 bits instead to report the correct value. This matches downstream examples, and has been confirmed on my device (OnePlus 7 Pro). Signed-off-by: Caleb Connolly <caleb@connolly.tech> --- drivers/input/touchscreen/s6sy761.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)