Re: [PATCH v3] mailbox: qcom-cpucp: read mbox hardware register using readq()

[Sibi Sankar]

On 9/28/26 10:59, Sivansh Gupta wrote:
> Mailbox hardware registers are 64 bit wide, but the interrupt handler
> currently reads only 32 bits with readl() at offset 0x4 (upper 32
> bits). This returns an incorrect value, while reading using readq()
> returns the expected value.
>
> Use readq() to access the 64 bit register and pass the expected value
> to the mailbox client.

Mailbox hardware registers are 64 bit wide, but the interrupt handler
currently reads only 32 bits with readl() at offset 0x4 (upper 32
bits). This results in a masked out value being passed on to the mailbox
clients.

Fix this by using readq instead and pass on the correct value to the
mailbox clients.

^^ seems more appropriate with these minor nits fixed.

Reviewed-by: Sibi Sankar <sibi.sankar@xxxxxxxxxxxxxxxx>

> Fixes: 0e2a9a03106c ("mailbox: Add support for QTI CPUCP mailbox controller")
> Signed-off-by: Sivansh Gupta <sivansh.gupta@xxxxxxxxxxxxxxxx>
> ---
> v3:
> - Removed references to an out of tree driver to focus on the incorrect
>    value returned by readl()
>    https://lore.kernel.org/all/93ce8588-7500-45da-9b9a-424c202590cf@xxxxxxxxxxxxxxxx/#t
>
> v2:
> - Expanded the commit message to explain why this change is required.
>
>   drivers/mailbox/qcom-cpucp-mbox.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/qcom-cpucp-mbox.c b/drivers/mailbox/qcom-cpucp-mbox.c
> index 862e45e8fbd5..0b584d9aad6c 100644
> --- a/drivers/mailbox/qcom-cpucp-mbox.c
> +++ b/drivers/mailbox/qcom-cpucp-mbox.c
> @@ -61,7 +61,7 @@ static irqreturn_t qcom_cpucp_mbox_irq_fn(int irq, void *data)
>   	status = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_STAT);
>   
>   	for_each_set_bit(i, (unsigned long *)&status, cpucp->mbox.num_chans) {
> -		u32 val = readl(cpucp->rx_base + APSS_CPUCP_RX_MBOX_CMD(i) + APSS_CPUCP_MBOX_CMD_OFF);
> +		u64 val = readq(cpucp->rx_base + APSS_CPUCP_RX_MBOX_CMD(i));
>   		struct mbox_chan *chan = &cpucp->chans[i];
>   		unsigned long flags;
>