Dear Dmitry Torokhov,
[...]
> > > + enum mxs_lradc_ts use_touchscreen;
> > > + unsigned int stop_touchscreen:1;
> > > + unsigned int use_touchbutton:1;
>
> Can we make them bools instead of bit fields?
Sure.
[...]
> > > +static void mxs_lradc_ts_close(struct input_dev *dev)
> > > +{
> > > + struct mxs_lradc *lradc = input_get_drvdata(dev);
> > > +
> > > + /* Indicate the touchscreen is stopping. */
> > > + lradc->stop_touchscreen = 1;
> > > +
> > > + /* Disable touchscreen touch-detect IRQ. */
> > > + writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> > > + lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> > > +
> > > + /* Power-down touchscreen touch-detect circuitry. */
> > > + writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
> > > + lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
>
> These 2 writes are racing with writes in mxs_lradc_ts_work(). I think
> you need to:
>
> lradc->stop_touchscreen = true;
> mb();
Nice catch, do we need the memory barrier here though, is it not enough to
reorder the cancel_work_sync() just before the register writes?
> cancel_work_sync(&lradc->ts_work);
> writel(LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
> lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
> ...
[...]
> > > + lradc->ts_input = input;
> > > + input_set_drvdata(input, lradc);
> > > + ret = input_register_device(input);
> > > + if (ret)
> > > + input_free_device(lradc->ts_input);
> > > +
> > > + return ret;
>
> So why is allocation failure is not fatal bur registration is? I'd make
> both fatal.
Since it's still possible to operate the block even if touchscreen fails to
probe. If that happens, certainly something is already weird. I'd say both
should be fatal.
> > > +}
> > > +
> > > +static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> > > +{
> > > + if (!lradc->use_touchscreen)
> > > + return;
> > > +
> > > + if (!lradc->ts_input)
> > > + return;
>
> Do we really need to check both conditions?
Better safe than sorry, but lradc->ts_input should be non-NULL in case lradc-
>use_touchscreen is set, so I'll remove it.
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
