| Message ID | 1483744980-25898-3-git-send-email-ddaney.cavm@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Jan 7, 2017 at 12:22 AM, David Daney <ddaney.cavm@gmail.com> wrote: > From: David Daney <david.daney@cavium.com> > > Cavium ThunderX and OCTEON-TX are arm64 based SoCs. Add driver for > the on-chip GPIO pins. > > Signed-off-by: David Daney <david.daney@cavium.com> (...) > +config GPIO_THUNDERX > + tristate "Cavium ThunderX/OCTEON-TX GPIO" Do you really load this as module? OK then... > +#include <linux/gpio.h> No. #include <linux/gpio/driver.h> Nothing else. > +#define GLITCH_FILTER_400NS ((4ull << GPIO_BIT_CFG_FIL_SEL_SHIFT) | \ > + (9ull << GPIO_BIT_CFG_FIL_CNT_SHIFT)) > + > +static unsigned int bit_cfg_reg(unsigned int line) > +{ > + return 8 * line + GPIO_BIT_CFG; > +} > + > +static unsigned int intr_reg(unsigned int line) > +{ > + return 8 * line + GPIO_INTR; > +} This looks a bit overzealous, but OK. > +struct thunderx_gpio; > + > +struct thunderx_irqdev { > + struct thunderx_gpio *gpio; > + char *name; > + unsigned int line; > +}; > + > +struct thunderx_gpio { > + struct gpio_chip chip; > + u8 __iomem *register_base; > + struct msix_entry *msix_entries; > + struct thunderx_irqdev *irqdev_entries; > + raw_spinlock_t lock; > + unsigned long invert_mask[2]; > + unsigned long od_mask[2]; > + int base_msi; > +}; Why can't you just move the thunderx_irqdev fields into thunderx_gpio? It will save very little memory for non-irq systems, I do not think footprint warrants it. > + > +static bool thunderx_gpio_is_gpio(struct thunderx_gpio *gpio, > + unsigned int line) > +{ > + u64 bit_cfg = readq(gpio->register_base + bit_cfg_reg(line)); > + bool rv = (bit_cfg & GPIO_BIT_CFG_PIN_SEL_MASK) == 0; > + > + WARN_RATELIMIT(!rv, "Pin %d not available for GPIO\n", line); > + > + return rv; > +} Nifty. So this actually has a pin controller back-end? I haven't seen the code for that yet, I think. This seems like a cheap version of /* External interface to pin control */ extern int pinctrl_request_gpio(unsigned gpio); extern void pinctrl_free_gpio(unsigned gpio); extern int pinctrl_gpio_direction_input(unsigned gpio); extern int pinctrl_gpio_direction_output(unsigned gpio); From <linux/pinctrl/consumer.h> So are you planning pin control support separately in drivers/pinctrl/* in the future? Maybe some comment to replace this with proper pin control in the future is warranted? > +static int thunderx_gpio_dir_in(struct gpio_chip *chip, unsigned int line) > +{ > + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); 1. Please use gpiochip_get_data() instead of the container_of() construction, utilized devm_gpiochip_add_data() in your probe() call. 2. Do not call this local variable "gpio" that is superconfusing, at least call it txgpio or something. 1 & 2 applies EVERYWHERE in thid driver. > +static void thunderx_gpio_set(struct gpio_chip *chip, unsigned int line, > + int value) > +{ > + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); > + int bank = line / 64; > + int bank_bit = line % 64; > + > + void __iomem *reg = gpio->register_base + > + (bank * GPIO_2ND_BANK) + (value ? GPIO_TX_SET : GPIO_TX_CLR); > + > + writeq(1ull << bank_bit, reg); Use this: #include <linus/bitops.h> writeq(BIT(bank_bit), reg); Applies EVERYWHERE you use (1ULL << n) > +static int thunderx_gpio_set_single_ended(struct gpio_chip *chip, > + unsigned int line, > + enum single_ended_mode mode) Thanks for implementing this properly. > + read_bits >>= bank_bit; > + > + if (test_bit(line, gpio->invert_mask)) > + return !(read_bits & 1); > + else > + return read_bits & 1; This looks superconvoluted. Can't you just: if (test_bit(line, gpio->invert_mask)) return !(read_bits & BIT(bank_bit)); else return !!(read_bits & BIT(bank_bit)); OK maybe not much clearer but seems clearer to me. > +static int thunderx_gpio_irq_request_resources(struct irq_data *data) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); > + unsigned int line = data->hwirq; > + struct thunderx_irqdev *irqdev; > + int err; > + > + if (!thunderx_gpio_is_gpio(gpio, line)) > + return -EIO; > + > + irqdev = gpio->irqdev_entries + line; > + > + irqdev->gpio = gpio; > + irqdev->line = line; > + irqdev->name = devm_kasprintf(chip->parent, GFP_KERNEL, > + "gpio-%d", line + chip->base); > + > + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); > + > + err = devm_request_irq(chip->parent, gpio->msix_entries[line].vector, > + thunderx_gpio_chain_handler, IRQF_NO_THREAD, irqdev->name, irqdev); > + return err; > +} > + > +static void thunderx_gpio_irq_release_resources(struct irq_data *data) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); > + unsigned int line = data->hwirq; > + struct thunderx_irqdev *irqdev; > + > + irqdev = gpio->irqdev_entries + line; > + > + /* > + * The request_resources/release_resources functions may be > + * called multiple times in the lifitime of the driver, so we > + * need to clean up the devm_* things to avoid a resource > + * leak. > + */ > + devm_free_irq(chip->parent, gpio->msix_entries[line].vector, irqdev); > + > + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); > + > + devm_kfree(chip->parent, irqdev->name); > +} Then just do not use the devm* variants. Explicitly allocate and free instead. These callbacks should be called on all resources anyways. > +static void thunderx_gpio_irq_unmask(struct irq_data *data) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); > + unsigned int line = data->hwirq; > + > + writeq(GPIO_INTR_ENA_W1S, gpio->register_base + intr_reg(line)); > +} > + > +static int thunderx_gpio_irq_set_type(struct irq_data *data, > + unsigned int flow_type) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); > + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); > + unsigned int line = data->hwirq; > + u64 bit_cfg; > + > + irqd_set_trigger_type(data, flow_type); > + > + bit_cfg = GLITCH_FILTER_400NS | GPIO_BIT_CFG_INT_EN; > + > + if (flow_type & IRQ_TYPE_EDGE_BOTH) { > + irq_set_handler_locked(data, handle_edge_irq); > + bit_cfg |= GPIO_BIT_CFG_INT_TYPE; > + } else { > + irq_set_handler_locked(data, handle_level_irq); > + } > + > + raw_spin_lock(&gpio->lock); > + if (flow_type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_LEVEL_LOW)) { > + bit_cfg |= GPIO_BIT_CFG_PIN_XOR; > + set_bit(line, gpio->invert_mask); > + } else { > + clear_bit(line, gpio->invert_mask); > + } > + clear_bit(line, gpio->od_mask); > + writeq(bit_cfg, gpio->register_base + bit_cfg_reg(line)); > + raw_spin_unlock(&gpio->lock); > + > + return IRQ_SET_MASK_OK; > +} > + > +/* > + * Interrupts are chained from underlying MSI-X vectors. We have > + * these irq_chip functions to be able to handle level triggering > + * semantics and other acknowledgment tasks associated with the GPIO > + * mechanism. > + */ > +static struct irq_chip thunderx_gpio_irq_chip = { > + .name = "GPIO", > + .irq_enable = thunderx_gpio_irq_unmask, > + .irq_disable = thunderx_gpio_irq_mask, > + .irq_ack = thunderx_gpio_irq_ack, > + .irq_mask = thunderx_gpio_irq_mask, > + .irq_mask_ack = thunderx_gpio_irq_mask_ack, > + .irq_unmask = thunderx_gpio_irq_unmask, > + .irq_set_type = thunderx_gpio_irq_set_type, > + .irq_request_resources = thunderx_gpio_irq_request_resources, > + .irq_release_resources = thunderx_gpio_irq_release_resources, > + .flags = IRQCHIP_SET_TYPE_MASKED > +}; This looks wrong. If you're calling devm_request_irq() on *every* *single* *irq* like you do here, you are dealing with a hieararchical irqdomain, not a linear one, and GPIOLIB_IRQCHIP may not be used. Look at drivers/gpio/gpio-xgene-sb.c for inspiration. That is the only hierarchical GPIO irqdomain I have right now. Consult Marc Zyngier's IRQ domain lecture if you have time: https://www.youtube.com/watch?v=YE8cRHVIM4E If you have ideas how to combine hierarchical irqdomain and GPIOLIB_IRQCHIP pls help out. > + gpio->irqdev_entries = devm_kzalloc(dev, > + sizeof(struct thunderx_irqdev) * ngpio, > + GFP_KERNEL); > + if (!gpio->irqdev_entries) { > + err = -ENOMEM; > + goto out; > + } I think this is overkill. Use hierarchical irqdomain. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/09/2017 11:35 AM, Linus Walleij wrote: > On Sat, Jan 7, 2017 at 12:22 AM, David Daney <ddaney.cavm@gmail.com> wrote: > >> From: David Daney <david.daney@cavium.com> >> >> Cavium ThunderX and OCTEON-TX are arm64 based SoCs. Add driver for >> the on-chip GPIO pins. >> >> Signed-off-by: David Daney <david.daney@cavium.com> > > (...) >> +config GPIO_THUNDERX >> + tristate "Cavium ThunderX/OCTEON-TX GPIO" > > Do you really load this as module? OK then... The driver does work when loaded as a module. I have tested loading and unloading it many times. > >> +#include <linux/gpio.h> > > No. > #include <linux/gpio/driver.h> > > Nothing else. Right, I will fix that. > >> +#define GLITCH_FILTER_400NS ((4ull << GPIO_BIT_CFG_FIL_SEL_SHIFT) | \ >> + (9ull << GPIO_BIT_CFG_FIL_CNT_SHIFT)) >> + >> +static unsigned int bit_cfg_reg(unsigned int line) >> +{ >> + return 8 * line + GPIO_BIT_CFG; >> +} >> + >> +static unsigned int intr_reg(unsigned int line) >> +{ >> + return 8 * line + GPIO_INTR; >> +} > > This looks a bit overzealous, but OK. > >> +struct thunderx_gpio; >> + >> +struct thunderx_irqdev { >> + struct thunderx_gpio *gpio; >> + char *name; >> + unsigned int line; >> +}; >> + >> +struct thunderx_gpio { >> + struct gpio_chip chip; >> + u8 __iomem *register_base; >> + struct msix_entry *msix_entries; >> + struct thunderx_irqdev *irqdev_entries; >> + raw_spinlock_t lock; >> + unsigned long invert_mask[2]; >> + unsigned long od_mask[2]; >> + int base_msi; >> +}; > > Why can't you just move the thunderx_irqdev fields > into thunderx_gpio? There is a variable length array of them. It is not a single instance of the structure. Same for the msix_entries. > It will save very little memory for > non-irq systems, I do not think footprint warrants it. > >> + >> +static bool thunderx_gpio_is_gpio(struct thunderx_gpio *gpio, >> + unsigned int line) >> +{ >> + u64 bit_cfg = readq(gpio->register_base + bit_cfg_reg(line)); >> + bool rv = (bit_cfg & GPIO_BIT_CFG_PIN_SEL_MASK) == 0; >> + >> + WARN_RATELIMIT(!rv, "Pin %d not available for GPIO\n", line); >> + >> + return rv; >> +} > > Nifty. So this actually has a pin controller back-end? The hardware does function like a "pin controller" > I haven't seen the code for that yet, I think. > > This seems like a cheap version of > > /* External interface to pin control */ > extern int pinctrl_request_gpio(unsigned gpio); > extern void pinctrl_free_gpio(unsigned gpio); > extern int pinctrl_gpio_direction_input(unsigned gpio); > extern int pinctrl_gpio_direction_output(unsigned gpio); > > From <linux/pinctrl/consumer.h> > > So are you planning pin control support separately in drivers/pinctrl/* > in the future? Not at this time. Currently early boot firmware is programming the pin functions, and we don't see a need to move the complexity of pinctrl into the kernel, especially as there is not really any ACPI support for pinctrl at this point, and we must also support ACPI. > Maybe some comment to replace this with proper pin control > in the future is warranted? > Good idea. I will add a comment about this. >> +static int thunderx_gpio_dir_in(struct gpio_chip *chip, unsigned int line) >> +{ >> + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); > > 1. Please use gpiochip_get_data() instead of the container_of() construction, > utilized devm_gpiochip_add_data() in your probe() call. > > 2. Do not call this local variable "gpio" that is superconfusing, at least call > it txgpio or something. > > 1 & 2 applies EVERYWHERE in thid driver. OK, I will make that change for the next revision of the patch set. > >> +static void thunderx_gpio_set(struct gpio_chip *chip, unsigned int line, >> + int value) >> +{ >> + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); >> + int bank = line / 64; >> + int bank_bit = line % 64; >> + >> + void __iomem *reg = gpio->register_base + >> + (bank * GPIO_2ND_BANK) + (value ? GPIO_TX_SET : GPIO_TX_CLR); >> + >> + writeq(1ull << bank_bit, reg); > > Use this: > > #include <linus/bitops.h> > > writeq(BIT(bank_bit), reg); > > Applies EVERYWHERE you use (1ULL << n) OK. > >> +static int thunderx_gpio_set_single_ended(struct gpio_chip *chip, >> + unsigned int line, >> + enum single_ended_mode mode) > > Thanks for implementing this properly. > >> + read_bits >>= bank_bit; >> + >> + if (test_bit(line, gpio->invert_mask)) >> + return !(read_bits & 1); >> + else >> + return read_bits & 1; > > This looks superconvoluted. > > Can't you just: > > if (test_bit(line, gpio->invert_mask)) > return !(read_bits & BIT(bank_bit)); > else > return !!(read_bits & BIT(bank_bit)); > > OK maybe not much clearer but seems clearer to me. As I really dislike the "!!" idiom, would you settle for: if (test_bit(line, gpio->invert_mask)) return (read_bits & BIT(bank_bit)) == 0; else return (read_bits & BIT(bank_bit)) != 0; > >> +static int thunderx_gpio_irq_request_resources(struct irq_data *data) >> +{ >> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); >> + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); >> + unsigned int line = data->hwirq; >> + struct thunderx_irqdev *irqdev; >> + int err; >> + >> + if (!thunderx_gpio_is_gpio(gpio, line)) >> + return -EIO; >> + >> + irqdev = gpio->irqdev_entries + line; >> + >> + irqdev->gpio = gpio; >> + irqdev->line = line; >> + irqdev->name = devm_kasprintf(chip->parent, GFP_KERNEL, >> + "gpio-%d", line + chip->base); >> + >> + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); >> + >> + err = devm_request_irq(chip->parent, gpio->msix_entries[line].vector, >> + thunderx_gpio_chain_handler, IRQF_NO_THREAD, irqdev->name, irqdev); >> + return err; >> +} >> + >> +static void thunderx_gpio_irq_release_resources(struct irq_data *data) >> +{ >> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); >> + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); >> + unsigned int line = data->hwirq; >> + struct thunderx_irqdev *irqdev; >> + >> + irqdev = gpio->irqdev_entries + line; >> + >> + /* >> + * The request_resources/release_resources functions may be >> + * called multiple times in the lifitime of the driver, so we >> + * need to clean up the devm_* things to avoid a resource >> + * leak. >> + */ >> + devm_free_irq(chip->parent, gpio->msix_entries[line].vector, irqdev); >> + >> + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); >> + >> + devm_kfree(chip->parent, irqdev->name); >> +} > > Then just do not use the devm* variants. Explicitly allocate and free instead. > > These callbacks should be called on all resources anyways. Rithe. > >> +static void thunderx_gpio_irq_unmask(struct irq_data *data) >> +{ >> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); >> + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); >> + unsigned int line = data->hwirq; >> + >> + writeq(GPIO_INTR_ENA_W1S, gpio->register_base + intr_reg(line)); >> +} >> + >> +static int thunderx_gpio_irq_set_type(struct irq_data *data, >> + unsigned int flow_type) >> +{ >> + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); >> + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); >> + unsigned int line = data->hwirq; >> + u64 bit_cfg; >> + >> + irqd_set_trigger_type(data, flow_type); >> + >> + bit_cfg = GLITCH_FILTER_400NS | GPIO_BIT_CFG_INT_EN; >> + >> + if (flow_type & IRQ_TYPE_EDGE_BOTH) { >> + irq_set_handler_locked(data, handle_edge_irq); >> + bit_cfg |= GPIO_BIT_CFG_INT_TYPE; >> + } else { >> + irq_set_handler_locked(data, handle_level_irq); >> + } >> + >> + raw_spin_lock(&gpio->lock); >> + if (flow_type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_LEVEL_LOW)) { >> + bit_cfg |= GPIO_BIT_CFG_PIN_XOR; >> + set_bit(line, gpio->invert_mask); >> + } else { >> + clear_bit(line, gpio->invert_mask); >> + } >> + clear_bit(line, gpio->od_mask); >> + writeq(bit_cfg, gpio->register_base + bit_cfg_reg(line)); >> + raw_spin_unlock(&gpio->lock); >> + >> + return IRQ_SET_MASK_OK; >> +} >> + >> +/* >> + * Interrupts are chained from underlying MSI-X vectors. We have >> + * these irq_chip functions to be able to handle level triggering >> + * semantics and other acknowledgment tasks associated with the GPIO >> + * mechanism. >> + */ >> +static struct irq_chip thunderx_gpio_irq_chip = { >> + .name = "GPIO", >> + .irq_enable = thunderx_gpio_irq_unmask, >> + .irq_disable = thunderx_gpio_irq_mask, >> + .irq_ack = thunderx_gpio_irq_ack, >> + .irq_mask = thunderx_gpio_irq_mask, >> + .irq_mask_ack = thunderx_gpio_irq_mask_ack, >> + .irq_unmask = thunderx_gpio_irq_unmask, >> + .irq_set_type = thunderx_gpio_irq_set_type, >> + .irq_request_resources = thunderx_gpio_irq_request_resources, >> + .irq_release_resources = thunderx_gpio_irq_release_resources, >> + .flags = IRQCHIP_SET_TYPE_MASKED >> +}; > > This looks wrong. > > If you're calling devm_request_irq() on *every* *single* *irq* like you > do here, you are dealing with a hieararchical irqdomain, not a linear one, > and GPIOLIB_IRQCHIP may not be used. > > Look at drivers/gpio/gpio-xgene-sb.c for inspiration. That is the only > hierarchical GPIO irqdomain I have right now. > > Consult Marc Zyngier's IRQ domain lecture if you have time: > https://www.youtube.com/watch?v=YE8cRHVIM4E > > If you have ideas how to combine hierarchical irqdomain and GPIOLIB_IRQCHIP > pls help out. > >> + gpio->irqdev_entries = devm_kzalloc(dev, >> + sizeof(struct thunderx_irqdev) * ngpio, >> + GFP_KERNEL); >> + if (!gpio->irqdev_entries) { >> + err = -ENOMEM; >> + goto out; >> + } > > I think this is overkill. Use hierarchical irqdomain. I will look into it. I suspect it will require more lines of driver code to implement it than what I have here (that does actually work). Thanks for looking at this, David -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 9, 2017 at 9:02 PM, David Daney <ddaney@caviumnetworks.com> wrote: >> if (test_bit(line, gpio->invert_mask)) >> return !(read_bits & BIT(bank_bit)); >> else >> return !!(read_bits & BIT(bank_bit)); >> >> OK maybe not much clearer but seems clearer to me. > > As I really dislike the "!!" idiom, would you settle for: > > if (test_bit(line, gpio->invert_mask)) > return (read_bits & BIT(bank_bit)) == 0; > else > return (read_bits & BIT(bank_bit)) != 0; Not the biggest issue in the world. But I maintain a huge stack of GPIO drivers and it drives me crazy that each one has to bear the mark of the authors habits rather than mine. >> I think this is overkill. Use hierarchical irqdomain. > > I will look into it. I suspect it will require more lines of driver code to > implement it than what I have here (that does actually work). I understand. But at the same time, the kernel needs to have the right idea of what it is dealing with here. The generic IRQ handling code will take a shorter fastpath if you are using hierarchical irqdomain (I think?) but I can't claim to be an expert. When in doubt, consult Marc Z. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index d5d3654..e691a5e 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -421,6 +421,14 @@ config GPIO_TS4800 help This driver support TS-4800 FPGA GPIO controllers. +config GPIO_THUNDERX + tristate "Cavium ThunderX/OCTEON-TX GPIO" + depends on ARCH_THUNDER || (64BIT && COMPILE_TEST) + select GPIOLIB_IRQCHIP + help + Say yes here to support the on-chip GPIO lines on the ThunderX + and OCTEON-TX families of SoCs. + config GPIO_TZ1090 bool "Toumaz Xenif TZ1090 GPIO support" depends on SOC_TZ1090 diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index a7676b8..c62bc72 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -108,6 +108,7 @@ obj-$(CONFIG_GPIO_SYSCON) += gpio-syscon.o obj-$(CONFIG_GPIO_TB10X) += gpio-tb10x.o obj-$(CONFIG_GPIO_TC3589X) += gpio-tc3589x.o obj-$(CONFIG_GPIO_TEGRA) += gpio-tegra.o +obj-$(CONFIG_GPIO_THUNDERX) += gpio-thunderx.o obj-$(CONFIG_GPIO_TIMBERDALE) += gpio-timberdale.o obj-$(CONFIG_GPIO_PALMAS) += gpio-palmas.o obj-$(CONFIG_GPIO_TPIC2810) += gpio-tpic2810.o diff --git a/drivers/gpio/gpio-thunderx.c b/drivers/gpio/gpio-thunderx.c new file mode 100644 index 0000000..089d8d4 --- /dev/null +++ b/drivers/gpio/gpio-thunderx.c @@ -0,0 +1,487 @@ +/* + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * Copyright (C) 2016, 2017 Cavium Inc. + */ + +#include <linux/gpio.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/spinlock.h> + + +#define GPIO_RX_DAT 0x0 +#define GPIO_TX_SET 0x8 +#define GPIO_TX_CLR 0x10 +#define GPIO_CONST 0x90 +#define GPIO_CONST_GPIOS_MASK 0xff +#define GPIO_BIT_CFG 0x400 +#define GPIO_BIT_CFG_TX_OE BIT(0) +#define GPIO_BIT_CFG_PIN_XOR BIT(1) +#define GPIO_BIT_CFG_INT_EN BIT(2) +#define GPIO_BIT_CFG_INT_TYPE BIT(3) +#define GPIO_BIT_CFG_FIL_CNT_SHIFT 4 +#define GPIO_BIT_CFG_FIL_SEL_SHIFT 8 +#define GPIO_BIT_CFG_TX_OD BIT(12) +#define GPIO_BIT_CFG_PIN_SEL_MASK GENMASK(25, 16) +#define GPIO_INTR 0x800 +#define GPIO_INTR_INTR BIT(0) +#define GPIO_INTR_INTR_W1S BIT(1) +#define GPIO_INTR_ENA_W1C BIT(2) +#define GPIO_INTR_ENA_W1S BIT(3) +#define GPIO_2ND_BANK 0x1400 + +#define GLITCH_FILTER_400NS ((4ull << GPIO_BIT_CFG_FIL_SEL_SHIFT) | \ + (9ull << GPIO_BIT_CFG_FIL_CNT_SHIFT)) + +static unsigned int bit_cfg_reg(unsigned int line) +{ + return 8 * line + GPIO_BIT_CFG; +} + +static unsigned int intr_reg(unsigned int line) +{ + return 8 * line + GPIO_INTR; +} + +struct thunderx_gpio; + +struct thunderx_irqdev { + struct thunderx_gpio *gpio; + char *name; + unsigned int line; +}; + +struct thunderx_gpio { + struct gpio_chip chip; + u8 __iomem *register_base; + struct msix_entry *msix_entries; + struct thunderx_irqdev *irqdev_entries; + raw_spinlock_t lock; + unsigned long invert_mask[2]; + unsigned long od_mask[2]; + int base_msi; +}; + + +/* + * Check (and WARN) that the pin is available for GPIO. We will not + * allow modification of the state of non-GPIO pins from this driver. + */ +static bool thunderx_gpio_is_gpio(struct thunderx_gpio *gpio, + unsigned int line) +{ + u64 bit_cfg = readq(gpio->register_base + bit_cfg_reg(line)); + bool rv = (bit_cfg & GPIO_BIT_CFG_PIN_SEL_MASK) == 0; + + WARN_RATELIMIT(!rv, "Pin %d not available for GPIO\n", line); + + return rv; +} + +static int thunderx_gpio_dir_in(struct gpio_chip *chip, unsigned int line) +{ + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + + if (!thunderx_gpio_is_gpio(gpio, line)) + return -EIO; + + raw_spin_lock(&gpio->lock); + clear_bit(line, gpio->invert_mask); + clear_bit(line, gpio->od_mask); + writeq(GLITCH_FILTER_400NS, gpio->register_base + bit_cfg_reg(line)); + raw_spin_unlock(&gpio->lock); + return 0; +} + +static void thunderx_gpio_set(struct gpio_chip *chip, unsigned int line, + int value) +{ + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + int bank = line / 64; + int bank_bit = line % 64; + + void __iomem *reg = gpio->register_base + + (bank * GPIO_2ND_BANK) + (value ? GPIO_TX_SET : GPIO_TX_CLR); + + writeq(1ull << bank_bit, reg); +} + +static int thunderx_gpio_dir_out(struct gpio_chip *chip, unsigned int line, + int value) +{ + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + u64 bit_cfg = GPIO_BIT_CFG_TX_OE; + + if (!thunderx_gpio_is_gpio(gpio, line)) + return -EIO; + + raw_spin_lock(&gpio->lock); + + thunderx_gpio_set(chip, line, value); + + if (test_bit(line, gpio->invert_mask)) + bit_cfg |= GPIO_BIT_CFG_PIN_XOR; + + if (test_bit(line, gpio->od_mask)) + bit_cfg |= GPIO_BIT_CFG_TX_OD; + + writeq(bit_cfg, gpio->register_base + bit_cfg_reg(line)); + + raw_spin_unlock(&gpio->lock); + return 0; +} + +/* + * Weird, setting open-drain mode causes signal inversion. Note this + * so we can compensate in the dir_out function. + */ +static int thunderx_gpio_set_single_ended(struct gpio_chip *chip, + unsigned int line, + enum single_ended_mode mode) +{ + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + + if (mode == LINE_MODE_OPEN_SOURCE) + return -ENOTSUPP; + + if (!thunderx_gpio_is_gpio(gpio, line)) + return -EIO; + + raw_spin_lock(&gpio->lock); + if (mode == LINE_MODE_OPEN_DRAIN) { + set_bit(line, gpio->invert_mask); + set_bit(line, gpio->od_mask); + } else { + clear_bit(line, gpio->invert_mask); + clear_bit(line, gpio->od_mask); + } + raw_spin_unlock(&gpio->lock); + + return 0; +} + +static int thunderx_gpio_get(struct gpio_chip *chip, unsigned int line) +{ + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + int bank = line / 64; + int bank_bit = line % 64; + u64 read_bits = readq(gpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_RX_DAT); + + read_bits >>= bank_bit; + + if (test_bit(line, gpio->invert_mask)) + return !(read_bits & 1); + else + return read_bits & 1; +} + +static void thunderx_gpio_set_multiple(struct gpio_chip *chip, + unsigned long *mask, + unsigned long *bits) +{ + int bank; + u64 set_bits, clear_bits; + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + + for (bank = 0; bank <= chip->ngpio / 64; bank++) { + set_bits = bits[bank] & mask[bank]; + clear_bits = ~bits[bank] & mask[bank]; + writeq(set_bits, gpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_SET); + writeq(clear_bits, gpio->register_base + (bank * GPIO_2ND_BANK) + GPIO_TX_CLR); + } +} + +static irqreturn_t thunderx_gpio_chain_handler(int irq, void *dev) +{ + struct thunderx_irqdev *irqdev = dev; + int chained_irq; + int ret; + + chained_irq = irq_find_mapping(irqdev->gpio->chip.irqdomain, + irqdev->line); + if (!chained_irq) + return IRQ_NONE; + + ret = generic_handle_irq(chained_irq); + + return ret ? IRQ_NONE : IRQ_HANDLED; +} + +static int thunderx_gpio_irq_request_resources(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + struct thunderx_irqdev *irqdev; + int err; + + if (!thunderx_gpio_is_gpio(gpio, line)) + return -EIO; + + irqdev = gpio->irqdev_entries + line; + + irqdev->gpio = gpio; + irqdev->line = line; + irqdev->name = devm_kasprintf(chip->parent, GFP_KERNEL, + "gpio-%d", line + chip->base); + + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); + + err = devm_request_irq(chip->parent, gpio->msix_entries[line].vector, + thunderx_gpio_chain_handler, IRQF_NO_THREAD, irqdev->name, irqdev); + return err; +} + +static void thunderx_gpio_irq_release_resources(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + struct thunderx_irqdev *irqdev; + + irqdev = gpio->irqdev_entries + line; + + /* + * The request_resources/release_resources functions may be + * called multiple times in the lifitime of the driver, so we + * need to clean up the devm_* things to avoid a resource + * leak. + */ + devm_free_irq(chip->parent, gpio->msix_entries[line].vector, irqdev); + + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); + + devm_kfree(chip->parent, irqdev->name); +} + +static void thunderx_gpio_irq_ack(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + + writeq(GPIO_INTR_INTR, + gpio->register_base + intr_reg(line)); +} + +static void thunderx_gpio_irq_mask(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + + writeq(GPIO_INTR_ENA_W1C, gpio->register_base + intr_reg(line)); +} + +static void thunderx_gpio_irq_mask_ack(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + + writeq(GPIO_INTR_ENA_W1C | GPIO_INTR_INTR, + gpio->register_base + intr_reg(line)); +} + +static void thunderx_gpio_irq_unmask(struct irq_data *data) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + + writeq(GPIO_INTR_ENA_W1S, gpio->register_base + intr_reg(line)); +} + +static int thunderx_gpio_irq_set_type(struct irq_data *data, + unsigned int flow_type) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct thunderx_gpio *gpio = container_of(chip, struct thunderx_gpio, chip); + unsigned int line = data->hwirq; + u64 bit_cfg; + + irqd_set_trigger_type(data, flow_type); + + bit_cfg = GLITCH_FILTER_400NS | GPIO_BIT_CFG_INT_EN; + + if (flow_type & IRQ_TYPE_EDGE_BOTH) { + irq_set_handler_locked(data, handle_edge_irq); + bit_cfg |= GPIO_BIT_CFG_INT_TYPE; + } else { + irq_set_handler_locked(data, handle_level_irq); + } + + raw_spin_lock(&gpio->lock); + if (flow_type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_LEVEL_LOW)) { + bit_cfg |= GPIO_BIT_CFG_PIN_XOR; + set_bit(line, gpio->invert_mask); + } else { + clear_bit(line, gpio->invert_mask); + } + clear_bit(line, gpio->od_mask); + writeq(bit_cfg, gpio->register_base + bit_cfg_reg(line)); + raw_spin_unlock(&gpio->lock); + + return IRQ_SET_MASK_OK; +} + +/* + * Interrupts are chained from underlying MSI-X vectors. We have + * these irq_chip functions to be able to handle level triggering + * semantics and other acknowledgment tasks associated with the GPIO + * mechanism. + */ +static struct irq_chip thunderx_gpio_irq_chip = { + .name = "GPIO", + .irq_enable = thunderx_gpio_irq_unmask, + .irq_disable = thunderx_gpio_irq_mask, + .irq_ack = thunderx_gpio_irq_ack, + .irq_mask = thunderx_gpio_irq_mask, + .irq_mask_ack = thunderx_gpio_irq_mask_ack, + .irq_unmask = thunderx_gpio_irq_unmask, + .irq_set_type = thunderx_gpio_irq_set_type, + .irq_request_resources = thunderx_gpio_irq_request_resources, + .irq_release_resources = thunderx_gpio_irq_release_resources, + .flags = IRQCHIP_SET_TYPE_MASKED +}; + +static int thunderx_gpio_probe(struct pci_dev *pdev, + const struct pci_device_id *id) +{ + void __iomem * const *tbl; + struct device *dev = &pdev->dev; + struct thunderx_gpio *gpio; + struct gpio_chip *chip; + int ngpio, i; + int err = 0; + + gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL); + if (!gpio) + return -ENOMEM; + + raw_spin_lock_init(&gpio->lock); + chip = &gpio->chip; + + pci_set_drvdata(pdev, gpio); + + err = pcim_enable_device(pdev); + if (err) { + dev_err(dev, "Failed to enable PCI device: err %d\n", err); + goto out; + } + + err = pcim_iomap_regions(pdev, 1 << 0, KBUILD_MODNAME); + if (err) { + dev_err(dev, "Failed to iomap PCI device: err %d\n", err); + goto out; + } + + tbl = pcim_iomap_table(pdev); + gpio->register_base = tbl[0]; + if (!gpio->register_base) { + dev_err(dev, "Cannot map PCI resource\n"); + err = -ENOMEM; + goto out; + } + + if (pdev->subsystem_device == 0xa10a) { + /* CN88XX has no GPIO_CONST register*/ + ngpio = 50; + gpio->base_msi = 48; + } else { + u64 c = readq(gpio->register_base + GPIO_CONST); + + ngpio = c & GPIO_CONST_GPIOS_MASK; + gpio->base_msi = (c >> 8) & 0xff; + } + + gpio->msix_entries = devm_kzalloc(dev, + sizeof(struct msix_entry) * ngpio, + GFP_KERNEL); + if (!gpio->msix_entries) { + err = -ENOMEM; + goto out; + } + + gpio->irqdev_entries = devm_kzalloc(dev, + sizeof(struct thunderx_irqdev) * ngpio, + GFP_KERNEL); + if (!gpio->irqdev_entries) { + err = -ENOMEM; + goto out; + } + + for (i = 0; i < ngpio; i++) + gpio->msix_entries[i].entry = gpio->base_msi + (2 * i); + + err = pci_enable_msix(pdev, gpio->msix_entries, ngpio); + if (err < 0) + goto out; + + chip->label = KBUILD_MODNAME; + chip->parent = dev; + chip->owner = THIS_MODULE; + chip->base = -1; /* System allocated */ + chip->can_sleep = false; + chip->ngpio = ngpio; + chip->direction_input = thunderx_gpio_dir_in; + chip->get = thunderx_gpio_get; + chip->direction_output = thunderx_gpio_dir_out; + chip->set = thunderx_gpio_set; + chip->set_multiple = thunderx_gpio_set_multiple; + chip->set_single_ended = thunderx_gpio_set_single_ended; + err = gpiochip_add(chip); + if (err) + goto out; + + err = gpiochip_irqchip_add(chip, &thunderx_gpio_irq_chip, 0, + handle_level_irq, IRQ_TYPE_NONE); + if (err) { + dev_err(dev, "gpiochip_irqchip_add failed: %d\n", err); + goto irqchip_out; + } + + dev_info(dev, "ThunderX GPIO: %d lines with base %d.\n", + ngpio, chip->base); + return 0; + +irqchip_out: + gpiochip_remove(chip); +out: + pci_set_drvdata(pdev, NULL); + return err; +} + +static void thunderx_gpio_remove(struct pci_dev *pdev) +{ + struct thunderx_gpio *gpio = pci_get_drvdata(pdev); + + gpiochip_remove(&gpio->chip); + pci_set_drvdata(pdev, NULL); +} + +static const struct pci_device_id thunderx_gpio_id_table[] = { + { PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xA00A) }, + { 0, } /* end of table */ +}; + +MODULE_DEVICE_TABLE(pci, thunderx_gpio_id_table); + +static struct pci_driver thunderx_gpio_driver = { + .name = KBUILD_MODNAME, + .id_table = thunderx_gpio_id_table, + .probe = thunderx_gpio_probe, + .remove = thunderx_gpio_remove, +}; + +module_pci_driver(thunderx_gpio_driver); + +MODULE_DESCRIPTION("Cavium Inc. ThunderX/OCTEON-TX GPIO Driver"); +MODULE_LICENSE("GPL");