On 3/16/22 16:55, Robin Murphy wrote: >> To me that NIU quirk should be internal to the clk h/w module, so it >> doesn't feel nice to mix the clk h/w description with the HDMI h/w >> description. >> >> On the other hand, making clk driver to handle this case indeed will >> take some effort as I see now. For example, clk driver of NVIDIA Tegra >> has concept of shared gates, but bringing it to the RK clk driver will >> be quite messy. > > From a quick look, it seems like it could be straightforward > conceptually at least. Presumably: subclass clk_gate_ops to > enable/disable a required clock before enabling/disabling normally, have > rockchip_clk_register_branch() resolve an optional required clock and > pick gate_ops as appropriate, then the rest is basically just > boilerplate for describing the dependencies in the first place. However > I'd agree that in practical implementation terms it does look even > simpler and cleaner for the clk_hw abstraction to provide the > appropriate ops and resolution itself. > >> Alright, let's work around the clk limitation like you're suggesting. I >> agree that it shouldn't really be a problem to deprecate the extra clock >> later on. > > If there's a realistic chance that someone will actually work on a > proper coupled/dependent/whatever clock abstraction before the rest of > RK3588 is supported well enough for mainline users to start really > caring about power efficiency, then arguably the simplest and cleanest > workaround would be the other option that Elaine mentioned, of just > marking hclk_vo as critical for now. If it's likely to turn into a > "nothing's as permanent as a temporary fix" situation, though, then the > DT binding has less functional impact, even if it does leave us > developers with baggage down the line. I missed that suggestion about marking hclk_vo as critical. That's a good idea, I like it.