+Cc: Michael (GPIO regmap maintainer) On Thu, Dec 14, 2023 at 07:57:55AM +0000, TY_Chang[張子逸] wrote: > >On Tue, Dec 12, 2023 at 09:55:59AM +0000, TY_Chang[張子逸] wrote: > >> >On Thu, Dec 07, 2023 at 06:07:23PM +0800, TY Chang wrote: ... > >> >> This driver enables configuration of GPIO direction, GPIO values, > >> >> GPIO debounce settings and handles GPIO interrupts. > >> > > >> >Why gpio-regmap can't be used? > >> > >> I will try to use gpio-remap in the next version. > > > >If it appears that it makes code uglier / complicated, please add the note > >somewhere to answer the above question. > > I've traced the gpio-regmap.c file. It appears that for the driver to register > gpio_irq_chip, it must create the irq_domain and add it into gpio_regmap_config. > Additionally, the driver needs to register the irq handler by itself. > However, this process can be managed by the gpiolib if the driver fills in the struct > gpio_irq_chip inside struct gpio_chip before invoking gpiochip_add_data. Hmm... I thought this is solvable issue. Michael, is there a limitation in GPIO regmap that this driver can't be converted? > Moreover, apart from managing the registers for gpio direction and value, there > are several other registers that require access(interrupt enable, debounce...). > The GPIO IRQ status registers are located at different base addresses and are > not contiguous. It may need to create an additional regmap and assign the access > table to this regmap. AFAIK this is not a problem as you can provide your own xlate function that will take care about register mapping. > With the above consideration, I tend to keep using the existing method. I would like to hear from Michael if it's indeed a big obstacle. ... > >> >> + if (irq == data->irqs[0]) > >> >> + get_reg_offset = &rtd_gpio_gpa_offset; > >> >> + else if (irq == data->irqs[1]) > >> >> + get_reg_offset = &rtd_gpio_gpda_offset; > >> > > >> >Can't it be done before entering into chained IRQ handler? > >> > >> I will revise it. > > > >Thinking about this more, perhaps you can register two IRQ chips with > >different functions, so this won't be part of the very critical interrupt > >handler (as we all want to reduce overhead in it as much as possible). > >Anyway, think about this and try different options, choose the one you think > >the best. > > In the previous patch (v1), I had registered two IRQ chips with different > handlers. However, these two handlers appeared quite similar and the > gpio_irq_chip only allows the registration of a single handler. Therefore, > I ended up registering one handler for both IRQs and included conditional > checks within the handler to differentiate between the two. What is the performance impact that you have that condition in the interrupt handler? -- With Best Regards, Andy Shevchenko
