linux-kernel - Re: [PATCH v2 2/6] firmware: ti

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <iswnvyfwxq6xie2elmbzzixpriy74o4plk3adz6nthf2eekrgk@wwlgjhbygj5f>
Date: Tue, 6 Aug 2024 09:19:29 +0200
From: Markus Schneider-Pargmann <msp@...libre.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Nishanth Menon <nm@...com>, Tero Kristo <kristo@...nel.org>, 
	Santosh Shilimkar <ssantosh@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Vignesh Raghavendra <vigneshr@...com>, Vibhore Vardhan <vibhore@...com>, 
	Kevin Hilman <khilman@...libre.com>, Dhruva Gole <d-gole@...com>, 
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/6] firmware: ti_sci: Partial-IO support

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
>