Message ID | 20240119132521.3609945-1-o.rempel@pengutronix.de |
---|---|
Headers | show |
Series | Introduction of PSCR Framework and Related Components | expand |
On Fri, 19 Jan 2024 14:25:15 +0100, Oleksij Rempel wrote: > Add binding for Power State Change Reason (PSCR) subsystem > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > .../devicetree/bindings/power/reset/pscr.yaml | 51 +++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/reset/pscr.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/reset/pscr.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename $id: http://devicetree.org/schemas/power/state-change/pscr.yaml file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/reset/pscr.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240119132521.3609945-2-o.rempel@pengutronix.de The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Fri, Jan 19, 2024 at 11:28:38AM -0600, Rob Herring wrote: > On Fri, Jan 19, 2024 at 02:25:15PM +0100, Oleksij Rempel wrote: > > Add binding for Power State Change Reason (PSCR) subsystem > > Why? > > How is this different from the reboot reason binding? I was not able to find "reboot reason", you probably refer to "reboot mode". Reboot Mode: Purpose: Dictates how the system should reboot (e.g., normal, recovery, bootloader). Usage: Provides instructions for the next boot mode. Scenario: Utilized in planned reboots or software-triggered reset scenarios. PSCR (Power State Change Reasons): Purpose: Logs the reason behind a power state change (e.g., voltage drop, over-temperature). Usage: Used for rapid logging and post-event analysis, potentially informing automatic decision-making in subsequent boots. Scenario: Critical in abrupt power-down situations where immediate, detailed decision-making is not possible. Regards, Oleksij
On Fri, Jan 19, 2024 at 8:34 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > On Fri, Jan 19, 2024 at 07:34:26PM +0100, Rafael J. Wysocki wrote: > > On Fri, Jan 19, 2024 at 2:25 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > > > Store the state change reason to some black box for later investigation. > > > > Seriously? > > > > What black box, where, how this is useful and who is going to use it, > > pretty please. > > The 'black box' refers to a non-volatile memory (NVMEM) cell used by the > Power State Change Reasons (PSCR) framework. This cell stores reasons > for sudden power state changes, like voltage drops or over-temperature > events. This data is invaluable for post-mortem analysis to understand > system failures or abrupt shutdowns. It's particularly useful for > systems where PMICs or watchdogs cannot record such events. The data can > inform recovery routines in the bootloader or early kernel stages during > subsequent boots, enhancing system reliability and aiding in debugging > and diagnostics. OK, so please add all of the above to the patch changelog.
Hi, On Fri, Jan 19, 2024 at 02:25:14PM +0100, Oleksij Rempel wrote: > This patch series introduces the Power State Change Reasons (PSCR) > tracking framework and its related components into the kernel. The PSCR > framework is designed for systems where traditional methods of storing > power state change reasons, like PMICs or watchdogs, are inadequate. It > provides a structured way to store reasons for system shutdowns and > reboots, such as under-voltage or software-triggered events, in > non-volatile hardware storage. > > These changes are critical for systems requiring detailed postmortem > analysis and where immediate power-down scenarios limit traditional > storage options. The framework also assists bootloaders and early-stage > system components in making informed recovery decisions. A couple of things come to my mind: 1. Do we need the DT based reason-string-to-integer mapping? Can we just use a fixed mapping instead? It simplifies the binding a lot. With that the generic part could be dropped completely. 2. I would expect the infrastructure to read and clear the reason during boot. If e.g. a watchdog triggers a reset you will otherwise get an incorrect value. 3. The reason is only stored, but not used? We have a sysfs ABI to expose the reboot reason to userspace since half a year ago, see d40befed9a58 (power: reset: at91-reset: add sysfs interface to the power on reason). 4. Available options should be synced with the list in include/linux/power/power_on_reason.h -- Sebastian
Hi, On Sat, Jan 20, 2024 at 12:19:09AM +0100, Sebastian Reichel wrote: > Hi, > > On Fri, Jan 19, 2024 at 02:25:14PM +0100, Oleksij Rempel wrote: > > This patch series introduces the Power State Change Reasons (PSCR) > > tracking framework and its related components into the kernel. The PSCR > > framework is designed for systems where traditional methods of storing > > power state change reasons, like PMICs or watchdogs, are inadequate. It > > provides a structured way to store reasons for system shutdowns and > > reboots, such as under-voltage or software-triggered events, in > > non-volatile hardware storage. > > > > These changes are critical for systems requiring detailed postmortem > > analysis and where immediate power-down scenarios limit traditional > > storage options. The framework also assists bootloaders and early-stage > > system components in making informed recovery decisions. > > A couple of things come to my mind: > > 1. Do we need the DT based reason-string-to-integer mapping? Can we > just use a fixed mapping instead? It simplifies the binding a > lot. With that the generic part could be dropped completely. The project I'm working is using RTC for storage. The RTC device in question provides 8 bits, 3 bits are assigned for PSCR. Currently all reasons provided in this patch set would fit int to 3 bits, but soon or later it may expand. > 2. I would expect the infrastructure to read and clear the reason > during boot. If e.g. a watchdog triggers a reset you will otherwise > get an incorrect value. Hm.. good point. I'll set a value on the boot that there is currently no attempt to shutdown at all. PSCR works only for software assisted shutdown/reboot. It should extend, not replace PMIC or watchdog detected reasons. > 3. The reason is only stored, but not used? We have a sysfs ABI to > expose the reboot reason to userspace since half a year ago, see > d40befed9a58 (power: reset: at91-reset: add sysfs interface to > the power on reason). ACK. I'll add sysfs support. For my use case, the user is the bootloader. The is one of reasons why DT is used for mappings, this is the stable ABI between this systems. > 4. Available options should be synced with the list in > include/linux/power/power_on_reason.h ACK Regards, Oleksij