On Tue, Aug 06, 2024 at 08:26:00AM GMT, Krzysztof Kozlowski wrote:
> On 29/07/2024 10:00, Markus Schneider-Pargmann wrote:
> > +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)) {
>
> ...
>
> > + 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.
Then during shutdown I am resolving the phandles to devices if there
are associated devices and check if wakeup is enabled for these devices.
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.
Best
Markus
>
> This property looks entirely useless. I already commented on the binding
> (which you did not respond to), so let's comment also here:
>
> No, it duplicates wakeup-source and your code shows that it is not even
> needed.
>
> Best regards,
> Krzysztof
>
