Quoting Mike Looijmans (2019-06-27 04:38:16) > On 26-06-19 23:24, Stephen Boyd wrote: > > 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? > > Actual scenario's. > > Clocks are required for FPGA logic, and a some of those do not involve > software drivers at all. > > The GTR transceivers on the Xilinx ZynqMP chips use these clocks for SATA and > PCIe, but the driver implementation from Xilinx for these is far from mature, > for example it passes the clock frequency as a PHY parameter and isn't even > aware of the clk framework at the moment. Xilinx hasn't even attempted > submitting this 3 year old driver to mainline. I think they may have submitted the "fixed rate from PHY parameter" support. I merged it because I suspected it would help in those rare cases where an MMIO register is used to express information about the configuration and the bootloader does nothing to help create a fixed rate clk in DT. > > > > 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. > > > > I can see where you going with this, and yes, we definitely should promote > that drivers should take care of their clock (enable) requirements. > > For the case of 'clock-critical', that would serve the purpose quite well > indeed. It does add the risk of people sprinkling that all over the devicetree. > > Short term, I guess the best thing to do here is to remove the "always-on" > property from my patch. Ok. Will you resend or should I look at the latest binding patch and remove always-on? I don't see it there so maybe everything is fine. > > I'll put the "clock-critical" idea on my list of generic clock patches to > sneak in on other budgets, it'll be right behind "allow sub-1Hz or fractional > clock rate accuracy" (yes I actually have a use case for that) and "allow > frequencies over 4GHz" (the 14GHz clock in the Si5341 luckily isn't available > on the outside so I can cheat)... Ok. Good to know it's not as high a priority as these other things.
