Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230326001535.149539-1-bigbeeshane@gmail.com>
 <20230326001535.149539-2-bigbeeshane@gmail.com> <10930783-e1dd-5e75-a2cc-a09af862d949@linaro.org>
In-Reply-To: <10930783-e1dd-5e75-a2cc-a09af862d949@linaro.org>
From:   Shane Francis <bigbeeshane@gmail.com>
Date:   Sun, 26 Mar 2023 13:15:15 +0100
Message-ID: <CABnpCuCh2kmvTRrmsY0cwNqjFR6nv1xwxrZPJieM4_Ph8qEGeA@mail.gmail.com>
Subject: Re: [PATCH 2/2] dt-bindings: clock: update rk3588 clock definitions
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        heiko@sntech.de, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):

Will do, I haven't submitted patches for a while totally forgot the
wrapping guidelines

> Unfortunately the reason is not good enough for ABI break. Replace
> vendor boot uboots with open-source one or just correct them (it's still
> U-Boot so even for vendor one you have the source).

Replacing uboot is fine for this case, however I can foresee that can
cause issues further down the line.


1. No uboot source from the vendor, we all know no everyone respects
code licencing

2. Secure environments (like android tables), this chipset will likely
end up in android tablets that have the secure boot chain enable.
These will be unable to replace uboot even if source is available.

As this SoC is new to the Linux kernel (not even useable for much it's
current state) would it not be better to aling on this so vendor and
mainline DTS "agree" now rather than possibly have to address is in
the future ?

I have also investigated setting these clock rates during kernel boot,
but the SoC locks up.


On Sun, Mar 26, 2023 at 10:37=E2=80=AFAM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/03/2023 01:15, Shane Francis wrote:
> > Some vendor uboot bootloaders use the target kernels
> > DTB image to determine the target clock speeds for
> > some PLLs, currently this can cause uboot to set the
> > clock rate for gpll incorrectly on to cpll (breaking)
>
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/s=
ubmitting-patches.rst#L586
>
> > RGMII.
> >
> > This change starts the PLL clock definitions from 1
> > to correct this miss-match
>
> Unfortunately the reason is not good enough for ABI break. Replace
> vendor boot uboots with open-source one or just correct them (it's still
> U-Boot so even for vendor one you have the source).
>
> >
> > Signed-off-by: Shane Francis <bigbeeshane@gmail.com>
> > ---
> >  .../dt-bindings/clock/rockchip,rk3588-cru.h   | 1442 ++++++++---------
> >  1 file changed, 721 insertions(+), 721 deletions(-)
> >
> > diff --git a/include/dt-bindings/clock/rockchip,rk3588-cru.h b/include/=
dt-bindings/clock/rockchip,rk3588-cru.h
> > index b5616bca7b44..d63b07d054b7 100644
> > --- a/include/dt-bindings/clock/rockchip,rk3588-cru.h
> > +++ b/include/dt-bindings/clock/rockchip,rk3588-cru.h
> > @@ -12,727 +12,727 @@
> >
> >  /* cru-clocks indices */
> >
> > -#define PLL_B0PLL                    0
>
>
> Best regards,
> Krzysztof
>