Hi, Thomas
Thanks for your comment.
> On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote:
[...]
> > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32
> > +reg, u32 mask, u32 data) {
> > + u32 value;
> > +
> > + value = ioread32(irqc->base + reg) & ~mask;
> > + data &= mask;
>
> Why?
>
If I want to update the reg GENMASK(7, 4) to value 5, the data I will pass in 5 << 4
> > + data |= value;
> > + iowrite32(data, irqc->base + reg);
>
> How is this serialized against concurrent invocations of this code on different
> CPUs for different interrupts?
>
> It's not and this requires a raw_spinlock for protection.
>
Will use raw_spinlock.
> > +}
> > +
> > +static void starfive_intc_unmask(struct irq_data *d) {
> > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> > +
> > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0);
> > +}
> > +
> > +static void starfive_intc_mask(struct irq_data *d) {
> > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> > +
> > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq),
> > +BIT(d->hwirq)); }
> > +
[...]
> > + irqc->root_domain = irq_domain_add_linear(intc,
> STARFIVE_INTC_SRC_IRQ_NUM,
> > + &starfive_intc_domain_ops, irqc);
> > + if (!irqc->root_domain) {
> > + pr_err("Unable to create IRQ domain\n");
> > + ret = -EINVAL;
> > + goto err_clk;
> > + }
> > +
> > + parent_irq = of_irq_get(intc, 0);
> > + if (parent_irq < 0) {
> > + pr_err("Failed to get main IRQ: %d\n", parent_irq);
> > + ret = parent_irq;
> > + goto err_clk;
>
> Leaks the interrupt domain, no?
>
> Thanks,
>
Will use irq_domain_remove() free domain.
regards
Changhuang
