Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On Thu, Mar 18, 2021 at 9:00 AM Enrico Weigelt, metux IT consult wrote: > Is there some compact notation for swnode's that's as small and simple > as some piece of DTS ? Yes it's really neat. It's all in and examples in e.g. the testsuite: drivers/base/test/property-entry-test.c You can just grep for PROPERTY_ENTRY and you find some examples of how we use it. Yours, Linus Walleij
Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On 11.03.21 11:42, Andy Shevchenko wrote: Hi, You are a bit late. We have built-in device properties (and corresponding API, which recently becomes swnode) which aims exactly this. Is there some compact notation for swnode's that's as small and simple as some piece of DTS ? My reasons for choosing built-in dtb have been: * it's a very small and compact notation for describing devices * no more open-coded registrations, etc * no more need for board drivers (except for the little piece of DT) --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On Thu, Mar 11, 2021 at 12:20 PM Enrico Weigelt, metux IT consult wrote: > On 01.03.21 15:51, Linus Walleij wrote: > > I don't know what the idea is with this but register are not normally > > defined > > in the DTS files. The registers are determined from the compatible value. > > The idea is basically replacing the pdata struct by oftree node. > (subsequent patches in this queue use this by doing the board setup via > compiled-in dtb, instead of the currently hardcoded tables). You are a bit late. We have built-in device properties (and corresponding API, which recently becomes swnode) which aims exactly this. -- With Best Regards, Andy Shevchenko
Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On 01.03.21 15:51, Linus Walleij wrote: Hi, I don't know what the idea is with this but register are not normally defined in the DTS files. The registers are determined from the compatible value. The idea is basically replacing the pdata struct by oftree node. (subsequent patches in this queue use this by doing the board setup via compiled-in dtb, instead of the currently hardcoded tables). On these SoCs, the gpio setup is a little bit more complex than just having a fixed range of registers (one per pin): the actual meaning depends und Soc model and board type - some regs aren't even gpios. (I'm still in progress of RE'ing the bios blob, to find out more, eg. pinmux setups, etc). Writing to the wrong regs can cause weird effects (actually not even sure whether it could lead to damage) In essence: only a specific subset of the register range can be used for GPIOs - the others shouldn't ever be touched. And this specific subset is soc/board specific. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287
Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On Mon, Feb 8, 2021 at 11:24 PM Enrico Weigelt, metux IT consult wrote: > Add support for probing via device tree. (...) > + pdata->gpio_num = of_property_count_elems_of_size(dev->of_node, > + "gpio-regs", > + sizeof(u32)); > + pdata->gpio_reg = devm_kzalloc(dev, sizeof(int)*pdata->gpio_num, > + GFP_KERNEL); > + if (!pdata->gpio_reg) > + goto nomem; I don't know what the idea is with this but register are not normally defined in the DTS files. The registers are determined from the compatible value. > + pdata->gpio_names = devm_kzalloc(dev, sizeof(char*)*pdata->gpio_num, > +GFP_KERNEL); > + if (!pdata->gpio_names) > + goto nomem; (...) > + ret = of_property_read_string_array(dev->of_node, "gpio-line-names", > + pdata->gpio_names, > pdata->gpio_num); And this is already handled by the core. Yours, Linus Walleij
Re: [RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
On Mon, Feb 8, 2021 at 11:22 PM Enrico Weigelt, metux IT consult
wrote:
>
> Add support for probing via device tree.
> ---
> drivers/gpio/gpio-amd-fch.c | 58
> +
> include/dt-bindings/gpio/amd-fch-gpio.h | 36 +++
> include/linux/platform_data/gpio/gpio-amd-fch.h | 24 ++
> 3 files changed, 98 insertions(+), 20 deletions(-)
> create mode 100644 include/dt-bindings/gpio/amd-fch-gpio.h
>
> diff --git a/drivers/gpio/gpio-amd-fch.c b/drivers/gpio/gpio-amd-fch.c
> index 2a21354ed6a0..32024f99dae5 100644
> --- a/drivers/gpio/gpio-amd-fch.c
> +++ b/drivers/gpio/gpio-amd-fch.c
> @@ -136,12 +136,61 @@ static int amd_fch_gpio_request(struct gpio_chip *chip,
> return 0;
> }
>
> +static struct amd_fch_gpio_pdata *load_pdata(struct device *dev)
Please stick to the amd_fch_ prefix to all symbols for consistency.
> +{
> + struct amd_fch_gpio_pdata *pdata;
> + int ret;
> +
> + pdata = devm_kzalloc(dev, sizeof(struct amd_fch_gpio_pdata),
> +GFP_KERNEL);
> + if (!pdata)
> + goto nomem;
> +
> + pdata->gpio_num = of_property_count_elems_of_size(dev->of_node,
> + "gpio-regs",
> + sizeof(u32));
> + pdata->gpio_reg = devm_kzalloc(dev, sizeof(int)*pdata->gpio_num,
> + GFP_KERNEL);
> + if (!pdata->gpio_reg)
> + goto nomem;
> +
> + pdata->gpio_names = devm_kzalloc(dev, sizeof(char*)*pdata->gpio_num,
> +GFP_KERNEL);
> + if (!pdata->gpio_names)
> + goto nomem;
> +
> + ret = of_property_read_variable_u32_array(dev->of_node, "gpio-regs",
> + pdata->gpio_reg,
> + pdata->gpio_num,
> + pdata->gpio_num);
> + if (ret != pdata->gpio_num) {
> + dev_err(dev, "failed reading gpio-regs from DT: %d\n", ret);
> + return NULL;
> + }
> +
> + ret = of_property_read_string_array(dev->of_node, "gpio-line-names",
> + pdata->gpio_names,
> pdata->gpio_num);
> + if (ret != pdata->gpio_num) {
> + dev_err(dev, "failed reading gpio-names from DT: %d\n", ret);
> + return NULL;
> + }
Since you're not iterating over DT nodes nor use any interfaces
specific to OF - I suggest you use the generic device properties to
load the fill the platform data.
> +
> + return pdata;
> +
> +nomem:
> + dev_err(dev, "load_pdata: failed allocating memory\n");
> + return NULL;
> +}
> +
> static int amd_fch_gpio_probe(struct platform_device *pdev)
> {
> struct amd_fch_gpio_priv *priv;
> struct amd_fch_gpio_pdata *pdata;
>
> pdata = dev_get_platdata(>dev);
> + if (!pdata)
> + pdata = load_pdata(>dev);
> +
> if (!pdata) {
> dev_err(>dev, "no platform_data\n");
> return -ENOENT;
> @@ -165,6 +214,9 @@ static int amd_fch_gpio_probe(struct platform_device
> *pdev)
> priv->gc.get_direction = amd_fch_gpio_get_direction;
> priv->gc.get= amd_fch_gpio_get;
> priv->gc.set= amd_fch_gpio_set;
> +#ifdef CONFIG_OF_GPIO
> + priv->gc.of_node= pdev->dev.of_node;
> +#endif
>
> spin_lock_init(>lock);
>
> @@ -177,9 +229,15 @@ static int amd_fch_gpio_probe(struct platform_device
> *pdev)
> return devm_gpiochip_add_data(>dev, >gc, priv);
> }
>
> +static const struct of_device_id amd_fch_gpio_of_match[] = {
> + { .compatible = "amd,fch-gpio" },
> + {}
> +};
Don't you need the module table?
> +
> static struct platform_driver amd_fch_gpio_driver = {
> .driver = {
> .name = AMD_FCH_GPIO_DRIVER_NAME,
> + .of_match_table = amd_fch_gpio_of_match,
> },
> .probe = amd_fch_gpio_probe,
> };
> diff --git a/include/dt-bindings/gpio/amd-fch-gpio.h
> b/include/dt-bindings/gpio/amd-fch-gpio.h
> new file mode 100644
> index ..7a47e6debcdb
> --- /dev/null
> +++ b/include/dt-bindings/gpio/amd-fch-gpio.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +/*
> + * AMD FCH gpio platform data definitions
> + *
> + * Copyright (C) 2020 metux IT consult
> + * Author: Enrico Weigelt
> + *
> + */
> +
> +#ifndef __DT_BINDINGS_GPIO_AMD_FCH_REGS_H
> +#define __DT_BINDINGS_GPIO_AMD_FCH_REGS_H
> +
> +/*
> + * gpio registers addresses
> + *
> + * these regs need to be assigned by board setup, since they're wired
> + * in very board specifici was, rarely documented, this should not be
> + * available to users.
> + */
> +#define AMD_FCH_GPIO_REG_GPIO49
[RFC PATCH 07/12] gpio: amd-fch: add oftree probing support
Add support for probing via device tree.
---
drivers/gpio/gpio-amd-fch.c | 58 +
include/dt-bindings/gpio/amd-fch-gpio.h | 36 +++
include/linux/platform_data/gpio/gpio-amd-fch.h | 24 ++
3 files changed, 98 insertions(+), 20 deletions(-)
create mode 100644 include/dt-bindings/gpio/amd-fch-gpio.h
diff --git a/drivers/gpio/gpio-amd-fch.c b/drivers/gpio/gpio-amd-fch.c
index 2a21354ed6a0..32024f99dae5 100644
--- a/drivers/gpio/gpio-amd-fch.c
+++ b/drivers/gpio/gpio-amd-fch.c
@@ -136,12 +136,61 @@ static int amd_fch_gpio_request(struct gpio_chip *chip,
return 0;
}
+static struct amd_fch_gpio_pdata *load_pdata(struct device *dev)
+{
+ struct amd_fch_gpio_pdata *pdata;
+ int ret;
+
+ pdata = devm_kzalloc(dev, sizeof(struct amd_fch_gpio_pdata),
+GFP_KERNEL);
+ if (!pdata)
+ goto nomem;
+
+ pdata->gpio_num = of_property_count_elems_of_size(dev->of_node,
+ "gpio-regs",
+ sizeof(u32));
+ pdata->gpio_reg = devm_kzalloc(dev, sizeof(int)*pdata->gpio_num,
+ GFP_KERNEL);
+ if (!pdata->gpio_reg)
+ goto nomem;
+
+ pdata->gpio_names = devm_kzalloc(dev, sizeof(char*)*pdata->gpio_num,
+GFP_KERNEL);
+ if (!pdata->gpio_names)
+ goto nomem;
+
+ ret = of_property_read_variable_u32_array(dev->of_node, "gpio-regs",
+ pdata->gpio_reg,
+ pdata->gpio_num,
+ pdata->gpio_num);
+ if (ret != pdata->gpio_num) {
+ dev_err(dev, "failed reading gpio-regs from DT: %d\n", ret);
+ return NULL;
+ }
+
+ ret = of_property_read_string_array(dev->of_node, "gpio-line-names",
+ pdata->gpio_names, pdata->gpio_num);
+ if (ret != pdata->gpio_num) {
+ dev_err(dev, "failed reading gpio-names from DT: %d\n", ret);
+ return NULL;
+ }
+
+ return pdata;
+
+nomem:
+ dev_err(dev, "load_pdata: failed allocating memory\n");
+ return NULL;
+}
+
static int amd_fch_gpio_probe(struct platform_device *pdev)
{
struct amd_fch_gpio_priv *priv;
struct amd_fch_gpio_pdata *pdata;
pdata = dev_get_platdata(>dev);
+ if (!pdata)
+ pdata = load_pdata(>dev);
+
if (!pdata) {
dev_err(>dev, "no platform_data\n");
return -ENOENT;
@@ -165,6 +214,9 @@ static int amd_fch_gpio_probe(struct platform_device *pdev)
priv->gc.get_direction = amd_fch_gpio_get_direction;
priv->gc.get= amd_fch_gpio_get;
priv->gc.set= amd_fch_gpio_set;
+#ifdef CONFIG_OF_GPIO
+ priv->gc.of_node= pdev->dev.of_node;
+#endif
spin_lock_init(>lock);
@@ -177,9 +229,15 @@ static int amd_fch_gpio_probe(struct platform_device *pdev)
return devm_gpiochip_add_data(>dev, >gc, priv);
}
+static const struct of_device_id amd_fch_gpio_of_match[] = {
+ { .compatible = "amd,fch-gpio" },
+ {}
+};
+
static struct platform_driver amd_fch_gpio_driver = {
.driver = {
.name = AMD_FCH_GPIO_DRIVER_NAME,
+ .of_match_table = amd_fch_gpio_of_match,
},
.probe = amd_fch_gpio_probe,
};
diff --git a/include/dt-bindings/gpio/amd-fch-gpio.h
b/include/dt-bindings/gpio/amd-fch-gpio.h
new file mode 100644
index ..7a47e6debcdb
--- /dev/null
+++ b/include/dt-bindings/gpio/amd-fch-gpio.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+/*
+ * AMD FCH gpio platform data definitions
+ *
+ * Copyright (C) 2020 metux IT consult
+ * Author: Enrico Weigelt
+ *
+ */
+
+#ifndef __DT_BINDINGS_GPIO_AMD_FCH_REGS_H
+#define __DT_BINDINGS_GPIO_AMD_FCH_REGS_H
+
+/*
+ * gpio registers addresses
+ *
+ * these regs need to be assigned by board setup, since they're wired
+ * in very board specifici was, rarely documented, this should not be
+ * available to users.
+ */
+#define AMD_FCH_GPIO_REG_GPIO490x40
+#define AMD_FCH_GPIO_REG_GPIO500x41
+#define AMD_FCH_GPIO_REG_GPIO510x42
+#define AMD_FCH_GPIO_REG_GPIO55_DEVSLP00x43
+#define AMD_FCH_GPIO_REG_GPIO570x44
+#define AMD_FCH_GPIO_REG_GPIO580x45
+#define AMD_FCH_GPIO_REG_GPIO59_DEVSLP10x46
+#define AMD_FCH_GPIO_REG_GPIO640x47
+#define AMD_FCH_GPIO_REG_GPIO680x48
+#define AMD_FCH_GPIO_REG_GPIO66_SPKR 0x5B
+#define AMD_FCH_GPIO_REG_GPIO710x4D
+#define AMD_FCH_GPIO_REG_GPIO32_GE10x59
+#define
