Messages in this thread | | | Subject | Re: [PATCH v4 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings | From | Taniya Das <> | Date | Fri, 15 Jun 2018 23:10:07 +0530 |
| |
On 6/15/2018 5:29 PM, Amit Kucheria wrote: > On Thu, Jun 14, 2018 at 9:24 PM, Taniya Das <tdas@codeaurora.org> 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 >> numbers. > > "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 > #define PERF_DESIRED_OFFSET 0x920 >
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 > devm_ioremap()s. > > Regards, > Amit >
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation.
--
| |