Thread (23 messages) 23 messages, 3 authors, 2023-08-07

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

From: Hugo Villeneuve <hidden>
Date: 2023-08-03 21:39:10
Also in: linux-gpio, linux-serial, lkml, stable

On Thu, 3 Aug 2023 14:10:30 -0600
Rob Herring [off-list ref] wrote:
On Thu, Aug 3, 2023 at 11:54 AM Hugo Villeneuve [off-list ref] wrote:
quoted
On Mon, 31 Jul 2023 14:41:15 -0400
Hugo Villeneuve [off-list ref] wrote:
quoted
On Mon, 31 Jul 2023 12:04:45 -0600
Rob Herring [off-list ref] wrote:
quoted
On Mon, Jul 31, 2023 at 10:46 AM Hugo Villeneuve [off-list ref] wrote:
quoted
On Mon, 31 Jul 2023 09:31:53 -0600
Rob Herring [off-list ref] wrote:
quoted
On Mon, Jul 24, 2023 at 9:54 AM Hugo Villeneuve [off-list ref] wrote:
quoted
On Sat, 22 Jul 2023 17:15:26 +0200
Greg KH [off-list ref] wrote:
quoted
On Sat, Jul 22, 2023 at 10:47:24AM -0400, Hugo Villeneuve wrote:
quoted
On Fri, 21 Jul 2023 13:24:19 -0600
Rob Herring [off-list ref] wrote:
quoted
On Fri, Jul 21, 2023 at 10:19 AM Hugo Villeneuve [off-list ref] wrote:
quoted
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

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.
Hi Rob,
I am sorry, but this is exactly what I "deciphered" from your
original email.

I am trying very hard to understand exactly what you mean, but it is
not that obvious for me. If something is not clear in my commit message,
I will try to improve it. But before, let's try to focus on making sure
I understand more clearly what you want exactly.
quoted
Obviously, someone still wants the
new feature.
I assume that you refer to the "new feature" as what was added in
the commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control
lines")?
Shrug. It's one of the 2 commits mentioned, I don't know which one
exactly. Whichever one changed default behavior from use GPIOs to use
modem ctrl lines.

Reading it again, I *think* this patch is correct. Default behavior is
restored to use GPIOs. The DT property is needed to enable modem ctrl
lines.
Hi,
this is correct.

quoted
What's not okay is just saying, these platforms may or may not need an update:

    arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
    mips/boot/dts/ingenic/cu1830-neo.dts
    mips/boot/dts/ingenic/cu1000-neo.dts
Yes, my bad. I initially mentioned them and hoped to get some
feedback, which I never got, and I kind of forgot about it.
quoted
You need to figure that out. Have you checked with maintainers of
these boards? When were they added and by who? At the same time or by
the same person would be a good indication the platform uses modem
ctrl lines. Or were these platforms in use before adding modem ctrl
support? Then they probably use GPIOs or nothing.

If there are platforms which would regress if the modem ctrl feature
was just reverted, which ones are those?
Ok, let me do some checks and get back to you on this.
Hi Rob,
for this board:
    arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts

it uses a SC16IS740, which doesn't have any GPIOs nor modem
control lines, so no DT changes required.

For these two Ingenic boards:
    mips/boot/dts/ingenic/cu1830-neo.dts
    mips/boot/dts/ingenic/cu1000-neo.dts

They use a SC16IS752, which has shared modem control lines and GPIOs.
Unfortunately, the maintainers have not (yet) responded to my
inquiries. Also, I tried to search for schematics or block diagrams on
the net but couldn't find anything.

These platforms were in use before the patch to add the modem control
lines was added. Then like you said they probably use these shared
lines as GPIOs or nothing, so no DT changes would be required.
Okay, that's useful (please add to the commit msg).
I added the information in the cover letter, but I can add it to the
actual patch commit message if you prefer.
Still, what platform(s) need the modem control feature? Presumably
that's whatever platform Lech and Tomasz work on. I guess given the
Reviewed-by they are fine with needing a DT change.
Ok. I have previously also emailed Lech about that, but he has not
responded yet.

Thank you,
Hugo.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help