Received-SPF: pass (google.com: domain of linux-kernel+bounces-159619-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <2630ef8c8363b4fa772a2ff2c95cf115ad3c509d.camel@gmail.com>
Subject: Re: [PATCH 0/3] iio: cleanup masklength usage
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: David Lechner <dlechner@baylibre.com>, Jonathan Cameron
 <jic23@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>, Michael Hennerich
 <Michael.Hennerich@analog.com>, Shawn Guo <shawnguo@kernel.org>, Sascha
 Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team
 <kernel@pengutronix.de>,  Fabio Estevam <festevam@gmail.com>,
 linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, 
 imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org
Date: Fri, 26 Apr 2024 09:13:13 +0200
In-Reply-To: <20240425-b4-iio-masklength-cleanup-v1-0-d3d16318274d@baylibre.com>
References: 
	<20240425-b4-iio-masklength-cleanup-v1-0-d3d16318274d@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Thu, 2024-04-25 at 10:03 -0500, David Lechner wrote:
> While working on other patches I noticed that a few drivers are setting
> the masklength field of struct iio_dev even though it is marked as
> [INTERN]. It looks like maybe this was not always the case, but we can
> safely clean it up now without breaking anything.
>=20
> ---
> David Lechner (3):
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iio: adc: ad7266: don't set masklength
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iio: adc: mxs-lradc-adc: don't set masklen=
gth
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iio: buffer: initialize masklength accumul=
ator to 0
>=20
> =C2=A0drivers/iio/adc/ad7266.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0 | 1 -
> =C2=A0drivers/iio/adc/mxs-lradc-adc.c=C2=A0=C2=A0 | 1 -
> =C2=A0drivers/iio/industrialio-buffer.c | 2 +-
> =C2=A03 files changed, 1 insertion(+), 3 deletions(-)
> ---
> base-commit: b80ad8e3cd2712b78b98804d1f59199680d8ed91
> change-id: 20240425-b4-iio-masklength-cleanup-86b632b19901
>=20

Hi David,

Nice cleanup. The patches look good to me but there's one thing missing :).=
 As you
correctly noted, the field should be internal to the IIO core and drivers s=
hould not
touch it. Hence, you need to make sure not driver is using it so we can mov=
e it into
struct iio_dev_opaque [1]. That's the place all the intern fields should, e=
ventually,
end up.

Now, quite some drivers in the trigger handler will read the masklength for=
 looping
with for_each_set_bit(). Hence, the straight thing would be an helper to ge=
t it.
Maybe there's a clever way...

I know this is more work than what you had in mind but I think it should be=
 fairly
simple (hopefully) and since you started it :), maybe we can get the whole =
thing done
and remove another [INTERN] member from the iio_dev struct.

[1]: https://elixir.bootlin.com/linux/latest/source/include/linux/iio/iio-o=
paque.h#L42

- Nuno S=C3=A1