diff mbox series

[v3,RESEND] gpio: rockchip: support acpi

Message ID 20220907092722.3333752-1-jay.xu@rock-chips.com (mailing list archive)
State New, archived
Headers show
Series [v3,RESEND] gpio: rockchip: support acpi | expand

Commit Message

Jianqun Xu Sept. 7, 2022, 9:27 a.m. UTC
This patch fix driver to support acpi by following changes:
 * support get gpio bank number from uid of acpi
 * try to get clocks for dt nodes but for acpi
 * try to get clocks by a char id first, if a dt patch applied

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
v3: suggested by Andy Shevchenko
 - fix irq size (32) to GPIO_MAX_PINS
 - fix to use is_of_node or is_acpi_node
 - use standard pattern for error handle
 - remove redundant assignment

v2:
 - fix rockchip_pin_output_deferred to rockchip_pin_deferred

 drivers/gpio/gpio-rockchip.c | 240 ++++++++++++++++++++++-------------
 1 file changed, 149 insertions(+), 91 deletions(-)

Comments

Andy Shevchenko Sept. 7, 2022, 2:39 p.m. UTC | #1
On Wed, Sep 07, 2022 at 05:27:22PM +0800, Jianqun Xu wrote:
> This patch fix driver to support acpi by following changes:
>  * support get gpio bank number from uid of acpi
>  * try to get clocks for dt nodes but for acpi
>  * try to get clocks by a char id first, if a dt patch applied

...

> -	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
> +	ret = irq_alloc_domain_generic_chips(bank->domain, GPIO_MAX_PINS , 1,

Extra space.

>  					     "rockchip_gpio_irq",
>  					     handle_level_irq,
>  					     clr, 0, 0);

...

> +	if (!gc->label) {
> +		gc->label = kasprintf(GFP_KERNEL, "gpio%d", bank->bank_num);

In the below code will be a memory leak. Also at ->remove().

> +		if (!gc->label)
> +			return -ENOMEM;
> +	}

...

> +static int rockchip_gpio_of_get_bank_id(struct device *dev)
> +{
> +	static int gpio;
> +	int bank_id = -1;

> +	if (is_of_node(dev->of_node)) {

	struct fwnode_handle *fwnode = dev_fwnode(...);

> +		bank_id = of_alias_get_id(dev->of_node, "gpio");

to_of_node(fwnode)

> +		if (bank_id < 0)
> +			bank_id = gpio++;
> +	}
> +
> +	return bank_id;
> +}

...

> +	bank_id = rockchip_gpio_acpi_get_bank_id(dev);
> +	if (bank_id < 0) {
> +		bank_id = rockchip_gpio_of_get_bank_id(dev);
> +		if (bank_id < 0)
> +			return bank_id;
> +	}

Easier to read:

	bank_id = rockchip_gpio_acpi_get_bank_id(dev);
	if (bank_id < 0)
		bank_id = rockchip_gpio_of_get_bank_id(dev);
	if (bank_id < 0)
		return bank_id;

...

> +	if (!is_acpi_node(dev_fwnode(dev)) {
> +		struct device_node *pctlnp = of_get_parent(dev->of_node);
>  
> +		pctldev = of_pinctrl_get(pctlnp);
> +		if (!pctldev)
> +			return -EPROBE_DEFER;
>  
> +		bank = rockchip_gpio_find_bank(pctldev, bank_id);
> +		if (!bank)
> +			return -ENODEV;

> +	}
> +	if (!bank) {

Simply '} else {' ?

> +		bank = devm_kzalloc(dev, sizeof(*bank), GFP_KERNEL);
> +		if (!bank)
> +			return -ENOMEM;
> +	}

...

> +	if (is_of_node(dev_fwnode(dev)) {
> +		bank->clk = devm_clk_get(dev, "bus");
> +		if (IS_ERR(bank->clk)) {
> +			bank->clk = of_clk_get(dev->of_node, 0);
> +			if (IS_ERR(bank->clk)) {
> +				dev_err(dev, "fail to get apb clock\n");
> +				return PTR_ERR(bank->clk);
> +			}
> +		}
> +
> +		bank->db_clk = devm_clk_get(dev, "db");
> +		if (IS_ERR(bank->db_clk)) {
> +			bank->db_clk = of_clk_get(dev->of_node, 1);
> +			if (IS_ERR(bank->db_clk))
> +				bank->db_clk = NULL;
> +		}
> +	}

This can be split to a separate

	int rockchip_gpio_get_clocks(...)

...

> +	clk_prepare_enable(bank->clk);
> +	clk_prepare_enable(bank->db_clk);

Either of them may fail.

...

> +	if (!device_property_read_bool(bank->dev, "gpio-ranges") && pctldev) {

Same question as before, why the GPIO library code is not okay for this?

> +		struct gpio_chip *gc = &bank->gpio_chip;
> +
> +		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 err_unlock;
> +		}
>  	}
kernel test robot Sept. 7, 2022, 11:52 p.m. UTC | #2
Hi Jianqun,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rockchip/for-next]
[also build test ERROR on linus/master v6.0-rc4 next-20220907]
[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/20220907-172920
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: loongarch-buildonly-randconfig-r002-20220907 (https://download.01.org/0day-ci/archive/20220908/202209080711.fRu3ImWL-lkp@intel.com/config)
compiler: loongarch64-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/6b62c564dce1d94e9256d98661e5a7ae396627fb
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jianqun-Xu/gpio-rockchip-support-acpi/20220907-172920
        git checkout 6b62c564dce1d94e9256d98661e5a7ae396627fb
        # 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=loongarch SHELL=/bin/bash drivers/gpio/

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

   In file included from include/linux/build_bug.h:5,
                    from include/linux/bits.h:22,
                    from include/linux/ioport.h:13,
                    from include/linux/acpi.h:12,
                    from drivers/gpio/gpio-rockchip.c:9:
   drivers/gpio/gpio-rockchip.c: In function 'rockchip_gpio_of_get_bank_id':
   drivers/gpio/gpio-rockchip.c:666:27: error: passing argument 1 of 'is_of_node' from incompatible pointer type [-Werror=incompatible-pointer-types]
     666 |         if (is_of_node(dev->of_node)) {
         |                        ~~~^~~~~~~~~
         |                           |
         |                           struct device_node *
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
      58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
         |                                                    ^~~~
   drivers/gpio/gpio-rockchip.c:666:9: note: in expansion of macro 'if'
     666 |         if (is_of_node(dev->of_node)) {
         |         ^~
   In file included from include/linux/irqdomain.h:35,
                    from include/linux/acpi.h:13:
   include/linux/of.h:155:59: note: expected 'const struct fwnode_handle *' but argument is of type 'struct device_node *'
     155 | static inline bool is_of_node(const struct fwnode_handle *fwnode)
         |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
   drivers/gpio/gpio-rockchip.c:666:27: error: passing argument 1 of 'is_of_node' from incompatible pointer type [-Werror=incompatible-pointer-types]
     666 |         if (is_of_node(dev->of_node)) {
         |                        ~~~^~~~~~~~~
         |                           |
         |                           struct device_node *
   include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
      58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
         |                                                             ^~~~
   drivers/gpio/gpio-rockchip.c:666:9: note: in expansion of macro 'if'
     666 |         if (is_of_node(dev->of_node)) {
         |         ^~
   include/linux/of.h:155:59: note: expected 'const struct fwnode_handle *' but argument is of type 'struct device_node *'
     155 | static inline bool is_of_node(const struct fwnode_handle *fwnode)
         |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
   drivers/gpio/gpio-rockchip.c:666:27: error: passing argument 1 of 'is_of_node' from incompatible pointer type [-Werror=incompatible-pointer-types]
     666 |         if (is_of_node(dev->of_node)) {
         |                        ~~~^~~~~~~~~
         |                           |
         |                           struct device_node *
   include/linux/compiler.h:69:10: note: in definition of macro '__trace_if_value'
      69 |         (cond) ?                                        \
         |          ^~~~
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
      56 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                            ^~~~~~~~~~~~~~
   drivers/gpio/gpio-rockchip.c:666:9: note: in expansion of macro 'if'
     666 |         if (is_of_node(dev->of_node)) {
         |         ^~
   include/linux/of.h:155:59: note: expected 'const struct fwnode_handle *' but argument is of type 'struct device_node *'
     155 | static inline bool is_of_node(const struct fwnode_handle *fwnode)
         |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
   drivers/gpio/gpio-rockchip.c: In function 'rockchip_gpio_probe':
>> drivers/gpio/gpio-rockchip.c:882:46: error: unterminated argument list invoking macro "if"
     882 | MODULE_DEVICE_TABLE(of, rockchip_gpio_match);
         |                                              ^
>> drivers/gpio/gpio-rockchip.c:883: error: expected '(' at end of input
   drivers/gpio/gpio-rockchip.c:721:9: note: '-Wmisleading-indentation' is disabled from this point onwards, since column-tracking was disabled due to the size of the code/headers
     721 |         if (!is_acpi_node(dev_fwnode(dev)) {
         |         ^~
   drivers/gpio/gpio-rockchip.c:721:9: note: adding '-flarge-source-files' will allow for more column-tracking support, at the expense of compilation time and memory
   drivers/gpio/gpio-rockchip.c:721:9: error: expected declaration or statement at end of input
   drivers/gpio/gpio-rockchip.c:712:13: warning: unused variable 'ret' [-Wunused-variable]
     712 |         int ret;
         |             ^~~
   drivers/gpio/gpio-rockchip.c:710:35: warning: unused variable 'bank' [-Wunused-variable]
     710 |         struct rockchip_pin_bank *bank = NULL;
         |                                   ^~~~
   drivers/gpio/gpio-rockchip.c:709:29: warning: unused variable 'pctldev' [-Wunused-variable]
     709 |         struct pinctrl_dev *pctldev = NULL;
         |                             ^~~~~~~
   drivers/gpio/gpio-rockchip.c: At top level:
   drivers/gpio/gpio-rockchip.c:706:12: warning: 'rockchip_gpio_probe' defined but not used [-Wunused-function]
     706 | static int rockchip_gpio_probe(struct platform_device *pdev)
         |            ^~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-rockchip.c:643:1: warning: 'rockchip_gpio_find_bank' defined but not used [-Wunused-function]
     643 | rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id)
         | ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-rockchip.c:628:13: warning: 'rockchip_gpio_get_ver' defined but not used [-Wunused-function]
     628 | static void rockchip_gpio_get_ver(struct rockchip_pin_bank *bank)
         |             ^~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-rockchip.c:584:12: warning: 'rockchip_gpiolib_register' defined but not used [-Wunused-function]
     584 | static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/if +882 drivers/gpio/gpio-rockchip.c

936ee2675eee1f Jianqun Xu 2021-08-16  878  
936ee2675eee1f Jianqun Xu 2021-08-16  879  MODULE_DESCRIPTION("Rockchip gpio driver");
936ee2675eee1f Jianqun Xu 2021-08-16  880  MODULE_ALIAS("platform:rockchip-gpio");
936ee2675eee1f Jianqun Xu 2021-08-16  881  MODULE_LICENSE("GPL v2");
936ee2675eee1f Jianqun Xu 2021-08-16 @882  MODULE_DEVICE_TABLE(of, rockchip_gpio_match);
Jianqun Xu Sept. 8, 2022, 7:23 a.m. UTC | #3
Hi Andy

--------------
jay.xu@rock-chips.com
>On Wed, Sep 07, 2022 at 05:27:22PM +0800, Jianqun Xu wrote:
>> This patch fix driver to support acpi by following changes:
>>  * support get gpio bank number from uid of acpi
>>  * try to get clocks for dt nodes but for acpi
>>  * try to get clocks by a char id first, if a dt patch applied
>
>...
>
>> -	ret = irq_alloc_domain_generic_chips(bank->domain, 32, 1,
>> +	ret = irq_alloc_domain_generic_chips(bank->domain, GPIO_MAX_PINS , 1,
>
>Extra space. 
Got it.

>
>>       "rockchip_gpio_irq",
>>       handle_level_irq,
>>       clr, 0, 0);
>
>...
>
>> +	if (!gc->label) {
>> +	gc->label = kasprintf(GFP_KERNEL, "gpio%d", bank->bank_num);
>
>In the below code will be a memory leak. Also at ->remove().
>
>> +	if (!gc->label)
>> +	return -ENOMEM;
>> +	}
>
>...
>
>> +static int rockchip_gpio_of_get_bank_id(struct device *dev)
>> +{
>> +	static int gpio;
>> +	int bank_id = -1;
>
>> +	if (is_of_node(dev->of_node)) {
>
>	struct fwnode_handle *fwnode = dev_fwnode(...);
>
>> +	bank_id = of_alias_get_id(dev->of_node, "gpio");
>
>to_of_node(fwnode) 
Got it.

>
>> +	if (bank_id < 0)
>> +	bank_id = gpio++;
>> +	}
>> +
>> +	return bank_id;
>> +}
>
>...
>
>> +	bank_id = rockchip_gpio_acpi_get_bank_id(dev);
>> +	if (bank_id < 0) {
>> +	bank_id = rockchip_gpio_of_get_bank_id(dev);
>> +	if (bank_id < 0)
>> +	return bank_id;
>> +	}
>
>Easier to read:
>
>	bank_id = rockchip_gpio_acpi_get_bank_id(dev);
>	if (bank_id < 0)
>	bank_id = rockchip_gpio_of_get_bank_id(dev);
>	if (bank_id < 0)
>	return bank_id;
> 
Got it.

>...
>
>> +	if (!is_acpi_node(dev_fwnode(dev)) {
>> +	struct device_node *pctlnp = of_get_parent(dev->of_node);
>> 
>> +	pctldev = of_pinctrl_get(pctlnp);
>> +	if (!pctldev)
>> +	return -EPROBE_DEFER;
>> 
>> +	bank = rockchip_gpio_find_bank(pctldev, bank_id);
>> +	if (!bank)
>> +	return -ENODEV;
>
>> +	}
>> +	if (!bank) {
>
>Simply '} else {' ? 
Got it.

>
>> +	bank = devm_kzalloc(dev, sizeof(*bank), GFP_KERNEL);
>> +	if (!bank)
>> +	return -ENOMEM;
>> +	}
>
>...
>
>> +	if (is_of_node(dev_fwnode(dev)) {
>> +	bank->clk = devm_clk_get(dev, "bus");
>> +	if (IS_ERR(bank->clk)) {
>> +	bank->clk = of_clk_get(dev->of_node, 0);
>> +	if (IS_ERR(bank->clk)) {
>> +	dev_err(dev, "fail to get apb clock\n");
>> +	return PTR_ERR(bank->clk);
>> +	}
>> +	}
>> +
>> +	bank->db_clk = devm_clk_get(dev, "db");
>> +	if (IS_ERR(bank->db_clk)) {
>> +	bank->db_clk = of_clk_get(dev->of_node, 1);
>> +	if (IS_ERR(bank->db_clk))
>> +	bank->db_clk = NULL;
>> +	}
>> +	}
>
>This can be split to a separate
>
>	int rockchip_gpio_get_clocks(...)
> 
Got it.

>...
>
>> +	clk_prepare_enable(bank->clk);
>> +	clk_prepare_enable(bank->db_clk);
>
>Either of them may fail.
> 
Got it.

>...
>
>> +	if (!device_property_read_bool(bank->dev, "gpio-ranges") && pctldev) {
>
>Same question as before, why the GPIO library code is not okay for this?
> 
The dev_name(pctldev->dev) is null since there is no pinctrl device when acpi enabled.
Currently we may set the pinctrl mux and config under uefi, and no pinctrl driver in linux.

>> +	struct gpio_chip *gc = &bank->gpio_chip;
>> +
>> +	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 err_unlock;
>> +	}
>>  }
>
>
>--
>With Best Regards,
>Andy Shevchenko
>
>
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index ebb50c25a461..6bc76b23cf80 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);
@@ -578,6 +583,16 @@  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 = kasprintf(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",
@@ -585,35 +600,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);
@@ -628,47 +614,18 @@  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_get_ver(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;
 	}
-
-	return 0;
 }
 
 static struct rockchip_pin_bank *
@@ -690,40 +647,119 @@  rockchip_gpio_find_bank(struct pinctrl_dev *pctldev, int id)
 	return found ? bank : NULL;
 }
 
+static int rockchip_gpio_of_get_bank_id(struct device *dev)
+{
+	static int gpio;
+	int bank_id = -1;
+
+	if (is_of_node(dev->of_node)) {
+		bank_id = of_alias_get_id(dev->of_node, "gpio");
+		if (bank_id < 0)
+			bank_id = gpio++;
+	}
+
+	return bank_id;
+}
+
+#ifdef CONFIG_ACPI
+static int rockchip_gpio_acpi_get_bank_id(struct device *dev)
+{
+	struct acpi_device *adev;
+	unsigned long bank_id = -1;
+	const char *uid;
+	int ret;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return -ENXIO;
+
+	uid = acpi_device_uid(adev);
+	if (!uid || !(*uid)) {
+		dev_err(dev, "Cannot retrieve UID\n");
+		return -ENODEV;
+	}
+
+	ret = kstrtoul(uid, 0, &bank_id);
+	if (ret)
+		return ret;
+
+	return bank_id;
+}
+#else
+static int rockchip_gpio_acpi_get_bank_id(struct device *dev)
+{
+	return -ENOENT;
+}
+#endif /* CONFIG_ACPI */
+
 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_deferred *cfg;
-	static int gpio;
-	int id, ret;
+	int bank_id;
+	int ret;
 
-	if (!np || !pctlnp)
-		return -ENODEV;
+	bank_id = rockchip_gpio_acpi_get_bank_id(dev);
+	if (bank_id < 0) {
+		bank_id = rockchip_gpio_of_get_bank_id(dev);
+		if (bank_id < 0)
+			return bank_id;
+	}
 
-	pctldev = of_pinctrl_get(pctlnp);
-	if (!pctldev)
-		return -EPROBE_DEFER;
+	if (!is_acpi_node(dev_fwnode(dev)) {
+		struct device_node *pctlnp = of_get_parent(dev->of_node);
 
-	id = of_alias_get_id(np, "gpio");
-	if (id < 0)
-		id = gpio++;
+		pctldev = of_pinctrl_get(pctlnp);
+		if (!pctldev)
+			return -EPROBE_DEFER;
 
-	bank = rockchip_gpio_find_bank(pctldev, id);
-	if (!bank)
-		return -EINVAL;
+		bank = rockchip_gpio_find_bank(pctldev, bank_id);
+		if (!bank)
+			return -ENODEV;
+	}
+
+	if (!bank) {
+		bank = devm_kzalloc(dev, sizeof(*bank), GFP_KERNEL);
+		if (!bank)
+			return -ENOMEM;
+	}
 
+	bank->bank_num = bank_id;
 	bank->dev = dev;
-	bank->of_node = np;
+
+	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;
 
 	raw_spin_lock_init(&bank->slock);
 
-	ret = rockchip_get_bank_data(bank);
-	if (ret)
-		return ret;
+	if (is_of_node(dev_fwnode(dev)) {
+		bank->clk = devm_clk_get(dev, "bus");
+		if (IS_ERR(bank->clk)) {
+			bank->clk = of_clk_get(dev->of_node, 0);
+			if (IS_ERR(bank->clk)) {
+				dev_err(dev, "fail to get apb clock\n");
+				return PTR_ERR(bank->clk);
+			}
+		}
+
+		bank->db_clk = devm_clk_get(dev, "db");
+		if (IS_ERR(bank->db_clk)) {
+			bank->db_clk = of_clk_get(dev->of_node, 1);
+			if (IS_ERR(bank->db_clk))
+				bank->db_clk = NULL;
+		}
+	}
+
+	clk_prepare_enable(bank->clk);
+	clk_prepare_enable(bank->db_clk);
+
+	rockchip_gpio_get_ver(bank);
 
 	/*
 	 * Prevent clashes with a deferred output setting
@@ -733,14 +769,29 @@  static int rockchip_gpio_probe(struct platform_device *pdev)
 
 	ret = rockchip_gpiolib_register(bank);
 	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;
+	}
+
+	if (!device_property_read_bool(bank->dev, "gpio-ranges") && pctldev) {
+		struct gpio_chip *gc = &bank->gpio_chip;
+
+		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 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 +816,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 +832,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;