1. Patchew
  2. linux
  3. [PATCH 00/14] mailbox: pcc: Fixes and cleanup/refactoring
  4. View patch

[PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags

Sudeep Holla posted 14 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags
Posted by Sudeep Holla 11 months, 1 week ago 
The Sparse static checker flags a type mismatch warning related to
endianness conversion:

  |  warning: incorrect type in argument 1 (different base types)
  |     expected restricted __le32 const [usertype] *p
  |     got unsigned int *

This is because an explicit endianness conversion (le32_to_cpu()) was
applied unnecessarily to a pcc_hdr.flags field that is already in
little-endian format.

The PCC driver is only enabled on little-endian kernels due to its
dependency on ACPI and EFI, making the explicit conversion unnecessary.

The redundant conversion occurs in pcc_chan_check_and_ack() for the
pcc_hdr.flags field. Drop this unnecessary endianness conversion of
pcc_hdr.flags.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 4c582fa2b8bf4c9a9368dba8220f567555dba963..c87a5b7fa6eaf7bcabe0d55f844961c499376938 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -292,7 +292,7 @@ static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
 	 *
 	 * The PCC master subspace channel clears chan_in_use to free channel.
 	 */
-	if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
+	if (pcc_hdr.flags & PCC_ACK_FLAG_MASK)
 		pcc_send_data(chan, NULL);
 	else
 		pcc_chan_reg_read_modify_write(&pchan->cmd_update);

-- 
2.34.1
Re: [PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags
Posted by lihuisong (C) 11 months, 1 week ago 
在 2025/3/3 18:51, Sudeep Holla 写道:
> The Sparse static checker flags a type mismatch warning related to
> endianness conversion:
>
>    |  warning: incorrect type in argument 1 (different base types)
>    |     expected restricted __le32 const [usertype] *p
>    |     got unsigned int *
>
> This is because an explicit endianness conversion (le32_to_cpu()) was
> applied unnecessarily to a pcc_hdr.flags field that is already in
> little-endian format.
>
> The PCC driver is only enabled on little-endian kernels due to its
> dependency on ACPI and EFI, making the explicit conversion unnecessary.
How to confirm ACPI works only on little-endian?
>
> The redundant conversion occurs in pcc_chan_check_and_ack() for the
> pcc_hdr.flags field. Drop this unnecessary endianness conversion of
> pcc_hdr.flags.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/mailbox/pcc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 4c582fa2b8bf4c9a9368dba8220f567555dba963..c87a5b7fa6eaf7bcabe0d55f844961c499376938 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -292,7 +292,7 @@ static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
>   	 *
>   	 * The PCC master subspace channel clears chan_in_use to free channel.
>   	 */
> -	if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
> +	if (pcc_hdr.flags & PCC_ACK_FLAG_MASK)
It's recommanded to delete PCC_ACK_FLAG_MASK and use 
PCC_CMD_COMPLETION_NOTIFY.
They are from the same place, namely, 'Initiator Responder 
Communications Channel Flags'.
>   		pcc_send_data(chan, NULL);
>   	else
>   		pcc_chan_reg_read_modify_write(&pchan->cmd_update);
>
Re: [PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags
Posted by Sudeep Holla 11 months, 1 week ago 
On Wed, Mar 05, 2025 at 12:02:13PM +0800, lihuisong (C) wrote:
> 
> 在 2025/3/3 18:51, Sudeep Holla 写道:
> > The Sparse static checker flags a type mismatch warning related to
> > endianness conversion:
> > 
> >    |  warning: incorrect type in argument 1 (different base types)
> >    |     expected restricted __le32 const [usertype] *p
> >    |     got unsigned int *
> > 
> > This is because an explicit endianness conversion (le32_to_cpu()) was
> > applied unnecessarily to a pcc_hdr.flags field that is already in
> > little-endian format.
> > 
> > The PCC driver is only enabled on little-endian kernels due to its
> > dependency on ACPI and EFI, making the explicit conversion unnecessary.
> How to confirm ACPI works only on little-endian?

Sorry I didn't notice this question. ACPI depends on ARCH_SUPPORTS_ACPI
and it is selected only from EFI which is disabled if CPU_BIG_ENDIAN=y

--
Regards,
Sudeep
Re: [PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags
Posted by lihuisong (C) 11 months, 1 week ago 
在 2025/3/5 18:36, Sudeep Holla 写道:
> On Wed, Mar 05, 2025 at 12:02:13PM +0800, lihuisong (C) wrote:
>> 在 2025/3/3 18:51, Sudeep Holla 写道:
>>> The Sparse static checker flags a type mismatch warning related to
>>> endianness conversion:
>>>
>>>     |  warning: incorrect type in argument 1 (different base types)
>>>     |     expected restricted __le32 const [usertype] *p
>>>     |     got unsigned int *
>>>
>>> This is because an explicit endianness conversion (le32_to_cpu()) was
>>> applied unnecessarily to a pcc_hdr.flags field that is already in
>>> little-endian format.
>>>
>>> The PCC driver is only enabled on little-endian kernels due to its
>>> dependency on ACPI and EFI, making the explicit conversion unnecessary.
>> How to confirm ACPI works only on little-endian?
> Sorry I didn't notice this question. ACPI depends on ARCH_SUPPORTS_ACPI
> and it is selected only from EFI which is disabled if CPU_BIG_ENDIAN=y
>
got it. I found it.
> .
Re: [PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags
Posted by Sudeep Holla 11 months, 1 week ago 
On Wed, Mar 05, 2025 at 12:02:13PM +0800, lihuisong (C) wrote:
> 
> 在 2025/3/3 18:51, Sudeep Holla 写道:
> > The Sparse static checker flags a type mismatch warning related to
> > endianness conversion:
> > 
> >    |  warning: incorrect type in argument 1 (different base types)
> >    |     expected restricted __le32 const [usertype] *p
> >    |     got unsigned int *
> > 
> > This is because an explicit endianness conversion (le32_to_cpu()) was
> > applied unnecessarily to a pcc_hdr.flags field that is already in
> > little-endian format.
> > 
> > The PCC driver is only enabled on little-endian kernels due to its
> > dependency on ACPI and EFI, making the explicit conversion unnecessary.
> How to confirm ACPI works only on little-endian?
> > 
> > The redundant conversion occurs in pcc_chan_check_and_ack() for the
> > pcc_hdr.flags field. Drop this unnecessary endianness conversion of
> > pcc_hdr.flags.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >   drivers/mailbox/pcc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> > index 4c582fa2b8bf4c9a9368dba8220f567555dba963..c87a5b7fa6eaf7bcabe0d55f844961c499376938 100644
> > --- a/drivers/mailbox/pcc.c
> > +++ b/drivers/mailbox/pcc.c
> > @@ -292,7 +292,7 @@ static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
> >   	 *
> >   	 * The PCC master subspace channel clears chan_in_use to free channel.
> >   	 */
> > -	if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
> > +	if (pcc_hdr.flags & PCC_ACK_FLAG_MASK)
> It's recommanded to delete PCC_ACK_FLAG_MASK and use
> PCC_CMD_COMPLETION_NOTIFY.
> They are from the same place, namely, 'Initiator Responder Communications
> Channel Flags'.

Good point, I will use PCC_CMD_COMPLETION_NOTIFY and drop even the definition
of PCC_ACK_FLAG_MASK as it got added for no good reason.

--
Regards,
Sudeep

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.