* [PATCH V4 0/5] Enable crashdump collection support for IPQ9574
@ 2023-02-14 5:14 Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 1/5] dt-bindings: scm: Add compatible " Poovendhan Selvaraj
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Poovendhan Selvaraj @ 2023-02-14 5:14 UTC (permalink / raw)
To: agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
Crashdump collection is enabled based on the DLOAD bit in the TCSR register.
This bit is set during bootup and clearing during shutdown. During crash,
dload bit is not cleared, due to which uboot starts crashdump collection.
Enable the support for download mode to collect the crashdumps if
system crashes, to debug crashes extensively.
During the bootup, bootloaders initialize the SMEM. However the bootup
after crash, SMEM will not be initialized again. If the memory for the
SMEM is not reserved, linux consumes that region, which leads to the
loss of SMEM data. So, during the next bootup after crash, bootloaders
will hang due to invalid data present in the SMEM region. Due to this,
added the SMEM support along with crashdump collection series.
This patch series adds the support for crashdump collection.
DTS patch depends on the IPQ9574 baseport series
https://lore.kernel.org/linux-arm-msm/20230206103337.21000-1-quic_devipriy@quicinc.com/
V4:
- change logs are added to the respective patches.
V3 can be found at
https://lore.kernel.org/linux-arm-msm/20230208053332.16537-1-quic_poovendh@quicinc.com/
Changes in V2:
- rebased on linux-next/master
- dropped co-developed by tag wherever applicable
- V1 can be found at
https://lore.kernel.org/linux-arm-kernel/20230113160012.14893-1-quic_poovendh@quicinc.com/
Poovendhan Selvaraj (5):
dt-bindings: scm: Add compatible for IPQ9574
dt-bindings: mfd: Add the tcsr compatible for IPQ9574
arm64: dts: qcom: ipq9574: Enable the download mode support
arm64: dts: qcom: ipq9574: Add SMEM support
firmware: scm: Modify only the DLOAD bit in TCSR register for download
mode
.../bindings/firmware/qcom,scm.yaml | 1 +
.../devicetree/bindings/mfd/qcom,tcsr.yaml | 1 +
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 26 ++++++++++++++++++-
drivers/firmware/qcom_scm.c | 15 +++++++----
4 files changed, 37 insertions(+), 6 deletions(-)
base-commit: 09e41676e35ab06e4bce8870ea3bf1f191c3cb90
--
2.17.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V4 1/5] dt-bindings: scm: Add compatible for IPQ9574
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
@ 2023-02-14 5:14 ` Poovendhan Selvaraj
2023-02-14 7:54 ` Krzysztof Kozlowski
2023-02-14 5:14 ` [PATCH V4 2/5] dt-bindings: mfd: Add the tcsr " Poovendhan Selvaraj
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Poovendhan Selvaraj @ 2023-02-14 5:14 UTC (permalink / raw)
To: agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
Add the scm compatible string for IPQ9574 SoC
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
---
Changes in V4:
- No Changes
Changes in V3:
- No Changes
Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index a66e99812b1f..868fc7e555a6 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -27,6 +27,7 @@ properties:
- qcom,scm-ipq6018
- qcom,scm-ipq806x
- qcom,scm-ipq8074
+ - qcom,scm-ipq9574
- qcom,scm-mdm9607
- qcom,scm-msm8226
- qcom,scm-msm8660
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V4 2/5] dt-bindings: mfd: Add the tcsr compatible for IPQ9574
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 1/5] dt-bindings: scm: Add compatible " Poovendhan Selvaraj
@ 2023-02-14 5:14 ` Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 3/5] arm64: dts: qcom: ipq9574: Enable the download mode support Poovendhan Selvaraj
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Poovendhan Selvaraj @ 2023-02-14 5:14 UTC (permalink / raw)
To: agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
Add the tcsr compatible string for IPQ9574 SoC
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
---
Changes in V4:
- No changes
Changes in V3:
- Updated the subject
- Added the Acked by tag
Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml b/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
index 2eeebe920e6e..203fc890c9a3 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
@@ -31,6 +31,7 @@ properties:
- qcom,tcsr-ipq5332
- qcom,tcsr-ipq6018
- qcom,tcsr-ipq8064
+ - qcom,tcsr-ipq9574
- qcom,tcsr-mdm9615
- qcom,tcsr-msm8226
- qcom,tcsr-msm8660
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V4 3/5] arm64: dts: qcom: ipq9574: Enable the download mode support
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 1/5] dt-bindings: scm: Add compatible " Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 2/5] dt-bindings: mfd: Add the tcsr " Poovendhan Selvaraj
@ 2023-02-14 5:14 ` Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 4/5] arm64: dts: qcom: ipq9574: Add SMEM support Poovendhan Selvaraj
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Poovendhan Selvaraj @ 2023-02-14 5:14 UTC (permalink / raw)
To: agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
Enable the support for download mode to collect the crashdumps if
system crashes, to debug crashes extensively.
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
---
Changes in V4:
- Dropped smem related nodes from this patch
- Mapped the entire TCSR_REGS region and updated the offset in
qcom,dload-mode property
- updated the commit message
Changes in V3:
- No changes
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 2b86ba17bb32..898d7ce3f28d 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -81,6 +81,13 @@
reg = <0x0 0x40000000 0x0 0x0>;
};
+ firmware {
+ scm {
+ compatible = "qcom,scm-ipq9574", "qcom,scm";
+ qcom,dload-mode = <&tcsr 0x6100>;
+ };
+ };
+
pmu {
compatible = "arm,cortex-a73-pmu";
interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
@@ -150,6 +157,11 @@
#power-domain-cells = <1>;
};
+ tcsr: syscon@1937000 {
+ compatible = "qcom,tcsr-ipq9574", "syscon";
+ reg = <0x01937000 0x21000>;
+ };
+
sdhc_1: mmc@7804000 {
compatible = "qcom,ipq9574-sdhci", "qcom,sdhci-msm-v5";
reg = <0x07804000 0x1000>, <0x07805000 0x1000>;
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V4 4/5] arm64: dts: qcom: ipq9574: Add SMEM support
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
` (2 preceding siblings ...)
2023-02-14 5:14 ` [PATCH V4 3/5] arm64: dts: qcom: ipq9574: Enable the download mode support Poovendhan Selvaraj
@ 2023-02-14 5:14 ` Poovendhan Selvaraj
2023-02-14 7:54 ` Krzysztof Kozlowski
2023-02-14 5:14 ` [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode Poovendhan Selvaraj
2023-02-15 2:34 ` [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Stephen Boyd
5 siblings, 1 reply; 12+ messages in thread
From: Poovendhan Selvaraj @ 2023-02-14 5:14 UTC (permalink / raw)
To: agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
Add the required nodes to support SMEM
Changes in V4:
- Added required nodes for smem support
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
---
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 898d7ce3f28d..2b41416d7f91 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -102,11 +102,17 @@
#address-cells = <2>;
#size-cells = <2>;
ranges;
-
tz_region: tz@4a600000 {
reg = <0x0 0x4a600000 0x0 0x400000>;
no-map;
};
+
+ smem@4aa00000 {
+ compatible = "qcom,smem";
+ reg = <0x0 0x4aa00000 0x0 0x00100000>;
+ hwlocks = <&tcsr_mutex 0>;
+ no-map;
+ };
};
soc: soc@0 {
@@ -157,6 +163,12 @@
#power-domain-cells = <1>;
};
+ tcsr_mutex: hwlock@1905000 {
+ compatible = "qcom,tcsr-mutex";
+ reg = <0x01905000 0x20000>;
+ #hwlock-cells = <1>;
+ };
+
tcsr: syscon@1937000 {
compatible = "qcom,tcsr-ipq9574", "syscon";
reg = <0x01937000 0x21000>;
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
` (3 preceding siblings ...)
2023-02-14 5:14 ` [PATCH V4 4/5] arm64: dts: qcom: ipq9574: Add SMEM support Poovendhan Selvaraj
@ 2023-02-14 5:14 ` Poovendhan Selvaraj
2023-02-14 12:57 ` Srinivas Kandagatla
2023-02-15 2:34 ` [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Stephen Boyd
5 siblings, 1 reply; 12+ messages in thread
From: Poovendhan Selvaraj @ 2023-02-14 5:14 UTC (permalink / raw)
To: agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
CrashDump collection is based on the DLOAD bit of TCSR register.
To retain other bits, we read the register and modify only the DLOAD bit as
the other bits have their own significance.
Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
Co-developed-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
---
Changes in V4:
- retain the orginal value of tcsr register when download mode
is not set
drivers/firmware/qcom_scm.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 468d4d5ab550..8a34b386ac3a 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -407,7 +407,7 @@ int qcom_scm_set_remote_state(u32 state, u32 id)
}
EXPORT_SYMBOL(qcom_scm_set_remote_state);
-static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
+static int __qcom_scm_set_dload_mode(struct device *dev, u32 val, bool enable)
{
struct qcom_scm_desc desc = {
.svc = QCOM_SCM_SVC_BOOT,
@@ -417,7 +417,8 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
.owner = ARM_SMCCC_OWNER_SIP,
};
- desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
+ desc.args[1] = enable ? val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
+ val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE);
return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
}
@@ -426,15 +427,19 @@ static void qcom_scm_set_download_mode(bool enable)
{
bool avail;
int ret = 0;
+ u32 dload_addr_val;
avail = __qcom_scm_is_call_available(__scm->dev,
QCOM_SCM_SVC_BOOT,
QCOM_SCM_BOOT_SET_DLOAD_MODE);
+ ret = qcom_scm_io_readl(__scm->dload_mode_addr, &dload_addr_val);
+
if (avail) {
- ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
+ ret = __qcom_scm_set_dload_mode(__scm->dev, dload_addr_val, enable);
} else if (__scm->dload_mode_addr) {
- ret = qcom_scm_io_writel(__scm->dload_mode_addr,
- enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
+ ret = qcom_scm_io_writel(__scm->dload_mode_addr, enable ?
+ dload_addr_val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
+ dload_addr_val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE));
} else {
dev_err(__scm->dev,
"No available mechanism for setting download mode\n");
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V4 4/5] arm64: dts: qcom: ipq9574: Add SMEM support
2023-02-14 5:14 ` [PATCH V4 4/5] arm64: dts: qcom: ipq9574: Add SMEM support Poovendhan Selvaraj
@ 2023-02-14 7:54 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-02-14 7:54 UTC (permalink / raw)
To: Poovendhan Selvaraj, agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
On 14/02/2023 06:14, Poovendhan Selvaraj wrote:
> Add the required nodes to support SMEM
>
> Changes in V4:
> - Added required nodes for smem support
>
> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 898d7ce3f28d..2b41416d7f91 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -102,11 +102,17 @@
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
> -
No improvements here. For the third time.
This is a friendly reminder during the review process.
It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.
Thank you.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 1/5] dt-bindings: scm: Add compatible for IPQ9574
2023-02-14 5:14 ` [PATCH V4 1/5] dt-bindings: scm: Add compatible " Poovendhan Selvaraj
@ 2023-02-14 7:54 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-02-14 7:54 UTC (permalink / raw)
To: Poovendhan Selvaraj, agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
On 14/02/2023 06:14, Poovendhan Selvaraj wrote:
> Add the scm compatible string for IPQ9574 SoC
>
> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode
2023-02-14 5:14 ` [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode Poovendhan Selvaraj
@ 2023-02-14 12:57 ` Srinivas Kandagatla
[not found] ` <b5135d69-3783-8147-bda5-8131cea726f7@quicinc.com>
0 siblings, 1 reply; 12+ messages in thread
From: Srinivas Kandagatla @ 2023-02-14 12:57 UTC (permalink / raw)
To: Poovendhan Selvaraj, agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
On 14/02/2023 05:14, Poovendhan Selvaraj wrote:
> CrashDump collection is based on the DLOAD bit of TCSR register.
> To retain other bits, we read the register and modify only the DLOAD bit as
> the other bits have their own significance.
>
> Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
> Co-developed-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
> ---
> Changes in V4:
> - retain the orginal value of tcsr register when download mode
> is not set
>
> drivers/firmware/qcom_scm.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 468d4d5ab550..8a34b386ac3a 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -407,7 +407,7 @@ int qcom_scm_set_remote_state(u32 state, u32 id)
> }
> EXPORT_SYMBOL(qcom_scm_set_remote_state);
>
> -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
> +static int __qcom_scm_set_dload_mode(struct device *dev, u32 val, bool enable)
> {
> struct qcom_scm_desc desc = {
> .svc = QCOM_SCM_SVC_BOOT,
> @@ -417,7 +417,8 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
> .owner = ARM_SMCCC_OWNER_SIP,
> };
>
> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
> + desc.args[1] = enable ? val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
> + val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE);
why not read the value here before setting the DLOAD Mode instead of
doing it in qcom_scm_set_download_mode()?
that would make the code simple and readable.
>
> return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
> }
> @@ -426,15 +427,19 @@ static void qcom_scm_set_download_mode(bool enable)
> {
> bool avail;
> int ret = 0;
> + u32 dload_addr_val;
>
> avail = __qcom_scm_is_call_available(__scm->dev,
> QCOM_SCM_SVC_BOOT,
> QCOM_SCM_BOOT_SET_DLOAD_MODE);
> + ret = qcom_scm_io_readl(__scm->dload_mode_addr, &dload_addr_val);
> +
not checking ret value here before proceeding?
> if (avail) {
> - ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
> + ret = __qcom_scm_set_dload_mode(__scm->dev, dload_addr_val, enable);
> } else if (__scm->dload_mode_addr) {
> - ret = qcom_scm_io_writel(__scm->dload_mode_addr,
> - enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
> + ret = qcom_scm_io_writel(__scm->dload_mode_addr, enable ?
> + dload_addr_val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
> + dload_addr_val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE));
> } else {
> dev_err(__scm->dev,
> "No available mechanism for setting download mode\n");
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 0/5] Enable crashdump collection support for IPQ9574
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
` (4 preceding siblings ...)
2023-02-14 5:14 ` [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode Poovendhan Selvaraj
@ 2023-02-15 2:34 ` Stephen Boyd
5 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2023-02-15 2:34 UTC (permalink / raw)
To: Poovendhan Selvaraj, agross, andersson, arnd, broonie,
catalin.marinas, devicetree, dmitry.baryshkov, jassisinghbrar,
konrad.dybcio, krzysztof.kozlowski+dt, lee, linux-arm-kernel,
linux-arm-msm, linux-clk, linux-kernel, marcel.ziswiler,
mturquette, nfraprado, quic_gurus, robh+dt, robimarko, shawnguo,
will
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
Quoting Poovendhan Selvaraj (2023-02-13 21:14:09)
>
> Poovendhan Selvaraj (5):
> dt-bindings: scm: Add compatible for IPQ9574
> dt-bindings: mfd: Add the tcsr compatible for IPQ9574
> arm64: dts: qcom: ipq9574: Enable the download mode support
> arm64: dts: qcom: ipq9574: Add SMEM support
> firmware: scm: Modify only the DLOAD bit in TCSR register for download
> mode
>
> .../bindings/firmware/qcom,scm.yaml | 1 +
> .../devicetree/bindings/mfd/qcom,tcsr.yaml | 1 +
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 26 ++++++++++++++++++-
> drivers/firmware/qcom_scm.c | 15 +++++++----
> 4 files changed, 37 insertions(+), 6 deletions(-)
I don't see any clk driver changes. Please stop Ccing linux-clk and clk
maintainers on this series.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode
[not found] ` <b5135d69-3783-8147-bda5-8131cea726f7@quicinc.com>
@ 2023-02-15 15:17 ` POOVENDHAN SELVARAJ
2023-02-15 15:17 ` Srinivas Kandagatla
1 sibling, 0 replies; 12+ messages in thread
From: POOVENDHAN SELVARAJ @ 2023-02-15 15:17 UTC (permalink / raw)
To: Srinivas Kandagatla, agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, catalin.marinas, will, shawnguo,
arnd, marcel.ziswiler, robimarko, dmitry.baryshkov, nfraprado,
broonie, quic_gurus, linux-arm-msm, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
On 2/15/2023 4:25 PM, POOVENDHAN SELVARAJ wrote:
>
>
> On 2/14/2023 6:27 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 14/02/2023 05:14, Poovendhan Selvaraj wrote:
>>> CrashDump collection is based on the DLOAD bit of TCSR register.
>>> To retain other bits, we read the register and modify only the DLOAD
>>> bit as
>>> the other bits have their own significance.
>>>
>>> Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
>>> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
>>> Co-developed-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
>>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
>>> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
>>> ---
>>> Changes in V4:
>>> - retain the orginal value of tcsr register when download mode
>>> is not set
>>>
>>> drivers/firmware/qcom_scm.c | 15 ++++++++++-----
>>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>>> index 468d4d5ab550..8a34b386ac3a 100644
>>> --- a/drivers/firmware/qcom_scm.c
>>> +++ b/drivers/firmware/qcom_scm.c
>>> @@ -407,7 +407,7 @@ int qcom_scm_set_remote_state(u32 state, u32 id)
>>> }
>>> EXPORT_SYMBOL(qcom_scm_set_remote_state);
>>> -static int __qcom_scm_set_dload_mode(struct device *dev, bool
>>> enable)
>>> +static int __qcom_scm_set_dload_mode(struct device *dev, u32 val,
>>> bool enable)
>>> {
>>> struct qcom_scm_desc desc = {
>>> .svc = QCOM_SCM_SVC_BOOT,
>>> @@ -417,7 +417,8 @@ static int __qcom_scm_set_dload_mode(struct
>>> device *dev, bool enable)
>>> .owner = ARM_SMCCC_OWNER_SIP,
>>> };
>>> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
>>> + desc.args[1] = enable ? val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
>>> + val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE);
>>
>> why not read the value here before setting the DLOAD Mode instead of
>> doing it in qcom_scm_set_download_mode()?
>> that would make the code simple and readable.
dload_addr_val is used in both if and else if cases in
qcom_scm_set_download_mode(),
so we read in qcom_scm_set_download_mode() function and pass to
__qcom_scm_set_dload_mode().
>>
>>> return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
>>> }
>>> @@ -426,15 +427,19 @@ static void qcom_scm_set_download_mode(bool
>>> enable)
>>> {
>>> bool avail;
>>> int ret = 0;
>>> + u32 dload_addr_val;
>>> avail = __qcom_scm_is_call_available(__scm->dev,
>>> QCOM_SCM_SVC_BOOT,
>>> QCOM_SCM_BOOT_SET_DLOAD_MODE);
>>> + ret = qcom_scm_io_readl(__scm->dload_mode_addr, &dload_addr_val);
>>> +
>> not checking ret value here before proceeding?
>>
Okay, sure..will address in next patch series.
>>> if (avail) {
>>> - ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
>>> + ret = __qcom_scm_set_dload_mode(__scm->dev, dload_addr_val,
>>> enable);
>>> } else if (__scm->dload_mode_addr) {
>>> - ret = qcom_scm_io_writel(__scm->dload_mode_addr,
>>> - enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
>>> + ret = qcom_scm_io_writel(__scm->dload_mode_addr, enable ?
>>> + dload_addr_val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
>>> + dload_addr_val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE));
>>> } else {
>>> dev_err(__scm->dev,
>>> "No available mechanism for setting download mode\n");
Please excuse the format in Previous mail response.
Had some issues in email client and now it's fixed.
Regards,
Poovendhan S
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode
[not found] ` <b5135d69-3783-8147-bda5-8131cea726f7@quicinc.com>
2023-02-15 15:17 ` POOVENDHAN SELVARAJ
@ 2023-02-15 15:17 ` Srinivas Kandagatla
1 sibling, 0 replies; 12+ messages in thread
From: Srinivas Kandagatla @ 2023-02-15 15:17 UTC (permalink / raw)
To: POOVENDHAN SELVARAJ, agross, andersson, konrad.dybcio, robh+dt,
krzysztof.kozlowski+dt, lee, mturquette, sboyd, jassisinghbrar,
catalin.marinas, will, shawnguo, arnd, marcel.ziswiler,
robimarko, dmitry.baryshkov, nfraprado, broonie, quic_gurus,
linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
Cc: quic_srichara, quic_gokulsri, quic_sjaganat, quic_kathirav,
quic_arajkuma, quic_anusha, quic_devipriy
On 15/02/2023 10:55, POOVENDHAN SELVARAJ wrote:
>
> On 2/14/2023 6:27 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 14/02/2023 05:14, Poovendhan Selvaraj wrote:
>>> CrashDump collection is based on the DLOAD bit of TCSR register.
>>> To retain other bits, we read the register and modify only the DLOAD
>>> bit as
>>> the other bits have their own significance.
>>>
>>> Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
>>> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
>>> Co-developed-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
>>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
>>> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
>>> ---
>>> Changes in V4:
>>> - retain the orginal value of tcsr register when download mode
>>> is not set
>>>
>>> drivers/firmware/qcom_scm.c | 15 ++++++++++-----
>>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>>> index 468d4d5ab550..8a34b386ac3a 100644
>>> --- a/drivers/firmware/qcom_scm.c
>>> +++ b/drivers/firmware/qcom_scm.c
>>> @@ -407,7 +407,7 @@ int qcom_scm_set_remote_state(u32 state, u32 id)
>>> }
>>> EXPORT_SYMBOL(qcom_scm_set_remote_state);
>>> -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
>>> +static int __qcom_scm_set_dload_mode(struct device *dev, u32 val,
>>> bool enable)
>>> {
>>> struct qcom_scm_desc desc = {
>>> .svc = QCOM_SCM_SVC_BOOT,
>>> @@ -417,7 +417,8 @@ static int __qcom_scm_set_dload_mode(struct
>>> device *dev, bool enable)
>>> .owner = ARM_SMCCC_OWNER_SIP,
>>> };
>>> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0;
>>> + desc.args[1] = enable ? val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
>>> + val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE);
>>
>> why not read the value here before setting the DLOAD Mode instead of
>> doing it in qcom_scm_set_download_mode()?
>> that would make the code simple and readable.
>
> dload_addr_val is used in both if and else if cases in
> qcom_scm_set_download_mode(), so we read in qcom_scm_set_download_mode()
> function and pass to __qcom_scm_set_dload_mode().
that is fine as it is, I missread else part as calling
__qcom_scm_set_dload_mode() too.
just check the ret value should be good.
--srini
>
> if (avail) {
> - ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
> *+ ret = __qcom_scm_set_dload_mode(__scm->dev, dload_addr_val,
> enable); *
> } else if (__scm->dload_mode_addr) {
> - ret = qcom_scm_io_writel(__scm->dload_mode_addr,
> - enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
> + ret = qcom_scm_io_writel(__scm->dload_mode_addr, enable ?
> + * dload_addr_val | QCOM_SCM_BOOT_SET_DLOAD_MODE : **
> **+ dload_addr_val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE)); *
> } else {
>
>>
>>> return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
>>> }
>>> @@ -426,15 +427,19 @@ static void qcom_scm_set_download_mode(bool
>>> enable)
>>> {
>>> bool avail;
>>> int ret = 0;
>>> + u32 dload_addr_val;
>>> avail = __qcom_scm_is_call_available(__scm->dev,
>>> QCOM_SCM_SVC_BOOT,
>>> QCOM_SCM_BOOT_SET_DLOAD_MODE);
>>> + ret = qcom_scm_io_readl(__scm->dload_mode_addr, &dload_addr_val);
>>> +
>> not checking ret value here before proceeding?
>>
> Okay, sure..will address in next patch series.
>>> if (avail) {
>>> - ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
>>> + ret = __qcom_scm_set_dload_mode(__scm->dev, dload_addr_val,
>>> enable);
>>> } else if (__scm->dload_mode_addr) {
>>> - ret = qcom_scm_io_writel(__scm->dload_mode_addr,
>>> - enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
>>> + ret = qcom_scm_io_writel(__scm->dload_mode_addr, enable ?
>>> + dload_addr_val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
>>> + dload_addr_val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE));
>>> } else {
>>> dev_err(__scm->dev,
>>> "No available mechanism for setting download mode\n");
>
> Regards,
> Poovendhan S
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-02-15 15:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 5:14 [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 1/5] dt-bindings: scm: Add compatible " Poovendhan Selvaraj
2023-02-14 7:54 ` Krzysztof Kozlowski
2023-02-14 5:14 ` [PATCH V4 2/5] dt-bindings: mfd: Add the tcsr " Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 3/5] arm64: dts: qcom: ipq9574: Enable the download mode support Poovendhan Selvaraj
2023-02-14 5:14 ` [PATCH V4 4/5] arm64: dts: qcom: ipq9574: Add SMEM support Poovendhan Selvaraj
2023-02-14 7:54 ` Krzysztof Kozlowski
2023-02-14 5:14 ` [PATCH V4 5/5] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode Poovendhan Selvaraj
2023-02-14 12:57 ` Srinivas Kandagatla
[not found] ` <b5135d69-3783-8147-bda5-8131cea726f7@quicinc.com>
2023-02-15 15:17 ` POOVENDHAN SELVARAJ
2023-02-15 15:17 ` Srinivas Kandagatla
2023-02-15 2:34 ` [PATCH V4 0/5] Enable crashdump collection support for IPQ9574 Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).