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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aKWy0lWrsiz7yaCw@smile.fi.intel.com>
Date: Wed, 20 Aug 2025 14:34:42 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Marcos Del Sol Vives <marcos@...a.pet>,
	Michael Walle <mwalle@...nel.org>
Cc: William Breathitt Gray <wbg@...nel.org>, linux-kernel@...r.kernel.org,
	Linus Walleij <linus.walleij@...aro.org>,
	Bartosz Golaszewski <brgl@...ev.pl>, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v2] gpio: vortex: add new GPIO device driver

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