[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DDYZOCXFX1J2.30UFCSI059R9V@baylibre.com>
Date: Mon, 03 Nov 2025 11:31:35 +0100
From: "Markus Schneider-Pargmann" <msp@...libre.com>
To: "Andrew Davis" <afd@...com>, "Markus Schneider-Pargmann (TI.com)"
<msp@...libre.com>, "Nishanth Menon" <nm@...com>, "Tero Kristo"
<kristo@...nel.org>, "Santosh Shilimkar" <ssantosh@...nel.org>
Cc: "Vishal Mahaveer" <vishalm@...com>, "Kevin Hilman"
<khilman@...libre.com>, "Dhruva Gole" <d-gole@...com>, "Sebin Francis"
<sebin.francis@...com>, "Kendall Willis" <k-willis@...com>, "Akashdeep
Kaur" <a-kaur@...com>, <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 3/3] firmware: ti_sci: Partial-IO support
On Thu Oct 30, 2025 at 3:24 PM CET, Andrew Davis wrote:
> On 10/30/25 4:26 AM, Markus Schneider-Pargmann (TI.com) wrote:
>> 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
>> reference a "Partial-IO" system state in the list of wakeup-source
>> system 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 (TI.com) <msp@...libre.com>
>> ---
>> drivers/firmware/ti_sci.c | 132 +++++++++++++++++++++++++++++++++++++++++++++-
>> drivers/firmware/ti_sci.h | 5 ++
>> 2 files changed, 136 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index 4db84a92a517b0aa7bb8d47e809d9848a16e2cc4..f2922fccfbe748a436cb9aa0a8c8e5f48db02ef9 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -6,6 +6,7 @@
>> * Nishanth Menon
>> */
>>
>> +#include "linux/dev_printk.h"
>> #define pr_fmt(fmt) "%s: " fmt, __func__
>>
>> #include <linux/bitmap.h>
>> @@ -3663,6 +3664,116 @@ devm_ti_sci_get_resource(const struct ti_sci_handle *handle, struct device *dev,
>> }
>> EXPORT_SYMBOL_GPL(devm_ti_sci_get_resource);
>>
>> +/*
>> + * Enter Partial-IO, which disables everything including DDR with only a small
>> + * logic being active for wakeup.
>> + */
>> +static int ti_sci_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;
>
> This whole function is almost identical to ti_sci_cmd_prepare_sleep() other
> than you use a different mode here, which this different mode can be passed
> into ti_sci_cmd_prepare_sleep() just the same. Only other difference would
> be the NORESPONSE flag which you could just check "mode" passed in and
> use the right flag for the mode.
I thought this to be nicer as it avoided ifs, but as Kendall and you
both requested this, I changed it for the next version.
>
>> + req->ctx_lo = 0;
>> + req->ctx_hi = 0;
>> + req->debug_flags = 0;
>> +
>> + 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;
>> +}
>> +
>> +/*
>> + * Iterate all device nodes that have a wakeup-source property and check if one
>> + * of the possible phandles points to a Partial-IO system state. If it
>> + * does resolve the device node to an actual device and check if wakeup is
>> + * enabled.
>> + */
>> +static bool ti_sci_partial_io_wakeup_enabled(struct ti_sci_info *info)
>> +{
>> + struct device_node *wakeup_node = NULL;
>> +
>> + for_each_node_with_property(wakeup_node, "wakeup-source") {
>> + struct of_phandle_iterator it;
>> + int err;
>> +
>> + of_for_each_phandle(&it, err, wakeup_node, "wakeup-source", NULL, 0) {
>> + struct platform_device *pdev;
>> + bool may_wakeup;
>> +
>> + /*
>> + * Continue if idle-state-name is not off-wake. Return
>> + * value is the index of the string which should be 0 if
>> + * off-wake is present.
>> + */
>> + if (of_property_match_string(it.node, "idle-state-name", "off-wake"))
>> + continue;
>> +
>> + pdev = of_find_device_by_node(wakeup_node);
>> + if (!pdev)
>> + continue;
>> +
>> + may_wakeup = device_may_wakeup(&pdev->dev);
>> + put_device(&pdev->dev);
>> +
>> + if (may_wakeup) {
>> + dev_dbg(info->dev, "%pOF identified as wakeup source for Partial-IO\n",
>> + wakeup_node);
>> + of_node_put(it.node);
>> + of_node_put(wakeup_node);
>> + return true;
>> + }
>> + }
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int ti_sci_sys_off_handler(struct sys_off_data *data)
>> +{
>> + struct ti_sci_info *info = data->cb_data;
>> + bool enter_partial_io = ti_sci_partial_io_wakeup_enabled(info);
>> + int ret;
>> +
>> + if (!enter_partial_io)
>> + return NOTIFY_DONE;
>> +
>> + dev_info(info->dev, "Entering Partial-IO because a powered wakeup-enabled device was found.\n");
>> +
>> + ret = ti_sci_enter_partial_io(info);
>> +
>
> No need for newline here.
>
>> + if (ret) {
>> + dev_err(info->dev,
>> + "Failed to enter Partial-IO %pe, trying to do an emergency restart\n",
>> + ERR_PTR(ret));
>
> Why cast this int to a pointer before printing it out?
I am casting this to an error pointer to get the resolution of the error
value to a symbolic name with %pe. This will print 'EBUSY' etc.
Thanks!
Best
Markus
Download attachment "signature.asc" of type "application/pgp-signature" (290 bytes)
Powered by blists - more mailing lists