On Mon, Oct 2, 2017 at 10:47 PM, Timur Tabi <timur@xxxxxxxxxxxxxx> wrote: > On 10/02/2017 12:44 PM, Bjorn Andersson wrote: >>> >>> + /* >>> + * If irq_need_valid_mask is true, then gpiochip_add_data() will >>> + * initialize irq_valid_mask to all 1s. We need to clear all the >>> + * GPIOs that are unavailable, and we need to find each block >>> + * of consecutive available GPIOs are add them as pin ranges. >>> + */ >>> + if (chip->irq_need_valid_mask) { >>> + for (i = 0; i < ngpio; i++) >>> + if (!groups[i].npins) >>> + clear_bit(i, pctrl->chip.irq_valid_mask); >>> + >>> + while ((count = msm_gpio_get_next_range(pctrl, &start))) >>> { >>> + ret = gpiochip_add_pin_range(&pctrl->chip, >>> + >>> dev_name(pctrl->dev), >>> + start, start, >>> count); >>> + if (ret) >>> + break; >>> + start += count; >> >> I do not fancy the idea of specifying a bitmap of valid irq pins and >> then having the driver register the pin-ranges in-between. > > But that's exactly what abx500_gpio_probe() in pinctrl-abx500.c does. Here's > even a reference to holes in the GPIO space: This driver is not a good example of what is desireable. I am sorry that the kernel contains a lot of bad examples. These are historical artifacts, they cannot be used as an argument to write code in the same style. >> If we provide >> >> a bitmap of validity to the core it should support using this for the >> pins as well. (Which I believe is what Linus answered in the discussion >> following patch 0/2) > > So you want to change "gpio_chip" to add an "available" callback? And every > time gpiolib wants to call a gpio_chip callback, it should call ->available > first? Like this: > > if (chip->available && chip->available()) > status = chip->direction_input(chip, gpio_chip_hwgpio(desc)); I mean that you add unsigned long *line_valid_mask; to struct gpio_chip using the same type of logic as .irq_valid_mask. The mask should be optional. Then the operation gpiod_get[_index]() will FAIL if the line is not valid. There is no need to check on every operation, there should be no way to get a handle on the pin any other way. All new code should be using descriptors at this point so we only need to design for that case. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html