On Fri, Jun 15, 2018 at 8:40 PM, Taniya Das <t...@codeaurora.org> wrote: > > > On 6/15/2018 5:29 PM, Amit Kucheria wrote: >> >> On Thu, Jun 14, 2018 at 9:24 PM, Taniya Das <t...@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.
If that is a known fact internally, you should already introduce versioning information in the DT. e.g qcom,cpufreq-fw-v1. This will give you the ability to deal with IP versions across SoC families. We're currently trying to do the same thing for the TSENS IP. Regards, Amit
