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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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