Re: [PATCH v8 07/34] clk: tegra: Support runtime PM and power domain

Ulf Hansson
2021-08-20T13:08:56Z

[...]

> >
> > I'm creating platform device for the clocks that require DVFS. These
> > clocks don't use regulator, they are attached to the CORE domain.
> > GENPD framework manages the performance state, aggregating perf votes
> > from each device, i.e. from each clock individually.
> >
> > You want to reinvent another layer of aggregation on top of GENPD.
> > This doesn't worth the effort, we won't get anything from it, it
> > should be a lot of extra complexity for nothing. We will also lose
> > from it because pm_genpd_summary won't show you a per-device info.
> >
> > domain status children performance
> > /device runtime status
> > ----------------------------------------------------------------------------------------------
> > heg on 1000000
> > /devices/soc0/50000000.host1x active 1000000
> > /devices/soc0/50000000.host1x/54140000.gr2d suspended 0
> > mpe off-0 0
> > vdec off-0 0
> > /devices/soc0/6001a000.vde suspended 0
> > venc off-0 0
> > 3d1 off-0 0
> > /devices/genpd:1:54180000.gr3d suspended 0
> > 3d0 off-0 0
> > /devices/genpd:0:54180000.gr3d suspended 0
> > core-domain on 1000000
> > 3d0, 3d1, venc, vdec, mpe, heg
> > /devices/soc0/7d000000.usb active 1000000
> > /devices/soc0/78000400.mmc active 950000
> > /devices/soc0/7000f400.memory-controller unsupported 1000000
> > /devices/soc0/7000a000.pwm active 1000000
> > /devices/soc0/60006000.clock/tegra_clk_pll_c active 1000000
> > /devices/soc0/60006000.clock/tegra_clk_pll_e suspended 0
> > /devices/soc0/60006000.clock/tegra_clk_pll_m active 1000000
> > /devices/soc0/60006000.clock/tegra_clk_sclk active 1000000
> >
>
> I suppose if there's really no good way of doing this other than
> providing a struct device, then so be it. I think the cleaned up sysfs
> shown in the summary above looks much better than what the original
> would've looked like.
>
> Perhaps an additional tweak to that would be to not create platform
> devices. Instead, just create struct device. Those really have
> everything you need (.of_node, and can be used with RPM and GENPD). As I
> mentioned earlier, platform device implies a CPU-memory-mapped bus,
> which this clearly isn't. It's kind of a separate "bus" if you want, so
> just using struct device directly seems more appropriate.

Just a heads up. If you don't use a platform device or have a driver
associated with it for probing, you need to manage the attachment to
genpd yourself. That means calling one of the dev_pm_domain_attach*()
APIs, but that's perfectly fine, ofcourse.

>
> We did something similar for XUSB pads, see drivers/phy/tegra/xusb.[ch]
> for an example of how that was done. I think you can do something
> similar here.
>
> Thierry

Kind regards
Uffe