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

Message-ID: <7hcymdphky.fsf@baylibre.com>
Date: Mon, 12 Aug 2024 14:16:45 -0700
From: Kevin Hilman <khilman@...libre.com>
To: Dhruva Gole <d-gole@...com>, Markus Schneider-Pargmann <msp@...libre.com>
Cc: 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>, Akashdeep Kaur
 <a-kaur@...com>
Subject: Re: [PATCH v9 3/4] firmware: ti_sci: Introduce Power Management Ops

Dhruva Gole <d-gole@...com> writes:

> On Aug 09, 2024 at 15:53:46 +0200, Markus Schneider-Pargmann wrote:
>> From: Dave Gerlach <d-gerlach@...com>
>> 
>> Introduce power management ops supported by the TISCI
>> Low Power Mode API [1].
>> 
>> 1) TISCI_MSG_LPM_WAKE_REASON
>> Get which wake up source woke the SoC from Low Power Mode.
>> The wake up source IDs will be common for all K3 platforms.
>> 
>> 2) TISCI_MSG_LPM_SET_DEVICE_CONSTRAINT
>> Set LPM constraint on behalf of a device. By setting a constraint, the
>> device ensures that it will not be powered off or reset in the selected
>> mode.
>> 
>> 3) TISCI_MSG_LPM_SET_LATENCY_CONSTRAINT
>> Set LPM resume latency constraint. By setting a constraint, the host
>> ensures that the resume time from selected mode will be less than the
>> constraint value.
>> 
>> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html
>> 
>> Signed-off-by: Dave Gerlach <d-gerlach@...com>
>> [g-vlaev@...com: LPM_WAKE_REASON and IO_ISOLATION support]
>> Signed-off-by: Georgi Vlaev <g-vlaev@...com>
>> [a-kaur@...com: SET_DEVICE_CONSTRAINT support]
>> Signed-off-by: Akashdeep Kaur <a-kaur@...com>
>> [vibhore@...com: SET_LATENCY_CONSTRAINT support]
>> Signed-off-by: Vibhore Vardhan <vibhore@...com>
>> Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>
>> ---
> [...]
>> diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
>> index 8efe4d0e61fb..9dd40b6711d4 100644
>> --- a/drivers/firmware/ti_sci.h
>> +++ b/drivers/firmware/ti_sci.h
>> @@ -38,7 +38,10 @@
>>  
>>  /* Low Power Mode Requests */
>>  #define TI_SCI_MSG_PREPARE_SLEEP	0x0300
>> +#define TI_SCI_MSG_LPM_WAKE_REASON	0x0306
>>  #define TI_SCI_MSG_SET_IO_ISOLATION	0x0307
>> +#define TI_SCI_MSG_LPM_SET_DEVICE_CONSTRAINT	0x0309
>> +#define TI_SCI_MSG_LPM_SET_LATENCY_CONSTRAINT	0x030A
>>  
>>  /* Resource Management Requests */
>>  #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
>> @@ -610,6 +613,73 @@ struct ti_sci_msg_req_set_io_isolation {
>>  	u8 state;
>>  } __packed;
>>  
>> +/**
>> + * 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