Re: [PATCH] pinctrl: sirf/atlas7: stop poking around in GPIO internals

From: Barry Song
Date: Thu Feb 18 2016 - 22:22:08 EST


2016-02-19 4:42 GMT+08:00 Linus Walleij <linus.walleij@xxxxxxxxxx>:
> On Thu, Feb 18, 2016 at 3:50 AM, Barry Song <baohua@xxxxxxxxxx> wrote:
>
>> gpiochip_add(chip will call this automatically since range is set in dtsi:
>> gpio_0: gpio_mediam@17040000 {
>> #gpio-cells = <2>;
>> #interrupt-cells = <2>;
>> compatible = "sirf,atlas7-gpio";
>> reg = <0x17040000 0x1000>;
>> interrupts = <0 13 0>, <0 14 0>;
>> clocks = <&car 98>;
>> clock-names = "gpio0_io";
>> gpio-controller;
>> interrupt-controller;
>>
>> gpio-banks = <2>;
>> gpio-ranges = <&pinctrl 0 0 0>,
>> <&pinctrl 32 0 0>;
>> gpio-ranges-group-names = "lvds_gpio_grp",
>> "jtag_uart_nand_gpio_grp";
>
> Aha I see.
>
>>> -
>>> - /* Records gpio_pin_range to a7gc */
>>> - list_for_each_entry(pin_range, &chip->pin_ranges, node) {
>>> - struct pinctrl_gpio_range *range;
>>> -
>>> - range = &pin_range->range;
>>> - if (range->id == NGPIO_OF_BANK * idx) {
>>> - bank->gpio_offset = range->id;
>>> - bank->ngpio = range->npins;
>>> - bank->gpio_pins = range->pins;
>>> - bank->pctldev = pin_range->pctldev;
>>> - break;
>>> - }
>>> - }
>>> -
>>> - BUG_ON(!bank->pctldev);
>>
>> this doesn't work. my pin range is not continuous and linear, so we
>> need the "if (range->id == NGPIO_OF_BANK * idx)" to calculate the
>> gpio_offset.
>> and gpio_offset is used in many places:
>
> Can't you use:
>
> struct pinctrl_gpio_range *range =
> pinctrl_find_gpio_range_from_pin(pctldev, pin);
>
> In these places instead?

this is why gpio_offset was involved to avoid doing complex
pinctrl_find_gpio_range_from_pin() everytime.
since the range mapping is not linear, gpio_offset is a cache to do that.

>
> ?
>
> That is what most drivers do.
>
> Yours,
> Linus Walleij

-barry