[PATCH v2] gpio: vortex: add new GPIO device driver

[PATCH v2] gpio: vortex: add new GPIO device driver

[Marcos Del Sol Vives]

Add a new simple GPIO device driver for Vortex86 lines of SoCs,
implemented according to their programming reference manual [1].

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 available for less than half the
GPIO pins, and they are not the ones required for the poweroff stuff, so
polling will be required anyway.

[1]: http://www.dmp.com.tw/tech/DMP_Vortex86_Series_Software_Programming_Reference_091216.pdf

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 1ff244fe3e1a..0ad462e8cceb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26568,6 +26568,12 @@ VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
 R:	Matti Vaittinen <mazziesaccount@gmail.com>
 F:	drivers/regulator/irq_helpers.c
 
+VORTEX GPIO DRIVER
+R:	Marcos Del Sol Vives <marcos@orca.pet>
+L:	linux-gpio@vger.kernel.org
+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 44f922e10db2..796fd6d43910 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1066,6 +1066,16 @@ config GPIO_TS5500
 	  blocks of the TS-5500: DIO1, DIO2 and the LCD port, and the TS-5600
 	  LCD port.
 
+config GPIO_VORTEX
+	tristate "Vortex86 SoC GPIO support"
+	depends on CPU_SUP_VORTEX_32 || COMPILE_TEST
+	help
+	  Driver to access the five 8-bit bidirectional GPIO ports present on
+	  all DM&P Vortex86 SoCs.
+
+	  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 88dedd298256..d226fc751686 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -196,6 +196,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..cf14d56d22d9
--- /dev/null
+++ b/drivers/gpio/gpio-vortex.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  GPIO driver for Vortex86 SoCs
+ *
+ *  Author: Marcos Del Sol Vives <marcos@orca.pet>
+ *
+ *  Based on the it87xx GPIO driver by Diego Elio Pettenò
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/spinlock.h>
+#include <linux/gpio/driver.h>
+#include <linux/platform_device.h>
+
+#define GPIO_PORTS	5
+#define GPIO_PER_PORT	8
+#define GPIO_COUNT	(GPIO_PORTS * GPIO_PER_PORT)
+
+#define GPIO_DATA_BASE		0x78
+#define GPIO_DIRECTION_BASE	0x98
+
+static struct platform_device *pdev;
+
+static DEFINE_SPINLOCK(gpio_lock);
+
+static int vortex_gpio_get(struct gpio_chip *chip, unsigned int gpio_num)
+{
+	uint8_t port = gpio_num / GPIO_PER_PORT;
+	uint8_t bit  = gpio_num % GPIO_PER_PORT;
+	uint8_t val;
+
+	val = inb(GPIO_DATA_BASE + port);
+	return !!(val & (1 << bit));
+}
+
+static int vortex_gpio_direction_in(struct gpio_chip *chip, unsigned int gpio_num)
+{
+	uint8_t port = gpio_num / GPIO_PER_PORT;
+	uint8_t bit  = gpio_num % GPIO_PER_PORT;
+	unsigned long flags;
+	uint8_t dir;
+
+	spin_lock_irqsave(&gpio_lock, flags);
+
+	dir = inb(GPIO_DIRECTION_BASE + port);
+	dir &= ~(1 << bit); /* 0 = input */
+	outb(dir, GPIO_DIRECTION_BASE + port);
+
+	spin_unlock_irqrestore(&gpio_lock, flags);
+
+	return 0;
+}
+
+static int vortex_gpio_set(struct gpio_chip *chip, unsigned int gpio_num, int value)
+{
+	uint8_t port = gpio_num / GPIO_PER_PORT;
+	uint8_t bit  = gpio_num % GPIO_PER_PORT;
+	unsigned long flags;
+	uint8_t dat;
+
+	spin_lock_irqsave(&gpio_lock, flags);
+
+	dat = inb(GPIO_DATA_BASE + port);
+	if (value)
+		dat |= (1 << bit);
+	else
+		dat &= ~(1 << bit);
+	outb(dat, GPIO_DATA_BASE + port);
+
+	spin_unlock_irqrestore(&gpio_lock, flags);
+
+	return 0;
+}
+
+static int vortex_gpio_direction_out(struct gpio_chip *chip, unsigned int gpio_num, int value)
+{
+	uint8_t port = gpio_num / GPIO_PER_PORT;
+	uint8_t bit  = gpio_num % GPIO_PER_PORT;
+	unsigned long flags;
+	uint8_t dir, dat;
+
+	spin_lock_irqsave(&gpio_lock, flags);
+
+	/* Have to set direction first. Else writes to data are ignored. */
+	dir = inb(GPIO_DIRECTION_BASE + port);
+	dir |= (1 << bit); /* 1 = output */
+	outb(dir, GPIO_DIRECTION_BASE + port);
+
+	dat = inb(GPIO_DATA_BASE + port);
+	if (value)
+		dat |= (1 << bit);
+	else
+		dat &= ~(1 << bit);
+	outb(dat, GPIO_DATA_BASE + port);
+
+	spin_unlock_irqrestore(&gpio_lock, flags);
+
+	return 0;
+}
+
+static char labels[GPIO_COUNT][sizeof("vortex_gpXY")];
+static char *labels_table[GPIO_COUNT];
+
+static struct gpio_chip gpio_chip = {
+	.label			= KBUILD_MODNAME,
+	.owner			= THIS_MODULE,
+	.get			= vortex_gpio_get,
+	.direction_input	= vortex_gpio_direction_in,
+	.set_rv			= vortex_gpio_set,
+	.direction_output	= vortex_gpio_direction_out,
+	.base			= -1,
+	.ngpio			= GPIO_COUNT,
+	.names			= (const char * const *)labels_table,
+};
+
+static int vortex_gpio_probe(struct platform_device *pdev)
+{
+	/* Set up GPIO labels */
+	for (int i = 0; i < GPIO_COUNT; i++) {
+		sprintf(labels[i], "vortex_gp%u%u", i / 8, i % 8);
+		labels_table[i] = &labels[i][0];
+	}
+
+	return devm_gpiochip_add_data(&pdev->dev, &gpio_chip, NULL);
+}
+
+static struct platform_driver vortex_gpio_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.owner = THIS_MODULE,
+	},
+	.probe = vortex_gpio_probe,
+};
+
+static struct resource vortex_gpio_resources[] = {
+	DEFINE_RES_IO_NAMED(GPIO_DATA_BASE, GPIO_PORTS, KBUILD_MODNAME " data"),
+	DEFINE_RES_IO_NAMED(GPIO_DIRECTION_BASE, GPIO_PORTS, KBUILD_MODNAME " dir"),
+};
+
+static int __init vortex_gpio_init(void)
+{
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_VORTEX) {
+		pr_err("Not a Vortex86 CPU, refusing to load\n");
+		return -ENODEV;
+	}
+
+	pdev = platform_create_bundle(&vortex_gpio_driver, vortex_gpio_probe,
+			vortex_gpio_resources, ARRAY_SIZE(vortex_gpio_resources),
+			NULL, 0);
+	return PTR_ERR_OR_ZERO(pdev);
+}
+
+static void __exit vortex_gpio_exit(void)
+{
+	platform_device_unregister(pdev);
+	platform_driver_unregister(&vortex_gpio_driver);
+}
+
+module_init(vortex_gpio_init);
+module_exit(vortex_gpio_exit);
+
+MODULE_AUTHOR("Marcos Del Sol Vives <marcos@orca.pet>");
+MODULE_DESCRIPTION("GPIO driver for Vortex86 SoCs");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Bartosz Golaszewski]

On Wed, Jul 9, 2025 at 11:16 AM Marcos Del Sol Vives <marcos@orca.pet> wrote:
>
> Add a new simple GPIO device driver for Vortex86 lines of SoCs,
> implemented according to their programming reference manual [1].
>
> 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 available for less than half the
> GPIO pins, and they are not the ones required for the poweroff stuff, so
> polling will be required anyway.
>

(...)

> +
> +static int __init vortex_gpio_init(void)
> +{
> +       if (boot_cpu_data.x86_vendor != X86_VENDOR_VORTEX) {
> +               pr_err("Not a Vortex86 CPU, refusing to load\n");
> +               return -ENODEV;
> +       }
> +
> +       pdev = platform_create_bundle(&vortex_gpio_driver, vortex_gpio_probe,
> +                       vortex_gpio_resources, ARRAY_SIZE(vortex_gpio_resources),
> +                       NULL, 0);
> +       return PTR_ERR_OR_ZERO(pdev);
> +}
> +

This looks better but I admit I'm not an expert in x86 platforms so
I'll allow myself to Cc Andy. Is this how it's typically done in x86?
Is this module visible in ACPI in any way that would allow us to
leverage the platform device core? Or do we need to try to register
the device unconditionally on all Vortex platforms?

Bart

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Marcos Del Sol Vives]

El 11/07/2025 a las 12:19, Bartosz Golaszewski escribió:
> This looks better but I admit I'm not an expert in x86 platforms so
> I'll allow myself to Cc Andy. Is this how it's typically done in x86?
> Is this module visible in ACPI in any way that would allow us to
> leverage the platform device core? Or do we need to try to register
> the device unconditionally on all Vortex platforms?

Again I want to point out I am not an expert by any means. This is my first
kernel driver and I am writting it as a hobbyst, not as a company employee.

Regarding ACPI: I have just now decompiled the DSDT for the Vortex86DX3
machine and I do not see any Device() claiming ownership of 0x78 or 0x98,
or mentioning GPIO at all:

root@vdx3:/home/marcos/acpi# ls -l *.dsl
-rw-r--r-- 1 root root   3459 Jul 11 16:05 APIC.dsl
-rw-r--r-- 1 root root 196800 Jul 11 16:05 DSDT.dsl
-rw-r--r-- 1 root root   9211 Jul 11 16:22 FACP.dsl
-rw-r--r-- 1 root root   1364 Jul 11 16:22 FACS.dsl
-rw-r--r-- 1 root root   1552 Jul 11 16:23 MSDM.dsl
-rw-r--r-- 1 root root      0 Jul 11 16:07 OEMB.dsl
-rw-r--r-- 1 root root   3957 Jul 11 16:07 SLIC.dsl
root@vdx3:/home/marcos/acpi# grep -Ri gpio *.dsl
root@vdx3:/home/marcos/acpi# grep -Ri 0x0078 *.dsl

Manually skimming through DSDT does not yield anything useful either.

This kinda confirms what the company told me: the machine does not properly
support ACPI, it has a set of fake tables enough to convince Windows 7 into
booting.

The Vortex86MX board does not even have those, and has no ACPI tables
whatsoever.

And regarding something I forgot to answer on the previous email about the
future of these machines: the Vortex86MX is a i586 processor, and this
Vortex86DX3 is a dual-core i686 processor with SSE1 support that I myself
bought brand new from the manufacturer, indicating they are still making
and supporting them.

The company is seemingly launching next year also a Vortex86EX3 with proper
i686 and SSE2 support, I guess because Intel's patents have finally expired.

So I do not think removing i486 support is gonna be an issue except for
very ancient processors that the company is not making anymore anyway.

Thanks,
Marcos

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Andy Shevchenko]

On Fri, Jul 11, 2025 at 04:24:41PM +0200, Marcos Del Sol Vives wrote:
> El 11/07/2025 a las 12:19, Bartosz Golaszewski escribió:

> > This looks better but I admit I'm not an expert in x86 platforms so
> > I'll allow myself to Cc Andy. Is this how it's typically done in x86?
> > Is this module visible in ACPI in any way that would allow us to
> > leverage the platform device core? Or do we need to try to register
> > the device unconditionally on all Vortex platforms?
> 
> Again I want to point out I am not an expert by any means. This is my first
> kernel driver and I am writting it as a hobbyst, not as a company employee.

Oh, I see. Sorry that I'm asking too much (do I?) Unfortunately the review
might be not well appreciated process by the author, but we are all for
having the better solutions. That's the part of learning and R&D process.

As for this discussion it seems you already committed into enormous work
to gather all pieces and knowledge, but we need more.

> Regarding ACPI: I have just now decompiled the DSDT for the Vortex86DX3
> machine and I do not see any Device() claiming ownership of 0x78 or 0x98,
> or mentioning GPIO at all:
> 
> root@vdx3:/home/marcos/acpi# ls -l *.dsl
> -rw-r--r-- 1 root root   3459 Jul 11 16:05 APIC.dsl
> -rw-r--r-- 1 root root 196800 Jul 11 16:05 DSDT.dsl
> -rw-r--r-- 1 root root   9211 Jul 11 16:22 FACP.dsl
> -rw-r--r-- 1 root root   1364 Jul 11 16:22 FACS.dsl
> -rw-r--r-- 1 root root   1552 Jul 11 16:23 MSDM.dsl
> -rw-r--r-- 1 root root      0 Jul 11 16:07 OEMB.dsl
> -rw-r--r-- 1 root root   3957 Jul 11 16:07 SLIC.dsl
> root@vdx3:/home/marcos/acpi# grep -Ri gpio *.dsl
> root@vdx3:/home/marcos/acpi# grep -Ri 0x0078 *.dsl

Oh, it's not as easy as grepping. One need to read and understand the ACPI
language for that.

> Manually skimming through DSDT does not yield anything useful either.
> 
> This kinda confirms what the company told me: the machine does not properly
> support ACPI, it has a set of fake tables enough to convince Windows 7 into
> booting.

Let's see! As I asked in the previous mail, please share some information in
full.

> The Vortex86MX board does not even have those, and has no ACPI tables
> whatsoever.

Yeah, but you can create them from scratch if you wish. I once have done
this for Intel Merrifield (in U-Boot) and it went pretty well. From that
time we use that board as regular PC.

> And regarding something I forgot to answer on the previous email about the
> future of these machines: the Vortex86MX is a i586 processor, and this
> Vortex86DX3 is a dual-core i686 processor with SSE1 support that I myself
> bought brand new from the manufacturer, indicating they are still making
> and supporting them.

Would be nice to get the updated documentation... But it might be not available
or at best under NDA.

> The company is seemingly launching next year also a Vortex86EX3 with proper
> i686 and SSE2 support, I guess because Intel's patents have finally expired.
> 
> So I do not think removing i486 support is gonna be an issue except for
> very ancient processors that the company is not making anymore anyway.

Thanks for clarifications here!

And thank you for your efforts, I would like to help as much as I can.
Again, sorry, if my first messages looked a bit rejecting. I do not mean
that, it's just a professional deformation due to tons of reviews I have
done in the past.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Marcos Del Sol Vives]

El 11/07/2025 a las 16:53, Andy Shevchenko escribió:
> Can you share (via some sharing resource) the following (as root user!):
> 1) `dmesg` to the boot to shell when kernel command line has 'ignore_loglevel'
> 2) `lspci -nk -vv`
> 3) `acpidump -o vortex-dx3.dat` (the *.dat file)
> 4) `grep -H 15 /sys/bus/acpi/devices/*/status`
> 5) `cat /proc/interrupts`
> 6) `cat /proc/iomem`
> 7) `cat /proc/ioport`

I have uploaded all the files to https://orca.pet/vortex/gpio-patch-files.tar.xz.
I have also added the configuration file I used for the kernel, as well as
the cmdline.

The machine is currently running Linux on commit
d006330be3f782ff3fb7c3ed51e617e01f29a465 with this patch and another that
builds upon it for poweroff applied.

The cmdline has pci=nomsi due to a hardware issue acknowledged by DM&P
(https://2018.compactpc.com.tw/2014%20DMP%20Webiste/Linux%20Support%20List/debian_9.4_installation_guide.pdf)
for which I have already submited another patch to the PCI mailing list to
disable it automatically
(https://lore.kernel.org/all/20250705233209.721507-1-marcos@orca.pet/).

If you need any other files please let me know :)

> Is that the only datasheet you have?

Yes. I guess I could ask DM&P or ICOP for another under NDA but as of today
I do not have access to anything else.

El 11/07/2025 a las 17:01, Andy Shevchenko escribió:
>> Again I want to point out I am not an expert by any means. This is my first
>> kernel driver and I am writting it as a hobbyst, not as a company employee.
> 
> Oh, I see. Sorry that I'm asking too much (do I?) Unfortunately the review
> might be not well appreciated process by the author, but we are all for
> having the better solutions. That's the part of learning and R&D process.

Not at all, you're not asking for too much at all! I understand you don't
want to merge a half baked driver in the kernel.

I was just mentioning that to clarify that if the driver looks sloppy at
the moment is not because of a lack of care, it's rather because of a lack
of experience - I am not familiar with neither the Linux driver model nor
the technologies that are deprecated and should be avoided.

Also and to clarify that, as I said above, I do not have access to the
documentation an employee would have about the SoC's inner workings.

>> The company is seemingly launching next year also a Vortex86EX3 with proper
>> i686 and SSE2 support, I guess because Intel's patents have finally expired.
>>
>> So I do not think removing i486 support is gonna be an issue except for
>> very ancient processors that the company is not making anymore anyway.
> 
> Thanks for clarifications here!
> 
> And thank you for your efforts, I would like to help as much as I can.
> Again, sorry, if my first messages looked a bit rejecting. I do not mean
> that, it's just a professional deformation due to tons of reviews I have
> done in the past.
> 

No problem, sir! :)

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Andy Shevchenko]

+Cc: William,
who is an expert in embedded industrial x86 platforms and might help with this.

Bart, thanks for Cc'ing me. I have tons of questions and comments regarding this.

On Wed, Jul 09, 2025 at 11:15:40AM +0200, Marcos Del Sol Vives wrote:
> Add a new simple GPIO device driver for Vortex86 lines of SoCs,
> implemented according to their programming reference manual [1].
> 
> 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 available for less than half the
> GPIO pins, and they are not the ones required for the poweroff stuff, so
> polling will be required anyway.

> [1]: http://www.dmp.com.tw/tech/DMP_Vortex86_Series_Software_Programming_Reference_091216.pdf
> 

Make this a Link tag by removing a blank line and converting above to

Link: http://www.dmp.com.tw/tech/DMP_Vortex86_Series_Software_Programming_Reference_091216.pdf [1]

> Signed-off-by: Marcos Del Sol Vives <marcos@orca.pet>

But the first question first. Is this not a dead code? I mean I have heard that
i486SX is not supported in Linux kernel as there is no more FP emu layer. Of
course it might be not a problem in some cases, but still. Second point is that
there is an ongoing (still ongoing?) discussion on removal i486 support completely.
Would it affect these SoCs? (According to the shared manual seems the answer is
"yes, it will".

...

> +	help
> +	  Driver to access the five 8-bit bidirectional GPIO ports present on
> +	  all DM&P Vortex86 SoCs.

...

> + *  Based on the it87xx GPIO driver by Diego Elio Pettenò

Why that driver can't be reused?

> + */

...

> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Why? The driver should use dev_*() macros which will uniquely define the device
for what the message is printed.

...

> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/ioport.h>
> +#include <linux/spinlock.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/platform_device.h>

This should be sorted alphabetically. Also follow IWYU principle.

...

> +static DEFINE_SPINLOCK(gpio_lock);

Global lock? Why?

...

> +static int vortex_gpio_get(struct gpio_chip *chip, unsigned int gpio_num)
> +{
> +	uint8_t port = gpio_num / GPIO_PER_PORT;
> +	uint8_t bit  = gpio_num % GPIO_PER_PORT;
> +	uint8_t val;

Use kernel types for kernel. Include types.h (you probably missed it anyway)
and then convert above to u8.

> +	val = inb(GPIO_DATA_BASE + port);

Better to use ioread8() / iowrite8() and "map" IO ports (see, for example,
how drivers/pinctrl/intel-pinctrl-lynxpoint.c does).

> +	return !!(val & (1 << bit));

BIT() from bits.h will help a lot.

> +}
> +
> +static int vortex_gpio_direction_in(struct gpio_chip *chip, unsigned int gpio_num)
> +{
> +	uint8_t port = gpio_num / GPIO_PER_PORT;
> +	uint8_t bit  = gpio_num % GPIO_PER_PORT;
> +	unsigned long flags;
> +	uint8_t dir;

All the same comments, plus...

> +	spin_lock_irqsave(&gpio_lock, flags);

...use APIs from cleanup.h, i.e. guard() in this case.

> +	dir = inb(GPIO_DIRECTION_BASE + port);
> +	dir &= ~(1 << bit); /* 0 = input */
> +	outb(dir, GPIO_DIRECTION_BASE + port);
> +
> +	spin_unlock_irqrestore(&gpio_lock, flags);
> +
> +	return 0;
> +}

...

So, the above is pretty much simple, why doesn't it use gpio-regmap with the
respective configuration? Moreover, SX and DX variants are differ since the
latter one may provide an IRQ chip, for which gpio-regmap also can be used,
i.o.w. with that done, it will be quite easy to support both.

...

> +static int vortex_gpio_probe(struct platform_device *pdev)
> +{
> +	/* Set up GPIO labels */
> +	for (int i = 0; i < GPIO_COUNT; i++) {
> +		sprintf(labels[i], "vortex_gp%u%u", i / 8, i % 8);
> +		labels_table[i] = &labels[i][0];

'&...[0]' is redundant.

> +	}

Why this can't be made static once?

> +	return devm_gpiochip_add_data(&pdev->dev, &gpio_chip, NULL);
> +}
> +
> +static struct platform_driver vortex_gpio_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,

> +		.owner = THIS_MODULE,

This field is not needed for ages (15+ years). Is this driver got dusted for this long?

> +	},
> +	.probe = vortex_gpio_probe,
> +};

...

> +static struct resource vortex_gpio_resources[] = {
> +	DEFINE_RES_IO_NAMED(GPIO_DATA_BASE, GPIO_PORTS, KBUILD_MODNAME " data"),
> +	DEFINE_RES_IO_NAMED(GPIO_DIRECTION_BASE, GPIO_PORTS, KBUILD_MODNAME " dir"),

Named resources? Why?

> +};

...

> +static int __init vortex_gpio_init(void)
> +{
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_VORTEX) {
> +		pr_err("Not a Vortex86 CPU, refusing to load\n");
> +		return -ENODEV;
> +	}
> +
> +	pdev = platform_create_bundle(&vortex_gpio_driver, vortex_gpio_probe,
> +			vortex_gpio_resources, ARRAY_SIZE(vortex_gpio_resources),
> +			NULL, 0);
> +	return PTR_ERR_OR_ZERO(pdev);
> +}

Oh my... Can you elaborate more on this ugly hack. Why do we need this at all?
What's wrong with the BIOS or other firmware that is provided?
(The documentation mentions BIOS, btw.)

Also, is this anyhow visible as a PCI device? Is it part of LPC (docs suggests
so for SX, but not so clear in DX diagram)?

...

On top of that the GPIO3 is marked as one with the pin muxing. Where is the driver
for it? Or what are the plans about it?

GPIO4 seems muxed with UART, so also subject to pin muxing.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Bartosz Golaszewski]

On Fri, Jul 11, 2025 at 1:43 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> +Cc: William,
> who is an expert in embedded industrial x86 platforms and might help with this.
>
> Bart, thanks for Cc'ing me. I have tons of questions and comments regarding this.
>

FYI: I haven't given it a thorough review yet as I wanted to clarify
the way the driver is registered first so thanks for looking into it.

Bart

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Andy Shevchenko]

On Fri, Jul 11, 2025 at 01:56:26PM +0200, Bartosz Golaszewski wrote:
> On Fri, Jul 11, 2025 at 1:43 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > +Cc: William,
> > who is an expert in embedded industrial x86 platforms and might help with this.
> >
> > Bart, thanks for Cc'ing me. I have tons of questions and comments regarding this.
> 
> FYI: I haven't given it a thorough review yet as I wanted to clarify
> the way the driver is registered first so thanks for looking into it.

Here is even more. There are drivers for 0x6030 chipset (rdc321x-southbridge.c),
this one is unclear to what chipset it is. Is it compatible with gpio-rdc321x.c?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Marcos Del Sol Vives]

El 11/07/2025 a las 13:43, Andy Shevchenko escribió:
>> + *  Based on the it87xx GPIO driver by Diego Elio Pettenò
> 
> Why that driver can't be reused?
> 

The driver uses a completely different port address and operation. It is
based in the sense I used it as an example of how a I/O mapped GPIO driver
should work, but nothing else.

>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> Why? The driver should use dev_*() macros which will uniquely define the device
> for what the message is printed.

The error thrown if the module is attempted to load on a non-Vortex
processor happens before the platform device is created.

>> +static DEFINE_SPINLOCK(gpio_lock);
> 
> Global lock? Why?

Becase at most there'll be one GPIO device of this kind loaded, so it didn't
make sense to me to create a dynamically-allocated private structure of data
for a single global lock.

> So, the above is pretty much simple, why doesn't it use gpio-regmap with the
> respective configuration? Moreover, SX and DX variants are differ since the
> latter one may provide an IRQ chip, for which gpio-regmap also can be used,
> i.o.w. with that done, it will be quite easy to support both.

Again, I am not an expert on the Linux kernel, but I did not see any code
or examples using neither gpio-mmio nor gpio-regmap for I/O-mapped registers.

IRQ is only available for the first two ports out of the five available.
As said in the comment, for shutting down the machine, port 3 is required
so I'm gonna need to poll anyway.

>> +static int vortex_gpio_probe(struct platform_device *pdev)
>> +{
>> +	/* Set up GPIO labels */
>> +	for (int i = 0; i < GPIO_COUNT; i++) {
>> +		sprintf(labels[i], "vortex_gp%u%u", i / 8, i % 8);
>> +		labels_table[i] = &labels[i][0];
> '&...[0]' is redundant.
> Why this can't be made static once?

It absolutely can.

>> +		.owner = THIS_MODULE,
> 
> This field is not needed for ages (15+ years). Is this driver got dusted for this long?

I saw the field on the documentation as well as on the IT87 driver I was
using as a reference, so I kept it.

>> +static struct resource vortex_gpio_resources[] = {
>> +	DEFINE_RES_IO_NAMED(GPIO_DATA_BASE, GPIO_PORTS, KBUILD_MODNAME " data"),
>> +	DEFINE_RES_IO_NAMED(GPIO_DIRECTION_BASE, GPIO_PORTS, KBUILD_MODNAME " dir"),
> 
> Named resources? Why?

So they appear with the proper name in /proc/iomem. That's the only reason:

0000-0cf7 : PCI Bus 0000:00
  0000-001f : dma1
  0020-0021 : pic1
  0040-0043 : timer0
  0050-0053 : timer1
  0060-0060 : keyboard
  0061-0061 : PNP0800:00
  0064-0064 : keyboard
  0070-0071 : rtc0
  0078-007c : gpio_vortex-data
  0080-008f : dma page reg
  0098-009c : gpio_vortex-dir
  00a0-00a1 : pic2
  00c0-00df : dma2
  00f0-00ff : PNP0C04:00
...

>> +static int __init vortex_gpio_init(void)
>> +{
>> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_VORTEX) {
>> +		pr_err("Not a Vortex86 CPU, refusing to load\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pdev = platform_create_bundle(&vortex_gpio_driver, vortex_gpio_probe,
>> +			vortex_gpio_resources, ARRAY_SIZE(vortex_gpio_resources),
>> +			NULL, 0);
>> +	return PTR_ERR_OR_ZERO(pdev);
>> +}
> 
> Oh my... Can you elaborate more on this ugly hack. Why do we need this at all?
> What's wrong with the BIOS or other firmware that is provided?
> (The documentation mentions BIOS, btw.)
> 
> Also, is this anyhow visible as a PCI device? Is it part of LPC (docs suggests
> so for SX, but not so clear in DX diagram)?

The device is available at a hardcoded address for all Vortex86 devices, but
it is not part of any device in particular, and I don't see any good way
to enable it on all Vortex devices other than checking the CPU vendor.

These are the PCI devices on my DX3:

00:00.0 Host bridge [0600]: RDC Semiconductor, Inc. R6023 Host Bridge [17f3:6023] (rev 02)
00:01.0 PCI bridge [0604]: RDC Semiconductor, Inc. PCI/PCI-X to PCI-E Bridge [17f3:1031] (rev 01)
00:02.0 PCI bridge [0604]: RDC Semiconductor, Inc. PCI/PCI-X to PCI-E Bridge [17f3:1031] (rev 01)
00:07.0 ISA bridge [0601]: RDC Semiconductor, Inc. R6035 ISA Bridge [17f3:6035] (rev 01)
00:07.1 ISA bridge [0601]: RDC Semiconductor, Inc. R6035 ISA Bridge [17f3:6035] (rev 01)
00:0a.0 USB controller [0c03]: RDC Semiconductor, Inc. R6060 USB 1.1 Controller [17f3:6060] (rev 14)
00:0a.1 USB controller [0c03]: RDC Semiconductor, Inc. R6061 USB 2.0 Controller [17f3:6061] (rev 08)
00:0c.0 IDE interface [0101]: RDC Semiconductor, Inc. R1012 IDE Controller [17f3:1012] (rev 02)
00:0d.0 VGA compatible controller [0300]: RDC Semiconductor, Inc. RDC M2015 VGA-compatible graphics adapter [17f3:2015]
00:0e.0 Audio device [0403]: RDC Semiconductor, Inc. R3010 HD Audio Controller [17f3:3010] (rev 01)
01:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev 07)

There's no multipurpose device that actually claims ownership of these I/O
ports according to the PCI's configuration section.

In fact the Vortex86MX-based EBOX-3350MX mini PC has a different host bridge
(R6021) and different ISA bridges (R6031), but has the very same GPIO.

And in "dmidecode" I just see a lot of lies (this device certainly does NOT
have fans or a parallel port) plus "To Be Filled By O.E.M." fields. Also
matching on BIOS does not seem a good idea since there are other industrial
machines that may not be using or reporting the same BIOS versions.

> On top of that the GPIO3 is marked as one with the pin muxing. Where is the driver
> for it? Or what are the plans about it?
> 
> GPIO4 seems muxed with UART, so also subject to pin muxing.

The documentation does not cover how to use those UARTs and the pins default
to I/O, so I would say they're not a problem for now.

Ultimately, as mentioned, the goal is implementing a correct power off
sequence for ICOP EBOX machines and DM&P evaluation boards, which require
manually polling for the power off button, then setting an output pin
low to shut off power to the machine.

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Andy Shevchenko]

On Fri, Jul 11, 2025 at 03:52:39PM +0200, Marcos Del Sol Vives wrote:
> El 11/07/2025 a las 13:43, Andy Shevchenko escribió:

...

> >> + *  Based on the it87xx GPIO driver by Diego Elio Pettenò

> > Why that driver can't be reused?
> 
> The driver uses a completely different port address and operation. It is
> based in the sense I used it as an example of how a I/O mapped GPIO driver
> should work, but nothing else.

Yeah, I got it from reading the code.

...

> >> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > 
> > Why? The driver should use dev_*() macros which will uniquely define the device
> > for what the message is printed.
> 
> The error thrown if the module is attempted to load on a non-Vortex
> processor happens before the platform device is created.

This is a sign of a problematic design of the driver most likely.
And we know now that it's indeed the case.

...

> >> +static DEFINE_SPINLOCK(gpio_lock);
> > 
> > Global lock? Why?
> 
> Becase at most there'll be one GPIO device of this kind loaded, so it didn't
> make sense to me to create a dynamically-allocated private structure of data
> for a single global lock.

Btw, gpio-regmap provides a lcok capability as well.

...

> > So, the above is pretty much simple, why doesn't it use gpio-regmap with the
> > respective configuration? Moreover, SX and DX variants are differ since the
> > latter one may provide an IRQ chip, for which gpio-regmap also can be used,
> > i.o.w. with that done, it will be quite easy to support both.
> 
> Again, I am not an expert on the Linux kernel, but I did not see any code
> or examples using neither gpio-mmio nor gpio-regmap for I/O-mapped registers.

$ git grep -lw '\.io_port[[:space:]]\+= true,'
drivers/counter/104-quad-8.c
drivers/gpio/gpio-104-dio-48e.c
drivers/gpio/gpio-104-idi-48.c
drivers/gpio/gpio-104-idio-16.c
drivers/gpio/gpio-exar.c
drivers/gpio/gpio-gpio-mm.c
drivers/gpio/gpio-pci-idio-16.c
drivers/gpio/gpio-pcie-idio-24.c
drivers/gpio/gpio-ws16c48.c
drivers/iio/addac/stx104.c
drivers/iio/dac/cio-dac.c

Take a look.

...

> IRQ is only available for the first two ports out of the five available.

Would  it be a problem to support them?

> As said in the comment, for shutting down the machine, port 3 is required
> so I'm gonna need to poll anyway.

Comment? Perhaps this all information should be provided in the commit message.

...

> >> +		labels_table[i] = &labels[i][0];
> > '&...[0]' is redundant.
> > Why this can't be made static once?
> 
> It absolutely can.

Better to do that way if even needed. Usually the labeling is part of the
firmware description and I'm quite skeptical about the idea to have it hard
coded. So, let's just drop it altogether.

...

> >> +		.owner = THIS_MODULE,
> > 
> > This field is not needed for ages (15+ years). Is this driver got dusted for this long?
> 
> I saw the field on the documentation as well as on the IT87 driver I was
> using as a reference, so I kept it.

No need, that driver is too old to be a good example for a modern code.

...

> >> +	DEFINE_RES_IO_NAMED(GPIO_DATA_BASE, GPIO_PORTS, KBUILD_MODNAME " data"),
> >> +	DEFINE_RES_IO_NAMED(GPIO_DIRECTION_BASE, GPIO_PORTS, KBUILD_MODNAME " dir"),
> > 
> > Named resources? Why?
> 
> So they appear with the proper name in /proc/iomem. That's the only reason:
> 
> 0000-0cf7 : PCI Bus 0000:00
>   0000-001f : dma1
>   0020-0021 : pic1
>   0040-0043 : timer0
>   0050-0053 : timer1
>   0060-0060 : keyboard
>   0061-0061 : PNP0800:00
>   0064-0064 : keyboard
>   0070-0071 : rtc0
>   0078-007c : gpio_vortex-data
>   0080-008f : dma page reg
>   0098-009c : gpio_vortex-dir
>   00a0-00a1 : pic2
>   00c0-00df : dma2
>   00f0-00ff : PNP0C04:00

Use of the proper device driver design this won't be needed.

...

> >> +static int __init vortex_gpio_init(void)
> >> +{
> >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_VORTEX) {
> >> +		pr_err("Not a Vortex86 CPU, refusing to load\n");
> >> +		return -ENODEV;
> >> +	}
> >> +
> >> +	pdev = platform_create_bundle(&vortex_gpio_driver, vortex_gpio_probe,
> >> +			vortex_gpio_resources, ARRAY_SIZE(vortex_gpio_resources),
> >> +			NULL, 0);
> >> +	return PTR_ERR_OR_ZERO(pdev);
> >> +}
> > 
> > Oh my... Can you elaborate more on this ugly hack. Why do we need this at all?
> > What's wrong with the BIOS or other firmware that is provided?
> > (The documentation mentions BIOS, btw.)
> > 
> > Also, is this anyhow visible as a PCI device? Is it part of LPC (docs suggests
> > so for SX, but not so clear in DX diagram)?
> 
> The device is available at a hardcoded address for all Vortex86 devices, but
> it is not part of any device in particular, and I don't see any good way
> to enable it on all Vortex devices other than checking the CPU vendor.

As the driver for 0x6030 shows, you need a South Bridge driver that will do all
this. We do like this on many x86 devices (see lpc_ich.c as an example).

So, this hack is not needed at all. Just use normal PCI ID (either 0x6023 in
your case or 0x1031, depending on what `lspci -nk` shows in terms of driver in
use without this driver being enabled.

> These are the PCI devices on my DX3:
> 
> 00:00.0 Host bridge [0600]: RDC Semiconductor, Inc. R6023 Host Bridge [17f3:6023] (rev 02)
> 00:01.0 PCI bridge [0604]: RDC Semiconductor, Inc. PCI/PCI-X to PCI-E Bridge [17f3:1031] (rev 01)
> 00:02.0 PCI bridge [0604]: RDC Semiconductor, Inc. PCI/PCI-X to PCI-E Bridge [17f3:1031] (rev 01)
> 00:07.0 ISA bridge [0601]: RDC Semiconductor, Inc. R6035 ISA Bridge [17f3:6035] (rev 01)
> 00:07.1 ISA bridge [0601]: RDC Semiconductor, Inc. R6035 ISA Bridge [17f3:6035] (rev 01)
> 00:0a.0 USB controller [0c03]: RDC Semiconductor, Inc. R6060 USB 1.1 Controller [17f3:6060] (rev 14)
> 00:0a.1 USB controller [0c03]: RDC Semiconductor, Inc. R6061 USB 2.0 Controller [17f3:6061] (rev 08)
> 00:0c.0 IDE interface [0101]: RDC Semiconductor, Inc. R1012 IDE Controller [17f3:1012] (rev 02)
> 00:0d.0 VGA compatible controller [0300]: RDC Semiconductor, Inc. RDC M2015 VGA-compatible graphics adapter [17f3:2015]
> 00:0e.0 Audio device [0403]: RDC Semiconductor, Inc. R3010 HD Audio Controller [17f3:3010] (rev 01)
> 01:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev 07)
> 
> There's no multipurpose device that actually claims ownership of these I/O
> ports according to the PCI's configuration section.

Can you share (via some sharing resource) the following (as root user!):
1) `dmesg` to the boot to shell when kernel command line has 'ignore_loglevel'
2) `lspci -nk -vv`
3) `acpidump -o vortex-dx3.dat` (the *.dat file)
4) `grep -H 15 /sys/bus/acpi/devices/*/status`
5) `cat /proc/interrupts`
6) `cat /proc/iomem`
7) `cat /proc/ioport`

?

> In fact the Vortex86MX-based EBOX-3350MX mini PC has a different host bridge
> (R6021) and different ISA bridges (R6031), but has the very same GPIO.

Right, so they need to re-use the platform device driver that is a child of the
PCI driver that enumerates everything which is not enumerable. But let's first
see the above mentioned data (output) to understand which of them is the most
plausible to use.

> And in "dmidecode" I just see a lot of lies (this device certainly does NOT
> have fans or a parallel port) plus "To Be Filled By O.E.M." fields. Also
> matching on BIOS does not seem a good idea since there are other industrial
> machines that may not be using or reporting the same BIOS versions.

Yeah, DMI often is not much helpful on the mass-marked devices.

> > On top of that the GPIO3 is marked as one with the pin muxing. Where is the driver
> > for it? Or what are the plans about it?
> > 
> > GPIO4 seems muxed with UART, so also subject to pin muxing.
> 
> The documentation does not cover how to use those UARTs and the pins default
> to I/O, so I would say they're not a problem for now.

Is that the only datasheet you have?

> Ultimately, as mentioned, the goal is implementing a correct power off
> sequence for ICOP EBOX machines and DM&P evaluation boards, which require
> manually polling for the power off button, then setting an output pin
> low to shut off power to the machine.

While fully understanding the goal, the upstreaming process may require to
follow the common Linux device model, which tries to replicate HW topology in
the SW as much as possible (taking into account firmwares and PCB level
solutions). That's why we don't usually support shortcuts. It's better to
implement a set of very basic drivers according to the model, than hack
everything up in one driver somewhere. That simply does not scale.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Marcos Del Sol Vives]

El 11/07/2025 a las 16:53, Andy Shevchenko escribió:
>> Again, I am not an expert on the Linux kernel, but I did not see any code
>> or examples using neither gpio-mmio nor gpio-regmap for I/O-mapped registers.
> 
> $ git grep -lw '\.io_port[[:space:]]\+= true,'
> drivers/counter/104-quad-8.c
> drivers/gpio/gpio-104-dio-48e.c
> drivers/gpio/gpio-104-idi-48.c
> drivers/gpio/gpio-104-idio-16.c
> drivers/gpio/gpio-exar.c
> drivers/gpio/gpio-gpio-mm.c
> drivers/gpio/gpio-pci-idio-16.c
> drivers/gpio/gpio-pcie-idio-24.c
> drivers/gpio/gpio-ws16c48.c
> drivers/iio/addac/stx104.c
> drivers/iio/dac/cio-dac.c
> 
> Take a look.

I've already made a third version of the patch, using gpio-regmap this time.
This time I'm also using a Southbridge driver that pulls it as a platform
device, much like the rdc321x-southbridge.c does. It's not yet ready for
merging, but it's available for now at
https://github.com/socram8888/linux/tree/vortex-gpio

I have found a small issue though regarding gpio-regmap, and before making
a third version of the patch, I'd prefer to know the way to approach it.

The Vortex86 SoCs require the direction of the GPIO pin to be set before
writing the pin's value. Otherwise, writes to the data ports are ignored.

Currently gpio-regmap does it in the opposite order:

> static int gpio_regmap_direction_output(struct gpio_chip *chip,
> 					unsigned int offset, int value)
> {
>	gpio_regmap_set(chip, offset, value);
>
> 	return gpio_regmap_set_direction(chip, offset, true);
> }

(I have also noticed that it does not properly check the return value of
gpio_regmap_set, but that's another thing)

So there are IMO three different approaches:

1. Add a boolean flag that allows changing the behaviour. If set, invert
the order of operations. Else do as before.
2. Same, but with a "flags" bitfield, in case more flags need to be added
in the future.
3. Do an additional "gpio_regmap_set" after setting the direction. This
means no new fields need to be added to the structures but causes an extra
write that may not be needed on other drivers.

>> IRQ is only available for the first two ports out of the five available.
> Would  it be a problem to support them?

I cannot test that on my platform: as mentioned before, only ports 0 and 1
have IRQs, and in my mini PC I only have two pins available, and they're
both on port 4. 

Any code I'd write would be completely untested and IMHO sounds like a
terrible idea to have such code merged.

Thanks,
Marcos

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Andy Shevchenko]

On Tue, Aug 19, 2025 at 10:17:14PM +0200, Marcos Del Sol Vives wrote:
> El 11/07/2025 a las 16:53, Andy Shevchenko escribió:

...

> >> Again, I am not an expert on the Linux kernel, but I did not see any code
> >> or examples using neither gpio-mmio nor gpio-regmap for I/O-mapped registers.
> > 
> > $ git grep -lw '\.io_port[[:space:]]\+= true,'
> > drivers/counter/104-quad-8.c
> > drivers/gpio/gpio-104-dio-48e.c
> > drivers/gpio/gpio-104-idi-48.c
> > drivers/gpio/gpio-104-idio-16.c
> > drivers/gpio/gpio-exar.c
> > drivers/gpio/gpio-gpio-mm.c
> > drivers/gpio/gpio-pci-idio-16.c
> > drivers/gpio/gpio-pcie-idio-24.c
> > drivers/gpio/gpio-ws16c48.c
> > drivers/iio/addac/stx104.c
> > drivers/iio/dac/cio-dac.c
> > 
> > Take a look.
> 
> I've already made a third version of the patch, using gpio-regmap this time.
> This time I'm also using a Southbridge driver that pulls it as a platform
> device, much like the rdc321x-southbridge.c does. It's not yet ready for
> merging, but it's available for now at
> https://github.com/socram8888/linux/tree/vortex-gpio
> 
> I have found a small issue though regarding gpio-regmap, and before making
> a third version of the patch, I'd prefer to know the way to approach it.
> 
> The Vortex86 SoCs require the direction of the GPIO pin to be set before
> writing the pin's value. Otherwise, writes to the data ports are ignored.
> 
> Currently gpio-regmap does it in the opposite order:

Which is correct for the proper HW, the Vortex86 HW is broken, but...

> > static int gpio_regmap_direction_output(struct gpio_chip *chip,
> > 					unsigned int offset, int value)
> > {
> >	gpio_regmap_set(chip, offset, value);
> >
> > 	return gpio_regmap_set_direction(chip, offset, true);
> > }
> 
> (I have also noticed that it does not properly check the return value of
> gpio_regmap_set, but that's another thing)
> 
> So there are IMO three different approaches:
> 
> 1. Add a boolean flag that allows changing the behaviour. If set, invert
> the order of operations. Else do as before.
> 2. Same, but with a "flags" bitfield, in case more flags need to be added
> in the future.
> 3. Do an additional "gpio_regmap_set" after setting the direction. This
> means no new fields need to be added to the structures but causes an extra
> write that may not be needed on other drivers.

...if you want to still support this configuration which is prone to glitches
on the lines with who knows what the possible issues will come with that, you
may modify gpio-regmap on your needs. TBH, I have no preferences on the
approach, but it would be nice if others can share their opinions (I would ask
the author of gpio-regmap first (Cc'ed).

> >> IRQ is only available for the first two ports out of the five available.
> > Would  it be a problem to support them?
> 
> I cannot test that on my platform: as mentioned before, only ports 0 and 1
> have IRQs, and in my mini PC I only have two pins available, and they're
> both on port 4. 
> 
> Any code I'd write would be completely untested and IMHO sounds like a
> terrible idea to have such code merged.

I see... But what I meant is to support in terms of gpio-regmap. I think you
can still test the code that you have hardware for.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[William Breathitt Gray]

On Fri, Jul 11, 2025 at 02:43:13PM +0300, Andy Shevchenko wrote:
> +Cc: William,
> who is an expert in embedded industrial x86 platforms and might help with this.

Hi Marcos,

Just a heads-up that this is on my radar and I do have some general
comments about ISA drivers (in particular, this reminds me of the
gpio-ws16c48 driver which you may use as a rough guide). I'm currently
catching up on a backlog of emails so I should have a response for you
some time this weekend; but if you already have a v3 feel free to submit
it and I'll reply to that instead.

William Breathitt Gray

Re: [PATCH v2] gpio: vortex: add new GPIO device driver

[Andy Shevchenko]

On Wed, Aug 20, 2025 at 09:00:48PM +0900, William Breathitt Gray wrote:
> On Fri, Jul 11, 2025 at 02:43:13PM +0300, Andy Shevchenko wrote:
> > +Cc: William,
> > who is an expert in embedded industrial x86 platforms and might help with this.
> 
> Just a heads-up that this is on my radar and I do have some general
> comments about ISA drivers (in particular, this reminds me of the
> gpio-ws16c48 driver which you may use as a rough guide). I'm currently
> catching up on a backlog of emails so I should have a response for you
> some time this weekend; but if you already have a v3 feel free to submit
> it and I'll reply to that instead.

+1 here, as I'm also piled with a huge backlog after long leave. Perhaps
a clean fresh start will help here.

-- 
With Best Regards,
Andy Shevchenko