Re: [PATCH v2] irqchip: add TS7800v1 fpga based controller driver

[firas ashkar <firas.ashkar@savoirfairelinux.com>]

-- 
Firas Ashkar
Developpeur Système Embarqué

savoirfairelinux.com | Montréal, Québec

Tél.: +1 514 276 5468 ext. 118


On Sat, 2022-10-22 at 11:04 +0100, Marc Zyngier wrote:
> On Fri, 21 Oct 2022 19:14:48 +0100,
> firas ashkar <firas.ashkar@savoirfairelinux.com> wrote:
> > > > > > +static void ts7800_ic_chained_handle_irq(struct irq_desc
> > > > > > *desc)
> > > > > > +{
> > > > > > +       struct ts7800_irq_data *data =
> > > > > > irq_desc_get_handler_data(desc);
> > > > > > +       struct irq_chip *chip = irq_desc_get_chip(desc);
> > > > > > +       u32 mask_bits = readl(data->base + IRQ_MASK_REG);
> > > > > > +       u32 status = readl(data->bridge);
> > > > > > +
> > > > > > +       chained_irq_enter(chip, desc);
> > > > > > +
> > > > > > +       if (unlikely(status == 0)) {
> > > > > > +               handle_bad_irq(desc);
> > > > > > +               goto out;
> > > > > > +       }
> > > > > > +
> > > > > > +       do {
> > > > > > +               unsigned int bit = __ffs(status);
> > > > > > +               int irq = irq_find_mapping(data->domain,
> > > > > > bit);
> > > > > > +
> > > > > > +               if (irq && (mask_bits & BIT(bit)))
> > > > > > +                       generic_handle_irq(irq);
> > > > > 
> > > > > Again, this is not appropriate. I've pointed you to the right
> > > > > API
> > > > > in
> > > > > my previous review of this patch.
> > > > 'generic_handle_domain_irq' causing some issues
> > > 
> > > Which issue?
> > # insmod /tmp/virt-dma.ko 
> > # insmod /tmp/ts-dmac.ko 
> > generic_handle_domain_irq failed, error -22
> > irq 2, desc: a285c39f, depth: 0, count: 0, unhandled: 0
> > ->handle_irq():  e7046ee9, ts7800_ic_chained_handle_irq+0x0/0x1ec
> > [irq_ts7800]
> > ->irq_data.chip(): 70d81143, 0xc14db44c
> > ->action(): f799c8dd
> > ->action->handler(): dcf07981, bad_chained_irq+0x0/0x64
> >      IRQ_LEVEL set
> >    IRQ_NOPROBE set
> >  IRQ_NOREQUEST set
> >   IRQ_NOTHREAD set
> > unexpected IRQ trap at vector 02
> > generic_handle_domain_irq failed, error -22
> 
> [...]
> 
> News flash, your code is buggy. How about you debug it? The domain
> name you use definitely indicates that you "know how" to do it...
Ack 
1. using domain hwirq as in 
'generic_handle_domain_irq(data->domain, domain_hwirq);', and
2. changing the irq ack callback a bit 
fixed the issue

# cat /proc/interrupts 
           CPU0       
  1:    7730781  orion_irq     Level     orion_tick
  4:       2706  orion_irq     Level     ttyS0
 13:          0  orion_irq     Level     ehci_hcd:usb2
 18:          0  orion_irq     Level     ehci_hcd:usb1
 22:     101488  orion_irq     Level     eth0
 23:       5075  orion_irq     Level     orion-mdio
 29:          0  orion_irq     Level     mv_crypto
 31:          2  orion_irq     Level     mv_xor.0
 65:     292763  ts7800-irqc   0 Edge      ts-dmac-cpupci
 66:     292760  ts7800-irqc   1 Edge      ts-dmac-fpga
Err:          0
# uname -a
Linux ts-7800 6.1.0-rc1 #6 PREEMPT Fri Oct 21 11:25:55 EDT 2022
armv5tel GNU/Linux
# lsmod
Module                  Size  Used by    Not tainted
ts7800v1_sdmmc         36893  0 
ts_dmac                17970  1 
virt_dma                3716  1 ts_dmac
irq_ts7800              4035  0 [permanent]
#

ty
> 
>         M.
>