2024-04-14 19:32:20

by Elliot Berman

[permalink] [raw]
Subject: [PATCH v2 4/4] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp

Add nodes for the vendor-defined system resets. "bootloader" will cause
device to reboot and stop in the bootloader's fastboot mode. "edl" will
cause device to reboot into "emergency download mode", which permits
loading images via the Firehose protocol.

Co-developed-by: Shivendra Pratap <[email protected]>
Signed-off-by: Shivendra Pratap <[email protected]>
Signed-off-by: Elliot Berman <[email protected]>
---
arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index e4bfad50a669..a966f6c8dd7c 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -126,6 +126,11 @@ debug_vm_mem: debug-vm@d0600000 {
};
};

+ psci {
+ mode-bootloader = <0x10001 0x2>;
+ mode-edl = <0 0x1>;
+ };
+
vph_pwr: vph-pwr-regulator {
compatible = "regulator-fixed";
regulator-name = "vph_pwr";

--
2.34.1



2024-04-14 23:13:53

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp

On Sun, 14 Apr 2024 at 22:32, Elliot Berman <[email protected]> wrote:
>
> Add nodes for the vendor-defined system resets. "bootloader" will cause
> device to reboot and stop in the bootloader's fastboot mode. "edl" will
> cause device to reboot into "emergency download mode", which permits
> loading images via the Firehose protocol.
>
> Co-developed-by: Shivendra Pratap <[email protected]>
> Signed-off-by: Shivendra Pratap <[email protected]>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index e4bfad50a669..a966f6c8dd7c 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> @@ -126,6 +126,11 @@ debug_vm_mem: debug-vm@d0600000 {
> };
> };
>
> + psci {

Please use a label instead. Otherwise it looks as if you are adding
new device node.

> + mode-bootloader = <0x10001 0x2>;
> + mode-edl = <0 0x1>;
> + };
> +
> vph_pwr: vph-pwr-regulator {
> compatible = "regulator-fixed";
> regulator-name = "vph_pwr";
>
> --
> 2.34.1
>
>


--
With best wishes
Dmitry

2024-04-15 00:33:15

by Elliot Berman

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp

On Mon, Apr 15, 2024 at 02:13:29AM +0300, Dmitry Baryshkov wrote:
> On Sun, 14 Apr 2024 at 22:32, Elliot Berman <[email protected]> wrote:
> >
> > Add nodes for the vendor-defined system resets. "bootloader" will cause
> > device to reboot and stop in the bootloader's fastboot mode. "edl" will
> > cause device to reboot into "emergency download mode", which permits
> > loading images via the Firehose protocol.
> >
> > Co-developed-by: Shivendra Pratap <[email protected]>
> > Signed-off-by: Shivendra Pratap <[email protected]>
> > Signed-off-by: Elliot Berman <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > index e4bfad50a669..a966f6c8dd7c 100644
> > --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > @@ -126,6 +126,11 @@ debug_vm_mem: debug-vm@d0600000 {
> > };
> > };
> >
> > + psci {
>
> Please use a label instead. Otherwise it looks as if you are adding
> new device node.
>

Right. Fixed for the next revision.

Thanks,
Elliot

> > + mode-bootloader = <0x10001 0x2>;
> > + mode-edl = <0 0x1>;
> > + };
> > +
> > vph_pwr: vph-pwr-regulator {
> > compatible = "regulator-fixed";
> > regulator-name = "vph_pwr";
> >
> > --
> > 2.34.1
> >
> >
>
>
> --
> With best wishes
> Dmitry

2024-04-15 19:50:32

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp



On 4/15/24 02:32, Elliot Berman wrote:
> On Mon, Apr 15, 2024 at 02:13:29AM +0300, Dmitry Baryshkov wrote:
>> On Sun, 14 Apr 2024 at 22:32, Elliot Berman <[email protected]> wrote:
>>>
>>> Add nodes for the vendor-defined system resets. "bootloader" will cause
>>> device to reboot and stop in the bootloader's fastboot mode. "edl" will
>>> cause device to reboot into "emergency download mode", which permits
>>> loading images via the Firehose protocol.
>>>
>>> Co-developed-by: Shivendra Pratap <[email protected]>
>>> Signed-off-by: Shivendra Pratap <[email protected]>
>>> Signed-off-by: Elliot Berman <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>> index e4bfad50a669..a966f6c8dd7c 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>> @@ -126,6 +126,11 @@ debug_vm_mem: debug-vm@d0600000 {
>>> };
>>> };
>>>
>>> + psci {
>>
>> Please use a label instead. Otherwise it looks as if you are adding
>> new device node.
>>
>
> Right. Fixed for the next revision.

Are you guys planning to make this sorta ABI-like?

If so (which would be greatly appreciated by the way..), perhaps you
could stick these magic values in dt-bindings and give them cool names

FWIW DEN0022 (my second-favorite book) suggests these values are almost
totally vendor-defined, so if I were Qualcomm, I'd take the creative
liberty to come up with a set of numbers and never ever ever change
them

Konrad

2024-04-16 01:19:10

by Elliot Berman

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp

On Mon, Apr 15, 2024 at 09:42:40PM +0200, Konrad Dybcio wrote:
>
>
> On 4/15/24 02:32, Elliot Berman wrote:
> > On Mon, Apr 15, 2024 at 02:13:29AM +0300, Dmitry Baryshkov wrote:
> > > On Sun, 14 Apr 2024 at 22:32, Elliot Berman <[email protected]> wrote:
> > > >
> > > > Add nodes for the vendor-defined system resets. "bootloader" will cause
> > > > device to reboot and stop in the bootloader's fastboot mode. "edl" will
> > > > cause device to reboot into "emergency download mode", which permits
> > > > loading images via the Firehose protocol.
> > > >
> > > > Co-developed-by: Shivendra Pratap <[email protected]>
> > > > Signed-off-by: Shivendra Pratap <[email protected]>
> > > > Signed-off-by: Elliot Berman <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > > > index e4bfad50a669..a966f6c8dd7c 100644
> > > > --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > > > @@ -126,6 +126,11 @@ debug_vm_mem: debug-vm@d0600000 {
> > > > };
> > > > };
> > > >
> > > > + psci {
> > >
> > > Please use a label instead. Otherwise it looks as if you are adding
> > > new device node.
> > >
> >
> > Right. Fixed for the next revision.
>
> Are you guys planning to make this sorta ABI-like?
>
> If so (which would be greatly appreciated by the way..), perhaps you
> could stick these magic values in dt-bindings and give them cool names
>
> FWIW DEN0022 (my second-favorite book) suggests these values are almost
> totally vendor-defined, so if I were Qualcomm, I'd take the creative
> liberty to come up with a set of numbers and never ever ever change
> them

This is my goal as well. I'd like to keep the magic values out of
dt-bindings until we get the vendor SYSTEM_RESET2 spread across more
devices, as things might need a bit of settling. Since having stable
vendor reset2 is (IMO) primarily a benefit to Qualcomm, I expect this
will happen naturally.