gpio: mvebu: fix devres LIFO ordering between GPIO chip and IRQ domain
PATCH v2 1/1 gpio: mvebu: fix devres LIFO ordering between GPIO chip and IRQ domain
During driver removal, devres cleans up in LIFO order. The IRQ domain was created and its devm cleanup action registered after devm_gpiochip_add_data(), so the domain was destroyed before the GPIO chip was deregistered. If gpiod_to_irq() is called on a pin during this window, mvebu_gpio_to_irq() passes the freed mvchip->domain to irq_create_mapping().
Fix by moving the IRQ domain creation, devm cleanup action registration, generic chip allocation, and chip type setup before devm_gpiochip_add_data(). This ensures the GPIO chip is torn down first (preventing new IRQ mappings), then the IRQ domain is removed, and finally mvchip is freed.
---
v2: fix Fixes tag
drivers/gpio/gpio-mvebu.c | 133 +++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 65 deletions(-)
drivers/gpio/gpio-mvebu.c +68 -65
@@ -1262,6 +1262,59 @@ static int mvebu_gpio_probe(struct platform_device *pdev)BUG (); } +/* Some gpio controllers do not provide irq support */ +if (have_irqs) { + mvchip->domain =irq_domain_create_linear (dev_fwnode (&pdev->dev), + ngpios, + &irq_generic_chip_ops, +NULL ); +if (!mvchip->domain) +return dev_err_probe (&pdev->dev, -ENODEV , +"couldn't allocate irq domain %s (DT).\n" , + mvchip->chip.label); + + err =devm_add_action_or_reset (&pdev->dev, + mvebu_gpio_remove_irq_domain, + mvchip->domain); +if (err) +return err; + + err =irq_alloc_domain_generic_chips ( + mvchip->domain, ngpios,2 , np->name, handle_level_irq, +IRQ_NOREQUEST |IRQ_NOPROBE |IRQ_LEVEL ,0 , +IRQ_GC_INIT_NESTED_LOCK ); +if (err) +return dev_err_probe (&pdev->dev, err, +"couldn't allocate irq chips %s (DT).\n" , + mvchip->chip.label); + +/* +* NOTE: The common accessors cannot be used because of the +* percpu access to the mask registers +*/ + gc =irq_get_domain_generic_chip (mvchip->domain,0 ); + gc->private = mvchip; + ct = &gc->chip_types[0 ]; + ct->type =IRQ_TYPE_LEVEL_HIGH |IRQ_TYPE_LEVEL_LOW ; + ct->chip.irq_mask = mvebu_gpio_level_irq_mask; + ct->chip.irq_unmask = mvebu_gpio_level_irq_unmask; + ct->chip.irq_set_type = mvebu_gpio_irq_set_type; + ct->chip.irq_set_wake = mvebu_gpio_set_wake_irq; + ct->chip.flags =IRQCHIP_SET_TYPE_MASKED |IRQCHIP_MASK_ON_SUSPEND ; + ct->chip.name = mvchip->chip.label; + + ct = &gc->chip_types[1 ]; + ct->type =IRQ_TYPE_EDGE_RISING |IRQ_TYPE_EDGE_FALLING ; + ct->chip.irq_ack = mvebu_gpio_irq_ack; + ct->chip.irq_mask = mvebu_gpio_edge_irq_mask; + ct->chip.irq_unmask = mvebu_gpio_edge_irq_unmask; + ct->chip.irq_set_type = mvebu_gpio_irq_set_type; + ct->chip.irq_set_wake = mvebu_gpio_set_wake_irq; + ct->chip.flags =IRQCHIP_SET_TYPE_MASKED |IRQCHIP_MASK_ON_SUSPEND ; + ct->handler = handle_edge_irq; + ct->chip.name = mvchip->chip.label; + } + err =devm_gpiochip_add_data (&pdev->dev, &mvchip->chip, mvchip);if (err) return dev_err_probe(&pdev->dev, err,
@@ -1274,71 +1327,21 @@ static int mvebu_gpio_probe(struct platform_device *pdev)return err; } -/* Some gpio controllers do not provide irq support */ -if (!have_irqs) -return 0 ; - - mvchip->domain =irq_domain_create_linear (dev_fwnode (&pdev->dev), ngpios, - &irq_generic_chip_ops,NULL ); -if (!mvchip->domain) { -dev_err (&pdev->dev,"couldn't allocate irq domain %s (DT).\n" , - mvchip->chip.label); -return -ENODEV ; - } - - err =devm_add_action_or_reset (&pdev->dev, mvebu_gpio_remove_irq_domain, - mvchip->domain); -if (err) -return err; - - err =irq_alloc_domain_generic_chips ( - mvchip->domain, ngpios,2 , np->name, handle_level_irq, -IRQ_NOREQUEST |IRQ_NOPROBE |IRQ_LEVEL ,0 ,IRQ_GC_INIT_NESTED_LOCK ); -if (err) { -dev_err (&pdev->dev,"couldn't allocate irq chips %s (DT).\n" , - mvchip->chip.label); -return err; - } - -/* -* NOTE: The common accessors cannot be used because of the percpu -* access to the mask registers -*/ - gc =irq_get_domain_generic_chip (mvchip->domain,0 ); - gc->private = mvchip; - ct = &gc->chip_types[0 ]; - ct->type =IRQ_TYPE_LEVEL_HIGH |IRQ_TYPE_LEVEL_LOW ; - ct->chip.irq_mask = mvebu_gpio_level_irq_mask; - ct->chip.irq_unmask = mvebu_gpio_level_irq_unmask; - ct->chip.irq_set_type = mvebu_gpio_irq_set_type; - ct->chip.irq_set_wake = mvebu_gpio_set_wake_irq; - ct->chip.flags =IRQCHIP_SET_TYPE_MASKED |IRQCHIP_MASK_ON_SUSPEND ; - ct->chip.name = mvchip->chip.label; - - ct = &gc->chip_types[1 ]; - ct->type =IRQ_TYPE_EDGE_RISING |IRQ_TYPE_EDGE_FALLING ; - ct->chip.irq_ack = mvebu_gpio_irq_ack; - ct->chip.irq_mask = mvebu_gpio_edge_irq_mask; - ct->chip.irq_unmask = mvebu_gpio_edge_irq_unmask; - ct->chip.irq_set_type = mvebu_gpio_irq_set_type; - ct->chip.irq_set_wake = mvebu_gpio_set_wake_irq; - ct->chip.flags =IRQCHIP_SET_TYPE_MASKED |IRQCHIP_MASK_ON_SUSPEND ; - ct->handler = handle_edge_irq; - ct->chip.name = mvchip->chip.label; - -/* -* Setup the interrupt handlers. Each chip can have up to 4 -* interrupt handlers, with each handler dealing with 8 GPIO -* pins. -*/ -for (i =0 ; i <ARRAY_SIZE (mvchip->bank_irq); i++) { -int irq =platform_get_irq_optional (pdev, i); - -if (irq <0 ) -continue ; -irq_set_chained_handler_and_data (irq, mvebu_gpio_irq_handler, - mvchip); - mvchip->bank_irq[i] = irq; +if (have_irqs) { +/* +* Setup the interrupt handlers. Each chip can have up to 4 +* interrupt handlers, with each handler dealing with 8 GPIO +* pins. +*/ +for (i =0 ; i <ARRAY_SIZE (mvchip->bank_irq); i++) { +int irq =platform_get_irq_optional (pdev, i); + +if (irq <0 ) +continue ; +irq_set_chained_handler_and_data (irq, + mvebu_gpio_irq_handler, mvchip); + mvchip->bank_irq[i] = irq; + } }return 0 ;
Signature · 1 line
2.55.0
Quoted 13 lines · Rosen Penev
During driver removal, devres cleans up in LIFO order. The IRQ domain was created and its devm cleanup action registered after
devm_gpiochip_add_data(), so the domain was destroyed before the GPIO chip was deregistered. Ifgpiod_to_irq()is called on a pin during this window,mvebu_gpio_to_irq()passes the freedmvchip->domaintoirq_create_mapping().Fix by moving the IRQ domain creation, devm cleanup action registration, generic chip allocation, and chip type setup before
devm_gpiochip_add_data(). This ensures the GPIO chip is torn down first (preventing new IRQ mappings), then the IRQ domain is removed, and finally mvchip is freed.
Quoted 3 lines
Assisted-by opencode:big-pickleFixes fefe7b092345 ("gpio: introduce gpio-mvebu driver for Marvell SoCs")Signed-off-by Rosen Penev <[email protected]>
Excellent work on this!
Yours,
Linus Walleij