On 06/08/2024 09:19, Markus Schneider-Pargmann wrote:

On Tue, Aug 06, 2024 at 08:26:00AM GMT, Krzysztof Kozlowski wrote:

quoted

On 29/07/2024 10:00, Markus Schneider-Pargmann wrote:

quoted

+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)) {

...

quoted

+			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);
+
+	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")) {
+		info->nr_wakeup_sources =
+			of_count_phandle_with_args(dev->of_node,
+						   "ti,partial-io-wakeup-sources",
+						   NULL);

I don't see the point of this. You have quite a static list of devices -
just look at your DTS. Then you don't even do anything useful with the
phandles you got here.

I am gathering the list of phandles in probe.

I know what the code is doing.

Then during shutdown I am resolving the phandles to devices if there
are associated devices and check if wakeup is enabled for these devices.

I can read the code.

If wakeup is enabled for one of the devices in the list, I put the
SoC into Partial-IO instead of a normal poweroff. This way the
devices which have wakeup enabled can actually wakeup the SoC as
requested by the user by setting wakeup enable.

I see all this. I said there is no point in doing this. Instead of
repeating the code, you can say what is the point of doing it.

I said once, so repeat one more time - you have *static* list of
devices, therefore any DTS is meaningless. It is just fixed, not flexible.

The code here is still not doing anything useful with the phandles. By
useful I mean - actually enable the wakeup. No, the property is totally
misplaced just to satisfy your code. That's a "no".

Best regards,
Krzysztof