On Wed, May 14, 2014 at 09:35:18PM +0200, Mike Turquette wrote: > Quoting Thierry Reding (2014-05-14 07:27:40) > > On Tue, May 13, 2014 at 12:09:49PM -0600, Stephen Warren wrote: > > > On 05/13/2014 08:06 AM, Peter De Schrijver wrote: > > > > Add shared and cbus clocks to the Tegra124 clock implementation. > > > > > > > diff --git a/include/dt-bindings/clock/tegra124-car.h b/include/dt-bindings/clock/tegra124-car.h > > > > > > > +#define TEGRA124_CLK_C2BUS 401 > > > > +#define TEGRA124_CLK_C3BUS 402 > > > > +#define TEGRA124_CLK_GR3D_CBUS 403 > > > > +#define TEGRA124_CLK_GR2D_CBUS 404 > > > ... > > > > > > I worry about this a bit. IIUC, these clocks don't actually exist in HW, > > > but are more a way of SW applying policy to the clock that do exist in > > > HW. As such, I'm not convinced it's a good idea to expose these clock > > > IDS to DT, since DT is supposed to represent the HW, and not be > > > influenced by internal SW implementation details. > > > > > > Do any DTs actually need to used these new clock IDs? I don't think we > > > could actually use these value in e.g. tegra124.dtsi's clocks > > > properties, since these clocks don't exist in HW. Was it your intent to > > > do that? If not, can't we just define these SW-internal clock IDs in the > > > header inside the Tegra clock driver, so the values are invisible to DT? > > > > I'm beginning to wonder if abusing clocks in this way is really the best > > solution. From what I understand there are two problems here that are > > mostly orthogonal though they're implemented using similar techniques. > > Ack. "Virtual clocks" have been implemented by vendors before as a way > to manage complicated clock rate changes. I do not think we should > support such a method upstream. > > I'm working with another engineer in Linaro on a "coordinated clock rate > change" series that might help solve some of the problems that this > patch series is trying to achieve. > Any preview? :) For us to be useful it needs to be possible to: 1) change to a different parent during a rate change 2) adjust a clocks divider when changing parents 3) ignore disabled child clocks 4) have notifiers to hook voltage scaling into > > > > The reason for introducing cbus clocks are still unclear to me. From the > > cover letter of this patch series it seems like these should be > > completely hidden from drivers and as such they don't belong in device > > tree. Also if they are an implementation detail, why are they even > > implemented as clocks? Perhaps an example use-case would help illustrate > > the need for this. > > I also have this question. Does "cbus" come from your TRM or data sheet? > Or is it purely a software solution to coordinating rate changes within > known limits and for validated combinations? > cbus is a software solution. It's not menioned in any TRM or hardware document. Cheers, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
