Re: [RESEND PATCH v8 06/10] serial: sc16is7xx: fix regression with GPIO configuration

[Rob Herring]

On Mon, Jul 24, 2023 at 9:54 AM Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:
>
> On Sat, 22 Jul 2023 17:15:26 +0200
> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> > On Sat, Jul 22, 2023 at 10:47:24AM -0400, Hugo Villeneuve wrote:
> > > On Fri, 21 Jul 2023 13:24:19 -0600
> > > Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> > >
> > > > On Fri, Jul 21, 2023 at 10:19 AM Hugo Villeneuve <hugo@xxxxxxxxxxx> wrote:
> > > > >
> > > > > From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> > > > >
> > > > > Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > > > > and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > > > > changed the function of the GPIOs pins to act as modem control
> > > > > lines without any possibility of selecting GPIO function.
> > > >
> > > > Requiring a new DT property is not fixing a kernel regression. You
> > > > should be returning the kernel to original behavior and then have a
> > > > new DT property for new behavior.
> > >
> > > Hi Rob,
> > > please read the entire patch history starting from V1
> > >  and you will understand why this course of action was
> > >  not selected.
> >
> > That's not going to happen, sorry, you need to explain it here, in this
> > patch series, why a specific action is being taken over another one, as
> > no one has time to go dig through past history, sorry.
>
> Hi Rob,
> I initially submitted a patch to revert the kernel to original
> behavior, but it created more problems because the patch was
> unfortunately split in two separate patches, and mixed with other non
> closely-related changes. It was also noted to me that reverting to the
> old behavior would break things for some users.
>
> It was suggested to me by a more experienced kernel developer to
> "suggest a fix, instead of hurrying a revert":
>
>     https://lkml.org/lkml/2023/5/17/758

Do I have to go read this to decipher the justification and reasoning?
When Greg says "in this patch series", he means in the commit messages
of the patches. You send v9 already and it doesn't have that. The
patchset needs to stand on its own summarizing any relevant prior
discussions.

I never suggested doing a revert. Obviously, someone still wants the
new feature. The issue is a new feature was added to the kernel, but
you are requiring a DT change to platforms NOT using the feature. Make
the platforms wanting the new feature to need a DT change. That's
still not great, but it's much better to affect new platforms rather
than old, stable platforms. The period of time that regresses is much
smaller (a few kernel releases vs. years potentially). Of course, if
it's just those 3 platforms and their maintainers are fine with
needing this DT change, then that works too. But there's no evidence
here that they are okay with it. You didn't even do the update of the
dts files and just left them broken.

Rob