Re: [PATCH v9 3/3] firmware: ti_sci: Partial-IO support

[Andrew Davis <afd@ti.com>]

On 11/3/25 4:31 AM, Markus Schneider-Pargmann wrote:
> 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@baylibre.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.
> 

Ah I see, but why not use errname() directly? I see dev_err_probe() also goes though
a ERR_PTR() cast, so maybe there is some reason I'm not seeing, so I'm fine either way.

Thanks,
Andrew

> Thanks!
> 
> Best
> Markus