hw/net/can: Remove redundant status bit setting in can_sja1000

[PATCH] hw/net/can: Remove redundant status bit setting in can_sja1000

Posted by SillyZ 1 week, 5 days ago

In PeliCAN mode reception, the RBS (Receive Buffer Status) bit
is set twice at line 842 and 845 with identical operations:
  s->status_pel |= 0x01;
  s->status_pel |= (1 << 0);

Between these two operations, only interrupt_pel is modified and
status_pel bit 4 is cleared, neither affecting bit 0. The second
operation is redundant.

This cleanup aligns PeliCAN mode with BasicCAN mode, which correctly
sets this bit only once (line 883).

Signed-off-by: SillyZ <1357816113@qq.com>
---
 hw/net/can/can_sja1000.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 5b6ba9df6c..6b08e977a1 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -842,7 +842,6 @@ ssize_t can_sja_receive(CanBusClientState *client, const qemu_can_frame *frames,
         s->status_pel |= 0x01; /* Set the Receive Buffer Status. DS-p23 */
         s->interrupt_pel |= 0x01;
         s->status_pel &= ~(1 << 4);
-        s->status_pel |= (1 << 0);
         can_sja_update_pel_irq(s);
     } else { /* BasicCAN mode */
 
-- 
2.47.1.windows.2

Re: [PATCH] hw/net/can: Remove redundant status bit setting in can_sja1000

Posted by Michael Tokarev 1 week, 4 days ago

On 16.09.2025 11:10, SillyZ wrote:
> In PeliCAN mode reception, the RBS (Receive Buffer Status) bit
> is set twice at line 842 and 845 with identical operations:
>    s->status_pel |= 0x01;
>    s->status_pel |= (1 << 0);
> 
> Between these two operations, only interrupt_pel is modified and
> status_pel bit 4 is cleared, neither affecting bit 0. The second
> operation is redundant.
> 
> This cleanup aligns PeliCAN mode with BasicCAN mode, which correctly
> sets this bit only once (line 883).
> 
> Signed-off-by: SillyZ <1357816113@qq.com>

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

And applied to the trivial-patches tree.

Thanks,

/mjt

Re: [PATCH] hw/net/can: Remove redundant status bit setting in can_sja1000

Posted by Peter Maydell 1 week, 5 days ago

On Tue, 16 Sept 2025 at 14:50, SillyZ <1357816113@qq.com> wrote:
>
> In PeliCAN mode reception, the RBS (Receive Buffer Status) bit
> is set twice at line 842 and 845 with identical operations:
>   s->status_pel |= 0x01;
>   s->status_pel |= (1 << 0);
>
> Between these two operations, only interrupt_pel is modified and
> status_pel bit 4 is cleared, neither affecting bit 0. The second
> operation is redundant.
>
> This cleanup aligns PeliCAN mode with BasicCAN mode, which correctly
> sets this bit only once (line 883).
>
> Signed-off-by: SillyZ <1357816113@qq.com>
> ---
>  hw/net/can/can_sja1000.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 5b6ba9df6c..6b08e977a1 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -842,7 +842,6 @@ ssize_t can_sja_receive(CanBusClientState *client, const qemu_can_frame *frames,
>          s->status_pel |= 0x01; /* Set the Receive Buffer Status. DS-p23 */
>          s->interrupt_pel |= 0x01;
>          s->status_pel &= ~(1 << 4);
> -        s->status_pel |= (1 << 0);
>          can_sja_update_pel_irq(s);
>      } else { /* BasicCAN mode */
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM