soc: qcom: Add Qualcomm Memshare QMI service
cover soc: qcom: Add Qualcomm Memshare QMI service
From: Nikita Travkin <[email protected]>
This series adds memshare QMI service.
It receives messages from other subsystems (like modem subsystem) and answers with a message that contains the memory address of the allocated region. This is used on many msm8916 based devices for location service which requests 2 MB region when the modem starts. If the memory isn't provided, GPS doesn't work. Newer platforms may also use it in other subsystems.
The driver implements:
- legacy alloc/free messages (known to be used by Samsung A2015 devices)
- generic alloc/free messages (used on most msm8916 devices)
- query_size message (sent by at least one modem firmware for msm8916)
Downstream driver [1] seems to use dynamic memory allocation but it uses concept of "Guaranteed Memory" which means that the region is allocated on driver probe and not released even after "free" request. It also seems to support memory allocation "on demand" without prior client description.
My driver implements "guaranteed" allocation using reserved-memory regions and does not allow allocation for any request that was not described in the device tree. It also additionally checks that qrtr node that sent the message is correct (to prevent, for example, processing messages sent from userspace).
memshare_qmi_msg.c/h files are mostly copied from [1] (query_size was added from later version of the driver) and cleaned up. Those files keep copyright line from the originals.
This driver is tested to work on multiple msm8916 devices, including devices that have dts upstream (with an exception of mtp; db410c doesn't use it).
Nikita Travkin (2):
dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service
soc: qcom: Add Qualcomm Memshare QMI service
.../bindings/soc/qcom/qcom,memshare.yaml | 109 ++++ drivers/soc/qcom/Kconfig | 8 + drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/memshare.c | 501 ++++++++++++++++++ drivers/soc/qcom/memshare_qmi_msg.c | 370 +++++++++++++ drivers/soc/qcom/memshare_qmi_msg.h | 228 ++++++++ include/dt-bindings/soc/qcom,memshare.h | 10 + 7 files changed, 1228 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml create mode 100644 drivers/soc/qcom/memshare.c create mode 100644 drivers/soc/qcom/memshare_qmi_msg.c create mode 100644 drivers/soc/qcom/memshare_qmi_msg.h create mode 100644 include/dt-bindings/soc/qcom,memshare.h
Signature · 1 line
2.27.0
PATCH 1/2 dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service
From: Nikita Travkin <[email protected]>
Add DT bindings for memshare: QMI service that allocates memory per remote processor request.
--- .../bindings/soc/qcom/qcom,memshare.yaml | 109 ++++++++++++++++++ include/dt-bindings/soc/qcom,memshare.h | 10 ++ 2 files changed, 119 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml create mode 100644 include/dt-bindings/soc/qcom,memshare.h
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +109 -0
@@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/soc/qcom/qcom,memshare.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm QMI Shared Memory Service + +description: | + This driver provides a QMI service that allows remote processors (like modem) + to request additional memory. It is used for applications like GPS in modem. + +maintainers: + - Nikita Travkin <[email protected]> + +properties: + compatible: + const: qcom,memshare + + qcom,legacy-client: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to a memshare client node used for legacy requests. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: Proc-ID for clients in this node. + + qcom,qrtr-node: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Node from which the requests are expected. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: ID of this client. + + memory-region: + $ref: /schemas/types.yaml#/definitions/phandle + description: | + Reserved memory region that should be used for allocation. + + required: + - reg + + required: + - reg + - qcom,qrtr-node + +required: + - compatible + +additionalProperties: false + +examples: + - | + #include <dt-bindings/soc/qcom,memshare.h> + + reserved-memory { + + #address-cells = <2>; + #size-cells = <2>; + + gps_mem: gps@93c00000 { + reg = <0x0 0x93c00000 0x0 0x200000>; + no-map; + }; + }; + + memshare { + compatible = "qcom,memshare"; + qcom,legacy-client = <&memshare_gps>; + + #address-cells = <1>; + #size-cells = <0>; + + mpss@0 { + reg = <MEMSHARE_PROC_MPSS_V01>; + qcom,qrtr-node = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + memshare_gps: gps@0 { + reg = <0>; + memory-region = <&gps_mem>; + }; + }; + }; + +...
include/dt-bindings/soc/qcom,memshare.h new file +10 -0
@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __DT_QCOM_MEMSHARE_H__ +#define __DT_QCOM_MEMSHARE_H__ + +#define MEMSHARE_PROC_MPSS_V01 0 +#define MEMSHARE_PROC_ADSP_V01 1 +#define MEMSHARE_PROC_WCNSS_V01 2 + +#endif /* __DT_QCOM_MEMSHARE_H__ */
Signature · 1 line
2.27.0
Rob Herring
maintainerRe: [PATCH 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service
Quoted 19 lines · On Fri, Mar 19, 2021 at 10:23:20PM +0500, [email protected] wrote:
From: Nikita Travkin <[email protected]>
Add DT bindings for memshare: QMI service that allocates memory per remote processor request.
Signed-off-by Nikita Travkin <[email protected]>--- .../bindings/soc/qcom/qcom,memshare.yaml | 109 ++++++++++++++++++ include/dt-bindings/soc/qcom,memshare.h | 10 ++ 2 files changed, 119 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml create mode 100644 include/dt-bindings/soc/qcom,memshare.hDocumentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +0
-0
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0@@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/soc/qcom/qcom,memshare.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm QMI Shared Memory Service
How many shared memory interfaces does Qcom have...
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +4
-0+ +description: | + This driver provides a QMI service that allows remote processors (like modem) + to request additional memory. It is used for applications like GPS in modem.
If the memory region is defined in reserved-memory, how are you allocating additional memory?
Quoted 15 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +15
-0+ +maintainers: + - Nikita Travkin <[email protected]> + +properties: + compatible: + const: qcom,memshare + + qcom,legacy-client: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to a memshare client node used for legacy requests. + + "#address-cells": + const: 1 +
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 36
Quoted 10 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +10
-0+ "#size-cells": + const: 0 + +patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: Proc-ID for clients in this node.
What's Proc-ID?
Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +8
-0+ + qcom,qrtr-node: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Node from which the requests are expected. + + "#address-cells": + const: 1 +
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 54
Quoted 10 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +10
-0+ "#size-cells": + const: 0 + + patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: ID of this client.
How does one determine the ID?
Quoted 20 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +20
-0+ + memory-region: + $ref: /schemas/types.yaml#/definitions/phandle + description: | + Reserved memory region that should be used for allocation. + + required: + - reg + + required: + - reg + - qcom,qrtr-node + +required: + - compatible + +additionalProperties: false + +examples: + - |
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 84
Quoted 10 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +10
-0+ #include <dt-bindings/soc/qcom,memshare.h> + + reserved-memory { + + #address-cells = <2>; + #size-cells = <2>; + + gps_mem: gps@93c00000 { + reg = <0x0 0x93c00000 0x0 0x200000>; + no-map;
We support 'compatible' in reserved-memory nodes, can you simplify the binding and put everything in here?
Quoted 44 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +25
-0+ }; + }; + + memshare { + compatible = "qcom,memshare"; + qcom,legacy-client = <&memshare_gps>; + + #address-cells = <1>; + #size-cells = <0>; + + mpss@0 { + reg = <MEMSHARE_PROC_MPSS_V01>; + qcom,qrtr-node = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + memshare_gps: gps@0 { + reg = <0>; + memory-region = <&gps_mem>; + }; + }; + }; + +...include/dt-bindings/soc/qcom,memshare.h new file +10
-0@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __DT_QCOM_MEMSHARE_H__ +#define __DT_QCOM_MEMSHARE_H__ + +#define MEMSHARE_PROC_MPSS_V01 0 +#define MEMSHARE_PROC_ADSP_V01 1 +#define MEMSHARE_PROC_WCNSS_V01 2 + +#endif /* __DT_QCOM_MEMSHARE_H__ */ Signature · 1 line
2.27.0
Re: [PATCH 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service
Hi, sorry for a late reply but I couldn't answer earlier.
30.03.2021 19:40, Rob Herring пишет:
Quoted 29 lines · On Fri, Mar 19, 2021 at 10:23:20PM +0500, [email protected] wrote:
Quoted 19 lines
From: Nikita Travkin <[email protected]>
Add DT bindings for memshare: QMI service that allocates memory per remote processor request.
Signed-off-by Nikita Travkin <[email protected]>--- .../bindings/soc/qcom/qcom,memshare.yaml | 109 ++++++++++++++++++ include/dt-bindings/soc/qcom,memshare.h | 10 ++ 2 files changed, 119 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml create mode 100644 include/dt-bindings/soc/qcom,memshare.hDocumentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +0
-0Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0@@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/soc/qcom/qcom,memshare.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm QMI Shared Memory ServiceHow many shared memory interfaces does Qcom have...
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +4
-0+ +description: | + This driver provides a QMI service that allows remote processors (like modem) + to request additional memory. It is used for applications like GPS in modem.If the memory region is defined in reserved-memory, how are you allocating additional memory?
Initially remoteproc is loaded into it's own reserved-memory region but qcom decided that they sometimes need more memory than that. Memshare driver in msm8916 downstream tree seem to blindly allocate DMA region for every request that it gets. Additionally for those clients described in the DT, they do the DMA allocation on boot time and never free the region. They call it "guaranteed" allocation.
On msm8916 only one "guaranteed" client seem to be used so I decided to implement it with reserved-memory node. On newer platforms they seem to have more clients but I think that the driver can be easily extended to support dynamic allocation if someone really needs it.
I tried to explain that in the cover letter but I think I made some mistake as I don't see it in the Patchwork.
Quoted 18 lines
Quoted 18 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +18
-0+ +maintainers: + - Nikita Travkin <[email protected]> + +properties: + compatible: + const: qcom,memshare + + qcom,legacy-client: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to a memshare client node used for legacy requests. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 +
Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 36
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0+patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: Proc-ID for clients in this node.What's Proc-ID?
The requests from the remote nodes contain client-id and proc-id that are supposed to differentiate the clients. It's possible to find the values in downstream DT or by observing what messages are received by the memshare service (I left dev_dbg logging in the driver for that reason)
I think I should reword it to make this more apparent, maybe "Proc-ID that clients in this node send."?
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +11
-0+ + qcom,qrtr-node: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Node from which the requests are expected. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 +
Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 54
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0+ patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: ID of this client.How does one determine the ID?
As with proc-id, maybe reword to "ID that this client sends."?
I will change those in v2, I still expect comments on the driver itself, so I'll wait for that before submitting it with just a couple lines changed.
Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +24
-0+ + memory-region: + $ref: /schemas/types.yaml#/definitions/phandle + description: | + Reserved memory region that should be used for allocation. + + required: + - reg + + required: + - reg + - qcom,qrtr-node + +required: + - compatible + +additionalProperties: false + +examples: + - | + #include <dt-bindings/soc/qcom,memshare.h> + + reserved-memory { +
Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 84
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +6
-0+ #address-cells = <2>; + #size-cells = <2>; + + gps_mem: gps@93c00000 { + reg = <0x0 0x93c00000 0x0 0x200000>; + no-map;We support 'compatible' in reserved-memory nodes, can you simplify the binding and put everything in here?
If I understand this correctly, each reserved-memory node will then load a new instance of memshare. Since the driver registers a QMI service that handles multiple clients, there should be only one instance. Additionally, as I mentioned earlier, some clients may not need reserved-memory at all
Quoted 44 lines
Quoted 44 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +25
-0+ }; + }; + + memshare { + compatible = "qcom,memshare"; + qcom,legacy-client = <&memshare_gps>; + + #address-cells = <1>; + #size-cells = <0>; + + mpss@0 { + reg = <MEMSHARE_PROC_MPSS_V01>; + qcom,qrtr-node = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + memshare_gps: gps@0 { + reg = <0>; + memory-region = <&gps_mem>; + }; + }; + }; + +...include/dt-bindings/soc/qcom,memshare.h new file +10
-0@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __DT_QCOM_MEMSHARE_H__ +#define __DT_QCOM_MEMSHARE_H__ + +#define MEMSHARE_PROC_MPSS_V01 0 +#define MEMSHARE_PROC_ADSP_V01 1 +#define MEMSHARE_PROC_WCNSS_V01 2 + +#endif /* __DT_QCOM_MEMSHARE_H__ */ Signature · 1 line
2.27.0
Re: [PATCH 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service
Quoted 46 lines · Nikita Travkin
Hi, sorry for a late reply but I couldn't answer earlier.
30.03.2021 19:40, Rob Herring ??????????:
Quoted 29 lines · On Fri, Mar 19, 2021 at 10:23:20PM +0500, [email protected] wrote:
Quoted 19 lines
From: Nikita Travkin <[email protected]>
Add DT bindings for memshare: QMI service that allocates memory per remote processor request.
Signed-off-by Nikita Travkin <[email protected]>--- .../bindings/soc/qcom/qcom,memshare.yaml | 109 ++++++++++++++++++ include/dt-bindings/soc/qcom,memshare.h | 10 ++ 2 files changed, 119 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml create mode 100644 include/dt-bindings/soc/qcom,memshare.hDocumentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +0
-0Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0@@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/soc/qcom/qcom,memshare.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm QMI Shared Memory ServiceHow many shared memory interfaces does Qcom have...
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +4
-0+ +description: | + This driver provides a QMI service that allows remote processors (like modem) + to request additional memory. It is used for applications like GPS in modem.If the memory region is defined in reserved-memory, how are you allocating additional memory?
Initially remoteproc is loaded into it's own reserved-memory region but qcom decided that they sometimes need more memory than that. Memshare driver in msm8916 downstream tree seem to blindly allocate DMA region for every request that it gets. Additionally for those clients described in the DT, they do the DMA allocation on boot time and never free the region. They call it "guaranteed" allocation.
Quoted 5 lines
On msm8916 only one "guaranteed" client seem to be used so I decided to implement it with reserved-memory node. On newer platforms they seem to have more clients but I think that the driver can be easily extended to support dynamic allocation if someone really needs it.
Is the "guaranteed" memory required to come from the reserved-memory part of memory, or could it simply be allocated on demand as well (or preallocated, but at a dynamic address)?
If these allocations always came from a reserved-memory region, then adding a "qcom,memshare" compatible to the reserved-memory node itself seems like a reasonable approach. But if dma_alloc is sufficient, and there's cases where there's no "guaranteed" region, perhaps we should just describe this as part of the remoteproc node (i.e. essentially flipping the node/subnode in your current binding).
E.g. can we get away with simply adding an optional qcom,memshare-node to the remoteproc binding and when that's present we make the Qualcomm remoteproc drivers spawn the memshare handler and listen for requests from that node?
Quoted 30 lines
I tried to explain that in the cover letter but I think I made some mistake as I don't see it in the Patchwork.
Quoted 18 lines
Quoted 18 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +18
-0+ +maintainers: + - Nikita Travkin <[email protected]> + +properties: + compatible: + const: qcom,memshare + + qcom,legacy-client: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to a memshare client node used for legacy requests. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 +Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 36
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0+patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: Proc-ID for clients in this node.What's Proc-ID?
Quoted 9 lines
The requests from the remote nodes contain client-id and proc-id that are supposed to differentiate the clients. It's possible to find the values in downstream DT or by observing what messages are received by the memshare service (I left
dev_dbglogging in the driver for that reason)I think I should reword it to make this more apparent, maybe "Proc-ID that clients in this node send."?
If this is a constant for each remote and we make this a child thing of remoteproc perhaps encode the number in the remoteproc nodes?
(We still need something in DT to state that we want a memshare for a given platform/remoteproc)
Quoted 61 lines
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +11
-0+ + qcom,qrtr-node: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Node from which the requests are expected. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 +Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 54
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0+ patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: ID of this client.How does one determine the ID?
As with proc-id, maybe reword to "ID that this client sends."?
I will change those in v2, I still expect comments on the driver itself, so I'll wait for that before submitting it with just a couple lines changed.
Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +24
-0+ + memory-region: + $ref: /schemas/types.yaml#/definitions/phandle + description: | + Reserved memory region that should be used for allocation. + + required: + - reg + + required: + - reg + - qcom,qrtr-node + +required: + - compatible + +additionalProperties: false + +examples: + - | + #include <dt-bindings/soc/qcom,memshare.h> + + reserved-memory { +Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 84
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +6
-0+ #address-cells = <2>; + #size-cells = <2>; + + gps_mem: gps@93c00000 { + reg = <0x0 0x93c00000 0x0 0x200000>; + no-map;We support 'compatible' in reserved-memory nodes, can you simplify the binding and put everything in here?
Quoted 4 lines
If I understand this correctly, each reserved-memory node will then load a new instance of memshare. Since the driver registers a QMI service that handles multiple clients, there should be only one instance.
This you could work around in the driver implementation, to refcount a single implementation shared among all the instances.
Quoted 3 lines
Additionally, as I mentioned earlier, some clients may not need reserved-memory at all
This on the other hand, makes me feel like we shouldn't go that route.
Regards,
Bjorn
Quoted 45 lines
Quoted 44 lines
Quoted 44 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +25
-0+ }; + }; + + memshare { + compatible = "qcom,memshare"; + qcom,legacy-client = <&memshare_gps>; + + #address-cells = <1>; + #size-cells = <0>; + + mpss@0 { + reg = <MEMSHARE_PROC_MPSS_V01>; + qcom,qrtr-node = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + memshare_gps: gps@0 { + reg = <0>; + memory-region = <&gps_mem>; + }; + }; + }; + +...include/dt-bindings/soc/qcom,memshare.h new file +10
-0@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __DT_QCOM_MEMSHARE_H__ +#define __DT_QCOM_MEMSHARE_H__ + +#define MEMSHARE_PROC_MPSS_V01 0 +#define MEMSHARE_PROC_ADSP_V01 1 +#define MEMSHARE_PROC_WCNSS_V01 2 + +#endif /* __DT_QCOM_MEMSHARE_H__ */ Signature · 1 line
2.27.0
Re: [PATCH 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Memshare service
14.04.2021 08:15, Bjorn Andersson пишет:
Quoted 47 lines · Nikita Travkin
Quoted 46 lines
Hi, sorry for a late reply but I couldn't answer earlier.
30.03.2021 19:40, Rob Herring ??????????:
Quoted 29 lines · On Fri, Mar 19, 2021 at 10:23:20PM +0500, [email protected] wrote:
Quoted 19 lines
From: Nikita Travkin <[email protected]>
Add DT bindings for memshare: QMI service that allocates memory per remote processor request.
Signed-off-by Nikita Travkin <[email protected]>--- .../bindings/soc/qcom/qcom,memshare.yaml | 109 ++++++++++++++++++ include/dt-bindings/soc/qcom,memshare.h | 10 ++ 2 files changed, 119 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml create mode 100644 include/dt-bindings/soc/qcom,memshare.hDocumentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +0
-0Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0@@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/soc/qcom/qcom,memshare.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm QMI Shared Memory ServiceHow many shared memory interfaces does Qcom have...
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +4
-0+ +description: | + This driver provides a QMI service that allows remote processors (like modem) + to request additional memory. It is used for applications like GPS in modem.If the memory region is defined in reserved-memory, how are you allocating additional memory?
Initially remoteproc is loaded into it's own reserved-memory region but qcom decided that they sometimes need more memory than that. Memshare driver in msm8916 downstream tree seem to blindly allocate DMA region for every request that it gets. Additionally for those clients described in the DT, they do the DMA allocation on boot time and never free the region. They call it "guaranteed" allocation.
Quoted 8 lines
Quoted 5 lines
On msm8916 only one "guaranteed" client seem to be used so I decided to implement it with reserved-memory node. On newer platforms they seem to have more clients but I think that the driver can be easily extended to support dynamic allocation if someone really needs it.
Is the "guaranteed" memory required to come from the reserved-memory part of memory, or could it simply be allocated on demand as well (or preallocated, but at a dynamic address)?
This is rather complicated.
For most (msm8916) devices it works with a region from dma_alloc but there are at least three devices where it causes problems.
If the region was allocated by dma_alloc (somewhere near 0xfe100000 if I remember correctly) then
- Wileyfox Swift (Longcheer L8150): Location service "crashes" (repeats request every 10-ish seconds while location session is open and gives no location data)
- Samsung A3, A5: The entire modem crashes after it gets the response with such address.
Downstream kernel allocates the region at slightly different address which works fine.
It's probably possible to change the allocation address with dma mask but I have no idea why the crash happens on those devices, if it's even possible to debug this or find all the "bad" regions. Because of that I prefer using a known-good address at least for the location client that keeps it forever.
Quoted 8 lines
If these allocations always came from a reserved-memory region, then adding a "qcom,memshare" compatible to the reserved-memory node itself seems like a reasonable approach. But if
dma_allocis sufficient, and there's cases where there's no "guaranteed" region, perhaps we should just describe this as part of the remoteproc node (i.e. essentially flipping the node/subnode in your current binding).
Quoted 4 lines
E.g. can we get away with simply adding an optional qcom,memshare-node to the remoteproc binding and when that's present we make the Qualcomm remoteproc drivers spawn the memshare handler and listen for requests from that node?
I'm having a hard time imagining how this would be implemented...
Assuming that I need to keep reserved-memory for some clients and other clients will need proper dynamic allocation (maybe on a newer platform), there will be an optional subnode in each remoteproc node, that will be detected by the remoteproc driver which will then start memshare. It will have all the id-s, size or phandle to reserved-memory. Then there may be multiple of those in one remoteproc. Or one memshare subnode will contain multiple client nodes? Then if I understand correctly, there are multiple different remoteproc drivers so each will have to be modified. They will need to spawn only one memshare instance and pass the clients to it. Or maybe the subnode can contain a compatible and the code to keep only one instance will be in the memshare driver itself...
To be honest, I'm getting very confused by even trying to lay this down in my mind. I think it just unnecessarily complicates both binding and the driver to "hide" it's nodes through the device tree.
Maybe I just didn't understand the proposal...
Quoted 160 lines
Quoted 29 lines
I tried to explain that in the cover letter but I think I made some mistake as I don't see it in the Patchwork.
Quoted 18 lines
Quoted 18 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +18
-0+ +maintainers: + - Nikita Travkin <[email protected]> + +properties: + compatible: + const: qcom,memshare + + qcom,legacy-client: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to a memshare client node used for legacy requests. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 +Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 36
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0+patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: Proc-ID for clients in this node.What's Proc-ID?
Quoted 9 lines
The requests from the remote nodes contain client-id and proc-id that are supposed to differentiate the clients. It's possible to find the values in downstream DT or by observing what messages are received by the memshare service (I left
dev_dbglogging in the driver for that reason)I think I should reword it to make this more apparent, maybe "Proc-ID that clients in this node send."?
If this is a constant for each remote and we make this a child thing of remoteproc perhaps encode the number in the remoteproc nodes?
(We still need something in DT to state that we want a memshare for a given platform/remoteproc)
Quoted 57 lines
Quoted 11 lines
Quoted 11 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +11
-0+ + qcom,qrtr-node: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Node from which the requests are expected. + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 +Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 54
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +7
-0+ patternProperties: + "^.*@[0-9]+$": + type: object + + properties: + reg: + description: ID of this client.How does one determine the ID?
As with proc-id, maybe reword to "ID that this client sends."?
I will change those in v2, I still expect comments on the driver itself, so I'll wait for that before submitting it with just a couple lines changed.
Quoted 24 lines
Quoted 24 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +24
-0+ + memory-region: + $ref: /schemas/types.yaml#/definitions/phandle + description: | + Reserved memory region that should be used for allocation. + + required: + - reg + + required: + - reg + - qcom,qrtr-node + +required: + - compatible + +additionalProperties: false + +examples: + - | + #include <dt-bindings/soc/qcom,memshare.h> + + reserved-memory { +Quoted 8 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml line 84
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +6
-0+ #address-cells = <2>; + #size-cells = <2>; + + gps_mem: gps@93c00000 { + reg = <0x0 0x93c00000 0x0 0x200000>; + no-map;We support 'compatible' in reserved-memory nodes, can you simplify the binding and put everything in here?
Quoted 4 lines
If I understand this correctly, each reserved-memory node will then load a new instance of memshare. Since the driver registers a QMI service that handles multiple clients, there should be only one instance.
This you could work around in the driver implementation, to refcount a single implementation shared among all the instances.
Quoted 3 lines
Additionally, as I mentioned earlier, some clients may not need reserved-memory at all
This on the other hand, makes me feel like we shouldn't go that route.
Regards,
BjornQuoted 44 lines
Quoted 44 lines
Quoted 44 lines
Documentation/devicetree/bindings/soc/qcom/qcom,memshare.yaml new file +25
-0+ }; + }; + + memshare { + compatible = "qcom,memshare"; + qcom,legacy-client = <&memshare_gps>; + + #address-cells = <1>; + #size-cells = <0>; + + mpss@0 { + reg = <MEMSHARE_PROC_MPSS_V01>; + qcom,qrtr-node = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + memshare_gps: gps@0 { + reg = <0>; + memory-region = <&gps_mem>; + }; + }; + }; + +...include/dt-bindings/soc/qcom,memshare.h new file +10
-0@@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __DT_QCOM_MEMSHARE_H__ +#define __DT_QCOM_MEMSHARE_H__ + +#define MEMSHARE_PROC_MPSS_V01 0 +#define MEMSHARE_PROC_ADSP_V01 1 +#define MEMSHARE_PROC_WCNSS_V01 2 + +#endif /* __DT_QCOM_MEMSHARE_H__ */ Signature · 1 line
2.27.0
PATCH 2/2 soc: qcom: Add Qualcomm Memshare QMI service
In reply to [email protected]From: Nikita Travkin <[email protected]>
This QMI service provides memory region for remote processors (like modem) on request. It is known to be used in GPS service in modem firmware for some (most) of msm8916 devices.
This commit implements "guaranteed allocation" of memory using reserved-memory regions because gps service crashes on some devices unless specific address was provided.
--- drivers/soc/qcom/Kconfig | 8 + drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/memshare.c | 501 ++++++++++++++++++++++++++++ drivers/soc/qcom/memshare_qmi_msg.c | 370 ++++++++++++++++++++ drivers/soc/qcom/memshare_qmi_msg.h | 228 +++++++++++++ 5 files changed, 1109 insertions(+) create mode 100644 drivers/soc/qcom/memshare.c create mode 100644 drivers/soc/qcom/memshare_qmi_msg.c create mode 100644 drivers/soc/qcom/memshare_qmi_msg.h
drivers/soc/qcom/Kconfig +8 -0
@@ -209,4 +209,12 @@ config QCOM_APR application processor and QDSP6. APR is used by audio driver to configure QDSP6 ASM, ADM and AFE modules. + +config QCOM_MEMSHARE_QMI_SERVICE + select QCOM_QMI_HELPERS + tristate "Qualcomm QMI Shared Memory Service (MEMSHARE)" + help + This service provides additional memory regions on request from + remote processors. On some platforms it is used for GPS by + the location service in the modem subsystem. endmenu
drivers/soc/qcom/Makefile +2 -0
@@ -26,3 +26,5 @@ obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) += kryo-l2-accessors.o +obj-$(CONFIG_QCOM_MEMSHARE_QMI_SERVICE) += qcom_memshare.o +qcom_memshare-y := memshare.o memshare_qmi_msg.o
drivers/soc/qcom/memshare.c new file +501 -0
501 lines not on this page · raw message
drivers/soc/qcom/memshare_qmi_msg.c new file +370 -0
370 lines not on this page · raw message
drivers/soc/qcom/memshare_qmi_msg.h new file +228 -0
228 lines not on this page · raw message
Signature · 1 line
2.27.0