Received-SPF: pass (google.com: domain of linux-kernel+bounces-197982-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 1 Jun 2024 19:40:27 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Nuno =?UTF-8?B?U8Oh?= <noname.nuno@gmail.com>
Cc: "Ceclan, Dumitru" <mitrutzceclan@gmail.com>, dumitru.ceclan@analog.com,
 Lars-Peter Clausen <lars@metafoo.de>, Michael Hennerich
 <Michael.Hennerich@analog.com>, Rob Herring <robh@kernel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, David Lechner <dlechner@baylibre.com>,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/6] iio: adc: ad7173: refactor ain and vref
 selection
Message-ID: <20240601194027.67c97d76@jic23-huawei>
In-Reply-To: <5e263d5ce90d6ad187adb7d4a007ce2e79095829.camel@gmail.com>
References: <20240527-ad4111-v3-0-7e9eddbbd3eb@analog.com>
	<20240527-ad4111-v3-3-7e9eddbbd3eb@analog.com>
	<71452f6882efe6a181d477914488617d28a38e2f.camel@gmail.com>
	<4b704b553282c0689dfef714c49ba97a33198898.camel@gmail.com>
	<d2f4e6da-75b1-4450-b295-45772dfb3f33@gmail.com>
	<5e263d5ce90d6ad187adb7d4a007ce2e79095829.camel@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Fri, 31 May 2024 09:10:43 +0200
Nuno S=C3=A1 <noname.nuno@gmail.com> wrote:

> On Thu, 2024-05-30 at 17:45 +0300, Ceclan, Dumitru wrote:
> > On 29/05/2024 15:49, Nuno S=C3=A1 wrote: =20
> > > On Wed, 2024-05-29 at 14:27 +0200, Nuno S=C3=A1 wrote: =20
> > > > On Mon, 2024-05-27 at 20:02 +0300, Dumitru Ceclan via B4 Relay wrot=
e: =20
> > > > > From: Dumitru Ceclan <dumitru.ceclan@analog.com> =20
> >=20
> > ...
> >  =20
> > > > > +static int ad7173_validate_voltage_ain_inputs(struct ad7173_stat=
e *st,
> > > > > +					=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 unsigned int ain[2]) =20
> > >=20
> > > Pass the pointer and size of it... Also, it should be made 'const'
> > >  =20
> >=20
> > I'm learning here: what is the purpose of passing the size of it?
> > This is a specific case where the size will always be 2
> >  =20
>=20
> Basically readability... Yes, in this case it will be a stretch to assume=
 we'll ever
> have anything bigger than 2 (so the scalability argument is not so applic=
able) so I'm
> ok if you don't pass the size. It's just I really dislike (as a practice)=
 to have
> raw/magic numbers in the code. In here, it won't be that bad as by the co=
ntext, one
> can easily understand the meaning of 2. Nevertheless, I would, still, at =
the very
> least consider to either use a #define or a better name for the iterator =
(anything
> more meaningful than 'i' so that it looks more understandable than 'i < 2=
')
>=20

I'm late to the game, but I'd just split it into two parameters.
Code is shorter as well.

static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
					      unsigned int ain0, unsigned int ain1)
{
	if (ain0 >=3D st->info->num_inputs ||
	    ain1 >=3D st->info->num_inputs)
		return dev_err_probe(&st->sd.spi->dev, -EINVAL,
				     "Input pin number out of range for pair (%d %d).\n",
				     ain0, ain1);
	return 0;
}

> - Nuno S=C3=A1=20
> >  =20
>=20