New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 897992 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature



Sign in to add a comment

crossystem / vboot: teach to use new gpio character device?

Project Member Reported by briannorris@chromium.org, Oct 23

Issue description

See existing discussion here: https://issuetracker.google.com/111454688

It seems that the chromeos_arm driver is showing its age, and as it happens, we managed to break the way that crossystem/vboot tries to use it in chromeos-4.14 -- the sysfs path changed here:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1163907/

There are a number of ways we could patch over crossystem, including various ways of parsing sysfs or trying to hook into udev:

https://buganizer.corp.google.com/issues/111454688#comment13

All of those seem somewhat fragile, although slightly better than the existing code.

Instead of having our own out-of-tree Linux driver just to expose a couple of GPIOs though, maybe we should look at ways to scrap it?

See the new method of naming GPIOs in device tree ('gpio-line-names'):

https://elixir.bootlin.com/linux/v4.19/source/Documentation/devicetree/bindings/gpio/gpio.txt#L158

Coupled with the new GPIO character ABI (partially available in v4.6):

https://www.kernel.org/doc/Documentation/ABI/testing/gpio-cdev
https://elixir.bootlin.com/linux/v4.19/source/include/uapi/linux/gpio.h

it's relatively easy to write user space code to poke at named GPIOs. Coupled with libgpiod, it's basically trivial. For example:

#include <gpiod.h>
#include <stdio.h>

int main()
{
        const char *name = "BIOS_FLASH_WP_R_L";
        int ret;
        struct gpiod_line *line;

        line = gpiod_line_find(name);
        if (!line) {
                perror("gpiod_line_find");
                return -1;
        }

        ret = gpiod_line_request_input(line, "foo");
        if (ret < 0) {
                perror("gpiod_line_request_input");
                goto out;
        }

        ret = gpiod_line_get_value(line);
        if (ret < 0) {
                perror("gpiod_line_get_value");
                goto out;
        }

        printf("%d\n", ret);

out:
        gpiod_line_close_chip(line);
        return ret;
}

So, is this a reasonable approach? Among the noted concerns:

1. additional vboot dependencies, if we use libgpiod
2. naming: that 'gpio-line-names' array can be pretty fugly
3. naming: will they be consistent across multiple boards?
4. backwards compatibility: we'd still have to maintain the old methods for pre-4.14 kernels

#4 doesn't seem terrible to me, given that eventually, we can scrap an entire driver that is unlikely to ever be merged in upstream Linux

#3 seems solvable, given that the DT docs for this list seem to suggest "reasonable" names -- so we can probably just pick something that makes sense. Cheza currently lists "BIOS_FLASH_WP_R_L", but that actually seems kinda bad -- I don't think we want to be codifying the "active" level, since the gpiod API is supposed to hide that for us? I'd nominate "AP Flash WP". Commence painting the bikeshed.

#1: Julius suggested the dependency might be OK as long as this doesn't get into the core vboot stuff that projects like coreboot use.

Any other thoughts?
 
> 3. naming: will they be consistent across multiple boards?

[insert prequel meme] We will make it legal^Wconsistent.

No, but seriously, we'll just have to pay attention to this then. I don't really see a fundamental difference between either using a well-known pinctrl name for that pin, or defining a well-known device tree path with a phandle to it (which is what we're doing right now). Both just need to be set right in the kernel DT. Also, since this is solely handled in the kernel DT with no firmware involvement, we should be able to update old boards without issues (as long as the kernel supports that interface itself, which apparently it doesn't for any Arm boards we've actually released... but... in theory we could).

> I'd nominate "AP Flash WP". Commence painting the bikeshed.

not sure whether spaces in there are the greatest idea, but otherwise SGTM.
> using a well-known pinctrl name for that pin

To be clear: the mention of "pinctrl name" was a misstatement from me on b/. Nothing here uses the pinctrl name for a user-facing ABI. The GPIO naming (as used in the new GPIO character device ABI) is purely from the 'gpio-line-names' property, which we would have to write for each gpiochip in a system.

> I don't really see a fundamental difference

Not really a fundamental difference. But there are follow-on effects with how one deals with arrays of strings in DT, especially when they can be so unwieldy. (Try reading the 150-line array in arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dts.)

But mostly, I agree.

> Also, since this is solely handled in the kernel DT with no firmware involvement, we should be able to update old boards without issues [...]

Yep. If uprevs happen (a huge "if"). Or if we bother to backport these features to older kernels (unlikely).

> not sure whether spaces in there are the greatest idea, but otherwise SGTM.

I mostly just felt like taking the DT docs at their word:

<quote>
For example "MMC-CD", "Red LED Vdd" and "ethernet reset" are
reasonable line names as they describe what the line is used for.
</quote>

It also might help reinforce that these names don't really need to be literal schematic names (if we were being too literal with schematic names, we'd never get 2 boards to agree).

But anyway, consider that a neon orange shade of paint, to encourage somebody to give it a different color.
Cc: rspangler@google.com hungte@chromium.org
Let's keep the overall design discussion in this thread for simplicity, and leave the Buganizer component only for stuff related to that specific board.

Copying what Randall said over there:

> Re#15: Yuck, more dependencies.  Pretty soon, vboot_reference won't build outside a full chroot at all.
> 
> Maybe we should repartition the functionality between crossystem and mosys? 
 The crossystem lib would own the data formats for the VBSD and NVStorage blobs.   It gets linked into mosys, which knows how to interact with the rest of the system.

Actually, I just remembered, futility now also links in crossystem libraries for Hung-Te's new 'futility update' (and that's one of the things we especially want to keep portable). So I don't think keeping the two separate will really work. For the same reason I also don't think pushing stuff into mosys is a great solution.

I guess the main question is: how widespread is that libgpiod? Looks like it's hosted on kernel.org, so is it something that all Linux distributions just install by default anyway? In that case, maybe this wouldn't be that bad (I'm assuming it can be statically linked, too, so this only really matters at compile-time for upstream coreboot and other futility users).

But if it's not widely installed, or if it's so new that most distributions from ~2 years ago or so won't have it yet, I think it's probably better not to rely on it. Brian, how terrible would your "hack the ioctls by hand" fallback plan look in practice?
> It also might help reinforce that these names don't really need to be literal schematic names

Personally I kinda like that they are schematic names.  It makes it really easy to match them up to schematics and also happens to make it really easy to come up with a name (AKA: no bike shedding about the name because it's just whatever the schematic says).

I wonder if upstream would accept it if we could give some pins a secondary name.  Then we wouldn't have to come up with non-schematic names for all pins--just for those that we care about from an ABI-compatibility point of view.


In any case it does seem like a good idea to use libgpiod for this and remove a bunch of out-of-tree code.  If upstream doesn't want to accept secondary names (or if everyone here hates the idea) then just picking a name for these GPIOs seems fine.  I guess we'll have to decide if we keep schematic names for everything else or try to pretty them up.

---

> I don't think we want to be codifying the "active" level, since the gpiod API is supposed to hide that for us

How does that work exactly?  If nobody in the kernel is using a given pin who specifies that it's active low when userspace tries to use it...
Owner: briannorris@chromium.org
Status: Assigned (was: Untriaged)
> how widespread is that libgpiod? ... if it's so new that most distributions from ~2 years ago or so won't have it yet, I think it's probably better not to rely on it.

It looks like the v0.1 release was in Jan 2017, and the v1.0 release ("major release", "breaks API compatibility") was Feb 2018.

My gLinux workstation has it though!

$ apt-cache search libgpiod
gpiod - Tools for interacting with Linux GPIO character device - binary
libgpiod-dev - C library for interacting with Linux GPIO device - static libraries and headers
libgpiod-doc - C library for interacting with Linux GPIO device - library documentation
libgpiod0 - C library for interacting with Linux GPIO device - shared libraries
libgpiod0-dbgsym - debug symbols for libgpiod0

Except it's the v0.3.2 version:

$ apt-cache show libgpiod0
Package: libgpiod0
Source: libgpiod
Version: 0.3.2+git20171201-2
[...]

So that probably doesn't qualify as widespread and common yet.

> Brian, how terrible would your "hack the ioctls by hand" fallback plan look in practice?

I think it looks something like this:

 * Iterate over all /dev/gpiochip* (e.g., 3 or 4 controllers) with GPIO_GET_CHIPINFO_IOCTL to get number of lines per chip
 * Iterate over each line with GPIO_GET_LINEINFO_IOCTL, to get line names
 * Once a matching line name is found, call GPIO_GET_LINEHANDLE_IOCTL with the appropriate index info -- this gives you a new file descriptor
 * Finally!! Call GPIOHANDLE_GET_LINE_VALUES_IOCTL using the fd from the previous step

I'm not very good at approximating the number of lines of code. Probably ~50, with all the right error handling and readability? I can write it up for real soon.

Side note: it's sorta amusing to count the number of syscalls it takes just to read the state of a GPIO.

> I wonder if upstream would accept it if we could give some pins a secondary name.  Then we wouldn't have to come up with non-schematic names for all pins--just for those that we care about from an ABI-compatibility point of view.

You wanna ask? Trying to extract ABI-guarantee types of answers out of DT people (when it really matters) is like pulling teeth. See our so-far-stalled efforts to even document what typical /aliases properties should be named.

> How does that work exactly?  If nobody in the kernel is using a given pin who specifies that it's active low when userspace tries to use it...

Ack, I was confused. The naming of the library ('gpiod') confused me into thinking the API itself handled that...you have to specify GPIOHANDLE_REQUEST_ACTIVE_LOW in the request flags yourself. So I guess you have to figure out the polarity by convention or by other means. So settling on something with "_L", "_N" or "#" suffixes probably makes sense.

(BTW, I don't think you can use this API if someone in the kernel is holding the pin -- you just get EBUSY or similar. Same as with the old sysfs export API.)

---

Altogether, it sounds like there may be a path forward with this. I'll poke at whether open-coding the ioctls looks OK, so assigning to me.
An addendum about adding secondary names to GPIOs (chatted with Doug about this already): that would require augmenting not only the DT bindings, but also the chardev ABI. The current ABI lets you look up names by requesting info about an index -- and the struct that only has a single 'name' field:

/**
 * struct gpioline_info - Information about a certain GPIO line
 * @line_offset: the local offset on this GPIO device, fill this in when
 * requesting the line information from the kernel
 * @flags: various flags for this line
 * @name: the name of this GPIO line, such as the output pin of the line on the
 * chip, a rail or a pin header name on a board, as specified by the gpio
 * chip, may be NULL
 * @consumer: a functional name for the consumer of this GPIO line as set by
 * whatever is using it, will be NULL if there is no current user but may
 * also be NULL if the consumer doesn't set this up
 */
struct gpioline_info {
        __u32 line_offset;
        __u32 flags;
        char name[32];
        char consumer[32];
};

So it seems much more expedient to simply work with the single 'name' field as-is, and agree on some particular way to name this consistently.
Status: Started (was: Assigned)
Alright, you asked for it; is 121 lines (not including main()) worth it? (Please, no detailed code review; just a question of library vs. open-coded.)

#include <dirent.h>
#include <fcntl.h>
#include <linux/gpio.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/ioctl.h>
#include <unistd.h>

static int gpioline_get_value(int chip_fd, int idx)
{
	struct gpiohandle_request request = {
		.lineoffsets = { idx },
		.flags = GPIOHANDLE_REQUEST_INPUT | GPIOHANDLE_REQUEST_ACTIVE_LOW,
		.lines = 1,
	};
	struct gpiohandle_data data;
	int ret;

	ret = ioctl(chip_fd, GPIO_GET_LINEHANDLE_IOCTL, &request);
	if (ret < 0) {
		perror("GPIO_GET_LINEHANDLE_IOCTL");
		return -1;
	}
	if (request.fd < 0) {
		fprintf(stderr, "bad LINEHANDLE fd %d\n", request.fd);
		return -1;
	}

	ret = ioctl(request.fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, &data);
	if (ret < 0) {
		perror("GPIOHANDLE_GET_LINE_VALUES_IOCTL");
		return -1;
	}
	return data.values[0];
}

// Return 1 if idx name; 0 if no match; negative if error.
static int gpiochip_name_match(int chip_fd, int idx, const char *name)
{
	struct gpioline_info info = {
		.line_offset = idx,
	};
	int ret;

	ret = ioctl(chip_fd, GPIO_GET_LINEINFO_IOCTL, &info);
	if (ret < 0) {
		perror("GPIO_GET_LINEINFO_IOCTL");
		return -1;
	}

	return strncmp(info.name, name, sizeof(info.name)) == 0;
}

// Return fd for gpio_line, if found. Negative for error codes.
static int gpiochip_request_byname(int chip_fd, const char *name)
{
	struct gpiochip_info info;
	int i, ret;

	ret = ioctl(chip_fd, GPIO_GET_CHIPINFO_IOCTL, &info);
	if (ret < 0) {
		perror("GPIO_GET_CHIPINFO_IOCTL");
		return -1;
	}

	for (i = 0; i < info.lines; i++) {
		if (gpiochip_name_match(chip_fd, i, name) != 1)
			continue;
		return gpioline_get_value(chip_fd, i);
	}

	return -1;
}

// Return nonzero for entries with a 'gpiochip'-prefixed name.
static int gpiochip_scan_filter(const struct dirent *d)
{
	const char prefix[] = "gpiochip";
	return !strncmp(prefix, d->d_name, strlen(prefix));
}

static int read_gpio(const char *name)
{
	struct dirent **list;
	int i, max, ret;

	ret = scandir("/dev", &list, gpiochip_scan_filter, alphasort);
	if (ret < 0) {
		perror("scandir");
		return -1;
	}
	max = ret;

	for (i = 0; i < max; i++) {
		char buf[30];
		int fd;

		snprintf(buf, sizeof(buf), "/dev/%s", list[i]->d_name);
		fd = open(buf, O_RDWR);
		if (fd < 0) {
			perror("open");
			continue;
		}
		ret = gpiochip_request_byname(fd, name);
		close(fd);
		if (ret >= 0)
			break;
	}

	for (i = 0; i < max; i++)
		free(list[i]);
	free(list);

	if (ret < 0) {
		fprintf(stderr, "found no matching GPIO: %s\n", name);
		return -1;
	}
	
	return ret;
}

int main(int argc, const char **argv)
{
	const char *name = "BIOS_FLASH_WP_R_L";
	int ret;

	if (argc > 1)
		name = argv[1];

	ret = read_gpio(name);
	if (ret < 0)
		return ret;

	printf("%d\n", ret);

	return ret;
}


If so, I'll stick something like that in crossystem.
> just a question of library vs. open-coded

I have a slight bias toward library.  It's not like this is some random 3rd party library.  ...but I wasn't the one who balked at adding a dependency so I guess this is a question for Randall?  
> It's not like this is some random 3rd party library.

But it does seem to be a super new and not yet very widespread library according to what Brian said, so I'd prefer not to add this dependency. The problem is that due to our integration of vboot into coreboot, every upstream coreboot build now potentially builds futility. People are building these on all sorts of systems (maybe even FreeBSD, which might create another problem due to missing headers, not sure). Since we're just in the process of slowly convincing the whole upstream coreboot community that vboot is great and just what they need anyway, I don't want to jeopardize that with a bunch of complaints of how people suddenly can't build anymore because something in long tail of supporting host utilities suddenly depends on this library that nobody has. If we can skirt around this with 120 lines of code, I think that's worth it. (The alternative would be to make the dependency optional in a way that crossystem just skips that and goes straight to the legacy mechanisms if it wasn't available at compile time.)

> Personally I kinda like that they are schematic names.  It makes it really easy to match them up to schematics and also happens to make it really easy to come up with a name (AKA: no bike shedding about the name because it's just whatever the schematic says).

Since only new boards would be able to use it (due to the kernel version restriction), maybe we can just use schematic names here and instead mandate that every future board will name that line the same way (safe for inclusion/lack of an "_L" to denote polarity).
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 25

Labels: merge-merged-chromeos-4.19
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/bed65fe25e1774d6d9b11573eefc74c2c11d53af

commit bed65fe25e1774d6d9b11573eefc74c2c11d53af
Author: Brian Norris <briannorris@chromium.org>
Date: Thu Oct 25 07:32:26 2018

CHROMIUM: arm64: dts: qcom: sdm845-cheza: drop write-protect from 'chromeos-firmware'

We shouldn't need this here. Named GPIOs can just be accessed through
/dev/gpiochip*, which should be more stable. We can probably just kill
the chromeos_arm driver entirely now.

I move the pinctrl reference for bios_flash_wp_r_l to a pinctrl hog,
since there's not a specific owner for this any more.

BUG=b:111454688,  chromium:897992 
TEST=`crossystem wpsw_cur` works with new gpio API

Change-Id: I7217c02c11810ac24846129536e3a8f18b35e817
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1296522
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/bed65fe25e1774d6d9b11573eefc74c2c11d53af/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
[modify] https://crrev.com/bed65fe25e1774d6d9b11573eefc74c2c11d53af/arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dts
[modify] https://crrev.com/bed65fe25e1774d6d9b11573eefc74c2c11d53af/arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dts

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 25

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0e94313962fee48e913caed7ec977de38ed9e0ea

commit 0e94313962fee48e913caed7ec977de38ed9e0ea
Author: Brian Norris <briannorris@chromium.org>
Date: Thu Oct 25 07:32:24 2018

CHROMIUM: arm64: dts: qcom: sdm845-cheza: drop write-protect from 'chromeos-firmware'

We shouldn't need this here. Named GPIOs can just be accessed through
/dev/gpiochip*, which should be more stable. We can probably just kill
the chromeos_arm driver entirely now.

I move the pinctrl reference for bios_flash_wp_r_l to a pinctrl hog,
since there's not a specific owner for this any more.

BUG=b:111454688,  chromium:897992 
TEST=`crossystem wpsw_cur` works with new gpio API

Change-Id: I7217c02c11810ac24846129536e3a8f18b35e817
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1297832
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/0e94313962fee48e913caed7ec977de38ed9e0ea/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
[modify] https://crrev.com/0e94313962fee48e913caed7ec977de38ed9e0ea/arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dts
[modify] https://crrev.com/0e94313962fee48e913caed7ec977de38ed9e0ea/arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dts

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/733b332212ec5fe7fbfb1edf76dfb9126e48401d

commit 733b332212ec5fe7fbfb1edf76dfb9126e48401d
Author: Brian Norris <briannorris@chromium.org>
Date: Thu Oct 25 22:18:58 2018

CHROMIUM: platform: chrome: drop chromeos_arm driver

This driver is no longer needed. It only served to:
 (a) register sysfs shortcuts for a few GPIOs
 (b) register a panic handler for stashing eventlog info in memory (?)

Item (a) is supported via named GPIO lookups (/dev/gpiochip* chardev
API), and we'd like to stop using this driver for that.

Item (b) was only used on Peach AFAICT. We could probably resurrect the
driver and do this differently, if that's really the way this should go.

BUG= chromium:897992 
TEST=boot, `crossystem wpsw_cur` works without this driver

Change-Id: If6c0bc0140134dece306833a3717e84056a77dea
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1297837
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

[delete] https://crrev.com/8c44e89ecf091bb53ee252acd046bf15d92aa0f1/drivers/platform/chrome/chromeos_arm.c
[delete] https://crrev.com/8c44e89ecf091bb53ee252acd046bf15d92aa0f1/drivers/platform/chrome/elog.h
[modify] https://crrev.com/733b332212ec5fe7fbfb1edf76dfb9126e48401d/drivers/platform/chrome/Kconfig
[modify] https://crrev.com/733b332212ec5fe7fbfb1edf76dfb9126e48401d/drivers/platform/chrome/Makefile

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7f02eaec4129357cd115f1a942f61a668e14112c

commit 7f02eaec4129357cd115f1a942f61a668e14112c
Author: Brian Norris <briannorris@chromium.org>
Date: Fri Oct 26 07:41:19 2018

CHROMIUM: arm64: dts: qcom: sdm845-cheza: rename flash WP# gpio

We've painted the bikeshed. The paint is dry.

With luck, we can get future schematics changed anyway.

BUG=b:111454688,  chromium:897992 
TEST=`crossystem wpsw_cur` works with new gpio API

Change-Id: I1ae6e9a88c483d2e04649c7fd9f09d3868edff34
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1298754
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/7f02eaec4129357cd115f1a942f61a668e14112c/arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dts

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c4bdcca3163f54060673d8d67db633daaf0b9ade

commit c4bdcca3163f54060673d8d67db633daaf0b9ade
Author: Brian Norris <briannorris@chromium.org>
Date: Sat Oct 27 00:18:55 2018

CHROMIUM: platform: chrome: drop chromeos_arm driver

This driver is no longer needed. It only served to:
 (a) register sysfs shortcuts for a few GPIOs
 (b) register a panic handler for stashing eventlog info in memory (?)

Item (a) is supported via named GPIO lookups (/dev/gpiochip* chardev
API), and we'd like to stop using this driver for that.

Item (b) was only used on Peach AFAICT. We could probably resurrect the
driver and do this differently, if that's really the way this should go.

BUG= chromium:897992 
TEST=boot, `crossystem wpsw_cur` works without this driver

Change-Id: If6c0bc0140134dece306833a3717e84056a77dea
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1300174
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[delete] https://crrev.com/4ff4d9b08103580c84fa8d0ee196e1148a3f6714/drivers/platform/chrome/chromeos_arm.c
[delete] https://crrev.com/4ff4d9b08103580c84fa8d0ee196e1148a3f6714/drivers/platform/chrome/elog.h
[modify] https://crrev.com/c4bdcca3163f54060673d8d67db633daaf0b9ade/drivers/platform/chrome/Kconfig
[modify] https://crrev.com/c4bdcca3163f54060673d8d67db633daaf0b9ade/drivers/platform/chrome/Makefile

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/4b5879805ac75dea9ea74a338f0f8a5d7f5edca1

commit 4b5879805ac75dea9ea74a338f0f8a5d7f5edca1
Author: Brian Norris <briannorris@chromium.org>
Date: Sat Oct 27 00:18:59 2018

crossystem: replace 'chromeos_arm' device with new GPIO chardev API

Upstream Linux supports a new ioctl API for GPIO chips, via new
/dev/gpiochip* device nodes. This new API supports name lookups, which
is a much nicer way than the index-based stuff in /sys/class/gpio/. We
can finally use this instead of our custom, downstream "chromeos_arm"
driver.

GPIO line names are defined in a 'gpio-line-names' property in the
Device Tree. For now, we have exactly one board using this, and we're
calling it 'AP_FLASH_WP_L'. We will need to ensure future devices use
this same naming.

Per others' suggestions, I'm avoiding using libgpiod, because it's a
relatively new library (with breaking changes in v1.0 as recently as
this year), and vboot_reference is used by plenty of other projects. And
it wasn't that hard to hand-roll the ioctls.

Side note: the chromeos_arm device is not guaranteed to be found at
/sys/devices/platform/chromeos_arm any more (especially on kernel
>=4.14), so this is a handy excuse to just kill use of the driver
entirely.

BRANCH=none
BUG= chromium:897992 
TEST=`crossystem wpsw_cur` on 4.14 kernels (with this API) and older
     kernels (without this API)

Change-Id: I7553801fb0e97c8a0aa6f4341d297ad0071c3dac
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1298274
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/4b5879805ac75dea9ea74a338f0f8a5d7f5edca1/host/arch/arm/lib/crossystem_arch.c

Status: Fixed (was: Started)
Q: $subject
A: yes
Cc: drinkcat@chromium.org hsinyi@chromium.org

Sign in to add a comment