| Message ID | 1698052857-6918-2-git-send-email-quic_zhenhuah@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/5] dt-bindings: soc: qcom: Add memory_dump driver bindings | expand |
On 2023/10/23 20:52, Krzysztof Kozlowski wrote: > On 23/10/2023 13:54, Zhenhua Huang wrote: >> >> >> On 2023/10/23 17:27, Krzysztof Kozlowski wrote: >>> On 23/10/2023 11:20, Zhenhua Huang wrote: >>>> Add bindings for the QCOM Memory Dump driver providing debug >>> >>> Bindings are for hardware, not driver. This suggests it is not suitable >>> for bindings at all. >>> >>>> facilities. Firmware dumps system cache, internal memory, >>>> peripheral registers to reserved DDR as per the table which >>>> populated by the driver, after crash and warm reset. >>> >>> Again driver :/ >> >> Thanks for pointing out. Qualcomm memory dump device is a reserved >> memory region which is used to communicate with firmware. I will update >> description in next version. > > I have still doubts that it is suitable for DT. I usually expect such > firmware feature-drivers to be instantiated by existing firmware > drivers. You do not need DT for this. Got it, as it interacts with firmware, you think it should be a firmware driver? But it seems there should not be existing suitable place to put it now(qcom_scm.c is for TZ). Shall we create one new file like *qcom_sdi.c* in drivers/firmware and put it there? Because SDI(system debug image, which is part of bootloader) is the firmware doing the things. > > Best regards, > Krzysztof >
Hi Zhenhua, On 10/23/2023 2:27 AM, Krzysztof Kozlowski wrote: > On 23/10/2023 11:20, Zhenhua Huang wrote: >> Add bindings for the QCOM Memory Dump driver providing debug > > Bindings are for hardware, not driver. This suggests it is not suitable > for bindings at all. > >> facilities. Firmware dumps system cache, internal memory, >> peripheral registers to reserved DDR as per the table which >> populated by the driver, after crash and warm reset. > > Again driver :/ > >> >> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> >> --- >> .../bindings/soc/qcom/qcom,mem-dump.yaml | 42 +++++++++++++++++++++ >> .../devicetree/bindings/sram/qcom,imem.yaml | 44 ++++++++++++++++++++++ >> 2 files changed, 86 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml >> new file mode 100644 >> index 0000000..87f8f51 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml >> @@ -0,0 +1,42 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/soc/qcom/qcom,mem-dump.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > Drop quotes. > > It does not look like you tested the bindings, at least after quick > look. Please run `make dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > Maybe you need to update your dtschema and yamllint. > >> + >> +title: Qualcomm memory dump > > Describe hardware, not driver. > >> + >> +description: | >> + Qualcomm memory dump driver dynamically reserves memory and provides hints(id and size) > > Again, driver, so not suitable for DTS and bindings. Could you create platform driver which binds directly to the compatible = "qcom,qcom-imem-mem-dump-table" You can look up the size from the dump table driver or have 2 reg properties in the -table node itself (so no need for the table-size node either).
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml new file mode 100644 index 0000000..87f8f51 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml @@ -0,0 +1,42 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/soc/qcom/qcom,mem-dump.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm memory dump + +description: | + Qualcomm memory dump driver dynamically reserves memory and provides hints(id and size) + of debugging information based on specified protocols with firmware. Firmware then does + the real data capture. The debugging information includes cache contents, internal + memory, registers. After crash and warm reboot, firmware scans ids, + sizes and stores contents into reserved memory accordingly. Firmware then enters into full + dump mode which dumps whole DDR to PC through USB. + +maintainers: + - Zhenhua Huang <quic_zhenhuah@quicinc.com> + +properties: + compatible: + const: qcom,mem-dump + + memory-region: + maxItems: 1 + description: phandle to memory reservation for qcom,mem-dump region. + +required: + - compatible + - memory-region + +additionalProperties: false + +examples: + # minimum memory dump definition. + - | + mem-dump { + compatible = "qcom,mem-dump"; + memory-region = <&dump_mem>; + }; + +... diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml index 8025a85..e9eaa7a 100644 --- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml +++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml @@ -52,6 +52,40 @@ patternProperties: $ref: /schemas/remoteproc/qcom,pil-info.yaml# description: Peripheral image loader relocation region +patternProperties: + "^mem-dump-table@[0-9a-f]+$": + type: object + description: Used to store dump table base addr + properties: + compatible: + const: "qcom,qcom-imem-mem-dump-table" + + reg: + maxItems: 1 + + required: + - compatible + - reg + + additionalProperties: false + +patternProperties: + "^mem-dump-table-size@[0-9a-f]+$": + type: object + description: Used to store dump table size + properties: + compatible: + const: "qcom,qcom-imem-mem-dump-table-size" + + reg: + maxItems: 1 + + required: + - compatible + - reg + + additionalProperties: false + required: - compatible - reg @@ -76,5 +110,15 @@ examples: compatible = "qcom,pil-reloc-info"; reg = <0x94c 0xc8>; }; + + mem-dump-table@10 { + compatible = "qcom,qcom-imem-mem-dump-table"; + reg = <0x10 0x8>; + }; + + mem-dump-table-size@724 { + compatible = "qcom,qcom-imem-mem-dump-table-size"; + reg = <0x724 0x8>; + }; }; };
Add bindings for the QCOM Memory Dump driver providing debug facilities. Firmware dumps system cache, internal memory, peripheral registers to reserved DDR as per the table which populated by the driver, after crash and warm reset. Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com> --- .../bindings/soc/qcom/qcom,mem-dump.yaml | 42 +++++++++++++++++++++ .../devicetree/bindings/sram/qcom,imem.yaml | 44 ++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,mem-dump.yaml