Received-SPF: pass (google.com: domain of linux-kernel+bounces-186408-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Feedback-ID: ifd894703:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-480-g515a2f54a-fm-20240515.001-g515a2f54
Precedence: bulk
MIME-Version: 1.0
Message-Id: <a62c1c0b-0b2a-4b3f-85df-da586e103fcb@app.fastmail.com>
In-Reply-To: <13aa508f-6830-4d52-87fd-5063f737c990@app.fastmail.com>
References: <20240502-mips_debug_ll-v3-0-3b61f30e484c@flygoat.com>
 <3dcf3ac1-5494-482a-a80a-df4126e6ae59@app.fastmail.com>
 <3d6883ed-f8f4-44e5-a184-e5499c44f0f7@app.fastmail.com>
 <Zk2mt/FsgltvhVzf@alpha.franken.de>
 <13aa508f-6830-4d52-87fd-5063f737c990@app.fastmail.com>
Date: Wed, 22 May 2024 16:07:37 +0100
From: "Jiaxun Yang" <jiaxun.yang@flygoat.com>
To: "Thomas Bogendoerfer" <tsbogend@alpha.franken.de>
Cc: "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
 linux-kernel@vger.kernel.org,
 =?UTF-8?Q?Philippe_Mathieu-Daud=C3=A9?= <philmd@linaro.org>
Subject: Re: [PATCH v3 0/9] MIPS: Unify low-level debugging functionalities
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable


=E5=9C=A82024=E5=B9=B45=E6=9C=8822=E6=97=A5=E4=BA=94=E6=9C=88 =E4=B8=8A=E5=
=8D=889:28=EF=BC=8CJiaxun Yang=E5=86=99=E9=81=93=EF=BC=9A
> =E5=9C=A82024=E5=B9=B45=E6=9C=8822=E6=97=A5=E4=BA=94=E6=9C=88 =E4=B8=8A=
=E5=8D=889:03=EF=BC=8CThomas Bogendoerfer=E5=86=99=E9=81=93=EF=BC=9A
> [...]
>>
>> hmmm, I thought I was clear enough on version 1 of this series.
>>
>> I don't want an additional printk like debug interface, There is
>> prom_putchar() and early printk console, which always got me past
>> any boot issue.
>
> So it's not an additional printk like debug interface, it actually
> merged 3 existing debug interfaces, the first being zboot's assembly
> print routines, the second being CPS's assembly print routines, the
> third being some platform specific early printk. I think they are
> all essential for debugging early faults, for zboot that's the only
> way to print something at decompressing stage, for CPS as other cores
> are booting in non-coherent state we can't safely use any kernel
> functions, for early_printk that can help us *reduce* the amount
> of early printk code by just adding UART base to config.
>
> The only thing being added is the ability to debug very early exceptio=
n,
> even that is partially ported from existing CPS assembly debugging rou=
tines.
>
> Please let me know your thoughts.

That being said, have you noticed that prom_putchar and early_printk is
a non-extant on generic mach, ingenic, ralink etc? That's because we
really don't want to introduce any platform specific UART code for
early debugging on new platforms. With DEBUG_LL introduced by Arm it's
only a Kconfig option to do the trick.

I've got review tags in PATCH v2, that means not only me feeling that
this series is reasonable.

arm64 / riscv doesn't need that because they are well standardized
and it's almost guaranteed that kernel can boot into earlycon without
much drama. For MIPS that's not the case, there are too many things that
may go wrong, from zboot decompressor to cpu-probe and memblock. We real=
ly
lacks a way to debug things early, we need something that is available
at 1st instruction at kernel entry. Furthermore, many MIPS processors
don't come with JTAG or alike debugging support, that makes debugging
even harder, there is no way to debug an early exception if your firmware
doesn't handle it. That's all the motivation behind the series.

Besides, I think our communication needs to be improved. At PATCH v1
you made your point in reply, that's fair. So I also replied twice for
clarification. I heard nothing back, so I assume you want to see how
would it develop to address your concern. Then I posted PATCH v2 and
v3 to further improve the series, after that I pinged twice on PATCH v3.

That's in a 6-month timeframe with multiple transactions, you need to
inform us your intention, even if it's a NAK or you don't want to engage
on this topic further.

Quoting the maintainer handbook [1]: "If the review process or validation
for a particular change will take longer than the expected review timeli=
ne
for the subsystem, maintainer should reply to the submission indicating
that the work is being done, and when to expect full results." Radio sil=
ence
won't help anything, it's wasting time for both of us. Please, give a
shout if it's possible.

I can see some other series being slipped away this way, like I6500
multi-cluster patch, which is sent even earlier and respined many times
over. I can recall Mobileye series had a hard time on getting your
attention, luckily we went through it.

Quoting the maintainer handbook [1]: "Nonetheless when the work does
arrive (in form of patches which need review, user bug reports etc.)
it has to be acted upon promptly.". I understand Linux/MIPS is not
your day job, also you need to take breaks or go on holidays. Sometimes
you may burn out from your maintainer duties. That's fine, we are all
human beings. I'm not expecting a 1-week SLA or something, but 6 months
or longer to expect an action is appalling to me.

I'd strongly recommend you to look for a secondary maintainer, as mentio=
ned
in maintainer handbook [1]: "Modern best practices dictate that there sh=
ould
be at least two maintainers for any piece of code, no matter how trivial=
".
I understand you reject the idea once when Paul handed maintainership to
you, but there are clear evidences to show that something needs to be do=
ne.
You might need a hand on handling stuff promptly and understanding some
modern MIPS stuff.

I have many, many tiny improvements to MIPS kernel locally. Furthermore,
I do bring-up for both new and ancient MIPS systems. I never got a chance
to send them out because I want you prioritise on those fundamental seri=
es.

Apologise for potential aggressive tone in this email. I just can't clam
down when I think back about your reply, and I think we really need to
talk about it.

Thanks

[1]: https://docs.kernel.org/maintainer/feature-and-driver-maintainers.h=
tml
--=20
- Jiaxun