diff mbox series

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

Message ID 20210306155712.4298-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 6, 2021, 3:57 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>
---
 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 | 112 ++++++++++++++++++++++++++
 drivers/pinctrl/bcm/pinctrl-bcm63xx.h |  43 ++++++++++
 4 files changed, 163 insertions(+)
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.c
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm63xx.h

Comments

Andy Shevchenko March 7, 2021, 7:05 p.m. UTC | #1
On Sat, Mar 6, 2021 at 5:57 PM Álvaro Fernández Rojas <noltari@gmail.com> wrote:
>
> Add a helper for registering BCM63XX pin controllers.

Thanks for this, but I think we may use the fwnode API. See below.

...

> +#include <linux/gpio/regmap.h>
> +#include <linux/mfd/syscon.h>

> +#include <linux/of.h>

+ property.h
+ mod_devicetable.h

> +#include <linux/platform_device.h>
> +
> +#include "pinctrl-bcm63xx.h"

> +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 int bcm63xx_gpio_probe(struct device *dev, struct device_node *node,

device_node *node -> fwnode_handle *fwnode

> +                             const struct bcm63xx_pinctrl_soc *soc,
> +                             struct bcm63xx_pinctrl *pc)
> +{
> +       struct gpio_regmap_config grc = {0};
> +
> +       grc.parent = dev;

> +       grc.fwnode = &node->fwnode;

grc.fwnode = fwnode;

> +       grc.ngpio = soc->ngpios;
> +       grc.ngpio_per_reg = BCM63XX_BANK_GPIOS;
> +       grc.regmap = pc->regs;
> +       grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;

> +       if (of_property_read_u32(node, "data", &grc.reg_dat_base))

fwnode_property_read_u32()

> +               grc.reg_dat_base = BCM63XX_DATA_REG;
> +       grc.reg_set_base = grc.reg_dat_base;

> +       if (of_property_read_u32(node, "dirout", &grc.reg_dir_out_base))

Ditto.

> +               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 device_node *node;

struct fwnode_handle *fwnode;

> +       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);

> +       for_each_child_of_node(dev->of_node, node) {

device_for_each_child_node(dev, fwnode) {

> +               if (of_match_node(bcm63xx_gpio_of_match, node)) {

// for now, since we have not an analogue (yet)
node ==> to_of_node(fwnode)

> +                       err = bcm63xx_gpio_probe(dev, node, soc, pc);

...(dev, fwnode, soc, pc);

> +                       if (err) {
> +                               dev_err(dev, "could not add GPIO chip\n");

> +                               of_node_put(node);

fwnode_handle_put(fwnode);

> +                               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
>


--
With Best Regards,
Andy Shevchenko
Álvaro Fernández Rojas March 10, 2021, 7:25 a.m. UTC | #2
Hi Andy,

> El 7 mar 2021, a las 20:05, Andy Shevchenko <andy.shevchenko@gmail.com> escribió:
> 
> On Sat, Mar 6, 2021 at 5:57 PM Álvaro Fernández Rojas <noltari@gmail.com> wrote:
>> 
>> Add a helper for registering BCM63XX pin controllers.
> 
> Thanks for this, but I think we may use the fwnode API. See below.
> 
> ...
> 
>> +#include <linux/gpio/regmap.h>
>> +#include <linux/mfd/syscon.h>
> 
>> +#include <linux/of.h>
> 
> + property.h
> + mod_devicetable.h
> 
>> +#include <linux/platform_device.h>
>> +
>> +#include "pinctrl-bcm63xx.h"
> 
>> +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 int bcm63xx_gpio_probe(struct device *dev, struct device_node *node,
> 
> device_node *node -> fwnode_handle *fwnode
> 
>> +                             const struct bcm63xx_pinctrl_soc *soc,
>> +                             struct bcm63xx_pinctrl *pc)
>> +{
>> +       struct gpio_regmap_config grc = {0};
>> +
>> +       grc.parent = dev;
> 
>> +       grc.fwnode = &node->fwnode;
> 
> grc.fwnode = fwnode;
> 
>> +       grc.ngpio = soc->ngpios;
>> +       grc.ngpio_per_reg = BCM63XX_BANK_GPIOS;
>> +       grc.regmap = pc->regs;
>> +       grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;
> 
>> +       if (of_property_read_u32(node, "data", &grc.reg_dat_base))
> 
> fwnode_property_read_u32()
> 
>> +               grc.reg_dat_base = BCM63XX_DATA_REG;
>> +       grc.reg_set_base = grc.reg_dat_base;
> 
>> +       if (of_property_read_u32(node, "dirout", &grc.reg_dir_out_base))
> 
> Ditto.
> 
>> +               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 device_node *node;
> 
> struct fwnode_handle *fwnode;
> 
>> +       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);
> 
>> +       for_each_child_of_node(dev->of_node, node) {
> 
> device_for_each_child_node(dev, fwnode) {
> 
>> +               if (of_match_node(bcm63xx_gpio_of_match, node)) {
> 
> // for now, since we have not an analogue (yet)
> node ==> to_of_node(fwnode)

So you want me to convert everything to fwnode, but then I would need to use of_node here…
It makes more sense to me to use of_node for now and convert it to fwnode in the future…
@Linus, what do you think?

> 
>> +                       err = bcm63xx_gpio_probe(dev, node, soc, pc);
> 
> ...(dev, fwnode, soc, pc);
> 
>> +                       if (err) {
>> +                               dev_err(dev, "could not add GPIO chip\n");
> 
>> +                               of_node_put(node);
> 
> fwnode_handle_put(fwnode);
> 
>> +                               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
>> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

Best regards,
Álvaro.
Linus Walleij March 10, 2021, 11:59 p.m. UTC | #3
On Wed, Mar 10, 2021 at 8:25 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:

> > // for now, since we have not an analogue (yet)

> > node ==> to_of_node(fwnode)
>
> So you want me to convert everything to fwnode, but then I would need to use of_node here…
> It makes more sense to me to use of_node for now and convert it to fwnode in the future…
> @Linus, what do you think?

I am aware of the ambition to use fwnode more to more things.

To me it is most important on things that will potentially run with
both OF and ACPI.
Typical example: ARM Qualcomm SoCs. New drivers for Aarch64 platforms.

This is a legacy MIPS platform, and we have tons of legacy platforms for ARM
etc which will never get converted to fwnode, sadly.

Is it realistic that these MIPS platforms will run ACPI in addition to
OF? Is ACPI even available on MIPS? Isn't OF the preferred HW description
language for anything MIPS?

Yours,
Linus Walleij
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..0a8906e4261b
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
@@ -0,0 +1,112 @@ 
+// 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 device_node *node,
+			      const struct bcm63xx_pinctrl_soc *soc,
+			      struct bcm63xx_pinctrl *pc)
+{
+	struct gpio_regmap_config grc = {0};
+
+	grc.parent = dev;
+	grc.fwnode = &node->fwnode;
+	grc.ngpio = soc->ngpios;
+	grc.ngpio_per_reg = BCM63XX_BANK_GPIOS;
+	grc.regmap = pc->regs;
+	grc.reg_mask_xlate = bcm63xx_reg_mask_xlate;
+
+	if (of_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 (of_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 device_node *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);
+
+	for_each_child_of_node(dev->of_node, node) {
+		if (of_match_node(bcm63xx_gpio_of_match, node)) {
+			err = bcm63xx_gpio_probe(dev, node, soc, pc);
+			if (err) {
+				dev_err(dev, "could not add GPIO chip\n");
+				of_node_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__ */