All of lore.kernel.org
 help / color / mirror / Atom feed
* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-10 15:12 ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-10 15:12 UTC (permalink / raw)
  To: linux-rpi-kernel, linux-arm-kernel, linux-serial, linux-clk; +Cc: Eric Anholt

Hi!

I got a strange effect when using the auxiliar UART as the logging console:

The kernel is configured for drivers/tty/serial/8250/8250_bcm2835aux.c to
get linked into the main kernel (requires also 8250.c to be linked in).

As long as I boot using the main UART (PL011) in the kernel parameters:
  earlyprintk consoleblank=0 console=ttyAMA0 root=/dev/mmcblk0p2 rootwait

Everything works fine - early_printk works and normal logging works as well.
Both configured ttys (/dev/ttyAMA0 and /dev/ttyS0) in /etc/initttab
work fine.

Now when I switch the console to use the auxiliar UART
(which right now does not support early_printk) like this:
  earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait

I get the situation where:
* u-boot starts and writes to the main UART
* Kernel boots using early_printk logging to the main UART until we get to:
  [    2.657090] bootconsole [earlycon0] disabled
* then we see the earlyprintk buffer dumped on the aux-uart
* then booting continues and logging happens on the aux-uart:
  [    2.657090] bootconsole [earlycon0] disabled
  [    2.662673] 20201000.serial: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
  [    2.663793] console [ttyS0] disabled
  [    2.663899] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
  [    3.384166] console [ttyS0] enabled
* everything works fine and I get networking and everything up
  [....] Starting NTP server: ntpd. ok
  [....] Starting OpenBSD Secure Shell server: sshd. ok
  My IP address is 10.10.10.41 ::127.0.0.1

  Raspbian GNU/Linux 7 raspcm.intern.sperl.org ttyS0
* I can even log in via SSH and networking works.
  Note that I have the getty for ttyAMA0 disabled in inittab
* when I then enable the tty on the main UART like this:
  root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
* I get the following:
  [   72.962845] uart-pl011 20201000.serial: no DMA platform data
* the system freezes…
* sometimes I still get the following on the auxiliar UART before the system “crashes":
  (at least when starting the tty from inittab)
  [   73.072985] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
  [   73.079132] ------------[ cut here ]------------
  [   73.083848] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
  [   73.093067] ---[ end trace 416e78cea88f5fb5 ]---
  [   73.097848] ------------[ cut here ]------------
  [   73.102616] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
  [   73.112219] ---[ end trace 416e78cea88f5fb6 ]---

A similar effect I get when using stty: 
  root@raspcm:~# stty -F /dev/ttyAMA0
  [  128.878127] uart-pl011 20201000.serial: no DMA platform data
  speed 9600 baud; line = 0;
  -brkint -imaxbel
Then the system freezes.

As if the plld does not lock - even if it (probably) is already locked.

Or the message actually comes from bcm2835_clock_wait_busy which also
produces this message - so maybe we should better identify the message.

Note that the same also applies if I remove earlyprintk with
the only difference being that I do not get the initial boot
messages on the main UART.
(at that time only: "Uncompressing Linux... done, booting the kernel.”
shows on the main uart after u-boot start..)

Unfortunately it is only possible to really test this on a
rpi-Compute module, as only there we can expose both uarts
on distinct GPIOs at the same time:
  GPIO 14: level=1 fsel=4 alt=0 func=TXD0
  GPIO 15: level=1 fsel=4 alt=0 func=RXD0
  GPIO 32: level=1 fsel=2 alt=5 func=TXD1
  GPIO 33: level=1 fsel=2 alt=5 func=RXD1

Here the commits I have used based on spi/for-next (based on 4.4.0)
with the patches (* prefix) for the driver for aux-uart
added separately as they are not in the tree I have used:
* a96f6c7 serial: bcm2835: add driver for bcm2835-aux-uart
* 464e876 clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
* 1af1b28 clk: bcm2835: Add a driver for the auxiliary peripheral clock gates.
ece19b8 Merge remote-tracking branch 'spi/fix/loopback' into spi-linus
887e6a3 Merge tag 'spi-v4.5' into spi-linus
ebea7c0 spi: fix counting in spi-loopback-test code
cabeea9 Merge remote-tracking branches 'spi/topic/sun4i', 'spi/topic/topcliff-pc
4f95307 Merge remote-tracking branches 'spi/topic/overlay', 'spi/topic/pxa2xx',
41d5a70 Merge remote-tracking branches 'spi/topic/lm70llp', 'spi/topic/loopback'
635b9b2 Merge remote-tracking branches 'spi/topic/dw', 'spi/topic/dw-mid', 'spi/
3c27892 Merge remote-tracking branches 'spi/topic/bcm63xx', 'spi/topic/butterfly
9b17e80 Merge remote-tracking branch 'spi/topic/sunxi' into spi-next
174c211 Merge remote-tracking branch 'spi/topic/core' into spi-next
f30f072 Merge remote-tracking branch 'spi/fix/mtk' into spi-linus
afd2ff9 Linux 4.4

Maybe someone has gotten any idea?

In the meantime I will try to reproduce on a 4.5-rc3 based branch.

Thanks,
	Martin

 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-10 15:12 ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-10 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

Hi!

I got a strange effect when using the auxiliar UART as the logging console:

The kernel is configured for drivers/tty/serial/8250/8250_bcm2835aux.c to
get linked into the main kernel (requires also 8250.c to be linked in).

As long as I boot using the main UART (PL011) in the kernel parameters:
  earlyprintk consoleblank=0 console=ttyAMA0 root=/dev/mmcblk0p2 rootwait

Everything works fine - early_printk works and normal logging works as well.
Both configured ttys (/dev/ttyAMA0 and /dev/ttyS0) in /etc/initttab
work fine.

Now when I switch the console to use the auxiliar UART
(which right now does not support early_printk) like this:
  earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait

I get the situation where:
* u-boot starts and writes to the main UART
* Kernel boots using early_printk logging to the main UART until we get to:
  [    2.657090] bootconsole [earlycon0] disabled
* then we see the earlyprintk buffer dumped on the aux-uart
* then booting continues and logging happens on the aux-uart:
  [    2.657090] bootconsole [earlycon0] disabled
  [    2.662673] 20201000.serial: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
  [    2.663793] console [ttyS0] disabled
  [    2.663899] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
  [    3.384166] console [ttyS0] enabled
* everything works fine and I get networking and everything up
  [....] Starting NTP server: ntpd. ok
  [....] Starting OpenBSD Secure Shell server: sshd. ok
  My IP address is 10.10.10.41 ::127.0.0.1

  Raspbian GNU/Linux 7 raspcm.intern.sperl.org ttyS0
* I can even log in via SSH and networking works.
  Note that I have the getty for ttyAMA0 disabled in inittab
* when I then enable the tty on the main UART like this:
  root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
* I get the following:
  [   72.962845] uart-pl011 20201000.serial: no DMA platform data
* the system freezes?
* sometimes I still get the following on the auxiliar UART before the system ?crashes":
  (at least when starting the tty from inittab)
  [   73.072985] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
  [   73.079132] ------------[ cut here ]------------
  [   73.083848] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
  [   73.093067] ---[ end trace 416e78cea88f5fb5 ]---
  [   73.097848] ------------[ cut here ]------------
  [   73.102616] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
  [   73.112219] ---[ end trace 416e78cea88f5fb6 ]---

A similar effect I get when using stty: 
  root at raspcm:~# stty -F /dev/ttyAMA0
  [  128.878127] uart-pl011 20201000.serial: no DMA platform data
  speed 9600 baud; line = 0;
  -brkint -imaxbel
Then the system freezes.

As if the plld does not lock - even if it (probably) is already locked.

Or the message actually comes from bcm2835_clock_wait_busy which also
produces this message - so maybe we should better identify the message.

Note that the same also applies if I remove earlyprintk with
the only difference being that I do not get the initial boot
messages on the main UART.
(at that time only: "Uncompressing Linux... done, booting the kernel.?
shows on the main uart after u-boot start..)

Unfortunately it is only possible to really test this on a
rpi-Compute module, as only there we can expose both uarts
on distinct GPIOs at the same time:
  GPIO 14: level=1 fsel=4 alt=0 func=TXD0
  GPIO 15: level=1 fsel=4 alt=0 func=RXD0
  GPIO 32: level=1 fsel=2 alt=5 func=TXD1
  GPIO 33: level=1 fsel=2 alt=5 func=RXD1

Here the commits I have used based on spi/for-next (based on 4.4.0)
with the patches (* prefix) for the driver for aux-uart
added separately as they are not in the tree I have used:
* a96f6c7 serial: bcm2835: add driver for bcm2835-aux-uart
* 464e876 clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
* 1af1b28 clk: bcm2835: Add a driver for the auxiliary peripheral clock gates.
ece19b8 Merge remote-tracking branch 'spi/fix/loopback' into spi-linus
887e6a3 Merge tag 'spi-v4.5' into spi-linus
ebea7c0 spi: fix counting in spi-loopback-test code
cabeea9 Merge remote-tracking branches 'spi/topic/sun4i', 'spi/topic/topcliff-pc
4f95307 Merge remote-tracking branches 'spi/topic/overlay', 'spi/topic/pxa2xx',
41d5a70 Merge remote-tracking branches 'spi/topic/lm70llp', 'spi/topic/loopback'
635b9b2 Merge remote-tracking branches 'spi/topic/dw', 'spi/topic/dw-mid', 'spi/
3c27892 Merge remote-tracking branches 'spi/topic/bcm63xx', 'spi/topic/butterfly
9b17e80 Merge remote-tracking branch 'spi/topic/sunxi' into spi-next
174c211 Merge remote-tracking branch 'spi/topic/core' into spi-next
f30f072 Merge remote-tracking branch 'spi/fix/mtk' into spi-linus
afd2ff9 Linux 4.4

Maybe someone has gotten any idea?

In the meantime I will try to reproduce on a 4.5-rc3 based branch.

Thanks,
	Martin

 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-10 15:12 ` Martin Sperl
@ 2016-02-10 17:21   ` Stefan Wahren
  -1 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-10 17:21 UTC (permalink / raw)
  To: Martin Sperl
  Cc: linux-rpi-kernel, linux-arm-kernel, linux-serial, linux-clk, Eric Anholt

Hi,

Am 10.02.2016 um 16:12 schrieb Martin Sperl:
> Hi!
>
> I got a strange effect when using the auxiliar UART as the logging console:
>
> The kernel is configured for drivers/tty/serial/8250/8250_bcm2835aux.c to
> get linked into the main kernel (requires also 8250.c to be linked in).
>
> As long as I boot using the main UART (PL011) in the kernel parameters:
>   earlyprintk consoleblank=0 console=ttyAMA0 root=/dev/mmcblk0p2 rootwait
>
> Everything works fine - early_printk works and normal logging works as well.
> Both configured ttys (/dev/ttyAMA0 and /dev/ttyS0) in /etc/initttab
> work fine.
>
> Now when I switch the console to use the auxiliar UART
> (which right now does not support early_printk) like this:
>   earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait
>
> I get the situation where:
> * u-boot starts and writes to the main UART
> * Kernel boots using early_printk logging to the main UART until we get to:
>   [    2.657090] bootconsole [earlycon0] disabled
> * then we see the earlyprintk buffer dumped on the aux-uart
> * then booting continues and logging happens on the aux-uart:
>   [    2.657090] bootconsole [earlycon0] disabled
>   [    2.662673] 20201000.serial: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
>   [    2.663793] console [ttyS0] disabled
>   [    2.663899] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
>   [    3.384166] console [ttyS0] enabled
> * everything works fine and I get networking and everything up
>   [....] Starting NTP server: ntpd. ok
>   [....] Starting OpenBSD Secure Shell server: sshd. ok
>   My IP address is 10.10.10.41 ::127.0.0.1
>
>   Raspbian GNU/Linux 7 raspcm.intern.sperl.org ttyS0
> * I can even log in via SSH and networking works.
>   Note that I have the getty for ttyAMA0 disabled in inittab
> * when I then enable the tty on the main UART like this:
>   root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
> * I get the following:
>   [   72.962845] uart-pl011 20201000.serial: no DMA platform data
> * the system freezes…
> * sometimes I still get the following on the auxiliar UART before the system “crashes":
>   (at least when starting the tty from inittab)
>   [   73.072985] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
>   [   73.079132] ------------[ cut here ]------------
>   [   73.083848] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
>   [   73.093067] ---[ end trace 416e78cea88f5fb5 ]---
>   [   73.097848] ------------[ cut here ]------------
>   [   73.102616] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
>   [   73.112219] ---[ end trace 416e78cea88f5fb6 ]---
>
> A similar effect I get when using stty: 
>   root@raspcm:~# stty -F /dev/ttyAMA0
>   [  128.878127] uart-pl011 20201000.serial: no DMA platform data
>   speed 9600 baud; line = 0;
>   -brkint -imaxbel
> Then the system freezes.
>
> As if the plld does not lock - even if it (probably) is already locked.
>
> Or the message actually comes from bcm2835_clock_wait_busy which also
> produces this message - so maybe we should better identify the message.
>
> Note that the same also applies if I remove earlyprintk with
> the only difference being that I do not get the initial boot
> messages on the main UART.
> (at that time only: "Uncompressing Linux... done, booting the kernel.”
> shows on the main uart after u-boot start..)
>
> Unfortunately it is only possible to really test this on a
> rpi-Compute module, as only there we can expose both uarts
> on distinct GPIOs at the same time:
>   GPIO 14: level=1 fsel=4 alt=0 func=TXD0
>   GPIO 15: level=1 fsel=4 alt=0 func=RXD0
>   GPIO 32: level=1 fsel=2 alt=5 func=TXD1
>   GPIO 33: level=1 fsel=2 alt=5 func=RXD1
>
> Here the commits I have used based on spi/for-next (based on 4.4.0)
> with the patches (* prefix) for the driver for aux-uart
> added separately as they are not in the tree I have used:
> * a96f6c7 serial: bcm2835: add driver for bcm2835-aux-uart
> * 464e876 clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
> * 1af1b28 clk: bcm2835: Add a driver for the auxiliary peripheral clock gates.
> ece19b8 Merge remote-tracking branch 'spi/fix/loopback' into spi-linus
> 887e6a3 Merge tag 'spi-v4.5' into spi-linus
> ebea7c0 spi: fix counting in spi-loopback-test code
> cabeea9 Merge remote-tracking branches 'spi/topic/sun4i', 'spi/topic/topcliff-pc
> 4f95307 Merge remote-tracking branches 'spi/topic/overlay', 'spi/topic/pxa2xx',
> 41d5a70 Merge remote-tracking branches 'spi/topic/lm70llp', 'spi/topic/loopback'
> 635b9b2 Merge remote-tracking branches 'spi/topic/dw', 'spi/topic/dw-mid', 'spi/
> 3c27892 Merge remote-tracking branches 'spi/topic/bcm63xx', 'spi/topic/butterfly
> 9b17e80 Merge remote-tracking branch 'spi/topic/sunxi' into spi-next
> 174c211 Merge remote-tracking branch 'spi/topic/core' into spi-next
> f30f072 Merge remote-tracking branch 'spi/fix/mtk' into spi-linus
> afd2ff9 Linux 4.4
>
> Maybe someone has gotten any idea?

this sounds like an issue i had with the compute module here:

http://permalink.gmane.org/gmane.linux.kernel.rpi/2197

Unfortunately i don't have the cm anymore.

Maybe you can try this downstream patch:

https://github.com/raspberrypi/linux/commit/09041f7ccbd1cde792edae91c41266886d5d1382

Regards



^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-10 17:21   ` Stefan Wahren
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-10 17:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Am 10.02.2016 um 16:12 schrieb Martin Sperl:
> Hi!
>
> I got a strange effect when using the auxiliar UART as the logging console:
>
> The kernel is configured for drivers/tty/serial/8250/8250_bcm2835aux.c to
> get linked into the main kernel (requires also 8250.c to be linked in).
>
> As long as I boot using the main UART (PL011) in the kernel parameters:
>   earlyprintk consoleblank=0 console=ttyAMA0 root=/dev/mmcblk0p2 rootwait
>
> Everything works fine - early_printk works and normal logging works as well.
> Both configured ttys (/dev/ttyAMA0 and /dev/ttyS0) in /etc/initttab
> work fine.
>
> Now when I switch the console to use the auxiliar UART
> (which right now does not support early_printk) like this:
>   earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait
>
> I get the situation where:
> * u-boot starts and writes to the main UART
> * Kernel boots using early_printk logging to the main UART until we get to:
>   [    2.657090] bootconsole [earlycon0] disabled
> * then we see the earlyprintk buffer dumped on the aux-uart
> * then booting continues and logging happens on the aux-uart:
>   [    2.657090] bootconsole [earlycon0] disabled
>   [    2.662673] 20201000.serial: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
>   [    2.663793] console [ttyS0] disabled
>   [    2.663899] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
>   [    3.384166] console [ttyS0] enabled
> * everything works fine and I get networking and everything up
>   [....] Starting NTP server: ntpd. ok
>   [....] Starting OpenBSD Secure Shell server: sshd. ok
>   My IP address is 10.10.10.41 ::127.0.0.1
>
>   Raspbian GNU/Linux 7 raspcm.intern.sperl.org ttyS0
> * I can even log in via SSH and networking works.
>   Note that I have the getty for ttyAMA0 disabled in inittab
> * when I then enable the tty on the main UART like this:
>   root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
> * I get the following:
>   [   72.962845] uart-pl011 20201000.serial: no DMA platform data
> * the system freezes?
> * sometimes I still get the following on the auxiliar UART before the system ?crashes":
>   (at least when starting the tty from inittab)
>   [   73.072985] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
>   [   73.079132] ------------[ cut here ]------------
>   [   73.083848] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
>   [   73.093067] ---[ end trace 416e78cea88f5fb5 ]---
>   [   73.097848] ------------[ cut here ]------------
>   [   73.102616] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
>   [   73.112219] ---[ end trace 416e78cea88f5fb6 ]---
>
> A similar effect I get when using stty: 
>   root at raspcm:~# stty -F /dev/ttyAMA0
>   [  128.878127] uart-pl011 20201000.serial: no DMA platform data
>   speed 9600 baud; line = 0;
>   -brkint -imaxbel
> Then the system freezes.
>
> As if the plld does not lock - even if it (probably) is already locked.
>
> Or the message actually comes from bcm2835_clock_wait_busy which also
> produces this message - so maybe we should better identify the message.
>
> Note that the same also applies if I remove earlyprintk with
> the only difference being that I do not get the initial boot
> messages on the main UART.
> (at that time only: "Uncompressing Linux... done, booting the kernel.?
> shows on the main uart after u-boot start..)
>
> Unfortunately it is only possible to really test this on a
> rpi-Compute module, as only there we can expose both uarts
> on distinct GPIOs at the same time:
>   GPIO 14: level=1 fsel=4 alt=0 func=TXD0
>   GPIO 15: level=1 fsel=4 alt=0 func=RXD0
>   GPIO 32: level=1 fsel=2 alt=5 func=TXD1
>   GPIO 33: level=1 fsel=2 alt=5 func=RXD1
>
> Here the commits I have used based on spi/for-next (based on 4.4.0)
> with the patches (* prefix) for the driver for aux-uart
> added separately as they are not in the tree I have used:
> * a96f6c7 serial: bcm2835: add driver for bcm2835-aux-uart
> * 464e876 clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
> * 1af1b28 clk: bcm2835: Add a driver for the auxiliary peripheral clock gates.
> ece19b8 Merge remote-tracking branch 'spi/fix/loopback' into spi-linus
> 887e6a3 Merge tag 'spi-v4.5' into spi-linus
> ebea7c0 spi: fix counting in spi-loopback-test code
> cabeea9 Merge remote-tracking branches 'spi/topic/sun4i', 'spi/topic/topcliff-pc
> 4f95307 Merge remote-tracking branches 'spi/topic/overlay', 'spi/topic/pxa2xx',
> 41d5a70 Merge remote-tracking branches 'spi/topic/lm70llp', 'spi/topic/loopback'
> 635b9b2 Merge remote-tracking branches 'spi/topic/dw', 'spi/topic/dw-mid', 'spi/
> 3c27892 Merge remote-tracking branches 'spi/topic/bcm63xx', 'spi/topic/butterfly
> 9b17e80 Merge remote-tracking branch 'spi/topic/sunxi' into spi-next
> 174c211 Merge remote-tracking branch 'spi/topic/core' into spi-next
> f30f072 Merge remote-tracking branch 'spi/fix/mtk' into spi-linus
> afd2ff9 Linux 4.4
>
> Maybe someone has gotten any idea?

this sounds like an issue i had with the compute module here:

http://permalink.gmane.org/gmane.linux.kernel.rpi/2197

Unfortunately i don't have the cm anymore.

Maybe you can try this downstream patch:

https://github.com/raspberrypi/linux/commit/09041f7ccbd1cde792edae91c41266886d5d1382

Regards

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-10 17:21   ` Stefan Wahren
@ 2016-02-10 17:42     ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-10 17:42 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: linux-rpi-kernel, linux-arm-kernel, linux-serial, linux-clk, Eric Anholt


> On 10.02.2016, at 18:21, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> Hi,
>> Maybe someone has gotten any idea?
> 
> this sounds like an issue i had with the compute module here:
> 
> http://permalink.gmane.org/gmane.linux.kernel.rpi/2197
> 
This one was fixed - if I remember correctly.

> Unfortunately i don't have the cm anymore.
> 
> Maybe you can try this downstream patch:
> 
> https://github.com/raspberrypi/linux/commit/09041f7ccbd1cde792edae91c41266886d5d1382
> 
I wonder if it is really interrupts that are the root cause
to mee it looks like a clock problem - especially as it actually
impacts the PL011 driver and not the aux driver…

And initializing the other way (pl011 is console, aux as tty)
works without any issues - so it HAS to do with accessing the
PL011 or the clock framework.

Martin


^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-10 17:42     ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-10 17:42 UTC (permalink / raw)
  To: linux-arm-kernel


> On 10.02.2016, at 18:21, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> Hi,
>> Maybe someone has gotten any idea?
> 
> this sounds like an issue i had with the compute module here:
> 
> http://permalink.gmane.org/gmane.linux.kernel.rpi/2197
> 
This one was fixed - if I remember correctly.

> Unfortunately i don't have the cm anymore.
> 
> Maybe you can try this downstream patch:
> 
> https://github.com/raspberrypi/linux/commit/09041f7ccbd1cde792edae91c41266886d5d1382
> 
I wonder if it is really interrupts that are the root cause
to mee it looks like a clock problem - especially as it actually
impacts the PL011 driver and not the aux driver?

And initializing the other way (pl011 is console, aux as tty)
works without any issues - so it HAS to do with accessing the
PL011 or the clock framework.

Martin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-10 15:12 ` Martin Sperl
@ 2016-02-11 13:15   ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-11 13:15 UTC (permalink / raw)
  To: linux-rpi-kernel, linux-arm-kernel, linux-serial, linux-clk; +Cc: Eric Anholt


On 10.02.2016 16:12, Martin Sperl wrote:
> Hi!
>
> I got a strange effect when using the auxiliar UART as the logging console:
>
> The kernel is configured for drivers/tty/serial/8250/8250_bcm2835aux.c to
> get linked into the main kernel (requires also 8250.c to be linked in).
>
> As long as I boot using the main UART (PL011) in the kernel parameters:
>    earlyprintk consoleblank=0 console=ttyAMA0 root=/dev/mmcblk0p2 rootwait
>
> Everything works fine - early_printk works and normal logging works as well.
> Both configured ttys (/dev/ttyAMA0 and /dev/ttyS0) in /etc/initttab
> work fine.
>
> Now when I switch the console to use the auxiliar UART
> (which right now does not support early_printk) like this:
>    earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait
>
> I get the situation where:
> * u-boot starts and writes to the main UART
> * Kernel boots using early_printk logging to the main UART until we get to:
>    [    2.657090] bootconsole [earlycon0] disabled
> * then we see the earlyprintk buffer dumped on the aux-uart
> * then booting continues and logging happens on the aux-uart:
>    [    2.657090] bootconsole [earlycon0] disabled
>    [    2.662673] 20201000.serial: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
>    [    2.663793] console [ttyS0] disabled
>    [    2.663899] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
>    [    3.384166] console [ttyS0] enabled
> * everything works fine and I get networking and everything up
>    [....] Starting NTP server: ntpd. ok
>    [....] Starting OpenBSD Secure Shell server: sshd. ok
>    My IP address is 10.10.10.41 ::127.0.0.1
>
>    Raspbian GNU/Linux 7 raspcm.intern.sperl.org ttyS0
> * I can even log in via SSH and networking works.
>    Note that I have the getty for ttyAMA0 disabled in inittab
> * when I then enable the tty on the main UART like this:
>    root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
> * I get the following:
>    [   72.962845] uart-pl011 20201000.serial: no DMA platform data
> * the system freezes…
> * sometimes I still get the following on the auxiliar UART before the system “crashes":
>    (at least when starting the tty from inittab)
>    [   73.072985] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
>    [   73.079132] ------------[ cut here ]------------
>    [   73.083848] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
>    [   73.093067] ---[ end trace 416e78cea88f5fb5 ]---
>    [   73.097848] ------------[ cut here ]------------
>    [   73.102616] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
>    [   73.112219] ---[ end trace 416e78cea88f5fb6 ]---
>
> A similar effect I get when using stty:
>    root@raspcm:~# stty -F /dev/ttyAMA0
>    [  128.878127] uart-pl011 20201000.serial: no DMA platform data
>    speed 9600 baud; line = 0;
>    -brkint -imaxbel
> Then the system freezes.
>
> As if the plld does not lock - even if it (probably) is already locked.
>
> Or the message actually comes from bcm2835_clock_wait_busy which also
> produces this message - so maybe we should better identify the message.
>
> Note that the same also applies if I remove earlyprintk with
> the only difference being that I do not get the initial boot
> messages on the main UART.
> (at that time only: "Uncompressing Linux... done, booting the kernel.”
> shows on the main uart after u-boot start..)
>
> Unfortunately it is only possible to really test this on a
> rpi-Compute module, as only there we can expose both uarts
> on distinct GPIOs at the same time:
>    GPIO 14: level=1 fsel=4 alt=0 func=TXD0
>    GPIO 15: level=1 fsel=4 alt=0 func=RXD0
>    GPIO 32: level=1 fsel=2 alt=5 func=TXD1
>    GPIO 33: level=1 fsel=2 alt=5 func=RXD1
>
> Here the commits I have used based on spi/for-next (based on 4.4.0)
> with the patches (* prefix) for the driver for aux-uart
> added separately as they are not in the tree I have used:
> * a96f6c7 serial: bcm2835: add driver for bcm2835-aux-uart
> * 464e876 clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
> * 1af1b28 clk: bcm2835: Add a driver for the auxiliary peripheral clock gates.
> ece19b8 Merge remote-tracking branch 'spi/fix/loopback' into spi-linus
> 887e6a3 Merge tag 'spi-v4.5' into spi-linus
> ebea7c0 spi: fix counting in spi-loopback-test code
> cabeea9 Merge remote-tracking branches 'spi/topic/sun4i', 'spi/topic/topcliff-pc
> 4f95307 Merge remote-tracking branches 'spi/topic/overlay', 'spi/topic/pxa2xx',
> 41d5a70 Merge remote-tracking branches 'spi/topic/lm70llp', 'spi/topic/loopback'
> 635b9b2 Merge remote-tracking branches 'spi/topic/dw', 'spi/topic/dw-mid', 'spi/
> 3c27892 Merge remote-tracking branches 'spi/topic/bcm63xx', 'spi/topic/butterfly
> 9b17e80 Merge remote-tracking branch 'spi/topic/sunxi' into spi-next
> 174c211 Merge remote-tracking branch 'spi/topic/core' into spi-next
> f30f072 Merge remote-tracking branch 'spi/fix/mtk' into spi-linus
> afd2ff9 Linux 4.4
>
> Maybe someone has gotten any idea?
>
> In the meantime I will try to reproduce on a 4.5-rc3 based branch.
Same issue with 4.5-rc3 - here the commits:
* bec2300c serial: bcm2835: add driver for bcm2835-aux-uart
388f7b1 Linux 4.5-rc3

Here the clock calls I have instrumented for the console=ttyAMA0 case:
[    1.884107] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plla
[    1.890723] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllb
[    1.897308] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllc
[    1.903849] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plld
[    1.910429] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllh
[    1.917184] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - timer
[    1.924143] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - timer
[    1.931237] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - otp
[    1.938081] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - otp
[    1.944757] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - tsens
[    1.951714] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - tsens
[    1.958605] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vpu
[    1.965429] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vpu
[    1.972088] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - v3d
[    1.978909] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - v3d
[    1.985622] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - isp
[    1.992405] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - isp
[    1.999101] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - h264
[    2.006011] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - h264
[    2.012774] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - sdram
[    2.019772] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - sdram
[    2.026848] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - uart
[    2.033725] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - uart
[    2.040521] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vec
[    2.047366] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vec
[    2.054207] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.061065] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - hsm
[    2.067720] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.074699] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - emmc
[    2.081574] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - emmc
[    2.088316] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.095253] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.102112] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.108951] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - pwm
[    2.115594] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.122374] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.129667] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.136523] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.143329] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.150148] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.157157] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.163942] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.230330] bcm2835-clk 20101000.cprman: bcm2835_pll_on - pllc
[    2.245552] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[    2.251559] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[    2.284246] bcm2835-clk 20101000.cprman: bcm2835_clock_on - emmc

And for the console=ttyS0,115200 case:
[    1.884106] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plla
[    1.890721] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllb
[    1.897307] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllc
[    1.903849] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plld
[    1.910429] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllh
[    1.917183] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - timer
[    1.924143] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - timer
[    1.931234] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - otp
[    1.938077] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - otp
[    1.944759] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - tsens
[    1.951717] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - tsens
[    1.958607] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vpu
[    1.965429] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vpu
[    1.972089] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - v3d
[    1.978908] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - v3d
[    1.985620] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - isp
[    1.992406] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - isp
[    1.999102] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - h264
[    2.006010] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - h264
[    2.012772] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - sdram
[    2.019769] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - sdram
[    2.026842] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - uart
[    2.033719] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - uart
[    2.040518] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vec
[    2.047362] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vec
[    2.054199] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.061052] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - hsm
[    2.067706] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.074688] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - emmc
[    2.081561] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - emmc
[    2.088303] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.095241] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.102100] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.108938] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - pwm
[    2.115582] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.122362] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.129648] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.136509] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.143315] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.150135] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.157145] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.163929] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.230333] bcm2835-clk 20101000.cprman: bcm2835_pll_on - pllc
[    2.275803] bcm2835-clk 20101000.cprman: bcm2835_clock_on - emmc

The only real difference is this diff:
@@ -41,6 +41,4 @@
   20101000.cprman: bcm2835_clock_get_parent - pwm
   20101000.cprman: bcm2835_clock_get_parent - hsm
   20101000.cprman: bcm2835_pll_on - pllc
- 20101000.cprman: bcm2835_pll_on - plld
- 20101000.cprman: bcm2835_clock_on - uart
   20101000.cprman: bcm2835_clock_on - emmc

so plld and uart are not started for the console=ttyS0 case.

After I instrumented clk-bcm2835.c to see what happens in the driver
and running: /sbin/getty -a root -L ttyAMA0 115200 vt100

I get the following:
[  146.342341] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  146.348426] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[  146.354639] uart-pl011 20201000.uart: no DMA platform data
[  146.375535] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[  146.381776] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[  146.388595] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[  146.396574] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  146.502551] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[  146.508726] ------------[ cut here ]------------
[  146.513448] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:680 
clk_core_disa)
[  146.522673] ---[ end trace 33268753126338e9 ]---
[  146.527478] ------------[ cut here ]------------
[  146.532224] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:575 
clk_core_unpr)
[  146.541866] ---[ end trace 33268753126338ea ]---

So I wonder why we would disable and reenable the plld and uart clock

Note that just adding a return in bcm2835_pll_off like this:
@@ -900,6 +905,9 @@ static void bcm2835_pll_off(struct clk_hw *hw)
      struct bcm2835_cprman *cprman = pll->cprman;
      const struct bcm2835_pll_data *data = pll->data;

+    dev_err(cprman->dev, "%s - %s\n", __FUNCTION__, clk_hw_get_name(hw));
+    return;
+
      cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
      cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
  }

avoids the crash, but the tty is non-functional.

Calling sequence of the clock driver is:
[  145.022546] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  145.028619] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[  145.034807] uart-pl011 20201000.uart: no DMA platform data
[  145.045089] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[  145.051418] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[  145.058215] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[  145.065779] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  145.071761] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart

debugfs shows the following after the above:
root@raspcm:~# head /sys/kernel/debug/clk/uart*/clk_rate
==> /sys/kernel/debug/clk/uart0_pclk/clk_rate <==
3000000

==> /sys/kernel/debug/clk/uart1_pclk/clk_rate <==
125000000

==> /sys/kernel/debug/clk/uart/clk_rate <==
2997598

Is this maybe related to the uart0_pclk, uart1_pclk, apb_pclk
clocks created and registered in bcm2835_init_clocks?

Any Ideas?

Thanks,
                 Martin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-11 13:15   ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-11 13:15 UTC (permalink / raw)
  To: linux-arm-kernel


On 10.02.2016 16:12, Martin Sperl wrote:
> Hi!
>
> I got a strange effect when using the auxiliar UART as the logging console:
>
> The kernel is configured for drivers/tty/serial/8250/8250_bcm2835aux.c to
> get linked into the main kernel (requires also 8250.c to be linked in).
>
> As long as I boot using the main UART (PL011) in the kernel parameters:
>    earlyprintk consoleblank=0 console=ttyAMA0 root=/dev/mmcblk0p2 rootwait
>
> Everything works fine - early_printk works and normal logging works as well.
> Both configured ttys (/dev/ttyAMA0 and /dev/ttyS0) in /etc/initttab
> work fine.
>
> Now when I switch the console to use the auxiliar UART
> (which right now does not support early_printk) like this:
>    earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait
>
> I get the situation where:
> * u-boot starts and writes to the main UART
> * Kernel boots using early_printk logging to the main UART until we get to:
>    [    2.657090] bootconsole [earlycon0] disabled
> * then we see the earlyprintk buffer dumped on the aux-uart
> * then booting continues and logging happens on the aux-uart:
>    [    2.657090] bootconsole [earlycon0] disabled
>    [    2.662673] 20201000.serial: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
>    [    2.663793] console [ttyS0] disabled
>    [    2.663899] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
>    [    3.384166] console [ttyS0] enabled
> * everything works fine and I get networking and everything up
>    [....] Starting NTP server: ntpd. ok
>    [....] Starting OpenBSD Secure Shell server: sshd. ok
>    My IP address is 10.10.10.41 ::127.0.0.1
>
>    Raspbian GNU/Linux 7 raspcm.intern.sperl.org ttyS0
> * I can even log in via SSH and networking works.
>    Note that I have the getty for ttyAMA0 disabled in inittab
> * when I then enable the tty on the main UART like this:
>    root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
> * I get the following:
>    [   72.962845] uart-pl011 20201000.serial: no DMA platform data
> * the system freezes?
> * sometimes I still get the following on the auxiliar UART before the system ?crashes":
>    (at least when starting the tty from inittab)
>    [   73.072985] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
>    [   73.079132] ------------[ cut here ]------------
>    [   73.083848] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
>    [   73.093067] ---[ end trace 416e78cea88f5fb5 ]---
>    [   73.097848] ------------[ cut here ]------------
>    [   73.102616] WARNING: CPU: 0 PID: 2329 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
>    [   73.112219] ---[ end trace 416e78cea88f5fb6 ]---
>
> A similar effect I get when using stty:
>    root at raspcm:~# stty -F /dev/ttyAMA0
>    [  128.878127] uart-pl011 20201000.serial: no DMA platform data
>    speed 9600 baud; line = 0;
>    -brkint -imaxbel
> Then the system freezes.
>
> As if the plld does not lock - even if it (probably) is already locked.
>
> Or the message actually comes from bcm2835_clock_wait_busy which also
> produces this message - so maybe we should better identify the message.
>
> Note that the same also applies if I remove earlyprintk with
> the only difference being that I do not get the initial boot
> messages on the main UART.
> (at that time only: "Uncompressing Linux... done, booting the kernel.?
> shows on the main uart after u-boot start..)
>
> Unfortunately it is only possible to really test this on a
> rpi-Compute module, as only there we can expose both uarts
> on distinct GPIOs at the same time:
>    GPIO 14: level=1 fsel=4 alt=0 func=TXD0
>    GPIO 15: level=1 fsel=4 alt=0 func=RXD0
>    GPIO 32: level=1 fsel=2 alt=5 func=TXD1
>    GPIO 33: level=1 fsel=2 alt=5 func=RXD1
>
> Here the commits I have used based on spi/for-next (based on 4.4.0)
> with the patches (* prefix) for the driver for aux-uart
> added separately as they are not in the tree I have used:
> * a96f6c7 serial: bcm2835: add driver for bcm2835-aux-uart
> * 464e876 clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
> * 1af1b28 clk: bcm2835: Add a driver for the auxiliary peripheral clock gates.
> ece19b8 Merge remote-tracking branch 'spi/fix/loopback' into spi-linus
> 887e6a3 Merge tag 'spi-v4.5' into spi-linus
> ebea7c0 spi: fix counting in spi-loopback-test code
> cabeea9 Merge remote-tracking branches 'spi/topic/sun4i', 'spi/topic/topcliff-pc
> 4f95307 Merge remote-tracking branches 'spi/topic/overlay', 'spi/topic/pxa2xx',
> 41d5a70 Merge remote-tracking branches 'spi/topic/lm70llp', 'spi/topic/loopback'
> 635b9b2 Merge remote-tracking branches 'spi/topic/dw', 'spi/topic/dw-mid', 'spi/
> 3c27892 Merge remote-tracking branches 'spi/topic/bcm63xx', 'spi/topic/butterfly
> 9b17e80 Merge remote-tracking branch 'spi/topic/sunxi' into spi-next
> 174c211 Merge remote-tracking branch 'spi/topic/core' into spi-next
> f30f072 Merge remote-tracking branch 'spi/fix/mtk' into spi-linus
> afd2ff9 Linux 4.4
>
> Maybe someone has gotten any idea?
>
> In the meantime I will try to reproduce on a 4.5-rc3 based branch.
Same issue with 4.5-rc3 - here the commits:
* bec2300c serial: bcm2835: add driver for bcm2835-aux-uart
388f7b1 Linux 4.5-rc3

Here the clock calls I have instrumented for the console=ttyAMA0 case:
[    1.884107] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plla
[    1.890723] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllb
[    1.897308] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllc
[    1.903849] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plld
[    1.910429] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllh
[    1.917184] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - timer
[    1.924143] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - timer
[    1.931237] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - otp
[    1.938081] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - otp
[    1.944757] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - tsens
[    1.951714] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - tsens
[    1.958605] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vpu
[    1.965429] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vpu
[    1.972088] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - v3d
[    1.978909] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - v3d
[    1.985622] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - isp
[    1.992405] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - isp
[    1.999101] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - h264
[    2.006011] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - h264
[    2.012774] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - sdram
[    2.019772] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - sdram
[    2.026848] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - uart
[    2.033725] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - uart
[    2.040521] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vec
[    2.047366] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vec
[    2.054207] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.061065] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - hsm
[    2.067720] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.074699] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - emmc
[    2.081574] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - emmc
[    2.088316] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.095253] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.102112] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.108951] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - pwm
[    2.115594] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.122374] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.129667] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.136523] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.143329] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.150148] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.157157] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.163942] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.230330] bcm2835-clk 20101000.cprman: bcm2835_pll_on - pllc
[    2.245552] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[    2.251559] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[    2.284246] bcm2835-clk 20101000.cprman: bcm2835_clock_on - emmc

And for the console=ttyS0,115200 case:
[    1.884106] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plla
[    1.890721] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllb
[    1.897307] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllc
[    1.903849] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plld
[    1.910429] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllh
[    1.917183] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - timer
[    1.924143] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - timer
[    1.931234] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - otp
[    1.938077] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - otp
[    1.944759] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - tsens
[    1.951717] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - tsens
[    1.958607] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vpu
[    1.965429] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vpu
[    1.972089] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - v3d
[    1.978908] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - v3d
[    1.985620] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - isp
[    1.992406] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - isp
[    1.999102] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - h264
[    2.006010] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - h264
[    2.012772] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - sdram
[    2.019769] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - sdram
[    2.026842] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - uart
[    2.033719] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - uart
[    2.040518] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vec
[    2.047362] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vec
[    2.054199] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.061052] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - hsm
[    2.067706] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.074688] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - emmc
[    2.081561] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - emmc
[    2.088303] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.095241] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.102100] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.108938] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - pwm
[    2.115582] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.122362] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.129648] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.136509] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.143315] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.150135] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.157145] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.163929] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.230333] bcm2835-clk 20101000.cprman: bcm2835_pll_on - pllc
[    2.275803] bcm2835-clk 20101000.cprman: bcm2835_clock_on - emmc

The only real difference is this diff:
@@ -41,6 +41,4 @@
   20101000.cprman: bcm2835_clock_get_parent - pwm
   20101000.cprman: bcm2835_clock_get_parent - hsm
   20101000.cprman: bcm2835_pll_on - pllc
- 20101000.cprman: bcm2835_pll_on - plld
- 20101000.cprman: bcm2835_clock_on - uart
   20101000.cprman: bcm2835_clock_on - emmc

so plld and uart are not started for the console=ttyS0 case.

After I instrumented clk-bcm2835.c to see what happens in the driver
and running: /sbin/getty -a root -L ttyAMA0 115200 vt100

I get the following:
[  146.342341] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  146.348426] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[  146.354639] uart-pl011 20201000.uart: no DMA platform data
[  146.375535] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[  146.381776] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[  146.388595] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[  146.396574] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  146.502551] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[  146.508726] ------------[ cut here ]------------
[  146.513448] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:680 
clk_core_disa)
[  146.522673] ---[ end trace 33268753126338e9 ]---
[  146.527478] ------------[ cut here ]------------
[  146.532224] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:575 
clk_core_unpr)
[  146.541866] ---[ end trace 33268753126338ea ]---

So I wonder why we would disable and reenable the plld and uart clock

Note that just adding a return in bcm2835_pll_off like this:
@@ -900,6 +905,9 @@ static void bcm2835_pll_off(struct clk_hw *hw)
      struct bcm2835_cprman *cprman = pll->cprman;
      const struct bcm2835_pll_data *data = pll->data;

+    dev_err(cprman->dev, "%s - %s\n", __FUNCTION__, clk_hw_get_name(hw));
+    return;
+
      cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
      cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
  }

avoids the crash, but the tty is non-functional.

Calling sequence of the clock driver is:
[  145.022546] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  145.028619] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[  145.034807] uart-pl011 20201000.uart: no DMA platform data
[  145.045089] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[  145.051418] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[  145.058215] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[  145.065779] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[  145.071761] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart

debugfs shows the following after the above:
root at raspcm:~# head /sys/kernel/debug/clk/uart*/clk_rate
==> /sys/kernel/debug/clk/uart0_pclk/clk_rate <==
3000000

==> /sys/kernel/debug/clk/uart1_pclk/clk_rate <==
125000000

==> /sys/kernel/debug/clk/uart/clk_rate <==
2997598

Is this maybe related to the uart0_pclk, uart1_pclk, apb_pclk
clocks created and registered in bcm2835_init_clocks?

Any Ideas?

Thanks,
                 Martin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-11 13:15   ` Martin Sperl
@ 2016-02-11 17:55     ` Stefan Wahren
  -1 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-11 17:55 UTC (permalink / raw)
  To: Martin Sperl, Eric Anholt
  Cc: linux-arm-kernel, linux-serial, linux-clk, linux-rpi-kernel

Hi,

> Martin Sperl <martin@sperl.org> hat am 11. Februar 2016 um 14:15 geschrieben:
>
> The only real difference is this diff:
> @@ -41,6 +41,4 @@
> 20101000.cprman: bcm2835_clock_get_parent - pwm
> 20101000.cprman: bcm2835_clock_get_parent - hsm
> 20101000.cprman: bcm2835_pll_on - pllc
> - 20101000.cprman: bcm2835_pll_on - plld
> - 20101000.cprman: bcm2835_clock_on - uart
> 20101000.cprman: bcm2835_clock_on - emmc
>
> so plld and uart are not started for the console=ttyS0 case.
>
> After I instrumented clk-bcm2835.c to see what happens in the driver
> and running: /sbin/getty -a root -L ttyAMA0 115200 vt100
>
> I get the following:
> [ 146.342341] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 146.348426] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
> [ 146.354639] uart-pl011 20201000.uart: no DMA platform data
> [ 146.375535] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
> [ 146.381776] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
> [ 146.388595] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
> [ 146.396574] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 146.502551] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
> [ 146.508726] ------------[ cut here ]------------
> [ 146.513448] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:680
> clk_core_disa)
> [ 146.522673] ---[ end trace 33268753126338e9 ]---
> [ 146.527478] ------------[ cut here ]------------
> [ 146.532224] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:575
> clk_core_unpr)
> [ 146.541866] ---[ end trace 33268753126338ea ]---
>
> So I wonder why we would disable and reenable the plld and uart clock

I think the reason for this behavior can be found in the uart-pl011 driver which
plays with the clocks.

According to the clock tree "plld" seems to be critical and shouldn't be
disabled?

>
> Note that just adding a return in bcm2835_pll_off like this:
> @@ -900,6 +905,9 @@ static void bcm2835_pll_off(struct clk_hw *hw)
> struct bcm2835_cprman *cprman = pll->cprman;
> const struct bcm2835_pll_data *data = pll->data;
>
> + dev_err(cprman->dev, "%s - %s\n", __FUNCTION__, clk_hw_get_name(hw));
> + return;
> +
> cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
> cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
> }
>
> avoids the crash, but the tty is non-functional.
>
> Calling sequence of the clock driver is:
> [ 145.022546] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 145.028619] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
> [ 145.034807] uart-pl011 20201000.uart: no DMA platform data
> [ 145.045089] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
> [ 145.051418] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
> [ 145.058215] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
> [ 145.065779] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 145.071761] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
>
> debugfs shows the following after the above:
> root@raspcm:~# head /sys/kernel/debug/clk/uart*/clk_rate
> ==> /sys/kernel/debug/clk/uart0_pclk/clk_rate <==
> 3000000
>
> ==> /sys/kernel/debug/clk/uart1_pclk/clk_rate <==
> 125000000
>
> ==> /sys/kernel/debug/clk/uart/clk_rate <==
> 2997598
>
> Is this maybe related to the uart0_pclk, uart1_pclk, apb_pclk
> clocks created and registered in bcm2835_init_clocks?

Related or not i think we should get the rid off them.

>
> Any Ideas?
>
> Thanks,
> Martin
>
> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-11 17:55     ` Stefan Wahren
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-11 17:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

> Martin Sperl <martin@sperl.org> hat am 11. Februar 2016 um 14:15 geschrieben:
>
> The only real difference is this diff:
> @@ -41,6 +41,4 @@
> 20101000.cprman: bcm2835_clock_get_parent - pwm
> 20101000.cprman: bcm2835_clock_get_parent - hsm
> 20101000.cprman: bcm2835_pll_on - pllc
> - 20101000.cprman: bcm2835_pll_on - plld
> - 20101000.cprman: bcm2835_clock_on - uart
> 20101000.cprman: bcm2835_clock_on - emmc
>
> so plld and uart are not started for the console=ttyS0 case.
>
> After I instrumented clk-bcm2835.c to see what happens in the driver
> and running: /sbin/getty -a root -L ttyAMA0 115200 vt100
>
> I get the following:
> [ 146.342341] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 146.348426] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
> [ 146.354639] uart-pl011 20201000.uart: no DMA platform data
> [ 146.375535] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
> [ 146.381776] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
> [ 146.388595] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
> [ 146.396574] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 146.502551] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
> [ 146.508726] ------------[ cut here ]------------
> [ 146.513448] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:680
> clk_core_disa)
> [ 146.522673] ---[ end trace 33268753126338e9 ]---
> [ 146.527478] ------------[ cut here ]------------
> [ 146.532224] WARNING: CPU: 0 PID: 2349 at drivers/clk/clk.c:575
> clk_core_unpr)
> [ 146.541866] ---[ end trace 33268753126338ea ]---
>
> So I wonder why we would disable and reenable the plld and uart clock

I think the reason for this behavior can be found in the uart-pl011 driver which
plays with the clocks.

According to the clock tree "plld" seems to be critical and shouldn't be
disabled?

>
> Note that just adding a return in bcm2835_pll_off like this:
> @@ -900,6 +905,9 @@ static void bcm2835_pll_off(struct clk_hw *hw)
> struct bcm2835_cprman *cprman = pll->cprman;
> const struct bcm2835_pll_data *data = pll->data;
>
> + dev_err(cprman->dev, "%s - %s\n", __FUNCTION__, clk_hw_get_name(hw));
> + return;
> +
> cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
> cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
> }
>
> avoids the crash, but the tty is non-functional.
>
> Calling sequence of the clock driver is:
> [ 145.022546] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 145.028619] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
> [ 145.034807] uart-pl011 20201000.uart: no DMA platform data
> [ 145.045089] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
> [ 145.051418] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
> [ 145.058215] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
> [ 145.065779] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
> [ 145.071761] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
>
> debugfs shows the following after the above:
> root at raspcm:~# head /sys/kernel/debug/clk/uart*/clk_rate
> ==> /sys/kernel/debug/clk/uart0_pclk/clk_rate <==
> 3000000
>
> ==> /sys/kernel/debug/clk/uart1_pclk/clk_rate <==
> 125000000
>
> ==> /sys/kernel/debug/clk/uart/clk_rate <==
> 2997598
>
> Is this maybe related to the uart0_pclk, uart1_pclk, apb_pclk
> clocks created and registered in bcm2835_init_clocks?

Related or not i think we should get the rid off them.

>
> Any Ideas?
>
> Thanks,
> Martin
>
> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-11 17:55     ` Stefan Wahren
@ 2016-02-12 11:56       ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-12 11:56 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Eric Anholt, linux-arm-kernel, linux-serial, linux-clk, linux-rpi-kernel


> On 11.02.2016, at 18:55, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> I think the reason for this behavior can be found in the uart-pl011 driver which
> plays with the clocks.
> 
I will look into the why and share what I find...

> According to the clock tree "plld" seems to be critical and shouldn't be
> disabled?

Then we may need to avoid this - but as far as I can tell
the amba-pl011 driver only disables the clock in error cases
(or when shutting down).
Maybe we just need to disable the shutdown of the plld?

>> Is this maybe related to the uart0_pclk, uart1_pclk, apb_pclk
>> clocks created and registered in bcm2835_init_clocks?
> 
> Related or not i think we should get the rid off them.

A quick test shows that an empty bcm2835_init_clocks
does not change the basic kernel-boot behavior and everything
seems to work.

So we probably should remove it in:
* arch/arm/mach-bcm/board_bcm2835.c (call)
* drivers/clk/bcm/clk-bcm2835.c (function definition)

Martin


^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-12 11:56       ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-12 11:56 UTC (permalink / raw)
  To: linux-arm-kernel


> On 11.02.2016, at 18:55, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> I think the reason for this behavior can be found in the uart-pl011 driver which
> plays with the clocks.
> 
I will look into the why and share what I find...

> According to the clock tree "plld" seems to be critical and shouldn't be
> disabled?

Then we may need to avoid this - but as far as I can tell
the amba-pl011 driver only disables the clock in error cases
(or when shutting down).
Maybe we just need to disable the shutdown of the plld?

>> Is this maybe related to the uart0_pclk, uart1_pclk, apb_pclk
>> clocks created and registered in bcm2835_init_clocks?
> 
> Related or not i think we should get the rid off them.

A quick test shows that an empty bcm2835_init_clocks
does not change the basic kernel-boot behavior and everything
seems to work.

So we probably should remove it in:
* arch/arm/mach-bcm/board_bcm2835.c (call)
* drivers/clk/bcm/clk-bcm2835.c (function definition)

Martin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-12 11:56       ` Martin Sperl
@ 2016-02-12 17:34         ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-12 17:34 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Eric Anholt, linux-arm-kernel, linux-serial, linux-clk, linux-rpi-kernel


> On 12.02.2016, at 12:56, Martin Sperl <martin@sperl.org> wrote:
> 
> 
>> On 11.02.2016, at 18:55, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> I think the reason for this behavior can be found in the uart-pl011 driver which
>> plays with the clocks.
>> 
> I will look into the why and share what I find…

Well - just disabling CONFIG_SERIAL_AMBA_PL011_CONSOLE
in config still results in the early-boot getting
dumped to ttyAMA0 and the same behaviour
when starting getty on that tty: crash…

Similarly when compiled amba-pl011 as a module:
* strangely the boot messages still goes to ttyAMA
  - this may be something that uboot sets up?
  Maybe: earlycon-arm-semihost using SWI?
* loading the module works fine:
  [   65.458754] Serial: AMBA PL011 UART driver
  [   65.547204] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
* using ttyAMA0 (via getty) crashes the system.
  [   73.560708] uart-pl011 20201000.uart: no DMA platform data
  [   73.667150] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
  [   73.673305] ------------[ cut here ]------------
  [   73.678039] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
  [   73.687320] ---[ end trace e38a11a59bfd67ea ]---
  [   73.692196] ------------[ cut here ]------------
  [   73.696973] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
  [   73.706709] ---[ end trace e38a11a59bfd67eb ]---


Now having instrumented clock and amba-pl011 with debug prints I get:

Boot sequence (logged to ttyAMA0 without a driver / earlycon0):

Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.5.0-rc3+ (root@raspcm.intern.sperl.org) (gcc version 4.7.2 (Debian 4.7.2-5+rpi1) ) #91 Fri Feb 12 13:12:31 UTC 2016
[    0.000000] CPU: ARMv6-compatible processor [410fb767] revision 7 (ARMv7), cr=00c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
[    0.000000] Machine model: Raspberry Pi Model B+
[    0.000000] bootconsole [earlycon0] enabled
[    0.000000] Memory policy: Data cache writeback
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 113792
[    0.000000] Kernel command line: earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait
[    0.000000] PID hash table entries: 2048 (order: 1, 8192 bytes)
[    0.000000] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
[    0.000000] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Memory: 445500K/458752K available (5768K kernel code, 418K rwdata, 1840K rodata, 420K init, 697K bss, 13252K reserved, 0K cma-reserved)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
[    0.000000]     vmalloc : 0xdc800000 - 0xff800000   ( 560 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xdc000000   ( 448 MB)
[    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
[    0.000000]       .text : 0xc0008000 - 0xc0776394   (7609 kB)
[    0.000000]       .init : 0xc0777000 - 0xc07e0000   ( 420 kB)
[    0.000000]       .data : 0xc07e0000 - 0xc0848960   ( 419 kB)
[    0.000000]        .bss : 0xc0848960 - 0xc08f6f68   ( 698 kB)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000030] sched_clock: 32 bits at 1000kHz, resolution 1000ns, wraps every 2147483647500ns
[    0.008591] clocksource: timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
[    0.018176] bcm2835: system timer (irq = 27)
[    0.023023] Console: colour dummy device 80x30
[    0.027636] Calibrating delay loop... 697.95 BogoMIPS (lpj=3489792)
[    0.094228] pid_max: default: 32768 minimum: 301
[    0.099359] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.106197] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.114578] CPU: Testing write buffer coherency: ok
[    0.119730] ftrace: allocating 19776 entries in 58 pages
[    0.247172] Setting up static identity map for 0x8220 - 0x8258
[    0.257687] devtmpfs: initialized
[    0.267447] VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 5
[    0.275647] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.286277] pinctrl core: initialized pinctrl subsystem
[    0.293168] NET: Registered protocol family 16
[    0.298438] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    0.313885] No ATAGs?
[    0.316051] hw-breakpoint: found 6 breakpoint and 1 watchpoint registers.
[    0.323345] hw-breakpoint: maximum watchpoint size is 4 bytes.
…
[    0.366218] clocksource: Switched to clocksource timer
[    0.424382] simple-framebuffer 5e887000.framebuffer: framebuffer at 0x5e887000, 0x36c600 bytes, mapped to 0xdcc00000
[    0.435245] simple-framebuffer 5e887000.framebuffer: format=r5g6b5, mode=1824x984x16, linelength=3648
[    0.482137] Console: switching to colour frame buffer device 228x61
[    0.524351] simple-framebuffer 5e887000.framebuffer: fb0: simplefb registered
!
…
[    0.702577] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.711152] bcm2835-aux-uart 20215040.serial: could not get clk: -517
…
[    1.962031] usbhid: USB HID core driver
[    1.966329] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plla
[    1.972879] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllb
[    1.979474] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllc
[    1.986019] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plld
[    1.992608] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllh
[    1.999375] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - timer
[    2.006434] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - timer
[    2.013468] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - otp
[    2.020318] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - otp
[    2.027000] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - tsens
[    2.033959] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - tsens
[    2.040831] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vpu
[    2.047679] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vpu
[    2.054431] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - v3d
[    2.061271] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - v3d
[    2.067966] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - isp
[    2.074749] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - isp
[    2.081445] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - h264
[    2.088358] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - h264
[    2.095120] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - sdram
[    2.102122] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - sdram
[    2.109230] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - uart
[    2.116108] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - uart
[    2.122907] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vec
[    2.129734] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vec
[    2.136449] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.143234] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - hsm
[    2.149939] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - emmc
[    2.156853] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - emmc
[    2.163615] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.170448] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - pwm
[    2.177114] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.184451] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.191391] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.198247] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
…
[    2.259033] bcm2835-clk 20101000.cprman: bcm2835_pll_on - pllc
[    2.265485] console [ttyS0] disabled
[    2.269377] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 3122499
9) is a 16550
[    2.277931] console [ttyS0] enabled
[    2.285083] bootconsole [earlycon0] disabled

Boot sequence after earlycon0 to ttyS0:
[    2.277931] console [ttyS0] enabled
[    2.285083] bootconsole [earlycon0] disabled
[    2.294646] bcm2835-clk 20101000.cprman: bcm2835_clock_on - emmc
[    2.306321] usb 1-1: new high-speed USB device number 2 using dwc2
[    2.346289] mmc0: SDHCI controller on 20300000.sdhci [20300000.sdhci] using PIO
[    2.361549] Waiting for root device /dev/mmcblk0p2...
[    2.409495] mmc0: MAN_BKOPS_EN bit is not set
[    2.415026] mmc0: new MMC card at address 0001
[    2.420585] mmcblk0: mmc0:0001 4FEACB 3.64 GiB
[    2.425646] mmcblk0boot0: mmc0:0001 4FEACB partition 1 4.00 MiB
[    2.432466] mmcblk0boot1: mmc0:0001 4FEACB partition 2 4.00 MiB
[    2.439214] mmcblk0rpmb: mmc0:0001 4FEACB partition 3 512 KiB
[    2.447959]  mmcblk0: p1 p2
[    2.477939] EXT4-fs (mmcblk0p2): couldn't mount as ext3 due to feature incompatibilities
…
getty - Serial open on ttyS0, network up

Loading module:
root@raspcm:~# modprobe amba-pl011
[   59.238155] Serial: AMBA PL011 UART driver
[   59.245951] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud =2
root@raspcm:~#

And when starting getty ttyAMA0:
root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   67.998916] pl011_startup - start
[   68.002326] pl011_hwinit - prepare-enable
[   68.006434] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[   68.012524] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   68.018733] pl011_hwinit - prepare-enable - 0
[   68.024790] uart-pl011 20201000.uart: no DMA platform data
[   68.030693] pl011_startup - exit
[   68.038628] pl011_shutdown - start
[   68.042152] pl011_shutdown - disable_unprepare
[   68.046715] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[   68.053040] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[   68.059818] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[   68.065856] pl011_shutdown - exit

Note that I have now a hdmi display connected and the display
stops showing the framebuffer but the display goes off.

So it seems as if pl011_shutdown gets called for some reason,
which turns off the uart clock and as a consequence also the
plld gets shut down (probably ref-counting.

I wonder why we have the VPU as a secondary clock in the dt - 
amba-pll11 does not reference it anywhere.

When adding a return to bcm2835_pll_off it looks like this:
root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   73.862152] pl011_startup - start
[   73.865558] pl011_hwinit - prepare-enable
[   73.869886] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[   73.875854] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   73.882044] pl011_hwinit - prepare-enable - ret = 0
[   73.888126] uart-pl011 20201000.uart: no DMA platform data
[   73.893757] pl011_startup - exit
[   73.901495] pl011_shutdown - start
[   73.905034] pl011_shutdown - disable_unprepare
[   73.909742] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[   73.915967] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[   73.922775] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[   73.928844] pl011_shutdown - exit
[   73.934152] pl011_startup - start
[   73.937617] pl011_hwinit - prepare-enable
[   73.941726] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[   73.947738] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   73.953865] pl011_hwinit - prepare-enable - ret = 0
[   73.959776] pl011_startup - exit

The ttyAMA0 console does not work, hdmi display blinks,
but the network works...

When removing the pll_on/off from the pll_ops I have a similar effect
but in this case PLL is not touched at all:
root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   83.345055] pl011_startup - start
[   83.348580] pl011_hwinit - prepare-enable
[   83.352725] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   83.358910] pl011_hwinit - prepare-enable - 0
[   83.365083] uart-pl011 20201000.uart: no DMA platform data
[   83.370790] pl011_startup - exit
[   83.378653] pl011_shutdown - start
[   83.382176] pl011_shutdown - disable_unprepare
[   83.386873] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[   83.393097] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[   83.399888] pl011_shutdown - exit
[   83.405164] pl011_startup - start
[   83.408652] pl011_hwinit - prepare-enable
[   83.412771] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   83.418952] pl011_hwinit - prepare-enable - 0
[   83.423665] pl011_startup - exit

The system continues, but the UART still is not working and the
HDMI-Display is flashing only (but with a certain pattern on the
display: totally blue, then some wait, then a black horizontal bar
finally a full dark screen flash - that happens in a 3-4 second
cycle.

As if there is something else that requires the UART clock running
- maybe in the firmware?

Even making the uart clock a VPU clock does not help:
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..4856657 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -758,6 +761,7 @@ static const struct bcm2835_clock_data bcm2835_clock_uart_da
        .div_reg = CM_UARTDIV,
        .int_bits = 10,
        .frac_bits = 12,
+       .is_vpu_clock = true,
 };

 /* HDMI state machine */

This produces the following debug messages:
[   56.409818] pl011_startup - start
[   56.413225] pl011_hwinit - prepare-enable
[   56.417443] pl011_hwinit - prepare-enable - ret = 0
[   56.423297] uart-pl011 20201000.uart: no DMA platform data
[   56.428989] pl011_startup - exit
[   56.435485] pl011_shutdown - start
[   56.440337] pl011_shutdown - disable_unprepare
[   56.444913] pl011_shutdown - exit
[   56.449762] pl011_startup - start
[   56.453165] pl011_hwinit - prepare-enable
[   56.457371] pl011_hwinit - prepare-enable - ret = 0
[   56.462175] pl011_startup - exit

The screen is still blinking and UART does not work.
Maybe the driver is not able to handle the “remapping”
of registers to a different range and is touching ram
used by the FW?

The only solution that I found is using fixed clocks
in the device-tree (which is not what we intended:
/ {
                clk_uart0: clock@3 {
                        compatible = "fixed-clock";
                        reg = <3>;
                        #clock-cells = <0>;
                        clock-output-names = "uart0_pclk";
                        clock-frequency = <3000000>;
                };

                clk_apb_p: clock@4 {
                        compatible = "fixed-clock";
                        reg = <4>;
                        #clock-cells = <0>;
                        clock-output-names = "apb_pclk";
                        clock-frequency = <126000000>;
                };
};
&uart0 {
        clocks = <&clk_uart0 &clk_apb_p>;
};

(so I have ruled out that the amba-pl011 writes to
the wrong addresses).

So as far as I can tell it is only clock related
and when the new clock-framework is used it fails...

Martin


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-12 17:34         ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-12 17:34 UTC (permalink / raw)
  To: linux-arm-kernel


> On 12.02.2016, at 12:56, Martin Sperl <martin@sperl.org> wrote:
> 
> 
>> On 11.02.2016, at 18:55, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> I think the reason for this behavior can be found in the uart-pl011 driver which
>> plays with the clocks.
>> 
> I will look into the why and share what I find?

Well - just disabling CONFIG_SERIAL_AMBA_PL011_CONSOLE
in config still results in the early-boot getting
dumped to ttyAMA0 and the same behaviour
when starting getty on that tty: crash?

Similarly when compiled amba-pl011 as a module:
* strangely the boot messages still goes to ttyAMA
  - this may be something that uboot sets up?
  Maybe: earlycon-arm-semihost using SWI?
* loading the module works fine:
  [   65.458754] Serial: AMBA PL011 UART driver
  [   65.547204] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
* using ttyAMA0 (via getty) crashes the system.
  [   73.560708] uart-pl011 20201000.uart: no DMA platform data
  [   73.667150] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
  [   73.673305] ------------[ cut here ]------------
  [   73.678039] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
  [   73.687320] ---[ end trace e38a11a59bfd67ea ]---
  [   73.692196] ------------[ cut here ]------------
  [   73.696973] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
  [   73.706709] ---[ end trace e38a11a59bfd67eb ]---


Now having instrumented clock and amba-pl011 with debug prints I get:

Boot sequence (logged to ttyAMA0 without a driver / earlycon0):

Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.5.0-rc3+ (root at raspcm.intern.sperl.org) (gcc version 4.7.2 (Debian 4.7.2-5+rpi1) ) #91 Fri Feb 12 13:12:31 UTC 2016
[    0.000000] CPU: ARMv6-compatible processor [410fb767] revision 7 (ARMv7), cr=00c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
[    0.000000] Machine model: Raspberry Pi Model B+
[    0.000000] bootconsole [earlycon0] enabled
[    0.000000] Memory policy: Data cache writeback
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 113792
[    0.000000] Kernel command line: earlyprintk consoleblank=0 console=ttyS0,115200n1 root=/dev/mmcblk0p2 rootwait
[    0.000000] PID hash table entries: 2048 (order: 1, 8192 bytes)
[    0.000000] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
[    0.000000] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Memory: 445500K/458752K available (5768K kernel code, 418K rwdata, 1840K rodata, 420K init, 697K bss, 13252K reserved, 0K cma-reserved)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
[    0.000000]     vmalloc : 0xdc800000 - 0xff800000   ( 560 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xdc000000   ( 448 MB)
[    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
[    0.000000]       .text : 0xc0008000 - 0xc0776394   (7609 kB)
[    0.000000]       .init : 0xc0777000 - 0xc07e0000   ( 420 kB)
[    0.000000]       .data : 0xc07e0000 - 0xc0848960   ( 419 kB)
[    0.000000]        .bss : 0xc0848960 - 0xc08f6f68   ( 698 kB)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000030] sched_clock: 32 bits at 1000kHz, resolution 1000ns, wraps every 2147483647500ns
[    0.008591] clocksource: timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
[    0.018176] bcm2835: system timer (irq = 27)
[    0.023023] Console: colour dummy device 80x30
[    0.027636] Calibrating delay loop... 697.95 BogoMIPS (lpj=3489792)
[    0.094228] pid_max: default: 32768 minimum: 301
[    0.099359] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.106197] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.114578] CPU: Testing write buffer coherency: ok
[    0.119730] ftrace: allocating 19776 entries in 58 pages
[    0.247172] Setting up static identity map for 0x8220 - 0x8258
[    0.257687] devtmpfs: initialized
[    0.267447] VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 5
[    0.275647] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.286277] pinctrl core: initialized pinctrl subsystem
[    0.293168] NET: Registered protocol family 16
[    0.298438] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    0.313885] No ATAGs?
[    0.316051] hw-breakpoint: found 6 breakpoint and 1 watchpoint registers.
[    0.323345] hw-breakpoint: maximum watchpoint size is 4 bytes.
?
[    0.366218] clocksource: Switched to clocksource timer
[    0.424382] simple-framebuffer 5e887000.framebuffer: framebuffer at 0x5e887000, 0x36c600 bytes, mapped to 0xdcc00000
[    0.435245] simple-framebuffer 5e887000.framebuffer: format=r5g6b5, mode=1824x984x16, linelength=3648
[    0.482137] Console: switching to colour frame buffer device 228x61
[    0.524351] simple-framebuffer 5e887000.framebuffer: fb0: simplefb registered
!
?
[    0.702577] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.711152] bcm2835-aux-uart 20215040.serial: could not get clk: -517
?
[    1.962031] usbhid: USB HID core driver
[    1.966329] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plla
[    1.972879] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllb
[    1.979474] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllc
[    1.986019] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - plld
[    1.992608] bcm2835-clk 20101000.cprman: bcm2835_pll_get_rate - pllh
[    1.999375] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - timer
[    2.006434] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - timer
[    2.013468] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - otp
[    2.020318] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - otp
[    2.027000] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - tsens
[    2.033959] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - tsens
[    2.040831] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vpu
[    2.047679] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vpu
[    2.054431] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - v3d
[    2.061271] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - v3d
[    2.067966] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - isp
[    2.074749] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - isp
[    2.081445] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - h264
[    2.088358] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - h264
[    2.095120] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - sdram
[    2.102122] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - sdram
[    2.109230] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - uart
[    2.116108] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - uart
[    2.122907] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - vec
[    2.129734] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - vec
[    2.136449] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - hsm
[    2.143234] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - hsm
[    2.149939] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - emmc
[    2.156853] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - emmc
[    2.163615] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.170448] bcm2835-clk 20101000.cprman: bcm2835_clock_get_rate - pwm
[    2.177114] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.184451] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.191391] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
[    2.198247] bcm2835-clk 20101000.cprman: bcm2835_clock_get_parent - pwm
?
[    2.259033] bcm2835-clk 20101000.cprman: bcm2835_pll_on - pllc
[    2.265485] console [ttyS0] disabled
[    2.269377] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 3122499
9) is a 16550
[    2.277931] console [ttyS0] enabled
[    2.285083] bootconsole [earlycon0] disabled

Boot sequence after earlycon0 to ttyS0:
[    2.277931] console [ttyS0] enabled
[    2.285083] bootconsole [earlycon0] disabled
[    2.294646] bcm2835-clk 20101000.cprman: bcm2835_clock_on - emmc
[    2.306321] usb 1-1: new high-speed USB device number 2 using dwc2
[    2.346289] mmc0: SDHCI controller on 20300000.sdhci [20300000.sdhci] using PIO
[    2.361549] Waiting for root device /dev/mmcblk0p2...
[    2.409495] mmc0: MAN_BKOPS_EN bit is not set
[    2.415026] mmc0: new MMC card at address 0001
[    2.420585] mmcblk0: mmc0:0001 4FEACB 3.64 GiB
[    2.425646] mmcblk0boot0: mmc0:0001 4FEACB partition 1 4.00 MiB
[    2.432466] mmcblk0boot1: mmc0:0001 4FEACB partition 2 4.00 MiB
[    2.439214] mmcblk0rpmb: mmc0:0001 4FEACB partition 3 512 KiB
[    2.447959]  mmcblk0: p1 p2
[    2.477939] EXT4-fs (mmcblk0p2): couldn't mount as ext3 due to feature incompatibilities
?
getty - Serial open on ttyS0, network up

Loading module:
root at raspcm:~# modprobe amba-pl011
[   59.238155] Serial: AMBA PL011 UART driver
[   59.245951] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud =2
root at raspcm:~#

And when starting getty ttyAMA0:
root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   67.998916] pl011_startup - start
[   68.002326] pl011_hwinit - prepare-enable
[   68.006434] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[   68.012524] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   68.018733] pl011_hwinit - prepare-enable - 0
[   68.024790] uart-pl011 20201000.uart: no DMA platform data
[   68.030693] pl011_startup - exit
[   68.038628] pl011_shutdown - start
[   68.042152] pl011_shutdown - disable_unprepare
[   68.046715] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[   68.053040] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[   68.059818] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[   68.065856] pl011_shutdown - exit

Note that I have now a hdmi display connected and the display
stops showing the framebuffer but the display goes off.

So it seems as if pl011_shutdown gets called for some reason,
which turns off the uart clock and as a consequence also the
plld gets shut down (probably ref-counting.

I wonder why we have the VPU as a secondary clock in the dt - 
amba-pll11 does not reference it anywhere.

When adding a return to bcm2835_pll_off it looks like this:
root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   73.862152] pl011_startup - start
[   73.865558] pl011_hwinit - prepare-enable
[   73.869886] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[   73.875854] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   73.882044] pl011_hwinit - prepare-enable - ret = 0
[   73.888126] uart-pl011 20201000.uart: no DMA platform data
[   73.893757] pl011_startup - exit
[   73.901495] pl011_shutdown - start
[   73.905034] pl011_shutdown - disable_unprepare
[   73.909742] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[   73.915967] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[   73.922775] bcm2835-clk 20101000.cprman: bcm2835_pll_off - plld
[   73.928844] pl011_shutdown - exit
[   73.934152] pl011_startup - start
[   73.937617] pl011_hwinit - prepare-enable
[   73.941726] bcm2835-clk 20101000.cprman: bcm2835_pll_on - plld
[   73.947738] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   73.953865] pl011_hwinit - prepare-enable - ret = 0
[   73.959776] pl011_startup - exit

The ttyAMA0 console does not work, hdmi display blinks,
but the network works...

When removing the pll_on/off from the pll_ops I have a similar effect
but in this case PLL is not touched at all:
root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   83.345055] pl011_startup - start
[   83.348580] pl011_hwinit - prepare-enable
[   83.352725] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   83.358910] pl011_hwinit - prepare-enable - 0
[   83.365083] uart-pl011 20201000.uart: no DMA platform data
[   83.370790] pl011_startup - exit
[   83.378653] pl011_shutdown - start
[   83.382176] pl011_shutdown - disable_unprepare
[   83.386873] bcm2835-clk 20101000.cprman: bcm2835_clock_off - uart
[   83.393097] bcm2835-clk 20101000.cprman: bcm2835_clock_wait_busy - uart
[   83.399888] pl011_shutdown - exit
[   83.405164] pl011_startup - start
[   83.408652] pl011_hwinit - prepare-enable
[   83.412771] bcm2835-clk 20101000.cprman: bcm2835_clock_on - uart
[   83.418952] pl011_hwinit - prepare-enable - 0
[   83.423665] pl011_startup - exit

The system continues, but the UART still is not working and the
HDMI-Display is flashing only (but with a certain pattern on the
display: totally blue, then some wait, then a black horizontal bar
finally a full dark screen flash - that happens in a 3-4 second
cycle.

As if there is something else that requires the UART clock running
- maybe in the firmware?

Even making the uart clock a VPU clock does not help:
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..4856657 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -758,6 +761,7 @@ static const struct bcm2835_clock_data bcm2835_clock_uart_da
        .div_reg = CM_UARTDIV,
        .int_bits = 10,
        .frac_bits = 12,
+       .is_vpu_clock = true,
 };

 /* HDMI state machine */

This produces the following debug messages:
[   56.409818] pl011_startup - start
[   56.413225] pl011_hwinit - prepare-enable
[   56.417443] pl011_hwinit - prepare-enable - ret = 0
[   56.423297] uart-pl011 20201000.uart: no DMA platform data
[   56.428989] pl011_startup - exit
[   56.435485] pl011_shutdown - start
[   56.440337] pl011_shutdown - disable_unprepare
[   56.444913] pl011_shutdown - exit
[   56.449762] pl011_startup - start
[   56.453165] pl011_hwinit - prepare-enable
[   56.457371] pl011_hwinit - prepare-enable - ret = 0
[   56.462175] pl011_startup - exit

The screen is still blinking and UART does not work.
Maybe the driver is not able to handle the ?remapping?
of registers to a different range and is touching ram
used by the FW?

The only solution that I found is using fixed clocks
in the device-tree (which is not what we intended:
/ {
                clk_uart0: clock at 3 {
                        compatible = "fixed-clock";
                        reg = <3>;
                        #clock-cells = <0>;
                        clock-output-names = "uart0_pclk";
                        clock-frequency = <3000000>;
                };

                clk_apb_p: clock at 4 {
                        compatible = "fixed-clock";
                        reg = <4>;
                        #clock-cells = <0>;
                        clock-output-names = "apb_pclk";
                        clock-frequency = <126000000>;
                };
};
&uart0 {
        clocks = <&clk_uart0 &clk_apb_p>;
};

(so I have ruled out that the amba-pl011 writes to
the wrong addresses).

So as far as I can tell it is only clock related
and when the new clock-framework is used it fails...

Martin

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-12 17:34         ` Martin Sperl
@ 2016-02-12 19:44           ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-12 19:44 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Eric Anholt, linux-arm-kernel, linux-serial, linux-clk, linux-rpi-kernel


> On 12.02.2016, at 18:34, Martin Sperl <kernel@martin.sperl.org> wrote:
> 
> The screen is still blinking and UART does not work.
> Maybe the driver is not able to handle the “remapping”
> of registers to a different range and is touching ram
> used by the FW?
> 
> The only solution that I found is using fixed clocks
> in the device-tree (which is not what we intended:
> / {
>                clk_uart0: clock@3 {
>                        compatible = "fixed-clock";
>                        reg = <3>;
>                        #clock-cells = <0>;
>                        clock-output-names = "uart0_pclk";
>                        clock-frequency = <3000000>;
>                };
> 
>                clk_apb_p: clock@4 {
>                        compatible = "fixed-clock";
>                        reg = <4>;
>                        #clock-cells = <0>;
>                        clock-output-names = "apb_pclk";
>                        clock-frequency = <126000000>;
>                };
> };
> &uart0 {
>        clocks = <&clk_uart0 &clk_apb_p>;
> };
> 
> (so I have ruled out that the amba-pl011 writes to
> the wrong addresses).
> 
> So as far as I can tell it is only clock related
> and when the new clock-framework is used it fails...

So the issue is triggered as soon as the plld_per
pll divider gets disabled/reenabled.

This happens because the clk_hw.core.prepare_count
drops down to 0 and then unprepare is called.

So we need to increase the ref-count for the pll
and pll_dividers to at least 1 so that these never
get disabled - at least for now until we can come
up with a better contract with the firmware.

Obviously this may impact other drivers as well
where a pll is used for the first time - if nothing
else uses it and the clock gets released, then
the clock would trigger a unprepare of the whole
branch of the clock tree.

The question is: how can we solve it in an acceptable
manner?

Do we need a driver that just holds a reference to
those clocks? Or should we just prepare the clock
after registering it in clk-bcm2835.c?

As for why does this not show up when compiled in?
It seems that in that case the amba_pl011 driver
never gets removed and then probed again.

This is possibly related to the optional use of DMA,
with the amba-pl011 driver that retries the install,
which is not supported on the bcm2835 - at least that
is what the datasheet says. And DMA is (probably) not
enabled during the early boot stages, so it does not
fail once when it tries to register DMA.

Thanks,
	Martin



^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-12 19:44           ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-12 19:44 UTC (permalink / raw)
  To: linux-arm-kernel


> On 12.02.2016, at 18:34, Martin Sperl <kernel@martin.sperl.org> wrote:
> 
> The screen is still blinking and UART does not work.
> Maybe the driver is not able to handle the ?remapping?
> of registers to a different range and is touching ram
> used by the FW?
> 
> The only solution that I found is using fixed clocks
> in the device-tree (which is not what we intended:
> / {
>                clk_uart0: clock at 3 {
>                        compatible = "fixed-clock";
>                        reg = <3>;
>                        #clock-cells = <0>;
>                        clock-output-names = "uart0_pclk";
>                        clock-frequency = <3000000>;
>                };
> 
>                clk_apb_p: clock at 4 {
>                        compatible = "fixed-clock";
>                        reg = <4>;
>                        #clock-cells = <0>;
>                        clock-output-names = "apb_pclk";
>                        clock-frequency = <126000000>;
>                };
> };
> &uart0 {
>        clocks = <&clk_uart0 &clk_apb_p>;
> };
> 
> (so I have ruled out that the amba-pl011 writes to
> the wrong addresses).
> 
> So as far as I can tell it is only clock related
> and when the new clock-framework is used it fails...

So the issue is triggered as soon as the plld_per
pll divider gets disabled/reenabled.

This happens because the clk_hw.core.prepare_count
drops down to 0 and then unprepare is called.

So we need to increase the ref-count for the pll
and pll_dividers to at least 1 so that these never
get disabled - at least for now until we can come
up with a better contract with the firmware.

Obviously this may impact other drivers as well
where a pll is used for the first time - if nothing
else uses it and the clock gets released, then
the clock would trigger a unprepare of the whole
branch of the clock tree.

The question is: how can we solve it in an acceptable
manner?

Do we need a driver that just holds a reference to
those clocks? Or should we just prepare the clock
after registering it in clk-bcm2835.c?

As for why does this not show up when compiled in?
It seems that in that case the amba_pl011 driver
never gets removed and then probed again.

This is possibly related to the optional use of DMA,
with the amba-pl011 driver that retries the install,
which is not supported on the bcm2835 - at least that
is what the datasheet says. And DMA is (probably) not
enabled during the early boot stages, so it does not
fail once when it tries to register DMA.

Thanks,
	Martin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-12 19:44           ` Martin Sperl
@ 2016-02-13 10:01             ` Stefan Wahren
  -1 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-13 10:01 UTC (permalink / raw)
  To: Martin Sperl
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel

Hi Martin,

> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
> geschrieben:
>
> So the issue is triggered as soon as the plld_per
> pll divider gets disabled/reenabled.
>
> This happens because the clk_hw.core.prepare_count
> drops down to 0 and then unprepare is called.
>
> So we need to increase the ref-count for the pll
> and pll_dividers to at least 1 so that these never
> get disabled - at least for now until we can come
> up with a better contract with the firmware.
>
> Obviously this may impact other drivers as well
> where a pll is used for the first time - if nothing
> else uses it and the clock gets released, then
> the clock would trigger a unprepare of the whole
> branch of the clock tree.
>
> The question is: how can we solve it in an acceptable
> manner?
>
> Do we need a driver that just holds a reference to
> those clocks? Or should we just prepare the clock
> after registering it in clk-bcm2835.c?
>
> As for why does this not show up when compiled in?
> It seems that in that case the amba_pl011 driver
> never gets removed and then probed again.
>
> This is possibly related to the optional use of DMA,
> with the amba-pl011 driver that retries the install,
> which is not supported on the bcm2835 - at least that
> is what the datasheet says. And DMA is (probably) not
> enabled during the early boot stages, so it does not
> fail once when it tries to register DMA.
>
> Thanks,
> Martin
>

i think i must correct myself. The fixed apb should stay since there is no
dynamic replacement and a proper disabling of plld shouldn't cause this freeze.

I have the suspicion the freeze is caused by a clock glitch or lock-up.

Could you please revert your changes and apply the attached patch? Maybe we can
see more.

------------------------------8<-----------------------------------------
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 637f8ae..03d95c1 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -364,6 +364,7 @@ struct bcm2835_cprman {
 	void __iomem *regs;
 	spinlock_t regs_lock;
 	const char *osc_name;
+	int func_code;
 
 	struct clk_onecell_data onecell;
 	struct clk *clks[];
@@ -1222,8 +1223,13 @@ static void bcm2835_pll_off(struct clk_hw *hw)
 	struct bcm2835_cprman *cprman = pll->cprman;
 	const struct bcm2835_pll_data *data = pll->data;
 
+	if (!(cprman_read(cprman, CM_LOCK) & data->lock_mask))
+		pr_warn("%s: PLL still locked from %d\n", __func__, cprman->func_code);
+
 	cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
 	cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
+
+	cprman->func_code = 1;
 }
 
 static int bcm2835_pll_on(struct clk_hw *hw)
@@ -1233,10 +1239,15 @@ static int bcm2835_pll_on(struct clk_hw *hw)
 	const struct bcm2835_pll_data *data = pll->data;
 	ktime_t timeout;
 
+	if (!(cprman_read(cprman, CM_LOCK) & data->lock_mask))
+		pr_warn("%s: PLL still locked from %d\n", __func__, cprman->func_code);
+
 	/* Take the PLL out of reset. */
 	cprman_write(cprman, data->cm_ctrl_reg,
 		     cprman_read(cprman, data->cm_ctrl_reg) & ~CM_PLL_ANARST);
 
+	cprman->func_code = 2;
+
 	/* Wait for the PLL to lock. */
 	timeout = ktime_add_ns(ktime_get(), LOCK_TIMEOUT_NS);
 	while (!(cprman_read(cprman, CM_LOCK) & data->lock_mask)) {
@@ -1267,6 +1278,8 @@ bcm2835_pll_write_ana(struct bcm2835_cprman *cprman, u32
ana_reg_base, u32 *ana)
 	 */
 	for (i = 3; i >= 0; i--)
 		cprman_write(cprman, ana_reg_base + i * 4, ana[i]);
+
+	cprman->func_code = 3;
 }
 
 static int bcm2835_pll_set_rate(struct clk_hw *hw,
@@ -1339,6 +1352,8 @@ static int bcm2835_pll_set_rate(struct clk_hw *hw,
 	if (!do_ana_setup_first)
 		bcm2835_pll_write_ana(cprman, data->ana_reg_base, ana);
 
+	cprman->func_code = 4;
+
 	return 0;
 }
 
@@ -1404,6 +1419,8 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
 		     (cprman_read(cprman, data->cm_reg) &
 		      ~data->load_mask) | data->hold_mask);
 	cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
+
+	cprman->func_code = 5;
 }
 
 static int bcm2835_pll_divider_on(struct clk_hw *hw)
@@ -1419,6 +1436,8 @@ static int bcm2835_pll_divider_on(struct clk_hw *hw)
 	cprman_write(cprman, data->cm_reg,
 		     cprman_read(cprman, data->cm_reg) & ~data->hold_mask);
 
+	cprman->func_code = 6;
+
 	return 0;
 }
 
@@ -1440,6 +1459,8 @@ static int bcm2835_pll_divider_set_rate(struct clk_hw *hw,
 	cprman_write(cprman, data->cm_reg, cm | data->load_mask);
 	cprman_write(cprman, data->cm_reg, cm & ~data->load_mask);
 
+	cprman->func_code = 7;
+
 	return 0;
 }
 
@@ -1588,7 +1609,7 @@ static void bcm2835_clock_wait_busy(struct bcm2835_clock
*clock)
 
 	while (cprman_read(cprman, data->ctl_reg) & CM_BUSY) {
 		if (ktime_after(ktime_get(), timeout)) {
-			dev_err(cprman->dev, "%s: couldn't lock PLL\n",
+			dev_err(cprman->dev, "%s: clk busy timeout\n",
 				clk_hw_get_name(&clock->hw));
 			return;
 		}
@@ -1602,6 +1623,9 @@ static void bcm2835_clock_off(struct clk_hw *hw)
 	struct bcm2835_cprman *cprman = clock->cprman;
 	const struct bcm2835_clock_data *data = clock->data;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	spin_lock(&cprman->regs_lock);
 	cprman_write(cprman, data->ctl_reg,
 		     cprman_read(cprman, data->ctl_reg) & ~CM_ENABLE);
@@ -1609,6 +1633,11 @@ static void bcm2835_clock_off(struct clk_hw *hw)
 
 	/* BUSY will remain high until the divider completes its cycle. */
 	bcm2835_clock_wait_busy(clock);
+
+	cprman->func_code = 8;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
 }
 
 static int bcm2835_clock_on(struct clk_hw *hw)
@@ -1617,6 +1646,9 @@ static int bcm2835_clock_on(struct clk_hw *hw)
 	struct bcm2835_cprman *cprman = clock->cprman;
 	const struct bcm2835_clock_data *data = clock->data;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	spin_lock(&cprman->regs_lock);
 	cprman_write(cprman, data->ctl_reg,
 		     cprman_read(cprman, data->ctl_reg) |
@@ -1624,6 +1656,11 @@ static int bcm2835_clock_on(struct clk_hw *hw)
 		     CM_GATE);
 	spin_unlock(&cprman->regs_lock);
 
+	cprman->func_code = 9;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
+
 	return 0;
 }
 
@@ -1638,6 +1675,9 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
 	enum bcm2835_clock_mash_type mash = divmash_get_mash(dm);
 	u32 ctl;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	spin_lock(&cprman->regs_lock);
 
 	/* if div and mash are identical, then there is nothing to do */
@@ -1663,6 +1703,11 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
 unlock_exit:
 	spin_unlock(&cprman->regs_lock);
 
+	cprman->func_code = 10;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
+
 	return 0;
 }
 
@@ -1713,7 +1758,16 @@ static int bcm2835_clock_set_parent(struct clk_hw *hw, u8
index)
 	const struct bcm2835_clock_data *data = clock->data;
 	u8 src = (index << CM_SRC_SHIFT) & CM_SRC_MASK;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	cprman_write(cprman, data->ctl_reg, src);
+
+	cprman->func_code = 11;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
+
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-13 10:01             ` Stefan Wahren
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-13 10:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Martin,

> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
> geschrieben:
>
> So the issue is triggered as soon as the plld_per
> pll divider gets disabled/reenabled.
>
> This happens because the clk_hw.core.prepare_count
> drops down to 0 and then unprepare is called.
>
> So we need to increase the ref-count for the pll
> and pll_dividers to at least 1 so that these never
> get disabled - at least for now until we can come
> up with a better contract with the firmware.
>
> Obviously this may impact other drivers as well
> where a pll is used for the first time - if nothing
> else uses it and the clock gets released, then
> the clock would trigger a unprepare of the whole
> branch of the clock tree.
>
> The question is: how can we solve it in an acceptable
> manner?
>
> Do we need a driver that just holds a reference to
> those clocks? Or should we just prepare the clock
> after registering it in clk-bcm2835.c?
>
> As for why does this not show up when compiled in?
> It seems that in that case the amba_pl011 driver
> never gets removed and then probed again.
>
> This is possibly related to the optional use of DMA,
> with the amba-pl011 driver that retries the install,
> which is not supported on the bcm2835 - at least that
> is what the datasheet says. And DMA is (probably) not
> enabled during the early boot stages, so it does not
> fail once when it tries to register DMA.
>
> Thanks,
> Martin
>

i think i must correct myself. The fixed apb should stay since there is no
dynamic replacement and a proper disabling of plld shouldn't cause this freeze.

I have the suspicion the freeze is caused by a clock glitch or lock-up.

Could you please revert your changes and apply the attached patch? Maybe we can
see more.

------------------------------8<-----------------------------------------
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 637f8ae..03d95c1 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -364,6 +364,7 @@ struct bcm2835_cprman {
 	void __iomem *regs;
 	spinlock_t regs_lock;
 	const char *osc_name;
+	int func_code;
 
 	struct clk_onecell_data onecell;
 	struct clk *clks[];
@@ -1222,8 +1223,13 @@ static void bcm2835_pll_off(struct clk_hw *hw)
 	struct bcm2835_cprman *cprman = pll->cprman;
 	const struct bcm2835_pll_data *data = pll->data;
 
+	if (!(cprman_read(cprman, CM_LOCK) & data->lock_mask))
+		pr_warn("%s: PLL still locked from %d\n", __func__, cprman->func_code);
+
 	cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
 	cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
+
+	cprman->func_code = 1;
 }
 
 static int bcm2835_pll_on(struct clk_hw *hw)
@@ -1233,10 +1239,15 @@ static int bcm2835_pll_on(struct clk_hw *hw)
 	const struct bcm2835_pll_data *data = pll->data;
 	ktime_t timeout;
 
+	if (!(cprman_read(cprman, CM_LOCK) & data->lock_mask))
+		pr_warn("%s: PLL still locked from %d\n", __func__, cprman->func_code);
+
 	/* Take the PLL out of reset. */
 	cprman_write(cprman, data->cm_ctrl_reg,
 		     cprman_read(cprman, data->cm_ctrl_reg) & ~CM_PLL_ANARST);
 
+	cprman->func_code = 2;
+
 	/* Wait for the PLL to lock. */
 	timeout = ktime_add_ns(ktime_get(), LOCK_TIMEOUT_NS);
 	while (!(cprman_read(cprman, CM_LOCK) & data->lock_mask)) {
@@ -1267,6 +1278,8 @@ bcm2835_pll_write_ana(struct bcm2835_cprman *cprman, u32
ana_reg_base, u32 *ana)
 	 */
 	for (i = 3; i >= 0; i--)
 		cprman_write(cprman, ana_reg_base + i * 4, ana[i]);
+
+	cprman->func_code = 3;
 }
 
 static int bcm2835_pll_set_rate(struct clk_hw *hw,
@@ -1339,6 +1352,8 @@ static int bcm2835_pll_set_rate(struct clk_hw *hw,
 	if (!do_ana_setup_first)
 		bcm2835_pll_write_ana(cprman, data->ana_reg_base, ana);
 
+	cprman->func_code = 4;
+
 	return 0;
 }
 
@@ -1404,6 +1419,8 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
 		     (cprman_read(cprman, data->cm_reg) &
 		      ~data->load_mask) | data->hold_mask);
 	cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
+
+	cprman->func_code = 5;
 }
 
 static int bcm2835_pll_divider_on(struct clk_hw *hw)
@@ -1419,6 +1436,8 @@ static int bcm2835_pll_divider_on(struct clk_hw *hw)
 	cprman_write(cprman, data->cm_reg,
 		     cprman_read(cprman, data->cm_reg) & ~data->hold_mask);
 
+	cprman->func_code = 6;
+
 	return 0;
 }
 
@@ -1440,6 +1459,8 @@ static int bcm2835_pll_divider_set_rate(struct clk_hw *hw,
 	cprman_write(cprman, data->cm_reg, cm | data->load_mask);
 	cprman_write(cprman, data->cm_reg, cm & ~data->load_mask);
 
+	cprman->func_code = 7;
+
 	return 0;
 }
 
@@ -1588,7 +1609,7 @@ static void bcm2835_clock_wait_busy(struct bcm2835_clock
*clock)
 
 	while (cprman_read(cprman, data->ctl_reg) & CM_BUSY) {
 		if (ktime_after(ktime_get(), timeout)) {
-			dev_err(cprman->dev, "%s: couldn't lock PLL\n",
+			dev_err(cprman->dev, "%s: clk busy timeout\n",
 				clk_hw_get_name(&clock->hw));
 			return;
 		}
@@ -1602,6 +1623,9 @@ static void bcm2835_clock_off(struct clk_hw *hw)
 	struct bcm2835_cprman *cprman = clock->cprman;
 	const struct bcm2835_clock_data *data = clock->data;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	spin_lock(&cprman->regs_lock);
 	cprman_write(cprman, data->ctl_reg,
 		     cprman_read(cprman, data->ctl_reg) & ~CM_ENABLE);
@@ -1609,6 +1633,11 @@ static void bcm2835_clock_off(struct clk_hw *hw)
 
 	/* BUSY will remain high until the divider completes its cycle. */
 	bcm2835_clock_wait_busy(clock);
+
+	cprman->func_code = 8;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
 }
 
 static int bcm2835_clock_on(struct clk_hw *hw)
@@ -1617,6 +1646,9 @@ static int bcm2835_clock_on(struct clk_hw *hw)
 	struct bcm2835_cprman *cprman = clock->cprman;
 	const struct bcm2835_clock_data *data = clock->data;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	spin_lock(&cprman->regs_lock);
 	cprman_write(cprman, data->ctl_reg,
 		     cprman_read(cprman, data->ctl_reg) |
@@ -1624,6 +1656,11 @@ static int bcm2835_clock_on(struct clk_hw *hw)
 		     CM_GATE);
 	spin_unlock(&cprman->regs_lock);
 
+	cprman->func_code = 9;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
+
 	return 0;
 }
 
@@ -1638,6 +1675,9 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
 	enum bcm2835_clock_mash_type mash = divmash_get_mash(dm);
 	u32 ctl;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	spin_lock(&cprman->regs_lock);
 
 	/* if div and mash are identical, then there is nothing to do */
@@ -1663,6 +1703,11 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
 unlock_exit:
 	spin_unlock(&cprman->regs_lock);
 
+	cprman->func_code = 10;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
+
 	return 0;
 }
 
@@ -1713,7 +1758,16 @@ static int bcm2835_clock_set_parent(struct clk_hw *hw, u8
index)
 	const struct bcm2835_clock_data *data = clock->data;
 	u8 src = (index << CM_SRC_SHIFT) & CM_SRC_MASK;
 
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
+
 	cprman_write(cprman, data->ctl_reg, src);
+
+	cprman->func_code = 11;
+
+	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
+		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
+
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-13 10:01             ` Stefan Wahren
@ 2016-02-13 11:24               ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-13 11:24 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel


> On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> Hi Martin,
> 
>> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
>> geschrieben:
>> 
>> So the issue is triggered as soon as the plld_per
>> pll divider gets disabled/reenabled.
>> 
>> This happens because the clk_hw.core.prepare_count
>> drops down to 0 and then unprepare is called.
>> 
>> So we need to increase the ref-count for the pll
>> and pll_dividers to at least 1 so that these never
>> get disabled - at least for now until we can come
>> up with a better contract with the firmware.
>> 
>> Obviously this may impact other drivers as well
>> where a pll is used for the first time - if nothing
>> else uses it and the clock gets released, then
>> the clock would trigger a unprepare of the whole
>> branch of the clock tree.
>> 
>> The question is: how can we solve it in an acceptable
>> manner?
>> 
>> Do we need a driver that just holds a reference to
>> those clocks? Or should we just prepare the clock
>> after registering it in clk-bcm2835.c?
>> 
>> As for why does this not show up when compiled in?
>> It seems that in that case the amba_pl011 driver
>> never gets removed and then probed again.
>> 
>> This is possibly related to the optional use of DMA,
>> with the amba-pl011 driver that retries the install,
>> which is not supported on the bcm2835 - at least that
>> is what the datasheet says. And DMA is (probably) not
>> enabled during the early boot stages, so it does not
>> fail once when it tries to register DMA.
>> 
>> Thanks,
>> Martin
>> 
> 
> i think i must correct myself. The fixed apb should stay since there is no
> dynamic replacement and a proper disabling of plld shouldn't cause this freeze.
> 
> I have the suspicion the freeze is caused by a clock glitch or lock-up.
> 
> Could you please revert your changes and apply the attached patch? Maybe we can
> see more.

I will give it a try.

But to me it seems as if the disabling of PLLD produces a hickup in the 
firmware (which relies on PLLD-DIV and thus PLL).

This would also explain the 3-4 second bar/blanking pattern seen on HDMI,
which I would guess is related to a reset loop...

I got a simple fix for now, that just prepares a few clocks/pll/pll_divs
during probing, so that they will never get unprepared/fully stopped):

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..fe0c401 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -37,6 +37,7 @@
  * generator).
  */

+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
 #include <linux/clk/bcm2835.h>
@@ -1504,6 +1505,7 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
        struct clk **clks;
        struct bcm2835_cprman *cprman;
        struct resource *res;
+       int ret;

        cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
        if (!cprman)
@@ -1597,6 +1599,37 @@ static int bcm2835_clk_probe(struct platform_device *pdev
        clks[BCM2835_CLOCK_PWM] =
                bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);

+       /*
+        * Several PLLs, PLL-dividers and clocks need to get prepared
+        * so that they are never unprepared and released.
+        * We run this separately as there may be dependencies that would
+        * need to be fullfilled first...
+        * Note: Especially unpreparing PLLD_PER will kill the system
+        *       (even if it is prepared again almost immediately
+        *        the HDMI display will fail)
+        */
+#define CLK_PREPARE_INITIAL(clkid)                                     \
+       if (clks[clkid] && (ret = clk_prepare(clks[clkid])))            \
+               dev_err(dev, "could not prepare clock %pC - %d\n",      \
+                       clks[clkid], ret);
+       CLK_PREPARE_INITIAL(BCM2835_PLLA);
+       CLK_PREPARE_INITIAL(BCM2835_PLLB);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC);
+       CLK_PREPARE_INITIAL(BCM2835_PLLD);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH);
+       CLK_PREPARE_INITIAL(BCM2835_PLLA_CORE);
+       CLK_PREPARE_INITIAL(BCM2835_PLLA_PER);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE0);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE1);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE2);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_PER);
+       CLK_PREPARE_INITIAL(BCM2835_PLLD_CORE);
+       CLK_PREPARE_INITIAL(BCM2835_PLLD_PER);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH_RCAL);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH_AUX);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH_PIX);
+       CLK_PREPARE_INITIAL(BCM2835_CLOCK_VPU);
+
        return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
                                   &cprman->onecell);
 }

And that resolves the issue and also allows loading/unloading of the
amba-pl011 module as well without any hickups.

I guess it would be better if we did make this configurable in the DT.
(See patch 1 of my RFC patchset, which could get extended to handle this
as well - at least a portion thereof).

But in more general terms: we need to come up with some agreement
on ownership of the clocks between linux and the firmware and how
to change them without impacting the other. 

One example is the “overclocking” that currently happens for
downstream kernels in the firmware, which impacts some of the
plls and PLLdivs as well as the sram control registers.

The reason for this seems to be that for overclocking the SRAM
parameters also need to get changed and that can only get done
reliably when running the code from L1/L2 caches - and that
requirement of not touching SRAM while changing the SRAM parameters
is probably hard to achieve cleanly on ARM alone.

Ciao,
	Martin

P.s: unloading amba_pl011 gives the following error:
root@raspcm:~# rmmod amba_pl011
[  142.318263] Trying to free nonexistent resource <0000000020201000-0000000020201fff>
but that is an issue not related to clock...


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-13 11:24               ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-13 11:24 UTC (permalink / raw)
  To: linux-arm-kernel


> On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> Hi Martin,
> 
>> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
>> geschrieben:
>> 
>> So the issue is triggered as soon as the plld_per
>> pll divider gets disabled/reenabled.
>> 
>> This happens because the clk_hw.core.prepare_count
>> drops down to 0 and then unprepare is called.
>> 
>> So we need to increase the ref-count for the pll
>> and pll_dividers to at least 1 so that these never
>> get disabled - at least for now until we can come
>> up with a better contract with the firmware.
>> 
>> Obviously this may impact other drivers as well
>> where a pll is used for the first time - if nothing
>> else uses it and the clock gets released, then
>> the clock would trigger a unprepare of the whole
>> branch of the clock tree.
>> 
>> The question is: how can we solve it in an acceptable
>> manner?
>> 
>> Do we need a driver that just holds a reference to
>> those clocks? Or should we just prepare the clock
>> after registering it in clk-bcm2835.c?
>> 
>> As for why does this not show up when compiled in?
>> It seems that in that case the amba_pl011 driver
>> never gets removed and then probed again.
>> 
>> This is possibly related to the optional use of DMA,
>> with the amba-pl011 driver that retries the install,
>> which is not supported on the bcm2835 - at least that
>> is what the datasheet says. And DMA is (probably) not
>> enabled during the early boot stages, so it does not
>> fail once when it tries to register DMA.
>> 
>> Thanks,
>> Martin
>> 
> 
> i think i must correct myself. The fixed apb should stay since there is no
> dynamic replacement and a proper disabling of plld shouldn't cause this freeze.
> 
> I have the suspicion the freeze is caused by a clock glitch or lock-up.
> 
> Could you please revert your changes and apply the attached patch? Maybe we can
> see more.

I will give it a try.

But to me it seems as if the disabling of PLLD produces a hickup in the 
firmware (which relies on PLLD-DIV and thus PLL).

This would also explain the 3-4 second bar/blanking pattern seen on HDMI,
which I would guess is related to a reset loop...

I got a simple fix for now, that just prepares a few clocks/pll/pll_divs
during probing, so that they will never get unprepared/fully stopped):

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 015e687..fe0c401 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -37,6 +37,7 @@
  * generator).
  */

+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
 #include <linux/clk/bcm2835.h>
@@ -1504,6 +1505,7 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
        struct clk **clks;
        struct bcm2835_cprman *cprman;
        struct resource *res;
+       int ret;

        cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
        if (!cprman)
@@ -1597,6 +1599,37 @@ static int bcm2835_clk_probe(struct platform_device *pdev
        clks[BCM2835_CLOCK_PWM] =
                bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);

+       /*
+        * Several PLLs, PLL-dividers and clocks need to get prepared
+        * so that they are never unprepared and released.
+        * We run this separately as there may be dependencies that would
+        * need to be fullfilled first...
+        * Note: Especially unpreparing PLLD_PER will kill the system
+        *       (even if it is prepared again almost immediately
+        *        the HDMI display will fail)
+        */
+#define CLK_PREPARE_INITIAL(clkid)                                     \
+       if (clks[clkid] && (ret = clk_prepare(clks[clkid])))            \
+               dev_err(dev, "could not prepare clock %pC - %d\n",      \
+                       clks[clkid], ret);
+       CLK_PREPARE_INITIAL(BCM2835_PLLA);
+       CLK_PREPARE_INITIAL(BCM2835_PLLB);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC);
+       CLK_PREPARE_INITIAL(BCM2835_PLLD);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH);
+       CLK_PREPARE_INITIAL(BCM2835_PLLA_CORE);
+       CLK_PREPARE_INITIAL(BCM2835_PLLA_PER);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE0);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE1);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE2);
+       CLK_PREPARE_INITIAL(BCM2835_PLLC_PER);
+       CLK_PREPARE_INITIAL(BCM2835_PLLD_CORE);
+       CLK_PREPARE_INITIAL(BCM2835_PLLD_PER);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH_RCAL);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH_AUX);
+       CLK_PREPARE_INITIAL(BCM2835_PLLH_PIX);
+       CLK_PREPARE_INITIAL(BCM2835_CLOCK_VPU);
+
        return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
                                   &cprman->onecell);
 }

And that resolves the issue and also allows loading/unloading of the
amba-pl011 module as well without any hickups.

I guess it would be better if we did make this configurable in the DT.
(See patch 1 of my RFC patchset, which could get extended to handle this
as well - at least a portion thereof).

But in more general terms: we need to come up with some agreement
on ownership of the clocks between linux and the firmware and how
to change them without impacting the other. 

One example is the ?overclocking? that currently happens for
downstream kernels in the firmware, which impacts some of the
plls and PLLdivs as well as the sram control registers.

The reason for this seems to be that for overclocking the SRAM
parameters also need to get changed and that can only get done
reliably when running the code from L1/L2 caches - and that
requirement of not touching SRAM while changing the SRAM parameters
is probably hard to achieve cleanly on ARM alone.

Ciao,
	Martin

P.s: unloading amba_pl011 gives the following error:
root at raspcm:~# rmmod amba_pl011
[  142.318263] Trying to free nonexistent resource <0000000020201000-0000000020201fff>
but that is an issue not related to clock...

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-13 10:01             ` Stefan Wahren
@ 2016-02-13 11:53               ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-13 11:53 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel


> On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 637f8ae..03d95c1 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -1638,6 +1675,9 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
> 	enum bcm2835_clock_mash_type mash = divmash_get_mash(dm);
> 	u32 ctl;
> 
> +	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
> +		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
> +
> 	spin_lock(&cprman->regs_lock);
> 
> 	/* if div and mash are identical, then there is nothing to do */
> @@ -1663,6 +1703,11 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
> unlock_exit:
> 	spin_unlock(&cprman->regs_lock);
> 
> +	cprman->func_code = 10;
> +
> +	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
> +		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
> +
> 	return 0;
> }
> 

Seems as if you apply it on top of (parts of) my patchset - not on top of a 
clean 4.5-rc3.

Anyway - after some fixing I get the following outputs -
the amba-pl011 driver is still patched with debug messages:

After boot with aux-uart as tty:
root@raspcm:~# dmesg  | grep bcm2835
[    0.018172] bcm2835: system timer (irq = 27)
[    0.711388] bcm2835-aux-uart 20215040.serial: could not get clk: -517
[    1.649124] bcm2835-rng 20104000.rng: hwrng registered
[    2.051190] bcm2835_clock_on: clk still busy from 6
[    2.056159] bcm2835_clock_on: clk now busy from 9
[    8.062743] bcm2835-wdt 20100000.watchdog: Broadcom BCM2835 watchdog timer
[   12.042105] bcm2835-i2s 20203000.i2s: can't request region for resource [mem 0x20101098-0x20101099]
[   12.191701] bcm2835-i2s: probe of 20203000.i2s failed with error -16

Loading the module:
root@raspcm:~# modprobe amba-pl011
[  106.685812] Serial: AMBA PL011 UART driver
[  106.693702] uart-pl011 20201000.uart: pl011_setup_port: f0201000 20201000
[  106.702109] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
root@raspcm:~#

and starting getty:
root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[  137.851123] pl011_startup - start
[  137.854538] pl011_hwinit - prepare-enable
[  137.858772] bcm2835_clock_on: clk still busy from 6
[  137.863755] bcm2835_clock_on: clk now busy from 9
[  137.868590] pl011_hwinit - prepare-enable - ret = 0
[  137.875249] uart-pl011 20201000.uart: no DMA platform data
[  137.880940] pl011_startup - exit
[  137.888840] pl011_shutdown - start
[  137.892360] pl011_shutdown - disable_unprepare
[  137.896933] bcm2835_clock_off: clk still busy from 9
[  137.902112] pl011_shutdown - exit
[  137.907233] pl011_startup - start
[  137.910713] pl011_hwinit - prepare-enable
[  137.914808] bcm2835_pll_on: PLL still locked from 1
[  138.019876] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[  138.026000] pl011_hwinit - prepare-enable - ret = -110
[  138.031266] pl011_startup - error = -110 - disable_unprepare
[  138.037048] ------------[ cut here ]------------
[  138.041777] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
[  138.051063] ---[ end trace dd2f225b2af4c32c ]---
[  138.055859] ------------[ cut here ]------------
[  138.060627] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
[  138.070321] ---[ end trace dd2f225b2af4c32d ]---

No HDMI output - this time no “flashing” - just no signal.
Machine is crashed - the attached AXIS USB network card (0b95:772b)
just transmits identical packets on the network without stopping…

I have also disabled the DMA-engine in the amba-pl011 driver
(in the driver itself add: #undef CONFIG_DMA_ENGINE) and then I get:
root@raspcm:/build/linux# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   97.010287] pl011_startup - start
[   97.013698] pl011_hwinit - prepare-enable
[   97.017931] bcm2835_clock_on: clk still busy from 6
[   97.022905] bcm2835_clock_on: clk now busy from 9
[   97.027755] pl011_hwinit - prepare-enable - ret = 0
[   97.034378] pl011_startup - exit
[   97.041197] pl011_shutdown - start
[   97.044728] pl011_shutdown - disable_unprepare
[   97.049386] bcm2835_clock_off: clk still busy from 9
[   97.054458] pl011_shutdown - exit
[   97.059633] pl011_startup - start
[   97.063033] pl011_hwinit - prepare-enable
[   97.067533] bcm2835_pll_on: PLL still locked from 1
[   97.172655] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[   97.178838] pl011_hwinit - prepare-enable - ret = -110
[   97.184075] pl011_startup - error = -110 - disable_unprepare
[   97.189905] ------------[ cut here ]------------
[   97.194640] WARNING: CPU: 0 PID: 2442 at drivers/clk/clk.c:680 clk_core_disab
le+0x34/0xf0()
[   97.203924] ---[ end trace 3e878f70606eba69 ]---
[   97.208692] ------------[ cut here ]------------
[   97.213429] WARNING: CPU: 0 PID: 2442 at drivers/clk/clk.c:575 clk_core_unpre
pare+0x34/0x110()
[   97.223096] ---[ end trace 3e878f70606eba6a ]—

Not sure why the pl011 driver runs the startup/shutdown/startup
loop.

Martin





^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-13 11:53               ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-13 11:53 UTC (permalink / raw)
  To: linux-arm-kernel


> On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 637f8ae..03d95c1 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -1638,6 +1675,9 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
> 	enum bcm2835_clock_mash_type mash = divmash_get_mash(dm);
> 	u32 ctl;
> 
> +	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
> +		pr_warn("%s: clk still busy from %d\n", __func__, cprman->func_code);
> +
> 	spin_lock(&cprman->regs_lock);
> 
> 	/* if div and mash are identical, then there is nothing to do */
> @@ -1663,6 +1703,11 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
> unlock_exit:
> 	spin_unlock(&cprman->regs_lock);
> 
> +	cprman->func_code = 10;
> +
> +	if (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
> +		pr_warn("%s: clk now busy from %d\n", __func__, cprman->func_code);
> +
> 	return 0;
> }
> 

Seems as if you apply it on top of (parts of) my patchset - not on top of a 
clean 4.5-rc3.

Anyway - after some fixing I get the following outputs -
the amba-pl011 driver is still patched with debug messages:

After boot with aux-uart as tty:
root at raspcm:~# dmesg  | grep bcm2835
[    0.018172] bcm2835: system timer (irq = 27)
[    0.711388] bcm2835-aux-uart 20215040.serial: could not get clk: -517
[    1.649124] bcm2835-rng 20104000.rng: hwrng registered
[    2.051190] bcm2835_clock_on: clk still busy from 6
[    2.056159] bcm2835_clock_on: clk now busy from 9
[    8.062743] bcm2835-wdt 20100000.watchdog: Broadcom BCM2835 watchdog timer
[   12.042105] bcm2835-i2s 20203000.i2s: can't request region for resource [mem 0x20101098-0x20101099]
[   12.191701] bcm2835-i2s: probe of 20203000.i2s failed with error -16

Loading the module:
root at raspcm:~# modprobe amba-pl011
[  106.685812] Serial: AMBA PL011 UART driver
[  106.693702] uart-pl011 20201000.uart: pl011_setup_port: f0201000 20201000
[  106.702109] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud = 0) is a PL011 rev2
root at raspcm:~#

and starting getty:
root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[  137.851123] pl011_startup - start
[  137.854538] pl011_hwinit - prepare-enable
[  137.858772] bcm2835_clock_on: clk still busy from 6
[  137.863755] bcm2835_clock_on: clk now busy from 9
[  137.868590] pl011_hwinit - prepare-enable - ret = 0
[  137.875249] uart-pl011 20201000.uart: no DMA platform data
[  137.880940] pl011_startup - exit
[  137.888840] pl011_shutdown - start
[  137.892360] pl011_shutdown - disable_unprepare
[  137.896933] bcm2835_clock_off: clk still busy from 9
[  137.902112] pl011_shutdown - exit
[  137.907233] pl011_startup - start
[  137.910713] pl011_hwinit - prepare-enable
[  137.914808] bcm2835_pll_on: PLL still locked from 1
[  138.019876] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[  138.026000] pl011_hwinit - prepare-enable - ret = -110
[  138.031266] pl011_startup - error = -110 - disable_unprepare
[  138.037048] ------------[ cut here ]------------
[  138.041777] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:680 clk_core_disable+0x34/0xf0()
[  138.051063] ---[ end trace dd2f225b2af4c32c ]---
[  138.055859] ------------[ cut here ]------------
[  138.060627] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:575 clk_core_unprepare+0x34/0x110()
[  138.070321] ---[ end trace dd2f225b2af4c32d ]---

No HDMI output - this time no ?flashing? - just no signal.
Machine is crashed - the attached AXIS USB network card (0b95:772b)
just transmits identical packets on the network without stopping?

I have also disabled the DMA-engine in the amba-pl011 driver
(in the driver itself add: #undef CONFIG_DMA_ENGINE) and then I get:
root at raspcm:/build/linux# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   97.010287] pl011_startup - start
[   97.013698] pl011_hwinit - prepare-enable
[   97.017931] bcm2835_clock_on: clk still busy from 6
[   97.022905] bcm2835_clock_on: clk now busy from 9
[   97.027755] pl011_hwinit - prepare-enable - ret = 0
[   97.034378] pl011_startup - exit
[   97.041197] pl011_shutdown - start
[   97.044728] pl011_shutdown - disable_unprepare
[   97.049386] bcm2835_clock_off: clk still busy from 9
[   97.054458] pl011_shutdown - exit
[   97.059633] pl011_startup - start
[   97.063033] pl011_hwinit - prepare-enable
[   97.067533] bcm2835_pll_on: PLL still locked from 1
[   97.172655] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[   97.178838] pl011_hwinit - prepare-enable - ret = -110
[   97.184075] pl011_startup - error = -110 - disable_unprepare
[   97.189905] ------------[ cut here ]------------
[   97.194640] WARNING: CPU: 0 PID: 2442 at drivers/clk/clk.c:680 clk_core_disab
le+0x34/0xf0()
[   97.203924] ---[ end trace 3e878f70606eba69 ]---
[   97.208692] ------------[ cut here ]------------
[   97.213429] WARNING: CPU: 0 PID: 2442 at drivers/clk/clk.c:575 clk_core_unpre
pare+0x34/0x110()
[   97.223096] ---[ end trace 3e878f70606eba6a ]?

Not sure why the pl011 driver runs the startup/shutdown/startup
loop.

Martin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-13 11:53               ` Martin Sperl
@ 2016-02-13 20:45                 ` Stefan Wahren
  -1 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-13 20:45 UTC (permalink / raw)
  To: Martin Sperl
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel

Hi Martin,

> Martin Sperl <kernel@martin.sperl.org> hat am 13. Februar 2016 um 12:53
> geschrieben:
>
>
> Seems as if you apply it on top of (parts of) my patchset - not on top of a
> clean 4.5-rc3.

sorry about that. I took the wrong directory.

>
> Anyway - after some fixing I get the following outputs -
> the amba-pl011 driver is still patched with debug messages:
>
> After boot with aux-uart as tty:
> root@raspcm:~# dmesg | grep bcm2835
> [ 0.018172] bcm2835: system timer (irq = 27)
> [ 0.711388] bcm2835-aux-uart 20215040.serial: could not get clk: -517
> [ 1.649124] bcm2835-rng 20104000.rng: hwrng registered
> [ 2.051190] bcm2835_clock_on: clk still busy from 6
> [ 2.056159] bcm2835_clock_on: clk now busy from 9

I get the same output here. It belongs to mmc clock.

> [ 8.062743] bcm2835-wdt 20100000.watchdog: Broadcom BCM2835 watchdog timer
> [ 12.042105] bcm2835-i2s 20203000.i2s: can't request region for resource [mem
> 0x20101098-0x20101099]
> [ 12.191701] bcm2835-i2s: probe of 20203000.i2s failed with error -16
>
> Loading the module:
> root@raspcm:~# modprobe amba-pl011
> [ 106.685812] Serial: AMBA PL011 UART driver
> [ 106.693702] uart-pl011 20201000.uart: pl011_setup_port: f0201000 20201000
> [ 106.702109] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud =
> 0) is a PL011 rev2
> root@raspcm:~#
>
> and starting getty:
> root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
> [ 137.851123] pl011_startup - start
> [ 137.854538] pl011_hwinit - prepare-enable
> [ 137.858772] bcm2835_clock_on: clk still busy from 6
> [ 137.863755] bcm2835_clock_on: clk now busy from 9
> [ 137.868590] pl011_hwinit - prepare-enable - ret = 0
> [ 137.875249] uart-pl011 20201000.uart: no DMA platform data
> [ 137.880940] pl011_startup - exit
> [ 137.888840] pl011_shutdown - start
> [ 137.892360] pl011_shutdown - disable_unprepare
> [ 137.896933] bcm2835_clock_off: clk still busy from 9
> [ 137.902112] pl011_shutdown - exit
> [ 137.907233] pl011_startup - start
> [ 137.910713] pl011_hwinit - prepare-enable
> [ 137.914808] bcm2835_pll_on: PLL still locked from 1
> [ 138.019876] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
> [ 138.026000] pl011_hwinit - prepare-enable - ret = -110
> [ 138.031266] pl011_startup - error = -110 - disable_unprepare
> [ 138.037048] ------------[ cut here ]------------
> [ 138.041777] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:680
> clk_core_disable+0x34/0xf0()
> [ 138.051063] ---[ end trace dd2f225b2af4c32c ]---
> [ 138.055859] ------------[ cut here ]------------
> [ 138.060627] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:575
> clk_core_unprepare+0x34/0x110()
> [ 138.070321] ---[ end trace dd2f225b2af4c32d ]---
>
> No HDMI output - this time no “flashing” - just no signal.
> Machine is crashed - the attached AXIS USB network card (0b95:772b)
> just transmits identical packets on the network without stopping…
>

According to the datasheet busy bit shouldn't be set while changing the clock.
So this isn't good. I hope this could be fixed, too.

Regards

>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-13 20:45                 ` Stefan Wahren
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-13 20:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Martin,

> Martin Sperl <kernel@martin.sperl.org> hat am 13. Februar 2016 um 12:53
> geschrieben:
>
>
> Seems as if you apply it on top of (parts of) my patchset - not on top of a
> clean 4.5-rc3.

sorry about that. I took the wrong directory.

>
> Anyway - after some fixing I get the following outputs -
> the amba-pl011 driver is still patched with debug messages:
>
> After boot with aux-uart as tty:
> root at raspcm:~# dmesg | grep bcm2835
> [ 0.018172] bcm2835: system timer (irq = 27)
> [ 0.711388] bcm2835-aux-uart 20215040.serial: could not get clk: -517
> [ 1.649124] bcm2835-rng 20104000.rng: hwrng registered
> [ 2.051190] bcm2835_clock_on: clk still busy from 6
> [ 2.056159] bcm2835_clock_on: clk now busy from 9

I get the same output here. It belongs to mmc clock.

> [ 8.062743] bcm2835-wdt 20100000.watchdog: Broadcom BCM2835 watchdog timer
> [ 12.042105] bcm2835-i2s 20203000.i2s: can't request region for resource [mem
> 0x20101098-0x20101099]
> [ 12.191701] bcm2835-i2s: probe of 20203000.i2s failed with error -16
>
> Loading the module:
> root at raspcm:~# modprobe amba-pl011
> [ 106.685812] Serial: AMBA PL011 UART driver
> [ 106.693702] uart-pl011 20201000.uart: pl011_setup_port: f0201000 20201000
> [ 106.702109] 20201000.uart: ttyAMA0 at MMIO 0x20201000 (irq = 81, base_baud =
> 0) is a PL011 rev2
> root at raspcm:~#
>
> and starting getty:
> root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
> [ 137.851123] pl011_startup - start
> [ 137.854538] pl011_hwinit - prepare-enable
> [ 137.858772] bcm2835_clock_on: clk still busy from 6
> [ 137.863755] bcm2835_clock_on: clk now busy from 9
> [ 137.868590] pl011_hwinit - prepare-enable - ret = 0
> [ 137.875249] uart-pl011 20201000.uart: no DMA platform data
> [ 137.880940] pl011_startup - exit
> [ 137.888840] pl011_shutdown - start
> [ 137.892360] pl011_shutdown - disable_unprepare
> [ 137.896933] bcm2835_clock_off: clk still busy from 9
> [ 137.902112] pl011_shutdown - exit
> [ 137.907233] pl011_startup - start
> [ 137.910713] pl011_hwinit - prepare-enable
> [ 137.914808] bcm2835_pll_on: PLL still locked from 1
> [ 138.019876] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
> [ 138.026000] pl011_hwinit - prepare-enable - ret = -110
> [ 138.031266] pl011_startup - error = -110 - disable_unprepare
> [ 138.037048] ------------[ cut here ]------------
> [ 138.041777] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:680
> clk_core_disable+0x34/0xf0()
> [ 138.051063] ---[ end trace dd2f225b2af4c32c ]---
> [ 138.055859] ------------[ cut here ]------------
> [ 138.060627] WARNING: CPU: 0 PID: 2377 at drivers/clk/clk.c:575
> clk_core_unprepare+0x34/0x110()
> [ 138.070321] ---[ end trace dd2f225b2af4c32d ]---
>
> No HDMI output - this time no ?flashing? - just no signal.
> Machine is crashed - the attached AXIS USB network card (0b95:772b)
> just transmits identical packets on the network without stopping?
>

According to the datasheet busy bit shouldn't be set while changing the clock.
So this isn't good. I hope this could be fixed, too.

Regards

>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-13 20:45                 ` Stefan Wahren
@ 2016-02-16 16:31                   ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-16 16:31 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel


> On 13.02.2016, at 21:45, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> 
> According to the datasheet busy bit shouldn't be set while changing the clock.
> So this isn't good. I hope this could be fixed, too.
> 

I had hoped that the patch by Eric:
"clk: bcm2835: Fix setting of PLL divider clock rates"
would fix the issue, but that was unfortunately not the case.

Some more investigation using the debugfs patch I had posted,
which exposes the registers showed that:
plld_per_a2w had a value of 4 before starting getty
and a value of 0 after starting getty.

That made me investigate why this value was changed and I found
the culprit in the form of:
@@ -1167,7 +1171,9 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
        cprman_write(cprman, data->cm_reg,
                     (cprman_read(cprman, data->cm_reg) &
                      ~data->load_mask) | data->hold_mask);
-       cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
+       cprman_write(cprman, data->a2w_reg,
+                    cprman_read(cprman, data->a2w_reg) |
+                       A2W_PLL_CHANNEL_DISABLE);
 }

after that (with plld itself marked as “prepared” during probe) everything is
working fine.

bcm2835_pll_off shows a similar pattern, but patching that
the same way:
@@ -955,8 +955,12 @@ static void bcm2835_pll_off(struct clk_hw *hw)
        struct bcm2835_cprman *cprman = pll->cprman;
        const struct bcm2835_pll_data *data = pll->data;

-       cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
-       cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
+       cprman_write(cprman, data->cm_ctrl_reg,
+                    cprman_read(cprman, data->cm_ctrl_reg) |
+                    CM_PLL_ANARST);
+       cprman_write(cprman, data->a2w_ctrl_reg,
+                    cprman_read(cprman, data->a2w_ctrl_reg) |
+                    A2W_PLL_CTRL_PWRDN);
 }

does not resolve the issue - the system crashes - but without he pll
lock message…

With some debugging messages in place (the register names are
added as a comments indicated via >):

the complete list of writes to the clocks during boot:
[    2.021473] bcm2835_pll_on - pllc - start
[    2.025635] cprman_write - 0x0108 = 0x00000228
> CM_PLLC
[    2.030313] bcm2835_pll_on - pllc - before_wait_pll_lock
[    2.035773] bcm2835_pll_on - pllc - end
[    2.039785] cprman_write - 0x1620 = 0x00000002
> A2W_PLLC_CORE0
[    2.044373] cprman_write - 0x0108 = 0x00000228
> CM_PLLC
[    2.049430] console [ttyS0] disabled
[    2.053237] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
[    2.061878] console [ttyS0] enabled
[    2.069085] bootconsole [earlycon0] disabled
[    2.078668] cprman_write - 0x1520 = 0x00000002
> A2W_PLLC_PER
[    2.083201] cprman_write - 0x0108 = 0x00000228
> CM_PLLC
[    2.087801] cprman_write - 0x01c0 = 0x000002d5
> CM_EMMCCTL
[    2.136432] mmc0: SDHCI controller on 20300000.sdhci [20300000.sdhci] using PIO

dumping the registers for PLLD 
root@raspcm:~# cat /sys/kernel/debug/clk/plld/regdump
cm_ctrl = 0x0000020a
> CM_PLLD
a2w_ctrl = 0x00021034
> A2W_PLLD_PER
frac = 0x00015554
> A2W_PLLD_FRAC
ana0 = 0x00000000
> A2W_PLLD_ANA0
ana1 = 0x00144000
> A2W_PLLD_ANA1
ana2 = 0x00000000
> A2W_PLLD_ANA2
ana3 = 0x00000100
> A2W_PLLD_ANA3

running getty:
root@raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   71.110032] pl011_startup - start
[   71.113452] pl011_hwinit - prepare-enable
[   71.117656] bcm2835_pll_on - plld - start
[   71.121750] cprman_write - 0x010c = 0x0000020a
> CM_PLLD
[   71.126274] bcm2835_pll_on - plld - before_wait_pll_lock
[   71.131728] bcm2835_pll_on - plld - end
[   71.135648] cprman_write - 0x1540 = 0x00000004
> A2W_PLLD_PER
[   71.140228] cprman_write - 0x010c = 0x0000020a
> CM_PLLD
[   71.144760] cprman_write - 0x00f0 = 0x000002d6
> CM_UARTCTL
[   71.149340] pl011_hwinit - prepare-enable - ret = 0
[   71.156743] pl011_startup - exit
[   71.163324] pl011_shutdown - start
[   71.168032] pl011_shutdown - disable_unprepare
[   71.172603] cprman_write - 0x00f0 = 0x00000286
> CM_UARTCTL
[   71.177269] cprman_write - 0x010c = 0x0000028a
> CM_PLLD
[   71.181809] cprman_write - 0x1540 = 0x00000104
> A2W_PLLD_PER
[   71.186334] bcm2835_pll_off - plld - start
[   71.190547] cprman_write - 0x010c = 0x0000038a
> CM_PLLD
[   71.195088] cprman_write - 0x1140 = 0x00031034
> A2W_PLLD_CTRL
[   71.199643] bcm2835_pll_off - plld - end
[   71.203643] pl011_shutdown - exit
[   71.209122] pl011_startup - start
[   71.212522] pl011_hwinit - prepare-enable
[   71.216695] bcm2835_pll_on - plld - start
[   71.220801] cprman_write - 0x010c = 0x0000028a
>  CM_PLLD
[   71.225326] bcm2835_pll_on - plld - before_wait_pll_lock
[   71.230775] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.235840] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.240926] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.245983] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.251067] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.256123] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.261206] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.266272] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.271364] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.276419] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.281523] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.286691] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.291753] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.296838] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.301937] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.307030] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.312087] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.317180] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.322235] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.327319] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.332387] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[   71.338532] bcm2835_pll_on - plld - timeout
[   71.342801] pl011_hwinit - prepare-enable - ret = -110
[   71.348080] pl011_startup - error = -110 - disable_unprepare
[   71.353862] ------------[ cut here ]------------
[   71.358593] WARNING: CPU: 0 PID: 2339 at drivers/clk/clk.c:680 clk_core_disab
le+0x34/0xf0()
[   71.367840] ---[ end trace f080e315d858793e ]---
[   71.372614] ------------[ cut here ]------------
[   71.377408] WARNING: CPU: 0 PID: 2339 at drivers/clk/clk.c:575 clk_core_unpre
pare+0x34/0x110()
[   71.387073] ---[ end trace f080e315d858793f ]—

no more activity - system crashed - HDMI looses signal
So disabling PLL itself seems not to be recommended

I could create a patch with the following content as a bandaid:
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 5293338..4dfb8e3 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -37,6 +37,7 @@
  * generator).
  */

+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
 #include <linux/clk/bcm2835.h>
@@ -1750,6 +1767,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
			clks[i] = desc->clk_register(cprman, desc->data);
 	}
 
+	/* prepare PLLD, so that it never gets unprepared */
+	clk_prepare(clks[BCM2835_PLLD]);
+
 	return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
 				   &cprman->onecell);
 }

Note that this this needed on top of the patch to bcm2835_pll_divider_off,
which is applied in the kernel used during reporting...

Martin

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-16 16:31                   ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-16 16:31 UTC (permalink / raw)
  To: linux-arm-kernel


> On 13.02.2016, at 21:45, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> 
> According to the datasheet busy bit shouldn't be set while changing the clock.
> So this isn't good. I hope this could be fixed, too.
> 

I had hoped that the patch by Eric:
"clk: bcm2835: Fix setting of PLL divider clock rates"
would fix the issue, but that was unfortunately not the case.

Some more investigation using the debugfs patch I had posted,
which exposes the registers showed that:
plld_per_a2w had a value of 4 before starting getty
and a value of 0 after starting getty.

That made me investigate why this value was changed and I found
the culprit in the form of:
@@ -1167,7 +1171,9 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
        cprman_write(cprman, data->cm_reg,
                     (cprman_read(cprman, data->cm_reg) &
                      ~data->load_mask) | data->hold_mask);
-       cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
+       cprman_write(cprman, data->a2w_reg,
+                    cprman_read(cprman, data->a2w_reg) |
+                       A2W_PLL_CHANNEL_DISABLE);
 }

after that (with plld itself marked as ?prepared? during probe) everything is
working fine.

bcm2835_pll_off shows a similar pattern, but patching that
the same way:
@@ -955,8 +955,12 @@ static void bcm2835_pll_off(struct clk_hw *hw)
        struct bcm2835_cprman *cprman = pll->cprman;
        const struct bcm2835_pll_data *data = pll->data;

-       cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
-       cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
+       cprman_write(cprman, data->cm_ctrl_reg,
+                    cprman_read(cprman, data->cm_ctrl_reg) |
+                    CM_PLL_ANARST);
+       cprman_write(cprman, data->a2w_ctrl_reg,
+                    cprman_read(cprman, data->a2w_ctrl_reg) |
+                    A2W_PLL_CTRL_PWRDN);
 }

does not resolve the issue - the system crashes - but without he pll
lock message?

With some debugging messages in place (the register names are
added as a comments indicated via >):

the complete list of writes to the clocks during boot:
[    2.021473] bcm2835_pll_on - pllc - start
[    2.025635] cprman_write - 0x0108 = 0x00000228
> CM_PLLC
[    2.030313] bcm2835_pll_on - pllc - before_wait_pll_lock
[    2.035773] bcm2835_pll_on - pllc - end
[    2.039785] cprman_write - 0x1620 = 0x00000002
> A2W_PLLC_CORE0
[    2.044373] cprman_write - 0x0108 = 0x00000228
> CM_PLLC
[    2.049430] console [ttyS0] disabled
[    2.053237] 20215040.serial: ttyS0 at MMIO 0x0 (irq = 53, base_baud = 31224999) is a 16550
[    2.061878] console [ttyS0] enabled
[    2.069085] bootconsole [earlycon0] disabled
[    2.078668] cprman_write - 0x1520 = 0x00000002
> A2W_PLLC_PER
[    2.083201] cprman_write - 0x0108 = 0x00000228
> CM_PLLC
[    2.087801] cprman_write - 0x01c0 = 0x000002d5
> CM_EMMCCTL
[    2.136432] mmc0: SDHCI controller on 20300000.sdhci [20300000.sdhci] using PIO

dumping the registers for PLLD 
root at raspcm:~# cat /sys/kernel/debug/clk/plld/regdump
cm_ctrl = 0x0000020a
> CM_PLLD
a2w_ctrl = 0x00021034
> A2W_PLLD_PER
frac = 0x00015554
> A2W_PLLD_FRAC
ana0 = 0x00000000
> A2W_PLLD_ANA0
ana1 = 0x00144000
> A2W_PLLD_ANA1
ana2 = 0x00000000
> A2W_PLLD_ANA2
ana3 = 0x00000100
> A2W_PLLD_ANA3

running getty:
root at raspcm:~# /sbin/getty -a root -L ttyAMA0 115200 vt100
[   71.110032] pl011_startup - start
[   71.113452] pl011_hwinit - prepare-enable
[   71.117656] bcm2835_pll_on - plld - start
[   71.121750] cprman_write - 0x010c = 0x0000020a
> CM_PLLD
[   71.126274] bcm2835_pll_on - plld - before_wait_pll_lock
[   71.131728] bcm2835_pll_on - plld - end
[   71.135648] cprman_write - 0x1540 = 0x00000004
> A2W_PLLD_PER
[   71.140228] cprman_write - 0x010c = 0x0000020a
> CM_PLLD
[   71.144760] cprman_write - 0x00f0 = 0x000002d6
> CM_UARTCTL
[   71.149340] pl011_hwinit - prepare-enable - ret = 0
[   71.156743] pl011_startup - exit
[   71.163324] pl011_shutdown - start
[   71.168032] pl011_shutdown - disable_unprepare
[   71.172603] cprman_write - 0x00f0 = 0x00000286
> CM_UARTCTL
[   71.177269] cprman_write - 0x010c = 0x0000028a
> CM_PLLD
[   71.181809] cprman_write - 0x1540 = 0x00000104
> A2W_PLLD_PER
[   71.186334] bcm2835_pll_off - plld - start
[   71.190547] cprman_write - 0x010c = 0x0000038a
> CM_PLLD
[   71.195088] cprman_write - 0x1140 = 0x00031034
> A2W_PLLD_CTRL
[   71.199643] bcm2835_pll_off - plld - end
[   71.203643] pl011_shutdown - exit
[   71.209122] pl011_startup - start
[   71.212522] pl011_hwinit - prepare-enable
[   71.216695] bcm2835_pll_on - plld - start
[   71.220801] cprman_write - 0x010c = 0x0000028a
>  CM_PLLD
[   71.225326] bcm2835_pll_on - plld - before_wait_pll_lock
[   71.230775] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.235840] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.240926] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.245983] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.251067] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.256123] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.261206] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.266272] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.271364] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.276419] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.281523] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.286691] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.291753] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.296838] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.301937] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.307030] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.312087] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.317180] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.322235] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.327319] bcm2835_pll_on - plld - in_pll_lock_loop
[   71.332387] bcm2835-clk 20101000.cprman: plld: couldn't lock PLL
[   71.338532] bcm2835_pll_on - plld - timeout
[   71.342801] pl011_hwinit - prepare-enable - ret = -110
[   71.348080] pl011_startup - error = -110 - disable_unprepare
[   71.353862] ------------[ cut here ]------------
[   71.358593] WARNING: CPU: 0 PID: 2339 at drivers/clk/clk.c:680 clk_core_disab
le+0x34/0xf0()
[   71.367840] ---[ end trace f080e315d858793e ]---
[   71.372614] ------------[ cut here ]------------
[   71.377408] WARNING: CPU: 0 PID: 2339 at drivers/clk/clk.c:575 clk_core_unpre
pare+0x34/0x110()
[   71.387073] ---[ end trace f080e315d858793f ]?

no more activity - system crashed - HDMI looses signal
So disabling PLL itself seems not to be recommended

I could create a patch with the following content as a bandaid:
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 5293338..4dfb8e3 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -37,6 +37,7 @@
  * generator).
  */

+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
 #include <linux/clk/bcm2835.h>
@@ -1750,6 +1767,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
			clks[i] = desc->clk_register(cprman, desc->data);
 	}
 
+	/* prepare PLLD, so that it never gets unprepared */
+	clk_prepare(clks[BCM2835_PLLD]);
+
 	return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
 				   &cprman->onecell);
 }

Note that this this needed on top of the patch to bcm2835_pll_divider_off,
which is applied in the kernel used during reporting...

Martin

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-13 11:24               ` Martin Sperl
  (?)
@ 2016-02-16 18:57                 ` Michael Turquette
  -1 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-02-16 18:57 UTC (permalink / raw)
  To: Martin Sperl, Stefan Wahren
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel

Hi Martin & Stefan,

Quoting Martin Sperl (2016-02-13 03:24:43)
> =

> > On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > =

> > Hi Martin,
> > =

> >> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
> >> geschrieben:
> >> =

> >> So the issue is triggered as soon as the plld_per
> >> pll divider gets disabled/reenabled.
> >> =

> >> This happens because the clk_hw.core.prepare_count
> >> drops down to 0 and then unprepare is called.
> >> =

> >> So we need to increase the ref-count for the pll
> >> and pll_dividers to at least 1 so that these never
> >> get disabled - at least for now until we can come
> >> up with a better contract with the firmware.
> >> =

> >> Obviously this may impact other drivers as well
> >> where a pll is used for the first time - if nothing
> >> else uses it and the clock gets released, then
> >> the clock would trigger a unprepare of the whole
> >> branch of the clock tree.
> >> =

> >> The question is: how can we solve it in an acceptable
> >> manner?
> >> =

> >> Do we need a driver that just holds a reference to
> >> those clocks? Or should we just prepare the clock
> >> after registering it in clk-bcm2835.c?
> >> =

> >> As for why does this not show up when compiled in?
> >> It seems that in that case the amba_pl011 driver
> >> never gets removed and then probed again.
> >> =

> >> This is possibly related to the optional use of DMA,
> >> with the amba-pl011 driver that retries the install,
> >> which is not supported on the bcm2835 - at least that
> >> is what the datasheet says. And DMA is (probably) not
> >> enabled during the early boot stages, so it does not
> >> fail once when it tries to register DMA.
> >> =

> >> Thanks,
> >> Martin
> >> =

> > =

> > i think i must correct myself. The fixed apb should stay since there is=
 no
> > dynamic replacement and a proper disabling of plld shouldn't cause this=
 freeze.
> > =

> > I have the suspicion the freeze is caused by a clock glitch or lock-up.
> > =

> > Could you please revert your changes and apply the attached patch? Mayb=
e we can
> > see more.
> =

> I will give it a try.
> =

> But to me it seems as if the disabling of PLLD produces a hickup in the =

> firmware (which relies on PLLD-DIV and thus PLL).
> =

> This would also explain the 3-4 second bar/blanking pattern seen on HDMI,
> which I would guess is related to a reset loop...
> =

> I got a simple fix for now, that just prepares a few clocks/pll/pll_divs
> during probing, so that they will never get unprepared/fully stopped):
> =

> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 015e687..fe0c401 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -37,6 +37,7 @@
>   * generator).
>   */
> =

> +#include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clkdev.h>
>  #include <linux/clk/bcm2835.h>
> @@ -1504,6 +1505,7 @@ static int bcm2835_clk_probe(struct platform_device=
 *pdev)
>         struct clk **clks;
>         struct bcm2835_cprman *cprman;
>         struct resource *res;
> +       int ret;
> =

>         cprman =3D devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
>         if (!cprman)
> @@ -1597,6 +1599,37 @@ static int bcm2835_clk_probe(struct platform_devic=
e *pdev
>         clks[BCM2835_CLOCK_PWM] =3D
>                 bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
> =

> +       /*
> +        * Several PLLs, PLL-dividers and clocks need to get prepared
> +        * so that they are never unprepared and released.
> +        * We run this separately as there may be dependencies that would
> +        * need to be fullfilled first...
> +        * Note: Especially unpreparing PLLD_PER will kill the system
> +        *       (even if it is prepared again almost immediately
> +        *        the HDMI display will fail)
> +        */
> +#define CLK_PREPARE_INITIAL(clkid)                                     \
> +       if (clks[clkid] && (ret =3D clk_prepare(clks[clkid])))           =
 \
> +               dev_err(dev, "could not prepare clock %pC - %d\n",      \
> +                       clks[clkid], ret);

You should not only prepare the clock, but enable it as well, even if
your .enable callback is no-op. s/clk_prepare/clk_prepare_enable/ should
do it. But instead of all of this stuff below...

> +       CLK_PREPARE_INITIAL(BCM2835_PLLA);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLB);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLA_CORE);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLA_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE0);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE1);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE2);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD_CORE);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_RCAL);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_AUX);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_PIX);
> +       CLK_PREPARE_INITIAL(BCM2835_CLOCK_VPU);

... how about using the shiny new HAND_OFF clocks feature? See:

http://lkml.kernel.org/r/<1455225554-13267-1-git-send-email-mturquette@bayl=
ibre.com>

It seems like you need clocks to remain enabled from the time they are
registered until a clk consumer driver claims them and Does The Right
Thing. (e.g. only gates them when it is safe to do so, such as display
being turned off by user).

Let me know if those patches will help you. The first three patches in
the series introduce CRITICAL clocks, which never gate. Patches 4-6 are
a solution that I prefer, allowing the clocks to remain enabled by the
bootloader, and only gated later on when a consumer driver claims them.
No changes to clock consumer drivers are required, the framework handles
the transition of reference counts and the clock provider driver only
has to set the CLK_ENABLE_HAND_OFF flag for the affected clocks.

Regards,
Mike

> +
>         return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
>                                    &cprman->onecell);
>  }
> =

> And that resolves the issue and also allows loading/unloading of the
> amba-pl011 module as well without any hickups.
> =

> I guess it would be better if we did make this configurable in the DT.
> (See patch 1 of my RFC patchset, which could get extended to handle this
> as well - at least a portion thereof).
> =

> But in more general terms: we need to come up with some agreement
> on ownership of the clocks between linux and the firmware and how
> to change them without impacting the other. =

> =

> One example is the =E2=80=9Coverclocking=E2=80=9D that currently happens =
for
> downstream kernels in the firmware, which impacts some of the
> plls and PLLdivs as well as the sram control registers.
> =

> The reason for this seems to be that for overclocking the SRAM
> parameters also need to get changed and that can only get done
> reliably when running the code from L1/L2 caches - and that
> requirement of not touching SRAM while changing the SRAM parameters
> is probably hard to achieve cleanly on ARM alone.
> =

> Ciao,
>         Martin
> =

> P.s: unloading amba_pl011 gives the following error:
> root@raspcm:~# rmmod amba_pl011
> [  142.318263] Trying to free nonexistent resource <0000000020201000-0000=
000020201fff>
> but that is an issue not related to clock...
> =

> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-16 18:57                 ` Michael Turquette
  0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-02-16 18:57 UTC (permalink / raw)
  To: Martin Sperl, Stefan Wahren
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel

Hi Martin & Stefan,

Quoting Martin Sperl (2016-02-13 03:24:43)
> 
> > On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > 
> > Hi Martin,
> > 
> >> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
> >> geschrieben:
> >> 
> >> So the issue is triggered as soon as the plld_per
> >> pll divider gets disabled/reenabled.
> >> 
> >> This happens because the clk_hw.core.prepare_count
> >> drops down to 0 and then unprepare is called.
> >> 
> >> So we need to increase the ref-count for the pll
> >> and pll_dividers to at least 1 so that these never
> >> get disabled - at least for now until we can come
> >> up with a better contract with the firmware.
> >> 
> >> Obviously this may impact other drivers as well
> >> where a pll is used for the first time - if nothing
> >> else uses it and the clock gets released, then
> >> the clock would trigger a unprepare of the whole
> >> branch of the clock tree.
> >> 
> >> The question is: how can we solve it in an acceptable
> >> manner?
> >> 
> >> Do we need a driver that just holds a reference to
> >> those clocks? Or should we just prepare the clock
> >> after registering it in clk-bcm2835.c?
> >> 
> >> As for why does this not show up when compiled in?
> >> It seems that in that case the amba_pl011 driver
> >> never gets removed and then probed again.
> >> 
> >> This is possibly related to the optional use of DMA,
> >> with the amba-pl011 driver that retries the install,
> >> which is not supported on the bcm2835 - at least that
> >> is what the datasheet says. And DMA is (probably) not
> >> enabled during the early boot stages, so it does not
> >> fail once when it tries to register DMA.
> >> 
> >> Thanks,
> >> Martin
> >> 
> > 
> > i think i must correct myself. The fixed apb should stay since there is no
> > dynamic replacement and a proper disabling of plld shouldn't cause this freeze.
> > 
> > I have the suspicion the freeze is caused by a clock glitch or lock-up.
> > 
> > Could you please revert your changes and apply the attached patch? Maybe we can
> > see more.
> 
> I will give it a try.
> 
> But to me it seems as if the disabling of PLLD produces a hickup in the 
> firmware (which relies on PLLD-DIV and thus PLL).
> 
> This would also explain the 3-4 second bar/blanking pattern seen on HDMI,
> which I would guess is related to a reset loop...
> 
> I got a simple fix for now, that just prepares a few clocks/pll/pll_divs
> during probing, so that they will never get unprepared/fully stopped):
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 015e687..fe0c401 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -37,6 +37,7 @@
>   * generator).
>   */
> 
> +#include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clkdev.h>
>  #include <linux/clk/bcm2835.h>
> @@ -1504,6 +1505,7 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>         struct clk **clks;
>         struct bcm2835_cprman *cprman;
>         struct resource *res;
> +       int ret;
> 
>         cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
>         if (!cprman)
> @@ -1597,6 +1599,37 @@ static int bcm2835_clk_probe(struct platform_device *pdev
>         clks[BCM2835_CLOCK_PWM] =
>                 bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
> 
> +       /*
> +        * Several PLLs, PLL-dividers and clocks need to get prepared
> +        * so that they are never unprepared and released.
> +        * We run this separately as there may be dependencies that would
> +        * need to be fullfilled first...
> +        * Note: Especially unpreparing PLLD_PER will kill the system
> +        *       (even if it is prepared again almost immediately
> +        *        the HDMI display will fail)
> +        */
> +#define CLK_PREPARE_INITIAL(clkid)                                     \
> +       if (clks[clkid] && (ret = clk_prepare(clks[clkid])))            \
> +               dev_err(dev, "could not prepare clock %pC - %d\n",      \
> +                       clks[clkid], ret);

You should not only prepare the clock, but enable it as well, even if
your .enable callback is no-op. s/clk_prepare/clk_prepare_enable/ should
do it. But instead of all of this stuff below...

> +       CLK_PREPARE_INITIAL(BCM2835_PLLA);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLB);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLA_CORE);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLA_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE0);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE1);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE2);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD_CORE);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_RCAL);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_AUX);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_PIX);
> +       CLK_PREPARE_INITIAL(BCM2835_CLOCK_VPU);

... how about using the shiny new HAND_OFF clocks feature? See:

http://lkml.kernel.org/r/<1455225554-13267-1-git-send-email-mturquette@baylibre.com>

It seems like you need clocks to remain enabled from the time they are
registered until a clk consumer driver claims them and Does The Right
Thing. (e.g. only gates them when it is safe to do so, such as display
being turned off by user).

Let me know if those patches will help you. The first three patches in
the series introduce CRITICAL clocks, which never gate. Patches 4-6 are
a solution that I prefer, allowing the clocks to remain enabled by the
bootloader, and only gated later on when a consumer driver claims them.
No changes to clock consumer drivers are required, the framework handles
the transition of reference counts and the clock provider driver only
has to set the CLK_ENABLE_HAND_OFF flag for the affected clocks.

Regards,
Mike

> +
>         return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
>                                    &cprman->onecell);
>  }
> 
> And that resolves the issue and also allows loading/unloading of the
> amba-pl011 module as well without any hickups.
> 
> I guess it would be better if we did make this configurable in the DT.
> (See patch 1 of my RFC patchset, which could get extended to handle this
> as well - at least a portion thereof).
> 
> But in more general terms: we need to come up with some agreement
> on ownership of the clocks between linux and the firmware and how
> to change them without impacting the other. 
> 
> One example is the “overclocking” that currently happens for
> downstream kernels in the firmware, which impacts some of the
> plls and PLLdivs as well as the sram control registers.
> 
> The reason for this seems to be that for overclocking the SRAM
> parameters also need to get changed and that can only get done
> reliably when running the code from L1/L2 caches - and that
> requirement of not touching SRAM while changing the SRAM parameters
> is probably hard to achieve cleanly on ARM alone.
> 
> Ciao,
>         Martin
> 
> P.s: unloading amba_pl011 gives the following error:
> root@raspcm:~# rmmod amba_pl011
> [  142.318263] Trying to free nonexistent resource <0000000020201000-0000000020201fff>
> but that is an issue not related to clock...
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-16 18:57                 ` Michael Turquette
  0 siblings, 0 replies; 33+ messages in thread
From: Michael Turquette @ 2016-02-16 18:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Martin & Stefan,

Quoting Martin Sperl (2016-02-13 03:24:43)
> 
> > On 13.02.2016, at 11:01, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > 
> > Hi Martin,
> > 
> >> Martin Sperl <kernel@martin.sperl.org> hat am 12. Februar 2016 um 20:44
> >> geschrieben:
> >> 
> >> So the issue is triggered as soon as the plld_per
> >> pll divider gets disabled/reenabled.
> >> 
> >> This happens because the clk_hw.core.prepare_count
> >> drops down to 0 and then unprepare is called.
> >> 
> >> So we need to increase the ref-count for the pll
> >> and pll_dividers to at least 1 so that these never
> >> get disabled - at least for now until we can come
> >> up with a better contract with the firmware.
> >> 
> >> Obviously this may impact other drivers as well
> >> where a pll is used for the first time - if nothing
> >> else uses it and the clock gets released, then
> >> the clock would trigger a unprepare of the whole
> >> branch of the clock tree.
> >> 
> >> The question is: how can we solve it in an acceptable
> >> manner?
> >> 
> >> Do we need a driver that just holds a reference to
> >> those clocks? Or should we just prepare the clock
> >> after registering it in clk-bcm2835.c?
> >> 
> >> As for why does this not show up when compiled in?
> >> It seems that in that case the amba_pl011 driver
> >> never gets removed and then probed again.
> >> 
> >> This is possibly related to the optional use of DMA,
> >> with the amba-pl011 driver that retries the install,
> >> which is not supported on the bcm2835 - at least that
> >> is what the datasheet says. And DMA is (probably) not
> >> enabled during the early boot stages, so it does not
> >> fail once when it tries to register DMA.
> >> 
> >> Thanks,
> >> Martin
> >> 
> > 
> > i think i must correct myself. The fixed apb should stay since there is no
> > dynamic replacement and a proper disabling of plld shouldn't cause this freeze.
> > 
> > I have the suspicion the freeze is caused by a clock glitch or lock-up.
> > 
> > Could you please revert your changes and apply the attached patch? Maybe we can
> > see more.
> 
> I will give it a try.
> 
> But to me it seems as if the disabling of PLLD produces a hickup in the 
> firmware (which relies on PLLD-DIV and thus PLL).
> 
> This would also explain the 3-4 second bar/blanking pattern seen on HDMI,
> which I would guess is related to a reset loop...
> 
> I got a simple fix for now, that just prepares a few clocks/pll/pll_divs
> during probing, so that they will never get unprepared/fully stopped):
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 015e687..fe0c401 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -37,6 +37,7 @@
>   * generator).
>   */
> 
> +#include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clkdev.h>
>  #include <linux/clk/bcm2835.h>
> @@ -1504,6 +1505,7 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>         struct clk **clks;
>         struct bcm2835_cprman *cprman;
>         struct resource *res;
> +       int ret;
> 
>         cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL);
>         if (!cprman)
> @@ -1597,6 +1599,37 @@ static int bcm2835_clk_probe(struct platform_device *pdev
>         clks[BCM2835_CLOCK_PWM] =
>                 bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data);
> 
> +       /*
> +        * Several PLLs, PLL-dividers and clocks need to get prepared
> +        * so that they are never unprepared and released.
> +        * We run this separately as there may be dependencies that would
> +        * need to be fullfilled first...
> +        * Note: Especially unpreparing PLLD_PER will kill the system
> +        *       (even if it is prepared again almost immediately
> +        *        the HDMI display will fail)
> +        */
> +#define CLK_PREPARE_INITIAL(clkid)                                     \
> +       if (clks[clkid] && (ret = clk_prepare(clks[clkid])))            \
> +               dev_err(dev, "could not prepare clock %pC - %d\n",      \
> +                       clks[clkid], ret);

You should not only prepare the clock, but enable it as well, even if
your .enable callback is no-op. s/clk_prepare/clk_prepare_enable/ should
do it. But instead of all of this stuff below...

> +       CLK_PREPARE_INITIAL(BCM2835_PLLA);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLB);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLA_CORE);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLA_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE0);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE1);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE2);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLC_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD_CORE);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLD_PER);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_RCAL);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_AUX);
> +       CLK_PREPARE_INITIAL(BCM2835_PLLH_PIX);
> +       CLK_PREPARE_INITIAL(BCM2835_CLOCK_VPU);

... how about using the shiny new HAND_OFF clocks feature? See:

http://lkml.kernel.org/r/<1455225554-13267-1-git-send-email-mturquette@baylibre.com>

It seems like you need clocks to remain enabled from the time they are
registered until a clk consumer driver claims them and Does The Right
Thing. (e.g. only gates them when it is safe to do so, such as display
being turned off by user).

Let me know if those patches will help you. The first three patches in
the series introduce CRITICAL clocks, which never gate. Patches 4-6 are
a solution that I prefer, allowing the clocks to remain enabled by the
bootloader, and only gated later on when a consumer driver claims them.
No changes to clock consumer drivers are required, the framework handles
the transition of reference counts and the clock provider driver only
has to set the CLK_ENABLE_HAND_OFF flag for the affected clocks.

Regards,
Mike

> +
>         return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
>                                    &cprman->onecell);
>  }
> 
> And that resolves the issue and also allows loading/unloading of the
> amba-pl011 module as well without any hickups.
> 
> I guess it would be better if we did make this configurable in the DT.
> (See patch 1 of my RFC patchset, which could get extended to handle this
> as well - at least a portion thereof).
> 
> But in more general terms: we need to come up with some agreement
> on ownership of the clocks between linux and the firmware and how
> to change them without impacting the other. 
> 
> One example is the ?overclocking? that currently happens for
> downstream kernels in the firmware, which impacts some of the
> plls and PLLdivs as well as the sram control registers.
> 
> The reason for this seems to be that for overclocking the SRAM
> parameters also need to get changed and that can only get done
> reliably when running the code from L1/L2 caches - and that
> requirement of not touching SRAM while changing the SRAM parameters
> is probably hard to achieve cleanly on ARM alone.
> 
> Ciao,
>         Martin
> 
> P.s: unloading amba_pl011 gives the following error:
> root at raspcm:~# rmmod amba_pl011
> [  142.318263] Trying to free nonexistent resource <0000000020201000-0000000020201fff>
> but that is an issue not related to clock...
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-16 16:31                   ` Martin Sperl
@ 2016-02-16 19:29                     ` Stefan Wahren
  -1 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-16 19:29 UTC (permalink / raw)
  To: Martin Sperl
  Cc: linux-serial, linux-clk, Eric Anholt, linux-arm-kernel, linux-rpi-kernel

Hi Martin,

> Martin Sperl <kernel@martin.sperl.org> hat am 16. Februar 2016 um 17:31
> geschrieben:
>
>
>
> > On 13.02.2016, at 21:45, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >
> >
> > According to the datasheet busy bit shouldn't be set while changing the
> > clock.
> > So this isn't good. I hope this could be fixed, too.
> >
>
> I had hoped that the patch by Eric:
> "clk: bcm2835: Fix setting of PLL divider clock rates"
> would fix the issue, but that was unfortunately not the case.
>
> Some more investigation using the debugfs patch I had posted,
> which exposes the registers showed that:
> plld_per_a2w had a value of 4 before starting getty
> and a value of 0 after starting getty.
>
> That made me investigate why this value was changed and I found
> the culprit in the form of:
> @@ -1167,7 +1171,9 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
> cprman_write(cprman, data->cm_reg,
> (cprman_read(cprman, data->cm_reg) &
> ~data->load_mask) | data->hold_mask);
> - cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
> + cprman_write(cprman, data->a2w_reg,
> + cprman_read(cprman, data->a2w_reg) |
> + A2W_PLL_CHANNEL_DISABLE);
> }

good job. This change make sense because the divider value shouldn't be zero.

But maybe we should protect the RMW by a lock like in bcm2835_clock_off?

>
> after that (with plld itself marked as “prepared” during probe) everything is
> working fine.
>
> bcm2835_pll_off shows a similar pattern, but patching that
> the same way:
> @@ -955,8 +955,12 @@ static void bcm2835_pll_off(struct clk_hw *hw)
> struct bcm2835_cprman *cprman = pll->cprman;
> const struct bcm2835_pll_data *data = pll->data;
>
> - cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
> - cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
> + cprman_write(cprman, data->cm_ctrl_reg,
> + cprman_read(cprman, data->cm_ctrl_reg) |
> + CM_PLL_ANARST);
> + cprman_write(cprman, data->a2w_ctrl_reg,
> + cprman_read(cprman, data->a2w_ctrl_reg) |
> + A2W_PLL_CTRL_PWRDN);
> }
>
> does not resolve the issue - the system crashes - but without he pll
> lock message…

Without the knowledge about the registers it's hard to say what exactly causes
the crash.

Regards
Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-16 19:29                     ` Stefan Wahren
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Wahren @ 2016-02-16 19:29 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Martin,

> Martin Sperl <kernel@martin.sperl.org> hat am 16. Februar 2016 um 17:31
> geschrieben:
>
>
>
> > On 13.02.2016, at 21:45, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >
> >
> > According to the datasheet busy bit shouldn't be set while changing the
> > clock.
> > So this isn't good. I hope this could be fixed, too.
> >
>
> I had hoped that the patch by Eric:
> "clk: bcm2835: Fix setting of PLL divider clock rates"
> would fix the issue, but that was unfortunately not the case.
>
> Some more investigation using the debugfs patch I had posted,
> which exposes the registers showed that:
> plld_per_a2w had a value of 4 before starting getty
> and a value of 0 after starting getty.
>
> That made me investigate why this value was changed and I found
> the culprit in the form of:
> @@ -1167,7 +1171,9 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
> cprman_write(cprman, data->cm_reg,
> (cprman_read(cprman, data->cm_reg) &
> ~data->load_mask) | data->hold_mask);
> - cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
> + cprman_write(cprman, data->a2w_reg,
> + cprman_read(cprman, data->a2w_reg) |
> + A2W_PLL_CHANNEL_DISABLE);
> }

good job. This change make sense because the divider value shouldn't be zero.

But maybe we should protect the RMW by a lock like in bcm2835_clock_off?

>
> after that (with plld itself marked as ?prepared? during probe) everything is
> working fine.
>
> bcm2835_pll_off shows a similar pattern, but patching that
> the same way:
> @@ -955,8 +955,12 @@ static void bcm2835_pll_off(struct clk_hw *hw)
> struct bcm2835_cprman *cprman = pll->cprman;
> const struct bcm2835_pll_data *data = pll->data;
>
> - cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
> - cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
> + cprman_write(cprman, data->cm_ctrl_reg,
> + cprman_read(cprman, data->cm_ctrl_reg) |
> + CM_PLL_ANARST);
> + cprman_write(cprman, data->a2w_ctrl_reg,
> + cprman_read(cprman, data->a2w_ctrl_reg) |
> + A2W_PLL_CTRL_PWRDN);
> }
>
> does not resolve the issue - the system crashes - but without he pll
> lock message?

Without the knowledge about the registers it's hard to say what exactly causes
the crash.

Regards
Stefan

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: serial: clk: bcm2835: Strange effects when using aux-uart in console
  2016-02-16 18:57                 ` Michael Turquette
@ 2016-02-17 10:43                   ` Martin Sperl
  -1 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-17 10:43 UTC (permalink / raw)
  To: Michael Turquette, Stefan Wahren
  Cc: linux-serial, linux-clk, Eric Anholt, linux-rpi-kernel, linux-arm-kernel


On 16.02.2016 19:57, Michael Turquette wrote:
> Hi Martin & Stefan,
...
> ... how about using the shiny new HAND_OFF clocks feature? See:
>
> http://lkml.kernel.org/r/<1455225554-13267-1-git-send-email-mturquette@baylibre.com>
>
> It seems like you need clocks to remain enabled from the time they are
> registered until a clk consumer driver claims them and Does The Right
> Thing. (e.g. only gates them when it is safe to do so, such as display
> being turned off by user).
>
> Let me know if those patches will help you. The first three patches in
> the series introduce CRITICAL clocks, which never gate. Patches 4-6 are
> a solution that I prefer, allowing the clocks to remain enabled by the
> bootloader, and only gated later on when a consumer driver claims them.
> No changes to clock consumer drivers are required, the framework handles
> the transition of reference counts and the clock provider driver only
> has to set the CLK_ENABLE_HAND_OFF flag for the affected clocks.

I wonder if those patches would really help in this use-case.

HAND_OFF seems to show an issue around the fact that as soon as a
clock is claimed once it is handled "normally".

So when the amba-pl011 driver requests and prepares the uart clock
the "lower" PLLD_per (divider) clock as well as the PLLD (pll) clock
become "prepared" as they are parents.

So fine so good - and that works right now fine.

The problem is that the amba-pl011 driver when loaded as a module
- for some reason - runs clk_unprepare/disable when the tty is closed.

Using stty instead of getty for simplicity (as getty closes and reopens
the tty - or so it seems):
root@raspcm:~# stty -F /dev/ttyAMA0
[ 94.328906] pl011_startup - start
[ 94.332401] pl011_hwinit - prepare-enable
 > HERE PLLD gets prepared/enabled
[ 94.336499] bcm2835_pll_on - plld - start
[ 94.340642] cprman_write - 0x010c = 0x0000020a
[ 94.345170] bcm2835_pll_on - plld - before_wait_pll_lock
[ 94.350637] bcm2835_pll_on - plld - end
 > HERE PLLD_PER gets prepared/enabled
[ 94.354558] cprman_write - 0x1540 = 0x00000004
[ 94.359097] cprman_write - 0x010c = 0x0000020a
 > HERE UART gets prepares/enabled
[ 94.363660] cprman_write - 0x00f0 = 0x000002d6
[ 94.368203] pl011_hwinit - prepare-enable - ret = 0
[ 94.375212] pl011_startup - exit
OUTPUT BY STTY: speed 9600 baud; line = 0;
OUTPUT BY STTY: -brkint -imaxbel
[ 94.381528] pl011_shutdown - start
[ 94.385722] pl011_shutdown - disable_unprepare
 > HERE UART gets disabled/unprepared
[ 94.391024] cprman_write - 0x00f0 = 0x00000286
 > here PLLD_PER gets disabled/unprepared
[ 94.396037] cprman_write - 0x010c = 0x0000028a
[ 94.400616] cprman_write - 0x1540 = 0x00000104
 > HERE PLL gets disabled/unprepared
[ 94.405143] bcm2835_pll_off - plld - start
[ 94.409316] cprman_write - 0x010c = 0x0000038a
[ 94.413906] cprman_write - 0x1140 = 0x00031034
[ 94.418435] bcm2835_pll_off - plld - end
[ 94.422500] pl011_shutdown - exit
root@raspcm:~#

So with the HAND_OFF the PLLD (as it has been prepared) would follow
thru the "release" code as expected, which results in stopping the
PLLD_per clock and subsequently the PLLD clock.

And this disabling of PLLD is what - as far as I can tell - brings
the system to it knees, so this needs to be avoided

And that is what the HAND_OFF flag would not stop, as the clock - at
that time has been claimed - at least when using it directly on PLLD.

Using CLK_IS_CRITICAL instead would solve that issue, as it just does
the prepare/enable inside the framework once and never unprepares it.

I guess it would also help to understand the clock-tree, but
the information on the clock-tree is scarce (or under NDA),
so it is hard to understand why the PLLD impacts the system
in such a way.

Possibly some of the CPU interfaces are sourced from PLLD
(say PLLD_core) and that is why the CPU stops working.

Things that come to mind are:
* SRAM
* AXI bus
* ARM - core (even though that is supposed to be driven of PLLB)
* ARM - L2 cache
* GPU - run by firmware...

And thus disabling the PLLD would impact those interfaces...
(but I could be wrong)

But - as said - documentation on the clock tree details
is unfortunately lacking...

A bit of digging in the source-code shows that the sdram
clock has the clocks in bcm2835_clock_vpu_parents assigned
as parents.

The regdump for the sdram clock shows:
root@raspcm:/build/linux# cat /sys/kernel/debug/clk/sdram/regdump
ctl = 0x00004006
div = 0x00003000

So we use the 6th clock from the list of vpu clocks. (ctl & 0x0f)
and that is the plld_core clock!

This means that obviously disabling PLLD is NOT a good idea, as it
will effectively disable sdram.

Maybe we actually should be forcing the sdram clock to be always
prepared/enabled manually or via CLK_IS_CRITICAL or CLK_HAND_OFF.

Martin

P.s: As far as I understand the reason why the amba_pl011 driver
compiled in is working is because then the driver is used as
the console, so it is always opened and thus the clocks are
always prepared.

Starting getty at a later stage during the boot will thus never
risk getting the prepare_count of the PLLD to 0 - the kernel
console will still hold the prepare reference and thus we
are at 1.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* serial: clk: bcm2835: Strange effects when using aux-uart in console
@ 2016-02-17 10:43                   ` Martin Sperl
  0 siblings, 0 replies; 33+ messages in thread
From: Martin Sperl @ 2016-02-17 10:43 UTC (permalink / raw)
  To: linux-arm-kernel


On 16.02.2016 19:57, Michael Turquette wrote:
> Hi Martin & Stefan,
...
> ... how about using the shiny new HAND_OFF clocks feature? See:
>
> http://lkml.kernel.org/r/<1455225554-13267-1-git-send-email-mturquette@baylibre.com>
>
> It seems like you need clocks to remain enabled from the time they are
> registered until a clk consumer driver claims them and Does The Right
> Thing. (e.g. only gates them when it is safe to do so, such as display
> being turned off by user).
>
> Let me know if those patches will help you. The first three patches in
> the series introduce CRITICAL clocks, which never gate. Patches 4-6 are
> a solution that I prefer, allowing the clocks to remain enabled by the
> bootloader, and only gated later on when a consumer driver claims them.
> No changes to clock consumer drivers are required, the framework handles
> the transition of reference counts and the clock provider driver only
> has to set the CLK_ENABLE_HAND_OFF flag for the affected clocks.

I wonder if those patches would really help in this use-case.

HAND_OFF seems to show an issue around the fact that as soon as a
clock is claimed once it is handled "normally".

So when the amba-pl011 driver requests and prepares the uart clock
the "lower" PLLD_per (divider) clock as well as the PLLD (pll) clock
become "prepared" as they are parents.

So fine so good - and that works right now fine.

The problem is that the amba-pl011 driver when loaded as a module
- for some reason - runs clk_unprepare/disable when the tty is closed.

Using stty instead of getty for simplicity (as getty closes and reopens
the tty - or so it seems):
root at raspcm:~# stty -F /dev/ttyAMA0
[ 94.328906] pl011_startup - start
[ 94.332401] pl011_hwinit - prepare-enable
 > HERE PLLD gets prepared/enabled
[ 94.336499] bcm2835_pll_on - plld - start
[ 94.340642] cprman_write - 0x010c = 0x0000020a
[ 94.345170] bcm2835_pll_on - plld - before_wait_pll_lock
[ 94.350637] bcm2835_pll_on - plld - end
 > HERE PLLD_PER gets prepared/enabled
[ 94.354558] cprman_write - 0x1540 = 0x00000004
[ 94.359097] cprman_write - 0x010c = 0x0000020a
 > HERE UART gets prepares/enabled
[ 94.363660] cprman_write - 0x00f0 = 0x000002d6
[ 94.368203] pl011_hwinit - prepare-enable - ret = 0
[ 94.375212] pl011_startup - exit
OUTPUT BY STTY: speed 9600 baud; line = 0;
OUTPUT BY STTY: -brkint -imaxbel
[ 94.381528] pl011_shutdown - start
[ 94.385722] pl011_shutdown - disable_unprepare
 > HERE UART gets disabled/unprepared
[ 94.391024] cprman_write - 0x00f0 = 0x00000286
 > here PLLD_PER gets disabled/unprepared
[ 94.396037] cprman_write - 0x010c = 0x0000028a
[ 94.400616] cprman_write - 0x1540 = 0x00000104
 > HERE PLL gets disabled/unprepared
[ 94.405143] bcm2835_pll_off - plld - start
[ 94.409316] cprman_write - 0x010c = 0x0000038a
[ 94.413906] cprman_write - 0x1140 = 0x00031034
[ 94.418435] bcm2835_pll_off - plld - end
[ 94.422500] pl011_shutdown - exit
root at raspcm:~#

So with the HAND_OFF the PLLD (as it has been prepared) would follow
thru the "release" code as expected, which results in stopping the
PLLD_per clock and subsequently the PLLD clock.

And this disabling of PLLD is what - as far as I can tell - brings
the system to it knees, so this needs to be avoided

And that is what the HAND_OFF flag would not stop, as the clock - at
that time has been claimed - at least when using it directly on PLLD.

Using CLK_IS_CRITICAL instead would solve that issue, as it just does
the prepare/enable inside the framework once and never unprepares it.

I guess it would also help to understand the clock-tree, but
the information on the clock-tree is scarce (or under NDA),
so it is hard to understand why the PLLD impacts the system
in such a way.

Possibly some of the CPU interfaces are sourced from PLLD
(say PLLD_core) and that is why the CPU stops working.

Things that come to mind are:
* SRAM
* AXI bus
* ARM - core (even though that is supposed to be driven of PLLB)
* ARM - L2 cache
* GPU - run by firmware...

And thus disabling the PLLD would impact those interfaces...
(but I could be wrong)

But - as said - documentation on the clock tree details
is unfortunately lacking...

A bit of digging in the source-code shows that the sdram
clock has the clocks in bcm2835_clock_vpu_parents assigned
as parents.

The regdump for the sdram clock shows:
root at raspcm:/build/linux# cat /sys/kernel/debug/clk/sdram/regdump
ctl = 0x00004006
div = 0x00003000

So we use the 6th clock from the list of vpu clocks. (ctl & 0x0f)
and that is the plld_core clock!

This means that obviously disabling PLLD is NOT a good idea, as it
will effectively disable sdram.

Maybe we actually should be forcing the sdram clock to be always
prepared/enabled manually or via CLK_IS_CRITICAL or CLK_HAND_OFF.

Martin

P.s: As far as I understand the reason why the amba_pl011 driver
compiled in is working is because then the driver is used as
the console, so it is always opened and thus the clocks are
always prepared.

Starting getty at a later stage during the boot will thus never
risk getting the prepare_count of the PLLD to 0 - the kernel
console will still hold the prepare reference and thus we
are at 1.

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2016-02-17 10:43 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-10 15:12 serial: clk: bcm2835: Strange effects when using aux-uart in console Martin Sperl
2016-02-10 15:12 ` Martin Sperl
2016-02-10 17:21 ` Stefan Wahren
2016-02-10 17:21   ` Stefan Wahren
2016-02-10 17:42   ` Martin Sperl
2016-02-10 17:42     ` Martin Sperl
2016-02-11 13:15 ` Martin Sperl
2016-02-11 13:15   ` Martin Sperl
2016-02-11 17:55   ` Stefan Wahren
2016-02-11 17:55     ` Stefan Wahren
2016-02-12 11:56     ` Martin Sperl
2016-02-12 11:56       ` Martin Sperl
2016-02-12 17:34       ` Martin Sperl
2016-02-12 17:34         ` Martin Sperl
2016-02-12 19:44         ` Martin Sperl
2016-02-12 19:44           ` Martin Sperl
2016-02-13 10:01           ` Stefan Wahren
2016-02-13 10:01             ` Stefan Wahren
2016-02-13 11:24             ` Martin Sperl
2016-02-13 11:24               ` Martin Sperl
2016-02-16 18:57               ` Michael Turquette
2016-02-16 18:57                 ` Michael Turquette
2016-02-16 18:57                 ` Michael Turquette
2016-02-17 10:43                 ` Martin Sperl
2016-02-17 10:43                   ` Martin Sperl
2016-02-13 11:53             ` Martin Sperl
2016-02-13 11:53               ` Martin Sperl
2016-02-13 20:45               ` Stefan Wahren
2016-02-13 20:45                 ` Stefan Wahren
2016-02-16 16:31                 ` Martin Sperl
2016-02-16 16:31                   ` Martin Sperl
2016-02-16 19:29                   ` Stefan Wahren
2016-02-16 19:29                     ` Stefan Wahren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.