| Message ID | 20210730134552.853350-5-bert@biot.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for EcoNet EN7523 SoC | expand |
On Fri, Jul 30, 2021 at 3:45 PM Bert Vermeulen <bert@biot.com> wrote: > > From: John Crispin <john@phrozen.org> > > EN7523 is an armv7 based silicon used inside broadband access type devices > such as xPON and xDSL. It shares various silicon blocks with MediaTek > silicon such as the MT7622. > > Signed-off-by: John Crispin <john@phrozen.org> > Signed-off-by: Bert Vermeulen <bert@biot.com> It's always nice to see a new SoC family. > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -580,6 +580,20 @@ config ARCH_VIRT > select HAVE_ARM_ARCH_TIMER > select ARCH_SUPPORTS_BIG_ENDIAN > > +config ARCH_ECONET > + bool "Econet SoC Support" > + depends on ARCH_MULTI_V7 > + select ARM_AMBA > + select ARM_GIC > + select ARM_GIC_V3 > + select ARM_DMA_USE_IOMMU > + select ARM_PSCI > + select HAVE_ARM_ARCH_TIMER > + select IOMMU_DMA > + select COMMON_CLK > + help > + Support for Econet EN7523 SoCs Given how closely related this probably is to MT7623/MT7622, should this perhaps just be part of arch/arm/mach-mediatek? According to https://wikidevi.wi-cat.ru/MediaTek#xPON, the older (mips based) MT752x chips are apparently just rebranded to EN752x after the business unit was spun off, but I guess they are still in the same family. > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index 173da685a52e..1bff0aa29c07 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000 > textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000 > textofs-$(CONFIG_ARCH_MESON) := 0x00208000 > textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000 > +textofs-$(CONFIG_ARCH_ECONET) := 0x00088000 Why is this needed? Note also the comment directly above it exlaining # Text offset. This list is sorted numerically by address in order to # provide a means to avoid/resolve conflicts in multi-arch kernels. Arnd
On 30.07.21 16:48, Arnd Bergmann wrote: > Given how closely related this probably is to MT7623/MT7622, should this > perhaps just be part of arch/arm/mach-mediatek? According to > https://wikidevi.wi-cat.ru/MediaTek#xPON, the older (mips based) MT752x > chips are apparently just rebranded to EN752x after the business unit > was spun off, but I guess they are still in the same family. Hi, ECNT (what was once known as trendchip) is now a subsidary of MTK (and not a BU if I am understanding it correctly). the EN7523 is rather similar to the MT7622 for some parts, other parts (spi, flash, wdt, gpio, .. drivers all needed to be rewritten and will be part of the next series). the older MIPS silicon shares almost no IP with the current ARM silicon. not really my call to decide which folder this should live in. it seemed natural to just give it its own folder, as ECNT is not a BU of MTK. we can change that however if required. John
On Fri, Jul 30, 2021 at 5:16 PM John Crispin <john@phrozen.org> wrote: > On 30.07.21 16:48, Arnd Bergmann wrote: > > Given how closely related this probably is to MT7623/MT7622, should this > > perhaps just be part of arch/arm/mach-mediatek? According to > > https://wikidevi.wi-cat.ru/MediaTek#xPON, the older (mips based) MT752x > > chips are apparently just rebranded to EN752x after the business unit > > was spun off, but I guess they are still in the same family. > > Hi, > > ECNT (what was once known as trendchip) is now a subsidary of MTK (and > not a BU if I am understanding it correctly). > > the EN7523 is rather similar to the MT7622 for some parts, other parts > (spi, flash, wdt, gpio, .. drivers all needed to be rewritten and will > be part of the next series). > > the older MIPS silicon shares almost no IP with the current ARM silicon. Ah, so I guess the old Trendchip parts were separately developed separately from the Ralink parts before the original acquisition, and then Ralink/Mediatek combined the two product lines before spinning off the dsl products into a new subsidiary. > not really my call to decide which folder this should live in. it seemed > natural to just give it its own folder, as ECNT is not a BU of MTK. > > we can change that however if required. My preference would be to have a common directory for both, but I'm not going to require that. From the kernel perspective the main question is actually not who makes the parts but who is going to maintain the code. Matthias is doing a good job taking care of the Mediatek parts, and he's familiar with the arm-soc process. If there is enough overlap between the Mediatek and EcoNet devices that we would expect either conflicts between binding/driver patches, or that you would want each other to review the patches for related parts, then it would make most sense to have a common directory and maintainer entry for both, with all patches going through the same git tree. It would also be nice to have someone listed as second maintainer for mediatek, since Matthias is currently the only one listed there. (Doesn't have to be you, I don't mean to drag you into taking up more work if you don't want to). Please discuss this between the three of you (Bert, John and Matthias), and let me know what you think works best for all of you. Arnd
On Fri, 30 Jul 2021 at 16:48, Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Jul 30, 2021 at 3:45 PM Bert Vermeulen <bert@biot.com> wrote: > > > > From: John Crispin <john@phrozen.org> > > > > EN7523 is an armv7 based silicon used inside broadband access type devices > > such as xPON and xDSL. It shares various silicon blocks with MediaTek > > silicon such as the MT7622. > > > > Signed-off-by: John Crispin <john@phrozen.org> > > Signed-off-by: Bert Vermeulen <bert@biot.com> > > It's always nice to see a new SoC family. > > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -580,6 +580,20 @@ config ARCH_VIRT > > select HAVE_ARM_ARCH_TIMER > > select ARCH_SUPPORTS_BIG_ENDIAN > > > > +config ARCH_ECONET > > + bool "Econet SoC Support" > > + depends on ARCH_MULTI_V7 > > + select ARM_AMBA > > + select ARM_GIC > > + select ARM_GIC_V3 > > + select ARM_DMA_USE_IOMMU > > + select ARM_PSCI > > + select HAVE_ARM_ARCH_TIMER > > + select IOMMU_DMA > > + select COMMON_CLK > > + help > > + Support for Econet EN7523 SoCs > > Given how closely related this probably is to MT7623/MT7622, should this > perhaps just be part of arch/arm/mach-mediatek? According to > https://wikidevi.wi-cat.ru/MediaTek#xPON, the older (mips based) MT752x > chips are apparently just rebranded to EN752x after the business unit > was spun off, but I guess they are still in the same family. > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > > index 173da685a52e..1bff0aa29c07 100644 > > --- a/arch/arm/Makefile > > +++ b/arch/arm/Makefile > > @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000 > > textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000 > > textofs-$(CONFIG_ARCH_MESON) := 0x00208000 > > textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000 > > +textofs-$(CONFIG_ARCH_ECONET) := 0x00088000 > > Why is this needed? > > Note also the comment directly above it exlaining > # Text offset. This list is sorted numerically by address in order to > # provide a means to avoid/resolve conflicts in multi-arch kernels. > Yes, please drop this - it is a horrible hack and it's already quite disappointing that we are stuck with it for the foreseeable future. So I assume the purpose of this is to protect the first 128k of DRAM to be protected from being overwritten by the decompressor? It would be best to move this reserved region elsewhere, but I can understand that this is no longer an option. So the alternatives are - omit this window from the /memory node, and rely on Geert's recent decompressor changes which make it discover the usable memory from the DT, or - better would be to use a /memreserve/ here (which you may already have?), and teach the newly added decompressor code to take those into account when choosing the target window for decompressing the kernel. -- Ard.
On 7/30/21 4:48 PM, Arnd Bergmann wrote: > On Fri, Jul 30, 2021 at 3:45 PM Bert Vermeulen <bert@biot.com> wrote: >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile >> index 173da685a52e..1bff0aa29c07 100644 >> --- a/arch/arm/Makefile >> +++ b/arch/arm/Makefile >> @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000 >> textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000 >> textofs-$(CONFIG_ARCH_MESON) := 0x00208000 >> textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000 >> +textofs-$(CONFIG_ARCH_ECONET) := 0x00088000 > > Why is this needed? > > Note also the comment directly above it exlaining > # Text offset. This list is sorted numerically by address in order to > # provide a means to avoid/resolve conflicts in multi-arch kernels. I didn't make that patch, but it turns out it's needed to get PSCI working; detection hangs without it. That makes no sense to me, but I'll examine further.
Hoi Bert, On Wed, Aug 4, 2021 at 6:43 PM Bert Vermeulen <bert@biot.com> wrote: > On 7/30/21 4:48 PM, Arnd Bergmann wrote: > > On Fri, Jul 30, 2021 at 3:45 PM Bert Vermeulen <bert@biot.com> wrote: > >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile > >> index 173da685a52e..1bff0aa29c07 100644 > >> --- a/arch/arm/Makefile > >> +++ b/arch/arm/Makefile > >> @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000 > >> textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000 > >> textofs-$(CONFIG_ARCH_MESON) := 0x00208000 > >> textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000 > >> +textofs-$(CONFIG_ARCH_ECONET) := 0x00088000 > > > > Why is this needed? > > > > Note also the comment directly above it exlaining > > # Text offset. This list is sorted numerically by address in order to > > # provide a means to avoid/resolve conflicts in multi-arch kernels. > > I didn't make that patch, but it turns out it's needed to get PSCI working; > detection hangs without it. That makes no sense to me, but I'll examine further. Probably PSCI relies on the memory contents at the start of RAM not being overwritten? Does it help if you remove the first 512 KiB from the /memory node (which should be declared in en7523-evb.dts instead of en7523.dtsi BTW)? Gr{oetje,eeting}s, Geert
On Mon, 9 Aug 2021 at 14:46, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hoi Bert, > > On Wed, Aug 4, 2021 at 6:43 PM Bert Vermeulen <bert@biot.com> wrote: > > On 7/30/21 4:48 PM, Arnd Bergmann wrote: > > > On Fri, Jul 30, 2021 at 3:45 PM Bert Vermeulen <bert@biot.com> wrote: > > >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile > > >> index 173da685a52e..1bff0aa29c07 100644 > > >> --- a/arch/arm/Makefile > > >> +++ b/arch/arm/Makefile > > >> @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000 > > >> textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000 > > >> textofs-$(CONFIG_ARCH_MESON) := 0x00208000 > > >> textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000 > > >> +textofs-$(CONFIG_ARCH_ECONET) := 0x00088000 > > > > > > Why is this needed? > > > > > > Note also the comment directly above it exlaining > > > # Text offset. This list is sorted numerically by address in order to > > > # provide a means to avoid/resolve conflicts in multi-arch kernels. > > > > I didn't make that patch, but it turns out it's needed to get PSCI working; > > detection hangs without it. That makes no sense to me, but I'll examine further. > > Probably PSCI relies on the memory contents at the start of RAM not > being overwritten? > Does it help if you remove the first 512 KiB from the /memory node I /think/ we rely on the first memblock being mappable using section mappings, so it might be better to remove the first 1 MB (or 2 MB in case the platform is LPAE capable). Note that this memory is discarded in any case, so this change is not as costly as it may seem. > (which should be declared in en7523-evb.dts instead of en7523.dtsi > BTW)? > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On 8/9/21 2:46 PM, Geert Uytterhoeven wrote: >> I didn't make that patch, but it turns out it's needed to get PSCI working; >> detection hangs without it. That makes no sense to me, but I'll examine further. > > Probably PSCI relies on the memory contents at the start of RAM not > being overwritten? It turns out to hang at the first SMC call, for PSCI_0_2_FN_PSCI_VERSION. I assume the receiver of that call, ATF, got dropped in that region by the vendor's U-Boot. > Does it help if you remove the first 512 KiB from the /memory node > (which should be declared in en7523-evb.dts instead of en7523.dtsi > BTW)? No it doesn't, was just trying to work out why not, in your fdt_check_mem_start(). Anyway that was Arnd's first suggestion, but I think his second suggestion (teach fdt_check_mem_start() about /memreserve/) is the cleaner approach to this. Opinions?
On 2021-08-01 18:44, Ard Biesheuvel wrote: > On Fri, 30 Jul 2021 at 16:48, Arnd Bergmann <arnd@arndb.de> wrote: >> >> Why is this needed? >> >> Note also the comment directly above it exlaining >> # Text offset. This list is sorted numerically by address in order to >> # provide a means to avoid/resolve conflicts in multi-arch kernels. >> > > Yes, please drop this - it is a horrible hack and it's already quite > disappointing that we are stuck with it for the foreseeable future. > > So I assume the purpose of this is to protect the first 128k of DRAM > to be protected from being overwritten by the decompressor? > > It would be best to move this reserved region elsewhere, but I can > understand that this is no longer an option. So the alternatives are > - omit this window from the /memory node, and rely on Geert's recent > decompressor changes which make it discover the usable memory from the > DT, or > - better would be to use a /memreserve/ here (which you may already > have?), and teach the newly added decompressor code to take those into > account when choosing the target window for decompressing the kernel. I looked into this issue myself and found that this approach has a significant drawback: 2 MiB of RAM is permanently wasted for something that only needs to be preserved during boot time. If the first 256 or 512 KiB of RAM are reserved in the decompressor, it means that the first 2 MiB need to be reserved, because that's the granularity for the kernel page mapping when the MMU is turned on. If we reserve it, we also need to need to take it out of the physical RAM address range, so there's no way to reclaim it later. On the other hand, with the simple textofs solution, I believe it gets freed in a late initcall, making it usable. So what's the right approach to deal with this? - Felix
On Fri, 3 Sept 2021 at 18:20, Felix Fietkau <nbd@nbd.name> wrote: > > > On 2021-08-01 18:44, Ard Biesheuvel wrote: > > On Fri, 30 Jul 2021 at 16:48, Arnd Bergmann <arnd@arndb.de> wrote: > >> > >> Why is this needed? > >> > >> Note also the comment directly above it exlaining > >> # Text offset. This list is sorted numerically by address in order to > >> # provide a means to avoid/resolve conflicts in multi-arch kernels. > >> > > > > Yes, please drop this - it is a horrible hack and it's already quite > > disappointing that we are stuck with it for the foreseeable future. > > > > So I assume the purpose of this is to protect the first 128k of DRAM > > to be protected from being overwritten by the decompressor? > > > > It would be best to move this reserved region elsewhere, but I can > > understand that this is no longer an option. So the alternatives are > > - omit this window from the /memory node, and rely on Geert's recent > > decompressor changes which make it discover the usable memory from the > > DT, or > > - better would be to use a /memreserve/ here (which you may already > > have?), and teach the newly added decompressor code to take those into > > account when choosing the target window for decompressing the kernel. > I looked into this issue myself and found that this approach has a > significant drawback: 2 MiB of RAM is permanently wasted for something > that only needs to be preserved during boot time. > How so? If that memory region carries your PSCI implementation, it should be preserved permanently. So at least the 512k are permanently reserved. > If the first 256 or 512 KiB of RAM are reserved in the decompressor, it > means that the first 2 MiB need to be reserved, because that's the > granularity for the kernel page mapping when the MMU is turned on. > Indeed. > If we reserve it, we also need to need to take it out of the physical > RAM address range, so there's no way to reclaim it later. > > On the other hand, with the simple textofs solution, I believe it gets > freed in a late initcall, making it usable. > > So what's the right approach to deal with this? > The right solution here is to fix your firmware/bootloader so that the PSCI reserved region is moved to the top of memory. Adding more TEXT_OFFSET hacks is really not the right approach here.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 82f908fa5676..e4a9401f8513 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -580,6 +580,20 @@ config ARCH_VIRT select HAVE_ARM_ARCH_TIMER select ARCH_SUPPORTS_BIG_ENDIAN +config ARCH_ECONET + bool "Econet SoC Support" + depends on ARCH_MULTI_V7 + select ARM_AMBA + select ARM_GIC + select ARM_GIC_V3 + select ARM_DMA_USE_IOMMU + select ARM_PSCI + select HAVE_ARM_ARCH_TIMER + select IOMMU_DMA + select COMMON_CLK + help + Support for Econet EN7523 SoCs + # # This is sorted alphabetically by mach-* pathname. However, plat-* # Kconfigs may be included either alphabetically (according to the diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 173da685a52e..1bff0aa29c07 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -152,6 +152,7 @@ textofs-$(CONFIG_ARCH_MSM8X60) := 0x00208000 textofs-$(CONFIG_ARCH_MSM8960) := 0x00208000 textofs-$(CONFIG_ARCH_MESON) := 0x00208000 textofs-$(CONFIG_ARCH_AXXIA) := 0x00308000 +textofs-$(CONFIG_ARCH_ECONET) := 0x00088000 # Machine directory name. This list is sorted alphanumerically # by CONFIG_* macro name.