linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taniya Das <tdas@codeaurora.org>
To: Amit Kucheria <amit.kucheria@verdurent.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Rob Herring <robh@kernel.org>,
	Saravana Kannan <skannan@codeaurora.org>
Subject: Re: [PATCH v4 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings
Date: Fri, 15 Jun 2018 23:10:07 +0530	[thread overview]
Message-ID: <32e8f874-a58b-8ba3-7a53-dc89cb34f7d9@codeaurora.org> (raw)
In-Reply-To: <CAHLCerMCM1qf1MJ3bz+ZwBQMjshKSCWKXjHCiuqxJTDBdauamQ@mail.gmail.com>



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.

--

  parent reply	other threads:[~2018-06-15 17:40 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-12 11:02 [PATCH v4 0/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver Taniya Das
2018-06-12 11:02 ` [PATCH v4 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings Taniya Das
2018-06-13 11:26   ` Sudeep Holla
2018-06-13 18:13     ` Taniya Das
2018-06-14 10:47       ` Sudeep Holla
2018-06-14 18:24         ` Taniya Das
2018-06-15 11:59           ` Amit Kucheria
2018-06-15 13:27             ` Sudeep Holla
2018-06-15 17:40             ` Taniya Das [this message]
2018-06-15 17:45               ` Sudeep Holla
2018-06-17  9:03               ` Amit Kucheria
2018-06-18  9:21               ` Sudeep Holla
2018-06-19  7:53                 ` Taniya Das
2018-06-19  9:21                   ` Viresh Kumar
2018-06-19  9:34                   ` Sudeep Holla
2018-06-19 10:44                     ` Taniya Das
2018-06-15 13:23           ` Sudeep Holla
2018-06-15 17:31             ` Taniya Das
2018-06-15 17:42               ` Sudeep Holla
2018-06-15 13:07   ` Amit Kucheria
2018-06-12 11:02 ` [PATCH v4 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver Taniya Das
2018-06-15 12:02   ` Amit Kucheria
2018-06-19  9:30   ` Viresh Kumar
2018-07-11 20:37   ` Matthias Kaehlcke
2018-07-12 18:06     ` Taniya Das

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32e8f874-a58b-8ba3-7a53-dc89cb34f7d9@codeaurora.org \
    --to=tdas@codeaurora.org \
    --cc=amit.kucheria@verdurent.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=rnayak@codeaurora.org \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=skannan@codeaurora.org \
    --cc=sudeep.holla@arm.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).