* [PATCH v3 RESEND] gpio: rockchip: support acpi
@ 2022-09-07 9:27 ` Jianqun Xu
0 siblings, 0 replies; 7+ messages in thread
From: Jianqun Xu @ 2022-09-07 9:27 UTC (permalink / raw)
To: heiko, linus.walleij, brgl
Cc: andriy.shevchenko, linux-gpio, linux-rockchip, Jianqun Xu
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(-)
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;
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 RESEND] gpio: rockchip: support acpi
@ 2022-09-07 9:27 ` Jianqun Xu
0 siblings, 0 replies; 7+ messages in thread
From: Jianqun Xu @ 2022-09-07 9:27 UTC (permalink / raw)
To: heiko, linus.walleij, brgl
Cc: andriy.shevchenko, linux-gpio, linux-rockchip, Jianqun Xu
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(-)
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;
--
2.25.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 RESEND] gpio: rockchip: support acpi
2022-09-07 9:27 ` Jianqun Xu
@ 2022-09-07 14:39 ` Andy Shevchenko
-1 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2022-09-07 14:39 UTC (permalink / raw)
To: Jianqun Xu; +Cc: heiko, linus.walleij, brgl, linux-gpio, linux-rockchip
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;
> + }
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 RESEND] gpio: rockchip: support acpi
@ 2022-09-07 14:39 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2022-09-07 14:39 UTC (permalink / raw)
To: Jianqun Xu; +Cc: heiko, linus.walleij, brgl, linux-gpio, linux-rockchip
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;
> + }
> }
--
With Best Regards,
Andy Shevchenko
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 RESEND] gpio: rockchip: support acpi
2022-09-07 9:27 ` Jianqun Xu
@ 2022-09-07 23:52 ` kernel test robot
-1 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2022-09-07 23:52 UTC (permalink / raw)
To: Jianqun Xu, heiko, linus.walleij, brgl
Cc: kbuild-all, andriy.shevchenko, linux-gpio, linux-rockchip, Jianqun Xu
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);
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 RESEND] gpio: rockchip: support acpi
@ 2022-09-07 23:52 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2022-09-07 23:52 UTC (permalink / raw)
To: Jianqun Xu, heiko, linus.walleij, brgl
Cc: kbuild-all, andriy.shevchenko, linux-gpio, linux-rockchip, Jianqun Xu
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);
--
0-DAY CI Kernel Test Service
https://01.org/lkp
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Re: [PATCH v3 RESEND] gpio: rockchip: support acpi
2022-09-07 14:39 ` Andy Shevchenko
(?)
@ 2022-09-08 7:23 ` jay.xu
-1 siblings, 0 replies; 7+ messages in thread
From: jay.xu @ 2022-09-08 7:23 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Heiko Stübner, linus.walleij, Bartosz Golaszewski,
linux-gpio, open list:ARM/Rockchip SoC...
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
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-08 7:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07 9:27 [PATCH v3 RESEND] gpio: rockchip: support acpi Jianqun Xu
2022-09-07 9:27 ` Jianqun Xu
2022-09-07 14:39 ` Andy Shevchenko
2022-09-07 14:39 ` Andy Shevchenko
2022-09-08 7:23 ` jay.xu
2022-09-07 23:52 ` kernel test robot
2022-09-07 23:52 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.