| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20151208220722.GG4000@usrtlx11787.corpusers.net>
Date: Tue, 8 Dec 2015 14:07:22 -0800
From: Bjorn Andersson <bjorn.andersson@...ymobile.com>
To: John Stultz <john.stultz@...aro.org>
CC: lkml <linux-kernel@...r.kernel.org>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Vinay Simha BN <simhavcs@...il.com>,
Haojian Zhuang <haojian.zhuang@...aro.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Android Kernel Team <kernel-team@...roid.com>,
<agross@...eaurora.org>, <linux-arm-msm@...r.kernel.org>
Subject: Re: [RFC][PATCH] misc: Introduce reboot_reason driver
On Tue 08 Dec 13:29 PST 2015, John Stultz wrote:
> This patch adds a basic driver to allow for commands like
> "reboot bootloader" and "reboot recovery" to communicate this
> reboot-reason to the bootloader.
>
> This is commonly done on Android devices, in order to reboot
> the device into fastboot or recovery mode. It also supports
> custom OEM specific commands, via "reboot oem-<value>".
>
> This driver pulls the phys memory address from DT as well as
> the magic reason values that are written to the address for
> each mode.
>
> For an example, this patch also adds the DT support for
> the nexus7 device via its dts (which is not yet upstream).
>
> Thoughts and feedback would be appreciated!
>
Good to see some work on this, I want it too :)
I do however think this is Qualcomm specific in its implementation, so
adding Andy and linux-arm-msm.
[..]
> diff --git a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> index 5183d18..ee5dcb7 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> @@ -282,6 +282,15 @@
> };
> };
>
> + reboot_reason: reboot_reason@...3f65c {
> + compatible = "reboot_reason";
> + reg = <0x2A03F65C 0x4>;
> + reason,none = <0x77665501>;
> + reason,bootloader = <0x77665500>;
> + reason,recovery = <0x77665502>;
> + reason,oem = <0x6f656d00>;
> + };
> +
This address refers to IMEM, which is shared with a number of other
uses. So I think we should have a simple-mfd (and syscon) with this
within.
I like the fact that you don't hard code the magics in the
implementation, as I've seen additions from multiple places - so perhaps
it should be made even more flexible.
OMAP seems to use strings here instead of magics, but the delivery
mechanism looks to be the same. But I think of this as Qualcomm
specific.
> gpio-keys {
> compatible = "gpio-keys";
> power {
[..]
> diff --git a/drivers/misc/reboot_reason.c b/drivers/misc/reboot_reason.c
[..]
> +
> +static int reboot_reason(struct notifier_block *nb, unsigned long action,
> + void *data)
> +{
> + char *cmd = (char *)data;
> + long reason = reasons[NONE];
> +
> + if (!reboot_reason_addr)
> + return NOTIFY_DONE;
> +
> + if (cmd != NULL) {
> + if (!strncmp(cmd, "bootloader", 10))
> + reason = reasons[BOOTLOADER];
> + else if (!strncmp(cmd, "recovery", 8))
> + reason = reasons[RECOVERY];
> + else if (!strncmp(cmd, "oem-", 4)) {
> + unsigned long code;
> +
> + if (!kstrtoul(cmd+4, 0, &code))
> + reason = reasons[OEM] | (code & 0xff);
> + }
> + }
In the case where we didn't find a match you should write the "normal"
reason here, otherwise I think you will end up in recovery when recovery
issues a reboot (in Android that is).
> +
> + if (reason != -1)
> + writel(reason, reboot_reason_addr);
> + return NOTIFY_DONE;
> +}
> +
> +static int reboot_reason_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + u32 val;
> + int i;
> +
> + /* initialize the reasons */
> + for (i = 0; i < MAX_REASONS; i++)
> + reasons[i] = -1;
> +
> + /* Try to grab the reason io address */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reboot_reason_addr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(reboot_reason_addr))
> + return PTR_ERR(reboot_reason_addr);
> +
Please acquire this memory from a syscon (preferably the the parent
simple-mfd).
> + /* initialize specified reasons from DT */
> + if (!of_property_read_u32(pdev->dev.of_node, "reason,none", &val))
> + reasons[NONE] = val;
> + if (!of_property_read_u32(pdev->dev.of_node, "reason,bootloader", &val))
> + reasons[BOOTLOADER] = val;
> + if (!of_property_read_u32(pdev->dev.of_node, "reason,recovery", &val))
> + reasons[RECOVERY] = val;
> + if (!of_property_read_u32(pdev->dev.of_node, "reason,oem", &val))
> + reasons[OEM] = val;
I would like for this to be less hard coded.
> +
> + /* Install the notifier */
> + restart_nb.notifier_call = reboot_reason;
> + restart_nb.priority = 256;
> + if (register_restart_handler(&restart_nb)) {
> + dev_err(&pdev->dev,
> + "failed to setup restart handler.\n");
> + }
> + return 0;
> +}
> +
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists