2022-09-20 10:57:01

by Jianqun Xu

[permalink] [raw]
Subject: [PATCH 00/20] Rockchip pinctrl/GPIO support ACPI

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


2022-09-20 10:57:59

by Jianqun Xu

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

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 <[email protected]>
---
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

2022-09-20 10:59:04

by Jianqun Xu

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

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 <[email protected]>
---
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

2022-09-20 11:00:07

by Jianqun Xu

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

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

Signed-off-by: Jianqun Xu <[email protected]>
---
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

2022-10-04 07:22:43

by Linus Walleij

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

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <[email protected]> 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 <[email protected]>

> +#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

2022-10-04 07:22:48

by Linus Walleij

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

On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <[email protected]> wrote:

> When gpiolib register failed, the clocks should be disabled and put.
>
> Signed-off-by: Jianqun Xu <[email protected]>
(...)
> +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

2022-10-04 07:23:12

by Linus Walleij

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

On Tue, Oct 4, 2022 at 9:16 AM Linus Walleij <[email protected]> wrote:

> On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <[email protected]> wrote:
>
> > When gpiolib register failed, the clocks should be disabled and put.
> >
> > Signed-off-by: Jianqun Xu <[email protected]>
> (...)
> > +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

2022-10-04 09:17:01

by Andy Shevchenko

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

On Tue, Oct 04, 2022 at 09:14:38AM +0200, Linus Walleij wrote:
> On Tue, Sep 20, 2022 at 12:31 PM Jianqun Xu <[email protected]> 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 <[email protected]>
>
> > +#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


2022-10-04 09:23:41

by Linus Walleij

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

On Tue, Oct 4, 2022 at 10:30 AM Andy Shevchenko
<[email protected]> 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 <[email protected]> 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 <[email protected]>
> >
> > > +#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

2022-10-08 18:53:56

by Andy Shevchenko

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

On Sat, Oct 08, 2022 at 02:19:28PM +0800, [email protected] 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