* [PATCH 1/2] gpio: xilinx: Add clock adaptation support @ 2020-02-17 10:57 Srinivas Neeli 2020-02-17 10:57 ` [PATCH 2/2] gpio: xilinx: Add irq support to the driver Srinivas Neeli 2020-02-18 16:02 ` [PATCH 1/2] gpio: xilinx: Add clock adaptation support Bartosz Golaszewski 0 siblings, 2 replies; 7+ messages in thread From: Srinivas Neeli @ 2020-02-17 10:57 UTC (permalink / raw) To: bgolaszewski, michal.simek, shubhrajyoti.datta, sgoud Cc: linus.walleij, linux-kernel, linux-arm-kernel, linux-gpio, git Add support of clock adaptation for AXI GPIO driver. Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com> --- drivers/gpio/gpio-xilinx.c | 105 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c index a9748b5198e6..26753ae58295 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -14,6 +14,8 @@ #include <linux/io.h> #include <linux/gpio/driver.h> #include <linux/slab.h> +#include <linux/pm_runtime.h> +#include <linux/clk.h> /* Register Offset Definitions */ #define XGPIO_DATA_OFFSET (0x0) /* Data register */ @@ -38,6 +40,7 @@ * @gpio_state: GPIO state shadow register * @gpio_dir: GPIO direction shadow register * @gpio_lock: Lock used for synchronization + * @clk: clock resource for this driver */ struct xgpio_instance { struct gpio_chip gc; @@ -45,7 +48,8 @@ struct xgpio_instance { unsigned int gpio_width[2]; u32 gpio_state[2]; u32 gpio_dir[2]; - spinlock_t gpio_lock[2]; + spinlock_t gpio_lock[2]; /* For serializing operations */ + struct clk *clk; }; static inline int xgpio_index(struct xgpio_instance *chip, int gpio) @@ -255,6 +259,70 @@ static void xgpio_save_regs(struct xgpio_instance *chip) chip->gpio_dir[1]); } +static int xgpio_request(struct gpio_chip *chip, unsigned int offset) +{ + int ret = pm_runtime_get_sync(chip->parent); + + /* + * If the device is already active pm_runtime_get() will return 1 on + * success, but gpio_request still needs to return 0. + */ + return ret < 0 ? ret : 0; +} + +static void xgpio_free(struct gpio_chip *chip, unsigned int offset) +{ + pm_runtime_put(chip->parent); +} + +static int __maybe_unused xgpio_suspend(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + int irq = platform_get_irq(pdev, 0); + struct irq_data *data = irq_get_irq_data(irq); + + if (!irqd_is_wakeup_set(data)) + return pm_runtime_force_suspend(dev); + + return 0; +} + +static int __maybe_unused xgpio_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + int irq = platform_get_irq(pdev, 0); + struct irq_data *data = irq_get_irq_data(irq); + + if (!irqd_is_wakeup_set(data)) + return pm_runtime_force_resume(dev); + + return 0; +} + +static int __maybe_unused xgpio_runtime_suspend(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct xgpio_instance *gpio = platform_get_drvdata(pdev); + + clk_disable(gpio->clk); + + return 0; +} + +static int __maybe_unused xgpio_runtime_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct xgpio_instance *gpio = platform_get_drvdata(pdev); + + return clk_enable(gpio->clk); +} + +static const struct dev_pm_ops xgpio_dev_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(xgpio_suspend, xgpio_resume) + SET_RUNTIME_PM_OPS(xgpio_runtime_suspend, + xgpio_runtime_resume, NULL) +}; + /** * xgpio_of_probe - Probe method for the GPIO device. * @pdev: pointer to the platform device @@ -323,6 +391,8 @@ static int xgpio_probe(struct platform_device *pdev) chip->gc.direction_output = xgpio_dir_out; chip->gc.get = xgpio_get; chip->gc.set = xgpio_set; + chip->gc.request = xgpio_request; + chip->gc.free = xgpio_free; chip->gc.set_multiple = xgpio_set_multiple; chip->gc.label = dev_name(&pdev->dev); @@ -333,15 +403,45 @@ static int xgpio_probe(struct platform_device *pdev) return PTR_ERR(chip->regs); } + chip->clk = devm_clk_get(&pdev->dev, "s_axi_aclk"); + if (IS_ERR(chip->clk)) { + if (PTR_ERR(chip->clk) != -ENOENT) { + if (PTR_ERR(chip->clk) != -EPROBE_DEFER) + dev_err(&pdev->dev, "Input clock not found\n"); + return PTR_ERR(chip->clk); + } + /* + * Clock framework support is optional, continue on + * anyways if we don't find a matching clock. + */ + chip->clk = NULL; + } + status = clk_prepare_enable(chip->clk); + if (status < 0) { + dev_err(&pdev->dev, "Failed to prepare clk\n"); + return status; + } + pm_runtime_enable(&pdev->dev); + status = pm_runtime_get_sync(&pdev->dev); + if (status < 0) + goto err_unprepare_clk; + xgpio_save_regs(chip); status = devm_gpiochip_add_data(&pdev->dev, &chip->gc, chip); if (status) { dev_err(&pdev->dev, "failed to add GPIO chip\n"); - return status; + goto err_pm_put; } + pm_runtime_put(&pdev->dev); return 0; +err_pm_put: + pm_runtime_put(&pdev->dev); +err_unprepare_clk: + pm_runtime_disable(&pdev->dev); + clk_unprepare(chip->clk); + return status; } static const struct of_device_id xgpio_of_match[] = { @@ -356,6 +456,7 @@ static struct platform_driver xgpio_plat_driver = { .driver = { .name = "gpio-xilinx", .of_match_table = xgpio_of_match, + .pm = &xgpio_dev_pm_ops, }, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] gpio: xilinx: Add irq support to the driver 2020-02-17 10:57 [PATCH 1/2] gpio: xilinx: Add clock adaptation support Srinivas Neeli @ 2020-02-17 10:57 ` Srinivas Neeli 2020-02-19 12:39 ` kbuild test robot ` (2 more replies) 2020-02-18 16:02 ` [PATCH 1/2] gpio: xilinx: Add clock adaptation support Bartosz Golaszewski 1 sibling, 3 replies; 7+ messages in thread From: Srinivas Neeli @ 2020-02-17 10:57 UTC (permalink / raw) To: bgolaszewski, michal.simek, shubhrajyoti.datta, sgoud Cc: linus.walleij, linux-kernel, linux-arm-kernel, linux-gpio, git Allocate single chip for both channels. Add irq support to the driver. Supporting rising edge interrupts and in cascade mode supporting first channel for interrupts on 32bit machines. Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com> --- drivers/gpio/gpio-xilinx.c | 233 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 232 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c index 26753ae58295..f6dd316b2c62 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -16,6 +16,11 @@ #include <linux/slab.h> #include <linux/pm_runtime.h> #include <linux/clk.h> +#include <linux/of_irq.h> +#include <linux/interrupt.h> +#include <linux/irq.h> +#include <linux/irqchip/chained_irq.h> +#include <linux/irqdomain.h> /* Register Offset Definitions */ #define XGPIO_DATA_OFFSET (0x0) /* Data register */ @@ -23,8 +28,13 @@ #define XGPIO_CHANNEL_OFFSET 0x8 +#define XGPIO_GIER_OFFSET 0x11c /* Global Interrupt Enable */ +#define XGPIO_GIER_IE BIT(31) +#define XGPIO_IPISR_OFFSET 0x120 /* IP Interrupt Status */ +#define XGPIO_IPIER_OFFSET 0x128 /* IP Interrupt Enable */ + /* Read/Write access to the GPIO registers */ -#if defined(CONFIG_ARCH_ZYNQ) || defined(CONFIG_X86) +#if defined(CONFIG_ARCH_ZYNQ) || defined(CONFIG_X86) || defined(CONFIG_ARM64) # define xgpio_readreg(offset) readl(offset) # define xgpio_writereg(offset, val) writel(val, offset) #else @@ -41,7 +51,11 @@ * @gpio_dir: GPIO direction shadow register * @gpio_lock: Lock used for synchronization * @clk: clock resource for this driver + * @irq_base: GPIO channel irq base address + * @irq_enable: GPIO irq enable/disable bitfield + * @irq_domain: irq_domain of the controller */ + struct xgpio_instance { struct gpio_chip gc; void __iomem *regs; @@ -50,6 +64,9 @@ struct xgpio_instance { u32 gpio_dir[2]; spinlock_t gpio_lock[2]; /* For serializing operations */ struct clk *clk; + int irq_base; + u32 irq_enable; + struct irq_domain *irq_domain; }; static inline int xgpio_index(struct xgpio_instance *chip, int gpio) @@ -324,6 +341,211 @@ static const struct dev_pm_ops xgpio_dev_pm_ops = { }; /** + * xgpiops_irq_mask - Write the specified signal of the GPIO device. + * @irq_data: per irq and chip data passed down to chip functions + */ +static void xgpio_irq_mask(struct irq_data *irq_data) +{ + unsigned long flags; + struct xgpio_instance *chip = irq_data_get_irq_chip_data(irq_data); + u32 offset = irq_data->irq - chip->irq_base; + u32 temp; + s32 val; + int index = xgpio_index(chip, 0); + + pr_debug("%s: Disable %d irq, irq_enable_mask 0x%x\n", + __func__, offset, chip->irq_enable); + + spin_lock_irqsave(&chip->gpio_lock[index], flags); + + chip->irq_enable &= ~BIT(offset); + + if (!chip->irq_enable) { + /* Enable per channel interrupt */ + temp = xgpio_readreg(chip->regs + XGPIO_IPIER_OFFSET); + val = offset - chip->gpio_width[0] + 1; + if (val > 0) + temp &= 1; + else + temp &= 2; + xgpio_writereg(chip->regs + XGPIO_IPIER_OFFSET, temp); + + /* Disable global interrupt if channel interrupts are unused */ + temp = xgpio_readreg(chip->regs + XGPIO_IPIER_OFFSET); + if (!temp) + xgpio_writereg(chip->regs + XGPIO_GIER_OFFSET, + ~XGPIO_GIER_IE); + } + spin_unlock_irqrestore(&chip->gpio_lock[index], flags); +} + +/** + * xgpio_irq_unmask - Write the specified signal of the GPIO device. + * @irq_data: per irq and chip data passed down to chip functions + */ +static void xgpio_irq_unmask(struct irq_data *irq_data) +{ + unsigned long flags; + struct xgpio_instance *chip = irq_data_get_irq_chip_data(irq_data); + u32 offset = irq_data->irq - chip->irq_base; + u32 temp; + s32 val; + int index = xgpio_index(chip, 0); + + pr_debug("%s: Enable %d irq, irq_enable_mask 0x%x\n", + __func__, offset, chip->irq_enable); + + /* Setup pin as input */ + xgpio_dir_in(&chip->gc, offset); + + spin_lock_irqsave(&chip->gpio_lock[index], flags); + + chip->irq_enable |= BIT(offset); + + if (chip->irq_enable) { + /* Enable per channel interrupt */ + temp = xgpio_readreg(chip->regs + XGPIO_IPIER_OFFSET); + val = offset - (chip->gpio_width[0] - 1); + if (val > 0) + temp |= 2; + else + temp |= 1; + xgpio_writereg(chip->regs + XGPIO_IPIER_OFFSET, temp); + + /* Enable global interrupts */ + xgpio_writereg(chip->regs + XGPIO_GIER_OFFSET, XGPIO_GIER_IE); + } + + spin_unlock_irqrestore(&chip->gpio_lock[index], flags); +} + +/** + * xgpio_set_irq_type - Write the specified signal of the GPIO device. + * @irq_data: Per irq and chip data passed down to chip functions + * @type: Interrupt type that is to be set for the gpio pin + * + * Return: + * 0 if interrupt type is supported otherwise otherwise -EINVAL + */ +static int xgpio_set_irq_type(struct irq_data *irq_data, unsigned int type) +{ + /* Only rising edge case is supported now */ + if (type & IRQ_TYPE_EDGE_RISING) + return 0; + + return -EINVAL; +} + +/* irq chip descriptor */ +static struct irq_chip xgpio_irqchip = { + .name = "xgpio", + .irq_mask = xgpio_irq_mask, + .irq_unmask = xgpio_irq_unmask, + .irq_set_type = xgpio_set_irq_type, +}; + +/** + * xgpio_to_irq - Find out gpio to Linux irq mapping + * @gc: Pointer to gpio_chip device structure. + * @offset: Gpio pin offset + * + * Return: + * irq number otherwise -EINVAL + */ +static int xgpio_to_irq(struct gpio_chip *gc, unsigned int offset) +{ + struct xgpio_instance *chip = gpiochip_get_data(gc); + + return irq_find_mapping(chip->irq_domain, offset); +} + +/** + * xgpio_irqhandler - Gpio interrupt service routine + * @desc: Pointer to interrupt description + */ +static void xgpio_irqhandler(struct irq_desc *desc) +{ + unsigned int irq = irq_desc_get_irq(desc); + struct xgpio_instance *chip = (struct xgpio_instance *) + irq_get_handler_data(irq); + struct irq_chip *irqchip = irq_desc_get_chip(desc); + u32 offset, status, channel = 1; + unsigned long val; + + chained_irq_enter(irqchip, desc); + + val = xgpio_readreg(chip->regs); + if (!val) { + channel = 2; + val = xgpio_readreg(chip->regs + XGPIO_CHANNEL_OFFSET); + val = val << chip->gpio_width[0]; + } + + /* Only rising edge is supported */ + val &= chip->irq_enable; + for_each_set_bit(offset, &val, chip->gc.ngpio) { + generic_handle_irq(chip->irq_base + offset); + } + + status = xgpio_readreg(chip->regs + XGPIO_IPISR_OFFSET); + xgpio_writereg(chip->regs + XGPIO_IPISR_OFFSET, channel); + + chained_irq_exit(irqchip, desc); +} + +static struct lock_class_key gpio_lock_class; +static struct lock_class_key gpio_request_class; + +/** + * xgpio_irq_setup - Allocate irq for gpio and setup appropriate functions + * @np: Device node of the GPIO chip + * @chip: Pointer to private gpio channel structure + * + * Return: + * 0 if success, otherwise -1 + */ +static int xgpio_irq_setup(struct device_node *np, struct xgpio_instance *chip) +{ + u32 pin_num; + struct resource res; + int ret = of_irq_to_resource(np, 0, &res); + + if (ret <= 0) { + pr_info("GPIO IRQ not connected\n"); + return 0; + } + + chip->gc.to_irq = xgpio_to_irq; + chip->irq_base = irq_alloc_descs(-1, 0, chip->gc.ngpio, 0); + if (chip->irq_base < 0) { + pr_err("Couldn't allocate IRQ numbers\n"); + return -1; + } + chip->irq_domain = irq_domain_add_legacy(np, chip->gc.ngpio, + chip->irq_base, 0, + &irq_domain_simple_ops, NULL); + /* + * set the irq chip, handler and irq chip data for callbacks for + * each pin + */ + for (pin_num = 0; pin_num < chip->gc.ngpio; pin_num++) { + u32 gpio_irq = irq_find_mapping(chip->irq_domain, pin_num); + + irq_set_lockdep_class(gpio_irq, &gpio_lock_class, + &gpio_request_class); + pr_debug("IRQ Base: %d, Pin %d = IRQ %d\n", + chip->irq_base, pin_num, gpio_irq); + irq_set_chip_and_handler(gpio_irq, &xgpio_irqchip, + handle_simple_irq); + irq_set_chip_data(gpio_irq, (void *)chip); + } + irq_set_handler_data(res.start, (void *)chip); + irq_set_chained_handler(res.start, xgpio_irqhandler); + + return 0; +} + +/** * xgpio_of_probe - Probe method for the GPIO device. * @pdev: pointer to the platform device * @@ -434,6 +656,15 @@ static int xgpio_probe(struct platform_device *pdev) goto err_pm_put; } + status = xgpio_irq_setup(np, chip); + if (status) { + pr_err("%s: GPIO IRQ initialization failed %d\n", + np->full_name, status); + goto err_pm_put; + } + pr_info("XGpio: %s: registered, base is %d\n", np->full_name, + chip->gc.base); + pm_runtime_put(&pdev->dev); return 0; err_pm_put: -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio: xilinx: Add irq support to the driver 2020-02-17 10:57 ` [PATCH 2/2] gpio: xilinx: Add irq support to the driver Srinivas Neeli @ 2020-02-19 12:39 ` kbuild test robot 2020-02-19 15:00 ` kbuild test robot 2020-04-29 13:56 ` Daniel Mack 2 siblings, 0 replies; 7+ messages in thread From: kbuild test robot @ 2020-02-19 12:39 UTC (permalink / raw) To: Srinivas Neeli Cc: kbuild-all, bgolaszewski, michal.simek, shubhrajyoti.datta, sgoud, linus.walleij, linux-kernel, linux-arm-kernel, linux-gpio, git [-- Attachment #1: Type: text/plain, Size: 1098 bytes --] Hi Srinivas, Thank you for the patch! Yet something to improve: [auto build test ERROR on v5.6-rc2] [also build test ERROR on next-20200219] [cannot apply to xlnx/master] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Srinivas-Neeli/gpio-xilinx-Add-clock-adaptation-support/20200219-110158 base: 11a48a5a18c63fd7621bb050228cebf13566e4d8 config: x86_64-randconfig-b001-20200219 (attached as .config) compiler: gcc-7 (Debian 7.5.0-5) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> ERROR: "of_irq_to_resource" [drivers/gpio/gpio-xilinx.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 35445 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio: xilinx: Add irq support to the driver 2020-02-17 10:57 ` [PATCH 2/2] gpio: xilinx: Add irq support to the driver Srinivas Neeli 2020-02-19 12:39 ` kbuild test robot @ 2020-02-19 15:00 ` kbuild test robot 2020-04-29 13:56 ` Daniel Mack 2 siblings, 0 replies; 7+ messages in thread From: kbuild test robot @ 2020-02-19 15:00 UTC (permalink / raw) To: Srinivas Neeli Cc: kbuild-all, bgolaszewski, michal.simek, shubhrajyoti.datta, sgoud, linus.walleij, linux-kernel, linux-arm-kernel, linux-gpio, git [-- Attachment #1: Type: text/plain, Size: 2939 bytes --] Hi Srinivas, Thank you for the patch! Yet something to improve: [auto build test ERROR on v5.6-rc2] [also build test ERROR on next-20200219] [cannot apply to xlnx/master] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Srinivas-Neeli/gpio-xilinx-Add-clock-adaptation-support/20200219-110158 base: 11a48a5a18c63fd7621bb050228cebf13566e4d8 config: i386-randconfig-b003-20200219 (attached as .config) compiler: gcc-7 (Debian 7.5.0-5) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): ld: drivers/gpio/gpio-xilinx.o: in function `xgpio_irq_setup': >> drivers/gpio/gpio-xilinx.c:512: undefined reference to `of_irq_to_resource' vim +512 drivers/gpio/gpio-xilinx.c 499 500 /** 501 * xgpio_irq_setup - Allocate irq for gpio and setup appropriate functions 502 * @np: Device node of the GPIO chip 503 * @chip: Pointer to private gpio channel structure 504 * 505 * Return: 506 * 0 if success, otherwise -1 507 */ 508 static int xgpio_irq_setup(struct device_node *np, struct xgpio_instance *chip) 509 { 510 u32 pin_num; 511 struct resource res; > 512 int ret = of_irq_to_resource(np, 0, &res); 513 514 if (ret <= 0) { 515 pr_info("GPIO IRQ not connected\n"); 516 return 0; 517 } 518 519 chip->gc.to_irq = xgpio_to_irq; 520 chip->irq_base = irq_alloc_descs(-1, 0, chip->gc.ngpio, 0); 521 if (chip->irq_base < 0) { 522 pr_err("Couldn't allocate IRQ numbers\n"); 523 return -1; 524 } 525 chip->irq_domain = irq_domain_add_legacy(np, chip->gc.ngpio, 526 chip->irq_base, 0, 527 &irq_domain_simple_ops, NULL); 528 /* 529 * set the irq chip, handler and irq chip data for callbacks for 530 * each pin 531 */ 532 for (pin_num = 0; pin_num < chip->gc.ngpio; pin_num++) { 533 u32 gpio_irq = irq_find_mapping(chip->irq_domain, pin_num); 534 535 irq_set_lockdep_class(gpio_irq, &gpio_lock_class, 536 &gpio_request_class); 537 pr_debug("IRQ Base: %d, Pin %d = IRQ %d\n", 538 chip->irq_base, pin_num, gpio_irq); 539 irq_set_chip_and_handler(gpio_irq, &xgpio_irqchip, 540 handle_simple_irq); 541 irq_set_chip_data(gpio_irq, (void *)chip); 542 } 543 irq_set_handler_data(res.start, (void *)chip); 544 irq_set_chained_handler(res.start, xgpio_irqhandler); 545 546 return 0; 547 } 548 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 37146 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gpio: xilinx: Add irq support to the driver 2020-02-17 10:57 ` [PATCH 2/2] gpio: xilinx: Add irq support to the driver Srinivas Neeli 2020-02-19 12:39 ` kbuild test robot 2020-02-19 15:00 ` kbuild test robot @ 2020-04-29 13:56 ` Daniel Mack 2 siblings, 0 replies; 7+ messages in thread From: Daniel Mack @ 2020-04-29 13:56 UTC (permalink / raw) To: Srinivas Neeli, bgolaszewski, michal.simek, shubhrajyoti.datta, sgoud Cc: linus.walleij, linux-kernel, linux-arm-kernel, linux-gpio, git Hi Srinivas, Thanks for these patches. We're using them on a custom board, and I have some remarks as they didn't work as intended. See below. On 2/17/20 11:57 AM, Srinivas Neeli wrote: > Allocate single chip for both channels. > Add irq support to the driver. > Supporting rising edge interrupts and in cascade mode supporting > first channel for interrupts on 32bit machines. > > Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com> > --- > drivers/gpio/gpio-xilinx.c | 233 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 232 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c > index 26753ae58295..f6dd316b2c62 100644 > --- a/drivers/gpio/gpio-xilinx.c > +++ b/drivers/gpio/gpio-xilinx.c [...] > /** > + * xgpiops_irq_mask - Write the specified signal of the GPIO device. > + * @irq_data: per irq and chip data passed down to chip functions > + */ > +static void xgpio_irq_mask(struct irq_data *irq_data) > +{ > + unsigned long flags; > + struct xgpio_instance *chip = irq_data_get_irq_chip_data(irq_data); > + u32 offset = irq_data->irq - chip->irq_base; > + u32 temp; > + s32 val; > + int index = xgpio_index(chip, 0); > + > + pr_debug("%s: Disable %d irq, irq_enable_mask 0x%x\n", > + __func__, offset, chip->irq_enable); > + > + spin_lock_irqsave(&chip->gpio_lock[index], flags); > + > + chip->irq_enable &= ~BIT(offset); > + > + if (!chip->irq_enable) { > + /* Enable per channel interrupt */ > + temp = xgpio_readreg(chip->regs + XGPIO_IPIER_OFFSET); > + val = offset - chip->gpio_width[0] + 1; > + if (val > 0) > + temp &= 1; > + else > + temp &= 2; This is a bit confusing. Why not write if (offset <= chip->gpio_width[0]) temp &= 1; else temp &= 2; ? > + xgpio_writereg(chip->regs + XGPIO_IPIER_OFFSET, temp); > + > + /* Disable global interrupt if channel interrupts are unused */ > + temp = xgpio_readreg(chip->regs + XGPIO_IPIER_OFFSET); You know that interrupts are unused when you get here, right? Why this extra check? > + if (!temp) > + xgpio_writereg(chip->regs + XGPIO_GIER_OFFSET, > + ~XGPIO_GIER_IE); > + } > + spin_unlock_irqrestore(&chip->gpio_lock[index], flags); > +} > + > +/** > + * xgpio_irq_unmask - Write the specified signal of the GPIO device. > + * @irq_data: per irq and chip data passed down to chip functions > + */ > +static void xgpio_irq_unmask(struct irq_data *irq_data) > +{ > + unsigned long flags; > + struct xgpio_instance *chip = irq_data_get_irq_chip_data(irq_data); > + u32 offset = irq_data->irq - chip->irq_base; > + u32 temp; > + s32 val; > + int index = xgpio_index(chip, 0); > + > + pr_debug("%s: Enable %d irq, irq_enable_mask 0x%x\n", > + __func__, offset, chip->irq_enable); > + > + /* Setup pin as input */ > + xgpio_dir_in(&chip->gc, offset); > + > + spin_lock_irqsave(&chip->gpio_lock[index], flags); > + > + chip->irq_enable |= BIT(offset); > + > + if (chip->irq_enable) { As you set a bit in the instruction above, this condition will always be true. So I guess the check can be omitted. > + /* Enable per channel interrupt */ > + temp = xgpio_readreg(chip->regs + XGPIO_IPIER_OFFSET); > + val = offset - (chip->gpio_width[0] - 1); This is different from the the statement in the mask function, but it can be simplified as noted above. > + if (val > 0) > + temp |= 2; > + else > + temp |= 1; > + xgpio_writereg(chip->regs + XGPIO_IPIER_OFFSET, temp); > + > + /* Enable global interrupts */ > + xgpio_writereg(chip->regs + XGPIO_GIER_OFFSET, XGPIO_GIER_IE); > + } > + > + spin_unlock_irqrestore(&chip->gpio_lock[index], flags); > +} [...] > +/** > + * xgpio_irqhandler - Gpio interrupt service routine > + * @desc: Pointer to interrupt description > + */ > +static void xgpio_irqhandler(struct irq_desc *desc) > +{ > + unsigned int irq = irq_desc_get_irq(desc); > + struct xgpio_instance *chip = (struct xgpio_instance *) > + irq_get_handler_data(irq); > + struct irq_chip *irqchip = irq_desc_get_chip(desc); > + u32 offset, status, channel = 1; > + unsigned long val; > + > + chained_irq_enter(irqchip, desc); > + > + val = xgpio_readreg(chip->regs); > + if (!val) { > + channel = 2; > + val = xgpio_readreg(chip->regs + XGPIO_CHANNEL_OFFSET); > + val = val << chip->gpio_width[0]; > + } > + > + /* Only rising edge is supported */ > + val &= chip->irq_enable; > + for_each_set_bit(offset, &val, chip->gc.ngpio) { > + generic_handle_irq(chip->irq_base + offset); This needs to include irq_find_mapping(chip->irq_domain, gpio). > + } > + > + status = xgpio_readreg(chip->regs + XGPIO_IPISR_OFFSET); The value assigned here is not used. Typo? > + xgpio_writereg(chip->regs + XGPIO_IPISR_OFFSET, channel); This function causes issues of general IRQ handling that makes the entire system deadlock for reasons I don't fully grok. I changed the logic to the following to make it work: 1. Read IPISR 2. Write the read value back to IPISR 3. Depending on the value of IPISR, read the state of either channel 1 or 2 4. chained_irq_enter() 5. Iterate over bits and call generic_handle_irq() 6. chained_irq_exit() > + > + chained_irq_exit(irqchip, desc); > +} > + > +static struct lock_class_key gpio_lock_class; > +static struct lock_class_key gpio_request_class; > + > +/** > + * xgpio_irq_setup - Allocate irq for gpio and setup appropriate functions > + * @np: Device node of the GPIO chip > + * @chip: Pointer to private gpio channel structure > + * > + * Return: > + * 0 if success, otherwise -1 > + */ > +static int xgpio_irq_setup(struct device_node *np, struct xgpio_instance *chip) > +{ > + u32 pin_num; > + struct resource res; > + int ret = of_irq_to_resource(np, 0, &res); > + > + if (ret <= 0) { > + pr_info("GPIO IRQ not connected\n"); > + return 0; > + } > + > + chip->gc.to_irq = xgpio_to_irq; > + chip->irq_base = irq_alloc_descs(-1, 0, chip->gc.ngpio, 0); This should use the devm_ variant to automatically free the resources. > + if (chip->irq_base < 0) { > + pr_err("Couldn't allocate IRQ numbers\n"); > + return -1; > + } > + chip->irq_domain = irq_domain_add_legacy(np, chip->gc.ngpio, > + chip->irq_base, 0, > + &irq_domain_simple_ops, NULL); This can fail, so the return value should be checked for NULL. > + /* > + * set the irq chip, handler and irq chip data for callbacks for > + * each pin > + */ > + for (pin_num = 0; pin_num < chip->gc.ngpio; pin_num++) { > + u32 gpio_irq = irq_find_mapping(chip->irq_domain, pin_num); > + > + irq_set_lockdep_class(gpio_irq, &gpio_lock_class, > + &gpio_request_class); > + pr_debug("IRQ Base: %d, Pin %d = IRQ %d\n", > + chip->irq_base, pin_num, gpio_irq); > + irq_set_chip_and_handler(gpio_irq, &xgpio_irqchip, > + handle_simple_irq); > + irq_set_chip_data(gpio_irq, (void *)chip); > + } > + irq_set_handler_data(res.start, (void *)chip); > + irq_set_chained_handler(res.start, xgpio_irqhandler); I guess all this can be achieved by setting chip->gc.irq* and let the GPIO core handle the IRQ chip allocation and setup. There are some examples in Documentation/driver-api/gpio/driver.rst. I'm happy to test the next iteration of these patches. Thanks, Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] gpio: xilinx: Add clock adaptation support 2020-02-17 10:57 [PATCH 1/2] gpio: xilinx: Add clock adaptation support Srinivas Neeli 2020-02-17 10:57 ` [PATCH 2/2] gpio: xilinx: Add irq support to the driver Srinivas Neeli @ 2020-02-18 16:02 ` Bartosz Golaszewski 2020-02-19 11:23 ` Srinivas Neeli 1 sibling, 1 reply; 7+ messages in thread From: Bartosz Golaszewski @ 2020-02-18 16:02 UTC (permalink / raw) To: Srinivas Neeli Cc: Michal Simek, shubhrajyoti.datta, sgoud, Linus Walleij, LKML, arm-soc, linux-gpio, git pon., 17 lut 2020 o 11:57 Srinivas Neeli <srinivas.neeli@xilinx.com> napisał(a): > > Add support of clock adaptation for AXI GPIO driver. > > Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com> > --- > drivers/gpio/gpio-xilinx.c | 105 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 103 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c > index a9748b5198e6..26753ae58295 100644 > --- a/drivers/gpio/gpio-xilinx.c > +++ b/drivers/gpio/gpio-xilinx.c > @@ -14,6 +14,8 @@ > #include <linux/io.h> > #include <linux/gpio/driver.h> > #include <linux/slab.h> > +#include <linux/pm_runtime.h> > +#include <linux/clk.h> > > /* Register Offset Definitions */ > #define XGPIO_DATA_OFFSET (0x0) /* Data register */ > @@ -38,6 +40,7 @@ > * @gpio_state: GPIO state shadow register > * @gpio_dir: GPIO direction shadow register > * @gpio_lock: Lock used for synchronization > + * @clk: clock resource for this driver > */ > struct xgpio_instance { > struct gpio_chip gc; > @@ -45,7 +48,8 @@ struct xgpio_instance { > unsigned int gpio_width[2]; > u32 gpio_state[2]; > u32 gpio_dir[2]; > - spinlock_t gpio_lock[2]; > + spinlock_t gpio_lock[2]; /* For serializing operations */ > + struct clk *clk; > }; > > static inline int xgpio_index(struct xgpio_instance *chip, int gpio) > @@ -255,6 +259,70 @@ static void xgpio_save_regs(struct xgpio_instance *chip) > chip->gpio_dir[1]); > } > > +static int xgpio_request(struct gpio_chip *chip, unsigned int offset) > +{ > + int ret = pm_runtime_get_sync(chip->parent); > + > + /* > + * If the device is already active pm_runtime_get() will return 1 on > + * success, but gpio_request still needs to return 0. > + */ > + return ret < 0 ? ret : 0; > +} > + > +static void xgpio_free(struct gpio_chip *chip, unsigned int offset) > +{ > + pm_runtime_put(chip->parent); > +} > + > +static int __maybe_unused xgpio_suspend(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + int irq = platform_get_irq(pdev, 0); > + struct irq_data *data = irq_get_irq_data(irq); > + > + if (!irqd_is_wakeup_set(data)) > + return pm_runtime_force_suspend(dev); > + > + return 0; > +} > + > +static int __maybe_unused xgpio_resume(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + int irq = platform_get_irq(pdev, 0); > + struct irq_data *data = irq_get_irq_data(irq); > + > + if (!irqd_is_wakeup_set(data)) > + return pm_runtime_force_resume(dev); > + > + return 0; > +} > + > +static int __maybe_unused xgpio_runtime_suspend(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct xgpio_instance *gpio = platform_get_drvdata(pdev); > + > + clk_disable(gpio->clk); > + > + return 0; > +} > + > +static int __maybe_unused xgpio_runtime_resume(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct xgpio_instance *gpio = platform_get_drvdata(pdev); > + > + return clk_enable(gpio->clk); > +} > + > +static const struct dev_pm_ops xgpio_dev_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(xgpio_suspend, xgpio_resume) > + SET_RUNTIME_PM_OPS(xgpio_runtime_suspend, > + xgpio_runtime_resume, NULL) > +}; > + > /** > * xgpio_of_probe - Probe method for the GPIO device. > * @pdev: pointer to the platform device > @@ -323,6 +391,8 @@ static int xgpio_probe(struct platform_device *pdev) > chip->gc.direction_output = xgpio_dir_out; > chip->gc.get = xgpio_get; > chip->gc.set = xgpio_set; > + chip->gc.request = xgpio_request; > + chip->gc.free = xgpio_free; > chip->gc.set_multiple = xgpio_set_multiple; > > chip->gc.label = dev_name(&pdev->dev); > @@ -333,15 +403,45 @@ static int xgpio_probe(struct platform_device *pdev) > return PTR_ERR(chip->regs); > } > > + chip->clk = devm_clk_get(&pdev->dev, "s_axi_aclk"); > + if (IS_ERR(chip->clk)) { > + if (PTR_ERR(chip->clk) != -ENOENT) { > + if (PTR_ERR(chip->clk) != -EPROBE_DEFER) > + dev_err(&pdev->dev, "Input clock not found\n"); > + return PTR_ERR(chip->clk); > + } > + /* > + * Clock framework support is optional, continue on > + * anyways if we don't find a matching clock. > + */ Why not use devm_clk_get_optional() then? > + chip->clk = NULL; > + } > + status = clk_prepare_enable(chip->clk); > + if (status < 0) { > + dev_err(&pdev->dev, "Failed to prepare clk\n"); > + return status; > + } > + pm_runtime_enable(&pdev->dev); > + status = pm_runtime_get_sync(&pdev->dev); > + if (status < 0) > + goto err_unprepare_clk; > + > xgpio_save_regs(chip); > > status = devm_gpiochip_add_data(&pdev->dev, &chip->gc, chip); > if (status) { > dev_err(&pdev->dev, "failed to add GPIO chip\n"); > - return status; > + goto err_pm_put; > } > > + pm_runtime_put(&pdev->dev); > return 0; > +err_pm_put: > + pm_runtime_put(&pdev->dev); > +err_unprepare_clk: > + pm_runtime_disable(&pdev->dev); > + clk_unprepare(chip->clk); > + return status; > } > > static const struct of_device_id xgpio_of_match[] = { > @@ -356,6 +456,7 @@ static struct platform_driver xgpio_plat_driver = { > .driver = { > .name = "gpio-xilinx", > .of_match_table = xgpio_of_match, > + .pm = &xgpio_dev_pm_ops, > }, > }; > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] gpio: xilinx: Add clock adaptation support 2020-02-18 16:02 ` [PATCH 1/2] gpio: xilinx: Add clock adaptation support Bartosz Golaszewski @ 2020-02-19 11:23 ` Srinivas Neeli 0 siblings, 0 replies; 7+ messages in thread From: Srinivas Neeli @ 2020-02-19 11:23 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Michal Simek, Shubhrajyoti Datta, Srinivas Goud, Linus Walleij, LKML, arm-soc, linux-gpio, git Hi, I agree ,will address comments in V2. > -----Original Message----- > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > Sent: Tuesday, February 18, 2020 9:33 PM > To: Srinivas Neeli <sneeli@xilinx.com> > Cc: Michal Simek <michals@xilinx.com>; Shubhrajyoti Datta > <shubhraj@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>; Linus Walleij > <linus.walleij@linaro.org>; LKML <linux-kernel@vger.kernel.org>; arm-soc > <linux-arm-kernel@lists.infradead.org>; linux-gpio <linux- > gpio@vger.kernel.org>; git <git@xilinx.com> > Subject: Re: [PATCH 1/2] gpio: xilinx: Add clock adaptation support > > pon., 17 lut 2020 o 11:57 Srinivas Neeli <srinivas.neeli@xilinx.com> napisał(a): > > > > Add support of clock adaptation for AXI GPIO driver. > > > > Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com> > > --- > > drivers/gpio/gpio-xilinx.c | 105 > > ++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 103 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c > > index a9748b5198e6..26753ae58295 100644 > > --- a/drivers/gpio/gpio-xilinx.c > > +++ b/drivers/gpio/gpio-xilinx.c > > @@ -14,6 +14,8 @@ > > #include <linux/io.h> > > #include <linux/gpio/driver.h> > > #include <linux/slab.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/clk.h> > > > > /* Register Offset Definitions */ > > #define XGPIO_DATA_OFFSET (0x0) /* Data register */ > > @@ -38,6 +40,7 @@ > > * @gpio_state: GPIO state shadow register > > * @gpio_dir: GPIO direction shadow register > > * @gpio_lock: Lock used for synchronization > > + * @clk: clock resource for this driver > > */ > > struct xgpio_instance { > > struct gpio_chip gc; > > @@ -45,7 +48,8 @@ struct xgpio_instance { > > unsigned int gpio_width[2]; > > u32 gpio_state[2]; > > u32 gpio_dir[2]; > > - spinlock_t gpio_lock[2]; > > + spinlock_t gpio_lock[2]; /* For serializing operations */ > > + struct clk *clk; > > }; > > > > static inline int xgpio_index(struct xgpio_instance *chip, int gpio) > > @@ -255,6 +259,70 @@ static void xgpio_save_regs(struct xgpio_instance > *chip) > > chip->gpio_dir[1]); } > > > > +static int xgpio_request(struct gpio_chip *chip, unsigned int offset) > > +{ > > + int ret = pm_runtime_get_sync(chip->parent); > > + > > + /* > > + * If the device is already active pm_runtime_get() will return 1 on > > + * success, but gpio_request still needs to return 0. > > + */ > > + return ret < 0 ? ret : 0; > > +} > > + > > +static void xgpio_free(struct gpio_chip *chip, unsigned int offset) { > > + pm_runtime_put(chip->parent); > > +} > > + > > +static int __maybe_unused xgpio_suspend(struct device *dev) { > > + struct platform_device *pdev = to_platform_device(dev); > > + int irq = platform_get_irq(pdev, 0); > > + struct irq_data *data = irq_get_irq_data(irq); > > + > > + if (!irqd_is_wakeup_set(data)) > > + return pm_runtime_force_suspend(dev); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused xgpio_resume(struct device *dev) { > > + struct platform_device *pdev = to_platform_device(dev); > > + int irq = platform_get_irq(pdev, 0); > > + struct irq_data *data = irq_get_irq_data(irq); > > + > > + if (!irqd_is_wakeup_set(data)) > > + return pm_runtime_force_resume(dev); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused xgpio_runtime_suspend(struct device *dev) { > > + struct platform_device *pdev = to_platform_device(dev); > > + struct xgpio_instance *gpio = platform_get_drvdata(pdev); > > + > > + clk_disable(gpio->clk); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused xgpio_runtime_resume(struct device *dev) { > > + struct platform_device *pdev = to_platform_device(dev); > > + struct xgpio_instance *gpio = platform_get_drvdata(pdev); > > + > > + return clk_enable(gpio->clk); > > +} > > + > > +static const struct dev_pm_ops xgpio_dev_pm_ops = { > > + SET_SYSTEM_SLEEP_PM_OPS(xgpio_suspend, xgpio_resume) > > + SET_RUNTIME_PM_OPS(xgpio_runtime_suspend, > > + xgpio_runtime_resume, NULL) }; > > + > > /** > > * xgpio_of_probe - Probe method for the GPIO device. > > * @pdev: pointer to the platform device @@ -323,6 +391,8 @@ static > > int xgpio_probe(struct platform_device *pdev) > > chip->gc.direction_output = xgpio_dir_out; > > chip->gc.get = xgpio_get; > > chip->gc.set = xgpio_set; > > + chip->gc.request = xgpio_request; > > + chip->gc.free = xgpio_free; > > chip->gc.set_multiple = xgpio_set_multiple; > > > > chip->gc.label = dev_name(&pdev->dev); @@ -333,15 +403,45 @@ > > static int xgpio_probe(struct platform_device *pdev) > > return PTR_ERR(chip->regs); > > } > > > > + chip->clk = devm_clk_get(&pdev->dev, "s_axi_aclk"); > > + if (IS_ERR(chip->clk)) { > > + if (PTR_ERR(chip->clk) != -ENOENT) { > > + if (PTR_ERR(chip->clk) != -EPROBE_DEFER) > > + dev_err(&pdev->dev, "Input clock not found\n"); > > + return PTR_ERR(chip->clk); > > + } > > + /* > > + * Clock framework support is optional, continue on > > + * anyways if we don't find a matching clock. > > + */ > > Why not use devm_clk_get_optional() then? > > > + chip->clk = NULL; > > + } > > + status = clk_prepare_enable(chip->clk); > > + if (status < 0) { > > + dev_err(&pdev->dev, "Failed to prepare clk\n"); > > + return status; > > + } > > + pm_runtime_enable(&pdev->dev); > > + status = pm_runtime_get_sync(&pdev->dev); > > + if (status < 0) > > + goto err_unprepare_clk; > > + > > xgpio_save_regs(chip); > > > > status = devm_gpiochip_add_data(&pdev->dev, &chip->gc, chip); > > if (status) { > > dev_err(&pdev->dev, "failed to add GPIO chip\n"); > > - return status; > > + goto err_pm_put; > > } > > > > + pm_runtime_put(&pdev->dev); > > return 0; > > +err_pm_put: > > + pm_runtime_put(&pdev->dev); > > +err_unprepare_clk: > > + pm_runtime_disable(&pdev->dev); > > + clk_unprepare(chip->clk); > > + return status; > > } > > > > static const struct of_device_id xgpio_of_match[] = { @@ -356,6 > > +456,7 @@ static struct platform_driver xgpio_plat_driver = { > > .driver = { > > .name = "gpio-xilinx", > > .of_match_table = xgpio_of_match, > > + .pm = &xgpio_dev_pm_ops, > > }, > > }; > > > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-29 14:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-17 10:57 [PATCH 1/2] gpio: xilinx: Add clock adaptation support Srinivas Neeli 2020-02-17 10:57 ` [PATCH 2/2] gpio: xilinx: Add irq support to the driver Srinivas Neeli 2020-02-19 12:39 ` kbuild test robot 2020-02-19 15:00 ` kbuild test robot 2020-04-29 13:56 ` Daniel Mack 2020-02-18 16:02 ` [PATCH 1/2] gpio: xilinx: Add clock adaptation support Bartosz Golaszewski 2020-02-19 11:23 ` Srinivas Neeli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).