[PATCH 00/20] Rockchip pinctrl/GPIO support ACPI

[PATCH 00/20] Rockchip pinctrl/GPIO support ACPI

[Jianqun Xu]

The patchset will fix the pinctrl and GPIO driver to support ACPI
enabled. There are several patches sent seperated for pinctrl and gpio
branch, now let send them together make it easy to review.

Andy Shevchenko (1):
  ACPI: utils: Add acpi_dev_uid_to_integer() helper to get _UID as
    integer

Jianqun Xu (19):
  pinctrl/rockchip: populate GPIO platform early
  pinctrl/rockchip: use fwnode instead of of_node
  pinctrl/rockchip: switch to use device_get_match_data
  pinctrl/rockchip: of_populate_platform for gpio only for of node
  pinctrl/rockchip: parse dt only if the fwnode is of node
  pinctrl/rockchip: print a message if driver probed successfully
  gpiolib: make gpiochip_find_by_name to be common function
  gpio/rockchip: drop 'bank->name' from the driver
  gpio/rockchip: revert deferred output settings on probe
  gpio/rockchip: add of_node for gpiochip
  gpio/rockchip: add return check for clock rate set
  gpio/rockchip: disable and put clocks when remove
  gpio/rockchip: switch to use irq_domain_create_linear
  gpio/rockchip: disable and put clocks when gpiolib register failed
  gpio/rockchip: try to get gpio id from uid when ACPI enabled
  gpio/rockchip: print device with fwnode name when probe successful
  gpio/rockchip: make use of device property
  gpio/rockchip: make GPIO module work well under ACPI enabled
  pinctrl/rockchip: find existed gpiochip by gpio label

 drivers/acpi/utils.c               |  24 +++
 drivers/gpio/gpio-rockchip.c       | 325 ++++++++++++++++-------------
 drivers/gpio/gpiolib.c             |  16 +-
 drivers/pinctrl/pinctrl-rockchip.c | 108 +++++-----
 drivers/pinctrl/pinctrl-rockchip.h |   9 +-
 include/acpi/acpi_bus.h            |   1 +
 include/linux/acpi.h               |   5 +
 include/linux/gpio/driver.h        |  12 ++
 8 files changed, 289 insertions(+), 211 deletions(-)

-- 
v1:
 - continue with the suggestion from Andy and Johan

2.25.1

[PATCH 01/20] ACPI: utils: Add acpi_dev_uid_to_integer() helper to get _UID as integer

[Jianqun Xu]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Some users interpret _UID only as integer and for them it's easier to
have an integer representation of _UID. Add respective helper for that.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/utils.c    | 24 ++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 include/linux/acpi.h    |  5 +++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 5a7b8065e77f..febf9b8da3a0 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -793,6 +793,30 @@ bool acpi_dev_hid_uid_match(struct acpi_device *adev,
 }
 EXPORT_SYMBOL(acpi_dev_hid_uid_match);
 
+/**
+ * acpi_dev_uid_to_integer - treat ACPI device _UID as integer
+ * @adev: ACPI device to get _UID from
+ * @integer: output buffer for integer
+ *
+ * Considers _UID as integer and converts it to @integer.
+ *
+ * Returns 0 on success, or negative error code otherwise.
+ */
+int acpi_dev_uid_to_integer(struct acpi_device *adev, u64 *integer)
+{
+	const char *uid;
+
+	if (!adev)
+		return -ENODEV;
+
+	uid = acpi_device_uid(adev);
+	if (!uid)
+		return -ENODATA;
+
+	return kstrtou64(uid, 0, integer);
+}
+EXPORT_SYMBOL(acpi_dev_uid_to_integer);
+
 /**
  * acpi_dev_found - Detect presence of a given ACPI device in the namespace.
  * @hid: Hardware ID of the device.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e7d27373ff71..bd0db916f330 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -733,6 +733,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
 }
 
 bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
+int acpi_dev_uid_to_integer(struct acpi_device *adev, u64 *integer);
 
 void acpi_dev_clear_dependencies(struct acpi_device *supplier);
 bool acpi_dev_ready_for_enumeration(const struct acpi_device *device);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6f64b2f3dc54..9434db02cb60 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -798,6 +798,11 @@ acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *u
 	return false;
 }
 
+static inline int acpi_dev_uid_to_integer(struct acpi_device *adev, u64 *integer)
+{
+	return -ENODEV;
+}
+
 static inline struct acpi_device *
 acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
 {
-- 
2.25.1

[PATCH 02/20] pinctrl/rockchip: populate GPIO platform early

[Jianqun Xu]

It is better to register GPIO devices before pinctrl device, so move the
populate GPIO platform early before pinctrl do really probe.

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

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index a91061f9c2ac..25633c7b4244 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3302,6 +3302,10 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 	if (!dev->of_node)
 		return dev_err_probe(dev, -ENODEV, "device tree node not found\n");
 
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register gpio device\n");
+
 	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
@@ -3360,10 +3364,6 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, info);
 
-	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to register gpio device\n");
-
 	return 0;
 }
 
-- 
2.25.1

[PATCH 03/20] pinctrl/rockchip: use fwnode instead of of_node

[Jianqun Xu]

Switch the pinctrl driver to use fwnode instead of the of_node.

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

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 25633c7b4244..723b44edc1f2 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3033,8 +3033,8 @@ static int rockchip_pinctrl_parse_dt(struct platform_device *pdev,
 					      struct rockchip_pinctrl *info)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
-	struct device_node *child;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	struct device_node *np = to_of_node(fwnode), *child;
 	int ret;
 	int i;
 
@@ -3129,7 +3129,8 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 						struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *node = dev->of_node;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	struct device_node *node = to_of_node(fwnode);
 	const struct of_device_id *match;
 	struct rockchip_pin_ctrl *ctrl;
 	struct rockchip_pin_bank *bank;
@@ -3291,15 +3292,16 @@ static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
 
 static int rockchip_pinctrl_probe(struct platform_device *pdev)
 {
-	struct rockchip_pinctrl *info;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node, *node;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	struct device_node *np = to_of_node(fwnode), *node;
+	struct rockchip_pinctrl *info;
 	struct rockchip_pin_ctrl *ctrl;
 	struct resource *res;
 	void __iomem *base;
 	int ret;
 
-	if (!dev->of_node)
+	if (!is_of_node(fwnode))
 		return dev_err_probe(dev, -ENODEV, "device tree node not found\n");
 
 	ret = of_platform_populate(np, NULL, NULL, dev);
-- 
2.25.1

[PATCH 04/20] pinctrl/rockchip: switch to use device_get_match_data

[Jianqun Xu]

Switch to use device_get_match_data to get match data.

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

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 723b44edc1f2..8f102f327af8 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3129,16 +3129,11 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 						struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct fwnode_handle *fwnode = dev_fwnode(dev);
-	struct device_node *node = to_of_node(fwnode);
-	const struct of_device_id *match;
 	struct rockchip_pin_ctrl *ctrl;
 	struct rockchip_pin_bank *bank;
 	int grf_offs, pmu_offs, drv_grf_offs, drv_pmu_offs, i, j;
 
-	match = of_match_node(rockchip_pinctrl_dt_match, node);
-	ctrl = (struct rockchip_pin_ctrl *)match->data;
-
+	ctrl = (struct rockchip_pin_ctrl *)device_get_match_data(dev);
 	grf_offs = ctrl->grf_mux_offset;
 	pmu_offs = ctrl->pmu_mux_offset;
 	drv_pmu_offs = ctrl->pmu_drv_offset;
-- 
2.25.1

Re: [PATCH 04/20] pinctrl/rockchip: switch to use device_get_match_data

[Linus Walleij]

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:

> -       match = of_match_node(rockchip_pinctrl_dt_match, node);
> -       ctrl = (struct rockchip_pin_ctrl *)match->data;
> -
> +       ctrl = (struct rockchip_pin_ctrl *)device_get_match_data(dev);

Drop the cast: these pointers are void * and can be assigned to anything.

Yours,
Linus Walleij

[PATCH 05/20] pinctrl/rockchip: of_populate_platform for gpio only for of node

[Jianqun Xu]

As the of_populate_platform named with prefix "of_", it should be done
only when the of node is exist.

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

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 8f102f327af8..42aa3552417a 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3296,12 +3296,11 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int ret;
 
-	if (!is_of_node(fwnode))
-		return dev_err_probe(dev, -ENODEV, "device tree node not found\n");
-
-	ret = of_platform_populate(np, NULL, NULL, dev);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to register gpio device\n");
+	if (is_of_node(fwnode)) {
+		ret = of_platform_populate(np, NULL, NULL, dev);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to register gpio device\n");
+	}
 
 	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
-- 
2.25.1

[PATCH 06/20] pinctrl/rockchip: parse dt only if the fwnode is of node

[Jianqun Xu]

Check if the fwnode is a of node before parsing dt, otherwise return a
'0' to allow driver to continue.

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

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 42aa3552417a..7b8d3bff9721 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3038,6 +3038,9 @@ static int rockchip_pinctrl_parse_dt(struct platform_device *pdev,
 	int ret;
 	int i;
 
+	if (!is_of_node(fwnode))
+		return 0;
+
 	rockchip_pinctrl_child_count(info, np);
 
 	dev_dbg(dev, "nfunctions = %d\n", info->nfunctions);
-- 
2.25.1

[PATCH 07/20] pinctrl/rockchip: print a message if driver probed successfully

[Jianqun Xu]

Add a print for the pinctrl driver to output a message when driver
probed successfully.

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

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 7b8d3bff9721..801a1f6ca4db 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3362,6 +3362,7 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 		return ret;
 
 	platform_set_drvdata(pdev, info);
+	dev_info(dev, "probed %pfw\n", fwnode);
 
 	return 0;
 }
-- 
2.25.1

[PATCH 08/20] gpiolib: make gpiochip_find_by_name to be common function

[Jianqun Xu]

Move find_chip_by_name from gpiolib to the gpio/driver.h, also rename to
gpiochip_find_by_name, make it to be a common function.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/gpio/gpiolib.c      | 16 ++--------------
 include/linux/gpio/driver.h | 12 ++++++++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cc9c0a12259e..c06334772c47 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -935,18 +935,6 @@ struct gpio_chip *gpiochip_find(void *data,
 }
 EXPORT_SYMBOL_GPL(gpiochip_find);
 
-static int gpiochip_match_name(struct gpio_chip *gc, void *data)
-{
-	const char *name = data;
-
-	return !strcmp(gc->label, name);
-}
-
-static struct gpio_chip *find_chip_by_name(const char *name)
-{
-	return gpiochip_find((void *)name, gpiochip_match_name);
-}
-
 #ifdef CONFIG_GPIOLIB_IRQCHIP
 
 /*
@@ -3660,7 +3648,7 @@ void gpiod_add_hogs(struct gpiod_hog *hogs)
 		 * The chip may have been registered earlier, so check if it
 		 * exists and, if so, try to hog the line now.
 		 */
-		gc = find_chip_by_name(hog->chip_label);
+		gc = gpiochip_find_by_name(hog->chip_label);
 		if (gc)
 			gpiochip_machine_hog(gc, hog);
 	}
@@ -3745,7 +3733,7 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
 			return ERR_PTR(-EPROBE_DEFER);
 		}
 
-		gc = find_chip_by_name(p->key);
+		gc = gpiochip_find_by_name(p->key);
 
 		if (!gc) {
 			/*
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 6aeea1071b1b..4ed26a7d98ff 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -618,6 +618,18 @@ extern int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip
 extern struct gpio_chip *gpiochip_find(void *data,
 			      int (*match)(struct gpio_chip *gc, void *data));
 
+static int gpiochip_match_name(struct gpio_chip *gc, void *data)
+{
+	const char *name = data;
+
+	return !strcmp(gc->label, name);
+}
+
+static inline struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	return gpiochip_find((void *)name, gpiochip_match_name);
+}
+
 bool gpiochip_line_is_irq(struct gpio_chip *gc, unsigned int offset);
 int gpiochip_reqres_irq(struct gpio_chip *gc, unsigned int offset);
 void gpiochip_relres_irq(struct gpio_chip *gc, unsigned int offset);
-- 
2.25.1

[PATCH 09/20] gpio/rockchip: drop 'bank->name' from the driver

[Jianqun Xu]

Not to use the 'bank->name' and create 'gc->lable' by the bank number.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index bb50335239ac..dafcc8be1687 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -327,7 +327,7 @@ static void rockchip_irq_demux(struct irq_desc *desc)
 	struct rockchip_pin_bank *bank = irq_desc_get_handler_data(desc);
 	u32 pend;
 
-	dev_dbg(bank->dev, "got irq for bank %s\n", bank->name);
+	dev_dbg(bank->dev, "got irq\n");
 
 	chained_irq_enter(chip, desc);
 
@@ -521,8 +521,7 @@ static int rockchip_interrupts_register(struct rockchip_pin_bank *bank)
 	bank->domain = irq_domain_add_linear(bank->of_node, 32,
 					&irq_generic_chip_ops, NULL);
 	if (!bank->domain) {
-		dev_warn(bank->dev, "could not init irq domain for bank %s\n",
-			 bank->name);
+		dev_warn(bank->dev, "could not init irq domain\n");
 		return -EINVAL;
 	}
 
@@ -531,8 +530,7 @@ static int rockchip_interrupts_register(struct rockchip_pin_bank *bank)
 					     handle_level_irq,
 					     clr, 0, 0);
 	if (ret) {
-		dev_err(bank->dev, "could not alloc generic chips for bank %s\n",
-			bank->name);
+		dev_err(bank->dev, "could not alloc generic chips\n");
 		irq_domain_remove(bank->domain);
 		return -EINVAL;
 	}
@@ -586,8 +584,10 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
 	gc = &bank->gpio_chip;
 	gc->base = bank->pin_base;
 	gc->ngpio = bank->nr_pins;
-	gc->label = bank->name;
 	gc->parent = bank->dev;
+	gc->label = devm_kasprintf(bank->dev, GFP_KERNEL, "gpio%d", bank->bank_num);
+	if (!gc->label)
+		return -ENOMEM;
 
 	ret = gpiochip_add_data(gc, bank);
 	if (ret) {
-- 
2.25.1

[PATCH 10/20] gpio/rockchip: revert deferred output settings on probe

[Jianqun Xu]

This patch revert 3 patches:
  Revert "gpio/rockchip: handle deferring input-enable pinconfs"
  This reverts commit 7ff11357810fd124825fdd9aaf0df90262b77844.

  Revert "pinctrl/rockchip: support deferring other gpio params"
  This reverts commit 8ce5ef64546850294b021497046588a7abcebe96.

  Revert "gpio/rockchip: fetch deferred output settings on probe"
  This reverts commit 59dd178e1d7cb6cac03b32aba7ed9bbce6761b6f.

A patch for pinctrl-rockchip must be applied to make sure the gpiochips
have been registered before the pinctrl to register.
---
 drivers/gpio/gpio-rockchip.c       | 35 -------------------
 drivers/pinctrl/pinctrl-rockchip.c | 54 ++++++++++++++----------------
 drivers/pinctrl/pinctrl-rockchip.h |  7 ++--
 3 files changed, 27 insertions(+), 69 deletions(-)

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index dafcc8be1687..b294ef009daf 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -19,7 +19,6 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
-#include <linux/pinctrl/pinconf-generic.h>
 #include <linux/regmap.h>
 
 #include "../pinctrl/core.h"
@@ -708,7 +707,6 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	struct device_node *pctlnp = of_get_parent(np);
 	struct pinctrl_dev *pctldev = NULL;
 	struct rockchip_pin_bank *bank = NULL;
-	struct rockchip_pin_deferred *cfg;
 	static int gpio;
 	int id, ret;
 
@@ -736,45 +734,12 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/*
-	 * Prevent clashes with a deferred output setting
-	 * being added right at this moment.
-	 */
-	mutex_lock(&bank->deferred_lock);
-
 	ret = rockchip_gpiolib_register(bank);
 	if (ret) {
 		clk_disable_unprepare(bank->clk);
-		mutex_unlock(&bank->deferred_lock);
 		return ret;
 	}
 
-	while (!list_empty(&bank->deferred_pins)) {
-		cfg = list_first_entry(&bank->deferred_pins,
-				       struct rockchip_pin_deferred, head);
-		list_del(&cfg->head);
-
-		switch (cfg->param) {
-		case PIN_CONFIG_OUTPUT:
-			ret = rockchip_gpio_direction_output(&bank->gpio_chip, cfg->pin, cfg->arg);
-			if (ret)
-				dev_warn(dev, "setting output pin %u to %u failed\n", cfg->pin,
-					 cfg->arg);
-			break;
-		case PIN_CONFIG_INPUT_ENABLE:
-			ret = rockchip_gpio_direction_input(&bank->gpio_chip, cfg->pin);
-			if (ret)
-				dev_warn(dev, "setting input pin %u failed\n", cfg->pin);
-			break;
-		default:
-			dev_warn(dev, "unknown deferred config param %d\n", cfg->param);
-			break;
-		}
-		kfree(cfg);
-	}
-
-	mutex_unlock(&bank->deferred_lock);
-
 	platform_set_drvdata(pdev, bank);
 	dev_info(dev, "probed %pOF\n", np);
 
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 801a1f6ca4db..97af52bb5481 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -2702,20 +2702,19 @@ static bool rockchip_pinconf_pull_valid(struct rockchip_pin_ctrl *ctrl,
 	return false;
 }
 
-static int rockchip_pinconf_defer_pin(struct rockchip_pin_bank *bank,
-					 unsigned int pin, u32 param, u32 arg)
+static int rockchip_pinconf_defer_output(struct rockchip_pin_bank *bank,
+					 unsigned int pin, u32 arg)
 {
-	struct rockchip_pin_deferred *cfg;
+	struct rockchip_pin_output_deferred *cfg;
 
 	cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
 	if (!cfg)
 		return -ENOMEM;
 
 	cfg->pin = pin;
-	cfg->param = param;
 	cfg->arg = arg;
 
-	list_add_tail(&cfg->head, &bank->deferred_pins);
+	list_add_tail(&cfg->head, &bank->deferred_output);
 
 	return 0;
 }
@@ -2736,25 +2735,6 @@ static int rockchip_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 		param = pinconf_to_config_param(configs[i]);
 		arg = pinconf_to_config_argument(configs[i]);
 
-		if (param == PIN_CONFIG_OUTPUT || param == PIN_CONFIG_INPUT_ENABLE) {
-			/*
-			 * Check for gpio driver not being probed yet.
-			 * The lock makes sure that either gpio-probe has completed
-			 * or the gpio driver hasn't probed yet.
-			 */
-			mutex_lock(&bank->deferred_lock);
-			if (!gpio || !gpio->direction_output) {
-				rc = rockchip_pinconf_defer_pin(bank, pin - bank->pin_base, param,
-								arg);
-				mutex_unlock(&bank->deferred_lock);
-				if (rc)
-					return rc;
-
-				break;
-			}
-			mutex_unlock(&bank->deferred_lock);
-		}
-
 		switch (param) {
 		case PIN_CONFIG_BIAS_DISABLE:
 			rc =  rockchip_set_pull(bank, pin - bank->pin_base,
@@ -2783,6 +2763,22 @@ static int rockchip_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 			if (rc != RK_FUNC_GPIO)
 				return -EINVAL;
 
+			/*
+			 * Check for gpio driver not being probed yet.
+			 * The lock makes sure that either gpio-probe has completed
+			 * or the gpio driver hasn't probed yet.
+			 */
+			mutex_lock(&bank->deferred_lock);
+			if (!gpio || !gpio->direction_output) {
+				rc = rockchip_pinconf_defer_output(bank, pin - bank->pin_base, arg);
+				mutex_unlock(&bank->deferred_lock);
+				if (rc)
+					return rc;
+
+				break;
+			}
+			mutex_unlock(&bank->deferred_lock);
+
 			rc = gpio->direction_output(gpio, pin - bank->pin_base,
 						    arg);
 			if (rc)
@@ -3109,7 +3105,7 @@ static int rockchip_pinctrl_register(struct platform_device *pdev,
 			pdesc++;
 		}
 
-		INIT_LIST_HEAD(&pin_bank->deferred_pins);
+		INIT_LIST_HEAD(&pin_bank->deferred_output);
 		mutex_init(&pin_bank->deferred_lock);
 	}
 
@@ -3371,7 +3367,7 @@ static int rockchip_pinctrl_remove(struct platform_device *pdev)
 {
 	struct rockchip_pinctrl *info = platform_get_drvdata(pdev);
 	struct rockchip_pin_bank *bank;
-	struct rockchip_pin_deferred *cfg;
+	struct rockchip_pin_output_deferred *cfg;
 	int i;
 
 	of_platform_depopulate(&pdev->dev);
@@ -3380,9 +3376,9 @@ static int rockchip_pinctrl_remove(struct platform_device *pdev)
 		bank = &info->ctrl->pin_banks[i];
 
 		mutex_lock(&bank->deferred_lock);
-		while (!list_empty(&bank->deferred_pins)) {
-			cfg = list_first_entry(&bank->deferred_pins,
-					       struct rockchip_pin_deferred, head);
+		while (!list_empty(&bank->deferred_output)) {
+			cfg = list_first_entry(&bank->deferred_output,
+					       struct rockchip_pin_output_deferred, head);
 			list_del(&cfg->head);
 			kfree(cfg);
 		}
diff --git a/drivers/pinctrl/pinctrl-rockchip.h b/drivers/pinctrl/pinctrl-rockchip.h
index 4759f336941e..3531633c0397 100644
--- a/drivers/pinctrl/pinctrl-rockchip.h
+++ b/drivers/pinctrl/pinctrl-rockchip.h
@@ -338,7 +338,7 @@ struct rockchip_pin_bank {
 	u32				toggle_edge_mode;
 	u32				recalced_mask;
 	u32				route_mask;
-	struct list_head		deferred_pins;
+	struct list_head		deferred_output;
 	struct mutex			deferred_lock;
 };
 
@@ -414,12 +414,9 @@ struct rockchip_pin_config {
 	unsigned int		nconfigs;
 };
 
-enum pin_config_param;
-
-struct rockchip_pin_deferred {
+struct rockchip_pin_output_deferred {
 	struct list_head head;
 	unsigned int pin;
-	enum pin_config_param param;
 	u32 arg;
 };
 
-- 
2.25.1

[PATCH 11/20] gpio/rockchip: add of_node for gpiochip

[Jianqun Xu]

The Rockchip GPIO driver will probe before pinctrl and has no parent dt
node, lack of the of_node will cause the driver probe failure.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index b294ef009daf..e36cdbd4bbef 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -588,6 +588,10 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
 	if (!gc->label)
 		return -ENOMEM;
 
+#ifdef CONFIG_OF_GPIO
+	gc->of_node = of_node_get(bank->dev->of_node);
+#endif
+
 	ret = gpiochip_add_data(gc, bank);
 	if (ret) {
 		dev_err(bank->dev, "failed to add gpiochip %s, %d\n",
-- 
2.25.1

Re: [PATCH 11/20] gpio/rockchip: add of_node for gpiochip

[Linus Walleij]

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:

> The Rockchip GPIO driver will probe before pinctrl and has no parent dt
> node, lack of the of_node will cause the driver probe failure.
>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>

> +#ifdef CONFIG_OF_GPIO
> +       gc->of_node = of_node_get(bank->dev->of_node);
> +#endif

Any introduction of of_node_get() needs to be balanced with a
corresponding of_node_put().

Yours,
Linus Walleij

Re: [PATCH 11/20] gpio/rockchip: add of_node for gpiochip

[Andy Shevchenko]

On Tue, Oct 04, 2022 at 09:14:38AM +0200, Linus Walleij wrote:
> On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
> > The Rockchip GPIO driver will probe before pinctrl and has no parent dt
> > node, lack of the of_node will cause the driver probe failure.
> >
> > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> 
> > +#ifdef CONFIG_OF_GPIO
> > +       gc->of_node = of_node_get(bank->dev->of_node);
> > +#endif
> 
> Any introduction of of_node_get() needs to be balanced with a
> corresponding of_node_put().

No, this code should not have been existed in the first place. We don't allow
anymore any of of_node usage in the GPIO drivers. There is an fwnode and parent
and logic to retrieve fwnode from parent in the GPIO library for the most of
the cases.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 11/20] gpio/rockchip: add of_node for gpiochip

[Linus Walleij]

On Tue, Oct 4, 2022 at 10:30 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, Oct 04, 2022 at 09:14:38AM +0200, Linus Walleij wrote:
> > On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
> > > The Rockchip GPIO driver will probe before pinctrl and has no parent dt
> > > node, lack of the of_node will cause the driver probe failure.
> > >
> > > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> >
> > > +#ifdef CONFIG_OF_GPIO
> > > +       gc->of_node = of_node_get(bank->dev->of_node);
> > > +#endif
> >
> > Any introduction of of_node_get() needs to be balanced with a
> > corresponding of_node_put().
>
> No, this code should not have been existed in the first place. We don't allow
> anymore any of of_node usage in the GPIO drivers. There is an fwnode and parent
> and logic to retrieve fwnode from parent in the GPIO library for the most of
> the cases.

Hm yeah given that the series want to introduce ACPI as well it makes
a lot of sense.

Yours,
Linus Walleij

Re: Re: [PATCH 11/20] gpio/rockchip: add of_node for gpiochip

[Andy Shevchenko]

On Sat, Oct 08, 2022 at 02:19:28PM +0800, jay.xu@rock-chips.com wrote:

...

> the gc->of_node is a fix for my patch serial, without this the gpiochip will fail to register.

Do not use of_node. We have parent and fwnode members, use them according to your needs.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 12/20] gpio/rockchip: add return check for clock rate set

[Jianqun Xu]

Check if the clock rate set for the apb clock is successful or not.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index e36cdbd4bbef..511e93a6a429 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -199,6 +199,9 @@ static int rockchip_gpio_set_debounce(struct gpio_chip *gc,
 	if (bank->gpio_type == GPIO_TYPE_V2 && !IS_ERR(bank->db_clk)) {
 		div_debounce_support = true;
 		freq = clk_get_rate(bank->db_clk);
+		if (!freq)
+			return -EINVAL;
+
 		max_debounce = (GENMASK(23, 0) + 1) * 2 * 1000000 / freq;
 		if (debounce > max_debounce)
 			return -EINVAL;
-- 
2.25.1

Re: [PATCH 12/20] gpio/rockchip: add return check for clock rate set

[Bartosz Golaszewski]

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
>
> Check if the clock rate set for the apb clock is successful or not.
>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/gpio/gpio-rockchip.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index e36cdbd4bbef..511e93a6a429 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -199,6 +199,9 @@ static int rockchip_gpio_set_debounce(struct gpio_chip *gc,
>         if (bank->gpio_type == GPIO_TYPE_V2 && !IS_ERR(bank->db_clk)) {
>                 div_debounce_support = true;
>                 freq = clk_get_rate(bank->db_clk);
> +               if (!freq)
> +                       return -EINVAL;
> +
>                 max_debounce = (GENMASK(23, 0) + 1) * 2 * 1000000 / freq;
>                 if (debounce > max_debounce)
>                         return -EINVAL;
> --
> 2.25.1
>

This cannot happen, clk_get_rate() can only return 0 for clk == NULL.
We're not using an optional clock.

Bart

Re: [PATCH 12/20] gpio/rockchip: add return check for clock rate set

[Bartosz Golaszewski]

On Fri, Sep 30, 2022 at 11:17 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
> >
> > Check if the clock rate set for the apb clock is successful or not.
> >
> > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> > ---
> >  drivers/gpio/gpio-rockchip.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> > index e36cdbd4bbef..511e93a6a429 100644
> > --- a/drivers/gpio/gpio-rockchip.c
> > +++ b/drivers/gpio/gpio-rockchip.c
> > @@ -199,6 +199,9 @@ static int rockchip_gpio_set_debounce(struct gpio_chip *gc,
> >         if (bank->gpio_type == GPIO_TYPE_V2 && !IS_ERR(bank->db_clk)) {
> >                 div_debounce_support = true;
> >                 freq = clk_get_rate(bank->db_clk);
> > +               if (!freq)
> > +                       return -EINVAL;
> > +
> >                 max_debounce = (GENMASK(23, 0) + 1) * 2 * 1000000 / freq;
> >                 if (debounce > max_debounce)
> >                         return -EINVAL;
> > --
> > 2.25.1
> >
>
> This cannot happen, clk_get_rate() can only return 0 for clk == NULL.
> We're not using an optional clock.
>

Ah, sorry actually the db_clk can be NULL for certain variants. But in
that case shouldn't we just silently ignore it and return 0? Or return
-ENOTSUPP at the very least?

Bart

[PATCH 13/20] gpio/rockchip: disable and put clocks when remove

[Jianqun Xu]

Match to the probe, do disable and put the clocks when module to remove.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 511e93a6a429..1a800f972594 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -757,7 +757,10 @@ static int rockchip_gpio_remove(struct platform_device *pdev)
 {
 	struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
 
+	clk_put(bank->clk);
+	clk_put(bank->db_clk);
 	clk_disable_unprepare(bank->clk);
+	clk_disable_unprepare(bank->db_clk);
 	gpiochip_remove(&bank->gpio_chip);
 
 	return 0;
-- 
2.25.1

Re: [PATCH 13/20] gpio/rockchip: disable and put clocks when remove

[Bartosz Golaszewski]

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
>
> Match to the probe, do disable and put the clocks when module to remove.
>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/gpio/gpio-rockchip.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index 511e93a6a429..1a800f972594 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -757,7 +757,10 @@ static int rockchip_gpio_remove(struct platform_device *pdev)
>  {
>         struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
>
> +       clk_put(bank->clk);
> +       clk_put(bank->db_clk);
>         clk_disable_unprepare(bank->clk);
> +       clk_disable_unprepare(bank->db_clk);
>         gpiochip_remove(&bank->gpio_chip);
>
>         return 0;
> --
> 2.25.1
>

You're putting the clock before disabling it? That doesn't look right.

Bart

[PATCH 14/20] gpio/rockchip: switch to use irq_domain_create_linear

[Jianqun Xu]

Switch to use irq_domain_create_linear to create the irq domain for the
Rockchip GPIO bank, whose argument is fwnode istead of of_node.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 1a800f972594..4fcd75d710c1 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -28,6 +28,8 @@
 #define GPIO_TYPE_V2		(0x01000C2B)  /* GPIO Version ID 0x01000C2B */
 #define GPIO_TYPE_V2_1		(0x0101157C)  /* GPIO Version ID 0x0101157C */
 
+#define GPIO_MAX_PINS	(32)
+
 static const struct rockchip_gpio_regs gpio_regs_v1 = {
 	.port_dr = 0x00,
 	.port_ddr = 0x04,
@@ -520,14 +522,15 @@ static int rockchip_interrupts_register(struct rockchip_pin_bank *bank)
 	struct irq_chip_generic *gc;
 	int ret;
 
-	bank->domain = irq_domain_add_linear(bank->of_node, 32,
-					&irq_generic_chip_ops, NULL);
+	bank->domain = irq_domain_create_linear(dev_fwnode(bank->dev),
+						GPIO_MAX_PINS,
+						&irq_generic_chip_ops, NULL);
 	if (!bank->domain) {
 		dev_warn(bank->dev, "could not init irq domain\n");
 		return -EINVAL;
 	}
 
-	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
+	ret = irq_alloc_domain_generic_chips(bank->domain, GPIO_MAX_PINS, 1,
 					     "rockchip_gpio_irq",
 					     handle_level_irq,
 					     clr, 0, 0);
-- 
2.25.1

[PATCH 15/20] gpio/rockchip: disable and put clocks when gpiolib register failed

[Jianqun Xu]

When gpiolib register failed, the clocks should be disabled and put.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 4fcd75d710c1..09ed5c880dde 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -746,14 +746,21 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 
 	ret = rockchip_gpiolib_register(bank);
 	if (ret) {
-		clk_disable_unprepare(bank->clk);
-		return ret;
+		dev_err(bank->dev, "Failed to register gpio %d\n", ret);
+		goto err_put_clk;
 	}
 
 	platform_set_drvdata(pdev, bank);
 	dev_info(dev, "probed %pOF\n", np);
 
 	return 0;
+err_put_clk:
+	clk_put(bank->clk);
+	clk_put(bank->db_clk);
+	clk_disable_unprepare(bank->clk);
+	clk_disable_unprepare(bank->db_clk);
+
+	return ret;
 }
 
 static int rockchip_gpio_remove(struct platform_device *pdev)
-- 
2.25.1

Re: [PATCH 15/20] gpio/rockchip: disable and put clocks when gpiolib register failed

[Linus Walleij]

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:

> When gpiolib register failed, the clocks should be disabled and put.
>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
(...)
> +err_put_clk:
> +       clk_put(bank->clk);
> +       clk_put(bank->db_clk);
> +       clk_disable_unprepare(bank->clk);
> +       clk_disable_unprepare(bank->db_clk);

Always clk_disable_unprepare() before clk_put().

I think you can drop clk_put() if you switch to devres dev_clk_get()
instead.

Yours,
Linus Walleij

Re: [PATCH 15/20] gpio/rockchip: disable and put clocks when gpiolib register failed

[Linus Walleij]

On Tue, Oct 4, 2022 at 9:16 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
>
> > When gpiolib register failed, the clocks should be disabled and put.
> >
> > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> (...)
> > +err_put_clk:
> > +       clk_put(bank->clk);
> > +       clk_put(bank->db_clk);
> > +       clk_disable_unprepare(bank->clk);
> > +       clk_disable_unprepare(bank->db_clk);
>
> Always clk_disable_unprepare() before clk_put().
>
> I think you can drop clk_put() if you switch to devres dev_clk_get()
> instead.

devm_clk_get() I mean.

Yours,
Linus Walleij

[PATCH 16/20] gpio/rockchip: try to get gpio id from uid when ACPI enabled

[Jianqun Xu]

When the ACPI is enabled, the GPIO index should be get from the uid,
otherwise get it from the dt alias or a static count to the legency dt
files.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 09ed5c880dde..11586d93549b 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -6,6 +6,7 @@
  * Copyright (c) 2021 Rockchip Electronics Co. Ltd.
  */
 
+#include <linux/acpi.h>
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/device.h>
@@ -710,6 +711,29 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id)
 	return found ? bank : NULL;
 }
 
+static int rockchip_gpio_get_bank_id(struct device *dev)
+{
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	int bank_id = -EINVAL;
+	u64 uid;
+	int ret;
+	static int gpio;
+
+	if (is_acpi_node(fwnode)) {
+		ret = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
+		if (ret < 0)
+			return ret;
+
+		bank_id = uid;
+	} else {
+		bank_id = of_alias_get_id(to_of_node(fwnode), "gpio");
+		if (bank_id < 0)
+			bank_id = gpio++;
+	}
+
+	return bank_id;
+}
+
 static int rockchip_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -717,8 +741,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	struct device_node *pctlnp = of_get_parent(np);
 	struct pinctrl_dev *pctldev = NULL;
 	struct rockchip_pin_bank *bank = NULL;
-	static int gpio;
-	int id, ret;
+	int ret;
 
 	if (!np || !pctlnp)
 		return -ENODEV;
@@ -727,13 +750,11 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	if (!pctldev)
 		return -EPROBE_DEFER;
 
-	id = of_alias_get_id(np, "gpio");
-	if (id < 0)
-		id = gpio++;
-
-	bank = rockchip_gpio_find_bank(pctldev, id);
-	if (!bank)
-		return -EINVAL;
+	ret = rockchip_gpio_get_bank_id(dev);
+	if (ret >= 0)
+		bank->bank_num = ret;
+	else
+		goto err;
 
 	bank->dev = dev;
 	bank->of_node = np;
@@ -759,6 +780,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	clk_put(bank->db_clk);
 	clk_disable_unprepare(bank->clk);
 	clk_disable_unprepare(bank->db_clk);
+err:
 
 	return ret;
 }
-- 
2.25.1

[PATCH 17/20] gpio/rockchip: print device with fwnode name when probe successful

[Jianqun Xu]

Print the device probe message with the fwnode instead of of_node.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 11586d93549b..d08435619f12 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -772,7 +772,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, bank);
-	dev_info(dev, "probed %pOF\n", np);
+	dev_info(dev, "probed %pfw\n", dev_fwnode(dev));
 
 	return 0;
 err_put_clk:
-- 
2.25.1

Re: [PATCH 17/20] gpio/rockchip: print device with fwnode name when probe successful

[Bartosz Golaszewski]

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <jay.xu@rock-chips.com> wrote:
>
> Print the device probe message with the fwnode instead of of_node.
>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  drivers/gpio/gpio-rockchip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index 11586d93549b..d08435619f12 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -772,7 +772,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
>         }
>
>         platform_set_drvdata(pdev, bank);
> -       dev_info(dev, "probed %pOF\n", np);
> +       dev_info(dev, "probed %pfw\n", dev_fwnode(dev));
>
>         return 0;
>  err_put_clk:
> --
> 2.25.1
>

I would instead just drop this log message. If the user wants to, they
can obtain that information from the device core logs.

Bart

[PATCH 18/20] gpio/rockchip: make use of device property

[Jianqun Xu]

Get the iomem resource by devm_platform_ioremap_resource and get the irq
by platform_get_irq.

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index d08435619f12..a94ebd95d285 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -9,7 +9,6 @@
 #include <linux/acpi.h>
 #include <linux/bitops.h>
 #include <linux/clk.h>
-#include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio/driver.h>
 #include <linux/init.h>
@@ -17,6 +16,8 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
@@ -651,22 +652,8 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
 
 static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
 {
-	struct resource res;
 	int id = 0;
 
-	if (of_address_to_resource(bank->of_node, 0, &res)) {
-		dev_err(bank->dev, "cannot find IO resource for bank\n");
-		return -ENOENT;
-	}
-
-	bank->reg_base = devm_ioremap_resource(bank->dev, &res);
-	if (IS_ERR(bank->reg_base))
-		return PTR_ERR(bank->reg_base);
-
-	bank->irq = irq_of_parse_and_map(bank->of_node, 0);
-	if (!bank->irq)
-		return -EINVAL;
-
 	bank->clk = of_clk_get(bank->of_node, 0);
 	if (IS_ERR(bank->clk))
 		return PTR_ERR(bank->clk);
@@ -765,6 +752,14 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	bank->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(bank->reg_base))
+		return PTR_ERR(bank->reg_base);
+
+	bank->irq = platform_get_irq(pdev, 0);
+	if (bank->irq < 0)
+		return bank->irq;
+
 	ret = rockchip_gpiolib_register(bank);
 	if (ret) {
 		dev_err(bank->dev, "Failed to register gpio %d\n", ret);
-- 
2.25.1

[PATCH 19/20] gpio/rockchip: make GPIO module work well under ACPI enabled

[Jianqun Xu]

The GPIO driver for Rockchip GPIO controller is seperated from rockchip
pinctrl driver, at the first version, it keeps things original to make
the patch easy to be reviewed, such as the GPIO driver must work with
the pinctrl dt node to be its parent node.

This patch wants to fix the driver to support ACPI since GPIO controller
should work well during ACPI is enabled.

This patch do changes including:
- only get clocks when is_of_node since ACPI case not need.
- use fwnode instead of of_node
- use private device structure mostly copy from pinctrl
- drop pinctrl related codes, pinctrl self should to find the gpiochip

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

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index a94ebd95d285..01b214b84c71 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -18,20 +18,88 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
-#include <linux/of_address.h>
-#include <linux/of_device.h>
-#include <linux/of_irq.h>
 #include <linux/regmap.h>
 
-#include "../pinctrl/core.h"
-#include "../pinctrl/pinctrl-rockchip.h"
-
 #define GPIO_TYPE_V1		(0)           /* GPIO Version ID reserved */
 #define GPIO_TYPE_V2		(0x01000C2B)  /* GPIO Version ID 0x01000C2B */
 #define GPIO_TYPE_V2_1		(0x0101157C)  /* GPIO Version ID 0x0101157C */
 
 #define GPIO_MAX_PINS	(32)
 
+/**
+ * struct rockchip_gpio_regs
+ * @port_dr: data register
+ * @port_ddr: data direction register
+ * @int_en: interrupt enable
+ * @int_mask: interrupt mask
+ * @int_type: interrupt trigger type, such as high, low, edge trigger type.
+ * @int_polarity: interrupt polarity enable register
+ * @int_bothedge: interrupt bothedge enable register
+ * @int_status: interrupt status register
+ * @int_rawstatus: int_status = int_rawstatus & int_mask
+ * @debounce: enable debounce for interrupt signal
+ * @dbclk_div_en: enable divider for debounce clock
+ * @dbclk_div_con: setting for divider of debounce clock
+ * @port_eoi: end of interrupt of the port
+ * @ext_port: port data from external
+ * @version_id: controller version register
+ */
+struct rockchip_gpio_regs {
+	u32 port_dr;
+	u32 port_ddr;
+	u32 int_en;
+	u32 int_mask;
+	u32 int_type;
+	u32 int_polarity;
+	u32 int_bothedge;
+	u32 int_status;
+	u32 int_rawstatus;
+	u32 debounce;
+	u32 dbclk_div_en;
+	u32 dbclk_div_con;
+	u32 port_eoi;
+	u32 ext_port;
+	u32 version_id;
+};
+
+/**
+ * struct rockchip_pin_bank
+ * @dev: the pinctrl device bind to the bank
+ * @reg_base: register base of the gpio bank
+ * @clk: clock of the gpio bank
+ * @db_clk: clock of the gpio debounce
+ * @irq: interrupt of the gpio bank
+ * @saved_masks: Saved content of GPIO_INTEN at suspend time.
+ * @pin_base: first pin number
+ * @nr_pins: number of pins in this bank
+ * @name: name of the bank
+ * @bank_num: number of the bank, to account for holes
+ * @domain: irqdomain of the gpio bank
+ * @gpio_chip: gpiolib chip
+ * @grange: gpio range
+ * @slock: spinlock for the gpio bank
+ * @toggle_edge_mode: bit mask to toggle (falling/rising) edge mode
+ */
+struct rockchip_pin_bank {
+	struct gpio_chip		gpio_chip;
+	struct device			*dev;
+	void __iomem			*reg_base;
+	struct clk			*clk;
+	struct clk			*db_clk;
+	int				irq;
+	u32				saved_masks;
+	u32				pin_base;
+	u8				nr_pins;
+	char				*name;
+	u8				bank_num;
+	struct device_node		*of_node;
+	struct irq_domain		*domain;
+	raw_spinlock_t			slock;
+	const struct rockchip_gpio_regs	*gpio_regs;
+	u32				gpio_type;
+	u32				toggle_edge_mode;
+};
+
 static const struct rockchip_gpio_regs gpio_regs_v1 = {
 	.port_dr = 0x00,
 	.port_ddr = 0x04,
@@ -607,35 +675,6 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
 		return ret;
 	}
 
-	/*
-	 * For DeviceTree-supported systems, the gpio core checks the
-	 * pinctrl's device node for the "gpio-ranges" property.
-	 * If it is present, it takes care of adding the pin ranges
-	 * for the driver. In this case the driver can skip ahead.
-	 *
-	 * In order to remain compatible with older, existing DeviceTree
-	 * files which don't set the "gpio-ranges" property or systems that
-	 * utilize ACPI the driver has to call gpiochip_add_pin_range().
-	 */
-	if (!of_property_read_bool(bank->of_node, "gpio-ranges")) {
-		struct device_node *pctlnp = of_get_parent(bank->of_node);
-		struct pinctrl_dev *pctldev = NULL;
-
-		if (!pctlnp)
-			return -ENODATA;
-
-		pctldev = of_pinctrl_get(pctlnp);
-		if (!pctldev)
-			return -ENODEV;
-
-		ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0,
-					     gc->base, gc->ngpio);
-		if (ret) {
-			dev_err(bank->dev, "Failed to add pin range\n");
-			goto fail;
-		}
-	}
-
 	ret = rockchip_interrupts_register(bank);
 	if (ret) {
 		dev_err(bank->dev, "failed to register interrupt, %d\n", ret);
@@ -650,52 +689,53 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
 	return ret;
 }
 
-static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
+static int rockchip_gpio_set_regs(struct rockchip_pin_bank *bank)
 {
-	int id = 0;
+	struct device *dev = bank->dev;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	int version_id;
+	int ret;
 
-	bank->clk = of_clk_get(bank->of_node, 0);
-	if (IS_ERR(bank->clk))
-		return PTR_ERR(bank->clk);
+	if (is_of_node(fwnode)) {
+		bank->clk = of_clk_get(to_of_node(fwnode), 0);
+		if (IS_ERR(bank->clk))
+			return PTR_ERR(bank->clk);
 
-	clk_prepare_enable(bank->clk);
-	id = readl(bank->reg_base + gpio_regs_v2.version_id);
+		ret = clk_prepare_enable(bank->clk);
+		if (ret < 0)
+			goto put_clk;
+	}
 
-	/* If not gpio v2, that is default to v1. */
-	if (id == GPIO_TYPE_V2 || id == GPIO_TYPE_V2_1) {
+	version_id = readl(bank->reg_base + gpio_regs_v2.version_id);
+	if (version_id == GPIO_TYPE_V2 || version_id == GPIO_TYPE_V2_1) {
 		bank->gpio_regs = &gpio_regs_v2;
 		bank->gpio_type = GPIO_TYPE_V2;
-		bank->db_clk = of_clk_get(bank->of_node, 1);
-		if (IS_ERR(bank->db_clk)) {
-			dev_err(bank->dev, "cannot find debounce clk\n");
-			clk_disable_unprepare(bank->clk);
-			return -EINVAL;
-		}
 	} else {
 		bank->gpio_regs = &gpio_regs_v1;
 		bank->gpio_type = GPIO_TYPE_V1;
 	}
 
-	return 0;
-}
-
-static struct rockchip_pin_bank *
-rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id)
-{
-	struct rockchip_pinctrl *info;
-	struct rockchip_pin_bank *bank;
-	int i, found = 0;
+	if (bank->gpio_type == GPIO_TYPE_V2 && is_of_node(fwnode)) {
+		bank->db_clk = of_clk_get(to_of_node(fwnode), 1);
+		if (IS_ERR(bank->db_clk)) {
+			ret = PTR_ERR(bank->db_clk);
+			goto unprepare_clk;
+		}
 
-	info = pinctrl_dev_get_drvdata(pctldev);
-	bank = info->ctrl->pin_banks;
-	for (i = 0; i < info->ctrl->nr_banks; i++, bank++) {
-		if (bank->bank_num == id) {
-			found = 1;
-			break;
+		ret = clk_prepare_enable(bank->db_clk);
+		if (ret < 0) {
+			clk_put(bank->db_clk);
+			goto unprepare_clk;
 		}
 	}
 
-	return found ? bank : NULL;
+	return 0;
+unprepare_clk:
+	clk_disable_unprepare(bank->clk);
+put_clk:
+	clk_put(bank->clk);
+
+	return ret;
 }
 
 static int rockchip_gpio_get_bank_id(struct device *dev)
@@ -724,18 +764,20 @@ static int rockchip_gpio_get_bank_id(struct device *dev)
 static int rockchip_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
-	struct device_node *pctlnp = of_get_parent(np);
-	struct pinctrl_dev *pctldev = NULL;
-	struct rockchip_pin_bank *bank = NULL;
+	struct rockchip_pin_bank *bank;
 	int ret;
 
-	if (!np || !pctlnp)
-		return -ENODEV;
+	bank = devm_kzalloc(dev, sizeof(*bank), GFP_KERNEL);
+	if (!bank)
+		return -ENOMEM;
+
+	bank->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(bank->reg_base))
+		return PTR_ERR(bank->reg_base);
 
-	pctldev = of_pinctrl_get(pctlnp);
-	if (!pctldev)
-		return -EPROBE_DEFER;
+	bank->irq = platform_get_irq(pdev, 0);
+	if (bank->irq < 0)
+		return bank->irq;
 
 	ret = rockchip_gpio_get_bank_id(dev);
 	if (ret >= 0)
@@ -744,22 +786,15 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 		goto err;
 
 	bank->dev = dev;
-	bank->of_node = np;
+	bank->pin_base = GPIO_MAX_PINS * bank->bank_num;
+	bank->nr_pins = GPIO_MAX_PINS;
 
 	raw_spin_lock_init(&bank->slock);
 
-	ret = rockchip_get_bank_data(bank);
+	ret = rockchip_gpio_set_regs(bank);
 	if (ret)
 		return ret;
 
-	bank->reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(bank->reg_base))
-		return PTR_ERR(bank->reg_base);
-
-	bank->irq = platform_get_irq(pdev, 0);
-	if (bank->irq < 0)
-		return bank->irq;
-
 	ret = rockchip_gpiolib_register(bank);
 	if (ret) {
 		dev_err(bank->dev, "Failed to register gpio %d\n", ret);
-- 
2.25.1

[PATCH 20/20] pinctrl/rockchip: find existed gpiochip by gpio label

[Jianqun Xu]

Currently the pinctrl device acts as parent for the GPIO devices for
Rockchip platform, the pinctrl driver creates the gpiochips and pass to
the GPIO driver.

This patch makes the member of pinctrl structure from gpiochip to
*gpiochip, switch to find existed gpiochips instead of creating them,
that requires the GPIO driver to create them and earlier than the
pinctrl driver.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 24 +++++++++++++++++++++---
 drivers/pinctrl/pinctrl-rockchip.h |  2 +-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 97af52bb5481..05e688cd8536 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -2725,7 +2725,7 @@ static int rockchip_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 {
 	struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
 	struct rockchip_pin_bank *bank = pin_to_bank(info, pin);
-	struct gpio_chip *gpio = &bank->gpio_chip;
+	struct gpio_chip *gpio = bank->gpio_chip;
 	enum pin_config_param param;
 	u32 arg;
 	int i;
@@ -2828,7 +2828,7 @@ static int rockchip_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
 {
 	struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
 	struct rockchip_pin_bank *bank = pin_to_bank(info, pin);
-	struct gpio_chip *gpio = &bank->gpio_chip;
+	struct gpio_chip *gpio = bank->gpio_chip;
 	enum pin_config_param param = pinconf_to_config_param(*config);
 	u16 arg;
 	int rc;
@@ -3293,7 +3293,7 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 	struct rockchip_pin_ctrl *ctrl;
 	struct resource *res;
 	void __iomem *base;
-	int ret;
+	int ret, i;
 
 	if (is_of_node(fwnode)) {
 		ret = of_platform_populate(np, NULL, NULL, dev);
@@ -3357,6 +3357,24 @@ static int rockchip_pinctrl_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	for (i = 0; i < ctrl->nr_banks; i++) {
+		struct gpio_chip *gc;
+		struct rockchip_pin_bank *bank = &ctrl->pin_banks[i];
+
+		gc = gpiochip_find_by_name((void *)ctrl->pin_banks[i].name);
+		if (!gc) {
+			dev_err(dev, "fail to find gpiochip\n");
+			return ret;
+		}
+
+		ret = gpiochip_add_pin_range(gc, dev_name(dev), 0, gc->base, gc->ngpio);
+		if (ret) {
+			dev_err(dev, "fail to add pin range\n");
+			return ret;
+		}
+		bank->gpio_chip = gc;
+	}
+
 	platform_set_drvdata(pdev, info);
 	dev_info(dev, "probed %pfw\n", fwnode);
 
diff --git a/drivers/pinctrl/pinctrl-rockchip.h b/drivers/pinctrl/pinctrl-rockchip.h
index 3531633c0397..8b44e02bce97 100644
--- a/drivers/pinctrl/pinctrl-rockchip.h
+++ b/drivers/pinctrl/pinctrl-rockchip.h
@@ -330,7 +330,7 @@ struct rockchip_pin_bank {
 	struct device_node		*of_node;
 	struct rockchip_pinctrl		*drvdata;
 	struct irq_domain		*domain;
-	struct gpio_chip		gpio_chip;
+	struct gpio_chip		*gpio_chip;
 	struct pinctrl_gpio_range	grange;
 	raw_spinlock_t			slock;
 	const struct rockchip_gpio_regs	*gpio_regs;
-- 
2.25.1