diff mbox series

[v2,4/4] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp

Message ID 20240414-arm-psci-system_reset2-vendor-reboots-v2-4-da9a055a648f@quicinc.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series Implement vendor resets for PSCI SYSTEM_RESET2 | expand

Commit Message

Elliot Berman April 14, 2024, 7:30 p.m. UTC
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 <quic_spratap@quicinc.com>
Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Dmitry Baryshkov April 14, 2024, 11:13 p.m. UTC | #1
On Sun, 14 Apr 2024 at 22:32, Elliot Berman <quic_eberman@quicinc.com> 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 <quic_spratap@quicinc.com>
> Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  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
>
>
Elliot Berman April 15, 2024, 12:32 a.m. UTC | #2
On Mon, Apr 15, 2024 at 02:13:29AM +0300, Dmitry Baryshkov wrote:
> On Sun, 14 Apr 2024 at 22:32, Elliot Berman <quic_eberman@quicinc.com> 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 <quic_spratap@quicinc.com>
> > Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
> > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > ---
> >  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
Konrad Dybcio April 15, 2024, 7:42 p.m. UTC | #3
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 <quic_eberman@quicinc.com> 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 <quic_spratap@quicinc.com>
>>> Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
>>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>>> ---
>>>   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
Elliot Berman April 16, 2024, 1:11 a.m. UTC | #4
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 <quic_eberman@quicinc.com> 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 <quic_spratap@quicinc.com>
> > > > Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
> > > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > > > ---
> > > >   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.
diff mbox series

Patch

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";