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

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: <CAGsJ_4xf22j-QDxujuRzUY8eG9krR3Uy+_ZsqAoEUCamYV4PWQ@mail.gmail.com>
Date:	Fri, 19 Feb 2016 11:21:38 +0800
From:	Barry Song <baohua@...nel.org>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Barry Song <Baohua.Song@....com>,
	Linux GPIO List <linux-gpio@...r.kernel.org>,
	Guoying Zhang <Guoying.Zhang@....com>,
	Wei Chen <Wei.Chen@....com>
Subject: Re: [PATCH] pinctrl: sirf/atlas7: stop poking around in GPIO internals

2016-02-19 4:42 GMT+08:00 Linus Walleij <linus.walleij@...aro.org>:
> On Thu, Feb 18, 2016 at 3:50 AM, Barry Song <baohua@...nel.org> wrote:
>
>> gpiochip_add(chip will call this automatically since range is set in dtsi:
>> gpio_0: gpio_mediam@...40000 {
>> #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