Re: [V3] [TWL4030 MADC] Fix ADC[3:6] readings
Hello Jonathan, thanks for your comment.
Just submitted V4 with corrections:
1. regulator configuration is now done before iio_device_register
2. devm_regulator_put has been removed as it is not necessary
Let me know!
Adam
On Sun, Aug 2, 2015 at 9:55 AM, Jonathan Cameron wrote:
> On 21/07/15 01:49, Adam YH Lee wrote:
>> MADC[3:6] reads incorrect values without these two following changes:
>>
>> - enable the 3v1 bias regulator for ADC[3:6]
>> - configure ADC[3:6] lines as input, not as USB
>>
>> Signed-off-by: Adam YH Lee
>> ---
>> drivers/iio/adc/twl4030-madc.c | 36
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
>> index 94c5f05..ae33eba 100644
>> --- a/drivers/iio/adc/twl4030-madc.c
>> +++ b/drivers/iio/adc/twl4030-madc.c
>> @@ -45,13 +45,18 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> +#define TWL4030_USB_SEL_MADC_MCPC(1<<3)
>> +#define TWL4030_USB_CARKIT_ANA_CTRL 0xBB
>> +
>> /**
>> * struct twl4030_madc_data - a container for madc info
>> * @dev: Pointer to device structure for madc
>> * @lock:Mutex protecting this data structure
>> + * @regulator: Pointer to bias regulator for madc
>> * @requests:Array of request struct corresponding to SW1,
>> SW2 and RT
>> * @use_second_irq: IRQ selection (main or co-processor)
>> * @imr: Interrupt mask register of MADC
>> @@ -60,6 +65,7 @@
>> struct twl4030_madc_data {
>> struct device *dev;
>> struct mutex lock; /* mutex protecting this data structure */
>> + struct regulator *usb3v1;
>> struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
>> bool use_second_irq;
>> u8 imr;
>> @@ -848,6 +854,33 @@ static int twl4030_madc_probe(struct platform_device
>> *pdev)
>> goto err_i2c;
>> }
>>
> At this point in the driver the userspace (an in kernel) interfaces to read
> from the device are exposed. Thus if you need this stuff set up right, it
> wants to be done before the iio_device_register call.
>> + /* Configure MADC[3:6] */
>> + ret = twl_i2c_read_u8(TWL_MODULE_USB, ®val,
>> + TWL4030_USB_CARKIT_ANA_CTRL);
>> + if (ret) {
>> + dev_err(&pdev->dev, "unable to read reg CARKIT_ANA_CTRL
>> 0x%X\n",
>> + TWL4030_USB_CARKIT_ANA_CTRL);
>> + goto err_i2c;
>> + }
>> + regval |= TWL4030_USB_SEL_MADC_MCPC;
>> + ret = twl_i2c_write_u8(TWL_MODULE_USB, regval,
>> + TWL4030_USB_CARKIT_ANA_CTRL);
>> + if (ret) {
>> + dev_err(&pdev->dev, "unable to write reg CARKIT_ANA_CTRL
>> 0x%X\n",
>> + TWL4030_USB_CARKIT_ANA_CTRL);
>> + goto err_i2c;
>> + }
>> +
>> +
>> + /* Enable 3v1 bias regulator for MADC[3:6] */
>> + madc->usb3v1 = devm_regulator_get(madc->dev, "vusb3v1");
>> + if (IS_ERR(madc->usb3v1))
>> + return -ENODEV;
>> +
>> + ret = regulator_enable(madc->usb3v1);
>> + if (ret)
>> + dev_err(madc->dev, "could not enable 3v1 bias regulator\n");
>> +
>> return 0;
>>
>> err_i2c:
>> @@ -867,6 +900,9 @@ static int twl4030_madc_remove(struct platform_device
>> *pdev)
>> twl4030_madc_set_current_generator(madc, 0, 0);
>> twl4030_madc_set_power(madc, 0);
>>
>> + regulator_disable(madc->usb3v1);
>> + devm_regulator_put(madc->usb3v1);
> The whole point of devm_ allocators (or in this case _gets) is that when
> the driver is removed they are cleaned up automatically. Hence you don't
> need them in your remove call. Obviously it also clears up the error
> handling in probe as well.
>> +
>> return 0;
>> }
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V3] [TWL4030 MADC] Fix ADC[3:6] readings
On 21/07/15 01:49, Adam YH Lee wrote:
> MADC[3:6] reads incorrect values without these two following changes:
>
> - enable the 3v1 bias regulator for ADC[3:6]
> - configure ADC[3:6] lines as input, not as USB
>
> Signed-off-by: Adam YH Lee
> ---
> drivers/iio/adc/twl4030-madc.c | 36
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
> index 94c5f05..ae33eba 100644
> --- a/drivers/iio/adc/twl4030-madc.c
> +++ b/drivers/iio/adc/twl4030-madc.c
> @@ -45,13 +45,18 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> +#define TWL4030_USB_SEL_MADC_MCPC(1<<3)
> +#define TWL4030_USB_CARKIT_ANA_CTRL 0xBB
> +
> /**
> * struct twl4030_madc_data - a container for madc info
> * @dev: Pointer to device structure for madc
> * @lock:Mutex protecting this data structure
> + * @regulator: Pointer to bias regulator for madc
> * @requests:Array of request struct corresponding to SW1,
> SW2 and RT
> * @use_second_irq: IRQ selection (main or co-processor)
> * @imr: Interrupt mask register of MADC
> @@ -60,6 +65,7 @@
> struct twl4030_madc_data {
> struct device *dev;
> struct mutex lock; /* mutex protecting this data structure */
> + struct regulator *usb3v1;
> struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
> bool use_second_irq;
> u8 imr;
> @@ -848,6 +854,33 @@ static int twl4030_madc_probe(struct platform_device
> *pdev)
> goto err_i2c;
> }
>
At this point in the driver the userspace (an in kernel) interfaces to read
from the device are exposed. Thus if you need this stuff set up right, it
wants to be done before the iio_device_register call.
> + /* Configure MADC[3:6] */
> + ret = twl_i2c_read_u8(TWL_MODULE_USB, ®val,
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to read reg CARKIT_ANA_CTRL
> 0x%X\n",
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + goto err_i2c;
> + }
> + regval |= TWL4030_USB_SEL_MADC_MCPC;
> + ret = twl_i2c_write_u8(TWL_MODULE_USB, regval,
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to write reg CARKIT_ANA_CTRL
> 0x%X\n",
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + goto err_i2c;
> + }
> +
> +
> + /* Enable 3v1 bias regulator for MADC[3:6] */
> + madc->usb3v1 = devm_regulator_get(madc->dev, "vusb3v1");
> + if (IS_ERR(madc->usb3v1))
> + return -ENODEV;
> +
> + ret = regulator_enable(madc->usb3v1);
> + if (ret)
> + dev_err(madc->dev, "could not enable 3v1 bias regulator\n");
> +
> return 0;
>
> err_i2c:
> @@ -867,6 +900,9 @@ static int twl4030_madc_remove(struct platform_device
> *pdev)
> twl4030_madc_set_current_generator(madc, 0, 0);
> twl4030_madc_set_power(madc, 0);
>
> + regulator_disable(madc->usb3v1);
> + devm_regulator_put(madc->usb3v1);
The whole point of devm_ allocators (or in this case _gets) is that when
the driver is removed they are cleaned up automatically. Hence you don't
need them in your remove call. Obviously it also clears up the error
handling in probe as well.
> +
> return 0;
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V3] [TWL4030 MADC] Fix ADC[3:6] readings
Hello Peter,
Just sent up the V3 of my patch. Register configuration is now done in
madc driver code.
Another difference from V2 and V1 is that I am using
`devm_regulator_put` instead of
`regulator_put` to match the `devm_regulator_get` call.
I've tested it on Gumstix Overo (OMAP3 + TPS65950).
Let me know,
Adam
On Mon, Jul 20, 2015 at 5:49 PM, Adam YH Lee wrote:
> MADC[3:6] reads incorrect values without these two following changes:
>
> - enable the 3v1 bias regulator for ADC[3:6]
> - configure ADC[3:6] lines as input, not as USB
>
> Signed-off-by: Adam YH Lee
> ---
> drivers/iio/adc/twl4030-madc.c | 36
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
> index 94c5f05..ae33eba 100644
> --- a/drivers/iio/adc/twl4030-madc.c
> +++ b/drivers/iio/adc/twl4030-madc.c
> @@ -45,13 +45,18 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> +#define TWL4030_USB_SEL_MADC_MCPC (1<<3)
> +#define TWL4030_USB_CARKIT_ANA_CTRL0xBB
> +
> /**
> * struct twl4030_madc_data - a container for madc info
> * @dev: Pointer to device structure for madc
> * @lock: Mutex protecting this data structure
> + * @regulator: Pointer to bias regulator for madc
> * @requests: Array of request struct corresponding to SW1, SW2 and
> RT
> * @use_second_irq:IRQ selection (main or co-processor)
> * @imr: Interrupt mask register of MADC
> @@ -60,6 +65,7 @@
> struct twl4030_madc_data {
> struct device *dev;
> struct mutex lock; /* mutex protecting this data structure */
> + struct regulator *usb3v1;
> struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
> bool use_second_irq;
> u8 imr;
> @@ -848,6 +854,33 @@ static int twl4030_madc_probe(struct platform_device
> *pdev)
> goto err_i2c;
> }
>
> + /* Configure MADC[3:6] */
> + ret = twl_i2c_read_u8(TWL_MODULE_USB, ®val,
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to read reg CARKIT_ANA_CTRL
> 0x%X\n",
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + goto err_i2c;
> + }
> + regval |= TWL4030_USB_SEL_MADC_MCPC;
> + ret = twl_i2c_write_u8(TWL_MODULE_USB, regval,
> +TWL4030_USB_CARKIT_ANA_CTRL);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to write reg CARKIT_ANA_CTRL
> 0x%X\n",
> + TWL4030_USB_CARKIT_ANA_CTRL);
> + goto err_i2c;
> + }
> +
> +
> + /* Enable 3v1 bias regulator for MADC[3:6] */
> + madc->usb3v1 = devm_regulator_get(madc->dev, "vusb3v1");
> + if (IS_ERR(madc->usb3v1))
> + return -ENODEV;
> +
> + ret = regulator_enable(madc->usb3v1);
> + if (ret)
> + dev_err(madc->dev, "could not enable 3v1 bias regulator\n");
> +
> return 0;
>
> err_i2c:
> @@ -867,6 +900,9 @@ static int twl4030_madc_remove(struct platform_device
> *pdev)
> twl4030_madc_set_current_generator(madc, 0, 0);
> twl4030_madc_set_power(madc, 0);
>
> + regulator_disable(madc->usb3v1);
> + devm_regulator_put(madc->usb3v1);
> +
> return 0;
> }
>
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
