* [PATCH 0/3] Add gpio interrupt support for Actions Semi S900 SoC @ 2018-06-02 16:54 ` Manivannan Sadhasivam 0 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linus.walleij, robh+dt, afaerber Cc: liuwei, mp-cs, 96boards, devicetree, andy.shevchenko, daniel.thompson, amit.kucheria, linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, Manivannan Sadhasivam This patchset adds interrupt support for Actions Semi S900 GPIO's. Each port has individual register sets for configuring the below interrupt types: 1. Rising Edge Interrupt 2. Falling Edge Interrupt 3. High Level Interrupt 4. Low Level Interrupt Thanks, Mani Manivannan Sadhasivam (3): dt-bindings: pinctrl: Add gpio interrupt bindings for Actions S900 SoC arm64: dts: actions: Add interrupt properties to pinctrl node for S900 pinctrl: actions: Add interrupt support for OWL S900 SoC .../bindings/pinctrl/actions,s900-pinctrl.txt | 10 + arch/arm64/boot/dts/actions/s900.dtsi | 8 + drivers/pinctrl/actions/Kconfig | 1 + drivers/pinctrl/actions/pinctrl-owl.c | 231 +++++++++++++++++- drivers/pinctrl/actions/pinctrl-owl.h | 22 +- drivers/pinctrl/actions/pinctrl-s900.c | 31 ++- 6 files changed, 285 insertions(+), 18 deletions(-) -- 2.17.0 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] Add gpio interrupt support for Actions Semi S900 SoC @ 2018-06-02 16:54 ` Manivannan Sadhasivam 0 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linux-arm-kernel This patchset adds interrupt support for Actions Semi S900 GPIO's. Each port has individual register sets for configuring the below interrupt types: 1. Rising Edge Interrupt 2. Falling Edge Interrupt 3. High Level Interrupt 4. Low Level Interrupt Thanks, Mani Manivannan Sadhasivam (3): dt-bindings: pinctrl: Add gpio interrupt bindings for Actions S900 SoC arm64: dts: actions: Add interrupt properties to pinctrl node for S900 pinctrl: actions: Add interrupt support for OWL S900 SoC .../bindings/pinctrl/actions,s900-pinctrl.txt | 10 + arch/arm64/boot/dts/actions/s900.dtsi | 8 + drivers/pinctrl/actions/Kconfig | 1 + drivers/pinctrl/actions/pinctrl-owl.c | 231 +++++++++++++++++- drivers/pinctrl/actions/pinctrl-owl.h | 22 +- drivers/pinctrl/actions/pinctrl-s900.c | 31 ++- 6 files changed, 285 insertions(+), 18 deletions(-) -- 2.17.0 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] dt-bindings: pinctrl: Add gpio interrupt bindings for Actions S900 SoC 2018-06-02 16:54 ` Manivannan Sadhasivam @ 2018-06-02 16:54 ` Manivannan Sadhasivam -1 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linus.walleij, robh+dt, afaerber Cc: liuwei, mp-cs, 96boards, devicetree, andy.shevchenko, daniel.thompson, amit.kucheria, linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, Manivannan Sadhasivam Add gpio interrupt bindings for Actions Semi S900 SoC. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- .../bindings/pinctrl/actions,s900-pinctrl.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt index 8fb5a53775e8..81b58dddd3ed 100644 --- a/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt @@ -19,6 +19,10 @@ Required Properties: defines the interrupt number, the second encodes the trigger flags described in bindings/interrupt-controller/interrupts.txt +- interrupts: The interrupt outputs from the controller. There is one GPIO + interrupt per GPIO bank. The number of interrupts listed depends + on the number of GPIO banks on the SoC. The interrupts must be + ordered by bank, starting with bank 0. Please refer to pinctrl-bindings.txt in this directory for details of the common pinctrl bindings used by client devices, including the meaning of the @@ -180,6 +184,12 @@ Example: #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; uart2-default: uart2-default { pinmux { -- 2.17.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/3] dt-bindings: pinctrl: Add gpio interrupt bindings for Actions S900 SoC @ 2018-06-02 16:54 ` Manivannan Sadhasivam 0 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linux-arm-kernel Add gpio interrupt bindings for Actions Semi S900 SoC. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- .../bindings/pinctrl/actions,s900-pinctrl.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt index 8fb5a53775e8..81b58dddd3ed 100644 --- a/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt @@ -19,6 +19,10 @@ Required Properties: defines the interrupt number, the second encodes the trigger flags described in bindings/interrupt-controller/interrupts.txt +- interrupts: The interrupt outputs from the controller. There is one GPIO + interrupt per GPIO bank. The number of interrupts listed depends + on the number of GPIO banks on the SoC. The interrupts must be + ordered by bank, starting with bank 0. Please refer to pinctrl-bindings.txt in this directory for details of the common pinctrl bindings used by client devices, including the meaning of the @@ -180,6 +184,12 @@ Example: #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; uart2-default: uart2-default { pinmux { -- 2.17.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] dt-bindings: pinctrl: Add gpio interrupt bindings for Actions S900 SoC 2018-06-02 16:54 ` Manivannan Sadhasivam @ 2018-06-12 20:58 ` Rob Herring -1 siblings, 0 replies; 16+ messages in thread From: Rob Herring @ 2018-06-12 20:58 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: linus.walleij, afaerber, liuwei, mp-cs, 96boards, devicetree, andy.shevchenko, daniel.thompson, amit.kucheria, linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong, manivannanece23, thomas.liau, jeff.chen On Sat, Jun 02, 2018 at 10:24:13PM +0530, Manivannan Sadhasivam wrote: > Add gpio interrupt bindings for Actions Semi S900 SoC. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > .../bindings/pinctrl/actions,s900-pinctrl.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] dt-bindings: pinctrl: Add gpio interrupt bindings for Actions S900 SoC @ 2018-06-12 20:58 ` Rob Herring 0 siblings, 0 replies; 16+ messages in thread From: Rob Herring @ 2018-06-12 20:58 UTC (permalink / raw) To: linux-arm-kernel On Sat, Jun 02, 2018 at 10:24:13PM +0530, Manivannan Sadhasivam wrote: > Add gpio interrupt bindings for Actions Semi S900 SoC. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > .../bindings/pinctrl/actions,s900-pinctrl.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] arm64: dts: actions: Add interrupt properties to pinctrl node for S900 2018-06-02 16:54 ` Manivannan Sadhasivam @ 2018-06-02 16:54 ` Manivannan Sadhasivam -1 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linus.walleij, robh+dt, afaerber Cc: liuwei, mp-cs, 96boards, devicetree, andy.shevchenko, daniel.thompson, amit.kucheria, linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, Manivannan Sadhasivam Add interrupt properties to pinctrl node for Actions Semi S900 SoC. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- arch/arm64/boot/dts/actions/s900.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/actions/s900.dtsi b/arch/arm64/boot/dts/actions/s900.dtsi index aa3a49b0d646..7ae8b931f000 100644 --- a/arch/arm64/boot/dts/actions/s900.dtsi +++ b/arch/arm64/boot/dts/actions/s900.dtsi @@ -181,6 +181,14 @@ gpio-controller; gpio-ranges = <&pinctrl 0 0 146>; #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; }; timer: timer@e0228000 { -- 2.17.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] arm64: dts: actions: Add interrupt properties to pinctrl node for S900 @ 2018-06-02 16:54 ` Manivannan Sadhasivam 0 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linux-arm-kernel Add interrupt properties to pinctrl node for Actions Semi S900 SoC. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- arch/arm64/boot/dts/actions/s900.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/actions/s900.dtsi b/arch/arm64/boot/dts/actions/s900.dtsi index aa3a49b0d646..7ae8b931f000 100644 --- a/arch/arm64/boot/dts/actions/s900.dtsi +++ b/arch/arm64/boot/dts/actions/s900.dtsi @@ -181,6 +181,14 @@ gpio-controller; gpio-ranges = <&pinctrl 0 0 146>; #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; }; timer: timer at e0228000 { -- 2.17.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC 2018-06-02 16:54 ` Manivannan Sadhasivam @ 2018-06-02 16:54 ` Manivannan Sadhasivam -1 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linus.walleij, robh+dt, afaerber Cc: liuwei, mp-cs, 96boards, devicetree, andy.shevchenko, daniel.thompson, amit.kucheria, linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong, manivannanece23, thomas.liau, jeff.chen, Manivannan Sadhasivam Add interrupt support for Actions Semi OWL S900 SoC. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pinctrl/actions/Kconfig | 1 + drivers/pinctrl/actions/pinctrl-owl.c | 231 ++++++++++++++++++++++++- drivers/pinctrl/actions/pinctrl-owl.h | 22 ++- drivers/pinctrl/actions/pinctrl-s900.c | 31 ++-- 4 files changed, 267 insertions(+), 18 deletions(-) diff --git a/drivers/pinctrl/actions/Kconfig b/drivers/pinctrl/actions/Kconfig index 490927b4ea76..2397cb0f6011 100644 --- a/drivers/pinctrl/actions/Kconfig +++ b/drivers/pinctrl/actions/Kconfig @@ -5,6 +5,7 @@ config PINCTRL_OWL select PINCONF select GENERIC_PINCONF select GPIOLIB + select GPIOLIB_IRQCHIP help Say Y here to enable Actions Semi OWL pinctrl driver diff --git a/drivers/pinctrl/actions/pinctrl-owl.c b/drivers/pinctrl/actions/pinctrl-owl.c index 76243caa08c6..d40c61caeea3 100644 --- a/drivers/pinctrl/actions/pinctrl-owl.c +++ b/drivers/pinctrl/actions/pinctrl-owl.c @@ -45,6 +45,8 @@ struct owl_pinctrl { struct clk *clk; const struct owl_pinctrl_soc_data *soc; void __iomem *base; + struct irq_domain *domain; + int *irq; }; static void owl_update_bits(void __iomem *base, u32 mask, u32 val) @@ -701,16 +703,193 @@ static int owl_gpio_direction_output(struct gpio_chip *chip, return 0; } +static int owl_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) +{ + struct owl_pinctrl *pctrl = gpiochip_get_data(chip); + + return irq_find_mapping(pctrl->domain, offset); +} + +static void owl_gpio_irq_mask(struct irq_data *data) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + u32 val; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return; + + gpio_base = pctrl->base + port->offset; + + raw_spin_lock_irqsave(&pctrl->lock, flags); + + owl_gpio_update_reg(gpio_base + port->intc_msk, gpio, false); + + /* disable port interrupt if no interrupt pending bit is active */ + val = readl_relaxed(gpio_base + port->intc_msk); + if (val == 0) + owl_gpio_update_reg(gpio_base + port->intc_ctl, + OWL_GPIO_CTLR_ENABLE, false); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static void owl_gpio_irq_unmask(struct irq_data *data) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + u32 value; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return; + + gpio_base = pctrl->base + port->offset; + raw_spin_lock_irqsave(&pctrl->lock, flags); + + /* enable port interrupt */ + value = readl_relaxed(gpio_base + port->intc_ctl); + value |= BIT(OWL_GPIO_CTLR_ENABLE) | BIT(OWL_GPIO_CTLR_SAMPLE_CLK_24M); + writel_relaxed(value, gpio_base + port->intc_ctl); + + /* enable GPIO interrupt */ + owl_gpio_update_reg(gpio_base + port->intc_msk, gpio, true); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static void owl_gpio_irq_ack(struct irq_data *data) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return; + + gpio_base = pctrl->base + port->offset; + + raw_spin_lock_irqsave(&pctrl->lock, flags); + + owl_gpio_update_reg(gpio_base + port->intc_ctl, + OWL_GPIO_CTLR_PENDING, true); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static int owl_gpio_irq_set_type(struct irq_data *data, unsigned int type) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + unsigned int offset, value, irq_type = 0; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return -ENODEV; + + gpio_base = pctrl->base + port->offset; + + switch (type) { + case IRQ_TYPE_EDGE_RISING: + irq_type = OWL_GPIO_INT_EDGE_RISING; + break; + + case IRQ_TYPE_EDGE_FALLING: + irq_type = OWL_GPIO_INT_EDGE_FALLING; + break; + + case IRQ_TYPE_LEVEL_HIGH: + irq_type = OWL_GPIO_INT_LEVEL_HIGH; + break; + + case IRQ_TYPE_LEVEL_LOW: + irq_type = OWL_GPIO_INT_LEVEL_LOW; + break; + + default: + return -EINVAL; + } + + raw_spin_lock_irqsave(&pctrl->lock, flags); + + offset = gpio < 16 ? 4 : 0; + value = readl_relaxed(gpio_base + port->intc_type + offset); + value &= ~(OWL_GPIO_INT_MASK << ((gpio % 16) * 2)); + value |= irq_type << ((gpio % 16) * 2); + writel_relaxed(value, gpio_base + port->intc_type + offset); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); + + if (type & IRQ_TYPE_EDGE_BOTH) + irq_set_handler_locked(data, handle_edge_irq); + else + irq_set_handler_locked(data, handle_level_irq); + + return 0; +} + +static void owl_gpio_irq_handler(struct irq_desc *desc) +{ + struct owl_pinctrl *pctrl = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + const struct owl_gpio_port *port; + void __iomem *base; + unsigned int pin, irq, offset = 0, i; + unsigned long pending_irq; + + chained_irq_enter(chip, desc); + + for (i = 0; i < pctrl->soc->nports; i++) { + port = &pctrl->soc->ports[i]; + base = pctrl->base + port->offset; + pending_irq = readl_relaxed(base + port->intc_pd); + + for_each_set_bit(pin, &pending_irq, port->pins) { + irq = irq_find_mapping(pctrl->domain, offset + pin); + generic_handle_irq(irq); + + /* clear pending interrupt */ + owl_gpio_update_reg(base + port->intc_pd, pin, true); + } + + offset += port->pins; + } + + chained_irq_exit(chip, desc); +} + +static struct irq_chip owl_gpio_irq_chip = { + .name = "owlgpio", + .irq_mask = owl_gpio_irq_mask, + .irq_unmask = owl_gpio_irq_unmask, + .irq_ack = owl_gpio_irq_ack, + .irq_set_type = owl_gpio_irq_set_type, +}; + static int owl_gpio_init(struct owl_pinctrl *pctrl) { struct gpio_chip *chip; - int ret; + int irqno, ret, i; chip = &pctrl->chip; chip->base = -1; chip->ngpio = pctrl->soc->ngpios; chip->label = dev_name(pctrl->dev); chip->parent = pctrl->dev; + chip->to_irq = owl_gpio_to_irq; chip->owner = THIS_MODULE; chip->of_node = pctrl->dev->of_node; @@ -720,6 +899,29 @@ static int owl_gpio_init(struct owl_pinctrl *pctrl) return ret; } + pctrl->domain = irq_domain_add_linear(chip->of_node, + chip->ngpio, + &irq_domain_simple_ops, + NULL); + if (!pctrl->domain) { + dev_err(pctrl->dev, "Couldn't register IRQ domain\n"); + gpiochip_remove(&pctrl->chip); + return -ENOMEM; + } + + for (i = 0; i < chip->ngpio; i++) { + irqno = irq_create_mapping(pctrl->domain, i); + irq_set_chip_and_handler(irqno, &owl_gpio_irq_chip, + handle_edge_irq); + irq_set_chip_data(irqno, pctrl); + } + + for (i = 0; i < pctrl->soc->nports; i++) { + irq_set_chained_handler_and_data(pctrl->irq[i], + owl_gpio_irq_handler, + pctrl); + } + return 0; } @@ -728,7 +930,7 @@ int owl_pinctrl_probe(struct platform_device *pdev, { struct resource *res; struct owl_pinctrl *pctrl; - int ret; + int ret, i; pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL); if (!pctrl) @@ -772,14 +974,35 @@ int owl_pinctrl_probe(struct platform_device *pdev, &owl_pinctrl_desc, pctrl); if (IS_ERR(pctrl->pctrldev)) { dev_err(&pdev->dev, "could not register Actions OWL pinmux driver\n"); - return PTR_ERR(pctrl->pctrldev); + ret = PTR_ERR(pctrl->pctrldev); + goto err_exit; + } + + pctrl->irq = devm_kcalloc(&pdev->dev, pctrl->soc->nports, + sizeof(*pctrl->irq), GFP_KERNEL); + if (!pctrl->irq) { + ret = -ENOMEM; + goto err_exit; + } + + for (i = 0; i < pctrl->soc->nports ; i++) { + pctrl->irq[i] = platform_get_irq(pdev, i); + if (pctrl->irq[i] < 0) { + ret = pctrl->irq[i]; + goto err_exit; + } } ret = owl_gpio_init(pctrl); if (ret) - return ret; + goto err_exit; platform_set_drvdata(pdev, pctrl); return 0; + +err_exit: + clk_disable_unprepare(pctrl->clk); + + return ret; } diff --git a/drivers/pinctrl/actions/pinctrl-owl.h b/drivers/pinctrl/actions/pinctrl-owl.h index 74342378937c..a724d1d406d4 100644 --- a/drivers/pinctrl/actions/pinctrl-owl.h +++ b/drivers/pinctrl/actions/pinctrl-owl.h @@ -29,6 +29,18 @@ enum owl_pinconf_drv { OWL_PINCONF_DRV_12MA, }; +/* GPIO CTRL Bit Definition */ +#define OWL_GPIO_CTLR_PENDING 0 +#define OWL_GPIO_CTLR_ENABLE 1 +#define OWL_GPIO_CTLR_SAMPLE_CLK_24M 2 + +/* GPIO TYPE Bit Definition */ +#define OWL_GPIO_INT_LEVEL_HIGH 0 +#define OWL_GPIO_INT_LEVEL_LOW 1 +#define OWL_GPIO_INT_EDGE_RISING 2 +#define OWL_GPIO_INT_EDGE_FALLING 3 +#define OWL_GPIO_INT_MASK 3 + /** * struct owl_pullctl - Actions pad pull control register * @reg: offset to the pull control register @@ -121,6 +133,10 @@ struct owl_pinmux_func { * @outen: offset of the output enable register. * @inen: offset of the input enable register. * @dat: offset of the data register. + * @intc_ctl: offset of the interrupt control register. + * @intc_pd: offset of the interrupt pending register. + * @intc_msk: offset of the interrupt mask register. + * @intc_type: offset of the interrupt type register. */ struct owl_gpio_port { unsigned int offset; @@ -128,6 +144,10 @@ struct owl_gpio_port { unsigned int outen; unsigned int inen; unsigned int dat; + unsigned int intc_ctl; + unsigned int intc_pd; + unsigned int intc_msk; + unsigned int intc_type; }; /** @@ -140,7 +160,7 @@ struct owl_gpio_port { * @ngroups: number of entries in @groups. * @padinfo: array describing the pad info of this SoC. * @ngpios: number of pingroups the driver should expose as GPIOs. - * @port: array describing all GPIO ports of this SoC. + * @ports: array describing all GPIO ports of this SoC. * @nports: number of GPIO ports in this SoC. */ struct owl_pinctrl_soc_data { diff --git a/drivers/pinctrl/actions/pinctrl-s900.c b/drivers/pinctrl/actions/pinctrl-s900.c index 5503c7945764..ea67b14ef93b 100644 --- a/drivers/pinctrl/actions/pinctrl-s900.c +++ b/drivers/pinctrl/actions/pinctrl-s900.c @@ -1821,22 +1821,27 @@ static struct owl_padinfo s900_padinfo[NUM_PADS] = { [SGPIO3] = PAD_INFO_PULLCTL_ST(SGPIO3) }; -#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat) \ - [OWL_GPIO_PORT_##port] = { \ - .offset = base, \ - .pins = count, \ - .outen = _outen, \ - .inen = _inen, \ - .dat = _dat, \ +#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat, \ + _intc_ctl, _intc_pd, _intc_msk, _intc_type) \ + [OWL_GPIO_PORT_##port] = { \ + .offset = base, \ + .pins = count, \ + .outen = _outen, \ + .inen = _inen, \ + .dat = _dat, \ + .intc_ctl = _intc_ctl, \ + .intc_pd = _intc_pd, \ + .intc_msk = _intc_msk, \ + .intc_type = _intc_type, \ } static const struct owl_gpio_port s900_gpio_ports[] = { - OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8) + OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8, 0x204, 0x208, 0x20C, 0x240), + OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8, 0x534, 0x204, 0x208, 0x23C), + OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8, 0x52C, 0x200, 0x204, 0x238), + OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8, 0x524, 0x1FC, 0x200, 0x234), + OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8, 0x51C, 0x1F8, 0x1FC, 0x230), + OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8, 0x460, 0x140, 0x144, 0x178) }; static struct owl_pinctrl_soc_data s900_pinctrl_data = { -- 2.17.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC @ 2018-06-02 16:54 ` Manivannan Sadhasivam 0 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-02 16:54 UTC (permalink / raw) To: linux-arm-kernel Add interrupt support for Actions Semi OWL S900 SoC. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pinctrl/actions/Kconfig | 1 + drivers/pinctrl/actions/pinctrl-owl.c | 231 ++++++++++++++++++++++++- drivers/pinctrl/actions/pinctrl-owl.h | 22 ++- drivers/pinctrl/actions/pinctrl-s900.c | 31 ++-- 4 files changed, 267 insertions(+), 18 deletions(-) diff --git a/drivers/pinctrl/actions/Kconfig b/drivers/pinctrl/actions/Kconfig index 490927b4ea76..2397cb0f6011 100644 --- a/drivers/pinctrl/actions/Kconfig +++ b/drivers/pinctrl/actions/Kconfig @@ -5,6 +5,7 @@ config PINCTRL_OWL select PINCONF select GENERIC_PINCONF select GPIOLIB + select GPIOLIB_IRQCHIP help Say Y here to enable Actions Semi OWL pinctrl driver diff --git a/drivers/pinctrl/actions/pinctrl-owl.c b/drivers/pinctrl/actions/pinctrl-owl.c index 76243caa08c6..d40c61caeea3 100644 --- a/drivers/pinctrl/actions/pinctrl-owl.c +++ b/drivers/pinctrl/actions/pinctrl-owl.c @@ -45,6 +45,8 @@ struct owl_pinctrl { struct clk *clk; const struct owl_pinctrl_soc_data *soc; void __iomem *base; + struct irq_domain *domain; + int *irq; }; static void owl_update_bits(void __iomem *base, u32 mask, u32 val) @@ -701,16 +703,193 @@ static int owl_gpio_direction_output(struct gpio_chip *chip, return 0; } +static int owl_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) +{ + struct owl_pinctrl *pctrl = gpiochip_get_data(chip); + + return irq_find_mapping(pctrl->domain, offset); +} + +static void owl_gpio_irq_mask(struct irq_data *data) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + u32 val; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return; + + gpio_base = pctrl->base + port->offset; + + raw_spin_lock_irqsave(&pctrl->lock, flags); + + owl_gpio_update_reg(gpio_base + port->intc_msk, gpio, false); + + /* disable port interrupt if no interrupt pending bit is active */ + val = readl_relaxed(gpio_base + port->intc_msk); + if (val == 0) + owl_gpio_update_reg(gpio_base + port->intc_ctl, + OWL_GPIO_CTLR_ENABLE, false); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static void owl_gpio_irq_unmask(struct irq_data *data) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + u32 value; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return; + + gpio_base = pctrl->base + port->offset; + raw_spin_lock_irqsave(&pctrl->lock, flags); + + /* enable port interrupt */ + value = readl_relaxed(gpio_base + port->intc_ctl); + value |= BIT(OWL_GPIO_CTLR_ENABLE) | BIT(OWL_GPIO_CTLR_SAMPLE_CLK_24M); + writel_relaxed(value, gpio_base + port->intc_ctl); + + /* enable GPIO interrupt */ + owl_gpio_update_reg(gpio_base + port->intc_msk, gpio, true); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static void owl_gpio_irq_ack(struct irq_data *data) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return; + + gpio_base = pctrl->base + port->offset; + + raw_spin_lock_irqsave(&pctrl->lock, flags); + + owl_gpio_update_reg(gpio_base + port->intc_ctl, + OWL_GPIO_CTLR_PENDING, true); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static int owl_gpio_irq_set_type(struct irq_data *data, unsigned int type) +{ + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); + const struct owl_gpio_port *port; + void __iomem *gpio_base; + unsigned long flags; + unsigned int gpio = data->hwirq; + unsigned int offset, value, irq_type = 0; + + port = owl_gpio_get_port(pctrl, &gpio); + if (WARN_ON(port == NULL)) + return -ENODEV; + + gpio_base = pctrl->base + port->offset; + + switch (type) { + case IRQ_TYPE_EDGE_RISING: + irq_type = OWL_GPIO_INT_EDGE_RISING; + break; + + case IRQ_TYPE_EDGE_FALLING: + irq_type = OWL_GPIO_INT_EDGE_FALLING; + break; + + case IRQ_TYPE_LEVEL_HIGH: + irq_type = OWL_GPIO_INT_LEVEL_HIGH; + break; + + case IRQ_TYPE_LEVEL_LOW: + irq_type = OWL_GPIO_INT_LEVEL_LOW; + break; + + default: + return -EINVAL; + } + + raw_spin_lock_irqsave(&pctrl->lock, flags); + + offset = gpio < 16 ? 4 : 0; + value = readl_relaxed(gpio_base + port->intc_type + offset); + value &= ~(OWL_GPIO_INT_MASK << ((gpio % 16) * 2)); + value |= irq_type << ((gpio % 16) * 2); + writel_relaxed(value, gpio_base + port->intc_type + offset); + + raw_spin_unlock_irqrestore(&pctrl->lock, flags); + + if (type & IRQ_TYPE_EDGE_BOTH) + irq_set_handler_locked(data, handle_edge_irq); + else + irq_set_handler_locked(data, handle_level_irq); + + return 0; +} + +static void owl_gpio_irq_handler(struct irq_desc *desc) +{ + struct owl_pinctrl *pctrl = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + const struct owl_gpio_port *port; + void __iomem *base; + unsigned int pin, irq, offset = 0, i; + unsigned long pending_irq; + + chained_irq_enter(chip, desc); + + for (i = 0; i < pctrl->soc->nports; i++) { + port = &pctrl->soc->ports[i]; + base = pctrl->base + port->offset; + pending_irq = readl_relaxed(base + port->intc_pd); + + for_each_set_bit(pin, &pending_irq, port->pins) { + irq = irq_find_mapping(pctrl->domain, offset + pin); + generic_handle_irq(irq); + + /* clear pending interrupt */ + owl_gpio_update_reg(base + port->intc_pd, pin, true); + } + + offset += port->pins; + } + + chained_irq_exit(chip, desc); +} + +static struct irq_chip owl_gpio_irq_chip = { + .name = "owlgpio", + .irq_mask = owl_gpio_irq_mask, + .irq_unmask = owl_gpio_irq_unmask, + .irq_ack = owl_gpio_irq_ack, + .irq_set_type = owl_gpio_irq_set_type, +}; + static int owl_gpio_init(struct owl_pinctrl *pctrl) { struct gpio_chip *chip; - int ret; + int irqno, ret, i; chip = &pctrl->chip; chip->base = -1; chip->ngpio = pctrl->soc->ngpios; chip->label = dev_name(pctrl->dev); chip->parent = pctrl->dev; + chip->to_irq = owl_gpio_to_irq; chip->owner = THIS_MODULE; chip->of_node = pctrl->dev->of_node; @@ -720,6 +899,29 @@ static int owl_gpio_init(struct owl_pinctrl *pctrl) return ret; } + pctrl->domain = irq_domain_add_linear(chip->of_node, + chip->ngpio, + &irq_domain_simple_ops, + NULL); + if (!pctrl->domain) { + dev_err(pctrl->dev, "Couldn't register IRQ domain\n"); + gpiochip_remove(&pctrl->chip); + return -ENOMEM; + } + + for (i = 0; i < chip->ngpio; i++) { + irqno = irq_create_mapping(pctrl->domain, i); + irq_set_chip_and_handler(irqno, &owl_gpio_irq_chip, + handle_edge_irq); + irq_set_chip_data(irqno, pctrl); + } + + for (i = 0; i < pctrl->soc->nports; i++) { + irq_set_chained_handler_and_data(pctrl->irq[i], + owl_gpio_irq_handler, + pctrl); + } + return 0; } @@ -728,7 +930,7 @@ int owl_pinctrl_probe(struct platform_device *pdev, { struct resource *res; struct owl_pinctrl *pctrl; - int ret; + int ret, i; pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL); if (!pctrl) @@ -772,14 +974,35 @@ int owl_pinctrl_probe(struct platform_device *pdev, &owl_pinctrl_desc, pctrl); if (IS_ERR(pctrl->pctrldev)) { dev_err(&pdev->dev, "could not register Actions OWL pinmux driver\n"); - return PTR_ERR(pctrl->pctrldev); + ret = PTR_ERR(pctrl->pctrldev); + goto err_exit; + } + + pctrl->irq = devm_kcalloc(&pdev->dev, pctrl->soc->nports, + sizeof(*pctrl->irq), GFP_KERNEL); + if (!pctrl->irq) { + ret = -ENOMEM; + goto err_exit; + } + + for (i = 0; i < pctrl->soc->nports ; i++) { + pctrl->irq[i] = platform_get_irq(pdev, i); + if (pctrl->irq[i] < 0) { + ret = pctrl->irq[i]; + goto err_exit; + } } ret = owl_gpio_init(pctrl); if (ret) - return ret; + goto err_exit; platform_set_drvdata(pdev, pctrl); return 0; + +err_exit: + clk_disable_unprepare(pctrl->clk); + + return ret; } diff --git a/drivers/pinctrl/actions/pinctrl-owl.h b/drivers/pinctrl/actions/pinctrl-owl.h index 74342378937c..a724d1d406d4 100644 --- a/drivers/pinctrl/actions/pinctrl-owl.h +++ b/drivers/pinctrl/actions/pinctrl-owl.h @@ -29,6 +29,18 @@ enum owl_pinconf_drv { OWL_PINCONF_DRV_12MA, }; +/* GPIO CTRL Bit Definition */ +#define OWL_GPIO_CTLR_PENDING 0 +#define OWL_GPIO_CTLR_ENABLE 1 +#define OWL_GPIO_CTLR_SAMPLE_CLK_24M 2 + +/* GPIO TYPE Bit Definition */ +#define OWL_GPIO_INT_LEVEL_HIGH 0 +#define OWL_GPIO_INT_LEVEL_LOW 1 +#define OWL_GPIO_INT_EDGE_RISING 2 +#define OWL_GPIO_INT_EDGE_FALLING 3 +#define OWL_GPIO_INT_MASK 3 + /** * struct owl_pullctl - Actions pad pull control register * @reg: offset to the pull control register @@ -121,6 +133,10 @@ struct owl_pinmux_func { * @outen: offset of the output enable register. * @inen: offset of the input enable register. * @dat: offset of the data register. + * @intc_ctl: offset of the interrupt control register. + * @intc_pd: offset of the interrupt pending register. + * @intc_msk: offset of the interrupt mask register. + * @intc_type: offset of the interrupt type register. */ struct owl_gpio_port { unsigned int offset; @@ -128,6 +144,10 @@ struct owl_gpio_port { unsigned int outen; unsigned int inen; unsigned int dat; + unsigned int intc_ctl; + unsigned int intc_pd; + unsigned int intc_msk; + unsigned int intc_type; }; /** @@ -140,7 +160,7 @@ struct owl_gpio_port { * @ngroups: number of entries in @groups. * @padinfo: array describing the pad info of this SoC. * @ngpios: number of pingroups the driver should expose as GPIOs. - * @port: array describing all GPIO ports of this SoC. + * @ports: array describing all GPIO ports of this SoC. * @nports: number of GPIO ports in this SoC. */ struct owl_pinctrl_soc_data { diff --git a/drivers/pinctrl/actions/pinctrl-s900.c b/drivers/pinctrl/actions/pinctrl-s900.c index 5503c7945764..ea67b14ef93b 100644 --- a/drivers/pinctrl/actions/pinctrl-s900.c +++ b/drivers/pinctrl/actions/pinctrl-s900.c @@ -1821,22 +1821,27 @@ static struct owl_padinfo s900_padinfo[NUM_PADS] = { [SGPIO3] = PAD_INFO_PULLCTL_ST(SGPIO3) }; -#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat) \ - [OWL_GPIO_PORT_##port] = { \ - .offset = base, \ - .pins = count, \ - .outen = _outen, \ - .inen = _inen, \ - .dat = _dat, \ +#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat, \ + _intc_ctl, _intc_pd, _intc_msk, _intc_type) \ + [OWL_GPIO_PORT_##port] = { \ + .offset = base, \ + .pins = count, \ + .outen = _outen, \ + .inen = _inen, \ + .dat = _dat, \ + .intc_ctl = _intc_ctl, \ + .intc_pd = _intc_pd, \ + .intc_msk = _intc_msk, \ + .intc_type = _intc_type, \ } static const struct owl_gpio_port s900_gpio_ports[] = { - OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8), - OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8) + OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8, 0x204, 0x208, 0x20C, 0x240), + OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8, 0x534, 0x204, 0x208, 0x23C), + OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8, 0x52C, 0x200, 0x204, 0x238), + OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8, 0x524, 0x1FC, 0x200, 0x234), + OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8, 0x51C, 0x1F8, 0x1FC, 0x230), + OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8, 0x460, 0x140, 0x144, 0x178) }; static struct owl_pinctrl_soc_data s900_pinctrl_data = { -- 2.17.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC 2018-06-02 16:54 ` Manivannan Sadhasivam @ 2018-06-03 8:37 ` Andy Shevchenko -1 siblings, 0 replies; 16+ messages in thread From: Andy Shevchenko @ 2018-06-03 8:37 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Linus Walleij, Rob Herring, Andreas Färber, 刘炜, mp-cs, 96boards, devicetree, Daniel Thompson, amit.kucheria, linux-arm Mailing List, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, hzhang, bdong, Mani Sadhasivam, Thomas Liau, jeff.chen On Sat, Jun 2, 2018 at 7:54 PM, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > Add interrupt support for Actions Semi OWL S900 SoC. > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return; At which circumstances the above possible? > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return; Ditto. > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return; Ditto. > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return -ENODEV; Ditto. > + for (i = 0; i < chip->ngpio; i++) { > + irqno = irq_create_mapping(pctrl->domain, i); > + irq_set_chip_and_handler(irqno, &owl_gpio_irq_chip, > + handle_edge_irq); > + irq_set_chip_data(irqno, pctrl); > + } I'm not sure the handle_edge_irq() is a correct handler here. It would be handle_bad_irq() until IRQ has been requested properly. No? > +/* GPIO TYPE Bit Definition */ > +#define OWL_GPIO_INT_LEVEL_HIGH 0 > +#define OWL_GPIO_INT_LEVEL_LOW 1 > +#define OWL_GPIO_INT_EDGE_RISING 2 > +#define OWL_GPIO_INT_EDGE_FALLING 3 > +#define OWL_GPIO_INT_MASK 3 GENMASK? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC @ 2018-06-03 8:37 ` Andy Shevchenko 0 siblings, 0 replies; 16+ messages in thread From: Andy Shevchenko @ 2018-06-03 8:37 UTC (permalink / raw) To: linux-arm-kernel On Sat, Jun 2, 2018 at 7:54 PM, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > Add interrupt support for Actions Semi OWL S900 SoC. > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return; At which circumstances the above possible? > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return; Ditto. > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return; Ditto. > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return -ENODEV; Ditto. > + for (i = 0; i < chip->ngpio; i++) { > + irqno = irq_create_mapping(pctrl->domain, i); > + irq_set_chip_and_handler(irqno, &owl_gpio_irq_chip, > + handle_edge_irq); > + irq_set_chip_data(irqno, pctrl); > + } I'm not sure the handle_edge_irq() is a correct handler here. It would be handle_bad_irq() until IRQ has been requested properly. No? > +/* GPIO TYPE Bit Definition */ > +#define OWL_GPIO_INT_LEVEL_HIGH 0 > +#define OWL_GPIO_INT_LEVEL_LOW 1 > +#define OWL_GPIO_INT_EDGE_RISING 2 > +#define OWL_GPIO_INT_EDGE_FALLING 3 > +#define OWL_GPIO_INT_MASK 3 GENMASK? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC 2018-06-03 8:37 ` Andy Shevchenko @ 2018-06-03 16:57 ` Manivannan Sadhasivam -1 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-03 16:57 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Rob Herring, Andreas Färber, 刘炜, mp-cs, 96boards, devicetree, Daniel Thompson, amit.kucheria, linux-arm Mailing List, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, hzhang, bdong, Mani Sadhasivam, Thomas Liau, jeff.chen Hi Andy, On Sun, Jun 03, 2018 at 11:37:53AM +0300, Andy Shevchenko wrote: > On Sat, Jun 2, 2018 at 7:54 PM, Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > Add interrupt support for Actions Semi OWL S900 SoC. > > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return; > > At which circumstances the above possible? > Only possible when the requested GPIO exceeds chip->ngpio. I know it is a kind of redundant check, but it is good to have this during development. > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return; > > Ditto. > Same as above. > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return; > > Ditto. > Same as above. > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return -ENODEV; > > Ditto. > Same as above. > > > + for (i = 0; i < chip->ngpio; i++) { > > + irqno = irq_create_mapping(pctrl->domain, i); > > + irq_set_chip_and_handler(irqno, &owl_gpio_irq_chip, > > + handle_edge_irq); > > + irq_set_chip_data(irqno, pctrl); > > + } > > I'm not sure the handle_edge_irq() is a correct handler here. It would > be handle_bad_irq() until IRQ has been requested properly. > No? > Hmmm, good question. Since the handler used in irq_set_chip_and_handler is superseded by irq_set_chained_handler_and_data, this doesn't matter anyway. But I would like to hear what Linus suggests here! > > +/* GPIO TYPE Bit Definition */ > > +#define OWL_GPIO_INT_LEVEL_HIGH 0 > > +#define OWL_GPIO_INT_LEVEL_LOW 1 > > +#define OWL_GPIO_INT_EDGE_RISING 2 > > +#define OWL_GPIO_INT_EDGE_FALLING 3 > > > +#define OWL_GPIO_INT_MASK 3 > > GENMASK? > Ack. Thanks, Mani > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC @ 2018-06-03 16:57 ` Manivannan Sadhasivam 0 siblings, 0 replies; 16+ messages in thread From: Manivannan Sadhasivam @ 2018-06-03 16:57 UTC (permalink / raw) To: linux-arm-kernel Hi Andy, On Sun, Jun 03, 2018 at 11:37:53AM +0300, Andy Shevchenko wrote: > On Sat, Jun 2, 2018 at 7:54 PM, Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> wrote: > > Add interrupt support for Actions Semi OWL S900 SoC. > > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return; > > At which circumstances the above possible? > Only possible when the requested GPIO exceeds chip->ngpio. I know it is a kind of redundant check, but it is good to have this during development. > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return; > > Ditto. > Same as above. > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return; > > Ditto. > Same as above. > > + port = owl_gpio_get_port(pctrl, &gpio); > > + if (WARN_ON(port == NULL)) > > + return -ENODEV; > > Ditto. > Same as above. > > > + for (i = 0; i < chip->ngpio; i++) { > > + irqno = irq_create_mapping(pctrl->domain, i); > > + irq_set_chip_and_handler(irqno, &owl_gpio_irq_chip, > > + handle_edge_irq); > > + irq_set_chip_data(irqno, pctrl); > > + } > > I'm not sure the handle_edge_irq() is a correct handler here. It would > be handle_bad_irq() until IRQ has been requested properly. > No? > Hmmm, good question. Since the handler used in irq_set_chip_and_handler is superseded by irq_set_chained_handler_and_data, this doesn't matter anyway. But I would like to hear what Linus suggests here! > > +/* GPIO TYPE Bit Definition */ > > +#define OWL_GPIO_INT_LEVEL_HIGH 0 > > +#define OWL_GPIO_INT_LEVEL_LOW 1 > > +#define OWL_GPIO_INT_EDGE_RISING 2 > > +#define OWL_GPIO_INT_EDGE_FALLING 3 > > > +#define OWL_GPIO_INT_MASK 3 > > GENMASK? > Ack. Thanks, Mani > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC 2018-06-02 16:54 ` Manivannan Sadhasivam @ 2018-06-12 8:38 ` Linus Walleij -1 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2018-06-12 8:38 UTC (permalink / raw) To: Manivannan Sadhasivam, thierry.reding Cc: Rob Herring, Andreas Färber, liuwei, mp-cs, 96boards, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Andy Shevchenko, Daniel Thompson, Amit Kucheria, Linux ARM, open list:GPIO SUBSYSTEM, linux-kernel, hzhang, bdong, Mani Sadhasivam, Thomas C. Liau, Jeff Chen On Sat, Jun 2, 2018 at 6:54 PM, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > Add interrupt support for Actions Semi OWL S900 SoC. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> (...) > +++ b/drivers/pinctrl/actions/Kconfig > @@ -5,6 +5,7 @@ config PINCTRL_OWL > select PINCONF > select GENERIC_PINCONF > select GPIOLIB > + select GPIOLIB_IRQCHIP I don't think you're really using that (certain sign: you're implementing .to_irq().) GPIOLIB_IRQCHIP is nice, but can only be used when there is a 1-1 correspondence betweem GPIO line offsets and IRQ lines and they are numbered 0..n. However I think you can use it! Look below for my reference to the tegra186 and how you can probably cut out the custom irqdomain with some manouvers. > + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); > + const struct owl_gpio_port *port; (...) > + unsigned int gpio = data->hwirq; > + > + port = owl_gpio_get_port(pctrl, &gpio); > + > + gpio_base = pctrl->base + port->offset; It is all about this calculation really. > +static int owl_gpio_irq_set_type(struct irq_data *data, unsigned int type) > +{ > + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); > + const struct owl_gpio_port *port; > + void __iomem *gpio_base; > + unsigned long flags; > + unsigned int gpio = data->hwirq; > + unsigned int offset, value, irq_type = 0; > + > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return -ENODEV; > + > + gpio_base = pctrl->base + port->offset; > + > + switch (type) { > + case IRQ_TYPE_EDGE_RISING: > + irq_type = OWL_GPIO_INT_EDGE_RISING; > + break; > + > + case IRQ_TYPE_EDGE_FALLING: > + irq_type = OWL_GPIO_INT_EDGE_FALLING; > + break; Very often things such as keys will request trigger on both edges. This means IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING are both set, which will cause you problems here, since it is unhandled. I guess it is fine to set both? > + offset = gpio < 16 ? 4 : 0; I think even the compiler should suggest putting he (gpio < 16) expression in paranthesis. > + value = readl_relaxed(gpio_base + port->intc_type + offset); > + value &= ~(OWL_GPIO_INT_MASK << ((gpio % 16) * 2)); > + value |= irq_type << ((gpio % 16) * 2); > + writel_relaxed(value, gpio_base + port->intc_type + offset); > + > + raw_spin_unlock_irqrestore(&pctrl->lock, flags); > + > + if (type & IRQ_TYPE_EDGE_BOTH) > + irq_set_handler_locked(data, handle_edge_irq); > + else > + irq_set_handler_locked(data, handle_level_irq); Here you handle both edges, but not in the switch clause. > +static void owl_gpio_irq_handler(struct irq_desc *desc) This looks fine. > +static struct irq_chip owl_gpio_irq_chip = { > + .name = "owlgpio", > + .irq_mask = owl_gpio_irq_mask, > + .irq_unmask = owl_gpio_irq_unmask, > + .irq_ack = owl_gpio_irq_ack, > + .irq_set_type = owl_gpio_irq_set_type, > +}; If you implement your own irqchip you need to implement .irq_request_resources and .irq_free_resources locking the GPIO lines for interrupt, see other drivers that do this or the gpiolib core code in gpiolib.c. It would be really neat if you could instead use the GPIOLIB_IRQCHIP though, but I know it will be a bit tricky and maybe not possible. > + for (i = 0; i < pctrl->soc->nports; i++) { > + irq_set_chained_handler_and_data(pctrl->irq[i], > + owl_gpio_irq_handler, > + pctrl); > + } If you use GPIOLIB_IRQCHIP with several parent IRQs. See Thierry's work in drivers/gpio/gpio-tegra186.c for the only example. I think that is what you want to do here. We do want to add helpers in gpiolib to do this in a more organized and easy-to-use fashion. Patches welcome ;) Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC @ 2018-06-12 8:38 ` Linus Walleij 0 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2018-06-12 8:38 UTC (permalink / raw) To: linux-arm-kernel On Sat, Jun 2, 2018 at 6:54 PM, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > Add interrupt support for Actions Semi OWL S900 SoC. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> (...) > +++ b/drivers/pinctrl/actions/Kconfig > @@ -5,6 +5,7 @@ config PINCTRL_OWL > select PINCONF > select GENERIC_PINCONF > select GPIOLIB > + select GPIOLIB_IRQCHIP I don't think you're really using that (certain sign: you're implementing .to_irq().) GPIOLIB_IRQCHIP is nice, but can only be used when there is a 1-1 correspondence betweem GPIO line offsets and IRQ lines and they are numbered 0..n. However I think you can use it! Look below for my reference to the tegra186 and how you can probably cut out the custom irqdomain with some manouvers. > + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); > + const struct owl_gpio_port *port; (...) > + unsigned int gpio = data->hwirq; > + > + port = owl_gpio_get_port(pctrl, &gpio); > + > + gpio_base = pctrl->base + port->offset; It is all about this calculation really. > +static int owl_gpio_irq_set_type(struct irq_data *data, unsigned int type) > +{ > + struct owl_pinctrl *pctrl = irq_data_get_irq_chip_data(data); > + const struct owl_gpio_port *port; > + void __iomem *gpio_base; > + unsigned long flags; > + unsigned int gpio = data->hwirq; > + unsigned int offset, value, irq_type = 0; > + > + port = owl_gpio_get_port(pctrl, &gpio); > + if (WARN_ON(port == NULL)) > + return -ENODEV; > + > + gpio_base = pctrl->base + port->offset; > + > + switch (type) { > + case IRQ_TYPE_EDGE_RISING: > + irq_type = OWL_GPIO_INT_EDGE_RISING; > + break; > + > + case IRQ_TYPE_EDGE_FALLING: > + irq_type = OWL_GPIO_INT_EDGE_FALLING; > + break; Very often things such as keys will request trigger on both edges. This means IRQ_TYPE_EDGE_RISING and IRQ_TYPE_EDGE_FALLING are both set, which will cause you problems here, since it is unhandled. I guess it is fine to set both? > + offset = gpio < 16 ? 4 : 0; I think even the compiler should suggest putting he (gpio < 16) expression in paranthesis. > + value = readl_relaxed(gpio_base + port->intc_type + offset); > + value &= ~(OWL_GPIO_INT_MASK << ((gpio % 16) * 2)); > + value |= irq_type << ((gpio % 16) * 2); > + writel_relaxed(value, gpio_base + port->intc_type + offset); > + > + raw_spin_unlock_irqrestore(&pctrl->lock, flags); > + > + if (type & IRQ_TYPE_EDGE_BOTH) > + irq_set_handler_locked(data, handle_edge_irq); > + else > + irq_set_handler_locked(data, handle_level_irq); Here you handle both edges, but not in the switch clause. > +static void owl_gpio_irq_handler(struct irq_desc *desc) This looks fine. > +static struct irq_chip owl_gpio_irq_chip = { > + .name = "owlgpio", > + .irq_mask = owl_gpio_irq_mask, > + .irq_unmask = owl_gpio_irq_unmask, > + .irq_ack = owl_gpio_irq_ack, > + .irq_set_type = owl_gpio_irq_set_type, > +}; If you implement your own irqchip you need to implement .irq_request_resources and .irq_free_resources locking the GPIO lines for interrupt, see other drivers that do this or the gpiolib core code in gpiolib.c. It would be really neat if you could instead use the GPIOLIB_IRQCHIP though, but I know it will be a bit tricky and maybe not possible. > + for (i = 0; i < pctrl->soc->nports; i++) { > + irq_set_chained_handler_and_data(pctrl->irq[i], > + owl_gpio_irq_handler, > + pctrl); > + } If you use GPIOLIB_IRQCHIP with several parent IRQs. See Thierry's work in drivers/gpio/gpio-tegra186.c for the only example. I think that is what you want to do here. We do want to add helpers in gpiolib to do this in a more organized and easy-to-use fashion. Patches welcome ;) Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-06-12 20:58 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-02 16:54 [PATCH 0/3] Add gpio interrupt support for Actions Semi S900 SoC Manivannan Sadhasivam 2018-06-02 16:54 ` Manivannan Sadhasivam 2018-06-02 16:54 ` [PATCH 1/3] dt-bindings: pinctrl: Add gpio interrupt bindings for Actions " Manivannan Sadhasivam 2018-06-02 16:54 ` Manivannan Sadhasivam 2018-06-12 20:58 ` Rob Herring 2018-06-12 20:58 ` Rob Herring 2018-06-02 16:54 ` [PATCH 2/3] arm64: dts: actions: Add interrupt properties to pinctrl node for S900 Manivannan Sadhasivam 2018-06-02 16:54 ` Manivannan Sadhasivam 2018-06-02 16:54 ` [PATCH 3/3] pinctrl: actions: Add interrupt support for OWL S900 SoC Manivannan Sadhasivam 2018-06-02 16:54 ` Manivannan Sadhasivam 2018-06-03 8:37 ` Andy Shevchenko 2018-06-03 8:37 ` Andy Shevchenko 2018-06-03 16:57 ` Manivannan Sadhasivam 2018-06-03 16:57 ` Manivannan Sadhasivam 2018-06-12 8:38 ` Linus Walleij 2018-06-12 8:38 ` Linus Walleij
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.