On Tue, Jul 30, 2024 at 07:28:01AM GMT, Nishanth Menon wrote: > On 10:00-20240729, Markus Schneider-Pargmann wrote: > > Add support for Partial-IO poweroff. In Partial-IO pins of a few modules > > can generate system wakeups while DDR memory is not powered resulting in > > a fresh boot of the system. The modules that can be wakeup sources are > > defined by the devicetree. > > > > Only wakeup sources that are actually enabled by the user will be > > considered as a an active wakeup source. If none of the wakeup sources > > are 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 enter an infinite loop. > > > > The wakeup source device nodes are gathered during probe. But they are > > only resolved to the actual devices in the sys_off handler, if they > > exist. If they do not exist, they are ignored. > > > > 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 | 160 +++++++++++++++++++++++++++++++++----- > > drivers/firmware/ti_sci.h | 34 ++++++++ > > 2 files changed, 175 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c > > index 160968301b1f..ba2e56da0215 100644 > > --- a/drivers/firmware/ti_sci.c > > +++ b/drivers/firmware/ti_sci.c > > @@ -99,6 +99,9 @@ struct ti_sci_desc { > > * @node: list head > > * @host_id: Host ID > > * @users: Number of users of this instance > > + * @nr_wakeup_sources: Number of device nodes in wakeup_source_nodes > > + * @wakeup_source_nodes: Array of all device_nodes listed as wakeup sources in > > + * the devicetree > > */ > > struct ti_sci_info { > > struct device *dev; > > @@ -116,6 +119,9 @@ struct ti_sci_info { > > u8 host_id; > > /* protected by ti_sci_list_mutex */ > > int users; > > + > > + int nr_wakeup_sources; > > + struct device_node **wakeup_source_nodes; > > }; > > > > #define cl_to_ti_sci_info(c) container_of(c, struct ti_sci_info, cl) > > @@ -392,10 +398,13 @@ static void ti_sci_put_one_xfer(struct ti_sci_xfers_info *minfo, > > static inline int ti_sci_do_xfer(struct ti_sci_info *info, > > struct ti_sci_xfer *xfer) > > { > > + struct ti_sci_msg_hdr *hdr = (struct ti_sci_msg_hdr *)xfer->tx_message.buf; > > int ret; > > int timeout; > > struct device *dev = info->dev; > > bool done_state = true; > > + bool response_expected = !!(hdr->flags & (TI_SCI_FLAG_REQ_ACK_ON_PROCESSED | > > + TI_SCI_FLAG_REQ_ACK_ON_RECEIVED)); > > I think a separate patch to introduce a no_response expected patch would > make sense on which we build tisci_sys_off_handler in the next patch? > > > > > ret = mbox_send_message(info->chan_tx, &xfer->tx_message); > > if (ret < 0) > > @@ -403,25 +412,27 @@ static inline int ti_sci_do_xfer(struct ti_sci_info *info, > > > > ret = 0; > > > > - if (system_state <= SYSTEM_RUNNING) { > > - /* And we wait for the response. */ > > - timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); > > - if (!wait_for_completion_timeout(&xfer->done, timeout)) > > - ret = -ETIMEDOUT; > > - } else { > > - /* > > - * If we are !running, we cannot use wait_for_completion_timeout > > - * during noirq phase, so we must manually poll the completion. > > - */ > > - ret = read_poll_timeout_atomic(try_wait_for_completion, done_state, > > - done_state, 1, > > - info->desc->max_rx_timeout_ms * 1000, > > - false, &xfer->done); > > - } > > + if (response_expected) { > > How about a goto? Yes, thanks, looks cleaner. > > if (!response_expected) > goto no_response; > > + if (system_state <= SYSTEM_RUNNING) { > > + /* And we wait for the response. */ > > + timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); > > + if (!wait_for_completion_timeout(&xfer->done, timeout)) > > + ret = -ETIMEDOUT; > > + } else { > > + /* > > + * If we are !running, we cannot use wait_for_completion_timeout > > + * during noirq phase, so we must manually poll the completion. > > + */ > > + ret = read_poll_timeout_atomic(try_wait_for_completion, done_state, > > + done_state, 1, > > + info->desc->max_rx_timeout_ms * 1000, > > + false, &xfer->done); > > + } > > > > - if (ret == -ETIMEDOUT) > > - dev_err(dev, "Mbox timedout in resp(caller: %pS)\n", > > - (void *)_RET_IP_); > > + if (ret == -ETIMEDOUT) > > + dev_err(dev, "Mbox timedout in resp(caller: %pS)\n", > > + (void *)_RET_IP_); > > + } > > > no_response: > > > /* > > * NOTE: we might prefer not to need the mailbox ticker to manage the > > @@ -3262,6 +3273,82 @@ static int tisci_reboot_handler(struct sys_off_data *data) > > return NOTIFY_BAD; > > } > > > [...] > > > +static int tisci_sys_off_handler(struct sys_off_data *data) > > +{ > > + struct ti_sci_info *info = data->cb_data; > > + int i; > > + int ret; > > + bool enter_partial_io = false; > > + > > + for (i = 0; i != info->nr_wakeup_sources; ++i) { > > + struct platform_device *pdev = > > + of_find_device_by_node(info->wakeup_source_nodes[i]); > > + > > + if (!pdev) > > + continue; > > + > > + if (device_may_wakeup(&pdev->dev)) { > > + dev_dbg(info->dev, "%pOFp identified as wakeup source\n", > > + info->wakeup_source_nodes[i]); > > + enter_partial_io = true; > > + } > > + } > > + > > + if (!enter_partial_io) > > + return NOTIFY_DONE; > > + > > + ret = tisci_enter_partial_io(info); > > + > > + if (ret) { > > + dev_err(info->dev, > > + "Failed to enter Partial-IO %pe, trying to do an emergency restart\n", > > + ERR_PTR(ret)); > > + emergency_restart(); > > + } > > + > > + while (1); > > Why not fall through OR go through emergency_restart (since there is > no fall through for shutdown path) if it acks, but actually fails to > enter LPM state after a dt described or a default timeout period? > > > + > > + return NOTIFY_DONE; > > +} > > + > > /* Description for K2G */ > > static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = { > > .default_host_id = 2, > > @@ -3398,6 +3485,35 @@ static int ti_sci_probe(struct platform_device *pdev) > > goto out; > > } > > > > + if (of_property_read_bool(dev->of_node, "ti,partial-io-wakeup-sources")) { > > You should probably check on TISCI_MSG_QUERY_FW_CAPS[1] if > Partial IO on low power mode is supported as well? if there is a > mismatch, report so? I actually have another series in my queue that introduces this check. I just implemented this check for Partial-IO yesterday in the patch that introduces fw capabilities. If you like I can switch these series around. > > > + info->nr_wakeup_sources = > > + of_count_phandle_with_args(dev->of_node, > > + "ti,partial-io-wakeup-sources", > > + NULL); > > + info->wakeup_source_nodes = > > + devm_kzalloc(dev, sizeof(*info->wakeup_source_nodes), > > + GFP_KERNEL); > > + > > + for (i = 0; i != info->nr_wakeup_sources; ++i) { > > + struct device_node *devnode = > > + of_parse_phandle(dev->of_node, > > + "ti,partial-io-wakeup-sources", > > + i); > > + info->wakeup_source_nodes[i] = devnode; > > Curious: Don't we need to maintain reference counting for the devnode > if CONFIG_OF_DYNAMIC? In case you mean I missed of_node_put(), yes, I did, thank you. I added it in a ti_sci_remove(). Best Markus
