RE: [PATCH net-next v8 6/9] net: txgbe: Support GPIO to SFP socket

Jiawen Wu
2023-05-22T09:03:07Z

On Friday, May 19, 2023 9:13 PM, Andrew Lunn wrote:
> > I have one MSI-X interrupt for all general MAC interrupt (see TXGBE_PX_MISC_IEN_MASK).
> > It has 32 bits to indicate various interrupts, GPIOs are the one of them. When GPIO
> > interrupt is determined, GPIO_INT_STATUS register should be read to determine
> > which GPIO line has changed state.
>
> So you have another interrupt controller above the GPIO interrupt
> controller. regmap-gpio is pushing you towards describing this
> interrupt controller as a Linux interrupt controller.
>
> When you look at drivers handling interrupts, most leaf interrupt
> controllers are not described as Linux interrupt controllers. The
> driver interrupt handler reads the interrupt status register and
> internally dispatches to the needed handler. This works well when
> everything is internal to one driver.
>
> However, here, you have two drivers involved, your MAC driver and a
> GPIO driver instantiated by the MAC driver. So i think you are going
> to need to described the MAC interrupt controller as a Linux interrupt
> controller.
>
> Take a look at the mv88e6xxx driver, which does this. It has two
> interrupt controller embedded within it, and they are chained.

Now I add two interrupt controllers, the first one for the MAC interrupt,
and the second one for regmap-gpio. In the second adding flow,

irq = irq_find_mapping(txgbe->misc.domain, TXGBE_PX_MISC_GPIO_OFFSET);
err = regmap_add_irq_chip_fwnode(fwnode, regmap, irq, 0, 0,
chip, &chip_data);

and then,

config.irq_domain = regmap_irq_get_domain(chip_data);
gpio_regmap = gpio_regmap_register(&config);

"txgbe->misc.domain" is the MAC interrupt domain. I think this flow should
be correct, but still failed to get gpio_irq from gpio_desc with err -517.

And I still have doubts about what I said earlier:
https://lore.kernel.org/netdev/20230515063200.301026-1-jiawenwu@trustnetic.com/T/#me1be68e1a1e44426ecc0dd8edf0f6b224e50630d

There really is nothing wrong with gpiochip_to_irq()??

> > > If you are getting errors when removing the driver it means you are
> > > missing some level of undoing what us done in probe. Are you sure
> > > regmap_del_irq_chip() is being called on unload?
> >
> > I used devm_* all when I registered them.
>
> Look at the ordering. Is regmap_del_irq_chip() being called too late?
> I've had problems like this with the mv88e6xxx driver and its
> interrupt controllers. I ended up not using devm_ so i had full
> control over the order things got undone. In that case, the external
> devices was PHYs, with the PHY interrupt being inside the Ethernet
> switch, which i exposed using a Linux interrupt controller.

I use no devm_ functions to add regmap irq chip, register gpio regmap,
and call their del/unregister functions at the position corresponding to
release. irq_domain_remove() call trace still exist.

[ 104.553182] Call Trace:
[ 104.553184] <TASK>
[ 104.553185] irq_domain_remove+0x2b/0xe0
[ 104.553190] regmap_del_irq_chip.part.0+0x8a/0x160
[ 104.553196] txgbe_remove_phy+0x57/0x80 [txgbe]
[ 104.553201] txgbe_remove+0x2a/0x90 [txgbe]
[ 104.553205] pci_device_remove+0x36/0xa0
[ 104.553208] device_release_driver_internal+0xaa/0x140
[ 104.553213] driver_detach+0x44/0x90
[ 104.553215] bus_remove_driver+0x69/0xf0
[ 104.553217] pci_unregister_driver+0x29/0xb0
[ 104.553220] __x64_sys_delete_module+0x145/0x240
[ 104.553223] ? exit_to_user_mode_prepare+0x3c/0x1a0
[ 104.553226] do_syscall_64+0x3b/0x90
[ 104.553230] entry_SYSCALL_64_after_hwframe+0x72/0xdc