On 12/01/2017 05:38 AM, Archit Taneja wrote:

We're hitting an issue here ^ when a consumer calls the devm_gpiod_get() 
API. This API
internally sets idx in gpiod_get_index to always 0. For example, in the 
call sequence
below, the idx argument to gpiochip_irqchip_line_valid is always 0:

devm_gpiod_get(dev, con_id, flags)
 ?? devm_gpiod_get_index(dev, con_id, 0, flags)
 ????? gpiod_get_index(dev, con_id, 0, flags)
 ???????? if (!gpiochip_irqchip_line_valid(desc->gdev->chip, 0))
 ???????????? return ERR_PTR(-EACCES);

Therefore, with these patches, if we create a sparse gpio map, and the
0th gpio is set to "not accessible", all gpio requests end up failing 
with EACCESS
because idx is always passed as 0.

Ok, I can see the problem, but I don't know how to fix it.

struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
					      const char *con_id,
					      enum gpiod_flags flags)
{
	return devm_gpiod_get_index(dev, con_id, 0, flags);
}

I thought that by putting the check for "availability" inside of the 
GPIO request, it would handle all situations where a client (whether 
kernel or user-space) tries to access a specific GPIO.

However, this doesn't work with [devm_]gpiod_get.  This function 
attempts to claim the entire GPIO block by claiming only the first GPIO.

This is straining my understanding of gpiolib.  Maybe we need to do 
something like this:

struct gpio_desc *__must_check gpiod_get(struct device *dev, const char 
*con_id,
					 enum gpiod_flags flags)
{
	return gpiod_get_index(dev, con_id, -1, flags);
}

Where idx == -1 is a special-case for gpiod_get_index() where it returns 
the gpio_desc without actually requesting it?

In gpiochip_irqchip_line_valid, shouldn't we test the nth bit (that 
corresponds to
this gpio_desc) in chip->line_valid_mask instead of idx passed above, 
which is always
0?

The code has changed upstream, so I need to refactor that function.  But 
I think it's already testing the nth bit:

static bool gpiochip_irqchip_line_valid(const struct gpio_chip gpiochip,
					unsigned int offset)
{
	/* No mask means all valid */
	if (likely(!gpiochip->line_valid_mask))
		return true;
	return test_bit(offset, gpiochip->line_valid_mask);

'offset' is the nth bit.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.