pt., 22 lis 2019 o 13:28 Linus Walleij <linus.walleij@xxxxxxxxxx> napisał(a): > > On Tue, Nov 19, 2019 at 5:42 PM Bartosz Golaszewski > <bgolaszewski@xxxxxxxxxxxx> wrote: > > wt., 19 lis 2019 o 16:03 Linus Walleij <linus.walleij@xxxxxxxxxx> napisał(a): > > > On Mon, Nov 18, 2019 at 11:15 AM Bartosz Golaszewski > > > <bgolaszewski@xxxxxxxxxxxx> wrote: > > > > > pon., 18 lis 2019 o 11:03 Yash Shah <yash.shah@xxxxxxxxxx> napisał(a): > > > Is it really so? The bgpio_lock does protect the registers used > > > by regmap-mmio but unless the interrupt code is also using the > > > same registers it is fine to have a different lock for those. > > > > > > Is the interrupt code really poking into the very same registers > > > as passed to bgpio_init()? > > > > > > Of course it could be seen as a bit dirty to poke around in the > > > same memory space with regmap and the bgpio_* accessors > > > but in practice it's no problem if they never touch the same > > > things. > > > > > > Yours, > > > Linus Walleij > > > > I'm wondering if it won't cause any inconsistencies when for example > > interrupts are being triggered on input lines while we're also reading > > their values? Seems to me it's just more clear to use a single lock > > for a register range. Most drivers using gpio-mmio do just that in > > their irq-related routines. > > OK good point. Just one lock for the whole thing is likely > more maintainable even if it works with two different locks. > > > Anyway: even without using bgpio_lock this code is inconsistent: if > > we're using regmap for interrupt registers, we should either decide to > > rely on locking provided by regmap or disable it and use a locally > > defined lock. > > OK makes sense, let's say we use the bgpio_lock everywhere > for this. > > Yash: are you OK with this? (Haven't read the new patch set > yet, maybe it is already fixed...) > > > Also: if we're using regmap, then let's use it > > everywhere, not only when it's convenient for updating registers. > > I think what you are saying is that we should extend gpio-mmio.c > with some optional regmap API (or create a separate MMIO library > for regmap consumers) which makes sense, but it feels a bit > heavy task to toss at contributors. > > We could add it to the TODO file, where I already have some > item like this for port-mapped I/O. > It's been on my personal TODO list too for some time as it seems it could benefit some i2c drivers too. Also: I think such a helper could eventually completely replace the generic drivers such as gpio-mmio and gpio-reg. In other words: good idea to put it into the TODO. If there are no objections I was thinking about starting the work soon aiming at v5.6, as soon as we get the recent changes in uAPI out of the way. Bart > Yours, > Linus Walleij
