Re: [PATCH RFC v6 7/7] mfd: ls2kbmc: Capture the reset event of BMC through GPIO
Bartosz Golaszewski <[email protected]>
| Newsgroups | org.kernel.vger.linux-gpio,dev.linux.lists.mfd,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAMRc=Me0DVNYfZJYesvpzTwkVzYZyVzNH+aOAM+DojfV_M9mww@mail.gmail.com> |
On Tue, 4 Aug 2026 21:14:25 +0200, Miao Wang via B4 Relay <[email protected]> said: > From: Miao Wang <[email protected]> > > The reset event of BMC is captured through GPIO. However, this driver > bypasses the GPIO framework and directly accesses the GPIO controller > through the fixed address. When the same GPIO controller is also > exposed through ACPI and probed by the corresponding GPIO driver, > there would be a conflict between the two drivers. > > This patch will try to find the GPIO through declared GPIO pin in the > _CRS resources of the ACPI node. If no such delaration is found, the > driver will fall back to search for the correct GPIO controller and pin > according to the fixed address and pin number. A possible DSDT > declaration for the GPIO pin might be as follows: > > Device (BMC0) { > Name (_ADR, ...) // Match the PCI address of the BMC device > // \_SB.GPO1 is the ACPI path of the GPIO controller > Name (_CRS, ResourceTemplate () { > GpioInt (Edge, ActiveLow, Exclusive, PullNone, 0, > "\\_SB.GPO1", 0) { > 14 // 14 is the GPIO pin number > } > } > > Signed-off-by: Miao Wang <[email protected]> > --- > drivers/mfd/ls2k-bmc-core.c | 163 +++++++++++++++++++++++++++++++------------- > 1 file changed, 116 insertions(+), 47 deletions(-) > > diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c > index e7bebfe696ac4554eb6283279811a27ed865ab73..97a666a136fe41ed4268cb70770201533aebb1d5 100644 > --- a/drivers/mfd/ls2k-bmc-core.c > +++ b/drivers/mfd/ls2k-bmc-core.c > @@ -26,6 +26,10 @@ > #include <linux/stop_machine.h> > #include <linux/vt_kern.h> > #include <linux/console.h> > +#include <linux/gpio/consumer.h> > +#include <linux/gpio/driver.h> > +#include <linux/gpio/property.h> > +#include <linux/gpio/machine.h> > > /* LS2K BMC resources */ > #define LS2K_DISPLAY_RES_START (SZ_16M + SZ_2M) > @@ -81,18 +85,6 @@ > > #define PCI_REG_STRIDE 0x4 > > -#define LS2K_BMC_RESET_GPIO 14 > -#define LOONGSON_GPIO_REG_BASE 0x1FE00500 > -#define LOONGSON_GPIO_REG_SIZE 0x18 > -#define LOONGSON_GPIO_OEN 0x0 > -#define LOONGSON_GPIO_FUNC 0x4 > -#define LOONGSON_GPIO_INTPOL 0x10 > -#define LOONGSON_GPIO_INTEN 0x14 > - > -#define LOONGSON_IO_INT_BASE 16 > -#define LS2K_BMC_RESET_GPIO_INT_VEC (LS2K_BMC_RESET_GPIO % 8) > -#define LS2K_BMC_RESET_GPIO_GSI (LOONGSON_IO_INT_BASE + LS2K_BMC_RESET_GPIO_INT_VEC) > - > enum { > LS2K_BMC_DISPLAY, > LS2K_BMC_IPMI0, > @@ -186,6 +178,7 @@ struct ls2k_bmc_ddata { > struct work_struct bmc_reset_work; > struct ls2k_bmc_pci_data bmc_pci_data; > struct ls2k_bmc_bridge_pci_data bridge_pci_data; > + struct gpio_desc *reset_gpio; > }; > > static bool ls2k_bmc_bar0_addr_is_set(struct pci_dev *pdev) > @@ -375,6 +368,91 @@ static void ls2k_bmc_save_pci_data(struct pci_dev *pdev, struct ls2k_bmc_ddata * > pci_read_config_dword(pdev, PCI_INTERRUPT_LINE, &ddata->bmc_pci_data.interrupt_line); > } > > +static struct fwnode_handle *gpio_dev_get_fwnode(struct gpio_device *gdev) > +{ > + struct device *gdev_dev; > + > + gdev_dev = gpio_device_to_device(gdev); > + return dev_fwnode(gdev_dev); You can just do a one-liner: return dev_fwnode(gpio_device_to_device(gdev)); > +} > + > +static int ls2k_bmc_gpiochip_find(struct gpio_chip *gc, const void *data) > +{ > + struct acpi_device *adev; > + struct list_head resource_list; > + struct resource_entry *rentry; > + struct fwnode_handle *fwnode = gpio_dev_get_fwnode(gc->gpiodev); > + phys_addr_t start_addr = (phys_addr_t) data; > + int ret, found = 0; > + > + if (!is_acpi_node(fwnode)) > + goto out; > + > + adev = to_acpi_device_node(fwnode); > + if (!adev) > + goto out; > + > + INIT_LIST_HEAD(&resource_list); > + > + ret = acpi_dev_get_memory_resources(adev, &resource_list); > + if (ret < 0) > + goto out; > + /* > + * ACPI memory resources are ordered and only the first one is > + * considered by the driver of the expected GPIO controller. So > + * here we also only check the first one to see if it matches the > + * expected address. > + */ > + rentry = list_first_entry_or_null(&resource_list, struct resource_entry, node); > + if (!rentry) > + goto free_resource_list; > + if (rentry->res->start == start_addr) > + found = 1; > + > +free_resource_list: > + acpi_dev_free_resource_list(&resource_list); > +out: > + return found; > +} > + > +static struct gpio_desc *ls2k_bmc_find_gpio(struct ls2k_bmc_ddata *ddata) > +{ > + /* > + * In conventional way, the GPIO should be obtained through ACPI or > + * device tree. However, when the information is not available, > + * we should find the GPIO according to the convention of the server > + * boards with LS2K BMC, the gpio signal reflecting the reset event > + * of the BMC should be connected to pin 14 of the GPIO input of > + * the first CPU node. The address of that GPIO controller is fixed. > + */ > + static const phys_addr_t LOONGSON_GPIO_REG_BASE = 0x1FE00500; > + static const unsigned int LS2K_BMC_RESET_GPIO = 14; > + int ret; > + struct gpio_device *gdev __free(gpio_device_put) = NULL; Please declare the variable where you initialize it when using __free(). > + struct property_entry ls2k_bmc_swnode_properties[2] = { 0 }; Prefer `= { };`. > + > + dev_dbg(ddata->dev, "Searching for GPIO chip at address %pa\n", &LOONGSON_GPIO_REG_BASE); > + > + gdev = gpio_device_find((void *)LOONGSON_GPIO_REG_BASE, ls2k_bmc_gpiochip_find); > + No need for a blank line here. > + if (!gdev) { > + dev_dbg(ddata->dev, "cannot find GPIO chip at address %pa, deferring\n", > + &LOONGSON_GPIO_REG_BASE); > + return ERR_PTR(-EPROBE_DEFER); > + } > + > + ls2k_bmc_swnode_properties[0] = PROPERTY_ENTRY_GPIO("gpio", > + gpio_dev_get_fwnode(gdev), LS2K_BMC_RESET_GPIO, GPIO_ACTIVE_HIGH); TBH this could become a GPIOLIB function as gpio_device_get_fwnode(). > + > + ret = device_create_managed_software_node(ddata->dev, ls2k_bmc_swnode_properties, NULL); > + if (ret) { > + dev_err(ddata->dev, "Failed to create software node for GPIO reset: %d\n", ret); > + return ERR_PTR(ret); return dev_err_probe()? This can only be called from probe(). > + } > + > + return devm_gpiod_get_index(ddata->dev, NULL, 0, GPIOD_IN); > +} > + > static void ls2k_bmc_cancel_wq(void *data) > { > struct ls2k_bmc_ddata *ddata = data; > @@ -384,8 +462,7 @@ static void ls2k_bmc_cancel_wq(void *data) > static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata) > { > struct pci_dev *pdev = to_pci_dev(ddata->dev); > - void __iomem *gpio_base; > - int gpio_irq, ret, val; > + int gpio_irq, ret; > > ls2k_bmc_save_pci_data(pdev, ddata); > > @@ -402,44 +479,36 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata) > return ret; > } > > - gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE); > - if (!gpio_base) > - return -ENOMEM; > - > - /* Disable GPIO output */ > - val = readl(gpio_base + LOONGSON_GPIO_OEN); > - writel(val | BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_OEN); > - > - /* Enable GPIO functionality */ > - val = readl(gpio_base + LOONGSON_GPIO_FUNC); > - writel(val & ~BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_FUNC); > - > - /* Set GPIO interrupts to low-level active */ > - val = readl(gpio_base + LOONGSON_GPIO_INTPOL); > - writel(val & ~BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_INTPOL); > - > - /* Enable GPIO interrupts */ > - val = readl(gpio_base + LOONGSON_GPIO_INTEN); > - writel(val | BIT(LS2K_BMC_RESET_GPIO), gpio_base + LOONGSON_GPIO_INTEN); > + ddata->reset_gpio = devm_gpiod_get_index_optional(&pdev->dev, NULL, 0, GPIOD_IN); > + if (IS_ERR(ddata->reset_gpio)) { > + ret = PTR_ERR(ddata->reset_gpio); > + ddata->reset_gpio = NULL; > + return dev_err_probe(ddata->dev, ret, "Failed to get GPIO pin for reset signal\n"); > + } > + if (ddata->reset_gpio == NULL) { > + ddata->reset_gpio = ls2k_bmc_find_gpio(ddata); > + if (IS_ERR(ddata->reset_gpio)) { > + ret = PTR_ERR(ddata->reset_gpio); > + ddata->reset_gpio = NULL; > + return dev_err_probe(ddata->dev, ret, > + "Failed to find GPIO pin for reset signal\n"); Why not just: return dev_err_probe(ddata->dev, PTR_ERR(ddata->reset_gpio), ...)? > + } > + } > > - iounmap(gpio_base); > + gpio_irq = gpiod_to_irq(ddata->reset_gpio); > > - /* > - * Since gpio_chip->to_irq is not implemented in the Loongson-3 GPIO driver, > - * acpi_register_gsi() is used to obtain the GPIO IRQ. The GPIO interrupt is a > - * watchdog interrupt that is triggered when the BMC resets. > - */ > - gpio_irq = acpi_register_gsi(NULL, LS2K_BMC_RESET_GPIO_GSI, ACPI_EDGE_SENSITIVE, > - ACPI_ACTIVE_LOW); > if (gpio_irq < 0) > - return gpio_irq; > + return dev_err_probe(ddata->dev, gpio_irq, > + "Failed to get IRQ for GPIO reset signal input\n"); > > - ret = devm_request_irq(ddata->dev, gpio_irq, ls2k_bmc_interrupt, > - IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc gpio", ddata); > - if (ret) > - dev_err(ddata->dev, "Failed to request LS2KBMC GPIO IRQ %d.\n", gpio_irq); > + ret = devm_request_irq(&pdev->dev, gpio_irq, ls2k_bmc_interrupt, > + IRQF_SHARED | IRQF_TRIGGER_FALLING, "ls2kbmc reset", ddata); > + > + if (ret != 0) > + return dev_err_probe(ddata->dev, ret, > + "Failed to request IRQ %d for GPIO reset signal input.\n", > + gpio_irq); > > - acpi_unregister_gsi(LS2K_BMC_RESET_GPIO_GSI); > return ret; > } > > > -- > 2.49.0 > > > Obviously this is ugly but I understand the need for this workaround so with the issues above addressed, this will work for me. Bart