netdev - Re: [PATCH net-next v7 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: <ab8852ce-72e8-4d5b-8c88-772a6c9f1485@lunn.ch>
Date: Thu, 11 May 2023 14:31:44 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: 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 v7 6/9] net: txgbe: Support GPIO to SFP socket

> +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	struct txgbe *txgbe = wx->priv;
> +	int val;
> +
> +	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> +
> +	txgbe->gpio_orig &= ~BIT(offset);
> +	txgbe->gpio_orig |= val;
> +
> +	return !!(val & BIT(offset));
> +}

> +static void txgbe_irq_handler(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct wx *wx = irq_desc_get_handler_data(desc);
> +	struct txgbe *txgbe = wx->priv;
> +	irq_hw_number_t hwirq;
> +	unsigned long gpioirq;
> +	struct gpio_chip *gc;
> +	u32 gpio;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
> +
> +	/* workaround for hysteretic gpio interrupts */
> +	gpio = rd32(wx, WX_GPIO_EXT);
> +	if (!gpioirq)
> +		gpioirq = txgbe->gpio_orig ^ gpio;

Please could you expand on the comment. Are you saying that
WX_GPIO_INTSTATUS sometimes does not contain the GPIO which caused the
interrupt? If so, you then compare the last gpio_get with the current
value and assume that is what caused the interrupt?

> +
> +	gc = txgbe->gpio;
> +	for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
> +		generic_handle_domain_irq(gc->irq.domain, hwirq);
> +
> +	chained_irq_exit(chip, desc);
> +
> +	/* unmask interrupt */
> +	if (netif_running(wx->netdev))
> +		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));

Is that a hardware requirement, that interrupts only work when the
interface is running? Interrupts are not normally conditional like
this, at least when the SoC provides the GPIO controller. 

Andrew

---
pw-bot: cr