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@7800 {
> > + 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@1b200 {
> > + 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@1b400 {
> > + 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@ff100000 {
> > + compatible = "arm,gic-v3";
> > + reg = <0xff100000 0x10000>,
> > + <0xff140000 0xc0000>;
> > + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > + };
> > + };
> > +};
>
Regards,
James
