| 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
| ||
|
Message-ID: <a673b4e4bb454bad99ad483e1cccbbb1@realtek.com>
Date: Fri, 27 Dec 2019 07:17:25 +0000
From: James Tai <james.tai@...ltek.com>
To: Andreas Färber <afaerber@...e.de>
CC: "mark.rutland@....com" <mark.rutland@....com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-realtek-soc@...ts.infradead.org"
<linux-realtek-soc@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Robin Murphy <robin.murphy@....com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Subject: RE: [PATCH 2/2] arm64: dts: realtek: Add RTD1319 SoC and Realtek PymParticle EVB
Hi Andreas,
> This hunk is lacking rtd1395, so is not based on the latest patches I posted. I
> expect you to be developing against linux-next.git tree, and when there's
> relevant in-flight patches, you'll need to either apply my patches via git-am to
> your tree, or for convenience you can use the beginning of my (but better not
> the full experimental) rtd1295-next branch (git-rebase -i, or (careful!) git-reset
> --hard). Yes, neither is super-easy.
>
I will use your rtd1295-next branch for development.
> Same as with the binding, it would seem better to not add this at the end, even
> if it's your newest family. Consider this: Someone finds an
> RTD1036 in their household and wants to contribute a patch - where would
> they add it? I don't want all newly added stuff to go into the bottom of the file
> (then it'll be hard to find and potentially causes conflicts), so we need a stable
> sort order where I don't need to do historical research of whether 1036 is
> newer or older than 1195/1296 to determine where to insert it in a file.
> Alphanumerical sort order seems simplest to understand and is proven
> elsewhere to reduce merge conflicts.
Thanks for your kind reminder.
> > diff --git a/arch/arm64/boot/dts/realtek/rtd1319-pymparticle.dts
> > b/arch/arm64/boot/dts/realtek/rtd1319-pymparticle.dts
> > new file mode 100644
> > index 000000000000..d8bfe2304b71
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/realtek/rtd1319-pymparticle.dts
> > @@ -0,0 +1,43 @@
> > +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> > +/*
> > + * Copyright (c) 2019 Realtek Semiconductor Corp.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "rtd1319.dtsi"
> > +
> > +/ {
> > + compatible = "realtek,pymparticle", "realtek,rtd1319";
>
> Thanks, correct order now.
>
> > + model = "Realtek PymParticle EVB";
> > +
> > + memory@0 {
> > + device_type = "memory";
> > + reg = <0x0 0x80000000>;
> > + };
>
> No! I understood RTD1319 has the same boot ROM size 184 KiB as RTD1619,
> so please look at the patches I posted, including fix for RTD1619 [1], and fix
> this yourself here. A comment for humans would also be nice.
>
> In the public BPI-M4-bsp code I see one -pymparticle-1GB.CMAx2.dts file.
> If this board is available with less than 2 GiB RAM then please use the lower
> value to be safe - you can run a 2 GiB board with 1 GiB RAM used, but using
> more RAM than available will break.
>
> [1] https://patchwork.kernel.org/patch/11268969/
>
I will fix it in next version patch.
> > +++ b/arch/arm64/boot/dts/realtek/rtd1319.dtsi
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> > +/*
> > + * Realtek RTD1319 SoC
> > + *
> > + * Copyright (c) 2019 Realtek Semiconductor Corp.
> > + */
> > +
> > +#include "rtd13xx.dtsi"
> > +
> > +/ {
> > + compatible = "realtek,rtd1319";
> > +};
>
> What other contents are you expecting to add in this file?
I expect add USB, SATA, PCIE and so on.
> > + cpus {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + cpu0: cpu@0 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a55";
> > + reg = <0x0>;
> > + enable-method = "psci";
> > + next-level-cache = <&l2>;
> > + };
> > +
> > + cpu1: cpu@100 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a55";
> > + reg = <0x100>;
> > + enable-method = "psci";
> > + next-level-cache = <&l2>;
> > + };
> > +
> > + cpu2: cpu@200 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a55";
> > + reg = <0x200>;
> > + enable-method = "psci";
> > + next-level-cache = <&l2>;
> > + };
> > +
> > + cpu3: cpu@300 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a55";
> > + reg = <0x300>;
> > + enable-method = "psci";
> > + next-level-cache = <&l2>;
> > + };
> > +
> > + l2: l2-cache {
> > + compatible = "cache";
> > + };
>
> I note this seems a different cache topology than RTD1619?
>
Yes, It is different cache topology than RTD1619.
The RTD1319 don't have an L3 cache.
> > + osc27M: osc {
> > + compatible = "fixed-clock";
> > + clock-frequency = <27000000>;
> > + clock-output-names = "osc27M";
>
> BTW I recall seeing "osc27m" in your clk patchset. We should decide on one
> name and stick with it consistently, and I think it's best to have this as a node
> here in .dtsi (or in .dts), in case OEMs ever choose to have it generated by
> some other non-trivial IC.
I understand.
> > + #clock-cells = <0>;
> > + };
> > +
> > + soc {
> > + compatible = "simple-bus";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0x98000000 0x98000000 0x68000000>;
>
> No! Lacking a range for boot ROM. And your range is probably too large due to
> high RAM. Please see [1] and fix for both. r-bus ranges below would indicate
> that above soc range should be 0x00200000 long only, plus extra ranges for
> whatever besides r-bus is shadowing RAM (e.g., GIC).
>
I will fix it in next version patch.
> > +
> > + uart0: serial0@...0 {
> > + compatible = "snps,dw-apb-uart";
> > + reg = <0x7800 0x400>;
> > + reg-shift = <2>;
> > + reg-io-width = <4>;
> > + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> > + clock-frequency = <432000000>;
> > + status = "disabled";
> > + };
> > +
> > + uart1: serial1@...00 {
> > + compatible = "snps,dw-apb-uart";
> > + reg = <0x1b200 0x400>;
> > + reg-shift = <2>;
> > + reg-io-width = <4>;
> > + interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > + clock-frequency = <432000000>;
> > + status = "disabled";
> > + };
> > +
> > + uart2: serial2@...00 {
> > + compatible = "snps,dw-apb-uart";
> > + reg = <0x1b400 0x400>;
> > + reg-shift = <2>;
> > + reg-io-width = <4>;
> > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > + clock-frequency = <432000000>;
> > + status = "disabled";
> > + };
>
> Here you appear to ignore my patches introducing syscon for ISO & MISC!
>
> See https://patchwork.kernel.org/cover/11269453/
I will apply the syscon for ISO & MISC in next version patch.
> > + };
> > +
> > + gic: interrupt-controller@...00000 {
> > + compatible = "arm,gic-v3";
> > + reg = <0xff100000 0x10000>,
> > + <0xff140000 0xc0000>;
> > + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > + };
> > + };
> > +};
>
Regards,
James
Powered by blists - more mailing lists