| 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
| ||
|
Message-ID: <ZHjQbaFjD3D45C/v@smile.fi.intel.com>
Date: Thu, 1 Jun 2023 20:07:57 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: netdev@...r.kernel.org, jarkko.nikula@...ux.intel.com,
mika.westerberg@...ux.intel.com, jsd@...ihalf.com,
Jose.Abreu@...opsys.com, andrew@...n.ch, 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 v10 6/9] net: txgbe: Support GPIO to SFP socket
On Thu, Jun 01, 2023 at 11:01:37AM +0800, Jiawen Wu wrote:
> Register GPIO chip and handle GPIO IRQ for SFP socket.
LGTM, but one comment below.
After addressing,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
> drivers/net/ethernet/wangxun/Kconfig | 2 +
> drivers/net/ethernet/wangxun/libwx/wx_lib.c | 3 +-
> drivers/net/ethernet/wangxun/libwx/wx_type.h | 3 +
> .../net/ethernet/wangxun/txgbe/txgbe_main.c | 20 +-
> .../net/ethernet/wangxun/txgbe/txgbe_phy.c | 248 ++++++++++++++++++
> .../net/ethernet/wangxun/txgbe/txgbe_type.h | 23 ++
> 6 files changed, 280 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
> index 59f3a3f492cf..3744735fa708 100644
> --- a/drivers/net/ethernet/wangxun/Kconfig
> +++ b/drivers/net/ethernet/wangxun/Kconfig
> @@ -47,6 +47,8 @@ config TXGBE
> select PHYLINK
> select HWMON if TXGBE=y
> select SFP
> + select GPIOLIB
> + select GPIOLIB_IRQCHIP
> select LIBWX
> help
> This driver supports Wangxun(R) 10GbE PCI Express family of
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> index 1e8d8b7b0c62..590215303d45 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> @@ -1348,7 +1348,8 @@ void wx_free_irq(struct wx *wx)
> free_irq(entry->vector, q_vector);
> }
>
> - free_irq(wx->msix_entries[vector].vector, wx);
> + if (wx->mac.type == wx_mac_em)
> + free_irq(wx->msix_entries[vector].vector, wx);
> }
> EXPORT_SYMBOL(wx_free_irq);
>
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index f59066d7375c..297c389fa890 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -79,7 +79,9 @@
> #define WX_GPIO_INTMASK 0x14834
> #define WX_GPIO_INTTYPE_LEVEL 0x14838
> #define WX_GPIO_POLARITY 0x1483C
> +#define WX_GPIO_INTSTATUS 0x14844
> #define WX_GPIO_EOI 0x1484C
> +#define WX_GPIO_EXT 0x14850
>
> /*********************** Transmit DMA registers **************************/
> /* transmit global control */
> @@ -643,6 +645,7 @@ struct wx {
> bool wol_enabled;
> bool ncsi_enabled;
> bool gpio_ctrl;
> + raw_spinlock_t gpio_lock;
>
> /* Tx fast path data */
> int num_tx_queues;
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index e10296abf5b4..ded04e9e136f 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -82,6 +82,8 @@ static int txgbe_enumerate_functions(struct wx *wx)
> **/
> static void txgbe_irq_enable(struct wx *wx, bool queues)
> {
> + wr32(wx, WX_PX_MISC_IEN, TXGBE_PX_MISC_IEN_MASK);
> +
> /* unmask interrupt */
> wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
> if (queues)
> @@ -129,17 +131,6 @@ static irqreturn_t txgbe_intr(int __always_unused irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static irqreturn_t txgbe_msix_other(int __always_unused irq, void *data)
> -{
> - struct wx *wx = data;
> -
> - /* re-enable the original interrupt state */
> - if (netif_running(wx->netdev))
> - txgbe_irq_enable(wx, false);
> -
> - return IRQ_HANDLED;
> -}
> -
> /**
> * txgbe_request_msix_irqs - Initialize MSI-X interrupts
> * @wx: board private structure
> @@ -171,13 +162,6 @@ static int txgbe_request_msix_irqs(struct wx *wx)
> }
> }
>
> - err = request_irq(wx->msix_entries[vector].vector,
> - txgbe_msix_other, 0, netdev->name, wx);
> - if (err) {
> - wx_err(wx, "request_irq for msix_other failed: %d\n", err);
> - goto free_queue_irqs;
> - }
> -
> return 0;
>
> free_queue_irqs:
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> index d95dc131e91b..32fff693e073 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
>
> +#include <linux/gpio/machine.h>
> +#include <linux/gpio/driver.h>
> #include <linux/gpio/property.h>
> #include <linux/clk-provider.h>
> #include <linux/clkdev.h>
> @@ -10,6 +12,7 @@
> #include <linux/regmap.h>
>
> #include "../libwx/wx_type.h"
> +#include "../libwx/wx_hw.h"
> #include "txgbe_type.h"
> #include "txgbe_phy.h"
>
> @@ -74,6 +77,245 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
> return software_node_register_node_group(nodes->group);
> }
>
> +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct wx *wx = gpiochip_get_data(chip);
> + int val;
> +
> + val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> +
> + return !!(val & BIT(offset));
> +}
> +
> +static int txgbe_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct wx *wx = gpiochip_get_data(chip);
> + u32 val;
> +
> + val = rd32(wx, WX_GPIO_DDR);
> + if (BIT(offset) & val)
> + return GPIO_LINE_DIRECTION_OUT;
> +
> + return GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int txgbe_gpio_direction_in(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct wx *wx = gpiochip_get_data(chip);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> + wr32m(wx, WX_GPIO_DDR, BIT(offset), 0);
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
> + int val)
> +{
> + struct wx *wx = gpiochip_get_data(chip);
> + unsigned long flags;
> + u32 set;
> +
> + set = val ? BIT(offset) : 0;
> +
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> + wr32m(wx, WX_GPIO_DR, BIT(offset), set);
> + wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static void txgbe_gpio_irq_ack(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + irq_hw_number_t hwirq = irqd_to_hwirq(d);
> + struct wx *wx = gpiochip_get_data(gc);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> + wr32(wx, WX_GPIO_EOI, BIT(hwirq));
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> +}
> +
> +static void txgbe_gpio_irq_mask(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + irq_hw_number_t hwirq = irqd_to_hwirq(d);
> + struct wx *wx = gpiochip_get_data(gc);
> + unsigned long flags;
> +
> + gpiochip_disable_irq(gc, hwirq);
> +
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> + wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), BIT(hwirq));
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> +}
> +
> +static void txgbe_gpio_irq_unmask(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + irq_hw_number_t hwirq = irqd_to_hwirq(d);
> + struct wx *wx = gpiochip_get_data(gc);
> + unsigned long flags;
> +
> + gpiochip_enable_irq(gc, hwirq);
> +
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> + wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), 0);
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> +}
> +
> +static void txgbe_toggle_trigger(struct gpio_chip *gc, unsigned int offset)
> +{
> + struct wx *wx = gpiochip_get_data(gc);
> + u32 pol, val;
> +
> + pol = rd32(wx, WX_GPIO_POLARITY);
> + val = rd32(wx, WX_GPIO_EXT);
> +
> + if (val & BIT(offset))
> + pol &= ~BIT(offset);
> + else
> + pol |= BIT(offset);
> +
> + wr32(wx, WX_GPIO_POLARITY, pol);
> +}
> +
> +static int txgbe_gpio_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + irq_hw_number_t hwirq = irqd_to_hwirq(d);
> + struct wx *wx = gpiochip_get_data(gc);
> + u32 level, polarity, mask;
> + unsigned long flags;
> +
> + mask = BIT(hwirq);
> +
> + if (type & IRQ_TYPE_LEVEL_MASK) {
> + level = 0;
> + irq_set_handler_locked(d, handle_level_irq);
> + } else {
> + level = mask;
> + irq_set_handler_locked(d, handle_edge_irq);
> + }
> +
> + if (type == IRQ_TYPE_EDGE_RISING || type == IRQ_TYPE_LEVEL_HIGH)
> + polarity = mask;
> + else
> + polarity = 0;
> +
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> +
> + wr32m(wx, WX_GPIO_INTEN, mask, mask);
> + wr32m(wx, WX_GPIO_INTTYPE_LEVEL, mask, level);
> + if (type == IRQ_TYPE_EDGE_BOTH)
> + txgbe_toggle_trigger(gc, hwirq);
> + else
> + wr32m(wx, WX_GPIO_POLARITY, mask, polarity);
> +
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static const struct irq_chip txgbe_gpio_irq_chip = {
> + .name = "txgbe_gpio_irq",
> + .irq_ack = txgbe_gpio_irq_ack,
> + .irq_mask = txgbe_gpio_irq_mask,
> + .irq_unmask = txgbe_gpio_irq_unmask,
> + .irq_set_type = txgbe_gpio_set_type,
> + .flags = IRQCHIP_IMMUTABLE,
> + GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +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;
> + unsigned long flags;
> +
> + chained_irq_enter(chip, desc);
> +
> + gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
> +
> + gc = txgbe->gpio;
> + for_each_set_bit(hwirq, &gpioirq, gc->ngpio) {
> + int gpio = irq_find_mapping(gc->irq.domain, hwirq);
> + u32 irq_type = irq_get_trigger_type(gpio);
> +
> + generic_handle_domain_irq(gc->irq.domain, hwirq);
> +
> + if ((irq_type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH) {
> + raw_spin_lock_irqsave(&wx->gpio_lock, flags);
> + txgbe_toggle_trigger(gc, hwirq);
> + raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
> + }
> + }
> +
> + chained_irq_exit(chip, desc);
> +
> + /* unmask interrupt */
> + wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
> +}
> +
> +static int txgbe_gpio_init(struct txgbe *txgbe)
> +{
> + struct gpio_irq_chip *girq;
> + struct gpio_chip *gc;
> + struct device *dev;
> + struct wx *wx;
> + int ret;
> +
> + wx = txgbe->wx;
> + dev = &wx->pdev->dev;
> +
> + raw_spin_lock_init(&wx->gpio_lock);
> +
> + gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
> + if (!gc)
> + return -ENOMEM;
> +
> + gc->label = devm_kasprintf(dev, GFP_KERNEL, "txgbe_gpio-%x",
> + (wx->pdev->bus->number << 8) | wx->pdev->devfn);
Missing NULL check.
> + gc->base = -1;
> + gc->ngpio = 6;
> + gc->owner = THIS_MODULE;
> + gc->parent = dev;
> + gc->fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_GPIO]);
> + gc->get = txgbe_gpio_get;
> + gc->get_direction = txgbe_gpio_get_direction;
> + gc->direction_input = txgbe_gpio_direction_in;
> + gc->direction_output = txgbe_gpio_direction_out;
> +
> + girq = &gc->irq;
> + gpio_irq_chip_set_chip(girq, &txgbe_gpio_irq_chip);
> + girq->parent_handler = txgbe_irq_handler;
> + girq->parent_handler_data = wx;
> + girq->num_parents = 1;
> + girq->parents = devm_kcalloc(dev, girq->num_parents,
> + sizeof(*girq->parents), GFP_KERNEL);
> + if (!girq->parents)
> + return -ENOMEM;
> + girq->parents[0] = wx->msix_entries[wx->num_q_vectors].vector;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_bad_irq;
> +
> + ret = devm_gpiochip_add_data(dev, gc, wx);
> + if (ret)
> + return ret;
> +
> + txgbe->gpio = gc;
> +
> + return 0;
> +}
> +
> static int txgbe_clock_register(struct txgbe *txgbe)
> {
> struct pci_dev *pdev = txgbe->wx->pdev;
> @@ -187,6 +429,12 @@ int txgbe_init_phy(struct txgbe *txgbe)
> return ret;
> }
>
> + ret = txgbe_gpio_init(txgbe);
> + if (ret) {
> + wx_err(txgbe->wx, "failed to init gpio\n");
> + goto err_unregister_swnode;
> + }
> +
> ret = txgbe_clock_register(txgbe);
> if (ret) {
> wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> index fc91e0fc37a6..6c903e4517c7 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> @@ -55,6 +55,28 @@
> #define TXGBE_TS_CTL 0x10300
> #define TXGBE_TS_CTL_EVAL_MD BIT(31)
>
> +/* GPIO register bit */
> +#define TXGBE_GPIOBIT_0 BIT(0) /* I:tx fault */
> +#define TXGBE_GPIOBIT_1 BIT(1) /* O:tx disabled */
> +#define TXGBE_GPIOBIT_2 BIT(2) /* I:sfp module absent */
> +#define TXGBE_GPIOBIT_3 BIT(3) /* I:rx signal lost */
> +#define TXGBE_GPIOBIT_4 BIT(4) /* O:rate select, 1G(0) 10G(1) */
> +#define TXGBE_GPIOBIT_5 BIT(5) /* O:rate select, 1G(0) 10G(1) */
> +
> +/* Extended Interrupt Enable Set */
> +#define TXGBE_PX_MISC_ETH_LKDN BIT(8)
> +#define TXGBE_PX_MISC_DEV_RST BIT(10)
> +#define TXGBE_PX_MISC_ETH_EVENT BIT(17)
> +#define TXGBE_PX_MISC_ETH_LK BIT(18)
> +#define TXGBE_PX_MISC_ETH_AN BIT(19)
> +#define TXGBE_PX_MISC_INT_ERR BIT(20)
> +#define TXGBE_PX_MISC_GPIO BIT(26)
> +#define TXGBE_PX_MISC_IEN_MASK \
> + (TXGBE_PX_MISC_ETH_LKDN | TXGBE_PX_MISC_DEV_RST | \
> + TXGBE_PX_MISC_ETH_EVENT | TXGBE_PX_MISC_ETH_LK | \
> + TXGBE_PX_MISC_ETH_AN | TXGBE_PX_MISC_INT_ERR | \
> + TXGBE_PX_MISC_GPIO)
> +
> /* I2C registers */
> #define TXGBE_I2C_BASE 0x14900
>
> @@ -153,6 +175,7 @@ struct txgbe {
> struct platform_device *i2c_dev;
> struct clk_lookup *clock;
> struct clk *clk;
> + struct gpio_chip *gpio;
> };
>
> #endif /* _TXGBE_TYPE_H_ */
> --
> 2.27.0
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists