linux-kernel - Re: [PATCH] tty: serial: samsung: add clock comment for earlycon on gs101

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2024071030-gravy-backwater-88ec@gregkh>
Date: Wed, 10 Jul 2024 15:40:07 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>,
	Alim Akhtar <alim.akhtar@...sung.com>,
	Jiri Slaby <jirislaby@...nel.org>,
	Peter Griffin <peter.griffin@...aro.org>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	Will McVicker <willmcvicker@...gle.com>,
	Sam Protsenko <semen.protsenko@...aro.org>, kernel-team@...roid.com,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH] tty: serial: samsung: add clock comment for earlycon on
 gs101

On Wed, Jul 10, 2024 at 02:33:29PM +0100, André Draszik wrote:
> As pointed out in [1] before, the hand-over between earlycon and serial
> console is fragile due to clocking issues:
> 
>     ... causing earlycon to stop to work sometime into the boot for two
>     reasons:
>     * peric0_top1_ipclk_0 requires its parent gout_cmu_peric0_ip to be
>       running, but because earlycon doesn't deal with clocks that
>       parent will be disabled when none of the other drivers that
>       actually deal with clocks correctly require it to be running and
>       the real serial driver (which does deal with clocks) hasn't taken
>       over yet
> 
>     The console UART, and I2C bus 8 are on the same cmu_peric0 controller,
>     and that cmu_peric0 has two clocks coming from cmu_top, ip and bus. For
>     I2C8 & UART to work, both of these clocks from cmu_top need to to be on
>     as they are the parent of the i2c8-(ip|pclk) and uart-(ip|pclk) each.
> 
>     The bootloader leaves those clocks running, yes. So earlycon works (for
>     a while).
> 
>     At some point into the boot, one of two things happens:
>     1) Linux will load the i2c driver. That driver does clock handling
>     (correctly), it will initialise and then it has nothing to do, therefore
>     it disables cmu_peric0's i2c8 ip and pclk clocks. Because at that stage
>     nothing appears to be using the cmu_peric0's ip clock (the real serial
>     driver hasn't initialised yet), Linux decides to also disable the parent
>     ip clock coming from cmu_top.
> 
>     At this stage, the earlycon driver stops working, as the parent ip clock
>     of the uart ip clock is not running any more. No serial output can be
>     observed from this stage onwards. I think what is probably happening is
>     that the console uart FIFO doesn't get emptied anymore, and earlycon
>     will simply wait forever for space to become available in the FIFO (but
>     I didn't debug this).
> 
>     Anyway, the boot doesn't progress, the system appears to hang. In any
>     case it's not usable as we have no other means of using it at this stage
>     (network / usb / display etc.).
> 
>     2) Alternatively, the UART driver will load at this stage. Again, it
>     will tweak the clocks and after probe it will leave its clocks disabled.
>     The serial console driver hasn't taken over at this stage and earlycon
>     is still active. Again, the system will hang, because IP and PCLK have
>     been disabled by the UART driver. Once the serial console is enabled,
>     clocks are being enabled again, but because earlycon is still waiting
>     for progress, the boot doesn't progress past disabling ip and pclk. It
>     never gets to enabling the serial console (re-enabling the clocks).
> 
>     So in both cases we get some output from earlycon, but the system hangs
>     once the first consumer driver of an IP attached to cmu_peric0 has
>     completed probing.
> 
>     ...
> 
>     If earlycon is not enabled in kernel command line, everything works
>     fine, the kernel buffers its messages and once the real serial console
>     driver starts, all messages since boot are being printed.
> 
> As requested, add a comment to the code for posterity, so the
> information is not lost. The patch referenced in the comment can be
> found at [2].

That should also be in the comment in the .c file, right?  Along with
the git id that you feel should be reverted?

thanks,

greg k-h