[PATCH 00/13] Switch realtek target to upstream platform

[Sander Vanheule]

Hi Birger,

On Thu, 2021-12-09 at 13:26 +0100, Birger Koblitz wrote:
> Hi,
> 
> thanks for the quick reply.

Thank you for the input!

> On 09.12.21 10:38, Sander Vanheule wrote:
> > Hi Birger,
> > 
> > > 
> > > I would suggest to use sub-targets for these platforms with optimized
> > > compiler settings and SMP/single processor selected correctly for each,
> > > see https://github.com/BrainSlayer/pie/commits/pie-5.10-rtl9313
> > > 
> > > You already set up SMP build support in the config, but don't activate it.
> > 
> > Currently there are no supported devices in OpenWrt that have one of the SMP-
> > capable SoCs, which is the main reason it is not yet included here. I've had a 
> > very quick look on enabling SMP for RTL839x with a mainline build, but haven't
> > gotten it to work yet.
> I don't think you can pull the argument about that there are currently no supported
> devices in master as this concerns an architectural decision for all SoC generations.
> Also people are already using these devices, plus there are several developers trying
> to get support for the 839x up to the 931x into master bit-by-bit so it can see at
> least a bit of peer-review. We need to find a long-term solution, here and this
> change is very disruptive for development. As you can imagine, a lot of
> the code I use is never in a git, but instrumentation of core mips code to
> be able to debug deadlocks and RCU stalls.

This patch series is intended to reduce the maintenance burden, without introducing
breaking changes to devices already in OpenWrt. No promises are made for out-of-tree code.

Of course we need to discuss how we should move forward to implement working SMT support
etc., but I feel that this patch series should not depend on that discussion. There is
upstream code that runs on RTL8380 and RTL8390, and OpenWrt code provides the same
features in a different way. If we want to support SMT upstream, we may have to submit
code there anyway, so why do things twice?

If you would like code to be supported, please submit patches we can review. Frequent
small, incremental changes would be fine for me; maybe even preferable. IMHO you're not
doing yourself a favour by keeping months worth of changes out-of-tree.

> 
> > Hiroshi and I are still learning as we go along here. I don't know if we can
> > reasonably expect to have one kernel that supports all SoCs, or if there are
> > really conflicting build requirements.
> > 
> > 
> > > However with 2 small patches to the IRQ controller
> > > https://github.com/BrainSlayer/pie/commit/61860e04b89fa4c76a0e41af3e5d3480dabdf63c#diff-8b9c10bdba2282c6926cb9f7207ac869b011e248a6171f8132e79e1807c9f12a
> > > 
> > 
> > The patch you link here doesn't seem to apply to the current IRQ controller, but
> > to an already modified version of it. It also appears to implement interrupt
> > balancing, but is that a hard requirement for SMP on RTL839x?
> The IRQ controller needs a lot of update and the patch is not on the backported
> patch for the controller but the actual file with the controller code.
> I really don't like to debug or do development on top of patches, it is much
> harder. I would suggest to just copy the new version into the tree, there was
> nothing there before.
> 
> The IRQ balancing is not necessary strictly speaking for 8390, but if you want
> to use it with the RTL9300 timer, which needs to send IRQs to both CPUs, it is
> needed. So you would need it for your proposal below to test this on the 8390 with the
> timer.
> 
> > 
> > > and the Ethernet driver
> > > https://github.com/BrainSlayer/pie/commit/cdbc6598a1e69f74aa5fbaac9c896595a06d21f4#diff-2adae55db41693c452fcc10fb3925ff8c5a38760243f0b77bbd54acc75170d0a
> > > SMP can be enabled on the RTL8390 and the 9310 platform.
> > > 
> > > I am a bit worried of using the generic MIPS platform for the RTL9300.
> > > The SoC has a broken R4K Clock Event Interrupt and requires the RTL9300
> > > timer to work plus SMP support for the IRQ controller. There are only
> > > 2 examples in Linux which have such a combination of dedicated Timer
> > > for SMP support and IRQ controller, the Sibyte SB1250 and the BCM1480
> > > platforms.
> > 
> > A cursory search (and limited understanding of clocksources etc.) also turned up
> > the Ingenic SoCs for me. These appear to have custom timers too, but are still
> > based on MIPS_GENERIC.
> I had not noticed that one, it looks promising. There is now also a stand-alone
> CEVT-reimplementation of the RTL9300 timer here:
> https://github.com/BrainSlayer/pie/blob/pie-5.10-rtl9313/target/linux/realtek/files-5.10/arch/mips/kernel/cevt-rtl9300.c
> 
> > 
> > Since this Realtek timer is also available on RTL838x/RTL839x, we could use
> > these platforms for testing as well. The kernel also supports the clocksource=
> > parameter, which appears to allow for alternative clock selection. So if a
> > (generic) kernel with R4K-CEVT support is running on RTL930x, shouldn't we be
> > able to convince it to really use the Realtek timer?
> This should work. The RTL9300 event source actually pretends to be more accurate than
> the R4K-CEVT (it isn't but has more features such as one-shot), so that it is
> being picked up instead.
> 
> > If Realtek did really funny stuff that makes MIPS_GENERIC incompatible with some
> > of these SoCs, I still feel we should try going through upstream. But at least
> > then we would be quite sure new code is the only way to support these chips.
> Good idea. Could you try using the RTL9300-CEVT timer and the new IRQ controller
> with MIPS_GENERIC and SMP? Maybe it works. Sebastian and I are quite sure the
> timer does what it is supposed to do and the IRQ controller works like a charm
> on the 8390 under SMP, so either MIPS_GENERIC works, or there are more issues
> with the SoC, including ones that make it so broken that it does not actually work
> with SMP. Realtek does not provide any support for SMP to device makers, so
> maybe the silicon is kaputt.
> 
> In any case, could you look into the suggestion with the sub-targets? The different
> compile options should make quite a difference in performance.

Does enabling those different options on one build have that big an impact on SoCs that
don't support all selected features? (I really don't know, don't normally do any
performance testing.)

I would be happy to continue discussing the interrupt controller, RTL9300 timer code, and
SMT support, but maybe not in this thread.

Cheers,
Sander