Hi,
On 7/16/22 21:31, Sander Vanheule wrote:
On Sat, 2022-07-16 at 21:09 +0200, Sander Vanheule wrote:
This RFC series introduces a new MFD device for the switch core found
in the Realtek SoCs. Currently only an implementation is provided for
RTL8380, but it written with the register structure of other generations
in mind.
this looks very promising as it offers the pin control we always needed.
I have not looked through the code in detail, yet. The biggest question
I have at this point is that at this point the code does not include the
other SoC generations. Especially when it comes to the LEDs, they are
very different. In fact the RTL838x is the odd-man-out. I would really
like to see how they will be included. In the past several design
decision turned out to be not so optimal after we learned about the
newer SoCs. Today we have all of them very well understood, so it should
not be an issue to add at least some code for the RTL93xx generation,
which has the LEDs modernized quite a bit.
The other question I have is whether this allows us to solve the other
big issue we have with these SoCs: Providing information between
drivers. For example you have written the 2nd copy of the model name
reading function in rtl8380_probe_model_name(), and it does not even
probe the other 3 SoC generations. But this information is needed
already at the very start of the boot process and several other drivers.
So why not provide a global structure like on other MIPS architectures
and populate it with information such as machine name and especially
switch structure for other drivers to use plus all the other
configuration details of a particular SoC (there are nearly 2 dozen
different RTL9300 SoCs all having a different structure for the MII
ports). In a very simple example, NOR access needs information about the
3/4 byte strapping pin from the switch core, but its registers live at a
completely different place in the SoC. The lack of such global
information is evident from e.g. your Netgear .dts. Although you know
that the first used port is 8, from the fact its an RTL8382M SoC or
alternatively the ports in the .dts, you need to add another time the
information about port to LED number.
Another point I have is more general: The pinctrl driver is a lot of
code for controlling the LEDs, which are not even pins on the SoC, but
controlled through a serial bus. Is this really the right type of
driver? Many of the LED drivers in Linux work similarly and provide a
standard interface to users. Also the LEDs are actually controlled by
the RTL8231 via a serial link on many models, not the Switch Core.
Placing the LED control into the switch core is a bit confusing, should
there not at least be a node for the RTL8231 in the .dts reflecting the
actual HW setup? Why not a small setup function for automatic HW control
of all LEDs as already done for the other SoC generations and a
dedicated LED driver. This might reduce code complexity and reflect the
actual hardware setup.
Cheers,
Birger
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
