| Message ID | 20250303-pcc_fixes_updates-v1-3-3b44f3d134b1@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | mailbox: pcc: Fixes and cleanup/refactoring | expand |
在 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); >
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
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
在 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. > .
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);
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(-)