* [PATCH v3 0/2] gpio: mlxbf2: Introduce proper interrupt handling @ 2021-09-23 20:18 Asmaa Mnebhi 2021-09-23 20:18 ` [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support Asmaa Mnebhi 2021-09-23 20:18 ` [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling Asmaa Mnebhi 0 siblings, 2 replies; 5+ messages in thread From: Asmaa Mnebhi @ 2021-09-23 20:18 UTC (permalink / raw) To: andy.shevchenko, linux-gpio, netdev, linux-kernel, linux-acpi Cc: Asmaa Mnebhi, andrew, kuba, linus.walleij, bgolaszewski, davem, rjw, davthompson This is a follow up on a discussion regarding proper handling of GPIO interrupts within the gpio-mlxbf2.c driver. Link to discussion: https://lore.kernel.org/netdev/20210816115953.72533-7-andriy.shevchenko@linux.intel.com/T/ Patch 1 adds support to a GPIO IRQ handler in gpio-mlxbf2.c. Patch 2 is a follow up removal of custom GPIO IRQ handling from the mlxbf_gige driver and replacing it with a simple IRQ request. The ACPI table for the mlxbf_gige driver is responsible for instantiating the PHY GPIO interrupt via GpioInt. Andy Shevchenko, could you please review this patch series. v3 vs. v2 patch: - Add IRQ_TYPE_LEVEL* back to mlxbf2_gpio_irq_set_type. YU_GPIO_CAUSE_FALL_EN and YU_GPIO_CAUSE_RISE_EN are configured in Both level and edge interrupts cases. Asmaa Mnebhi (2): gpio: mlxbf2: Introduce IRQ support net: mellanox: mlxbf_gige: Replace non-standard interrupt handling drivers/gpio/gpio-mlxbf2.c | 150 ++++++++++++- .../net/ethernet/mellanox/mlxbf_gige/Makefile | 1 - .../ethernet/mellanox/mlxbf_gige/mlxbf_gige.h | 12 - .../mellanox/mlxbf_gige/mlxbf_gige_gpio.c | 212 ------------------ .../mellanox/mlxbf_gige/mlxbf_gige_main.c | 22 +- 5 files changed, 157 insertions(+), 240 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c -- 2.30.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support 2021-09-23 20:18 [PATCH v3 0/2] gpio: mlxbf2: Introduce proper interrupt handling Asmaa Mnebhi @ 2021-09-23 20:18 ` Asmaa Mnebhi 2021-09-23 20:18 ` [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling Asmaa Mnebhi 1 sibling, 0 replies; 5+ messages in thread From: Asmaa Mnebhi @ 2021-09-23 20:18 UTC (permalink / raw) To: andy.shevchenko, linux-gpio, netdev, linux-kernel, linux-acpi Cc: Asmaa Mnebhi, andrew, kuba, linus.walleij, bgolaszewski, davem, rjw, davthompson Introduce standard IRQ handling in the gpio-mlxbf2.c driver. Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> --- drivers/gpio/gpio-mlxbf2.c | 150 ++++++++++++++++++++++++++++++++++++- 1 file changed, 148 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c index 177d03ef4529..21c53d18ecd3 100644 --- a/drivers/gpio/gpio-mlxbf2.c +++ b/drivers/gpio/gpio-mlxbf2.c @@ -1,9 +1,14 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020-2021 NVIDIA CORPORATION & AFFILIATES + */ + #include <linux/bitfield.h> #include <linux/bitops.h> #include <linux/device.h> #include <linux/gpio/driver.h> +#include <linux/interrupt.h> #include <linux/io.h> #include <linux/ioport.h> #include <linux/kernel.h> @@ -43,9 +48,14 @@ #define YU_GPIO_MODE0 0x0c #define YU_GPIO_DATASET 0x14 #define YU_GPIO_DATACLEAR 0x18 +#define YU_GPIO_CAUSE_RISE_EN 0x44 +#define YU_GPIO_CAUSE_FALL_EN 0x48 #define YU_GPIO_MODE1_CLEAR 0x50 #define YU_GPIO_MODE0_SET 0x54 #define YU_GPIO_MODE0_CLEAR 0x58 +#define YU_GPIO_CAUSE_OR_CAUSE_EVTEN0 0x80 +#define YU_GPIO_CAUSE_OR_EVTEN0 0x94 +#define YU_GPIO_CAUSE_OR_CLRCAUSE 0x98 struct mlxbf2_gpio_context_save_regs { u32 gpio_mode0; @@ -55,6 +65,7 @@ struct mlxbf2_gpio_context_save_regs { /* BlueField-2 gpio block context structure. */ struct mlxbf2_gpio_context { struct gpio_chip gc; + struct irq_chip irq_chip; /* YU GPIO blocks address */ void __iomem *gpio_io; @@ -218,15 +229,117 @@ static int mlxbf2_gpio_direction_output(struct gpio_chip *chip, return ret; } +static void mlxbf2_gpio_irq_enable(struct irq_data *irqd) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc); + int offset = irqd_to_hwirq(irqd); + unsigned long flags; + u32 val; + + spin_lock_irqsave(&gs->gc.bgpio_lock, flags); + val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE); + val |= BIT(offset); + writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE); + + val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0); + val |= BIT(offset); + writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0); + spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags); +} + +static void mlxbf2_gpio_irq_disable(struct irq_data *irqd) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc); + int offset = irqd_to_hwirq(irqd); + unsigned long flags; + u32 val; + + spin_lock_irqsave(&gs->gc.bgpio_lock, flags); + val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0); + val &= ~BIT(offset); + writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0); + spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags); +} + +static irqreturn_t mlxbf2_gpio_irq_handler(int irq, void *ptr) +{ + struct mlxbf2_gpio_context *gs = ptr; + struct gpio_chip *gc = &gs->gc; + unsigned long pending; + u32 level; + + pending = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_CAUSE_EVTEN0); + writel(pending, gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE); + + for_each_set_bit(level, &pending, gc->ngpio) { + int gpio_irq = irq_find_mapping(gc->irq.domain, level); + generic_handle_irq(gpio_irq); + } + + return IRQ_RETVAL(pending); +} + +static int +mlxbf2_gpio_irq_set_type(struct irq_data *irqd, unsigned int type) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc); + int offset = irqd_to_hwirq(irqd); + unsigned long flags; + bool fall = false; + bool rise = false; + u32 val; + + switch (type & IRQ_TYPE_SENSE_MASK) { + case IRQ_TYPE_EDGE_BOTH: + case IRQ_TYPE_LEVEL_MASK: + fall = true; + rise = true; + break; + case IRQ_TYPE_EDGE_RISING: + case IRQ_TYPE_LEVEL_HIGH: + rise = true; + break; + case IRQ_TYPE_EDGE_FALLING: + case IRQ_TYPE_LEVEL_LOW: + fall = true; + break; + default: + return -EINVAL; + } + + spin_lock_irqsave(&gs->gc.bgpio_lock, flags); + if (fall) { + val = readl(gs->gpio_io + YU_GPIO_CAUSE_FALL_EN); + val |= BIT(offset); + writel(val, gs->gpio_io + YU_GPIO_CAUSE_FALL_EN); + } + + if (rise) { + val = readl(gs->gpio_io + YU_GPIO_CAUSE_RISE_EN); + val |= BIT(offset); + writel(val, gs->gpio_io + YU_GPIO_CAUSE_RISE_EN); + } + spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags); + + return 0; +} + /* BlueField-2 GPIO driver initialization routine. */ static int mlxbf2_gpio_probe(struct platform_device *pdev) { struct mlxbf2_gpio_context *gs; struct device *dev = &pdev->dev; + struct gpio_irq_chip *girq; struct gpio_chip *gc; unsigned int npins; - int ret; + const char *name; + int ret, irq; + + name = dev_name(dev); gs = devm_kzalloc(dev, sizeof(*gs), GFP_KERNEL); if (!gs) @@ -256,11 +369,44 @@ mlxbf2_gpio_probe(struct platform_device *pdev) NULL, 0); + if (ret) { + dev_err(dev, "bgpio_init failed\n"); + return ret; + } + gc->direction_input = mlxbf2_gpio_direction_input; gc->direction_output = mlxbf2_gpio_direction_output; gc->ngpio = npins; gc->owner = THIS_MODULE; + irq = platform_get_irq(pdev, 0); + if (irq >= 0) { + gs->irq_chip.name = name; + gs->irq_chip.irq_set_type = mlxbf2_gpio_irq_set_type; + gs->irq_chip.irq_enable = mlxbf2_gpio_irq_enable; + gs->irq_chip.irq_disable = mlxbf2_gpio_irq_disable; + + girq = &gs->gc.irq; + girq->chip = &gs->irq_chip; + girq->handler = handle_simple_irq; + girq->default_type = IRQ_TYPE_NONE; + /* This will let us handle the parent IRQ in the driver */ + girq->num_parents = 0; + girq->parents = NULL; + girq->parent_handler = NULL; + + /* + * Directly request the irq here instead of passing + * a flow-handler because the irq is shared. + */ + ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler, + IRQF_SHARED, name, gs); + if (ret) { + dev_err(dev, "failed to request IRQ"); + return ret; + } + } + platform_set_drvdata(pdev, gs); ret = devm_gpiochip_add_data(dev, &gs->gc, gs); @@ -315,5 +461,5 @@ static struct platform_driver mlxbf2_gpio_driver = { module_platform_driver(mlxbf2_gpio_driver); MODULE_DESCRIPTION("Mellanox BlueField-2 GPIO Driver"); -MODULE_AUTHOR("Mellanox Technologies"); +MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>"); MODULE_LICENSE("GPL v2"); -- 2.30.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling 2021-09-23 20:18 [PATCH v3 0/2] gpio: mlxbf2: Introduce proper interrupt handling Asmaa Mnebhi 2021-09-23 20:18 ` [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support Asmaa Mnebhi @ 2021-09-23 20:18 ` Asmaa Mnebhi 1 sibling, 0 replies; 5+ messages in thread From: Asmaa Mnebhi @ 2021-09-23 20:18 UTC (permalink / raw) To: andy.shevchenko, linux-gpio, netdev, linux-kernel, linux-acpi Cc: Asmaa Mnebhi, andrew, kuba, linus.walleij, bgolaszewski, davem, rjw, davthompson Since the GPIO driver (gpio-mlxbf2.c) supports interrupt handling, replace the custom routine with simple IRQ request. Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> --- .../net/ethernet/mellanox/mlxbf_gige/Makefile | 1 - .../ethernet/mellanox/mlxbf_gige/mlxbf_gige.h | 12 - .../mellanox/mlxbf_gige/mlxbf_gige_gpio.c | 212 ------------------ .../mellanox/mlxbf_gige/mlxbf_gige_main.c | 22 +- 4 files changed, 9 insertions(+), 238 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile b/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile index e57c1375f236..a97c2bef846b 100644 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile @@ -3,7 +3,6 @@ obj-$(CONFIG_MLXBF_GIGE) += mlxbf_gige.o mlxbf_gige-y := mlxbf_gige_ethtool.o \ - mlxbf_gige_gpio.o \ mlxbf_gige_intr.o \ mlxbf_gige_main.o \ mlxbf_gige_mdio.o \ diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h index e3509e69ed1c..86826a70f9dd 100644 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h @@ -51,11 +51,6 @@ #define MLXBF_GIGE_ERROR_INTR_IDX 0 #define MLXBF_GIGE_RECEIVE_PKT_INTR_IDX 1 #define MLXBF_GIGE_LLU_PLU_INTR_IDX 2 -#define MLXBF_GIGE_PHY_INT_N 3 - -#define MLXBF_GIGE_MDIO_DEFAULT_PHY_ADDR 0x3 - -#define MLXBF_GIGE_DEFAULT_PHY_INT_GPIO 12 struct mlxbf_gige_stats { u64 hw_access_errors; @@ -81,11 +76,7 @@ struct mlxbf_gige { struct platform_device *pdev; void __iomem *mdio_io; struct mii_bus *mdiobus; - void __iomem *gpio_io; - struct irq_domain *irqdomain; - u32 phy_int_gpio_mask; spinlock_t lock; /* for packet processing indices */ - spinlock_t gpio_lock; /* for GPIO bus access */ u16 rx_q_entries; u16 tx_q_entries; u64 *tx_wqe_base; @@ -184,7 +175,4 @@ int mlxbf_gige_poll(struct napi_struct *napi, int budget); extern const struct ethtool_ops mlxbf_gige_ethtool_ops; void mlxbf_gige_update_tx_wqe_next(struct mlxbf_gige *priv); -int mlxbf_gige_gpio_init(struct platform_device *pdev, struct mlxbf_gige *priv); -void mlxbf_gige_gpio_free(struct mlxbf_gige *priv); - #endif /* !defined(__MLXBF_GIGE_H__) */ diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c deleted file mode 100644 index a8d966db5715..000000000000 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c +++ /dev/null @@ -1,212 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause - -/* Initialize and handle GPIO interrupt triggered by INT_N PHY signal. - * This GPIO interrupt triggers the PHY state machine to bring the link - * up/down. - * - * Copyright (C) 2021 NVIDIA CORPORATION & AFFILIATES - */ - -#include <linux/acpi.h> -#include <linux/bitfield.h> -#include <linux/device.h> -#include <linux/err.h> -#include <linux/gpio/driver.h> -#include <linux/interrupt.h> -#include <linux/io.h> -#include <linux/irq.h> -#include <linux/irqdomain.h> -#include <linux/irqreturn.h> -#include <linux/platform_device.h> -#include <linux/property.h> - -#include "mlxbf_gige.h" -#include "mlxbf_gige_regs.h" - -#define MLXBF_GIGE_GPIO_CAUSE_FALL_EN 0x48 -#define MLXBF_GIGE_GPIO_CAUSE_OR_CAUSE_EVTEN0 0x80 -#define MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0 0x94 -#define MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE 0x98 - -static void mlxbf_gige_gpio_enable(struct mlxbf_gige *priv) -{ - unsigned long flags; - u32 val; - - spin_lock_irqsave(&priv->gpio_lock, flags); - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - - /* The INT_N interrupt level is active low. - * So enable cause fall bit to detect when GPIO - * state goes low. - */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_FALL_EN); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_FALL_EN); - - /* Enable PHY interrupt by setting the priority level */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - spin_unlock_irqrestore(&priv->gpio_lock, flags); -} - -static void mlxbf_gige_gpio_disable(struct mlxbf_gige *priv) -{ - unsigned long flags; - u32 val; - - spin_lock_irqsave(&priv->gpio_lock, flags); - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - val &= ~priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - spin_unlock_irqrestore(&priv->gpio_lock, flags); -} - -static irqreturn_t mlxbf_gige_gpio_handler(int irq, void *ptr) -{ - struct mlxbf_gige *priv; - u32 val; - - priv = ptr; - - /* Check if this interrupt is from PHY device. - * Return if it is not. - */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CAUSE_EVTEN0); - if (!(val & priv->phy_int_gpio_mask)) - return IRQ_NONE; - - /* Clear interrupt when done, otherwise, no further interrupt - * will be triggered. - */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - - generic_handle_irq(priv->phy_irq); - - return IRQ_HANDLED; -} - -static void mlxbf_gige_gpio_mask(struct irq_data *irqd) -{ - struct mlxbf_gige *priv = irq_data_get_irq_chip_data(irqd); - - mlxbf_gige_gpio_disable(priv); -} - -static void mlxbf_gige_gpio_unmask(struct irq_data *irqd) -{ - struct mlxbf_gige *priv = irq_data_get_irq_chip_data(irqd); - - mlxbf_gige_gpio_enable(priv); -} - -static struct irq_chip mlxbf_gige_gpio_chip = { - .name = "mlxbf_gige_phy", - .irq_mask = mlxbf_gige_gpio_mask, - .irq_unmask = mlxbf_gige_gpio_unmask, -}; - -static int mlxbf_gige_gpio_domain_map(struct irq_domain *d, - unsigned int irq, - irq_hw_number_t hwirq) -{ - irq_set_chip_data(irq, d->host_data); - irq_set_chip_and_handler(irq, &mlxbf_gige_gpio_chip, handle_simple_irq); - irq_set_noprobe(irq); - - return 0; -} - -static const struct irq_domain_ops mlxbf_gige_gpio_domain_ops = { - .map = mlxbf_gige_gpio_domain_map, - .xlate = irq_domain_xlate_twocell, -}; - -#ifdef CONFIG_ACPI -static int mlxbf_gige_gpio_resources(struct acpi_resource *ares, - void *data) -{ - struct acpi_resource_gpio *gpio; - u32 *phy_int_gpio = data; - - if (ares->type == ACPI_RESOURCE_TYPE_GPIO) { - gpio = &ares->data.gpio; - *phy_int_gpio = gpio->pin_table[0]; - } - - return 1; -} -#endif - -void mlxbf_gige_gpio_free(struct mlxbf_gige *priv) -{ - irq_dispose_mapping(priv->phy_irq); - irq_domain_remove(priv->irqdomain); -} - -int mlxbf_gige_gpio_init(struct platform_device *pdev, - struct mlxbf_gige *priv) -{ - struct device *dev = &pdev->dev; - struct resource *res; - u32 phy_int_gpio = 0; - int ret; - - LIST_HEAD(resources); - - res = platform_get_resource(pdev, IORESOURCE_MEM, MLXBF_GIGE_RES_GPIO0); - if (!res) - return -ENODEV; - - priv->gpio_io = devm_ioremap(dev, res->start, resource_size(res)); - if (!priv->gpio_io) - return -ENOMEM; - -#ifdef CONFIG_ACPI - ret = acpi_dev_get_resources(ACPI_COMPANION(dev), - &resources, mlxbf_gige_gpio_resources, - &phy_int_gpio); - acpi_dev_free_resource_list(&resources); - if (ret < 0 || !phy_int_gpio) { - dev_err(dev, "Error retrieving the gpio phy pin"); - return -EINVAL; - } -#endif - - priv->phy_int_gpio_mask = BIT(phy_int_gpio); - - mlxbf_gige_gpio_disable(priv); - - priv->hw_phy_irq = platform_get_irq(pdev, MLXBF_GIGE_PHY_INT_N); - - priv->irqdomain = irq_domain_add_simple(NULL, 1, 0, - &mlxbf_gige_gpio_domain_ops, - priv); - if (!priv->irqdomain) { - dev_err(dev, "Failed to add IRQ domain\n"); - return -ENOMEM; - } - - priv->phy_irq = irq_create_mapping(priv->irqdomain, 0); - if (!priv->phy_irq) { - irq_domain_remove(priv->irqdomain); - priv->irqdomain = NULL; - dev_err(dev, "Error mapping PHY IRQ\n"); - return -EINVAL; - } - - ret = devm_request_irq(dev, priv->hw_phy_irq, mlxbf_gige_gpio_handler, - IRQF_ONESHOT | IRQF_SHARED, "mlxbf_gige_phy", priv); - if (ret) { - dev_err(dev, "Failed to request PHY IRQ"); - mlxbf_gige_gpio_free(priv); - return ret; - } - - return ret; -} diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c index 3e85b17f5857..4382ec8f7d64 100644 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c @@ -273,8 +273,8 @@ static int mlxbf_gige_probe(struct platform_device *pdev) void __iomem *llu_base; void __iomem *plu_base; void __iomem *base; + int addr, phy_irq; u64 control; - int addr; int err; base = devm_platform_ioremap_resource(pdev, MLXBF_GIGE_RES_MAC); @@ -309,20 +309,12 @@ static int mlxbf_gige_probe(struct platform_device *pdev) priv->pdev = pdev; spin_lock_init(&priv->lock); - spin_lock_init(&priv->gpio_lock); /* Attach MDIO device */ err = mlxbf_gige_mdio_probe(pdev, priv); if (err) return err; - err = mlxbf_gige_gpio_init(pdev, priv); - if (err) { - dev_err(&pdev->dev, "PHY IRQ initialization failed\n"); - mlxbf_gige_mdio_remove(priv); - return -ENODEV; - } - priv->base = base; priv->llu_base = llu_base; priv->plu_base = plu_base; @@ -343,6 +335,12 @@ static int mlxbf_gige_probe(struct platform_device *pdev) priv->rx_irq = platform_get_irq(pdev, MLXBF_GIGE_RECEIVE_PKT_INTR_IDX); priv->llu_plu_irq = platform_get_irq(pdev, MLXBF_GIGE_LLU_PLU_INTR_IDX); + phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev), "phy-gpios", 0); + if (phy_irq < 0) { + dev_err(&pdev->dev, "Error getting PHY irq. Use polling instead"); + phy_irq = PHY_POLL; + } + phydev = phy_find_first(priv->mdiobus); if (!phydev) { err = -ENODEV; @@ -350,8 +348,8 @@ static int mlxbf_gige_probe(struct platform_device *pdev) } addr = phydev->mdio.addr; - priv->mdiobus->irq[addr] = priv->phy_irq; - phydev->irq = priv->phy_irq; + priv->mdiobus->irq[addr] = phy_irq; + phydev->irq = phy_irq; err = phy_connect_direct(netdev, phydev, mlxbf_gige_adjust_link, @@ -387,7 +385,6 @@ static int mlxbf_gige_probe(struct platform_device *pdev) return 0; out: - mlxbf_gige_gpio_free(priv); mlxbf_gige_mdio_remove(priv); return err; } @@ -398,7 +395,6 @@ static int mlxbf_gige_remove(struct platform_device *pdev) unregister_netdev(priv->netdev); phy_disconnect(priv->netdev->phydev); - mlxbf_gige_gpio_free(priv); mlxbf_gige_mdio_remove(priv); platform_set_drvdata(pdev, NULL); -- 2.30.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 0/2] gpio: mlxbf2: Introduce proper interrupt handling @ 2021-09-20 21:22 Asmaa Mnebhi 2021-09-20 21:22 ` [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard " Asmaa Mnebhi 0 siblings, 1 reply; 5+ messages in thread From: Asmaa Mnebhi @ 2021-09-20 21:22 UTC (permalink / raw) To: andy.shevchenko, linux-gpio, netdev, linux-kernel, linux-acpi Cc: Asmaa Mnebhi, andrew, kuba, linus.walleij, bgolaszewski, davem, rjw, davthompson This is a follow up on a discussion regarding proper handling of GPIO interrupts within the gpio-mlxbf2.c driver. Link to discussion: https://lore.kernel.org/netdev/20210816115953.72533-7-andriy.shevchenko@linux.intel.com/T/ Patch 1 adds support to a GPIO IRQ handler in gpio-mlxbf2.c. Patch 2 is a follow up removal of custom GPIO IRQ handling from the mlxbf_gige driver and replacing it with a simple IRQ request. The ACPI table for the mlxbf_gige driver is responsible for instantiating the PHY GPIO interrupt via GpioInt. Andy Shevchenko, could you please review this patch series. v2 vs. v1 patch: All changes were made in gpio-mlxbf2.c - removed unnecessary comments - removed irq_ack as it is not needed - removed irq_mask/irq_unmask as they are not invoked - remove IRQ_TYPE_LEVEL* from mlxbf2_gpio_irq_set_type since they are not supported Asmaa Mnebhi (2): gpio: mlxbf2: Introduce IRQ support net: mellanox: mlxbf_gige: Replace non-standard interrupt handling drivers/gpio/gpio-mlxbf2.c | 147 +++++++++++- .../net/ethernet/mellanox/mlxbf_gige/Makefile | 1 - .../ethernet/mellanox/mlxbf_gige/mlxbf_gige.h | 12 - .../mellanox/mlxbf_gige/mlxbf_gige_gpio.c | 212 ------------------ .../mellanox/mlxbf_gige/mlxbf_gige_main.c | 22 +- 5 files changed, 154 insertions(+), 240 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c -- 2.30.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling 2021-09-20 21:22 [PATCH v2 0/2] gpio: mlxbf2: Introduce proper " Asmaa Mnebhi @ 2021-09-20 21:22 ` Asmaa Mnebhi 2021-09-21 22:29 ` Andrew Lunn 0 siblings, 1 reply; 5+ messages in thread From: Asmaa Mnebhi @ 2021-09-20 21:22 UTC (permalink / raw) To: andy.shevchenko, linux-gpio, netdev, linux-kernel, linux-acpi Cc: Asmaa Mnebhi, andrew, kuba, linus.walleij, bgolaszewski, davem, rjw, davthompson Since the GPIO driver (gpio-mlxbf2.c) supports interrupt handling, replace the custom routine with simple IRQ request. Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> --- .../net/ethernet/mellanox/mlxbf_gige/Makefile | 1 - .../ethernet/mellanox/mlxbf_gige/mlxbf_gige.h | 12 - .../mellanox/mlxbf_gige/mlxbf_gige_gpio.c | 212 ------------------ .../mellanox/mlxbf_gige/mlxbf_gige_main.c | 22 +- 4 files changed, 9 insertions(+), 238 deletions(-) delete mode 100644 drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile b/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile index e57c1375f236..a97c2bef846b 100644 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/Makefile @@ -3,7 +3,6 @@ obj-$(CONFIG_MLXBF_GIGE) += mlxbf_gige.o mlxbf_gige-y := mlxbf_gige_ethtool.o \ - mlxbf_gige_gpio.o \ mlxbf_gige_intr.o \ mlxbf_gige_main.o \ mlxbf_gige_mdio.o \ diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h index e3509e69ed1c..86826a70f9dd 100644 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h @@ -51,11 +51,6 @@ #define MLXBF_GIGE_ERROR_INTR_IDX 0 #define MLXBF_GIGE_RECEIVE_PKT_INTR_IDX 1 #define MLXBF_GIGE_LLU_PLU_INTR_IDX 2 -#define MLXBF_GIGE_PHY_INT_N 3 - -#define MLXBF_GIGE_MDIO_DEFAULT_PHY_ADDR 0x3 - -#define MLXBF_GIGE_DEFAULT_PHY_INT_GPIO 12 struct mlxbf_gige_stats { u64 hw_access_errors; @@ -81,11 +76,7 @@ struct mlxbf_gige { struct platform_device *pdev; void __iomem *mdio_io; struct mii_bus *mdiobus; - void __iomem *gpio_io; - struct irq_domain *irqdomain; - u32 phy_int_gpio_mask; spinlock_t lock; /* for packet processing indices */ - spinlock_t gpio_lock; /* for GPIO bus access */ u16 rx_q_entries; u16 tx_q_entries; u64 *tx_wqe_base; @@ -184,7 +175,4 @@ int mlxbf_gige_poll(struct napi_struct *napi, int budget); extern const struct ethtool_ops mlxbf_gige_ethtool_ops; void mlxbf_gige_update_tx_wqe_next(struct mlxbf_gige *priv); -int mlxbf_gige_gpio_init(struct platform_device *pdev, struct mlxbf_gige *priv); -void mlxbf_gige_gpio_free(struct mlxbf_gige *priv); - #endif /* !defined(__MLXBF_GIGE_H__) */ diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c deleted file mode 100644 index a8d966db5715..000000000000 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_gpio.c +++ /dev/null @@ -1,212 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause - -/* Initialize and handle GPIO interrupt triggered by INT_N PHY signal. - * This GPIO interrupt triggers the PHY state machine to bring the link - * up/down. - * - * Copyright (C) 2021 NVIDIA CORPORATION & AFFILIATES - */ - -#include <linux/acpi.h> -#include <linux/bitfield.h> -#include <linux/device.h> -#include <linux/err.h> -#include <linux/gpio/driver.h> -#include <linux/interrupt.h> -#include <linux/io.h> -#include <linux/irq.h> -#include <linux/irqdomain.h> -#include <linux/irqreturn.h> -#include <linux/platform_device.h> -#include <linux/property.h> - -#include "mlxbf_gige.h" -#include "mlxbf_gige_regs.h" - -#define MLXBF_GIGE_GPIO_CAUSE_FALL_EN 0x48 -#define MLXBF_GIGE_GPIO_CAUSE_OR_CAUSE_EVTEN0 0x80 -#define MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0 0x94 -#define MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE 0x98 - -static void mlxbf_gige_gpio_enable(struct mlxbf_gige *priv) -{ - unsigned long flags; - u32 val; - - spin_lock_irqsave(&priv->gpio_lock, flags); - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - - /* The INT_N interrupt level is active low. - * So enable cause fall bit to detect when GPIO - * state goes low. - */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_FALL_EN); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_FALL_EN); - - /* Enable PHY interrupt by setting the priority level */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - spin_unlock_irqrestore(&priv->gpio_lock, flags); -} - -static void mlxbf_gige_gpio_disable(struct mlxbf_gige *priv) -{ - unsigned long flags; - u32 val; - - spin_lock_irqsave(&priv->gpio_lock, flags); - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - val &= ~priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_EVTEN0); - spin_unlock_irqrestore(&priv->gpio_lock, flags); -} - -static irqreturn_t mlxbf_gige_gpio_handler(int irq, void *ptr) -{ - struct mlxbf_gige *priv; - u32 val; - - priv = ptr; - - /* Check if this interrupt is from PHY device. - * Return if it is not. - */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CAUSE_EVTEN0); - if (!(val & priv->phy_int_gpio_mask)) - return IRQ_NONE; - - /* Clear interrupt when done, otherwise, no further interrupt - * will be triggered. - */ - val = readl(priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - val |= priv->phy_int_gpio_mask; - writel(val, priv->gpio_io + MLXBF_GIGE_GPIO_CAUSE_OR_CLRCAUSE); - - generic_handle_irq(priv->phy_irq); - - return IRQ_HANDLED; -} - -static void mlxbf_gige_gpio_mask(struct irq_data *irqd) -{ - struct mlxbf_gige *priv = irq_data_get_irq_chip_data(irqd); - - mlxbf_gige_gpio_disable(priv); -} - -static void mlxbf_gige_gpio_unmask(struct irq_data *irqd) -{ - struct mlxbf_gige *priv = irq_data_get_irq_chip_data(irqd); - - mlxbf_gige_gpio_enable(priv); -} - -static struct irq_chip mlxbf_gige_gpio_chip = { - .name = "mlxbf_gige_phy", - .irq_mask = mlxbf_gige_gpio_mask, - .irq_unmask = mlxbf_gige_gpio_unmask, -}; - -static int mlxbf_gige_gpio_domain_map(struct irq_domain *d, - unsigned int irq, - irq_hw_number_t hwirq) -{ - irq_set_chip_data(irq, d->host_data); - irq_set_chip_and_handler(irq, &mlxbf_gige_gpio_chip, handle_simple_irq); - irq_set_noprobe(irq); - - return 0; -} - -static const struct irq_domain_ops mlxbf_gige_gpio_domain_ops = { - .map = mlxbf_gige_gpio_domain_map, - .xlate = irq_domain_xlate_twocell, -}; - -#ifdef CONFIG_ACPI -static int mlxbf_gige_gpio_resources(struct acpi_resource *ares, - void *data) -{ - struct acpi_resource_gpio *gpio; - u32 *phy_int_gpio = data; - - if (ares->type == ACPI_RESOURCE_TYPE_GPIO) { - gpio = &ares->data.gpio; - *phy_int_gpio = gpio->pin_table[0]; - } - - return 1; -} -#endif - -void mlxbf_gige_gpio_free(struct mlxbf_gige *priv) -{ - irq_dispose_mapping(priv->phy_irq); - irq_domain_remove(priv->irqdomain); -} - -int mlxbf_gige_gpio_init(struct platform_device *pdev, - struct mlxbf_gige *priv) -{ - struct device *dev = &pdev->dev; - struct resource *res; - u32 phy_int_gpio = 0; - int ret; - - LIST_HEAD(resources); - - res = platform_get_resource(pdev, IORESOURCE_MEM, MLXBF_GIGE_RES_GPIO0); - if (!res) - return -ENODEV; - - priv->gpio_io = devm_ioremap(dev, res->start, resource_size(res)); - if (!priv->gpio_io) - return -ENOMEM; - -#ifdef CONFIG_ACPI - ret = acpi_dev_get_resources(ACPI_COMPANION(dev), - &resources, mlxbf_gige_gpio_resources, - &phy_int_gpio); - acpi_dev_free_resource_list(&resources); - if (ret < 0 || !phy_int_gpio) { - dev_err(dev, "Error retrieving the gpio phy pin"); - return -EINVAL; - } -#endif - - priv->phy_int_gpio_mask = BIT(phy_int_gpio); - - mlxbf_gige_gpio_disable(priv); - - priv->hw_phy_irq = platform_get_irq(pdev, MLXBF_GIGE_PHY_INT_N); - - priv->irqdomain = irq_domain_add_simple(NULL, 1, 0, - &mlxbf_gige_gpio_domain_ops, - priv); - if (!priv->irqdomain) { - dev_err(dev, "Failed to add IRQ domain\n"); - return -ENOMEM; - } - - priv->phy_irq = irq_create_mapping(priv->irqdomain, 0); - if (!priv->phy_irq) { - irq_domain_remove(priv->irqdomain); - priv->irqdomain = NULL; - dev_err(dev, "Error mapping PHY IRQ\n"); - return -EINVAL; - } - - ret = devm_request_irq(dev, priv->hw_phy_irq, mlxbf_gige_gpio_handler, - IRQF_ONESHOT | IRQF_SHARED, "mlxbf_gige_phy", priv); - if (ret) { - dev_err(dev, "Failed to request PHY IRQ"); - mlxbf_gige_gpio_free(priv); - return ret; - } - - return ret; -} diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c index 3e85b17f5857..4382ec8f7d64 100644 --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c @@ -273,8 +273,8 @@ static int mlxbf_gige_probe(struct platform_device *pdev) void __iomem *llu_base; void __iomem *plu_base; void __iomem *base; + int addr, phy_irq; u64 control; - int addr; int err; base = devm_platform_ioremap_resource(pdev, MLXBF_GIGE_RES_MAC); @@ -309,20 +309,12 @@ static int mlxbf_gige_probe(struct platform_device *pdev) priv->pdev = pdev; spin_lock_init(&priv->lock); - spin_lock_init(&priv->gpio_lock); /* Attach MDIO device */ err = mlxbf_gige_mdio_probe(pdev, priv); if (err) return err; - err = mlxbf_gige_gpio_init(pdev, priv); - if (err) { - dev_err(&pdev->dev, "PHY IRQ initialization failed\n"); - mlxbf_gige_mdio_remove(priv); - return -ENODEV; - } - priv->base = base; priv->llu_base = llu_base; priv->plu_base = plu_base; @@ -343,6 +335,12 @@ static int mlxbf_gige_probe(struct platform_device *pdev) priv->rx_irq = platform_get_irq(pdev, MLXBF_GIGE_RECEIVE_PKT_INTR_IDX); priv->llu_plu_irq = platform_get_irq(pdev, MLXBF_GIGE_LLU_PLU_INTR_IDX); + phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev), "phy-gpios", 0); + if (phy_irq < 0) { + dev_err(&pdev->dev, "Error getting PHY irq. Use polling instead"); + phy_irq = PHY_POLL; + } + phydev = phy_find_first(priv->mdiobus); if (!phydev) { err = -ENODEV; @@ -350,8 +348,8 @@ static int mlxbf_gige_probe(struct platform_device *pdev) } addr = phydev->mdio.addr; - priv->mdiobus->irq[addr] = priv->phy_irq; - phydev->irq = priv->phy_irq; + priv->mdiobus->irq[addr] = phy_irq; + phydev->irq = phy_irq; err = phy_connect_direct(netdev, phydev, mlxbf_gige_adjust_link, @@ -387,7 +385,6 @@ static int mlxbf_gige_probe(struct platform_device *pdev) return 0; out: - mlxbf_gige_gpio_free(priv); mlxbf_gige_mdio_remove(priv); return err; } @@ -398,7 +395,6 @@ static int mlxbf_gige_remove(struct platform_device *pdev) unregister_netdev(priv->netdev); phy_disconnect(priv->netdev->phydev); - mlxbf_gige_gpio_free(priv); mlxbf_gige_mdio_remove(priv); platform_set_drvdata(pdev, NULL); -- 2.30.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling 2021-09-20 21:22 ` [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard " Asmaa Mnebhi @ 2021-09-21 22:29 ` Andrew Lunn 0 siblings, 0 replies; 5+ messages in thread From: Andrew Lunn @ 2021-09-21 22:29 UTC (permalink / raw) To: Asmaa Mnebhi Cc: andy.shevchenko, linux-gpio, netdev, linux-kernel, linux-acpi, kuba, linus.walleij, bgolaszewski, davem, rjw, davthompson On Mon, Sep 20, 2021 at 05:22:27PM -0400, Asmaa Mnebhi wrote: > Since the GPIO driver (gpio-mlxbf2.c) supports interrupt > handling, replace the custom routine with simple IRQ > request. > > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-23 20:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-23 20:18 [PATCH v3 0/2] gpio: mlxbf2: Introduce proper interrupt handling Asmaa Mnebhi 2021-09-23 20:18 ` [PATCH v3 1/2] gpio: mlxbf2: Introduce IRQ support Asmaa Mnebhi 2021-09-23 20:18 ` [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling Asmaa Mnebhi -- strict thread matches above, loose matches on Subject: below -- 2021-09-20 21:22 [PATCH v2 0/2] gpio: mlxbf2: Introduce proper " Asmaa Mnebhi 2021-09-20 21:22 ` [PATCH v2 2/2] net: mellanox: mlxbf_gige: Replace non-standard " Asmaa Mnebhi 2021-09-21 22:29 ` Andrew Lunn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.