Hi Andy, Am Dienstag, 22. Dezember 2015, 17:16:52 schrieb Andy Yan: > Add reboot mode driver DT node for rk3xxx,rk3288 platform in general, I like that concept a lot :-) . Below some small issues. > Signed-off-by: Andy Yan <andy.yan@xxxxxxxxxxxxxx> > > --- > > Changes in v1: > - correct the maskrom magic number > - use macro defined in rockchip_boot-mode.h for reboot-mode DT node > > arch/arm/boot/dts/rk3288.dtsi | 26 ++++++++++++++++++++++++++ > arch/arm/boot/dts/rk3xxx.dtsi | 26 ++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi > index 04ea209..c6ea207 100644 > --- a/arch/arm/boot/dts/rk3288.dtsi > +++ b/arch/arm/boot/dts/rk3288.dtsi > @@ -45,6 +45,7 @@ > #include <dt-bindings/clock/rk3288-cru.h> > #include <dt-bindings/thermal/thermal.h> > #include <dt-bindings/power/rk3288-power.h> > +#include <dt-bindings/soc/rockchip_boot-mode.h> > #include "skeleton.dtsi" > > / { > @@ -170,6 +171,31 @@ > }; > }; > > + reboot_mode { nodes are commonly written with a dash "-" instead of an underscore "_". > + compatible = "rockchip,reboot-mode"; > + rockchip,regmap = <&pmu>; I do believe this should be a sub-node of the pmu, similar to the power- domains. Due to the "simple-mfd" compatible of the pmu, it will get probed automatically and the driver can than get the regmap via parent = dev->parent; regmap = syscon_node_to_regmap(parent->of_node); as the power-domains do and therefore also doesn't need the rockchip,regmap property. > + offset = <0x94>; > + loader { > + linux,mode = "loader"; > + linux,magic = <BOOT_LOADER>; linux,mode seems correct, but the magic value is actually not linux-specific but instead firmware-specific, so I'm not sure about that linux-prefix there But I guess that would be for the dt-people :-) > + }; > + > + maskrom { > + linux,mode = "maskrom"; > + linux,magic = <BOOT_MASKROM>; > + }; Just for my understanding, while I expect the bootloader to interpret the other values, does the maskrom read and handle this one? > + > + recovery { > + linux,mode = "recovery"; > + linux,magic = <BOOT_RECOVERY>; > + }; > + > + fastboot { > + linux,mode = "fastboot"; > + linux,magic = <BOOT_FASTBOOT>; > + }; > + }; > + > reserved-memory { > #address-cells = <1>; > #size-cells = <1>; I do believe rk3288-veyron.dtsi would need some sort of special handling. I don't think coreboot interprets most of those handles, but I guess only the maskrom one would be handled by the maskrom? Heiko -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html