Hi Markus, On Dec 19, 2024 at 21:02:13 +0100, Markus Schneider-Pargmann wrote: > Add support for Partial-IO poweroff. In Partial-IO pins of a few Maybe add a comma after the In partial-IO, a few pins in this SOC can generate.... > hardware units can generate system wakeups while DDR memory is not > powered resulting in a fresh boot of the system. These hardware units in > the SoC are always powered so that some logic can detect pin activity. > > If the system supports Partial-IO as described in the fw capabilities, a > sys_off handler is added. This sys_off handler decides if the poweroff > is executed by entering normal poweroff or Partial-IO instead. The > decision is made by checking if wakeup is enabled on all devices that > may wake up the SoC from Partial-IO. > > The possible wakeup devices are found by checking which devices have the > "poweroff" in the list of wakeup-source power states. Only devices that > are actually enabled by the user will be considered as an active wakeup > source. If none of the wakeup sources is enabled the system will do a > normal poweroff. If at least one wakeup source is enabled it will > instead send a TI_SCI_MSG_PREPARE_SLEEP message from the sys_off > handler. Sending this message will result in an immediate shutdown of > the system. No execution is expected after this point. The code will > wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't > entered at that point. > > A short documentation about Partial-IO can be found in section 6.2.4.5 > of the TRM at > https://www.ti.com/lit/pdf/spruiv7 > > Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx> > --- > drivers/firmware/ti_sci.c | 115 +++++++++++++++++++++++++++++++++++++++++++++- > drivers/firmware/ti_sci.h | 5 ++ > 2 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c > index ec0c54935ac0d667323d98b86ac9d288b73be6aa..693ac816f8ba3941a9156bd39524099ca476d712 100644 > --- a/drivers/firmware/ti_sci.c > +++ b/drivers/firmware/ti_sci.c > @@ -3746,6 +3746,100 @@ static const struct dev_pm_ops ti_sci_pm_ops = { > #endif > }; > > +/* > + * Enter Partial-IO, which disables everything including DDR with only a small > + * logic being active for wakeup. > + */ > +static int tisci_enter_partial_io(struct ti_sci_info *info) Isn't the function naming style in the driver ti_sci_XXX ? You're missing one `_` I guess > +{ > + struct ti_sci_msg_req_prepare_sleep *req; > + struct ti_sci_xfer *xfer; > + struct device *dev = info->dev; > + int ret = 0; > + > + xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_PREPARE_SLEEP, > + TI_SCI_FLAG_REQ_GENERIC_NORESPONSE, > + sizeof(*req), sizeof(struct ti_sci_msg_hdr)); > + if (IS_ERR(xfer)) { > + ret = PTR_ERR(xfer); > + dev_err(dev, "Message alloc failed(%d)\n", ret); > + return ret; > + } > + > + req = (struct ti_sci_msg_req_prepare_sleep *)xfer->xfer_buf; > + req->mode = TISCI_MSG_VALUE_SLEEP_MODE_PARTIAL_IO; > + req->ctx_lo = 0; > + req->ctx_hi = 0; > + req->debug_flags = 0; > + > + dev_info(dev, "Entering Partial-IO because a powered wakeup-enabled device was found.\n"); > + > + ret = ti_sci_do_xfer(info, xfer); > + if (ret) { > + dev_err(dev, "Mbox send fail %d\n", ret); > + goto fail; > + } > + > +fail: > + ti_sci_put_one_xfer(&info->minfo, xfer); > + > + return ret; > +} > + > +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info) Add some documentation around this please. > +{ > + struct device_node *wakeup_node = NULL; > + > + for (wakeup_node = of_find_node_with_property(NULL, "wakeup-source"); > + wakeup_node; > + wakeup_node = of_find_node_with_property(wakeup_node, "wakeup-source")) { > + struct platform_device *pdev; > + int index; > + > + index = of_property_match_string(wakeup_node, "wakeup-source", "poweroff"); > + if (index < 0) > + continue; Doesn't the fact that we're inside the for loop already ensure this is > 0? > + > + pdev = of_find_device_by_node(wakeup_node); > + if (!pdev) > + break; Same here? Would we otherwise be in the loop? Just having a quick look here, I could be wrong, please just check once. -- Best regards, Dhruva Gole Texas Instruments Incorporated