2019-09-14 21:06:03

by Brian Masney

[permalink] [raw]
Subject: [PATCH] qcom: ssbi-gpio: convert to hierarchical IRQ helpers in gpio core

Now that the GPIO core has support for hierarchical IRQ chips, convert
Qualcomm's ssbi-gpio over to use these new helpers to reduce duplicated
code across drivers.

Signed-off-by: Brian Masney <[email protected]>
---
Linus: I've only compile tested this driver. Hopefully you have time to
test this on your DragonBoard.

drivers/pinctrl/qcom/Kconfig | 1 +
drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 121 +++++++----------------
2 files changed, 34 insertions(+), 88 deletions(-)

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 32fc2458b8eb..41fab621de1b 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -152,6 +152,7 @@ config PINCTRL_QCOM_SSBI_PMIC
select PINMUX
select PINCONF
select GENERIC_PINCONF
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
This is the pinctrl, pinmux, pinconf and gpiolib driver for the
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
index c1f7d0799ebe..dca86886b1f9 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
@@ -56,7 +56,6 @@
/**
* struct pm8xxx_pin_data - dynamic configuration for a pin
* @reg: address of the control register
- * @irq: IRQ from the PMIC interrupt controller
* @power_source: logical selected voltage source, mapping in static data
* is used translate to register values
* @mode: operating mode for the pin (input/output)
@@ -72,7 +71,6 @@
*/
struct pm8xxx_pin_data {
unsigned reg;
- int irq;
u8 power_source;
u8 mode;
bool open_drain;
@@ -93,9 +91,6 @@ struct pm8xxx_gpio {

struct pinctrl_desc desc;
unsigned npins;
-
- struct fwnode_handle *fwnode;
- struct irq_domain *domain;
};

static const struct pinconf_generic_params pm8xxx_gpio_bindings[] = {
@@ -491,13 +486,16 @@ static int pm8xxx_gpio_get(struct gpio_chip *chip, unsigned offset)
{
struct pm8xxx_gpio *pctrl = gpiochip_get_data(chip);
struct pm8xxx_pin_data *pin = pctrl->desc.pins[offset].drv_data;
+ int ret, irq;
bool state;
- int ret;

- if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
- ret = pin->output_value;
- } else if (pin->irq >= 0) {
- ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
+ if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT)
+ return pin->output_value;
+
+ irq = chip->to_irq(chip, offset);
+ if (irq >= 0) {
+ ret = irq_get_irqchip_state(irq, IRQCHIP_STATE_LINE_LEVEL,
+ &state);
if (!ret)
ret = !!state;
} else
@@ -535,37 +533,6 @@ static int pm8xxx_gpio_of_xlate(struct gpio_chip *chip,
}


-static int pm8xxx_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
-{
- struct pm8xxx_gpio *pctrl = gpiochip_get_data(chip);
- struct pm8xxx_pin_data *pin = pctrl->desc.pins[offset].drv_data;
- struct irq_fwspec fwspec;
- int ret;
-
- fwspec.fwnode = pctrl->fwnode;
- fwspec.param_count = 2;
- fwspec.param[0] = offset + PM8XXX_GPIO_PHYSICAL_OFFSET;
- fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
-
- ret = irq_create_fwspec_mapping(&fwspec);
-
- /*
- * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
- * line level.
- */
- pin->irq = ret;
-
- return ret;
-}
-
-static void pm8xxx_gpio_free(struct gpio_chip *chip, unsigned int offset)
-{
- struct pm8xxx_gpio *pctrl = gpiochip_get_data(chip);
- struct pm8xxx_pin_data *pin = pctrl->desc.pins[offset].drv_data;
-
- pin->irq = -1;
-}
-
#ifdef CONFIG_DEBUG_FS
#include <linux/seq_file.h>

@@ -624,13 +591,11 @@ static void pm8xxx_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
#endif

static const struct gpio_chip pm8xxx_gpio_template = {
- .free = pm8xxx_gpio_free,
.direction_input = pm8xxx_gpio_direction_input,
.direction_output = pm8xxx_gpio_direction_output,
.get = pm8xxx_gpio_get,
.set = pm8xxx_gpio_set,
.of_xlate = pm8xxx_gpio_of_xlate,
- .to_irq = pm8xxx_gpio_to_irq,
.dbg_show = pm8xxx_gpio_dbg_show,
.owner = THIS_MODULE,
};
@@ -712,43 +677,24 @@ static int pm8xxx_domain_translate(struct irq_domain *domain,
return 0;
}

-static int pm8xxx_domain_alloc(struct irq_domain *domain, unsigned int virq,
- unsigned int nr_irqs, void *data)
+static unsigned int pm8xxx_child_offset_to_irq(struct gpio_chip *chip,
+ unsigned int offset)
{
- struct pm8xxx_gpio *pctrl = container_of(domain->host_data,
- struct pm8xxx_gpio, chip);
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret, i;
-
- ret = pm8xxx_domain_translate(domain, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
- for (i = 0; i < nr_irqs; i++)
- irq_domain_set_info(domain, virq + i, hwirq + i,
- &pm8xxx_irq_chip, pctrl, handle_level_irq,
- NULL, NULL);
+ return offset + PM8XXX_GPIO_PHYSICAL_OFFSET;
+}

- parent_fwspec.fwnode = domain->parent->fwnode;
- parent_fwspec.param_count = 2;
- parent_fwspec.param[0] = hwirq + 0xc0;
- parent_fwspec.param[1] = fwspec->param[1];
+static int pm8xxx_child_to_parent_hwirq(struct gpio_chip *chip,
+ unsigned int child_hwirq,
+ unsigned int child_type,
+ unsigned int *parent_hwirq,
+ unsigned int *parent_type)
+{
+ *parent_hwirq = child_hwirq + 0xc0;
+ *parent_type = child_type;

- return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
- &parent_fwspec);
+ return 0;
}

-static const struct irq_domain_ops pm8xxx_domain_ops = {
- .activate = gpiochip_irq_domain_activate,
- .alloc = pm8xxx_domain_alloc,
- .deactivate = gpiochip_irq_domain_deactivate,
- .free = irq_domain_free_irqs_common,
- .translate = pm8xxx_domain_translate,
-};
-
static const struct of_device_id pm8xxx_gpio_of_match[] = {
{ .compatible = "qcom,pm8018-gpio", .data = (void *) 6 },
{ .compatible = "qcom,pm8038-gpio", .data = (void *) 12 },
@@ -765,6 +711,7 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)
struct irq_domain *parent_domain;
struct device_node *parent_node;
struct pinctrl_pin_desc *pins;
+ struct gpio_irq_chip *girq;
struct pm8xxx_gpio *pctrl;
int ret, i;

@@ -800,7 +747,6 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)

for (i = 0; i < pctrl->desc.npins; i++) {
pin_data[i].reg = SSBI_REG_ADDR_GPIO(i);
- pin_data[i].irq = -1;

ret = pm8xxx_pin_populate(pctrl, &pin_data[i]);
if (ret)
@@ -841,19 +787,21 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)
if (!parent_domain)
return -ENXIO;

- pctrl->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
- pctrl->domain = irq_domain_create_hierarchy(parent_domain, 0,
- pctrl->chip.ngpio,
- pctrl->fwnode,
- &pm8xxx_domain_ops,
- &pctrl->chip);
- if (!pctrl->domain)
- return -ENODEV;
+ girq = &pctrl->chip.irq;
+ girq->chip = &pm8xxx_irq_chip;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_level_irq;
+ girq->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
+ girq->parent_domain = parent_domain;
+ girq->child_to_parent_hwirq = pm8xxx_child_to_parent_hwirq;
+ girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
+ girq->child_offset_to_irq = pm8xxx_child_offset_to_irq;
+ girq->child_irq_domain_ops.translate = pm8xxx_domain_translate;

ret = gpiochip_add_data(&pctrl->chip, pctrl);
if (ret) {
dev_err(&pdev->dev, "failed register gpiochip\n");
- goto err_chip_add_data;
+ return ret;
}

/*
@@ -883,8 +831,6 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)

unregister_gpiochip:
gpiochip_remove(&pctrl->chip);
-err_chip_add_data:
- irq_domain_remove(pctrl->domain);

return ret;
}
@@ -894,7 +840,6 @@ static int pm8xxx_gpio_remove(struct platform_device *pdev)
struct pm8xxx_gpio *pctrl = platform_get_drvdata(pdev);

gpiochip_remove(&pctrl->chip);
- irq_domain_remove(pctrl->domain);

return 0;
}
--
2.21.0


2019-09-16 11:36:13

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] qcom: ssbi-gpio: convert to hierarchical IRQ helpers in gpio core

Hi Brian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Brian-Masney/qcom-ssbi-gpio-convert-to-hierarchical-IRQ-helpers-in-gpio-core/20190916-134112
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c: In function 'pm8xxx_gpio_probe':
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:794:6: error: 'struct gpio_irq_chip' has no member named 'fwnode'
girq->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
^~
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:795:8: error: 'struct gpio_irq_chip' has no member named 'parent_domain'; did you mean 'parent_handler'?
girq->parent_domain = parent_domain;
^~~~~~~~~~~~~
parent_handler
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:796:6: error: 'struct gpio_irq_chip' has no member named 'child_to_parent_hwirq'
girq->child_to_parent_hwirq = pm8xxx_child_to_parent_hwirq;
^~
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:797:6: error: 'struct gpio_irq_chip' has no member named 'populate_parent_fwspec'
girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
^~
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:797:33: error: 'gpiochip_populate_parent_fwspec_fourcell' undeclared (first use in this function); did you mean 'gpiochip_line_is_open_source'?
girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
gpiochip_line_is_open_source
drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:797:33: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:798:6: error: 'struct gpio_irq_chip' has no member named 'child_offset_to_irq'
girq->child_offset_to_irq = pm8xxx_child_offset_to_irq;
^~
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:799:8: error: 'struct gpio_irq_chip' has no member named 'child_irq_domain_ops'; did you mean 'domain_ops'?
girq->child_irq_domain_ops.translate = pm8xxx_domain_translate;
^~~~~~~~~~~~~~~~~~~~
domain_ops

vim +794 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c

766
767 pctrl->pctrl = devm_pinctrl_register(&pdev->dev, &pctrl->desc, pctrl);
768 if (IS_ERR(pctrl->pctrl)) {
769 dev_err(&pdev->dev, "couldn't register pm8xxx gpio driver\n");
770 return PTR_ERR(pctrl->pctrl);
771 }
772
773 pctrl->chip = pm8xxx_gpio_template;
774 pctrl->chip.base = -1;
775 pctrl->chip.parent = &pdev->dev;
776 pctrl->chip.of_node = pdev->dev.of_node;
777 pctrl->chip.of_gpio_n_cells = 2;
778 pctrl->chip.label = dev_name(pctrl->dev);
779 pctrl->chip.ngpio = pctrl->npins;
780
781 parent_node = of_irq_find_parent(pctrl->dev->of_node);
782 if (!parent_node)
783 return -ENXIO;
784
785 parent_domain = irq_find_host(parent_node);
786 of_node_put(parent_node);
787 if (!parent_domain)
788 return -ENXIO;
789
790 girq = &pctrl->chip.irq;
791 girq->chip = &pm8xxx_irq_chip;
792 girq->default_type = IRQ_TYPE_NONE;
793 girq->handler = handle_level_irq;
> 794 girq->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
> 795 girq->parent_domain = parent_domain;
> 796 girq->child_to_parent_hwirq = pm8xxx_child_to_parent_hwirq;
> 797 girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
> 798 girq->child_offset_to_irq = pm8xxx_child_offset_to_irq;
> 799 girq->child_irq_domain_ops.translate = pm8xxx_domain_translate;
800
801 ret = gpiochip_add_data(&pctrl->chip, pctrl);
802 if (ret) {
803 dev_err(&pdev->dev, "failed register gpiochip\n");
804 return ret;
805 }
806
807 /*
808 * For DeviceTree-supported systems, the gpio core checks the
809 * pinctrl's device node for the "gpio-ranges" property.
810 * If it is present, it takes care of adding the pin ranges
811 * for the driver. In this case the driver can skip ahead.
812 *
813 * In order to remain compatible with older, existing DeviceTree
814 * files which don't set the "gpio-ranges" property or systems that
815 * utilize ACPI the driver has to call gpiochip_add_pin_range().
816 */
817 if (!of_property_read_bool(pctrl->dev->of_node, "gpio-ranges")) {
818 ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
819 0, 0, pctrl->chip.ngpio);
820 if (ret) {
821 dev_err(pctrl->dev, "failed to add pin range\n");
822 goto unregister_gpiochip;
823 }
824 }
825
826 platform_set_drvdata(pdev, pctrl);
827
828 dev_dbg(&pdev->dev, "Qualcomm pm8xxx gpio driver probed\n");
829
830 return 0;
831
832 unregister_gpiochip:
833 gpiochip_remove(&pctrl->chip);
834
835 return ret;
836 }
837

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (5.69 kB)
.config.gz (69.71 kB)
Download all attachments

2019-09-17 09:56:36

by Brian Masney

[permalink] [raw]
Subject: Re: [PATCH] qcom: ssbi-gpio: convert to hierarchical IRQ helpers in gpio core

On Mon, Sep 16, 2019 at 07:31:33PM +0800, kbuild test robot wrote:
> Hi Brian,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [cannot apply to v5.3 next-20190915]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Brian-Masney/qcom-ssbi-gpio-convert-to-hierarchical-IRQ-helpers-in-gpio-core/20190916-134112
> config: arm-allmodconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=arm
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c: In function 'pm8xxx_gpio_probe':
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:794:6: error: 'struct gpio_irq_chip' has no member named 'fwnode'
> girq->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
> ^~
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:795:8: error: 'struct gpio_irq_chip' has no member named 'parent_domain'; did you mean 'parent_handler'?
> girq->parent_domain = parent_domain;
> ^~~~~~~~~~~~~
> parent_handler
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:796:6: error: 'struct gpio_irq_chip' has no member named 'child_to_parent_hwirq'
> girq->child_to_parent_hwirq = pm8xxx_child_to_parent_hwirq;
> ^~
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:797:6: error: 'struct gpio_irq_chip' has no member named 'populate_parent_fwspec'
> girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
> ^~
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:797:33: error: 'gpiochip_populate_parent_fwspec_fourcell' undeclared (first use in this function); did you mean 'gpiochip_line_is_open_source'?
> girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> gpiochip_line_is_open_source
> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:797:33: note: each undeclared identifier is reported only once for each function it appears in
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:798:6: error: 'struct gpio_irq_chip' has no member named 'child_offset_to_irq'
> girq->child_offset_to_irq = pm8xxx_child_offset_to_irq;
> ^~
> >> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c:799:8: error: 'struct gpio_irq_chip' has no member named 'child_irq_domain_ops'; did you mean 'domain_ops'?
> girq->child_irq_domain_ops.translate = pm8xxx_domain_translate;
> ^~~~~~~~~~~~~~~~~~~~
> domain_ops

This patch applies fine and builds cleanly on linux-next-20190916 and
linux-next-20190915 as well. The patch this depends on landed upstream
yesterday:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fdd61a013a24f2699aec1a446f0168682b6f9ec4

The compiler errors shown above came from kernel 5.3, which is missing
the dependent patch from above:
https://github.com/0day-ci/linux/commits/Brian-Masney/qcom-ssbi-gpio-convert-to-hierarchical-IRQ-helpers-in-gpio-core/20190916-134112

Brian

2019-10-03 14:30:26

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] qcom: ssbi-gpio: convert to hierarchical IRQ helpers in gpio core

On Sat, Sep 14, 2019 at 1:10 PM Brian Masney <[email protected]> wrote:

> Now that the GPIO core has support for hierarchical IRQ chips, convert
> Qualcomm's ssbi-gpio over to use these new helpers to reduce duplicated
> code across drivers.
>
> Signed-off-by: Brian Masney <[email protected]>

Patch applied for v5.5. I didn't have time to test before the merge
window, sorry, not it's in!

Yours,
Linus Walleij