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

[Birger Koblitz]

Hi,

On 09.12.21 22:26, Sander Vanheule wrote:
> 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.
This patch will unfortunately considerably increase the maintenance burden, because it
pushes the code into a direction which does not reflect what these devices are. They
use SMT, SMP and they have different cpu-architectures. This should be reflected
by any new choice for the core architecture of the realtek code in OpenWRT.

With regard to out-of tree code: This is where the features come from. They come from
that development and it's the features users care for, the reason they are willing
to test new stuff and the reason some users provide access to development devices. Also,
the RTL9300 and RTL9310 code is in-tree. This is how we make sure that new developments
are consistent across all architectures.

> 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?
It is much too early to submit this upstream. We have already provided code upstream
before we had sufficient understanding of how things work several times. Example:
the interrupt controller code was pushed to mainline before we understood that this actually is
at the core of how to make VSMP work on the RTL9300. We did not even understand why
there was a double set of registers already on the RTL8390, because of course there
this is not necessary. But now you just told me that the patches for this IRQ controller
do not apply to the patches that provide that controller from mainline? Patches on top of patches
are not a maintenance burden? When in addition the controller needs a complete re-write
because we had no clue what it really was supposed to do?
And another example: the GPIO driver went to mainline before we understood how it works
on the RTL9300. Again, in order to use it on these devices we have patches on top of
patches and this is not a figure of speech, this is reality. And it still does not work
on the RTL9300, at least not for me. But I need this to work reliably to be able to
provide support. And don't let me get started how it works on the RTL931x.
I am really happy I resisted the push to have the RTL9300 timer code upstreamed, because
again, it is doing something completely different from what we thought at that time:
the timer is not about being able to do precision cable diagnostics, it is about making
VSMP work on the RTL9300. And for that it needs to work quite differently including
a big re-write.

> 
> 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.
I have been providing code bit by bit in the past, such as the RTL9300 support and there
are even bits of the RTL931x architecture which allow booting of such devices. But
now this is not being recognised, since there are no devices in tree to use that code.
Well, I tried to add those, I tried with the GS1900-48 using the RTL8390 and I tried
with the XGS1210 using the RTL9300. But guess what? I was told to come back in a couple
of months because the support was not complete and SFP/SFP+ and the like worked.

I really would like to provide more often code, but something as simple as adding Routing
Offload needed 3k lines of code and that needs time to develop. Providing half of it is not
going to fly: which user wants half of their traffic end in nirvana, or all of their
traffic being uncontrollable by any firewall? Add on top of that that routing offload works
quite differently on the different platforms and the fact that you need to make a reasonable
choice how to support them under one hat, and unfortunately this means months of work.
I would love to provide 10GBit SFP+ support on the RTL9300 quickly at this point,
but then I am having real trouble with the GPIO driver on that platform which I need for
the SFP cages but is patches on top of mainline patches.
> 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.)
The differences to be expected are about 50-120% more speed for optimized architectures plus (V)SMP support.
50% for VSMP, 100% for SMP, plus maybe 20% for optimized compiler settings. I did some promising
tests with dropbearkey.

> 
> I would be happy to continue discussing the interrupt controller, RTL9300 timer code, and
> SMT support, but maybe not in this thread.
Fine, there is no need for that. I just would like from your side to take the RTL9300 architecture
into account when you make a proposal to go MIPS_GENERIC so that we do not need to go back
with all our code again or have to provide RTL9300 support by patches on top of patches.
And I would like that this patch does not close its eyes on the fact that the architectures
now support SMP and VSMP and takes this into account, too, even if it is based on more than
1 year old code that was pushed into mainline at a point in time we were lacking very basic
understanding of the RTL SoCs.

Cheers,
  Birger