| Message ID | 1364465583-19835-1-git-send-email-alexandre.belloni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28/03/2013 11:13, Alexandre Belloni wrote: > We need to initialize hardware before registering the touchscreen. Else, > we end up setting registers in mxs_lradc_ts_open(), getting called just > after registering the touchscreen with input_register_device() and by > the end of mxs_lradc_probe(), we reset the LRADC block hence losing the > correct configuration. Sorry, I fumbled and forgot my SoB: Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > Cc: Jonathan Cameron <jic23@kernel.org> > Cc: Marek Vasut <marex@denx.de> > --- > drivers/staging/iio/adc/mxs-lradc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c > index 55a459b..e927e3f 100644 > --- a/drivers/staging/iio/adc/mxs-lradc.c > +++ b/drivers/staging/iio/adc/mxs-lradc.c > @@ -933,6 +933,9 @@ static int mxs_lradc_probe(struct platform_device *pdev) > > INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work); > > + /* Configure the hardware. */ > + mxs_lradc_hw_init(lradc); > + > /* Check if touchscreen is enabled in DT. */ > ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires", > &ts_wires); > @@ -995,9 +998,6 @@ static int mxs_lradc_probe(struct platform_device *pdev) > goto err_ts; > } > > - /* Configure the hardware. */ > - mxs_lradc_hw_init(lradc); > - > return 0; > > err_ts:
Dear Alexandre Belloni, > We need to initialize hardware before registering the touchscreen. Else, > we end up setting registers in mxs_lradc_ts_open(), getting called just > after registering the touchscreen with input_register_device() and by > the end of mxs_lradc_probe(), we reset the LRADC block hence losing the > correct configuration. > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > Cc: Jonathan Cameron <jic23@kernel.org> > Cc: Marek Vasut <marex@denx.de> > --- > drivers/staging/iio/adc/mxs-lradc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c > b/drivers/staging/iio/adc/mxs-lradc.c index 55a459b..e927e3f 100644 > --- a/drivers/staging/iio/adc/mxs-lradc.c > +++ b/drivers/staging/iio/adc/mxs-lradc.c > @@ -933,6 +933,9 @@ static int mxs_lradc_probe(struct platform_device > *pdev) > > INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work); > > + /* Configure the hardware. */ > + mxs_lradc_hw_init(lradc); > + > /* Check if touchscreen is enabled in DT. */ > ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires", Isn't this too early? At least lradc->use_touchscreen is configured only after you call mxs_lradc_hw_init() in this case, while mxs_lradc_hw_init() uses it's value. I think mxs_lradc_hw_init() shall be called just before mxs_lradc_ts_register(). Best regards, Marek Vasut
On 28/03/2013 12:22, Marek Vasut wrote: > Dear Alexandre Belloni, > >> We need to initialize hardware before registering the touchscreen. Else, >> we end up setting registers in mxs_lradc_ts_open(), getting called just >> after registering the touchscreen with input_register_device() and by >> the end of mxs_lradc_probe(), we reset the LRADC block hence losing the >> correct configuration. >> >> Cc: Fabio Estevam <fabio.estevam@freescale.com> >> Cc: Jonathan Cameron <jic23@kernel.org> >> Cc: Marek Vasut <marex@denx.de> >> --- >> drivers/staging/iio/adc/mxs-lradc.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c >> b/drivers/staging/iio/adc/mxs-lradc.c index 55a459b..e927e3f 100644 >> --- a/drivers/staging/iio/adc/mxs-lradc.c >> +++ b/drivers/staging/iio/adc/mxs-lradc.c >> @@ -933,6 +933,9 @@ static int mxs_lradc_probe(struct platform_device >> *pdev) >> >> INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work); >> >> + /* Configure the hardware. */ >> + mxs_lradc_hw_init(lradc); >> + >> /* Check if touchscreen is enabled in DT. */ >> ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires", > Isn't this too early? At least lradc->use_touchscreen is configured only after > you call mxs_lradc_hw_init() in this case, while mxs_lradc_hw_init() uses it's > value. > > I think mxs_lradc_hw_init() shall be called just before mxs_lradc_ts_register(). > > That's right, this would break 5 wires touchscreens. I'll fix that.
Dear Alexandre Belloni, > On 28/03/2013 12:22, Marek Vasut wrote: > > Dear Alexandre Belloni, > > > >> We need to initialize hardware before registering the touchscreen. Else, > >> we end up setting registers in mxs_lradc_ts_open(), getting called just > >> after registering the touchscreen with input_register_device() and by > >> the end of mxs_lradc_probe(), we reset the LRADC block hence losing the > >> correct configuration. > >> > >> Cc: Fabio Estevam <fabio.estevam@freescale.com> > >> Cc: Jonathan Cameron <jic23@kernel.org> > >> Cc: Marek Vasut <marex@denx.de> > >> --- > >> > >> drivers/staging/iio/adc/mxs-lradc.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c > >> b/drivers/staging/iio/adc/mxs-lradc.c index 55a459b..e927e3f 100644 > >> --- a/drivers/staging/iio/adc/mxs-lradc.c > >> +++ b/drivers/staging/iio/adc/mxs-lradc.c > >> @@ -933,6 +933,9 @@ static int mxs_lradc_probe(struct platform_device > >> *pdev) > >> > >> INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work); > >> > >> + /* Configure the hardware. */ > >> + mxs_lradc_hw_init(lradc); > >> + > >> > >> /* Check if touchscreen is enabled in DT. */ > >> ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires", > > > > Isn't this too early? At least lradc->use_touchscreen is configured only > > after you call mxs_lradc_hw_init() in this case, while > > mxs_lradc_hw_init() uses it's value. > > > > I think mxs_lradc_hw_init() shall be called just before > > mxs_lradc_ts_register(). > > That's right, this would break 5 wires touchscreens. I'll fix that. Thanks! Best regards, Marek Vasut
diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c index 55a459b..e927e3f 100644 --- a/drivers/staging/iio/adc/mxs-lradc.c +++ b/drivers/staging/iio/adc/mxs-lradc.c @@ -933,6 +933,9 @@ static int mxs_lradc_probe(struct platform_device *pdev) INIT_WORK(&lradc->ts_work, mxs_lradc_ts_work); + /* Configure the hardware. */ + mxs_lradc_hw_init(lradc); + /* Check if touchscreen is enabled in DT. */ ret = of_property_read_u32(node, "fsl,lradc-touchscreen-wires", &ts_wires); @@ -995,9 +998,6 @@ static int mxs_lradc_probe(struct platform_device *pdev) goto err_ts; } - /* Configure the hardware. */ - mxs_lradc_hw_init(lradc); - return 0; err_ts: