Re: [PATCH v2 02/12] pinctrl: add a pincontrol driver for BCM6328
From: Michael Walle <hidden>
Date: 2021-03-02 22:37:47
Also in:
linux-devicetree, lkml
Am 2021-03-02 20:16, schrieb Álvaro Fernández Rojas:
quoted hunk ↗ jump to hunk
Add a pincontrol driver for BCM6328. BCM628 supports muxing 32 pins as GPIOs, as LEDs for the integrated LED controller, or various other functions. Its pincontrol mux registers also control other aspects, like switching the second USB port between host and device mode. Signed-off-by: Álvaro Fernández Rojas <redacted> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- v2: switch to GPIO_REGMAP drivers/pinctrl/bcm/Kconfig | 13 + drivers/pinctrl/bcm/Makefile | 1 + drivers/pinctrl/bcm/pinctrl-bcm6328.c | 481 ++++++++++++++++++++++++++ 3 files changed, 495 insertions(+) create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6328.cdiff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig index 0ed14de0134c..76728f097c25 100644 --- a/drivers/pinctrl/bcm/Kconfig +++ b/drivers/pinctrl/bcm/Kconfig@@ -29,6 +29,19 @@ config PINCTRL_BCM2835 help Say Y here to enable the Broadcom BCM2835 GPIO driver. +config PINCTRL_BCM6328 + bool "Broadcom BCM6328 GPIO driver" + depends on OF_GPIO && (BMIPS_GENERIC || COMPILE_TEST) + select GPIO_REGMAP + select GPIOLIB_IRQCHIP + select IRQ_DOMAIN_HIERARCHY + select PINMUX + select PINCONF + select GENERIC_PINCONF
select GPIO_REGMAP ?
quoted hunk ↗ jump to hunk
+ default BMIPS_GENERIC + help + Say Y here to enable the Broadcom BCM6328 GPIO driver. + config PINCTRL_IPROC_GPIO bool "Broadcom iProc GPIO (with PINCONF) driver" depends on OF_GPIO && (ARCH_BCM_IPROC || COMPILE_TEST)diff --git a/drivers/pinctrl/bcm/Makefileb/drivers/pinctrl/bcm/Makefile index 79d5e49fdd9a..7e7c6e25b26d 100644--- a/drivers/pinctrl/bcm/Makefile +++ b/drivers/pinctrl/bcm/Makefile@@ -3,6 +3,7 @@ obj-$(CONFIG_PINCTRL_BCM281XX) += pinctrl-bcm281xx.o obj-$(CONFIG_PINCTRL_BCM2835) += pinctrl-bcm2835.o +obj-$(CONFIG_PINCTRL_BCM6328) += pinctrl-bcm6328.o obj-$(CONFIG_PINCTRL_IPROC_GPIO) += pinctrl-iproc-gpio.o obj-$(CONFIG_PINCTRL_CYGNUS_MUX) += pinctrl-cygnus-mux.o obj-$(CONFIG_PINCTRL_NS) += pinctrl-ns.odiff --git a/drivers/pinctrl/bcm/pinctrl-bcm6328.cb/drivers/pinctrl/bcm/pinctrl-bcm6328.c new file mode 100644 index 000000000000..f2b1a14e7903--- /dev/null +++ b/drivers/pinctrl/bcm/pinctrl-bcm6328.c
[..]
+static int bcm6328_reg_mask_xlate(struct gpio_regmap *gpio,
+ unsigned int base, unsigned int offset,
+ unsigned int *reg, unsigned int *mask)
+{
+ unsigned int line = offset % gpio->ngpio_per_reg;
+ unsigned int stride = offset / gpio->ngpio_per_reg;
+
+ *reg = base - stride * gpio->reg_stride;
+ *mask = BIT(line);
+
+ return 0;
+}How many registers are there? npgio_per_reg is 32 but so is ngpio. So isn't there only one register? And thus, can you use the default gpio_regmap_simple_xlat()? [..]
+static int bcm6328_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct gpio_regmap_config grc = {0};
+ struct gpio_regmap *gr;
+ struct bcm6328_pinctrl *pc;
+ int err;
+
+ pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+ if (!pc)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, pc);
+ pc->dev = dev;
+
+ pc->regs = syscon_node_to_regmap(dev->parent->of_node);
+ if (IS_ERR(pc->regs))
+ return PTR_ERR(pc->regs);
+
+ grc.parent = dev;
+ grc.ngpio = BCM6328_NUM_GPIOS;
+ grc.ngpio_per_reg = BCM6328_BANK_GPIOS;
+ grc.regmap = pc->regs;
+ grc.reg_dat_base = BCM6328_DATA_REG;
+ grc.reg_dir_out_base = BCM6328_DIROUT_REG;
+ grc.reg_mask_xlate = bcm6328_reg_mask_xlate;
+ grc.reg_set_base = BCM6328_DATA_REG;
+ grc.reg_stride = 4;
+
+ gr = devm_gpio_regmap_register(dev, &grc);
+ err = PTR_ERR_OR_ZERO(gr);
+ if (err) {
+ dev_err(dev, "could not add GPIO chip\n");
+ return err;
+ }
+
+ pc->pctl_desc.name = MODULE_NAME;
+ pc->pctl_desc.pins = bcm6328_pins;
+ pc->pctl_desc.npins = ARRAY_SIZE(bcm6328_pins);
+ pc->pctl_desc.pctlops = &bcm6328_pctl_ops;
+ pc->pctl_desc.pmxops = &bcm6328_pmx_ops;
+ pc->pctl_desc.owner = THIS_MODULE;
+
+ pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
+ if (IS_ERR(pc->pctl_dev)) {
+ gpiochip_remove(&gr->gpio_chip);
+ return PTR_ERR(pc->pctl_dev);
+ }
+
+ pc->gpio_range.name = MODULE_NAME;
+ pc->gpio_range.npins = BCM6328_NUM_GPIOS;
+ pc->gpio_range.base = gr->gpio_chip.base;
+ pc->gpio_range.gc = &gr->gpio_chip;
+ pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);Ahh I see. What about adding a new function in gpio-regmap.c: gpio_regmap_pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range)? gpio-regmap should have all the information to fill all the required properties. I'm unsure whether gpio-regmap should also allocate the gpio_range. Maybe someone can come up with a better function name though. -michael