| Message ID | 20241012-topic-am62-partialio-v6-12-b4-v3-2-f7c6c2739681@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: ti_sci: Partial-IO support | expand |
On 16:39-20241012, Markus Schneider-Pargmann wrote: [...] > > The possible wakeup devices are found by checking which devices are > powered by the regulator supplying the "vddshv_canuart" line. These are > considered possible wakeup sources. 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 > wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't > entered at that point. > [...] > +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info) > +{ > + static const char canuart_name[] = "vddshv_canuart"; > + struct device_node *wakeup_node = NULL; > + > + for (wakeup_node = of_find_node_with_property(NULL, "vio-supply"); > + wakeup_node; > + wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) { > + struct device_node *supply_node; > + const char *supply_name; > + struct platform_device *pdev; > + int ret; > + > + supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0); > + if (!supply_node) > + continue; > + > + ret = of_property_read_string(supply_node, "regulator-name", &supply_name); > + of_node_put(supply_node); > + if (ret) { > + dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n", > + wakeup_node, ret); > + continue; > + } > + > + if (strncmp(canuart_name, supply_name, strlen(canuart_name))) > + continue; > + > + pdev = of_find_device_by_node(wakeup_node); > + if (!pdev) > + continue; > + > + if (device_may_wakeup(&pdev->dev)) { > + dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n", > + wakeup_node); > + put_device(&pdev->dev); > + of_node_put(wakeup_node); > + return true; > + } > + put_device(&pdev->dev); > + } > + > + return false; > +} > + What is the binding that supports this? I just do not think that scanning the entire tree for vio-supply implies you will get thr right property here. Just giving an example to illustrate this point: Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt says it needs vio-supply -> so i have a node with the wireless supply as vio-supply -> Since we are scanning from NULL for vio-supply, we hit that, that is a bad choice for enabling io-retention.
Hi Nishanth, On Fri, Oct 25, 2024 at 12:42:04PM GMT, Nishanth Menon wrote: > On 16:39-20241012, Markus Schneider-Pargmann wrote: > [...] > > > > The possible wakeup devices are found by checking which devices are > > powered by the regulator supplying the "vddshv_canuart" line. These are > > considered possible wakeup sources. 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 > > wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't > > entered at that point. > > > [...] > > > +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info) > > +{ > > + static const char canuart_name[] = "vddshv_canuart"; > > + struct device_node *wakeup_node = NULL; > > + > > + for (wakeup_node = of_find_node_with_property(NULL, "vio-supply"); > > + wakeup_node; > > + wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) { > > + struct device_node *supply_node; > > + const char *supply_name; > > + struct platform_device *pdev; > > + int ret; > > + > > + supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0); > > + if (!supply_node) > > + continue; > > + > > + ret = of_property_read_string(supply_node, "regulator-name", &supply_name); > > + of_node_put(supply_node); > > + if (ret) { > > + dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n", > > + wakeup_node, ret); > > + continue; > > + } > > + > > + if (strncmp(canuart_name, supply_name, strlen(canuart_name))) > > + continue; > > + > > + pdev = of_find_device_by_node(wakeup_node); > > + if (!pdev) > > + continue; > > + > > + if (device_may_wakeup(&pdev->dev)) { > > + dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n", > > + wakeup_node); > > + put_device(&pdev->dev); > > + of_node_put(wakeup_node); > > + return true; > > + } > > + put_device(&pdev->dev); > > + } > > + > > + return false; > > +} > > + > > What is the binding that supports this? I just do not think that > scanning the entire tree for vio-supply implies you will get thr right > property here. > > Just giving an example to illustrate this point: > Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt says it > needs vio-supply -> so i have a node with the wireless supply as > vio-supply -> Since we are scanning from NULL for vio-supply, we hit > that, that is a bad choice for enabling io-retention. There is no bining that specifically supports this as I think it is not needed. The devices that are capable to wakeup the system from Partial-IO are all powered through one supply line that is always-on. It is called 'vddshv_canuart' and the name of this supply is checked in the above code as well. Yes I am using 'vio-supply', but only to search for the potential consumers of this supply. So wl1251 will be skipped in above code at if (strncmp(canuart_name, supply_name, strlen(canuart_name))) Best Markus
On 15:49-20241028, Markus Schneider-Pargmann wrote: > Hi Nishanth, > > On Fri, Oct 25, 2024 at 12:42:04PM GMT, Nishanth Menon wrote: > > On 16:39-20241012, Markus Schneider-Pargmann wrote: > > [...] > > > > > > The possible wakeup devices are found by checking which devices are > > > powered by the regulator supplying the "vddshv_canuart" line. These are > > > considered possible wakeup sources. 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 > > > wait for 5s and do an emergency_restart afterwards if Partial-IO wasn't > > > entered at that point. > > > > > [...] > > > > > +static bool tisci_canuart_wakeup_enabled(struct ti_sci_info *info) > > > +{ > > > + static const char canuart_name[] = "vddshv_canuart"; > > > + struct device_node *wakeup_node = NULL; > > > + > > > + for (wakeup_node = of_find_node_with_property(NULL, "vio-supply"); > > > + wakeup_node; > > > + wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) { > > > + struct device_node *supply_node; > > > + const char *supply_name; > > > + struct platform_device *pdev; > > > + int ret; > > > + > > > + supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0); > > > + if (!supply_node) > > > + continue; > > > + > > > + ret = of_property_read_string(supply_node, "regulator-name", &supply_name); > > > + of_node_put(supply_node); > > > + if (ret) { > > > + dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n", > > > + wakeup_node, ret); > > > + continue; > > > + } > > > + > > > + if (strncmp(canuart_name, supply_name, strlen(canuart_name))) > > > + continue; > > > + > > > + pdev = of_find_device_by_node(wakeup_node); > > > + if (!pdev) > > > + continue; > > > + > > > + if (device_may_wakeup(&pdev->dev)) { > > > + dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n", > > > + wakeup_node); > > > + put_device(&pdev->dev); > > > + of_node_put(wakeup_node); > > > + return true; > > > + } > > > + put_device(&pdev->dev); > > > + } > > > + > > > + return false; > > > +} > > > + > > > > What is the binding that supports this? I just do not think that > > scanning the entire tree for vio-supply implies you will get thr right > > property here. > > > > Just giving an example to illustrate this point: > > Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt says it > > needs vio-supply -> so i have a node with the wireless supply as > > vio-supply -> Since we are scanning from NULL for vio-supply, we hit > > that, that is a bad choice for enabling io-retention. > > There is no bining that specifically supports this as I think it is not > needed. The devices that are capable to wakeup the system from > Partial-IO are all powered through one supply line that is always-on. It > is called 'vddshv_canuart' and the name of this supply is checked > in the above code as well. Yes I am using 'vio-supply', but only to In effect, you are looking for nodes that have vio-supply pointing to a regulator called vddshv_canuart. Not only is it too specific to a device, but a board as well. Without documentation in binding, users who don't have sufficient information is bound to mess this up. Further, vddshv_canuart may not even be a regulator - there will need to be checks for that on top of just a strncmp. > search for the potential consumers of this supply. > So wl1251 will be skipped in above code at > > if (strncmp(canuart_name, supply_name, strlen(canuart_name))) Aah, thanks, but sorry, but I would prefer the drivers handle the specifics. If a new peripheral comes on to the list for a different device, or a different regulator name appears for can, we would be dealing with name mapping etc.
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c index 9ef86ea27a3c9ac6b9aa4a838a4f5e9fc09a81a9..fe964e5e2b2a06ba2fb9754537d28661951a6b78 100644 --- a/drivers/firmware/ti_sci.c +++ b/drivers/firmware/ti_sci.c @@ -3746,6 +3746,115 @@ 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) +{ + 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) +{ + static const char canuart_name[] = "vddshv_canuart"; + struct device_node *wakeup_node = NULL; + + for (wakeup_node = of_find_node_with_property(NULL, "vio-supply"); + wakeup_node; + wakeup_node = of_find_node_with_property(wakeup_node, "vio-supply")) { + struct device_node *supply_node; + const char *supply_name; + struct platform_device *pdev; + int ret; + + supply_node = of_parse_phandle(wakeup_node, "vio-supply", 0); + if (!supply_node) + continue; + + ret = of_property_read_string(supply_node, "regulator-name", &supply_name); + of_node_put(supply_node); + if (ret) { + dev_warn(info->dev, "Failed to parse vio-supply phandle at %pOF %d\n", + wakeup_node, ret); + continue; + } + + if (strncmp(canuart_name, supply_name, strlen(canuart_name))) + continue; + + pdev = of_find_device_by_node(wakeup_node); + if (!pdev) + continue; + + if (device_may_wakeup(&pdev->dev)) { + dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n", + wakeup_node); + put_device(&pdev->dev); + of_node_put(wakeup_node); + return true; + } + put_device(&pdev->dev); + } + + return false; +} + +static int tisci_sys_off_handler(struct sys_off_data *data) +{ + struct ti_sci_info *info = data->cb_data; + bool enter_partial_io = tisci_canuart_wakeup_enabled(info); + int ret; + + 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(); + } + + mdelay(5000); + emergency_restart(); + + return NOTIFY_DONE; +} + /* Description for K2G */ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = { .default_host_id = 2, @@ -3889,6 +3998,19 @@ static int ti_sci_probe(struct platform_device *pdev) goto out; } + if (info->fw_caps & MSG_FLAG_CAPS_LPM_PARTIAL_IO) { + ret = devm_register_sys_off_handler(dev, + SYS_OFF_MODE_POWER_OFF, + SYS_OFF_PRIO_FIRMWARE, + tisci_sys_off_handler, + info); + if (ret) { + dev_err(dev, "Failed to register sys_off_handler %pe\n", + ERR_PTR(ret)); + goto out; + } + } + dev_info(dev, "ABI: %d.%d (firmware rev 0x%04x '%s')\n", info->handle.version.abi_major, info->handle.version.abi_minor, info->handle.version.firmware_revision, @@ -3898,7 +4020,13 @@ static int ti_sci_probe(struct platform_device *pdev) list_add_tail(&info->node, &ti_sci_list); mutex_unlock(&ti_sci_list_mutex); - return of_platform_populate(dev->of_node, NULL, NULL, dev); + ret = of_platform_populate(dev->of_node, NULL, NULL, dev); + if (ret) { + dev_err(dev, "platform_populate failed %pe\n", ERR_PTR(ret)); + goto out; + } + return 0; + out: if (!IS_ERR(info->chan_tx)) mbox_free_channel(info->chan_tx); diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h index 053387d7baa064498e6a208daa7f70040ef87281..dec9e20cbe5da8f6d9393d56bb9a1e73cb083a42 100644 --- a/drivers/firmware/ti_sci.h +++ b/drivers/firmware/ti_sci.h @@ -592,6 +592,11 @@ struct ti_sci_msg_resp_get_clock_freq { struct ti_sci_msg_req_prepare_sleep { struct ti_sci_msg_hdr hdr; +/* + * When sending perpare_sleep with MODE_PARTIAL_IO no response will be sent, + * no further steps are required. + */ +#define TISCI_MSG_VALUE_SLEEP_MODE_PARTIAL_IO 0x03 #define TISCI_MSG_VALUE_SLEEP_MODE_DM_MANAGED 0xfd u8 mode; u32 ctx_lo;
Add support for Partial-IO poweroff. In Partial-IO pins of a few 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 are powered by the regulator supplying the "vddshv_canuart" line. These are considered possible wakeup sources. 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 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@baylibre.com> --- drivers/firmware/ti_sci.c | 130 +++++++++++++++++++++++++++++++++++++++++++++- drivers/firmware/ti_sci.h | 5 ++ 2 files changed, 134 insertions(+), 1 deletion(-)