lkml.org 
[lkml]   [2021]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] pinctrl: Add driver for Awinic AW9523/B I2C GPIO Expander
From
Date
Il 10/01/21 20:35, Linus Walleij ha scritto:
> On Sun, Jan 10, 2021 at 3:32 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@somainline.org> wrote:
>
>> So, I've retried some basic usage of the regcache, relevant snippets here:
>> static bool aw9523_volatile_reg(struct device *dev, unsigned int reg)
>> {
>>
>> return reg == AW9523_REG_IN_STATE(0) ||
>> reg == AW9523_REG_IN_STATE(AW9523_PINS_PER_PORT) ||
>> reg == AW9523_REG_CHIPID;
>> }
> (...)
>> Since REG_IN_STATE is used to read the GPIO input level, it's not
>> cacheable,
>
> Fair enough.
>
>> then CHIPID was set as not cacheable for safety: that may be
>> avoided, but that may make no sense.. since it's a one-time readout for
>> init putposes, it'd be useless to keep it cached.
>
> I guess.
>
>> Then, the set_bit/clear_bit in aw9523_irq_mask(), aw9523_irq_unmask were
>> replaced with calls to regmap_update_bits_async, example:
>>
>> regmap_update_bits_async(awi->regmap,
>> AW9523_REG_INTR_DIS(d->hwirq),
>> BIT(n), BIT(n));
>>
>> Where of course the value is either BIT(n) or 0 for mask and unmask
>> respectively.
>> Also, the bus_sync_unlock callback was changed as follows:
>>
>> static void aw9523_irq_bus_sync_unlock(struct irq_data *d)
>>
>> {
>> struct aw9523 *awi = gpiochip_get_data(irq_data_get_irq_chip_data(d));
>> regcache_mark_dirty(awi->regmap);
>> regcache_sync_region(awi->regmap, AW9523_REG_INTR_DIS(0),
>> AW9523_REG_INTR_DIS(AW9523_PINS_PER_PORT));
>> mutex_unlock(&awi->irq->lock);
> (...)
>> One of the biggest / oddest issues that I get when trying to use
>> regcache is that I'm getting badbadbad scheduling while atomic warnings
>> all over and I don't get why, since regcache_default_sync is just
>> calling _regmap_write, which is exactly what (non _prefix) regmap_write
>> also calls...
>
> OK that is the real problem to solve then.
>
>> As a reference, this is one out of "many" (as you can imagine) stacktraces:
>>
>> <3>[ 1.061428] BUG: scheduling while atomic: kworker/3:1/119/0x00000000
> (...)
>> <4>[ 1.063134] wait_for_completion_timeout+0x8c/0x110
>> <4>[ 1.063257] qup_i2c_wait_for_complete.isra.18+0x1c/0x80
>> <4>[ 1.063429] qup_i2c_xfer_v2_msg+0x2d4/0x3f0
>> <4>[ 1.063543] qup_i2c_xfer_v2+0x290/0xa28
>> <4>[ 1.063652] __i2c_transfer+0x16c/0x380
>> <4>[ 1.063798] i2c_transfer+0x5c/0x138
>> <4>[ 1.063903] i2c_transfer_buffer_flags+0x58/0x80
>> <4>[ 1.064060] regmap_i2c_write+0x1c/0x50
>> <4>[ 1.064168] _regmap_raw_write_impl+0x35c/0x688
>> <4>[ 1.064285] _regmap_bus_raw_write+0x64/0x80
>> <4>[ 1.064440] _regmap_write+0x58/0xa8
>> <4>[ 1.064545] regcache_default_sync+0xcc/0x1a0
>> <4>[ 1.064660] regcache_sync_region+0xdc/0xe8
>> <4>[ 1.064811] aw9523_irq_bus_sync_unlock+0x30/0x48
>> <4>[ 1.064931] __setup_irq+0x798/0x890
>> <4>[ 1.065034] request_threaded_irq+0xe0/0x198
>> <4>[ 1.065188] devm_request_threaded_irq+0x78/0xf8
>> <4>[ 1.065311] gpio_keyboard_probe+0x2a8/0x468
>
> scheduling while atomic happens when this trace gets called with interrupts
> disabled, usually because someone has taken a spinlock.
>
> Looking in __setup_irq() it looks safe.
>
> I would turn on lock debugging (lockdep) and see if I can find it that way.
>
> Yours,
> Linus Walleij
>

Hey!
Good news around the corner!

So, the issues were relative to the gpio matrix_keypad driver, which is
protecting with spinlocks (!), "throwing" us in atomic context and
obviously producing this kind of havoc.

Regarding this, I feel like we should bring this to the attention of the
matrix_keypad driver maintainer, Marek Vasut, which I'm including to the
Cc list of this email... but at the same time, that driver seems to be
largely outdated and for this reason I've decided to make one on-the-fly
that uses modern APIs instead and also seems to solve slowness issues on
my KB matrix connected to the AW9523.

Back to our topic, I have solved the issues that were preventing the
usage of a FLAT regcache, cleaned up a bit and tested the entire thing
again.
This works even better than before.

The V2 of this series is coming in a few minutes.
A huge thank you for your help!

-- Angelo

\
 
 \ /
  Last update: 2021-01-11 18:56    [W:0.078 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site