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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 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.

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.

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.
Hi Linus,

I recenly verfied other peripheral on upstream, some peripheral driver
need use gpio number, but if use dynamic base that gpio number will be
meaningless.  in additon I notice that many gpio driver don't use
dynamic base, although bgpio_int was called.

so I think the gpio number should be keep consistent with datasheet for
some platform that need use gpio number.

Yinbo.


Yours,
Linus Walleij





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux