On Wed 09 Dec 17:32 PST 2015, John Stultz wrote:

On Tue, Dec 8, 2015 at 2:07 PM, Bjorn Andersson
[off-list ref] wrote:

quoted

On Tue 08 Dec 13:29 PST 2015, John Stultz wrote:

quoted

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@2a03f65c {
+                     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.

So talking with Arnd some more it looked like IMEM was really just
SRAM. Is that not the case, or is there something else special about
it?  Does it really need simple-mfd and syscon? I'm still fuzzy on how
to use those for this.

I'm pretty sure it's just SRAM, but I hadn't looked at that binding
before, sounds like a conceptually better fit.

The part that I was looking for was the convenience of having a regmap
available for the uses that we will find later on, but I guess sram
provides similar means of accessing various pieces of the memory.

quoted

quoted

+     /* 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.

So thinking of this more. Is having something like:

cmds = "default", "bootloader", "recovery";
vals  = <0xmagic1>, <0xmagic2>, <0xmagic3>;

what you're thinking about?

As these are normally just ascii strings I was thinking we could have
them as individual properties and then use for_each_property_of_node()
on the implementation side. But it doesn't really matter.

This wouldn't quite handle the "oem-N" options as simply, but they
could define each oem- case explicitly in the DT to support it.

If we have a reasonably dynamic way of defining these there's little to
no reason to treat oem-N specially from the others.

Regards,
Bjorn