Hi Rob, could you help me/us with Northstar bindings design, please?
On 04.05.2020 17:24, Rafał Miłecki wrote:
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].
"simple-bus" apparently is a no-way as some single registers may need to
be referenced using syscon.
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)
Here I tried "syscon", "simple-mfd" which lets me:
1. Have subnodes for all small hardware subblocks
2. Reference single registers using syscon
but it appears I can't mix those two.
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@xxxxxxxxx/
[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@xxxxxxxxx/
