linux-kernel - Re: [PATCH v5 2/3] gpio: loongson: add gpio driver support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c02570e-03d5-85f1-73fb-b66d6170c875@loongson.cn>
Date:   Thu, 24 Nov 2022 10:22:12 +0800
From:   Yinbo Zhu <zhuyinbo@...ngson.cn>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Bartosz Golaszewski <brgl@...ev.pl>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        WANG Xuerui <kernel@...0n.name>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Juxin Gao <gaojuxin@...ngson.cn>,
        Bibo Mao <maobibo@...ngson.cn>,
        Yanteng Si <siyanteng@...ngson.cn>, linux-gpio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        loongarch@...ts.linux.dev, linux-mips@...r.kernel.org,
        Arnaud Patard <apatard@...driva.com>,
        Huacai Chen <chenhuacai@...nel.org>,
        Jianmin Lv <lvjianmin@...ngson.cn>,
        Hongchen Zhang <zhanghongchen@...ngson.cn>,
        Liu Peibao <liupeibao@...ngson.cn>
Subject: Re: [PATCH v5 2/3] gpio: loongson: add gpio driver support



在 2022/11/24 上午6:05, Linus Walleij 写道:
> On Wed, Nov 23, 2022 at 9:02 AM Yinbo Zhu <zhuyinbo@...ngson.cn> wrote:
>> 在 2022/11/21 下午9:24, Linus Walleij 写道:
> 
>>>> +static int loongson_gpio_request(
>>>> +                       struct gpio_chip *chip, unsigned int pin)
>>>> +{
>>>> +       if (pin >= chip->ngpio)
>>>> +               return -EINVAL;
>>>
>>> This is not needed, the gpiolib core already checks this. Drop it.
>> I check gpio_request in gpilib, I notice gpio_is_valid is not equal to
>> this condition, so I still kept it for byte mode.
> 
> This is because descriptors can only be obtained from gpiod_get() and
> similar and gpiod_get() falls to gpiod_get_index() which will not
> return a valid descriptor from either HW backend. gpiod_get()
> will call gpiod_request() for if and only if the descriptor is valid.
> 
> The only reason to implement something like this is because of
> using the legacy GPIO numberspace which we are getting rid
> of so it is irrelevant, the consumers of your driver will only be
> using gpio descriptors, will only come in through gpiod_get_index()
> and will have desc validity check done before calling gpiod_request().
> 
> So drop this.
okay , I got it.
Thank you for your detailed explanation.
> 
>>> I am bit suspicious that your IRQchip implementation expects consumers
>>> to call gpiod_to_irq() first and this is not legal.
>>
>> okay, I got it, and other driver use gpio interrupt doesn't rely on
>> gpiod_to_irq, but can use gpiod_to_irq.
> 
> Yes it can be used to look up the irq corresponding to a GPIO
> but it is not mandatory to do that.
> 
>> The reason is that gpio interrupt wasn't an independent module,  The
>> loongson interrupt controller liointc include lots of interrupt was
>> route to perpherial, such as i2c/spi .. gpio, so gpio interrupt as
>> normal perpherial interrupt, It is unnecessary and redundant to
>> implement a gpio irq chip. The liointc controller driver had cover all
>> interrupt.
> 
> This is fine, and it is common for GPIO drivers to implement
> their own IRQchips.
> 
> But these drivers can not rely on the .gpio_to_irq() callback
> to be called before an IRQ is requested and used.

I may not have made it clear before that the gpio irq chip for other
platforms may need to be implemented, but the loongson platform may be
special.

I mean that the loongson platform use gpio irq does not need to rely on
gpio_to_irq, because loongson interrupt controller driver has covered
gpio irq.  The specific reason is my above explanation.

so, Can I not realize gpio irq chip?
> 
>> in addition,  I don't like to use the dynamically allocated gpio base,
>> so I set the gpio base after call bgpio_init.
> 
> Don't do that because the GPIO maintainers love the
> dynamic base and hate hardcoded bases. Set the base to -1
> If you wonder why, read drivers/gpio/TODO.
okay, I got it.
> 
> Yours,
> Linus Walleij
>