From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: linux-aspeed@lists.ozlabs.org
Subject: [v2 5/8] iio: adc: aspeed: Add func to set sampling rate.
Date: Fri, 23 Jul 2021 16:37:17 +0100 [thread overview]
Message-ID: <20210723163717.000066fa@Huawei.com> (raw)
In-Reply-To: <20210723081621.29477-6-billy_tsai@aspeedtech.com>
On Fri, 23 Jul 2021 16:16:18 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> Add the function to set the sampling rate and keep the sampling period
> for a driver used to wait the lastest value.
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
A few follow on comments from suggestion in early patch, but otherwise this
looks good to me.
Jonathan
> ---
> drivers/iio/adc/aspeed_adc.c | 48 +++++++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 84f079195375..bb6100228cae 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -55,6 +55,12 @@
>
> #define ASPEED_ADC_INIT_POLLING_TIME 500
> #define ASPEED_ADC_INIT_TIMEOUT 500000
> +/*
> + * When the sampling rate is too high, the ADC may not have enough charging
> + * time, resulting in a low voltage value. Thus, default use slow sampling
> + * rate for most user case.
> + */
> +#define ASPEED_ADC_DEF_SAMPLING_RATE 65000
>
> enum aspeed_adc_version {
> aspeed_adc_ast2400,
> @@ -77,6 +83,7 @@ struct aspeed_adc_data {
> struct clk_hw *clk_scaler;
> struct reset_control *rst;
> int vref;
> + u32 sample_period_ns;
> };
>
> #define ASPEED_CHAN(_idx, _data_reg_addr) { \
> @@ -108,6 +115,26 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
> ASPEED_CHAN(15, 0x2E),
> };
>
> +static int aspeed_adc_set_sampling_rate(struct iio_dev *indio_dev, u32 rate)
> +{
> + struct aspeed_adc_data *data = iio_priv(indio_dev);
> + const struct aspeed_adc_model_data *model_data =
> + of_device_get_match_data(data->dev);
As in previous patch, this would be cleaner if there was a cached copy of
the model_data pointer in data rather than looking it up each time.
> +
> + if (rate < model_data->min_sampling_rate ||
> + rate > model_data->max_sampling_rate)
> + return -EINVAL;
> + /* Each sampling needs 12 clocks to covert.*/
> + 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);
blank line here.
> + return 0;
> +}
> +
> static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -138,19 +165,9 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> {
> - struct aspeed_adc_data *data = iio_priv(indio_dev);
> - const struct aspeed_adc_model_data *model_data =
> - of_device_get_match_data(data->dev);
> -
> switch (mask) {
> case IIO_CHAN_INFO_SAMP_FREQ:
> - if (val < model_data->min_sampling_rate ||
> - val > model_data->max_sampling_rate)
> - return -EINVAL;
> -
> - clk_set_rate(data->clk_scaler->clk,
> - val * ASPEED_CLOCKS_PER_SAMPLE);
> - return 0;
> + return aspeed_adc_set_sampling_rate(indio_dev, val);
>
> case IIO_CHAN_INFO_SCALE:
> case IIO_CHAN_INFO_RAW:
> @@ -302,6 +319,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> goto reset_error;
> }
> reset_control_deassert(data->rst);
> + ret = clk_prepare_enable(data->clk_scaler->clk);
> + if (ret)
> + goto clk_enable_error;
> + aspeed_adc_set_sampling_rate(indio_dev, ASPEED_ADC_DEF_SAMPLING_RATE);
>
> ret = aspeed_adc_vref_config(pdev);
> if (ret)
> @@ -327,9 +348,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> goto poll_timeout_error;
> }
>
> - ret = clk_prepare_enable(data->clk_scaler->clk);
> - if (ret)
> - goto clk_enable_error;
> adc_engine_control_reg_val =
> readl(data->base + ASPEED_REG_ENGINE_CONTROL);
> /* Start all channels in normal mode. */
> @@ -354,8 +372,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> iio_register_error:
> writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN,
> data->base + ASPEED_REG_ENGINE_CONTROL);
> - clk_disable_unprepare(data->clk_scaler->clk);
> vref_config_error:
> + clk_disable_unprepare(data->clk_scaler->clk);
> clk_enable_error:
> poll_timeout_error:
> reset_control_assert(data->rst);
WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Billy Tsai <billy_tsai@aspeedtech.com>
Cc: <jic23@kernel.org>, <lars@metafoo.de>, <pmeerw@pmeerw.net>,
<robh+dt@kernel.org>, <joel@jms.id.au>, <andrew@aj.id.au>,
<p.zabel@pengutronix.de>, <linux-iio@vger.kernel.org>,
<devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-aspeed@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>,
<BMC-SW@aspeedtech.com>
Subject: Re: [v2 5/8] iio: adc: aspeed: Add func to set sampling rate.
Date: Fri, 23 Jul 2021 16:37:17 +0100 [thread overview]
Message-ID: <20210723163717.000066fa@Huawei.com> (raw)
In-Reply-To: <20210723081621.29477-6-billy_tsai@aspeedtech.com>
On Fri, 23 Jul 2021 16:16:18 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> Add the function to set the sampling rate and keep the sampling period
> for a driver used to wait the lastest value.
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
A few follow on comments from suggestion in early patch, but otherwise this
looks good to me.
Jonathan
> ---
> drivers/iio/adc/aspeed_adc.c | 48 +++++++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 84f079195375..bb6100228cae 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -55,6 +55,12 @@
>
> #define ASPEED_ADC_INIT_POLLING_TIME 500
> #define ASPEED_ADC_INIT_TIMEOUT 500000
> +/*
> + * When the sampling rate is too high, the ADC may not have enough charging
> + * time, resulting in a low voltage value. Thus, default use slow sampling
> + * rate for most user case.
> + */
> +#define ASPEED_ADC_DEF_SAMPLING_RATE 65000
>
> enum aspeed_adc_version {
> aspeed_adc_ast2400,
> @@ -77,6 +83,7 @@ struct aspeed_adc_data {
> struct clk_hw *clk_scaler;
> struct reset_control *rst;
> int vref;
> + u32 sample_period_ns;
> };
>
> #define ASPEED_CHAN(_idx, _data_reg_addr) { \
> @@ -108,6 +115,26 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
> ASPEED_CHAN(15, 0x2E),
> };
>
> +static int aspeed_adc_set_sampling_rate(struct iio_dev *indio_dev, u32 rate)
> +{
> + struct aspeed_adc_data *data = iio_priv(indio_dev);
> + const struct aspeed_adc_model_data *model_data =
> + of_device_get_match_data(data->dev);
As in previous patch, this would be cleaner if there was a cached copy of
the model_data pointer in data rather than looking it up each time.
> +
> + if (rate < model_data->min_sampling_rate ||
> + rate > model_data->max_sampling_rate)
> + return -EINVAL;
> + /* Each sampling needs 12 clocks to covert.*/
> + 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);
blank line here.
> + return 0;
> +}
> +
> static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -138,19 +165,9 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> {
> - struct aspeed_adc_data *data = iio_priv(indio_dev);
> - const struct aspeed_adc_model_data *model_data =
> - of_device_get_match_data(data->dev);
> -
> switch (mask) {
> case IIO_CHAN_INFO_SAMP_FREQ:
> - if (val < model_data->min_sampling_rate ||
> - val > model_data->max_sampling_rate)
> - return -EINVAL;
> -
> - clk_set_rate(data->clk_scaler->clk,
> - val * ASPEED_CLOCKS_PER_SAMPLE);
> - return 0;
> + return aspeed_adc_set_sampling_rate(indio_dev, val);
>
> case IIO_CHAN_INFO_SCALE:
> case IIO_CHAN_INFO_RAW:
> @@ -302,6 +319,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> goto reset_error;
> }
> reset_control_deassert(data->rst);
> + ret = clk_prepare_enable(data->clk_scaler->clk);
> + if (ret)
> + goto clk_enable_error;
> + aspeed_adc_set_sampling_rate(indio_dev, ASPEED_ADC_DEF_SAMPLING_RATE);
>
> ret = aspeed_adc_vref_config(pdev);
> if (ret)
> @@ -327,9 +348,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> goto poll_timeout_error;
> }
>
> - ret = clk_prepare_enable(data->clk_scaler->clk);
> - if (ret)
> - goto clk_enable_error;
> adc_engine_control_reg_val =
> readl(data->base + ASPEED_REG_ENGINE_CONTROL);
> /* Start all channels in normal mode. */
> @@ -354,8 +372,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> iio_register_error:
> writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN,
> data->base + ASPEED_REG_ENGINE_CONTROL);
> - clk_disable_unprepare(data->clk_scaler->clk);
> vref_config_error:
> + clk_disable_unprepare(data->clk_scaler->clk);
> clk_enable_error:
> poll_timeout_error:
> reset_control_assert(data->rst);
WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Billy Tsai <billy_tsai@aspeedtech.com>
Cc: <jic23@kernel.org>, <lars@metafoo.de>, <pmeerw@pmeerw.net>,
<robh+dt@kernel.org>, <joel@jms.id.au>, <andrew@aj.id.au>,
<p.zabel@pengutronix.de>, <linux-iio@vger.kernel.org>,
<devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-aspeed@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>,
<BMC-SW@aspeedtech.com>
Subject: Re: [v2 5/8] iio: adc: aspeed: Add func to set sampling rate.
Date: Fri, 23 Jul 2021 16:37:17 +0100 [thread overview]
Message-ID: <20210723163717.000066fa@Huawei.com> (raw)
In-Reply-To: <20210723081621.29477-6-billy_tsai@aspeedtech.com>
On Fri, 23 Jul 2021 16:16:18 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> Add the function to set the sampling rate and keep the sampling period
> for a driver used to wait the lastest value.
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
A few follow on comments from suggestion in early patch, but otherwise this
looks good to me.
Jonathan
> ---
> drivers/iio/adc/aspeed_adc.c | 48 +++++++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 84f079195375..bb6100228cae 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -55,6 +55,12 @@
>
> #define ASPEED_ADC_INIT_POLLING_TIME 500
> #define ASPEED_ADC_INIT_TIMEOUT 500000
> +/*
> + * When the sampling rate is too high, the ADC may not have enough charging
> + * time, resulting in a low voltage value. Thus, default use slow sampling
> + * rate for most user case.
> + */
> +#define ASPEED_ADC_DEF_SAMPLING_RATE 65000
>
> enum aspeed_adc_version {
> aspeed_adc_ast2400,
> @@ -77,6 +83,7 @@ struct aspeed_adc_data {
> struct clk_hw *clk_scaler;
> struct reset_control *rst;
> int vref;
> + u32 sample_period_ns;
> };
>
> #define ASPEED_CHAN(_idx, _data_reg_addr) { \
> @@ -108,6 +115,26 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
> ASPEED_CHAN(15, 0x2E),
> };
>
> +static int aspeed_adc_set_sampling_rate(struct iio_dev *indio_dev, u32 rate)
> +{
> + struct aspeed_adc_data *data = iio_priv(indio_dev);
> + const struct aspeed_adc_model_data *model_data =
> + of_device_get_match_data(data->dev);
As in previous patch, this would be cleaner if there was a cached copy of
the model_data pointer in data rather than looking it up each time.
> +
> + if (rate < model_data->min_sampling_rate ||
> + rate > model_data->max_sampling_rate)
> + return -EINVAL;
> + /* Each sampling needs 12 clocks to covert.*/
> + 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);
blank line here.
> + return 0;
> +}
> +
> static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -138,19 +165,9 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> {
> - struct aspeed_adc_data *data = iio_priv(indio_dev);
> - const struct aspeed_adc_model_data *model_data =
> - of_device_get_match_data(data->dev);
> -
> switch (mask) {
> case IIO_CHAN_INFO_SAMP_FREQ:
> - if (val < model_data->min_sampling_rate ||
> - val > model_data->max_sampling_rate)
> - return -EINVAL;
> -
> - clk_set_rate(data->clk_scaler->clk,
> - val * ASPEED_CLOCKS_PER_SAMPLE);
> - return 0;
> + return aspeed_adc_set_sampling_rate(indio_dev, val);
>
> case IIO_CHAN_INFO_SCALE:
> case IIO_CHAN_INFO_RAW:
> @@ -302,6 +319,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> goto reset_error;
> }
> reset_control_deassert(data->rst);
> + ret = clk_prepare_enable(data->clk_scaler->clk);
> + if (ret)
> + goto clk_enable_error;
> + aspeed_adc_set_sampling_rate(indio_dev, ASPEED_ADC_DEF_SAMPLING_RATE);
>
> ret = aspeed_adc_vref_config(pdev);
> if (ret)
> @@ -327,9 +348,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> goto poll_timeout_error;
> }
>
> - ret = clk_prepare_enable(data->clk_scaler->clk);
> - if (ret)
> - goto clk_enable_error;
> adc_engine_control_reg_val =
> readl(data->base + ASPEED_REG_ENGINE_CONTROL);
> /* Start all channels in normal mode. */
> @@ -354,8 +372,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> iio_register_error:
> writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN,
> data->base + ASPEED_REG_ENGINE_CONTROL);
> - clk_disable_unprepare(data->clk_scaler->clk);
> vref_config_error:
> + clk_disable_unprepare(data->clk_scaler->clk);
> clk_enable_error:
> poll_timeout_error:
> reset_control_assert(data->rst);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-07-23 15:37 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-23 8:16 [v2 0/8] Add support for ast2600 ADC Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` [v2 1/8] dt-bindings: iio: adc: rename the aspeed adc yaml Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 14:44 ` Jonathan Cameron
2021-07-23 14:44 ` Jonathan Cameron
2021-07-23 14:44 ` Jonathan Cameron
2021-07-26 6:53 ` Billy Tsai
2021-07-26 6:53 ` Billy Tsai
2021-07-26 6:53 ` Billy Tsai
2021-07-29 20:31 ` Rob Herring
2021-07-29 20:31 ` Rob Herring
2021-07-29 20:31 ` Rob Herring
2021-07-31 17:27 ` Jonathan Cameron
2021-07-31 17:27 ` Jonathan Cameron
2021-07-31 17:27 ` Jonathan Cameron
2021-07-23 8:16 ` [v2 2/8] dt-bindings: iio: adc: Binding ast2600 adc Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 14:51 ` Jonathan Cameron
2021-07-23 14:51 ` Jonathan Cameron
2021-07-23 14:51 ` Jonathan Cameron
2021-07-26 7:21 ` Billy Tsai
2021-07-26 7:21 ` Billy Tsai
2021-07-26 7:21 ` Billy Tsai
2021-07-31 17:24 ` Jonathan Cameron
2021-07-23 8:16 ` [v2 3/8] iio: adc: aspeed: completes the bitfield declare Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 14:55 ` Jonathan Cameron
2021-07-23 14:55 ` Jonathan Cameron
2021-07-23 14:55 ` Jonathan Cameron
2021-07-23 8:16 ` [v2 4/8] iio: adc: aspeed: Allow driver to support ast2600 Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 15:29 ` Jonathan Cameron
2021-07-23 15:29 ` Jonathan Cameron
2021-07-23 15:29 ` Jonathan Cameron
2021-07-23 8:16 ` [v2 5/8] iio: adc: aspeed: Add func to set sampling rate Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 15:37 ` Jonathan Cameron [this message]
2021-07-23 15:37 ` Jonathan Cameron
2021-07-23 15:37 ` Jonathan Cameron
2021-07-23 8:16 ` [v2 6/8] iio: adc: aspeed: Add compensation phase Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 15:44 ` Jonathan Cameron
2021-07-23 15:44 ` Jonathan Cameron
2021-07-23 15:44 ` Jonathan Cameron
2021-07-23 8:16 ` [v2 7/8] iio: adc: aspeed: Fix the calculate error of clock Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` [v2 8/8] iio: adc: aspeed: Support battery sensing Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 8:16 ` Billy Tsai
2021-07-23 15:56 ` Jonathan Cameron
2021-07-23 15:56 ` Jonathan Cameron
2021-07-23 15:56 ` Jonathan Cameron
2021-07-26 6:54 ` Billy Tsai
2021-07-26 6:54 ` Billy Tsai
2021-07-26 6:54 ` Billy Tsai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210723163717.000066fa@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=linux-aspeed@lists.ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.