Quoting Michael Tretter (2020-12-15 03:56:32) > On Sat, 12 Dec 2020 21:50:00 -0800, Stephen Boyd wrote: > > Quoting Michael Tretter (2020-11-15 23:55:20) > > > Hello, > > > > > > the xlnx_vcu soc driver is actually a clock provider of a PLL and four output > > > clocks created from the PLL via dividers. > > > > > > This series reworks the xlnx_vcu driver to use the common clock framework to > > > enable other drivers to use the clocks. I originally posted a series to expose > > > the output clocks as fixed clocks [0]. This series now implements the full > > > tree from the PLL to the output clocks. Therefore, I am sending a separate > > > series that focuses on the clocks, but it depends on v4 of the previous series > > > [1]. > > > > After this series is this anything besides a clk provider? If it's only > > providing clks it would make sense to move the driver into drivers/clk/ > > > > 1. The driver is also responsible for resetting the entire VCU (the > VCU_GASKET_INIT register). This isn't something that an individual encoder or > decoder driver should be doing. However, other clock drivers also implement a > reset controller. Right. > > 2. There are several registers for AXI performance monitoring in the VCU > System-Level Control register space. Right now, these are not used by the > driver and I have no plans to actually use them, but this might be an argument > against the move. I suppose if/when that happens we can have a small parent driver that probes the compatible string and makes two child platform devices, one for the clk part and one for the PMU? That would let us keep the code in drivers/clk/ for ease of find-ability. This assumes that the PMU registers don't overlap with the clk/reset registers. We usually put the clk and reset controllers together if they use the same registers and need to make sure the frameworks don't stomp on each other. > > I think it is OK to move the driver to drivers/clk/, but I don't have a strong > opinion about it. > Ok. I'm not too strong on it either, but drivers/soc/ is sort of a dumping ground for random soc things. I'm not looking at it closely but if the driver is in drivers/clk/ I'd be more inclined to look after the clk bits.
