All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading.
@ 2022-02-18  8:57 ` Billy Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Billy Tsai @ 2022-02-18  8:57 UTC (permalink / raw)
  To: jic23, lars, joel, andrew, billy_tsai, colin.king, linux-iio,
	linux-arm-kernel, linux-aspeed, linux-kernel
  Cc: Konstantin Klubnichkin

The formula for the ADC sampling period in ast2400/ast2500 is:
ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0])
When ADC0C[9:0] is set to 0 the sampling voltage will be lower than
expected, because the hardware may not have enough time to
charge/discharge to a stable voltage.

Reported-by: Konstantin Klubnichkin <kitsok@yandex-team.ru>
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index a957cad1bfab..ffae64f39221 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -539,7 +539,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	data->clk_scaler = devm_clk_hw_register_divider(
 		&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);
+		data->model_data->scaler_bit_width,
+		data->model_data->need_prescaler ? CLK_DIVIDER_ONE_BASED : 0,
+		&data->clk_lock);
 	if (IS_ERR(data->clk_scaler))
 		return PTR_ERR(data->clk_scaler);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading.
@ 2022-02-18  8:57 ` Billy Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Billy Tsai @ 2022-02-18  8:57 UTC (permalink / raw)
  To: jic23, lars, joel, andrew, billy_tsai, colin.king, linux-iio,
	linux-arm-kernel, linux-aspeed, linux-kernel
  Cc: Konstantin Klubnichkin

The formula for the ADC sampling period in ast2400/ast2500 is:
ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0])
When ADC0C[9:0] is set to 0 the sampling voltage will be lower than
expected, because the hardware may not have enough time to
charge/discharge to a stable voltage.

Reported-by: Konstantin Klubnichkin <kitsok@yandex-team.ru>
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index a957cad1bfab..ffae64f39221 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -539,7 +539,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	data->clk_scaler = devm_clk_hw_register_divider(
 		&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);
+		data->model_data->scaler_bit_width,
+		data->model_data->need_prescaler ? CLK_DIVIDER_ONE_BASED : 0,
+		&data->clk_lock);
 	if (IS_ERR(data->clk_scaler))
 		return PTR_ERR(data->clk_scaler);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading.
  2022-02-18  8:57 ` Billy Tsai
@ 2022-02-19 16:00   ` Jonathan Cameron
  -1 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-02-19 16:00 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, joel, andrew, colin.king, linux-iio, linux-arm-kernel,
	linux-aspeed, linux-kernel, Konstantin Klubnichkin

On Fri, 18 Feb 2022 16:57:08 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

> The formula for the ADC sampling period in ast2400/ast2500 is:
> ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0])
> When ADC0C[9:0] is set to 0 the sampling voltage will be lower than
> expected, because the hardware may not have enough time to
> charge/discharge to a stable voltage.
> 
> Reported-by: Konstantin Klubnichkin <kitsok@yandex-team.ru>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Hi Billy,

Fixes tag?

Also, would be good to call out in the patch description that
CLK_DIVIDER_ONE_BASED rules at 0 as a valid value and hence
avoids the ADC0C[9:0] value of 0 that is causing problems.

That may be obvious to people who make frequent use of clk dividers
but it's not locally obvious when looking at this patch.

Otherwise looks good to me.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/aspeed_adc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index a957cad1bfab..ffae64f39221 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -539,7 +539,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	data->clk_scaler = devm_clk_hw_register_divider(
>  		&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);
> +		data->model_data->scaler_bit_width,
> +		data->model_data->need_prescaler ? CLK_DIVIDER_ONE_BASED : 0,
> +		&data->clk_lock);
>  	if (IS_ERR(data->clk_scaler))
>  		return PTR_ERR(data->clk_scaler);
>  


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading.
@ 2022-02-19 16:00   ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-02-19 16:00 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, joel, andrew, colin.king, linux-iio, linux-arm-kernel,
	linux-aspeed, linux-kernel, Konstantin Klubnichkin

On Fri, 18 Feb 2022 16:57:08 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

> The formula for the ADC sampling period in ast2400/ast2500 is:
> ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0])
> When ADC0C[9:0] is set to 0 the sampling voltage will be lower than
> expected, because the hardware may not have enough time to
> charge/discharge to a stable voltage.
> 
> Reported-by: Konstantin Klubnichkin <kitsok@yandex-team.ru>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Hi Billy,

Fixes tag?

Also, would be good to call out in the patch description that
CLK_DIVIDER_ONE_BASED rules at 0 as a valid value and hence
avoids the ADC0C[9:0] value of 0 that is causing problems.

That may be obvious to people who make frequent use of clk dividers
but it's not locally obvious when looking at this patch.

Otherwise looks good to me.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/aspeed_adc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index a957cad1bfab..ffae64f39221 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -539,7 +539,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	data->clk_scaler = devm_clk_hw_register_divider(
>  		&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);
> +		data->model_data->scaler_bit_width,
> +		data->model_data->need_prescaler ? CLK_DIVIDER_ONE_BASED : 0,
> +		&data->clk_lock);
>  	if (IS_ERR(data->clk_scaler))
>  		return PTR_ERR(data->clk_scaler);
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading.
  2022-02-19 16:00   ` Jonathan Cameron
@ 2022-02-21  0:47     ` Billy Tsai
  -1 siblings, 0 replies; 6+ messages in thread
From: Billy Tsai @ 2022-02-21  0:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, joel, andrew, colin.king, linux-iio, linux-arm-kernel,
	linux-aspeed, linux-kernel, Konstantin Klubnichkin

Hi Jonathan,

On 2022/2/19, 11:53 PM, "Jonathan Cameron" <jic23@kernel.org> wrote:

    On Fri, 18 Feb 2022 16:57:08 +0800
    Billy Tsai <billy_tsai@aspeedtech.com> wrote:

    >   > The formula for the ADC sampling period in ast2400/ast2500 is:
    >   > ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0])
    >   > When ADC0C[9:0] is set to 0 the sampling voltage will be lower than
    >   > expected, because the hardware may not have enough time to
    >   > charge/discharge to a stable voltage.
    >   > 
    >   > Reported-by: Konstantin Klubnichkin <kitsok@yandex-team.ru>
    >   > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
    >   Hi Billy,
   
    >   Fixes tag?
   
    >   Also, would be good to call out in the patch description that
    >   CLK_DIVIDER_ONE_BASED rules at 0 as a valid value and hence
    >   avoids the ADC0C[9:0] value of 0 that is causing problems.
   
    >   That may be obvious to people who make frequent use of clk dividers
    >   but it's not locally obvious when looking at this patch.
   
    >   Otherwise looks good to me.
   
    >   Thanks,
   
    >   Jonathan

I will add fixes tag and add more description about the CLK_DIVIDER_ONE_BASED flag in v2.

Thanks,

Billy

    >   > ---
    >   >  drivers/iio/adc/aspeed_adc.c | 4 +++-
    >   >  1 file changed, 3 insertions(+), 1 deletion(-)
    >   > 
    >   > diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
    >   > index a957cad1bfab..ffae64f39221 100644
    >   > --- a/drivers/iio/adc/aspeed_adc.c
    >   > +++ b/drivers/iio/adc/aspeed_adc.c
    >   > @@ -539,7 +539,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
    >   >  	data->clk_scaler = devm_clk_hw_register_divider(
    >   >  		&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);
    >   > +		data->model_data->scaler_bit_width,
    >   > +		data->model_data->need_prescaler ? CLK_DIVIDER_ONE_BASED : 0,
    >   > +		&data->clk_lock);
    >   >  	if (IS_ERR(data->clk_scaler))
    >   >  		return PTR_ERR(data->clk_scaler);
    >   >  



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading.
@ 2022-02-21  0:47     ` Billy Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Billy Tsai @ 2022-02-21  0:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, joel, andrew, colin.king, linux-iio, linux-arm-kernel,
	linux-aspeed, linux-kernel, Konstantin Klubnichkin

Hi Jonathan,

On 2022/2/19, 11:53 PM, "Jonathan Cameron" <jic23@kernel.org> wrote:

    On Fri, 18 Feb 2022 16:57:08 +0800
    Billy Tsai <billy_tsai@aspeedtech.com> wrote:

    >   > The formula for the ADC sampling period in ast2400/ast2500 is:
    >   > ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0])
    >   > When ADC0C[9:0] is set to 0 the sampling voltage will be lower than
    >   > expected, because the hardware may not have enough time to
    >   > charge/discharge to a stable voltage.
    >   > 
    >   > Reported-by: Konstantin Klubnichkin <kitsok@yandex-team.ru>
    >   > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
    >   Hi Billy,
   
    >   Fixes tag?
   
    >   Also, would be good to call out in the patch description that
    >   CLK_DIVIDER_ONE_BASED rules at 0 as a valid value and hence
    >   avoids the ADC0C[9:0] value of 0 that is causing problems.
   
    >   That may be obvious to people who make frequent use of clk dividers
    >   but it's not locally obvious when looking at this patch.
   
    >   Otherwise looks good to me.
   
    >   Thanks,
   
    >   Jonathan

I will add fixes tag and add more description about the CLK_DIVIDER_ONE_BASED flag in v2.

Thanks,

Billy

    >   > ---
    >   >  drivers/iio/adc/aspeed_adc.c | 4 +++-
    >   >  1 file changed, 3 insertions(+), 1 deletion(-)
    >   > 
    >   > diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
    >   > index a957cad1bfab..ffae64f39221 100644
    >   > --- a/drivers/iio/adc/aspeed_adc.c
    >   > +++ b/drivers/iio/adc/aspeed_adc.c
    >   > @@ -539,7 +539,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
    >   >  	data->clk_scaler = devm_clk_hw_register_divider(
    >   >  		&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);
    >   > +		data->model_data->scaler_bit_width,
    >   > +		data->model_data->need_prescaler ? CLK_DIVIDER_ONE_BASED : 0,
    >   > +		&data->clk_lock);
    >   >  	if (IS_ERR(data->clk_scaler))
    >   >  		return PTR_ERR(data->clk_scaler);
    >   >  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-21  0:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-18  8:57 [PATCH] iio: adc: aspeed: Add divider flag to fix incorrect voltage reading Billy Tsai
2022-02-18  8:57 ` Billy Tsai
2022-02-19 16:00 ` Jonathan Cameron
2022-02-19 16:00   ` Jonathan Cameron
2022-02-21  0:47   ` Billy Tsai
2022-02-21  0:47     ` Billy Tsai

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.