Proper DT bindings for Broadcom's DMU node

["Rafał Miłecki" <zajec5@gmail.com>]

Hi,

I need some help with designing proper bindings for Broadcom's DMU
block. We already have it partially covered but it's inconsistent, some
cleanups were rejected and so I want to redesign it as it should be.

DMU ("Device Management Unit") is a block that can be found on Broadcom
iProc / Northstar devices. On Northstar it's mapped at:
ranges = <0x1800c000 0x1000>

It contains:
1. Few random registers, some of them shared by various hardware
    blocks (and possibly hard to abstract?)
2. At least one sub-block with even more random registers

Some of known DMU registers are:
reg = <0x100 0x14>	CRU LCPLL control0
reg = <0x140 0x24>	CRU GENPLL
reg = <0x164 0x04>	CRU_USB2_CONTROL
reg = <0x180 0x04>	CRU_CLKSET_KEY
reg = <0x184 0x04>	CRU_RESET
reg = <0x1c0 0x24>	pinctrl
reg = <0x2a0 0x04>	CRU_STRAPS_CTRL
reg = <0x2c0 0x04>	PVTMON control0
(Broadcom never released a proper documentation)


As you can see there are a few CRU registers (depending on a source it's
a "Clock and Reset Unit" or "Central Resource Unit"). It's said to be
separated block and was described by Scott (from Broadcom) as: "unit
with a lot of random registers to perform various operations".

As I said, there are also some shared registers:

CRU_CLKSET_KEY is accessed by:
1. USB 2.0 PHY driver for (un)locking DMU PLL settings
2. GMAC for changing 2.66G line rate to 2Gbps

CRU_STRAPS_CTRL needs to be accessed by:
1. USB 3.0 PHY driver for PHY connected to MDIO
2. PCIE driver for PHY connected to MDIO


My initial idea was to have something like:

dmu@1800c000 {
	compatible = "simple-bus";
	ranges = <0 0x1800c000 0x1000>;
	#address-cells = <1>;
	#size-cells = <1>;

	cru@100 {
		compatible = "simple-bus";
		reg = <0x100 0x1a4>;

		lcpll { ... };
		genpll { ... };
		reset { ... };
	};
};

but Rob noticed that "simple-bus" requires everything in DMU to have
sub-nodes [0] [1].


I thought it can be solved by using compatible = "syscon", "simple-mfd"
and I even got one patch for that accepted [2] [3] (pinctrl). It seems
it slipped through and was possibly a mistake. Another similar patch was
rejected [4] [5] (bcm-ns-usb2-phy).

What I tried to achieve was something like this:

dmu@1800c000 {
	compatible = "simple-bus";
	ranges = <0 0x1800c000 0x1000>;
	#address-cells = <1>;
	#size-cells = <1>;

	cru: syscon@100 {
		compatible = "syscon", "simple-mfd";
		reg = <0x100 0x1a4>;
		ranges;
		#address-cells = <1>;
		#size-cells = <1>;

		lcpll0@0 {
			#clock-cells = <1>;
			compatible = "brcm,nsp-lcpll0";
			reg = <0x0 0x14>;
		};

		genpll@40 {
			#clock-cells = <1>;
			compatible = "brcm,nsp-genpll";
			reg = <0x40 0x24>;
		};

		pin-controller@c0 {
			compatible = "brcm,bcm4708-pinmux";
			reg = <0xc0 0x24>;
			reg-names = "cru_gpio_control";
		};

		thermal@1c0 {
			compatible = "brcm,ns-thermal";
			reg = <0x1c0 0x10>;
			#thermal-sensor-cells = <0>;
		};
	};
};

cru-reset@??? {
	compatible = "brcm,ns-cru-reset";
	syscon-cru = <&cru>; /* CRU_RESET */
	#reset-cells = <1>;
};

usb2-phy@??? {
	compatible = "brcm,ns-usb2-phy";
	syscon-cru = <&cru>; /* CRU_CLKSET_KEY */
	#phy-cells = <0>;
};

(apparently it wasn't a good idea)

So my question is: how to properly handle this? I'm not sure what's the
proper "compatible" string to use. Is my idea of:
1. Using sub-node for registers explicitly used by one driver
2. Using syscon for shared registers
OK?


[0] https://www.spinics.net/lists/arm-kernel/msg682838.html
[1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20181015093013.31651-1-zajec5@gmail.com/
[2] https://spinics.net/lists/linux-gpio/msg35285.html
[3] https://patchwork.kernel.org/patch/10735931/
[4] https://lkml.org/lkml/2019/1/15/913
[5] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20190108123907.19816-1-zajec5@gmail.com/