Hi Claudiu, On Mon, Nov 27, 2023 at 8:37 AM claudiu beznea <claudiu.beznea@xxxxxxxxx> wrote: > On 24.11.2023 11:08, Geert Uytterhoeven wrote: > > On Thu, Nov 23, 2023 at 5:35 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > >> On Mon, Nov 20, 2023 at 8:01 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote: > >>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > >>> > >>> RZ/{G2L, V2L, G3S} based CPG versions have support for saving extra > >>> power when clocks are disabled by activating module standby. This is done > >>> though MSTOP specific registers that are part of CPG. Each individual > >>> module have one or more bits associated in one MSTOP register (see table > >>> "Registers for Module Standby Mode" from HW manuals). Hardware manual > >>> associates modules' clocks to one or more MSTOP bits. There are 3 mappings > >>> available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals): > >>> > >>> case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X}) > >>> case 2: N clocks mapped to 1 MSTOP bit (with N={0, ..., X}) > >>> case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y}) > >>> > >>> Case 3 has been currently identified on RZ/V2L for VCPL4 module. > >>> > >>> To cover all 3 cases the individual platform drivers will provide to > >>> clock driver MSTOP register offset and associated bits in this register > >>> as a bitmask and the clock driver will apply this bitmask to proper > >>> MSTOP register. > >>> > >>> As most of the modules have more than one clock and these clocks are > >>> mapped to 1 MSTOP bitmap that need to be applied to MSTOP registers, > >>> to avoid switching the module to/out of standby when the module has > >>> enabled/disabled clocks a counter has been associated to each module > >>> (though struct mstop::count) which is incremented/decremented every > >>> time a module's clock is enabled/disabled and the settings to MSTOP > >>> register are applied only when the counter reaches zero (counter zero > >>> means either 1st clock of the module is going to be enabled or all clocks > >>> of the module are going to be disabled). > > After giving this some more thought, it feels odd to derive the standby > > state of a module from the state of its module clocks, while the latter > > are already controlled through Runtime PM and a Clock Domain. > > > > A first alternative solution could be to drop the GENPD_FLAG_PM_CLK > > flag from the RZ/G2L CPG clock domain, and provide your own > > gpd_dev_ops.start() and .stop() callbacks that take care of both > > module standby and clocks (through pm_clk_{resume,suspend}(). > > (See https://elixir.bootlin.com/linux/v6.7-rc2/source/drivers/base/power/domain.c#L2093 > > for the GENPD_FLAG_PM_CLK case). > > That still leaves you with a need to associate an MSTOP register and > > bitmask with a device through its module clocks. > > > > A second alternative solution could be to increase #power-domain-cells > > from zero to one, and register individual PM Domains for each module, > > and control module standby from the generic_pm_domain.power_{on,off}() > > callbacks. Devices would specify the module using the power-domains = > > <&cpg <id> > property in DT, with <id> one of the to-be-added list of > > modules in include/dt-bindings/clock/r9a08g045-cpg.h. The RZ/G2L CPG > > driver can handle the mapping from <id> to MSTOP register and bitmask. > > This solution requires updates to DT, but you can keep compatibility > > with old DTBs by only registering the new PM Domains when > > #power-domain-cells is one. > > The extra power saving would only be applicable with new DTBs, though. > > I prefer this alternative even though it cannot be applied for old DTBs, it > looks to me that is more modular. What do you think? I prefer the second alternative, too. > The only thing is that MSTOP is not really a power off/on switch (if it > would be implemented with generic_pm_domain.power_{on, off}) but is more That's fine: Linux' PM Domains are fairly generic and abstract, and not limited to pure power domains/areas. > like a clock disable/enable functionality (it should not be an issue > though, just saying)... According to manual (I'm referring to Figure 41.4 > Block Connection Overview for Module Standby Mode of HW manula of RZ/G3S), > it disables/enables the module's bus clock. Thanks for the pointer! That picture nicely shows the internal behavior. For comparison, on SH/R-Mobile and R-Car SoCs there is a similar internal structure, but it is less visible to the programmer: there are no individual controls for each clock or reset that is fed into a module. These are all hidden behind a single Module Stop resp. Reset control bit. In Linux, we modeled the module stop bit as a gate clock, controlled by Runtime PM through the Clock Domain's .start()/.stop() callbacks. Note that you also have to take into account Figure 41.2 ("Modules in Power Domain"). When adding support for power transitions later, you can register a PM Domain representing PD_ISOVCC, and use that as the parent PM Domain for the individual PM Domains for modules belonging to PD_ISOVCC. All of that can be handled in the driver, and would not need any changes to DT. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds