DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 87BD960227
Subject: Re: [PATCH v3 2/3] firmware: qcom: scm: Expose download-mode control
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
        Andy Gross <andy.gross@linaro.org>,
        David Brown <david.brown@linaro.org>, Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-soc@vger.kernel.org
References: <20170809201033.4967-1-bjorn.andersson@linaro.org>
 <20170809201033.4967-3-bjorn.andersson@linaro.org>
From: Stephen Boyd <sboyd@codeaurora.org>
Message-ID: <6d3e78d7-9fdb-c99f-9309-363cbb54ec04@codeaurora.org>
Date: Thu, 10 Aug 2017 16:48:06 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20170809201033.4967-3-bjorn.andersson@linaro.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3255
Lines: 85

On 08/09/2017 01:10 PM, Bjorn Andersson wrote:
> In order to aid post-mortem debugging the Qualcomm platforms provides a

s/provides/provide/

> "memory download mode", where the boot loader will provide an interface
> for custom tools to "download" the content of RAM to a host machine.
>
> The mode is triggered by writing a magic value somehwere in RAM, that is

s/somehwere/somewhere/

> read in the boot code path after a warm-restart. Two mechanism for
> setting this magic value are supported in modern platforms; a direct SCM
> call to enable the mode or through a secure io write of a magic value.
>
> In order for a normal reboot not to trigger "download mode" the magic
> must be cleared during a clean reboot.
>
> Download mode has to be enabled by including qcom_scm.download_mode=1 on
> the command line.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - Define memory address by reference to TCSR
> - Provide Kconfig option for defaulting to enabled
> - Provided some comment in the code
>
> Changes since v1:
> - Specify DT propert being two-cell
> - Correct handling of scm-call return code
>
>  .../devicetree/bindings/firmware/qcom,scm.txt      |  2 +
>  drivers/firmware/Kconfig                           | 11 ++++
>  drivers/firmware/qcom_scm-32.c                     |  6 ++
>  drivers/firmware/qcom_scm-64.c                     | 13 ++++
>  drivers/firmware/qcom_scm.c                        | 69 ++++++++++++++++++++++
>  drivers/firmware/qcom_scm.h                        |  2 +
>  6 files changed, 103 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> index 20f26fbce875..838dba841797 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> @@ -18,6 +18,8 @@ Required properties:
>   * Core, iface, and bus clocks required for "qcom,scm"
>  - clock-names: Must contain "core" for the core clock, "iface" for the interface
>    clock and "bus" for the bus clock per the requirements of the compatible.
> +- qcom,dload-mode: phandle reference to the TCSR hardware block containing the
> +		   download mode control register (optional)
>  
>  Example for MSM8916:
>  
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 6e4ed5a9c6fd..ed23c77d6361 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -215,6 +215,17 @@ config QCOM_SCM_64
>  	def_bool y
>  	depends on QCOM_SCM && ARM64
>  
> +config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
> +	bool "Qualcomm Download mode enabled by default"

Decapitalize download?

> +	depends on QCOM_SCM
> +	help
> +	  A device with "download mode" enabled will upon an unexpected
> +	  warm-restart enter a special debug mode that allows the user to
> +	  "download" memory content over USB for offline postmortem analysis.
> +	  The feature can be enabled/disabled on the kernel command line.
> +
> +	  Say Y here to enable "download mode" by default.
> +

otherwise

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project