On 5/15/25 11:54 AM, Jan Beulich wrote:
On 06.05.2025 18:51, Oleksii Kurochko wrote:
+static void cf_check aplic_set_irq_type(struct irq_desc *desc, unsigned int
type)
+{
+ /*
+ * Interrupt 0 isn't possible based on the spec:
+ * Each of an APLIC’s interrupt sources has a fixed unique identity
number in the range 1 to N,
+ * where N is the total number of sources at the APLIC. The number zero
is not a valid interrupt
+ * identity number at an APLIC. The maximum number of interrupt sources
an APLIC may support
+ * is 1023.
+ *
+ * Thereby interrupt 1 will correspond to bit 0 in sourcecfg[] register,
+ * interrupt 2 ->sourcecfg[1] and so on.
+ *
+ * And that is the reason why we need -1.
+ */
+ unsigned int irq_bit = desc->irq - 1;
+
+ spin_lock(&aplic.lock);
+
+ switch(type)
Nit: style
+ {
+ case IRQ_TYPE_EDGE_RISING:
+ writel(APLIC_SOURCECFG_SM_EDGE_RISE, &aplic.regs->sourcecfg[irq_bit]);
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ writel(APLIC_SOURCECFG_SM_EDGE_FALL, &aplic.regs->sourcecfg[irq_bit]);
+ break;
+
+ case IRQ_TYPE_LEVEL_HIGH:
+ writel(APLIC_SOURCECFG_SM_LEVEL_HIGH, &aplic.regs->sourcecfg[irq_bit]);
+ break;
+
+ case IRQ_TYPE_LEVEL_LOW:
+ writel(APLIC_SOURCECFG_SM_LEVEL_LOW, &aplic.regs->sourcecfg[irq_bit]);
+ break;
+
+ case IRQ_TYPE_NONE:
+ fallthrough;
This is pointless (and hampering readability) when there are no other
statements.
Oh, okay, it should be:
case IRQ_TYPE_NONE:
case IRQ_TYPE_INVALID:
I thought fallthrough should be used always in such cases.
Anyway, I'll drop it.
With both taken care of:
Acked-by: Jan Beulich<jbeul...@suse.com>
Thanks.
I am going also to add "ASSERT(spin_is_locked(&desc->lock));" at the start of
this
function to be algined with other callbacks which uses
spin_{un}lock(&aplic.lock).
~ Oleksii
