2011-06-30 11:52:36

by Marek Vasut

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On Thursday, June 23, 2011 06:09:40 PM Stanislav Brabec wrote:
> Hallo.
>
> These Oops and kernel panic were observed on a Zaurus (spitz) machine
> (ARMv5, PXA270).

Looks similar to "Re: [PATCH v2] Input: Make ADS7846 independent on regulator"

CCing previously involved people, I'd like to see their opinion.
>
> Kernel version: config-3.0.0-rc4+ (2992c4b)
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 pgd = c0004000
> [00000000] *pgd=00000000
> Internal error: Oops: 17 [#1] PREEMPT
> Modules linked in: uinput g_ether cryptomgr aead arc4 crypto_algapi
> rt2800usb rt2800lib rt2x00usb rt2x00lib mac80211 cfg80211 sg pcmciamtd
> mousedev snd_soc_wm8750 snd_soc_pxa2xx_i2s snd_soc_core ohci_hcd usbcore
> pxa27x_udc physmap snd_pcm_oss snd_pcm snd_timer snd_page_alloc
> snd_mixer_oss snd soundcore rfcomm pxaficp_ir ircomm_tty ircomm irda ipv6
> hidp hid bluetooth rfkill crc16 CPU: 0 Not tainted (3.0.0-rc4+ #5)
> PC is at complete+0x28/0x7c
> LR is at complete+0x28/0x7c
> pc : [<c0036b6c>] lr : [<c0036b6c>] psr: 80000093
> sp : c3897b68 ip : c3897b68 fp : c3897b84
> r10: c4806000 r9 : c381f3e0 r8 : 0000000a
> r7 : c30f0da8 r6 : 00000000 r5 : 00000000 r4 : a0000013
> r3 : c3896000 r2 : 00000000 r1 : 00000103 r0 : 00000004
> Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
> Control: 0000397f Table: a080c000 DAC: 00000017
> Process kswapd0 (pid: 270, stack limit = 0xc3896278)
> Stack: (0xc3897b68 to 0xc3898000)
> 7b60: 00000000 c3808158 c3808158 c30f0da8 c3897b94
> c3897b88 7b80: c01e9b0c c0036b50 c3897bb4 c3897b98 c01eac2c c01e9b08
> c3808158 c388e220 7ba0: c30f0df8 c30f0da8 c3897bf4 c3897bb8 c01eb60c
> c01eab24 00000000 00000010 7bc0: 00000000 00000000 00000001 c03b3f00
> c039b5d4 00000000 00000103 0000000a 7be0: 00000006 00000000 c3897c14
> c3897bf8 c0044924 c01eb4dc c0044894 c3896000 7c00: 00000001 c03b3f3c
> c3897c4c c3897c18 c0044eb8 c00448a0 00000000 c38046c0 7c20: c3897c44
> 00000010 00000000 00100000 00000002 00000001 c3896000 00000000 7c40:
> c3897c5c c3897c50 c00453a0 c0044e2c c3897c74 c3897c60 c002406c c0045360
> 7c60: ffffffff c3897cac c3897cfc c3897c78 c0024b84 c002400c 4022d320
> 4022e000 7c80: 08000075 00001000 c32273c0 c03ce1c0 c2b49b78 4022d000
> c2b420b4 00000001 7ca0: 00000000 c3897cfc 00000000 c3897cc0 c00afc54
> c002edd8 00000013 ffffffff 7cc0: 00000000 c0447550 00000002 c32273f4
> c3897d3c c2b49b78 c3897d3c 00000000 7ce0: c341fbec 00000003 00000001
> c03ce1c0 c3897d84 c3897d00 c00affd8 c00afb9c 7d00: c00af774 c003505c
> 00000000 c03ce1c0 00000000 c341fc10 c03ce1c0 00000000 7d20: c30d0c28
> c3897d50 c341fc10 c3897e44 c3897d9c c341fbec c00b09cc c3be9308 7d40:
> 00000080 00000000 00000000 c341fc00 00000074 00000074 00000000 c03ce1c0
> 7d60: c03ce1c0 c3897f44 c341fbec 00000003 00000001 c3897e44 c3897d9c
> c3897d88 7d80: c00b07ac c00aff4c 00000001 c03ce1d8 c3897e2c c3897da0
> c009b940 c00b0778 7da0: 00000001 c03acfc8 00000000 00000000 c3896000
> 00000000 c04032a0 c04032c0 7dc0: c040cc20 c040cc40 c040cc60 c040cc80
> c3897e2c c3897de0 c009a908 c009a7c0 7de0: c0403220 c03bbd14 c3897e54
> c03ce198 c03cec38 c3897df4 c3897df4 00000000 7e00: 00000000 c03acfc8
> c3897f44 00000020 00000020 0000000c c3896000 00000001 7e20: c3897e7c
> c3897e30 c009c448 c009b6a4 00000000 00000000 00000001 c3897e38 7e40:
> c3897e40 c04445d8 c03ce1f8 00000020 00000000 00000020 c03acfc8 00000002
> 7e60: c3897f44 00000020 0000000c 00000001 c3897f04 c3897e80 c009c8f8
> c009c23c 7e80: 00000001 00000014 c3897f44 00000020 c3897f44 0000000c
> c03ad224 c03ad228 7ea0: 00000000 ffffffff 00000001 00000003 c3897ecc
> 00000000 00000000 00000000 7ec0: 00000020 c3897ed0 00000056 00000000
> 000018c9 00000000 c03acfc8 c03acfc8 7ee0: 00000000 c03acfc8 c03acfc8
> 00000000 0000000c 00000000 c3897fbc c3897f08 7f00: c009ceb0 c009c580
> 00000000 c039af90 c3897fbc 00000000 c03acfc8 c3897f84 7f20: 00000000
> c3897f8c 00000001 c3896000 00000000 00002f2e 00000000 00000000 7f40:
> 184c1a27 00000048 00000000 ffffffff 00000000 000000d0 00000001 00000001
> 7f60: 00000001 0000003c 00000000 00000003 00000000 00000000 000000d0
> 00000000 7f80: c0059f8c c3897f84 c3897f84 00000000 c3897fbc c3825f38
> c03acfc8 c009ca20 7fa0: 00000013 00000000 00000000 00000000 c3897ff4
> c3897fc0 c0059be0 c009ca2c 7fc0: c3825f38 00000000 c03acfc8 00000000
> c3897fd0 c3897fd0 00000000 c3825f38 7fe0: c0059b50 c00258d8 00000000
> c3897ff8 c00258d8 c0059b5c 00000000 00000000 [<c0036b6c>]
> (complete+0x28/0x7c) from [<c01e9b0c>] (spi_complete+0x10/0x14)
> [<c01e9b0c>] (spi_complete+0x10/0x14) from [<c01eac2c>]
> (giveback+0x114/0x12c) [<c01eac2c>] (giveback+0x114/0x12c) from
> [<c01eb60c>] (pump_transfers+0x13c/0x6f8) [<c01eb60c>]
> (pump_transfers+0x13c/0x6f8) from [<c0044924>] (tasklet_action+0x90/0xf0)
> [<c0044924>] (tasklet_action+0x90/0xf0) from [<c0044eb8>]
> (__do_softirq+0x98/0x138) [<c0044eb8>] (__do_softirq+0x98/0x138) from
> [<c00453a0>] (irq_exit+0x4c/0xa8) [<c00453a0>] (irq_exit+0x4c/0xa8) from
> [<c002406c>] (asm_do_IRQ+0x6c/0x8c) [<c002406c>] (asm_do_IRQ+0x6c/0x8c)
> from [<c0024b84>] (__irq_svc+0x44/0xcc) Exception stack(0xc3897c78 to
> 0xc3897cc0)
> 7c60: 4022d320
> 4022e000 7c80: 08000075 00001000 c32273c0 c03ce1c0 c2b49b78 4022d000
> c2b420b4 00000001 7ca0: 00000000 c3897cfc 00000000 c3897cc0 c00afc54
> c002edd8 00000013 ffffffff [<c0024b84>] (__irq_svc+0x44/0xcc) from
> [<c002edd8>] (xscale_flush_user_cache_range+0x18/0x3c) [<c002edd8>]
> (xscale_flush_user_cache_range+0x18/0x3c) from [<c00affd8>]
> (try_to_unmap_file+0x98/0x4ec) [<c00affd8>] (try_to_unmap_file+0x98/0x4ec)
> from [<c00b07ac>] (try_to_unmap+0x40/0x60) [<c00b07ac>]
> (try_to_unmap+0x40/0x60) from [<c009b940>] (shrink_page_list+0x2a8/0x8cc)
> [<c009b940>] (shrink_page_list+0x2a8/0x8cc) from [<c009c448>]
> (shrink_inactive_list+0x218/0x344) [<c009c448>]
> (shrink_inactive_list+0x218/0x344) from [<c009c8f8>]
> (shrink_zone+0x384/0x4ac) [<c009c8f8>] (shrink_zone+0x384/0x4ac) from
> [<c009ceb0>] (kswapd+0x490/0x7d0) [<c009ceb0>] (kswapd+0x490/0x7d0) from
> [<c0059be0>] (kthread+0x90/0x98) [<c0059be0>] (kthread+0x90/0x98) from
> [<c00258d8>] (kernel_thread_exit+0x0/0x8) Code: e3843080 e121f003 e3a00001
> ebfff96a (e5953000)
> ---[ end trace a4b97c165577d355 ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> [<c00298fc>] (unwind_backtrace+0x0/0xe4) from [<c0295e1c>]
> (dump_stack+0x18/0x1c) [<c0295e1c>] (dump_stack+0x18/0x1c) from
> [<c0295e84>] (panic+0x64/0x18c) [<c0295e84>] (panic+0x64/0x18c) from
> [<c002842c>] (die+0x300/0x358) [<c002842c>] (die+0x300/0x358) from
> [<c002c294>] (__do_kernel_fault+0x6c/0x90) [<c002c294>]
> (__do_kernel_fault+0x6c/0x90) from [<c002c508>]
> (do_page_fault+0x250/0x270) [<c002c508>] (do_page_fault+0x250/0x270) from
> [<c0024214>] (do_DataAbort+0x38/0xa0) [<c0024214>]
> (do_DataAbort+0x38/0xa0) from [<c0024b2c>] (__dabt_svc+0x4c/0x60)
> Exception stack(0xc3897b20 to 0xc3897b68)
> 7b20: 00000004 00000103 00000000 c3896000 a0000013 00000000 00000000
> c30f0da8 7b40: 0000000a c381f3e0 c4806000 c3897b84 c3897b68 c3897b68
> c0036b6c c0036b6c 7b60: 80000093 ffffffff
> [<c0024b2c>] (__dabt_svc+0x4c/0x60) from [<c0036b6c>] (complete+0x28/0x7c)
> [<c0036b6c>] (complete+0x28/0x7c) from [<c01e9b0c>]
> (spi_complete+0x10/0x14) [<c01e9b0c>] (spi_complete+0x10/0x14) from
> [<c01eac2c>] (giveback+0x114/0x12c) [<c01eac2c>] (giveback+0x114/0x12c)
> from [<c01eb60c>] (pump_transfers+0x13c/0x6f8) [<c01eb60c>]
> (pump_transfers+0x13c/0x6f8) from [<c0044924>] (tasklet_action+0x90/0xf0)
> [<c0044924>] (tasklet_action+0x90/0xf0) from [<c0044eb8>]
> (__do_softirq+0x98/0x138) [<c0044eb8>] (__do_softirq+0x98/0x138) from
> [<c00453a0>] (irq_exit+0x4c/0xa8) [<c00453a0>] (irq_exit+0x4c/0xa8) from
> [<c002406c>] (asm_do_IRQ+0x6c/0x8c) [<c002406c>] (asm_do_IRQ+0x6c/0x8c)
> from [<c0024b84>] (__irq_svc+0x44/0xcc) Exception stack(0xc3897c78 to
> 0xc3897cc0)
> 7c60: 4022d320
> 4022e000 7c80: 08000075 00001000 c32273c0 c03ce1c0 c2b49b78 4022d000
> c2b420b4 00000001 7ca0: 00000000 c3897cfc 00000000 c3897cc0 c00afc54
> c002edd8 00000013 ffffffff [<c0024b84>] (__irq_svc+0x44/0xcc) from
> [<c002edd8>] (xscale_flush_user_cache_range+0x18/0x3c) [<c002edd8>]
> (xscale_flush_user_cache_range+0x18/0x3c) from [<c00affd8>]
> (try_to_unmap_file+0x98/0x4ec) [<c00affd8>] (try_to_unmap_file+0x98/0x4ec)
> from [<c00b07ac>] (try_to_unmap+0x40/0x60) [<c00b07ac>]
> (try_to_unmap+0x40/0x60) from [<c009b940>] (shrink_page_list+0x2a8/0x8cc)
> [<c009b940>] (shrink_page_list+0x2a8/0x8cc) from [<c009c448>]
> (shrink_inactive_list+0x218/0x344) [<c009c448>]
> (shrink_inactive_list+0x218/0x344) from [<c009c8f8>]
> (shrink_zone+0x384/0x4ac) [<c009c8f8>] (shrink_zone+0x384/0x4ac) from
> [<c009ceb0>] (kswapd+0x490/0x7d0) [<c009ceb0>] (kswapd+0x490/0x7d0) from
> [<c0059be0>] (kthread+0x90/0x98) [<c0059be0>] (kthread+0x90/0x98) from
> [<c00258d8>] (kernel_thread_exit+0x0/0x8)
>
>
> My config:
> http://www.penguin.cz/~utx/zaurus/feed/images/spitz/config-3.0.0-rc4+-spitz
>
> Only small ads7846 patch was applied on top of it:
> http://www.penguin.cz/~utx/zaurus/feed/images/spitz/zImage-3.0.0-rc4+-spitz
> .diff


2011-06-30 15:15:49

by Stanislav Brabec

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

Marek Vasut wrote:
> On Thursday, June 23, 2011 06:09:40 PM Stanislav Brabec wrote:
> > Hallo.
> >
> > These Oops and kernel panic were observed on a Zaurus (spitz) machine
> > (ARMv5, PXA270).
>
> Looks similar to "Re: [PATCH v2] Input: Make ADS7846 independent on regulator"

I don't see any trace in that thread. But yes, I was testing your patch
from this thread. Without it, ADS7846 does not work on spitz (ADS7846
does not have any dedicated regulator there). I want to
add .needs_regulator bool to ads7846.c and send the patch again to the
list.

After sending the trace I was able to reproduced it several times by
attaching of external charger.

Then I tried to apply "[PATCH] MAX1111: Fix race condition causing NULL
pointer exception", connected charger that periodically disconnects and
not seen the crash again. No OOPS was seen after ~100 reconnects.

So I guess that MAX1111 AC voltage reading (via SPI) was involved in an
incorrect moment and race happened there and your MAX1111 race condition
fix fixes it.

--
Stanislav Brabec
http://www.penguin.cz/~utx

2011-06-30 15:09:58

by Marek Vasut

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On Thursday, June 30, 2011 04:45:18 PM Stanislav Brabec wrote:
> Marek Vasut wrote:
> > On Thursday, June 23, 2011 06:09:40 PM Stanislav Brabec wrote:
> > > Hallo.
> > >
> > > These Oops and kernel panic were observed on a Zaurus (spitz) machine
> > > (ARMv5, PXA270).
> >
> > Looks similar to "Re: [PATCH v2] Input: Make ADS7846 independent on
> > regulator"
>
> I don't see any trace in that thread. But yes, I was testing your patch
> from this thread. Without it, ADS7846 does not work on spitz (ADS7846
> does not have any dedicated regulator there). I want to
> add .needs_regulator bool to ads7846.c and send the patch again to the
> list.
>
> After sending the trace I was able to reproduced it several times by
> attaching of external charger.
>
> Then I tried to apply "[PATCH] MAX1111: Fix race condition causing NULL
> pointer exception", connected charger that periodically disconnects and
> not seen the crash again. No OOPS was seen after ~100 reconnects.
>
> So I guess that MAX1111 AC voltage reading (via SPI) was involved in an
> incorrect moment and race happened there and your MAX1111 race condition
> fix fixes it.

It's not mine, it's from Pavel Herrmann actually ;-)

But yes, it's likely either this or the regulator stuff again (which I believed
was fixed).

2011-06-30 15:26:36

by Igor Grinberg

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On 06/30/11 17:45, Stanislav Brabec wrote:

> Marek Vasut wrote:
>> On Thursday, June 23, 2011 06:09:40 PM Stanislav Brabec wrote:
>>> Hallo.
>>>
>>> These Oops and kernel panic were observed on a Zaurus (spitz) machine
>>> (ARMv5, PXA270).
>> Looks similar to "Re: [PATCH v2] Input: Make ADS7846 independent on regulator"
> I don't see any trace in that thread. But yes, I was testing your patch
> from this thread. Without it, ADS7846 does not work on spitz (ADS7846
> does not have any dedicated regulator there). I want to
> add .needs_regulator bool to ads7846.c and send the patch again to the
> list.

Please, don't...
I thought we've finished discussing the regulator issue...

Instead of patching the driver and the platform data and the board file,
you should do _either_ of the following:
1) add regulator definition for ads7846 into the board file
2) enable the CONFIG_REGULATOR_DUMMY in your kernel configuration
3) use regulator_use_dummy_regulator() call in the board file.

Using either of the above, you will not need the patch for ads7846, only for the board file.


--
Regards,
Igor.

2011-06-30 15:37:21

by Pavel Herrmann

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On Thursday, June 30, 2011 04:45:18 PM Stanislav Brabec wrote:
> Then I tried to apply "[PATCH] MAX1111: Fix race condition causing NULL
> pointer exception", connected charger that periodically disconnects and
> not seen the crash again. No OOPS was seen after ~100 reconnects.
>
> So I guess that MAX1111 AC voltage reading (via SPI) was involved in an
> incorrect moment and race happened there and your MAX1111 race condition
> fix fixes it.

Hi,

Are you using the first or second version of the patch? if the former, please
use v2 (sent a few days later), which has solved the same problem by using a
mutex instead of allocating message data on stack (which is not good for DMA)

as for the backstory, this crash ocurrs when a short (measured in time spent)
message was enqueued after a long message, so that the short one finished first
(the actual bug was present even if the long one finished first, but in that
case there was double complete() on the one completion instead of a NULL
dereference)

Pavel

2011-06-30 16:00:07

by Mark Brown

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On Thu, Jun 30, 2011 at 06:25:51PM +0300, Igor Grinberg wrote:
> On 06/30/11 17:45, Stanislav Brabec wrote:

> > from this thread. Without it, ADS7846 does not work on spitz (ADS7846
> > does not have any dedicated regulator there). I want to
> > add .needs_regulator bool to ads7846.c and send the patch again to the
> > list.

> Please, don't...
> I thought we've finished discussing the regulator issue...

> Instead of patching the driver and the platform data and the board file,
> you should do _either_ of the following:
> 1) add regulator definition for ads7846 into the board file
> 2) enable the CONFIG_REGULATOR_DUMMY in your kernel configuration
> 3) use regulator_use_dummy_regulator() call in the board file.

> Using either of the above, you will not need the patch for ads7846, only for the board file.

Yes, we've been through this repeatedly. None of these options take any
appreciable time to implement, it'd be much more productive to just do
so.

2011-06-30 16:13:33

by Stanislav Brabec

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

Igor Grinberg wrote:
> On 06/30/11 17:45, Stanislav Brabec wrote:
> I want to
> > add .needs_regulator bool to ads7846.c and send the patch again to the
> > list.
>
> Please, don't...
> I thought we've finished discussing the regulator issue...

The discussion ended without a fix (at least for spitz). I seen two
proposals:
- As ADS7846 hardware does not require dedicated regulator, don't
require it in driver and fail only on platforms that have a dedicated
regulator.
- Use dummy regulators for all platforms without dedicated ADS7846
regulator.

>1) add regulator definition for ads7846 into the board file
There is no dedicated regulator on spitz, ADS7846 uses common always-on
power supply.

>2) enable the CONFIG_REGULATOR_DUMMY in your kernel configuration
>3) use regulator_use_dummy_regulator() call in the board file.

OK, I will do 2 or 3. In the new kernel spitz has a regulator, but it is
not related to ADS7846. And it is actually broken:

I2C: i2c-0: PXA I2C adapter
Resetting I2C Controller Unit
(null): i2c_pxa_abort: called in slave mode
i2c i2c-1: adapter [pxa_i2c-i2c.1] registered
i2c 1-000c: uevent
isl6271a 1-000c: probe
i2c i2c-1: master_xfer[0] R, addr=0x0c, len=1
i2c i2c-1: setting to bus master
i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000442, ICR=000007e0, IBMR=03
i2c i2c-1: state:i2c_pxa_irq_txempty:932: ISR=00000002, ICR=000007e0, IBMR=03
i2c i2c-1: Retrying transmission
i2c i2c-1: setting to bus master
i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000045, ICR=000007e1, IBMR=00
i2c i2c-1: state:i2c_pxa_irq_txempty:932: ISR=00000005, ICR=000017ee, IBMR=00
i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000087, ICR=000017e6, IBMR=00
print_constraints: vcc_core range: 850 <--> 1600 mV at 1350 mV
print_constraints: vcc_core range: 1100 mV
isl6271a 1-000c: Failed to set supply vcc_core
isl6271a 1-000c: failed to register isl6271a
isl6271a: probe of 1-000c failed with error -16
i2c i2c-1: client [isl6271a] registered with bus id 1-000c
I2C: i2c-1: PXA I2C adapter
...
cpufreq: Didn't find vcc_core regulator

--
Stanislav Brabec
http://www.penguin.cz/~utx

2011-06-30 16:22:06

by Stanislav Brabec

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

Pavel Herrmann wrote:
> On Thursday, June 30, 2011 04:45:18 PM Stanislav Brabec wrote:
> > Then I tried to apply "[PATCH] MAX1111: Fix race condition causing NULL
>
> > So I guess that MAX1111 AC voltage reading (via SPI) was involved in an
> > incorrect moment and race happened there and your MAX1111 race condition
> > fix fixes it.

> Are you using the first or second version of the patch? if the former, please
> use v2 (sent a few days later), which has solved the same problem by using a
> mutex instead of allocating message data on stack (which is not good for DMA)

I guess the second one with mutex.
This is my work-in-progress-mix patch:
http://www.penguin.cz/~utx/zaurus/feed/images/spitz/zImage-3.0.0-rc5+-spitz.diff

Several hours later, charging was turned on/off at least 1000 times
without any crash. So it seems that it was the correct race condition
fix.

> as for the backstory, this crash ocurrs when a short (measured in time spent)
> message was enqueued after a long message, so that the short one finished first
> (the actual bug was present even if the long one finished first, but in that
> case there was double complete() on the one completion instead of a NULL
> dereference)

I guess that inserting of power supply initiates reading of voltages on
MAX1111. The long one may be touchscreen or LCD control.

--
Stanislav Brabec
http://www.penguin.cz/~utx

2011-06-30 17:41:02

by Igor Grinberg

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On 06/30/11 19:13, Stanislav Brabec wrote:

> Igor Grinberg wrote:
>> On 06/30/11 17:45, Stanislav Brabec wrote:
>> I want to
>>> add .needs_regulator bool to ads7846.c and send the patch again to the
>>> list.
>> Please, don't...
>> I thought we've finished discussing the regulator issue...
> The discussion ended without a fix (at least for spitz).

Well, the discussion was mainly about how the regulator API should be used.
The proposed patch, used the regulator API incorrectly.
If I recall correctly, in the end of the discussion,
Mark provided another option for configuring the regulator API,
which is number 3 below.

> I seen two
> proposals:
> - As ADS7846 hardware does not require dedicated regulator, don't
> require it in driver and fail only on platforms that have a dedicated
> regulator.

The thing is that ads7846 chip itself just requires power supply
(as most of the peripheral chips), the one who decides if it will be switchable
is the board vendor and the vendor may use any source for the power supply.
It can be fixed, it can be some kind of discrete LDO or one of the power
supplies coming from the system PMIC.
The ads7846 driver provides the regulator functionality via the regulator API
and it is up to board code to provide the appropriate regulator definitions or
configure the regulator API so the ads7846 will function correctly.

> - Use dummy regulators for all platforms without dedicated ADS7846
> regulator.

IMO, this is the right way.

>> 1) add regulator definition for ads7846 into the board file
> There is no dedicated regulator on spitz, ADS7846 uses common always-on
> power supply.

Mark,

Is there a kind of regulator for this case (except dummy)?
Some kind of fixed regulator which is not binded to any supply?

>> 2) enable the CONFIG_REGULATOR_DUMMY in your kernel configuration
>> 3) use regulator_use_dummy_regulator() call in the board file.
> OK, I will do 2 or 3. In the new kernel spitz has a regulator, but it is
> not related to ADS7846. And it is actually broken:
>
> I2C: i2c-0: PXA I2C adapter
> Resetting I2C Controller Unit
> (null): i2c_pxa_abort: called in slave mode
> i2c i2c-1: adapter [pxa_i2c-i2c.1] registered
> i2c 1-000c: uevent
> isl6271a 1-000c: probe
> i2c i2c-1: master_xfer[0] R, addr=0x0c, len=1
> i2c i2c-1: setting to bus master
> i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000442, ICR=000007e0, IBMR=03
> i2c i2c-1: state:i2c_pxa_irq_txempty:932: ISR=00000002, ICR=000007e0, IBMR=03
> i2c i2c-1: Retrying transmission
> i2c i2c-1: setting to bus master
> i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000045, ICR=000007e1, IBMR=00
> i2c i2c-1: state:i2c_pxa_irq_txempty:932: ISR=00000005, ICR=000017ee, IBMR=00
> i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000087, ICR=000017e6, IBMR=00
> print_constraints: vcc_core range: 850 <--> 1600 mV at 1350 mV
> print_constraints: vcc_core range: 1100 mV
> isl6271a 1-000c: Failed to set supply vcc_core
> isl6271a 1-000c: failed to register isl6271a
> isl6271a: probe of 1-000c failed with error -16
> i2c i2c-1: client [isl6271a] registered with bus id 1-000c
> I2C: i2c-1: PXA I2C adapter
> ...
> cpufreq: Didn't find vcc_core regulator

This looks like the regulator definitions for spitz are broken and need to be fixed.

--
Regards,
Igor.

2011-06-30 18:01:44

by Mark Brown

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

On Thu, Jun 30, 2011 at 08:40:40PM +0300, Igor Grinberg wrote:
> On 06/30/11 19:13, Stanislav Brabec wrote:
> > Igor Grinberg wrote:

> >> I thought we've finished discussing the regulator issue...

> > The discussion ended without a fix (at least for spitz).

> Well, the discussion was mainly about how the regulator API should be used.
> The proposed patch, used the regulator API incorrectly.
> If I recall correctly, in the end of the discussion,
> Mark provided another option for configuring the regulator API,
> which is number 3 below.

The discussion ended with you guys having the three different options
that would work, you just need to use one of them.

> > I seen two
> > proposals:
> > - As ADS7846 hardware does not require dedicated regulator, don't
> > require it in driver and fail only on platforms that have a dedicated
> > regulator.

> The thing is that ads7846 chip itself just requires power supply

Right, and the regulator API hides the non-switchability of the supply
from the driver so there's no need for the driver to worry about how the
supplies are wired up. It just turns the regulator on when it needs it
and turns it off when it doesn't.

> >> 1) add regulator definition for ads7846 into the board file
> > There is no dedicated regulator on spitz, ADS7846 uses common always-on
> > power supply.

> Is there a kind of regulator for this case (except dummy)?
> Some kind of fixed regulator which is not binded to any supply?

This is just a fixed voltage regulator, support for that has been in the
kernel since the regulator API was merged. This is the best solution,
it ensures that you don't mistakenly activate dummy reglators for
supplies that really need software control.

> > i2c i2c-1: Retrying transmission
> > i2c i2c-1: setting to bus master
> > i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000045, ICR=000007e1, IBMR=00
> > i2c i2c-1: state:i2c_pxa_irq_txempty:932: ISR=00000005, ICR=000017ee, IBMR=00
> > i2c i2c-1: state:i2c_pxa_handler:981: ISR=00000087, ICR=000017e6, IBMR=00

This looks like you've got I2C I/O issues talking to something.

> > print_constraints: vcc_core range: 850 <--> 1600 mV at 1350 mV
> > print_constraints: vcc_core range: 1100 mV

This looks like there's two definitions for vcc_core.

2011-06-30 20:50:44

by Igor Grinberg

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)



On 06/30/11 21:01, Mark Brown wrote:
> On Thu, Jun 30, 2011 at 08:40:40PM +0300, Igor Grinberg wrote:
>> On 06/30/11 19:13, Stanislav Brabec wrote:
>
>>> I seen two
>>> proposals:
>>> - As ADS7846 hardware does not require dedicated regulator, don't
>>> require it in driver and fail only on platforms that have a dedicated
>>> regulator.
>> The thing is that ads7846 chip itself just requires power supply
> Right, and the regulator API hides the non-switchability of the supply
> from the driver so there's no need for the driver to worry about how the
> supplies are wired up. It just turns the regulator on when it needs it
> and turns it off when it doesn't.
>
>>>> 1) add regulator definition for ads7846 into the board file
>>> There is no dedicated regulator on spitz, ADS7846 uses common always-on
>>> power supply.
>> Is there a kind of regulator for this case (except dummy)?
>> Some kind of fixed regulator which is not binded to any supply?
> This is just a fixed voltage regulator, support for that has been in the
> kernel since the regulator API was merged. This is the best solution,
> it ensures that you don't mistakenly activate dummy reglators for
> supplies that really need software control.

Right, just as I thought (I still haven't made myself familiar with all the
regulator API aspects).

Stanislav,
Can't you define a fixed voltage regulator for the ads7846 in spitz board file?
This shouldn't be a hard task and is the right solution from the regulator API POV.


--
Regards,
Igor.

2011-06-30 22:20:22

by Stanislav Brabec

[permalink] [raw]
Subject: Re: kernel panic in spi_complete() on spitz (PXA270)

Igor Grinberg wrote:

> Stanislav,
> Can't you define a fixed voltage regulator for the ads7846 in spitz board file?
> This shouldn't be a hard task and is the right solution from the regulator API POV.

Yes, I can.

ADS7846 needs more definitions in the platform file to work well on
spitz. The current platform definition is not sufficient and pointer
noise makes it unusable (e. g. 100 pixels jumps). I am comparing Lineo's
2.4 code with 2.6 algorithm and experimenting to get the best results.

--

________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx