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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/18/2018 2:51 PM, Sudeep Holla wrote:


On 15/06/18 18:40, Taniya Das wrote:


On 6/15/2018 5:29 PM, Amit Kucheria wrote:

[...]

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.


Just get compatibles to identify the version of the hardware if it can't
be probed and detected. Please don't use DT to get the addresses of each
register you use in the driver. That's neither scalable nor nice
solution to the problem.
Hello Sudeep and Amit,

Thanks for the comments, I am consolidating the understanding from the other emails in a single one.

I understand that you are looking for this IP to map the full region and define offsets according to access them.

But I still not sure how do you want this common driver to scale in the cases where the offsets could vary across version change.

 DT
====
  freq-node {
	reg = < X x_size>;   Where X is the start of the IP address.
  }

Driver code (The below representation is just for example).
=============

V1
#define ENABLE	0x0
#define LUT_V1	0x110
#define PERF_V1	0x920

V2
#define LUT_V2	0x150
#define PERF_V2	0x980

V3
#define LUT_V3	0x120
....

Do you want me to use "compatible" flag to

if (compatible == v1)
 enable =  readl_relaxed(X + LUT_V1);
else if (compatible == v2)
 enable = readl_relaxed(X + LUT_V2);
else if (compatible == v3)
 enable = readl_relaxed(X + LUT_V2);

With the current design I do not need such compatible checks and unmap the ones which are not required after probe. Please let me know your comments.

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

--
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux