On Wed, Oct 30, 2019 at 05:07:17PM +0100, Linus Walleij wrote: > On Wed, Oct 30, 2019 at 4:48 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > On 30-10-2019 15:49, Linus Walleij wrote: > > > This does not seem to work I've tested in both a Bay Trail and > > a Cherry Trail device and neither will boot the kernel after > > these changes I'm afraid. > > > > I think it might be best to pass in an array of ranges (*) to > > gpiochip_add_data and have it register the ranges before > > doing the irq-chip setup. > > Yeah this new way of populating GPIO irqchips seems to be > pretty imperialistic and pull the whole world into the gpiolib. > I don't mind, once we have the semantic in one place we > can get it right. (As with the previous ordering issues.) > Hopefully it saves us from other problems. > > Thierry: is this approach for pin control ranges in line with your > initial design of the new way of registering gpio irqchips? I'm not sure I fully understand what the pin control ranges have to do with the embedded IRQ chip. The embedded IRQ chip represents a different way of using the GPIO lines, so anything that impacts the IRQ chip would presumably also impact the GPIO lines. Generally speaking, though, I don't think there's anything wrong with adding ranges to a GPIO chip descriptor before registering it. In fact I think that's really the only way to make it work if we've got a dependency on these ranges being available. So technically at any point after gpiochip_add_data() somebody could start using the GPIO chip, which means that technically there could be a race between the consumers and the call to gpiochip_add_pin_range(). If there's a dependency on the pin control ranges, they must be registered before the GPIO chip becomes available to consumers. Again, I don't think this has anything to do with IRQ chip. If you want to provide pin control ranges as part of adding the GPIO chip, they should be specified as part of the GPIO chip structure. Regarding the imperialistic nature of this: back at the time I think your argument had been that you wanted to move as much boiler plate as possible into gpiolib. Seems like this is very much in line with that. Thierry
Attachment:
signature.asc
Description: PGP signature
