Re: [PATCH 2/3] dt-bindings: qcom,pon: Add 'qcom,mode-in-imem' support

[Stephan Gerhold <stephan@gerhold.net>]

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@linaro.org>
> ---
>  .../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.)

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?

Thanks!
Stephan