| Message ID | 20230803132818.2092166-1-luxu.kernel@bytedance.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [RESEND] riscv: Fix local irq restore when flags indicates irq disabled | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be fixes at HEAD 9d3e8e1ff0d8 |
| conchuod/fixes_present | success | Fixes tag present in non-next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 2810 this patch: 2810 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 15873 this patch: 15873 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 12 this patch: 12 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 11 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | Fixes tag looks correct |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Hey, On Thu, Aug 03, 2023 at 09:28:18PM +0800, Xu Lu wrote: > When arch_local_irq_restore() is called with flags indicating irqs > disabled, we need to clear SR_IE bit in CSR_STATUS, whereas current > implementation based on csr_set() function only sets SR_IE bit of > CSR_STATUS when SR_IE bit of flags is high and does nothing when > SR_IE bit of flags is low. > > This commit supplies csr clear operation when calling irq restore > function with flags indicating irq disabled. > > Fixes: 6d60b6ee0c97 ("RISC-V: Device, timer, IRQs, and the SBI") > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> > --- There's no changelog or explanation, so what is the reason for the resend? Cheers, Conor. > arch/riscv/include/asm/irqflags.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/irqflags.h b/arch/riscv/include/asm/irqflags.h > index 08d4d6a5b7e9..7c31fc3c3559 100644 > --- a/arch/riscv/include/asm/irqflags.h > +++ b/arch/riscv/include/asm/irqflags.h > @@ -49,7 +49,10 @@ static inline int arch_irqs_disabled(void) > /* set interrupt enabled status */ > static inline void arch_local_irq_restore(unsigned long flags) > { > - csr_set(CSR_STATUS, flags & SR_IE); > + if (flags & SR_IE) > + csr_set(CSR_STATUS, SR_IE); > + else > + csr_clear(CSR_STATUS, SR_IE); > } > > #endif /* _ASM_RISCV_IRQFLAGS_H */ > -- > 2.20.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, Aug 3, 2023 at 10:36 PM Conor Dooley <conor@kernel.org> wrote: > > Hey, > > On Thu, Aug 03, 2023 at 09:28:18PM +0800, Xu Lu wrote: > > When arch_local_irq_restore() is called with flags indicating irqs > > disabled, we need to clear SR_IE bit in CSR_STATUS, whereas current > > implementation based on csr_set() function only sets SR_IE bit of > > CSR_STATUS when SR_IE bit of flags is high and does nothing when > > SR_IE bit of flags is low. > > > > This commit supplies csr clear operation when calling irq restore > > function with flags indicating irq disabled. > > > > Fixes: 6d60b6ee0c97 ("RISC-V: Device, timer, IRQs, and the SBI") > > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> > > --- > > There's no changelog or explanation, so what is the reason for the > resend? > > Cheers, > Conor. Sorry for disturbing you. I haven't received any response since the patch was sent last week. I think maintainers are busy and thus I want to have a ping. This patch was resent because I found it is said that resending patch is one of the methods to ping maintainers in this kernel doc: https://www.kernel.org/doc/html/latest/process/submitting-patches.html. > > arch/riscv/include/asm/irqflags.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/irqflags.h b/arch/riscv/include/asm/irqflags.h > > index 08d4d6a5b7e9..7c31fc3c3559 100644 > > --- a/arch/riscv/include/asm/irqflags.h > > +++ b/arch/riscv/include/asm/irqflags.h > > @@ -49,7 +49,10 @@ static inline int arch_irqs_disabled(void) > > /* set interrupt enabled status */ > > static inline void arch_local_irq_restore(unsigned long flags) > > { > > - csr_set(CSR_STATUS, flags & SR_IE); > > + if (flags & SR_IE) > > + csr_set(CSR_STATUS, SR_IE); > > + else > > + csr_clear(CSR_STATUS, SR_IE); > > } > > > > #endif /* _ASM_RISCV_IRQFLAGS_H */ > > -- > > 2.20.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, Aug 03, 2023 at 11:20:53PM +0800, 旭路 wrote: > On Thu, Aug 3, 2023 at 10:36 PM Conor Dooley <conor@kernel.org> wrote: > > > > Hey, > > > > On Thu, Aug 03, 2023 at 09:28:18PM +0800, Xu Lu wrote: > > > When arch_local_irq_restore() is called with flags indicating irqs > > > disabled, we need to clear SR_IE bit in CSR_STATUS, whereas current > > > implementation based on csr_set() function only sets SR_IE bit of > > > CSR_STATUS when SR_IE bit of flags is high and does nothing when > > > SR_IE bit of flags is low. > > > > > > This commit supplies csr clear operation when calling irq restore > > > function with flags indicating irq disabled. > > > > > > Fixes: 6d60b6ee0c97 ("RISC-V: Device, timer, IRQs, and the SBI") > > > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> > > > --- > > > > There's no changelog or explanation, so what is the reason for the > > resend? > > > > Cheers, > > Conor. > > Sorry for disturbing you. Nah, tis fine. > I haven't received any response since the patch was sent last week. I > think maintainers are busy and thus I want to have a ping. This patch > was resent because I found it is said that resending patch is one of > the methods to ping maintainers in this kernel doc: > https://www.kernel.org/doc/html/latest/process/submitting-patches.html. Right, resending is a valid way to do that - but leaving a comment about why you are resending under the --- line is advised so that people know why it is that you are resending. Thanks, Conor.
On Thu, Aug 3, 2023 at 11:24 PM Conor Dooley <conor@kernel.org> wrote: > > On Thu, Aug 03, 2023 at 11:20:53PM +0800, 旭路 wrote: > > On Thu, Aug 3, 2023 at 10:36 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > Hey, > > > > > > On Thu, Aug 03, 2023 at 09:28:18PM +0800, Xu Lu wrote: > > > > When arch_local_irq_restore() is called with flags indicating irqs > > > > disabled, we need to clear SR_IE bit in CSR_STATUS, whereas current > > > > implementation based on csr_set() function only sets SR_IE bit of > > > > CSR_STATUS when SR_IE bit of flags is high and does nothing when > > > > SR_IE bit of flags is low. > > > > > > > > This commit supplies csr clear operation when calling irq restore > > > > function with flags indicating irq disabled. > > > > > > > > Fixes: 6d60b6ee0c97 ("RISC-V: Device, timer, IRQs, and the SBI") > > > > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> > > > > --- > > > > > > There's no changelog or explanation, so what is the reason for the > > > resend? > > > > > > Cheers, > > > Conor. > > > > Sorry for disturbing you. > > Nah, tis fine. > > > I haven't received any response since the patch was sent last week. I > > think maintainers are busy and thus I want to have a ping. This patch > > was resent because I found it is said that resending patch is one of > > the methods to ping maintainers in this kernel doc: > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html. > > Right, resending is a valid way to do that - but leaving a comment about > why you are resending under the --- line is advised so that people know > why it is that you are resending. > > Thanks, > Conor. Got that. Thanks very much.
diff --git a/arch/riscv/include/asm/irqflags.h b/arch/riscv/include/asm/irqflags.h index 08d4d6a5b7e9..7c31fc3c3559 100644 --- a/arch/riscv/include/asm/irqflags.h +++ b/arch/riscv/include/asm/irqflags.h @@ -49,7 +49,10 @@ static inline int arch_irqs_disabled(void) /* set interrupt enabled status */ static inline void arch_local_irq_restore(unsigned long flags) { - csr_set(CSR_STATUS, flags & SR_IE); + if (flags & SR_IE) + csr_set(CSR_STATUS, SR_IE); + else + csr_clear(CSR_STATUS, SR_IE); } #endif /* _ASM_RISCV_IRQFLAGS_H */
When arch_local_irq_restore() is called with flags indicating irqs disabled, we need to clear SR_IE bit in CSR_STATUS, whereas current implementation based on csr_set() function only sets SR_IE bit of CSR_STATUS when SR_IE bit of flags is high and does nothing when SR_IE bit of flags is low. This commit supplies csr clear operation when calling irq restore function with flags indicating irq disabled. Fixes: 6d60b6ee0c97 ("RISC-V: Device, timer, IRQs, and the SBI") Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> --- arch/riscv/include/asm/irqflags.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)