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.