[RFC PATCH 0/7] realtek: MFD for switch core

[Birger Koblitz]

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