On 10/03/2025 11:47, Christian Marangi wrote:

On Mon, Mar 10, 2025 at 10:21:45AM +0100, Krzysztof Kozlowski wrote:

quoted

On 09/03/2025 14:29, Christian Marangi wrote:

quoted

Add Documentation for Airoha EN7581 SCU.

Airoha EN7581 SoC expose registers to control miscellaneous pheriperals
via the SCU (System Controller Unit).

Example of these pheriperals are reset-controller, clock-controller,
PCIe line speed controller and bits to configure different Serdes ports
for USB or Ethernet usage.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../mfd/airoha,en7581-scu-sysctl.yaml         | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/airoha,en7581-scu-sysctl.yaml

diff --git a/Documentation/devicetree/bindings/mfd/airoha,en7581-scu-sysctl.yaml b/Documentation/devicetree/bindings/mfd/airoha,en7581-scu-sysctl.yaml
new file mode 100644
index 000000000000..d7dc66f912c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/airoha,en7581-scu-sysctl.yaml

@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/airoha,en7581-scu-sysctl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 SCU (System Controller Unit)
+
+maintainers:
+  - Christian Marangi <ansuelsmth@gmail.com>
+
+description:
+  Airoha EN7581 SoC expose registers to control miscellaneous
+  pheriperals via the SCU (System Controller Unit).
+

One more comment - there is no such thing as "sysctl" in your hardware.
Look at the SCU binding which clearly says that it is the hardware you
are duplicating here, so the "System Control Unit".

So you have existing "This node defines the System Control Unit of the
EN7523 SoC" and you add one more node which defines the "System Control
Unit", so you have two "System Control Unit" device nodes?

Look also what Stephen asked for:

https://lore.kernel.org/all/20220106013100.842FCC36AEB@smtp.kernel.org/ (local)

so how system-controller can now became clock-controller? Now, it was
the system controller since the beginning.

The main problem here (and we had a similar problem with GPIO and PWM)
is that the Vendor (Airoha) wasn't so bright in placing the different
registers for the SoC so we have case where everything is mixed and not
one after another... 

Example we have 
- CLK register part 1
- Some bits that configure PCIe
- CLK register part 2
- GPIO
- CLK register part 3
- ...

This does not explain that binding said "This node defines the System
Control Unit".

So what are you adding here if not SCU?

The driver solution for this is syscon and the simple-mfd node
structure.

Let's keep driver entirely separate, we don't talk about them and mixing
arguments won't make it easier.

Now the main problem is how to modle this in DT. There are lots of case
where the simple-mfd model is used (like the one proposed) but probably
this is not accepted anymore. But again this should be clearly stated or
we have a chicken-egg problem when other devs implement similar thing and
have to implement simple MFD driver to handle this. (and driver
maintainers say "Use the simple-mfd model like it was already done)

simple-mfd has nothing to do here. Describe the hardware - what is the SCU?

For this specific case (and to give an answer to the clock patch after
this) the problem is that this register space was originally used only
to control the clock and I wasn't aware that it was also used to control
USB. Now that I'm implementing support for it, the disaster happened.

So In short SCU is lots of thing, both a system-controller, a
clock-controller and even a reset-controller.

And you have bindings for that already. Done.

To make it short, 2 different solution:
1. We can keep the current node structure of the node-controller and add a
child node for the SSR part (with a dedicated compatible).

No, you do not add child nodes just because you want some drivers.

What is SSR? How is it a device?

2. Those property need to be be defined in the clock-controller node?

In the SCU node. Do you have only one SCU or more?

The ideal solution is 1. Does it work for you?

Sorry for the long post and hope you understand why this mess of
reworking the binding.

Best regards,
Krzysztof