linux-kernel - Re: [PATCH 1/2] soc: qcom: rpmh: Add support to read back resource settings

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: <c4947591-4d09-4f49-8458-b993b5f95a2a@oss.qualcomm.com>
Date: Tue, 24 Jun 2025 16:44:12 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Kamal Wadhwa <kamal.wadhwa@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Maulik Shah <maulik.shah@....qualcomm.com>
Subject: Re: [PATCH 1/2] soc: qcom: rpmh: Add support to read back resource
 settings

On 6/23/25 6:43 PM, Kamal Wadhwa wrote:
> From: Maulik Shah <maulik.shah@....qualcomm.com>
> 
> All rpmh_*() APIs so far have supported placing votes for various
> resource settings but the H/W also have option to read resource
> settings.
> 
> This change adds a new rpmh_read() API to allow clients
> to read back resource setting from H/W. This will be useful for
> clients like regulators, which currently don't have a way to know
> the settings applied during bootloader stage.
> 
> Signed-off-by: Maulik Shah <maulik.shah@....qualcomm.com>
> ---

[...]

>  	u32 msgid;
> -	u32 cmd_msgid = CMD_MSGID_LEN | CMD_MSGID_WRITE;
> +	u32 cmd_msgid = CMD_MSGID_LEN;
>  	u32 cmd_enable = 0;
>  	struct tcs_cmd *cmd;
>  	int i, j;
>  
>  	/* Convert all commands to RR when the request has wait_for_compl set */
>  	cmd_msgid |= msg->wait_for_compl ? CMD_MSGID_RESP_REQ : 0;
> +	cmd_msgid |= (!msg->is_read) ? CMD_MSGID_WRITE : 0;

if (!msg->is_read)
	cmd_msgid |= CMD_MSGID_WRITE

looks more human-readable

[...]

> +/**
> + * rpmh_read: Read a resource value
> + *
> + * @dev: The device making the request
> + * @state: Must be Active state
> + * @cmd: The payload having address of resource to read
> + * @n: The number of elements in @cmd, must be single command
> + *
> + * Reads the value for the resource address given in tcs_cmd->addr
> + * and returns the tcs_cmd->data filled with same.
> + *
> + * May sleep. Do not call from atomic contexts.
> + *
> + * Return:
> + * * 0			- Success
> + * * -Error             - Error code

This isn't valid kerneldoc

> + */
> +int rpmh_read(const struct device *dev, enum rpmh_state state, struct tcs_cmd *cmd, u32 n)
> +{
> +	int ret;
> +	DECLARE_COMPLETION_ONSTACK(compl);
> +	DEFINE_RPMH_MSG_ONSTACK(dev, state, &compl, rpm_msg);

A reverse-Christmas-tree sorting would be nice here

> +
> +	if (n != 1 || state != RPMH_ACTIVE_ONLY_STATE)

if n must be one, why is it a parameter?

Konrad