diff mbox series

[v4,1/3] mfd: mc13xxx: Add mc34708 adc support

Message ID 20190717222602.2912-2-lukma@denx.de (mailing list archive)
State Superseded
Headers show
Series mfd: mc13xxx: Fixes and enhancements for NXP's mc34708 | expand

Commit Message

Lukasz Majewski July 17, 2019, 10:26 p.m. UTC
From: Sascha Hauer <s.hauer@pengutronix.de>

The mc34708 has an improved adc. The older variants will always convert
a fixed order of channels. The mc34708 can do up to eight conversions
in arbitrary channel order. Currently this extended feature is not
supported. We only support touchscreen conversions now, which will
be sampled in a data format compatible to the older chips in order
to keep the API between the mfd and the touchscreen driver.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Lukasz Majewski <lukma@denx.de>

---
Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- Change the return code patch when the mc13xxx ADC is performing conversion
- Introduce new include/linux/mfd/mc34708.h header file for mc34708 specific
  defines

Changes from the original patches:
- ADC conversion functions prototypes added to fix build error
- Adjustments to make checkpatch clean (-ENOSYS, line over 80 char)

This patch applies on top of v5.2 - SHA1: 0ecfebd2b52404ae0c54a878c872bb93363ada36
---
 drivers/mfd/mc13xxx-core.c  | 102 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/mfd/mc13xxx.h       |   3 ++
 include/linux/mfd/mc34708.h |  37 ++++++++++++++++
 3 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/mfd/mc34708.h

Comments

Lee Jones July 25, 2019, 12:36 p.m. UTC | #1
On Thu, 18 Jul 2019, Lukasz Majewski wrote:

> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> The mc34708 has an improved adc. The older variants will always convert
> a fixed order of channels. The mc34708 can do up to eight conversions
> in arbitrary channel order. Currently this extended feature is not
> supported. We only support touchscreen conversions now, which will
> be sampled in a data format compatible to the older chips in order
> to keep the API between the mfd and the touchscreen driver.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> ---
> Changes for v4:
> - None
> 
> Changes for v3:
> - None
> 
> Changes for v2:
> - Change the return code patch when the mc13xxx ADC is performing conversion
> - Introduce new include/linux/mfd/mc34708.h header file for mc34708 specific
>   defines
> 
> Changes from the original patches:
> - ADC conversion functions prototypes added to fix build error
> - Adjustments to make checkpatch clean (-ENOSYS, line over 80 char)
> 
> This patch applies on top of v5.2 - SHA1: 0ecfebd2b52404ae0c54a878c872bb93363ada36
> ---
>  drivers/mfd/mc13xxx-core.c  | 102 +++++++++++++++++++++++++++++++++++++++++++-
>  drivers/mfd/mc13xxx.h       |   3 ++
>  include/linux/mfd/mc34708.h |  37 ++++++++++++++++
>  3 files changed, 141 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/mfd/mc34708.h
> 
> diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> index 1abe7432aad8..01473d6fda21 100644
> --- a/drivers/mfd/mc13xxx-core.c
> +++ b/drivers/mfd/mc13xxx-core.c
> @@ -12,6 +12,7 @@
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/mfd/core.h>
> +#include <linux/mfd/mc34708.h>
>  
>  #include "mc13xxx.h"
>  
> @@ -45,6 +46,8 @@
>  
>  #define MC13XXX_ADC2		45
>  
> +#define MC13XXX_ADC_WORKING		(1 << 0)

BIT(0) ?

>  void mc13xxx_lock(struct mc13xxx *mc13xxx)
>  {
>  	if (!mutex_trylock(&mc13xxx->lock)) {
> @@ -198,22 +201,30 @@ static void mc34708_print_revision(struct mc13xxx *mc13xxx, u32 revision)
>  			maskval(revision, MC34708_REVISION_FAB));
>  }
>  
> +static int mc13xxx_adc_conversion(struct mc13xxx *, unsigned int,
> +				  unsigned int, u8, bool, unsigned int *);
> +static int mc34708_adc_conversion(struct mc13xxx *, unsigned int,
> +				  unsigned int, u8, bool, unsigned int *);
> +
>  /* These are only exported for mc13xxx-i2c and mc13xxx-spi */
>  struct mc13xxx_variant mc13xxx_variant_mc13783 = {
>  	.name = "mc13783",
>  	.print_revision = mc13xxx_print_revision,
> +	.adc_do_conversion = mc13xxx_adc_conversion,
>  };
>  EXPORT_SYMBOL_GPL(mc13xxx_variant_mc13783);

I'd prefer to keep the call-back functions as close to zero as
possible.

It would be better to turn mc13xxx_adc_conversion() in to the catch
function choose an execution route based on some platform matching.

If you could do the same for print_revision too, that would be even
better.
Lukasz Majewski July 25, 2019, 4:20 p.m. UTC | #2
Hi Lee,

> On Thu, 18 Jul 2019, Lukasz Majewski wrote:
> 
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > 
> > The mc34708 has an improved adc. The older variants will always
> > convert a fixed order of channels. The mc34708 can do up to eight
> > conversions in arbitrary channel order. Currently this extended
> > feature is not supported. We only support touchscreen conversions
> > now, which will be sampled in a data format compatible to the older
> > chips in order to keep the API between the mfd and the touchscreen
> > driver.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > 
> > ---
> > Changes for v4:
> > - None
> > 
> > Changes for v3:
> > - None
> > 
> > Changes for v2:
> > - Change the return code patch when the mc13xxx ADC is performing
> > conversion
> > - Introduce new include/linux/mfd/mc34708.h header file for mc34708
> > specific defines
> > 
> > Changes from the original patches:
> > - ADC conversion functions prototypes added to fix build error
> > - Adjustments to make checkpatch clean (-ENOSYS, line over 80 char)
> > 
> > This patch applies on top of v5.2 - SHA1:
> > 0ecfebd2b52404ae0c54a878c872bb93363ada36 ---
> >  drivers/mfd/mc13xxx-core.c  | 102
> > +++++++++++++++++++++++++++++++++++++++++++-
> > drivers/mfd/mc13xxx.h       |   3 ++ include/linux/mfd/mc34708.h |
> > 37 ++++++++++++++++ 3 files changed, 141 insertions(+), 1
> > deletion(-) create mode 100644 include/linux/mfd/mc34708.h
> > 
> > diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> > index 1abe7432aad8..01473d6fda21 100644
> > --- a/drivers/mfd/mc13xxx-core.c
> > +++ b/drivers/mfd/mc13xxx-core.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/mfd/core.h>
> > +#include <linux/mfd/mc34708.h>
> >  
> >  #include "mc13xxx.h"
> >  
> > @@ -45,6 +46,8 @@
> >  
> >  #define MC13XXX_ADC2		45
> >  
> > +#define MC13XXX_ADC_WORKING		(1 << 0)  
> 
> BIT(0) ?

The same convention - i.e. (1 << 0) is used in the rest of the file.

> 
> >  void mc13xxx_lock(struct mc13xxx *mc13xxx)
> >  {
> >  	if (!mutex_trylock(&mc13xxx->lock)) {
> > @@ -198,22 +201,30 @@ static void mc34708_print_revision(struct
> > mc13xxx *mc13xxx, u32 revision) maskval(revision,
> > MC34708_REVISION_FAB)); }
> >  
> > +static int mc13xxx_adc_conversion(struct mc13xxx *, unsigned int,
> > +				  unsigned int, u8, bool, unsigned
> > int *); +static int mc34708_adc_conversion(struct mc13xxx *,
> > unsigned int,
> > +				  unsigned int, u8, bool, unsigned
> > int *); +
> >  /* These are only exported for mc13xxx-i2c and mc13xxx-spi */
> >  struct mc13xxx_variant mc13xxx_variant_mc13783 = {
> >  	.name = "mc13783",
> >  	.print_revision = mc13xxx_print_revision,
> > +	.adc_do_conversion = mc13xxx_adc_conversion,
> >  };
> >  EXPORT_SYMBOL_GPL(mc13xxx_variant_mc13783);  
> 
> I'd prefer to keep the call-back functions as close to zero as
> possible.

If I may ask - what is wrong with having per device callback(s) ?

> 
> It would be better to turn mc13xxx_adc_conversion() in to the catch
> function

Could you share any example? 

> choose an execution route based on some platform matching.
> 

Could you help me with giving a hint of how shall I do the "platform
matching" in this particular driver ? 

The mc13xxx driver seems rather complex with SPI and I2C support and in
which the subdevices are added (e.g. rtc, adc, etc).

This particular patch just follows current driver design and fixes its
usability for mc13708 drvice.

> If you could do the same for print_revision too, that would be even
> better.
> 

I would prefer to fix the driver (for mc13708) without the need to
change the working code.


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-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski Aug. 2, 2019, 1:19 p.m. UTC | #3
Hi Lee,

> Hi Lee,
> 
> > On Thu, 18 Jul 2019, Lukasz Majewski wrote:
> >   
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > 
> > > The mc34708 has an improved adc. The older variants will always
> > > convert a fixed order of channels. The mc34708 can do up to eight
> > > conversions in arbitrary channel order. Currently this extended
> > > feature is not supported. We only support touchscreen conversions
> > > now, which will be sampled in a data format compatible to the
> > > older chips in order to keep the API between the mfd and the
> > > touchscreen driver.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > 
> > > ---
> > > Changes for v4:
> > > - None
> > > 
> > > Changes for v3:
> > > - None
> > > 
> > > Changes for v2:
> > > - Change the return code patch when the mc13xxx ADC is performing
> > > conversion
> > > - Introduce new include/linux/mfd/mc34708.h header file for
> > > mc34708 specific defines
> > > 
> > > Changes from the original patches:
> > > - ADC conversion functions prototypes added to fix build error
> > > - Adjustments to make checkpatch clean (-ENOSYS, line over 80
> > > char)
> > > 
> > > This patch applies on top of v5.2 - SHA1:
> > > 0ecfebd2b52404ae0c54a878c872bb93363ada36 ---
> > >  drivers/mfd/mc13xxx-core.c  | 102
> > > +++++++++++++++++++++++++++++++++++++++++++-
> > > drivers/mfd/mc13xxx.h       |   3 ++ include/linux/mfd/mc34708.h |
> > > 37 ++++++++++++++++ 3 files changed, 141 insertions(+), 1
> > > deletion(-) create mode 100644 include/linux/mfd/mc34708.h
> > > 
> > > diff --git a/drivers/mfd/mc13xxx-core.c
> > > b/drivers/mfd/mc13xxx-core.c index 1abe7432aad8..01473d6fda21
> > > 100644 --- a/drivers/mfd/mc13xxx-core.c
> > > +++ b/drivers/mfd/mc13xxx-core.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/of_device.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/mfd/core.h>
> > > +#include <linux/mfd/mc34708.h>
> > >  
> > >  #include "mc13xxx.h"
> > >  
> > > @@ -45,6 +46,8 @@
> > >  
> > >  #define MC13XXX_ADC2		45
> > >  
> > > +#define MC13XXX_ADC_WORKING		(1 << 0)    
> > 
> > BIT(0) ?  
> 
> The same convention - i.e. (1 << 0) is used in the rest of the file.
> 
> >   
> > >  void mc13xxx_lock(struct mc13xxx *mc13xxx)
> > >  {
> > >  	if (!mutex_trylock(&mc13xxx->lock)) {
> > > @@ -198,22 +201,30 @@ static void mc34708_print_revision(struct
> > > mc13xxx *mc13xxx, u32 revision) maskval(revision,
> > > MC34708_REVISION_FAB)); }
> > >  
> > > +static int mc13xxx_adc_conversion(struct mc13xxx *, unsigned int,
> > > +				  unsigned int, u8, bool,
> > > unsigned int *); +static int mc34708_adc_conversion(struct
> > > mc13xxx *, unsigned int,
> > > +				  unsigned int, u8, bool,
> > > unsigned int *); +
> > >  /* These are only exported for mc13xxx-i2c and mc13xxx-spi */
> > >  struct mc13xxx_variant mc13xxx_variant_mc13783 = {
> > >  	.name = "mc13783",
> > >  	.print_revision = mc13xxx_print_revision,
> > > +	.adc_do_conversion = mc13xxx_adc_conversion,
> > >  };
> > >  EXPORT_SYMBOL_GPL(mc13xxx_variant_mc13783);    
> > 
> > I'd prefer to keep the call-back functions as close to zero as
> > possible.  
> 
> If I may ask - what is wrong with having per device callback(s) ?
> 
> > 
> > It would be better to turn mc13xxx_adc_conversion() in to the catch
> > function  
> 
> Could you share any example? 
> 
> > choose an execution route based on some platform matching.
> >   
> 
> Could you help me with giving a hint of how shall I do the "platform
> matching" in this particular driver ? 
> 
> The mc13xxx driver seems rather complex with SPI and I2C support and
> in which the subdevices are added (e.g. rtc, adc, etc).
> 
> This particular patch just follows current driver design and fixes its
> usability for mc13708 drvice.
> 
> > If you could do the same for print_revision too, that would be even
> > better.
> >   
> 
> I would prefer to fix the driver (for mc13708) without the need to
> change the working code.
> 

Lee, would you find time to reply to this message?

> 
> 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-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




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-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lee Jones Aug. 12, 2019, 10:08 a.m. UTC | #4
On Thu, 25 Jul 2019, Lukasz Majewski wrote:
> > On Thu, 18 Jul 2019, Lukasz Majewski wrote:
> > 
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > 
> > > The mc34708 has an improved adc. The older variants will always
> > > convert a fixed order of channels. The mc34708 can do up to eight
> > > conversions in arbitrary channel order. Currently this extended
> > > feature is not supported. We only support touchscreen conversions
> > > now, which will be sampled in a data format compatible to the older
> > > chips in order to keep the API between the mfd and the touchscreen
> > > driver.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > 
> > > ---
> > > Changes for v4:
> > > - None
> > > 
> > > Changes for v3:
> > > - None
> > > 
> > > Changes for v2:
> > > - Change the return code patch when the mc13xxx ADC is performing
> > > conversion
> > > - Introduce new include/linux/mfd/mc34708.h header file for mc34708
> > > specific defines
> > > 
> > > Changes from the original patches:
> > > - ADC conversion functions prototypes added to fix build error
> > > - Adjustments to make checkpatch clean (-ENOSYS, line over 80 char)
> > > 
> > > This patch applies on top of v5.2 - SHA1:
> > > 0ecfebd2b52404ae0c54a878c872bb93363ada36 ---
> > >  drivers/mfd/mc13xxx-core.c  | 102
> > > +++++++++++++++++++++++++++++++++++++++++++-
> > > drivers/mfd/mc13xxx.h       |   3 ++ include/linux/mfd/mc34708.h |
> > > 37 ++++++++++++++++ 3 files changed, 141 insertions(+), 1
> > > deletion(-) create mode 100644 include/linux/mfd/mc34708.h
> > > 
> > > diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> > > index 1abe7432aad8..01473d6fda21 100644
> > > --- a/drivers/mfd/mc13xxx-core.c
> > > +++ b/drivers/mfd/mc13xxx-core.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/of_device.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/mfd/core.h>
> > > +#include <linux/mfd/mc34708.h>
> > >  
> > >  #include "mc13xxx.h"
> > >  
> > > @@ -45,6 +46,8 @@
> > >  
> > >  #define MC13XXX_ADC2		45
> > >  
> > > +#define MC13XXX_ADC_WORKING		(1 << 0)  
> > 
> > BIT(0) ?
> 
> The same convention - i.e. (1 << 0) is used in the rest of the file.

Very well, but please consider converting it subsequently.

> > >  void mc13xxx_lock(struct mc13xxx *mc13xxx)
> > >  {
> > >  	if (!mutex_trylock(&mc13xxx->lock)) {
> > > @@ -198,22 +201,30 @@ static void mc34708_print_revision(struct
> > > mc13xxx *mc13xxx, u32 revision) maskval(revision,
> > > MC34708_REVISION_FAB)); }
> > >  
> > > +static int mc13xxx_adc_conversion(struct mc13xxx *, unsigned int,
> > > +				  unsigned int, u8, bool, unsigned
> > > int *); +static int mc34708_adc_conversion(struct mc13xxx *,
> > > unsigned int,
> > > +				  unsigned int, u8, bool, unsigned
> > > int *); +
> > >  /* These are only exported for mc13xxx-i2c and mc13xxx-spi */
> > >  struct mc13xxx_variant mc13xxx_variant_mc13783 = {
> > >  	.name = "mc13783",
> > >  	.print_revision = mc13xxx_print_revision,
> > > +	.adc_do_conversion = mc13xxx_adc_conversion,
> > >  };
> > >  EXPORT_SYMBOL_GPL(mc13xxx_variant_mc13783);  
> > 
> > I'd prefer to keep the call-back functions as close to zero as
> > possible.
> 
> If I may ask - what is wrong with having per device callback(s) ?

Call-backs are ugly and hard to read/debug.

If they can be avoided, they should be, IMHO>

> > It would be better to turn mc13xxx_adc_conversion() in to the catch
> > function
> 
> Could you share any example? 

Just put the code doing the conversion into mc13xxx_adc_conversion()
and remove the call-back.

> > choose an execution route based on some platform matching.
> > 
> 
> Could you help me with giving a hint of how shall I do the "platform
> matching" in this particular driver ? 

We normally match on some platform ID, rather than passing around
pointers to structures containing pointers to device specific
functions.

> The mc13xxx driver seems rather complex with SPI and I2C support and in
> which the subdevices are added (e.g. rtc, adc, etc).

Not sure I follow your point?

> This particular patch just follows current driver design and fixes its
> usability for mc13708 drvice.

Right, but it is the current driver design that I'm trying to change.

> > If you could do the same for print_revision too, that would be even
> > better.
> 
> I would prefer to fix the driver (for mc13708) without the need to
> change the working code.

Okay, but no additional call-backs please.
diff mbox series

Patch

diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 1abe7432aad8..01473d6fda21 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -12,6 +12,7 @@ 
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/mfd/mc34708.h>
 
 #include "mc13xxx.h"
 
@@ -45,6 +46,8 @@ 
 
 #define MC13XXX_ADC2		45
 
+#define MC13XXX_ADC_WORKING		(1 << 0)
+
 void mc13xxx_lock(struct mc13xxx *mc13xxx)
 {
 	if (!mutex_trylock(&mc13xxx->lock)) {
@@ -198,22 +201,30 @@  static void mc34708_print_revision(struct mc13xxx *mc13xxx, u32 revision)
 			maskval(revision, MC34708_REVISION_FAB));
 }
 
+static int mc13xxx_adc_conversion(struct mc13xxx *, unsigned int,
+				  unsigned int, u8, bool, unsigned int *);
+static int mc34708_adc_conversion(struct mc13xxx *, unsigned int,
+				  unsigned int, u8, bool, unsigned int *);
+
 /* These are only exported for mc13xxx-i2c and mc13xxx-spi */
 struct mc13xxx_variant mc13xxx_variant_mc13783 = {
 	.name = "mc13783",
 	.print_revision = mc13xxx_print_revision,
+	.adc_do_conversion = mc13xxx_adc_conversion,
 };
 EXPORT_SYMBOL_GPL(mc13xxx_variant_mc13783);
 
 struct mc13xxx_variant mc13xxx_variant_mc13892 = {
 	.name = "mc13892",
 	.print_revision = mc13xxx_print_revision,
+	.adc_do_conversion = mc13xxx_adc_conversion,
 };
 EXPORT_SYMBOL_GPL(mc13xxx_variant_mc13892);
 
 struct mc13xxx_variant mc13xxx_variant_mc34708 = {
 	.name = "mc34708",
 	.print_revision = mc34708_print_revision,
+	.adc_do_conversion = mc34708_adc_conversion,
 };
 EXPORT_SYMBOL_GPL(mc13xxx_variant_mc34708);
 
@@ -249,7 +260,7 @@  static irqreturn_t mc13xxx_handler_adcdone(int irq, void *data)
 
 #define MC13XXX_ADC_WORKING (1 << 0)
 
-int mc13xxx_adc_do_conversion(struct mc13xxx *mc13xxx, unsigned int mode,
+static int mc13xxx_adc_conversion(struct mc13xxx *mc13xxx, unsigned int mode,
 		unsigned int channel, u8 ato, bool atox,
 		unsigned int *sample)
 {
@@ -358,6 +369,95 @@  int mc13xxx_adc_do_conversion(struct mc13xxx *mc13xxx, unsigned int mode,
 
 	return ret;
 }
+
+static int mc34708_adc_conversion(struct mc13xxx *mc13xxx, unsigned int mode,
+		unsigned int channel, u8 ato, bool atox,
+		unsigned int *sample)
+{
+	int ret, i;
+	u32 adc0, adc3, adc1, old_adc0;
+	struct mc13xxx_adcdone_data adcdone_data = {
+		.mc13xxx = mc13xxx,
+	};
+
+	switch (mode) {
+	case MC13XXX_ADC_MODE_TS:
+		adc0 = MC34708_ADC0_TSEN | MC34708_ADC0_TSSTART |
+			MC34708_ADC0_TSSTOP(7);
+
+		adc1 = MC34708_ADC1_TSDLY1(0xf) |
+			MC34708_ADC1_TSDLY2(0xf) |
+			MC34708_ADC1_TSDLY3(0xf);
+
+		adc3 = MC34708_ADC3_TSSEL(0, MC34708_TS_X) |
+			MC34708_ADC3_TSSEL(1, MC34708_TS_Y) |
+			MC34708_ADC3_TSSEL(2, MC34708_TS_X) |
+			MC34708_ADC3_TSSEL(3, MC34708_TS_Y) |
+			MC34708_ADC3_TSSEL(4, MC34708_TS_X) |
+			MC34708_ADC3_TSSEL(5, MC34708_TS_R) |
+			MC34708_ADC3_TSSEL(6, MC34708_TS_Y) |
+			MC34708_ADC3_TSSEL(7, MC34708_TS_R);
+		break;
+
+	case MC13XXX_ADC_MODE_SINGLE_CHAN:
+	case MC13XXX_ADC_MODE_MULT_CHAN:
+	default:
+		return -EINVAL;
+	}
+
+	init_completion(&adcdone_data.done);
+
+	mc13xxx_lock(mc13xxx);
+
+	if (mc13xxx->adcflags & MC13XXX_ADC_WORKING) {
+		mc13xxx_unlock(mc13xxx);
+		return -EBUSY;
+	}
+
+	mc13xxx->adcflags |= MC13XXX_ADC_WORKING;
+
+	mc13xxx_reg_read(mc13xxx, MC13XXX_ADC0, &old_adc0);
+
+	mc13xxx_irq_request(mc13xxx, MC34708_IRQ_TSDONE,
+			mc13xxx_handler_adcdone, __func__, &adcdone_data);
+	mc13xxx_irq_ack(mc13xxx, MC34708_IRQ_TSDONE);
+
+	mc13xxx_reg_write(mc13xxx, MC34708_ADC3, adc3);
+	mc13xxx_reg_write(mc13xxx, MC13XXX_ADC1, adc1);
+	mc13xxx_reg_write(mc13xxx, MC13XXX_ADC0, adc0);
+
+	mc13xxx_unlock(mc13xxx);
+
+	ret = wait_for_completion_interruptible_timeout(&adcdone_data.done, HZ);
+
+	mc13xxx_lock(mc13xxx);
+
+	mc13xxx_irq_free(mc13xxx, MC34708_IRQ_TSDONE, &adcdone_data);
+
+	if (!ret) {
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	for (i = 0; i < 4; i++)
+		mc13xxx_reg_read(mc13xxx, MC34708_ADC4 + i, &sample[i]);
+
+out:
+	ret = mc13xxx_reg_write(mc13xxx, MC13XXX_ADC0, old_adc0);
+
+	mc13xxx->adcflags &= ~MC13XXX_ADC_WORKING;
+	mc13xxx_unlock(mc13xxx);
+
+	return ret;
+}
+
+int mc13xxx_adc_do_conversion(struct mc13xxx *mc13xxx, unsigned int mode,
+		unsigned int channel, u8 ato, bool atox,
+		unsigned int *sample)
+{
+	return mc13xxx->variant->adc_do_conversion(mc13xxx, mode, channel, ato,
+			atox, sample);
+}
 EXPORT_SYMBOL_GPL(mc13xxx_adc_do_conversion);
 
 static int mc13xxx_add_subdevice_pdata(struct mc13xxx *mc13xxx,
diff --git a/drivers/mfd/mc13xxx.h b/drivers/mfd/mc13xxx.h
index ce6eec52e8eb..0a79fbb8bcb4 100644
--- a/drivers/mfd/mc13xxx.h
+++ b/drivers/mfd/mc13xxx.h
@@ -19,6 +19,9 @@  struct mc13xxx;
 struct mc13xxx_variant {
 	const char *name;
 	void (*print_revision)(struct mc13xxx *mc13xxx, u32 revision);
+	int (*adc_do_conversion)(struct mc13xxx *mc13xxx, unsigned int mode,
+		unsigned int channel, u8 ato, bool atox,
+		unsigned int *sample);
 };
 
 extern struct mc13xxx_variant
diff --git a/include/linux/mfd/mc34708.h b/include/linux/mfd/mc34708.h
new file mode 100644
index 000000000000..c812104dc53d
--- /dev/null
+++ b/include/linux/mfd/mc34708.h
@@ -0,0 +1,37 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2019
+ * Lukasz Majewski, DENX Software Engineering, lukma@denx.de
+ */
+#ifndef __LINUX_MFD_MC34708_H
+#define __LINUX_MFD_MC34708_H
+
+#define MC34708_ADC3			46
+#define MC34708_ADC4			47
+
+#define MC34708_IRQ_TSDONE		1
+
+#define MC34708_ADC0_TSEN		BIT(12)
+#define MC34708_ADC0_TSSTART		BIT(13)
+#define MC34708_ADC0_TSCONT		BIT(14)
+#define MC34708_ADC0_TSHOLD		BIT(15)
+#define MC34708_ADC0_TSPENDETEN		BIT(20)
+
+#define MC34708_ADC0_TSMASK            (MC34708_ADC0_TSPENDETEN | \
+					MC34708_ADC0_TSEN |       \
+					MC34708_ADC0_TSSTART |    \
+					MC34708_ADC0_TSCONT |     \
+					MC34708_ADC0_TSHOLD)
+
+#define MC34708_ADC0_TSSTOP(x)		(((x) & 0x7) << 16)
+
+#define MC34708_ADC3_TSSEL(step, ch)	((ch) << (8 + 2 * (step)))
+#define MC34708_ADC1_TSDLY1(d)		((d) << 12)
+#define MC34708_ADC1_TSDLY2(d)		((d) << 16)
+#define MC34708_ADC1_TSDLY3(d)		((d) << 20)
+
+#define MC34708_TS_X			1
+#define MC34708_TS_Y			2
+#define MC34708_TS_R			3
+
+#endif /* __LINUX_MFD_MC34708_H */