linux-kernel - Re: [PATCH v9 3/4] firmware: ti_sci: Introduce Power Management Ops

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] [day] [month] [year] [list]

Message-ID: <7hy150njzs.fsf@baylibre.com>
Date: Tue, 13 Aug 2024 15:19:51 -0700
From: Kevin Hilman <khilman@...libre.com>
To: a0230503 <a-kaur@...com>, Dhruva Gole <d-gole@...com>
Cc: Markus Schneider-Pargmann <msp@...libre.com>, Nishanth Menon
 <nm@...com>, Tero Kristo <kristo@...nel.org>, Santosh Shilimkar
 <ssantosh@...nel.org>, Vibhore Vardhan <vibhore@...com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, Dave
 Gerlach <d-gerlach@...com>, Georgi Vlaev <g-vlaev@...com>
Subject: Re: [PATCH v9 3/4] firmware: ti_sci: Introduce Power Management Ops

a0230503 <a-kaur@...com> writes:

> On 13/08/24 09:19, Dhruva Gole wrote:
>> On Aug 12, 2024 at 14:16:45 -0700, Kevin Hilman wrote:
>> [...]
>>>>> +/**
>>>>> + * struct ti_sci_msg_resp_lpm_wake_reason - Response for TI_SCI_MSG_LPM_WAKE_REASON.
>>>>> + *
>>>>> + * @hdr:		Generic header.
>>>>> + * @wake_source:	The wake up source that woke soc from LPM.
>>>>> + * @wake_timestamp:	Timestamp at which soc woke.
>>>>> + *
>>>>> + * Response to a generic message with message type TI_SCI_MSG_LPM_WAKE_REASON,
>>>>> + * used to query the wake up source from low power mode.
>>>>> + */
>>>>> +struct ti_sci_msg_resp_lpm_wake_reason {
>>>>> +	struct ti_sci_msg_hdr hdr;
>>>>> +	u32 wake_source;
>>>>> +	u64 wake_timestamp;
>>>>> +} __packed;
>>>>> +
>>>>
>>>> It looks like we forgot to update the parameters in this API.
>>>> See [1]:
>>>>
>>>> struct tisci_msg_lpm_wake_reason_req
>>>>
>>>> We're missing here the wake_pin, mode and 2 rsvd fields as well.
>>>>
>>>> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-lpm-wake-reason
>>>>
>>>
>>> The docs mention how to interpret the pin, but not the mode.  This
>>> driver should translate this mode number to some human-readable state
>>> for better debug messages.
>>>
>>> Kevin
>> 
>> Agreed, we would all benefit from a logic that prints the human-readable
>> state. The way to interpret the mode would be just as:
>> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#supported-low-power-modes
>> 
>> So for eg. 0x0 = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP
>> 
>
> Missing the call to ti_sci_msg_cmd_lpm_wake_reason in resume path.
>

Oops, OK.  Will be added for v10.

Thanks,

Kevin