linux-kernel - Re: [PATCH] soc: mediatek: mediatek-regulator-coupler: Fix comment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAC=S1ng=ZLB+vB=kNywLGMdeN1RRted0j6d-fC9ANoyu=ni3Mg@mail.gmail.com>
Date: Wed, 23 Oct 2024 21:47:39 +0800
From: Fei Shao <fshao@...omium.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] soc: mediatek: mediatek-regulator-coupler: Fix comment

On Wed, Oct 23, 2024 at 8:03 PM Fei Shao <fshao@...omium.org> wrote:
>
> On Wed, Oct 23, 2024 at 6:40 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
> >
> > Il 23/10/24 12:19, Fei Shao ha scritto:
> > > Fix two minor issues in the comments.
> > >
> > > 1. We balance VSRAM voltage based on the target VGPU voltage, so the
> > >     comment likely refers to VGPU.
> >
> > Function `mediatek_regulator_balance_voltage()` refers, as stated in the comment
> > located at the top of its signature, to "GPU<->SRAM" voltages relationships.
> >
> > So, we're taking into consideration only two regulators:
> >                    VGPU and VSRAM
> >
> > The first comment says:
> > "If we're asked to set a voltage (implicit: to VGPU) less than VSRAM min_uV[...]"
> >
> > ...so, I think that you've misunderstood what the comment says :-)
>
> Let me make sure we're on the same page - VGPU never goes higher than
> VSRAM (VGPU <= VSRAM), is that correct?
>
> [ min VGPU, max VGPU ] ... spread ... [    min VSRAM  ,  max VSRAM    ]
>  (min_uV)  (max_uV)                         (vsram_min_uV) (vsram_max_uV)
>
> The longer comment is
> "If we're asked to set a voltage less than VSRAM min_uV, set the
> minimum allowed voltage on VSRAM, ..."
> So VSRAM is the subject here I think? Because we "set voltage *on*
> VSRAM" based on its own minimum allowed voltage? We never set either
> VGPU min/max voltage to vsram_min_uV?
> And it's attached to the line that decides vsram_target_min_uV, with
> the maximum of (1) vsram_min_uV (i.e. minimum allowed VSRAM voltage)
> or (2) min_uV + max_spread (min VGPU + spread)

Ah, I just got your point. Both interpretations can explain..
I linked the "min_uV" in the comment to the one used in code, which
stands for VGPU min_uV, so I had the impression that it was a typo.

Regards,
Fei