Thread (27 messages) 27 messages, 6 authors, 2025-08-25

Re: [PATCH v3 4/4] watchdog: qcom: add support to read the restart reason from IMEM

From: Kathiravan Thirumoorthy <hidden>
Date: 2025-05-06 11:01:42
Also in: linux-arm-msm, linux-watchdog, lkml

On 5/3/2025 3:53 AM, Konrad Dybcio wrote:
On 5/2/25 6:28 PM, Kathiravan Thirumoorthy wrote:
quoted
On 5/2/2025 7:33 PM, Konrad Dybcio wrote:
quoted
quoted
+static int  qcom_wdt_get_restart_reason(struct qcom_wdt *wdt,
+                    const struct qcom_wdt_match_data *data)
+{
+    struct regmap *imem;
+    unsigned int val;
+    int ret;
+
+    imem = syscon_regmap_lookup_by_compatible(data->imem_compatible);
Try syscon_regmap_lookup_by_phandle_args() and pass a phandle, see e.g.
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c & phy@1bfc000 in x1e80100.dtsi

That way all platform specifics will live in the DT, requiring no
hardcode-y driver changes on similar platforms
Thanks. I thought about this API but it didn't strike that I can use the args to fetch and match the value.

I need a suggestion here. There is a plan to extend this feature to other IPQ targets and also support WDIOF_POWERUNDER and WDIOF_OVERHEAT cause as well. For IPQ5424, all 3 cause will support and for other IPQ platforms, we are exploring how to integrate WDIOF_OVERHEAT. In any case, can I define the DT entry like below

         imem,phandle = <&imem 0x7b0 <Non secure WDT value> <Power Under value> <Overheat value>>;

and store these in values args[1], args[2] and args[3] respectively and use it for manipulation? If any of the platform doesn't support all 3, I can update the bindings and define the number of args as required.
Let's call the property qcom,restart-reason and only pass the register value

Because we may have any number of crazy combinations of various restart
reasons, we can go two paths:

1. promise really really really hard we won't be too crazy with the number
    of possible values and put them in the driver
2. go all out on DT properties (such as `bootstatus-overheat`,
`bootstatus-fanfault` etc.

Thanks Konrad for the suggestions and the offline discussions.

@Guenter, I need a suggestion here. Currently as part of this series, we 
are planning to expose WDIOF_CARDRESET, WDIOF_POWERUNDER, WDIOF_OVERHEAT 
reasons.

Once this is done, we do have the custom reason codes like Kernel Panic, 
Secure Watchdog Bite, Bus error timeout, Bus error access and few many. 
Is it okay to expose these values also via the bootstatus sysFS by 
extending the current list of reasons? Since these are outside the scope 
of watchdog, need your thoughts on this.

I'd much prefer to go with 1 really.. If we used nvmem, we could have a map
of cell names to restart reasons, but we've already established IMEM is
volatile and we shouldn't mess up the convention just because that
subsystem has nicer APIs..

Unless we rename the subsystem to `fuses`, `magic-values` or something..
+Srini? :P

Konrad
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help