[<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