[6/6] gpio: rockchip: support acpi

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 driver to support acpi since gpio controller
should work well during acpi is enabled. But during upstream, driver is
better to fix other thing together includes:
 - add 'clock-names' to allow driver to get clocks by devm_clk_get().
 - get io resource and irq by platform common apis.
 - use fwnode instead of of_node from device structure.

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

Hi Jianqun,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rockchip/for-next]
[also build test ERROR on rafael-pm/linux-next brgl/gpio/for-next linus/master v6.0-rc4 next-20220909]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jianqun-Xu/gpio-rockchip-support-acpi/20220909-170917
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: openrisc-randconfig-r015-20220907
compiler: or1k-linux-gcc (GCC) 12.1.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
        # https://github.com/intel-lab-lkp/linux/commit/49cfab100d90f21d176a0433211e5b5b996afa17
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jianqun-Xu/gpio-rockchip-support-acpi/20220909-170917
        git checkout 49cfab100d90f21d176a0433211e5b5b996afa17
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

   or1k-linux-ld: drivers/gpio/gpio-rockchip.o: in function `rockchip_gpio_probe':
>> drivers/gpio/gpio-rockchip.c:746: undefined reference to `get_pinctrl_dev_from_devname'
   drivers/gpio/gpio-rockchip.c:746:(.text+0xb88): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `get_pinctrl_dev_from_devname'
   or1k-linux-ld: drivers/gpio/gpio-rockchip.o: in function `rockchip_gpio_find_bank':
>> drivers/gpio/gpio-rockchip.c:703: undefined reference to `pinctrl_dev_get_drvdata'
   drivers/gpio/gpio-rockchip.c:703:(.text+0xb9c): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `pinctrl_dev_get_drvdata'
   pahole: .tmp_vmlinux.btf: No such file or directory
   .btf.vmlinux.bin.o: file not recognized: file format not recognized


vim +746 drivers/gpio/gpio-rockchip.c

   695	
   696	static struct rockchip_pin_bank *
   697	rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id)
   698	{
   699		struct rockchip_pinctrl *info;
   700		struct rockchip_pin_bank *bank;
   701		int i, found = 0;
   702	
 > 703		info = pinctrl_dev_get_drvdata(pctldev);
   704		bank = info->ctrl->pin_banks;
   705		for (i = 0; i < info->ctrl->nr_banks; i++, bank++) {
   706			if (bank->bank_num == id) {
   707				found = 1;
   708				break;
   709			}
   710		}
   711	
   712		return found ? bank : NULL;
   713	}
   714	
   715	static int rockchip_gpio_get_bank_id(struct device *dev)
   716	{
   717		struct fwnode_handle *fwnode = dev_fwnode(dev);
   718		int bank_id = -EINVAL;
   719		u64 uid;
   720		static int gpio;
   721	
   722		if (is_acpi_node(fwnode)) {
   723			if (!acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid))
   724				bank_id = (int)uid;
   725		} else {
   726			bank_id = of_alias_get_id(to_of_node(fwnode), "gpio");
   727			if (bank_id < 0)
   728				bank_id = gpio++;
   729		}
   730	
   731		return bank_id;
   732	}
   733	
   734	static int rockchip_gpio_probe(struct platform_device *pdev)
   735	{
   736		struct device *dev = &pdev->dev;
   737		struct rockchip_pin_bank *bank;
   738		struct pinctrl_dev *pctldev;
   739		int bank_id;
   740		int ret;
   741	
   742		bank_id = rockchip_gpio_get_bank_id(dev);
   743		if (bank_id < 0)
   744			return bank_id;
   745	
 > 746		pctldev = get_pinctrl_dev_from_devname("pinctrl");
   747		if (pctldev) {
   748			bank = rockchip_gpio_find_bank(pctldev, bank_id);
   749			if (!bank)
   750				return -ENODEV;
   751		} else {
   752			bank = devm_kzalloc(dev, sizeof(*bank), GFP_KERNEL);
   753			if (!bank)
   754				return -ENOMEM;
   755		}
   756	
   757		bank->bank_num = bank_id;
   758		bank->dev = dev;
   759	
   760		bank->reg_base = devm_platform_ioremap_resource(pdev, 0);
   761		if (IS_ERR(bank->reg_base))
   762			return PTR_ERR(bank->reg_base);
   763	
   764		bank->irq = platform_get_irq(pdev, 0);
   765		if (bank->irq < 0)
   766			return bank->irq;
   767	
   768		ret = rockchip_gpio_get_clocks(bank);
   769		if (ret)
   770			return ret;
   771	
   772		raw_spin_lock_init(&bank->slock);
   773		rockchip_gpio_set_regs(bank);
   774	
   775		/*
   776		 * Prevent clashes with a deferred output setting
   777		 * being added right at this moment.
   778		 */
   779		mutex_lock(&bank->deferred_lock);
   780	
   781		ret = rockchip_gpiolib_register(bank, pctldev);
   782		if (ret) {
   783			dev_err(bank->dev, "Failed to register gpio %d\n", ret);
   784			goto err_unlock;
   785		}
   786	
   787		while (!list_empty(&bank->deferred_pins)) {
   788			struct rockchip_pin_deferred *cfg;
   789	
   790			cfg = list_first_entry(&bank->deferred_pins,
   791					       struct rockchip_pin_deferred, head);
   792			if (!cfg)
   793				break;
   794	
   795			list_del(&cfg->head);
   796	
   797			switch (cfg->param) {
   798			case PIN_CONFIG_OUTPUT:
   799				ret = rockchip_gpio_direction_output(&bank->gpio_chip, cfg->pin, cfg->arg);
   800				if (ret)
   801					dev_warn(dev, "setting output pin %u to %u failed\n", cfg->pin,
   802						 cfg->arg);
   803				break;
   804			case PIN_CONFIG_INPUT_ENABLE:
   805				ret = rockchip_gpio_direction_input(&bank->gpio_chip, cfg->pin);
   806				if (ret)
   807					dev_warn(dev, "setting input pin %u failed\n", cfg->pin);
   808				break;
   809			default:
   810				dev_warn(dev, "unknown deferred config param %d\n", cfg->param);
   811				break;
   812			}
   813			kfree(cfg);
   814		}
   815	
   816		mutex_unlock(&bank->deferred_lock);
   817	
   818		platform_set_drvdata(pdev, bank);
   819		dev_info(dev, "probed %pfw\n", dev_fwnode(dev));
   820	
   821		return 0;
   822	err_unlock:
   823		mutex_unlock(&bank->deferred_lock);
   824		clk_disable_unprepare(bank->clk);
   825		clk_disable_unprepare(bank->db_clk);
   826	
   827		return ret;
   828	}
   829

On 9/13/22 03:12, jay.xu@rock-chips.com wrote:
> Hi Johan
> 
> --------------
> jay.xu@rock-chips.com
>> Hi Jianqun,
>>
>> Some comments about clocks. Have a look if it's usefull.
>>
>> On 9/9/22 11:05, Jianqun Xu wrote:
>>> 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 driver to support acpi since gpio controller
>>> should work well during acpi is enabled. But during upstream, driver is
>>> better to fix other thing together includes:
>>>   - add 'clock-names' to allow driver to get clocks by devm_clk_get().
>>>   - get io resource and irq by platform common apis.
>>>   - use fwnode instead of of_node from device structure.
>>>
>>> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
>>> ---
>>>   drivers/gpio/gpio-rockchip.c | 197 ++++++++++++++++++++++-------------
>>>   1 file changed, 122 insertions(+), 75 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
>>> index ebb50c25a461..ca012cf199a6 100644
>>> --- a/drivers/gpio/gpio-rockchip.c
>>> +++ b/drivers/gpio/gpio-rockchip.c
>>> @@ -6,9 +6,9 @@
>>>    * Copyright (c) 2021 Rockchip Electronics Co. Ltd.
>>>    */
>>>  
>>> +#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>
>>> @@ -16,10 +16,9 @@
>>>   #include <linux/io.h>
>>>   #include <linux/module.h>
>>>   #include <linux/of.h>
>>> -#include <linux/of_address.h>
>>> -#include <linux/of_device.h>
>>> -#include <linux/of_irq.h>
>>> +#include <linux/platform_device.h>
>>>   #include <linux/pinctrl/pinconf-generic.h>
>>> +#include <linux/property.h>
>>>   #include <linux/regmap.h>
>>>  
>>>   #include "../pinctrl/core.h"
>>> @@ -29,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,
>>> @@ -200,6 +201,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;
>>> @@ -507,15 +511,16 @@ 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 for bank %s\n",
>>>   bank->name);
>>>   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);
>>> @@ -565,7 +570,8 @@ static int rockchip_interrupts_register(struct rockchip_pin_bank *bank)
>>>   return 0;
>>>   }
>>>  
>>> -static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
>>> +static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank,
>>> +	     struct pinctrl_dev *pctldev)
>>>   {
>>>   struct gpio_chip *gc;
>>>   int ret;
>>> @@ -578,6 +584,17 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
>>>   gc->label = bank->name;
>>>   gc->parent = bank->dev;
>>>  
>>> +	if (!gc->base)
>>> +	gc->base = GPIO_MAX_PINS * bank->bank_num;
>>> +	if (!gc->ngpio)
>>> +	gc->ngpio = GPIO_MAX_PINS;
>>> +	if (!gc->label) {
>>> +	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) {
>>>   dev_err(bank->dev, "failed to add gpiochip %s, %d\n",
>>> @@ -595,17 +612,7 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
>>>   * 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;
>>> -
>>> +	if (!device_property_read_bool(bank->dev, "gpio-ranges") && pctldev) {
>>>   ret = gpiochip_add_pin_range(gc, dev_name(pctldev->dev), 0,
>>>        gc->base, gc->ngpio);
>>>   if (ret) {
>>> @@ -628,45 +635,49 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
>>>   return ret;
>>>   }
>>>  
>>> -static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
>>> +static void rockchip_gpio_set_regs(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);
>>> -
>>> -	clk_prepare_enable(bank->clk);
>>> -	id = readl(bank->reg_base + gpio_regs_v2.version_id);
>>> +	int id = readl(bank->reg_base + gpio_regs_v2.version_id);
>>>  
>>>   /* If not gpio v2, that is default to v1. */
>>>   if (id == GPIO_TYPE_V2 || 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;
>>>   }
>>> +}
>>> +
>>> +static int rockchip_gpio_get_clocks(struct rockchip_pin_bank *bank)
>>> +{
>>> +	struct device *dev = bank->dev;
>>> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
>>> +	int ret;
>>> +
>>> +	if (!is_of_node(fwnode))
>>> +	return 0;
>>> +
>>
>>> +	bank->clk = devm_clk_get(dev, "bus");
>>
>> .. but existing DTs also come without clock-name and must still work.
>>

> Could some one can share me that does the driver should work well with the old dt files?
> I think that if some one update the kernel, they should update the driver and the dt files ?
> the new driver with a new core dt files, now if i update the dt files and driver once time, should
> the driver still need to work with old dt files ?

There's one or more comments in the past by Heiko which I just can't find in the "lore archive" about that right now.

Because it's a too abrupt change. Kernel DT changes/bindings have a more long term commitment in it.
We shouldn't be in a situation that someones kernel doesn't work any more when he/she upgrades and that just happened after your patch was applied.
The use of clocks without clock-names was a valid case in the past. When adding new futures that same functionality should continue to be available as expected. 

> 
>> Check clock-names first else fallback to clock index or keep using of_clk_get().
>> Up to the GPIO maintainer of course.
>> Not sure if a clk index function for devm exist, else maybe use: ???
>>
>> 	bank->clk = clk_get(dev, "bus");
>> vs.
>> 	bank->clk = of_clk_get_by_name(np, "bus");
>>
>>
>> 	if (IS_ERR(bank->clk)) {
>> 	bank->clk = of_clk_get(bank->of_node, 0);
>> 	if (IS_ERR(bank->clk))
>> 	return PTR_ERR(bank->clk);
>>
>> Could you check if this still works?
>>
>> ====
>>
>> Previous comments:
>>
>>>> You can't mix devm_ with non-devm_ calls.
>>>>
>>> Okay, I add 'clock-names' property for all existed related dt files and fix driver to only use
>>> devm_clk_get(), I push a version please help to review
>>
>> ====
>>
>>> +	if (IS_ERR(bank->clk)) {
>>> +	dev_err(dev, "fail to get apb clock\n");
>>> +	return PTR_ERR(bank->clk);
>>> +	}
>>> +
>>
>>> +	ret = clk_prepare_enable(bank->clk);
>>> +	if (ret < 0)
>>> +	return ret;
>>> +
>>
>> Block order: first find all your clocks then enable.
>> Get your resources first.
>>
>>> +	bank->db_clk = devm_clk_get(dev, "db");
>>> +	if (IS_ERR(bank->db_clk)) {
>>> +	bank->db_clk = NULL;
>>> +	}
>>
>> The clock-name "db" is only available in rk356x.dtsi
>>
>>> +
>>> +	ret = clk_prepare_enable(bank->db_clk);
>>> +	if (ret < 0) {
>>
>> It is not an error if it doesn't exists.
>> One might use the function devm_clk_get_optional().
>>
>>> +	clk_disable_unprepare(bank->clk);
>>> +	return ret;
>>> +	}
>>>  
>>>   return 0;
>>>   }
>>> @@ -690,57 +701,86 @@ rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id)
>>>   return found ? bank : NULL;
>>>   }
>>>  
>>> -static int rockchip_gpio_probe(struct platform_device *pdev)
>>> +static int rockchip_gpio_get_bank_id(struct device *dev)
>>>   {
>>> -	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_deferred *cfg;
>>> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
>>> +	int bank_id = -EINVAL;
>>> +	u64 uid;
>>>   static int gpio;
>>> -	int id, ret;
>>>  
>>> -	if (!np || !pctlnp)
>>> -	return -ENODEV;
>>> +	if (is_acpi_node(fwnode)) {
>>> +	if (!acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid))
>>> +	bank_id = (int)uid;
>>> +	} else {
>>> +	bank_id = of_alias_get_id(to_of_node(fwnode), "gpio");
>>> +	if (bank_id < 0)
>>> +	bank_id = gpio++;
>>> +	}
>>>  
>>> -	pctldev = of_pinctrl_get(pctlnp);
>>> -	if (!pctldev)
>>> -	return -EPROBE_DEFER;
>>> +	return bank_id;
>>> +}
>>>  
>>> -	id = of_alias_get_id(np, "gpio");
>>> -	if (id < 0)
>>> -	id = gpio++;
>>> +static int rockchip_gpio_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct rockchip_pin_bank *bank;
>>> +	struct pinctrl_dev *pctldev;
>>> +	int bank_id;
>>> +	int ret;
>>>  
>>> -	bank = rockchip_gpio_find_bank(pctldev, id);
>>> -	if (!bank)
>>> -	return -EINVAL;
>>> +	bank_id = rockchip_gpio_get_bank_id(dev);
>>> +	if (bank_id < 0)
>>> +	return bank_id;
>>>  
>>> +	pctldev = get_pinctrl_dev_from_devname("pinctrl");
>>> +	if (pctldev) {
>>> +	bank = rockchip_gpio_find_bank(pctldev, bank_id);
>>> +	if (!bank)
>>> +	return -ENODEV;
>>> +	} else {
>>> +	bank = devm_kzalloc(dev, sizeof(*bank), GFP_KERNEL);
>>> +	if (!bank)
>>> +	return -ENOMEM;
>>> +	}
>>> +
>>> +	bank->bank_num = bank_id;
>>>   bank->dev = dev;
>>> -	bank->of_node = np;
>>>  
>>> -	raw_spin_lock_init(&bank->slock);
>>> +	bank->reg_base = devm_platform_ioremap_resource(pdev, 0);
>>> +	if (IS_ERR(bank->reg_base))
>>> +	return PTR_ERR(bank->reg_base);
>>>  
>>> -	ret = rockchip_get_bank_data(bank);
>>> +	bank->irq = platform_get_irq(pdev, 0);
>>> +	if (bank->irq < 0)
>>> +	return bank->irq;
>>> +
>>> +	ret = rockchip_gpio_get_clocks(bank);
>>>   if (ret)
>>>   return ret;
>>>  
>>> +	raw_spin_lock_init(&bank->slock);
>>> +	rockchip_gpio_set_regs(bank);
>>> +
>>>   /*
>>>   * Prevent clashes with a deferred output setting
>>>   * being added right at this moment.
>>>   */
>>>   mutex_lock(&bank->deferred_lock);
>>>  
>>> -	ret = rockchip_gpiolib_register(bank);
>>> +	ret = rockchip_gpiolib_register(bank, pctldev);
>>>   if (ret) {
>>> -	clk_disable_unprepare(bank->clk);
>>> -	mutex_unlock(&bank->deferred_lock);
>>> -	return ret;
>>> +	dev_err(bank->dev, "Failed to register gpio %d\n", ret);
>>> +	goto err_unlock;
>>>   }
>>>  
>>>   while (!list_empty(&bank->deferred_pins)) {
>>> +	struct rockchip_pin_deferred *cfg;
>>> +
>>>   cfg = list_first_entry(&bank->deferred_pins,
>>>          struct rockchip_pin_deferred, head);
>>> +	if (!cfg)
>>> +	break;
>>> +
>>>   list_del(&cfg->head);
>>>  
>>>   switch (cfg->param) {
>>> @@ -765,9 +805,15 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
>>>   mutex_unlock(&bank->deferred_lock);
>>>  
>>>   platform_set_drvdata(pdev, bank);
>>> -	dev_info(dev, "probed %pOF\n", np);
>>> +	dev_info(dev, "probed %pfw\n", dev_fwnode(dev));
>>>  
>>>   return 0;
>>> +err_unlock:
>>> +	mutex_unlock(&bank->deferred_lock);
>>> +	clk_disable_unprepare(bank->clk);
>>> +	clk_disable_unprepare(bank->db_clk);
>>> +
>>> +	return ret;
>>>   }
>>>  
>>>   static int rockchip_gpio_remove(struct platform_device *pdev)
>>> @@ -775,6 +821,7 @@ static int rockchip_gpio_remove(struct platform_device *pdev)
>>>   struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
>>>  
>>>   clk_disable_unprepare(bank->clk);
>>> +	clk_disable_unprepare(bank->db_clk);
>>>   gpiochip_remove(&bank->gpio_chip);
>>>  
>>>   return 0;