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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1e1615b3-566c-490c-8b1a-78f5521ca0b0@lunn.ch>
Date: Thu, 18 May 2023 14:48:35 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: 'Andy Shevchenko' <andy.shevchenko@...il.com>, netdev@...r.kernel.org,
	jarkko.nikula@...ux.intel.com, andriy.shevchenko@...ux.intel.com,
	mika.westerberg@...ux.intel.com, jsd@...ihalf.com,
	Jose.Abreu@...opsys.com, hkallweit1@...il.com,
	linux@...linux.org.uk, linux-i2c@...r.kernel.org,
	linux-gpio@...r.kernel.org, mengyuanlou@...-swift.com
Subject: Re: [PATCH net-next v8 6/9] net: txgbe: Support GPIO to SFP socket

> > I _think_ you are mixing upstream IRQs and downstream IRQs.
> > 
> > Interrupts are arranged in trees. The CPU itself only has one or two
> > interrupts. e.g. for ARM you have FIQ and IRQ. When the CPU gets an
> > interrupt, you look in the interrupt controller to see what external
> > or internal interrupt triggered the CPU interrupt. And that interrupt
> > controller might indicate the interrupt came from another interrupt
> > controller. Hence the tree structure. And each node in the tree is
> > considered an interrupt domain.
> > 
> > A GPIO controller can also be an interrupt controller. It has an
> > upstream interrupt, going to the controller above it. And it has
> > downstream interrupts, the GPIO lines coming into it which can cause
> > an interrupt. And the GPIO interrupt controller is a domain.
> > 
> > So what exactly does gpio_regmap_config.irq_domain mean? Is it the
> > domain of the upstream interrupt controller? Is it an empty domain
> > structure to be used by the GPIO interrupt controller? It is very
> > unlikely to have anything to do with the SFP devices below it.
> 
> Sorry, since I don't know much about interrupt,  it is difficult to understand
> regmap-irq in a short time. There are many questions about regmap-irq.
> 
> When I want to add an IRQ chip for regmap, for the further irq_domain,
> I need to pass a parameter of IRQ, and this IRQ will be requested with handler:
> regmap_irq_thread(). Which IRQ does it mean?

That is your upstream IRQ, the interrupt indicating one of your GPIO
lines has changed state.

> In the previous code of using
> devm_gpiochip_add_data(), I set the MSI-X interrupt as gpio-irq's parent, but
> it was used to set chained handler only. Should the parent be this IRQ? I found
> the error with irq_free_descs and irq_domain_remove when I remove txgbe.ko.

Do you have one MSI-X dedicated for GPIOs. Or is it your general MAC
interrupt, and you need to read an interrupt controller register to
determine it was GPIOs which triggered the interrupt?

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?

> As you said, the interrupt of each tree node has its domain. Can I understand
> that there are two layer in the interrupt tree for MSI-X and GPIOs, and requesting
> them separately is not conflicting? Although I thought so, but after I implement
> gpio-regmap, SFP driver even could not find gpio_desc. Maybe I missed something
> on registering gpio-regmap...

That is probably some sort of naming issue. You might want to add some
prints in swnode_find_gpio() and gpiochip_find() to see what it is
looking for vs what the name actually is.

	Andrew