2020-05-13 14:13:35

by Lars Povlsen

[permalink] [raw]
Subject: [PATCH 0/3] pinctrl: Adding support for Microchip serial GPIO controller

This is an add-on series to the main SoC Sparx5 series
(Message-ID: <[email protected]>).

The series add support for the serial GPIO controller used by Sparx5,
as well as (MSCC) ocelot/jaguar2.

The GPIO controller only supports output mode currently.

It is expected that the DT patches are to be taken directly by the arm-soc
maintainers.

Lars Povlsen (3):
dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio
pinctrl: mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO
arm64: dts: sparx5: Add SGPIO devices

.../bindings/pinctrl/mscc,ocelot-sgpio.yaml | 66 ++
MAINTAINERS | 2 +
arch/arm64/boot/dts/microchip/sparx5.dtsi | 52 ++
.../boot/dts/microchip/sparx5_pcb125.dts | 5 +
.../dts/microchip/sparx5_pcb134_board.dtsi | 5 +
drivers/pinctrl/Kconfig | 17 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-mchp-sgpio.c | 569 ++++++++++++++++++
include/dt-bindings/gpio/mchp-sgpio.h | 21 +
9 files changed, 738 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml
create mode 100644 drivers/pinctrl/pinctrl-mchp-sgpio.c
create mode 100644 include/dt-bindings/gpio/mchp-sgpio.h

--
2.26.2


2020-05-13 14:13:41

by Lars Povlsen

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio

This adds DT bindings for the Microsemi SGPIO controller, bindings
mscc,ocelot-sgpio and mscc,luton-sgpio.

Reviewed-by: Alexandre Belloni <[email protected]>
Signed-off-by: Lars Povlsen <[email protected]>
---
.../bindings/pinctrl/mscc,ocelot-sgpio.yaml | 66 +++++++++++++++++++
MAINTAINERS | 1 +
include/dt-bindings/gpio/mchp-sgpio.h | 21 ++++++
3 files changed, 88 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml
create mode 100644 include/dt-bindings/gpio/mchp-sgpio.h

diff --git a/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml b/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml
new file mode 100644
index 0000000000000..a332a0f4582fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/mscc,ocelot-sgpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microsemi Serial GPIO controller
+
+maintainers:
+ - Lars Povlsen <[email protected]>
+
+description: |
+ By using a serial interface, the SIO controller significantly extend
+ the number of available GPIOs with a minimum number of additional
+ pins on the device. The primary purpose of the SIO controllers is to
+ connect control signals from SFP modules and to act as an LED
+ controller.
+
+properties:
+ $nodename:
+ pattern: "gpio"
+
+ compatible:
+ enum:
+ - mscc,ocelot-sgpio
+ - mscc,luton-sgpio
+
+ clocks:
+ maxItems: 1
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ description: Specifying the pin number and flags, as defined in
+ include/dt-bindings/gpio/gpio.h
+ const: 2
+
+ gpio-ranges:
+ maxItems: 1
+
+ microchip,sgpio-ports:
+ description: This is a 32-bit bitmask, configuring whether a
+ particular port in the controller is enabled or not. This allows
+ unused ports to be removed from the bitstream and reduce latency.
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+
+required:
+ - compatible
+ - clocks
+ - gpio-controller
+ - '#gpio-cells'
+ - gpio-ranges
+
+examples:
+ - |
+ sgpio0: gpio@61101036c {
+ compatible = "mscc,ocelot-sgpio";
+ clocks = <&sys_clk>;
+ pinctrl-0 = <&sgpio0_pins>;
+ pinctrl-names = "default";
+ reg = <0x1101036c 0x100>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&gpio 0 64 64>;
+ microchip,sgpio-ports = <0x00ffffff>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index de64fd4548697..cdb63ca04670d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11233,6 +11233,7 @@ S: Supported
F: Documentation/devicetree/bindings/mips/mscc.txt
F: Documentation/devicetree/bindings/power/reset/ocelot-reset.txt
F: Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
+F: Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml
F: arch/mips/boot/dts/mscc/
F: arch/mips/configs/generic/board-ocelot.config
F: arch/mips/generic/board-ocelot.c
diff --git a/include/dt-bindings/gpio/mchp-sgpio.h b/include/dt-bindings/gpio/mchp-sgpio.h
new file mode 100644
index 0000000000000..0736158563f0a
--- /dev/null
+++ b/include/dt-bindings/gpio/mchp-sgpio.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * This header provides constants for binding mscc,*-sgpio
+ *
+ * The first cell in the SGPIO specifier is the GPIO ID. The macros below
+ * provide machros for this.
+ *
+ * The second cell contains standard flag values specified in gpio.h.
+ */
+
+#ifndef _DT_BINDINGS_GPIO_MSCC_SGPIO_H
+#define _DT_BINDINGS_GPIO_MSCC_SGPIO_H
+
+#include <dt-bindings/gpio/gpio.h>
+
+#define MSCC_SGPIOS_PER_BANK 32
+#define MSCC_SGPIO_BANK_DEPTH 4
+
+#define MSCC_SGPIO(port, bit) ((bit * MSCC_SGPIOS_PER_BANK) + port)
+
+#endif
--
2.26.2

2020-05-13 14:14:00

by Lars Povlsen

[permalink] [raw]
Subject: [PATCH 2/3] pinctrl: mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO

This adds a pinctrl driver for the Microsemi/Microchip Serial GPIO
(SGPIO) device used in various SoC's.

Reviewed-by: Alexandre Belloni <[email protected]>
Signed-off-by: Lars Povlsen <[email protected]>
---
MAINTAINERS | 1 +
drivers/pinctrl/Kconfig | 17 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-mchp-sgpio.c | 569 +++++++++++++++++++++++++++
4 files changed, 588 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-mchp-sgpio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cdb63ca04670d..e5a7c41cdb6cd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11237,6 +11237,7 @@ F: Documentation/devicetree/bindings/pinctrl/mscc,ocelot-sgpio.yaml
F: arch/mips/boot/dts/mscc/
F: arch/mips/configs/generic/board-ocelot.config
F: arch/mips/generic/board-ocelot.c
+F: drivers/pinctrl/pinctrl-mchp-sgpio.c

MICROSEMI SMART ARRAY SMARTPQI DRIVER (smartpqi)
M: Don Brace <[email protected]>
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 834c59950d1cf..2b0e9021fd7e0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -396,6 +396,23 @@ config PINCTRL_OCELOT
select OF_GPIO
select REGMAP_MMIO

+config PINCTRL_MSCC_SGPIO
+ bool "Pinctrl driver for Microsemi Serial GPIO"
+ depends on OF
+ depends on HAS_IOMEM
+ select GPIOLIB
+ select GENERIC_PINCONF
+ select GENERIC_PINCTRL_GROUPS
+ select GENERIC_PINMUX_FUNCTIONS
+ select OF_GPIO
+ help
+ Support for the VCoreIII SoC serial GPIO device. By using a
+ serial interface, the SIO controller significantly extends
+ the number of available GPIOs with a minimum number of
+ additional pins on the device. The primary purpose of the
+ SIO controller is to connect control signals from SFP
+ modules and to act as an LED controller.
+
source "drivers/pinctrl/actions/Kconfig"
source "drivers/pinctrl/aspeed/Kconfig"
source "drivers/pinctrl/bcm/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 0b36a1cfca8a1..1146ecc373edf 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_PINCTRL_ZYNQ) += pinctrl-zynq.o
obj-$(CONFIG_PINCTRL_INGENIC) += pinctrl-ingenic.o
obj-$(CONFIG_PINCTRL_RK805) += pinctrl-rk805.o
obj-$(CONFIG_PINCTRL_OCELOT) += pinctrl-ocelot.o
+obj-$(CONFIG_PINCTRL_MSCC_SGPIO) += pinctrl-mchp-sgpio.o
obj-$(CONFIG_PINCTRL_EQUILIBRIUM) += pinctrl-equilibrium.o

obj-y += actions/
diff --git a/drivers/pinctrl/pinctrl-mchp-sgpio.c b/drivers/pinctrl/pinctrl-mchp-sgpio.c
new file mode 100644
index 0000000000000..e609f11137f48
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mchp-sgpio.c
@@ -0,0 +1,569 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Microsemi SoCs serial gpio driver
+ *
+ * Author: <[email protected]>
+ *
+ * Copyright (c) 2020 Microchip Technology Inc. and its subsidiaries.
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/gpio/mchp-sgpio.h>
+
+#include "core.h"
+#include "pinconf.h"
+#include "pinmux.h"
+
+#define PIN_NAME_LEN (sizeof("SGPIO_pXXbY")+1)
+
+enum {
+ REG_INPUT_DATA,
+ REG_PORT_CONFIG,
+ REG_PORT_ENABLE,
+ REG_SIO_CONFIG,
+ REG_SIO_CLOCK,
+ MAXREG
+};
+
+struct mchp_sgpio_bf {
+ u8 beg;
+ u8 end;
+};
+
+struct mchp_sgpio_props {
+ const char *label;
+ u8 regoff[MAXREG];
+ struct mchp_sgpio_bf auto_repeat;
+ struct mchp_sgpio_bf port_width;
+ struct mchp_sgpio_bf clk_freq;
+ struct mchp_sgpio_bf bit_source;
+};
+
+#define __M(bf) GENMASK((bf).end, (bf).beg)
+#define __F(bf, x) (__M(bf) & ((x) << (bf).beg))
+#define __X(bf, x) (((x) >> (bf).beg) & GENMASK(((bf).end - (bf).beg), 0))
+
+#define MSCC_M_CFG_SIO_AUTO_REPEAT(p) BIT(p->props->auto_repeat.beg)
+#define MSCC_F_CFG_SIO_PORT_WIDTH(p, x) __F(p->props->port_width, x)
+#define MSCC_M_CFG_SIO_PORT_WIDTH(p) __M(p->props->port_width)
+#define MSCC_F_CLOCK_SIO_CLK_FREQ(p, x) __F(p->props->clk_freq, x)
+#define MSCC_M_CLOCK_SIO_CLK_FREQ(p) __M(p->props->clk_freq)
+#define MSCC_F_PORT_CFG_BIT_SOURCE(p, x) __F(p->props->bit_source, x)
+#define MSCC_X_PORT_CFG_BIT_SOURCE(p, x) __X(p->props->bit_source, x)
+
+const struct mchp_sgpio_props props_luton = {
+ .label = "luton-sgpio",
+ .regoff = { 0x00, 0x09, 0x29, 0x2a, 0x2b },
+ .auto_repeat = { 5, 5 },
+ .port_width = { 2, 3 },
+ .clk_freq = { 0, 11 },
+ .bit_source = { 0, 11 },
+};
+
+const struct mchp_sgpio_props props_ocelot = {
+ .label = "ocelot-sgpio",
+ .regoff = { 0x00, 0x06, 0x26, 0x04, 0x05 },
+ .auto_repeat = { 10, 10 },
+ .port_width = { 7, 8 },
+ .clk_freq = { 8, 19 },
+ .bit_source = { 12, 23 },
+};
+
+static const char * const functions[] = { "gpio" };
+
+struct mchp_sgpio_priv {
+ struct device *dev;
+ struct pinctrl_dev *pctl;
+ struct gpio_chip gpio;
+ u32 bitcount;
+ u32 ports;
+ u32 clock;
+ u32 mode[MSCC_SGPIOS_PER_BANK];
+ u32 __iomem *regs;
+ struct pinctrl_desc *desc;
+ const struct mchp_sgpio_props *props;
+};
+
+static inline u32 sgpio_readl(struct mchp_sgpio_priv *priv, u32 rno, u32 off)
+{
+ u32 __iomem *reg = &priv->regs[priv->props->regoff[rno] + off];
+
+ return readl(reg);
+}
+
+static inline void sgpio_writel(struct mchp_sgpio_priv *priv,
+ u32 val, u32 rno, u32 off)
+{
+ u32 __iomem *reg = &priv->regs[priv->props->regoff[rno] + off];
+
+ writel(val, reg);
+}
+
+static void sgpio_clrsetbits(struct mchp_sgpio_priv *priv,
+ u32 rno, u32 off, u32 clear, u32 set)
+{
+ u32 __iomem *reg = &priv->regs[priv->props->regoff[rno] + off];
+ u32 val = readl(reg);
+
+ val &= ~clear;
+ val |= set;
+
+ writel(val, reg);
+}
+
+static void sgpio_output_set(struct mchp_sgpio_priv *priv,
+ int pin, int value)
+{
+ u32 port = pin % MSCC_SGPIOS_PER_BANK;
+ u32 bit = pin / MSCC_SGPIOS_PER_BANK;
+ u32 mask = 3 << (3 * bit);
+
+ dev_dbg(priv->dev, "%s: port %d, bit %d, value %d\n",
+ __func__, port, bit, value);
+
+ value = (value & 3) << (3 * bit);
+ sgpio_clrsetbits(priv, REG_PORT_CONFIG, port,
+ MSCC_F_PORT_CFG_BIT_SOURCE(priv, mask),
+ MSCC_F_PORT_CFG_BIT_SOURCE(priv, value));
+}
+
+static int sgpio_output_get(struct mchp_sgpio_priv *priv, int pin)
+{
+ u32 port = pin % MSCC_SGPIOS_PER_BANK;
+ u32 bit = pin / MSCC_SGPIOS_PER_BANK;
+ u32 portval = sgpio_readl(priv, REG_PORT_CONFIG, port);
+ int ret;
+
+ ret = MSCC_X_PORT_CFG_BIT_SOURCE(priv, portval);
+ ret = !!(ret & (3 << (3 * bit)));
+
+ dev_dbg(priv->dev, "%s: port %d, bit %d, value %d\n",
+ __func__, port, bit, ret);
+
+ return ret;
+}
+
+static int sgpio_input_get(struct mchp_sgpio_priv *priv, int pin)
+{
+ u32 port = pin % MSCC_SGPIOS_PER_BANK;
+ u32 bit = pin / MSCC_SGPIOS_PER_BANK;
+ int ret;
+
+ ret = !!(sgpio_readl(priv, REG_INPUT_DATA, bit) & BIT(port));
+
+ dev_dbg(priv->dev, "%s: port %d, bit %d, value %d\n",
+ __func__, port, bit, ret);
+
+ return ret;
+}
+
+static int sgpio_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+ u32 param = pinconf_to_config_param(*config);
+ int val;
+
+ switch (param) {
+ case PIN_CONFIG_INPUT_ENABLE:
+ val = false;
+ break;
+
+ case PIN_CONFIG_OUTPUT_ENABLE:
+ val = true;
+ break;
+
+ case PIN_CONFIG_OUTPUT:
+ val = sgpio_output_get(priv, pin);
+ break;
+
+ default:
+ return -ENOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, val);
+
+ return 0;
+}
+
+int sgpio_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *configs, unsigned int num_configs)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+ u32 param, arg;
+ int cfg, err = 0;
+
+ for (cfg = 0; cfg < num_configs; cfg++) {
+ param = pinconf_to_config_param(configs[cfg]);
+ arg = pinconf_to_config_argument(configs[cfg]);
+
+ switch (param) {
+ case PIN_CONFIG_OUTPUT:
+ sgpio_output_set(priv, pin, arg);
+ break;
+
+ default:
+ err = -ENOTSUPP;
+ }
+ }
+
+ return err;
+}
+
+static const struct pinconf_ops sgpio_confops = {
+ .is_generic = true,
+ .pin_config_get = sgpio_pinconf_get,
+ .pin_config_set = sgpio_pinconf_set,
+ .pin_config_config_dbg_show = pinconf_generic_dump_config,
+};
+
+static int sgpio_get_functions_count(struct pinctrl_dev *pctldev)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ dev_dbg(priv->dev, "%s\n", __func__);
+ return 1;
+}
+
+static const char *sgpio_get_function_name(struct pinctrl_dev *pctldev,
+ unsigned int function)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ dev_dbg(priv->dev, "%s\n", __func__);
+
+ return functions[0];
+}
+
+static int sgpio_get_function_groups(struct pinctrl_dev *pctldev,
+ unsigned int function,
+ const char *const **groups,
+ unsigned *const num_groups)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ dev_dbg(priv->dev, "%s\n", __func__);
+
+ *groups = functions;
+ *num_groups = ARRAY_SIZE(functions);
+
+ return 0;
+}
+
+static int sgpio_pinmux_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int selector, unsigned int group)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ dev_dbg(priv->dev, "%s: sel %d grp %d\n", __func__, selector, group);
+
+ return 0;
+}
+
+static int sgpio_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int pin, bool input)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ dev_dbg(priv->dev, "%s: pin %d input %d\n", __func__, pin, input);
+
+ return 0;
+}
+
+static int sgpio_gpio_request_enable(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ if ((priv->ports & BIT(offset)) == 0) {
+ dev_warn(priv->dev, "%s: Request port pin %d is not activated\n",
+ __func__, offset);
+ }
+
+ return 0;
+}
+
+static const struct pinmux_ops sgpio_pmx_ops = {
+ .get_functions_count = sgpio_get_functions_count,
+ .get_function_name = sgpio_get_function_name,
+ .get_function_groups = sgpio_get_function_groups,
+ .set_mux = sgpio_pinmux_set_mux,
+ .gpio_set_direction = sgpio_gpio_set_direction,
+ .gpio_request_enable = sgpio_gpio_request_enable,
+};
+
+static int sgpio_pctl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ return priv->desc->npins;
+}
+
+static const char *sgpio_pctl_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int group)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ return priv->desc->pins[group].name;
+}
+
+static int sgpio_pctl_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct mchp_sgpio_priv *priv = pinctrl_dev_get_drvdata(pctldev);
+
+ *pins = &priv->desc->pins[group].number;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const struct pinctrl_ops sgpio_pctl_ops = {
+ .get_groups_count = sgpio_pctl_get_groups_count,
+ .get_group_name = sgpio_pctl_get_group_name,
+ .get_group_pins = sgpio_pctl_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+ .dt_free_map = pinconf_generic_dt_free_map,
+};
+
+static struct pinctrl_desc sgpio_desc = {
+ .name = "sgpio-pinctrl",
+ .pctlops = &sgpio_pctl_ops,
+ .pmxops = &sgpio_pmx_ops,
+ .confops = &sgpio_confops,
+ .owner = THIS_MODULE,
+};
+
+static int mchp_sgpio_direction_input(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct mchp_sgpio_priv *priv = gpiochip_get_data(gc);
+
+ u32 port = gpio % MSCC_SGPIOS_PER_BANK;
+ u32 bit = gpio / MSCC_SGPIOS_PER_BANK;
+
+ /* Set mode => input mode */
+ priv->mode[port] |= BIT(bit);
+
+ return 0;
+}
+
+static int mchp_sgpio_direction_output(struct gpio_chip *gc,
+ unsigned int gpio, int value)
+{
+ struct mchp_sgpio_priv *priv = gpiochip_get_data(gc);
+ u32 port = gpio % MSCC_SGPIOS_PER_BANK;
+ u32 bit = gpio / MSCC_SGPIOS_PER_BANK;
+
+ sgpio_output_set(priv, gpio, value);
+
+ /* Clear mode => output mode */
+ priv->mode[port] &= ~BIT(bit);
+
+ return 0;
+}
+
+static int mchp_sgpio_get_function(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct mchp_sgpio_priv *priv = gpiochip_get_data(gc);
+ u32 port = gpio % MSCC_SGPIOS_PER_BANK;
+ u32 bit = gpio / MSCC_SGPIOS_PER_BANK;
+ u32 val = priv->mode[port] & BIT(bit);
+
+ return !!val; /* 0=out, 1=in */
+}
+
+static void mchp_sgpio_set_value(struct gpio_chip *gc,
+ unsigned int gpio, int value)
+{
+ mchp_sgpio_direction_output(gc, gpio, value);
+}
+
+static int mchp_sgpio_get_value(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct mchp_sgpio_priv *priv = gpiochip_get_data(gc);
+ const struct pinctrl_pin_desc *pin = &priv->desc->pins[gpio];
+ int ret;
+
+ if (mchp_sgpio_get_function(gc, gpio))
+ ret = sgpio_input_get(priv, gpio);
+ else
+ ret = sgpio_output_get(priv, gpio);
+
+ dev_dbg(priv->dev, "get: gpio %d (%s), value %d\n",
+ gpio, pin->name, ret);
+
+ return ret;
+}
+
+static int mchp_sgpio_get_count(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct of_phandle_args pinspec;
+ int count, index, ret;
+
+ for (count = 0, index = 0;; index++) {
+ ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
+ index, &pinspec);
+ if (ret)
+ break;
+
+ dev_dbg(dev, "%s: Add %d gpios\n", __func__, pinspec.args[2]);
+ count += pinspec.args[2];
+ }
+ dev_dbg(dev, "%s: Have %d gpios\n", __func__, count);
+ return count;
+}
+
+static int mchp_sgpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mchp_sgpio_priv *priv;
+ int div_clock = 0, ret, port;
+ u32 val, ngpios;
+ struct resource *regs;
+ struct clk *clk;
+ struct pinctrl_pin_desc *pins;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk)) {
+ dev_err(dev, "Failed to get clock\n");
+ return PTR_ERR(clk);
+ }
+ div_clock = clk_get_rate(clk);
+
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->regs = devm_ioremap_resource(dev, regs);
+ if (IS_ERR(priv->regs))
+ return PTR_ERR(priv->regs);
+ priv->props = of_device_get_match_data(dev);
+ if (device_property_read_u32(dev, "microchip,sgpio-ports",
+ &priv->ports))
+ priv->ports = 0xFFFFFFFF;
+ if (device_property_read_u32(dev, "microchip,sgpio-frequency",
+ &priv->clock))
+ priv->clock = 12500000;
+ if (priv->clock <= 0 || priv->clock > div_clock) {
+ dev_err(dev, "Invalid frequency %d\n", priv->clock);
+ return -EINVAL;
+ }
+ ngpios = mchp_sgpio_get_count(dev);
+ if (ngpios < 1 ||
+ ngpios > (MSCC_SGPIO_BANK_DEPTH * MSCC_SGPIOS_PER_BANK)) {
+ dev_err(dev, "Invalid gpio count %d\n", ngpios);
+ return -EINVAL;
+ }
+
+ pins = devm_kzalloc(dev, sizeof(*pins)*ngpios, GFP_KERNEL);
+ if (pins) {
+ int i;
+ char *p, *names;
+
+ names = devm_kzalloc(dev, PIN_NAME_LEN*ngpios, GFP_KERNEL);
+
+ if (!names)
+ return -ENOMEM;
+
+ sgpio_desc.npins = ngpios;
+ sgpio_desc.pins = pins;
+
+ for (p = names, i = 0; i < ngpios; i++, p += PIN_NAME_LEN) {
+ snprintf(p, PIN_NAME_LEN, "SGPIO_p%db%d",
+ i % MSCC_SGPIOS_PER_BANK,
+ i / MSCC_SGPIOS_PER_BANK);
+ pins[i].number = i;
+ pins[i].name = p;
+ }
+ } else
+ return -ENOMEM;
+
+ priv->desc = &sgpio_desc;
+ priv->pctl = devm_pinctrl_register(&pdev->dev, priv->desc, priv);
+ if (IS_ERR(priv->pctl)) {
+ dev_err(&pdev->dev, "Failed to register pinctrl\n");
+ return PTR_ERR(priv->pctl);
+ }
+
+ priv->gpio.label = priv->props->label;
+ priv->gpio.parent = dev;
+ priv->gpio.of_node = dev->of_node;
+ priv->gpio.owner = THIS_MODULE;
+ priv->gpio.get_direction = mchp_sgpio_get_function;
+ priv->gpio.direction_input = mchp_sgpio_direction_input;
+ priv->gpio.direction_output = mchp_sgpio_direction_output;
+ priv->gpio.get = mchp_sgpio_get_value,
+ priv->gpio.set = mchp_sgpio_set_value;
+ priv->gpio.request = gpiochip_generic_request;
+ priv->gpio.free = gpiochip_generic_free;
+ priv->gpio.base = -1;
+ priv->gpio.ngpio = ngpios;
+
+ priv->bitcount = DIV_ROUND_UP(ngpios, MSCC_SGPIOS_PER_BANK);
+ dev_dbg(dev, "probe: gpios = %d, bit-count = %d\n",
+ ngpios, priv->bitcount);
+
+ sgpio_clrsetbits(priv, REG_SIO_CONFIG, 0,
+ MSCC_M_CFG_SIO_PORT_WIDTH(priv),
+ MSCC_F_CFG_SIO_PORT_WIDTH(priv, priv->bitcount - 1) |
+ MSCC_M_CFG_SIO_AUTO_REPEAT(priv));
+ val = div_clock / priv->clock;
+ dev_dbg(dev, "probe: div-clock = %d KHz, freq = %d KHz, div = %d\n",
+ div_clock / 1000, priv->clock / 1000, val);
+ sgpio_clrsetbits(priv, REG_SIO_CLOCK, 0,
+ MSCC_M_CLOCK_SIO_CLK_FREQ(priv),
+ MSCC_F_CLOCK_SIO_CLK_FREQ(priv, val));
+
+ for (port = 0; port < MSCC_SGPIOS_PER_BANK; port++)
+ sgpio_writel(priv, 0, REG_PORT_CONFIG, port);
+ sgpio_writel(priv, priv->ports, REG_PORT_ENABLE, 0);
+
+ ret = devm_gpiochip_add_data(dev, &priv->gpio, priv);
+ if (ret == 0)
+ dev_info(dev, "Registered %d GPIOs\n", ngpios);
+ else
+ dev_err(dev, "Failed to register: ret %d\n", ret);
+ return ret;
+}
+
+static const struct of_device_id mchp_sgpio_gpio_of_match[] = {
+ {
+ .compatible = "mscc,luton-sgpio",
+ .data = &props_luton,
+ }, {
+ .compatible = "mscc,ocelot-sgpio",
+ .data = &props_ocelot,
+ }, {
+ /* sentinel */
+ }
+};
+
+static struct platform_driver mchp_sgpio_pinctrl_driver = {
+ .driver = {
+ .name = "pinctrl-mchp-sgpio",
+ .of_match_table = mchp_sgpio_gpio_of_match,
+ .suppress_bind_attrs = true,
+ },
+ .probe = mchp_sgpio_probe,
+};
+builtin_platform_driver(mchp_sgpio_pinctrl_driver);
--
2.26.2

2020-05-13 14:14:12

by Lars Povlsen

[permalink] [raw]
Subject: [PATCH 3/3] arm64: dts: sparx5: Add SGPIO devices

This adds SGPIO devices for the Sparx5 SoC and configures it for the
applicable reference boards.

Reviewed-by: Alexandre Belloni <[email protected]>
Signed-off-by: Lars Povlsen <[email protected]>
---
arch/arm64/boot/dts/microchip/sparx5.dtsi | 52 +++++++++++++++++++
.../boot/dts/microchip/sparx5_pcb125.dts | 5 ++
.../dts/microchip/sparx5_pcb134_board.dtsi | 5 ++
3 files changed, 62 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 60629861a5157..b4fda5616536c 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -213,6 +213,22 @@ si2_pins: si2-pins {
function = "si2";
};

+ sgpio0_pins: sgpio-pins {
+ pins = "GPIO_0", "GPIO_1", "GPIO_2", "GPIO_3";
+ function = "sg0";
+ };
+
+ sgpio1_pins: sgpio1-pins {
+ pins = "GPIO_4", "GPIO_5", "GPIO_12", "GPIO_13";
+ function = "sg1";
+ };
+
+ sgpio2_pins: sgpio2-pins {
+ pins = "GPIO_30", "GPIO_31", "GPIO_32",
+ "GPIO_33";
+ function = "sg2";
+ };
+
uart_pins: uart-pins {
pins = "GPIO_10", "GPIO_11";
function = "uart";
@@ -243,6 +259,42 @@ emmc_pins: emmc-pins {
};
};

+ sgpio0: gpio@61101036c {
+ compatible = "mscc,ocelot-sgpio";
+ status = "disabled";
+ clocks = <&sys_clk>;
+ pinctrl-0 = <&sgpio0_pins>;
+ pinctrl-names = "default";
+ reg = <0x6 0x1101036c 0x100>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&gpio 0 64 64>;
+ };
+
+ sgpio1: gpio@611010484 {
+ compatible = "mscc,ocelot-sgpio";
+ status = "disabled";
+ clocks = <&sys_clk>;
+ pinctrl-0 = <&sgpio1_pins>;
+ pinctrl-names = "default";
+ reg = <0x6 0x11010484 0x100>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&gpio 0 128 64>;
+ };
+
+ sgpio2: gpio@61101059c {
+ compatible = "mscc,ocelot-sgpio";
+ status = "disabled";
+ clocks = <&sys_clk>;
+ pinctrl-0 = <&sgpio2_pins>;
+ pinctrl-names = "default";
+ reg = <0x6 0x1101059c 0x100>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&gpio 0 192 64>;
+ };
+
i2c0: i2c@600101000 {
compatible = "snps,designware-i2c";
status = "disabled";
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index 94c4c3fd5a786..fd4f5b3ddcc49 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -55,6 +55,11 @@ spi-flash@1 {
};
};

+&sgpio0 {
+ status = "okay";
+ microchip,sgpio-ports = <0x00FFFFFF>;
+};
+
&i2c1 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index 628a05d3f57ce..2f781258f8c99 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -47,6 +47,11 @@ spi-flash@0 {
};
};

+&sgpio0 {
+ status = "okay";
+ microchip,sgpio-ports = <0x00FFFFFF>;
+};
+
&gpio {
i2cmux_pins_i: i2cmux-pins-i {
pins = "GPIO_16", "GPIO_17", "GPIO_18", "GPIO_19",
--
2.26.2

2020-05-13 15:07:02

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 2/3] pinctrl: mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO

On 5/13/20 7:11 AM, Lars Povlsen wrote:
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 834c59950d1cf..2b0e9021fd7e0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -396,6 +396,23 @@ config PINCTRL_OCELOT
> select OF_GPIO
> select REGMAP_MMIO
>
> +config PINCTRL_MSCC_SGPIO
> + bool "Pinctrl driver for Microsemi Serial GPIO"
> + depends on OF
> + depends on HAS_IOMEM
> + select GPIOLIB
> + select GENERIC_PINCONF
> + select GENERIC_PINCTRL_GROUPS
> + select GENERIC_PINMUX_FUNCTIONS
> + select OF_GPIO
> + help
> + Support for the VCoreIII SoC serial GPIO device. By using a

Line above should be indented with one tab + 2 spaces...
like the lines below.

> + serial interface, the SIO controller significantly extends
> + the number of available GPIOs with a minimum number of
> + additional pins on the device. The primary purpose of the
> + SIO controller is to connect control signals from SFP
> + modules and to act as an LED controller.
> +

thanks.
--
~Randy

2020-05-18 07:42:17

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio

On Wed, May 13, 2020 at 4:11 PM Lars Povlsen <[email protected]> wrote:

> This adds DT bindings for the Microsemi SGPIO controller, bindings
> mscc,ocelot-sgpio and mscc,luton-sgpio.
>
> Reviewed-by: Alexandre Belloni <[email protected]>
> Signed-off-by: Lars Povlsen <[email protected]>

> + microchip,sgpio-ports:
> + description: This is a 32-bit bitmask, configuring whether a
> + particular port in the controller is enabled or not. This allows
> + unused ports to be removed from the bitstream and reduce latency.
> + $ref: "/schemas/types.yaml#/definitions/uint32"

I don't know about this.

You are saying this pin controller can have up to 32 GPIO "ports"
(also known as banks).

Why can't you just represent each such port as a separate GPIO
node:

pinctrl@nnn {
gpio@0 {
....
};
gpio@1 {
....
};
....
gpio@31 {
....
};
};

Then if some of them are unused just set it to status = "disabled";

This also makes your Linux driver simpler because each GPIO port
just becomes a set of 32bit registers and you can use
select GPIO_GENERIC and bgpio_init() and save a whole
slew of standard stock code.

Yours,
Linus Walleij

2020-05-18 20:04:50

by Lars Povlsen

[permalink] [raw]
Subject: Re: [PATCH 2/3] pinctrl: mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO


Randy Dunlap writes:

> On 5/13/20 7:11 AM, Lars Povlsen wrote:
>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
>> index 834c59950d1cf..2b0e9021fd7e0 100644
>> --- a/drivers/pinctrl/Kconfig
>> +++ b/drivers/pinctrl/Kconfig
>> @@ -396,6 +396,23 @@ config PINCTRL_OCELOT
>> select OF_GPIO
>> select REGMAP_MMIO
>>
>> +config PINCTRL_MSCC_SGPIO
>> + bool "Pinctrl driver for Microsemi Serial GPIO"
>> + depends on OF
>> + depends on HAS_IOMEM
>> + select GPIOLIB
>> + select GENERIC_PINCONF
>> + select GENERIC_PINCTRL_GROUPS
>> + select GENERIC_PINMUX_FUNCTIONS
>> + select OF_GPIO
>> + help
>> + Support for the VCoreIII SoC serial GPIO device. By using a
>
> Line above should be indented with one tab + 2 spaces...
> like the lines below.
>

Well spotted...

>> + serial interface, the SIO controller significantly extends
>> + the number of available GPIOs with a minimum number of
>> + additional pins on the device. The primary purpose of the
>> + SIO controller is to connect control signals from SFP
>> + modules and to act as an LED controller.
>> +
>
> thanks.

Thank you for your comments.

--
Lars Povlsen,
Microchip

2020-05-18 20:54:25

by Lars Povlsen

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio


Linus Walleij writes:

> On Wed, May 13, 2020 at 4:11 PM Lars Povlsen <[email protected]> wrote:
>
>> This adds DT bindings for the Microsemi SGPIO controller, bindings
>> mscc,ocelot-sgpio and mscc,luton-sgpio.
>>
>> Reviewed-by: Alexandre Belloni <[email protected]>
>> Signed-off-by: Lars Povlsen <[email protected]>
>
>> + microchip,sgpio-ports:
>> + description: This is a 32-bit bitmask, configuring whether a
>> + particular port in the controller is enabled or not. This allows
>> + unused ports to be removed from the bitstream and reduce latency.
>> + $ref: "/schemas/types.yaml#/definitions/uint32"
>
> I don't know about this.
>
> You are saying this pin controller can have up to 32 GPIO "ports"
> (also known as banks).
>
> Why can't you just represent each such port as a separate GPIO
> node:
>
> pinctrl@nnn {
> gpio@0 {
> ....
> };
> gpio@1 {
> ....
> };
> ....
> gpio@31 {
> ....
> };
> };
>
> Then if some of them are unused just set it to status = "disabled";
>
> This also makes your Linux driver simpler because each GPIO port
> just becomes a set of 32bit registers and you can use
> select GPIO_GENERIC and bgpio_init() and save a whole
> slew of standard stock code.
>

Linus, thank you for your input.

The controller handles an array of 32*n signals, where n >= 1 && n <=
4.

The problem with the above approach is that the ports are disabled
*port*-wise - so they remove all (upto) 4 bits. That would be across the
banks.

You could of course have the "implied" semantics that a disabled port at
any bit position disabled all (bit positions for the same port).

But I don't know if this would be easier to understand, DT-wise.

What do you think...?

> Yours,
> Linus Walleij

--
Lars Povlsen,
Microchip

2020-05-25 08:55:20

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio

On Mon, May 18, 2020 at 10:50 PM Lars Povlsen
<[email protected]> wrote:
> Linus Walleij writes:
>
> > On Wed, May 13, 2020 at 4:11 PM Lars Povlsen <[email protected]> wrote:
> >
> >> This adds DT bindings for the Microsemi SGPIO controller, bindings
> >> mscc,ocelot-sgpio and mscc,luton-sgpio.
> >>
> >> Reviewed-by: Alexandre Belloni <[email protected]>
> >> Signed-off-by: Lars Povlsen <[email protected]>
> >
> >> + microchip,sgpio-ports:
> >> + description: This is a 32-bit bitmask, configuring whether a
> >> + particular port in the controller is enabled or not. This allows
> >> + unused ports to be removed from the bitstream and reduce latency.
> >> + $ref: "/schemas/types.yaml#/definitions/uint32"
> >
> > I don't know about this.
> >
> > You are saying this pin controller can have up to 32 GPIO "ports"
> > (also known as banks).
> >
> > Why can't you just represent each such port as a separate GPIO
> > node:
> >
> > pinctrl@nnn {
> > gpio@0 {
> > ....
> > };
> > gpio@1 {
> > ....
> > };
> > ....
> > gpio@31 {
> > ....
> > };
> > };
> >
> > Then if some of them are unused just set it to status = "disabled";
> >
> > This also makes your Linux driver simpler because each GPIO port
> > just becomes a set of 32bit registers and you can use
> > select GPIO_GENERIC and bgpio_init() and save a whole
> > slew of standard stock code.
> >
>
> Linus, thank you for your input.
>
> The controller handles an array of 32*n signals, where n >= 1 && n <=
> 4.
>
> The problem with the above approach is that the ports are disabled
> *port*-wise - so they remove all (upto) 4 bits. That would be across the
> banks.
>
> You could of course have the "implied" semantics that a disabled port at
> any bit position disabled all (bit positions for the same port).

I don't understand this, you would have to elaborate...

In any case microchip,sgpio-ports is probably not the right thing,
use ngpios which is documented and just divide by 32 to get the
number of ports I think? But that is just in case they get
enabled strictly in sequence, otherwise you'd need a custom
property.

Yours,
Linus Walleij

2020-05-25 14:41:20

by Lars Povlsen

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio


Linus Walleij writes:

> On Mon, May 18, 2020 at 10:50 PM Lars Povlsen
> <[email protected]> wrote:
>> Linus Walleij writes:
>>
>> > On Wed, May 13, 2020 at 4:11 PM Lars Povlsen <[email protected]> wrote:
>> >
>> >> This adds DT bindings for the Microsemi SGPIO controller, bindings
>> >> mscc,ocelot-sgpio and mscc,luton-sgpio.
>> >>
>> >> Reviewed-by: Alexandre Belloni <[email protected]>
>> >> Signed-off-by: Lars Povlsen <[email protected]>
>> >
>> >> + microchip,sgpio-ports:
>> >> + description: This is a 32-bit bitmask, configuring whether a
>> >> + particular port in the controller is enabled or not. This allows
>> >> + unused ports to be removed from the bitstream and reduce latency.
>> >> + $ref: "/schemas/types.yaml#/definitions/uint32"
>> >
>> > I don't know about this.
>> >
>> > You are saying this pin controller can have up to 32 GPIO "ports"
>> > (also known as banks).
>> >
>> > Why can't you just represent each such port as a separate GPIO
>> > node:
>> >
>> > pinctrl@nnn {
>> > gpio@0 {
>> > ....
>> > };
>> > gpio@1 {
>> > ....
>> > };
>> > ....
>> > gpio@31 {
>> > ....
>> > };
>> > };
>> >
>> > Then if some of them are unused just set it to status = "disabled";
>> >
>> > This also makes your Linux driver simpler because each GPIO port
>> > just becomes a set of 32bit registers and you can use
>> > select GPIO_GENERIC and bgpio_init() and save a whole
>> > slew of standard stock code.
>> >
>>
>> Linus, thank you for your input.
>>
>> The controller handles an array of 32*n signals, where n >= 1 && n <=
>> 4.
>>
>> The problem with the above approach is that the ports are disabled
>> *port*-wise - so they remove all (upto) 4 bits. That would be across the
>> banks.
>>
>> You could of course have the "implied" semantics that a disabled port at
>> any bit position disabled all (bit positions for the same port).
>
> I don't understand this, you would have to elaborate...
>
> In any case microchip,sgpio-ports is probably not the right thing,
> use ngpios which is documented and just divide by 32 to get the
> number of ports I think? But that is just in case they get
> enabled strictly in sequence, otherwise you'd need a custom
> property.
>

Hi Linus,

Yes, the problem is they're not in sequence. F.ex. you could have ports
0,1 enabled, skip 2,3,4 and have 5,6,7 enabled.

In the data stream you would then have:

p0.0 p0.1 p0.2 p0.3
p1.0 p1.1 p1.2 p1.3
p5.0 p5.1 p5.2 p5.3
p6.0 p6.1 p6.2 p6.3
p7.0 p7.1 p7.2 p7.3

I will mull about this and try to come up with something better and more
understandable.

Luckily, this is not gating for integrating sparx5, so its possible
we'll just skip the SGPIO driver for now.

I'll provide an update as soon as possible.

---Lars

> Yours,
> Linus Walleij

--
Lars Povlsen,
Microchip

2020-05-26 14:12:35

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio

On Mon, May 25, 2020 at 4:38 PM Lars Povlsen <[email protected]> wrote:

> Yes, the problem is they're not in sequence. F.ex. you could have ports
> 0,1 enabled, skip 2,3,4 and have 5,6,7 enabled.

Just use disabled nodes.

That would look like this in my idea of a device tree:

pinctrl@nnn {
gpio0: gpio@0 {
compatible = "foo";
status = "ok";
....
};
gpio1: gpio@1 {
compatible = "foo";
status = "ok";
....
};
gpio2: gpio@2 {
compatible = "foo";
status = "disabled";
....
};
gpio3: gpio@3 {
compatible = "foo";
status = "disabled";
....
};
gpio4: gpio@4 {
compatible = "foo";
status = "disabled";
....
};
gpio5: gpio@5 {
compatible = "foo";
status = "ok";
....
};
gpio6: gpio@6 {
compatible = "foo";
status = "ok";
....
};
gpio7: gpio@7 {
compatible = "foo";
status = "ok";
....
};
};

It is common to use the status to enable/disable nodes like this.

In the Linux kernel is is possible to iterate over these subnodes and
check which ones are enabled and disabled while keeping the
index by using something like:

i = 0;
struct device_node *np, *child;
for_each_child_of_node(np, child) {
if (of_device_is_available(child)) {
pr_info("populating device %d\n", i);
}
i++;
}

Certainly you can use i in the above loop to populate your registers
etc from an indexed array.

This way the consumers can pick their GPIO from the right port
and everything just using e.g.
my-gpios = <&gpio6 4 GPIO_OUT_LOW>;

Yours,
Linus Walleij

2020-05-27 10:33:39

by Lars Povlsen

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio


Linus Walleij writes:

> On Mon, May 25, 2020 at 4:38 PM Lars Povlsen <[email protected]> wrote:
>
>> Yes, the problem is they're not in sequence. F.ex. you could have ports
>> 0,1 enabled, skip 2,3,4 and have 5,6,7 enabled.
>
> Just use disabled nodes.
>
> That would look like this in my idea of a device tree:
>
> pinctrl@nnn {
> gpio0: gpio@0 {
> compatible = "foo";
> status = "ok";
> ....
> };
> gpio1: gpio@1 {
> compatible = "foo";
> status = "ok";
> ....
> };
> gpio2: gpio@2 {
> compatible = "foo";
> status = "disabled";
> ....
> };
> gpio3: gpio@3 {
> compatible = "foo";
> status = "disabled";
> ....
> };
> gpio4: gpio@4 {
> compatible = "foo";
> status = "disabled";
> ....
> };
> gpio5: gpio@5 {
> compatible = "foo";
> status = "ok";
> ....
> };
> gpio6: gpio@6 {
> compatible = "foo";
> status = "ok";
> ....
> };
> gpio7: gpio@7 {
> compatible = "foo";
> status = "ok";
> ....
> };
> };
>
> It is common to use the status to enable/disable nodes like this.
>
> In the Linux kernel is is possible to iterate over these subnodes and
> check which ones are enabled and disabled while keeping the
> index by using something like:
>
> i = 0;
> struct device_node *np, *child;
> for_each_child_of_node(np, child) {
> if (of_device_is_available(child)) {
> pr_info("populating device %d\n", i);
> }
> i++;
> }
>
> Certainly you can use i in the above loop to populate your registers
> etc from an indexed array.
>
> This way the consumers can pick their GPIO from the right port
> and everything just using e.g.
> my-gpios = <&gpio6 4 GPIO_OUT_LOW>;
>

Linux, thank you for your input, it is much appreciated. I will use the
pattern in the driver in the next revision.

The only issue is that the gpios on the same "port" have restrictions on
their status - they can only be enabled "all" or "none" for gpios that
map to the same port. F.ex. gpio0, gpio32, gpio64 and gpio96 must all be
enabled or disabled because at the hardware level you control the
_port_. But as I noted earlier, that could just be the driver enforcing
this.

Thanks again.

---Lars

> Yours,
> Linus Walleij

--
Lars Povlsen,
Microchip

2020-05-27 17:49:43

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio

On Wed, May 27, 2020 at 10:05 AM Lars Povlsen
<[email protected]> wrote:

> The only issue is that the gpios on the same "port" have restrictions on
> their status - they can only be enabled "all" or "none" for gpios that
> map to the same port. F.ex. gpio0, gpio32, gpio64 and gpio96 must all be
> enabled or disabled because at the hardware level you control the
> _port_.

This is fairly common. For example that an entire port/block share
a clock.

> But as I noted earlier, that could just be the driver enforcing
> this.

Yeps.

Yours,
Linus Walleij