netdev - Re: [RFC PATCH net-next v2 2/2] net: txgbe: use irq

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 for Android: free password hash cracker in your pocket

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

Message-ID: <373c3c22-3fac-4ae7-9bdf-27ecfcc4a9f6@lunn.ch>
Date: Thu, 18 Jan 2024 14:33:29 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, linux@...linux.org.uk, horms@...nel.org,
	netdev@...r.kernel.org, mengyuanlou@...-swift.com
Subject: Re: [RFC PATCH net-next v2 2/2] net: txgbe: use irq_domain for
 interrupt controller

On Thu, Jan 18, 2024 at 03:40:29PM +0800, Jiawen Wu wrote:
> In the current interrupt controller, the MAC interrupt acts as the
> parent interrupt in the GPIO IRQ chip. But when the number of Rx/Tx
> ring changes, the PCI IRQ vector needs to be reallocated. Then this
> interrupt controller would be corrupted. So use irq_domain structure
> to avoid the above problem.

Thanks for expanding the commit message.

> +static int txgbe_request_gpio_irq(struct txgbe *txgbe)
> +{
> +	txgbe->gpio_irq = irq_find_mapping(txgbe->misc.domain, 0);
> +	return request_threaded_irq(txgbe->gpio_irq, NULL,
> +				    txgbe_gpio_irq_handler,
> +				    IRQF_ONESHOT, "txgbe-gpio-irq", txgbe);
> +}
> +
> +static int txgbe_request_link_irq(struct txgbe *txgbe)
> +{
> +	txgbe->link_irq = irq_find_mapping(txgbe->misc.domain, 1);

It would be good to replace 0 and 1 with a #define making it clear
what they mean.

> +				     unsigned int irq,
> +				     irq_hw_number_t hwirq)
> +{
> +	struct txgbe *txgbe = d->host_data;
> +
> +	irq_set_chip_data(irq, txgbe);
> +	irq_set_chip(irq, &txgbe->misc.chip);
> +	irq_set_nested_thread(irq, 1);

irq_set_nested_thread() takes a bool, so its better to pass TRUE than
1.

> +static irqreturn_t txgbe_misc_irq_handle(int irq, void *data)
> +{
> +	struct txgbe *txgbe = data;
> +	struct wx *wx = txgbe->wx;
> +	unsigned int nhandled = 0;
> +	unsigned int sub_irq;
> +	u32 eicr;
> +
> +	eicr = wx_misc_isb(wx, WX_ISB_MISC);
> +	if (eicr & TXGBE_PX_MISC_GPIO) {
> +		sub_irq = irq_find_mapping(txgbe->misc.domain, 0);
> +		handle_nested_irq(sub_irq);
> +		nhandled++;
> +	}
> +	if (eicr & (TXGBE_PX_MISC_ETH_LK | TXGBE_PX_MISC_ETH_LKDN |
> +		    TXGBE_PX_MISC_ETH_AN)) {
> +		sub_irq = irq_find_mapping(txgbe->misc.domain, 1);

You can reuse the #defines you add above here.

    Andrew