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;
MIME-Version: 1.0
References: <20220718153448.173652-1-jason.gerecke@wacom.com>
 <20220803145937.698603-1-jason.gerecke@wacom.com> <CAHp75Vd6yEctJoNT6TpJ1+h4ZQckyLsaUSeSCV4MHqg+LUDkcg@mail.gmail.com>
In-Reply-To: <CAHp75Vd6yEctJoNT6TpJ1+h4ZQckyLsaUSeSCV4MHqg+LUDkcg@mail.gmail.com>
From:   Jason Gerecke <killertofu@gmail.com>
Date:   Wed, 3 Aug 2022 11:06:18 -0700
Message-ID: <CANRwn3TutF6skHQHk08dFUa8gLMVGxui_QN7YK6nDacSpRHtLg@mail.gmail.com>
Subject: Re: [PATCH v2] i2c: Use u8 type in i2c transfer calls
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     linux-i2c <linux-i2c@vger.kernel.org>,
        linux-iio <linux-iio@vger.kernel.org>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Wolfram Sang <wsa-dev@sang-engineering.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Ping Cheng <pinglinux@gmail.com>,
        "Tobita, Tatsunosuke" <tatsunosuke.tobita@wacom.com>,
        Jason Gerecke <jason.gerecke@wacom.com>,
        Ping Cheng <ping.cheng@wacom.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Aug 3, 2022 at 9:47 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Aug 3, 2022 at 4:59 PM Jason Gerecke <killertofu@gmail.com> wrote:
> >
> > The 'i2c_transfer_buffer_flags' function (and related inlines) defines its
>
> We refer to the functions like func() (without any quotes as well).
>
> > 'buf' argument to be of type 'char*'. This is a poor choice of type given
>
> char *
>
> > that most callers actually pass a 'u8*' and that the function itself ends
>
> most of the callers
>
> u8 *
>
> > up just storing the variable to a 'u8*'-typed member of 'struct i2c_msg'
>
> u8 *
>
> > anyway.
> >
> > Changing the type of the 'buf' argument to 'u8*' vastly reduces the number
>
> u8 *
>
> > of (admittedly usually-silent) Wpointer-sign warnings that are generated
>
> -Wpointer-sign or replace with simple English words.
>
> > as the types get needlessly juggled back and forth.
> >
> > At the same time, update the max1363 driver to match the new interface so
> > we don't introduce a new Wincompatible-function-pointer-types warning.
>
> -Wincompatible-function-pointer-types
>
> ...
>
Ack to all.

> > Changes in v2:
> >   - Added modifications to the max1363 driver required to avoid warnings
>
> Have you really checked _all_ callers of APIs that you have changed here?
>
> For example, drivers/media/usb/em28xx/em28xx-input.c still uses
> unsigned char for i2c_master_recv().
>

This particular example shouldn't result in a new warning since
unsigned char and u8 are equivalent types, and u8 is used by the new
API.

Assuming you're referring to callers that are still using *signed*
variables with this API, however, I intentionally ignored them. IIRC,
there were about 400 files using unsigned and about 60 files using
signed. Those 60 files will now generate their own pointer-sign
warnings, but I rationalized it as a still-substantial improvement
over the current state of things.

As for normally-silent warnings *other* than Wpointer-sign (e.g. the
Wincompatible-pointer-types in max1363), I also did not explicitly
check for those. It is possible other warnings may be out there.

> I believe you need to create a coccinelle script and run it over the
> kernel source tree and then create a patch out of it.
>
> --
> With Best Regards,
> Andy Shevchenko

This would definitely be necessary to unify all callers to using
unsigned variables rather than just swapping which callers generate
the pointer-sign warnings.

Jason