From: Billy Tsai <hidden> Date: 2021-08-31 07:15:44
This patch serials make aspeed_adc.c can support ast2600 and backward
compatible.
Change since v4:
dt-bindings:
- Add clocks maxItems.
- Rename the property to meet the property-units.yaml.
- Add the description for the difference between adc0 and adc1.
aspeed_adc.c:
- Use new property name to get internal reference voltage: units from mv to
uv.
- Fix -Wnonnull warning caused by snprintf parameters.
- Add suffix mv to the vref parameters.
- Use ARRAY_SIZE instead of 32.
- Add a reset action for ADC power down and Use devm_iio_device_register.
- Fix typo error.
- Separate the offset interface of ch7 when battery sensing enable
Change since v3:
dt-bindings:
- Fix properties:aspeed,int_vref_mv type error.
Change since v2:
dt-bindings:
- Create a new dt-bindings for ast2600 adc
aspeed_adc.c:
- Splits the patch for more details
- Remove version enum and use the flags in model data to distinguish
hardware feature
- Support trimming data get and set.
- Use devm_add_action_or_reset to simplify probe error handling.
Changes since v1:
dt-bindings:
- Fix the aspeed,adc.yaml check error.
- Add battery-sensing property.
aspeed_adc.c:
- Change the init flow:
Clock and reference voltage setting should be completed before adc
engine enable.
- Change the default sampling rate to meet most user case.
- Add patch #8 to suppoert battery sensing mode.
Billy Tsai (15):
iio: adc: aspeed: set driver data when adc probe.
dt-bindings: iio: adc: Add ast2600-adc bindings
iio: adc: aspeed: completes the bitfield declare.
iio: adc: aspeed: Keep model data to driver data.
iio: adc: aspeed: Refactory model data structure
iio: adc: aspeed: Add vref config function
iio: adc: aspeed: Set num_channels with model data
iio: adc: aspeed: Use model_data to set clk scaler.
iio: adc: aspeed: Use devm_add_action_or_reset.
iio: adc: aspeed: Support ast2600 adc.
iio: adc: aspeed: Fix the calculate error of clock.
iio: adc: aspeed: Add func to set sampling rate.
iio: adc: aspeed: Add compensation phase.
iio: adc: aspeed: Support battery sensing.
iio: adc: aspeed: Get and set trimming data.
.../bindings/iio/adc/aspeed,ast2600-adc.yaml | 100 +++
drivers/iio/adc/aspeed_adc.c | 617 +++++++++++++++---
2 files changed, 619 insertions(+), 98 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml
--
2.25.1
From: Billy Tsai <hidden> Date: 2021-08-31 07:15:50
This patch refactors the model data structure to distinguish the
function form different versions of aspeed ADC.
- Rename the vref_voltage to vref_fixed_mv and add vref_mv driver data
When driver probe will check vref_fixed_mv value and store it to vref_mv
which isn't const value.
- Add num_channels
Make num_channles of iio device can be changed by different model_data
- Add need_prescaler flag and scaler_bit_width
The need_prescaler flag is used to tell the driver the clock divider needs
another Prescaler and the scaler_bit_width to set the clock divider
bitfield width.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-05 14:34:07
On Tue, 31 Aug 2021 15:14:48 +0800
Billy Tsai [off-list ref] wrote:
Title. Refactor (refactory isn't a word as far as I know though it perhaps should
be ;)
If you are rerolling the latter part of this series, merge patch 7 down into this one.
If not, it's fine as it stands. That one is trivial and a direct result of this one.
Jonathan
quoted hunk
This patch refactors the model data structure to distinguish the
function form different versions of aspeed ADC.
- Rename the vref_voltage to vref_fixed_mv and add vref_mv driver data
When driver probe will check vref_fixed_mv value and store it to vref_mv
which isn't const value.
- Add num_channels
Make num_channles of iio device can be changed by different model_data
- Add need_prescaler flag and scaler_bit_width
The need_prescaler flag is used to tell the driver the clock divider needs
another Prescaler and the scaler_bit_width to set the clock divider
bitfield width.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
From: Billy Tsai <hidden> Date: 2021-08-31 07:15:54
Add the function to check the vref_fixed_mv and set the value to driver
data.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
@@ -250,6 +261,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)}reset_control_deassert(data->rst);+ret=aspeed_adc_vref_config(indio_dev);+if(ret)+gotovref_config_error;+if(data->model_data->wait_init_sequence){/* Enable engine in normal mode. */writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
@@ -298,6 +313,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)clk_disable_unprepare(data->clk_scaler->clk);clk_enable_error:poll_timeout_error:+vref_config_error:reset_control_assert(data->rst);reset_error:clk_hw_unregister_divider(data->clk_scaler);
From: Billy Tsai <hidden> Date: 2021-08-31 07:15:56
This patch completes the declare of ADC register bitfields and uses the
same prefix ASPEED_ADC_* for these bitfields. In addition, tidy up space
alignment of the codes.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 64 ++++++++++++++++++++++++++----------
1 file changed, 47 insertions(+), 17 deletions(-)
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-05 14:27:39
On Tue, 31 Aug 2021 15:14:46 +0800
Billy Tsai [off-list ref] wrote:
This patch completes the declare of ADC register bitfields and uses the
same prefix ASPEED_ADC_* for these bitfields. In addition, tidy up space
alignment of the codes.
Signed-off-by: Billy Tsai <redacted>
LGTM
Applied to the togreg branch of iio.git.
Note this won't be going out as non rebasing for a while yet (given mid merge window)
so if anyone else has time to look at this that would be much appreciated!
Jonathan
@@ -0,0 +1,100 @@+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iio/adc/aspeed,ast2600-adc.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:ADC that forms part of an ASPEED server management processor.++maintainers:+-Billy Tsai <billy_tsai@aspeedtech.com>++description:|+?10-bits resolution for 16 voltage channels.+?The device split into two individual engine and each contains 8 voltage+channels.+?Channel scanning can be non-continuous.+?Programmable ADC clock frequency.+?Programmable upper and lower threshold for each channels.+?Interrupt when larger or less than threshold for each channels.+?Support hysteresis for each channels.+?Built-in a compensating method.+?Built-in a register to trim internal reference voltage.+?Internal or External reference voltage.+?Support 2 Internal reference voltage 1.2v or 2.5v.+?Integrate dividing circuit for battery sensing.++properties:+compatible:+enum:+-aspeed,ast2600-adc0+-aspeed,ast2600-adc1+description:+Their trimming data, which is used to calibrate internal reference volage,+locates in different address of OTP.++reg:+maxItems:1++clocks:+maxItems:1+description:+Input clock used to derive the sample clock. Expected to be the+SoC's APB clock.++resets:+maxItems:1++"#io-channel-cells":+const:1++vref-supply:+description:+The external regulator supply ADC reference voltage.++aspeed,int-vref-microvolt:+enum:[1200000,2500000]+description:+ADC internal reference voltage in microvolts.++aspeed,battery-sensing:+type:boolean+description:+Inform the driver that last channel will be used to sensor battery.++aspeed,trim-data-valid:+type:boolean+description:|+The ADC reference voltage can be calibrated to obtain the trimming+data which will be stored in otp. This property informs the driver that+the data store in the otp is valid.++required:+-compatible+-reg+-clocks+-resets+-"#io-channel-cells"++additionalProperties:false++examples:+-|+#include <dt-bindings/clock/ast2600-clock.h>+adc0:adc at 1e6e9000 {+compatible = "aspeed,ast2600-adc0";+reg = <0x1e6e9000 0x100>;+clocks = <&syscon ASPEED_CLK_APB2>;+resets = <&syscon ASPEED_RESET_ADC>;+#io-channel-cells = <1>;+aspeed,int-vref-microvolt = <2500000>;+};+adc1:adc at 1e6e9100 {+compatible = "aspeed,ast2600-adc1";+reg = <0x1e6e9100 0x100>;+clocks = <&syscon ASPEED_CLK_APB2>;+resets = <&syscon ASPEED_RESET_ADC>;+#io-channel-cells = <1>;+aspeed,int-vref-microvolt = <2500000>;+};+...
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-05 14:24:25
On Tue, 31 Aug 2021 20:38:36 -0500
Rob Herring [off-list ref] wrote:
On Tue, 31 Aug 2021 15:14:45 +0800, Billy Tsai wrote:
quoted
Add device tree bindings document for the aspeed ast2600 adc device
driver.
Signed-off-by: Billy Tsai <redacted>
---
.../bindings/iio/adc/aspeed,ast2600-adc.yaml | 100 ++++++++++++++++++
1 file changed, 100 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml
Reviewed-by: Rob Herring <robh@kernel.org>
I'm going to push on with this series until I hit anything that needs to wait
for the fix to be available. If that happens I'll have to wait until that's
in Linus' tree before taking the rest.
Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to work it's magic.
Thanks,
Jonathan
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:00
Keep the model data pointer to driver data for reducing the usage of
of_device_get_match_data().
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
@@ -118,8 +119,6 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);-conststructaspeed_adc_model_data*model_data=-of_device_get_match_data(data->dev);switch(mask){caseIIO_CHAN_INFO_RAW:
@@ -127,7 +126,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,returnIIO_VAL_INT;caseIIO_CHAN_INFO_SCALE:-*val=model_data->vref_voltage;+*val=data->model_data->vref_voltage;*val2=ASPEED_RESOLUTION_BITS;returnIIO_VAL_FRACTIONAL_LOG2;
@@ -146,13 +145,11 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,intval,intval2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);-conststructaspeed_adc_model_data*model_data=-of_device_get_match_data(data->dev);switch(mask){caseIIO_CHAN_INFO_SAMP_FREQ:-if(val<model_data->min_sampling_rate||-val>model_data->max_sampling_rate)+if(val<data->model_data->min_sampling_rate||+val>data->model_data->max_sampling_rate)return-EINVAL;clk_set_rate(data->clk_scaler->clk,
@@ -198,7 +195,6 @@ static int aspeed_adc_probe(struct platform_device *pdev){structiio_dev*indio_dev;structaspeed_adc_data*data;-conststructaspeed_adc_model_data*model_data;constchar*clk_parent_name;intret;u32adc_engine_control_reg_val;
@@ -209,6 +205,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)data=iio_priv(indio_dev);data->dev=&pdev->dev;+data->model_data=of_device_get_match_data(&pdev->dev);platform_set_drvdata(pdev,indio_dev);data->base=devm_platform_ioremap_resource(pdev,0);
@@ -249,9 +246,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)}reset_control_deassert(data->rst);-model_data=of_device_get_match_data(&pdev->dev);--if(model_data->wait_init_sequence){+if(data->model_data->wait_init_sequence){/* Enable engine in normal mode. */writel(FIELD_PREP(ASPEED_ADC_OP_MODE,ASPEED_ADC_OP_MODE_NORMAL)|
@@ -281,8 +276,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)writel(adc_engine_control_reg_val,data->base+ASPEED_REG_ENGINE_CONTROL);-model_data=of_device_get_match_data(&pdev->dev);-indio_dev->name=model_data->model_name;+indio_dev->name=data->model_data->model_name;indio_dev->info=&aspeed_adc_iio_info;indio_dev->modes=INDIO_DIRECT_MODE;indio_dev->channels=aspeed_adc_iio_channels;
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-05 14:30:25
On Tue, 31 Aug 2021 15:14:47 +0800
Billy Tsai [off-list ref] wrote:
Keep the model data pointer to driver data for reducing the usage of
of_device_get_match_data().
Signed-off-by: Billy Tsai <redacted>
This one starts to be impacted by the fix (as its in the context).
Rather than making a mess of things for linux-next etc I'll hold
off on these until that fix is upstream in a few weeks.
If I seem to have lost it (it's been known to happen :( ) then
feel free to poke me!
Thanks,
Jonathan
@@ -118,8 +119,6 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);-conststructaspeed_adc_model_data*model_data=-of_device_get_match_data(data->dev);switch(mask){caseIIO_CHAN_INFO_RAW:
@@ -127,7 +126,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,returnIIO_VAL_INT;caseIIO_CHAN_INFO_SCALE:-*val=model_data->vref_voltage;+*val=data->model_data->vref_voltage;*val2=ASPEED_RESOLUTION_BITS;returnIIO_VAL_FRACTIONAL_LOG2;
@@ -146,13 +145,11 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,intval,intval2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);-conststructaspeed_adc_model_data*model_data=-of_device_get_match_data(data->dev);switch(mask){caseIIO_CHAN_INFO_SAMP_FREQ:-if(val<model_data->min_sampling_rate||-val>model_data->max_sampling_rate)+if(val<data->model_data->min_sampling_rate||+val>data->model_data->max_sampling_rate)return-EINVAL;clk_set_rate(data->clk_scaler->clk,
@@ -198,7 +195,6 @@ static int aspeed_adc_probe(struct platform_device *pdev){structiio_dev*indio_dev;structaspeed_adc_data*data;-conststructaspeed_adc_model_data*model_data;constchar*clk_parent_name;intret;u32adc_engine_control_reg_val;
@@ -209,6 +205,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)data=iio_priv(indio_dev);data->dev=&pdev->dev;+data->model_data=of_device_get_match_data(&pdev->dev);platform_set_drvdata(pdev,indio_dev);data->base=devm_platform_ioremap_resource(pdev,0);
@@ -249,9 +246,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)}reset_control_deassert(data->rst);-model_data=of_device_get_match_data(&pdev->dev);--if(model_data->wait_init_sequence){+if(data->model_data->wait_init_sequence){/* Enable engine in normal mode. */writel(FIELD_PREP(ASPEED_ADC_OP_MODE,ASPEED_ADC_OP_MODE_NORMAL)|
@@ -281,8 +276,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)writel(adc_engine_control_reg_val,data->base+ASPEED_REG_ENGINE_CONTROL);-model_data=of_device_get_match_data(&pdev->dev);-indio_dev->name=model_data->model_name;+indio_dev->name=data->model_data->model_name;indio_dev->info=&aspeed_adc_iio_info;indio_dev->modes=INDIO_DIRECT_MODE;indio_dev->channels=aspeed_adc_iio_channels;
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-05 14:47:13
On Sun, 5 Sep 2021 15:33:39 +0100
Jonathan Cameron [off-list ref] wrote:
On Tue, 31 Aug 2021 15:14:47 +0800
Billy Tsai [off-list ref] wrote:
quoted
Keep the model data pointer to driver data for reducing the usage of
of_device_get_match_data().
Signed-off-by: Billy Tsai <redacted>
This one starts to be impacted by the fix (as its in the context).
Rather than making a mess of things for linux-next etc I'll hold
off on these until that fix is upstream in a few weeks.
If I seem to have lost it (it's been known to happen :( ) then
feel free to poke me!
Having taken another look at the rest of the series (and Philipp's review)
please do a v6 starting from this patch.
Thanks,
Jonathan
@@ -118,8 +119,6 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);-conststructaspeed_adc_model_data*model_data=-of_device_get_match_data(data->dev);switch(mask){caseIIO_CHAN_INFO_RAW:
@@ -127,7 +126,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,returnIIO_VAL_INT;caseIIO_CHAN_INFO_SCALE:-*val=model_data->vref_voltage;+*val=data->model_data->vref_voltage;*val2=ASPEED_RESOLUTION_BITS;returnIIO_VAL_FRACTIONAL_LOG2;
@@ -146,13 +145,11 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,intval,intval2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);-conststructaspeed_adc_model_data*model_data=-of_device_get_match_data(data->dev);switch(mask){caseIIO_CHAN_INFO_SAMP_FREQ:-if(val<model_data->min_sampling_rate||-val>model_data->max_sampling_rate)+if(val<data->model_data->min_sampling_rate||+val>data->model_data->max_sampling_rate)return-EINVAL;clk_set_rate(data->clk_scaler->clk,
@@ -198,7 +195,6 @@ static int aspeed_adc_probe(struct platform_device *pdev){structiio_dev*indio_dev;structaspeed_adc_data*data;-conststructaspeed_adc_model_data*model_data;constchar*clk_parent_name;intret;u32adc_engine_control_reg_val;
@@ -209,6 +205,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)data=iio_priv(indio_dev);data->dev=&pdev->dev;+data->model_data=of_device_get_match_data(&pdev->dev);platform_set_drvdata(pdev,indio_dev);data->base=devm_platform_ioremap_resource(pdev,0);
@@ -249,9 +246,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)}reset_control_deassert(data->rst);-model_data=of_device_get_match_data(&pdev->dev);--if(model_data->wait_init_sequence){+if(data->model_data->wait_init_sequence){/* Enable engine in normal mode. */writel(FIELD_PREP(ASPEED_ADC_OP_MODE,ASPEED_ADC_OP_MODE_NORMAL)|
@@ -281,8 +276,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)writel(adc_engine_control_reg_val,data->base+ASPEED_REG_ENGINE_CONTROL);-model_data=of_device_get_match_data(&pdev->dev);-indio_dev->name=model_data->model_name;+indio_dev->name=data->model_data->model_name;indio_dev->info=&aspeed_adc_iio_info;indio_dev->modes=INDIO_DIRECT_MODE;indio_dev->channels=aspeed_adc_iio_channels;
From: Joel Stanley <joel@jms.id.au> Date: 2021-09-16 03:52:39
On Sun, 5 Sept 2021 at 14:47, Jonathan Cameron [off-list ref] wrote:
On Sun, 5 Sep 2021 15:33:39 +0100
Jonathan Cameron [off-list ref] wrote:
quoted
On Tue, 31 Aug 2021 15:14:47 +0800
Billy Tsai [off-list ref] wrote:
quoted
Keep the model data pointer to driver data for reducing the usage of
of_device_get_match_data().
Signed-off-by: Billy Tsai <redacted>
This one starts to be impacted by the fix (as its in the context).
Rather than making a mess of things for linux-next etc I'll hold
off on these until that fix is upstream in a few weeks.
If I seem to have lost it (it's been known to happen :( ) then
feel free to poke me!
Having taken another look at the rest of the series (and Philipp's review)
please do a v6 starting from this patch.
I'd recommend against the practice of half applying a series. I have
just spent a good chunk of time looking at v6, and wondering why it
won't apply to any tags in Linus tree nor to next.
(It was made worse by the branch you applied them to not being part of
linux-next.)
Cheers,
Joel
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-18 17:51:57
On Thu, 16 Sep 2021 03:52:24 +0000
Joel Stanley [off-list ref] wrote:
On Sun, 5 Sept 2021 at 14:47, Jonathan Cameron [off-list ref] wrote:
quoted
On Sun, 5 Sep 2021 15:33:39 +0100
Jonathan Cameron [off-list ref] wrote:
quoted
On Tue, 31 Aug 2021 15:14:47 +0800
Billy Tsai [off-list ref] wrote:
quoted
Keep the model data pointer to driver data for reducing the usage of
of_device_get_match_data().
Signed-off-by: Billy Tsai <redacted>
This one starts to be impacted by the fix (as its in the context).
Rather than making a mess of things for linux-next etc I'll hold
off on these until that fix is upstream in a few weeks.
If I seem to have lost it (it's been known to happen :( ) then
feel free to poke me!
Having taken another look at the rest of the series (and Philipp's review)
please do a v6 starting from this patch.
I'd recommend against the practice of half applying a series. I have
just spent a good chunk of time looking at v6, and wondering why it
won't apply to any tags in Linus tree nor to next.
Hi Joel,
In this particular case it may been unwise, but in general it allows me to
handle a higher volume of patches than would otherwise be possible.
There are of course other approaches, but this one works well for me.
I did express to what tree and branch these were being applied
+ exposed for build tests.
(It was made worse by the branch you applied them to not being part of
linux-next.)
It will be shortly. That was just unfortunate timing because of the end of the
merge window and a some issues that 0-day found in other patches in the tree
that needed to be fixed up before making a mess in next.
Jonathan
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:02
Use the model_data member num_channels to set the num_channels of iio
device.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:05
This patch use devm_add_action_or_reset to handle the error in probe
phase.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 112 +++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 46 deletions(-)
@@ -242,6 +271,11 @@ static int aspeed_adc_probe(struct platform_device *pdev)&data->clk_lock);if(IS_ERR(data->clk_prescaler))returnPTR_ERR(data->clk_prescaler);+ret=devm_add_action_or_reset(data->dev,+aspeed_adc_unregister_divider,+data->clk_prescaler);+if(ret)+returnret;snprintf(clk_parent_name,ARRAY_SIZE(clk_parent_name),clk_name);scaler_flags=CLK_SET_RATE_PARENT;
@@ -256,23 +290,30 @@ static int aspeed_adc_probe(struct platform_device *pdev)&pdev->dev,clk_name,clk_parent_name,scaler_flags,data->base+ASPEED_REG_CLOCK_CONTROL,0,data->model_data->scaler_bit_width,0,&data->clk_lock);-if(IS_ERR(data->clk_scaler)){-ret=PTR_ERR(data->clk_scaler);-gotoscaler_error;-}+if(IS_ERR(data->clk_scaler))+returnPTR_ERR(data->clk_scaler);++ret=devm_add_action_or_reset(data->dev,aspeed_adc_unregister_divider,+data->clk_scaler);+if(ret)+returnret;data->rst=devm_reset_control_get_exclusive(&pdev->dev,NULL);if(IS_ERR(data->rst)){dev_err(&pdev->dev,"invalid or missing reset controller device tree entry");-ret=PTR_ERR(data->rst);-gotoreset_error;+returnPTR_ERR(data->rst);}reset_control_deassert(data->rst);+ret=devm_add_action_or_reset(data->dev,aspeed_adc_reset_assert,+data->rst);+if(ret)+returnret;+ret=aspeed_adc_vref_config(indio_dev);if(ret)-gotovref_config_error;+returnret;if(data->model_data->wait_init_sequence){/* Enable engine in normal mode. */
@@ -281,6 +322,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)ASPEED_ADC_ENGINE_ENABLE,data->base+ASPEED_REG_ENGINE_CONTROL);+ret=devm_add_action_or_reset(data->dev,aspeed_adc_power_down,+data);+if(ret)+returnret;/* Wait for initial sequence complete. */ret=readl_poll_timeout(data->base+ASPEED_REG_ENGINE_CONTROL,adc_engine_control_reg_val,
@@ -289,13 +334,19 @@ static int aspeed_adc_probe(struct platform_device *pdev)ASPEED_ADC_INIT_POLLING_TIME,ASPEED_ADC_INIT_TIMEOUT);if(ret)-gotopoll_timeout_error;+returnret;}/* Start all channels in normal mode. */ret=clk_prepare_enable(data->clk_scaler->clk);if(ret)-gotoclk_enable_error;+returnret;++ret=devm_add_action_or_reset(data->dev,+aspeed_adc_clk_disable_unprepare,+data->clk_scaler->clk);+if(ret)+returnret;adc_engine_control_reg_val=ASPEED_ADC_CTRL_CHANNEL|
@@ -303,6 +354,11 @@ static int aspeed_adc_probe(struct platform_device *pdev)ASPEED_ADC_ENGINE_ENABLE;writel(adc_engine_control_reg_val,data->base+ASPEED_REG_ENGINE_CONTROL);+ret=devm_add_action_or_reset(data->dev,+aspeed_adc_power_down,+data);+if(ret)+returnret;indio_dev->name=data->model_data->model_name;indio_dev->info=&aspeed_adc_iio_info;
@@ -310,45 +366,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)indio_dev->channels=aspeed_adc_iio_channels;indio_dev->num_channels=data->model_data->num_channels;-ret=iio_device_register(indio_dev);-if(ret)-gotoiio_register_error;--return0;--iio_register_error:-writel(FIELD_PREP(ASPEED_ADC_OP_MODE,ASPEED_ADC_OP_MODE_PWR_DOWN),-data->base+ASPEED_REG_ENGINE_CONTROL);-clk_disable_unprepare(data->clk_scaler->clk);-clk_enable_error:-poll_timeout_error:-vref_config_error:-reset_control_assert(data->rst);-reset_error:-clk_hw_unregister_divider(data->clk_scaler);-scaler_error:-if(data->model_data->need_prescaler)-clk_hw_unregister_divider(data->clk_prescaler);+ret=devm_iio_device_register(data->dev,indio_dev);returnret;}-staticintaspeed_adc_remove(structplatform_device*pdev)-{-structiio_dev*indio_dev=platform_get_drvdata(pdev);-structaspeed_adc_data*data=iio_priv(indio_dev);--iio_device_unregister(indio_dev);-writel(FIELD_PREP(ASPEED_ADC_OP_MODE,ASPEED_ADC_OP_MODE_PWR_DOWN),-data->base+ASPEED_REG_ENGINE_CONTROL);-clk_disable_unprepare(data->clk_scaler->clk);-reset_control_assert(data->rst);-clk_hw_unregister_divider(data->clk_scaler);-if(data->model_data->need_prescaler)-clk_hw_unregister_divider(data->clk_prescaler);--return0;-}-staticconststructaspeed_adc_model_dataast2400_model_data={.model_name="ast2400-adc",.vref_fixed_mv=2500,
From: Philipp Zabel <p.zabel@pengutronix.de> Date: 2021-08-31 07:51:52
Hi Billy,
On Tue, 2021-08-31 at 15:14 +0800, Billy Tsai wrote:
quoted hunk
This patch use devm_add_action_or_reset to handle the error in probe
phase.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 112 +++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 46 deletions(-)
@@ -256,23 +290,30 @@ static int aspeed_adc_probe(struct platform_device *pdev) &pdev->dev, clk_name, clk_parent_name, scaler_flags, data->base + ASPEED_REG_CLOCK_CONTROL, 0, data->model_data->scaler_bit_width, 0, &data->clk_lock);- if (IS_ERR(data->clk_scaler)) {- ret = PTR_ERR(data->clk_scaler);- goto scaler_error;- }+ if (IS_ERR(data->clk_scaler))+ return PTR_ERR(data->clk_scaler);++ ret = devm_add_action_or_reset(data->dev, aspeed_adc_unregister_divider,+ data->clk_scaler);+ if (ret)+ return ret;
Same as above.
[...]
quoted hunk
@@ -281,6 +322,10 @@ static int aspeed_adc_probe(struct platform_device *pdev) ASPEED_ADC_ENGINE_ENABLE, data->base + ASPEED_REG_ENGINE_CONTROL);+ ret = devm_add_action_or_reset(data->dev, aspeed_adc_power_down,+ data);+ if (ret)+ return ret; /* Wait for initial sequence complete. */ ret = readl_poll_timeout(data->base + ASPEED_REG_ENGINE_CONTROL, adc_engine_control_reg_val,
[...]
quoted hunk
@@ -303,6 +354,11 @@ static int aspeed_adc_probe(struct platform_device *pdev) ASPEED_ADC_ENGINE_ENABLE; writel(adc_engine_control_reg_val, data->base + ASPEED_REG_ENGINE_CONTROL);+ ret = devm_add_action_or_reset(data->dev,+ aspeed_adc_power_down,+ data);+ if (ret)+ return ret;
Should this be if (!model_dta->wait_init_sequence) ? Otherwise it looks
like you've already registered the same aspeed_adc_power_down action
above.
regards
Philipp
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:07
Make driver to support ast2600 adc device.
- Use shared reset controller
- Complete the vref configure function
- Add the model data for ast2600 adc
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 105 +++++++++++++++++++++++++++++++++--
1 file changed, 99 insertions(+), 6 deletions(-)
@@ -224,14 +226,79 @@ static void aspeed_adc_power_down(void *data)priv_data->base+ASPEED_REG_ENGINE_CONTROL);}+staticvoidaspeed_adc_reg_disable(void*data)+{+structregulator*reg=data;++regulator_disable(reg);+}+staticintaspeed_adc_vref_config(structiio_dev*indio_dev){structaspeed_adc_data*data=iio_priv(indio_dev);+intret;+u32adc_engine_control_reg_val;if(data->model_data->vref_fixed_mv){data->vref_mv=data->model_data->vref_fixed_mv;return0;}+adc_engine_control_reg_val=+readl(data->base+ASPEED_REG_ENGINE_CONTROL);+data->regulator=devm_regulator_get_optional(data->dev,"vref");+if(!IS_ERR(data->regulator)){+ret=regulator_enable(data->regulator);+if(ret)+returnret;+ret=devm_add_action_or_reset(+data->dev,aspeed_adc_reg_disable,data->regulator);+if(ret)+returnret;+data->vref_mv=regulator_get_voltage(data->regulator);+/* Conversion from uV to mV */+data->vref_mv/=1000;+if((data->vref_mv>=1550)&&(data->vref_mv<=2700))+writel(adc_engine_control_reg_val|+FIELD_PREP(+ASPEED_ADC_REF_VOLTAGE,+ASPEED_ADC_REF_VOLTAGE_EXT_HIGH),+data->base+ASPEED_REG_ENGINE_CONTROL);+elseif((data->vref_mv>=900)&&(data->vref_mv<=1650))+writel(adc_engine_control_reg_val|+FIELD_PREP(+ASPEED_ADC_REF_VOLTAGE,+ASPEED_ADC_REF_VOLTAGE_EXT_LOW),+data->base+ASPEED_REG_ENGINE_CONTROL);+else{+dev_err(data->dev,"Regulator voltage %d not support",+data->vref_mv);+return-EOPNOTSUPP;+}+}else{+if(PTR_ERR(data->regulator)!=-ENODEV)+returnPTR_ERR(data->regulator);+data->vref_mv=2500000;+of_property_read_u32(data->dev->of_node,+"aspeed,int-vref-microvolt",+&data->vref_mv);+/* Conversion from uV to mV */+data->vref_mv/=1000;+if(data->vref_mv==2500)+writel(adc_engine_control_reg_val|+FIELD_PREP(ASPEED_ADC_REF_VOLTAGE,+ASPEED_ADC_REF_VOLTAGE_2500mV),+data->base+ASPEED_REG_ENGINE_CONTROL);+elseif(data->vref_mv==1200)+writel(adc_engine_control_reg_val|+FIELD_PREP(ASPEED_ADC_REF_VOLTAGE,+ASPEED_ADC_REF_VOLTAGE_1200mV),+data->base+ASPEED_REG_ENGINE_CONTROL);+else{+dev_err(data->dev,"Voltage %d not support",data->vref_mv);+return-EOPNOTSUPP;+}+}+return0;}
@@ -298,7 +365,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)if(ret)returnret;-data->rst=devm_reset_control_get_exclusive(&pdev->dev,NULL);+data->rst=devm_reset_control_get_shared(&pdev->dev,NULL);if(IS_ERR(data->rst)){dev_err(&pdev->dev,"invalid or missing reset controller device tree entry");
@@ -316,10 +383,14 @@ static int aspeed_adc_probe(struct platform_device *pdev)returnret;if(data->model_data->wait_init_sequence){+adc_engine_control_reg_val=+readl(data->base+ASPEED_REG_ENGINE_CONTROL);+adc_engine_control_reg_val|=+FIELD_PREP(ASPEED_ADC_OP_MODE,+ASPEED_ADC_OP_MODE_NORMAL)|+ASPEED_ADC_ENGINE_ENABLE;/* Enable engine in normal mode. */-writel(FIELD_PREP(ASPEED_ADC_OP_MODE,-ASPEED_ADC_OP_MODE_NORMAL)|-ASPEED_ADC_ENGINE_ENABLE,+writel(adc_engine_control_reg_val,data->base+ASPEED_REG_ENGINE_CONTROL);ret=devm_add_action_or_reset(data->dev,aspeed_adc_power_down,
@@ -349,6 +420,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)returnret;adc_engine_control_reg_val=+readl(data->base+ASPEED_REG_ENGINE_CONTROL);+adc_engine_control_reg_val|=ASPEED_ADC_CTRL_CHANNEL|FIELD_PREP(ASPEED_ADC_OP_MODE,ASPEED_ADC_OP_MODE_NORMAL)|ASPEED_ADC_ENGINE_ENABLE;
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:09
This patch uses need_prescaler and scaler_bit_width to set the ADC clock
scaler.
Reported-by: kernel test robot <redacted>
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 43 ++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 16 deletions(-)
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:11
The ADC clock formula is
ast2400/2500:
ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0] + 1)
ast2600:
ADC clock period = PCLK * 2 * (ADC0C[15:0] + 1)
They all have one fixed divided 2 and the legacy driver didn't handle it.
This patch register the fixed factory clock device as the parent of ADC
clock scaler to fix this issue.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
From: Jonathan Cameron <jic23@kernel.org> Date: 2021-09-05 14:44:22
On Tue, 31 Aug 2021 15:14:54 +0800
Billy Tsai [off-list ref] wrote:
quoted hunk
The ADC clock formula is
ast2400/2500:
ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0] + 1)
ast2600:
ADC clock period = PCLK * 2 * (ADC0C[15:0] + 1)
They all have one fixed divided 2 and the legacy driver didn't handle it.
This patch register the fixed factory clock device as the parent of ADC
clock scaler to fix this issue.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
@@ -328,6 +342,19 @@ static int aspeed_adc_probe(struct platform_device *pdev)spin_lock_init(&data->clk_lock);snprintf(clk_parent_name,ARRAY_SIZE(clk_parent_name),"%s",of_clk_get_parent_name(pdev->dev.of_node,0));+snprintf(clk_name,ARRAY_SIZE(clk_name),"%s-fixed-div",+data->model_data->model_name);+data->fixed_div_clk=clk_hw_register_fixed_factor(+&pdev->dev,clk_name,clk_parent_name,0,1,2);
Obvious follow on from Philipp's review - there is a devm_ version of
this as well which you can use rather than rolling a custom version.
As a side note, I'm fairly sure you could refactor all those devm_clk_hw
functions to use devm_add_action_or_reset() internally and simplify that
code nicely.
A recent series did the same for all the similar functions in IIO.
+ if (IS_ERR(data->fixed_div_clk))
+ return PTR_ERR(data->fixed_div_clk);
+
+ ret = devm_add_action_or_reset(data->dev,
+ aspeed_adc_unregister_fixed_divider,
+ data->clk_prescaler);
+ if (ret)
+ return ret;
+ snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name), clk_name);
if (data->model_data->need_prescaler) {
snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-prescaler",
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:12
The ADC controller has a trimming register for fine-tune the reference
voltage. The trimming value comes from the OTP register which will be
written during chip production. This patch will read this OTP value and
configure it to the ADC register when the ADC controller probes and using
dts property "aspeed,trim-data-valid" to determine whether to execute this
flow.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 71 ++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:14
In ast2600, ADC integrate dividing circuit at last input channel for
battery sensing. This patch use the dts property "battery-sensing" to
enable this feature makes the last channel of each adc can tolerance
higher voltage than reference voltage. The offset interface of ch7 will
be separated when enabling the battery sensing mode.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 81 ++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 3 deletions(-)
@@ -204,14 +233,39 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structaspeed_adc_data*data=iio_priv(indio_dev);+u32adc_engine_control_reg_val;switch(mask){caseIIO_CHAN_INFO_RAW:-*val=readw(data->base+chan->address);+if(data->battery_sensing&&chan->channel==7){+adc_engine_control_reg_val=+readl(data->base+ASPEED_REG_ENGINE_CONTROL);+writel(adc_engine_control_reg_val|+FIELD_PREP(ASPEED_ADC_CH7_MODE,+ASPEED_ADC_CH7_BAT)|+ASPEED_ADC_BAT_SENSING_ENABLE,+data->base+ASPEED_REG_ENGINE_CONTROL);+/*+*Afterenablebatterysensingmodeneedtowaitsometimeforadcstable+*Experimentresultis1ms.+*/+mdelay(1);+*val=readw(data->base+chan->address);+*val=(*val*data->battery_mode_gain.mult)/+data->battery_mode_gain.div;+/* Restore control register value */+writel(adc_engine_control_reg_val,+data->base+ASPEED_REG_ENGINE_CONTROL);+}else+*val=readw(data->base+chan->address);returnIIO_VAL_INT;caseIIO_CHAN_INFO_OFFSET:-*val=data->cv;+if(data->battery_sensing&&chan->channel==7)+*val=(data->cv*data->battery_mode_gain.mult)/+data->battery_mode_gain.div;+else+*val=data->cv;returnIIO_VAL_INT;caseIIO_CHAN_INFO_SCALE:
@@ -491,6 +545,23 @@ static int aspeed_adc_probe(struct platform_device *pdev)if(ret)returnret;+if(of_find_property(data->dev->of_node,"aspeed,battery-sensing",+NULL)){+if(data->model_data->bat_sense_sup){+data->battery_sensing=1;+if(readl(data->base+ASPEED_REG_ENGINE_CONTROL)&+ASPEED_ADC_BAT_SENSING_DIV){+data->battery_mode_gain.mult=3;+data->battery_mode_gain.div=1;+}else{+data->battery_mode_gain.mult=3;+data->battery_mode_gain.div=2;+}+}else+dev_warn(&pdev->dev,+"Failed to enable battey-sensing mode\n");+}+if(data->model_data->wait_init_sequence){adc_engine_control_reg_val=readl(data->base+ASPEED_REG_ENGINE_CONTROL);
@@ -536,7 +607,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)indio_dev->name=data->model_data->model_name;indio_dev->info=&aspeed_adc_iio_info;indio_dev->modes=INDIO_DIRECT_MODE;-indio_dev->channels=aspeed_adc_iio_channels;+indio_dev->channels=data->battery_sensing?+aspeed_adc_iio_bat_channels:+aspeed_adc_iio_channels;indio_dev->num_channels=data->model_data->num_channels;ret=devm_iio_device_register(data->dev,indio_dev);
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:25
This patch adds a compensation phase to improve the accuracy of ADC
measurement. This is the built-in function through input half of the
reference voltage to get the ADC offset.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 54 +++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
@@ -163,6 +210,10 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,*val=readw(data->base+chan->address);returnIIO_VAL_INT;+caseIIO_CHAN_INFO_OFFSET:+*val=data->cv;+returnIIO_VAL_INT;+caseIIO_CHAN_INFO_SCALE:*val=data->vref_mv;*val2=ASPEED_RESOLUTION_BITS;
@@ -466,6 +517,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)returnret;}+aspeed_adc_compensation(indio_dev);/* Start all channels in normal mode. */adc_engine_control_reg_val=readl(data->base+ASPEED_REG_ENGINE_CONTROL);
From: Billy Tsai <hidden> Date: 2021-08-31 07:16:27
Add the function to set the sampling rate and keep the sampling period
for a driver used to wait the fresh value.
In addition, since the ADC clock is required when initializing the ADC
device, move clk_prepare_enable ahead of the initialization phase.
Signed-off-by: Billy Tsai <redacted>
---
drivers/iio/adc/aspeed_adc.c | 59 ++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 19 deletions(-)
@@ -127,6 +134,24 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {ASPEED_CHAN(15,0x2E),};+staticintaspeed_adc_set_sampling_rate(structiio_dev*indio_dev,u32rate)+{+structaspeed_adc_data*data=iio_priv(indio_dev);++if(rate<data->model_data->min_sampling_rate||+rate>data->model_data->max_sampling_rate)+return-EINVAL;+/* Each sampling needs 12 clocks to convert.*/+clk_set_rate(data->clk_scaler->clk,rate*ASPEED_CLOCKS_PER_SAMPLE);+rate=clk_get_rate(data->clk_scaler->clk);+data->sample_period_ns=DIV_ROUND_UP_ULL(+(u64)NSEC_PER_SEC*ASPEED_CLOCKS_PER_SAMPLE,rate);+dev_dbg(data->dev,"Adc clock = %d sample period = %d ns",rate,+data->sample_period_ns);++return0;+}+staticintaspeed_adc_read_raw(structiio_dev*indio_dev,structiio_chan_specconst*chan,int*val,int*val2,longmask)
@@ -157,17 +182,9 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,structiio_chan_specconst*chan,intval,intval2,longmask){-structaspeed_adc_data*data=iio_priv(indio_dev);-switch(mask){caseIIO_CHAN_INFO_SAMP_FREQ:-if(val<data->model_data->min_sampling_rate||-val>data->model_data->max_sampling_rate)-return-EINVAL;--clk_set_rate(data->clk_scaler->clk,-val*ASPEED_CLOCKS_PER_SAMPLE);-return0;+returnaspeed_adc_set_sampling_rate(indio_dev,val);caseIIO_CHAN_INFO_SCALE:caseIIO_CHAN_INFO_RAW:
@@ -409,6 +426,20 @@ static int aspeed_adc_probe(struct platform_device *pdev)if(ret)returnret;+ret=clk_prepare_enable(data->clk_scaler->clk);+if(ret)+returnret;++ret=devm_add_action_or_reset(data->dev,+aspeed_adc_clk_disable_unprepare,+data->clk_scaler->clk);+if(ret)+returnret;++ret=aspeed_adc_set_sampling_rate(indio_dev,ASPEED_ADC_DEF_SAMPLING_RATE);+if(ret)+returnret;+if(data->model_data->wait_init_sequence){adc_engine_control_reg_val=readl(data->base+ASPEED_REG_ENGINE_CONTROL);
@@ -436,16 +467,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)}/* Start all channels in normal mode. */-ret=clk_prepare_enable(data->clk_scaler->clk);-if(ret)-returnret;--ret=devm_add_action_or_reset(data->dev,-aspeed_adc_clk_disable_unprepare,-data->clk_scaler->clk);-if(ret)-returnret;-adc_engine_control_reg_val=readl(data->base+ASPEED_REG_ENGINE_CONTROL);adc_engine_control_reg_val|=