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. > 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? > + bool "Qualcomm download mode enabled by default" > + depends on POWER_RESET_MSM > + 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. > + > config POWER_RESET_OCELOT_RESET > bool "Microsemi Ocelot reset driver" > depends on MSCC_OCELOT || COMPILE_TEST > diff --git a/drivers/power/reset/msm-poweroff.c b/drivers/power/reset/msm-poweroff.c > index 01b8c71..c33eac5 100644 > --- a/drivers/power/reset/msm-poweroff.c > +++ b/drivers/power/reset/msm-poweroff.c > @@ -18,11 +18,20 @@ > #include <linux/io.h> > #include <linux/of.h> > #include <linux/platform_device.h> > +#include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/reboot.h> > +#include <linux/regmap.h> > #include <linux/pm.h> > > +static bool download_mode = IS_ENABLED(CONFIG_POWER_RESET_MSM_DOWNLOAD_MODE); > +module_param(download_mode, bool, 0); Iirc it's possible to have multiple implementations of __setup() for the same string. I'm uncertain if this would be considered okay though. > + > +#define QCOM_SET_DLOAD_MODE 0x10 > static void __iomem *msm_ps_hold; > +static struct regmap *tcsr_regmap; > +static unsigned int dload_mode_offset; > + > static int deassert_pshold(struct notifier_block *nb, unsigned long action, > void *data) > { > @@ -44,9 +53,30 @@ static void do_msm_poweroff(void) > > static int msm_restart_probe(struct platform_device *pdev) > { > + int ret; > + struct of_phandle_args args; > struct device *dev = &pdev->dev; > struct resource *mem; > > + if (download_mode) { > + ret = of_parse_phandle_with_fixed_args(dev->of_node, > + "qcom,dload-mode", 1, 0, > + &args); > + if (ret < 0) > + return ret; > + > + tcsr_regmap = syscon_node_to_regmap(args.np); > + of_node_put(args.np); > + if (IS_ERR(tcsr_regmap)) > + return PTR_ERR(tcsr_regmap); > + > + dload_mode_offset = args.args[0]; > + > + /* Enable download mode by writing the cookie */ > + regmap_write(tcsr_regmap, dload_mode_offset, > + QCOM_SET_DLOAD_MODE); > + } > + > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > msm_ps_hold = devm_ioremap_resource(dev, mem); > if (IS_ERR(msm_ps_hold)) > @@ -59,6 +89,13 @@ static int msm_restart_probe(struct platform_device *pdev) > return 0; > } > > +static void msm_restart_shutdown(struct platform_device *pdev) > +{ > + /* Clean shutdown, disable download mode to allow normal restart */ > + if (download_mode) > + regmap_write(tcsr_regmap, dload_mode_offset, 0x0); > +} > + > static const struct of_device_id of_msm_restart_match[] = { > { .compatible = "qcom,pshold", }, > {}, > @@ -71,6 +108,7 @@ static int msm_restart_probe(struct platform_device *pdev) > .name = "msm-restart", > .of_match_table = of_match_ptr(of_msm_restart_match), > }, > + .shutdown = msm_restart_shutdown, > }; Implementation looks good. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html