diff mbox series

[v6,03/15] pinctrl: bcm: add bcm63xx base code

Message ID 20210310125504.31886-4-noltari@gmail.com (mailing list archive)
State New, archived
Headers show
Series pinctrl: add BCM63XX pincontrol support | expand

Commit Message

Álvaro Fernández Rojas March 10, 2021, 12:54 p.m. UTC
Add a helper for registering BCM63XX pin controllers.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
Co-developed-by: Jonas Gorski <jonas.gorski@gmail.com>
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v6: add changes suggested by Andy Shevchenko
 v5: add changes suggested by Andy Shevchenko
 v4: drop IRQ selects from Kconfig and add missing of_node_put()
 v3: add new patch with shared code

 drivers/pinctrl/bcm/Kconfig           |   7 ++
 drivers/pinctrl/bcm/Makefile          |   1 +
 drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 113 ++++++++++++++++++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm63xx.h |  43 ++++++++++
 4 files changed, 164 insertions(+)
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.h

Comments

Andy Shevchenko March 10, 2021, 2:07 p.m. UTC | #1
On Wed, Mar 10, 2021 at 2:55 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> Add a helper for registering BCM63XX pin controllers.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
(from use of the fwnode API perspective)

I'll think about of_match_node() case and perhaps come up with
corresponding fwnode API solution.

> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> Co-developed-by: Jonas Gorski <jonas.gorski@gmail.com>

Other way around (Co-DB followed by SoB), but this is minor thingy

> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  v6: add changes suggested by Andy Shevchenko
>  v5: add changes suggested by Andy Shevchenko
>  v4: drop IRQ selects from Kconfig and add missing of_node_put()
>  v3: add new patch with shared code
>
>  drivers/pinctrl/bcm/Kconfig           |   7 ++
>  drivers/pinctrl/bcm/Makefile          |   1 +
>  drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 113 ++++++++++++++++++++++++++
>  drivers/pinctrl/bcm/pinctrl-bcm63xx.h |  43 ++++++++++
>  4 files changed, 164 insertions(+)
>  create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.c
>  create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.h
>
> diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
> index 0ed14de0134c..882f19bdc243 100644
> --- a/drivers/pinctrl/bcm/Kconfig
> +++ b/drivers/pinctrl/bcm/Kconfig
> @@ -29,6 +29,13 @@ config PINCTRL_BCM2835
>         help
>            Say Y here to enable the Broadcom BCM2835 GPIO driver.
>
> +config PINCTRL_BCM63XX
> +       bool
> +       select GENERIC_PINCONF
> +       select GPIO_REGMAP
> +       select PINCONF
> +       select PINMUX
> +
>  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/Makefile b/drivers/pinctrl/bcm/Makefile
> index 79d5e49fdd9a..0e3cf9b15c65 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_BCM63XX)          += pinctrl-bcm63xx.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.o
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
> new file mode 100644
> index 000000000000..2eaac8e6f79f
> --- /dev/null
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for BCM63xx GPIO unit (pinctrl + GPIO)
> + *
> + * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
> + */
> +
> +#include <linux/gpio/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include "pinctrl-bcm63xx.h"
> +
> +#define BCM63XX_BANK_SIZE      4
> +
> +#define BCM63XX_DIROUT_REG     0x04
> +#define BCM63XX_DATA_REG       0x0c
> +
> +static int bcm63xx_reg_mask_xlate(struct gpio_regmap *gpio,
> +                                 unsigned int base, unsigned int offset,
> +                                 unsigned int *reg, unsigned int *mask)
> +{
> +       unsigned int line = offset % BCM63XX_BANK_GPIOS;
> +       unsigned int stride = offset / BCM63XX_BANK_GPIOS;
> +
> +       *reg = base - stride * BCM63XX_BANK_SIZE;
> +       *mask = BIT(line);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> +       { .compatible = "brcm,bcm6318-gpio", },
> +       { .compatible = "brcm,bcm6328-gpio", },
> +       { .compatible = "brcm,bcm6358-gpio", },
> +       { .compatible = "brcm,bcm6362-gpio", },
> +       { .compatible = "brcm,bcm6368-gpio", },
> +       { .compatible = "brcm,bcm63268-gpio", },
> +       { /* sentinel */ }
> +};
> +
> +static int bcm63xx_gpio_probe(struct device *dev,
> +                             struct fwnode_handle *node,
> +                             const struct bcm63xx_pinctrl_soc *soc,
> +                             struct bcm63xx_pinctrl *pc)
> +{
> +       struct gpio_regmap_config grc = {0};
> +
> +       grc.parent = dev;
> +       grc.fwnode = node;
> +       grc.ngpio = soc->ngpios;
> +       grc.ngpio_per_reg = BCM63XX_BANK_GPIOS;
> +       grc.regmap = pc->regs;
> +       grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;
> +
> +       if (fwnode_property_read_u32(node, "data", &grc.reg_dat_base))
> +               grc.reg_dat_base = BCM63XX_DATA_REG;
> +       grc.reg_set_base = grc.reg_dat_base;
> +
> +       if (fwnode_property_read_u32(node, "dirout", &grc.reg_dir_out_base))
> +               grc.reg_dir_out_base = BCM63XX_DIROUT_REG;
> +
> +       return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &grc));
> +}
> +
> +int bcm63xx_pinctrl_probe(struct platform_device *pdev,
> +                         const struct bcm63xx_pinctrl_soc *soc,
> +                         void *driver_data)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct bcm63xx_pinctrl *pc;
> +       struct fwnode_handle *node;
> +       int err;
> +
> +       pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
> +       if (!pc)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, pc);
> +
> +       pc->dev = dev;
> +       pc->driver_data = driver_data;
> +
> +       pc->regs = syscon_node_to_regmap(dev->parent->of_node);
> +       if (IS_ERR(pc->regs))
> +               return PTR_ERR(pc->regs);
> +
> +       pc->pctl_desc.name = dev_name(dev);
> +       pc->pctl_desc.pins = soc->pins;
> +       pc->pctl_desc.npins = soc->npins;
> +       pc->pctl_desc.pctlops = soc->pctl_ops;
> +       pc->pctl_desc.pmxops = soc->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))
> +               return PTR_ERR(pc->pctl_dev);
> +
> +       device_for_each_child_node(dev, node) {
> +               if (of_match_node(bcm63xx_gpio_of_match, to_of_node(node))) {
> +                       err = bcm63xx_gpio_probe(dev, node, soc, pc);
> +                       if (err) {
> +                               dev_err(dev, "could not add GPIO chip\n");
> +                               fwnode_handle_put(node);
> +                               return err;
> +                       }
> +               }
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.h b/drivers/pinctrl/bcm/pinctrl-bcm63xx.h
> new file mode 100644
> index 000000000000..3bdb50021f1b
> --- /dev/null
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.h
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
> + */
> +
> +#ifndef __PINCTRL_BCM63XX_H__
> +#define __PINCTRL_BCM63XX_H__
> +
> +#include <linux/pinctrl/pinctrl.h>
> +
> +#define BCM63XX_BANK_GPIOS 32
> +
> +struct bcm63xx_pinctrl_soc {
> +       struct pinctrl_ops *pctl_ops;
> +       struct pinmux_ops *pmx_ops;
> +
> +       const struct pinctrl_pin_desc *pins;
> +       unsigned npins;
> +
> +       unsigned int ngpios;
> +};
> +
> +struct bcm63xx_pinctrl {
> +       struct device *dev;
> +       struct regmap *regs;
> +
> +       struct pinctrl_desc pctl_desc;
> +       struct pinctrl_dev *pctl_dev;
> +
> +       void *driver_data;
> +};
> +
> +static inline unsigned int bcm63xx_bank_pin(unsigned int pin)
> +{
> +       return pin % BCM63XX_BANK_GPIOS;
> +}
> +
> +int bcm63xx_pinctrl_probe(struct platform_device *pdev,
> +                         const struct bcm63xx_pinctrl_soc *soc,
> +                         void *driver_data);
> +
> +#endif /* __PINCTRL_BCM63XX_H__ */
> --
> 2.20.1
>
Rob Herring March 10, 2021, 5:50 p.m. UTC | #2
On Wed, Mar 10, 2021 at 5:55 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> Add a helper for registering BCM63XX pin controllers.

Comments below based on my binding changes suggested.

>
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> Co-developed-by: Jonas Gorski <jonas.gorski@gmail.com>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  v6: add changes suggested by Andy Shevchenko
>  v5: add changes suggested by Andy Shevchenko
>  v4: drop IRQ selects from Kconfig and add missing of_node_put()
>  v3: add new patch with shared code
>
>  drivers/pinctrl/bcm/Kconfig           |   7 ++
>  drivers/pinctrl/bcm/Makefile          |   1 +
>  drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 113 ++++++++++++++++++++++++++
>  drivers/pinctrl/bcm/pinctrl-bcm63xx.h |  43 ++++++++++
>  4 files changed, 164 insertions(+)
>  create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.c
>  create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.h
>
> diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
> index 0ed14de0134c..882f19bdc243 100644
> --- a/drivers/pinctrl/bcm/Kconfig
> +++ b/drivers/pinctrl/bcm/Kconfig
> @@ -29,6 +29,13 @@ config PINCTRL_BCM2835
>         help
>            Say Y here to enable the Broadcom BCM2835 GPIO driver.
>
> +config PINCTRL_BCM63XX
> +       bool
> +       select GENERIC_PINCONF
> +       select GPIO_REGMAP
> +       select PINCONF
> +       select PINMUX
> +
>  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/Makefile b/drivers/pinctrl/bcm/Makefile
> index 79d5e49fdd9a..0e3cf9b15c65 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_BCM63XX)          += pinctrl-bcm63xx.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.o
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
> new file mode 100644
> index 000000000000..2eaac8e6f79f
> --- /dev/null
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for BCM63xx GPIO unit (pinctrl + GPIO)
> + *
> + * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
> + */
> +
> +#include <linux/gpio/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include "pinctrl-bcm63xx.h"
> +
> +#define BCM63XX_BANK_SIZE      4
> +
> +#define BCM63XX_DIROUT_REG     0x04
> +#define BCM63XX_DATA_REG       0x0c
> +
> +static int bcm63xx_reg_mask_xlate(struct gpio_regmap *gpio,
> +                                 unsigned int base, unsigned int offset,
> +                                 unsigned int *reg, unsigned int *mask)
> +{
> +       unsigned int line = offset % BCM63XX_BANK_GPIOS;
> +       unsigned int stride = offset / BCM63XX_BANK_GPIOS;
> +
> +       *reg = base - stride * BCM63XX_BANK_SIZE;
> +       *mask = BIT(line);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> +       { .compatible = "brcm,bcm6318-gpio", },
> +       { .compatible = "brcm,bcm6328-gpio", },
> +       { .compatible = "brcm,bcm6358-gpio", },
> +       { .compatible = "brcm,bcm6362-gpio", },
> +       { .compatible = "brcm,bcm6368-gpio", },
> +       { .compatible = "brcm,bcm63268-gpio", },

All these would be moved to gpio-mmio.c (or maybe that can have a
fallback compatible?).

> +       { /* sentinel */ }
> +};
> +
> +static int bcm63xx_gpio_probe(struct device *dev,
> +                             struct fwnode_handle *node,
> +                             const struct bcm63xx_pinctrl_soc *soc,
> +                             struct bcm63xx_pinctrl *pc)
> +{
> +       struct gpio_regmap_config grc = {0};
> +
> +       grc.parent = dev;
> +       grc.fwnode = node;
> +       grc.ngpio = soc->ngpios;
> +       grc.ngpio_per_reg = BCM63XX_BANK_GPIOS;
> +       grc.regmap = pc->regs;
> +       grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;
> +
> +       if (fwnode_property_read_u32(node, "data", &grc.reg_dat_base))
> +               grc.reg_dat_base = BCM63XX_DATA_REG;
> +       grc.reg_set_base = grc.reg_dat_base;
> +
> +       if (fwnode_property_read_u32(node, "dirout", &grc.reg_dir_out_base))
> +               grc.reg_dir_out_base = BCM63XX_DIROUT_REG;
> +
> +       return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &grc));
> +}

This function can go.

> +
> +int bcm63xx_pinctrl_probe(struct platform_device *pdev,
> +                         const struct bcm63xx_pinctrl_soc *soc,
> +                         void *driver_data)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct bcm63xx_pinctrl *pc;
> +       struct fwnode_handle *node;
> +       int err;
> +
> +       pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
> +       if (!pc)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, pc);
> +
> +       pc->dev = dev;
> +       pc->driver_data = driver_data;
> +
> +       pc->regs = syscon_node_to_regmap(dev->parent->of_node);
> +       if (IS_ERR(pc->regs))
> +               return PTR_ERR(pc->regs);
> +
> +       pc->pctl_desc.name = dev_name(dev);
> +       pc->pctl_desc.pins = soc->pins;
> +       pc->pctl_desc.npins = soc->npins;
> +       pc->pctl_desc.pctlops = soc->pctl_ops;
> +       pc->pctl_desc.pmxops = soc->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))
> +               return PTR_ERR(pc->pctl_dev);
> +
> +       device_for_each_child_node(dev, node) {
> +               if (of_match_node(bcm63xx_gpio_of_match, to_of_node(node))) {
> +                       err = bcm63xx_gpio_probe(dev, node, soc, pc);

This would just need an of_platform_populate call.

> +                       if (err) {
> +                               dev_err(dev, "could not add GPIO chip\n");
> +                               fwnode_handle_put(node);
> +                               return err;
> +                       }
> +               }
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.h b/drivers/pinctrl/bcm/pinctrl-bcm63xx.h
> new file mode 100644
> index 000000000000..3bdb50021f1b
> --- /dev/null
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.h
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
> + * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
> + */
> +
> +#ifndef __PINCTRL_BCM63XX_H__
> +#define __PINCTRL_BCM63XX_H__
> +
> +#include <linux/pinctrl/pinctrl.h>
> +
> +#define BCM63XX_BANK_GPIOS 32
> +
> +struct bcm63xx_pinctrl_soc {
> +       struct pinctrl_ops *pctl_ops;
> +       struct pinmux_ops *pmx_ops;
> +
> +       const struct pinctrl_pin_desc *pins;
> +       unsigned npins;
> +
> +       unsigned int ngpios;
> +};
> +
> +struct bcm63xx_pinctrl {
> +       struct device *dev;
> +       struct regmap *regs;
> +
> +       struct pinctrl_desc pctl_desc;
> +       struct pinctrl_dev *pctl_dev;
> +
> +       void *driver_data;
> +};
> +
> +static inline unsigned int bcm63xx_bank_pin(unsigned int pin)
> +{
> +       return pin % BCM63XX_BANK_GPIOS;
> +}
> +
> +int bcm63xx_pinctrl_probe(struct platform_device *pdev,
> +                         const struct bcm63xx_pinctrl_soc *soc,
> +                         void *driver_data);
> +
> +#endif /* __PINCTRL_BCM63XX_H__ */
> --
> 2.20.1
>
Linus Walleij March 11, 2021, 1:09 a.m. UTC | #3
On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote:

> > +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> > +       { .compatible = "brcm,bcm6318-gpio", },
> > +       { .compatible = "brcm,bcm6328-gpio", },
> > +       { .compatible = "brcm,bcm6358-gpio", },
> > +       { .compatible = "brcm,bcm6362-gpio", },
> > +       { .compatible = "brcm,bcm6368-gpio", },
> > +       { .compatible = "brcm,bcm63268-gpio", },
>
> All these would be moved to gpio-mmio.c (or maybe that can have a
> fallback compatible?).

This is gpio-regmap.c and it can only be used as a library
by a certain driver. gpio-mmio.c can be used stand-alone
for certain really simple hardware (though most use that
as a library as well).

Yours,
Linus Walleij
Rob Herring March 11, 2021, 2:57 p.m. UTC | #4
On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> > > +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> > > +       { .compatible = "brcm,bcm6318-gpio", },
> > > +       { .compatible = "brcm,bcm6328-gpio", },
> > > +       { .compatible = "brcm,bcm6358-gpio", },
> > > +       { .compatible = "brcm,bcm6362-gpio", },
> > > +       { .compatible = "brcm,bcm6368-gpio", },
> > > +       { .compatible = "brcm,bcm63268-gpio", },
> >
> > All these would be moved to gpio-mmio.c (or maybe that can have a
> > fallback compatible?).
>
> This is gpio-regmap.c and it can only be used as a library
> by a certain driver. gpio-mmio.c can be used stand-alone
> for certain really simple hardware (though most use that
> as a library as well).

I don't really care which one is used, but the problem is that this
choice is leaking into the binding design. The primary problem here is
once someone uses regmap, then they think they must have a syscon and
can abandon using 'reg' and normal address properties as Linux happens
to not use them (currently). I think we really need some better regmap
vs. mmio handling to eliminate this duplication of foo-mmio and
foo-regmap drivers and difference in binding design. Not sure exactly
what that looks like, but basically some sort of 'reg' property to
regmap creation.

Given we already have a Broadcom GPIO binding for what looks to be
similar to this one, I'm left wondering what's the real difference
here?

Rob
Linus Walleij March 11, 2021, 4:13 p.m. UTC | #5
On Thu, Mar 11, 2021 at 3:58 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > > > +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> > > > +       { .compatible = "brcm,bcm6318-gpio", },
> > > > +       { .compatible = "brcm,bcm6328-gpio", },
> > > > +       { .compatible = "brcm,bcm6358-gpio", },
> > > > +       { .compatible = "brcm,bcm6362-gpio", },
> > > > +       { .compatible = "brcm,bcm6368-gpio", },
> > > > +       { .compatible = "brcm,bcm63268-gpio", },
> > >
> > > All these would be moved to gpio-mmio.c (or maybe that can have a
> > > fallback compatible?).
> >
> > This is gpio-regmap.c and it can only be used as a library
> > by a certain driver. gpio-mmio.c can be used stand-alone
> > for certain really simple hardware (though most use that
> > as a library as well).
>
> I don't really care which one is used, but the problem is that this
> choice is leaking into the binding design.

Aha I guess I misunderstood your comment.

>The primary problem here is
> once someone uses regmap, then they think they must have a syscon and
> can abandon using 'reg' and normal address properties as Linux happens
> to not use them (currently). I think we really need some better regmap
> vs. mmio handling to eliminate this duplication of foo-mmio and
> foo-regmap drivers and difference in binding design. Not sure exactly
> what that looks like, but basically some sort of 'reg' property to
> regmap creation.

I see the problem. Yeah we should try to be more strict around
these things. To me there are syscons and "other regmaps",
where syscon is a real hurdle of registers while "other regmaps"
are just regmaps by convenience.

Documentation/devicetree/bindings/mfd/syscon.yaml
describes what a syscon really is so if everyone could
just read the documentation that would be great ...

> Given we already have a Broadcom GPIO binding for what looks to be
> similar to this one, I'm left wondering what's the real difference
> here?

Which one is similar? I can take a look.

We currently have four Broadcom GPIO bindings,
which are stand alone GPIO blocks and eight Broadcom
pin controllers that all do GPIO as well.

This family of pin controllers are (as per subject) is
the bcm63xx series which is a MIPS-based family of SoCs
found in routers, top bindings in
Documentation/devicetree/bindings/mips/brcm/soc.txt
These all have a GPIO block as part of the pin controller
and the GPIO block is a distinct sub-function of the
pin controller, and it has up to 32 GPIOs per block,
hence it has its own subnode inside the pin controller.

This driver follows the pattern of the Ingenic
pin controller, another MIPS SoC:
Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml

Another SoC with several GPIO blocks inside the pin
controller is SparX5 and that also follows this pattern:
Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml
(This has an example with more than one GPIO block
inside the pin controller.)

Yours,
Linus Walleij
Álvaro Fernández Rojas March 11, 2021, 5 p.m. UTC | #6
Hi Rob and Linus,

El 11/03/2021 a las 17:13, Linus Walleij escribió:
> On Thu, Mar 11, 2021 at 3:58 PM Rob Herring <robh+dt@kernel.org> wrote:
>> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote:
>>>
>>>>> +static const struct of_device_id bcm63xx_gpio_of_match[] = {
>>>>> +       { .compatible = "brcm,bcm6318-gpio", },
>>>>> +       { .compatible = "brcm,bcm6328-gpio", },
>>>>> +       { .compatible = "brcm,bcm6358-gpio", },
>>>>> +       { .compatible = "brcm,bcm6362-gpio", },
>>>>> +       { .compatible = "brcm,bcm6368-gpio", },
>>>>> +       { .compatible = "brcm,bcm63268-gpio", },
>>>>
>>>> All these would be moved to gpio-mmio.c (or maybe that can have a
>>>> fallback compatible?).
>>>
>>> This is gpio-regmap.c and it can only be used as a library
>>> by a certain driver. gpio-mmio.c can be used stand-alone
>>> for certain really simple hardware (though most use that
>>> as a library as well).
>>
>> I don't really care which one is used, but the problem is that this
>> choice is leaking into the binding design.
> 
> Aha I guess I misunderstood your comment.
> 
>> The primary problem here is
>> once someone uses regmap, then they think they must have a syscon and
>> can abandon using 'reg' and normal address properties as Linux happens
>> to not use them (currently). I think we really need some better regmap
>> vs. mmio handling to eliminate this duplication of foo-mmio and
>> foo-regmap drivers and difference in binding design. Not sure exactly
>> what that looks like, but basically some sort of 'reg' property to
>> regmap creation.
> 
> I see the problem. Yeah we should try to be more strict around
> these things. To me there are syscons and "other regmaps",
> where syscon is a real hurdle of registers while "other regmaps"
> are just regmaps by convenience.
> 
> Documentation/devicetree/bindings/mfd/syscon.yaml
> describes what a syscon really is so if everyone could
> just read the documentation that would be great ...
> 
>> Given we already have a Broadcom GPIO binding for what looks to be
>> similar to this one, I'm left wondering what's the real difference
>> here?
> 
> Which one is similar? I can take a look.

@Linus I think @Rob is referring to brcm,bcm6345-gpio:
https://github.com/torvalds/linux/blob/a74e6a014c9d4d4161061f770c9b4f98372ac778/drivers/gpio/gpio-mmio.c#L686

However, the real difference between BCM6345 (and BCM6338) is that these 
SoCs have no pin controller at all, only a GPIO controller:

BCM6345:
typedef struct GpioControl {
   uint16        unused0;
   byte          unused1;
   byte          TBusSel;
   uint16        unused2;
   uint16        GPIODir;
   byte          unused3;
   byte          Leds;
   uint16        GPIOio;
   uint32        UartCtl;
} GpioControl;

BCM6338:
typedef struct GpioControl {
   uint32        unused0;
   uint32        GPIODir;      /* bits 7:0 */
   uint32        unused1;
   uint32        GPIOio;       /* bits 7:0 */
   uint32        LEDCtrl;
   uint32        SpiSlaveCfg;
   uint32        vRegConfig;
} GpioControl;

BCM6348 and newer also have pinctrl.
That's the main difference between that driver @Rob's referring to and 
the ones in this patch series.

> 
> We currently have four Broadcom GPIO bindings,
> which are stand alone GPIO blocks and eight Broadcom
> pin controllers that all do GPIO as well.
> 
> This family of pin controllers are (as per subject) is
> the bcm63xx series which is a MIPS-based family of SoCs
> found in routers, top bindings in
> Documentation/devicetree/bindings/mips/brcm/soc.txt
> These all have a GPIO block as part of the pin controller
> and the GPIO block is a distinct sub-function of the
> pin controller, and it has up to 32 GPIOs per block,
> hence it has its own subnode inside the pin controller.
> 
> This driver follows the pattern of the Ingenic
> pin controller, another MIPS SoC:
> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml
> 
> Another SoC with several GPIO blocks inside the pin
> controller is SparX5 and that also follows this pattern:
> Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml
> (This has an example with more than one GPIO block
> inside the pin controller.)
> 
> Yours,
> Linus Walleij
>
Rob Herring March 11, 2021, 6:24 p.m. UTC | #7
On Thu, Mar 11, 2021 at 10:00 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> Hi Rob and Linus,
>
> El 11/03/2021 a las 17:13, Linus Walleij escribió:
> > On Thu, Mar 11, 2021 at 3:58 PM Rob Herring <robh+dt@kernel.org> wrote:
> >> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >>> On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote:
> >>>
> >>>>> +static const struct of_device_id bcm63xx_gpio_of_match[] = {
> >>>>> +       { .compatible = "brcm,bcm6318-gpio", },
> >>>>> +       { .compatible = "brcm,bcm6328-gpio", },
> >>>>> +       { .compatible = "brcm,bcm6358-gpio", },
> >>>>> +       { .compatible = "brcm,bcm6362-gpio", },
> >>>>> +       { .compatible = "brcm,bcm6368-gpio", },
> >>>>> +       { .compatible = "brcm,bcm63268-gpio", },
> >>>>
> >>>> All these would be moved to gpio-mmio.c (or maybe that can have a
> >>>> fallback compatible?).
> >>>
> >>> This is gpio-regmap.c and it can only be used as a library
> >>> by a certain driver. gpio-mmio.c can be used stand-alone
> >>> for certain really simple hardware (though most use that
> >>> as a library as well).
> >>
> >> I don't really care which one is used, but the problem is that this
> >> choice is leaking into the binding design.
> >
> > Aha I guess I misunderstood your comment.
> >
> >> The primary problem here is
> >> once someone uses regmap, then they think they must have a syscon and
> >> can abandon using 'reg' and normal address properties as Linux happens
> >> to not use them (currently). I think we really need some better regmap
> >> vs. mmio handling to eliminate this duplication of foo-mmio and
> >> foo-regmap drivers and difference in binding design. Not sure exactly
> >> what that looks like, but basically some sort of 'reg' property to
> >> regmap creation.
> >
> > I see the problem. Yeah we should try to be more strict around
> > these things. To me there are syscons and "other regmaps",
> > where syscon is a real hurdle of registers while "other regmaps"
> > are just regmaps by convenience.
> >
> > Documentation/devicetree/bindings/mfd/syscon.yaml
> > describes what a syscon really is so if everyone could
> > just read the documentation that would be great ...
> >
> >> Given we already have a Broadcom GPIO binding for what looks to be
> >> similar to this one, I'm left wondering what's the real difference
> >> here?
> >
> > Which one is similar? I can take a look.
>
> @Linus I think @Rob is referring to brcm,bcm6345-gpio:
> https://github.com/torvalds/linux/blob/a74e6a014c9d4d4161061f770c9b4f98372ac778/drivers/gpio/gpio-mmio.c#L686

Well, since it's the bindings we're talking about:
Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt

Which says this:
"These bindings can be used on any BCM63xx SoC. However, BCM6338 and
BCM6345 are the only ones which don't need a pinctrl driver."

Not that the 1 in tree user of this is perfect. Seems like it too
should be a child of a system controller if there's other registers.

>
> However, the real difference between BCM6345 (and BCM6338) is that these
> SoCs have no pin controller at all, only a GPIO controller:
>
> BCM6345:
> typedef struct GpioControl {
>    uint16        unused0;
>    byte          unused1;
>    byte          TBusSel;
>    uint16        unused2;
>    uint16        GPIODir;
>    byte          unused3;
>    byte          Leds;
>    uint16        GPIOio;
>    uint32        UartCtl;
> } GpioControl;
>
> BCM6338:
> typedef struct GpioControl {
>    uint32        unused0;
>    uint32        GPIODir;      /* bits 7:0 */
>    uint32        unused1;
>    uint32        GPIOio;       /* bits 7:0 */
>    uint32        LEDCtrl;
>    uint32        SpiSlaveCfg;
>    uint32        vRegConfig;
> } GpioControl;
>
> BCM6348 and newer also have pinctrl.
> That's the main difference between that driver @Rob's referring to and
> the ones in this patch series.
Álvaro Fernández Rojas March 11, 2021, 6:32 p.m. UTC | #8
Hi Rob,

> El 11 mar 2021, a las 19:24, Rob Herring <robh+dt@kernel.org> escribió:
> 
> On Thu, Mar 11, 2021 at 10:00 AM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
>> 
>> Hi Rob and Linus,
>> 
>> El 11/03/2021 a las 17:13, Linus Walleij escribió:
>>> On Thu, Mar 11, 2021 at 3:58 PM Rob Herring <robh+dt@kernel.org> wrote:
>>>> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>> On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote:
>>>>> 
>>>>>>> +static const struct of_device_id bcm63xx_gpio_of_match[] = {
>>>>>>> +       { .compatible = "brcm,bcm6318-gpio", },
>>>>>>> +       { .compatible = "brcm,bcm6328-gpio", },
>>>>>>> +       { .compatible = "brcm,bcm6358-gpio", },
>>>>>>> +       { .compatible = "brcm,bcm6362-gpio", },
>>>>>>> +       { .compatible = "brcm,bcm6368-gpio", },
>>>>>>> +       { .compatible = "brcm,bcm63268-gpio", },
>>>>>> 
>>>>>> All these would be moved to gpio-mmio.c (or maybe that can have a
>>>>>> fallback compatible?).
>>>>> 
>>>>> This is gpio-regmap.c and it can only be used as a library
>>>>> by a certain driver. gpio-mmio.c can be used stand-alone
>>>>> for certain really simple hardware (though most use that
>>>>> as a library as well).
>>>> 
>>>> I don't really care which one is used, but the problem is that this
>>>> choice is leaking into the binding design.
>>> 
>>> Aha I guess I misunderstood your comment.
>>> 
>>>> The primary problem here is
>>>> once someone uses regmap, then they think they must have a syscon and
>>>> can abandon using 'reg' and normal address properties as Linux happens
>>>> to not use them (currently). I think we really need some better regmap
>>>> vs. mmio handling to eliminate this duplication of foo-mmio and
>>>> foo-regmap drivers and difference in binding design. Not sure exactly
>>>> what that looks like, but basically some sort of 'reg' property to
>>>> regmap creation.
>>> 
>>> I see the problem. Yeah we should try to be more strict around
>>> these things. To me there are syscons and "other regmaps",
>>> where syscon is a real hurdle of registers while "other regmaps"
>>> are just regmaps by convenience.
>>> 
>>> Documentation/devicetree/bindings/mfd/syscon.yaml
>>> describes what a syscon really is so if everyone could
>>> just read the documentation that would be great ...
>>> 
>>>> Given we already have a Broadcom GPIO binding for what looks to be
>>>> similar to this one, I'm left wondering what's the real difference
>>>> here?
>>> 
>>> Which one is similar? I can take a look.
>> 
>> @Linus I think @Rob is referring to brcm,bcm6345-gpio:
>> https://github.com/torvalds/linux/blob/a74e6a014c9d4d4161061f770c9b4f98372ac778/drivers/gpio/gpio-mmio.c#L686
> 
> Well, since it's the bindings we're talking about:
> Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt
> 
> Which says this:
> "These bindings can be used on any BCM63xx SoC. However, BCM6338 and
> BCM6345 are the only ones which don't need a pinctrl driver."

Yes, I know because I’m the author of that driver…
What I meant at that time is that it could be used temporarily until a proper “full” pinctrl driver was added.

> 
> Not that the 1 in tree user of this is perfect. Seems like it too
> should be a child of a system controller if there's other registers.

There are other registers, but dirout and data registers are contiguous and separate from the others.

> 
>> 
>> However, the real difference between BCM6345 (and BCM6338) is that these
>> SoCs have no pin controller at all, only a GPIO controller:
>> 
>> BCM6345:
>> typedef struct GpioControl {
>>   uint16        unused0;
>>   byte          unused1;
>>   byte          TBusSel;
>>   uint16        unused2;
>>   uint16        GPIODir;
>>   byte          unused3;
>>   byte          Leds;
>>   uint16        GPIOio;
>>   uint32        UartCtl;
>> } GpioControl;
>> 
>> BCM6338:
>> typedef struct GpioControl {
>>   uint32        unused0;
>>   uint32        GPIODir;      /* bits 7:0 */
>>   uint32        unused1;
>>   uint32        GPIOio;       /* bits 7:0 */
>>   uint32        LEDCtrl;
>>   uint32        SpiSlaveCfg;
>>   uint32        vRegConfig;
>> } GpioControl;
>> 
>> BCM6348 and newer also have pinctrl.
>> That's the main difference between that driver @Rob's referring to and
>> the ones in this patch series.

Best regards,
Álvaro.
diff mbox series

Patch

diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 0ed14de0134c..882f19bdc243 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -29,6 +29,13 @@  config PINCTRL_BCM2835
 	help
 	   Say Y here to enable the Broadcom BCM2835 GPIO driver.
 
+config PINCTRL_BCM63XX
+	bool
+	select GENERIC_PINCONF
+	select GPIO_REGMAP
+	select PINCONF
+	select PINMUX
+
 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/Makefile b/drivers/pinctrl/bcm/Makefile
index 79d5e49fdd9a..0e3cf9b15c65 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_BCM63XX)		+= pinctrl-bcm63xx.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.o
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
new file mode 100644
index 000000000000..2eaac8e6f79f
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
@@ -0,0 +1,113 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for BCM63xx GPIO unit (pinctrl + GPIO)
+ *
+ * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
+ */
+
+#include <linux/gpio/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "pinctrl-bcm63xx.h"
+
+#define BCM63XX_BANK_SIZE	4
+
+#define BCM63XX_DIROUT_REG	0x04
+#define BCM63XX_DATA_REG	0x0c
+
+static int bcm63xx_reg_mask_xlate(struct gpio_regmap *gpio,
+				  unsigned int base, unsigned int offset,
+				  unsigned int *reg, unsigned int *mask)
+{
+	unsigned int line = offset % BCM63XX_BANK_GPIOS;
+	unsigned int stride = offset / BCM63XX_BANK_GPIOS;
+
+	*reg = base - stride * BCM63XX_BANK_SIZE;
+	*mask = BIT(line);
+
+	return 0;
+}
+
+static const struct of_device_id bcm63xx_gpio_of_match[] = {
+	{ .compatible = "brcm,bcm6318-gpio", },
+	{ .compatible = "brcm,bcm6328-gpio", },
+	{ .compatible = "brcm,bcm6358-gpio", },
+	{ .compatible = "brcm,bcm6362-gpio", },
+	{ .compatible = "brcm,bcm6368-gpio", },
+	{ .compatible = "brcm,bcm63268-gpio", },
+	{ /* sentinel */ }
+};
+
+static int bcm63xx_gpio_probe(struct device *dev,
+			      struct fwnode_handle *node,
+			      const struct bcm63xx_pinctrl_soc *soc,
+			      struct bcm63xx_pinctrl *pc)
+{
+	struct gpio_regmap_config grc = {0};
+
+	grc.parent = dev;
+	grc.fwnode = node;
+	grc.ngpio = soc->ngpios;
+	grc.ngpio_per_reg = BCM63XX_BANK_GPIOS;
+	grc.regmap = pc->regs;
+	grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;
+
+	if (fwnode_property_read_u32(node, "data", &grc.reg_dat_base))
+		grc.reg_dat_base = BCM63XX_DATA_REG;
+	grc.reg_set_base = grc.reg_dat_base;
+
+	if (fwnode_property_read_u32(node, "dirout", &grc.reg_dir_out_base))
+		grc.reg_dir_out_base = BCM63XX_DIROUT_REG;
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &grc));
+}
+
+int bcm63xx_pinctrl_probe(struct platform_device *pdev,
+			  const struct bcm63xx_pinctrl_soc *soc,
+			  void *driver_data)
+{
+	struct device *dev = &pdev->dev;
+	struct bcm63xx_pinctrl *pc;
+	struct fwnode_handle *node;
+	int err;
+
+	pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+	if (!pc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, pc);
+
+	pc->dev = dev;
+	pc->driver_data = driver_data;
+
+	pc->regs = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(pc->regs))
+		return PTR_ERR(pc->regs);
+
+	pc->pctl_desc.name = dev_name(dev);
+	pc->pctl_desc.pins = soc->pins;
+	pc->pctl_desc.npins = soc->npins;
+	pc->pctl_desc.pctlops = soc->pctl_ops;
+	pc->pctl_desc.pmxops = soc->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))
+		return PTR_ERR(pc->pctl_dev);
+
+	device_for_each_child_node(dev, node) {
+		if (of_match_node(bcm63xx_gpio_of_match, to_of_node(node))) {
+			err = bcm63xx_gpio_probe(dev, node, soc, pc);
+			if (err) {
+				dev_err(dev, "could not add GPIO chip\n");
+				fwnode_handle_put(node);
+				return err;
+			}
+		}
+	}
+
+	return 0;
+}
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.h b/drivers/pinctrl/bcm/pinctrl-bcm63xx.h
new file mode 100644
index 000000000000..3bdb50021f1b
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.h
@@ -0,0 +1,43 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
+ */
+
+#ifndef __PINCTRL_BCM63XX_H__
+#define __PINCTRL_BCM63XX_H__
+
+#include <linux/pinctrl/pinctrl.h>
+
+#define BCM63XX_BANK_GPIOS 32
+
+struct bcm63xx_pinctrl_soc {
+	struct pinctrl_ops *pctl_ops;
+	struct pinmux_ops *pmx_ops;
+
+	const struct pinctrl_pin_desc *pins;
+	unsigned npins;
+
+	unsigned int ngpios;
+};
+
+struct bcm63xx_pinctrl {
+	struct device *dev;
+	struct regmap *regs;
+
+	struct pinctrl_desc pctl_desc;
+	struct pinctrl_dev *pctl_dev;
+
+	void *driver_data;
+};
+
+static inline unsigned int bcm63xx_bank_pin(unsigned int pin)
+{
+	return pin % BCM63XX_BANK_GPIOS;
+}
+
+int bcm63xx_pinctrl_probe(struct platform_device *pdev,
+			  const struct bcm63xx_pinctrl_soc *soc,
+			  void *driver_data);
+
+#endif /* __PINCTRL_BCM63XX_H__ */