Re: [PATCH v1 2/2] iio: adc: add ADC driver for the TI TSC2046 controller

[Oleksij Rempel]

On Tue, Mar 09, 2021 at 01:46:55PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 9, 2021 at 1:42 PM Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:
> > On Tue, Mar 09, 2021 at 01:05:27PM +0200, Andy Shevchenko wrote:
> > > On Fri, Mar 5, 2021 at 9:05 PM Jonathan Cameron
> > > <Jonathan.Cameron@xxxxxxxxxx> wrote:
> > > >
> > > > On Fri, 5 Mar 2021 14:38:13 +0100
> > > > Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:
> > > >
> > > > > Basically the TI TSC2046 touchscreen controller is 8 channel ADC optimized for
> > > > > the touchscreen use case. By implementing it as IIO ADC device, we can
> > > > > make use of resistive-adc-touch and iio-hwmon drivers.
> > > > >
> > > > > So far, this driver was tested with custom version of resistive-adc-touch driver,
> > > > > since it need to be extended to make use of Z1 and Z2 channels. The X/Y
> > > > > are working without additional changes.
> > > > >
> > > > > Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> > > >
> > > > Hi Oleksij,
> > > >
> > > > To consider this as a possible long term route instead of just making this
> > > > a touchscreen driver, we'll want to see those mods to the resistive-adc-touch.
> > > > Of course that doesn't stop review of this in the meantime.
> > > >
> > > > There are quite a few things in here that feel pretty specific to the touchscreen
> > > > usecase. That makes me wonder if this is a sensible approach or not.
> > >
> > > I'm wondering if this has any similarities with existing drivers under
> > > drivers/input/touchscreen.
> >
> > Yes, for example: drivers/input/touchscreen/ads7846.c
> 
> Then I have a few questions here:
> 1/ why the above mentioned driver can't be extended to cover this?

It is not possible to keep old device tree binding compatible with the
new driver at least not for currently existing abstraction: ADC +
touchscreen node.

It is too expensive to overwrite the old driver, we do not have enough time and
resource to do it. I lardy spend some weeks to do it and I would need a
many more weeks to make it by tiny slices without solving actual
problem. Many resistive touchscreen driver should share a lot of code.

Since there is already existing IIO based components, it seems to me
better to spend available resource and making it properly in a way,
which reflect modern best practices.

> 2/ or why is the proposed driver named after the touchscreen instead
> of the real AD/C chip behind it?

I do not understand this question. The proposed driver is named after
the chip which provides ADC functionality, In this case, it is TSC2046.
The touchscreen is a separate physical module.

The idea of this proposition is to keep physically separate components
separately on the kernel side.

> 3/ maybe we can introduce a simple AD/C driver under IIO for that?

There are already simple ADC drivers for that:
iio-hwmon: drivers/hwmon/iio_hwmon.c
resistive-adc-touch: drivers/input/touchscreen/resistive-adc-touch.c

This two driver + the proposed one, will replace functionality of ads7846.c

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |