Il 15/02/22 10:08, Roger Lu ha scritto:
Hi AngeloGioacchino,
Excuse me for the late reply.
Hi Roger,
no worries about that.
On Mon, 2022-01-31 at 12:11 +0100, AngeloGioacchino Del Regno wrote:
Il 27/01/22 04:39, Roger Lu ha scritto:
The purpose of SVS is to help find the suitable voltages
for DVFS. Therefore, if SVS bank voltages are concerned
to be wrong, we can adjust SVS bank voltages by this patch.
Signed-off-by: Roger Lu <roger.lu@xxxxxxxxxxxx>
Hello Roger,
I was thinking about what this patch is adding... and I have a few
considerations.
It's nice to have a debugging mechanism to read the status and dump registers,
as
that's very helpful when doing heavy debugging of the IP... but adding the
possibility to write a voltage offset may be very dangerous: think about the
case
in which, either for misconfiguration, or for any other reason, the debugfs
entry
that allows writing voffset becomes user-writable, or a user writes an
impossibly
high voffset.
In case a very low (negative) voffset is entered, the platform would crash
(denial
of service); if a very high voffset is entered, hardware damage may occur.
For this reason, there are two proposals:
1. If you want to keep the debugfs voffset write, please constrain the
permissible
voffset to an acceptable range that at least makes it unlikely to damage
the HW;
Moreover, since voffset write is a feature that would be used in very
limited
debugging cases, I think that this should be implemented over a build-time
configuration barrier... something like CONFIG_MTK_SVS_DEBUG_ALLOW_WRITE,
or
similar;
2. Since it's very unlikely for someone to really play that much with a
voltage
offset during runtime, and since this looks like something very machine
specific
(perhaps addressing board-specific quirks?), I would suggest to add this
as a
device-tree parameter instead, such as "mediatek,svs-voffset", as it is
indeed
possible to specify both positive or negative values in DT.
I would prefer proposal 2, as it looks generally cleaner and way less risky.
Thanks for raising the considerations and give these great suggestions for us to
think about. Since these voffset read/write commands are used seldomly, we
decide to remove them for better system security.
Thank you for this ack, very much appreciated.
Eager to see v23!
Regards,
Angelo
