Re: [PATCH] power: reset: msm: Add support for download-mode control

Hi,

What's the status of this patch? It looks good to me, should it be
merged, did I miss a newer version?

-- Sebastian

On Fri, Jul 20, 2018 at 10:44:53AM -0700, Stephen Boyd wrote:

Quoting Rajendra Nayak (2018-07-18 23:59:20)

quoted

On 7/19/2018 11:12 AM, Bjorn Andersson wrote:

quoted

On Wed 18 Jul 22:18 PDT 2018, Rajendra Nayak wrote:

quoted

commit '8c1b7dc9b: firmware: qcom: scm: Expose download-mode control'
added support for download-mode control using the scm firmware
driver for platforms which require a secure call to write the magic
cookie into the tcsr location.

For platforms which *do not* need an scm call and where the kernel can
write the cookie by a direct read/write, add similar support in the
msm-poweroff driver.
Similar to the scm driver, the msm-poweroff driver clears the cookie
during a clean reboot.

Download mode using msm-poweroff driver can be enabled by including
msm-poweroff.download_mode=1 on the command line.

Should have thought about this when I wrote the scm code...

I would prefer if we could find a way to consolidate the two
implementations behind the same Kconfig and command line parameters.

The only problem I saw with this was that *both* drivers would think
its enabled and try their own ways to enable it, and one of it would
always fail. We could fail silently, which would mean we will miss
cases when its a genuine failure but that should be fine?

Should be fine if SCM fails silently. If TCSR is specified it better
work because that doesn't really rely on anything besides writing into a
memory location.

quoted

quoted

quoted

Signed-off-by: Rajendra Nayak <redacted>
---
  .../bindings/power/reset/msm-poweroff.txt          |  3 ++
  drivers/power/reset/Kconfig                        | 11 +++++++
  drivers/power/reset/msm-poweroff.c                 | 38 ++++++++++++++++++++++
  3 files changed, 52 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/reset/msm-poweroff.txt b/Documentation/devicetree/bindings/power/reset/msm-poweroff.txt
index ce44ad3..9dd489f 100644
--- a/Documentation/devicetree/bindings/power/reset/msm-poweroff.txt
+++ b/Documentation/devicetree/bindings/power/reset/msm-poweroff.txt

@@ -8,6 +8,9 @@ settings.
  Required Properties:
  -compatible: "qcom,pshold"
  -reg: Specifies the physical address of the ps-hold register
+Optional Properties:
+-qcom,dload-mode: phandle to the TCSR hardware block and offset of the
+             download mode control register
  
  Example:
  

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index df58fc8..0c97e34 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig

@@ -104,6 +104,17 @@ config POWER_RESET_MSM
     help
       Power off and restart support for Qualcomm boards.
  
+config POWER_RESET_MSM_DOWNLOAD_MODE

How about moving QCOM_SCM_DOWNLOAD_MODE_DEFAULT to
drivers/soc/qcom/Kconfig (and removing "SCM") and referencing this in
both drivers?

yes, thats possible, but I am not sure how to make the command line
option common for both. One other option I thought was if we could handle it
within the scm driver itself with an additional
binding to specify the non-secure download mode address.
something like qcom,dload-mode-ns?

Is the SCM device and driver always going to be present though? It may
be better to make a TCSR platform device driver on designs that would
configure the cookie with direct read/writes from Linux to break the
relationship with scm entirely. Then the different configurations could
flow from the DTS file either describing scm that has scm call, a
special scm_writel address for TCSR, or a specific TCSR node with the
address of the download mode cookie that triggers a TCSR driver to probe
and register a reboot handler.