Re: [PATCH v4 3/8] scsi: ufs: ufs-mediatek: Remove useless mediatek,ufs-boost-crypt property

From: AngeloGioacchino Del Regno
Date: Wed Apr 17 2024 - 04:15:11 EST


Il 16/04/24 15:05, Peter Wang (王信友) ha scritto:
On Tue, 2024-04-16 at 12:38 +0200, AngeloGioacchino Del Regno wrote:
Il 16/04/24 12:31, Peter Wang (王信友) ha scritto:

Yes this causes -> less than half of a millisecond <- of
additional
boot time
if the dvfsrc-supply is present but boost-microvolt is not.

I really don't see the problem with that :-)


Adding a little bit of boot time to one smartphone might not be a
problem, but when you consider a billion smartphones each adding a
little bit, the cumulative effect becomes significant. The power
consumption of these accumulated times will continue to increase,
contributing to the Earth's carbon emissions. Moreover, removing
the
master switch for this feature doesn't seem to have any benefits
other
than not having to set it in the DTS. Similarly, the master switch
for
VA09 seems to have more disadvantage.


Sorry, but I still don't see how a few *microseconds* more of boot
time can
be significant, even related to power consumption during boot.

If that was a few milliseconds, then I'd agree with you, but that's
not the case.

Removing the master switch has a benefit: you *lose* a few
microseconds of boot
time (so, boots in *few microseconds LESS*) on platforms that would
have this set
in devicetree, as this property is redundant with the other
activation checks
for those features.

So, there you go: if the majority of MediaTek platforms are already
using this
crypt boost feature, then this commit reduces carbon emissions, as
those would
boot in a few less microseconds.


But the majority platfomrs dosen't need this feature.
This feature is only for legacy chip which at least 4 years ago.


Upstream supports platforms that do and don't need this feature, and having
redundant device tree properties performing the same checks is not just
suboptimal but plain wrong.

Adding to this, devicetree describes the hardware - and there is no physical
hardware switch that needs this redundant property, this means that the
property that is getting removed in this commit (and the va09 one in another
commit of this series) is a *software switch*, not HW.

Keep in mind, also, that this feature (and again, the va09 one as well) has
a specific requirement to be supported - and this is what the code does even
without the software switch to add it.

In case there's any need to disallow such feature from a specific SoC, the DT
bindings can be modified such that a specific compatible string would disallow
adding the required regulator and/or boost-microvolt properties.

Besides, I want to remind you that there is no reason to drop support, or have
them unreliably working, or use hacks, for SoCs that are "old" - especially
when this is a driver that works on both old and new ones.

Regards,
Angelo