linux-kernel - Re: [PATCH 2/2] dt-bindings: clock: update rk3588 clock definitions

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]

Date:   Sun, 26 Mar 2023 13:15:15 +0100
From:   Shane Francis <bigbeeshane@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        heiko@...ech.de, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: clock: update rk3588 clock definitions

> 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 AM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.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/submitting-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@...il.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
>