* [PATCH 1/2] input: touchscreen mc13xxx: Make platform data optional @ 2018-04-11 14:13 Lukasz Majewski 2018-04-11 14:13 ` [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support Lukasz Majewski 0 siblings, 1 reply; 5+ messages in thread From: Lukasz Majewski @ 2018-04-11 14:13 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-kernel, linux-input, Sascha Hauer, Lukasz Majewski From: Sascha Hauer <s.hauer@pengutronix.de> The platform data once was optional, make it optional again. This is a first step towards device tree support for the mc13xxx touchscreen driver. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Lukasz Majewski <lukma@denx.de> --- Changes from the original patch: - Commit message typo fixes --- drivers/input/touchscreen/mc13783_ts.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c index ef64f36c5ffc..610f10e6aadf 100644 --- a/drivers/input/touchscreen/mc13783_ts.c +++ b/drivers/input/touchscreen/mc13783_ts.c @@ -38,7 +38,8 @@ struct mc13783_ts_priv { struct mc13xxx *mc13xxx; struct delayed_work work; unsigned int sample[4]; - struct mc13xxx_ts_platform_data *touch; + u8 ato; + bool atox; }; static irqreturn_t mc13783_ts_handler(int irq, void *data) @@ -128,7 +129,7 @@ static void mc13783_ts_work(struct work_struct *work) if (mc13xxx_adc_do_conversion(priv->mc13xxx, mode, channel, - priv->touch->ato, priv->touch->atox, + priv->ato, priv->atox, priv->sample) == 0) mc13783_ts_report_sample(priv); } @@ -172,6 +173,7 @@ static void mc13783_ts_close(struct input_dev *dev) static int __init mc13783_ts_probe(struct platform_device *pdev) { struct mc13783_ts_priv *priv; + struct mc13xxx_ts_platform_data *pdata = dev_get_platdata(&pdev->dev); struct input_dev *idev; int ret = -ENOMEM; @@ -183,11 +185,10 @@ static int __init mc13783_ts_probe(struct platform_device *pdev) INIT_DELAYED_WORK(&priv->work, mc13783_ts_work); priv->mc13xxx = dev_get_drvdata(pdev->dev.parent); priv->idev = idev; - priv->touch = dev_get_platdata(&pdev->dev); - if (!priv->touch) { - dev_err(&pdev->dev, "missing platform data\n"); - ret = -ENODEV; - goto err_free_mem; + + if (pdata) { + priv->atox = pdata->atox; + priv->ato = pdata->ato; } idev->name = MC13783_TS_NAME; -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support 2018-04-11 14:13 [PATCH 1/2] input: touchscreen mc13xxx: Make platform data optional Lukasz Majewski @ 2018-04-11 14:13 ` Lukasz Majewski 2018-04-11 16:01 ` Joe Perches 2018-04-13 22:27 ` Dmitry Torokhov 0 siblings, 2 replies; 5+ messages in thread From: Lukasz Majewski @ 2018-04-11 14:13 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-kernel, linux-input, Sascha Hauer, Lukasz Majewski From: Sascha Hauer <s.hauer@pengutronix.de> The mc34708 has a different bit to enable pen detection. This adds the driver data and devtype necessary to probe the device and to distinguish between the mc13783 and the mc34708. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Lukasz Majewski <lukma@denx.de> --- Changes from the original patch: - Simplify the mcXXXXX_set_pen_detection functions - Fix checkpatch warnings --- drivers/input/touchscreen/mc13783_ts.c | 65 +++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c index 610f10e6aadf..9fcc7069f633 100644 --- a/drivers/input/touchscreen/mc13783_ts.c +++ b/drivers/input/touchscreen/mc13783_ts.c @@ -33,6 +33,8 @@ MODULE_PARM_DESC(sample_tolerance, "is supposed to be wrong and is discarded. Set to 0 to " "disable this check."); +struct mc13xxx_driver_data; + struct mc13783_ts_priv { struct input_dev *idev; struct mc13xxx *mc13xxx; @@ -40,6 +42,43 @@ struct mc13783_ts_priv { unsigned int sample[4]; u8 ato; bool atox; + struct mc13xxx_driver_data *drvdata; +}; + +enum mc13xxx_type { + MC13XXX_TYPE_MC13783, + MC13XXX_TYPE_MC34708, +}; + +struct mc13xxx_driver_data { + int (*set_pen_detection)(struct mc13783_ts_priv *priv, bool enable); + enum mc13xxx_type type; +}; + +static int mc13783_set_pen_detection(struct mc13783_ts_priv *priv, bool enable) +{ + return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, + MC13XXX_ADC0_TSMOD_MASK, + enable ? MC13XXX_ADC0_TSMOD0 : 0); +} + +#define MC34708_ADC0_TSPENDETEN (1 << 20) + +static int mc34708_set_pen_detection(struct mc13783_ts_priv *priv, bool enable) +{ + return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, + MC34708_ADC0_TSPENDETEN, + enable ? MC34708_ADC0_TSPENDETEN : 0); +} + +static struct mc13xxx_driver_data mc13783_driver_data = { + .type = MC13XXX_TYPE_MC13783, + .set_pen_detection = mc13783_set_pen_detection, +}; + +static struct mc13xxx_driver_data mc34708_driver_data = { + .type = MC13XXX_TYPE_MC34708, + .set_pen_detection = mc34708_set_pen_detection, }; static irqreturn_t mc13783_ts_handler(int irq, void *data) @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv) cr0 = (cr0 + cr1) / 2; + if (priv->drvdata->type == MC13XXX_TYPE_MC34708) + if (cr0 > 4080) + cr0 = 0; + if (!cr0 || !sample_tolerance || (x2 - x0 < sample_tolerance && y2 - y0 < sample_tolerance)) { @@ -148,8 +191,7 @@ static int mc13783_ts_open(struct input_dev *dev) if (ret) goto out; - ret = mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, - MC13XXX_ADC0_TSMOD_MASK, MC13XXX_ADC0_TSMOD0); + ret = priv->drvdata->set_pen_detection(priv, 1); if (ret) mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv); out: @@ -162,8 +204,7 @@ static void mc13783_ts_close(struct input_dev *dev) struct mc13783_ts_priv *priv = input_get_drvdata(dev); mc13xxx_lock(priv->mc13xxx); - mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, - MC13XXX_ADC0_TSMOD_MASK, 0); + priv->drvdata->set_pen_detection(priv, 0); mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv); mc13xxx_unlock(priv->mc13xxx); @@ -175,6 +216,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev) struct mc13783_ts_priv *priv; struct mc13xxx_ts_platform_data *pdata = dev_get_platdata(&pdev->dev); struct input_dev *idev; + const struct platform_device_id *id = platform_get_device_id(pdev); int ret = -ENOMEM; priv = kzalloc(sizeof(*priv), GFP_KERNEL); @@ -185,6 +227,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev) INIT_DELAYED_WORK(&priv->work, mc13783_ts_work); priv->mc13xxx = dev_get_drvdata(pdev->dev.parent); priv->idev = idev; + priv->drvdata = (void *)id->driver_data; if (pdata) { priv->atox = pdata->atox; @@ -231,7 +274,21 @@ static int mc13783_ts_remove(struct platform_device *pdev) return 0; } +static const struct platform_device_id mc13xxx_ts_idtable[] = { + { + .name = "mc13783-ts", + .driver_data = (kernel_ulong_t)&mc13783_driver_data, + }, { + .name = "mc34708-ts", + .driver_data = (kernel_ulong_t)&mc34708_driver_data, + }, { + /* sentinel */ + } +}; +MODULE_DEVICE_TABLE(platform, mc13xxx_ts_idtable); + static struct platform_driver mc13783_ts_driver = { + .id_table = mc13xxx_ts_idtable, .remove = mc13783_ts_remove, .driver = { .name = MC13783_TS_NAME, -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support 2018-04-11 14:13 ` [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support Lukasz Majewski @ 2018-04-11 16:01 ` Joe Perches 2018-04-12 14:45 ` Lukasz Majewski 2018-04-13 22:27 ` Dmitry Torokhov 1 sibling, 1 reply; 5+ messages in thread From: Joe Perches @ 2018-04-11 16:01 UTC (permalink / raw) To: Lukasz Majewski, Dmitry Torokhov; +Cc: linux-kernel, linux-input, Sascha Hauer On Wed, 2018-04-11 at 16:13 +0200, Lukasz Majewski wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > The mc34708 has a different bit to enable pen detection. This > adds the driver data and devtype necessary to probe the device > and to distinguish between the mc13783 and the mc34708. style trivia: > diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c [] > @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv) > > cr0 = (cr0 + cr1) / 2; > > + if (priv->drvdata->type == MC13XXX_TYPE_MC34708) > + if (cr0 > 4080) > + cr0 = 0; For easy of reading, this multiple if block should either use braces around the first if like: if (foo) { if (bar) single_statement; } or be written if (foo && bar) single_statement; I generally prefer the latter style. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support 2018-04-11 16:01 ` Joe Perches @ 2018-04-12 14:45 ` Lukasz Majewski 0 siblings, 0 replies; 5+ messages in thread From: Lukasz Majewski @ 2018-04-12 14:45 UTC (permalink / raw) To: Joe Perches; +Cc: Dmitry Torokhov, linux-kernel, linux-input, Sascha Hauer [-- Attachment #1: Type: text/plain, Size: 1335 bytes --] On Wed, 11 Apr 2018 09:01:01 -0700 Joe Perches <joe@perches.com> wrote: > On Wed, 2018-04-11 at 16:13 +0200, Lukasz Majewski wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > The mc34708 has a different bit to enable pen detection. This > > adds the driver data and devtype necessary to probe the device > > and to distinguish between the mc13783 and the mc34708. > > style trivia: > > > diff --git a/drivers/input/touchscreen/mc13783_ts.c > > b/drivers/input/touchscreen/mc13783_ts.c > [] > > @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct > > mc13783_ts_priv *priv) > > cr0 = (cr0 + cr1) / 2; > > > > + if (priv->drvdata->type == MC13XXX_TYPE_MC34708) > > + if (cr0 > 4080) > > + cr0 = 0; > > For easy of reading, this multiple if block should either > use braces around the first if like: > > if (foo) { > if (bar) > single_statement; > } > > or be written > > if (foo && bar) > single_statement; > > I generally prefer the latter style. > +1. I will fix it in v2. Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support 2018-04-11 14:13 ` [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support Lukasz Majewski 2018-04-11 16:01 ` Joe Perches @ 2018-04-13 22:27 ` Dmitry Torokhov 1 sibling, 0 replies; 5+ messages in thread From: Dmitry Torokhov @ 2018-04-13 22:27 UTC (permalink / raw) To: Lukasz Majewski; +Cc: linux-kernel, linux-input, Sascha Hauer On Wed, Apr 11, 2018 at 04:13:40PM +0200, Lukasz Majewski wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > The mc34708 has a different bit to enable pen detection. This > adds the driver data and devtype necessary to probe the device > and to distinguish between the mc13783 and the mc34708. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Signed-off-by: Lukasz Majewski <lukma@denx.de> > > --- > Changes from the original patch: > - Simplify the mcXXXXX_set_pen_detection functions > - Fix checkpatch warnings > --- > drivers/input/touchscreen/mc13783_ts.c | 65 +++++++++++++++++++++++++++++++--- > 1 file changed, 61 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c > index 610f10e6aadf..9fcc7069f633 100644 > --- a/drivers/input/touchscreen/mc13783_ts.c > +++ b/drivers/input/touchscreen/mc13783_ts.c > @@ -33,6 +33,8 @@ MODULE_PARM_DESC(sample_tolerance, > "is supposed to be wrong and is discarded. Set to 0 to " > "disable this check."); > > +struct mc13xxx_driver_data; > + > struct mc13783_ts_priv { > struct input_dev *idev; > struct mc13xxx *mc13xxx; > @@ -40,6 +42,43 @@ struct mc13783_ts_priv { > unsigned int sample[4]; > u8 ato; > bool atox; > + struct mc13xxx_driver_data *drvdata; > +}; > + > +enum mc13xxx_type { > + MC13XXX_TYPE_MC13783, > + MC13XXX_TYPE_MC34708, > +}; > + > +struct mc13xxx_driver_data { > + int (*set_pen_detection)(struct mc13783_ts_priv *priv, bool enable); > + enum mc13xxx_type type; > +}; > + > +static int mc13783_set_pen_detection(struct mc13783_ts_priv *priv, bool enable) > +{ > + return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, > + MC13XXX_ADC0_TSMOD_MASK, > + enable ? MC13XXX_ADC0_TSMOD0 : 0); > +} > + > +#define MC34708_ADC0_TSPENDETEN (1 << 20) > + > +static int mc34708_set_pen_detection(struct mc13783_ts_priv *priv, bool enable) > +{ > + return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, > + MC34708_ADC0_TSPENDETEN, > + enable ? MC34708_ADC0_TSPENDETEN : 0); > +} Instead of having separate functions, can we have register mask and value in mc13xxx_driver_data structure? > + > +static struct mc13xxx_driver_data mc13783_driver_data = { > + .type = MC13XXX_TYPE_MC13783, > + .set_pen_detection = mc13783_set_pen_detection, > +}; > + > +static struct mc13xxx_driver_data mc34708_driver_data = { > + .type = MC13XXX_TYPE_MC34708, > + .set_pen_detection = mc34708_set_pen_detection, > }; > > static irqreturn_t mc13783_ts_handler(int irq, void *data) > @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv) > > cr0 = (cr0 + cr1) / 2; > > + if (priv->drvdata->type == MC13XXX_TYPE_MC34708) > + if (cr0 > 4080) > + cr0 = 0; 4080 is the max resistance for MC34708, right? I'd put it into drvdata as well (and 4096 for the rest), and used input_report_abs(idev, ABS_PRESSURE, cr0 ? priv->drvdata->max_resistance - cr0 : 0); down below. > + > if (!cr0 || !sample_tolerance || > (x2 - x0 < sample_tolerance && > y2 - y0 < sample_tolerance)) { > @@ -148,8 +191,7 @@ static int mc13783_ts_open(struct input_dev *dev) > if (ret) > goto out; > > - ret = mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, > - MC13XXX_ADC0_TSMOD_MASK, MC13XXX_ADC0_TSMOD0); > + ret = priv->drvdata->set_pen_detection(priv, 1); > if (ret) > mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv); > out: > @@ -162,8 +204,7 @@ static void mc13783_ts_close(struct input_dev *dev) > struct mc13783_ts_priv *priv = input_get_drvdata(dev); > > mc13xxx_lock(priv->mc13xxx); > - mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0, > - MC13XXX_ADC0_TSMOD_MASK, 0); > + priv->drvdata->set_pen_detection(priv, 0); > mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv); > mc13xxx_unlock(priv->mc13xxx); > > @@ -175,6 +216,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev) > struct mc13783_ts_priv *priv; > struct mc13xxx_ts_platform_data *pdata = dev_get_platdata(&pdev->dev); > struct input_dev *idev; > + const struct platform_device_id *id = platform_get_device_id(pdev); > int ret = -ENOMEM; > > priv = kzalloc(sizeof(*priv), GFP_KERNEL); > @@ -185,6 +227,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev) > INIT_DELAYED_WORK(&priv->work, mc13783_ts_work); > priv->mc13xxx = dev_get_drvdata(pdev->dev.parent); > priv->idev = idev; > + priv->drvdata = (void *)id->driver_data; > > if (pdata) { > priv->atox = pdata->atox; > @@ -231,7 +274,21 @@ static int mc13783_ts_remove(struct platform_device *pdev) > return 0; > } > > +static const struct platform_device_id mc13xxx_ts_idtable[] = { > + { > + .name = "mc13783-ts", > + .driver_data = (kernel_ulong_t)&mc13783_driver_data, > + }, { > + .name = "mc34708-ts", > + .driver_data = (kernel_ulong_t)&mc34708_driver_data, > + }, { > + /* sentinel */ > + } > +}; > +MODULE_DEVICE_TABLE(platform, mc13xxx_ts_idtable); > + > static struct platform_driver mc13783_ts_driver = { > + .id_table = mc13xxx_ts_idtable, > .remove = mc13783_ts_remove, > .driver = { > .name = MC13783_TS_NAME, > -- > 2.11.0 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-13 22:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-11 14:13 [PATCH 1/2] input: touchscreen mc13xxx: Make platform data optional Lukasz Majewski 2018-04-11 14:13 ` [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support Lukasz Majewski 2018-04-11 16:01 ` Joe Perches 2018-04-12 14:45 ` Lukasz Majewski 2018-04-13 22:27 ` Dmitry Torokhov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.