linux-kernel - Re: [PATCH] IIO: Adds ACPI support for ST gyroscopes

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: <551155C3.2030403@metafoo.de>
Date:	Tue, 24 Mar 2015 13:17:07 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Robert Dolca <robert.dolca@...el.com>, linux-iio@...r.kernel.org,
	Jonathan Cameron <jic23@...nel.org>
CC:	linux-kernel@...r.kernel.org, Hartmut Knaack <knaack.h@....de>,
	Peter Meerwald <pmeerw@...erw.net>,
	Linus Walleij <linus.walleij@...aro.org>,
	Denis CIOCCA <denis.ciocca@...com>
Subject: Re: [PATCH] IIO: Adds ACPI support for ST gyroscopes

[...]
> +int st_sensors_acpi_i2c_probe(struct i2c_client *client,
> +			       const struct acpi_device_id *match)
> +{
> +	const struct acpi_device_id *id;
> +	struct gpio_desc *gpiod_irq;
> +	int ret;
> +
> +	id = acpi_match_device(match, &client->dev);
> +	if (!id)
> +		return -ENODEV;
> +
> +	/* Get IRQ GPIO */
> +	gpiod_irq = devm_gpiod_get_index(&client->dev, 0, 0);
> +	if (IS_ERR(gpiod_irq))
> +		return -ENODEV;
> +
> +	/* Configure IRQ GPIO */
> +	ret = gpiod_direction_input(gpiod_irq);
> +	if (ret)
> +		return ret;

How exactly does this whole GPIO IRQ thing work with ACPI. Does the ACPI 
description just specify the GPIOs and the driver needs to know which GPIO 
is the is used for the IRQ or does the description indicate that a certain 
GPIO should be used as a IRQ. The reason why I'm asking is that same code 
pops up in pretty much every ACPI I2C sensor driver now. Which suggests that 
this should be factored out into common infrastructure.

And especially the requesting and the setting of the direction of the GPIO 
should not be necessary if the GPIO controller implements interrupt handling 
correctly as this is something that is, as far as I understand, taken care 
of by the GPIO framework when the IRQ is requested.

> +
> +	/* Map the pin to an IRQ */
> +	client->irq = gpiod_to_irq(gpiod_irq);
> +
> +	/* The name from the ACPI match takes precedence if present */
> +	memset(client->name, 0, sizeof(client->name));
> +	strncpy(client->name, (char *) id->driver_data,
> +		min(sizeof(client->name), strlen((char *) id->driver_data)));

Both client->irq and client->name should not be modified by the driver, 
these are only supposed to be set by the I2C framework. Modifying them in 
the driver can result in undefined behavior.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/