Messages in this thread |  | | Date | Wed, 18 Apr 2018 11:11:58 -0700 | From | Channa <> | Subject | Re: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc |
| |
On 2018-04-18 07:52, Rob Herring wrote: > On Tue, Apr 17, 2018 at 5:12 PM, <rishabhb@codeaurora.org> wrote: >> On 2018-04-17 10:43, rishabhb@codeaurora.org wrote: >>> >>> On 2018-04-16 07:59, Rob Herring wrote: >>>> >>>> On Tue, Apr 10, 2018 at 01:08:12PM -0700, Rishabh Bhatnagar wrote: >>>>> >>>>> Documentation for last level cache controller device tree bindings, >>>>> client bindings usage examples. >>>> >>>> >>>> "Documentation: Documentation ..."? That wastes a lot of the subject >>>> line... The preferred prefix is "dt-bindings: ..." >>>> >>>>> >>>>> Signed-off-by: Channagoud Kadabi <ckadabi@codeaurora.org> >>>>> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org> >>>>> --- >>>>> .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 >>>>> ++++++++++++++++++++++ >>>>> 1 file changed, 58 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt >>>>> b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt >>>>> new file mode 100644 >>>>> index 0000000..497cf0f >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt >>>>> @@ -0,0 +1,58 @@ >>>>> +== Introduction== >>>>> + >>>>> +LLCC (Last Level Cache Controller) provides last level of cache >>>>> memory >>>>> in SOC, >>>>> +that can be shared by multiple clients. Clients here are different >>>>> cores in the >>>>> +SOC, the idea is to minimize the local caches at the clients and >>>>> migrate to >>>>> +common pool of memory >>>>> + >>>>> +Properties: >>>>> +- compatible: >>>>> + Usage: required >>>>> + Value type: <string> >>>>> + Definition: must be "qcom,sdm845-llcc" >>>>> + >>>>> +- reg: >>>>> + Usage: required >>>>> + Value Type: <prop-encoded-array> >>>>> + Definition: must be addresses and sizes of the LLCC >>>>> registers >>>> >>>> >>>> How many address ranges? >>>> >>> It consists of just one address range. I'll edit the definition to >>> make >>> it more clear. >>>>> >>>>> + >>>>> +- #cache-cells: >>>> >>>> >>>> This is all written as it is a common binding, but it is not one. >>>> >>>> You already have most of the configuration data for each client in >>>> the >>>> driver, I think I'd just put the client connection there too. Is >>>> there >>>> any variation of this for a given SoC? >>>> >>> #cache-cells and max-slices won't change for a given SOC. So you want >>> me >>> to hard-code in the driver itself? >>> >> I can use of_parse_phandle_with_fixed_args function and fix the number >> of >> args as 1 instead of keeping #cache-cells here in DT. Does that look >> fine? > > No, I'm saying why even put cache-slices properties in DT to begin > with? You could just define client id's within the kernel and clients > can use those instead of getting the id from the DT.
The reason to add cache-slices here is to establish a connection between client and system cache. For example if we have multiple instances of system cache blocks and client wants to choose a system cache instance based on the usecase then its easier to establish this connection using device tree than hard coding in the driver.
> > I have a couple of hesitations with putting this into the DT. First, I > think a cache is just one aspect of describing the interconnect > between masters and memory (and there's been discussions on > interconnect bindings too) and any binding needs to consider all of > the aspects of the interconnect. Second, I'd expect this cache > architecture will change SoC to SoC and the binding here is pretty > closely tied to the current cache implementation (e.g. slices). If > there were a bunch of SoCs with the same design and just different > client IDs (like interrupt IDs), then I'd feel differently.
This is partially true, a bunch of SoCs would support this design but clients IDs are not expected to change. So Ideally client drivers could hard code these IDs.
However I have other concerns of moving the client Ids in the driver. The way the APIs implemented today are as follows: #1. Client calls into system cache driver to get cache slice handle with the usecase Id as input. #2. System cache driver gets the phandle of system cache instance from the client device to obtain the private data. #3. Based on the usecase Id perform look up in the private data to get cache slice handle. #4. Return the cache slice handle to client
If we don't have the connection between client & system cache then the private data needs to declared as static global in the system cache driver, that limits us to have just once instance of system cache block.
> > Rob
-- -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
|  |