Sorry, I'm getting through my inbox pile and saw this one. Quoting Mike Looijmans (2019-04-30 22:46:55) > On 30-04-19 02:17, Stephen Boyd wrote: > > > > Why can't that driver call clk_prepare_enable()? Is there some sort of > > assumption that this clk will always be enabled and not have a driver > > that configures the rate and gates/ungates it? > > Not only clk_prepare_enable(), but the driver could also call clk_set_rate() > and clk_set_parent() and the likes, but it doesn't, so that's why there is > "assigned-clocks" right? > > There are multiple scenario's, similar to why regulators also have properties > like these. > > - The clock is related to hardware that the kernel is not aware of. > - The clock is for a driver that isn't aware of its clock requirements. It > might be an extra clock for an FPGA implemented controller that mimics > existing hardware. Are these hypothetical scenarios or actual scenarios you need to support? > > I'd also consider patching "assigned-clocks" to call "clk_prepare_enable()", > would that make sense, or is it intentional that assigned-clocks doesn't do that? > It's intentional that assigned-clocks doesn't really do anything besides setup the list of clks to operate on with the rate or parent settings specified in other properties. We would need to add another property indicating which clks we want to mark as 'critical' or 'always-on'. There have been prior discussions where we had developers want to mark clks with the CLK_IS_CRITICAL flag from DT, but we felt like that was putting SoC level details into the DT. While that was correct for SoC specific clk drivers, I can see board designs where it's configurable and we want to express that a board has some clks that must be enabled early on and left enabled forever because the hardware engineer has design the board that way. In this case, marking the clk with the CLK_IS_CRITICAL flag needs to be done from DT. In fact, we pretty much already have support for this with of_clk_detect_critical(). Maybe we should re-use that binding with 'clock-critical' to let clk providers indicate that they have some clks that should be marked critical once they're registered. We could probably add another property too that indicates certain clks are enabled out of the bootloader, similar to the regulator-boot-on property, but where it takes a clock-cells wide list for the provider the property is inside of. We need to be careful though and make sure that clk drivers don't start putting everything in DT. In your example, it sounds like you have a consumer driver that wants to make sure the clk is prepared or enabled when the consumer probes. In this case the prepare/enable calls should be put directly into the consumer driver so it can manage the clk state. For the case of rates and parents, it's essentially a oneshot configuration of the rate or the parents of a clk. We don't need to "undo" the configuration when the device driver is removed. For prepare and enable though, we typically want to disable clks when the hardware is not in use to save power. Adding a property to DT should only be done to indicate a clk must always be on in this board configuration, not to avoid calling clk_prepare_enable() in the driver probe. To put it another way, I'm looking to describe how the board is designed and to indicate that certain clks are always enabled at power on or are enabled by the bootloader. Configuration has it's place too, just that configuration is a oneshot sort of thing that never needs to change at runtime.
