| Message ID | 20230724223057.1208122-1-quic_eberman@quicinc.com |
|---|---|
| Headers | show |
| Series | Implement a PSCI SYSTEM_RESET2 reboot-mode driver | expand |
On 7/25/2023 4:00 AM, Elliot Berman wrote: > Add devicetree bindings for using PSCI SYSTEM_RESET2 with vendor reset types. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > .../power/reset/arm,psci-vendor-reset.yaml | 35 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 36 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml > > diff --git a/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml b/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml > new file mode 100644 > index 000000000000..18b0b8c167a1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml > @@ -0,0 +1,35 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Qualcomm Innovation Center, Inc. All Rights Reserved. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/power/reset/arm,psci-vendor-reset.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: PSCI SYSTEM_RESET2 Vendor Resets > + > +maintainers: > + - Elliot Berman <quic_eberman@quicinc.com> > + > +description: | > + PSCI SYSTEM_RESET2 supports vendor-defined reset types. This describes > + the conversion of reboot modes to the reset types. > + > +properties: > + compatible: > + const: arm,psci-vendor-reset > + > +allOf: > + - $ref: reboot-mode.yaml# > + > +additionalProperties: false > + > +examples: > + - | > + firmware { > + psci-vendor-resets { > + compatible = "arm,psci-vendor-reset"; > + reboot-normal = <0x100>; > + reboot-bootloader = <0x101>; > + reboot-fastboot = <0x102>; Should it start with mode-* ? -Mukesh > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index d516295978a4..2da4c5f1917b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -16982,6 +16982,7 @@ M: Mark Rutland <mark.rutland@arm.com> > M: Lorenzo Pieralisi <lpieralisi@kernel.org> > L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > S: Maintained > +F: Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml > F: drivers/firmware/psci/ > F: include/linux/psci.h > F: include/uapi/linux/psci.h
On 7/24/2023 4:23 PM, Rob Herring wrote: > On Mon, Jul 24, 2023 at 03:30:53PM -0700, Elliot Berman wrote: >> Add devicetree bindings for using PSCI SYSTEM_RESET2 with vendor reset types. >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> .../power/reset/arm,psci-vendor-reset.yaml | 35 +++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 36 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >> >> diff --git a/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml b/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >> new file mode 100644 >> index 000000000000..18b0b8c167a1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >> @@ -0,0 +1,35 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +# Copyright 2023 Qualcomm Innovation Center, Inc. All Rights Reserved. >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/power/reset/arm,psci-vendor-reset.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: PSCI SYSTEM_RESET2 Vendor Resets >> + >> +maintainers: >> + - Elliot Berman <quic_eberman@quicinc.com> >> + >> +description: | >> + PSCI SYSTEM_RESET2 supports vendor-defined reset types. This describes >> + the conversion of reboot modes to the reset types. >> + >> +properties: >> + compatible: >> + const: arm,psci-vendor-reset >> + >> +allOf: >> + - $ref: reboot-mode.yaml# >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + firmware { >> + psci-vendor-resets { >> + compatible = "arm,psci-vendor-reset"; > > We already have a node for PSCI, we don't need a second one. You can > have a separate driver without a separate node. > I could also place the reboot-mode functionality straight into drivers/firwmare/psci/? I thought that might be more controversial than separate driver, but maybe not? Mark/Loreno, do you have any concerns to add the reboot-mode driver functionality directly in drivers/firmware/psci/psci.c? Sebastian, do you have any concerns to have this reboot-mode driver outside drivers/power/reset/? >> + reboot-normal = <0x100>; > > Wouldn't 'normal' be the normal PSCI reset? > Ah, right. I had my head buried in the reboot-mode code when creating the example. I can remove from the example. >> + reboot-bootloader = <0x101>; >> + reboot-fastboot = <0x102>; >> + }; >> + }; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index d516295978a4..2da4c5f1917b 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -16982,6 +16982,7 @@ M: Mark Rutland <mark.rutland@arm.com> >> M: Lorenzo Pieralisi <lpieralisi@kernel.org> >> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) >> S: Maintained >> +F: Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >> F: drivers/firmware/psci/ >> F: include/linux/psci.h >> F: include/uapi/linux/psci.h >> -- >> 2.41.0 >>
Hello, On 7/24/23 15:30, Elliot Berman wrote: > PSCI implements a restart notifier for architectural defined resets. > The SYSTEM_RESET2 call allows vendor firmware to define additional reset > types which could be mapped to the reboot reason. > > Implement a driver to wire the reboot-mode framework to make vendor > SYSTEM_RESET2 calls on reboot. > > This is a continuation from https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/ Would appreciate being CC'd on a the non-RFC postings of this patch. FWIW, my use case is better described with this earlier submission: https://lore.kernel.org/lkml/20220122035421.4086618-1-f.fainelli@gmail.com/T/#m74e4243c1af3a8d896e19b573b58f562fa09961d It would be neat if I could leverage your driver in order to implement this custom "reboot powercycle" implementation. Towards that goal, we would likely need to specify the desired reboot "sub" operation alongside its PSCI SYSTEM_RESET2 reboot type argument? Thanks!
On 7/25/2023 11:01 AM, Elliot Berman wrote: > > > On 7/24/2023 4:23 PM, Rob Herring wrote: >> On Mon, Jul 24, 2023 at 03:30:53PM -0700, Elliot Berman wrote: >>> Add devicetree bindings for using PSCI SYSTEM_RESET2 with vendor >>> reset types. >>> >>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >>> --- >>> .../power/reset/arm,psci-vendor-reset.yaml | 35 +++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 2 files changed, 36 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml b/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >>> new file mode 100644 >>> index 000000000000..18b0b8c167a1 >>> --- /dev/null >>> +++ >>> b/Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >>> @@ -0,0 +1,35 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +# Copyright 2023 Qualcomm Innovation Center, Inc. All Rights Reserved. >>> +%YAML 1.2 >>> +--- >>> +$id: >>> http://devicetree.org/schemas/power/reset/arm,psci-vendor-reset.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: PSCI SYSTEM_RESET2 Vendor Resets >>> + >>> +maintainers: >>> + - Elliot Berman <quic_eberman@quicinc.com> >>> + >>> +description: | >>> + PSCI SYSTEM_RESET2 supports vendor-defined reset types. This >>> describes >>> + the conversion of reboot modes to the reset types. >>> + >>> +properties: >>> + compatible: >>> + const: arm,psci-vendor-reset >>> + >>> +allOf: >>> + - $ref: reboot-mode.yaml# >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + firmware { >>> + psci-vendor-resets { >>> + compatible = "arm,psci-vendor-reset"; >> >> We already have a node for PSCI, we don't need a second one. You can >> have a separate driver without a separate node. >> > > I could also place the reboot-mode functionality straight into > drivers/firwmare/psci/? I thought that might be more controversial than > separate driver, but maybe not? > > Mark/Loreno, do you have any concerns to add the reboot-mode driver > functionality directly in drivers/firmware/psci/psci.c? > > Sebastian, do you have any concerns to have this reboot-mode driver > outside drivers/power/reset/? > Sebastian, please disregard this question. Mukesh pointed out that reboot-mode framework isn't the right option here since this driver does the actual reset and, as I understand, reboot-mode isn't intended to do actual reset. I'm going to implement something similar to what reboot-mode framework does but register against the restart_handler_list instead of reboot_notifier_list. >>> + reboot-normal = <0x100>; >> >> Wouldn't 'normal' be the normal PSCI reset? >> > > Ah, right. I had my head buried in the reboot-mode code when creating > the example. I can remove from the example. > >>> + reboot-bootloader = <0x101>; >>> + reboot-fastboot = <0x102>; >>> + }; >>> + }; >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index d516295978a4..2da4c5f1917b 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -16982,6 +16982,7 @@ M: Mark Rutland <mark.rutland@arm.com> >>> M: Lorenzo Pieralisi <lpieralisi@kernel.org> >>> L: linux-arm-kernel@lists.infradead.org (moderated for >>> non-subscribers) >>> S: Maintained >>> +F: >>> Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml >>> F: drivers/firmware/psci/ >>> F: include/linux/psci.h >>> F: include/uapi/linux/psci.h >>> -- >>> 2.41.0 >>>
On 7/25/2023 12:12 PM, Florian Fainelli wrote: > Hello, > > On 7/24/23 15:30, Elliot Berman wrote: >> PSCI implements a restart notifier for architectural defined resets. >> The SYSTEM_RESET2 call allows vendor firmware to define additional reset >> types which could be mapped to the reboot reason. >> >> Implement a driver to wire the reboot-mode framework to make vendor >> SYSTEM_RESET2 calls on reboot. >> >> This is a continuation from >> https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/ > > Would appreciate being CC'd on a the non-RFC postings of this patch. > FWIW, my use case is better described with this earlier submission: > > https://lore.kernel.org/lkml/20220122035421.4086618-1-f.fainelli@gmail.com/T/#m74e4243c1af3a8d896e19b573b58f562fa09961d > > It would be neat if I could leverage your driver in order to implement > this custom "reboot powercycle" implementation. Towards that goal, we > would likely need to specify the desired reboot "sub" operation > alongside its PSCI SYSTEM_RESET2 reboot type argument? > > Thanks! I think you you want to describe the PSCI vendor reset under a warm reboot with command "powercycle"? In other words, my series only lets DT describe either reboot_mode (warm) or cmd (powercycle) but not both simultaneously? Please correct me if I got it wrong! Otherwise, I can incorporate way to describe vendor reset type matching both reboot_mode and cmd in the DT. - Elliot
On 7/25/23 13:27, Elliot Berman wrote: > > > On 7/25/2023 12:12 PM, Florian Fainelli wrote: >> Hello, >> >> On 7/24/23 15:30, Elliot Berman wrote: >>> PSCI implements a restart notifier for architectural defined resets. >>> The SYSTEM_RESET2 call allows vendor firmware to define additional reset >>> types which could be mapped to the reboot reason. >>> >>> Implement a driver to wire the reboot-mode framework to make vendor >>> SYSTEM_RESET2 calls on reboot. >>> >>> This is a continuation from >>> https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/ >> >> Would appreciate being CC'd on a the non-RFC postings of this patch. >> FWIW, my use case is better described with this earlier submission: >> >> https://lore.kernel.org/lkml/20220122035421.4086618-1-f.fainelli@gmail.com/T/#m74e4243c1af3a8d896e19b573b58f562fa09961d >> >> It would be neat if I could leverage your driver in order to implement >> this custom "reboot powercycle" implementation. Towards that goal, we >> would likely need to specify the desired reboot "sub" operation >> alongside its PSCI SYSTEM_RESET2 reboot type argument? >> >> Thanks! > > I think you you want to describe the PSCI vendor reset under a warm > reboot with command "powercycle"? In other words, my series only lets DT > describe either reboot_mode (warm) or cmd (powercycle) but not both > simultaneously? I did not give a lot of thoughts into the different types of reboot (warm, soft, cold) and just went with an extension of whichever reboot type we have to be supplemented by the "powercycle" command. It seems like we should support both the reboot type and command, it would be fine with me if I had to specify reboot_mode + cmd for each reboot_mode.