[PATCH v4 0/3] Introduce support for Vortex GPIO pins

[PATCH v4 0/3] Introduce support for Vortex GPIO pins

[Marcos Del Sol Vives]

This series of patches add support for the GPIO pins exposed on the
southbridge most DM&P's Vortex86 SoCs, using a new GPIO driver plus a MFD
driver to automatically load the driver in supported platforms.

Supported SoCs are Vortex86SX/MX/MX+/DX/DX2/DX3, though I have only
personally tried with a MX and a DX3.

Marcos Del Sol Vives (3):
  gpio: gpio-regmap: add flag to set direction before value
  gpio: vortex: add new GPIO device driver
  mfd: vortex: implement new driver for Vortex southbridges

 MAINTAINERS                 |   6 ++
 drivers/gpio/Kconfig        |  13 +++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-regmap.c  |  17 +++-
 drivers/gpio/gpio-vortex.c  | 170 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig         |   9 ++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/vortex-sb.c     | 135 ++++++++++++++++++++++++++++
 include/linux/gpio/regmap.h |  19 ++++
 include/linux/pci_ids.h     |   3 +
 10 files changed, 373 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpio/gpio-vortex.c
 create mode 100644 drivers/mfd/vortex-sb.c

-- 
2.34.1

[PATCH v4 1/3] gpio: gpio-regmap: add flag to set direction before value

[Marcos Del Sol Vives]

When configuring a pin as an output, by default the gpio-regmap driver
writes first the value and then configures the direction.

The Vortex86 family of SoCs, however, need the direction set before the
value, else writes to the data ports are ignored.

This commit adds a new "flags" field plus a flag to reverse that order,
allowing the direction to be set before the value.

Also, added a missing error check in gpio_regmap_direction_output().

Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>
---
 drivers/gpio/gpio-regmap.c  | 17 ++++++++++++++++-
 include/linux/gpio/regmap.h | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index e8a32dfebdcb..24cefbd57637 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -31,6 +31,7 @@ struct gpio_regmap {
 	unsigned int reg_clr_base;
 	unsigned int reg_dir_in_base;
 	unsigned int reg_dir_out_base;
+	unsigned int flags;
 
 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
 			      unsigned int offset, unsigned int *reg,
@@ -196,7 +197,20 @@ static int gpio_regmap_direction_input(struct gpio_chip *chip,
 static int gpio_regmap_direction_output(struct gpio_chip *chip,
 					unsigned int offset, int value)
 {
-	gpio_regmap_set(chip, offset, value);
+	struct gpio_regmap *gpio = gpiochip_get_data(chip);
+	int ret;
+
+	if (gpio->flags & GPIO_REGMAP_DIR_BEFORE_SET) {
+		ret = gpio_regmap_set_direction(chip, offset, true);
+		if (ret)
+			return ret;
+
+		return gpio_regmap_set(chip, offset, value);
+	}
+
+	ret = gpio_regmap_set(chip, offset, value);
+	if (ret)
+		return ret;
 
 	return gpio_regmap_set_direction(chip, offset, true);
 }
@@ -247,6 +261,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 	gpio->reg_clr_base = config->reg_clr_base;
 	gpio->reg_dir_in_base = config->reg_dir_in_base;
 	gpio->reg_dir_out_base = config->reg_dir_out_base;
+	gpio->flags = config->flags;
 
 	chip = &gpio->gpio_chip;
 	chip->parent = config->parent;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index c722c67668c6..a2257a1288a8 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -3,6 +3,8 @@
 #ifndef _LINUX_GPIO_REGMAP_H
 #define _LINUX_GPIO_REGMAP_H
 
+#include <linux/bits.h>
+
 struct device;
 struct fwnode_handle;
 struct gpio_regmap;
@@ -12,6 +14,19 @@ struct regmap;
 #define GPIO_REGMAP_ADDR_ZERO ((unsigned int)(-1))
 #define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
 
+/**
+ * enum gpio_regmap_flags - flags to control GPIO operation
+ */
+enum gpio_regmap_flags {
+	/**
+	 * @GPIO_REGMAP_DIR_BEFORE_SET: when setting a pin as an output, set
+	 * its direction before the value. The output value will be undefined
+	 * for a short time which may have unwanted side effects, but some
+	 * hardware requires this.
+	 */
+	GPIO_REGMAP_DIR_BEFORE_SET	= BIT(0),
+};
+
 /**
  * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
  * @parent:		The parent device
@@ -23,6 +38,8 @@ struct regmap;
  *			If not given, the name of the device is used.
  * @ngpio:		(Optional) Number of GPIOs
  * @names:		(Optional) Array of names for gpios
+ * @flags:		(Optional) A bitmask of flags from
+ *			&enum gpio_regmap_flags
  * @reg_dat_base:	(Optional) (in) register base address
  * @reg_set_base:	(Optional) set register base address
  * @reg_clr_base:	(Optional) clear register base address
@@ -68,6 +85,7 @@ struct gpio_regmap_config {
 	const char *label;
 	int ngpio;
 	const char *const *names;
+	unsigned int flags;
 
 	unsigned int reg_dat_base;
 	unsigned int reg_set_base;
-- 
2.34.1

[PATCH v4 2/3] gpio: vortex: add new GPIO device driver

[Marcos Del Sol Vives]

Add a new simple GPIO device driver for most DM&P Vortex86 SoCs,
implemented according to their programming reference manuals [1][2][3].

Vortex86EX/EX2 use a radically different mechanism of GPIO control
and are not supported by this driver.

This is required for detecting the status of the poweroff button and
performing the poweroff sequence on ICOP eBox computers.

IRQs are not implemented, as they are only available for ports 0 and 1,
none which are accessible on my test machine (an EBOX-3352-GLW).

[1]: https://www.vortex86.com/downloadsStart?serial=Vortex86SX/DX/MXPLUS
[2]: https://www.vortex86.com/downloadsStart?serial=Vortex86DX2
[3]: https://www.vortex86.com/downloadsStart?serial=Vortex86DX3

Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>
---
 MAINTAINERS                |   5 ++
 drivers/gpio/Kconfig       |  13 +++
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-vortex.c | 170 +++++++++++++++++++++++++++++++++++++
 4 files changed, 189 insertions(+)
 create mode 100644 drivers/gpio/gpio-vortex.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2720544cd91f..afa88f446630 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26953,6 +26953,11 @@ VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
 R:	Matti Vaittinen <mazziesaccount@gmail.com>
 F:	drivers/regulator/irq_helpers.c
 
+VORTEX HARDWARE SUPPORT
+R:	Marcos Del Sol Vives <marcos@orca.pet>
+S:	Maintained
+F:	drivers/gpio/gpio-vortex.c
+
 VRF
 M:	David Ahern <dsahern@kernel.org>
 L:	netdev@vger.kernel.org
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e43abb322fa6..dd5b24843b41 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1077,6 +1077,19 @@ config GPIO_TS5500
 	  blocks of the TS-5500: DIO1, DIO2 and the LCD port, and the TS-5600
 	  LCD port.
 
+config GPIO_VORTEX
+	tristate "Vortex SoC GPIO support"
+	select REGMAP_MMIO
+	select GPIO_REGMAP
+	help
+	  Driver to access the 8-bit bidirectional GPIO ports present on DM&P
+	  Vortex86SX/MX/MX+/DX/DX2/DX3 SoCs.
+
+	  Vortex86EX-series parts are not supported.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called gpio-vortex.
+
 config GPIO_WINBOND
 	tristate "Winbond Super I/O GPIO support"
 	select ISA_BUS_API
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 379f55e9ed1e..7b8626c9bd75 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -197,6 +197,7 @@ obj-$(CONFIG_GPIO_VIPERBOARD)		+= gpio-viperboard.o
 obj-$(CONFIG_GPIO_VIRTUSER)		+= gpio-virtuser.o
 obj-$(CONFIG_GPIO_VIRTIO)		+= gpio-virtio.o
 obj-$(CONFIG_GPIO_VISCONTI)		+= gpio-visconti.o
+obj-$(CONFIG_GPIO_VORTEX)		+= gpio-vortex.o
 obj-$(CONFIG_GPIO_VX855)		+= gpio-vx855.o
 obj-$(CONFIG_GPIO_WCD934X)		+= gpio-wcd934x.o
 obj-$(CONFIG_GPIO_WHISKEY_COVE)		+= gpio-wcove.o
diff --git a/drivers/gpio/gpio-vortex.c b/drivers/gpio/gpio-vortex.c
new file mode 100644
index 000000000000..edac919a5799
--- /dev/null
+++ b/drivers/gpio/gpio-vortex.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * GPIO driver for Vortex86 SoCs
+ *
+ * Vortex SoCs may have either a single set (DX/MX/SX) of data/direction ports,
+ * or two non-contiguous sets (DX2/DX3).
+ *
+ * Because gpio-regmap is not designed to handle ranges with holes of arbitrary
+ * sizes in them, this driver reports a virtual layout where ports 0..n-1 are
+ * data ports, and n..n*2-1 are direction ports. The xlate function then maps
+ * these virtual ports back to the real hardware registers relative to the
+ * requested I/O window.
+ *
+ * Author: Marcos Del Sol Vives <marcos@orca.pet>
+ */
+
+#include <linux/bits.h>
+#include <linux/errno.h>
+#include <linux/gpio/regmap.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+struct vortex_gpio {
+	struct regmap_range ranges[4];
+	struct regmap_access_table access_table;
+	struct device *dev;
+};
+
+static int vortex_gpio_xlate(struct gpio_regmap *gpio, unsigned int base,
+			     unsigned int offset, unsigned int *reg,
+			     unsigned int *mask)
+{
+	struct vortex_gpio *priv = gpio_regmap_get_drvdata(gpio);
+	unsigned int virtual_port, r_min, r_size;
+	int i;
+
+	virtual_port = base + offset / 8;
+
+	for (i = 0; i < priv->access_table.n_yes_ranges; i++) {
+		r_min = priv->ranges[i].range_min;
+		r_size = priv->ranges[i].range_max - r_min + 1;
+
+		if (virtual_port < r_size) {
+			*reg = virtual_port + r_min;
+			*mask = BIT(offset % 8);
+			return 0;
+		}
+		virtual_port -= r_size;
+	}
+
+	/* should never happen */
+	dev_err(priv->dev, "tried to translate an out-of-bounds virtual port: %u\n",
+		base + offset / 8);
+	return -EINVAL;
+}
+
+static int vortex_gpio_add_range(struct vortex_gpio *priv,
+				 struct platform_device *pdev,
+				 const char *res_name)
+{
+	struct resource *res;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_IO, res_name);
+	if (!res)
+		return 0;
+
+	priv->ranges[priv->access_table.n_yes_ranges].range_min = res->start;
+	priv->ranges[priv->access_table.n_yes_ranges].range_max = res->end;
+	priv->access_table.n_yes_ranges++;
+
+	return resource_size(res);
+}
+
+static int vortex_gpio_probe(struct platform_device *pdev)
+{
+	struct gpio_regmap_config gpiocfg = {};
+	struct device *dev = &pdev->dev;
+	struct regmap_config rmcfg = {};
+	unsigned long io_min, io_max;
+	struct vortex_gpio *priv;
+	int i, dat_len, dir_len;
+	struct regmap *map;
+	void __iomem *regs;
+
+	/* Initialize private data */
+	priv = devm_kzalloc(dev, sizeof(struct vortex_gpio),
+			    GFP_KERNEL);
+	if (unlikely(!priv))
+		return -ENOMEM;
+	priv->dev = dev;
+	priv->access_table.yes_ranges = priv->ranges;
+
+	/* Add I/O ports from platform data to ranges */
+	dat_len = vortex_gpio_add_range(priv, pdev, "dat1");
+	if (unlikely(!dat_len)) {
+		dev_err(dev, "failed to get data register\n");
+		return -ENODEV;
+	}
+	dat_len += vortex_gpio_add_range(priv, pdev, "dat2");
+
+	dir_len = vortex_gpio_add_range(priv, pdev, "dir1");
+	if (unlikely(!dir_len)) {
+		dev_err(dev, "failed to get direction register\n");
+		return -ENODEV;
+	}
+	dir_len += vortex_gpio_add_range(priv, pdev, "dir2");
+
+	if (unlikely(dat_len != dir_len)) {
+		dev_err(dev, "data and direction size mismatch (%d vs %d)\n",
+			dat_len, dir_len);
+		return -EINVAL;
+	}
+
+	/* Request smallest I/O window that covers all registers */
+	io_min = priv->ranges[0].range_min;
+	io_max = priv->ranges[0].range_max;
+	for (i = 1; i < priv->access_table.n_yes_ranges; i++) {
+		io_min = min(io_min, priv->ranges[i].range_min);
+		io_max = max(io_max, priv->ranges[i].range_max);
+	}
+
+	regs = devm_ioport_map(dev, io_min, io_max - io_min + 1);
+	if (unlikely(!regs))
+		return -ENOMEM;
+
+	/* Subtract io_min to make them relative to the window */
+	for (i = 0; i < priv->access_table.n_yes_ranges; i++) {
+		priv->ranges[i].range_min -= io_min;
+		priv->ranges[i].range_max -= io_min;
+	}
+
+	rmcfg.reg_bits = 8;
+	rmcfg.val_bits = 8;
+	rmcfg.io_port = true;
+	rmcfg.wr_table = &priv->access_table;
+	rmcfg.rd_table = &priv->access_table;
+
+	map = devm_regmap_init_mmio(dev, regs, &rmcfg);
+	if (IS_ERR(map))
+		return dev_err_probe(dev, PTR_ERR(map),
+				     "Unable to initialize register map\n");
+
+	gpiocfg.parent = dev;
+	gpiocfg.regmap = map;
+	gpiocfg.drvdata = priv;
+	gpiocfg.ngpio = 8 * dat_len;
+	gpiocfg.ngpio_per_reg = 8;
+	gpiocfg.reg_dat_base = GPIO_REGMAP_ADDR(0);
+	gpiocfg.reg_set_base = GPIO_REGMAP_ADDR(0);
+	gpiocfg.reg_dir_out_base = GPIO_REGMAP_ADDR(dat_len);
+	gpiocfg.flags = GPIO_REGMAP_DIR_BEFORE_SET;
+	gpiocfg.reg_mask_xlate = vortex_gpio_xlate;
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpiocfg));
+}
+
+static struct platform_driver vortex_gpio_driver = {
+	.driver.name = "vortex-gpio",
+	.probe = vortex_gpio_probe,
+};
+
+module_platform_driver(vortex_gpio_driver);
+
+MODULE_AUTHOR("Marcos Del Sol Vives <marcos@orca.pet>");
+MODULE_DESCRIPTION("GPIO driver for Vortex86 SoCs");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:vortex-gpio");
-- 
2.34.1

[PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Marcos Del Sol Vives]

This new driver loads resources related to southbridges available in DM&P
Vortex devices, currently only the GPIO pins.

Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>
---
 MAINTAINERS             |   1 +
 drivers/mfd/Kconfig     |   9 +++
 drivers/mfd/Makefile    |   1 +
 drivers/mfd/vortex-sb.c | 135 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h |   3 +
 5 files changed, 149 insertions(+)
 create mode 100644 drivers/mfd/vortex-sb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index afa88f446630..63e410e23e95 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26957,6 +26957,7 @@ VORTEX HARDWARE SUPPORT
 R:	Marcos Del Sol Vives <marcos@orca.pet>
 S:	Maintained
 F:	drivers/gpio/gpio-vortex.c
+F:	drivers/mfd/vortex-sb.c
 
 VRF
 M:	David Ahern <dsahern@kernel.org>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 425c5fba6cb1..fe54bb22687d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2008,6 +2008,15 @@ config MFD_VX855
 	  VIA VX855/VX875 south bridge. You will need to enable the vx855_spi
 	  and/or vx855_gpio drivers for this to do anything useful.
 
+config MFD_VORTEX_SB
+	tristate "Vortex southbridge"
+	select MFD_CORE
+	depends on PCI
+	help
+	  Say yes here if you want to have support for the southbridge
+	  present on Vortex SoCs. You will need to enable the vortex-gpio
+	  driver for this to do anything useful.
+
 config MFD_ARIZONA
 	select REGMAP
 	select REGMAP_IRQ
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f7bdedd5a66d..2504ba311f1a 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -202,6 +202,7 @@ obj-$(CONFIG_MFD_JANZ_CMODIO)	+= janz-cmodio.o
 obj-$(CONFIG_MFD_TPS6586X)	+= tps6586x.o
 obj-$(CONFIG_MFD_VX855)		+= vx855.o
 obj-$(CONFIG_MFD_WL1273_CORE)	+= wl1273-core.o
+obj-$(CONFIG_MFD_VORTEX_SB)	+= vortex-sb.o
 
 si476x-core-y := si476x-cmd.o si476x-prop.o si476x-i2c.o
 obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
diff --git a/drivers/mfd/vortex-sb.c b/drivers/mfd/vortex-sb.c
new file mode 100644
index 000000000000..141fa19773e2
--- /dev/null
+++ b/drivers/mfd/vortex-sb.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  MFD southbridge driver for Vortex SoCs
+ *
+ *  Author: Marcos Del Sol Vives <marcos@orca.pet>
+ *
+ *  Based on the RDC321x MFD driver by Florian Fainelli and Bernhard Loos
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/pci.h>
+#include <linux/mfd/core.h>
+
+struct vortex_southbridge {
+	const struct mfd_cell *cells;
+	int n_cells;
+};
+
+/* Layout for Vortex86DX/MX */
+static const struct resource vortex_dx_gpio_resources[] = {
+	{
+		.name	= "dat1",
+		.start	= 0x78,
+		.end	= 0x7C,
+		.flags	= IORESOURCE_IO,
+	}, {
+		.name	= "dir1",
+		.start	= 0x98,
+		.end	= 0x9C,
+		.flags	= IORESOURCE_IO,
+	}
+};
+
+static const struct mfd_cell vortex_dx_sb_cells[] = {
+	{
+		.name		= "vortex-gpio",
+		.resources	= vortex_dx_gpio_resources,
+		.num_resources	= ARRAY_SIZE(vortex_dx_gpio_resources),
+	},
+};
+
+static const struct vortex_southbridge vortex_dx_sb = {
+	.cells = vortex_dx_sb_cells,
+	.n_cells = ARRAY_SIZE(vortex_dx_sb_cells),
+};
+
+/* Layout for Vortex86DX2/DX3 */
+static const struct resource vortex_dx2_gpio_resources[] = {
+	{
+		.name	= "dat1",
+		.start	= 0x78,
+		.end	= 0x7C,
+		.flags	= IORESOURCE_IO,
+	}, {
+		.name	= "dat2",
+		.start	= 0x100,
+		.end	= 0x105,
+		.flags	= IORESOURCE_IO,
+	}, {
+		.name	= "dir1",
+		.start	= 0x98,
+		.end	= 0x9D,
+		.flags	= IORESOURCE_IO,
+	}, {
+		.name	= "dir2",
+		.start	= 0x93,
+		.end	= 0x97,
+		.flags	= IORESOURCE_IO,
+	}
+};
+
+static const struct mfd_cell vortex_dx2_sb_cells[] = {
+	{
+		.name		= "vortex-gpio",
+		.resources	= vortex_dx2_gpio_resources,
+		.num_resources	= ARRAY_SIZE(vortex_dx2_gpio_resources),
+	},
+};
+
+static const struct vortex_southbridge vortex_dx2_sb = {
+	.cells = vortex_dx2_sb_cells,
+	.n_cells = ARRAY_SIZE(vortex_dx2_sb_cells),
+};
+
+static int vortex_sb_probe(struct pci_dev *pdev,
+			   const struct pci_device_id *ent)
+{
+	struct vortex_southbridge *priv = (struct vortex_southbridge *) ent->driver_data;
+	int err;
+
+	/*
+	 * In the Vortex86DX3, the southbridge appears twice (on both 00:07.0
+	 * and 00:07.1). Register only once for .0.
+	 *
+	 * Other Vortex boards (eg Vortex86MX+) have the southbridge exposed
+	 * only once, also at 00:07.0.
+	 */
+	if (PCI_FUNC(pdev->devfn) != 0)
+		return -ENODEV;
+
+	err = pci_enable_device(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "failed to enable device\n");
+		return err;
+	}
+
+	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
+				    priv->cells, priv->n_cells,
+				    NULL, 0, NULL);
+}
+
+static const struct pci_device_id vortex_sb_table[] = {
+	/* Vortex86DX */
+	{ PCI_DEVICE_DATA(RDC, R6031, &vortex_dx_sb) },
+	/* Vortex86DX2/DX3 */
+	{ PCI_DEVICE_DATA(RDC, R6035, &vortex_dx2_sb) },
+	/* Vortex86MX */
+	{ PCI_DEVICE_DATA(RDC, R6036, &vortex_dx_sb) },
+	{}
+};
+MODULE_DEVICE_TABLE(pci, vortex_sb_table);
+
+static struct pci_driver vortex_sb_driver = {
+	.name		= "vortex-sb",
+	.id_table	= vortex_sb_table,
+	.probe		= vortex_sb_probe,
+};
+
+module_pci_driver(vortex_sb_driver);
+
+MODULE_AUTHOR("Marcos Del Sol Vives <marcos@orca.pet>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Vortex MFD southbridge driver");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 92ffc4373f6d..2c7afebd94ea 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2412,6 +2412,9 @@
 #define PCI_VENDOR_ID_RDC		0x17f3
 #define PCI_DEVICE_ID_RDC_R6020		0x6020
 #define PCI_DEVICE_ID_RDC_R6030		0x6030
+#define PCI_DEVICE_ID_RDC_R6031		0x6031
+#define PCI_DEVICE_ID_RDC_R6035		0x6035
+#define PCI_DEVICE_ID_RDC_R6036		0x6036
 #define PCI_DEVICE_ID_RDC_R6040		0x6040
 #define PCI_DEVICE_ID_RDC_R6060		0x6060
 #define PCI_DEVICE_ID_RDC_R6061		0x6061
-- 
2.34.1

Re: [PATCH v4 2/3] gpio: vortex: add new GPIO device driver

[William Breathitt Gray]

On Fri, Aug 22, 2025 at 03:58:12PM +0200, Marcos Del Sol Vives wrote:
> Add a new simple GPIO device driver for most DM&P Vortex86 SoCs,
> implemented according to their programming reference manuals [1][2][3].
> 
> Vortex86EX/EX2 use a radically different mechanism of GPIO control
> and are not supported by this driver.
> 
> This is required for detecting the status of the poweroff button and
> performing the poweroff sequence on ICOP eBox computers.
> 
> IRQs are not implemented, as they are only available for ports 0 and 1,
> none which are accessible on my test machine (an EBOX-3352-GLW).
> 
> [1]: https://www.vortex86.com/downloadsStart?serial=Vortex86SX/DX/MXPLUS
> [2]: https://www.vortex86.com/downloadsStart?serial=Vortex86DX2
> [3]: https://www.vortex86.com/downloadsStart?serial=Vortex86DX3
> 
> Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>

Hi Marcos,

Thank you for taking the time to develop and improve this driver. It can
be intimidating to submit patches and interface changes for public
review (especially without the help of the hardware company), so I
commend your continual efforts.

Regarding this GPIO driver, you've incorporated much of what I had
intended to comment on for your v2, so I'm comfortable leaving an Ack
for this version here.

Acked-by: William Breathitt Gray <wbg@kernel.org>

However, I do have a couple minor suggestions below if you decide to
submit a v5.

> +VORTEX HARDWARE SUPPORT
> +R:	Marcos Del Sol Vives <marcos@orca.pet>
> +S:	Maintained
> +F:	drivers/gpio/gpio-vortex.c

This driver only covers GPIO support so a better title for this
MAINTAINERS entry would be "VORTEX86 GPIO SUPPORT".

> +	rmcfg.reg_bits = 8;
> +	rmcfg.val_bits = 8;
> +	rmcfg.io_port = true;
> +	rmcfg.wr_table = &priv->access_table;
> +	rmcfg.rd_table = &priv->access_table;

The direction ports are expected to hold their previous state until they
are changed, so perhaps it would be beneficial to enable caching with a
rmcfg.cache_type = REGCACHE_FLAT and set a volatile_table which
excludes the data port range.

William Breathitt Gray

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Lee Jones]

On Fri, 22 Aug 2025, Marcos Del Sol Vives wrote:

> This new driver loads resources related to southbridges available in DM&P
> Vortex devices, currently only the GPIO pins.
> 
> Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>
> ---
>  MAINTAINERS             |   1 +
>  drivers/mfd/Kconfig     |   9 +++
>  drivers/mfd/Makefile    |   1 +
>  drivers/mfd/vortex-sb.c | 135 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci_ids.h |   3 +
>  5 files changed, 149 insertions(+)
>  create mode 100644 drivers/mfd/vortex-sb.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index afa88f446630..63e410e23e95 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26957,6 +26957,7 @@ VORTEX HARDWARE SUPPORT
>  R:	Marcos Del Sol Vives <marcos@orca.pet>
>  S:	Maintained
>  F:	drivers/gpio/gpio-vortex.c
> +F:	drivers/mfd/vortex-sb.c
>  
>  VRF
>  M:	David Ahern <dsahern@kernel.org>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 425c5fba6cb1..fe54bb22687d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2008,6 +2008,15 @@ config MFD_VX855
>  	  VIA VX855/VX875 south bridge. You will need to enable the vx855_spi
>  	  and/or vx855_gpio drivers for this to do anything useful.
>  
> +config MFD_VORTEX_SB
> +	tristate "Vortex southbridge"
> +	select MFD_CORE
> +	depends on PCI
> +	help
> +	  Say yes here if you want to have support for the southbridge
> +	  present on Vortex SoCs. You will need to enable the vortex-gpio
> +	  driver for this to do anything useful.
> +
>  config MFD_ARIZONA
>  	select REGMAP
>  	select REGMAP_IRQ
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f7bdedd5a66d..2504ba311f1a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -202,6 +202,7 @@ obj-$(CONFIG_MFD_JANZ_CMODIO)	+= janz-cmodio.o
>  obj-$(CONFIG_MFD_TPS6586X)	+= tps6586x.o
>  obj-$(CONFIG_MFD_VX855)		+= vx855.o
>  obj-$(CONFIG_MFD_WL1273_CORE)	+= wl1273-core.o
> +obj-$(CONFIG_MFD_VORTEX_SB)	+= vortex-sb.o
>  
>  si476x-core-y := si476x-cmd.o si476x-prop.o si476x-i2c.o
>  obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
> diff --git a/drivers/mfd/vortex-sb.c b/drivers/mfd/vortex-sb.c
> new file mode 100644
> index 000000000000..141fa19773e2
> --- /dev/null
> +++ b/drivers/mfd/vortex-sb.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  MFD southbridge driver for Vortex SoCs

Drop references to MFD.

Call it "Core southbridge ..."

> + *
> + *  Author: Marcos Del Sol Vives <marcos@orca.pet>
> + *
> + *  Based on the RDC321x MFD driver by Florian Fainelli and Bernhard Loos

Drop this.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/pci.h>
> +#include <linux/mfd/core.h>

Alphabetical.

> +
> +struct vortex_southbridge {
> +	const struct mfd_cell *cells;
> +	int n_cells;
> +};

Why is this needed?

> +/* Layout for Vortex86DX/MX */
> +static const struct resource vortex_dx_gpio_resources[] = {
> +	{
> +		.name	= "dat1",
> +		.start	= 0x78,

Define _all_ magic numbers.


> +		.end	= 0x7C,
> +		.flags	= IORESOURCE_IO,
> +	}, {
> +		.name	= "dir1",
> +		.start	= 0x98,
> +		.end	= 0x9C,
> +		.flags	= IORESOURCE_IO,
> +	}

Use DEFINE_RES_*() macros.

> +};
> +
> +static const struct mfd_cell vortex_dx_sb_cells[] = {
> +	{
> +		.name		= "vortex-gpio",
> +		.resources	= vortex_dx_gpio_resources,
> +		.num_resources	= ARRAY_SIZE(vortex_dx_gpio_resources),
> +	},
> +};

It's not an MFD until you have more than one device.

> +static const struct vortex_southbridge vortex_dx_sb = {
> +	.cells = vortex_dx_sb_cells,
> +	.n_cells = ARRAY_SIZE(vortex_dx_sb_cells),
> +};
> +
> +/* Layout for Vortex86DX2/DX3 */
> +static const struct resource vortex_dx2_gpio_resources[] = {
> +	{
> +		.name	= "dat1",
> +		.start	= 0x78,
> +		.end	= 0x7C,
> +		.flags	= IORESOURCE_IO,
> +	}, {
> +		.name	= "dat2",
> +		.start	= 0x100,
> +		.end	= 0x105,
> +		.flags	= IORESOURCE_IO,
> +	}, {
> +		.name	= "dir1",
> +		.start	= 0x98,
> +		.end	= 0x9D,
> +		.flags	= IORESOURCE_IO,
> +	}, {
> +		.name	= "dir2",
> +		.start	= 0x93,
> +		.end	= 0x97,
> +		.flags	= IORESOURCE_IO,
> +	}
> +};

As above.

> +static const struct mfd_cell vortex_dx2_sb_cells[] = {
> +	{
> +		.name		= "vortex-gpio",
> +		.resources	= vortex_dx2_gpio_resources,
> +		.num_resources	= ARRAY_SIZE(vortex_dx2_gpio_resources),
> +	},
> +};
> +
> +static const struct vortex_southbridge vortex_dx2_sb = {
> +	.cells = vortex_dx2_sb_cells,
> +	.n_cells = ARRAY_SIZE(vortex_dx2_sb_cells),
> +};
> +
> +static int vortex_sb_probe(struct pci_dev *pdev,
> +			   const struct pci_device_id *ent)

Why line-feed here and not 2 lines down?

> +{
> +	struct vortex_southbridge *priv = (struct vortex_southbridge *) ent->driver_data;

s/priv/ddata/

> +	int err;
> +
> +	/*
> +	 * In the Vortex86DX3, the southbridge appears twice (on both 00:07.0
> +	 * and 00:07.1). Register only once for .0.
> +	 *
> +	 * Other Vortex boards (eg Vortex86MX+) have the southbridge exposed
> +	 * only once, also at 00:07.0.
> +	 */
> +	if (PCI_FUNC(pdev->devfn) != 0)
> +		return -ENODEV;
> +
> +	err = pci_enable_device(pdev);
> +	if (err) {
> +		dev_err(&pdev->dev, "failed to enable device\n");
> +		return err;
> +	}
> +
> +	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
> +				    priv->cells, priv->n_cells,
> +				    NULL, 0, NULL);
> +}
> +
> +static const struct pci_device_id vortex_sb_table[] = {
> +	/* Vortex86DX */
> +	{ PCI_DEVICE_DATA(RDC, R6031, &vortex_dx_sb) },

We're not passing one initialisation API's data (MFD) through another (PCI).

Pass a device ID (if you don't already have one) and match on that instead.

> +	/* Vortex86DX2/DX3 */
> +	{ PCI_DEVICE_DATA(RDC, R6035, &vortex_dx2_sb) },
> +	/* Vortex86MX */
> +	{ PCI_DEVICE_DATA(RDC, R6036, &vortex_dx_sb) },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(pci, vortex_sb_table);
> +
> +static struct pci_driver vortex_sb_driver = {
> +	.name		= "vortex-sb",
> +	.id_table	= vortex_sb_table,
> +	.probe		= vortex_sb_probe,
> +};
> +

Remove this line.

> +module_pci_driver(vortex_sb_driver);
> +
> +MODULE_AUTHOR("Marcos Del Sol Vives <marcos@orca.pet>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Vortex MFD southbridge driver");
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 92ffc4373f6d..2c7afebd94ea 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2412,6 +2412,9 @@
>  #define PCI_VENDOR_ID_RDC		0x17f3
>  #define PCI_DEVICE_ID_RDC_R6020		0x6020
>  #define PCI_DEVICE_ID_RDC_R6030		0x6030
> +#define PCI_DEVICE_ID_RDC_R6031		0x6031
> +#define PCI_DEVICE_ID_RDC_R6035		0x6035
> +#define PCI_DEVICE_ID_RDC_R6036		0x6036
>  #define PCI_DEVICE_ID_RDC_R6040		0x6040
>  #define PCI_DEVICE_ID_RDC_R6060		0x6060
>  #define PCI_DEVICE_ID_RDC_R6061		0x6061
> -- 
> 2.34.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Marcos Del Sol Vives]

El 02/09/2025 a las 17:18, Lee Jones escribió:
>> +
>> +struct vortex_southbridge {
>> +	const struct mfd_cell *cells;
>> +	int n_cells;
>> +};
> 
> Why is this needed?
> 

To have a variable amount of cells. Currently I am only implementing the
GPIO device because it's the most critical (required for device shutdown),
but I plan on implementing once this gets merged at least also the watchdog,
which is provided by the same southbridge.

Adding support for this is should make adding that simpler.

>> +static const struct mfd_cell vortex_dx_sb_cells[] = {
>> +	{
>> +		.name		= "vortex-gpio",
>> +		.resources	= vortex_dx_gpio_resources,
>> +		.num_resources	= ARRAY_SIZE(vortex_dx_gpio_resources),
>> +	},
>> +};
> 
> It's not an MFD until you have more than one device.

Same as above.

>> +static const struct pci_device_id vortex_sb_table[] = {
>> +	/* Vortex86DX */
>> +	{ PCI_DEVICE_DATA(RDC, R6031, &vortex_dx_sb) },
> 
> We're not passing one initialisation API's data (MFD) through another (PCI).

Unless I understood you incorrectly, you mean I should not pass MFD cells/
data as private data?

vortex_dx_sb are "struct vortex_southbridge" type, not raw MFD API data.

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Lee Jones]

On Tue, 02 Sep 2025, Marcos Del Sol Vives wrote:

> El 02/09/2025 a las 17:18, Lee Jones escribió:
> >> +
> >> +struct vortex_southbridge {
> >> +	const struct mfd_cell *cells;
> >> +	int n_cells;
> >> +};
> > 
> > Why is this needed?
> > 
> 
> To have a variable amount of cells. Currently I am only implementing the
> GPIO device because it's the most critical (required for device shutdown),
> but I plan on implementing once this gets merged at least also the watchdog,
> which is provided by the same southbridge.
> 
> Adding support for this is should make adding that simpler.

You don't need it.  Please find another way to achieve your goal.

> >> +static const struct mfd_cell vortex_dx_sb_cells[] = {
> >> +	{
> >> +		.name		= "vortex-gpio",
> >> +		.resources	= vortex_dx_gpio_resources,
> >> +		.num_resources	= ARRAY_SIZE(vortex_dx_gpio_resources),
> >> +	},
> >> +};
> > 
> > It's not an MFD until you have more than one device.
> 
> Same as above.

It will not be accepted with only a single device (SFD?).

> >> +static const struct pci_device_id vortex_sb_table[] = {
> >> +	/* Vortex86DX */
> >> +	{ PCI_DEVICE_DATA(RDC, R6031, &vortex_dx_sb) },
> > 
> > We're not passing one initialisation API's data (MFD) through another (PCI).
> 
> Unless I understood you incorrectly, you mean I should not pass MFD cells/
> data as private data?

Right.

> vortex_dx_sb are "struct vortex_southbridge" type, not raw MFD API data.

I like your style, but nope!

vortex_southbridge contains MFD data and shouldn't exist anyway.

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Marcos Del Sol Vives]

El 03/09/2025 a las 9:21, Lee Jones escribió:
>> vortex_dx_sb are "struct vortex_southbridge" type, not raw MFD API data.
> 
> I like your style, but nope!
> 
> vortex_southbridge contains MFD data and shouldn't exist anyway.

I'm not sure if I follow.

You're suggesting not using driver_data at all and using a big "if" instead,
matching manually myself on the correct cells to register against the PCI
device ID, instead of relying on PCI matching giving me already the cells
structure inside driver_data?

That seems to increase code size and be more error prone for no reason.

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Lee Jones]

On Wed, 03 Sep 2025, Marcos Del Sol Vives wrote:

> El 03/09/2025 a las 9:21, Lee Jones escribió:
> >> vortex_dx_sb are "struct vortex_southbridge" type, not raw MFD API data.
> > 
> > I like your style, but nope!
> > 
> > vortex_southbridge contains MFD data and shouldn't exist anyway.
> 
> I'm not sure if I follow.
> 
> You're suggesting not using driver_data at all and using a big "if" instead,
> matching manually myself on the correct cells to register against the PCI
> device ID, instead of relying on PCI matching giving me already the cells
> structure inside driver_data?

Yes.

> That seems to increase code size and be more error prone for no reason.

It may make sense for your use-case, but believe me, I've seen some
crazy implementations of this.  I found it's easier just to have a no
cross contamination of early init APSs rule and call it a day.

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Marcos Del Sol Vives]

El 03/09/2025 a las 9:21, Lee Jones escribió:
>>>> +static const struct mfd_cell vortex_dx_sb_cells[] = {
>>>> +	{
>>>> +		.name		= "vortex-gpio",
>>>> +		.resources	= vortex_dx_gpio_resources,
>>>> +		.num_resources	= ARRAY_SIZE(vortex_dx_gpio_resources),
>>>> +	},
>>>> +};
>>>
>>> It's not an MFD until you have more than one device.
>>
>> Same as above.
> 
> It will not be accepted with only a single device (SFD?).
> 

I've been working on making all the changes, except this one.

If you prefer, I can either implement the watchdog now, add it on this
patch series and thus make it a proper MFD (at the cost of delaying
even further the GPIO inclusion), or keep the struct mfd_cell array
as a single-element array and implement the watchdog later on another
merge request, using that very same array.

I am however not okay with wasting my time rewriting that to bypass
the MFD API for this, so I can waste even more time later
implementing again the MFD API, just because linguistically
one (right now) is technically not "multi".

That seems very unreasonable, specially when it wouldn't be a first
since at least these other devices are also using MFD with a single
device:

  - 88pm800
  - 88pm805
  - at91-usart
  - stw481x
  - vx855
  - wm8400
  - zynqmp (last changed in 2024, so certainly not legacy!)

And probably others since I did not look too deep into it.

Greetings,
Marcos

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Lee Jones]

On Wed, 03 Sep 2025, Marcos Del Sol Vives wrote:

> El 03/09/2025 a las 9:21, Lee Jones escribió:
> >>>> +static const struct mfd_cell vortex_dx_sb_cells[] = {
> >>>> +	{
> >>>> +		.name		= "vortex-gpio",
> >>>> +		.resources	= vortex_dx_gpio_resources,
> >>>> +		.num_resources	= ARRAY_SIZE(vortex_dx_gpio_resources),
> >>>> +	},
> >>>> +};
> >>>
> >>> It's not an MFD until you have more than one device.
> >>
> >> Same as above.
> > 
> > It will not be accepted with only a single device (SFD?).
> > 
> 
> I've been working on making all the changes, except this one.
> 
> If you prefer, I can either implement the watchdog now, add it on this

Yes, please implement the WDT now.

> patch series and thus make it a proper MFD (at the cost of delaying
> even further the GPIO inclusion), or keep the struct mfd_cell array
> as a single-element array and implement the watchdog later on another
> merge request, using that very same array.
> 
> I am however not okay with wasting my time rewriting that to bypass
> the MFD API for this, so I can waste even more time later
> implementing again the MFD API, just because linguistically
> one (right now) is technically not "multi".

I don't get this.  If you implement the WDT now, you will be "multi", so
what are you protesting against?

> That seems very unreasonable, specially when it wouldn't be a first
> since at least these other devices are also using MFD with a single
> device:
> 
>   - 88pm80

% grep name drivers/mfd/88pm800.c
	.name = "88pm80x-rtc",
	.name = "88pm80x-onkey",
	.name = "88pm80x-regulator",
	.name = "88pm800",

>   - 88pm805

% grep name drivers/mfd/88pm805.c       
	.name = "88pm80x-codec",
	.name = "88pm805",

>   - at91-usart

% grep NAME drivers/mfd/at91-usart.c
	MFD_CELL_NAME("at91_usart_spi");
	MFD_CELL_NAME("atmel_usart_serial");

>   - stw481x

* Copyright (C) 2013 ST-Ericsson SA

>   - vx855

* Copyright (C) 2009 VIA Technologies, Inc.

>   - wm8400

* Copyright 2008 Wolfson Microelectronics PLC.

>   - zynqmp (last changed in 2024, so certainly not legacy!)

This should _not_ be using the MFD API at all!

> And probably others since I did not look too deep into it.
> 
> Greetings,
> Marcos
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Marcos Del Sol Vives]

El 03/09/2025 a las 16:01, Lee Jones escribió:
>> patch series and thus make it a proper MFD (at the cost of delaying
>> even further the GPIO inclusion), or keep the struct mfd_cell array
>> as a single-element array and implement the watchdog later on another
>> merge request, using that very same array.
>>
>> I am however not okay with wasting my time rewriting that to bypass
>> the MFD API for this, so I can waste even more time later
>> implementing again the MFD API, just because linguistically
>> one (right now) is technically not "multi".
> 
> I don't get this.  If you implement the WDT now, you will be "multi", so
> what are you protesting against?

That GPIO is something required to perform the poweroff sequence, a must
for any machine, while WDT is just a "nice to have".

Implementing now the WDT just because of a linguistic preference means
delaying something more important in favour of a "nice to have".

>> That seems very unreasonable, specially when it wouldn't be a first
>> since at least these other devices are also using MFD with a single
>> device:
>>
>>   - 88pm80
> 
> % grep name drivers/mfd/88pm800.c
> 	.name = "88pm80x-rtc",
> 	.name = "88pm80x-onkey",
> 	.name = "88pm80x-regulator",
> 	.name = "88pm800",

If you open the file, you'll see it uses five single-element arrays.

>>   - 88pm805
> 
> % grep name drivers/mfd/88pm805.c       
> 	.name = "88pm80x-codec",
> 	.name = "88pm805",
> 

Same as above.

>>   - at91-usart
> 
> % grep NAME drivers/mfd/at91-usart.c
> 	MFD_CELL_NAME("at91_usart_spi");
> 	MFD_CELL_NAME("atmel_usart_serial");

Has two single-element arrays. It registers one or the other, never both
(just like my patch does!)

>>   - stw481x
> 
> * Copyright (C) 2013 ST-Ericsson SA
> 
>>   - vx855
> 
> * Copyright (C) 2009 VIA Technologies, Inc.
> 
>>   - wm8400
> 
> * Copyright 2008 Wolfson Microelectronics PLC.
> 

To my knowledge the definition of "multi" has not been changed
since any of those years.

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Lee Jones]

On Wed, 03 Sep 2025, Marcos Del Sol Vives wrote:

> El 03/09/2025 a las 16:01, Lee Jones escribió:
> >> patch series and thus make it a proper MFD (at the cost of delaying
> >> even further the GPIO inclusion), or keep the struct mfd_cell array
> >> as a single-element array and implement the watchdog later on another
> >> merge request, using that very same array.
> >>
> >> I am however not okay with wasting my time rewriting that to bypass
> >> the MFD API for this, so I can waste even more time later
> >> implementing again the MFD API, just because linguistically
> >> one (right now) is technically not "multi".
> > 
> > I don't get this.  If you implement the WDT now, you will be "multi", so
> > what are you protesting against?
> 
> That GPIO is something required to perform the poweroff sequence, a must
> for any machine, while WDT is just a "nice to have".
> 
> Implementing now the WDT just because of a linguistic preference means
> delaying something more important in favour of a "nice to have".

You use the word "delaying" here.  What's the rush?

If you only need a GPIO driver, then you don't need the MFD part.

> >> That seems very unreasonable, specially when it wouldn't be a first
> >> since at least these other devices are also using MFD with a single
> >> device:
> >>
> >>   - 88pm80
> > 
> > % grep name drivers/mfd/88pm800.c
> > 	.name = "88pm80x-rtc",
> > 	.name = "88pm80x-onkey",
> > 	.name = "88pm80x-regulator",
> > 	.name = "88pm800",
> 
> If you open the file, you'll see it uses five single-element arrays.

Which is fine.

> >>   - 88pm805
> > 
> > % grep name drivers/mfd/88pm805.c       
> > 	.name = "88pm80x-codec",
> > 	.name = "88pm805",
> > 
> 
> Same as above.
> 
> >>   - at91-usart
> > 
> > % grep NAME drivers/mfd/at91-usart.c
> > 	MFD_CELL_NAME("at91_usart_spi");
> > 	MFD_CELL_NAME("atmel_usart_serial");
> 
> Has two single-element arrays. It registers one or the other, never both
> (just like my patch does!)

Fair point.

I guess this is more of a selector than a true concurrent MFD.

> >>   - stw481x
> > 
> > * Copyright (C) 2013 ST-Ericsson SA
> > 
> >>   - vx855
> > 
> > * Copyright (C) 2009 VIA Technologies, Inc.
> > 
> >>   - wm8400
> > 
> > * Copyright 2008 Wolfson Microelectronics PLC.
> > 
> 
> To my knowledge the definition of "multi" has not been changed
> since any of those years.

If they were submitted during my tenure, it is likely that they would
not have been accepted.  Besides, past mistakes is no excuse for making
them in the present.

-- 
Lee Jones [李琼斯]

Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Marcos Del Sol Vives]

El 04/09/2025 a las 12:17, Lee Jones escribió:
>> That GPIO is something required to perform the poweroff sequence, a must
>> for any machine, while WDT is just a "nice to have".
>>
>> Implementing now the WDT just because of a linguistic preference means
>> delaying something more important in favour of a "nice to have".
> 
> You use the word "delaying" here.  What's the rush?
> 
> If you only need a GPIO driver, then you don't need the MFD part.
> 

I would honestly like that my machines can turn off properly and pretty
sure others using these platforms would agree on that, as having to yank
out the power cable is far from ideal.

Adding WDT would lengthen even further the review process. That ignoring
I am doing this as a hobby on my spare time and I'd rather spend my
scarce free time implementing the power off driver than the WDT
(something I'd do out of completion, I have absolutely no use for a WDT
in this machine).

The reason I am using an MFD is that I was asked to back in v2
(https://lore.kernel.org/all/aHElavFTptu0q4Kj@smile.fi.intel.com/).
I'll be CC'ing him.

I was told to create a southbridge driver that would match on PCI
and registered other devices exposed by it as platform drivers.
GPIO was the only functionality implemented at the time, and is
the only functionality implemented right now. So I simply delivered was
I was asked for.