* [PATCH 0/3] Pinctrl driver for Amlogic Meson SoCs
@ 2014-10-07 21:32 Beniamino Galvani
2014-10-07 21:32 ` [PATCH 1/3] pinctrl: add " Beniamino Galvani
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Beniamino Galvani @ 2014-10-07 21:32 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Russell King,
Carlo Caione, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao,
Victor Wan, Beniamino Galvani
Hi,
this series introduces a driver for Amlogic Meson pinctrl and GPIOs,
adding the basic infrastructure for all the SoCs of the Meson family
and configuration data specific for the Meson8.
The last patch depends on a previous series that adds meson8 device
tree files and can be found here:
http://lwn.net/Articles/615033/
I tested the pinmux and GPIO functionalities on a Tronsmart Vega S89e
TV box and everything seems to work, however I'm not so familiar with
the pinctrl subsystem and thus any feedback is very welcome.
Beniamino Galvani (3):
pinctrl: add driver for Amlogic Meson SoCs
pinctrl: meson: add device tree bindings documentation
ARM: dts: meson8: add pinctrl and gpio nodes
.../devicetree/bindings/pinctrl/meson,pinctrl.txt | 80 ++
arch/arm/boot/dts/meson8-vega-s89e.dts | 16 +-
arch/arm/boot/dts/meson8.dtsi | 35 +
arch/arm/mach-meson/Kconfig | 3 +
drivers/pinctrl/Kconfig | 8 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/meson/Makefile | 2 +
drivers/pinctrl/meson/pinctrl-meson.c | 823 +++++++++++++++++++++
drivers/pinctrl/meson/pinctrl-meson.h | 217 ++++++
drivers/pinctrl/meson/pinctrl-meson8.c | 374 ++++++++++
include/dt-bindings/gpio/meson8-gpio.h | 155 ++++
11 files changed, 1713 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
create mode 100644 drivers/pinctrl/meson/Makefile
create mode 100644 drivers/pinctrl/meson/pinctrl-meson.c
create mode 100644 drivers/pinctrl/meson/pinctrl-meson.h
create mode 100644 drivers/pinctrl/meson/pinctrl-meson8.c
create mode 100644 include/dt-bindings/gpio/meson8-gpio.h
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] pinctrl: add driver for Amlogic Meson SoCs 2014-10-07 21:32 [PATCH 0/3] Pinctrl driver for Amlogic Meson SoCs Beniamino Galvani @ 2014-10-07 21:32 ` Beniamino Galvani [not found] ` <1412717576-19737-2-git-send-email-b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2014-10-07 21:32 ` [PATCH 2/3] pinctrl: meson: add device tree bindings documentation Beniamino Galvani 2014-10-07 21:32 ` [PATCH 3/3] ARM: dts: meson8: add pinctrl and gpio nodes Beniamino Galvani 2 siblings, 1 reply; 10+ messages in thread From: Beniamino Galvani @ 2014-10-07 21:32 UTC (permalink / raw) To: Linus Walleij Cc: linux-arm-kernel, linux-kernel, Arnd Bergmann, Russell King, Carlo Caione, devicetree, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan, Beniamino Galvani This is a driver for the pinmux and GPIO controller available in Amlogic Meson SoCs. At the moment it only supports Meson8 devices, however other SoC families like Meson6 and Meson8b (the Cortex-A5 variant) appears to be similar, with just different sets of banks and registers. GPIO interrupts are not supported at the moment due to lack of documentation. Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> --- arch/arm/mach-meson/Kconfig | 3 + drivers/pinctrl/Kconfig | 8 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/meson/Makefile | 2 + drivers/pinctrl/meson/pinctrl-meson.c | 823 +++++++++++++++++++++++++++++++++ drivers/pinctrl/meson/pinctrl-meson.h | 217 +++++++++ drivers/pinctrl/meson/pinctrl-meson8.c | 374 +++++++++++++++ include/dt-bindings/gpio/meson8-gpio.h | 155 +++++++ 8 files changed, 1583 insertions(+) create mode 100644 drivers/pinctrl/meson/Makefile create mode 100644 drivers/pinctrl/meson/pinctrl-meson.c create mode 100644 drivers/pinctrl/meson/pinctrl-meson.h create mode 100644 drivers/pinctrl/meson/pinctrl-meson8.c create mode 100644 include/dt-bindings/gpio/meson8-gpio.h diff --git a/arch/arm/mach-meson/Kconfig b/arch/arm/mach-meson/Kconfig index 18301dc..a09d3f6 100644 --- a/arch/arm/mach-meson/Kconfig +++ b/arch/arm/mach-meson/Kconfig @@ -3,6 +3,9 @@ menuconfig ARCH_MESON select GENERIC_IRQ_CHIP select ARM_GIC select CACHE_L2X0 + select PINCTRL + select PINCTRL_MESON + select ARCH_REQUIRE_GPIOLIB if ARCH_MESON diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index c6a66de..22b05e1 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -108,6 +108,14 @@ config PINCTRL_FALCON depends on SOC_FALCON depends on PINCTRL_LANTIQ +config PINCTRL_MESON + bool "Amlogic Meson pin controller driver" + depends on ARCH_MESON + select PINMUX + select PINCONF + select GENERIC_PINCONF + select OF_GPIO + config PINCTRL_ROCKCHIP bool select PINMUX diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 51f52d3..7b22f89 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o obj-$(CONFIG_PINCTRL_BAYTRAIL) += pinctrl-baytrail.o obj-$(CONFIG_PINCTRL_BCM281XX) += pinctrl-bcm281xx.o obj-$(CONFIG_PINCTRL_FALCON) += pinctrl-falcon.o +obj-$(CONFIG_PINCTRL_MESON) += meson/ obj-$(CONFIG_PINCTRL_PALMAS) += pinctrl-palmas.o obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile new file mode 100644 index 0000000..03a90b4 --- /dev/null +++ b/drivers/pinctrl/meson/Makefile @@ -0,0 +1,2 @@ +obj-y += pinctrl-meson.o +obj-y += pinctrl-meson8.o diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c new file mode 100644 index 0000000..876102d --- /dev/null +++ b/drivers/pinctrl/meson/pinctrl-meson.c @@ -0,0 +1,823 @@ +/* + * Pin controller and GPIO driver for Amlogic Meson SoCs + * + * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/device.h> +#include <linux/gpio.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/pinctrl/pinconf-generic.h> +#include <linux/pinctrl/pinconf.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> +#include <linux/seq_file.h> +#include <linux/spinlock.h> + +#include "../core.h" +#include "../pinctrl-utils.h" +#include "pinctrl-meson.h" + +static void meson_domain_set_bit(struct meson_domain *domain, + void __iomem *addr, unsigned int bit, + unsigned int value) +{ + unsigned long flags; + unsigned int data; + + spin_lock_irqsave(&domain->lock, flags); + data = readl(addr); + + if (value) + data |= BIT(bit); + else + data &= ~BIT(bit); + + writel(data, addr); + spin_unlock_irqrestore(&domain->lock, flags); +} + +static struct meson_domain *meson_pinctrl_get_domain(struct meson_pinctrl *pc, + int pin) +{ + struct meson_domain *domain; + int i, j; + + for (i = 0; i < pc->num_domains; i++) { + domain = &pc->domains[i]; + for (j = 0; j < domain->data->num_banks; j++) { + if (pin >= domain->data->banks[j].first && + pin < domain->data->banks[j].last) + return domain; + } + } + + return NULL; +} + +static int meson_get_pin_reg_and_bit(struct meson_domain *domain, + unsigned pin, int reg_type, + unsigned int *reg_num, unsigned int *bit) +{ + struct meson_bank *bank; + int i, found = 0; + + for (i = 0; i < domain->data->num_banks; i++) { + bank = &domain->data->banks[i]; + if (pin >= bank->first && pin <= bank->last) { + found = 1; + break; + } + } + + if (!found) + return 1; + + *reg_num = bank->regs[reg_type].reg; + *bit = bank->regs[reg_type].bit + pin - bank->first; + + return 0; +} + +static void *meson_get_mux_reg(struct meson_domain *domain, + unsigned int reg_num) +{ + if (reg_num < domain->mux_size) + return domain->reg_mux + 4 * reg_num; + + return NULL; +} + +static int meson_get_groups_count(struct pinctrl_dev *pcdev) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + return pc->num_groups; +} + +static const char *meson_get_group_name(struct pinctrl_dev *pcdev, + unsigned selector) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + return pc->groups[selector].name; +} + +static int meson_get_group_pins(struct pinctrl_dev *pcdev, unsigned selector, + const unsigned **pins, unsigned *num_pins) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + *pins = pc->groups[selector].pins; + *num_pins = pc->groups[selector].num_pins; + + return 0; +} + +static void meson_pin_dbg_show(struct pinctrl_dev *pcdev, struct seq_file *s, + unsigned offset) +{ + seq_printf(s, " %s", dev_name(pcdev->dev)); +} + +static const struct pinctrl_ops meson_pctrl_ops = { + .get_groups_count = meson_get_groups_count, + .get_group_name = meson_get_group_name, + .get_group_pins = meson_get_group_pins, + .dt_node_to_map = pinconf_generic_dt_node_to_map_group, + .dt_free_map = pinctrl_utils_dt_free_map, + .pin_dbg_show = meson_pin_dbg_show, +}; + +static void meson_pmx_disable_other_groups(struct meson_pinctrl *pc, + unsigned int pin, int sel_group) +{ + struct meson_pmx_group *group; + struct meson_domain *domain; + void __iomem *reg; + int i, j; + + for (i = 0; i < pc->num_groups; i++) { + group = &pc->groups[i]; + + if (group->is_gpio || i == sel_group) + continue; + + for (j = 0; j < group->num_pins; j++) { + if (group->pins[j] == pin) { + domain = group->domain; + reg = meson_get_mux_reg(domain, group->reg); + meson_domain_set_bit(domain, reg, + group->bit, 0); + break; + } + } + } +} + +static int meson_pmx_set_mux(struct pinctrl_dev *pcdev, unsigned func_num, + unsigned group_num) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + struct meson_pmx_func *func = &pc->funcs[func_num]; + struct meson_pmx_group *group = &pc->groups[group_num]; + void __iomem *reg; + int i; + + dev_dbg(pc->dev, "enable function %s, group %s\n", func->name, + group->name); + + reg = meson_get_mux_reg(group->domain, group->reg); + if (!reg) + return -EINVAL; + + /* Disable other groups using the pin */ + for (i = 0; i < group->num_pins; i++) + meson_pmx_disable_other_groups(pc, group->pins[i], group_num); + + /* + * Function 0 (GPIO) is the default one and doesn't need any + * additional settings + */ + if (func_num) + meson_domain_set_bit(group->domain, reg, group->bit, 1); + + return 0; +} + +static int meson_pmx_request_gpio(struct pinctrl_dev *pcdev, + struct pinctrl_gpio_range *range, + unsigned offset) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + meson_pmx_disable_other_groups(pc, offset, -1); + + return 0; +} + +static int meson_pmx_get_funcs_count(struct pinctrl_dev *pcdev) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + return pc->num_funcs; +} + +static const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev, + unsigned selector) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + return pc->funcs[selector].name; +} + +static int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector, + const char * const **groups, + unsigned * const num_groups) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + + *groups = pc->funcs[selector].groups; + *num_groups = pc->funcs[selector].num_groups; + + return 0; +} + +static const struct pinmux_ops meson_pmx_ops = { + .get_functions_count = meson_pmx_get_funcs_count, + .get_function_name = meson_pmx_get_func_name, + .get_function_groups = meson_pmx_get_groups, + .gpio_request_enable = meson_pmx_request_gpio, + .set_mux = meson_pmx_set_mux, +}; + +static int meson_pinctrl_calc_reg_bit(struct meson_domain *domain, + unsigned int pin, int reg_type, + void **reg, unsigned int *bit) +{ + unsigned int reg_num; + int ret; + + *reg = NULL; + + ret = meson_get_pin_reg_and_bit(domain, pin, reg_type, + ®_num, bit); + if (!ret) + return -EINVAL; + + if (reg_type == REG_PULLEN) { + if (reg_num < domain->pullen_size) + *reg = domain->reg_pullen + 4 * reg_num; + } else { + if (reg_num < domain->pull_size) + *reg = domain->reg_pull + 4 * reg_num; + } + + return *reg ? 0 : -EINVAL; +} + +static int meson_pinconf_set(struct pinctrl_dev *pcdev, unsigned int pin, + unsigned long *configs, unsigned num_configs) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + void __iomem *pullen_reg, __iomem *pull_reg; + unsigned int pullen_bit, pull_bit; + enum pin_config_param param; + struct meson_domain *domain; + int i, ret; + u16 arg; + + domain = meson_pinctrl_get_domain(pc, pin); + if (!domain) + return -EINVAL; + + ret = meson_pinctrl_calc_reg_bit(domain, pin, REG_PULL, + &pull_reg, &pull_bit); + if (ret) + return -EINVAL; + + ret = meson_pinctrl_calc_reg_bit(domain, pin, REG_PULLEN, + &pullen_reg, &pullen_bit); + if (ret) + return -EINVAL; + + for (i = 0; i < num_configs; i++) { + param = pinconf_to_config_param(configs[i]); + arg = pinconf_to_config_argument(configs[i]); + + switch (param) { + case PIN_CONFIG_BIAS_DISABLE: + dev_dbg(pc->dev, "pin %d bias-disable\n", pin); + meson_domain_set_bit(domain, pullen_reg, pullen_bit, 0); + break; + case PIN_CONFIG_BIAS_PULL_UP: + dev_dbg(pc->dev, "pin %d pull-up\n", pin); + meson_domain_set_bit(domain, pullen_reg, pullen_bit, 1); + meson_domain_set_bit(domain, pull_reg, pull_bit, 1); + break; + case PIN_CONFIG_BIAS_PULL_DOWN: + dev_dbg(pc->dev, "pin %d pull-down\n", pin); + meson_domain_set_bit(domain, pullen_reg, pullen_bit, 1); + meson_domain_set_bit(domain, pull_reg, pull_bit, 0); + break; + default: + return -ENOTSUPP; + } + } + + return 0; +} + +static int meson_pinconf_get_pull(struct meson_pinctrl *pc, unsigned int pin) +{ + struct meson_domain *domain; + unsigned int bit; + int ret, conf; + void *reg; + + domain = meson_pinctrl_get_domain(pc, pin); + if (!domain) + return -EINVAL; + + ret = meson_pinctrl_calc_reg_bit(domain, pin, REG_PULLEN, ®, &bit); + if (ret) { + dev_err(pc->dev, "can't find register for pin %u\n", pin); + return -EINVAL; + } + + if (!(readl(reg) & BIT(bit))) { + conf = PIN_CONFIG_BIAS_DISABLE; + } else { + ret = meson_pinctrl_calc_reg_bit(domain, pin, REG_PULL, + ®, &bit); + if (ret) { + dev_err(pc->dev, "can't find register for pin %u\n", + pin); + return -EINVAL; + } + + if (!(readl(reg) & BIT(bit))) + conf = PIN_CONFIG_BIAS_PULL_DOWN; + else + conf = PIN_CONFIG_BIAS_PULL_UP; + } + + return conf; +} + +static int meson_pinconf_get(struct pinctrl_dev *pcdev, unsigned int pin, + unsigned long *config) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + enum pin_config_param param = pinconf_to_config_param(*config); + u16 arg; + + switch (param) { + case PIN_CONFIG_BIAS_DISABLE: + case PIN_CONFIG_BIAS_PULL_DOWN: + case PIN_CONFIG_BIAS_PULL_UP: + if (meson_pinconf_get_pull(pc, pin) == param) + arg = 1; + else + return -EINVAL; + break; + default: + return -ENOTSUPP; + } + + *config = pinconf_to_config_packed(param, arg); + + return 0; +} + +static int meson_pinconf_group_set(struct pinctrl_dev *pcdev, + unsigned int num_group, + unsigned long *configs, unsigned num_configs) +{ + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); + struct meson_pmx_group *group = &pc->groups[num_group]; + int i; + + dev_dbg(pc->dev, "set pinconf for group %s\n", group->name); + + for (i = 0; i < group->num_pins; i++) { + meson_pinconf_set(pcdev, group->pins[i], configs, + num_configs); + } + + return 0; +} + +static int meson_pinconf_group_get(struct pinctrl_dev *pcdev, + unsigned int group, unsigned long *config) +{ + return -ENOSYS; +} + +static const struct pinconf_ops meson_pinconf_ops = { + .pin_config_get = meson_pinconf_get, + .pin_config_set = meson_pinconf_set, + .pin_config_group_get = meson_pinconf_group_get, + .pin_config_group_set = meson_pinconf_group_set, + .is_generic = true, +}; + +static inline struct meson_domain *to_meson_domain(struct gpio_chip *chip) +{ + return container_of(chip, struct meson_domain, chip); +} + +static int meson_gpio_request(struct gpio_chip *chip, unsigned gpio) +{ + return pinctrl_request_gpio(chip->base + gpio); +} + +static void meson_gpio_free(struct gpio_chip *chip, unsigned gpio) +{ + pinctrl_free_gpio(chip->base + gpio); +} + +static int meson_gpio_calc_reg_and_bit(struct meson_domain *domain, + unsigned gpio, int reg_type, + void **reg, unsigned int *bit) +{ + unsigned int reg_num; + int ret; + + ret = meson_get_pin_reg_and_bit(domain, gpio, reg_type, ®_num, bit); + + if (ret) + return -EINVAL; + + if (reg_num >= domain->gpio_size) + return -EINVAL; + + *reg = domain->reg_gpio + 4 * reg_num; + + return 0; +} + +static int meson_gpio_direction_input(struct gpio_chip *chip, unsigned gpio) +{ + struct meson_domain *domain = to_meson_domain(chip); + void __iomem *addr; + unsigned int bit; + int ret; + + ret = meson_gpio_calc_reg_and_bit(domain, chip->base + gpio, REG_DIR, + &addr, &bit); + if (ret) + return ret; + + meson_domain_set_bit(domain, addr, bit, 1); + + return 0; +} + +static int meson_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, + int value) +{ + struct meson_domain *domain = to_meson_domain(chip); + void __iomem *addr; + unsigned int bit; + int ret; + + ret = meson_gpio_calc_reg_and_bit(domain, chip->base + gpio, REG_DIR, + &addr, &bit); + if (ret) + return ret; + + meson_domain_set_bit(domain, addr, bit, 0); + + return 0; +} + +static void meson_gpio_set(struct gpio_chip *chip, unsigned gpio, int value) +{ + struct meson_domain *domain = to_meson_domain(chip); + void __iomem *addr; + unsigned int bit; + + if (meson_gpio_calc_reg_and_bit(domain, chip->base + gpio, REG_OUT, + &addr, &bit)) + return; + + meson_domain_set_bit(domain, addr, bit, value); +} + +static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) +{ + struct meson_domain *domain = to_meson_domain(chip); + void __iomem *addr; + unsigned int bit; + + if (meson_gpio_calc_reg_and_bit(domain, chip->base + gpio, REG_IN, + &addr, &bit)) + return 0; + + return (readl(addr) >> bit) & 1; +} + +static const struct of_device_id meson_pinctrl_dt_match[] = { + { + .compatible = "amlogic,meson8-pinctrl", + .data = meson8_domain_data, + }, + { }, +}; +MODULE_DEVICE_TABLE(of, meson_pinctrl_dt_match); + +static int meson_gpio_of_xlate(struct gpio_chip *chip, + const struct of_phandle_args *gpiospec, + u32 *flags) +{ + unsigned gpio = gpiospec->args[0]; + + if (gpio < chip->base || gpio >= chip->base + chip->ngpio) + return -EINVAL; + + if (flags) + *flags = gpiospec->args[1]; + + return gpio - chip->base; +} + +static struct meson_domain_data *get_domain_data(struct device_node *node, + struct meson_domain_data *data) +{ + while (data->name) { + if (!strcmp(node->name, data->name)) + return data; + data++; + } + + return NULL; +} + +static int meson_pinctrl_init_data(struct meson_pinctrl *pc) +{ + struct meson_domain_data *data; + int i, j, pin = 0, func = 0, group = 0; + + /* Copy pin definitions from domains to pinctrl */ + pc->pins = devm_kzalloc(pc->dev, pc->num_pins * + sizeof(struct pinctrl_pin_desc), GFP_KERNEL); + if (!pc->pins) + return -ENOMEM; + + for (i = 0, j = 0; i < pc->num_domains; i++) { + data = pc->domains[i].data; + for (j = 0; j < data->num_pins; j++) { + pc->pins[pin].number = pin; + pc->pins[pin++].name = data->pin_names[j]; + } + } + + pc->num_groups = 0; + pc->num_funcs = 0; + + for (i = 0; i < pc->num_domains; i++) { + data = pc->domains[i].data; + pc->num_groups += data->num_groups; + pc->num_funcs += data->num_funcs; + } + + /* Copy group and function definitions from domains to pinctrl */ + pc->groups = devm_kzalloc(pc->dev, pc->num_groups * + sizeof(struct meson_pmx_group), GFP_KERNEL); + pc->funcs = devm_kzalloc(pc->dev, pc->num_funcs * + sizeof(struct meson_pmx_func), GFP_KERNEL); + if (!pc->groups || !pc->funcs) + return -ENOMEM; + + for (i = 0; i < pc->num_domains; i++) { + data = pc->domains[i].data; + + for (j = 0; j < data->num_groups; j++) { + memcpy(&pc->groups[group], &data->groups[j], + sizeof(struct meson_pmx_group)); + pc->groups[group++].domain = &pc->domains[i]; + } + + for (j = 0; j < data->num_funcs; j++) { + memcpy(&pc->funcs[func++], &data->funcs[j], + sizeof(struct meson_pmx_func)); + } + } + + /* Count pins in groups */ + for (i = 0; i < pc->num_groups; i++) { + for (j = 0; ; j++) { + if (pc->groups[i].pins[j] == PINS_END) { + pc->groups[i].num_pins = j; + break; + } + } + } + + /* Count groups in functions */ + for (i = 0; i < pc->num_funcs; i++) { + for (j = 0; ; j++) { + if (!pc->funcs[i].groups[j]) { + pc->funcs[i].num_groups = j; + break; + } + } + } + + return 0; +} + +static void __iomem *meson_map_resource(struct meson_pinctrl *pc, + struct device_node *node, + char *name, unsigned int *size) +{ + struct resource res; + int i; + + i = of_property_match_string(node, "reg-names", name); + if (of_address_to_resource(node, i, &res)) + return NULL; + + *size = resource_size(&res) / 4; + return devm_ioremap_resource(pc->dev, &res); +} + +static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc, + struct device_node *node, + struct meson_domain_data *data) +{ + struct device_node *np; + struct meson_domain *domain; + int i; + + for_each_child_of_node(node, np) { + if (!of_find_property(np, "gpio-controller", NULL)) + continue; + pc->num_domains++; + } + + pc->domains = devm_kzalloc(pc->dev, pc->num_domains * + sizeof(struct meson_domain), + GFP_KERNEL); + if (!pc->domains) + return -ENOMEM; + + i = 0; + for_each_child_of_node(node, np) { + if (!of_find_property(np, "gpio-controller", NULL)) + continue; + + domain = &pc->domains[i]; + + domain->reg_mux = meson_map_resource(pc, np, "mux", + &domain->mux_size); + if (!domain->reg_mux) { + dev_err(pc->dev, "mux registers not found\n"); + return -ENODEV; + } + + domain->reg_pull = meson_map_resource(pc, np, "pull", + &domain->pull_size); + if (!domain->reg_pull) { + dev_err(pc->dev, "pull registers not found\n"); + return -ENODEV; + } + + domain->reg_pullen = meson_map_resource(pc, np, "pull-enable", + &domain->pullen_size); + if (!domain->reg_pullen) { + /* Use pull region if pull-enable is not present */ + domain->reg_pullen = domain->reg_pull; + domain->pullen_size = domain->pull_size; + } + + + domain->reg_gpio = meson_map_resource(pc, np, "gpio", + &domain->gpio_size); + if (!domain->reg_gpio) { + dev_err(pc->dev, "gpio registers not found\n"); + return -ENODEV; + } + + domain->data = get_domain_data(np, data); + if (!domain->data) { + dev_err(pc->dev, "domain not found\n"); + return -ENODEV; + } + + domain->of_node = np; + pc->num_pins += domain->data->num_pins; + i++; + } + + return 0; +} + +static int meson_gpiolib_register(struct meson_pinctrl *pc) +{ + struct meson_domain *domain; + unsigned int base = 0; + int i, ret; + + for (i = 0; i < pc->num_domains; i++) { + domain = &pc->domains[i]; + + domain->chip.label = domain->data->name; + domain->chip.dev = pc->dev; + domain->chip.request = meson_gpio_request; + domain->chip.free = meson_gpio_free; + domain->chip.direction_input = meson_gpio_direction_input; + domain->chip.direction_output = meson_gpio_direction_output; + domain->chip.get = meson_gpio_get; + domain->chip.set = meson_gpio_set; + domain->chip.base = base; + domain->chip.ngpio = domain->data->num_pins; + domain->chip.names = domain->data->pin_names; + domain->chip.can_sleep = false; + domain->chip.of_node = domain->of_node; + domain->chip.of_gpio_n_cells = 2; + domain->chip.of_xlate = meson_gpio_of_xlate; + + ret = gpiochip_add(&domain->chip); + if (ret < 0) { + dev_err(pc->dev, "can't add gpio chip %s\n", + domain->data->name); + goto fail; + } + + domain->gpio_range.name = domain->data->name; + domain->gpio_range.id = i; + domain->gpio_range.base = base; + domain->gpio_range.pin_base = base; + domain->gpio_range.npins = domain->data->num_pins; + domain->gpio_range.gc = &domain->chip; + pinctrl_add_gpio_range(pc->pcdev, &domain->gpio_range); + + base += domain->data->num_pins; + } + + return 0; +fail: + for (i--; i >= 0; i--) + gpiochip_remove(&pc->domains[i].chip); + + return ret; +} + +static int meson_pinctrl_probe(struct platform_device *pdev) +{ + const struct of_device_id *match; + struct device *dev = &pdev->dev; + struct meson_domain_data *data; + struct meson_pinctrl *pc; + int ret; + + pc = devm_kzalloc(dev, sizeof(struct meson_pinctrl), GFP_KERNEL); + if (!pc) + return -ENOMEM; + + match = of_match_node(meson_pinctrl_dt_match, pdev->dev.of_node); + data = (struct meson_domain_data *)match->data; + pc->dev = dev; + + ret = meson_pinctrl_parse_dt(pc, pdev->dev.of_node, data); + if (ret) + return ret; + + ret = meson_pinctrl_init_data(pc); + if (ret) + return ret; + + pc->desc.name = "pinctrl-meson"; + pc->desc.owner = THIS_MODULE; + pc->desc.pctlops = &meson_pctrl_ops; + pc->desc.pmxops = &meson_pmx_ops; + pc->desc.confops = &meson_pinconf_ops; + pc->desc.pins = pc->pins; + pc->desc.npins = pc->num_pins; + + pc->pcdev = pinctrl_register(&pc->desc, pc->dev, pc); + if (!pc->pcdev) { + dev_err(pc->dev, "can't register pinctrl device"); + return -EINVAL; + } + + ret = meson_gpiolib_register(pc); + if (ret) { + pinctrl_unregister(pc->pcdev); + return ret; + } + + return 0; +} + +static struct platform_driver meson_pinctrl_driver = { + .probe = meson_pinctrl_probe, + .driver = { + .name = "meson-pinctrl", + .of_match_table = meson_pinctrl_dt_match, + }, +}; + +static int __init meson_pinctrl_drv_register(void) +{ + return platform_driver_register(&meson_pinctrl_driver); +} +postcore_initcall(meson_pinctrl_drv_register); + +MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>"); +MODULE_DESCRIPTION("Amlogic Meson pinctrl driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h new file mode 100644 index 0000000..992b62d --- /dev/null +++ b/drivers/pinctrl/meson/pinctrl-meson.h @@ -0,0 +1,217 @@ +/* + * Pin controller and GPIO driver for Amlogic Meson SoCs + * + * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/gpio.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/spinlock.h> +#include <linux/types.h> + +/** + * struct meson_pmx_group - a pinmux group + * + * @name: group name + * @pins: pins in the group + * @num_pins: number of pins in the group + * @is_gpio: flag set when the group is a single GPIO group + * @reg: register offset for the group in the domain mux registers + * @bit bit index enabling the group + * @domain: pin domain this group belongs to + */ +struct meson_pmx_group { + const char *name; + const unsigned int *pins; + unsigned int num_pins; + bool is_gpio; + unsigned int reg; + unsigned int bit; + struct meson_domain *domain; +}; + +/** + * struct meson_pmx_func - a pinmux function + * + * @name: function name + * @groups: groups in the function + * @num_groups: number of groups in the function + */ +struct meson_pmx_func { + const char *name; + const char **groups; + unsigned int num_groups; +}; + +/** + * struct meson_reg_offset + * + * @reg: register offset + * @bit: bit index + */ +struct meson_reg_offset { + unsigned int reg; + unsigned int bit; +}; + +enum { + REG_PULLEN, + REG_PULL, + REG_DIR, + REG_OUT, + REG_IN, + NUM_REG, +}; + +/** + * struct meson bank + * + * @name: bank name + * @first: first pin of the bank + * @last: last pin of the bank + * @regs: couples of <reg offset, bit index> controlling the + * functionalities of the bank pins (pull, direction, value) + * + * A bank represents a set of pins controlled by a contiguous set of + * bits in the domain registers. + */ +struct meson_bank { + const char *name; + unsigned int first; + unsigned int last; + struct meson_reg_offset regs[NUM_REG]; +}; + +/** + * struct meson_domain_data - domain platform data + * + * @name: label for the domain + * @pin_names: names of the pins in the domain + * @banks: set of banks belonging to the domain + * @funcs: available pinmux functions + * @groups: available pinmux groups + * @num_pins: number of pins in the domain + * @num_banks: number of banks in the domain + * @num_funcs: number of available pinmux functions + * @num_groups: number of available pinmux groups + * + */ +struct meson_domain_data { + const char *name; + const char **pin_names; + struct meson_bank *banks; + struct meson_pmx_func *funcs; + struct meson_pmx_group *groups; + unsigned int num_pins; + unsigned int num_banks; + unsigned int num_funcs; + unsigned int num_groups; +}; + +/** + * struct meson_domain + * + * @reg_mux: registers for mux settings + * @reg_pullen: registers for pull-enable settings + * @reg_pull: registers for pull settings + * @reg_gpio: registers for gpio settings + * @mux_size: size of mux register range (in words) + * @pullen_size:size of pull-enable register range + * @pull_size: size of pull register range + * @gpio_size: size of gpio register range + * @chip: gpio chip associated with the domain + * @data; platform data for the domain + * @node: device tree node for the domain + * @gpio_range: gpio range that maps domain gpios to the pin controller + * @lock: spinlock for accessing domain registers + * + * A domain represents a set of banks controlled by the same set of + * registers. Typically there is a domain for the normal banks and + * another one for the Always-On bus. + */ +struct meson_domain { + void __iomem *reg_mux; + void __iomem *reg_pullen; + void __iomem *reg_pull; + void __iomem *reg_gpio; + + unsigned int mux_size; + unsigned int pullen_size; + unsigned int pull_size; + unsigned int gpio_size; + + struct gpio_chip chip; + struct meson_domain_data *data; + struct device_node *of_node; + struct pinctrl_gpio_range gpio_range; + + spinlock_t lock; +}; + +struct meson_pinctrl { + struct device *dev; + struct pinctrl_dev *pcdev; + struct pinctrl_desc desc; + + struct pinctrl_pin_desc *pins; + struct meson_domain *domains; + struct meson_pmx_func *funcs; + struct meson_pmx_group *groups; + + unsigned int num_pins; + unsigned int num_domains; + unsigned int num_funcs; + unsigned int num_groups; +}; + +#define PINS_END 0xffff + +#define GROUP(_name, _reg, _bit, _pins...) \ + { \ + .name = #_name, \ + .pins = (const unsigned int[]) { _pins, PINS_END }, \ + .num_pins = 0, \ + .is_gpio = false, \ + .reg = _reg, \ + .bit = _bit, \ + } + +#define GPIO_GROUP(_gpio) \ + { \ + .name = #_gpio, \ + .pins = (const unsigned int[]){ _gpio, PINS_END }, \ + .num_pins = 0, \ + .is_gpio = true, \ + } + +#define FUNCTION(_name, _groups...) \ + { \ + .name = _name, \ + .groups = (const char *[]) { _groups, NULL }, \ + .num_groups = 0, \ + } + +#define BANK(n, f, l, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ + { \ + .name = n, \ + .first = f, \ + .last = l, \ + .regs = { \ + [REG_PULLEN] = { per, peb }, \ + [REG_PULL] = { pr, pb }, \ + [REG_DIR] = { dr, db }, \ + [REG_OUT] = { or, ob }, \ + [REG_IN] = { ir, ib }, \ + }, \ + } + +#define MESON_PIN(x) PINCTRL_PIN(x, #x) + +extern struct meson_domain_data meson8_domain_data[]; diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c new file mode 100644 index 0000000..4b16cc5 --- /dev/null +++ b/drivers/pinctrl/meson/pinctrl-meson8.c @@ -0,0 +1,374 @@ +/* + * Pin controller and GPIO data for Amlogic Meson8 + * + * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <dt-bindings/gpio/meson8-gpio.h> + +#include "pinctrl-meson.h" + +const char *meson8_pin_names[] = { + "GPIOX_0", "GPIOX_1", "GPIOX_2", "GPIOX_3", "GPIOX_4", + "GPIOX_5", "GPIOX_6", "GPIOX_7", "GPIOX_8", "GPIOX_9", + "GPIOX_10", "GPIOX_11", "GPIOX_12", "GPIOX_13", "GPIOX_14", + "GPIOX_15", "GPIOX_16", "GPIOX_17", "GPIOX_18", "GPIOX_19", + "GPIOX_20", "GPIOX_21", + + "GPIOY_0", "GPIOY_1", "GPIOY_2", "GPIOY_3", "GPIOY_4", + "GPIOY_5", "GPIOY_6", "GPIOY_7", "GPIOY_8", "GPIOY_9", + "GPIOY_10", "GPIOY_11", "GPIOY_12", "GPIOY_13", "GPIOY_14", + "GPIOY_15", "GPIOY_16", + + + "GPIODV_0", "GPIODV_1", "GPIODV_2", "GPIODV_3", "GPIODV_4", + "GPIODV_5", "GPIODV_6", "GPIODV_7", "GPIODV_8", "GPIODV_9", + "GPIODV_10", "GPIODV_11", "GPIODV_12", "GPIODV_13", "GPIODV_14", + "GPIODV_15", "GPIODV_16", "GPIODV_17", "GPIODV_18", "GPIODV_19", + "GPIODV_20", "GPIODV_21", "GPIODV_22", "GPIODV_23", "GPIODV_24", + "GPIODV_25", "GPIODV_26", "GPIODV_27", "GPIODV_28", "GPIODV_29", + + "GPIOH_0", "GPIOH_1", "GPIOH_2", "GPIOH_3", "GPIOH_4", + "GPIOH_5", "GPIOH_6", "GPIOH_7", "GPIOH_8", "GPIOH_9", + + "GPIOZ_0", "GPIOZ_1", "GPIOZ_2", "GPIOZ_3", "GPIOZ_4", + "GPIOZ_5", "GPIOZ_6", "GPIOZ_7", "GPIOZ_8", "GPIOZ_9", + "GPIOZ_10", "GPIOZ_11", "GPIOZ_12", "GPIOZ_13", "GPIOZ_14", + + "CARD_0", "CARD_1", "CARD_2", "CARD_3", "CARD_4", + "CARD_5", "CARD_6", + + "BOOT_0", "BOOT_1", "BOOT_2", "BOOT_3", "BOOT_4", + "BOOT_5", "BOOT_6", "BOOT_7", "BOOT_8", "BOOT_9", + "BOOT_10", "BOOT_11", "BOOT_12", "BOOT_13", "BOOT_14", + "BOOT_15", "BOOT_16", "BOOT_17", "BOOT_18", +}; + +const char *meson8_ao_pin_names[] = { + "GPIOAO_0", "GPIOAO_1", "GPIOAO_2", "GPIOAO_3", + "GPIOAO_4", "GPIOAO_5", "GPIOAO_6", "GPIOAO_7", + "GPIOAO_8", "GPIOAO_9", "GPIOAO_10", "GPIOAO_11", + "GPIOAO_12", "GPIOAO_13", "GPIO_BSD_EN", "GPIO_TEST_N", +}; + +struct meson_pmx_group meson8_groups[] = { + GPIO_GROUP(GPIOX_0), + GPIO_GROUP(GPIOX_1), + GPIO_GROUP(GPIOX_2), + GPIO_GROUP(GPIOX_3), + GPIO_GROUP(GPIOX_4), + GPIO_GROUP(GPIOX_5), + GPIO_GROUP(GPIOX_6), + GPIO_GROUP(GPIOX_7), + GPIO_GROUP(GPIOX_8), + GPIO_GROUP(GPIOX_9), + GPIO_GROUP(GPIOX_10), + GPIO_GROUP(GPIOX_11), + GPIO_GROUP(GPIOX_12), + GPIO_GROUP(GPIOX_13), + GPIO_GROUP(GPIOX_14), + GPIO_GROUP(GPIOX_15), + GPIO_GROUP(GPIOX_16), + GPIO_GROUP(GPIOX_17), + GPIO_GROUP(GPIOX_18), + GPIO_GROUP(GPIOX_19), + GPIO_GROUP(GPIOX_20), + GPIO_GROUP(GPIOX_21), + GPIO_GROUP(GPIOY_0), + GPIO_GROUP(GPIOY_1), + GPIO_GROUP(GPIOY_2), + GPIO_GROUP(GPIOY_3), + GPIO_GROUP(GPIOY_4), + GPIO_GROUP(GPIOY_5), + GPIO_GROUP(GPIOY_6), + GPIO_GROUP(GPIOY_7), + GPIO_GROUP(GPIOY_8), + GPIO_GROUP(GPIOY_9), + GPIO_GROUP(GPIOY_10), + GPIO_GROUP(GPIOY_11), + GPIO_GROUP(GPIOY_12), + GPIO_GROUP(GPIOY_13), + GPIO_GROUP(GPIOY_14), + GPIO_GROUP(GPIOY_15), + GPIO_GROUP(GPIOY_16), + GPIO_GROUP(GPIODV_0), + GPIO_GROUP(GPIODV_1), + GPIO_GROUP(GPIODV_2), + GPIO_GROUP(GPIODV_3), + GPIO_GROUP(GPIODV_4), + GPIO_GROUP(GPIODV_5), + GPIO_GROUP(GPIODV_6), + GPIO_GROUP(GPIODV_7), + GPIO_GROUP(GPIODV_8), + GPIO_GROUP(GPIODV_9), + GPIO_GROUP(GPIODV_10), + GPIO_GROUP(GPIODV_11), + GPIO_GROUP(GPIODV_12), + GPIO_GROUP(GPIODV_13), + GPIO_GROUP(GPIODV_14), + GPIO_GROUP(GPIODV_15), + GPIO_GROUP(GPIODV_16), + GPIO_GROUP(GPIODV_17), + GPIO_GROUP(GPIODV_18), + GPIO_GROUP(GPIODV_19), + GPIO_GROUP(GPIODV_20), + GPIO_GROUP(GPIODV_21), + GPIO_GROUP(GPIODV_22), + GPIO_GROUP(GPIODV_23), + GPIO_GROUP(GPIODV_24), + GPIO_GROUP(GPIODV_25), + GPIO_GROUP(GPIODV_26), + GPIO_GROUP(GPIODV_27), + GPIO_GROUP(GPIODV_28), + GPIO_GROUP(GPIODV_29), + GPIO_GROUP(GPIOH_0), + GPIO_GROUP(GPIOH_1), + GPIO_GROUP(GPIOH_2), + GPIO_GROUP(GPIOH_3), + GPIO_GROUP(GPIOH_4), + GPIO_GROUP(GPIOH_5), + GPIO_GROUP(GPIOH_6), + GPIO_GROUP(GPIOH_7), + GPIO_GROUP(GPIOH_8), + GPIO_GROUP(GPIOH_9), + GPIO_GROUP(GPIOZ_0), + GPIO_GROUP(GPIOZ_1), + GPIO_GROUP(GPIOZ_2), + GPIO_GROUP(GPIOZ_3), + GPIO_GROUP(GPIOZ_4), + GPIO_GROUP(GPIOZ_5), + GPIO_GROUP(GPIOZ_6), + GPIO_GROUP(GPIOZ_7), + GPIO_GROUP(GPIOZ_8), + GPIO_GROUP(GPIOZ_9), + GPIO_GROUP(GPIOZ_10), + GPIO_GROUP(GPIOZ_11), + GPIO_GROUP(GPIOZ_12), + GPIO_GROUP(GPIOZ_13), + GPIO_GROUP(GPIOZ_14), + /* SD A */ + GROUP(sd_d0_a, 8, 5, GPIOX_0), + GROUP(sd_d1_a, 8, 4, GPIOX_1), + GROUP(sd_d2_a, 8, 3, GPIOX_2), + GROUP(sd_d3_a, 8, 2, GPIOX_3), + GROUP(sd_clk_a, 8, 1, GPIOX_8), + GROUP(sd_cmd_a, 8, 0, GPIOX_9), + /* SD B */ + GROUP(sd_d1_b, 2, 14, CARD_0), + GROUP(sd_d0_b, 2, 15, CARD_1), + GROUP(sd_clk_b, 2, 11, CARD_2), + GROUP(sd_cmd_b, 2, 10, CARD_3), + GROUP(sd_d3_b, 2, 12, CARD_4), + GROUP(sd_d2_b, 2, 13, CARD_5), + /* SDXC A */ + GROUP(sdxc_d0_a, 5, 14, GPIOX_0), + GROUP(sdxc_d13_a, 5, 13, GPIOX_1, GPIOX_2, GPIOX_3), + GROUP(sdxc_d47_a, 5, 12, GPIOX_4, GPIOX_5, GPIOX_6, GPIOX_7), + GROUP(sdxc_clk_a, 5, 11, GPIOX_8), + GROUP(sdxc_cmd_a, 5, 10, GPIOX_9), + /* PCM A */ + GROUP(pcm_out_a, 3, 30, GPIOX_4), + GROUP(pcm_in_a, 3, 29, GPIOX_5), + GROUP(pcm_fs_a, 3, 28, GPIOX_6), + GROUP(pcm_clk_a, 3, 27, GPIOX_7), + /* UART A */ + GROUP(uart_tx_a0, 4, 17, GPIOX_4), + GROUP(uart_rx_a0, 4, 16, GPIOX_5), + GROUP(uart_cts_a0, 4, 15, GPIOX_6), + GROUP(uart_rts_a0, 4, 14, GPIOX_7), + GROUP(uart_tx_a1, 4, 13, GPIOX_12), + GROUP(uart_rx_a1, 4, 12, GPIOX_13), + GROUP(uart_cts_a1, 4, 11, GPIOX_14), + GROUP(uart_rts_a1, 4, 10, GPIOX_15), + /* XTAL */ + GROUP(xtal_32k_out, 3, 22, GPIOX_10), + GROUP(xtal_24m_out, 3, 23, GPIOX_11), + /* UART B */ + GROUP(uart_tx_b0, 4, 9, GPIOX_16), + GROUP(uart_rx_b0, 4, 8, GPIOX_17), + GROUP(uart_cts_b0, 4, 7, GPIOX_18), + GROUP(uart_rts_b0, 4, 6, GPIOX_19), + GROUP(uart_tx_b1, 6, 23, GPIODV_24), + GROUP(uart_rx_b1, 6, 22, GPIODV_25), + GROUP(uart_cts_b1, 6, 21, GPIODV_26), + GROUP(uart_rts_b1, 6, 20, GPIODV_27), + /* UART C */ + GROUP(uart_tx_c, 1, 19, GPIOY_0), + GROUP(uart_rx_c, 1, 18, GPIOY_1), + GROUP(uart_cts_c, 1, 17, GPIOY_2), + GROUP(uart_rts_c, 1, 16, GPIOY_3), + /* ETHERNET */ + GROUP(eth_tx_clk_50m, 6, 15, GPIOZ_4), + GROUP(eth_tx_en, 6, 14, GPIOZ_5), + GROUP(eth_txd1, 6, 13, GPIOZ_6), + GROUP(eth_txd0, 6, 12, GPIOZ_7), + GROUP(eth_rx_clk_in, 6, 10, GPIOZ_8), + GROUP(eth_rx_dv, 6, 11, GPIOZ_9), + GROUP(eth_rxd1, 6, 8, GPIOZ_10), + GROUP(eth_rxd0, 6, 7, GPIOZ_11), + GROUP(eth_mdio, 6, 6, GPIOZ_12), + GROUP(eth_mdc, 6, 5, GPIOZ_13), + /* NAND */ + GROUP(nand_io, 2, 26, BOOT_0, BOOT_1, BOOT_2, BOOT_3, + BOOT_4, BOOT_5, BOOT_6, BOOT_7), + GROUP(nand_io_ce0, 2, 25, BOOT_8), + GROUP(nand_io_ce1, 2, 24, BOOT_9), + GROUP(nand_io_rb0, 2, 17, BOOT_10), + GROUP(nand_ale, 2, 21, BOOT_11), + GROUP(nand_cle, 2, 20, BOOT_12), + GROUP(nand_wen_clk, 2, 19, BOOT_13), + GROUP(nand_ren_clk, 2, 18, BOOT_14), + GROUP(nand_dqs, 2, 27, BOOT_15), + GROUP(nand_ce2, 2, 23, BOOT_16), + GROUP(nand_ce3, 2, 22, BOOT_17), + /* SPI */ + GROUP(spi_ss0_0, 9, 13, GPIOH_3), + GROUP(spi_miso_0, 9, 12, GPIOH_4), + GROUP(spi_mosi_0, 9, 11, GPIOH_5), + GROUP(spi_sclk_0, 9, 10, GPIOH_6), + GROUP(spi_ss0_1, 8, 16, GPIOZ_9), + GROUP(spi_ss1_1, 8, 12, GPIOZ_10), + GROUP(spi_sclk_1, 8, 15, GPIOZ_11), + GROUP(spi_mosi_1, 8, 14, GPIOZ_12), + GROUP(spi_miso_1, 8, 13, GPIOZ_13), + GROUP(spi_ss2_1, 8, 17, GPIOZ_14), +}; + +struct meson_pmx_group meson8_ao_groups[] = { + GPIO_GROUP(GPIOAO_0), + GPIO_GROUP(GPIOAO_1), + GPIO_GROUP(GPIOAO_2), + GPIO_GROUP(GPIOAO_3), + GPIO_GROUP(GPIOAO_4), + GPIO_GROUP(GPIOAO_5), + GPIO_GROUP(GPIOAO_6), + GPIO_GROUP(GPIOAO_7), + GPIO_GROUP(GPIOAO_8), + GPIO_GROUP(GPIOAO_9), + GPIO_GROUP(GPIOAO_10), + GPIO_GROUP(GPIOAO_11), + GPIO_GROUP(GPIOAO_12), + GPIO_GROUP(GPIOAO_13), + GPIO_GROUP(GPIO_BSD_EN), + GPIO_GROUP(GPIO_TEST_N), + /* UART AO A */ + GROUP(uart_tx_ao_a, 0, 12, GPIOAO_0), + GROUP(uart_rx_ao_a, 0, 11, GPIOAO_1), + GROUP(uart_cts_ao_a, 0, 10, GPIOAO_2), + GROUP(uart_rts_ao_a, 0, 9, GPIOAO_3), + /* UART AO B */ + GROUP(uart_tx_ao_b0, 0, 26, GPIOAO_0), + GROUP(uart_rx_ao_b0, 0, 25, GPIOAO_1), + GROUP(uart_tx_ao_b1, 0, 24, GPIOAO_4), + GROUP(uart_rx_ao_b1, 0, 23, GPIOAO_5), + /* I2C master AO */ + GROUP(i2c_mst_sck_ao, 0, 6, GPIOAO_4), + GROUP(i2c_mst_sda_ao, 0, 5, GPIOAO_5), +}; + +struct meson_pmx_func meson8_functions[] = { + { + .name = "gpio", + .groups = meson8_pin_names, + .num_groups = ARRAY_SIZE(meson8_pin_names), + }, + FUNCTION("sd_a", "sd_d0_a", "sd_d1_a", "sd_d2_a", "sd_d3_a", + "sd_clk_a", "sd_cmd_a"), + + FUNCTION("sd_b", "sd_d1_b", "sd_d0_b", "sd_clk_b", "sd_cmd_b", + "sd_d3_b", "sd_d2_b"), + + FUNCTION("sdxc_a", "sdxc_d0_a", "sdxc_d13_a", "sdxc_d47_a", + "sdxc_clk_a", "sdxc_cmd_a"), + + FUNCTION("pcm_a", "pcm_out_a", "pcm_in_a", "pcm_fs_a", "pcm_clk_a"), + + FUNCTION("uart_a", "uart_tx_a0", "uart_rx_a0", "uart_cts_a0", "uart_rts_a0", + "uart_tx_a1", "uart_rx_a1", "uart_cts_a1", "uart_rts_a1"), + + FUNCTION("xtal", "xtal_32k_out", "xtal_24m_out"), + + FUNCTION("uart_b", "uart_tx_b0", "uart_rx_b0", "uart_cts_b0", "uart_rts_b0", + "uart_tx_b1", "uart_rx_b1", "uart_cts_b1", "uart_rts_b1"), + + FUNCTION("uart_c", "uart_tx_c", "uart_rx_c", "uart_cts_c", "uart_rts_c"), + + FUNCTION("ethernet", "eth_tx_clk_50m", "eth_tx_en", "eth_txd1", + "eth_txd0", "eth_rx_clk_in", "eth_rx_dv", + "eth_rxd1", "eth_rxd0", "eth_mdio", "eth_mdc"), + + FUNCTION("nand", "nand_io", "nand_io_ce0", "nand_io_ce1", + "nand_io_rb0", "nand_ale", "nand_cle", + "nand_wen_clk", "nand_ren_clk", "nand_dqs", + "nand_ce2", "nand_ce3"), + + FUNCTION("spi", "spi_ss0_0", "spi_miso_0", "spi_mosi_0", "spi_sclk_0", + "spi_ss0_1", "spi_ss1_1", "spi_sclk_1", "spi_mosi_1", + "spi_miso_1", "spi_ss2_1"), +}; + +struct meson_pmx_func meson8_ao_functions[] = { + { + .name = "gpio", + .groups = meson8_ao_pin_names, + .num_groups = ARRAY_SIZE(meson8_ao_pin_names), + }, + FUNCTION("uart_ao", "uart_tx_ao_a", "uart_rx_ao_a", + "uart_cts_ao_a", "uart_rts_ao_a"), + + FUNCTION("uart_ao_b", "uart_tx_ao_b0", "uart_rx_ao_b0", + "uart_tx_ao_b1", "uart_rx_ao_b1"), + + FUNCTION("i2c_mst_ao", "i2c_mst_sck_ao", "i2c_mst_sda_ao"), +}; + + +struct meson_bank meson8_banks[] = { + /* name first last pullen pull dir out in */ + BANK("X", GPIOX_0, GPIOX_21, 4, 0, 4, 0, 0, 0, 1, 0, 2, 0), + BANK("Y", GPIOY_0, GPIOY_16, 3, 0, 3, 0, 3, 0, 4, 0, 5, 0), + BANK("DV", GPIODV_0, GPIODV_29, 0, 0, 0, 0, 7, 0, 8, 0, 9, 0), + BANK("H", GPIOH_0, GPIOH_9, 1, 16, 1, 16, 9, 19, 10, 19, 11, 19), + BANK("Z", GPIOZ_0, GPIOZ_14, 1, 0, 1, 0, 3, 17, 4, 17, 5, 17), + BANK("CARD", CARD_0, CARD_6, 2, 20, 2, 20, 0, 22, 1, 22, 2, 22), + BANK("BOOT", BOOT_0, BOOT_18, 2, 0, 2, 0, 9, 0, 10, 0, 11, 0), +}; + +struct meson_bank meson8_ao_banks[] = { + /* name first last pullen pull dir out in */ + BANK("AO", GPIOAO_0, GPIO_TEST_N, 0, 0, 0, 16, 0, 0, 0, 16, 1, 0), +}; + +struct meson_domain_data meson8_domain_data[] = { + { + .name = "banks", + .banks = meson8_banks, + .pin_names = meson8_pin_names, + .funcs = meson8_functions, + .groups = meson8_groups, + .num_pins = ARRAY_SIZE(meson8_pin_names), + .num_banks = ARRAY_SIZE(meson8_banks), + .num_funcs = ARRAY_SIZE(meson8_functions), + .num_groups = ARRAY_SIZE(meson8_groups), + }, + { + .name = "ao-bank", + .banks = meson8_ao_banks, + .pin_names = meson8_ao_pin_names, + .funcs = meson8_ao_functions, + .groups = meson8_ao_groups, + .num_pins = ARRAY_SIZE(meson8_ao_pin_names), + .num_banks = ARRAY_SIZE(meson8_ao_banks), + .num_funcs = ARRAY_SIZE(meson8_ao_functions), + .num_groups = ARRAY_SIZE(meson8_ao_groups), + }, + { }, +}; + diff --git a/include/dt-bindings/gpio/meson8-gpio.h b/include/dt-bindings/gpio/meson8-gpio.h new file mode 100644 index 0000000..584dd92 --- /dev/null +++ b/include/dt-bindings/gpio/meson8-gpio.h @@ -0,0 +1,155 @@ +/* + * GPIO definitions for Amlogic Meson8 + * + * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef _DT_BINDINGS_MESON8_GPIO_H +#define _DT_BINDINGS_MESON8_GPIO_H + +#define GPIOX_0 0 +#define GPIOX_1 1 +#define GPIOX_2 2 +#define GPIOX_3 3 +#define GPIOX_4 4 +#define GPIOX_5 5 +#define GPIOX_6 6 +#define GPIOX_7 7 +#define GPIOX_8 8 +#define GPIOX_9 9 +#define GPIOX_10 10 +#define GPIOX_11 11 +#define GPIOX_12 12 +#define GPIOX_13 13 +#define GPIOX_14 14 +#define GPIOX_15 15 +#define GPIOX_16 16 +#define GPIOX_17 17 +#define GPIOX_18 18 +#define GPIOX_19 19 +#define GPIOX_20 20 +#define GPIOX_21 21 +#define GPIOY_0 22 +#define GPIOY_1 23 +#define GPIOY_2 24 +#define GPIOY_3 25 +#define GPIOY_4 26 +#define GPIOY_5 27 +#define GPIOY_6 28 +#define GPIOY_7 29 +#define GPIOY_8 30 +#define GPIOY_9 31 +#define GPIOY_10 32 +#define GPIOY_11 33 +#define GPIOY_12 34 +#define GPIOY_13 35 +#define GPIOY_14 36 +#define GPIOY_15 37 +#define GPIOY_16 38 +#define GPIODV_0 39 +#define GPIODV_1 40 +#define GPIODV_2 41 +#define GPIODV_3 42 +#define GPIODV_4 43 +#define GPIODV_5 44 +#define GPIODV_6 45 +#define GPIODV_7 46 +#define GPIODV_8 47 +#define GPIODV_9 48 +#define GPIODV_10 49 +#define GPIODV_11 50 +#define GPIODV_12 51 +#define GPIODV_13 52 +#define GPIODV_14 53 +#define GPIODV_15 54 +#define GPIODV_16 55 +#define GPIODV_17 56 +#define GPIODV_18 57 +#define GPIODV_19 58 +#define GPIODV_20 59 +#define GPIODV_21 60 +#define GPIODV_22 61 +#define GPIODV_23 62 +#define GPIODV_24 63 +#define GPIODV_25 64 +#define GPIODV_26 65 +#define GPIODV_27 66 +#define GPIODV_28 67 +#define GPIODV_29 68 +#define GPIOH_0 69 +#define GPIOH_1 70 +#define GPIOH_2 71 +#define GPIOH_3 72 +#define GPIOH_4 73 +#define GPIOH_5 74 +#define GPIOH_6 75 +#define GPIOH_7 76 +#define GPIOH_8 77 +#define GPIOH_9 78 +#define GPIOZ_0 79 +#define GPIOZ_1 80 +#define GPIOZ_2 81 +#define GPIOZ_3 82 +#define GPIOZ_4 83 +#define GPIOZ_5 84 +#define GPIOZ_6 85 +#define GPIOZ_7 86 +#define GPIOZ_8 87 +#define GPIOZ_9 88 +#define GPIOZ_10 89 +#define GPIOZ_11 90 +#define GPIOZ_12 91 +#define GPIOZ_13 92 +#define GPIOZ_14 93 +#define CARD_0 94 +#define CARD_1 95 +#define CARD_2 96 +#define CARD_3 97 +#define CARD_4 98 +#define CARD_5 99 +#define CARD_6 100 +#define BOOT_0 101 +#define BOOT_1 102 +#define BOOT_2 103 +#define BOOT_3 104 +#define BOOT_4 105 +#define BOOT_5 106 +#define BOOT_6 107 +#define BOOT_7 108 +#define BOOT_8 109 +#define BOOT_9 110 +#define BOOT_10 111 +#define BOOT_11 112 +#define BOOT_12 113 +#define BOOT_13 114 +#define BOOT_14 115 +#define BOOT_15 116 +#define BOOT_16 117 +#define BOOT_17 118 +#define BOOT_18 119 + +#define GPIOAO_0 120 +#define GPIOAO_1 121 +#define GPIOAO_2 122 +#define GPIOAO_3 123 +#define GPIOAO_4 124 +#define GPIOAO_5 125 +#define GPIOAO_6 126 +#define GPIOAO_7 127 +#define GPIOAO_8 128 +#define GPIOAO_9 129 +#define GPIOAO_10 130 +#define GPIOAO_11 131 +#define GPIOAO_12 132 +#define GPIOAO_13 133 +#define GPIO_BSD_EN 134 +#define GPIO_TEST_N 135 + +#endif /* _DT_BINDINGS_MESON8_GPIO_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <1412717576-19737-2-git-send-email-b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 1/3] pinctrl: add driver for Amlogic Meson SoCs [not found] ` <1412717576-19737-2-git-send-email-b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2014-10-21 13:39 ` Linus Walleij [not found] ` <CACRpkdZV9Gmx7Oy4QRh4y2MNN=FXbSF2W0cnjbtxkrB4vT2UOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2014-10-21 13:39 UTC (permalink / raw) To: Beniamino Galvani Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann, Russell King, Carlo Caione, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan, Mark Brown On Tue, Oct 7, 2014 at 11:32 PM, Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: Sorry for a quick and brief review, but should be enough for you to proceed to iterate the patch. > This is a driver for the pinmux and GPIO controller available in > Amlogic Meson SoCs. At the moment it only supports Meson8 devices, > however other SoC families like Meson6 and Meson8b (the Cortex-A5 > variant) appears to be similar, with just different sets of banks and > registers. > > GPIO interrupts are not supported at the moment due to lack of > documentation. > > Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > arch/arm/mach-meson/Kconfig | 3 + Please don't mix up driver submission with platform enablement. Put this Kconfig fragment in a separate patch. > +++ b/drivers/pinctrl/meson/pinctrl-meson.c (...) > +static void meson_domain_set_bit(struct meson_domain *domain, > + void __iomem *addr, unsigned int bit, > + unsigned int value) > +{ > + unsigned long flags; > + unsigned int data; > + > + spin_lock_irqsave(&domain->lock, flags); > + data = readl(addr); > + > + if (value) > + data |= BIT(bit); > + else > + data &= ~BIT(bit); > + > + writel(data, addr); > + spin_unlock_irqrestore(&domain->lock, flags); > +} Looks like you are re-implementing mmio regmap. Take a look at devm_regmap_init_mmio() from <linux/regmap.h> > +static int meson_get_pin_reg_and_bit(struct meson_domain *domain, > + unsigned pin, int reg_type, > + unsigned int *reg_num, unsigned int *bit) > +{ > + struct meson_bank *bank; > + int i, found = 0; bool found; > + > + for (i = 0; i < domain->data->num_banks; i++) { > + bank = &domain->data->banks[i]; > + if (pin >= bank->first && pin <= bank->last) { > + found = 1; > + break; > + } > + } > + > + if (!found) > + return 1; Can't you return a negative errorcode like everyone else? > + > + *reg_num = bank->regs[reg_type].reg; > + *bit = bank->regs[reg_type].bit + pin - bank->first; > + > + return 0; > +} This function is weird and could use some kerneldoc explanation to what it does I think. > +static int meson_pmx_request_gpio(struct pinctrl_dev *pcdev, > + struct pinctrl_gpio_range *range, > + unsigned offset) > +{ > + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); > + > + meson_pmx_disable_other_groups(pc, offset, -1); Passing the argument -1 is usually a bit ambiguous. > + > + return 0; > +} > +static inline struct meson_domain *to_meson_domain(struct gpio_chip *chip) > +{ > + return container_of(chip, struct meson_domain, chip); > +} I have a very vague idea what a "meson domain" is, can this be explained in some good place? Like in the struct meson_domain with kerneldoc... > +static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) > +{ > + struct meson_domain *domain = to_meson_domain(chip); > + void __iomem *addr; > + unsigned int bit; > + > + if (meson_gpio_calc_reg_and_bit(domain, chip->base + gpio, REG_IN, > + &addr, &bit)) > + return 0; > + > + return (readl(addr) >> bit) & 1; Do it like this: return !!(readl(addr) & BIT(bit)); > +static int meson_gpio_of_xlate(struct gpio_chip *chip, > + const struct of_phandle_args *gpiospec, > + u32 *flags) > +{ > + unsigned gpio = gpiospec->args[0]; > + > + if (gpio < chip->base || gpio >= chip->base + chip->ngpio) > + return -EINVAL; > + > + if (flags) > + *flags = gpiospec->args[1]; > + > + return gpio - chip->base; > +} Why is this necessary? We want to get rid of all use of chip->base so introducing new users is not nice. The default of_gpio_simple_xlate() should be enough, don't you agree? I guess this is a twocell binding? Else I suggest you alter your bindings to use two cells and be happy with that, as you can have your driver behave like all others. > +static int meson_pinctrl_init_data(struct meson_pinctrl *pc) > +{ > + struct meson_domain_data *data; > + int i, j, pin = 0, func = 0, group = 0; > + > + /* Copy pin definitions from domains to pinctrl */ > + pc->pins = devm_kzalloc(pc->dev, pc->num_pins * > + sizeof(struct pinctrl_pin_desc), GFP_KERNEL); > + if (!pc->pins) > + return -ENOMEM; > + > + for (i = 0, j = 0; i < pc->num_domains; i++) { > + data = pc->domains[i].data; > + for (j = 0; j < data->num_pins; j++) { > + pc->pins[pin].number = pin; > + pc->pins[pin++].name = data->pin_names[j]; > + } > + } This seems a little kludgy. Why can't these domains also simply use struct pinctrl_pin_desc? > + /* Copy group and function definitions from domains to pinctrl */ > + pc->groups = devm_kzalloc(pc->dev, pc->num_groups * > + sizeof(struct meson_pmx_group), GFP_KERNEL); > + pc->funcs = devm_kzalloc(pc->dev, pc->num_funcs * > + sizeof(struct meson_pmx_func), GFP_KERNEL); > + if (!pc->groups || !pc->funcs) > + return -ENOMEM; Again more copying. Why can't we just have one set of this data and only pass pointers? > + for (i = 0; i < pc->num_domains; i++) { > + data = pc->domains[i].data; > + > + for (j = 0; j < data->num_groups; j++) { > + memcpy(&pc->groups[group], &data->groups[j], > + sizeof(struct meson_pmx_group)); > + pc->groups[group++].domain = &pc->domains[i]; > + } > + > + for (j = 0; j < data->num_funcs; j++) { > + memcpy(&pc->funcs[func++], &data->funcs[j], > + sizeof(struct meson_pmx_func)); > + } > + } > + > + /* Count pins in groups */ > + for (i = 0; i < pc->num_groups; i++) { > + for (j = 0; ; j++) { > + if (pc->groups[i].pins[j] == PINS_END) { > + pc->groups[i].num_pins = j; > + break; > + } > + } > + } > + > + /* Count groups in functions */ > + for (i = 0; i < pc->num_funcs; i++) { > + for (j = 0; ; j++) { > + if (!pc->funcs[i].groups[j]) { > + pc->funcs[i].num_groups = j; > + break; > + } > + } > + } All this dynamic code also looks cumbersome to maintain. Why can't static arrays and ARRAY_SIZE() be used throughout instead, just pass around pointers? > +static int meson_gpiolib_register(struct meson_pinctrl *pc) > +{ > + struct meson_domain *domain; > + unsigned int base = 0; > + int i, ret; > + > + for (i = 0; i < pc->num_domains; i++) { > + domain = &pc->domains[i]; > + > + domain->chip.label = domain->data->name; > + domain->chip.dev = pc->dev; > + domain->chip.request = meson_gpio_request; > + domain->chip.free = meson_gpio_free; > + domain->chip.direction_input = meson_gpio_direction_input; > + domain->chip.direction_output = meson_gpio_direction_output; > + domain->chip.get = meson_gpio_get; > + domain->chip.set = meson_gpio_set; > + domain->chip.base = base; > + domain->chip.ngpio = domain->data->num_pins; > + domain->chip.names = domain->data->pin_names; > + domain->chip.can_sleep = false; > + domain->chip.of_node = domain->of_node; > + domain->chip.of_gpio_n_cells = 2; > + domain->chip.of_xlate = meson_gpio_of_xlate; > + > + ret = gpiochip_add(&domain->chip); > + if (ret < 0) { > + dev_err(pc->dev, "can't add gpio chip %s\n", > + domain->data->name); > + goto fail; > + } > + > + domain->gpio_range.name = domain->data->name; > + domain->gpio_range.id = i; > + domain->gpio_range.base = base; > + domain->gpio_range.pin_base = base; > + domain->gpio_range.npins = domain->data->num_pins; > + domain->gpio_range.gc = &domain->chip; > + pinctrl_add_gpio_range(pc->pcdev, &domain->gpio_range); No thanks, use gpiochip_add_pin_range() instead. That is much better as it's completely relative. (...) > +++ b/drivers/pinctrl/meson/pinctrl-meson.h > +/** > + * struct meson bank > + * > + * @name: bank name > + * @first: first pin of the bank > + * @last: last pin of the bank > + * @regs: couples of <reg offset, bit index> controlling the > + * functionalities of the bank pins (pull, direction, value) > + * > + * A bank represents a set of pins controlled by a contiguous set of > + * bits in the domain registers. > + */ > +struct meson_bank { > + const char *name; > + unsigned int first; > + unsigned int last; > + struct meson_reg_offset regs[NUM_REG]; > +}; That struct is actually documented! > +/** > + * struct meson_domain > + * > + * @reg_mux: registers for mux settings > + * @reg_pullen: registers for pull-enable settings > + * @reg_pull: registers for pull settings > + * @reg_gpio: registers for gpio settings > + * @mux_size: size of mux register range (in words) > + * @pullen_size:size of pull-enable register range > + * @pull_size: size of pull register range > + * @gpio_size: size of gpio register range > + * @chip: gpio chip associated with the domain > + * @data; platform data for the domain > + * @node: device tree node for the domain > + * @gpio_range: gpio range that maps domain gpios to the pin controller > + * @lock: spinlock for accessing domain registers > + * > + * A domain represents a set of banks controlled by the same set of > + * registers. Typically there is a domain for the normal banks and > + * another one for the Always-On bus. > + */ Can I get a long-ish explanation of the domains vs banks etc because that's really key to understanding this driver! Some example or something. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CACRpkdZV9Gmx7Oy4QRh4y2MNN=FXbSF2W0cnjbtxkrB4vT2UOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/3] pinctrl: add driver for Amlogic Meson SoCs [not found] ` <CACRpkdZV9Gmx7Oy4QRh4y2MNN=FXbSF2W0cnjbtxkrB4vT2UOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2014-10-22 20:06 ` Beniamino Galvani [not found] ` <20141022200633.GA31080-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Beniamino Galvani @ 2014-10-22 20:06 UTC (permalink / raw) To: Linus Walleij Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann, Russell King, Carlo Caione, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan, Mark Brown On Tue, Oct 21, 2014 at 03:39:25PM +0200, Linus Walleij wrote: > On Tue, Oct 7, 2014 at 11:32 PM, Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > Sorry for a quick and brief review, but should be enough for you to proceed > to iterate the patch. > > > This is a driver for the pinmux and GPIO controller available in > > Amlogic Meson SoCs. At the moment it only supports Meson8 devices, > > however other SoC families like Meson6 and Meson8b (the Cortex-A5 > > variant) appears to be similar, with just different sets of banks and > > registers. > > > > GPIO interrupts are not supported at the moment due to lack of > > documentation. > > > > Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > arch/arm/mach-meson/Kconfig | 3 + > > Please don't mix up driver submission with platform enablement. > Put this Kconfig fragment in a separate patch. Ok, will do. > > > +++ b/drivers/pinctrl/meson/pinctrl-meson.c > (...) > > > +static void meson_domain_set_bit(struct meson_domain *domain, > > + void __iomem *addr, unsigned int bit, > > + unsigned int value) > > +{ > > + unsigned long flags; > > + unsigned int data; > > + > > + spin_lock_irqsave(&domain->lock, flags); > > + data = readl(addr); > > + > > + if (value) > > + data |= BIT(bit); > > + else > > + data &= ~BIT(bit); > > + > > + writel(data, addr); > > + spin_unlock_irqrestore(&domain->lock, flags); > > +} > > Looks like you are re-implementing mmio regmap. Take a look at > devm_regmap_init_mmio() from <linux/regmap.h> I missed that, thanks. > > +static int meson_get_pin_reg_and_bit(struct meson_domain *domain, > > + unsigned pin, int reg_type, > > + unsigned int *reg_num, unsigned int *bit) > > +{ > > + struct meson_bank *bank; > > + int i, found = 0; > > bool found; > > > + > > + for (i = 0; i < domain->data->num_banks; i++) { > > + bank = &domain->data->banks[i]; > > + if (pin >= bank->first && pin <= bank->last) { > > + found = 1; > > + break; > > + } > > + } > > + > > + if (!found) > > + return 1; > > Can't you return a negative errorcode like everyone else? Sure. > > > + > > + *reg_num = bank->regs[reg_type].reg; > > + *bit = bank->regs[reg_type].bit + pin - bank->first; > > + > > + return 0; > > +} > > This function is weird and could use some kerneldoc explanation > to what it does I think. Ok. > > > +static int meson_pmx_request_gpio(struct pinctrl_dev *pcdev, > > + struct pinctrl_gpio_range *range, > > + unsigned offset) > > +{ > > + struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); > > + > > + meson_pmx_disable_other_groups(pc, offset, -1); > > Passing the argument -1 is usually a bit ambiguous. > > > + > > + return 0; > > +} > > > +static inline struct meson_domain *to_meson_domain(struct gpio_chip *chip) > > +{ > > + return container_of(chip, struct meson_domain, chip); > > +} > > I have a very vague idea what a "meson domain" is, can this be explained > in some good place? Like in the struct meson_domain with > kerneldoc... Yes, below there is an explanation. > > > +static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) > > +{ > > + struct meson_domain *domain = to_meson_domain(chip); > > + void __iomem *addr; > > + unsigned int bit; > > + > > + if (meson_gpio_calc_reg_and_bit(domain, chip->base + gpio, REG_IN, > > + &addr, &bit)) > > + return 0; > > + > > + return (readl(addr) >> bit) & 1; > > Do it like this: > > return !!(readl(addr) & BIT(bit)); > > > +static int meson_gpio_of_xlate(struct gpio_chip *chip, > > + const struct of_phandle_args *gpiospec, > > + u32 *flags) > > +{ > > + unsigned gpio = gpiospec->args[0]; > > + > > + if (gpio < chip->base || gpio >= chip->base + chip->ngpio) > > + return -EINVAL; > > + > > + if (flags) > > + *flags = gpiospec->args[1]; > > + > > + return gpio - chip->base; > > +} > > Why is this necessary? We want to get rid of all use of > chip->base so introducing new users is not nice. The driver instantiates one pinctrl device with 136 pins and two gpio_chips, one with 120 gpios and the other with 16 (for more details, see below). The macros for pins in the header file use a single range from 0 to 135 that matches the order of pins in the pinctrl device: /* First gpio-chip */ #define GPIOX_0 0 [...] #define BOOT_18 119 /* Second gpio-chip */ #define GPIOAO_0 120 [...] #define GPIO_TEST_N 135 Since the macros are also used in DT as GPIO numbers, this function translates that value to the relative offset for the gpio chip. > The default of_gpio_simple_xlate() should be enough, > don't you agree? Probably yes, but for it to work then I have either to: - change the pin macros to use a relative value /* First gpio-chip */ #define GPIOX_0 0 [...] #define BOOT_18 119 /* Second gpio-chip */ #define GPIOAO_0 0 [...] #define GPIO_TEST_N 15 - or use a single gpio chip > > I guess this is a twocell binding? Else I suggest you alter your > bindings to use two cells and be happy with that, as you can > have your driver behave like all others. It's a two-cell binding. > > > +static int meson_pinctrl_init_data(struct meson_pinctrl *pc) > > +{ > > + struct meson_domain_data *data; > > + int i, j, pin = 0, func = 0, group = 0; > > + > > + /* Copy pin definitions from domains to pinctrl */ > > + pc->pins = devm_kzalloc(pc->dev, pc->num_pins * > > + sizeof(struct pinctrl_pin_desc), GFP_KERNEL); > > + if (!pc->pins) > > + return -ENOMEM; > > + > > + for (i = 0, j = 0; i < pc->num_domains; i++) { > > + data = pc->domains[i].data; > > + for (j = 0; j < data->num_pins; j++) { > > + pc->pins[pin].number = pin; > > + pc->pins[pin++].name = data->pin_names[j]; > > + } > > + } > > This seems a little kludgy. Why can't these domains also simply > use struct pinctrl_pin_desc? I will fix this. > > > + /* Copy group and function definitions from domains to pinctrl */ > > + pc->groups = devm_kzalloc(pc->dev, pc->num_groups * > > + sizeof(struct meson_pmx_group), GFP_KERNEL); > > + pc->funcs = devm_kzalloc(pc->dev, pc->num_funcs * > > + sizeof(struct meson_pmx_func), GFP_KERNEL); > > + if (!pc->groups || !pc->funcs) > > + return -ENOMEM; > > Again more copying. Why can't we just have one set of this data > and only pass pointers? This code copies information on groups and functions from the different domains and merge it in a single array so that the driver can look it up easily. Probably the initial information should not be splitted so that it can be reused without additional copies. > > > + for (i = 0; i < pc->num_domains; i++) { > > + data = pc->domains[i].data; > > + > > + for (j = 0; j < data->num_groups; j++) { > > + memcpy(&pc->groups[group], &data->groups[j], > > + sizeof(struct meson_pmx_group)); > > + pc->groups[group++].domain = &pc->domains[i]; > > + } > > + > > + for (j = 0; j < data->num_funcs; j++) { > > + memcpy(&pc->funcs[func++], &data->funcs[j], > > + sizeof(struct meson_pmx_func)); > > + } > > + } > > + > > + /* Count pins in groups */ > > + for (i = 0; i < pc->num_groups; i++) { > > + for (j = 0; ; j++) { > > + if (pc->groups[i].pins[j] == PINS_END) { > > + pc->groups[i].num_pins = j; > > + break; > > + } > > + } > > + } > > + > > + /* Count groups in functions */ > > + for (i = 0; i < pc->num_funcs; i++) { > > + for (j = 0; ; j++) { > > + if (!pc->funcs[i].groups[j]) { > > + pc->funcs[i].num_groups = j; > > + break; > > + } > > + } > > + } > > All this dynamic code also looks cumbersome to maintain. > > Why can't static arrays and ARRAY_SIZE() be used throughout > instead, just pass around pointers? The goal of the dynamic code is to simplify the declaration of groups, which can be done with a single statement in the form: GROUP(_name, _reg, _bit, _pins...) for example: GROUP(sdxc_d0_c, 4, 30, BOOT_0), GROUP(sdxc_d13_c, 4, 29, BOOT_1, BOOT_2, BOOT_3), GROUP(sdxc_d47_c, 4, 28, BOOT_4, BOOT_5, BOOT_6, BOOT_7), GROUP(sdxc_cmd_c, 4, 27, BOOT_16), GROUP(sdxc_clk_c, 4, 26, BOOT_17), GROUP(nand_io, 2, 26, BOOT_0, BOOT_1, BOOT_2, BOOT_3, BOOT_4, BOOT_5, BOOT_6, BOOT_7), instead of having to define an array of pins and reference it in the group declaration. The same goes for functions. I admit that this is a bit hackish, I will stick to the classical way in the next respin. > > > +static int meson_gpiolib_register(struct meson_pinctrl *pc) > > +{ > > + struct meson_domain *domain; > > + unsigned int base = 0; > > + int i, ret; > > + > > + for (i = 0; i < pc->num_domains; i++) { > > + domain = &pc->domains[i]; > > + > > + domain->chip.label = domain->data->name; > > + domain->chip.dev = pc->dev; > > + domain->chip.request = meson_gpio_request; > > + domain->chip.free = meson_gpio_free; > > + domain->chip.direction_input = meson_gpio_direction_input; > > + domain->chip.direction_output = meson_gpio_direction_output; > > + domain->chip.get = meson_gpio_get; > > + domain->chip.set = meson_gpio_set; > > + domain->chip.base = base; > > + domain->chip.ngpio = domain->data->num_pins; > > + domain->chip.names = domain->data->pin_names; > > + domain->chip.can_sleep = false; > > + domain->chip.of_node = domain->of_node; > > + domain->chip.of_gpio_n_cells = 2; > > + domain->chip.of_xlate = meson_gpio_of_xlate; > > + > > + ret = gpiochip_add(&domain->chip); > > + if (ret < 0) { > > + dev_err(pc->dev, "can't add gpio chip %s\n", > > + domain->data->name); > > + goto fail; > > + } > > + > > + domain->gpio_range.name = domain->data->name; > > + domain->gpio_range.id = i; > > + domain->gpio_range.base = base; > > + domain->gpio_range.pin_base = base; > > + domain->gpio_range.npins = domain->data->num_pins; > > + domain->gpio_range.gc = &domain->chip; > > + pinctrl_add_gpio_range(pc->pcdev, &domain->gpio_range); > > No thanks, use gpiochip_add_pin_range() instead. That is much > better as it's completely relative. Ok. > > (...) > > +++ b/drivers/pinctrl/meson/pinctrl-meson.h > > +/** > > + * struct meson bank > > + * > > + * @name: bank name > > + * @first: first pin of the bank > > + * @last: last pin of the bank > > + * @regs: couples of <reg offset, bit index> controlling the > > + * functionalities of the bank pins (pull, direction, value) > > + * > > + * A bank represents a set of pins controlled by a contiguous set of > > + * bits in the domain registers. > > + */ > > +struct meson_bank { > > + const char *name; > > + unsigned int first; > > + unsigned int last; > > + struct meson_reg_offset regs[NUM_REG]; > > +}; > > That struct is actually documented! > > > +/** > > + * struct meson_domain > > + * > > + * @reg_mux: registers for mux settings > > + * @reg_pullen: registers for pull-enable settings > > + * @reg_pull: registers for pull settings > > + * @reg_gpio: registers for gpio settings > > + * @mux_size: size of mux register range (in words) > > + * @pullen_size:size of pull-enable register range > > + * @pull_size: size of pull register range > > + * @gpio_size: size of gpio register range > > + * @chip: gpio chip associated with the domain > > + * @data; platform data for the domain > > + * @node: device tree node for the domain > > + * @gpio_range: gpio range that maps domain gpios to the pin controller > > + * @lock: spinlock for accessing domain registers > > + * > > + * A domain represents a set of banks controlled by the same set of > > + * registers. Typically there is a domain for the normal banks and > > + * another one for the Always-On bus. > > + */ > > Can I get a long-ish explanation of the domains vs banks etc > because that's really key to understanding this driver! Yes, I hope that the following explanation is clear enough. If anyone wants to know the details, the relevant information can be found in the Amlogic sources available at: http://openlinux.amlogic.com:8000/download/ARM/kernel/ The GPIOs are organized in banks (X,Y,DV,H,Z,BOOT,CARD,AO) and each bank has a number of GPIOs variable from 7 to 30. There are different register sets for changing pins properties: - for all banks except AO: <0xc11080b0 0x28> for mux configuration <0xc11080e4 0x18> for pull-enable configuration <0xc1108120 0x18> for pull configuration <0xc1108030 0x30> for gpio (in/out/dir) configuration - for AO bank <0xc8100014 0x4> for mux configuration <0xc810002c 0x4> for pull and pull-enable configuration <0xc8100024 0x8> for gpio (in/out/dir) configuration The regular banks belong to the "standard" power domain, while AO belongs to the Always-On power domain, which, like the name says, can't be powered off. Each bank uses contiguous ranges of bits in the register sets described above and the same register can be used by different banks (for example, for the pull-enable configuration the BOOT bank uses bits [0:18] of 0xc11080f0 and the CARD bank uses bits [20:25]). In addition to this, there seem to be some other registers, shared between all the banks, for the interrupt functionality. My initial submission refers to these two group of banks as "domains" (for lack of a better name) and uses a separate node in the DT for each of them: pinctrl@ { compatible = "amlogic,meson8-pinctrl"; [...] gpio: banks@c11080b0 { reg = <0xc11080b0 0x28>, <0xc11080e4 0x18>, <0xc1108120 0x18>, <0xc1108030 0x30>; reg-names = "mux", "pull-enable", "pull", "gpio"; gpio-controller; #gpio-cells = <2>; }; gpio_ao: ao-bank@c1108030 { reg = <0xc8100014 0x4>, <0xc810002c 0x4>, <0xc8100024 0x8>; reg-names = "mux", "pull", "gpio"; gpio-controller; #gpio-cells = <2>; }; The driver then instantiates a gpio_chip for each subnode and a single pinctrl device. Anyway, I'll document better the driver in the next submission. Thanks for the review! Beniamino -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20141022200633.GA31080-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 1/3] pinctrl: add driver for Amlogic Meson SoCs [not found] ` <20141022200633.GA31080-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2014-10-28 14:11 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2014-10-28 14:11 UTC (permalink / raw) To: Beniamino Galvani Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann, Russell King, Carlo Caione, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan, Mark Brown On Wed, Oct 22, 2014 at 10:06 PM, Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Tue, Oct 21, 2014 at 03:39:25PM +0200, Linus Walleij wrote: >> > +static int meson_gpio_of_xlate(struct gpio_chip *chip, >> > + const struct of_phandle_args *gpiospec, >> > + u32 *flags) >> > +{ >> > + unsigned gpio = gpiospec->args[0]; >> > + >> > + if (gpio < chip->base || gpio >= chip->base + chip->ngpio) >> > + return -EINVAL; >> > + >> > + if (flags) >> > + *flags = gpiospec->args[1]; >> > + >> > + return gpio - chip->base; >> > +} >> >> Why is this necessary? We want to get rid of all use of >> chip->base so introducing new users is not nice. > > The driver instantiates one pinctrl device with 136 pins and two > gpio_chips, one with 120 gpios and the other with 16 (for more > details, see below). The macros for pins in the header file use a > single range from 0 to 135 that matches the order of pins in the > pinctrl device: > > /* First gpio-chip */ > #define GPIOX_0 0 > [...] > #define BOOT_18 119 > > /* Second gpio-chip */ > #define GPIOAO_0 120 > [...] > #define GPIO_TEST_N 135 > > Since the macros are also used in DT as GPIO numbers, this function > translates that value to the relative offset for the gpio chip. That is not wise. You should let each gpio_chip register its chip with base = -1, so they get whatever random GPIO numbers they get, then register the pin range relatively to what they get from the gpiochip side using gpiochip_add_pin_range(). >> The default of_gpio_simple_xlate() should be enough, >> don't you agree? > > Probably yes, but for it to work then I have either to: > - change the pin macros to use a relative value > > /* First gpio-chip */ > #define GPIOX_0 0 > [...] > #define BOOT_18 119 > > /* Second gpio-chip */ > #define GPIOAO_0 0 > [...] > #define GPIO_TEST_N 15 > > - or use a single gpio chip Or (C) register the pin ranges from the gpio_chip side instead of doing it from the pin controller side. Look at how gpiochip_add_pin_range() works, I inisist. >> > + /* Copy group and function definitions from domains to pinctrl */ >> > + pc->groups = devm_kzalloc(pc->dev, pc->num_groups * >> > + sizeof(struct meson_pmx_group), GFP_KERNEL); >> > + pc->funcs = devm_kzalloc(pc->dev, pc->num_funcs * >> > + sizeof(struct meson_pmx_func), GFP_KERNEL); >> > + if (!pc->groups || !pc->funcs) >> > + return -ENOMEM; >> >> Again more copying. Why can't we just have one set of this data >> and only pass pointers? > > This code copies information on groups and functions from the > different domains and merge it in a single array so that the driver > can look it up easily. > > Probably the initial information should not be splitted so that it can > be reused without additional copies. Yeah I will look at V2 and see how it looks... >> Why can't static arrays and ARRAY_SIZE() be used throughout >> instead, just pass around pointers? > > The goal of the dynamic code is to simplify the declaration of groups, > which can be done with a single statement in the form: > > GROUP(_name, _reg, _bit, _pins...) > > for example: > > GROUP(sdxc_d0_c, 4, 30, BOOT_0), > GROUP(sdxc_d13_c, 4, 29, BOOT_1, BOOT_2, BOOT_3), > GROUP(sdxc_d47_c, 4, 28, BOOT_4, BOOT_5, BOOT_6, BOOT_7), > GROUP(sdxc_cmd_c, 4, 27, BOOT_16), > GROUP(sdxc_clk_c, 4, 26, BOOT_17), > GROUP(nand_io, 2, 26, BOOT_0, BOOT_1, BOOT_2, BOOT_3, > BOOT_4, BOOT_5, BOOT_6, BOOT_7), > > instead of having to define an array of pins and reference it in the > group declaration. The same goes for functions. I admit that this is a > bit hackish, I will stick to the classical way in the next respin. OK thanks. >> > +/** >> > + * struct meson_domain >> > + * >> > + * @reg_mux: registers for mux settings >> > + * @reg_pullen: registers for pull-enable settings >> > + * @reg_pull: registers for pull settings >> > + * @reg_gpio: registers for gpio settings >> > + * @mux_size: size of mux register range (in words) >> > + * @pullen_size:size of pull-enable register range >> > + * @pull_size: size of pull register range >> > + * @gpio_size: size of gpio register range >> > + * @chip: gpio chip associated with the domain >> > + * @data; platform data for the domain >> > + * @node: device tree node for the domain >> > + * @gpio_range: gpio range that maps domain gpios to the pin controller >> > + * @lock: spinlock for accessing domain registers >> > + * >> > + * A domain represents a set of banks controlled by the same set of >> > + * registers. Typically there is a domain for the normal banks and >> > + * another one for the Always-On bus. >> > + */ >> >> Can I get a long-ish explanation of the domains vs banks etc >> because that's really key to understanding this driver! > > Yes, I hope that the following explanation is clear enough. If anyone > wants to know the details, the relevant information can be found in > the Amlogic sources available at: > > http://openlinux.amlogic.com:8000/download/ARM/kernel/ > > The GPIOs are organized in banks (X,Y,DV,H,Z,BOOT,CARD,AO) and each > bank has a number of GPIOs variable from 7 to 30. > > There are different register sets for changing pins properties: > - for all banks except AO: > <0xc11080b0 0x28> for mux configuration > <0xc11080e4 0x18> for pull-enable configuration > <0xc1108120 0x18> for pull configuration > <0xc1108030 0x30> for gpio (in/out/dir) configuration > > - for AO bank > <0xc8100014 0x4> for mux configuration > <0xc810002c 0x4> for pull and pull-enable configuration > <0xc8100024 0x8> for gpio (in/out/dir) configuration > > The regular banks belong to the "standard" power domain, while AO > belongs to the Always-On power domain, which, like the name says, > can't be powered off. > > Each bank uses contiguous ranges of bits in the register sets > described above and the same register can be used by different banks > (for example, for the pull-enable configuration the BOOT bank uses > bits [0:18] of 0xc11080f0 and the CARD bank uses bits [20:25]). > > In addition to this, there seem to be some other registers, shared > between all the banks, for the interrupt functionality. I get it now I think, thanks! Arguably the whole shebang is one big hardware unit so the right thing is indeed to have a single driver for all of it. > The driver then instantiates a gpio_chip for each subnode and a single > pinctrl device. This is the right design. We just need to hash out the details. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] pinctrl: meson: add device tree bindings documentation 2014-10-07 21:32 [PATCH 0/3] Pinctrl driver for Amlogic Meson SoCs Beniamino Galvani 2014-10-07 21:32 ` [PATCH 1/3] pinctrl: add " Beniamino Galvani @ 2014-10-07 21:32 ` Beniamino Galvani 2014-10-24 11:53 ` Linus Walleij 2014-10-07 21:32 ` [PATCH 3/3] ARM: dts: meson8: add pinctrl and gpio nodes Beniamino Galvani 2 siblings, 1 reply; 10+ messages in thread From: Beniamino Galvani @ 2014-10-07 21:32 UTC (permalink / raw) To: Linus Walleij Cc: linux-arm-kernel, linux-kernel, Arnd Bergmann, Russell King, Carlo Caione, devicetree, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan, Beniamino Galvani Add device tree bindings documentation for Amlogic Meson pinmux and GPIO controller. Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> --- .../devicetree/bindings/pinctrl/meson,pinctrl.txt | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt new file mode 100644 index 0000000..dd573d9 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt @@ -0,0 +1,80 @@ +* Amlogic Meson pinmux controller + +Pins are organized in banks; all banks except AO are controlled by the +same set of registers, while the AO bank uses a dedicated register +range. The device tree uses sub-nodes to represent set of banks which +share the same address space. + +Required properties for the root node: + - compatible: "amlogic,meson8-pinctrl" + - reg: address and size of the common registers controlling gpio irq + functionality + +Required properties for gpio sub-nodes: + - reg: should contain address and size for mux, pull-enable, pull and + gpio register sets + - reg-names: an array of strings describing the "reg" entries. Must + contain "mux", "pull" and "gpio". "pull-enable" is optional and + when it is missing the "pull" registers are used instead + - gpio-controller: identifies the node as a gpio controller + - #gpio-cells: must be 2 + +Valid gpio sub-nodes name are: + - "banks" for the standard banks + - "ao-bank" for the AO bank which belong to the special always-on + power domain + +Required properties for configuration nodes: + - pins: the name of a pin group. The list of all available groups can + be found in driver sources. + - function: the name of a function to activate for the specified set + of groups. The list of all available functions can be found in + driver sources. + +Example: + + pinctrl: pinctrl@c1109880 { + compatible = "amlogic,meson8-pinctrl"; + reg = <0xc1109880 0x10>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gpio: banks@c11080b0 { + reg = <0xc11080b0 0x28>, + <0xc11080e4 0x18>, + <0xc1108120 0x18>, + <0xc1108030 0x30>; + reg-names = "mux", "pull-enable", "pull", "gpio"; + gpio-controller; + #gpio-cells = <2>; + }; + + gpio_ao: ao-bank@c1108030 { + reg = <0xc8100014 0x4>, + <0xc810002c 0x4>, + <0xc8100024 0x8>; + reg-names = "mux", "pull", "gpio"; + gpio-controller; + #gpio-cells = <2>; + }; + + nand { + nand { + pins = "nand_io", "nand_io_ce0", "nand_io_ce1", + "nand_io_rb0", "nand_ale", "nand_cle", + "nand_wen_clk", "nand_ren_clk", "nand_dqs", + "nand_ce2", "nand_ce3"; + function = "nand"; + }; + }; + + uart_ao_a: uart_ao_a { + uart_ao_a { + pins = "uart_tx_ao_a", "uart_rx_ao_a"; + "uart_cts_ao_a", "uart_rts_ao_a"; + function = "uart_ao"; + }; + }; + }; + -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: meson: add device tree bindings documentation 2014-10-07 21:32 ` [PATCH 2/3] pinctrl: meson: add device tree bindings documentation Beniamino Galvani @ 2014-10-24 11:53 ` Linus Walleij 2014-10-26 18:25 ` Beniamino Galvani 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2014-10-24 11:53 UTC (permalink / raw) To: Beniamino Galvani Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Arnd Bergmann, Russell King, Carlo Caione, devicetree@vger.kernel.org, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan On Tue, Oct 7, 2014 at 11:32 PM, Beniamino Galvani <b.galvani@gmail.com> wrote: > Add device tree bindings documentation for Amlogic Meson pinmux and > GPIO controller. > > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> (...) > +Required properties for gpio sub-nodes: > + - reg: should contain address and size for mux, pull-enable, pull and > + gpio register sets > + - reg-names: an array of strings describing the "reg" entries. Must > + contain "mux", "pull" and "gpio". "pull-enable" is optional and > + when it is missing the "pull" registers are used instead So it seems segmenting the registers is done to sort of control the hardware versioning. I think it's better to use the compatible string to indicate different versions of the hardware and then have just have one big regs to cover all registers. > +Valid gpio sub-nodes name are: > + - "banks" for the standard banks > + - "ao-bank" for the AO bank which belong to the special always-on > + power domain I think it's unnecessary to split up banks, the compatible property should be enough to know how many banks this controller has and where they are located in relation to the base offset. > +Required properties for configuration nodes: > + - pins: the name of a pin group. The list of all available groups can > + be found in driver sources. > + - function: the name of a function to activate for the specified set > + of groups. The list of all available functions can be found in > + driver sources. This is interesting. I have established that for controllers mapping functions to groups we use "function" and "groups". So for per-pin configuration, "function" and "pins" would be apropriate. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: meson: add device tree bindings documentation 2014-10-24 11:53 ` Linus Walleij @ 2014-10-26 18:25 ` Beniamino Galvani 2014-10-28 14:12 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Beniamino Galvani @ 2014-10-26 18:25 UTC (permalink / raw) To: Linus Walleij Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Arnd Bergmann, Russell King, Carlo Caione, devicetree@vger.kernel.org, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan On Fri, Oct 24, 2014 at 01:53:28PM +0200, Linus Walleij wrote: > On Tue, Oct 7, 2014 at 11:32 PM, Beniamino Galvani <b.galvani@gmail.com> wrote: > > > Add device tree bindings documentation for Amlogic Meson pinmux and > > GPIO controller. > > > > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> > (...) > > +Required properties for gpio sub-nodes: > > + - reg: should contain address and size for mux, pull-enable, pull and > > + gpio register sets > > + - reg-names: an array of strings describing the "reg" entries. Must > > + contain "mux", "pull" and "gpio". "pull-enable" is optional and > > + when it is missing the "pull" registers are used instead > > So it seems segmenting the registers is done to sort of control the > hardware versioning. > > I think it's better to use the compatible string to indicate different > versions of the hardware and then have just have one big > regs to cover all registers. The problem here is that the register ranges are not contiguous and the holes in between are used by other devices, so I can't use a single range. > > > +Valid gpio sub-nodes name are: > > + - "banks" for the standard banks > > + - "ao-bank" for the AO bank which belong to the special always-on > > + power domain > > I think it's unnecessary to split up banks, the compatible property > should be enough to know how many banks this controller has > and where they are located in relation to the base offset. I wanted to avoid a reg property with a list of 7 ranges. Anyway, I agree that the split seems a bit arbitrary; I'll remove it. > > +Required properties for configuration nodes: > > + - pins: the name of a pin group. The list of all available groups can > > + be found in driver sources. > > + - function: the name of a function to activate for the specified set > > + of groups. The list of all available functions can be found in > > + driver sources. > > This is interesting. I have established that for controllers mapping > functions to groups we use > "function" and "groups". > > So for per-pin configuration, "function" and "pins" would be > apropriate. I will use "groups" instead of "pins" for the pinmux configuration. Thanks! Beniamino ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: meson: add device tree bindings documentation 2014-10-26 18:25 ` Beniamino Galvani @ 2014-10-28 14:12 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2014-10-28 14:12 UTC (permalink / raw) To: Beniamino Galvani Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Arnd Bergmann, Russell King, Carlo Caione, devicetree@vger.kernel.org, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan On Sun, Oct 26, 2014 at 7:25 PM, Beniamino Galvani <b.galvani@gmail.com> wrote: > On Fri, Oct 24, 2014 at 01:53:28PM +0200, Linus Walleij wrote: >> >> I think it's better to use the compatible string to indicate different >> versions of the hardware and then have just have one big >> regs to cover all registers. > > The problem here is that the register ranges are not contiguous and > the holes in between are used by other devices, so I can't use a > single range. OK I get it. >> So for per-pin configuration, "function" and "pins" would be >> apropriate. > > I will use "groups" instead of "pins" for the pinmux configuration. Excellent! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] ARM: dts: meson8: add pinctrl and gpio nodes 2014-10-07 21:32 [PATCH 0/3] Pinctrl driver for Amlogic Meson SoCs Beniamino Galvani 2014-10-07 21:32 ` [PATCH 1/3] pinctrl: add " Beniamino Galvani 2014-10-07 21:32 ` [PATCH 2/3] pinctrl: meson: add device tree bindings documentation Beniamino Galvani @ 2014-10-07 21:32 ` Beniamino Galvani 2 siblings, 0 replies; 10+ messages in thread From: Beniamino Galvani @ 2014-10-07 21:32 UTC (permalink / raw) To: Linus Walleij Cc: linux-arm-kernel, linux-kernel, Arnd Bergmann, Russell King, Carlo Caione, devicetree, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Jerry Cao, Victor Wan, Beniamino Galvani Add pinctrl node to meson8.dtsi and gpio-leds node to meson8-vega-s89e.dts Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> --- arch/arm/boot/dts/meson8-vega-s89e.dts | 16 +++++++++++++++- arch/arm/boot/dts/meson8.dtsi | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/meson8-vega-s89e.dts b/arch/arm/boot/dts/meson8-vega-s89e.dts index 950998f..70a05c1 100644 --- a/arch/arm/boot/dts/meson8-vega-s89e.dts +++ b/arch/arm/boot/dts/meson8-vega-s89e.dts @@ -45,7 +45,9 @@ /dts-v1/; -/include/ "meson8.dtsi" +#include "meson8.dtsi" +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/gpio/meson8-gpio.h> / { model = "Tronsmart Vega S89 Elite"; @@ -58,8 +60,20 @@ memory { reg = <0x40000000 0x80000000>; }; + + gpio-leds { + compatible = "gpio-leds"; + + power { + gpios = <&gpio_ao GPIO_TEST_N GPIO_ACTIVE_LOW>; + linux,default-trigger = "none"; + }; + }; + }; &uart_AO { status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&uart_ao_a>; }; diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi index 1f442a7..59c3af0 100644 --- a/arch/arm/boot/dts/meson8.dtsi +++ b/arch/arm/boot/dts/meson8.dtsi @@ -89,4 +89,39 @@ compatible = "fixed-clock"; clock-frequency = <141666666>; }; + + pinctrl: pinctrl@c1109880 { + compatible = "amlogic,meson8-pinctrl"; + reg = <0xc1109880 0x10>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gpio: banks@c11080b0 { + reg = <0xc11080b0 0x28>, + <0xc11080e4 0x18>, + <0xc1108120 0x18>, + <0xc1108030 0x30>; + reg-names = "mux", "pull-enable", "pull", "gpio"; + gpio-controller; + #gpio-cells = <2>; + }; + + gpio_ao: ao-bank@c1108030 { + reg = <0xc8100014 0x4>, + <0xc810002c 0x4>, + <0xc8100024 0x8>; + reg-names = "mux", "pull", "gpio"; + gpio-controller; + #gpio-cells = <2>; + }; + + uart_ao_a: uart_ao_a { + uart_ao_a { + pins = "uart_tx_ao_a", "uart_rx_ao_a"; + function = "uart_ao"; + }; + }; + }; + }; /* end of / */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-10-28 14:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-07 21:32 [PATCH 0/3] Pinctrl driver for Amlogic Meson SoCs Beniamino Galvani
2014-10-07 21:32 ` [PATCH 1/3] pinctrl: add " Beniamino Galvani
[not found] ` <1412717576-19737-2-git-send-email-b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-10-21 13:39 ` Linus Walleij
[not found] ` <CACRpkdZV9Gmx7Oy4QRh4y2MNN=FXbSF2W0cnjbtxkrB4vT2UOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-22 20:06 ` Beniamino Galvani
[not found] ` <20141022200633.GA31080-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-10-28 14:11 ` Linus Walleij
2014-10-07 21:32 ` [PATCH 2/3] pinctrl: meson: add device tree bindings documentation Beniamino Galvani
2014-10-24 11:53 ` Linus Walleij
2014-10-26 18:25 ` Beniamino Galvani
2014-10-28 14:12 ` Linus Walleij
2014-10-07 21:32 ` [PATCH 3/3] ARM: dts: meson8: add pinctrl and gpio nodes Beniamino Galvani
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).