diff mbox series

[1/4] Input: edt-ft5x06 - fix get_default register write access

Message ID 20200227112819.16754-2-m.felsch@pengutronix.de (mailing list archive)
State Accepted
Commit 255cdaf73412de13608fb776101402dca68bed2b
Headers show
Series EDT-FT5x06 Fixes and improvments | expand

Commit Message

Marco Felsch Feb. 27, 2020, 11:28 a.m. UTC 
Since commit b6eba86030bf ("Input: edt-ft5x06 - add offset support for
ev-ft5726") offset-x and offset-y is supported. Devices using those
offset parameters don't support the offset parameter so we need to add
the NO_REGISTER check for edt_ft5x06_ts_get_defaults().

Fixes: b6eba86030bf ("Input: edt-ft5x06 - add offset support for ev-ft5726")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/input/touchscreen/edt-ft5x06.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Dmitry Torokhov May 9, 2020, 7:05 p.m. UTC | #1 
Hi Macro,

On Thu, Feb 27, 2020 at 12:28:16PM +0100, Marco Felsch wrote:
> Since commit b6eba86030bf ("Input: edt-ft5x06 - add offset support for
> ev-ft5726") offset-x and offset-y is supported. Devices using those
> offset parameters don't support the offset parameter so we need to add
> the NO_REGISTER check for edt_ft5x06_ts_get_defaults().
> 
> Fixes: b6eba86030bf ("Input: edt-ft5x06 - add offset support for ev-ft5726")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

I'll apply this, but I wonder if we should not move this check into
edt_ft5x06_register_write(), and also have edt_ft5x06_register_read()
return error if address is "NO_REGISTER"?

Thanks.
Marco Felsch May 10, 2020, 11:06 a.m. UTC | #2 
Hi Dmitry,

On 20-05-09 12:05, Dmitry Torokhov wrote:
> Hi Macro,
> 
> On Thu, Feb 27, 2020 at 12:28:16PM +0100, Marco Felsch wrote:
> > Since commit b6eba86030bf ("Input: edt-ft5x06 - add offset support for
> > ev-ft5726") offset-x and offset-y is supported. Devices using those
> > offset parameters don't support the offset parameter so we need to add
> > the NO_REGISTER check for edt_ft5x06_ts_get_defaults().
> > 
> > Fixes: b6eba86030bf ("Input: edt-ft5x06 - add offset support for ev-ft5726")
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> I'll apply this, but I wonder if we should not move this check into
> edt_ft5x06_register_write(), and also have edt_ft5x06_register_read()
> return error if address is "NO_REGISTER"?

I tought so too but I wanted to keep the fix small and backportable.

Regards,
  Marco

> Thanks.
> 
> -- 
> Dmitry
>
Dmitry Torokhov May 11, 2020, 5:53 p.m. UTC | #3 
On Sun, May 10, 2020 at 01:06:44PM +0200, Marco Felsch wrote:
> Hi Dmitry,
> 
> On 20-05-09 12:05, Dmitry Torokhov wrote:
> > Hi Macro,
> > 
> > On Thu, Feb 27, 2020 at 12:28:16PM +0100, Marco Felsch wrote:
> > > Since commit b6eba86030bf ("Input: edt-ft5x06 - add offset support for
> > > ev-ft5726") offset-x and offset-y is supported. Devices using those
> > > offset parameters don't support the offset parameter so we need to add
> > > the NO_REGISTER check for edt_ft5x06_ts_get_defaults().
> > > 
> > > Fixes: b6eba86030bf ("Input: edt-ft5x06 - add offset support for ev-ft5726")
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > 
> > I'll apply this, but I wonder if we should not move this check into
> > edt_ft5x06_register_write(), and also have edt_ft5x06_register_read()
> > return error if address is "NO_REGISTER"?
> 
> I tought so too but I wanted to keep the fix small and backportable.

Any chance you can make a follow-up patch so that going forward it is
cleaner (and the fix can still be backported if needed)?

Thanks.
Marco Felsch May 12, 2020, 5:01 p.m. UTC | #4 
On 20-05-11 10:53, Dmitry Torokhov wrote:
> On Sun, May 10, 2020 at 01:06:44PM +0200, Marco Felsch wrote:
> > Hi Dmitry,
> > 
> > On 20-05-09 12:05, Dmitry Torokhov wrote:
> > > Hi Macro,
> > > 
> > > On Thu, Feb 27, 2020 at 12:28:16PM +0100, Marco Felsch wrote:
> > > > Since commit b6eba86030bf ("Input: edt-ft5x06 - add offset support for
> > > > ev-ft5726") offset-x and offset-y is supported. Devices using those
> > > > offset parameters don't support the offset parameter so we need to add
> > > > the NO_REGISTER check for edt_ft5x06_ts_get_defaults().
> > > > 
> > > > Fixes: b6eba86030bf ("Input: edt-ft5x06 - add offset support for ev-ft5726")
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > 
> > > I'll apply this, but I wonder if we should not move this check into
> > > edt_ft5x06_register_write(), and also have edt_ft5x06_register_read()
> > > return error if address is "NO_REGISTER"?
> > 
> > I tought so too but I wanted to keep the fix small and backportable.
> 
> Any chance you can make a follow-up patch so that going forward it is
> cleaner (and the fix can still be backported if needed)?

Of course I will prepare a fix.

Regards,
  Marco

> Thanks.
>
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d2587724c52a..9b8450794a8a 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -938,19 +938,25 @@  static void edt_ft5x06_ts_get_defaults(struct device *dev,
 
 	error = device_property_read_u32(dev, "offset", &val);
 	if (!error) {
-		edt_ft5x06_register_write(tsdata, reg_addr->reg_offset, val);
+		if (reg_addr->reg_offset != NO_REGISTER)
+			edt_ft5x06_register_write(tsdata,
+						  reg_addr->reg_offset, val);
 		tsdata->offset = val;
 	}
 
 	error = device_property_read_u32(dev, "offset-x", &val);
 	if (!error) {
-		edt_ft5x06_register_write(tsdata, reg_addr->reg_offset_x, val);
+		if (reg_addr->reg_offset_x != NO_REGISTER)
+			edt_ft5x06_register_write(tsdata,
+						  reg_addr->reg_offset_x, val);
 		tsdata->offset_x = val;
 	}
 
 	error = device_property_read_u32(dev, "offset-y", &val);
 	if (!error) {
-		edt_ft5x06_register_write(tsdata, reg_addr->reg_offset_y, val);
+		if (reg_addr->reg_offset_y != NO_REGISTER)
+			edt_ft5x06_register_write(tsdata,
+						  reg_addr->reg_offset_y, val);
 		tsdata->offset_y = val;
 	}
 }