Re: [PATCH 5.10.y-cip 10/31] iio: adc: Add driver for Renesas RZ/G2L A/D converter

[Pavel Machek <pavel@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]

Hi!

> commit d484c21bacfa8bd2fa9fc26393ec59108f508c4c upstream.
> 
> Add ADC driver support for Renesas RZ/G2L A/D converter in SW
> trigger mode.
> 
> A/D Converter block is a successive approximation analog-to-digital
> converter with a 12-bit accuracy and supports a maximum of 8 input
> channels.
> 
> new file mode 100644
> index 000000000000..919108d798ba
> --- /dev/null
> +++ b/drivers/iio/adc/rzg2l_adc.c
...
> +
> +#define RZG2L_ADSMP_DEFUALT_SAMPLING	0x578
> +

This should be "DEFAULT".

> +	do {
> +		usleep_range(100, 200);
> +		reg = rzg2l_adc_readl(adc, RZG2L_ADM(0));
> +		timeout--;
> +		if (!timeout) {
> +			pr_err("%s stopping ADC timed out\n", __func__);
> +			break;
> +		}
> +	} while (((reg & RZG2L_ADM0_ADBSY) || (reg & RZG2L_ADM0_ADCE)));
> +}

I'd write this as (reg & (RZG2L_ADM0_ADBSY | RZG2L_ADM0_ADCE)). Note
that we wait, then check for timeout without using the register
values. Which is strange and basically makes timeout one tick
lower. (But probably does not matter much).

> +static void rzg2l_set_trigger(struct rzg2l_adc *adc)
> +{
> +	u32 reg;
> +
> +	/*
> +	 * Setup ADM1 for SW trigger
> +	 * EGA[13:12] - Set 00 to indicate hardware trigger is invalid
> +	 * BS[4] - Enable 1-buffer mode
> +	 * MS[1] - Enable Select mode
> +	 * TRG[0] - Enable software trigger mode
> +	 */
> +	reg = rzg2l_adc_readl(adc, RZG2L_ADM(1));
> +	reg &= ~RZG2L_ADM1_EGA_MASK;
> +	reg &= ~RZG2L_ADM1_BS;
> +	reg &= ~RZG2L_ADM1_TRG;

reg &= ~(RZG2L_ADM1_EGA_MASK | RZG2L_ADM1_BS | ...) would be usual way
to write this. You can use it in more than one place in the file.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]