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

[Yinbo Zhu]

在 2022/11/24 上午6:05, Linus Walleij 写道:
> On Wed, Nov 23, 2022 at 9:02 AM Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> 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