Re: [PATCH 1/2] soc: qcom: rpmhpd: Use corner in power_off

From: Rajendra Nayak
Date: Thu Jul 08 2021 - 01:04:10 EST




On 7/8/2021 10:05 AM, Bjorn Andersson wrote:
On Wed 07 Jul 19:21 CDT 2021, Stephen Boyd wrote:

Quoting Bjorn Andersson (2021-07-02 17:54:15)
rpmhpd_aggregate_corner() takes a corner as parameter, but in
rpmhpd_power_off() the code requests the level of the first corner
instead.

In all (known) current cases the first corner has level 0, so this
change should be a nop, but in case that there's a power domain with a
non-zero lowest level this makes sure that rpmhpd_power_off() actually
requests the lowest level - which is the closest to "power off" we can
get.

While touching the code, also skip the unnecessary zero-initialization
of "ret".

Fixes: 279b7e8a62cc ("soc: qcom: rpmhpd: Add RPMh power domain driver")
Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
---

I think this is why qcom folks talk about "virtual corner" and "physical
corner" because there's the one in command DB and the one in hardware.

I think the driver uses "level" and "corner" to denote the two different
number spaces, so I think we're good...now that we after this patch
don't pass a "level" as "corner" during power_off ;)

Maybe we should change rpmhpd_aggregate_corner() to call the argument
'vcorner'?

So "virtual corner" is "corner" and "physical corner" is level? I.e. 256
is a "physical corner"?

I haven't heard of anything called a 'physical corner'. These were always
referred to as virtual corners, on older platforms it was just one contiguous
number space, on newer ones we added another higher level sparse number space
just for more fun :)
Command DB refers to these as hlvl and vlvl, I haven;t yet figured out what their
full forms are :/


Or did you get the suggestion backwards?

Unfortunately we can't really build a type system here to
make this problem easy to catch with a mismatched type, unless there's
some sort of typedef trick we can play?


s/i/corner/ in rpmhpd_set_performance_state() would further enforce the
naming scheme used and reduce the risk for future confusion.

But we did just squash the final bug... ;)

Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>

Thanks,
Bjorn


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation