On 09.12.2020 12:24, Philipp Zabel wrote:
On Tue, 2020-12-08 at 12:20 -0800, Florian Fainelli wrote:
On 12/8/20 3:02 AM, Philipp Zabel wrote:
Hi Rafał,
On Thu, 2020-11-19 at 13:56 +0100, Rafał Miłecki wrote:
From: Rafał Miłecki <rafal@xxxxxxxxxx>
PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
It's needed to power on and off SoC blocks like PCIe, SATA, USB.
This sentence, the register names, and
the brcm_pmb_power_on/off_device/zone functions below make me worry that
this should be a power domain controller (pm_genpd in drivers/soc)
instead.
Does PM in PMB stand for power management by chance?
It does, PMB stands for Power Management Bus.
If this actually cuts power to the USB and PCIe cores instead of just
controlling reset lines, it would be better to implement this
differently.
What is tricky is this is a combined reset/clock/power zones controller.
You rarely turn off/assert one of those without also turning
off/asserting the others.
It is common to have power management controllers also control clocks
and resets.
If the PMB controlled clocks and resets are only required for the power
on/off sequences, you should be able to move the current .(de)assert
implementations into a generic_pm_domain's .power_on and .power_off
methods.
Thanks for your comments Philipp! I just took a look at generic PM domains.
While I accept you know better that this hw support should be implemented
using generic PM domains, I'm not sure how to correctly proceed with that.
***
One great thing about reset controllers is that they have:
1. xlate function retuning int
2. (de)assert functions taking unsigned long id argument
That fits PMB hardware design very nicely as I can specify in DT:
1. Type of device to reset
2. ID of device to reset
Basically: I can use:
#reset-cells = <2>;
and then sth like:
resets = <&pmb BRCM_PMB_USB 17>;
***
In the struct generic_pm_domain we have "power_off" and "power_on" that take
no extra arguments however. Also its xlate has to return a single "struct
generic_pm_domain".
Please note that PMB can support for up 256 devices.
If I try to use generic PM domains to handle PMB hw in the same way I'll hit
two problems:
1. I'll have to register 256 "struct generic_pm_domain"s.
2. I won't have a clean way of passing device type (xlate limitation).
I can try to workaround the later just like in case of "ti,sci-pm-domain"
which uses #power-domain-cells = <2>. It seems hacky though as its "xlate"
handler stores args[1] in internal struct and hopes no another xlate call
will get executed before "power_on" or "power_off" call.
***
So I guess my real option is to store info about SoC devices in the PM
driver and create one "struct generic_pm_domain" for each known device only.
Then have something like:
#define BRCM_PMB_HOST_USB 0x01
#define BRCM_PMB_HOST_USB2 0x02
#define BRCM_PMB_HOST_USB3 0x03
#define BRCM_PMB_PCIE0 0x04
#define BRCM_PMB_PCIE1 0x05
#define BRCM_PMB_PCIE2 0x06
And finally:
power-domains = <&pmb0 BRCM_PMB_HOST_USB>;
power-domains = <&pmb0 BRCM_PMB_PCIE0>;
power-domains = <&pmb0 BRCM_PMB_PCIE1>;
What do you think about it? Is it the way to go?
