Re: [PATCH 2/7] pinctrl: bcm: Add STB family pin controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hi,

Am 31.07.24 um 08:28 schrieb Ivan T. Ivanov:

This driver provide pin muxing and configuration functionality
for BCM2712 SoC used by RPi5. According to [1] this chips is
incarnation of chip used in Broadcom STB  product line.

[1] https://lore.kernel.org/lkml/f6601f73-cb22-4ba3-88c5-241be8421fc3@xxxxxxxxxxxx/

Cc: Jonathan Bell <jonathan@xxxxxxxxxxxxxxx>
Cc: Phil Elwell <phil@xxxxxxxxxxxxxxx>
Signed-off-by: Ivan T. Ivanov <iivanov@xxxxxxx>
---
  drivers/pinctrl/bcm/Kconfig           |   13 +
  drivers/pinctrl/bcm/Makefile          |    1 +
  drivers/pinctrl/bcm/pinctrl-brcmstb.c | 1217 +++++++++++++++++++++++++
  3 files changed, 1231 insertions(+)
  create mode 100644 drivers/pinctrl/bcm/pinctrl-brcmstb.c

diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 35b51ce4298e..2e1b8d444f58 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -3,6 +3,19 @@
  # Broadcom pinctrl drivers
  #

+config PINCTRL_BRCMSTB
+	tristate "Broadcom STB product line pin controller driver"
+	depends on OF && (ARCH_BRCMSTB || COMPILE_TEST)
+	select PINMUX
+	select PINCONF
+	select GENERIC_PINCONF
+	help
+	  This driver provide pin muxing and configuration functionality
+	  for Broadcom STB product line chipsets. BCM2712 SoC is one of these
+	  chipsets.
+
+	  If M is selected the module will be called pinctrl-brcmstb.
+
  config PINCTRL_BCM281XX
  	bool "Broadcom BCM281xx pinctrl driver"
  	depends on OF && (ARCH_BCM_MOBILE || COMPILE_TEST)
diff --git a/drivers/pinctrl/bcm/Makefile b/drivers/pinctrl/bcm/Makefile
index 82b868ec1471..5f790c14cc4c 100644
--- a/drivers/pinctrl/bcm/Makefile
+++ b/drivers/pinctrl/bcm/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_PINCTRL_BCM6358)		+= pinctrl-bcm6358.o
  obj-$(CONFIG_PINCTRL_BCM6362)		+= pinctrl-bcm6362.o
  obj-$(CONFIG_PINCTRL_BCM6368)		+= pinctrl-bcm6368.o
  obj-$(CONFIG_PINCTRL_BCM63268)		+= pinctrl-bcm63268.o
+obj-$(CONFIG_PINCTRL_BRCMSTB)		+= pinctrl-brcmstb.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-brcmstb.c b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
new file mode 100644
index 000000000000..143c8c2e1d6d
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-brcmstb.c
@@ -0,0 +1,1217 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Broadcom brcmstb GPIO units (pinctrl only)
+ *

Maybe you want to add your copyright here

+ * Copyright (C) 2021-3 Raspberry Pi Ltd.
+ * Copyright (C) 2012 Chris Boot, Simon Arlott, Stephen Warren
+ *
+ * Based heavily on the BCM2835 GPIO & pinctrl driver, which was inspired by:
+ * pinctrl-nomadik.c, please see original file for copyright information
+ * pinctrl-tegra.c, please see original file for copyright information
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define BRCMSTB_PULL_NONE	0
+#define BRCMSTB_PULL_DOWN	1
+#define BRCMSTB_PULL_UP		2
+#define BRCMSTB_PULL_MASK	0x3
+
+#define BRCMSTB_FSEL_COUNT	9
+#define BRCMSTB_FSEL_MASK	0xf
+
+#define FUNC(f) \
+	[func_##f] = #f
+
+#define PIN(i, f1, f2, f3, f4, f5, f6, f7, f8) \
+	[i] = { \
+		.funcs = { \
+			func_##f1, \
+			func_##f2, \
+			func_##f3, \
+			func_##f4, \
+			func_##f5, \
+			func_##f6, \
+			func_##f7, \
+			func_##f8, \
+		}, \
+	}
+
+#define MUX_BIT_VALID		0x8000
+#define REG_BIT_INVALID		0xffff
+
+#define BIT_TO_REG(b)		(((b) >> 5) << 2)
+#define BIT_TO_SHIFT(b)	((b) & 0x1f)
+
+#define MUX_BIT(mr, mb)		(MUX_BIT_VALID + ((mr) * 4) * 8 + (mb) * 4)
+#define GPIO_REGS(n, mr, mb, pr, pb) \
+	[n] = { MUX_BIT(mr, mb), ((pr) * 4) * 8 + (pb) * 2 }

Please avoid these cryptic names: mr, mb, pr, pb

+
+#define EMMC_REGS(n, pr, pb) \
+	[n] = { 0, ((pr) * 4) * 8 + (pb) * 2 }
+
+#define AGPIO_REGS(n, mr, mb, pr, pb) \
+	[n] = { MUX_BIT(mr, mb), ((pr) * 4) * 8 + (pb) * 2 }
+
+#define SGPIO_REGS(n, mr, mb) \
+	[(n) + 32] = { MUX_BIT(mr, mb), REG_BIT_INVALID }
+
+#define GPIO_PIN(a)		PINCTRL_PIN(a, "gpio" #a)
+#define AGPIO_PIN(a)		PINCTRL_PIN(a, "aon_gpio" #a)
+#define SGPIO_PIN(a)		PINCTRL_PIN((a) + 32, "aon_sgpio" #a)
+
+struct pin_regs {
+	u16 mux_bit;
+	u16 pad_bit;
+};
+
+struct brcmstb_pin_funcs {
+	u8 funcs[BRCMSTB_FSEL_COUNT - 1];
+};
+
+struct brcmstb_pinctrl {
+	struct device *dev;
+	void __iomem *base;
+	struct pinctrl_dev *pctl_dev;
+	struct pinctrl_desc pctl_desc;
+	const struct pin_regs *pin_regs;
+	const struct brcmstb_pin_funcs *pin_funcs;
+	const char *const *gpio_groups;
+	struct pinctrl_gpio_range gpio_range;
+	/* Protect FSEL registers */
+	spinlock_t lock;

fsel_lock ?

+};
+
...
+
+static const struct brcmstb_pin_funcs bcm2712_d0_aon_gpio_pin_funcs[] = {
+	PIN(0, ir_in, vc_spi0, vc_uart0, vc_i2c3, uart0, vc_i2c0, _, _),
+	PIN(1, vc_pwm0, vc_spi0, vc_uart0, vc_i2c3, uart0, aon_pwm, vc_i2c0, vc_pwm1),
+	PIN(2, vc_pwm0, vc_spi0, vc_uart0, ctl_hdmi_5v, uart0, aon_pwm, ir_in, vc_pwm1),
+	PIN(3, ir_in, vc_spi0, vc_uart0, uart0, sd_card_g, aon_gpclk, _, _),
+	PIN(4, gpclk0, vc_spi0, pm_led_out, aon_pwm, sd_card_g, vc_pwm0, _, _),
+	PIN(5, gpclk1, ir_in, aon_pwm, sd_card_g, vc_pwm0, _, _, _),
+	PIN(6, uart1, vc_uart2, ctl_hdmi_5v, gpclk2, vc_spi3, _, _, _),
+	PIN(7, _, _, _, _, _, _, _, _),

In case this pin is reserved or something a comment would be nice

+	PIN(8, uart1, vc_uart2, ctl_hdmi_5v, vc_spi0, vc_spi3, _, _, _),
+	PIN(9, uart1, vc_uart2, vc_uart0, aon_pwm, vc_spi0, vc_uart2, vc_spi3, _),
+	PIN(10, _, _, _, _, _, _, _, _),
+	PIN(11, _, _, _, _, _, _, _, _),

same as above

+	PIN(12, uart1, vc_uart2, vc_uart0, vc_spi0, usb_pwr, vc_uart2, vc_spi3, _),
+	PIN(13, bsc_m1, vc_uart0, uui, vc_spi0, arm_jtag, vc_uart2, vc_i2c3, _),
+	PIN(14, bsc_m1, aon_gpclk, vc_uart0, uui, vc_spi0, arm_jtag, vc_uart2, vc_i2c3),
+};
+
...
+
+static inline u32 brcmstb_reg_rd(struct brcmstb_pinctrl *pc, unsigned int reg)
+{
+	return readl(pc->base + reg);
+}
+
+static inline void brcmstb_reg_wr(struct brcmstb_pinctrl *pc, unsigned int reg,
+				  u32 val)
+{
+	writel(val, pc->base + reg);
+}
+
+static enum brcmstb_funcs brcmstb_pinctrl_fsel_get(struct brcmstb_pinctrl *pc,
+						   unsigned int pin)
+{
+	u32 bit = pc->pin_regs[pin].mux_bit;
+	enum brcmstb_funcs func;
+	int fsel;
+	u32 val;
+
+	if (!bit)
+		return func_gpio;
+
+	bit &= ~MUX_BIT_VALID;
+
+	val = brcmstb_reg_rd(pc, BIT_TO_REG(bit));
+	fsel = (val >> BIT_TO_SHIFT(bit)) & BRCMSTB_FSEL_MASK;
+	func = pc->pin_funcs[pin].funcs[fsel];
+
+	if (func >= func_count)
+		func = (enum brcmstb_funcs)fsel;
+
+	dev_dbg(pc->dev, "get %04x: %08x (%u => %s)\n",
+		BIT_TO_REG(bit), val, pin,
+		brcmstb_func_names[func]);
+
+	return func;
+}
+
+static void brcmstb_pinctrl_fsel_set(struct brcmstb_pinctrl *pc,
+				     unsigned int pin, enum brcmstb_funcs func)

I would prefer this function returns errors as int instead of silently
ignore them

+{
+	u32 bit = pc->pin_regs[pin].mux_bit, val;
+	const u8 *pin_funcs;
+	unsigned long flags;
+	int fsel;
+	int cur;
+	int i;
+
+	if (!bit || func >= func_count)
+		return;
+
+	bit &= ~MUX_BIT_VALID;
+
+	fsel = BRCMSTB_FSEL_COUNT;
+
+	if (func >= BRCMSTB_FSEL_COUNT) {
+		/* Convert to an fsel number */
+		pin_funcs = pc->pin_funcs[pin].funcs;
+		for (i = 1; i < BRCMSTB_FSEL_COUNT; i++) {
+			if (pin_funcs[i - 1] == func) {
+				fsel = i;
+				break;
+			}
+		}
+	} else {
+		fsel = (enum brcmstb_funcs)func;
+	}
+
+	if (fsel >= BRCMSTB_FSEL_COUNT)
+		return;
+
+	spin_lock_irqsave(&pc->lock, flags);
+
+	val = brcmstb_reg_rd(pc, BIT_TO_REG(bit));
+	cur = (val >> BIT_TO_SHIFT(bit)) & BRCMSTB_FSEL_MASK;
+
+	dev_dbg(pc->dev, "read %04x: %08x (%u => %s)\n",
+		BIT_TO_REG(bit), val, pin,
+		brcmstb_func_names[cur]);
+
+	if (cur != fsel) {
+		val &= ~(BRCMSTB_FSEL_MASK << BIT_TO_SHIFT(bit));
+		val |= fsel << BIT_TO_SHIFT(bit);
+
+		dev_dbg(pc->dev, "write %04x: %08x (%u <= %s)\n",
+			BIT_TO_REG(bit), val, pin,
+			brcmstb_func_names[fsel]);
+		brcmstb_reg_wr(pc, BIT_TO_REG(bit), val);
+	}
+
+	spin_unlock_irqrestore(&pc->lock, flags);
+}
+
+static int brcmstb_pctl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	return pc->pctl_desc.npins;
+}
+
+static const char *brcmstb_pctl_get_group_name(struct pinctrl_dev *pctldev,
+					       unsigned int selector)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	return pc->gpio_groups[selector];
+}
+
+static int brcmstb_pctl_get_group_pins(struct pinctrl_dev *pctldev,
+				       unsigned int selector,
+				       const unsigned int **pins,
+				       unsigned int *num_pins)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	*pins = &pc->pctl_desc.pins[selector].number;
+	*num_pins = 1;
+
+	return 0;
+}
+
+static void brcmstb_pctl_pin_dbg_show(struct pinctrl_dev *pctldev,
+				      struct seq_file *s, unsigned int offset)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum brcmstb_funcs fsel = brcmstb_pinctrl_fsel_get(pc, offset);
+	const char *fname = brcmstb_func_names[fsel];
+
+	seq_printf(s, "function %s", fname);
+}
+
+static void brcmstb_pctl_dt_free_map(struct pinctrl_dev *pctldev,
+				     struct pinctrl_map *maps,
+				     unsigned int num_maps)
+{
+	int i;
+
+	for (i = 0; i < num_maps; i++)
+		if (maps[i].type == PIN_MAP_TYPE_CONFIGS_PIN)
+			kfree(maps[i].data.configs.configs);
+
+	kfree(maps);
+}
+
+static const struct pinctrl_ops brcmstb_pctl_ops = {
+	.get_groups_count = brcmstb_pctl_get_groups_count,
+	.get_group_name = brcmstb_pctl_get_group_name,
+	.get_group_pins = brcmstb_pctl_get_group_pins,
+	.pin_dbg_show = brcmstb_pctl_pin_dbg_show,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+	.dt_free_map = brcmstb_pctl_dt_free_map,
+};
+
+static int brcmstb_pmx_free(struct pinctrl_dev *pctldev, unsigned int offset)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	/* disable by setting to GPIO */
+	brcmstb_pinctrl_fsel_set(pc, offset, func_gpio);
+	return 0;
+}
+
+static int brcmstb_pmx_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	return func_count;
+}
+
+static const char *brcmstb_pmx_get_function_name(struct pinctrl_dev *pctldev,
+						 unsigned int selector)
+{
+	return (selector < func_count) ? brcmstb_func_names[selector] : NULL;
+}
+
+static int brcmstb_pmx_get_function_groups(struct pinctrl_dev *pctldev,
+					   unsigned int selector,
+					   const char *const **groups,
+					   unsigned *const num_groups)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = pc->gpio_groups;
+	*num_groups = pc->pctl_desc.npins;
+
+	return 0;
+}
+
+static int brcmstb_pmx_set(struct pinctrl_dev *pctldev,
+			   unsigned int func_selector,
+			   unsigned int group_selector)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	const struct pinctrl_desc *pctldesc = &pc->pctl_desc;
+	const struct pinctrl_pin_desc *pindesc;
+
+	if (group_selector >= pctldesc->npins)
+		return -EINVAL;
+
+	pindesc = &pctldesc->pins[group_selector];
+	brcmstb_pinctrl_fsel_set(pc, pindesc->number, func_selector);

return brcmstb_pinctrl_fsel_set

+
+	return 0;
+}
+
+static int brcmstb_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
+					   struct pinctrl_gpio_range *range,
+					   unsigned int pin)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	brcmstb_pinctrl_fsel_set(pc, pin, func_gpio);
+
+	return 0;

return brcmstb_pinctrl_fsel_set

+}
+
+static void brcmstb_pmx_gpio_disable_free(struct pinctrl_dev *pctldev,
+					  struct pinctrl_gpio_range *range,
+					  unsigned int offset)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	/* disable by setting to GPIO */
+	brcmstb_pinctrl_fsel_set(pc, offset, func_gpio);
+}
+
+static const struct pinmux_ops brcmstb_pmx_ops = {
+	.free = brcmstb_pmx_free,
+	.get_functions_count = brcmstb_pmx_get_functions_count,
+	.get_function_name = brcmstb_pmx_get_function_name,
+	.get_function_groups = brcmstb_pmx_get_function_groups,
+	.set_mux = brcmstb_pmx_set,
+	.gpio_request_enable = brcmstb_pmx_gpio_request_enable,
+	.gpio_disable_free = brcmstb_pmx_gpio_disable_free,
+};
+
+static unsigned int brcmstb_pull_config_get(struct brcmstb_pinctrl *pc,
+					    unsigned int pin)
+{
+	u32 bit = pc->pin_regs[pin].pad_bit, val;
+
+	if (bit == REG_BIT_INVALID)
+		return BRCMSTB_PULL_NONE;
+
+	val = brcmstb_reg_rd(pc, BIT_TO_REG(bit));
+	return (val >> BIT_TO_SHIFT(bit)) & BRCMSTB_PULL_MASK;
+}
+
+static void brcmstb_pull_config_set(struct brcmstb_pinctrl *pc,
+				    unsigned int pin, unsigned int arg)

The same here, returning an error would be better

+{
+	u32 bit = pc->pin_regs[pin].pad_bit, val;
+	unsigned long flags;
+
+	if (bit == REG_BIT_INVALID) {
+		dev_warn(pc->dev, "Can't set pulls for %s\n",
+			 pc->gpio_groups[pin]);
+		return;
+	}
+
+	spin_lock_irqsave(&pc->lock, flags);
+
+	val = brcmstb_reg_rd(pc, BIT_TO_REG(bit));
+	val &= ~(BRCMSTB_PULL_MASK << BIT_TO_SHIFT(bit));
+	val |= (arg << BIT_TO_SHIFT(bit));
+	brcmstb_reg_wr(pc, BIT_TO_REG(bit), val);
+
+	spin_unlock_irqrestore(&pc->lock, flags);
+}
+
+static int brcmstb_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
+			       unsigned long *config)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	u32 arg;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		arg = (brcmstb_pull_config_get(pc, pin) == BRCMSTB_PULL_NONE);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		arg = (brcmstb_pull_config_get(pc, pin) == BRCMSTB_PULL_DOWN);
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = (brcmstb_pull_config_get(pc, pin) == BRCMSTB_PULL_UP);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+
+	return 0;
+}
+
+static int brcmstb_pinconf_set(struct pinctrl_dev *pctldev,
+			       unsigned int pin, unsigned long *configs,
+			       unsigned int num_configs)
+{
+	struct brcmstb_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	u32 param, arg;
+	int i;
+
+	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:
+			brcmstb_pull_config_set(pc, pin, BRCMSTB_PULL_NONE);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			brcmstb_pull_config_set(pc, pin, BRCMSTB_PULL_DOWN);
+			break;
+		case PIN_CONFIG_BIAS_PULL_UP:
+			brcmstb_pull_config_set(pc, pin, BRCMSTB_PULL_UP);

In case brcmstb_pull_config_set would return an error, we shouldn't
ignore it here

Thanks

+			break;
+		default:
+			return -ENOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
+
[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux