linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] rockchip-pinctrl fixes for GKI
@ 2020-08-31  8:47 Jianqun Xu
  2020-08-31  8:47 ` [PATCH 1/6] pinctrl: rockchip: make driver be tristate module Jianqun Xu
                   ` (6 more replies)
  0 siblings, 7 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:47 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Fix rockchip pinctrl driver for GKI

Jianqun Xu (6):
  pinctrl: rockchip: make driver be tristate module
  pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
  pinctrl: rockchip: create irq mapping in gpio_to_irq
  pinctrl: rockchip: do not set gpio if bank invalid
  pinctrl: rockchip: fix crash caused by invalid gpio bank
  pinctrl: rockchip: populate platform device for rockchip gpio

 drivers/pinctrl/Kconfig            |   2 +-
 drivers/pinctrl/pinctrl-rockchip.c | 289 +++++++++++++++++------------
 2 files changed, 171 insertions(+), 120 deletions(-)

-- 
2.17.1




^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
@ 2020-08-31  8:47 ` Jianqun Xu
  2020-09-01 10:13   ` kernel test robot
  2020-09-05 22:01   ` Heiko Stübner
  2020-08-31  8:47 ` [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:47 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Make pinctrl-rockchip driver to be tristate module, support to build as
a module, this is useful for GKI.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/Kconfig            | 2 +-
 drivers/pinctrl/pinctrl-rockchip.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 8828613c4e0e..dd4874e2ac67 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -207,7 +207,7 @@ config PINCTRL_OXNAS
 	select MFD_SYSCON
 
 config PINCTRL_ROCKCHIP
-	bool
+	tristate "Rockchip gpio and pinctrl driver"
 	select PINMUX
 	select GENERIC_PINCONF
 	select GENERIC_IRQ_CHIP
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index c07324d1f265..24dfc814dee1 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -16,10 +16,12 @@
  */
 
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
 #include <linux/gpio/driver.h>
+#include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/pinctrl/machine.h>
@@ -4256,3 +4258,8 @@ static int __init rockchip_pinctrl_drv_register(void)
 	return platform_driver_register(&rockchip_pinctrl_driver);
 }
 postcore_initcall(rockchip_pinctrl_drv_register);
+
+MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:pinctrl-rockchip");
+MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
  2020-08-31  8:47 ` [PATCH 1/6] pinctrl: rockchip: make driver be tristate module Jianqun Xu
@ 2020-08-31  8:47 ` Jianqun Xu
  2020-09-05 21:54   ` Heiko Stübner
  2020-08-31  8:47 ` [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:47 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

There need to enable pclk_gpio when do irq_create_mapping, since it will
do access to gpio controller.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 24dfc814dee1..54abda7b7be8 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3155,7 +3155,9 @@ static int rockchip_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
 	if (!bank->domain)
 		return -ENXIO;
 
+	clk_enable(bank->clk);
 	virq = irq_create_mapping(bank->domain, offset);
+	clk_disable(bank->clk);
 
 	return (virq) ? : -ENXIO;
 }
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
  2020-08-31  8:47 ` [PATCH 1/6] pinctrl: rockchip: make driver be tristate module Jianqun Xu
  2020-08-31  8:47 ` [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
@ 2020-08-31  8:47 ` Jianqun Xu
  2020-09-05 22:03   ` Heiko Stübner
  2020-08-31  8:47 ` [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid Jianqun Xu
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:47 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Remove totally irq mappings create in probe, the gpio irq mapping will
be created when do
    gpio_to_irq ->
        rockchip_gpio_to_irq ->
            irq_create_mapping

This patch can speed up system boot on, also abandon many unused irq
mappings' create.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 54abda7b7be8..265d64b8c4f5 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3196,7 +3196,7 @@ static void rockchip_irq_demux(struct irq_desc *desc)
 
 		irq = __ffs(pend);
 		pend &= ~BIT(irq);
-		virq = irq_linear_revmap(bank->domain, irq);
+		virq = irq_find_mapping(bank->domain, irq);
 
 		if (!virq) {
 			dev_err(bank->drvdata->dev, "unmapped irq %d\n", irq);
@@ -3375,7 +3375,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	struct irq_chip_generic *gc;
 	int ret;
-	int i, j;
+	int i;
 
 	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
 		if (!bank->valid) {
@@ -3402,7 +3402,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 
 		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
 					 "rockchip_gpio_irq", handle_level_irq,
-					 clr, 0, IRQ_GC_INIT_MASK_CACHE);
+					 clr, 0, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
 				bank->name);
@@ -3411,14 +3411,6 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 			continue;
 		}
 
-		/*
-		 * Linux assumes that all interrupts start out disabled/masked.
-		 * Our driver only uses the concept of masked and always keeps
-		 * things enabled, so for us that's all masked and all enabled.
-		 */
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
-
 		gc = irq_get_domain_generic_chip(bank->domain, 0);
 		gc->reg_base = bank->reg_base;
 		gc->private = bank;
@@ -3435,13 +3427,17 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
 		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
 
+		/*
+		 * Linux assumes that all interrupts start out disabled/masked.
+		 * Our driver only uses the concept of masked and always keeps
+		 * things enabled, so for us that's all masked and all enabled.
+		 */
+		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
+		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
+		gc->mask_cache = 0xffffffff;
+
 		irq_set_chained_handler_and_data(bank->irq,
 						 rockchip_irq_demux, bank);
-
-		/* map the gpio irqs here, when the clock is still running */
-		for (j = 0 ; j < 32 ; j++)
-			irq_create_mapping(bank->domain, j);
-
 		clk_disable(bank->clk);
 	}
 
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
                   ` (2 preceding siblings ...)
  2020-08-31  8:47 ` [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
@ 2020-08-31  8:47 ` Jianqun Xu
  2020-09-05 22:09   ` Heiko Stübner
  2020-08-31  8:50 ` [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank Jianqun Xu
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:47 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Add valid check for gpio bank.

Change-Id: Ib03e2910a7316bd61df18236151e371c4d04077a
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 265d64b8c4f5..6080573155f6 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -2687,6 +2687,9 @@ static int rockchip_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 				return rc;
 			break;
 		case PIN_CONFIG_OUTPUT:
+			if (!bank->valid)
+				return -ENOTSUPP;
+
 			rockchip_gpio_set(&bank->gpio_chip,
 					  pin - bank->pin_base, arg);
 			rc = _rockchip_pmx_gpio_set_direction(&bank->gpio_chip,
@@ -2752,6 +2755,9 @@ static int rockchip_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
 		arg = 1;
 		break;
 	case PIN_CONFIG_OUTPUT:
+		if (!bank->valid)
+			return -ENOTSUPP;
+
 		rc = rockchip_get_mux(bank, pin - bank->pin_base);
 		if (rc != RK_FUNC_GPIO)
 			return -EINVAL;
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
                   ` (3 preceding siblings ...)
  2020-08-31  8:47 ` [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid Jianqun Xu
@ 2020-08-31  8:50 ` Jianqun Xu
  2020-09-05 22:14   ` Heiko Stübner
  2020-08-31  8:50 ` [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
  6 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:50 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Add valid check for gpio bank.

Change-Id: Ia4609c3045b5df7879beab3c15d791ff54a1f49b
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 6080573155f6..5b16b69e311f 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -2526,9 +2526,9 @@ static int rockchip_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 			break;
 	}
 
-	if (ret) {
+	if (ret && cnt) {
 		/* revert the already done pin settings */
-		for (cnt--; cnt >= 0; cnt--)
+		for (cnt--; cnt >= 0 && !data[cnt].func; cnt--)
 			rockchip_set_mux(bank, pins[cnt] - bank->pin_base, 0);
 
 		return ret;
@@ -2599,9 +2599,13 @@ static int rockchip_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
 					      unsigned offset, bool input)
 {
 	struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	struct rockchip_pin_bank *bank = &info->ctrl->pin_banks[offset / 32];
 	struct gpio_chip *chip;
 	int pin;
 
+	if (!bank || !bank->valid)
+		return 0;
+
 	chip = range->gc;
 	pin = offset - chip->base;
 	dev_dbg(info->dev, "gpio_direction for pin %u as %s-%d to %s\n",
@@ -3022,6 +3026,8 @@ static int rockchip_pinctrl_register(struct platform_device *pdev,
 
 	for (bank = 0; bank < info->ctrl->nr_banks; ++bank) {
 		pin_bank = &info->ctrl->pin_banks[bank];
+		if (!pin_bank->valid)
+			continue;
 		pin_bank->grange.name = pin_bank->name;
 		pin_bank->grange.id = bank;
 		pin_bank->grange.pin_base = pin_bank->pin_base;
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
                   ` (4 preceding siblings ...)
  2020-08-31  8:50 ` [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank Jianqun Xu
@ 2020-08-31  8:50 ` Jianqun Xu
  2020-09-06 10:20   ` Heiko Stübner
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
  6 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-08-31  8:50 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Register both gpio driver and device as part of driver model, so that
the '-gpio'/'-gpios' dependency in dts can be correctly handled by
of_devlink/of_fwlink.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 256 ++++++++++++++++-------------
 1 file changed, 145 insertions(+), 111 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 5b16b69e311f..9dc8daf38e63 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3380,139 +3380,121 @@ static void rockchip_irq_disable(struct irq_data *d)
 }
 
 static int rockchip_interrupts_register(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
+						struct rockchip_pin_bank *bank)
 {
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	struct irq_chip_generic *gc;
 	int ret;
-	int i;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid) {
-			dev_warn(&pdev->dev, "bank %s is not valid\n",
-				 bank->name);
-			continue;
-		}
+	if (!bank->valid) {
+		dev_warn(&pdev->dev, "bank %s is not valid\n",
+			 bank->name);
+		return -EINVAL;
+	}
 
-		ret = clk_enable(bank->clk);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
-				bank->name);
-			continue;
-		}
+	ret = clk_enable(bank->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
+			bank->name);
+		return ret;
+	}
 
-		bank->domain = irq_domain_add_linear(bank->of_node, 32,
-						&irq_generic_chip_ops, NULL);
-		if (!bank->domain) {
-			dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
-				 bank->name);
-			clk_disable(bank->clk);
-			continue;
-		}
+	bank->domain = irq_domain_add_linear(bank->of_node, 32,
+					&irq_generic_chip_ops, NULL);
+	if (!bank->domain) {
+		dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
+			 bank->name);
+		clk_disable(bank->clk);
+		return -EINVAL;
+	}
 
-		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
-					 "rockchip_gpio_irq", handle_level_irq,
-					 clr, 0, 0);
-		if (ret) {
-			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
-				bank->name);
-			irq_domain_remove(bank->domain);
-			clk_disable(bank->clk);
-			continue;
-		}
+	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
+				 "rockchip_gpio_irq", handle_level_irq,
+				 clr, 0, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
+			bank->name);
+		irq_domain_remove(bank->domain);
+		clk_disable(bank->clk);
+		return ret;
+	}
 
-		gc = irq_get_domain_generic_chip(bank->domain, 0);
-		gc->reg_base = bank->reg_base;
-		gc->private = bank;
-		gc->chip_types[0].regs.mask = GPIO_INTMASK;
-		gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
-		gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
-		gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
-		gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
-		gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
-		gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
-		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
-		gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
-		gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
-		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
-		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
+	gc = irq_get_domain_generic_chip(bank->domain, 0);
+	gc->reg_base = bank->reg_base;
+	gc->private = bank;
+	gc->chip_types[0].regs.mask = GPIO_INTMASK;
+	gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
+	gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
+	gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
+	gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
+	gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
+	gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
+	gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
+	gc->wake_enabled = IRQ_MSK(bank->nr_pins);
 
-		/*
-		 * Linux assumes that all interrupts start out disabled/masked.
-		 * Our driver only uses the concept of masked and always keeps
-		 * things enabled, so for us that's all masked and all enabled.
-		 */
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
-		gc->mask_cache = 0xffffffff;
+	/*
+	 * Linux assumes that all interrupts start out disabled/masked.
+	 * Our driver only uses the concept of masked and always keeps
+	 * things enabled, so for us that's all masked and all enabled.
+	 */
+	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
+	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
+	gc->mask_cache = 0xffffffff;
 
-		irq_set_chained_handler_and_data(bank->irq,
-						 rockchip_irq_demux, bank);
-		clk_disable(bank->clk);
-	}
+	irq_set_chained_handler_and_data(bank->irq,
+					 rockchip_irq_demux, bank);
+	clk_disable(bank->clk);
 
 	return 0;
 }
 
 static int rockchip_gpiolib_register(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
+						struct rockchip_pin_bank *bank)
 {
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
 	struct gpio_chip *gc;
 	int ret;
-	int i;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid) {
-			dev_warn(&pdev->dev, "bank %s is not valid\n",
-				 bank->name);
-			continue;
-		}
+	if (!bank->valid) {
+		dev_err(&pdev->dev, "bank %s is not valid\n", bank->name);
+		return -EINVAL;
+	}
 
-		bank->gpio_chip = rockchip_gpiolib_chip;
+	bank->gpio_chip = rockchip_gpiolib_chip;
 
-		gc = &bank->gpio_chip;
-		gc->base = bank->pin_base;
-		gc->ngpio = bank->nr_pins;
-		gc->parent = &pdev->dev;
-		gc->of_node = bank->of_node;
-		gc->label = bank->name;
+	gc = &bank->gpio_chip;
+	gc->base = bank->pin_base;
+	gc->ngpio = bank->nr_pins;
+	gc->parent = &pdev->dev;
+	gc->of_node = bank->of_node;
+	gc->label = bank->name;
 
-		ret = gpiochip_add_data(gc, bank);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n",
-							gc->label, ret);
-			goto fail;
-		}
+	ret = gpiochip_add_data(gc, bank);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register %s (%d)\n", gc->label, ret);
+		return ret;
 	}
 
-	rockchip_interrupts_register(pdev, info);
+	if (!of_property_read_bool(bank->of_node, "gpio-ranges")) {
+		struct device *parent = pdev->dev.parent;
+		struct rockchip_pinctrl *info = dev_get_drvdata(parent);
+		struct pinctrl_dev *pctldev;
 
-	return 0;
+		if (!info)
+			return -ENODATA;
 
-fail:
-	for (--i, --bank; i >= 0; --i, --bank) {
-		if (!bank->valid)
-			continue;
-		gpiochip_remove(&bank->gpio_chip);
-	}
-	return ret;
-}
-
-static int rockchip_gpiolib_unregister(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
-{
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
-	int i;
+		pctldev = info->pctl_dev;
+		if (!pctldev)
+			return -ENODEV;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid)
-			continue;
-		gpiochip_remove(&bank->gpio_chip);
+		ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0, gc->base, gc->ngpio);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to add pin range\n");
+			gpiochip_remove(&bank->gpio_chip);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -3752,6 +3734,46 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
 			 rockchip_pinctrl_resume);
 
+static int rockchip_gpio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *parent = pdev->dev.parent;
+	struct rockchip_pinctrl *info = dev_get_drvdata(parent);
+	struct rockchip_pin_ctrl *ctrl = info ? (info->ctrl) : NULL;
+	struct rockchip_pin_bank *bank;
+	int ret, i;
+
+	if (!info || !ctrl)
+		return -EINVAL;
+
+	if (!of_find_property(np, "gpio-controller", NULL))
+		return -ENODEV;
+
+	bank = ctrl->pin_banks;
+	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
+		if (!strcmp(bank->name, np->name)) {
+			bank->of_node = np;
+
+			if (!rockchip_get_bank_data(bank, info))
+				bank->valid = true;
+
+			break;
+		}
+	}
+
+	bank->of_node = pdev->dev.of_node;
+
+	ret = rockchip_gpiolib_register(pdev, bank);
+	if (ret)
+		return ret;
+
+	ret = rockchip_interrupts_register(pdev, bank);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int rockchip_pinctrl_probe(struct platform_device *pdev)
 {
 	struct rockchip_pinctrl *info;
@@ -3823,18 +3845,20 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 			return PTR_ERR(info->regmap_pmu);
 	}
 
-	ret = rockchip_gpiolib_register(pdev, info);
-	if (ret)
-		return ret;
-
 	ret = rockchip_pinctrl_register(pdev, info);
 	if (ret) {
-		rockchip_gpiolib_unregister(pdev, info);
+		dev_err(&pdev->dev, "failed to register pinctrl device\n");
 		return ret;
 	}
 
 	platform_set_drvdata(pdev, info);
 
+	ret = of_platform_populate(np, rockchip_bank_match, NULL, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register gpio device\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -4254,6 +4278,14 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
 	{},
 };
 
+static struct platform_driver rockchip_gpio_driver = {
+	.probe		= rockchip_gpio_probe,
+	.driver = {
+		.name	= "rockchip-gpio",
+		.of_match_table = rockchip_bank_match,
+	},
+};
+
 static struct platform_driver rockchip_pinctrl_driver = {
 	.probe		= rockchip_pinctrl_probe,
 	.driver = {
@@ -4265,7 +4297,9 @@ static struct platform_driver rockchip_pinctrl_driver = {
 
 static int __init rockchip_pinctrl_drv_register(void)
 {
-	return platform_driver_register(&rockchip_pinctrl_driver);
+	platform_driver_register(&rockchip_pinctrl_driver);
+
+	return platform_driver_register(&rockchip_gpio_driver);
 }
 postcore_initcall(rockchip_pinctrl_drv_register);
 
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
  2020-08-31  8:47 ` [PATCH 1/6] pinctrl: rockchip: make driver be tristate module Jianqun Xu
@ 2020-09-01 10:13   ` kernel test robot
  2020-09-05 21:51     ` Heiko Stübner
  2020-09-05 22:01   ` Heiko Stübner
  1 sibling, 1 reply; 32+ messages in thread
From: kernel test robot @ 2020-09-01 10:13 UTC (permalink / raw)
  To: Jianqun Xu, linus.walleij, heiko
  Cc: kbuild-all, linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

[-- Attachment #1: Type: text/plain, Size: 7313 bytes --]

Hi Jianqun,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rockchip/for-next]
[also build test ERROR on pinctrl/devel v5.9-rc3 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200831-165040
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: x86_64-randconfig-m031-20200901 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/pinctrl/pinctrl-rockchip.c: In function 'rockchip_pinctrl_parse_groups':
>> drivers/pinctrl/pinctrl-rockchip.c:2881:9: error: implicit declaration of function 'pinconf_generic_parse_dt_config'; did you mean 'pinconf_generic_dump_config'? [-Werror=implicit-function-declaration]
    2881 |   ret = pinconf_generic_parse_dt_config(np_config, NULL,
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |         pinconf_generic_dump_config
   drivers/pinctrl/pinctrl-rockchip.c: In function 'rockchip_gpiolib_register':
>> drivers/pinctrl/pinctrl-rockchip.c:3473:5: error: 'struct gpio_chip' has no member named 'of_node'
    3473 |   gc->of_node = bank->of_node;
         |     ^~
   At top level:
   drivers/pinctrl/pinctrl-rockchip.c:2804:34: warning: 'rockchip_bank_match' defined but not used [-Wunused-const-variable=]
    2804 | static const struct of_device_id rockchip_bank_match[] = {
         |                                  ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/38fa905767d010bbbc1035b48494d4a83bb72410
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200831-165040
git checkout 38fa905767d010bbbc1035b48494d4a83bb72410
vim +2881 drivers/pinctrl/pinctrl-rockchip.c

d3e5116119bd02 Heiko Stübner   2013-06-10  2823  
d3e5116119bd02 Heiko Stübner   2013-06-10  2824  static int rockchip_pinctrl_parse_groups(struct device_node *np,
d3e5116119bd02 Heiko Stübner   2013-06-10  2825  					      struct rockchip_pin_group *grp,
d3e5116119bd02 Heiko Stübner   2013-06-10  2826  					      struct rockchip_pinctrl *info,
d3e5116119bd02 Heiko Stübner   2013-06-10  2827  					      u32 index)
d3e5116119bd02 Heiko Stübner   2013-06-10  2828  {
d3e5116119bd02 Heiko Stübner   2013-06-10  2829  	struct rockchip_pin_bank *bank;
d3e5116119bd02 Heiko Stübner   2013-06-10  2830  	int size;
d3e5116119bd02 Heiko Stübner   2013-06-10  2831  	const __be32 *list;
d3e5116119bd02 Heiko Stübner   2013-06-10  2832  	int num;
d3e5116119bd02 Heiko Stübner   2013-06-10  2833  	int i, j;
d3e5116119bd02 Heiko Stübner   2013-06-10  2834  	int ret;
d3e5116119bd02 Heiko Stübner   2013-06-10  2835  
94f4e54cecaf3e Rob Herring     2018-08-27  2836  	dev_dbg(info->dev, "group(%d): %pOFn\n", index, np);
d3e5116119bd02 Heiko Stübner   2013-06-10  2837  
d3e5116119bd02 Heiko Stübner   2013-06-10  2838  	/* Initialise group */
d3e5116119bd02 Heiko Stübner   2013-06-10  2839  	grp->name = np->name;
d3e5116119bd02 Heiko Stübner   2013-06-10  2840  
d3e5116119bd02 Heiko Stübner   2013-06-10  2841  	/*
d3e5116119bd02 Heiko Stübner   2013-06-10  2842  	 * the binding format is rockchip,pins = <bank pin mux CONFIG>,
d3e5116119bd02 Heiko Stübner   2013-06-10  2843  	 * do sanity check and calculate pins number
d3e5116119bd02 Heiko Stübner   2013-06-10  2844  	 */
d3e5116119bd02 Heiko Stübner   2013-06-10  2845  	list = of_get_property(np, "rockchip,pins", &size);
d3e5116119bd02 Heiko Stübner   2013-06-10  2846  	/* we do not check return since it's safe node passed down */
d3e5116119bd02 Heiko Stübner   2013-06-10  2847  	size /= sizeof(*list);
d3e5116119bd02 Heiko Stübner   2013-06-10  2848  	if (!size || size % 4) {
d3e5116119bd02 Heiko Stübner   2013-06-10  2849  		dev_err(info->dev, "wrong pins number or pins and configs should be by 4\n");
d3e5116119bd02 Heiko Stübner   2013-06-10  2850  		return -EINVAL;
d3e5116119bd02 Heiko Stübner   2013-06-10  2851  	}
d3e5116119bd02 Heiko Stübner   2013-06-10  2852  
d3e5116119bd02 Heiko Stübner   2013-06-10  2853  	grp->npins = size / 4;
d3e5116119bd02 Heiko Stübner   2013-06-10  2854  
a86854d0c599b3 Kees Cook       2018-06-12  2855  	grp->pins = devm_kcalloc(info->dev, grp->npins, sizeof(unsigned int),
d3e5116119bd02 Heiko Stübner   2013-06-10  2856  						GFP_KERNEL);
a86854d0c599b3 Kees Cook       2018-06-12  2857  	grp->data = devm_kcalloc(info->dev,
a86854d0c599b3 Kees Cook       2018-06-12  2858  					grp->npins,
d3e5116119bd02 Heiko Stübner   2013-06-10  2859  					sizeof(struct rockchip_pin_config),
d3e5116119bd02 Heiko Stübner   2013-06-10  2860  					GFP_KERNEL);
d3e5116119bd02 Heiko Stübner   2013-06-10  2861  	if (!grp->pins || !grp->data)
d3e5116119bd02 Heiko Stübner   2013-06-10  2862  		return -ENOMEM;
d3e5116119bd02 Heiko Stübner   2013-06-10  2863  
d3e5116119bd02 Heiko Stübner   2013-06-10  2864  	for (i = 0, j = 0; i < size; i += 4, j++) {
d3e5116119bd02 Heiko Stübner   2013-06-10  2865  		const __be32 *phandle;
d3e5116119bd02 Heiko Stübner   2013-06-10  2866  		struct device_node *np_config;
d3e5116119bd02 Heiko Stübner   2013-06-10  2867  
d3e5116119bd02 Heiko Stübner   2013-06-10  2868  		num = be32_to_cpu(*list++);
d3e5116119bd02 Heiko Stübner   2013-06-10  2869  		bank = bank_num_to_bank(info, num);
d3e5116119bd02 Heiko Stübner   2013-06-10  2870  		if (IS_ERR(bank))
d3e5116119bd02 Heiko Stübner   2013-06-10  2871  			return PTR_ERR(bank);
d3e5116119bd02 Heiko Stübner   2013-06-10  2872  
d3e5116119bd02 Heiko Stübner   2013-06-10  2873  		grp->pins[j] = bank->pin_base + be32_to_cpu(*list++);
d3e5116119bd02 Heiko Stübner   2013-06-10  2874  		grp->data[j].func = be32_to_cpu(*list++);
d3e5116119bd02 Heiko Stübner   2013-06-10  2875  
d3e5116119bd02 Heiko Stübner   2013-06-10  2876  		phandle = list++;
d3e5116119bd02 Heiko Stübner   2013-06-10  2877  		if (!phandle)
d3e5116119bd02 Heiko Stübner   2013-06-10  2878  			return -EINVAL;
d3e5116119bd02 Heiko Stübner   2013-06-10  2879  
d3e5116119bd02 Heiko Stübner   2013-06-10  2880  		np_config = of_find_node_by_phandle(be32_to_cpup(phandle));
dd4d01f7bad886 Soren Brinkmann 2015-01-09 @2881  		ret = pinconf_generic_parse_dt_config(np_config, NULL,
d3e5116119bd02 Heiko Stübner   2013-06-10  2882  				&grp->data[j].configs, &grp->data[j].nconfigs);
d3e5116119bd02 Heiko Stübner   2013-06-10  2883  		if (ret)
d3e5116119bd02 Heiko Stübner   2013-06-10  2884  			return ret;
d3e5116119bd02 Heiko Stübner   2013-06-10  2885  	}
d3e5116119bd02 Heiko Stübner   2013-06-10  2886  
d3e5116119bd02 Heiko Stübner   2013-06-10  2887  	return 0;
d3e5116119bd02 Heiko Stübner   2013-06-10  2888  }
d3e5116119bd02 Heiko Stübner   2013-06-10  2889  

---
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: 36580 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
  2020-09-01 10:13   ` kernel test robot
@ 2020-09-05 21:51     ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 21:51 UTC (permalink / raw)
  To: Jianqun Xu, linus.walleij, kernel test robot
  Cc: kbuild-all, linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Hi,

Am Dienstag, 1. September 2020, 12:13:16 CEST schrieb kernel test robot:
> Hi Jianqun,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on rockchip/for-next]
> [also build test ERROR on pinctrl/devel v5.9-rc3 next-20200828]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200831-165040
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
> config: x86_64-randconfig-m031-20200901 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
>         # save the attached .config to linux build tree
>         make W=1 ARCH=x86_64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/pinctrl/pinctrl-rockchip.c: In function 'rockchip_pinctrl_parse_groups':
> >> drivers/pinctrl/pinctrl-rockchip.c:2881:9: error: implicit declaration of function 'pinconf_generic_parse_dt_config'; did you mean 'pinconf_generic_dump_config'? [-Werror=implicit-function-declaration]
>     2881 |   ret = pinconf_generic_parse_dt_config(np_config, NULL,
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          |         pinconf_generic_dump_config
>    drivers/pinctrl/pinctrl-rockchip.c: In function 'rockchip_gpiolib_register':
> >> drivers/pinctrl/pinctrl-rockchip.c:3473:5: error: 'struct gpio_chip' has no member named 'of_node'
>     3473 |   gc->of_node = bank->of_node;
>          |     ^~
>    At top level:
>    drivers/pinctrl/pinctrl-rockchip.c:2804:34: warning: 'rockchip_bank_match' defined but not used [-Wunused-const-variable=]
>     2804 | static const struct of_device_id rockchip_bank_match[] = {
>          |                                  ^~~~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors

these errors are unrelated to this patch, and I addressed them in
[PATCH] pinctrl: rockchip: depend on OF [0]

Heiko

[0] http://lore.kernel.org/r/20200905214955.907950-1-heiko@sntech.de


> 
> # https://github.com/0day-ci/linux/commit/38fa905767d010bbbc1035b48494d4a83bb72410
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200831-165040
> git checkout 38fa905767d010bbbc1035b48494d4a83bb72410
> vim +2881 drivers/pinctrl/pinctrl-rockchip.c
> 
> d3e5116119bd02 Heiko Stübner   2013-06-10  2823  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2824  static int rockchip_pinctrl_parse_groups(struct device_node *np,
> d3e5116119bd02 Heiko Stübner   2013-06-10  2825  					      struct rockchip_pin_group *grp,
> d3e5116119bd02 Heiko Stübner   2013-06-10  2826  					      struct rockchip_pinctrl *info,
> d3e5116119bd02 Heiko Stübner   2013-06-10  2827  					      u32 index)
> d3e5116119bd02 Heiko Stübner   2013-06-10  2828  {
> d3e5116119bd02 Heiko Stübner   2013-06-10  2829  	struct rockchip_pin_bank *bank;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2830  	int size;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2831  	const __be32 *list;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2832  	int num;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2833  	int i, j;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2834  	int ret;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2835  
> 94f4e54cecaf3e Rob Herring     2018-08-27  2836  	dev_dbg(info->dev, "group(%d): %pOFn\n", index, np);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2837  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2838  	/* Initialise group */
> d3e5116119bd02 Heiko Stübner   2013-06-10  2839  	grp->name = np->name;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2840  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2841  	/*
> d3e5116119bd02 Heiko Stübner   2013-06-10  2842  	 * the binding format is rockchip,pins = <bank pin mux CONFIG>,
> d3e5116119bd02 Heiko Stübner   2013-06-10  2843  	 * do sanity check and calculate pins number
> d3e5116119bd02 Heiko Stübner   2013-06-10  2844  	 */
> d3e5116119bd02 Heiko Stübner   2013-06-10  2845  	list = of_get_property(np, "rockchip,pins", &size);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2846  	/* we do not check return since it's safe node passed down */
> d3e5116119bd02 Heiko Stübner   2013-06-10  2847  	size /= sizeof(*list);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2848  	if (!size || size % 4) {
> d3e5116119bd02 Heiko Stübner   2013-06-10  2849  		dev_err(info->dev, "wrong pins number or pins and configs should be by 4\n");
> d3e5116119bd02 Heiko Stübner   2013-06-10  2850  		return -EINVAL;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2851  	}
> d3e5116119bd02 Heiko Stübner   2013-06-10  2852  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2853  	grp->npins = size / 4;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2854  
> a86854d0c599b3 Kees Cook       2018-06-12  2855  	grp->pins = devm_kcalloc(info->dev, grp->npins, sizeof(unsigned int),
> d3e5116119bd02 Heiko Stübner   2013-06-10  2856  						GFP_KERNEL);
> a86854d0c599b3 Kees Cook       2018-06-12  2857  	grp->data = devm_kcalloc(info->dev,
> a86854d0c599b3 Kees Cook       2018-06-12  2858  					grp->npins,
> d3e5116119bd02 Heiko Stübner   2013-06-10  2859  					sizeof(struct rockchip_pin_config),
> d3e5116119bd02 Heiko Stübner   2013-06-10  2860  					GFP_KERNEL);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2861  	if (!grp->pins || !grp->data)
> d3e5116119bd02 Heiko Stübner   2013-06-10  2862  		return -ENOMEM;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2863  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2864  	for (i = 0, j = 0; i < size; i += 4, j++) {
> d3e5116119bd02 Heiko Stübner   2013-06-10  2865  		const __be32 *phandle;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2866  		struct device_node *np_config;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2867  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2868  		num = be32_to_cpu(*list++);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2869  		bank = bank_num_to_bank(info, num);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2870  		if (IS_ERR(bank))
> d3e5116119bd02 Heiko Stübner   2013-06-10  2871  			return PTR_ERR(bank);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2872  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2873  		grp->pins[j] = bank->pin_base + be32_to_cpu(*list++);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2874  		grp->data[j].func = be32_to_cpu(*list++);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2875  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2876  		phandle = list++;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2877  		if (!phandle)
> d3e5116119bd02 Heiko Stübner   2013-06-10  2878  			return -EINVAL;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2879  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2880  		np_config = of_find_node_by_phandle(be32_to_cpup(phandle));
> dd4d01f7bad886 Soren Brinkmann 2015-01-09 @2881  		ret = pinconf_generic_parse_dt_config(np_config, NULL,
> d3e5116119bd02 Heiko Stübner   2013-06-10  2882  				&grp->data[j].configs, &grp->data[j].nconfigs);
> d3e5116119bd02 Heiko Stübner   2013-06-10  2883  		if (ret)
> d3e5116119bd02 Heiko Stübner   2013-06-10  2884  			return ret;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2885  	}
> d3e5116119bd02 Heiko Stübner   2013-06-10  2886  
> d3e5116119bd02 Heiko Stübner   2013-06-10  2887  	return 0;
> d3e5116119bd02 Heiko Stübner   2013-06-10  2888  }
> d3e5116119bd02 Heiko Stübner   2013-06-10  2889  
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
  2020-08-31  8:47 ` [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
@ 2020-09-05 21:54   ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 21:54 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Montag, 31. August 2020, 10:47:49 CEST schrieb Jianqun Xu:
> There need to enable pclk_gpio when do irq_create_mapping, since it will
> do access to gpio controller.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 24dfc814dee1..54abda7b7be8 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3155,7 +3155,9 @@ static int rockchip_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
>  	if (!bank->domain)
>  		return -ENXIO;
>  
> +	clk_enable(bank->clk);
>  	virq = irq_create_mapping(bank->domain, offset);
> +	clk_disable(bank->clk);
>  
>  	return (virq) ? : -ENXIO;
>  }
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
  2020-08-31  8:47 ` [PATCH 1/6] pinctrl: rockchip: make driver be tristate module Jianqun Xu
  2020-09-01 10:13   ` kernel test robot
@ 2020-09-05 22:01   ` Heiko Stübner
  2020-09-05 22:23     ` Heiko Stübner
  1 sibling, 1 reply; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 22:01 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Montag, 31. August 2020, 10:47:48 CEST schrieb Jianqun Xu:
> Make pinctrl-rockchip driver to be tristate module, support to build as
> a module, this is useful for GKI.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/pinctrl/Kconfig            | 2 +-
>  drivers/pinctrl/pinctrl-rockchip.c | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 8828613c4e0e..dd4874e2ac67 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
>  	select MFD_SYSCON
>  
>  config PINCTRL_ROCKCHIP
> -	bool
> +	tristate "Rockchip gpio and pinctrl driver"
>  	select PINMUX
>  	select GENERIC_PINCONF
>  	select GENERIC_IRQ_CHIP
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index c07324d1f265..24dfc814dee1 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -16,10 +16,12 @@
>   */
>  
>  #include <linux/init.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
>  #include <linux/bitops.h>
>  #include <linux/gpio/driver.h>
> +#include <linux/of_device.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/pinctrl/machine.h>
> @@ -4256,3 +4258,8 @@ static int __init rockchip_pinctrl_drv_register(void)
>  	return platform_driver_register(&rockchip_pinctrl_driver);
>  }
>  postcore_initcall(rockchip_pinctrl_drv_register);
> +
> +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:pinctrl-rockchip");
> +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq
  2020-08-31  8:47 ` [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
@ 2020-09-05 22:03   ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 22:03 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Montag, 31. August 2020, 10:47:50 CEST schrieb Jianqun Xu:
> Remove totally irq mappings create in probe, the gpio irq mapping will
> be created when do
>     gpio_to_irq ->
>         rockchip_gpio_to_irq ->
>             irq_create_mapping
> 
> This patch can speed up system boot on, also abandon many unused irq
> mappings' create.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 54abda7b7be8..265d64b8c4f5 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3196,7 +3196,7 @@ static void rockchip_irq_demux(struct irq_desc *desc)
>  
>  		irq = __ffs(pend);
>  		pend &= ~BIT(irq);
> -		virq = irq_linear_revmap(bank->domain, irq);
> +		virq = irq_find_mapping(bank->domain, irq);
>  
>  		if (!virq) {
>  			dev_err(bank->drvdata->dev, "unmapped irq %d\n", irq);
> @@ -3375,7 +3375,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
>  	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
>  	struct irq_chip_generic *gc;
>  	int ret;
> -	int i, j;
> +	int i;
>  
>  	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
>  		if (!bank->valid) {
> @@ -3402,7 +3402,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
>  
>  		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
>  					 "rockchip_gpio_irq", handle_level_irq,
> -					 clr, 0, IRQ_GC_INIT_MASK_CACHE);
> +					 clr, 0, 0);
>  		if (ret) {
>  			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
>  				bank->name);
> @@ -3411,14 +3411,6 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
>  			continue;
>  		}
>  
> -		/*
> -		 * Linux assumes that all interrupts start out disabled/masked.
> -		 * Our driver only uses the concept of masked and always keeps
> -		 * things enabled, so for us that's all masked and all enabled.
> -		 */
> -		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
> -		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
> -
>  		gc = irq_get_domain_generic_chip(bank->domain, 0);
>  		gc->reg_base = bank->reg_base;
>  		gc->private = bank;
> @@ -3435,13 +3427,17 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
>  		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
>  		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
>  
> +		/*
> +		 * Linux assumes that all interrupts start out disabled/masked.
> +		 * Our driver only uses the concept of masked and always keeps
> +		 * things enabled, so for us that's all masked and all enabled.
> +		 */
> +		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
> +		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
> +		gc->mask_cache = 0xffffffff;
> +
>  		irq_set_chained_handler_and_data(bank->irq,
>  						 rockchip_irq_demux, bank);
> -
> -		/* map the gpio irqs here, when the clock is still running */
> -		for (j = 0 ; j < 32 ; j++)
> -			irq_create_mapping(bank->domain, j);
> -
>  		clk_disable(bank->clk);
>  	}
>  
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid
  2020-08-31  8:47 ` [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid Jianqun Xu
@ 2020-09-05 22:09   ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 22:09 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Montag, 31. August 2020, 10:47:51 CEST schrieb Jianqun Xu:
> Add valid check for gpio bank.

As this obviously fixes a problem you encountered please elaborate a bit more.
Just so that people reading the log later understand when this issue surfaced.

Also - maybe even more important - why is this limited to PIN_CONFIG_OUTPUT?
Like when the whole bank is not valid, you should be able to return the -ENOTSUPP
even before entering the "for" loop in these functions.


> Change-Id: Ib03e2910a7316bd61df18236151e371c4d04077a

Please remove the changeId.

Thanks
Heiko

> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 265d64b8c4f5..6080573155f6 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -2687,6 +2687,9 @@ static int rockchip_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
>  				return rc;
>  			break;
>  		case PIN_CONFIG_OUTPUT:
> +			if (!bank->valid)
> +				return -ENOTSUPP;
> +
>  			rockchip_gpio_set(&bank->gpio_chip,
>  					  pin - bank->pin_base, arg);
>  			rc = _rockchip_pmx_gpio_set_direction(&bank->gpio_chip,
> @@ -2752,6 +2755,9 @@ static int rockchip_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
>  		arg = 1;
>  		break;
>  	case PIN_CONFIG_OUTPUT:
> +		if (!bank->valid)
> +			return -ENOTSUPP;
> +
>  		rc = rockchip_get_mux(bank, pin - bank->pin_base);
>  		if (rc != RK_FUNC_GPIO)
>  			return -EINVAL;
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank
  2020-08-31  8:50 ` [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank Jianqun Xu
@ 2020-09-05 22:14   ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 22:14 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Hi,

Am Montag, 31. August 2020, 10:50:10 CEST schrieb Jianqun Xu:
> Add valid check for gpio bank.

Please add more description on where this happened.


> Change-Id: Ia4609c3045b5df7879beab3c15d791ff54a1f49b

Please drop the change-id.


> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 6080573155f6..5b16b69e311f 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -2526,9 +2526,9 @@ static int rockchip_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
>  			break;
>  	}
>  
> -	if (ret) {
> +	if (ret && cnt) {
>  		/* revert the already done pin settings */
> -		for (cnt--; cnt >= 0; cnt--)
> +		for (cnt--; cnt >= 0 && !data[cnt].func; cnt--)

This looks unrelated and as it's not a "check for a valid gpio-bank" it
should become a separate patch with a commit message describing it nicely.

>  			rockchip_set_mux(bank, pins[cnt] - bank->pin_base, 0);
>  
>  		return ret;
> @@ -2599,9 +2599,13 @@ static int rockchip_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
>  					      unsigned offset, bool input)
>  {
>  	struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
> +	struct rockchip_pin_bank *bank = &info->ctrl->pin_banks[offset / 32];
>  	struct gpio_chip *chip;
>  	int pin;
>  
> +	if (!bank || !bank->valid)
> +		return 0;
> +
>  	chip = range->gc;
>  	pin = offset - chip->base;
>  	dev_dbg(info->dev, "gpio_direction for pin %u as %s-%d to %s\n",
> @@ -3022,6 +3026,8 @@ static int rockchip_pinctrl_register(struct platform_device *pdev,
>  
>  	for (bank = 0; bank < info->ctrl->nr_banks; ++bank) {
>  		pin_bank = &info->ctrl->pin_banks[bank];
> +		if (!pin_bank->valid)
> +			continue;

Please add a blank line here

>  		pin_bank->grange.name = pin_bank->name;
>  		pin_bank->grange.id = bank;
>  		pin_bank->grange.pin_base = pin_bank->pin_base;
> 


Thanks
Heiko



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/6] pinctrl: rockchip: make driver be tristate module
  2020-09-05 22:01   ` Heiko Stübner
@ 2020-09-05 22:23     ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-05 22:23 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Sonntag, 6. September 2020, 00:01:55 CEST schrieb Heiko Stübner:
> Am Montag, 31. August 2020, 10:47:48 CEST schrieb Jianqun Xu:
> > Make pinctrl-rockchip driver to be tristate module, support to build as
> > a module, this is useful for GKI.
> > 
> > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

I take this back.


What happens when you actually unload the module now?
I checked and all the pinctrl stuff itself is using devm-functions
so should be safe, but you're missing the 

platform_driver_unregister part that should happen as well.


Heiko

> > ---
> >  drivers/pinctrl/Kconfig            | 2 +-
> >  drivers/pinctrl/pinctrl-rockchip.c | 7 +++++++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index 8828613c4e0e..dd4874e2ac67 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> >  	select MFD_SYSCON
> >  
> >  config PINCTRL_ROCKCHIP
> > -	bool
> > +	tristate "Rockchip gpio and pinctrl driver"
> >  	select PINMUX
> >  	select GENERIC_PINCONF
> >  	select GENERIC_IRQ_CHIP
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> > index c07324d1f265..24dfc814dee1 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -16,10 +16,12 @@
> >   */
> >  
> >  #include <linux/init.h>
> > +#include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/io.h>
> >  #include <linux/bitops.h>
> >  #include <linux/gpio/driver.h>
> > +#include <linux/of_device.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/pinctrl/machine.h>
> > @@ -4256,3 +4258,8 @@ static int __init rockchip_pinctrl_drv_register(void)
> >  	return platform_driver_register(&rockchip_pinctrl_driver);
> >  }
> >  postcore_initcall(rockchip_pinctrl_drv_register);
> > +
> > +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:pinctrl-rockchip");
> > +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
> > 
> 
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio
  2020-08-31  8:50 ` [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
@ 2020-09-06 10:20   ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-06 10:20 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Hi Jianqun,

Am Montag, 31. August 2020, 10:50:21 CEST schrieb Jianqun Xu:
> Register both gpio driver and device as part of driver model, so that
> the '-gpio'/'-gpios' dependency in dts can be correctly handled by
> of_devlink/of_fwlink.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

[...]

> -static int rockchip_gpiolib_unregister(struct platform_device *pdev,
> -						struct rockchip_pinctrl *info)
> -{
> -	struct rockchip_pin_ctrl *ctrl = info->ctrl;
> -	struct rockchip_pin_bank *bank = ctrl->pin_banks;
> -	int i;
> +		pctldev = info->pctl_dev;
> +		if (!pctldev)
> +			return -ENODEV;
>  
> -	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> -		if (!bank->valid)
> -			continue;
> -		gpiochip_remove(&bank->gpio_chip);
> +		ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0, gc->base, gc->ngpio);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to add pin range\n");
> +			gpiochip_remove(&bank->gpio_chip);
> +			return ret;
> +		}
>  	}
>  
>  	return 0;
> @@ -3752,6 +3734,46 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
>  			 rockchip_pinctrl_resume);
>  
> +static int rockchip_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *parent = pdev->dev.parent;
> +	struct rockchip_pinctrl *info = dev_get_drvdata(parent);
> +	struct rockchip_pin_ctrl *ctrl = info ? (info->ctrl) : NULL;
> +	struct rockchip_pin_bank *bank;
> +	int ret, i;
> +
> +	if (!info || !ctrl)
> +		return -EINVAL;
> +
> +	if (!of_find_property(np, "gpio-controller", NULL))
> +		return -ENODEV;
> +
> +	bank = ctrl->pin_banks;
> +	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> +		if (!strcmp(bank->name, np->name)) {
> +			bank->of_node = np;
> +
> +			if (!rockchip_get_bank_data(bank, info))
> +				bank->valid = true;
> +
> +			break;
> +		}
> +	}
> +
> +	bank->of_node = pdev->dev.of_node;
> +
> +	ret = rockchip_gpiolib_register(pdev, bank);
> +	if (ret)
> +		return ret;
> +
> +	ret = rockchip_interrupts_register(pdev, bank);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int rockchip_pinctrl_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_pinctrl *info;
> @@ -3823,18 +3845,20 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
>  			return PTR_ERR(info->regmap_pmu);
>  	}
>  
> -	ret = rockchip_gpiolib_register(pdev, info);
> -	if (ret)
> -		return ret;
> -
>  	ret = rockchip_pinctrl_register(pdev, info);
>  	if (ret) {
> -		rockchip_gpiolib_unregister(pdev, info);
> +		dev_err(&pdev->dev, "failed to register pinctrl device\n");
>  		return ret;
>  	}
>  
>  	platform_set_drvdata(pdev, info);
>  
> +	ret = of_platform_populate(np, rockchip_bank_match, NULL, dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register gpio device\n");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -4254,6 +4278,14 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
>  	{},
>  };
>  
> +static struct platform_driver rockchip_gpio_driver = {
> +	.probe		= rockchip_gpio_probe,
> +	.driver = {
> +		.name	= "rockchip-gpio",
> +		.of_match_table = rockchip_bank_match,
> +	},
> +};
> +
>  static struct platform_driver rockchip_pinctrl_driver = {
>  	.probe		= rockchip_pinctrl_probe,
>  	.driver = {
> @@ -4265,7 +4297,9 @@ static struct platform_driver rockchip_pinctrl_driver = {
>  
>  static int __init rockchip_pinctrl_drv_register(void)
>  {
> -	return platform_driver_register(&rockchip_pinctrl_driver);
> +	platform_driver_register(&rockchip_pinctrl_driver);
> +
> +	return platform_driver_register(&rockchip_gpio_driver);
>  }
>  postcore_initcall(rockchip_pinctrl_drv_register);

This also seems to miss the whole unloading of module paths?
I'd expect a gpiochip_remove etc in some _remove function and
of course a platform_driver_unregister.


Heiko



^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2 0/5] rockchip-pinctrl fixes for GKI
  2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
                   ` (5 preceding siblings ...)
  2020-08-31  8:50 ` [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
@ 2020-09-07  2:59 ` Jianqun Xu
  2020-09-07  2:59   ` [PATCH 1/5] pinctrl: rockchip: depend on OF Jianqun Xu
                     ` (5 more replies)
  6 siblings, 6 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-09-07  2:59 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

These patches will fix some issues and modify for GKI.

Heiko Stuebner (1):
  pinctrl: rockchip: depend on OF

Jianqun Xu (4):
  pinctrl: rockchip: make driver be tristate module
  pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
  pinctrl: rockchip: create irq mapping in gpio_to_irq
  pinctrl: rockchip: populate platform device for rockchip gpio

 drivers/pinctrl/Kconfig            |   4 +-
 drivers/pinctrl/pinctrl-rockchip.c | 289 +++++++++++++++++------------
 2 files changed, 175 insertions(+), 118 deletions(-)

-- 
2.17.1




^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/5] pinctrl: rockchip: depend on OF
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
@ 2020-09-07  2:59   ` Jianqun Xu
  2020-09-07  2:59   ` [PATCH 2/5] pinctrl: rockchip: make driver be tristate module Jianqun Xu
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-09-07  2:59 UTC (permalink / raw)
  To: linus.walleij, heiko
  Cc: linux-gpio, linux-kernel, linux-rockchip, Heiko Stuebner

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

The Rockchip pinctrl driver needs to handle information from Devicetree
so only makes sense getting compiled on systems with CONFIG_OF enabled.

This also fixes a problem found by the "kernel-test-robot" when compiling
the driver on test-builds that do not have CONFIG_OF enabled:

  drivers/pinctrl/pinctrl-rockchip.c: In function 'rockchip_pinctrl_parse_groups':
>> drivers/pinctrl/pinctrl-rockchip.c:2881:9: error: implicit declaration of function 'pinconf_generic_parse_dt_config'; did you mean 'pinconf_generic_dump_config'? [-Werror=implicit-function-declaration]
    2881 |   ret = pinconf_generic_parse_dt_config(np_config, NULL,
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |         pinconf_generic_dump_config
   drivers/pinctrl/pinctrl-rockchip.c: In function 'rockchip_gpiolib_register':
>> drivers/pinctrl/pinctrl-rockchip.c:3473:5: error: 'struct gpio_chip' has no member named 'of_node'
    3473 |   gc->of_node = bank->of_node;

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/pinctrl/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 8828613c4e0e..4284f39a5c61 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -208,10 +208,12 @@ config PINCTRL_OXNAS
 
 config PINCTRL_ROCKCHIP
 	bool
+	depends on OF
 	select PINMUX
 	select GENERIC_PINCONF
 	select GENERIC_IRQ_CHIP
 	select MFD_SYSCON
+	select OF_GPIO
 
 config PINCTRL_RZA1
 	bool "Renesas RZ/A1 gpio and pinctrl driver"
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/5] pinctrl: rockchip: make driver be tristate module
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
  2020-09-07  2:59   ` [PATCH 1/5] pinctrl: rockchip: depend on OF Jianqun Xu
@ 2020-09-07  2:59   ` Jianqun Xu
  2020-09-07 11:35     ` kernel test robot
                       ` (2 more replies)
  2020-09-07  2:59   ` [PATCH 3/5] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
                     ` (3 subsequent siblings)
  5 siblings, 3 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-09-07  2:59 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Make pinctrl-rockchip driver to be tristate module, support to build as
a module, this is useful for GKI.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/Kconfig            |  2 +-
 drivers/pinctrl/pinctrl-rockchip.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 4284f39a5c61..743eb2bb8709 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -207,7 +207,7 @@ config PINCTRL_OXNAS
 	select MFD_SYSCON
 
 config PINCTRL_ROCKCHIP
-	bool
+	tristate "Rockchip gpio and pinctrl driver"
 	depends on OF
 	select PINMUX
 	select GENERIC_PINCONF
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 0401c1da79dd..cc7512acfc5f 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -16,10 +16,12 @@
  */
 
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
 #include <linux/gpio/driver.h>
+#include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/pinctrl/machine.h>
@@ -4257,4 +4259,20 @@ static int __init rockchip_pinctrl_drv_register(void)
 {
 	return platform_driver_register(&rockchip_pinctrl_driver);
 }
+
+static void __exit rockchip_pinctrl_drv_unregister(void)
+{
+	platform_driver_unregister(&rockchip_pinctrl_driver);
+}
+
+#ifdef CONFIG_PINCTRL_ROCKCHIP_MODULE
+module_init(rockchip_pinctrl_drv_register);
+#else
 postcore_initcall(rockchip_pinctrl_drv_register);
+#endif
+module_exit(rockchip_pinctrl_drv_unregister);
+
+MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:pinctrl-rockchip");
+MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/5] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
  2020-09-07  2:59   ` [PATCH 1/5] pinctrl: rockchip: depend on OF Jianqun Xu
  2020-09-07  2:59   ` [PATCH 2/5] pinctrl: rockchip: make driver be tristate module Jianqun Xu
@ 2020-09-07  2:59   ` Jianqun Xu
  2020-09-07  2:59   ` [PATCH 4/5] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-09-07  2:59 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

There need to enable pclk_gpio when do irq_create_mapping, since it will
do access to gpio controller.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index cc7512acfc5f..58fd4d822591 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3157,7 +3157,9 @@ static int rockchip_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
 	if (!bank->domain)
 		return -ENXIO;
 
+	clk_enable(bank->clk);
 	virq = irq_create_mapping(bank->domain, offset);
+	clk_disable(bank->clk);
 
 	return (virq) ? : -ENXIO;
 }
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/5] pinctrl: rockchip: create irq mapping in gpio_to_irq
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
                     ` (2 preceding siblings ...)
  2020-09-07  2:59   ` [PATCH 3/5] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
@ 2020-09-07  2:59   ` Jianqun Xu
  2020-09-07  3:38   ` [PATCH 5/5] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
  2020-09-12 11:35   ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Linus Walleij
  5 siblings, 0 replies; 32+ messages in thread
From: Jianqun Xu @ 2020-09-07  2:59 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Remove totally irq mappings create in probe, the gpio irq mapping will
be created when do
    gpio_to_irq ->
        rockchip_gpio_to_irq ->
            irq_create_mapping

This patch can speed up system boot on, also abandon many unused irq
mappings' create.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 58fd4d822591..c98bd352f831 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3198,7 +3198,7 @@ static void rockchip_irq_demux(struct irq_desc *desc)
 
 		irq = __ffs(pend);
 		pend &= ~BIT(irq);
-		virq = irq_linear_revmap(bank->domain, irq);
+		virq = irq_find_mapping(bank->domain, irq);
 
 		if (!virq) {
 			dev_err(bank->drvdata->dev, "unmapped irq %d\n", irq);
@@ -3377,7 +3377,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	struct irq_chip_generic *gc;
 	int ret;
-	int i, j;
+	int i;
 
 	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
 		if (!bank->valid) {
@@ -3404,7 +3404,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 
 		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
 					 "rockchip_gpio_irq", handle_level_irq,
-					 clr, 0, IRQ_GC_INIT_MASK_CACHE);
+					 clr, 0, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
 				bank->name);
@@ -3413,14 +3413,6 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 			continue;
 		}
 
-		/*
-		 * Linux assumes that all interrupts start out disabled/masked.
-		 * Our driver only uses the concept of masked and always keeps
-		 * things enabled, so for us that's all masked and all enabled.
-		 */
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
-
 		gc = irq_get_domain_generic_chip(bank->domain, 0);
 		gc->reg_base = bank->reg_base;
 		gc->private = bank;
@@ -3437,13 +3429,17 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
 		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
 		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
 
+		/*
+		 * Linux assumes that all interrupts start out disabled/masked.
+		 * Our driver only uses the concept of masked and always keeps
+		 * things enabled, so for us that's all masked and all enabled.
+		 */
+		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
+		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
+		gc->mask_cache = 0xffffffff;
+
 		irq_set_chained_handler_and_data(bank->irq,
 						 rockchip_irq_demux, bank);
-
-		/* map the gpio irqs here, when the clock is still running */
-		for (j = 0 ; j < 32 ; j++)
-			irq_create_mapping(bank->domain, j);
-
 		clk_disable(bank->clk);
 	}
 
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/5] pinctrl: rockchip: populate platform device for rockchip gpio
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
                     ` (3 preceding siblings ...)
  2020-09-07  2:59   ` [PATCH 4/5] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
@ 2020-09-07  3:38   ` Jianqun Xu
  2020-09-08  2:19     ` [PATCH] " Jianqun Xu
  2020-09-12 11:35   ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Linus Walleij
  5 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-09-07  3:38 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Register both gpio driver and device as part of driver model, so that
the '-gpio'/'-gpios' dependency in dts can be correctly handled by
of_devlink/of_fwlink.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 261 +++++++++++++++++------------
 1 file changed, 150 insertions(+), 111 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index c98bd352f831..67850a9386d7 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3370,139 +3370,121 @@ static void rockchip_irq_disable(struct irq_data *d)
 }
 
 static int rockchip_interrupts_register(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
+						struct rockchip_pin_bank *bank)
 {
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	struct irq_chip_generic *gc;
 	int ret;
-	int i;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid) {
-			dev_warn(&pdev->dev, "bank %s is not valid\n",
-				 bank->name);
-			continue;
-		}
+	if (!bank->valid) {
+		dev_warn(&pdev->dev, "bank %s is not valid\n",
+			 bank->name);
+		return -EINVAL;
+	}
 
-		ret = clk_enable(bank->clk);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
-				bank->name);
-			continue;
-		}
+	ret = clk_enable(bank->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
+			bank->name);
+		return ret;
+	}
 
-		bank->domain = irq_domain_add_linear(bank->of_node, 32,
-						&irq_generic_chip_ops, NULL);
-		if (!bank->domain) {
-			dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
-				 bank->name);
-			clk_disable(bank->clk);
-			continue;
-		}
+	bank->domain = irq_domain_add_linear(bank->of_node, 32,
+					&irq_generic_chip_ops, NULL);
+	if (!bank->domain) {
+		dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
+			 bank->name);
+		clk_disable(bank->clk);
+		return -EINVAL;
+	}
 
-		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
-					 "rockchip_gpio_irq", handle_level_irq,
-					 clr, 0, 0);
-		if (ret) {
-			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
-				bank->name);
-			irq_domain_remove(bank->domain);
-			clk_disable(bank->clk);
-			continue;
-		}
+	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
+				 "rockchip_gpio_irq", handle_level_irq,
+				 clr, 0, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
+			bank->name);
+		irq_domain_remove(bank->domain);
+		clk_disable(bank->clk);
+		return ret;
+	}
 
-		gc = irq_get_domain_generic_chip(bank->domain, 0);
-		gc->reg_base = bank->reg_base;
-		gc->private = bank;
-		gc->chip_types[0].regs.mask = GPIO_INTMASK;
-		gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
-		gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
-		gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
-		gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
-		gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
-		gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
-		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
-		gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
-		gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
-		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
-		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
+	gc = irq_get_domain_generic_chip(bank->domain, 0);
+	gc->reg_base = bank->reg_base;
+	gc->private = bank;
+	gc->chip_types[0].regs.mask = GPIO_INTMASK;
+	gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
+	gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
+	gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
+	gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
+	gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
+	gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
+	gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
+	gc->wake_enabled = IRQ_MSK(bank->nr_pins);
 
-		/*
-		 * Linux assumes that all interrupts start out disabled/masked.
-		 * Our driver only uses the concept of masked and always keeps
-		 * things enabled, so for us that's all masked and all enabled.
-		 */
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
-		gc->mask_cache = 0xffffffff;
+	/*
+	 * Linux assumes that all interrupts start out disabled/masked.
+	 * Our driver only uses the concept of masked and always keeps
+	 * things enabled, so for us that's all masked and all enabled.
+	 */
+	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
+	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
+	gc->mask_cache = 0xffffffff;
 
-		irq_set_chained_handler_and_data(bank->irq,
-						 rockchip_irq_demux, bank);
-		clk_disable(bank->clk);
-	}
+	irq_set_chained_handler_and_data(bank->irq,
+					 rockchip_irq_demux, bank);
+	clk_disable(bank->clk);
 
 	return 0;
 }
 
 static int rockchip_gpiolib_register(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
+						struct rockchip_pin_bank *bank)
 {
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
 	struct gpio_chip *gc;
 	int ret;
-	int i;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid) {
-			dev_warn(&pdev->dev, "bank %s is not valid\n",
-				 bank->name);
-			continue;
-		}
+	if (!bank->valid) {
+		dev_err(&pdev->dev, "bank %s is not valid\n", bank->name);
+		return -EINVAL;
+	}
 
-		bank->gpio_chip = rockchip_gpiolib_chip;
+	bank->gpio_chip = rockchip_gpiolib_chip;
 
-		gc = &bank->gpio_chip;
-		gc->base = bank->pin_base;
-		gc->ngpio = bank->nr_pins;
-		gc->parent = &pdev->dev;
-		gc->of_node = bank->of_node;
-		gc->label = bank->name;
+	gc = &bank->gpio_chip;
+	gc->base = bank->pin_base;
+	gc->ngpio = bank->nr_pins;
+	gc->parent = &pdev->dev;
+	gc->of_node = bank->of_node;
+	gc->label = bank->name;
 
-		ret = gpiochip_add_data(gc, bank);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n",
-							gc->label, ret);
-			goto fail;
-		}
+	ret = gpiochip_add_data(gc, bank);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register %s (%d)\n", gc->label, ret);
+		return ret;
 	}
 
-	rockchip_interrupts_register(pdev, info);
+	if (!of_property_read_bool(bank->of_node, "gpio-ranges")) {
+		struct device *parent = pdev->dev.parent;
+		struct rockchip_pinctrl *info = dev_get_drvdata(parent);
+		struct pinctrl_dev *pctldev;
 
-	return 0;
+		if (!info)
+			return -ENODATA;
 
-fail:
-	for (--i, --bank; i >= 0; --i, --bank) {
-		if (!bank->valid)
-			continue;
-		gpiochip_remove(&bank->gpio_chip);
-	}
-	return ret;
-}
+		pctldev = info->pctl_dev;
+		if (!pctldev)
+			return -ENODEV;
 
-static int rockchip_gpiolib_unregister(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
-{
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
-	int i;
-
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid)
-			continue;
-		gpiochip_remove(&bank->gpio_chip);
+		ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0, gc->base, gc->ngpio);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to add pin range\n");
+			gpiochip_remove(&bank->gpio_chip);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -3742,6 +3724,46 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
 			 rockchip_pinctrl_resume);
 
+static int rockchip_gpio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *parent = pdev->dev.parent;
+	struct rockchip_pinctrl *info = dev_get_drvdata(parent);
+	struct rockchip_pin_ctrl *ctrl = info ? (info->ctrl) : NULL;
+	struct rockchip_pin_bank *bank;
+	int ret, i;
+
+	if (!info || !ctrl)
+		return -EINVAL;
+
+	if (!of_find_property(np, "gpio-controller", NULL))
+		return -ENODEV;
+
+	bank = ctrl->pin_banks;
+	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
+		if (!strcmp(bank->name, np->name)) {
+			bank->of_node = np;
+
+			if (!rockchip_get_bank_data(bank, info))
+				bank->valid = true;
+
+			break;
+		}
+	}
+
+	bank->of_node = pdev->dev.of_node;
+
+	ret = rockchip_gpiolib_register(pdev, bank);
+	if (ret)
+		return ret;
+
+	ret = rockchip_interrupts_register(pdev, bank);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int rockchip_pinctrl_probe(struct platform_device *pdev)
 {
 	struct rockchip_pinctrl *info;
@@ -3813,18 +3835,20 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 			return PTR_ERR(info->regmap_pmu);
 	}
 
-	ret = rockchip_gpiolib_register(pdev, info);
-	if (ret)
-		return ret;
-
 	ret = rockchip_pinctrl_register(pdev, info);
 	if (ret) {
-		rockchip_gpiolib_unregister(pdev, info);
+		dev_err(&pdev->dev, "failed to register pinctrl device\n");
 		return ret;
 	}
 
 	platform_set_drvdata(pdev, info);
 
+	ret = of_platform_populate(np, rockchip_bank_match, NULL, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register gpio device\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -4244,6 +4268,14 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
 	{},
 };
 
+static struct platform_driver rockchip_gpio_driver = {
+	.probe		= rockchip_gpio_probe,
+	.driver = {
+		.name	= "rockchip-gpio",
+		.of_match_table = rockchip_bank_match,
+	},
+};
+
 static struct platform_driver rockchip_pinctrl_driver = {
 	.probe		= rockchip_pinctrl_probe,
 	.driver = {
@@ -4255,11 +4287,18 @@ static struct platform_driver rockchip_pinctrl_driver = {
 
 static int __init rockchip_pinctrl_drv_register(void)
 {
-	return platform_driver_register(&rockchip_pinctrl_driver);
+	int ret;
+
+	ret = platform_driver_register(&rockchip_pinctrl_driver);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&rockchip_gpio_driver);
 }
 
 static void __exit rockchip_pinctrl_drv_unregister(void)
 {
+	platform_driver_unregister(&rockchip_gpio_driver);
 	platform_driver_unregister(&rockchip_pinctrl_driver);
 }
 
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] pinctrl: rockchip: make driver be tristate module
  2020-09-07  2:59   ` [PATCH 2/5] pinctrl: rockchip: make driver be tristate module Jianqun Xu
@ 2020-09-07 11:35     ` kernel test robot
  2020-09-12 11:41     ` Heiko Stübner
  2020-09-14  0:38     ` [PATCH 2/2] " Jianqun Xu
  2 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2020-09-07 11:35 UTC (permalink / raw)
  To: Jianqun Xu, linus.walleij, heiko
  Cc: kbuild-all, linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

Hi Jianqun,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rockchip/for-next]
[also build test ERROR on pinctrl/devel v5.9-rc4 next-20200903]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jianqun-Xu/rockchip-pinctrl-fixes-for-GKI/20200907-114025
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: riscv-allmodconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "irq_gc_set_wake" [drivers/pinctrl/pinctrl-rockchip.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: 65925 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH] pinctrl: rockchip: populate platform device for rockchip gpio
  2020-09-07  3:38   ` [PATCH 5/5] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
@ 2020-09-08  2:19     ` Jianqun Xu
  2020-09-20 22:16       ` Heiko Stübner
  0 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-09-08  2:19 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Register both gpio driver and device as part of driver model, so that
the '-gpio'/'-gpios' dependency in dts can be correctly handled by
of_devlink/of_fwlink.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 305 +++++++++++++++++------------
 1 file changed, 175 insertions(+), 130 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index c98bd352f831..2e4fc711d0d1 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3370,139 +3370,121 @@ static void rockchip_irq_disable(struct irq_data *d)
 }
 
 static int rockchip_interrupts_register(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
+						struct rockchip_pin_bank *bank)
 {
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	struct irq_chip_generic *gc;
 	int ret;
-	int i;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid) {
-			dev_warn(&pdev->dev, "bank %s is not valid\n",
-				 bank->name);
-			continue;
-		}
+	if (!bank->valid) {
+		dev_warn(&pdev->dev, "bank %s is not valid\n",
+			 bank->name);
+		return -EINVAL;
+	}
 
-		ret = clk_enable(bank->clk);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
-				bank->name);
-			continue;
-		}
+	ret = clk_enable(bank->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
+			bank->name);
+		return ret;
+	}
 
-		bank->domain = irq_domain_add_linear(bank->of_node, 32,
-						&irq_generic_chip_ops, NULL);
-		if (!bank->domain) {
-			dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
-				 bank->name);
-			clk_disable(bank->clk);
-			continue;
-		}
+	bank->domain = irq_domain_add_linear(bank->of_node, 32,
+					&irq_generic_chip_ops, NULL);
+	if (!bank->domain) {
+		dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
+			 bank->name);
+		clk_disable(bank->clk);
+		return -EINVAL;
+	}
 
-		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
-					 "rockchip_gpio_irq", handle_level_irq,
-					 clr, 0, 0);
-		if (ret) {
-			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
-				bank->name);
-			irq_domain_remove(bank->domain);
-			clk_disable(bank->clk);
-			continue;
-		}
+	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
+				 "rockchip_gpio_irq", handle_level_irq,
+				 clr, 0, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
+			bank->name);
+		irq_domain_remove(bank->domain);
+		clk_disable(bank->clk);
+		return ret;
+	}
 
-		gc = irq_get_domain_generic_chip(bank->domain, 0);
-		gc->reg_base = bank->reg_base;
-		gc->private = bank;
-		gc->chip_types[0].regs.mask = GPIO_INTMASK;
-		gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
-		gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
-		gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
-		gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
-		gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
-		gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
-		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
-		gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
-		gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
-		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
-		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
+	gc = irq_get_domain_generic_chip(bank->domain, 0);
+	gc->reg_base = bank->reg_base;
+	gc->private = bank;
+	gc->chip_types[0].regs.mask = GPIO_INTMASK;
+	gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
+	gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
+	gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
+	gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
+	gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
+	gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
+	gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
+	gc->wake_enabled = IRQ_MSK(bank->nr_pins);
 
-		/*
-		 * Linux assumes that all interrupts start out disabled/masked.
-		 * Our driver only uses the concept of masked and always keeps
-		 * things enabled, so for us that's all masked and all enabled.
-		 */
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
-		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
-		gc->mask_cache = 0xffffffff;
+	/*
+	 * Linux assumes that all interrupts start out disabled/masked.
+	 * Our driver only uses the concept of masked and always keeps
+	 * things enabled, so for us that's all masked and all enabled.
+	 */
+	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
+	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
+	gc->mask_cache = 0xffffffff;
 
-		irq_set_chained_handler_and_data(bank->irq,
-						 rockchip_irq_demux, bank);
-		clk_disable(bank->clk);
-	}
+	irq_set_chained_handler_and_data(bank->irq,
+					 rockchip_irq_demux, bank);
+	clk_disable(bank->clk);
 
 	return 0;
 }
 
 static int rockchip_gpiolib_register(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
+						struct rockchip_pin_bank *bank)
 {
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
 	struct gpio_chip *gc;
 	int ret;
-	int i;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid) {
-			dev_warn(&pdev->dev, "bank %s is not valid\n",
-				 bank->name);
-			continue;
-		}
+	if (!bank->valid) {
+		dev_err(&pdev->dev, "bank %s is not valid\n", bank->name);
+		return -EINVAL;
+	}
 
-		bank->gpio_chip = rockchip_gpiolib_chip;
+	bank->gpio_chip = rockchip_gpiolib_chip;
 
-		gc = &bank->gpio_chip;
-		gc->base = bank->pin_base;
-		gc->ngpio = bank->nr_pins;
-		gc->parent = &pdev->dev;
-		gc->of_node = bank->of_node;
-		gc->label = bank->name;
+	gc = &bank->gpio_chip;
+	gc->base = bank->pin_base;
+	gc->ngpio = bank->nr_pins;
+	gc->parent = &pdev->dev;
+	gc->of_node = bank->of_node;
+	gc->label = bank->name;
 
-		ret = gpiochip_add_data(gc, bank);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n",
-							gc->label, ret);
-			goto fail;
-		}
+	ret = gpiochip_add_data(gc, bank);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register %s (%d)\n", gc->label, ret);
+		return ret;
 	}
 
-	rockchip_interrupts_register(pdev, info);
+	if (!of_property_read_bool(bank->of_node, "gpio-ranges")) {
+		struct device *parent = pdev->dev.parent;
+		struct rockchip_pinctrl *info = dev_get_drvdata(parent);
+		struct pinctrl_dev *pctldev;
 
-	return 0;
+		if (!info)
+			return -ENODATA;
 
-fail:
-	for (--i, --bank; i >= 0; --i, --bank) {
-		if (!bank->valid)
-			continue;
-		gpiochip_remove(&bank->gpio_chip);
-	}
-	return ret;
-}
-
-static int rockchip_gpiolib_unregister(struct platform_device *pdev,
-						struct rockchip_pinctrl *info)
-{
-	struct rockchip_pin_ctrl *ctrl = info->ctrl;
-	struct rockchip_pin_bank *bank = ctrl->pin_banks;
-	int i;
+		pctldev = info->pctl_dev;
+		if (!pctldev)
+			return -ENODEV;
 
-	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-		if (!bank->valid)
-			continue;
-		gpiochip_remove(&bank->gpio_chip);
+		ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0, gc->base, gc->ngpio);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to add pin range\n");
+			gpiochip_remove(&bank->gpio_chip);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -3571,7 +3553,6 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 {
 	const struct of_device_id *match;
 	struct device_node *node = pdev->dev.of_node;
-	struct device_node *np;
 	struct rockchip_pin_ctrl *ctrl;
 	struct rockchip_pin_bank *bank;
 	int grf_offs, pmu_offs, drv_grf_offs, drv_pmu_offs, i, j;
@@ -3579,23 +3560,6 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 	match = of_match_node(rockchip_pinctrl_dt_match, node);
 	ctrl = (struct rockchip_pin_ctrl *)match->data;
 
-	for_each_child_of_node(node, np) {
-		if (!of_find_property(np, "gpio-controller", NULL))
-			continue;
-
-		bank = ctrl->pin_banks;
-		for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
-			if (!strcmp(bank->name, np->name)) {
-				bank->of_node = np;
-
-				if (!rockchip_get_bank_data(bank, d))
-					bank->valid = true;
-
-				break;
-			}
-		}
-	}
-
 	grf_offs = ctrl->grf_mux_offset;
 	pmu_offs = ctrl->pmu_mux_offset;
 	drv_pmu_offs = ctrl->pmu_drv_offset;
@@ -3742,6 +3706,68 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
 			 rockchip_pinctrl_resume);
 
+static int rockchip_gpio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *parent = pdev->dev.parent;
+	struct rockchip_pinctrl *info = dev_get_drvdata(parent);
+	struct rockchip_pin_ctrl *ctrl = info ? (info->ctrl) : NULL;
+	struct rockchip_pin_bank *bank;
+	int ret, i;
+
+	if (!info || !ctrl)
+		return -EINVAL;
+
+	if (!of_find_property(np, "gpio-controller", NULL))
+		return -ENODEV;
+
+	bank = ctrl->pin_banks;
+	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
+		if (!strcmp(bank->name, np->name)) {
+			bank->of_node = np;
+
+			if (!rockchip_get_bank_data(bank, info))
+				bank->valid = true;
+
+			break;
+		}
+	}
+
+	if (!bank->valid) {
+		dev_info(parent, "gpio%d probed\n", bank->bank_num);
+		return -EINVAL;
+	}
+
+	bank->of_node = pdev->dev.of_node;
+
+	ret = rockchip_gpiolib_register(pdev, bank);
+	if (ret)
+		return ret;
+
+	ret = rockchip_interrupts_register(pdev, bank);
+	if (ret) {
+		gpiochip_remove(&bank->gpio_chip);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, bank);
+	dev_info(parent, "gpio%d probed\n", bank->bank_num);
+
+	return 0;
+}
+
+static int rockchip_gpio_remove(struct platform_device *pdev)
+{
+	struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
+
+	if (!bank)
+		return -EINVAL;
+
+	gpiochip_remove(&bank->gpio_chip);
+
+	return 0;
+}
+
 static int rockchip_pinctrl_probe(struct platform_device *pdev)
 {
 	struct rockchip_pinctrl *info;
@@ -3813,17 +3839,20 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 			return PTR_ERR(info->regmap_pmu);
 	}
 
-	ret = rockchip_gpiolib_register(pdev, info);
-	if (ret)
-		return ret;
-
 	ret = rockchip_pinctrl_register(pdev, info);
 	if (ret) {
-		rockchip_gpiolib_unregister(pdev, info);
+		dev_err(&pdev->dev, "failed to register pinctrl device\n");
 		return ret;
 	}
 
 	platform_set_drvdata(pdev, info);
+	dev_info(&pdev->dev, "pinctrl probed\n");
+
+	ret = of_platform_populate(np, rockchip_bank_match, NULL, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register gpio device\n");
+		return ret;
+	}
 
 	return 0;
 }
@@ -4244,8 +4273,17 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
 	{},
 };
 
+static struct platform_driver rockchip_gpio_driver = {
+	.probe	= rockchip_gpio_probe,
+	.remove = rockchip_gpio_remove,
+	.driver = {
+		.name	= "rockchip-gpio",
+		.of_match_table = rockchip_bank_match,
+	},
+};
+
 static struct platform_driver rockchip_pinctrl_driver = {
-	.probe		= rockchip_pinctrl_probe,
+	.probe	= rockchip_pinctrl_probe,
 	.driver = {
 		.name	= "rockchip-pinctrl",
 		.pm = &rockchip_pinctrl_dev_pm_ops,
@@ -4255,11 +4293,18 @@ static struct platform_driver rockchip_pinctrl_driver = {
 
 static int __init rockchip_pinctrl_drv_register(void)
 {
-	return platform_driver_register(&rockchip_pinctrl_driver);
+	int ret;
+
+	ret = platform_driver_register(&rockchip_pinctrl_driver);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&rockchip_gpio_driver);
 }
 
 static void __exit rockchip_pinctrl_drv_unregister(void)
 {
+	platform_driver_unregister(&rockchip_gpio_driver);
 	platform_driver_unregister(&rockchip_pinctrl_driver);
 }
 
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v2 0/5] rockchip-pinctrl fixes for GKI
  2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
                     ` (4 preceding siblings ...)
  2020-09-07  3:38   ` [PATCH 5/5] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
@ 2020-09-12 11:35   ` Linus Walleij
  5 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2020-09-12 11:35 UTC (permalink / raw)
  To: Jianqun Xu
  Cc: Heiko Stübner, open list:GPIO SUBSYSTEM, linux-kernel,
	open list:ARM/Rockchip SoC...

On Mon, Sep 7, 2020 at 4:59 AM Jianqun Xu <jay.xu@rock-chips.com> wrote:

> These patches will fix some issues and modify for GKI.

I am sorry that responses and review is slow. The GKI thing is a bit
controversial leading to slowdowns in the community because it is
unclear how this should be dealt with in some cases.

> Heiko Stuebner (1):
>   pinctrl: rockchip: depend on OF

I already applied this patch separately.

> Jianqun Xu (4):
>   pinctrl: rockchip: make driver be tristate module
>   pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq
>   pinctrl: rockchip: create irq mapping in gpio_to_irq
>   pinctrl: rockchip: populate platform device for rockchip gpio

Why have the big series of 13 patches from july been cut down to this?

I would prefer the "big" fix I even tried applying the old (13 patches)
series but it didn't work :(

We can apply this once they are reviewed, but I'd like the rest of the
13 patches as well I think.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/5] pinctrl: rockchip: make driver be tristate module
  2020-09-07  2:59   ` [PATCH 2/5] pinctrl: rockchip: make driver be tristate module Jianqun Xu
  2020-09-07 11:35     ` kernel test robot
@ 2020-09-12 11:41     ` Heiko Stübner
  2020-09-14  0:38     ` [PATCH 2/2] " Jianqun Xu
  2 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-12 11:41 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Hi,

Am Montag, 7. September 2020, 04:59:24 CEST schrieb Jianqun Xu:
> Make pinctrl-rockchip driver to be tristate module, support to build as
> a module, this is useful for GKI.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/pinctrl/Kconfig            |  2 +-
>  drivers/pinctrl/pinctrl-rockchip.c | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 4284f39a5c61..743eb2bb8709 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
>  	select MFD_SYSCON
>  
>  config PINCTRL_ROCKCHIP
> -	bool
> +	tristate "Rockchip gpio and pinctrl driver"
>  	depends on OF
>  	select PINMUX
>  	select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 0401c1da79dd..cc7512acfc5f 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -16,10 +16,12 @@
>   */
>  
>  #include <linux/init.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
>  #include <linux/bitops.h>
>  #include <linux/gpio/driver.h>
> +#include <linux/of_device.h>

of_device.h below of_address.h please


>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/pinctrl/machine.h>
> @@ -4257,4 +4259,20 @@ static int __init rockchip_pinctrl_drv_register(void)
>  {
>  	return platform_driver_register(&rockchip_pinctrl_driver);
>  }
> +
> +static void __exit rockchip_pinctrl_drv_unregister(void)
> +{
> +	platform_driver_unregister(&rockchip_pinctrl_driver);
> +}
> +
> +#ifdef CONFIG_PINCTRL_ROCKCHIP_MODULE
> +module_init(rockchip_pinctrl_drv_register);
> +#else
>  postcore_initcall(rockchip_pinctrl_drv_register);
> +#endif

You definitly don't need this hack. For modules postcore_initcall
already points to module_init ... see

https://elixir.bootlin.com/linux/latest/source/include/linux/module.h#L114


Heiko



^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
  2020-09-07  2:59   ` [PATCH 2/5] pinctrl: rockchip: make driver be tristate module Jianqun Xu
  2020-09-07 11:35     ` kernel test robot
  2020-09-12 11:41     ` Heiko Stübner
@ 2020-09-14  0:38     ` Jianqun Xu
  2020-09-20 22:14       ` Heiko Stübner
  2 siblings, 1 reply; 32+ messages in thread
From: Jianqun Xu @ 2020-09-14  0:38 UTC (permalink / raw)
  To: linus.walleij, heiko; +Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Make pinctrl-rockchip driver to be tristate module, support to build as
a module, this is useful for GKI.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/Kconfig            |  2 +-
 drivers/pinctrl/pinctrl-rockchip.c | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 4284f39a5c61..743eb2bb8709 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -207,7 +207,7 @@ config PINCTRL_OXNAS
 	select MFD_SYSCON
 
 config PINCTRL_ROCKCHIP
-	bool
+	tristate "Rockchip gpio and pinctrl driver"
 	depends on OF
 	select PINMUX
 	select GENERIC_PINCONF
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 0401c1da79dd..927d132d6716 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -16,10 +16,12 @@
  */
 
 #include <linux/init.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
 #include <linux/gpio/driver.h>
+#include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/pinctrl/machine.h>
@@ -4258,3 +4260,14 @@ static int __init rockchip_pinctrl_drv_register(void)
 	return platform_driver_register(&rockchip_pinctrl_driver);
 }
 postcore_initcall(rockchip_pinctrl_drv_register);
+
+static void __exit rockchip_pinctrl_drv_unregister(void)
+{
+	platform_driver_unregister(&rockchip_pinctrl_driver);
+}
+module_exit(rockchip_pinctrl_drv_unregister);
+
+MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:pinctrl-rockchip");
+MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
-- 
2.17.1




^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
  2020-09-14  0:38     ` [PATCH 2/2] " Jianqun Xu
@ 2020-09-20 22:14       ` Heiko Stübner
  2020-09-20 22:18         ` Heiko Stübner
  0 siblings, 1 reply; 32+ messages in thread
From: Heiko Stübner @ 2020-09-20 22:14 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Montag, 14. September 2020, 02:38:47 CEST schrieb Jianqun Xu:
> Make pinctrl-rockchip driver to be tristate module, support to build as
> a module, this is useful for GKI.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/pinctrl/Kconfig            |  2 +-
>  drivers/pinctrl/pinctrl-rockchip.c | 13 +++++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 4284f39a5c61..743eb2bb8709 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
>  	select MFD_SYSCON
>  
>  config PINCTRL_ROCKCHIP
> -	bool
> +	tristate "Rockchip gpio and pinctrl driver"
>  	depends on OF
>  	select PINMUX
>  	select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 0401c1da79dd..927d132d6716 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -16,10 +16,12 @@
>   */
>  
>  #include <linux/init.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
>  #include <linux/bitops.h>
>  #include <linux/gpio/driver.h>
> +#include <linux/of_device.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/pinctrl/machine.h>
> @@ -4258,3 +4260,14 @@ static int __init rockchip_pinctrl_drv_register(void)
>  	return platform_driver_register(&rockchip_pinctrl_driver);
>  }
>  postcore_initcall(rockchip_pinctrl_drv_register);
> +
> +static void __exit rockchip_pinctrl_drv_unregister(void)
> +{
> +	platform_driver_unregister(&rockchip_pinctrl_driver);
> +}
> +module_exit(rockchip_pinctrl_drv_unregister);
> +
> +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:pinctrl-rockchip");
> +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] pinctrl: rockchip: populate platform device for rockchip gpio
  2020-09-08  2:19     ` [PATCH] " Jianqun Xu
@ 2020-09-20 22:16       ` Heiko Stübner
  0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stübner @ 2020-09-20 22:16 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Am Dienstag, 8. September 2020, 04:19:13 CEST schrieb Jianqun Xu:
> Register both gpio driver and device as part of driver model, so that
> the '-gpio'/'-gpios' dependency in dts can be correctly handled by
> of_devlink/of_fwlink.
> 
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 305 +++++++++++++++++------------
>  1 file changed, 175 insertions(+), 130 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index c98bd352f831..2e4fc711d0d1 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3370,139 +3370,121 @@ static void rockchip_irq_disable(struct irq_data *d)
>  }
>  
>  static int rockchip_interrupts_register(struct platform_device *pdev,
> -						struct rockchip_pinctrl *info)
> +						struct rockchip_pin_bank *bank)
>  {
> -	struct rockchip_pin_ctrl *ctrl = info->ctrl;
> -	struct rockchip_pin_bank *bank = ctrl->pin_banks;
>  	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
>  	struct irq_chip_generic *gc;
>  	int ret;
> -	int i;
>  
> -	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> -		if (!bank->valid) {
> -			dev_warn(&pdev->dev, "bank %s is not valid\n",
> -				 bank->name);
> -			continue;
> -		}
> +	if (!bank->valid) {
> +		dev_warn(&pdev->dev, "bank %s is not valid\n",
> +			 bank->name);
> +		return -EINVAL;
> +	}
>  
> -		ret = clk_enable(bank->clk);
> -		if (ret) {
> -			dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
> -				bank->name);
> -			continue;
> -		}
> +	ret = clk_enable(bank->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
> +			bank->name);
> +		return ret;
> +	}
>  
> -		bank->domain = irq_domain_add_linear(bank->of_node, 32,
> -						&irq_generic_chip_ops, NULL);
> -		if (!bank->domain) {
> -			dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
> -				 bank->name);
> -			clk_disable(bank->clk);
> -			continue;
> -		}
> +	bank->domain = irq_domain_add_linear(bank->of_node, 32,
> +					&irq_generic_chip_ops, NULL);
> +	if (!bank->domain) {
> +		dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
> +			 bank->name);
> +		clk_disable(bank->clk);
> +		return -EINVAL;
> +	}
>  
> -		ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
> -					 "rockchip_gpio_irq", handle_level_irq,
> -					 clr, 0, 0);
> -		if (ret) {
> -			dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
> -				bank->name);
> -			irq_domain_remove(bank->domain);
> -			clk_disable(bank->clk);
> -			continue;
> -		}
> +	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
> +				 "rockchip_gpio_irq", handle_level_irq,
> +				 clr, 0, 0);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
> +			bank->name);
> +		irq_domain_remove(bank->domain);
> +		clk_disable(bank->clk);
> +		return ret;
> +	}
>  
> -		gc = irq_get_domain_generic_chip(bank->domain, 0);
> -		gc->reg_base = bank->reg_base;
> -		gc->private = bank;
> -		gc->chip_types[0].regs.mask = GPIO_INTMASK;
> -		gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
> -		gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
> -		gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> -		gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> -		gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
> -		gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
> -		gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
> -		gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
> -		gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
> -		gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
> -		gc->wake_enabled = IRQ_MSK(bank->nr_pins);
> +	gc = irq_get_domain_generic_chip(bank->domain, 0);
> +	gc->reg_base = bank->reg_base;
> +	gc->private = bank;
> +	gc->chip_types[0].regs.mask = GPIO_INTMASK;
> +	gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
> +	gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
> +	gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> +	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> +	gc->chip_types[0].chip.irq_enable = rockchip_irq_enable;
> +	gc->chip_types[0].chip.irq_disable = rockchip_irq_disable;
> +	gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
> +	gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
> +	gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
> +	gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type;
> +	gc->wake_enabled = IRQ_MSK(bank->nr_pins);
>  
> -		/*
> -		 * Linux assumes that all interrupts start out disabled/masked.
> -		 * Our driver only uses the concept of masked and always keeps
> -		 * things enabled, so for us that's all masked and all enabled.
> -		 */
> -		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
> -		writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
> -		gc->mask_cache = 0xffffffff;
> +	/*
> +	 * Linux assumes that all interrupts start out disabled/masked.
> +	 * Our driver only uses the concept of masked and always keeps
> +	 * things enabled, so for us that's all masked and all enabled.
> +	 */
> +	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTMASK);
> +	writel_relaxed(0xffffffff, bank->reg_base + GPIO_INTEN);
> +	gc->mask_cache = 0xffffffff;
>  
> -		irq_set_chained_handler_and_data(bank->irq,
> -						 rockchip_irq_demux, bank);
> -		clk_disable(bank->clk);
> -	}
> +	irq_set_chained_handler_and_data(bank->irq,
> +					 rockchip_irq_demux, bank);
> +	clk_disable(bank->clk);
>  
>  	return 0;
>  }
>  
>  static int rockchip_gpiolib_register(struct platform_device *pdev,
> -						struct rockchip_pinctrl *info)
> +						struct rockchip_pin_bank *bank)
>  {
> -	struct rockchip_pin_ctrl *ctrl = info->ctrl;
> -	struct rockchip_pin_bank *bank = ctrl->pin_banks;
>  	struct gpio_chip *gc;
>  	int ret;
> -	int i;
>  
> -	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> -		if (!bank->valid) {
> -			dev_warn(&pdev->dev, "bank %s is not valid\n",
> -				 bank->name);
> -			continue;
> -		}
> +	if (!bank->valid) {
> +		dev_err(&pdev->dev, "bank %s is not valid\n", bank->name);
> +		return -EINVAL;
> +	}
>  
> -		bank->gpio_chip = rockchip_gpiolib_chip;
> +	bank->gpio_chip = rockchip_gpiolib_chip;
>  
> -		gc = &bank->gpio_chip;
> -		gc->base = bank->pin_base;
> -		gc->ngpio = bank->nr_pins;
> -		gc->parent = &pdev->dev;
> -		gc->of_node = bank->of_node;
> -		gc->label = bank->name;
> +	gc = &bank->gpio_chip;
> +	gc->base = bank->pin_base;
> +	gc->ngpio = bank->nr_pins;
> +	gc->parent = &pdev->dev;
> +	gc->of_node = bank->of_node;
> +	gc->label = bank->name;
>  
> -		ret = gpiochip_add_data(gc, bank);
> -		if (ret) {
> -			dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n",
> -							gc->label, ret);
> -			goto fail;
> -		}
> +	ret = gpiochip_add_data(gc, bank);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register %s (%d)\n", gc->label, ret);
> +		return ret;
>  	}
>  
> -	rockchip_interrupts_register(pdev, info);
> +	if (!of_property_read_bool(bank->of_node, "gpio-ranges")) {
> +		struct device *parent = pdev->dev.parent;
> +		struct rockchip_pinctrl *info = dev_get_drvdata(parent);
> +		struct pinctrl_dev *pctldev;
>  
> -	return 0;
> +		if (!info)
> +			return -ENODATA;
>  
> -fail:
> -	for (--i, --bank; i >= 0; --i, --bank) {
> -		if (!bank->valid)
> -			continue;
> -		gpiochip_remove(&bank->gpio_chip);
> -	}
> -	return ret;
> -}
> -
> -static int rockchip_gpiolib_unregister(struct platform_device *pdev,
> -						struct rockchip_pinctrl *info)
> -{
> -	struct rockchip_pin_ctrl *ctrl = info->ctrl;
> -	struct rockchip_pin_bank *bank = ctrl->pin_banks;
> -	int i;
> +		pctldev = info->pctl_dev;
> +		if (!pctldev)
> +			return -ENODEV;
>  
> -	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> -		if (!bank->valid)
> -			continue;
> -		gpiochip_remove(&bank->gpio_chip);
> +		ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0, gc->base, gc->ngpio);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to add pin range\n");
> +			gpiochip_remove(&bank->gpio_chip);
> +			return ret;
> +		}
>  	}
>  
>  	return 0;
> @@ -3571,7 +3553,6 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
>  {
>  	const struct of_device_id *match;
>  	struct device_node *node = pdev->dev.of_node;
> -	struct device_node *np;
>  	struct rockchip_pin_ctrl *ctrl;
>  	struct rockchip_pin_bank *bank;
>  	int grf_offs, pmu_offs, drv_grf_offs, drv_pmu_offs, i, j;
> @@ -3579,23 +3560,6 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
>  	match = of_match_node(rockchip_pinctrl_dt_match, node);
>  	ctrl = (struct rockchip_pin_ctrl *)match->data;
>  
> -	for_each_child_of_node(node, np) {
> -		if (!of_find_property(np, "gpio-controller", NULL))
> -			continue;
> -
> -		bank = ctrl->pin_banks;
> -		for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> -			if (!strcmp(bank->name, np->name)) {
> -				bank->of_node = np;
> -
> -				if (!rockchip_get_bank_data(bank, d))
> -					bank->valid = true;
> -
> -				break;
> -			}
> -		}
> -	}
> -
>  	grf_offs = ctrl->grf_mux_offset;
>  	pmu_offs = ctrl->pmu_mux_offset;
>  	drv_pmu_offs = ctrl->pmu_drv_offset;
> @@ -3742,6 +3706,68 @@ static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
>  			 rockchip_pinctrl_resume);
>  
> +static int rockchip_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *parent = pdev->dev.parent;
> +	struct rockchip_pinctrl *info = dev_get_drvdata(parent);
> +	struct rockchip_pin_ctrl *ctrl = info ? (info->ctrl) : NULL;
> +	struct rockchip_pin_bank *bank;
> +	int ret, i;
> +
> +	if (!info || !ctrl)
> +		return -EINVAL;
> +
> +	if (!of_find_property(np, "gpio-controller", NULL))
> +		return -ENODEV;
> +
> +	bank = ctrl->pin_banks;
> +	for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
> +		if (!strcmp(bank->name, np->name)) {
> +			bank->of_node = np;
> +
> +			if (!rockchip_get_bank_data(bank, info))
> +				bank->valid = true;
> +
> +			break;
> +		}
> +	}
> +
> +	if (!bank->valid) {
> +		dev_info(parent, "gpio%d probed\n", bank->bank_num);
> +		return -EINVAL;
> +	}
> +
> +	bank->of_node = pdev->dev.of_node;
> +
> +	ret = rockchip_gpiolib_register(pdev, bank);
> +	if (ret)
> +		return ret;
> +
> +	ret = rockchip_interrupts_register(pdev, bank);
> +	if (ret) {
> +		gpiochip_remove(&bank->gpio_chip);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, bank);
> +	dev_info(parent, "gpio%d probed\n", bank->bank_num);
> +
> +	return 0;
> +}
> +
> +static int rockchip_gpio_remove(struct platform_device *pdev)
> +{
> +	struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
> +
> +	if (!bank)
> +		return -EINVAL;
> +
> +	gpiochip_remove(&bank->gpio_chip);
> +
> +	return 0;
> +}
> +
>  static int rockchip_pinctrl_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_pinctrl *info;
> @@ -3813,17 +3839,20 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
>  			return PTR_ERR(info->regmap_pmu);
>  	}
>  
> -	ret = rockchip_gpiolib_register(pdev, info);
> -	if (ret)
> -		return ret;
> -
>  	ret = rockchip_pinctrl_register(pdev, info);
>  	if (ret) {
> -		rockchip_gpiolib_unregister(pdev, info);
> +		dev_err(&pdev->dev, "failed to register pinctrl device\n");
>  		return ret;
>  	}
>  
>  	platform_set_drvdata(pdev, info);
> +	dev_info(&pdev->dev, "pinctrl probed\n");
> +
> +	ret = of_platform_populate(np, rockchip_bank_match, NULL, dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register gpio device\n");
> +		return ret;
> +	}
>  
>  	return 0;
>  }
> @@ -4244,8 +4273,17 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
>  	{},
>  };
>  
> +static struct platform_driver rockchip_gpio_driver = {
> +	.probe	= rockchip_gpio_probe,
> +	.remove = rockchip_gpio_remove,
> +	.driver = {
> +		.name	= "rockchip-gpio",
> +		.of_match_table = rockchip_bank_match,
> +	},
> +};
> +
>  static struct platform_driver rockchip_pinctrl_driver = {
> -	.probe		= rockchip_pinctrl_probe,
> +	.probe	= rockchip_pinctrl_probe,
>  	.driver = {
>  		.name	= "rockchip-pinctrl",
>  		.pm = &rockchip_pinctrl_dev_pm_ops,
> @@ -4255,11 +4293,18 @@ static struct platform_driver rockchip_pinctrl_driver = {
>  
>  static int __init rockchip_pinctrl_drv_register(void)
>  {
> -	return platform_driver_register(&rockchip_pinctrl_driver);
> +	int ret;
> +
> +	ret = platform_driver_register(&rockchip_pinctrl_driver);
> +	if (ret)
> +		return ret;
> +
> +	return platform_driver_register(&rockchip_gpio_driver);
>  }
>  
>  static void __exit rockchip_pinctrl_drv_unregister(void)
>  {
> +	platform_driver_unregister(&rockchip_gpio_driver);
>  	platform_driver_unregister(&rockchip_pinctrl_driver);
>  }
>  
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
  2020-09-20 22:14       ` Heiko Stübner
@ 2020-09-20 22:18         ` Heiko Stübner
  2020-09-29 13:21           ` Linus Walleij
  0 siblings, 1 reply; 32+ messages in thread
From: Heiko Stübner @ 2020-09-20 22:18 UTC (permalink / raw)
  To: linus.walleij, Jianqun Xu
  Cc: linux-gpio, linux-kernel, linux-rockchip, Jianqun Xu

Hi Jay,

Am Montag, 21. September 2020, 00:14:11 CEST schrieb Heiko Stübner:
> Am Montag, 14. September 2020, 02:38:47 CEST schrieb Jianqun Xu:
> > Make pinctrl-rockchip driver to be tristate module, support to build as
> > a module, this is useful for GKI.
> > 
> > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

It seems I've reviewed all patches of this series now, but I think
you might want to resend the series a final time as v3 in a cleaned up
state (drop patch1 and just post patches 2-5 in a full series) so that
we don't confuse Linus too much with the reposted patches we currently
have.

Thanks
Heiko




> 
> > ---
> >  drivers/pinctrl/Kconfig            |  2 +-
> >  drivers/pinctrl/pinctrl-rockchip.c | 13 +++++++++++++
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index 4284f39a5c61..743eb2bb8709 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -207,7 +207,7 @@ config PINCTRL_OXNAS
> >  	select MFD_SYSCON
> >  
> >  config PINCTRL_ROCKCHIP
> > -	bool
> > +	tristate "Rockchip gpio and pinctrl driver"
> >  	depends on OF
> >  	select PINMUX
> >  	select GENERIC_PINCONF
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> > index 0401c1da79dd..927d132d6716 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -16,10 +16,12 @@
> >   */
> >  
> >  #include <linux/init.h>
> > +#include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/io.h>
> >  #include <linux/bitops.h>
> >  #include <linux/gpio/driver.h>
> > +#include <linux/of_device.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/pinctrl/machine.h>
> > @@ -4258,3 +4260,14 @@ static int __init rockchip_pinctrl_drv_register(void)
> >  	return platform_driver_register(&rockchip_pinctrl_driver);
> >  }
> >  postcore_initcall(rockchip_pinctrl_drv_register);
> > +
> > +static void __exit rockchip_pinctrl_drv_unregister(void)
> > +{
> > +	platform_driver_unregister(&rockchip_pinctrl_driver);
> > +}
> > +module_exit(rockchip_pinctrl_drv_unregister);
> > +
> > +MODULE_DESCRIPTION("ROCKCHIP Pin Controller Driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:pinctrl-rockchip");
> > +MODULE_DEVICE_TABLE(of, rockchip_pinctrl_dt_match);
> > 
> 
> 





^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
  2020-09-20 22:18         ` Heiko Stübner
@ 2020-09-29 13:21           ` Linus Walleij
  2020-10-13  6:40             ` jay.xu
  0 siblings, 1 reply; 32+ messages in thread
From: Linus Walleij @ 2020-09-29 13:21 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Jianqun Xu, open list:GPIO SUBSYSTEM, linux-kernel,
	open list:ARM/Rockchip SoC...

On Mon, Sep 21, 2020 at 12:18 AM Heiko Stübner <heiko@sntech.de> wrote:

> It seems I've reviewed all patches of this series now, but I think
> you might want to resend the series a final time as v3 in a cleaned up
> state (drop patch1 and just post patches 2-5 in a full series) so that
> we don't confuse Linus too much with the reposted patches we currently
> have.

Yes please send a v3 like that so I can apply it!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Re: [PATCH 2/2] pinctrl: rockchip: make driver be tristate module
  2020-09-29 13:21           ` Linus Walleij
@ 2020-10-13  6:40             ` jay.xu
  0 siblings, 0 replies; 32+ messages in thread
From: jay.xu @ 2020-10-13  6:40 UTC (permalink / raw)
  To: Linus Walleij, Heiko Stübner
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, open list:ARM/Rockchip SoC...

Dear Walleij and Heiko

The patch "[PATCH] pinctrl: rockchip: populate platform device for rockchip gpio" has been remove outof
the patches, since we have a new patch to move gpio related codes to a separated driver.

So please ignore it for now. thanks very much.

--------------
jay.xu@rock-chips.com
>On Mon, Sep 21, 2020 at 12:18 AM Heiko Stübner <heiko@sntech.de> wrote:
>
>> It seems I've reviewed all patches of this series now, but I think
>> you might want to resend the series a final time as v3 in a cleaned up
>> state (drop patch1 and just post patches 2-5 in a full series) so that
>> we don't confuse Linus too much with the reposted patches we currently
>> have.
>
>Yes please send a v3 like that so I can apply it!
>
>Yours,
>Linus Walleij
>
>
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2020-10-13  6:48 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31  8:47 [PATCH 0/6] rockchip-pinctrl fixes for GKI Jianqun Xu
2020-08-31  8:47 ` [PATCH 1/6] pinctrl: rockchip: make driver be tristate module Jianqun Xu
2020-09-01 10:13   ` kernel test robot
2020-09-05 21:51     ` Heiko Stübner
2020-09-05 22:01   ` Heiko Stübner
2020-09-05 22:23     ` Heiko Stübner
2020-08-31  8:47 ` [PATCH 2/6] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
2020-09-05 21:54   ` Heiko Stübner
2020-08-31  8:47 ` [PATCH 3/6] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
2020-09-05 22:03   ` Heiko Stübner
2020-08-31  8:47 ` [PATCH 4/6] pinctrl: rockchip: do not set gpio if bank invalid Jianqun Xu
2020-09-05 22:09   ` Heiko Stübner
2020-08-31  8:50 ` [PATCH 5/6] pinctrl: rockchip: fix crash caused by invalid gpio bank Jianqun Xu
2020-09-05 22:14   ` Heiko Stübner
2020-08-31  8:50 ` [PATCH 6/6] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
2020-09-06 10:20   ` Heiko Stübner
2020-09-07  2:59 ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Jianqun Xu
2020-09-07  2:59   ` [PATCH 1/5] pinctrl: rockchip: depend on OF Jianqun Xu
2020-09-07  2:59   ` [PATCH 2/5] pinctrl: rockchip: make driver be tristate module Jianqun Xu
2020-09-07 11:35     ` kernel test robot
2020-09-12 11:41     ` Heiko Stübner
2020-09-14  0:38     ` [PATCH 2/2] " Jianqun Xu
2020-09-20 22:14       ` Heiko Stübner
2020-09-20 22:18         ` Heiko Stübner
2020-09-29 13:21           ` Linus Walleij
2020-10-13  6:40             ` jay.xu
2020-09-07  2:59   ` [PATCH 3/5] pinctrl: rockchip: enable gpio pclk for rockchip_gpio_to_irq Jianqun Xu
2020-09-07  2:59   ` [PATCH 4/5] pinctrl: rockchip: create irq mapping in gpio_to_irq Jianqun Xu
2020-09-07  3:38   ` [PATCH 5/5] pinctrl: rockchip: populate platform device for rockchip gpio Jianqun Xu
2020-09-08  2:19     ` [PATCH] " Jianqun Xu
2020-09-20 22:16       ` Heiko Stübner
2020-09-12 11:35   ` [PATCH v2 0/5] rockchip-pinctrl fixes for GKI Linus Walleij

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).