* Proper DT bindings for Broadcom's DMU node
@ 2020-05-04 15:24 Rafał Miłecki
2020-05-04 17:26 ` Ray Jui
2020-08-20 12:38 ` Rafał Miłecki
0 siblings, 2 replies; 4+ messages in thread
From: Rafał Miłecki @ 2020-05-04 15:24 UTC (permalink / raw)
To: devicetree, Rob Herring, Vivek Unune
Cc: BCM Kernel Feedback, Florian Fainelli
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/
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Proper DT bindings for Broadcom's DMU node
2020-05-04 15:24 Proper DT bindings for Broadcom's DMU node Rafał Miłecki
@ 2020-05-04 17:26 ` Ray Jui
2020-08-20 12:38 ` Rafał Miłecki
1 sibling, 0 replies; 4+ messages in thread
From: Ray Jui @ 2020-05-04 17:26 UTC (permalink / raw)
To: Rafał Miłecki, devicetree, Rob Herring, Vivek Unune
Cc: BCM Kernel Feedback, Florian Fainelli
Hi Rafal,
On 5/4/2020 8:24 AM, Rafał Miłecki wrote:
> 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?
The above two proposed scheme from you to handle CRU in NS makes sense
to me and they also model the ASIC architecture correctly.
Thanks,
Ray
>
>
> [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/
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Proper DT bindings for Broadcom's DMU node
2020-05-04 15:24 Proper DT bindings for Broadcom's DMU node Rafał Miłecki
2020-05-04 17:26 ` Ray Jui
@ 2020-08-20 12:38 ` Rafał Miłecki
2020-08-24 19:24 ` Florian Fainelli
1 sibling, 1 reply; 4+ messages in thread
From: Rafał Miłecki @ 2020-08-20 12:38 UTC (permalink / raw)
To: devicetree, Rob Herring, Vivek Unune
Cc: BCM Kernel Feedback, Florian Fainelli
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@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/
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Proper DT bindings for Broadcom's DMU node
2020-08-20 12:38 ` Rafał Miłecki
@ 2020-08-24 19:24 ` Florian Fainelli
0 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2020-08-24 19:24 UTC (permalink / raw)
To: Rafał Miłecki, devicetree, Rob Herring, Vivek Unune
Cc: BCM Kernel Feedback, Florian Fainelli
On 8/20/20 5:38 AM, Rafał Miłecki wrote:
> 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";
This should actually be a simple-bus no?
>> 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.
It does not seem to me that there is an use case for syscon except for
the USB2 PHY PLL which should arguably be a clock provider and the "DMP"
register base used by the USB 3.0 PHY which should be a reset provider.
--
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-24 19:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 15:24 Proper DT bindings for Broadcom's DMU node Rafał Miłecki
2020-05-04 17:26 ` Ray Jui
2020-08-20 12:38 ` Rafał Miłecki
2020-08-24 19:24 ` Florian Fainelli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.