[RESEND v4 12/15] iio: adc: aspeed: Add func to set sampling rate.

Billy Tsai billy_tsai at aspeedtech.com
Mon Aug 30 18:35:53 AEST 2021


Hi Jonathan,

On 2021/8/29, 11:33 PM, "Jonathan Cameron" <jic23 at kernel.org> wrote:

    On Tue, 24 Aug 2021 17:12:40 +0800
    Billy Tsai <billy_tsai at 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 at aspeedtech.com>

    > Why move the code as well as factoring out the setter function?
    > I doubt it does any harm, but I'd like to understand why you did it.

    > Jonathan

    >> +	ret = clk_prepare_enable(data->clk_scaler->clk);
    >> +	if (ret)
    >> +		return ret;
    >> +
    >> +	ret = devm_add_action_or_reset(data->dev,
    >> +				       aspeed_adc_clk_disable_unprepare,
    >> +				       data->clk_scaler->clk);
    >> +	if (ret)
    >> +		return ret;
    >> +
    >> +	ret = aspeed_adc_set_sampling_rate(indio_dev, ASPEED_ADC_DEF_SAMPLING_RATE);
    >> +	if (ret)
    >> +		return ret;
    >> +
    >>  	ret = aspeed_adc_vref_config(indio_dev);
    >>  	if (ret)
    >>  		return ret;
    >> @@ -413,16 +445,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
    >>  	}
    >>  
    >>  	/* Start all channels in normal mode. */

    > Why move this code up?

Because the ADC clock is required when initializing the ADC device.
In our system, the clock is always on. Thus, the legacy driver won't encounter any issues.
I move the clk_prepare_enable ahead of initializing phase for making the driver probe logically closer to the hardware. 

    >> -	ret = clk_prepare_enable(data->clk_scaler->clk);
    >> -	if (ret)
    >> -		return ret;
    >> -
    >> -	ret = devm_add_action_or_reset(data->dev,
    >> -				       aspeed_adc_clk_disable_unprepare,
    >> -				       data->clk_scaler->clk);
    >> -	if (ret)
    >> -		return ret;
    >> -
    >>  	adc_engine_control_reg_val =
    >>  		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
    >>  	adc_engine_control_reg_val |=


Best Regards,
Billy Tsai



More information about the Linux-aspeed mailing list