diff mbox series

[28/40] mfd: ti_am335x_tscadc: Add ADC1/magnetic reader support

Message ID 20210825152518.379386-29-miquel.raynal@bootlin.com (mailing list archive)
State Superseded
Headers show
Series TI AM437X ADC1 | expand

Commit Message

Miquel Raynal Aug. 25, 2021, 3:25 p.m. UTC
Introduce a new compatible that has another set of driver data,
targeting am437x SoCs with a magnetic reader instead of the
touchscreen and a more featureful set of registers.

Co-developed-by: Jason Reeder <jreeder@ti.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Jason Reeder <jreeder@ti.com>
---
 drivers/mfd/ti_am335x_tscadc.c       | 43 ++++++++++++++++++++++------
 include/linux/mfd/ti_am335x_tscadc.h |  9 +++++-
 2 files changed, 43 insertions(+), 9 deletions(-)

Comments

Jonathan Cameron Aug. 30, 2021, 2:10 p.m. UTC | #1
On Wed, 25 Aug 2021 17:25:06 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Introduce a new compatible that has another set of driver data,
> targeting am437x SoCs with a magnetic reader instead of the
> touchscreen and a more featureful set of registers.
> 
> Co-developed-by: Jason Reeder <jreeder@ti.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Jason Reeder <jreeder@ti.com>
There is one bit in here that I think should have been back in patch 16 but
maybe I'm missing something.

Jonathan

> ---
>  drivers/mfd/ti_am335x_tscadc.c       | 43 ++++++++++++++++++++++------
>  include/linux/mfd/ti_am335x_tscadc.h |  9 +++++-
>  2 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 1a30610dc65f..f4f6b9db4d2a 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -122,9 +122,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	const __be32 *cur;
>  	struct clk *clk;
>  	u32 val;
> -	bool use_tsc = false;
> +	bool use_tsc = false, use_mag = false;
>  	int tscmag_wires = 0, adc_channels = 0, readouts = 0, cell_idx = 0;
> -	int total_channels, err;
> +	int mag_tracks = 0, total_channels, err;
>  
>  	/* Allocate memory for device */
>  	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
> @@ -146,6 +146,12 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
>  		if (tscmag_wires)
>  			use_tsc = true;
> +	} else {
> +		node = of_get_child_by_name(pdev->dev.of_node, "mag");
> +		of_property_read_u32(node, "ti,tracks", &mag_tracks);
> +		tscmag_wires = mag_tracks * 2;
> +		if (tscmag_wires)
> +			use_mag = true;
>  	}
>  
>  	node = of_get_child_by_name(pdev->dev.of_node, "adc");
> @@ -206,8 +212,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	 * The TSC_ADC_Subsystem has 2 clock domains: OCP_CLK and ADC_CLK.
>  	 * ADCs produce a 12-bit sample every 15 ADC_CLK cycles.
>  	 * am33xx ADCs expect to capture 200ksps.
> -	 * We need the ADC clocks to run at 3MHz.
> -	 * This frequency is valid since TSC_ADC_SS controller design
> +	 * am47xx ADCs expect to capture 867ksps.
> +	 * We need ADC clocks respectively running at 3MHz and 13MHz.
> +	 * These frequencies are valid since TSC_ADC_SS controller design
>  	 * assumes the OCP clock is at least 6x faster than the ADC clock.
>  	 */
>  	clk = devm_clk_get(&pdev->dev, NULL);
> @@ -231,7 +238,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  			else
>  				tscadc->ctrl |= CNTRLREG_TSC_4WIRE;
>  		}
> -
> +	} else {
> +		tscadc->ctrl |= CNTRLREG_MAG_PREAMP_PWRDOWN |
> +				CNTRLREG_MAG_PREAMP_BYPASS;
>  	}
>  
>  	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
> @@ -241,7 +250,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  	/* Enable the TSC module enable bit */
>  	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl | CNTRLREG_SSENB);
>  
> -	/* TSC Cell */
> +	/* TSC or MAG Cell */
>  	if (tscmag_wires > 0) {
>  		cell = &tscadc->cells[cell_idx++];
>  		cell->name = tscadc->data->name_tscmag;
> @@ -329,6 +338,7 @@ static SIMPLE_DEV_PM_OPS(tscadc_pm_ops, tscadc_suspend, tscadc_resume);
>  
>  static const struct ti_tscadc_data tscdata = {
>  	.has_tsc = true,
> +	.has_mag = false,
>  	.name_tscmag = "TI-am335x-tsc",
>  	.compat_tscmag = "ti,am3359-tsc",
>  	.name_adc = "TI-am335x-adc",
> @@ -336,8 +346,25 @@ static const struct ti_tscadc_data tscdata = {
>  	.target_clk_rate = TSC_ADC_CLK,
>  };
>  
> +static const struct ti_tscadc_data magdata = {
> +	.has_tsc = false,
> +	.has_mag = true,
> +	.name_tscmag = "TI-am43xx-mag",
> +	.compat_tscmag = "ti,am4372-mag",
> +	.name_adc = "TI-am43xx-adc",
> +	.compat_adc = "ti,am4372-adc",
> +	.target_clk_rate = MAG_ADC_CLK,
> +};
> +
>  static const struct of_device_id ti_tscadc_dt_ids[] = {
> -	{ .compatible = "ti,am3359-tscadc", },
> +	{
> +		.compatible = "ti,am3359-tscadc",
> +		.data = &tscdata,
Interesting to see match data added here and not before.
Given you don't have any code in here that seems to have
changed to use the match data, was it buggy before or is this still
not used?

> +	},
> +	{
> +		.compatible = "ti,am4372-magadc",
> +		.data = &magdata,
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, ti_tscadc_dt_ids);
> @@ -355,6 +382,6 @@ static struct platform_driver ti_tscadc_driver = {
>  
>  module_platform_driver(ti_tscadc_driver);
>  
> -MODULE_DESCRIPTION("TI touchscreen / ADC MFD controller driver");
> +MODULE_DESCRIPTION("TI touchscreen/magnetic reader/ADC MFD controller driver");
>  MODULE_AUTHOR("Rachna Patil <rachna@ti.com>");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index 082b2af94263..31b22ec567e7 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -129,6 +129,11 @@
>  #define CNTRLREG_TSC_8WIRE	CNTRLREG_TSC_AFE_CTRL(3)
>  #define CNTRLREG_TSC_ENB	BIT(7)
>  
> +/*Control registers bitfields  for MAGADC IP */
> +#define CNTRLREG_MAGADCENB      BIT(0)
> +#define CNTRLREG_MAG_PREAMP_PWRDOWN BIT(5)
> +#define CNTRLREG_MAG_PREAMP_BYPASS  BIT(6)
> +
>  /* FIFO READ Register */
>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
> @@ -141,7 +146,8 @@
>  #define SEQ_STATUS BIT(5)
>  #define CHARGE_STEP		0x11
>  
> -#define TSC_ADC_CLK		3000000
> +#define TSC_ADC_CLK		3000000 /* 3 MHz */
> +#define MAG_ADC_CLK		13000000 /* 13 MHz */

Not sure on current status, but there is a proposed series floating
about that adds HZ_PER_MEGAHZ or something like that which would make
it easier to spot if these have right number of zeros.

>  #define TOTAL_STEPS		16
>  #define TOTAL_CHANNELS		8
>  #define FIFO1_THRESHOLD		19
> @@ -164,6 +170,7 @@
>  
>  struct ti_tscadc_data {
>  	bool has_tsc;
> +	bool has_mag;
>  	char *name_tscmag;
>  	char *compat_tscmag;
>  	char *name_adc;
Grygorii Strashko Sept. 1, 2021, 7:26 p.m. UTC | #2
On 25/08/2021 18:25, Miquel Raynal wrote:
> Introduce a new compatible that has another set of driver data,
> targeting am437x SoCs with a magnetic reader instead of the
> touchscreen and a more featureful set of registers.
> 
> Co-developed-by: Jason Reeder <jreeder@ti.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Jason Reeder <jreeder@ti.com>
> ---
>   drivers/mfd/ti_am335x_tscadc.c       | 43 ++++++++++++++++++++++------
>   include/linux/mfd/ti_am335x_tscadc.h |  9 +++++-
>   2 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index 1a30610dc65f..f4f6b9db4d2a 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -122,9 +122,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>   	const __be32 *cur;
>   	struct clk *clk;
>   	u32 val;
> -	bool use_tsc = false;
> +	bool use_tsc = false, use_mag = false;
>   	int tscmag_wires = 0, adc_channels = 0, readouts = 0, cell_idx = 0;
> -	int total_channels, err;
> +	int mag_tracks = 0, total_channels, err;
>   
>   	/* Allocate memory for device */
>   	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
> @@ -146,6 +146,12 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>   		of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
>   		if (tscmag_wires)
>   			use_tsc = true;
> +	} else {
> +		node = of_get_child_by_name(pdev->dev.of_node, "mag");
> +		of_property_read_u32(node, "ti,tracks", &mag_tracks);

"ti,tracks" seems undocumented?

[....]
Miquel Raynal Sept. 2, 2021, 6:47 a.m. UTC | #3
Hi Grygorii,

Grygorii Strashko <grygorii.strashko@ti.com> wrote on Wed, 1 Sep 2021
22:26:25 +0300:

> On 25/08/2021 18:25, Miquel Raynal wrote:
> > Introduce a new compatible that has another set of driver data,
> > targeting am437x SoCs with a magnetic reader instead of the
> > touchscreen and a more featureful set of registers.
> > 
> > Co-developed-by: Jason Reeder <jreeder@ti.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Jason Reeder <jreeder@ti.com>
> > ---
> >   drivers/mfd/ti_am335x_tscadc.c       | 43 ++++++++++++++++++++++------
> >   include/linux/mfd/ti_am335x_tscadc.h |  9 +++++-
> >   2 files changed, 43 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> > index 1a30610dc65f..f4f6b9db4d2a 100644
> > --- a/drivers/mfd/ti_am335x_tscadc.c
> > +++ b/drivers/mfd/ti_am335x_tscadc.c
> > @@ -122,9 +122,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
> >   	const __be32 *cur;
> >   	struct clk *clk;
> >   	u32 val;
> > -	bool use_tsc = false;
> > +	bool use_tsc = false, use_mag = false;
> >   	int tscmag_wires = 0, adc_channels = 0, readouts = 0, cell_idx = 0;
> > -	int total_channels, err;
> > +	int mag_tracks = 0, total_channels, err;  
> >   >   	/* Allocate memory for device */  
> >   	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
> > @@ -146,6 +146,12 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
> >   		of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
> >   		if (tscmag_wires)
> >   			use_tsc = true;
> > +	} else {
> > +		node = of_get_child_by_name(pdev->dev.of_node, "mag");
> > +		of_property_read_u32(node, "ti,tracks", &mag_tracks);  
> 
> "ti,tracks" seems undocumented?

Well that's true and almost on purpose, I am not focusing on the
magnetic reader feature, it is not supported, I don't have one, I don't
plan to add support for it. But in the driver I need to know how many
"tracks" are unavailable for the ADC in order to implement the entire
logic (this block comes from TI and the naming from Jason Reeder).

I am not comfortable writing a binding file for a device that I won't
use, it's the best way to miss something and have stable broken
bindings in the future. So I assumed it was not a big deal to have this
property in the code, which may be updated/removed/enhanced later if
needed without having to mess with the code too much. What do you think?

Thanks,
Miquèl
Grygorii Strashko Sept. 2, 2021, 9:57 a.m. UTC | #4
On 02/09/2021 09:47, Miquel Raynal wrote:
> Hi Grygorii,
> 
> Grygorii Strashko <grygorii.strashko@ti.com> wrote on Wed, 1 Sep 2021
> 22:26:25 +0300:
> 
>> On 25/08/2021 18:25, Miquel Raynal wrote:
>>> Introduce a new compatible that has another set of driver data,
>>> targeting am437x SoCs with a magnetic reader instead of the
>>> touchscreen and a more featureful set of registers.
>>>
>>> Co-developed-by: Jason Reeder <jreeder@ti.com>
>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>>> Signed-off-by: Jason Reeder <jreeder@ti.com>
>>> ---
>>>    drivers/mfd/ti_am335x_tscadc.c       | 43 ++++++++++++++++++++++------
>>>    include/linux/mfd/ti_am335x_tscadc.h |  9 +++++-
>>>    2 files changed, 43 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
>>> index 1a30610dc65f..f4f6b9db4d2a 100644
>>> --- a/drivers/mfd/ti_am335x_tscadc.c
>>> +++ b/drivers/mfd/ti_am335x_tscadc.c
>>> @@ -122,9 +122,9 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>>>    	const __be32 *cur;
>>>    	struct clk *clk;
>>>    	u32 val;
>>> -	bool use_tsc = false;
>>> +	bool use_tsc = false, use_mag = false;
>>>    	int tscmag_wires = 0, adc_channels = 0, readouts = 0, cell_idx = 0;
>>> -	int total_channels, err;
>>> +	int mag_tracks = 0, total_channels, err;
>>>    >   	/* Allocate memory for device */
>>>    	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
>>> @@ -146,6 +146,12 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>>>    		of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
>>>    		if (tscmag_wires)
>>>    			use_tsc = true;
>>> +	} else {
>>> +		node = of_get_child_by_name(pdev->dev.of_node, "mag");
>>> +		of_property_read_u32(node, "ti,tracks", &mag_tracks);
>>
>> "ti,tracks" seems undocumented?
> 
> Well that's true and almost on purpose, I am not focusing on the
> magnetic reader feature, it is not supported, I don't have one, I don't
> plan to add support for it. But in the driver I need to know how many
> "tracks" are unavailable for the ADC in order to implement the entire
> logic (this block comes from TI and the naming from Jason Reeder).
> 
> I am not comfortable writing a binding file for a device that I won't
> use, it's the best way to miss something and have stable broken
> bindings in the future. So I assumed it was not a big deal to have this
> property in the code, which may be updated/removed/enhanced later if
> needed without having to mess with the code too much. What do you think?

Sry, it's not ok.
You need to (a) add binding or (b) w/a it in some way -
like drop it and use const value instead, for example.
Miquel Raynal Sept. 2, 2021, 6:31 p.m. UTC | #5
Hi Jonathan,

Jonathan Cameron <jic23@kernel.org> wrote on Mon, 30 Aug 2021 15:10:10
+0100:

> On Wed, 25 Aug 2021 17:25:06 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Introduce a new compatible that has another set of driver data,
> > targeting am437x SoCs with a magnetic reader instead of the
> > touchscreen and a more featureful set of registers.
> > 
> > Co-developed-by: Jason Reeder <jreeder@ti.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Jason Reeder <jreeder@ti.com>  
> There is one bit in here that I think should have been back in patch 16 but
> maybe I'm missing something.

Good catch, indeed I should have wired the compatible with the driver
data since patch 16. I will fix this.

Thanks,
Miquèl
Miquel Raynal Sept. 2, 2021, 8:03 p.m. UTC | #6
Hi Jonathan,


> > +static const struct ti_tscadc_data magdata = {
> > +	.has_tsc = false,
> > +	.has_mag = true,
> > +	.name_tscmag = "TI-am43xx-mag",
> > +	.compat_tscmag = "ti,am4372-mag",
> > +	.name_adc = "TI-am43xx-adc",
> > +	.compat_adc = "ti,am4372-adc",
> > +	.target_clk_rate = MAG_ADC_CLK,
> > +};
> > +
> >  static const struct of_device_id ti_tscadc_dt_ids[] = {
> > -	{ .compatible = "ti,am3359-tscadc", },
> > +	{
> > +		.compatible = "ti,am3359-tscadc",
> > +		.data = &tscdata,  
> Interesting to see match data added here and not before.
> Given you don't have any code in here that seems to have
> changed to use the match data, was it buggy before or is this still
> not used?

As said earlier, it was buggy before. It is now fixed.

> 
> > +	},
> > +	{
> > +		.compatible = "ti,am4372-magadc",
> > +		.data = &magdata,
> > +	},
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(of, ti_tscadc_dt_ids);
> > @@ -355,6 +382,6 @@ static struct platform_driver ti_tscadc_driver = {
> >  
> >  module_platform_driver(ti_tscadc_driver);
> >  
> > -MODULE_DESCRIPTION("TI touchscreen / ADC MFD controller driver");
> > +MODULE_DESCRIPTION("TI touchscreen/magnetic reader/ADC MFD controller driver");
> >  MODULE_AUTHOR("Rachna Patil <rachna@ti.com>");
> >  MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> > index 082b2af94263..31b22ec567e7 100644
> > --- a/include/linux/mfd/ti_am335x_tscadc.h
> > +++ b/include/linux/mfd/ti_am335x_tscadc.h
> > @@ -129,6 +129,11 @@
> >  #define CNTRLREG_TSC_8WIRE	CNTRLREG_TSC_AFE_CTRL(3)
> >  #define CNTRLREG_TSC_ENB	BIT(7)
> >  
> > +/*Control registers bitfields  for MAGADC IP */
> > +#define CNTRLREG_MAGADCENB      BIT(0)
> > +#define CNTRLREG_MAG_PREAMP_PWRDOWN BIT(5)
> > +#define CNTRLREG_MAG_PREAMP_BYPASS  BIT(6)
> > +
> >  /* FIFO READ Register */
> >  #define FIFOREAD_DATA_MASK (0xfff << 0)
> >  #define FIFOREAD_CHNLID_MASK (0xf << 16)
> > @@ -141,7 +146,8 @@
> >  #define SEQ_STATUS BIT(5)
> >  #define CHARGE_STEP		0x11
> >  
> > -#define TSC_ADC_CLK		3000000
> > +#define TSC_ADC_CLK		3000000 /* 3 MHz */
> > +#define MAG_ADC_CLK		13000000 /* 13 MHz */  
> 
> Not sure on current status, but there is a proposed series floating
> about that adds HZ_PER_MEGAHZ or something like that which would make
> it easier to spot if these have right number of zeros.

Would be nice indeed, but it looks like it's not yet mainline :/

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 1a30610dc65f..f4f6b9db4d2a 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -122,9 +122,9 @@  static	int ti_tscadc_probe(struct platform_device *pdev)
 	const __be32 *cur;
 	struct clk *clk;
 	u32 val;
-	bool use_tsc = false;
+	bool use_tsc = false, use_mag = false;
 	int tscmag_wires = 0, adc_channels = 0, readouts = 0, cell_idx = 0;
-	int total_channels, err;
+	int mag_tracks = 0, total_channels, err;
 
 	/* Allocate memory for device */
 	tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
@@ -146,6 +146,12 @@  static	int ti_tscadc_probe(struct platform_device *pdev)
 		of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
 		if (tscmag_wires)
 			use_tsc = true;
+	} else {
+		node = of_get_child_by_name(pdev->dev.of_node, "mag");
+		of_property_read_u32(node, "ti,tracks", &mag_tracks);
+		tscmag_wires = mag_tracks * 2;
+		if (tscmag_wires)
+			use_mag = true;
 	}
 
 	node = of_get_child_by_name(pdev->dev.of_node, "adc");
@@ -206,8 +212,9 @@  static	int ti_tscadc_probe(struct platform_device *pdev)
 	 * The TSC_ADC_Subsystem has 2 clock domains: OCP_CLK and ADC_CLK.
 	 * ADCs produce a 12-bit sample every 15 ADC_CLK cycles.
 	 * am33xx ADCs expect to capture 200ksps.
-	 * We need the ADC clocks to run at 3MHz.
-	 * This frequency is valid since TSC_ADC_SS controller design
+	 * am47xx ADCs expect to capture 867ksps.
+	 * We need ADC clocks respectively running at 3MHz and 13MHz.
+	 * These frequencies are valid since TSC_ADC_SS controller design
 	 * assumes the OCP clock is at least 6x faster than the ADC clock.
 	 */
 	clk = devm_clk_get(&pdev->dev, NULL);
@@ -231,7 +238,9 @@  static	int ti_tscadc_probe(struct platform_device *pdev)
 			else
 				tscadc->ctrl |= CNTRLREG_TSC_4WIRE;
 		}
-
+	} else {
+		tscadc->ctrl |= CNTRLREG_MAG_PREAMP_PWRDOWN |
+				CNTRLREG_MAG_PREAMP_BYPASS;
 	}
 
 	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
@@ -241,7 +250,7 @@  static	int ti_tscadc_probe(struct platform_device *pdev)
 	/* Enable the TSC module enable bit */
 	regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl | CNTRLREG_SSENB);
 
-	/* TSC Cell */
+	/* TSC or MAG Cell */
 	if (tscmag_wires > 0) {
 		cell = &tscadc->cells[cell_idx++];
 		cell->name = tscadc->data->name_tscmag;
@@ -329,6 +338,7 @@  static SIMPLE_DEV_PM_OPS(tscadc_pm_ops, tscadc_suspend, tscadc_resume);
 
 static const struct ti_tscadc_data tscdata = {
 	.has_tsc = true,
+	.has_mag = false,
 	.name_tscmag = "TI-am335x-tsc",
 	.compat_tscmag = "ti,am3359-tsc",
 	.name_adc = "TI-am335x-adc",
@@ -336,8 +346,25 @@  static const struct ti_tscadc_data tscdata = {
 	.target_clk_rate = TSC_ADC_CLK,
 };
 
+static const struct ti_tscadc_data magdata = {
+	.has_tsc = false,
+	.has_mag = true,
+	.name_tscmag = "TI-am43xx-mag",
+	.compat_tscmag = "ti,am4372-mag",
+	.name_adc = "TI-am43xx-adc",
+	.compat_adc = "ti,am4372-adc",
+	.target_clk_rate = MAG_ADC_CLK,
+};
+
 static const struct of_device_id ti_tscadc_dt_ids[] = {
-	{ .compatible = "ti,am3359-tscadc", },
+	{
+		.compatible = "ti,am3359-tscadc",
+		.data = &tscdata,
+	},
+	{
+		.compatible = "ti,am4372-magadc",
+		.data = &magdata,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ti_tscadc_dt_ids);
@@ -355,6 +382,6 @@  static struct platform_driver ti_tscadc_driver = {
 
 module_platform_driver(ti_tscadc_driver);
 
-MODULE_DESCRIPTION("TI touchscreen / ADC MFD controller driver");
+MODULE_DESCRIPTION("TI touchscreen/magnetic reader/ADC MFD controller driver");
 MODULE_AUTHOR("Rachna Patil <rachna@ti.com>");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 082b2af94263..31b22ec567e7 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -129,6 +129,11 @@ 
 #define CNTRLREG_TSC_8WIRE	CNTRLREG_TSC_AFE_CTRL(3)
 #define CNTRLREG_TSC_ENB	BIT(7)
 
+/*Control registers bitfields  for MAGADC IP */
+#define CNTRLREG_MAGADCENB      BIT(0)
+#define CNTRLREG_MAG_PREAMP_PWRDOWN BIT(5)
+#define CNTRLREG_MAG_PREAMP_BYPASS  BIT(6)
+
 /* FIFO READ Register */
 #define FIFOREAD_DATA_MASK (0xfff << 0)
 #define FIFOREAD_CHNLID_MASK (0xf << 16)
@@ -141,7 +146,8 @@ 
 #define SEQ_STATUS BIT(5)
 #define CHARGE_STEP		0x11
 
-#define TSC_ADC_CLK		3000000
+#define TSC_ADC_CLK		3000000 /* 3 MHz */
+#define MAG_ADC_CLK		13000000 /* 13 MHz */
 #define TOTAL_STEPS		16
 #define TOTAL_CHANNELS		8
 #define FIFO1_THRESHOLD		19
@@ -164,6 +170,7 @@ 
 
 struct ti_tscadc_data {
 	bool has_tsc;
+	bool has_mag;
 	char *name_tscmag;
 	char *compat_tscmag;
 	char *name_adc;