Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 24 Jun 2022 14:05:08 +0200
From:   Marcus Folkesson <marcus.folkesson@gmail.com>
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Kent Gustavsson <kent@minoris.se>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-iio <linux-iio@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 10/10] iio: adc: mcp3911: add support to set PGA
Message-ID: <YrWodHgbTJrUJ2lZ@gmail.com>
References: <20220623170844.2189814-1-marcus.folkesson@gmail.com>
 <20220623170844.2189814-10-marcus.folkesson@gmail.com>
 <CAHp75Vf7=Pk7T8ysrFS9u1sKQXUpqczGJkaeX-MLbwpx-iM2rg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="bKRj2/f7JE0UQ8BO"
Content-Disposition: inline
In-Reply-To: <CAHp75Vf7=Pk7T8ysrFS9u1sKQXUpqczGJkaeX-MLbwpx-iM2rg@mail.gmail.com>
Precedence: bulk


--bKRj2/f7JE0UQ8BO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 23, 2022 at 09:15:51PM +0200, Andy Shevchenko wrote:
> On Thu, Jun 23, 2022 at 7:41 PM Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> >
> > Add support for setting the Programmable Gain Amplifiers by adjust the
> > scale value.
>=20
> ...
>=20
> > +       int ret =3D mcp3911_read(adc, MCP3911_REG_GAIN, val, 1);
> > +
> > +       if (ret)
> > +               return ret;
>=20
> Please split the assignment.
>=20
>   int ret;
>=20
>   ret =3D ...
>   if (ret)

OK

>=20
>=20
> ...
>=20
> > +       *val >>=3D channel * 3;
> > +       *val &=3D 0x07;
>=20
> GENMASK() ?

I will use GENMASK
>=20
> > +       *val =3D (1 << *val);
>=20
> Unneeded parentheses, perhaps BIT()?

I will switch to BIT()
>=20
> ...
>=20
> > +                               ret =3D mcp3911_update(adc, MCP3911_REG=
_GAIN,
> > +                                               MCP3911_GAIN_MASK(chann=
el->channel),
> > +                                               MCP3911_GAIN_VAL(channe=
l->channel,
> > +                                                       i), 1);
>=20
> This is not good indentation, at least i), should be on the previous line.

Agree

>=20
> ...
>=20
> > +static int mcp3911_calc_scale_table(struct mcp3911 *adc)
> > +{
> > +       u32 ref =3D MCP3911_INT_VREF_MV;
> > +       u32 div;
>=20
> > +       int ret =3D 0;
>=20
> Useless assignment.

Agree
>=20
>=20
> Return directly.

OK
>=20
>=20
> The useless label.

Agree
>=20
> > +       return ret;
>=20


Thanks,
Marcus Folkesson

--bKRj2/f7JE0UQ8BO
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEBVGi6LZstU1kwSxliIBOb1ldUjIFAmK1qGAACgkQiIBOb1ld
UjKJNRAAge69rOmGEVokFO/oISGAzXS1ZqArxPISfbiEhZnC08gxknW2F8FoFVYd
KF0vimNI8D6ZojSGrJ/3Dn000yFgFf/nhjKWbEu/Wz1hGr5T7c25lw/darA7iyuZ
KhzfVorHKEaZxDd/xVVVS2IU8ebDLsUDDyrBfOyvS5ERj25sz/kw7jZXDjsY3QAd
wtIve7zqV58QRD+TYulZCb4Ms5mTdGHDWktPgeQiOnI7hFQGjVtUuWrcuQAKHffa
QhEqHSVIqXB+Y6pjQuk/SjBZod9zSb5rlSCjId9wzFrBJLkhl1ITqTxcAse9/eyF
6vwqLnfGblOzcFHdovPTSG5IjPgrzeevoftvbera4t+uOAnKSxrrSyftvXSESarE
r92dxeP/nxtJ7sPuwReIHgOHvRNaamlFN5CPgzc0SL6i4xoCryVJswh6/J1SXHmE
b15T1/X6a9L7eFPn0JwgOcr03dh010ZMZF39LYP+TfW1eDkYusd6ePNuyctMjT/1
MhjgZ4uXFEYfCApWJ+0UWyFDnIvU8/SSP+GxE2JQuEzZgnqyGGgZd6gd0oGyQtHJ
y5QsDtfTAhuOh+6wsI447gjnxD0DCzEp0kropz3eaIMGTfrXnF3t9A3HyRdut6/h
LgEyb+kYTGF39sT6FLeR37vvyZKvACpaOalFGKrbz+jmDVltq7k=
=1Q1R
-----END PGP SIGNATURE-----

--bKRj2/f7JE0UQ8BO--