On Sun, Jul 04, 2021 at 11:36:41AM +0200, Stephan Gerhold wrote: > On Sun, Jul 04, 2021 at 03:40:44PM +0800, Shawn Guo wrote: > > It's not always the case that reboot mode value gets stored in PON > > register. For example, Sony Xperia M4 Aqua phone (MSM8939) uses a > > different set of mode value and stores them in IMEM. Add property > > 'qcom,mode-in-imem' to distinguish this mechanism from the existing one. > > > > Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxx> > > --- > > .../bindings/power/reset/qcom,pon.yaml | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml > > index 7764c804af1d..a6270e39b7a2 100644 > > --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml > > +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml > > @@ -23,6 +23,10 @@ properties: > > reg: > > maxItems: 1 > > > > + qcom,mode-in-imem: > > + description: Reboot mode is stored in IMEM rather than PON register > > + type: boolean > > + > > patternProperties: > > "^mode-.+": > > $ref: /schemas/types.yaml#/definitions/uint32 > > @@ -35,6 +39,7 @@ required: > > additionalProperties: false > > > > examples: > > + # Example 1: Reboot mode is stored in PON register > > - | > > pmic { > > #address-cells = <1>; > > @@ -47,3 +52,17 @@ examples: > > mode-recovery = <0x1>; > > }; > > }; > > + # Example 2: Reboot mode is stored in IMEM > > + - | > > + pmic { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pon@800 { > > + compatible = "qcom,pm8916-pon"; > > + reg = <0x860065c>; > > This is quite strange. pon@800 is a node of the PM8916 PMIC, > so the reg should refer to the address inside the PMIC, not some memory > address of the SoC. dtc will probably warn about this too since the unit > address (@800) should match the first reg. (At least on W=1.) Fair point. > Actually we already have some devices using IMEM for the reboot mode, > like this (qcom-msm8974.dtsi plus qcom-msm8974-fairphone-fp2.dts): > > imem@fe805000 { > compatible = "syscon", "simple-mfd"; > reg = <0xfe805000 0x1000>; > > reboot-mode { > compatible = "syscon-reboot-mode"; > offset = <0x65c>; > mode-normal = <0x77665501>; > mode-bootloader = <0x77665500>; > mode-recovery = <0x77665502>; > }; > }; > > Perhaps it would be cleaner to add a property to disable the reboot mode > functionality of pm8916-pon and then set it up like this? Ah, yes, this is sensible. Thanks for the msm8974 example! Shawn