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

[Greg Kroah-Hartman]

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