On Mon, Jun 03, 2019 at 02:12:27PM +0200, Thierry Reding wrote: > On Mon, Jun 03, 2019 at 12:58:02PM +0200, Linus Walleij wrote: > > On Mon, Jun 3, 2019 at 9:53 AM Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > > > Me > > > > > > Please drop this. The default .to_irq() should be good for everyone. > > > > Also patch 2/2 now contains a identical copy of the gpiolib > > > > .to_irq() which I suspect you indended to drop, actually. > > > > > > It's not actually identical to the gpiolib implementation. There's still > > > the conversion to the non-linear DT representation for GPIO specifiers > > > from the linear GPIO number space, which is not taken care of by the > > > gpiolib variant. That's precisely the point why this patch makes it > > > possible to let the driver override things. > > > > OK something is off here, because the purpose of the irqdomain > > is exactly to translate between different number spaces, so it should > > not happen in the .to_irq() function at all. > > > > Irqdomain uses .map() in the old variant and .translate() in the > > hierarchical variant to do this, so something is skewed. > > > > All .to_irq() should ever do is just call the irqdomain to do the > > translation, no other logic (unless I am mistaken) so we should > > be able to keep the simple .to_irq() logic inside gpiolib. > > Well, that's exactly the problem that I'm trying to solve. The problem > is that .translate() translates from the DT number space to the GPIO or > IRQ number space. However, since gpiochip_to_irq() now wants to call the > irq_create_fwspec_mapping() interface, it must convert from the offset > (in GPIO space) into the DT number space, which is what that function > expects. Hm... I wonder if we even need this irq_create_fwspec_mapping() there. Couldn't we just do an irq_create_mapping() since we already know which one of the GPIO IRQ controller's interrupts we want to create a mapping for? If we already convert to the GPIO number space in the .translate() then the offset already corresponds to the one that we need to map, no? I'll make a note to try that tomorrow. Thierry
Attachment:
signature.asc
Description: PGP signature
