Re: [PATCH v4 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings

From: Taniya Das
Date: Fri Jun 15 2018 - 13:40:27 EST

On 6/15/2018 5:29 PM, Amit Kucheria wrote:
On Thu, Jun 14, 2018 at 9:24 PM, Taniya Das <tdas@xxxxxxxxxxxxxx> wrote:

Sorry Sudeep I missed replying to your earlier query.
The High level OS(HLOS) would require to access only these specific
registers from this IP block and just mapping the whole block(huge
region) is unnecessary from the OS point of View. As of now it is a
generic binding for all using this IP block to manage frequency
requests. The OS would only have to know the frequencies supported i.e
to read the lookup table registers and put across the OS request using
the performance state register.

I am not sure if you need to defining bindings to save OSPM IO mapping.
In-fact you may be adding more mapping unnecessarily. The mappings are
page aligned and spiting the registers and mapping them individually may
result in more mappings.

I just need to know the rational for such specific choice of registers.
I assume it's aligned to some other standard specifications like CPPC
though not identical.

I am not sure of the query but there is no other register that the OS is
required to use other than the ones defined here.

Eg. Suppose you need some information on power curve for EAS energy
model, I really hate to update DT for that or even do a mix with DT just
because f/w is no longer modifiable.

For now we are safe.

What do you mean by that ?

I meant here was currently there is no such known case where the f/w is no
longer modifiable and we need to extend device tree bindings.

It should be easily extensible is what I am
trying to say. You can add more info and alter the information in the
driver with compatibles if you keep the register info as minimum as
possible. For now, you have enable, set and lut registers. What if you
want to provide power numbers ?

Yes I do understand the intent of mapping the whole register space, but as
per the HW specs these 3 registers would be the only ones required for now.
I do not think this hardware engine has any information on the power

"For now" - I think this is exactly the point that Sudeep is trying to make.

A future version of the HW engine, or more likely, a firmware
revision, will make more functionality available. Say, this needs
access to another register or two. This will require changing the DT
bindings. Instead, if you map the entire address space, you can just
add offsets to the new registers.

So in this case, I think you should define the following addresses
(size 0x1400) for the two frequency domains

0x17d43000, 0x1400 (power cluster)
0x17d45800, 0x1400 (perf cluster)

And in the driver simply add offsets as follows:

#define ENABLE_OFFSET 0x0
#define LUT_OFFSET 0x110

The offsets could vary across versions of this IP and that is the reason to provide them through the DT and not define any such offsets.

This will allow you add any new registers in the future w/o modifying
the DT binding and reduce qcom_cpu_resources_init() to a handful of
lines since you no longer need so many OF string matches, and


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