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 - 05:37:30 EST


Il 17/04/24 11:29, Peter Wang (王信友) ha scritto:
On Wed, 2024-04-17 at 10:14 +0200, AngeloGioacchino Del Regno wrote:


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

Hi Angelo,

These two property(boost and va09) is not software switch, they
are hardware switch. Because if platform support crypto boost, we set
boost property in device tree. And if platform support ufs va09, we set
va09 proberty in device tree. These property are main hardware switch.

I disagree. If a platform supports crypto boost, it will have the dvfsrc
regulator and the supported voltage for the boost; if a platform supports
ufs va09, it will have the va09 regulator assigned in the ufshci devicetree
node.


We don't use sub switch like voltage or clock setting becasue it is
not intiutive. Especially when va09 is not used by ufs (No va09
property but have va09 voltage), The behavior should be wrong if ufs
control va09 which used by other module.


As I said, devicetree describes hardware - and this strategy being intuitive
or not boils down to personal opinions and nothing else.
Aside personal opinions, again, properties not describing hardware are wrong.

And again, if VA09 shall not be controlled by the UFSHCI driver on a specific
platform, then the regulator shall not be assigned to the UFSHCI node on that
specific platform.

Regards,
Angelo