* [PATCH 0/2] tscadc: Couple of fixes @ 2018-11-19 6:42 Vignesh R 2018-11-19 6:42 ` [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells Vignesh R 2018-11-19 6:42 ` [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement Vignesh R 0 siblings, 2 replies; 11+ messages in thread From: Vignesh R @ 2018-11-19 6:42 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Lee Jones, Vignesh R, linux-iio, linux-omap, linux-kernel Couple of fixes for tscadc drivers that I found while adding support for new SoC. Patches are standalone and can go via individual domain trees. Vignesh R (2): mfd: ti_am335x_tscadc: Provide unique name for child mfd cells iio: adc: ti_am335x_tscadc: Improve accuracy of measurement drivers/iio/adc/ti_am335x_adc.c | 5 ++++- drivers/mfd/ti_am335x_tscadc.c | 6 ++++-- include/linux/mfd/ti_am335x_tscadc.h | 4 ++++ 3 files changed, 12 insertions(+), 3 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells 2018-11-19 6:42 [PATCH 0/2] tscadc: Couple of fixes Vignesh R @ 2018-11-19 6:42 ` Vignesh R 2018-11-28 9:07 ` Lee Jones 2018-11-19 6:42 ` [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement Vignesh R 1 sibling, 1 reply; 11+ messages in thread From: Vignesh R @ 2018-11-19 6:42 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Lee Jones, Vignesh R, linux-iio, linux-omap, linux-kernel Provide unique names for child mfd cells, this is required in order to support registering of multiple instances of same ti_am335x_tscadc IP. Signed-off-by: Vignesh R <vigneshr@ti.com> --- drivers/mfd/ti_am335x_tscadc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index c2d47d78705b..ee08af34f81d 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -248,7 +248,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) if (tsc_wires > 0) { tscadc->tsc_cell = tscadc->used_cells; cell = &tscadc->cells[tscadc->used_cells++]; - cell->name = "TI-am335x-tsc"; + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", + dev_name(&pdev->dev), "tsc"); cell->of_compatible = "ti,am3359-tsc"; cell->platform_data = &tscadc; cell->pdata_size = sizeof(tscadc); @@ -258,7 +259,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) if (adc_channels > 0) { tscadc->adc_cell = tscadc->used_cells; cell = &tscadc->cells[tscadc->used_cells++]; - cell->name = "TI-am335x-adc"; + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", + dev_name(&pdev->dev), "adc"); cell->of_compatible = "ti,am3359-adc"; cell->platform_data = &tscadc; cell->pdata_size = sizeof(tscadc); -- 2.19.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells 2018-11-19 6:42 ` [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells Vignesh R @ 2018-11-28 9:07 ` Lee Jones 2018-11-28 9:21 ` Vignesh R 0 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2018-11-28 9:07 UTC (permalink / raw) To: Vignesh R Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel On Mon, 19 Nov 2018, Vignesh R wrote: > Provide unique names for child mfd cells, this is required in order to > support registering of multiple instances of same ti_am335x_tscadc IP. I don't think it is. What is the error you are receiving? > Signed-off-by: Vignesh R <vigneshr@ti.com> > --- > drivers/mfd/ti_am335x_tscadc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c > index c2d47d78705b..ee08af34f81d 100644 > --- a/drivers/mfd/ti_am335x_tscadc.c > +++ b/drivers/mfd/ti_am335x_tscadc.c > @@ -248,7 +248,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) > if (tsc_wires > 0) { > tscadc->tsc_cell = tscadc->used_cells; > cell = &tscadc->cells[tscadc->used_cells++]; > - cell->name = "TI-am335x-tsc"; > + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", > + dev_name(&pdev->dev), "tsc"); > cell->of_compatible = "ti,am3359-tsc"; > cell->platform_data = &tscadc; > cell->pdata_size = sizeof(tscadc); > @@ -258,7 +259,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) > if (adc_channels > 0) { > tscadc->adc_cell = tscadc->used_cells; > cell = &tscadc->cells[tscadc->used_cells++]; > - cell->name = "TI-am335x-adc"; > + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", > + dev_name(&pdev->dev), "adc"); > cell->of_compatible = "ti,am3359-adc"; > cell->platform_data = &tscadc; > cell->pdata_size = sizeof(tscadc); -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells 2018-11-28 9:07 ` Lee Jones @ 2018-11-28 9:21 ` Vignesh R 2018-11-28 11:03 ` Lee Jones 0 siblings, 1 reply; 11+ messages in thread From: Vignesh R @ 2018-11-28 9:21 UTC (permalink / raw) To: Lee Jones Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel Hi, On 28/11/18 2:37 PM, Lee Jones wrote: > On Mon, 19 Nov 2018, Vignesh R wrote: > >> Provide unique names for child mfd cells, this is required in order to >> support registering of multiple instances of same ti_am335x_tscadc IP. > > I don't think it is. What is the error you are receiving? > W/o this patch I get: [ 2.296839] sysfs: cannot create duplicate filename '/bus/platform/devices/TI-am335x-adc' [ 2.305085] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 4.20.0-rc3-next-20181123-00018-g 7791ce8f123f-dirty #182 [ 2.315254] Hardware name: Texas Instruments AM654 Base Board (DT) [ 2.321447] Workqueue: events deferred_probe_work_func [ 2.326585] Call trace: [ 2.329036] dump_backtrace+0x0/0x1c0 [ 2.332699] show_stack+0x14/0x20 [ 2.336017] dump_stack+0x9c/0xbc [ 2.339333] sysfs_warn_dup+0x60/0x78 [ 2.342995] sysfs_do_create_link_sd.isra.0+0xdc/0xe8 [ 2.348042] sysfs_create_link+0x20/0x40 [ 2.351963] bus_add_device+0x68/0x130 [ 2.355711] device_add+0x398/0x618 [ 2.359199] platform_device_add+0x120/0x290 [ 2.363469] mfd_add_device+0x23c/0x2e8 [ 2.367302] mfd_add_devices+0xb4/0x158 [ 2.371136] ti_tscadc_probe+0x35c/0x4c8 [ 2.375056] platform_drv_probe+0x50/0xa0 [ 2.379062] really_probe+0x204/0x2a8 [ 2.382722] driver_probe_device+0x58/0x100 [ 2.386903] __device_attach_driver+0x98/0xf0 [ 2.391258] bus_for_each_drv+0x64/0xc8 [ 2.395091] __device_attach+0xd8/0x138 [ 2.398923] device_initial_probe+0x10/0x18 [ 2.403103] bus_probe_device+0x90/0x98 [ 2.406938] deferred_probe_work_func+0x74/0xb0 [ 2.411470] process_one_work+0x1e0/0x330 [ 2.415478] worker_thread+0x238/0x460 [ 2.419225] kthread+0x128/0x130 [ 2.422452] ret_from_fork+0x10/0x1c [ 2.426195] ti_am3359-tscadc: probe of 40210000.tscadc failed with error -1 >> Signed-off-by: Vignesh R <vigneshr@ti.com> >> --- >> drivers/mfd/ti_am335x_tscadc.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c >> index c2d47d78705b..ee08af34f81d 100644 >> --- a/drivers/mfd/ti_am335x_tscadc.c >> +++ b/drivers/mfd/ti_am335x_tscadc.c >> @@ -248,7 +248,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) >> if (tsc_wires > 0) { >> tscadc->tsc_cell = tscadc->used_cells; >> cell = &tscadc->cells[tscadc->used_cells++]; >> - cell->name = "TI-am335x-tsc"; >> + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", >> + dev_name(&pdev->dev), "tsc"); >> cell->of_compatible = "ti,am3359-tsc"; >> cell->platform_data = &tscadc; >> cell->pdata_size = sizeof(tscadc); >> @@ -258,7 +259,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) >> if (adc_channels > 0) { >> tscadc->adc_cell = tscadc->used_cells; >> cell = &tscadc->cells[tscadc->used_cells++]; >> - cell->name = "TI-am335x-adc"; >> + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", >> + dev_name(&pdev->dev), "adc"); >> cell->of_compatible = "ti,am3359-adc"; >> cell->platform_data = &tscadc; >> cell->pdata_size = sizeof(tscadc); > -- Regards Vignesh ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells 2018-11-28 9:21 ` Vignesh R @ 2018-11-28 11:03 ` Lee Jones 2018-11-28 11:43 ` Vignesh R 0 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2018-11-28 11:03 UTC (permalink / raw) To: Vignesh R Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel On Wed, 28 Nov 2018, Vignesh R wrote: > Hi, > > On 28/11/18 2:37 PM, Lee Jones wrote: > > On Mon, 19 Nov 2018, Vignesh R wrote: > > > >> Provide unique names for child mfd cells, this is required in order to > >> support registering of multiple instances of same ti_am335x_tscadc IP. > > > > I don't think it is. What is the error you are receiving? > > W/o this patch I get: > > [ 2.296839] sysfs: cannot create duplicate filename '/bus/platform/devices/TI-am335x-adc' > [ 2.305085] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 4.20.0-rc3-next-20181123-00018-g > 7791ce8f123f-dirty #182 > [ 2.315254] Hardware name: Texas Instruments AM654 Base Board (DT) > [ 2.321447] Workqueue: events deferred_probe_work_func > [ 2.326585] Call trace: > [ 2.329036] dump_backtrace+0x0/0x1c0 > [ 2.332699] show_stack+0x14/0x20 > [ 2.336017] dump_stack+0x9c/0xbc > [ 2.339333] sysfs_warn_dup+0x60/0x78 > [ 2.342995] sysfs_do_create_link_sd.isra.0+0xdc/0xe8 > [ 2.348042] sysfs_create_link+0x20/0x40 > [ 2.351963] bus_add_device+0x68/0x130 > [ 2.355711] device_add+0x398/0x618 > [ 2.359199] platform_device_add+0x120/0x290 > [ 2.363469] mfd_add_device+0x23c/0x2e8 > [ 2.367302] mfd_add_devices+0xb4/0x158 > [ 2.371136] ti_tscadc_probe+0x35c/0x4c8 > [ 2.375056] platform_drv_probe+0x50/0xa0 > [ 2.379062] really_probe+0x204/0x2a8 > [ 2.382722] driver_probe_device+0x58/0x100 > [ 2.386903] __device_attach_driver+0x98/0xf0 > [ 2.391258] bus_for_each_drv+0x64/0xc8 > [ 2.395091] __device_attach+0xd8/0x138 > [ 2.398923] device_initial_probe+0x10/0x18 > [ 2.403103] bus_probe_device+0x90/0x98 > [ 2.406938] deferred_probe_work_func+0x74/0xb0 > [ 2.411470] process_one_work+0x1e0/0x330 > [ 2.415478] worker_thread+0x238/0x460 > [ 2.419225] kthread+0x128/0x130 > [ 2.422452] ret_from_fork+0x10/0x1c > [ 2.426195] ti_am3359-tscadc: probe of 40210000.tscadc failed with error -1 That's because of the way the cells are being registered: err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells, tscadc->used_cells, NULL, 0, NULL); The cells are not numbered (tscadc->cells->id is always 0) and you are forcing the use of the platform ID from the parent platform device (with pdev->id here). Instead you should s/pdev->id/PLATFORM_DEVID_AUTO/ or number the cells manually. I suggest the former. > >> Signed-off-by: Vignesh R <vigneshr@ti.com> > >> --- > >> drivers/mfd/ti_am335x_tscadc.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c > >> index c2d47d78705b..ee08af34f81d 100644 > >> --- a/drivers/mfd/ti_am335x_tscadc.c > >> +++ b/drivers/mfd/ti_am335x_tscadc.c > >> @@ -248,7 +248,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) > >> if (tsc_wires > 0) { > >> tscadc->tsc_cell = tscadc->used_cells; > >> cell = &tscadc->cells[tscadc->used_cells++]; > >> - cell->name = "TI-am335x-tsc"; > >> + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", > >> + dev_name(&pdev->dev), "tsc"); > >> cell->of_compatible = "ti,am3359-tsc"; > >> cell->platform_data = &tscadc; > >> cell->pdata_size = sizeof(tscadc); > >> @@ -258,7 +259,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) > >> if (adc_channels > 0) { > >> tscadc->adc_cell = tscadc->used_cells; > >> cell = &tscadc->cells[tscadc->used_cells++]; > >> - cell->name = "TI-am335x-adc"; > >> + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", > >> + dev_name(&pdev->dev), "adc"); > >> cell->of_compatible = "ti,am3359-adc"; > >> cell->platform_data = &tscadc; > >> cell->pdata_size = sizeof(tscadc); > > > -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells 2018-11-28 11:03 ` Lee Jones @ 2018-11-28 11:43 ` Vignesh R 0 siblings, 0 replies; 11+ messages in thread From: Vignesh R @ 2018-11-28 11:43 UTC (permalink / raw) To: Lee Jones Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel On 28/11/18 4:33 PM, Lee Jones wrote: > On Wed, 28 Nov 2018, Vignesh R wrote: > >> Hi, >> >> On 28/11/18 2:37 PM, Lee Jones wrote: >>> On Mon, 19 Nov 2018, Vignesh R wrote: >>> >>>> Provide unique names for child mfd cells, this is required in order to >>>> support registering of multiple instances of same ti_am335x_tscadc IP. >>> >>> I don't think it is. What is the error you are receiving? >> >> W/o this patch I get: >> >> [ 2.296839] sysfs: cannot create duplicate filename '/bus/platform/devices/TI-am335x-adc' >> [ 2.305085] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 4.20.0-rc3-next-20181123-00018-g >> 7791ce8f123f-dirty #182 >> [ 2.315254] Hardware name: Texas Instruments AM654 Base Board (DT) >> [ 2.321447] Workqueue: events deferred_probe_work_func >> [ 2.326585] Call trace: >> [ 2.329036] dump_backtrace+0x0/0x1c0 >> [ 2.332699] show_stack+0x14/0x20 >> [ 2.336017] dump_stack+0x9c/0xbc >> [ 2.339333] sysfs_warn_dup+0x60/0x78 >> [ 2.342995] sysfs_do_create_link_sd.isra.0+0xdc/0xe8 >> [ 2.348042] sysfs_create_link+0x20/0x40 >> [ 2.351963] bus_add_device+0x68/0x130 >> [ 2.355711] device_add+0x398/0x618 >> [ 2.359199] platform_device_add+0x120/0x290 >> [ 2.363469] mfd_add_device+0x23c/0x2e8 >> [ 2.367302] mfd_add_devices+0xb4/0x158 >> [ 2.371136] ti_tscadc_probe+0x35c/0x4c8 >> [ 2.375056] platform_drv_probe+0x50/0xa0 >> [ 2.379062] really_probe+0x204/0x2a8 >> [ 2.382722] driver_probe_device+0x58/0x100 >> [ 2.386903] __device_attach_driver+0x98/0xf0 >> [ 2.391258] bus_for_each_drv+0x64/0xc8 >> [ 2.395091] __device_attach+0xd8/0x138 >> [ 2.398923] device_initial_probe+0x10/0x18 >> [ 2.403103] bus_probe_device+0x90/0x98 >> [ 2.406938] deferred_probe_work_func+0x74/0xb0 >> [ 2.411470] process_one_work+0x1e0/0x330 >> [ 2.415478] worker_thread+0x238/0x460 >> [ 2.419225] kthread+0x128/0x130 >> [ 2.422452] ret_from_fork+0x10/0x1c >> [ 2.426195] ti_am3359-tscadc: probe of 40210000.tscadc failed with error -1 > > That's because of the way the cells are being registered: > > err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells, > tscadc->used_cells, NULL, 0, NULL); > > The cells are not numbered (tscadc->cells->id is always 0) and you are > forcing the use of the platform ID from the parent platform device > (with pdev->id here). Instead you should s/pdev->id/PLATFORM_DEVID_AUTO/ > or number the cells manually. I suggest the former. > Using PLATFORM_DEVID_AUTO works fine. I will post v2 with this change. Thanks for the suggestion! Regards Vignesh >>>> Signed-off-by: Vignesh R <vigneshr@ti.com> >>>> --- >>>> drivers/mfd/ti_am335x_tscadc.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c >>>> index c2d47d78705b..ee08af34f81d 100644 >>>> --- a/drivers/mfd/ti_am335x_tscadc.c >>>> +++ b/drivers/mfd/ti_am335x_tscadc.c >>>> @@ -248,7 +248,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) >>>> if (tsc_wires > 0) { >>>> tscadc->tsc_cell = tscadc->used_cells; >>>> cell = &tscadc->cells[tscadc->used_cells++]; >>>> - cell->name = "TI-am335x-tsc"; >>>> + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", >>>> + dev_name(&pdev->dev), "tsc"); >>>> cell->of_compatible = "ti,am3359-tsc"; >>>> cell->platform_data = &tscadc; >>>> cell->pdata_size = sizeof(tscadc); >>>> @@ -258,7 +259,8 @@ static int ti_tscadc_probe(struct platform_device *pdev) >>>> if (adc_channels > 0) { >>>> tscadc->adc_cell = tscadc->used_cells; >>>> cell = &tscadc->cells[tscadc->used_cells++]; >>>> - cell->name = "TI-am335x-adc"; >>>> + cell->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s:%s", >>>> + dev_name(&pdev->dev), "adc"); >>>> cell->of_compatible = "ti,am3359-adc"; >>>> cell->platform_data = &tscadc; >>>> cell->pdata_size = sizeof(tscadc); >>> >> > -- Regards Vignesh ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement 2018-11-19 6:42 [PATCH 0/2] tscadc: Couple of fixes Vignesh R 2018-11-19 6:42 ` [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells Vignesh R @ 2018-11-19 6:42 ` Vignesh R 2018-11-25 13:46 ` Jonathan Cameron 2018-11-28 9:14 ` Lee Jones 1 sibling, 2 replies; 11+ messages in thread From: Vignesh R @ 2018-11-19 6:42 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Lee Jones, Vignesh R, linux-iio, linux-omap, linux-kernel When performing single ended measurements with TSCADC, its recommended to set negative input (SEL_INM_SWC_3_0) of ADC step to ADC's VREFN in the corresponding STEP_CONFIGx register. Also, the positive(SEL_RFP_SWC_2_0) and negative(SEL_RFM_SWC_1_0) reference voltage for ADC step needs to be set to VREFP and VREFN respectively in STEP_CONFIGx register. Without these changes, there may be variation of as much as ~2% in the ADC's digital output which is bad for precise measurement. Signed-off-by: Vignesh R <vigneshr@ti.com> --- drivers/iio/adc/ti_am335x_adc.c | 5 ++++- include/linux/mfd/ti_am335x_tscadc.h | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index cafb1dcadc48..9d984f2a8ba7 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -142,7 +142,10 @@ static void tiadc_step_config(struct iio_dev *indio_dev) stepconfig |= STEPCONFIG_MODE_SWCNT; tiadc_writel(adc_dev, REG_STEPCONFIG(steps), - stepconfig | STEPCONFIG_INP(chan)); + stepconfig | STEPCONFIG_INP(chan) | + STEPCONFIG_INM_ADCREFM | + STEPCONFIG_RFP_VREFP | + STEPCONFIG_RFM_VREFN); if (adc_dev->open_delay[i] > STEPDELAY_OPEN_MASK) { dev_warn(dev, "chan %d open delay truncating to 0x3FFFF\n", diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h index b9a53e013bff..483168403ae5 100644 --- a/include/linux/mfd/ti_am335x_tscadc.h +++ b/include/linux/mfd/ti_am335x_tscadc.h @@ -78,6 +78,8 @@ #define STEPCONFIG_YNN BIT(8) #define STEPCONFIG_XNP BIT(9) #define STEPCONFIG_YPN BIT(10) +#define STEPCONFIG_RFP(val) ((val) << 12) +#define STEPCONFIG_RFP_VREFP (0x3 << 12) #define STEPCONFIG_INM_MASK (0xF << 15) #define STEPCONFIG_INM(val) ((val) << 15) #define STEPCONFIG_INM_ADCREFM STEPCONFIG_INM(8) @@ -86,6 +88,8 @@ #define STEPCONFIG_INP_AN4 STEPCONFIG_INP(4) #define STEPCONFIG_INP_ADCREFM STEPCONFIG_INP(8) #define STEPCONFIG_FIFO1 BIT(26) +#define STEPCONFIG_RFM(val) ((val) << 23) +#define STEPCONFIG_RFM_VREFN (0x3 << 23) /* Delay register */ #define STEPDELAY_OPEN_MASK (0x3FFFF << 0) -- 2.19.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement 2018-11-19 6:42 ` [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement Vignesh R @ 2018-11-25 13:46 ` Jonathan Cameron 2018-11-28 9:14 ` Lee Jones 1 sibling, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2018-11-25 13:46 UTC (permalink / raw) To: Vignesh R Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Lee Jones, linux-iio, linux-omap, linux-kernel On Mon, 19 Nov 2018 12:12:36 +0530 Vignesh R <vigneshr@ti.com> wrote: > When performing single ended measurements with TSCADC, its recommended > to set negative input (SEL_INM_SWC_3_0) of ADC step to ADC's VREFN in the > corresponding STEP_CONFIGx register. > Also, the positive(SEL_RFP_SWC_2_0) and negative(SEL_RFM_SWC_1_0) > reference voltage for ADC step needs to be set to VREFP and VREFN > respectively in STEP_CONFIGx register. > Without these changes, there may be variation of as much as ~2% in the > ADC's digital output which is bad for precise measurement. > > Signed-off-by: Vignesh R <vigneshr@ti.com> Given the header changes in mfd and the fact not much is going on with this driver in IIO at the moment, I'm happy for this to go through the mfd tree. Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Thanks, Jonathan > --- > drivers/iio/adc/ti_am335x_adc.c | 5 ++++- > include/linux/mfd/ti_am335x_tscadc.h | 4 ++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > index cafb1dcadc48..9d984f2a8ba7 100644 > --- a/drivers/iio/adc/ti_am335x_adc.c > +++ b/drivers/iio/adc/ti_am335x_adc.c > @@ -142,7 +142,10 @@ static void tiadc_step_config(struct iio_dev *indio_dev) > stepconfig |= STEPCONFIG_MODE_SWCNT; > > tiadc_writel(adc_dev, REG_STEPCONFIG(steps), > - stepconfig | STEPCONFIG_INP(chan)); > + stepconfig | STEPCONFIG_INP(chan) | > + STEPCONFIG_INM_ADCREFM | > + STEPCONFIG_RFP_VREFP | > + STEPCONFIG_RFM_VREFN); > > if (adc_dev->open_delay[i] > STEPDELAY_OPEN_MASK) { > dev_warn(dev, "chan %d open delay truncating to 0x3FFFF\n", > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h > index b9a53e013bff..483168403ae5 100644 > --- a/include/linux/mfd/ti_am335x_tscadc.h > +++ b/include/linux/mfd/ti_am335x_tscadc.h > @@ -78,6 +78,8 @@ > #define STEPCONFIG_YNN BIT(8) > #define STEPCONFIG_XNP BIT(9) > #define STEPCONFIG_YPN BIT(10) > +#define STEPCONFIG_RFP(val) ((val) << 12) > +#define STEPCONFIG_RFP_VREFP (0x3 << 12) > #define STEPCONFIG_INM_MASK (0xF << 15) > #define STEPCONFIG_INM(val) ((val) << 15) > #define STEPCONFIG_INM_ADCREFM STEPCONFIG_INM(8) > @@ -86,6 +88,8 @@ > #define STEPCONFIG_INP_AN4 STEPCONFIG_INP(4) > #define STEPCONFIG_INP_ADCREFM STEPCONFIG_INP(8) > #define STEPCONFIG_FIFO1 BIT(26) > +#define STEPCONFIG_RFM(val) ((val) << 23) > +#define STEPCONFIG_RFM_VREFN (0x3 << 23) > > /* Delay register */ > #define STEPDELAY_OPEN_MASK (0x3FFFF << 0) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement 2018-11-19 6:42 ` [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement Vignesh R 2018-11-25 13:46 ` Jonathan Cameron @ 2018-11-28 9:14 ` Lee Jones 2018-12-01 15:04 ` Jonathan Cameron 1 sibling, 1 reply; 11+ messages in thread From: Lee Jones @ 2018-11-28 9:14 UTC (permalink / raw) To: Vignesh R Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel On Mon, 19 Nov 2018, Vignesh R wrote: > When performing single ended measurements with TSCADC, its recommended > to set negative input (SEL_INM_SWC_3_0) of ADC step to ADC's VREFN in the > corresponding STEP_CONFIGx register. > Also, the positive(SEL_RFP_SWC_2_0) and negative(SEL_RFM_SWC_1_0) > reference voltage for ADC step needs to be set to VREFP and VREFN > respectively in STEP_CONFIGx register. > Without these changes, there may be variation of as much as ~2% in the > ADC's digital output which is bad for precise measurement. > > Signed-off-by: Vignesh R <vigneshr@ti.com> > --- > drivers/iio/adc/ti_am335x_adc.c | 5 ++++- > include/linux/mfd/ti_am335x_tscadc.h | 4 ++++ Acked-by: Lee Jones <lee.jones@linaro.org> -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement 2018-11-28 9:14 ` Lee Jones @ 2018-12-01 15:04 ` Jonathan Cameron 2018-12-03 7:01 ` Lee Jones 0 siblings, 1 reply; 11+ messages in thread From: Jonathan Cameron @ 2018-12-01 15:04 UTC (permalink / raw) To: Lee Jones Cc: Vignesh R, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel On Wed, 28 Nov 2018 09:14:32 +0000 Lee Jones <lee.jones@linaro.org> wrote: > On Mon, 19 Nov 2018, Vignesh R wrote: > > > When performing single ended measurements with TSCADC, its recommended > > to set negative input (SEL_INM_SWC_3_0) of ADC step to ADC's VREFN in the > > corresponding STEP_CONFIGx register. > > Also, the positive(SEL_RFP_SWC_2_0) and negative(SEL_RFM_SWC_1_0) > > reference voltage for ADC step needs to be set to VREFP and VREFN > > respectively in STEP_CONFIGx register. > > Without these changes, there may be variation of as much as ~2% in the > > ADC's digital output which is bad for precise measurement. > > > > Signed-off-by: Vignesh R <vigneshr@ti.com> > > --- > > drivers/iio/adc/ti_am335x_adc.c | 5 ++++- > > > include/linux/mfd/ti_am335x_tscadc.h | 4 ++++ > > Acked-by: Lee Jones <lee.jones@linaro.org> > I'll leave this for v2 given changes in the first patch. My assumption is at the moment that both will go through mfd. Shout Lee if you have other plans. Jonathan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement 2018-12-01 15:04 ` Jonathan Cameron @ 2018-12-03 7:01 ` Lee Jones 0 siblings, 0 replies; 11+ messages in thread From: Lee Jones @ 2018-12-03 7:01 UTC (permalink / raw) To: Jonathan Cameron Cc: Vignesh R, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, linux-omap, linux-kernel On Sat, 01 Dec 2018, Jonathan Cameron wrote: > On Wed, 28 Nov 2018 09:14:32 +0000 > Lee Jones <lee.jones@linaro.org> wrote: > > > On Mon, 19 Nov 2018, Vignesh R wrote: > > > > > When performing single ended measurements with TSCADC, its recommended > > > to set negative input (SEL_INM_SWC_3_0) of ADC step to ADC's VREFN in the > > > corresponding STEP_CONFIGx register. > > > Also, the positive(SEL_RFP_SWC_2_0) and negative(SEL_RFM_SWC_1_0) > > > reference voltage for ADC step needs to be set to VREFP and VREFN > > > respectively in STEP_CONFIGx register. > > > Without these changes, there may be variation of as much as ~2% in the > > > ADC's digital output which is bad for precise measurement. > > > > > > Signed-off-by: Vignesh R <vigneshr@ti.com> > > > --- > > > drivers/iio/adc/ti_am335x_adc.c | 5 ++++- > > > > > include/linux/mfd/ti_am335x_tscadc.h | 4 ++++ > > > > Acked-by: Lee Jones <lee.jones@linaro.org> > > > I'll leave this for v2 given changes in the first patch. > > My assumption is at the moment that both will go through mfd. > Shout Lee if you have other plans. I'm fine with that. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-03 7:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-19 6:42 [PATCH 0/2] tscadc: Couple of fixes Vignesh R 2018-11-19 6:42 ` [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells Vignesh R 2018-11-28 9:07 ` Lee Jones 2018-11-28 9:21 ` Vignesh R 2018-11-28 11:03 ` Lee Jones 2018-11-28 11:43 ` Vignesh R 2018-11-19 6:42 ` [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement Vignesh R 2018-11-25 13:46 ` Jonathan Cameron 2018-11-28 9:14 ` Lee Jones 2018-12-01 15:04 ` Jonathan Cameron 2018-12-03 7:01 ` Lee Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).