| Message ID | 20131127201149.GB20734@saruman.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Nov 27, 2013 at 02:11:49PM -0600, Felipe Balbi wrote: > On Wed, Nov 27, 2013 at 10:02:47PM +0200, Aaro Koskinen wrote: > > On Wed, Nov 27, 2013 at 01:06:44PM -0600, Felipe Balbi wrote: > > > few cleanups on the old menelaus driver. I don't have > > > HW to test these patches, maybe Aaro can help here ? > > > > Hmm, I got: > > > > [ 1.330000] Unable to handle kernel NULL pointer dereference at virtual address 00000000 > > [ 1.340000] pgd = c0004000 > > [ 1.340000] [00000000] *pgd=00000000 > > [ 1.350000] Internal error: Oops: 17 [#1] ARM > > [ 1.350000] CPU: 0 PID: 1 Comm: swapper Not tainted 3.13.0-rc1-n8x0_tiny-los.git-729021f-00018-g74a0f39 #2 > > [ 1.350000] task: c782c000 ti: c782e000 task.ti: c782e000 > > [ 1.350000] PC is at mutex_lock+0x0/0x20 > > [ 1.350000] LR is at __irq_get_desc_lock+0x6c/0x88 [...] > > [ 1.350000] [<c016745c>] (mutex_lock+0x0/0x20) from [<c00509ac>] (__irq_get_desc_lock+0x6c/0x88) > > [ 1.350000] [<c00509ac>] (__irq_get_desc_lock+0x6c/0x88) from [<c0053518>] (__irq_set_handler+0x24/0x128) > > [ 1.350000] [<c0053518>] (__irq_set_handler+0x24/0x128) from [<c013ec80>] (menelaus_probe+0xbc/0x280) > > [ 1.350000] [<c013ec80>] (menelaus_probe+0xbc/0x280) from [<c0140ab8>] (i2c_device_probe+0x98/0xc0) [...] > hmm, irq_set_chip_and_handler() will call back into the irq_chip we just > registered, so my ->irq_bus_lock needs to have everything setup > (chip_data my mutex), this should solve it: Yes, that fixes it. Seems to work fine now. A. > diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c > index 376f01d..c9bd2fb 100644 > --- a/drivers/mfd/menelaus.c > +++ b/drivers/mfd/menelaus.c > @@ -1232,6 +1232,7 @@ static int menelaus_probe(struct i2c_client *client, > > the_menelaus = m; > m->client = client; > + mutex_init(&m->lock); > > irq_base = irq_alloc_descs(-1, 0, MENELAUS_NR_IRQS, 0); > if (irq_base < 0) { > @@ -1245,10 +1246,10 @@ static int menelaus_probe(struct i2c_client *client, > m->irq_base = irq_base; > > for (i = irq_base; i < irq_base + MENELAUS_NR_IRQS; i++) { > + irq_set_chip_data(i, m); > irq_set_chip_and_handler(i, &menelaus_irq_chip, > handle_simple_irq); > irq_set_nested_thread(i, 1); > - irq_set_chip_data(i, m); > set_irq_flags(i, IRQF_VALID); > } > > @@ -1280,8 +1281,6 @@ static int menelaus_probe(struct i2c_client *client, > } > } > > - mutex_init(&m->lock); > - > pr_info("Menelaus rev %d.%d\n", rev >> 4, rev & 0x0f); > > val = menelaus_read_reg(m, MENELAUS_VCORE_CTRL1); > > > let me know when you can, and I'll meld these hunks to proper patch. > > -- > balbi
Hi, On Wed, Nov 27, 2013 at 10:46:21PM +0200, Aaro Koskinen wrote: > Hi, > > On Wed, Nov 27, 2013 at 02:11:49PM -0600, Felipe Balbi wrote: > > On Wed, Nov 27, 2013 at 10:02:47PM +0200, Aaro Koskinen wrote: > > > On Wed, Nov 27, 2013 at 01:06:44PM -0600, Felipe Balbi wrote: > > > > few cleanups on the old menelaus driver. I don't have > > > > HW to test these patches, maybe Aaro can help here ? > > > > > > Hmm, I got: > > > > > > [ 1.330000] Unable to handle kernel NULL pointer dereference at virtual address 00000000 > > > [ 1.340000] pgd = c0004000 > > > [ 1.340000] [00000000] *pgd=00000000 > > > [ 1.350000] Internal error: Oops: 17 [#1] ARM > > > [ 1.350000] CPU: 0 PID: 1 Comm: swapper Not tainted 3.13.0-rc1-n8x0_tiny-los.git-729021f-00018-g74a0f39 #2 > > > [ 1.350000] task: c782c000 ti: c782e000 task.ti: c782e000 > > > [ 1.350000] PC is at mutex_lock+0x0/0x20 > > > [ 1.350000] LR is at __irq_get_desc_lock+0x6c/0x88 > [...] > > > [ 1.350000] [<c016745c>] (mutex_lock+0x0/0x20) from [<c00509ac>] (__irq_get_desc_lock+0x6c/0x88) > > > [ 1.350000] [<c00509ac>] (__irq_get_desc_lock+0x6c/0x88) from [<c0053518>] (__irq_set_handler+0x24/0x128) > > > [ 1.350000] [<c0053518>] (__irq_set_handler+0x24/0x128) from [<c013ec80>] (menelaus_probe+0xbc/0x280) > > > [ 1.350000] [<c013ec80>] (menelaus_probe+0xbc/0x280) from [<c0140ab8>] (i2c_device_probe+0x98/0xc0) > > [...] > > > hmm, irq_set_chip_and_handler() will call back into the irq_chip we just > > registered, so my ->irq_bus_lock needs to have everything setup > > (chip_data my mutex), this should solve it: > > Yes, that fixes it. Seems to work fine now. Awesome, should I add your tested-by ? I also added a few extra patches on top which I'll send soon.
> Awesome, should I add your tested-by ? I also added a few extra patches > on top which I'll send soon. When you next submit, can you capitalise the first character of the subject line after the final ':' also please, it will save me the trouble.
Hi, On Wed, Nov 27, 2013 at 09:00:34PM -0600, Felipe Balbi wrote: > Awesome, should I add your tested-by ? I also added a few extra patches > on top which I'll send soon. Yes, that's fine. Once you have the final set ready, I can also easily test it once more just in case. A.
diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c index 376f01d..c9bd2fb 100644 --- a/drivers/mfd/menelaus.c +++ b/drivers/mfd/menelaus.c @@ -1232,6 +1232,7 @@ static int menelaus_probe(struct i2c_client *client, the_menelaus = m; m->client = client; + mutex_init(&m->lock); irq_base = irq_alloc_descs(-1, 0, MENELAUS_NR_IRQS, 0); if (irq_base < 0) { @@ -1245,10 +1246,10 @@ static int menelaus_probe(struct i2c_client *client, m->irq_base = irq_base; for (i = irq_base; i < irq_base + MENELAUS_NR_IRQS; i++) { + irq_set_chip_data(i, m); irq_set_chip_and_handler(i, &menelaus_irq_chip, handle_simple_irq); irq_set_nested_thread(i, 1); - irq_set_chip_data(i, m); set_irq_flags(i, IRQF_VALID); } @@ -1280,8 +1281,6 @@ static int menelaus_probe(struct i2c_client *client, } } - mutex_init(&m->lock); - pr_info("Menelaus rev %d.%d\n", rev >> 4, rev & 0x0f); val = menelaus_read_reg(m, MENELAUS_VCORE_CTRL1);