-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 30 December 2021 11:01
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Cc: cip-dev@...; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek
<pavel@...>; Biju Das <biju.das.jz@...>
Subject: Re: [PATCH 5.10.y-cip 10/31] iio: adc: Add driver for Renesas RZ/G2L A/D converter

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.