Re: [PATCH v2 0/3] ASoC: rockchip: Use guard() for spin locks

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

On Thursday, 18 June 2026 05:16:24 Central European Summer Time Bui Duc Phuc wrote:
> Hi Nicolas,
> 
> Thanks for the feedback.
> 
> > FWIW, the SAI patch wasn't sent to the proper maintainer e-mail
> > for it which is why I didn't notice this series at all until now,
> > and the whole thing is pointless churn that wasn't even tested.
> >
> > From a cursory glance, whatever LLM was pointed at this and decided
> > to make the output my problem also didn't do a good job, the scoped
> > indentation of rockchip_sai_runtime_suspend seems pointless because
> > it could've been replaced by a pm guard followed by a spinlock guard,
> > without an additional level of indentation, and `if (ret == 0) {` is
> > not kernel style.
> >
> > It's not worth the revert but it sucks that I have to now set up a
> > new lei search for all the drivers I am supposed to receive mail for
> > because vibecoders can't be bothered to run get_maintainers as they
> > pad their CV with nonsense like this.
> >
> > >
> > > [1/3] ASoC: rockchip: rockchip_i2s: Use guard() for spin locks
> > >       https://git.kernel.org/broonie/sound/c/4bda5af16920
> > > [2/3] ASoC: rockchip: i2s-tdm: Use guard() for spin locks
> > >       https://git.kernel.org/broonie/sound/c/ec22437fc41a
> > > [3/3] ASoC: rockchip: rockchip_sai: Use guard() for spin locks
> > >       https://git.kernel.org/broonie/sound/c/f7fe9f707360
> > >
> 
> Regarding the claim that the Rockchip SAI patch was not sent to the proper
> maintainer, I believe there may be a misunderstanding.
> Before sending the series, I ran get_maintainer.pl on the patch set and
> included all recipients reported by the script, including:
> 
> Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> 
> along with the relevant mailing lists.
> 
> ------------------------------------------
> phuc@phuc-desktop:~/work/linux$ ls ../patch/sound/rockchip/clean/
> 0000-cover-letter.patch
> v2-0000-cover-letter.patch
> 0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-locks.patch
> v2-0001-ASoC-rockchip-rockchip_i2s-Use-guard-for-spin-loc.patch
> 0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch
> v2-0002-ASoC-rockchip-i2s-tdm-Use-guard-for-spin-locks.patch
> 0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-locks.patch
> v2-0003-ASoC-rockchip-rockchip_sai-Use-guard-for-spin-loc.patch
> phuc@phuc-desktop:~/work/linux$
> phuc@phuc-desktop:~/work/linux$
> phuc@phuc-desktop:~/work/linux$
> phuc@phuc-desktop:~/work/linux$ ./scripts/get_maintainer.pl
> ../patch/sound/rockchip/clean/000*.patch
> ./scripts/get_maintainer.pl: file
> '../patch/sound/rockchip/clean/0000-cover-letter.patch' doesn't appear
> to be a patch.  Add -f to options?
> Liam Girdwood <lgirdwood@gmail.com> (maintainer:SOUND - SOC LAYER /
> DYNAMIC AUDIO POWER MANAGEM...)
> Mark Brown <broonie@kernel.org> (maintainer:SOUND - SOC LAYER /
> DYNAMIC AUDIO POWER MANAGEM...)
> Jaroslav Kysela <perex@perex.cz> (maintainer:SOUND)
> Takashi Iwai <tiwai@suse.com> (maintainer:SOUND)
> Heiko Stuebner <heiko@sntech.de> (maintainer:ARM/Rockchip SoC support)
> Nicolas Frattaroli <frattaroli.nicolas@gmail.com> (maintainer:ROCKCHIP
> I2S TDM DRIVER)
> linux-sound@vger.kernel.org (open list:SOUND - SOC LAYER / DYNAMIC
> AUDIO POWER MANAGEM...)
> linux-arm-kernel@lists.infradead.org (moderated list:ARM/Rockchip SoC support)
> linux-rockchip@lists.infradead.org (open list:ARM/Rockchip SoC support)
> linux-kernel@vger.kernel.org (open list)
> SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC) status: Supported
> phuc@phuc-desktop:~/work/linux$
> phuc@phuc-desktop:~/work/linux$
> ---------------------------------------------

$ ./scripts/get_maintainer.pl sound/soc/rockchip/rockchip_sai.c 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> (maintainer:ROCKCHIP SAI DRIVER)
[...]

b4 will also pick this up. Running get_maintainer.pl on a patch with
rockchip_sai.* in the diff will also pick it up, see MAINTAINERS. It
likely wasn't picked up for you because by passing every patch into
it at once, it decided to "deduplicate" my gmail from my work e-mail
based on name, which seems like really silly default behaviour for
get_maintainers to have. `--no-remove-duplicates` works around this.

I should be setting up a forward or checking my gmail at work more
often.

> 
> If there are additional maintainers or mailing lists that should receive these
> patches but are not currently reported by get_maintainer.pl, please let me know
> so I can include them in future submissions.
> I would also appreciate avoiding assumptions that the series was generated
> by an LLM. The patches were prepared manually and submitted as part of
> my ongoing effort to convert lock/unlock patterns to guard()/scoped_guard()
> helpers where appropriate.
> The series was build-tested before submission, so I do not believe it
> is accurate
> to describe it as "wasn't even tested".
> Whether a particular conversion is worthwhile is certainly open to
> technical discussion,
> but I do not think it is reasonable to conclude that a patch was AI-generated
> simply based on disagreements about the implementation details.
> I understand maintainers have different preferences regarding how aggressively
> such conversions should be applied, and I am happy to adjust the approach
> based on review feedback.

If the patch worsens code legibility through excessive indentation and does
not make any functional changes, then I do not see the point of it.
Build-testing will tell you whether it compiles, but a lot of things in C
compile but aren't correct.

If you want to try again, do so in a way where you don't have to indent
large parts of a function another level, it's possible and the diff
will be something I can actually read. Remember that you can place a
guard for the spinlock in the middle of a function, and it'll only be
picked up then, which should reduce the need for the scoped_guards in
this. To avoid having to manually do pm unwind (and therefore require
that the spinlock is dropped somehow before function exit), use the
runtime pm guard macros as well. That should simplify this immensely.

Last but not least, please run things through checkpatch. Even at the
default options, it'll warn about the indentation level. With --strict,
it'll also tell you about the braces. Personally I just use whatever
preset `b4 prep --check` uses, which usually is quite reasonable.

> 
> Best regards,
> Phuc
>