Messages in this thread | | | Subject | Re: [PATCH v5 2/3] gpio: loongson: add gpio driver support | From | Yinbo Zhu <> | Date | Mon, 12 Dec 2022 16:34:52 +0800 |
| |
在 2022/11/24 上午6:05, Linus Walleij 写道: > On Wed, Nov 23, 2022 at 9:02 AM Yinbo Zhu <zhuyinbo@loongson.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. > >>> 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 >
| |