From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 081B0BD; Thu, 14 Dec 2023 02:14:19 -0800 (PST) Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4SrSpz4b1nz6J9fR; Thu, 14 Dec 2023 18:13:15 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id C39D61400CA; Thu, 14 Dec 2023 18:14:17 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 14 Dec 2023 10:14:17 +0000 Date: Thu, 14 Dec 2023 10:14:15 +0000 From: Jonathan Cameron To: David Lechner CC: , , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Jonathan Cameron , Michael Hennerich , Nuno =?ISO-8859-1?Q?S=E1?= , Liam Girdwood , Mark Brown , , Stefan Popa Subject: Re: [PATCH v2 3/3] iio: adc: ad7380: new driver for AD7380 ADCs Message-ID: <20231214101415.0000060c@Huawei.com> In-Reply-To: <20231213-ad7380-mainline-v2-3-cd32150d84a3@baylibre.com> References: <20231213-ad7380-mainline-v2-0-cd32150d84a3@baylibre.com> <20231213-ad7380-mainline-v2-3-cd32150d84a3@baylibre.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: lhrpeml100003.china.huawei.com (7.191.160.210) To lhrpeml500005.china.huawei.com (7.191.163.240) On Wed, 13 Dec 2023 05:21:20 -0600 David Lechner wrote: > This adds a new driver for the AD7380 family ADCs. > > The driver currently implements basic support for the AD7380, AD7381, > AD7383, and AD7384 2-channel differential ADCs. Support for additional > single-ended and 4-channel chips that use the same register map as well > as additional features of the chip will be added in future patches. > > Co-developed-by: Stefan Popa > Signed-off-by: Stefan Popa > Signed-off-by: David Lechner Just one additional comment. I 'might' sort both this an Nuno's comment if Mark is fine with the SPI and no on else has review comments. Feel free to send a v3 though if you like ;) > +/* fully differential */ > +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7380_channels, 16); > +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7381_channels, 14); > +/* pseudo differential */ > +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7383_channels, 16); > +DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL(ad7384_channels, 14); > + > +/* Since this is simultaneous sampling, we don't allow individual channels. */ > +static const unsigned long ad7380_2_channel_scan_masks[] = { > + GENMASK(2, 0), /* both ADC channels and soft timestamp */ > + GENMASK(1, 0), /* both ADC channels, no timestamp */ https://elixir.bootlin.com/linux/v6.7-rc5/source/include/linux/iio/iio.h#L567 See the comment (added recently!) Also, if I remember how this works correctly there is no need to include the timestamp in the mask. We do special handling for it to avoid having to double the number of provided masks. The details being that it uses iio_scan_el_ts_store rather than iio_scan_el_Store. So as you have it I think you'll always end up with the first entry and that will include a bonus bit that isn't a problem as it will match anyway. So just have the second entry and 0. Jonathan > + 0 > +};