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

Miquel Raynal
2021-09-02T20:03:42Z

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