Re: [PATCH v4 7/9] serial: sc16is7xx: fix regression with GPIO configuration

[Hugo Villeneuve]

On Tue, 30 May 2023 11:25:53 +0100
Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:

> On Mon, May 29, 2023 at 10:07:09AM -0400, Hugo Villeneuve 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.
> > 
> > As a consequence, applications that depends on GPIO lines configured
> > by default as GPIO pins no longer work as expected.
> > 
> > Also, the change to select modem control lines function was done only
> > for channel A of dual UART variants (752/762). This was not documented
> > in the log message.
> > 
> > Allow to specify GPIO or modem control line function in the device
> > tree, and for each of the ports (A or B).
> > 
> > Do so by using the new device-tree property named
> > "modem-control-line-ports" (property added in separate patch).
> > 
> > When registering GPIO chip controller, mask-out GPIO pins declared as
> > modem control lines according to this new "modem-control-line-ports"
> > DT property.
> > 
> > Boards that need to have GPIOS configured as modem control lines
> > should add that property to their device tree. Here is a list of
> > boards using the sc16is7xx driver in their device tree and that may
> > need to be modified:
> >     arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> >     mips/boot/dts/ingenic/cu1830-neo.dts
> >     mips/boot/dts/ingenic/cu1000-neo.dts
> > 
> > Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> 
> So you are marking this as a "bugfix" and yet, it is at the end of a
> much larger series of patches.  Does this fix require all of them?  If
> so, it's not really relevant for stable kernels, right?  Or is it?

Like I said to Andy, I will re-order the patches so that "bugfix" patches are first. See new order below.

> I'm confused, what should I, as a maintainer, do here?  Take just this
> one fix for 6.4-final, and the rest for 6.5-rc1?  And add a proper cc:
> stable@ tag?  Or queue them all up for 6.4-final?  Or all for 6.5-rc1?
> Or something else?
> 
> What would you want to see if you were in my position here to help make
> your life easier?

>From what I understand from https://www.kernel.org/doc/Documentation/process/stable-kernel-rules.rst,
here is the new proposed patches order as well as what I plan to do in the commit message:

2f0f23e598df serial: sc16is7xx: fix broken port 0 uart init
  I will add tag "Cc: <stable@xxxxxxxxxxxxxxx>"
  This patch is a prerequiste of "fix regression with GPIO configuration".

f292951c521e serial: sc16is7xx: mark IOCONTROL register as volatile
  I will add tag "Cc: <stable@xxxxxxxxxxxxxxx>"
  This patch is a prerequiste of "fix regression with GPIO configuration".
  This patch has no "Fixes:" tag because it doesn't fix a previous bug, but Lech Perczak reported that it was required for 
  patch "fix regression with GPIO configuration" to work.

78930d607121 serial: sc16is7xx: refactor GPIO controller registration
  This patch is a prerequiste of "fix regression with GPIO configuration".
  It was done separately to ease the review process, but from a stable kernel backport, maybe it would be best to integrate it directly into "fix regression with GPIO configuration"?
  If not, should I add tag "Cc: <stable@xxxxxxxxxxxxxxx>"?

f7ba105873d7 dt-bindings: sc16is7xx: Add property to change GPIO function
  This patch is a prerequiste of "fix regression with GPIO configuration".
  I will add tag "Cc: <stable@xxxxxxxxxxxxxxx>"
  Should I add a tag "Fixes: " like I did in patch "fix regression with GPIO configuration"?

f2238e8f69b0 serial: sc16is7xx: fix regression with GPIO configuration
  I will add tags:
    Cc: <stable@xxxxxxxxxxxxxxx> 2f0f23e5 serial: sc16is7xx: fix broken port 0 uart init
    Cc: <stable@xxxxxxxxxxxxxxx> f292951c serial: sc16is7xx: mark IOCONTROL register as volatile
    Cc: <stable@xxxxxxxxxxxxxxx> 78930d60 serial: sc16is7xx: refactor GPIO controller registration
    Cc: <stable@xxxxxxxxxxxxxxx> f7ba1058 dt-bindings: sc16is7xx: Add property to change GPIO function
    Cc: <stable@xxxxxxxxxxxxxxx>

2d98ab070b70 serial: sc16is7xx: fix bug when first setting GPIO direction
  This is a standalone bugfix
  I will add tag "Cc: <stable@xxxxxxxxxxxxxxx>"

658e39d9073e serial: sc16is7xx: add call to get rs485 DT flags and properties
  Enhancement

588aac544e00 serial: sc16is7xx: add post reset delay
  Enhancement

5bb1b45bca81 serial: sc16is7xx: improve comments about variants
  Comments enhancements

Please tell me if it makes sense and if some tags are wrong/missing.

Hugo.