Re: [PATCH 2/4] gpiolib: add bitmask for valid GPIO lines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux