On 17.11.2020 19:55, Florian Fainelli wrote:
On 11/17/20 10:44 AM, Rafał Miłecki wrote:
On 12.11.2020 15:55, Rafał Miłecki wrote:
*** PMB ***
It seems like PMB Master is reset controller on its own. Even though
we don't
have actual driver for the (documented) "brcm,bcm63138-pmb" binding,
arch code
treates it like a reset.
So it seems that a single PMB is capable at least of:
1. Resetting ARM CPU core (id by 8b addr)
2. Resetting devices (id by 8b addr) by (en|dis)abling zones
Above operations are performed in a different (programming) ways.
*** PMC ***
PMC seems to be a reset controller with firmware capable at least of:
1. Resetting devices (id by 8b addr) by (en|dis)abling zones
that fallback to using PMB blocks when needed / required.
This just got more complex as I started playing with PMC / PMB and PCIe
controller.
1. BPCM_CAPABILITIES reg for PCIe controller reports 0 num_zones
2. Enabling PCIe requires powering on zone 0 manually
3. After powering on zone, PCIe requires setting SR_CONTROL
It means that PMB driver (and so PMC one as it fallback to the PMB)
needs to
know what type of device we're addressing.
From prior attempts at getting the 63138 supported which was a fairly
early silicon revision with a mix of PMB and PMC ultimately, the reset
driver does need to know what kind of device ID is being reset/powered
on. I stopped after seeing that SATA, USB, PCIe, Switch would all need
to be treated differently, and the board I had should have been upgraded
to a B0:
https://github.com/ffainelli/linux/commit/50f1dfb17b624ee41a11dda01bc899e6756869b7
There are way too many quirks for Device Tree to express all of them,
and the whole point of the BPCM was to design a re-usable and
self-discoverable power/reset/clocking module. As you found out, if the
capabilities are wrong, it defeats the purpose.
Good to know we share conclusions. The problem with PMC (except having to
support firmware / ucode / you call it) is it's still not capable of
handling everything.
As mentioned earlier, powering up PCIe controller can be done by PMC and
powering up zone 0 BUT it still requires setting that SR_CTRL.
I see you ended up hardcoding IDs in the bcm63138-pmb.c . I still hope I
can get something slightly more generic.
Anything simple like:
resets = <&pmb0 14>;
resets = <&pmc 1 14>;
won't work (unless we hardcode ids in driver - which is unwanted for DTS).
So I guess that after all we'll need something like:
cpu@1 {
resets = <&pmb0 PMB_ARM 4>; /* ARM CPU */
}
foo {
resets = <&pmb1 PMB_USB_DEVICE 17>; /* Reset USB using PMB directly */
}
bar {
resets = <&pmb1 PMB_PCIE_DEVICE 15>; /* Reset PCIE using PMB
directly */
}
What is the second reset cell mapping to? Maybe you can define a C
preprocessor macro that expressed both the PMB bus + device ID?
Device ID. Just like 4 in the :
#define PMB_ADDR_AIP (4 | PMB_BUS_AIP << PMB_BUS_ID_SHIFT)
So for BCM4908 I could have:
resets = <&pmb1 PMB_PCIE_DEVICE 14>; /* PCIe 0 */
resets = <&pmb1 PMB_PCIE_DEVICE 15>; /* PCIe 1 */
Do you mean C macro for combining two values? Sure, I can do that, but is it
worth it, just to save n * 32 b of .dtb data?
and also
qux {
resets = <&pmc PMB_PCIE_DEVICE 1 15>; /* Reset PCIe using PMC */
}
