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. > > 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? > > and also > > qux { > resets = <&pmc PMB_PCIE_DEVICE 1 15>; /* Reset PCIe using PMC */ > } -- Florian