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

[Rob Herring]

On Wed, Jul 10, 2024 at 7:29 AM André Draszik <andre.draszik@xxxxxxxxxx> 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@xxxxxxxxxx/ [1]
> Signed-off-by: André Draszik <andre.draszik@xxxxxxxxxx>
> ---
>  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