Re: [PATCH] gpio: use raw spinlock for gpio chip shadowed data

[Schspa Shi <schspa@xxxxxxxxx>]

Andy Shevchenko <andy.shevchenko@xxxxxxxxx> writes:

> On Mon, Apr 18, 2022 at 2:38 PM Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> wrote:
>> On Mon, Apr 18, 2022 at 6:07 AM Schspa Shi <schspa@xxxxxxxxx> wrote:
>> > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> writes:
>>
>> ...
>>
>> > >   drivers/gpio/gpio-mmio.c    | 22 +++++++++++-----------
>> > >   include/linux/gpio/driver.h |  2 +-
>> > >
>> > > You can’t do it for one driver only. As I told it will require too much of additional churn to make this to be series.
>> > >
>> >
>> > It seems I have misunderstood your "too much of additional churn". Can
>> > you explain it?
>> > The gpio-mmio.c and driver.h here are the basics of other gpio
>> > drivers. In my opinion, these two files
>> > belong to the basic code of gpio, and functions such as bgpio_init are
>> > declared in
>> > include/linux/gpio/driver.h and implemented in
>> > drivers/gpio/gpio-mmio.c. So there is no churn.
>>
>> When you change the member of the data structure, you have to change
>> all its users. You can't change only one at a time because it will be
>> a (compile-time) bisectability issue.


Yes, I understand and will take for bisectability use case for the next time.

>
> Answering your question here, it will require moving to union with an
> additional member and corresponding core changes, convert all drivers
> one-by-one, and remove the old type. It's not worth doing it, but as I
> said let maintainers decide.

Okay, sorry for my misunderstanding, I thought you were saying it's
bad to modify too many different files in one patch, so I split the
patch into a series of patchsets.

So, let Linus Walleij or Bartosz Golaszewski to decide for it ?
I have the same options as you, it's a small change, and no need to
trouble everyone for it.

Because this structure will be used as the same instance in multiple
files, even if we change this variable to union first, it can be
compiled, but the program will still not work properly. This is
because bgpio_init is initialized with the type of raw_spinlock_t,
but is still accessed as spinlock_t in other drivers, which is a
serious abnormal initialization.