[PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Michael Grzeschik]

In case of devicetree, we currently don't have a way to append pdata for
the touchscreen. The current approach is to bail out in that case.
This patch makes it possible to probe the touchscreen without pdata
and use zero as default values for the atox and ato adc conversion.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/input/touchscreen/mc13783_ts.c | 14 +++++++++-----
 drivers/mfd/mc13xxx-core.c             | 11 +++++++----
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
index d6f099c..0e24304 100644
--- a/drivers/input/touchscreen/mc13783_ts.c
+++ b/drivers/input/touchscreen/mc13783_ts.c
@@ -124,10 +124,17 @@ static void mc13783_ts_work(struct work_struct *work)
 		container_of(work, struct mc13783_ts_priv, work.work);
 	unsigned int mode = MC13XXX_ADC_MODE_TS;
 	unsigned int channel = 12;
+	u8 ato = 0;
+	bool atox = 0;
+
+	if (priv->touch) {
+		ato = priv->touch->ato;
+		atox = priv->touch->atox;
+	}
 
 	if (mc13xxx_adc_do_conversion(priv->mc13xxx,
 				mode, channel,
-				priv->touch->ato, priv->touch->atox,
+				ato, atox,
 				priv->sample) == 0)
 		mc13783_ts_report_sample(priv);
 }
@@ -183,11 +190,8 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
 	priv->mc13xxx = dev_get_drvdata(pdev->dev.parent);
 	priv->idev = idev;
 	priv->touch = dev_get_platdata(&pdev->dev);
-	if (!priv->touch) {
+	if (!priv->touch)
 		dev_err(&pdev->dev, "missing platform data\n");
-		ret = -ENODEV;
-		goto err_free_mem;
-	}
 
 	/*
 	 * We need separate workqueue because mc13783_adc_do_conversion
diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 2a9b100..278601f 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -694,10 +694,6 @@ err_revision:
 	if (mc13xxx->flags & MC13XXX_USE_RTC)
 		mc13xxx_add_subdevice(mc13xxx, "%s-rtc");
 
-	if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
-		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-ts",
-				&pdata->touch, sizeof(pdata->touch));
-
 	if (pdata) {
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-regulator",
 			&pdata->regulators, sizeof(pdata->regulators));
@@ -705,10 +701,17 @@ err_revision:
 				pdata->leds, sizeof(*pdata->leds));
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
 				pdata->buttons, sizeof(*pdata->buttons));
+
+		if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
+			mc13xxx_add_subdevice_pdata(mc13xxx, "%s-ts",
+					&pdata->touch, sizeof(pdata->touch));
 	} else {
 		mc13xxx_add_subdevice(mc13xxx, "%s-regulator");
 		mc13xxx_add_subdevice(mc13xxx, "%s-led");
 		mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton");
+
+		if (mc13xxx->flags & MC13XXX_USE_TOUCHSCREEN)
+			mc13xxx_add_subdevice(mc13xxx, "%s-ts");
 	}
 
 	return 0;
-- 
1.8.4.rc2

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Dmitry Torokhov]

Hi Michael,

On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> In case of devicetree, we currently don't have a way to append pdata for
> the touchscreen. The current approach is to bail out in that case.
> This patch makes it possible to probe the touchscreen without pdata
> and use zero as default values for the atox and ato adc conversion.

I'd rather you added the devicetree support to the driver.

Thanks.

-- 
Dmitry

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Michael Grzeschik]

Hi Dimitry,

On Tue, Aug 13, 2013 at 09:46:09AM -0700, Dmitry Torokhov wrote:
> Hi Michael,
> 
> On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> > In case of devicetree, we currently don't have a way to append pdata for
> > the touchscreen. The current approach is to bail out in that case.
> > This patch makes it possible to probe the touchscreen without pdata
> > and use zero as default values for the atox and ato adc conversion.
> 
> I'd rather you added the devicetree support to the driver.

I know that we will need real devictree glue that generates pdata in the
long run. I am working on that. Beside that, for now this patch makes
sense anyway.

Regards,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Samuel Ortiz]

Hi Michael,

On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> In case of devicetree, we currently don't have a way to append pdata for
> the touchscreen. The current approach is to bail out in that case.
> This patch makes it possible to probe the touchscreen without pdata
> and use zero as default values for the atox and ato adc conversion.
Would that still make the touchscreen somehow functional ?
If that's the case, and if Dmitry is fine with the ts part of this
patch, could you please separate the mfd part of this patch into a
separate one ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Michael Grzeschik]

On Tue, Aug 20, 2013 at 03:34:33AM +0200, Samuel Ortiz wrote:
> Hi Michael,
> 
> On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> > In case of devicetree, we currently don't have a way to append pdata for
> > the touchscreen. The current approach is to bail out in that case.
> > This patch makes it possible to probe the touchscreen without pdata
> > and use zero as default values for the atox and ato adc conversion.
> Would that still make the touchscreen somehow functional ?

Yes, it still works. It just defaults to zero values in no pdata case.

> If that's the case, and if Dmitry is fine with the ts part of this
> patch, could you please separate the mfd part of this patch into a
> separate one ?

Why?

The purpose of this patch is to have both cases working, pdata and no
pdata. This patch is actually fixing the patch of Michael Thalmeier.
There we changed the mfd and touch in one patch as well.

http://lkml.indiana.edu/hypermail/linux/kernel/1201.1/01364.html

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Samuel Ortiz]

Hi Michael,

On Tue, Aug 20, 2013 at 11:34:32AM +0200, Michael Grzeschik wrote:
> On Tue, Aug 20, 2013 at 03:34:33AM +0200, Samuel Ortiz wrote:
> > On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> > > In case of devicetree, we currently don't have a way to append pdata for
> > > the touchscreen. The current approach is to bail out in that case.
> > > This patch makes it possible to probe the touchscreen without pdata
> > > and use zero as default values for the atox and ato adc conversion.
> > Would that still make the touchscreen somehow functional ?
> 
> Yes, it still works. It just defaults to zero values in no pdata case.
> 
> > If that's the case, and if Dmitry is fine with the ts part of this
> > patch, could you please separate the mfd part of this patch into a
> > separate one ?
> 
> Why?
To avoid cross tree commits as much as possible.


> The purpose of this patch is to have both cases working, pdata and no
> pdata. 
I understood that part. But you can still split the patch in 2 as the ts
driver will not be probed without pdata until the mfd patch is merged
upstream.

> This patch is actually fixing the patch of Michael Thalmeier.
> There we changed the mfd and touch in one patch as well.
There was a build time dependency between the ts driver and the MFD
changes, so the patch had to be merged atomically.
That is not the case with your patch.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Michael Grzeschik]

On Tue, Aug 20, 2013 at 11:50:48AM +0200, Samuel Ortiz wrote:
> Hi Michael,
> 
> On Tue, Aug 20, 2013 at 11:34:32AM +0200, Michael Grzeschik wrote:
> > On Tue, Aug 20, 2013 at 03:34:33AM +0200, Samuel Ortiz wrote:
> > > On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> > > > In case of devicetree, we currently don't have a way to append pdata for
> > > > the touchscreen. The current approach is to bail out in that case.
> > > > This patch makes it possible to probe the touchscreen without pdata
> > > > and use zero as default values for the atox and ato adc conversion.
> > > Would that still make the touchscreen somehow functional ?
> > 
> > Yes, it still works. It just defaults to zero values in no pdata case.
> > 
> > > If that's the case, and if Dmitry is fine with the ts part of this
> > > patch, could you please separate the mfd part of this patch into a
> > > separate one ?
> > 
> > Why?
> To avoid cross tree commits as much as possible.
> 
> 
> > The purpose of this patch is to have both cases working, pdata and no
> > pdata. 
> I understood that part. But you can still split the patch in 2 as the ts
> driver will not be probed without pdata until the mfd patch is merged
> upstream.
> 
> > This patch is actually fixing the patch of Michael Thalmeier.
> > There we changed the mfd and touch in one patch as well.
> There was a build time dependency between the ts driver and the MFD
> changes, so the patch had to be merged atomically.
> That is not the case with your patch.

All right then. I will resend them and the codec mfd patch together with
the oftree patches I am currently working on.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined

[Dmitry Torokhov]

On Tue, Aug 20, 2013 at 11:34:32AM +0200, Michael Grzeschik wrote:
> On Tue, Aug 20, 2013 at 03:34:33AM +0200, Samuel Ortiz wrote:
> > Hi Michael,
> > 
> > On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> > > In case of devicetree, we currently don't have a way to append pdata for
> > > the touchscreen. The current approach is to bail out in that case.
> > > This patch makes it possible to probe the touchscreen without pdata
> > > and use zero as default values for the atox and ato adc conversion.
> > Would that still make the touchscreen somehow functional ?
> 
> Yes, it still works. It just defaults to zero values in no pdata case.
> 
> > If that's the case, and if Dmitry is fine with the ts part of this
> > patch, could you please separate the mfd part of this patch into a
> > separate one ?
> 
> Why?
> 
> The purpose of this patch is to have both cases working, pdata and no
> pdata.

My concern with allowing defaults with missing pdata or device tree data
is that it makes it easy for the integrator to miss the necessity of the
parameters and then start piling on driver workarounds. I have seen a
few examples of this happening, so I'd rather prefer an explicit values
if possible.

Thanks.

-- 
Dmitry