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) > > > > > > On 7/19/2018 11:12 AM, Bjorn Andersson wrote: > > > On Wed 18 Jul 22:18 PDT 2018, Rajendra Nayak wrote: > > > > > >> 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. > > > > > >> Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> > > >> --- > > >> .../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. > >
Attachment:
signature.asc
Description: PGP signature
