Skip to contentSkip to conversation

tscadc: Couple of fixes

PATCH Superseded Vignesh R 3 participants 11 messages Nov 19, 2018 – Dec 3, 2018

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Vignesh R

author
#0 lore raw

cover tscadc: Couple of fixes

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(-)
Signature · 1 line
2.19.1
author
#1 lore raw

PATCH 1/2 mfd: ti_am335x_tscadc: Provide unique name for child mfd cells

Provide unique names for child mfd cells, this is required in order to support registering of multiple instances of same ti_am335x_tscadc IP.

---
 drivers/mfd/ti_am335x_tscadc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/mfd/ti_am335x_tscadc.c +4 -2
index c2d47d78705b..ee08af34f81d 100644
@@ -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);
Signature · 1 line
2.19.1
#2 lore raw

Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells

Quoted 2 lines · Vignesh R

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?

Quoted 29 lines
---
 drivers/mfd/ti_am335x_tscadc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/mfd/ti_am335x_tscadc.c +4 -2
index c2d47d78705b..ee08af34f81d 100644
@@ -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);
Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#3 lore raw

Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells

Hi,

Quoted 7 lines · Lee Jones
Quoted 2 lines · Vignesh R

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
Quoted 30 lines
Quoted 29 lines
---
 drivers/mfd/ti_am335x_tscadc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/mfd/ti_am335x_tscadc.c +4 -2
index c2d47d78705b..ee08af34f81d 100644
@@ -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);
Signature · 2 lines
Regards
Vignesh
#4 lore raw

Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells

Quoted 36 lines · Vignesh R

Hi,

Quoted 6 lines · Lee Jones
Quoted 2 lines · Vignesh R

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
Quoted 8 lines
[    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.

Quoted 31 lines
Quoted 30 lines
Quoted 29 lines
---
 drivers/mfd/ti_am335x_tscadc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/mfd/ti_am335x_tscadc.c +4 -2
index c2d47d78705b..ee08af34f81d 100644
@@ -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);
Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#5 lore raw

Re: [PATCH 1/2] mfd: ti_am335x_tscadc: Provide unique name for child mfd cells

Quoted 49 lines · Lee Jones
Quoted 36 lines · Vignesh R

Hi,

Quoted 6 lines · Lee Jones
Quoted 2 lines · Vignesh R

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
Quoted 8 lines
[    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:

Quoted 8 lines
        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

Quoted 32 lines
Quoted 31 lines
Quoted 30 lines
Quoted 29 lines
---
 drivers/mfd/ti_am335x_tscadc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/mfd/ti_am335x_tscadc.c +4 -2
index c2d47d78705b..ee08af34f81d 100644
@@ -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);
Signature · 2 lines
Regards
Vignesh
author
#6 lore raw

PATCH 2/2 iio: adc: ti_am335x_tscadc: Improve accuracy of measurement

In reply to Vignesh R

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.

---
 drivers/iio/adc/ti_am335x_adc.c      | 5 ++++-
 include/linux/mfd/ti_am335x_tscadc.h | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/iio/adc/ti_am335x_adc.c +4 -1
index cafb1dcadc48..9d984f2a8ba7 100644
@@ -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",
include/linux/mfd/ti_am335x_tscadc.h +4 -0
index b9a53e013bff..483168403ae5 100644
@@ -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)
Signature · 1 line
2.19.1
maintainer Acked-by on v1 2/2
#7 lore raw

Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement

Quoted 10 lines · On Mon, 19 Nov 2018 12:12:36 +0530 Vignesh R <[email protected]> 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.

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.

Thanks,

Jonathan

Quoted 43 lines
---
 drivers/iio/adc/ti_am335x_adc.c      | 5 ++++-
 include/linux/mfd/ti_am335x_tscadc.h | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/iio/adc/ti_am335x_adc.c +4 -1
index cafb1dcadc48..9d984f2a8ba7 100644
@@ -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",
include/linux/mfd/ti_am335x_tscadc.h +4 -0
index b9a53e013bff..483168403ae5 100644
@@ -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)
Acked-by on v1 2/2
#8 lore raw

Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement

In reply to Vignesh R
Quoted 9 lines · Vignesh R

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.

Quoted 5 lines
---
 drivers/iio/adc/ti_am335x_adc.c      | 5 ++++-
 include/linux/mfd/ti_am335x_tscadc.h | 4 ++++
Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#9 lore raw

Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement

Quoted 11 lines · On Wed, 28 Nov 2018 09:14:32 +0000 Lee Jones <[email protected]> wrote:
Quoted 9 lines · Vignesh R

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.

Quoted 8 lines
Quoted 5 lines
---
 drivers/iio/adc/ti_am335x_adc.c      | 5 ++++-  
 include/linux/mfd/ti_am335x_tscadc.h | 4 ++++  

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

#10 lore raw

Re: [PATCH 2/2] iio: adc: ti_am335x_tscadc: Improve accuracy of measurement

Quoted 18 lines · Jonathan Cameron
Quoted 15 lines · On Wed, 28 Nov 2018 09:14:32 +0000 Lee Jones <[email protected]> wrote:
Quoted 12 lines · Vignesh R

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.

---
 drivers/iio/adc/ti_am335x_adc.c      | 5 ++++-  
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
 include/linux/mfd/ti_am335x_tscadc.h | 4 ++++  

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.

Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog