Re: [PATCH v6 00/24] MediaTek UFS Cleanup and MT8196 Enablement

[AngeloGioacchino Del Regno]

Il 24/01/26 13:00, Nicolas Frattaroli ha scritto:
> In this series, the existing MediaTek UFS binding is expanded and
> completed to correctly describe not just the existing compatibles, but
> also to introduce a new compatible in the from of the MT8196 SoC.
>
> The resets, which until now were completely absent from both the UFS
> host controller binding and the UFS PHY binding, are introduced to both.
> This also means the driver's undocumented and, in mainline, unused reset
> logic is reworked. In particular, the PHY reset is no longer a reset of
> the host controller node, but of the PHY node.
>
> This means the host controller can reset the PHY through the common PHY
> framework.
>
> The resets remain optional.
>
> Additionally, a massive number of driver cleanups are introduced. These
> were prompted by me inspecting the driver more closely as I was
> adjusting it to correspond to the binding.
>
> The driver still implements vendor properties that are undocumented in
> the binding. I did not touch most of those, as I neither want to
> convince the bindings maintainers that they are needed without knowing
> precisely what they're for, nor do I want to argue with the driver
> authors when removing them.
>
> Due to the "Marie Kondo with a chainsaw" nature of the driver cleanup
> patches, I humbly request that reviewers do not comment on displeasing
> code they see in the context portion of a patch before they've read the
> whole patch series, as that displeasing code may in fact be reworked in
> a subsequent patch of this series. Please keep comments focused on the
> changed lines of the diff; I know there's more that can be done, but it
> doesn't necessarily need to be part of this series.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@xxxxxxxxxxxxx>

Nicolas, can you please rebase this series on the latest linux-next?

All that you have to do is resend everything as-is, but drop patch [14/24], as the
same thing that you've done there landed in form of..

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/ufs/host/ufs-mediatek.c?h=next-20260213&id=bbb8d98fb4536594cb104fd630ea0f7dce3771d6

Besides. It feels like we're again playing a resend-and-nobody-cares game again
with this UFS driver.

I want to remind everyone that we've been trying to cleanup this thing for more
than a year and that our patches keep getting mostly ignored, if not for some
random nitpick here and there. (we: as I myself tried to clean it up a year ago
with my own patches that I had sent to the list - multiple versions of those)

Expressing my huge frustration here: at this point I'm not even sure why we keep
trying to make this right, other than being stubborn about feeling the need to
have good stuff in the kernel, instead of downstream code dumps which should've
never landed in the first place.

Though everyone here is human, and humans make mistakes (a lot), that's fine: the
problem happens when people negate mistakes, don't fix those, and don't care (not
because of lack of time, but because not caring at all, again - we've tried to do
those cleanups for a year now). One. Year.
Actually, more than one year, I don't even remember anymore.

This is not good behavior in a community, and should never happen.

Yet, here we are. Again. On the same driver. In the same subsystem.

Oh well.

Regards,
Angelo

> ---
> Changes in v6:
> - Reword "Rework probe function" commit to better justify the changes
>    being made.
> - Drop "Add vendor prefix to clk-scale-up-vcore-min"
> - Add patch to remove clk-scale-up-vcore-min entirely, describing the
>    process for bringing it back (in a different form) in the commit
>    message.
> - Link to v5: https://lore.kernel.org/r/20260108-mt8196-ufs-v5-0-49215157ec41@xxxxxxxxxxxxx
>
> Changes in v5:
> - Drop "scsi: ufs: mediatek: Make scale_us in setup_clk_gating const" as
>    someone else already got a patch in for this into next.
> - Make mtk_init_boost_crypt void
> - Don't disable/enable misc regulators during suspend/resume, but enable
>    them once when acquiring with a devm helper.
> - Link to v4: https://lore.kernel.org/r/20251218-mt8196-ufs-v4-0-ddec7a369dd2@xxxxxxxxxxxxx
>
> Changes in v4:
> - bindings: Redo the supply situation, as the avdd pins don't describe
>    the vcc(q2) card supplies.
> - bindings: format clock in mt8196 example more tersely.
> - phy: use devm_reset_control_get_optional_exclusive directly
> - driver: get and enable/disable the aforementioned avdd supplies.
> - Link to v3: https://lore.kernel.org/r/20251023-mt8196-ufs-v3-0-0f04b4a795ff@xxxxxxxxxxxxx
>
> Changes in v3:
> - Split mediatek,ufs bindings change into two patches, one for
>    completing the existing binding, one for the MT8196
> - Add over a dozen driver cleanup patches
> - Add explicit support for the MT8196 compatible to the driver
> - Note: next-20251023, on which I based this, currently has a broken
>    build due to an unrelated OPP core change that was merged with no
>    build testing. I can't use next-20251022 either, as that lacks the
>    recent mediatek UFS changes. It is what it is.
> - Link to v2: https://lore.kernel.org/r/20251016-mt8196-ufs-v2-0-c373834c4e7a@xxxxxxxxxxxxx
>
> Changes in v2:
> - Reorder define in mtk_sip_svc.h
> - Use bulk reset APIs in UFS host driver
> - Link to v1: https://lore.kernel.org/r/20251014-mt8196-ufs-v1-0-195dceb83bc8@xxxxxxxxxxxxx
>
> ---
> Nicolas Frattaroli (24):
>        dt-bindings: phy: Add mediatek,mt8196-ufsphy variant
>        dt-bindings: ufs: mediatek,ufs: Complete the binding
>        dt-bindings: ufs: mediatek,ufs: Add mt8196 variant
>        scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h
>        phy: mediatek: ufs: Add support for resets
>        scsi: ufs: mediatek: Rework resets
>        scsi: ufs: mediatek: Rework 0.9V regulator
>        scsi: ufs: mediatek: Rework init function
>        scsi: ufs: mediatek: Rework the crypt-boost stuff
>        scsi: ufs: mediatek: Handle misc host voltage regulators
>        scsi: ufs: mediatek: Rework probe function
>        scsi: ufs: mediatek: Remove vendor kernel quirks cruft
>        scsi: ufs: mediatek: Use the common PHY framework
>        scsi: ufs: mediatek: Switch to newer PM ops helpers
>        scsi: ufs: mediatek: Remove mediatek,ufs-broken-rtc property
>        scsi: ufs: mediatek: Rework _ufs_mtk_clk_scale error paths
>        scsi: ufs: mediatek: Clean up logging prints
>        scsi: ufs: mediatek: Rework ufs_mtk_wait_idle_state
>        scsi: ufs: mediatek: Don't acquire dvfsrc-vcore twice
>        scsi: ufs: mediatek: Rework hardware version reading
>        scsi: ufs: mediatek: Back up idle timer in per-instance struct
>        scsi: ufs: mediatek: Remove ret local from link_startup_notify
>        scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min"
>        scsi: ufs: mediatek: Add MT8196 compatible, update copyright
>
>   .../devicetree/bindings/phy/mediatek,ufs-phy.yaml  |  16 +
>   .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 173 +++-
>   drivers/phy/mediatek/phy-mtk-ufs.c                 |  71 ++
>   drivers/ufs/host/ufs-mediatek-sip.h                |   9 -
>   drivers/ufs/host/ufs-mediatek.c                    | 973 +++++++++------------
>   drivers/ufs/host/ufs-mediatek.h                    |  17 +-
>   include/linux/soc/mediatek/mtk_sip_svc.h           |   3 +
>   7 files changed, 655 insertions(+), 607 deletions(-)
> ---
> base-commit: 4af4e95edc37ae54f64cbd75b46f16ce15f3a6b8
> change-id: 20251014-mt8196-ufs-cec4b9a97e53
>
> Best regards,