Re: [PATCH v3 1/2] dt-bindings: serial: samsung: fix maxItems for gs101 & document earlycon requirements

Rob Herring
2024-07-11T15:51:21Z

On Wed, Jul 10, 2024 at 7:29 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> While gs101 needs exactly two clocks for the UART, the schema doesn't
> currently limit the maximum number to this and instead the default of
> five from this schema is applied.
>
> Update the schema accordingly.
>
> Also, as pointed out in [1] before, the hand-over between earlycon and
> serial console is fragile due to clocking issues, at least on Google
> Tensor gs101. Therefore, document the clocking requirements for
> earlycon in the description for posterity, so the information is not
> lost.
>
> Link: https://lore.kernel.org/all/d45de3b2bb6b48653842cf1f74e58889ed6783ae.camel@linaro.org/ [1]
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
> Documentation/devicetree/bindings/serial/samsung_uart.yaml | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
> index 0f0131026911..2435c3d92158 100644
> --- a/Documentation/devicetree/bindings/serial/samsung_uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
> @@ -145,6 +145,20 @@ allOf:
> - samsung,uart-fifosize
> properties:
> reg-io-width: false

blank line between properties

> + clocks:
> + description: |
> + Note that for earlycon to work, the respective ipclk and pclk need
> + to be running! The bootloader normally leaves them enabled, but the
> + serial driver will start handling those clocks before the console
> + driver takes over from earlycon, breaking earlycon. If earlycon is
> + required, please revert the patch "clk: samsung: gs101: don't mark
> + non-essential (UART) clocks critical" locally first to mark them
> + CLK_IS_CRITICAL and avoid this problem.

That's a whole bunch of details that are Linux specific which have
little to do with the binding.

> + maxItems: 2
> + clock-names:
> + items:
> + - const: uart
> + - const: clk_uart_baud0

Which clock is pclk and ipclk? 'baud' would be sufficient for the
name. 'clk_' and 'uart' are redundant because it's all clocks and they
are all for the uart.

Rob