Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1620766020.git.lucas.p.stankus@gmail.com>
 <09e65d3a235febfc4c3ee172b573ba8c9cde94b8.1620766020.git.lucas.p.stankus@gmail.com>
 <CA+U=DsptfNDut3984MJkKckgWBhNd_0p17RfpidEXwYhMRfxtg@mail.gmail.com>
In-Reply-To: <CA+U=DsptfNDut3984MJkKckgWBhNd_0p17RfpidEXwYhMRfxtg@mail.gmail.com>
From:   Lucas Stankus <lucas.p.stankus@gmail.com>
Date:   Mon, 17 May 2021 21:55:20 -0300
Message-ID: <CACKVXZA9rPGLLxyq3gWTynoH7dPgYitqe-dO=YNad7VxHfwQhA@mail.gmail.com>
Subject: Re: [PATCH 2/2] staging: iio: cdc: ad7746: avoid overwrite of num_channels
To:     Alexandru Ardelean <ardeleanalex@gmail.com>
Cc:     Lars-Peter Clausen <lars@metafoo.de>,
        "Hennerich, Michael" <Michael.Hennerich@analog.com>,
        Jonathan Cameron <jic23@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-iio <linux-iio@vger.kernel.org>,
        linux-staging@lists.linux.dev, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, May 12, 2021 at 2:20 PM Alexandru Ardelean
<ardeleanalex@gmail.com> wrote:
>
> On Tue, May 11, 2021 at 11:55 PM Lucas Stankus
> <lucas.p.stankus@gmail.com> wrote:
> >
> > AD7745 devices don't have the CIN2 pins and therefore can't handle related
> > channels. Forcing the number of AD7746 channels may lead to enabling more
> > channels than what the hardware actually supports.
> > Avoid num_channels being overwritten after first assignment.
> >
> > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>
> > ---
> >  drivers/staging/iio/cdc/ad7746.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> > index e03d010b2f4c..9e0da43b2871 100644
> > --- a/drivers/staging/iio/cdc/ad7746.c
> > +++ b/drivers/staging/iio/cdc/ad7746.c
> > @@ -693,7 +693,6 @@ static int ad7746_probe(struct i2c_client *client,
> >                 indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
> >         else
> >                 indio_dev->num_channels =  ARRAY_SIZE(ad7746_channels) - 2;
> > -       indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
>
> ohh; good catch
>
> this falls into the category of a fix, so a Fixes tag is required;
> this looks so old, that i did not bother tracking it before
> 83e416f458d53  [which is 2011]

As Jonathan said, this bug was already fixed and the patch will be dropped,
but thank you for the review.

This was my first bug fix in the kernel, so sorry for the absence of a
Fixes tag, I'll make sure to add one next time.


>
> so, maybe something like:
>
> Fixes: 83e416f458d53 ("staging: iio: adc: Replace, rewrite ad7745 from
> scratch.")
>
> >         indio_dev->modes = INDIO_DIRECT_MODE;
> >
> >         if (pdata) {
> > --
> > 2.31.1
> >