Re: [PATCH v1] hw/intc/riscv_aplic: Remove redundant masking of hart_idx in riscv_aplic_msi_send()

[Andrew Jones <ajones@ventanamicro.com>]

Drop "in riscv_aplic_msi_send()" from the patch summary to make it more
concise.

On Tue, Jan 14, 2025 at 10:53:19AM +0800, Huang Borong wrote:
> The line "hart_idx &= APLIC_xMSICFGADDR_PPN_LHX_MASK(lhxw);" was removed

This just states what we can easily read from the patch.

> because the same operation is performed later in the address calculation.

This is useful information that should stay in the commit message.

> This change improves code clarity and avoids unnecessary operations.

You don't need to justify removing redundant lines of code, you just need
to justify that they're actually redundant.

Daniel's point about the log message is important and should be pointed
out in the commit message.

Thanks,
drew

> 
> Signed-off-by: Huang Borong <huangborong@bosc.ac.cn>
> ---
>  hw/intc/riscv_aplic.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index 4866649115..0974c6a5db 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -421,7 +421,6 @@ static void riscv_aplic_msi_send(RISCVAPLICState *aplic,
>              APLIC_xMSICFGADDRH_HHXW_MASK;
>  
>      group_idx = hart_idx >> lhxw;
> -    hart_idx &= APLIC_xMSICFGADDR_PPN_LHX_MASK(lhxw);
>  
>      addr = msicfgaddr;
>      addr |= ((uint64_t)(msicfgaddrH & APLIC_xMSICFGADDRH_BAPPN_MASK)) << 32;
> -- 
> 2.34.1
> 
>