SDM845 System Cache Driver
v4 cover SDM845 System Cache Driver
This series implements system cache or LLCC(Last Level Cache Controller) driver for SDM845 SOC. The purpose of the driver is to partition the system cache and program the settings such as priortiy, lines to probe while doing a look up in the system cache, low power related settings etc. The partitions are called cache slices. Each cache slice is associated with size and SCID(System Cache ID). The driver also provides API for clients to query the cache slice details,activate and deactivate them.
The driver can be broadly classified into:
* SOC specific driver: llcc-sdm845.c: Cache partitioning and cache slice properties for usecases on sdm845 that need to use system cache.
* API : llcc-slice.c: Exports APIs to clients to query cache slice details, activate and deactivate cache slices.
Changes since v3:
* Use the regmap_read_poll_timeout function
* Check for regmap read/write errors.
* Remove memory barrier after regmap write
* Derive memory bank offsets using stride macro variable
* Remove debug statements from code
* Remove the qcom_llcc_remove function
* Use if IS_ENABLED in place of ifdef for built-in module
* Change EXPORT_SYMBOL to EXPORT_SYMBOL_GPL
* Remove unnecessary free functions
* Change the variable names as per review comments
Changes since v2:
* Corrected the Makefile to fix compilation.
Changes since v1:
* Added Makefile and Kconfig.
Changes since v0:
* Removed the syscon and simple-mfd approach
* Updated the device tree nodes to mention LLCC as a single HW block
* Moved llcc bank offsets from device tree and handled the offset in the driver.
ckadabi at codeaurora.org (2):
Documentation: Documentation for qcom, llcc
drivers: soc: Add LLCC driver
.../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 +++ drivers/soc/qcom/Kconfig | 17 + drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++ drivers/soc/qcom/llcc-slice.c | 404 +++++++++++++++++++++ include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++ 6 files changed, 759 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt create mode 100644 drivers/soc/qcom/llcc-sdm845.c create mode 100644 drivers/soc/qcom/llcc-slice.c create mode 100644 include/linux/soc/qcom/llcc-qcom.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v4 1/2 Documentation: Documentation for qcom, llcc
Documentation for last level cache controller device tree bindings, client bindings usage examples.
--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +58 -0
@@ -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 + +- #cache-cells: + Usage: required + Value Type: <u32> + Definition: Number of cache cells, must be 1 + +- max-slices: + usage: required + Value Type: <u32> + Definition: Number of cache slices supported by hardware + +Example: + + llcc: qcom,llcc at 1100000 { + compatible = "qcom,sdm845-llcc"; + reg = <0x1100000 0x250000>; + #cache-cells = <1>; + max-slices = <32>; + }; + +== Client == + +Properties: +- cache-slice-names: + Usage: required + Value type: <stringlist> + Definition: A set of names that identify the usecase names of a + client that uses cache slice. These strings are + used to look up the cache slice entries by name. + +- cache-slices: + Usage: required + Value type: <prop-encoded-array> + Definition: The tuple has phandle to llcc device as the first + argument and the second argument is the usecase + id of the client. +For Example: + venus { + cache-slice-names = "vidsc0", "vidsc1"; + cache-slices = <&llcc VIDSC0_ID>, <&llcc VIDSC1_ID>; + };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
Quoted 7 lines · Rishabh Bhatnagar
Documentation for last level cache controller device tree bindings, client bindings usage examples.
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58
++++++++++++++++++++++
Quoted 2 lines
1 file changed, 58 insertions(+)create mode 100644
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
Review
Quoted 1 line
(quoted diff) +0
-0
b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 3
Quoted 8 lines
new file mode 100644
index 0000000..497cf0fDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0@@ -0,0 +1,58 @@ +== Introduction== + +LLCC (Last Level Cache Controller) provides last level of cache memory
in SOC,
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 4
Quoted 1 line
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +1
-0+that can be shared by multiple clients. Clients here are different cores
in the
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 5
Quoted 1 line
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +1
-0+SOC, the idea is to minimize the local caches at the clients and migrate
to
Quoted 42 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +42
-0+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 + +- #cache-cells: + Usage: required + Value Type: <u32> + Definition: Number of cache cells, must be 1 + +- max-slices: + usage: required + Value Type: <u32> + Definition: Number of cache slices supported by hardware + +Example: + + llcc: qcom,llcc at 1100000 { + compatible = "qcom,sdm845-llcc"; + reg = <0x1100000 0x250000>; + #cache-cells = <1>; + max-slices = <32>; + }; + +== Client == + +Properties: +- cache-slice-names: + Usage: required + Value type: <stringlist> + Definition: A set of names that identify the usecase names of a + client that uses cache slice. These strings are + used to look up the cache slice entries by name. +
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 57
Quoted 10 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +10
-0+- cache-slices: + Usage: required + Value type: <prop-encoded-array> + Definition: The tuple has phandle to llcc device as the first + argument and the second argument is the usecase + id of the client. +For Example: + venus { + cache-slice-names = "vidsc0", "vidsc1"; + cache-slices = <&llcc VIDSC0_ID>, <&llcc VIDSC1_ID>;
My git complains about some whitespace weirdness on the line above. Other than that:
-Evan
Rob Herring
maintainerRe: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
In reply to Rishabh BhatnagarQuoted 2 lines · Rishabh Bhatnagar
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: ..."
Quoted 21 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +7
-0@@ -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 +
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 17
Quoted 10 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +10
-0+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?
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 19
Quoted 2 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +2
-0+ +- #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?
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 27
Quoted 8 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +8
-0+ Usage: required + Value Type: <u32> + Definition: Number of cache cells, must be 1 + +- max-slices: + usage: required + Value Type: <u32> + Definition: Number of cache slices supported by hardware
What's a slice?
Quoted 39 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +31
-1+ +Example: + + llcc: qcom,llcc at 1100000 { + compatible = "qcom,sdm845-llcc"; + reg = <0x1100000 0x250000>; + #cache-cells = <1>; + max-slices = <32>; + }; + +== Client == + +Properties: +- cache-slice-names: + Usage: required + Value type: <stringlist> + Definition: A set of names that identify the usecase names of a + client that uses cache slice. These strings are + used to look up the cache slice entries by name. + +- cache-slices: + Usage: required + Value type: <prop-encoded-array> + Definition: The tuple has phandle to llcc device as the first + argument and the second argument is the usecase + id of the client. +For Example: + venus { + cache-slice-names = "vidsc0", "vidsc1"; + cache-slices = <&llcc VIDSC0_ID>, <&llcc VIDSC1_ID>; + }; --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
Quoted 38 lines · Rob Herring
Quoted 2 lines · Rishabh Bhatnagar
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: ..."
Quoted 31 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0@@ -0,0 +1,58 @@ +== Introduction== + +LLCC (Last Level Cache Controller) provides last level of cachememory 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 toDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +6
-0+common pool of memory + +Properties: +- compatible: + Usage: required + Value type: <string>
Quoted 9 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 17
Quoted 6 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +6
-0+ Definition: must be "qcom,sdm845-llcc" + +- reg: + Usage: required + Value Type: <prop-encoded-array> + Definition: must be addresses and sizes of the LLCC registersHow many address ranges?
It consists of just one address range. I'll edit the definition to make it more clear.
Quoted 9 lines
Quoted 2 lines
+
+- #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?
Quoted 11 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 27
Quoted 8 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +8
-0+ Usage: required + Value Type: <u32> + Definition: Number of cache cells, must be 1 + +- max-slices: + usage: required + Value Type: <u32> + Definition: Number of cache slices supported by hardwareWhat's a slice?
System cache memory provided by LLCC is divided into smaller chunks called slices. Each slice has its associated size and ID. Clients can query slice details, activate and deactivate them.
Quoted 41 lines
Quoted 41 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +31
-1+ +Example: + + llcc: qcom,llcc at 1100000 { + compatible = "qcom,sdm845-llcc"; + reg = <0x1100000 0x250000>; + #cache-cells = <1>; + max-slices = <32>; + }; + +== Client == + +Properties: +- cache-slice-names: + Usage: required + Value type: <stringlist> + Definition: A set of names that identify the usecase names of a + client that uses cache slice. These strings are + used to look up the cache slice entries by name. + +- cache-slices: + Usage: required + Value type: <prop-encoded-array> + Definition: The tuple has phandle to llcc device as the first + argument and the second argument is the usecase + id of the client. +For Example: + venus { + cache-slice-names = "vidsc0", "vidsc1"; + cache-slices = <&llcc VIDSC0_ID>, <&llcc VIDSC1_ID>; + }; --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
Quoted 55 lines · rishabhb at codeaurora.org
Quoted 38 lines · Rob Herring
Quoted 2 lines · Rishabh Bhatnagar
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: ..."
Quoted 31 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0@@ -0,0 +1,58 @@ +== Introduction== + +LLCC (Last Level Cache Controller) provides last level of cachememory 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 toDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +6
-0+common pool of memory + +Properties: +- compatible: + Usage: required + Value type: <string>Quoted 9 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 17
Quoted 6 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +6
-0+ Definition: must be "qcom,sdm845-llcc" + +- reg: + Usage: required + Value Type: <prop-encoded-array> + Definition: must be addresses and sizes of the LLCC registersHow many address ranges?
It consists of just one address range. I'll edit the definition to make it more clear.
Quoted 5 lines
Quoted 2 lines
+
+- #cache-cells:This is all written as it is a common binding, but it is not one.
Quoted 8 lines
Quoted 4 lines
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?
Quoted 55 lines
Quoted 11 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 27
Quoted 8 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +8
-0+ Usage: required + Value Type: <u32> + Definition: Number of cache cells, must be 1 + +- max-slices: + usage: required + Value Type: <u32> + Definition: Number of cache slices supported by hardwareWhat's a slice?
System cache memory provided by LLCC is divided into smaller chunks called slices. Each slice has its associated size and ID. Clients can query slice details, activate and deactivate them.
Quoted 41 lines
Quoted 41 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +31
-1+ +Example: + + llcc: qcom,llcc at 1100000 { + compatible = "qcom,sdm845-llcc"; + reg = <0x1100000 0x250000>; + #cache-cells = <1>; + max-slices = <32>; + }; + +== Client == + +Properties: +- cache-slice-names: + Usage: required + Value type: <stringlist> + Definition: A set of names that identify the usecase names of a + client that uses cache slice. These strings are + used to look up the cache slice entries by name. + +- cache-slices: + Usage: required + Value type: <prop-encoded-array> + Definition: The tuple has phandle to llcc device as the first + argument and the second argument is the usecase + id of the client. +For Example: + venus { + cache-slice-names = "vidsc0", "vidsc1"; + cache-slices = <&llcc VIDSC0_ID>, <&llcc VIDSC1_ID>; + }; --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob Herring
maintainerRe: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
Quoted 67 lines · On Tue, Apr 17, 2018 at 5:12 PM, <[email protected]> wrote:
Quoted 66 lines · rishabhb at codeaurora.org
Quoted 42 lines · Rob Herring
Quoted 3 lines · Rishabh Bhatnagar
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: ..."
Quoted 32 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0@@ -0,0 +1,58 @@ +== Introduction== + +LLCC (Last Level Cache Controller) provides last level of cache memoryin 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 toDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +7
-0+common pool of memory + +Properties: +- compatible: + Usage: required + Value type: <string> + Definition: must be "qcom,sdm845-llcc"Quoted 9 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt line 17
Quoted 5 lines
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +5
-0+ +- reg: + Usage: required + Value Type: <prop-encoded-array> + Definition: must be addresses and sizes of the LLCC registersHow many address ranges?
It consists of just one address range. I'll edit the definition to make it more clear.
Quoted 11 lines
Quoted 3 lines
+
+- #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?
Quoted 5 lines
Quoted 3 lines
#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_argsfunction 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.
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.
Rob
Re: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
Quoted 78 lines · Rob Herring
Quoted 77 lines · On Tue, Apr 17, 2018 at 5:12 PM, <[email protected]> wrote:
Quoted 76 lines · rishabhb at codeaurora.org
Quoted 45 lines · Rob Herring
Quoted 3 lines · Rishabh Bhatnagar
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: ..."
Quoted 35 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt(quoted diff) +0
-0
a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtb/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtnew file mode 100644
index 0000000..497cf0fDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0@@ -0,0 +1,58 @@ +== Introduction== + +LLCC (Last Level Cache Controller) provides last level of cachememory
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 toDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +8
-0+common pool of memory + +Properties: +- compatible: + Usage: required + Value type: <string> + Definition: must be "qcom,sdm845-llcc" +Quoted 9 lines
Quoted 5 lines
+- reg:
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0+ Usage: required + Value Type: <prop-encoded-array> + Definition: must be addresses and sizes of the LLCCregisters
How many address ranges?
It consists of just one address range. I'll edit the definition to make
it more clear.Quoted 5 lines
Quoted 3 lines
+
+- #cache-cells:Quoted 8 lines
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?
Quoted 8 lines
Quoted 4 lines
I can use
of_parse_phandle_with_fixed_argsfunction 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.
Quoted 10 lines
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.
Quoted 2 lines
Rob
Signature · 4 lines
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v4 1/2] Documentation: Documentation for qcom, llcc
Quoted 123 lines · Channa
Quoted 81 lines · Rob Herring
Quoted 80 lines · On Tue, Apr 17, 2018 at 5:12 PM, <[email protected]> wrote:
Quoted 79 lines · rishabhb at codeaurora.org
Quoted 48 lines · Rob Herring
Quoted 4 lines · Rishabh Bhatnagar
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: ..."Quoted 36 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- .../devicetree/bindings/arm/msm/qcom,llcc.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt(quoted diff) +0
-0
a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtb/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txtnew file mode 100644
index 0000000..497cf0fDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0@@ -0,0 +1,58 @@ +== Introduction== + +LLCC (Last Level Cache Controller) provides last level of cachememory
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 toDocumentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +8
-0+common pool of memory + +Properties: +- compatible: + Usage: required + Value type: <string> + Definition: must be "qcom,sdm845-llcc" +Quoted 9 lines
Quoted 5 lines
+- reg:
Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt new file +3
-0+ Usage: required + Value Type: <prop-encoded-array> + Definition: must be addresses and sizes of the LLCCregisters
How many address ranges?
It consists of just one address range. I'll edit the definition to make
it more clear.Quoted 5 lines
Quoted 3 lines
+
+- #cache-cells:Quoted 8 lines
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?Quoted 8 lines
Quoted 4 lines
I can use
of_parse_phandle_with_fixed_argsfunction 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.Quoted 10 lines
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
Quoted 8 lines
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.Quoted 2 lines
Rob
Hi Rob:
Can you please provide your opinion on the approach here?
Thanks,
Channa
Signature · 4 lines
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v4 2/2 drivers: soc: Add LLCC driver
In reply to Rishabh BhatnagarLLCC (Last Level Cache Controller) provides additional cache memory in the system. LLCC is partitioned into multiple slices and each slice gets its own priority, size, ID and other config parameters. LLCC driver programs these parameters for each slice. Clients that are assigned to use LLCC need to get information such size & ID of the slice they get and activate or deactivate the slice as needed. LLCC driver provides API for the clients to perform these operations.
--- drivers/soc/qcom/Kconfig | 17 ++ drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++++++ drivers/soc/qcom/llcc-slice.c | 404 +++++++++++++++++++++++++++++++++++++ include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++++++++ 5 files changed, 701 insertions(+) create mode 100644 drivers/soc/qcom/llcc-sdm845.c create mode 100644 drivers/soc/qcom/llcc-slice.c create mode 100644 include/linux/soc/qcom/llcc-qcom.h
drivers/soc/qcom/Kconfig +17 -0
@@ -21,6 +21,23 @@ config QCOM_GSBI functions for connecting the underlying serial UART, SPI, and I2C devices to the output pins. +config QCOM_LLCC + tristate "Qualcomm Technologies, Inc. LLCC driver" + depends on ARCH_QCOM + help + Qualcomm Technologies, Inc. platform specific + Last Level Cache Controller(LLCC) driver. This provides interfaces + to client's that use the LLCC. Say yes here to enable LLCC slice + driver. + +config QCOM_SDM845_LLCC + tristate "Qualcomm Technologies, Inc. SDM845 LLCC driver" + depends on QCOM_LLCC + help + Say yes here to enable the LLCC driver for SDM845. This provides + data required to configure LLCC so that clients can start using the + LLCC slices. + config QCOM_MDT_LOADER tristate select QCOM_SCM
drivers/soc/qcom/Makefile +2 -0
@@ -12,3 +12,5 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o obj-$(CONFIG_QCOM_SMP2P) += smp2p.o obj-$(CONFIG_QCOM_SMSM) += smsm.o obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o +obj-$(CONFIG_QCOM_LLCC) += llcc-slice.o +obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o
drivers/soc/qcom/llcc-sdm845.c new file +110 -0
110 lines not on this page · raw message
drivers/soc/qcom/llcc-slice.c new file +404 -0
404 lines not on this page · raw message
include/linux/soc/qcom/llcc-qcom.h new file +168 -0
168 lines not on this page · raw message
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v4 2/2] drivers: soc: Add LLCC driver
Quoted 20 lines · Rishabh Bhatnagar
LLCC (Last Level Cache Controller) provides additional cache memory in the system. LLCC is partitioned into multiple slices and each slice gets its own priority, size, ID and other config parameters. LLCC driver programs these parameters for each slice. Clients that are assigned to use LLCC need to get information such size & ID of the slice they get and activate or deactivate the slice as needed. LLCC driver provides API for the clients to perform these operations.
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- drivers/soc/qcom/Kconfig | 17 ++ drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++++++ drivers/soc/qcom/llcc-slice.c | 404 +++++++++++++++++++++++++++++++++++++ include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++++++++ 5 files changed, 701 insertions(+) create mode 100644 drivers/soc/qcom/llcc-sdm845.c create mode 100644 drivers/soc/qcom/llcc-slice.c create mode 100644 include/linux/soc/qcom/llcc-qcom.h
<snip>
Quoted 51 lines
drivers/soc/qcom/llcc-sdm845.c new file +45
-0@@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. +* +*/ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/soc/qcom/llcc-qcom.h> + +/* +* SCT(System Cache Table) entry contains of the following parameters +* name: Name of the client's use case for which the llcc slice is used +* uid: Unique id for the client's use case +* slice_id: llcc slice id for each client +* max_cap: The maximum capacity of the cache slice provided in KB +* priority: Priority of the client used to select victim line for replacement +* fixed_size: Determine if the slice has a fixed capacity +* bonus_ways: Bonus ways are additional ways to be used for any slice, +* if client ends up using more than reserved cache ways. Bonus +* ways are allocated only if they are not reserved for some +* other client. +* res_ways: Reserved ways for the cache slice, the reserved ways cannot +* be used by any other client than the one its assigned to. +* cache_mode: Each slice operates as a cache, this controls the mode of the +* slice: normal or TCM(Tightly Coupled Memory) +* probe_target_ways: Determines what ways to probe for access hit. When +* configured to 1 only bonus and reserved ways are probed. +* When configured to 0 all ways in llcc are probed. +* dis_cap_alloc: Disable capacity based allocation for a client +* retain_on_pc: If this bit is set and client has maintained active vote +* then the ways assigned to this client are not flushed on power +* collapse. +* activate_on_init: Activate the slice immediately after the SCT is programmed +*/ +#define SCT_ENTRY (n, uid, sid, mc, p, fs, bway, rway, cmod, ptw, dca, rp, a) \ + { \ + .name = n, \ + .usecase_id = uid, \ + .slice_id = sid, \ + .max_cap = mc, \ + .priority = p, \ + .fixed_size = fs, \
drivers/soc/qcom/llcc-sdm845.c line 55
Quoted 10 lines
drivers/soc/qcom/llcc-sdm845.c new file +10
-0+ .bonus_ways = bway, \ + .res_ways = rway, \ + .cache_mode = cmod, \ + .probe_target_ways = ptw, \ + .dis_cap_alloc = dca, \ + .retain_on_pc = rp, \ + .activate_on_init = a, \ + } + +
Extra blank line.
Quoted 105 lines
drivers/soc/qcom/llcc-sdm845.c new file +55
-0+static struct llcc_slice_config sdm845_data[] = { +SCT_ENTRY ("cpuss" ,1 ,1 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,1 ), +SCT_ENTRY ("vidsc0" ,2 ,2 ,512 ,2 ,1 ,0x0 ,0x0f0 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("vidsc1" ,3 ,3 ,512 ,2 ,1 ,0x0 ,0x0f0 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("rotator" ,4 ,4 ,563 ,2 ,1 ,0x0 ,0x00e ,2 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("voice" ,5 ,5 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("audio" ,6 ,6 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("mdmhpgr" ,7 ,7 ,1024 ,2 ,0 ,0xfc ,0xf00 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("modem" ,8 ,8 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("compute" ,10 ,10 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("gpuhtw" ,11 ,11 ,512 ,1 ,1 ,0xc ,0x0 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("gpu" ,12 ,12 ,2304 ,1 ,0 ,0xff0 ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("mmuhwt" ,13 ,13 ,256 ,2 ,0 ,0x0 ,0x1 ,0 ,0 ,1 ,0 ,1 ), +SCT_ENTRY ("cmptdma" ,15 ,15 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("display" ,16 ,16 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("videofw" ,17 ,17 ,2816 ,1 ,0 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("mdmhpfx" ,20 ,20 ,1024 ,2 ,1 ,0x0 ,0xf00 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("mdmpgng" ,21 ,21 ,1024 ,0 ,1 ,0x1e ,0x0 ,0 ,0 ,1 ,1 ,0 ), +SCT_ENTRY ("audiohw" ,22 ,22 ,1024 ,1 ,1 ,0xffc ,0x2 ,0 ,0 ,1 ,1 ,0 ), +}; + + +static int sdm845_qcom_llcc_probe (struct platform_device *pdev) +{ +return qcom_llcc_probe (pdev, sdm845_data,ARRAY_SIZE (sdm845_data)); +} + +static const struct of_device_id sdm845_qcom_llcc_of_match[] = { + { .compatible ="qcom,sdm845-llcc" , }, + { }, +}; + +static struct platform_driver sdm845_qcom_llcc_driver = { + .driver = { + .name ="sdm845-llcc" , + .owner =THIS_MODULE , + .of_match_table = sdm845_qcom_llcc_of_match, + }, + .probe = sdm845_qcom_llcc_probe, +}; + +static int __init sdm845_init_qcom_llcc_init (void ) +{ +return platform_driver_register (&sdm845_qcom_llcc_driver); +} +module_init (sdm845_init_qcom_llcc_init); + +static void __exit sdm845_exit_qcom_llcc_exit(void) +{ +platform_driver_unregister (&sdm845_qcom_llcc_driver); +} +module_exit (sdm845_exit_qcom_llcc_exit); + +MODULE_DESCRIPTION ("QTI sdm845 LLCC driver" ); +MODULE_LICENSE ("GPL v2" );drivers/soc/qcom/llcc-slice.c new file +44
-0@@ -0,0 +1,404 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. +* +*/ + +#include <linux/bitmap.h> +#include <linux/bitops.h> +#include <linux/device.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/soc/qcom/llcc-qcom.h> + +#define ACTIVATE 0x1 +#define DEACTIVATE 0x2 +#define ACT_CTRL_OPCODE_ACTIVATE 0x1 +#define ACT_CTRL_OPCODE_DEACTIVATE 0x2 +#define ACT_CTRL_ACT_TRIG 0x1 +#define ACT_CTRL_OPCODE_SHIFT 0x1 +#define ATTR1_PROBE_TARGET_WAYS_SHIFT 0x2 +#define ATTR1_FIXED_SIZE_SHIFT 0x3 +#define ATTR1_PRIORITY_SHIFT 0x4 +#define ATTR1_MAX_CAP_SHIFT 0x10 +#define ATTR0_RES_WAYS_MASK 0x00000fff +#define ATR0_BONUS_WAYS_MASK 0x0fff0000 +#define ATTR0_BONUS_WAYS_SHIFT 0x10 +#define LLCC_STATUS_READ_DELAY 100 + +#define CACHE_LINE_SIZE_SHIFT 6 + +#define LLCC_COMMON_STATUS0 0x0003000c +#define LLCC_LB_CNT_MASK 0xf0000000 +#define LLCC_LB_CNT_SHIFT 28 + +#define MAX_CAP_TO_BYTES (n) (n * 1024) +#define LLCC_TRP_ACT_CTRLn (n) (n * 0x1000) +#define LLCC_TRP_STATUSn (n) (4 + n * 0x1000) +#define LLCC_TRP_ATTR0_CFGn (n) (0x21000 + 0x8 * n) +#define LLCC_TRP_ATTR1_CFGn (n) (0x21004 + 0x8 * n) +
drivers/soc/qcom/llcc-slice.c line 54
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+#define BANK_OFFSET_STRIDE 0x80000 + +static const struct regmap_config llcc_regmap_config = { + .reg_bits =32 , + .reg_stride =4 , + .val_bits =32 , + .fast_io = true, +}; + +
Another extra blank line.
Quoted 55 lines
drivers/soc/qcom/llcc-slice.c new file +55
-0+/* Get the slice entry by index */ +static struct llcc_slice_desc *llcc_slice_get_entry (struct device *dev,int n) +{ +struct of_phandle_args phargs; +struct llcc_drv_data *drv; +const struct llcc_slice_config *llcc_data_ptr; +struct llcc_slice_desc *desc; +struct platform_device *pdev; +u32 sz, count; + +if (of_parse_phandle_with_args (dev->of_node,"cache-slices" , +"#cache-cells" , n, &phargs)) +return ERR_PTR (-ENODEV ); + + pdev =of_find_device_by_node (phargs.np); +if (!pdev) +return ERR_PTR (-ENODEV ); + + drv =platform_get_drvdata (pdev); +if (!drv) +return ERR_PTR (-EFAULT ); + + llcc_data_ptr = drv->cfg; + sz = drv->cfg_size; + count =0 ; + +while (llcc_data_ptr && count < sz) { +if (llcc_data_ptr->usecase_id == phargs.args[0 ]) +break ; + llcc_data_ptr++; + count++; + } + +if (llcc_data_ptr ==NULL || count == sz) +return ERR_PTR (-ENODEV ); + + desc =devm_kzalloc (dev,sizeof (struct llcc_slice_desc ),GFP_KERNEL ); +if (!desc) +return ERR_PTR (-ENOMEM ); + + desc->slice_id = llcc_data_ptr->slice_id; + desc->slice_size = llcc_data_ptr->max_cap; + desc->dev = &pdev->dev; + +return desc; +} + +/** +* llcc_slice_getd - get llcc slice descriptor +* @dev: Device pointer of the client +* @name: Name of the use case +* +* A pointer to llcc slice descriptor will be returned on success and +* and error pointer is returned on failure +*/
drivers/soc/qcom/llcc-slice.c line 119
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+struct llcc_slice_desc *llcc_slice_getd (struct device *dev,const char *name) +{ +struct device_node *np = dev->of_node; +int index =0 ; +const char *slice_name; +struct property *prop; + +if (!np) +return ERR_PTR (-ENOENT ); +
I'm not sure if dev->of_node can be null at this point, but even if it is of_get_property can safely handle a null pointer so you don't need this additional check.
Quoted 223 lines
drivers/soc/qcom/llcc-slice.c new file +223
-0+if (!of_get_property (np,"cache-slice-names" ,NULL )) +return ERR_PTR (-ENOENT ); + +of_property_for_each_string (np,"cache-slice-names" , prop, slice_name) { +if (!strcmp (name, slice_name)) +break ; + index++; + } +return llcc_slice_get_entry (dev, index); +} +EXPORT_SYMBOL_GPL (llcc_slice_getd); + +/** +* llcc_slice_putd - llcc slice descritpor +* @desc: Pointer to llcc slice descriptor +*/ +void llcc_slice_putd (struct llcc_slice_desc *desc) +{ +devm_kfree (desc->dev, desc); +} +EXPORT_SYMBOL_GPL (llcc_slice_putd); + +static int llcc_update_act_ctrl (struct llcc_drv_data *drv,u32 sid, +u32 act_ctrl_reg_val,u32 status) +{ +u32 act_ctrl_reg; +u32 status_reg; +u32 slice_status; +int ret =0 ; + + act_ctrl_reg = drv->bcast_off +LLCC_TRP_ACT_CTRLn (sid); + status_reg = drv->bcast_off +LLCC_TRP_STATUSn (sid); + +/*Set the ACTIVE trigger*/ + act_ctrl_reg_val |=ACT_CTRL_ACT_TRIG ; + ret =regmap_write (drv->regmap, act_ctrl_reg, act_ctrl_reg_val); +if (ret) +return ret; + +/* Clear the ACTIVE trigger */ + act_ctrl_reg_val &= ~ACT_CTRL_ACT_TRIG ; + ret =regmap_write (drv->regmap, act_ctrl_reg, act_ctrl_reg_val); +if (ret) +return ret; + + ret =regmap_read_poll_timeout (drv->regmap, status_reg, slice_status, + !(slice_status & status),0 ,LLCC_STATUS_READ_DELAY ); +return ret; +} + +/** +* llcc_slice_activate - Activate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will +* be returned in error cases +*/ +int llcc_slice_activate (struct llcc_slice_desc *desc) +{ +int ret; +u32 act_ctrl_val; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ; + + drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + + act_ctrl_val =ACT_CTRL_OPCODE_ACTIVATE <<ACT_CTRL_OPCODE_SHIFT ; + + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +DEACTIVATE ); + +__set_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_activate); + +/** +* llcc_slice_deactivate - Deactivate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will +* be returned in error cases +*/ +int llcc_slice_deactivate (struct llcc_slice_desc *desc) +{ +u32 act_ctrl_val; +int ret; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ; + + drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (!test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + act_ctrl_val =ACT_CTRL_OPCODE_DEACTIVATE <<ACT_CTRL_OPCODE_SHIFT ; + + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +ACTIVATE ); + +__clear_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_deactivate); + +/** +* llcc_get_slice_id - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and a negative errno will +* be returned on error +*/ +int llcc_get_slice_id (struct llcc_slice_desc *desc) +{ +if (!desc) +return -EINVAL ; + +return desc->slice_id; +} +EXPORT_SYMBOL_GPL (llcc_get_slice_id); + +/** +* llcc_get_slice_size - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and zero will returned on +* error +*/ +size_t llcc_get_slice_size (struct llcc_slice_desc *desc) +{ +if (!desc) +return 0 ; + +return desc->slice_size; +} +EXPORT_SYMBOL_GPL (llcc_get_slice_size); + +static int qcom_llcc_cfg_program (struct platform_device *pdev) +{ +int i; +u32 attr1_cfg; +u32 attr0_cfg; +u32 attr1_val; +u32 attr0_val; +u32 max_cap_cacheline; +u32 sz; +int ret =0 ; +const struct llcc_slice_config *llcc_table; +struct llcc_slice_desc desc; +struct llcc_drv_data *drv =platform_get_drvdata (pdev); +u32 bcast_off = drv->bcast_off; + + sz = drv->cfg_size; + llcc_table = drv->cfg; + +for (i =0 ; i < sz; i++) { + attr1_cfg = bcast_off + +LLCC_TRP_ATTR1_CFGn (llcc_table[i].slice_id); + attr0_cfg = bcast_off + +LLCC_TRP_ATTR0_CFGn (llcc_table[i].slice_id); + + attr1_val = llcc_table[i].cache_mode; + attr1_val |= llcc_table[i].probe_target_ways << +ATTR1_PROBE_TARGET_WAYS_SHIFT ; + attr1_val |= llcc_table[i].fixed_size << +ATTR1_FIXED_SIZE_SHIFT ; + attr1_val |= llcc_table[i].priority <<ATTR1_PRIORITY_SHIFT ; + + max_cap_cacheline =MAX_CAP_TO_BYTES (llcc_table[i].max_cap); + +/* LLCC instances can vary for each target. +* The SW writes to broadcast register which gets propagated +* to each llcc instace (llcc0,.. llccN). +* Since the size of the memory is divided equally amongst the +* llcc instances, we need to configure the max cap accordingly. +*/ + max_cap_cacheline = max_cap_cacheline / drv->num_banks; + max_cap_cacheline >>=CACHE_LINE_SIZE_SHIFT ; + attr1_val |= max_cap_cacheline <<ATTR1_MAX_CAP_SHIFT ; + + attr0_val = llcc_table[i].res_ways &ATTR0_RES_WAYS_MASK ; + attr0_val |= llcc_table[i].bonus_ways <<ATTR0_BONUS_WAYS_SHIFT ; + + ret =regmap_write (drv->regmap, attr1_cfg, attr1_val); +if (ret) +return ret; + ret =regmap_write (drv->regmap, attr0_cfg, attr0_val); +if (ret) +return ret; +if (llcc_table[i].activate_on_init) { + desc.slice_id = llcc_table[i].slice_id; + desc.dev = &pdev->dev; + ret =llcc_slice_activate (&desc); + } + } +return ret; +} + +int qcom_llcc_probe (struct platform_device *pdev, +const struct llcc_slice_config *llcc_cfg,u32 sz) +{ + +u32 num_banks =0 ;
drivers/soc/qcom/llcc-slice.c line 352
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+struct device *dev = &pdev->dev; +struct llcc_drv_data *drv_data; +struct resource *res; +void __iomem *base; +int ret =0 ; +int i; + + drv_data =devm_kzalloc (dev,sizeof (*drv_data),GFP_KERNEL ); +if (!drv_data) +return PTR_ERR (drv_data);
Oops, this is obviously not what you want. Should just be a return -ENOMEM.
Quoted 18 lines
drivers/soc/qcom/llcc-slice.c new file +18
-0+ res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (base)) +return PTR_ERR (base); + + drv_data->regmap =devm_regmap_init_mmio (dev, base, + &llcc_regmap_config); +if (IS_ERR (drv_data->regmap)) +return PTR_ERR (drv_data->regmap); + + ret =regmap_read (drv_data->regmap,LLCC_COMMON_STATUS0 , + &num_banks); +if (ret) +return ret; + + num_banks &=LLCC_LB_CNT_MASK ; + num_banks >>=LLCC_LB_CNT_SHIFT ; + drv_data->num_banks = num_banks;
drivers/soc/qcom/llcc-slice.c line 381
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+ + ret =of_property_read_u32 (pdev->dev.of_node,"max-slices" , + &drv_data->max_slices); +if (ret) +return ret; + + drv_data->offsets =devm_kzalloc (dev, num_banks *sizeof (u32), +GFP_KERNEL ); +if (!drv_data->offsets) +return PTR_ERR (drv_data->offsets);
And the same.
drivers/soc/qcom/llcc-slice.c line 392
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+for (i =0 ; i < num_banks; i++) + drv_data->offsets[i] = (i *BANK_OFFSET_STRIDE ); + + drv_data->bcast_off = num_banks *BANK_OFFSET_STRIDE ; + + drv_data->bitmap =devm_kcalloc (dev, +BITS_TO_LONGS (drv_data->max_slices),sizeof (unsigned long ), +GFP_KERNEL ); +if (!drv_data->bitmap) +return PTR_ERR (drv_data->bitmap);
Once again.
drivers/soc/qcom/llcc-slice.c line 402
Quoted 9 lines
drivers/soc/qcom/llcc-slice.c new file +9
-0+bitmap_zero (drv_data->bitmap, drv_data->max_slices); + drv_data->cfg = llcc_cfg; + drv_data->cfg_size = sz; +mutex_init (&drv_data->lock); +platform_set_drvdata (pdev, drv_data); + + ret =qcom_llcc_cfg_program (pdev); + +return ret;
return qcom_llc_cfg_program(pdev);
but thats my own personal nit to pick.
Quoted 64 lines
drivers/soc/qcom/llcc-slice.c new file +2
-0+} +EXPORT_SYMBOL_GPL (qcom_llcc_probe);include/linux/soc/qcom/llcc-qcom.h new file +56
-0@@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. +* +*/ + +#include <linux/platform_device.h> +#ifndef __LLCC_QCOM__ +#define __LLCC_QCOM__ +/** +* llcc_slice_desc - Cache slice descriptor +* @slice_id: llcc slice id +* @slice_size: Size allocated for the llcc slice +* @dev: pointer to llcc device +*/ +struct llcc_slice_desc { +u32 slice_id; +size_t slice_size; +struct device *dev; +}; + +/** +* llcc_slice_config - Data associated with the llcc slice +* @name: name of the use case associated with the llcc slice +* @usecase_id: usecase id for which the llcc slice is used +* @slice_id: llcc slice id assigned to each slice +* @max_cap: maximum capacity of the llcc slice +* @priority: priority of the llcc slice +* @fixed_size: whether the llcc slice can grow beyond its size +* @bonus_ways: bonus ways associated with llcc slice +* @res_ways: reserved ways associated with llcc slice +* @cache_mode: mode of the llcc slice +* @probe_target_ways: Probe only reserved and bonus ways on a cache miss +* @dis_cap_alloc: Disable capacity based allocation +* @retain_on_pc: Retain through power collapse +* @activate_on_init: activate the slice on init +*/ +struct llcc_slice_config { +const char *name; +u32 usecase_id; +u32 slice_id; +u32 max_cap; +u32 priority; +bool fixed_size; +u32 bonus_ways; +u32 res_ways; +u32 cache_mode; +u32 probe_target_ways; +bool dis_cap_alloc; +bool retain_on_pc; +bool activate_on_init; +}; + +/** +* llcc_drv_data - Data associated with the llcc driver +* @regmap: regmap associated with the llcc device
include/linux/soc/qcom/llcc-qcom.h line 66
Quoted 10 lines
include/linux/soc/qcom/llcc-qcom.h new file +10
-0+* @cfg: pointer to the data structure for slice configuration +* @lock: mutex associated with each slice +* @cfg_size: size of the config data table +* @max_slices: max slices as read from device tree +* @bcast_off: Offset of the broadcast bank +* @num_banks: Number of llcc banks +* @bitmap: Bit map to track the active slice ids +* @offsets: Pointer to the bank offsets array +*/ +
I'm no kerneldocs expert I think the blank line is frowned upon here.
Quoted 102 lines
include/linux/soc/qcom/llcc-qcom.h new file +102
-0+struct llcc_drv_data { +struct regmap *regmap; +const struct llcc_slice_config *cfg; +struct mutex lock; +u32 cfg_size; +u32 max_slices; +u32 bcast_off; +u32 num_banks; +unsigned long *bitmap; +u32 *offsets; +}; + + +#if IS_ENABLED (CONFIG_QCOM_LLCC ) +/** +* llcc_slice_getd - get llcc slice descriptor +* @dev: Device pointer of the client +* @name: Name of the use case +*/ +struct llcc_slice_desc *llcc_slice_getd (struct device *dev,const char *name); + +/** +* llcc_slice_putd - llcc slice descritpor +* @desc: Pointer to llcc slice descriptor +*/ +void llcc_slice_putd (struct llcc_slice_desc *desc); + +/** +* llcc_get_slice_id - get slice id +* @desc: Pointer to llcc slice descriptor +*/ +int llcc_get_slice_id (struct llcc_slice_desc *desc); + +/** +* llcc_get_slice_size - llcc slice size +* @desc: Pointer to llcc slice descriptor +*/ +size_t llcc_get_slice_size (struct llcc_slice_desc *desc); + +/** +* llcc_slice_activate - Activate the llcc slice +* @desc: Pointer to llcc slice descriptor +*/ +int llcc_slice_activate (struct llcc_slice_desc *desc); + +/** +* llcc_slice_deactivate - Deactivate the llcc slice +* @desc: Pointer to llcc slice descriptor +*/ +int llcc_slice_deactivate (struct llcc_slice_desc *desc); + +/** +* qcom_llcc_probe - program the sct table +* @pdev: platform device pointer +* @table: soc sct table +* @sz: Size of the config table +*/ +int qcom_llcc_probe (struct platform_device *pdev, +const struct llcc_slice_config *table,u32 sz); +#else +static inline struct llcc_slice_desc *llcc_slice_getd (struct device *dev, +const char *name) +{ +return NULL ; +} + +static inline void llcc_slice_putd (struct llcc_slice_desc *desc) +{ + +}; + +static inline int llcc_get_slice_id (struct llcc_slice_desc *desc) +{ +return -EINVAL ; +} + +static inline size_t llcc_get_slice_size (struct llcc_slice_desc *desc) +{ +return 0 ; +} +static inline int llcc_slice_activate (struct llcc_slice_desc *desc) +{ +return -EINVAL ; +} + +static inline int llcc_slice_deactivate (struct llcc_slice_desc *desc) +{ +return -EINVAL ; +} +static inline int qcom_llcc_probe (struct platform_device *pdev, +const struct llcc_slice_config *table,u32 sz) +{ +return -ENODEV ; +} + +static inline int qcom_llcc_remove (struct platform_device *pdev) +{ +return -ENODEV ; +} +#endif + +#endif
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v4 2/2] drivers: soc: Add LLCC driver
In reply to Rishabh BhatnagarHi Rishabh,
Quoted 8 lines · Rishabh Bhatnagar
LLCC (Last Level Cache Controller) provides additional cache memory in the system. LLCC is partitioned into multiple slices and each slice gets its own priority, size, ID and other config parameters. LLCC driver programs these parameters for each slice. Clients that are assigned to use LLCC need to get information such size & ID of the slice they get and activate or deactivate the slice as needed. LLCC driver provides API for the clients to perform these operations.
Quoted 7 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- drivers/soc/qcom/Kconfig | 17 ++ drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++++++ drivers/soc/qcom/llcc-slice.c | 404
+++++++++++++++++++++++++++++++++++++
Quoted 5 lines
include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++++++++ 5 files changed, 701 insertions(+)create mode 100644
drivers/soc/qcom/llcc-sdm845.ccreate mode 100644drivers/soc/qcom/llcc-slice.ccreate mode 100644include/linux/soc/qcom/llcc-qcom.h
[...]
Review
Quoted 1 line
(quoted diff) +0
-0
b/drivers/soc/qcom/llcc-sdm845.c
Quoted 9 lines
new file mode 100644
index 0000000..619b226drivers/soc/qcom/llcc-sdm845.c new file +4
-0@@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. +*
drivers/soc/qcom/llcc-sdm845.c line 14
Quoted 10 lines
drivers/soc/qcom/llcc-sdm845.c new file +10
-0+*/ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/soc/qcom/llcc-qcom.h> + +/* +* SCT(System Cache Table) entry contains of the following parameters
contains the following members:
drivers/soc/qcom/llcc-sdm845.c line 16
Quoted 2 lines
drivers/soc/qcom/llcc-sdm845.c new file +2
-0+* name: Name of the client's use case for which the llcc slice is used +* uid: Unique id for the client's use case
s/uid/usecase_id/
drivers/soc/qcom/llcc-sdm845.c
Quoted 3 lines
drivers/soc/qcom/llcc-sdm845.c new file +3
-0+* slice_id: llcc slice id for each client +* max_cap: The maximum capacity of the cache slice provided in KB +* priority: Priority of the client used to select victim line for
replacement
drivers/soc/qcom/llcc-sdm845.c line 20
Quoted 1 line
drivers/soc/qcom/llcc-sdm845.c new file +1
-0+* fixed_size: Determine if the slice has a fixed capacity
"Boolean indicating if the slice has a fixed capacity" might be better
Quoted 5 lines
drivers/soc/qcom/llcc-slice.c new file +0
-0
...
drivers/soc/qcom/llcc-slice.c line 153
Quoted 12 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+static int llcc_update_act_ctrl (struct llcc_drv_data *drv,u32 sid, +u32 act_ctrl_reg_val,u32 status) +{ +u32 act_ctrl_reg; +u32 status_reg; +u32 slice_status; +int ret =0 ; + + act_ctrl_reg = drv->bcast_off +LLCC_TRP_ACT_CTRLn (sid); + status_reg = drv->bcast_off +LLCC_TRP_STATUSn (sid); + +/*Set the ACTIVE trigger*/
Add spaces around /* */
drivers/soc/qcom/llcc-slice.c
Quoted 12 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+ act_ctrl_reg_val |=ACT_CTRL_ACT_TRIG ; + ret =regmap_write (drv->regmap, act_ctrl_reg, act_ctrl_reg_val); +if (ret) +return ret; + +/* Clear the ACTIVE trigger */ + act_ctrl_reg_val &= ~ACT_CTRL_ACT_TRIG ; + ret =regmap_write (drv->regmap, act_ctrl_reg, act_ctrl_reg_val); +if (ret) +return ret; + + ret = regmap_read_poll_timeout(drv->regmap, status_reg,
slice_status,
Quoted 1 line
+ !(slice_status & status), 0,
LLCC_STATUS_READ_DELAY);
drivers/soc/qcom/llcc-slice.c line 174
Quoted 8 lines
drivers/soc/qcom/llcc-slice.c new file +8
-0+return ret; +} + +/** +* llcc_slice_activate - Activate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will
a value of zero
drivers/soc/qcom/llcc-slice.c line 184
Quoted 10 lines
+ * be returned in error cases
+ */drivers/soc/qcom/llcc-slice.c new file +8
-0+int llcc_slice_activate (struct llcc_slice_desc *desc) +{ +int ret; +u32 act_ctrl_val; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ;
I think we can remove this check, right?
Quoted 39 lines
drivers/soc/qcom/llcc-slice.c new file +39
-0+ + drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + + act_ctrl_val =ACT_CTRL_OPCODE_ACTIVATE <<ACT_CTRL_OPCODE_SHIFT ; + + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +DEACTIVATE ); + +__set_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_activate); + +/** +* llcc_slice_deactivate - Deactivate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will +* be returned in error cases +*/ +int llcc_slice_deactivate (struct llcc_slice_desc *desc) +{ +u32 act_ctrl_val; +int ret; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ; +
drivers/soc/qcom/llcc-slice.c
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+ drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (!test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;
Quoted 12 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +ACTIVATE ); + +__clear_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_deactivate); + +/**
drivers/soc/qcom/llcc-slice.c line 255
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+* llcc_get_slice_id - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and a negative errno will +* be returned on error +*/ +int llcc_get_slice_id (struct llcc_slice_desc *desc) +{ +if (!desc) +return -EINVAL ;
Can we remove this check too?
Quoted 6 lines
drivers/soc/qcom/llcc-slice.c new file +6
-0+ +return desc->slice_id; +} +EXPORT_SYMBOL_GPL (llcc_get_slice_id); + +/**
drivers/soc/qcom/llcc-slice.c line 271
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+* llcc_get_slice_size - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and zero will returned on +* error +*/ +size_t llcc_get_slice_size (struct llcc_slice_desc *desc) +{ +if (!desc) +return 0 ;
And this one?
-Evan
Re: [PATCH v4 2/2] drivers: soc: Add LLCC driver
Quoted 200 lines · Evan Green
Hi Rishabh,
Quoted 9 lines · Rishabh Bhatnagar
LLCC (Last Level Cache Controller) provides additional cache memory in the system. LLCC is partitioned into multiple slices and each slice gets its own priority, size, ID and other config parameters. LLCC driver programs these parameters for each slice. Clients that are assigned to use LLCC need to get information such size & ID of the slice they get and activate or deactivate the slice as needed. LLCC driver
provides API for the clients to perform these operations.Quoted 7 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- drivers/soc/qcom/Kconfig | 17 ++ drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++++++ drivers/soc/qcom/llcc-slice.c | 404+++++++++++++++++++++++++++++++++++++
Quoted 5 lines
include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++++++++ 5 files changed, 701 insertions(+)create mode 100644
drivers/soc/qcom/llcc-sdm845.ccreate mode 100644drivers/soc/qcom/llcc-slice.ccreate mode 100644include/linux/soc/qcom/llcc-qcom.h[...]
Review
Quoted 1 line
(quoted diff) +0
-0
b/drivers/soc/qcom/llcc-sdm845.cQuoted 11 lines
new file mode 100644
index 0000000..619b226drivers/soc/qcom/llcc-sdm845.c new file +3
-0@@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ */drivers/soc/qcom/llcc-sdm845.c line 14
Quoted 9 lines
drivers/soc/qcom/llcc-sdm845.c new file +9
-0+ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/soc/qcom/llcc-qcom.h> + +/* +* SCT(System Cache Table) entry contains of the following parameters contains the following members:
Quoted 3 lines
+ * name: Name of the client's use case for which the llcc slice is used
+ * uid: Unique id for the client's use cases/uid/usecase_id/
Quoted 3 lines
+ *
slice_id: llcc slice id for each client
+ *max_cap: The maximum capacity of the cache slice provided in KB
+ * priority: Priority of the client used to select victim line forreplacement
Quoted 1 line
+ *
fixed_size: Determine if the slice has a fixed capacity"Boolean indicating if the slice has a fixed capacity" might be better
Quoted 6 lines
drivers/soc/qcom/llcc-slice.c new file +0
-0...
drivers/soc/qcom/llcc-slice.c line 153
Quoted 12 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+static int llcc_update_act_ctrl (struct llcc_drv_data *drv,u32 sid, +u32 act_ctrl_reg_val,u32 status) +{ +u32 act_ctrl_reg; +u32 status_reg; +u32 slice_status; +int ret =0 ; + + act_ctrl_reg = drv->bcast_off +LLCC_TRP_ACT_CTRLn (sid); + status_reg = drv->bcast_off +LLCC_TRP_STATUSn (sid); + +/*Set the ACTIVE trigger*/ Add spaces around /* */
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +2
-0+ act_ctrl_reg_val |=ACT_CTRL_ACT_TRIG ; + ret = regmap_write(drv->regmap, act_ctrl_reg,
act_ctrl_reg_val);drivers/soc/qcom/llcc-slice.c new file +6
-0+if (ret) +return ret; + +/* Clear the ACTIVE trigger */ + act_ctrl_reg_val &= ~ACT_CTRL_ACT_TRIG ; + ret = regmap_write(drv->regmap, act_ctrl_reg,
act_ctrl_reg_val);drivers/soc/qcom/llcc-slice.c
Quoted 4 lines
drivers/soc/qcom/llcc-slice.c new file +4
-0+if (ret) +return ret; + + ret = regmap_read_poll_timeout(drv->regmap, status_reg,
slice_status,Quoted 1 line
+ !(slice_status & status), 0,
LLCC_STATUS_READ_DELAY);drivers/soc/qcom/llcc-slice.c line 174
Quoted 8 lines
drivers/soc/qcom/llcc-slice.c new file +8
-0+return ret; +} + +/** +* llcc_slice_activate - Activate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will a value of zero
drivers/soc/qcom/llcc-slice.c line 184
Quoted 10 lines
+ * be returned in error cases
+ */drivers/soc/qcom/llcc-slice.c new file +8
-0+int llcc_slice_activate (struct llcc_slice_desc *desc) +{ +int ret; +u32 act_ctrl_val; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ;I think we can remove this check, right?
Quoted 40 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+ + drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + + act_ctrl_val = ACT_CTRL_OPCODE_ACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;drivers/soc/qcom/llcc-slice.c new file +27
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +DEACTIVATE ); + +__set_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_activate); + +/** +* llcc_slice_deactivate - Deactivate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will +* be returned in error cases +*/ +int llcc_slice_deactivate (struct llcc_slice_desc *desc) +{ +u32 act_ctrl_val; +int ret; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ; +drivers/soc/qcom/llcc-slice.c
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+ drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (!test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;Quoted 17 lines
drivers/soc/qcom/llcc-slice.c new file +16
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +ACTIVATE ); + +__clear_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_deactivate); + +/** +* llcc_get_slice_id - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and a negative errno will
Quoted 9 lines
drivers/soc/qcom/llcc-slice.c line 255
Quoted 6 lines
+ * be returned on error
+ */drivers/soc/qcom/llcc-slice.c new file +4
-0+int llcc_get_slice_id (struct llcc_slice_desc *desc) +{ +if (!desc) +return -EINVAL ;Can we remove this check too?
We need this check and the following one to protect against the null pointer access which might happen if client doesn't get the descriptor before accessing size and slice_id.
Quoted 21 lines
Quoted 13 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+ +return desc->slice_id; +} +EXPORT_SYMBOL_GPL (llcc_get_slice_id); + +/** +* llcc_get_slice_size - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and zero will returned on
+ * error
+ */drivers/soc/qcom/llcc-slice.c line 271
Quoted 4 lines
drivers/soc/qcom/llcc-slice.c new file +4
-0+size_t llcc_get_slice_size (struct llcc_slice_desc *desc) +{ +if (!desc) +return 0 ;And this one?
-Evan
Re: [PATCH v4 2/2] drivers: soc: Add LLCC driver
Quoted 210 lines · On Fri, Apr 13, 2018 at 4:08 PM <[email protected]> wrote:
Quoted 200 lines · Evan Green
Hi Rishabh,
Quoted 9 lines · Rishabh Bhatnagar
LLCC (Last Level Cache Controller) provides additional cache memory in the system. LLCC is partitioned into multiple slices and each slice gets its own priority, size, ID and other config parameters. LLCC driver programs these parameters for each slice. Clients that are assigned to use LLCC need to get information such size & ID of the slice they get and activate or deactivate the slice as needed. LLCC driver
provides API for the clients to perform these operations.Quoted 7 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- drivers/soc/qcom/Kconfig | 17 ++ drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++++++ drivers/soc/qcom/llcc-slice.c | 404+++++++++++++++++++++++++++++++++++++
Quoted 5 lines
include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++++++++ 5 files changed, 701 insertions(+)create mode 100644
drivers/soc/qcom/llcc-sdm845.ccreate mode 100644drivers/soc/qcom/llcc-slice.ccreate mode 100644include/linux/soc/qcom/llcc-qcom.h[...]
Review
Quoted 1 line
(quoted diff) +0
-0
b/drivers/soc/qcom/llcc-sdm845.cQuoted 11 lines
new file mode 100644
index 0000000..619b226drivers/soc/qcom/llcc-sdm845.c new file +3
-0@@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ */drivers/soc/qcom/llcc-sdm845.c line 14
Quoted 9 lines
drivers/soc/qcom/llcc-sdm845.c new file +9
-0+ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/soc/qcom/llcc-qcom.h> + +/* +* SCT(System Cache Table) entry contains of the following parameters contains the following members:
Quoted 3 lines
+ * name: Name of the client's use case for which the llcc slice is used
+ * uid: Unique id for the client's use cases/uid/usecase_id/
Quoted 3 lines
+ *
slice_id: llcc slice id for each client
+ *max_cap: The maximum capacity of the cache slice provided in KB
+ * priority: Priority of the client used to select victim line forreplacement
Quoted 1 line
+ *
fixed_size: Determine if the slice has a fixed capacity"Boolean indicating if the slice has a fixed capacity" might be better
Quoted 6 lines
drivers/soc/qcom/llcc-slice.c new file +0
-0...
drivers/soc/qcom/llcc-slice.c line 153
Quoted 12 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+static int llcc_update_act_ctrl (struct llcc_drv_data *drv,u32 sid, +u32 act_ctrl_reg_val,u32 status) +{ +u32 act_ctrl_reg; +u32 status_reg; +u32 slice_status; +int ret =0 ; + + act_ctrl_reg = drv->bcast_off +LLCC_TRP_ACT_CTRLn (sid); + status_reg = drv->bcast_off +LLCC_TRP_STATUSn (sid); + +/*Set the ACTIVE trigger*/ Add spaces around /* */
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +2
-0+ act_ctrl_reg_val |=ACT_CTRL_ACT_TRIG ; + ret = regmap_write(drv->regmap, act_ctrl_reg,
act_ctrl_reg_val);drivers/soc/qcom/llcc-slice.c new file +6
-0+if (ret) +return ret; + +/* Clear the ACTIVE trigger */ + act_ctrl_reg_val &= ~ACT_CTRL_ACT_TRIG ; + ret = regmap_write(drv->regmap, act_ctrl_reg,
act_ctrl_reg_val);drivers/soc/qcom/llcc-slice.c
Quoted 4 lines
drivers/soc/qcom/llcc-slice.c new file +4
-0+if (ret) +return ret; + + ret = regmap_read_poll_timeout(drv->regmap, status_reg,
slice_status,Quoted 1 line
+ !(slice_status & status), 0,
LLCC_STATUS_READ_DELAY);drivers/soc/qcom/llcc-slice.c line 174
Quoted 8 lines
drivers/soc/qcom/llcc-slice.c new file +8
-0+return ret; +} + +/** +* llcc_slice_activate - Activate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will a value of zero
drivers/soc/qcom/llcc-slice.c line 184
Quoted 10 lines
+ * be returned in error cases
+ */drivers/soc/qcom/llcc-slice.c new file +8
-0+int llcc_slice_activate (struct llcc_slice_desc *desc) +{ +int ret; +u32 act_ctrl_val; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ;I think we can remove this check, right?
Quoted 40 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+ + drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + + act_ctrl_val = ACT_CTRL_OPCODE_ACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;drivers/soc/qcom/llcc-slice.c new file +27
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +DEACTIVATE ); + +__set_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_activate); + +/** +* llcc_slice_deactivate - Deactivate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will +* be returned in error cases +*/ +int llcc_slice_deactivate (struct llcc_slice_desc *desc) +{ +u32 act_ctrl_val; +int ret; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ; +drivers/soc/qcom/llcc-slice.c
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+ drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (!test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;Quoted 17 lines
drivers/soc/qcom/llcc-slice.c new file +16
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +ACTIVATE ); + +__clear_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_deactivate); + +/** +* llcc_get_slice_id - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and a negative errno will
Quoted 9 lines
drivers/soc/qcom/llcc-slice.c line 255
Quoted 6 lines
+ * be returned on error
+ */drivers/soc/qcom/llcc-slice.c new file +4
-0+int llcc_get_slice_id (struct llcc_slice_desc *desc) +{ +if (!desc) +return -EINVAL ;Can we remove this check too?
Quoted 3 lines
We need this check and the following one to protect against the null pointer access which might happen if client doesn't get the descriptor before accessing size and
slice_id.
Is this just to protect against errors made during development, or is there an expected code path through which this might actually happen? If it's just to protect against developer error, then based on other feedback I've seen on these lists, the null pointer dereference is preferred, and we should remove this check. If it's an actual expected flow, then I guess clients will call llcc_slice_getd(), see the ERR_PTR(-ENOENT), transform that into NULL, and then call these other functions anyway?
-Evan
Re: [PATCH v4 2/2] drivers: soc: Add LLCC driver
Quoted 216 lines · Evan Green
Quoted 210 lines · On Fri, Apr 13, 2018 at 4:08 PM <[email protected]> wrote:
Quoted 200 lines · Evan Green
Hi Rishabh,
Quoted 9 lines · Rishabh Bhatnagar
LLCC (Last Level Cache Controller) provides additional cache memory in the system. LLCC is partitioned into multiple slices and each slice gets its own priority, size, ID and other config parameters. LLCC driver programs these parameters for each slice. Clients that are assigned to use LLCC need to get information such size & ID of the slice they get and activate or deactivate the slice as needed. LLCC driver
provides API for the clients to perform these operations.Quoted 7 lines
Signed-off-by Channagoud Kadabi <[email protected]>Signed-off-by Rishabh Bhatnagar <[email protected]>--- drivers/soc/qcom/Kconfig | 17 ++ drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/llcc-sdm845.c | 110 ++++++++++ drivers/soc/qcom/llcc-slice.c | 404+++++++++++++++++++++++++++++++++++++
Quoted 5 lines
include/linux/soc/qcom/llcc-qcom.h | 168 +++++++++++++++ 5 files changed, 701 insertions(+)create mode 100644
drivers/soc/qcom/llcc-sdm845.ccreate mode 100644drivers/soc/qcom/llcc-slice.ccreate mode 100644include/linux/soc/qcom/llcc-qcom.h[...]
Review
Quoted 1 line
(quoted diff) +0
-0
b/drivers/soc/qcom/llcc-sdm845.cQuoted 11 lines
new file mode 100644
index 0000000..619b226drivers/soc/qcom/llcc-sdm845.c new file +3
-0@@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ */drivers/soc/qcom/llcc-sdm845.c line 14
Quoted 9 lines
drivers/soc/qcom/llcc-sdm845.c new file +9
-0+ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/soc/qcom/llcc-qcom.h> + +/* +* SCT(System Cache Table) entry contains of the following parameters contains the following members:
Quoted 3 lines
+ * name: Name of the client's use case for which the llcc slice is used
+ * uid: Unique id for the client's use cases/uid/usecase_id/
Quoted 3 lines
+ *
slice_id: llcc slice id for each client
+ *max_cap: The maximum capacity of the cache slice provided in KB
+ * priority: Priority of the client used to select victim line forreplacement
Quoted 1 line
+ *
fixed_size: Determine if the slice has a fixed capacity"Boolean indicating if the slice has a fixed capacity" might be better
Quoted 6 lines
drivers/soc/qcom/llcc-slice.c new file +0
-0...
drivers/soc/qcom/llcc-slice.c line 153
Quoted 12 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+static int llcc_update_act_ctrl (struct llcc_drv_data *drv,u32 sid, +u32 act_ctrl_reg_val,u32 status) +{ +u32 act_ctrl_reg; +u32 status_reg; +u32 slice_status; +int ret =0 ; + + act_ctrl_reg = drv->bcast_off +LLCC_TRP_ACT_CTRLn (sid); + status_reg = drv->bcast_off +LLCC_TRP_STATUSn (sid); + +/*Set the ACTIVE trigger*/ Add spaces around /* */
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +2
-0+ act_ctrl_reg_val |=ACT_CTRL_ACT_TRIG ; + ret = regmap_write(drv->regmap, act_ctrl_reg,
act_ctrl_reg_val);drivers/soc/qcom/llcc-slice.c new file +6
-0+if (ret) +return ret; + +/* Clear the ACTIVE trigger */ + act_ctrl_reg_val &= ~ACT_CTRL_ACT_TRIG ; + ret = regmap_write(drv->regmap, act_ctrl_reg,
act_ctrl_reg_val);drivers/soc/qcom/llcc-slice.c
Quoted 4 lines
drivers/soc/qcom/llcc-slice.c new file +4
-0+if (ret) +return ret; + + ret = regmap_read_poll_timeout(drv->regmap, status_reg,
slice_status,Quoted 1 line
+ !(slice_status & status), 0,
LLCC_STATUS_READ_DELAY);drivers/soc/qcom/llcc-slice.c line 174
Quoted 8 lines
drivers/soc/qcom/llcc-slice.c new file +8
-0+return ret; +} + +/** +* llcc_slice_activate - Activate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will a value of zero
drivers/soc/qcom/llcc-slice.c line 184
Quoted 10 lines
+ * be returned in error cases
+ */drivers/soc/qcom/llcc-slice.c new file +8
-0+int llcc_slice_activate (struct llcc_slice_desc *desc) +{ +int ret; +u32 act_ctrl_val; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ;I think we can remove this check, right?
Quoted 40 lines
drivers/soc/qcom/llcc-slice.c new file +12
-0+ + drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + + act_ctrl_val = ACT_CTRL_OPCODE_ACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;drivers/soc/qcom/llcc-slice.c new file +27
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +DEACTIVATE ); + +__set_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_activate); + +/** +* llcc_slice_deactivate - Deactivate the llcc slice +* @desc: Pointer to llcc slice descriptor +* +* A value zero will be returned on success and a negative errno will +* be returned in error cases +*/ +int llcc_slice_deactivate (struct llcc_slice_desc *desc) +{ +u32 act_ctrl_val; +int ret; +struct llcc_drv_data *drv; + +if (desc ==NULL ) +return -EINVAL ; +drivers/soc/qcom/llcc-slice.c
Quoted 10 lines
drivers/soc/qcom/llcc-slice.c new file +10
-0+ drv =dev_get_drvdata (desc->dev); +if (!drv) +return -EINVAL ; + +mutex_lock (&drv->lock); +if (!test_bit (desc->slice_id, drv->bitmap)) { +mutex_unlock (&drv->lock); +return 0 ; + } + act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE <<
ACT_CTRL_OPCODE_SHIFT;Quoted 17 lines
drivers/soc/qcom/llcc-slice.c new file +16
-0+ + ret =llcc_update_act_ctrl (drv, desc->slice_id, act_ctrl_val, +ACTIVATE ); + +__clear_bit (desc->slice_id, drv->bitmap); +mutex_unlock (&drv->lock); + +return ret; +} +EXPORT_SYMBOL_GPL (llcc_slice_deactivate); + +/** +* llcc_get_slice_id - return the slice id +* @desc: Pointer to llcc slice descriptor +* +* A positive value will be returned on success and a negative errno will
Quoted 9 lines
drivers/soc/qcom/llcc-slice.c line 255
Quoted 6 lines
+ * be returned on error
+ */drivers/soc/qcom/llcc-slice.c new file +4
-0+int llcc_get_slice_id (struct llcc_slice_desc *desc) +{ +if (!desc) +return -EINVAL ;Can we remove this check too?
Quoted 3 lines
We need this check and the following one to protect against the null pointer access which might happen if client doesn't get the descriptor before accessing size and
slice_id.
Quoted 11 lines
Is this just to protect against errors made during development, or is there
an expected code path through which this might actually happen? If it's just to protect against developer error, then based on other feedback I've
seen on these lists, the null pointer dereference is preferred, and we should remove this check. If it's an actual expected flow, then I guess clients will callllcc_slice_getd(), see theERR_PTR(-ENOENT), transform
that into NULL, and then call these other functions anyway?
Yes, this is just to protect against development error. I will remove the
check here and above.
-Rishabh
Re: [PATCH v4 2/2] drivers: soc: Add LLCC driver
In reply to Rishabh BhatnagarHi Rishabh,
Quoted 1 line
+MODULE_DESCRIPTION("QTI sdm845 LLCC driver");
I think it should be QCOM or Qualcomm and not QTI
Review
Quoted 2 lines
(quoted diff) +2
-0+ + desc = devm_kzalloc(dev, sizeof(struct llcc_slice_desc), GFP_KERNEL);
Can use *desc instead
drivers/soc/qcom/llcc-slice.c line 118
Quoted 10 lines
+struct
llcc_slice_desc*llcc_slice_getd(struct device *dev, const char *name)drivers/soc/qcom/llcc-slice.c new file +8
-0+{ +struct device_node *np = dev->of_node; +int index =0 ; +const char *slice_name; +struct property *prop; + +if (!np) +return ERR_PTR (-ENOENT );
Is this check required?
include/linux/soc/qcom/llcc-qcom.h line 1
Quoted 8 lines
include/linux/soc/qcom/llcc-qcom.h new file +1
-0@@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0
Should be within /* */ for headers as per kernel licensing rules.
Regards,
Sai Prakash