On Thu, Aug 22, 2013 at 10:00:35PM +0100, Sascha Hauer wrote: > On Thu, Aug 22, 2013 at 01:09:31PM +0100, Mark Rutland wrote: > > On Thu, Aug 22, 2013 at 08:19:10AM +0100, Mike Turquette wrote: > > > Quoting Tomasz Figa (2013-08-21 14:34:55) > > > > On Wednesday 21 of August 2013 09:50:15 Mark Rutland wrote: > > > > > On Tue, Aug 20, 2013 at 01:06:25AM +0100, Mike Turquette wrote: > > > > > > Quoting Mark Rutland (2013-08-19 02:35:43) > > > > > > > > > > > > > On Sat, Aug 17, 2013 at 04:17:18PM +0100, Tomasz Figa wrote: > > > > > > > > On Saturday 17 of August 2013 16:53:16 Sascha Hauer wrote: > > > > > > > > > On Sat, Aug 17, 2013 at 02:28:04PM +0200, Tomasz Figa wrote: > > > > > > > > > > > > > Also I would make this option required. Use a dummy > > > > > > > > > > > > > clock for > > > > > > > > > > > > > mux > > > > > > > > > > > > > inputs that are grounded for a specific SoC. > > > > > > > > > > > > > > > > > > > > > > > > Some clocks are not from CCM and we haven't defined in > > > > > > > > > > > > imx6q-clk.txt, > > > > > > > > > > > > so in most cases we can't provide a phandle for them, eg: > > > > > > > > > > > > spdif_ext. > > > > > > > > > > > > I think it's a bit hard to force it to be 'required'. An > > > > > > > > > > > > 'optional' > > > > > > > > > > > > looks more flexible to me and a default one is ensured > > > > > > > > > > > > even if > > > > > > > > > > > > it's > > > > > > > > > > > > missing. > > > > > > > > > > > > > > > > > > > > > > <&clks 0> is the dummy clock. This can be used for all input > > > > > > > > > > > clocks > > > > > > > > > > > not > > > > > > > > > > > defined by the SoC. > > > > > > > > > > > > > > > > > > > > Where does this assumption come from? Is it documented > > > > > > > > > > anywhere? > > > > > > > > > > > > > > > > > > This is how all i.MX clock bindings currently are. See > > > > > > > > > Documentation/devicetree/bindings/clock/imx*-clock.txt > > > > > > > > > > > > > > > > OK, thanks. > > > > > > > > > > > > > > > > I guess we need some discussion on dummy clocks vs skipped clocks. > > > > > > > > I think we want some consistency on this, don't we? > > > > > > > > > > > > > > > > If we really need a dummy clock, then we might also want a generic > > > > > > > > way to specify it. > > > > > > > > > > > > > > What do we actually mean by a "dummy clock"? We already have > > > > > > > bindings > > > > > > > for "fixed-clock" and co friends describe relatively simple > > > > > > > preconfigured clocks. > > > > > > > > > > > > Some platforms have a fake clock which defines noops callbacks and > > > > > > basically doesn't do anything. This is analogous to the dummy > > > > > > regulator > > > > > > implementation. A central one could be registered by the clock core, > > > > > > as > > > > > > is done by the regulator core. > > > > > > > > > > When you say some platforms, you presumably mean the platform code in > > > > > Linux? A dummy clock sounds like a completely Linux-specific abstraction > > > > > rather than a description of the hardware, and I don't see why we need > > > > > that in the DT: > > > > > > > > > > * If a clock is wired up and running (as presumably the dummy clock is), > > > > > then surely it's a fixed-clock (it's running, we and we have no control > > > > > over it, but we presumably know its rate) and can be described as such? > > > > > > > > > > * If no clock is wired up, then we should be able to describe that. If a > > > > > driver believes that a clock is required when it isn't (for some level > > > > > of functionality), then that driver should be fixed up to support the > > > > > clock as being optional. > > > > > > > > > > Am I missing something? > > > > > > > > I second that. > > > > > > > > Moreover, I don't think that device tree should deal with dummy anything. > > > > It should be able to describe hardware that is available on given system, > > > > not list what hardware is not available. > > > > > > I wasn't clear. The dummy clock IS a completely Linux-specific > > > abstraction. > > > > > > I'm not advocating a dummy clock in DT. I am advocating consolidation of > > > the implementation of a clock that does nothing into the clock core. > > > This code could easily live in drivers/clk/clk.c instead of having > > > everyone open-code it. > > > > > > As far as specifying a dummy clock in DT? I dunno. DT should describe > > > real hardware so there isn't much use for a dummy clock. > > > > > > Sorry, I misunderstood. Good to hear we're on the same page :) > > > > > > > > I'm guessing one of the reasons for such a clock are drivers do not > > > honor the clk.h api and they freak out when clk_get gives them a NULL > > > pointer? > > > > I'm not sure. Sascha, could you shed some light on the matter? > > The original reason introducing the dummy clocks in the i.MX dtbs > was to provide devices a clock which the driver requests but is > not software controllable. We often have the case where the same > devices are on several SoCs, but not on all of them all clocks have > a bit to en/disable them. So those clocks are always on at some fixed rate? > > Anyway, to accomplish this we don't need dummy clocks. We can just > describe the real clocks. Ok. > > BTW with the S/PDIF core on which not all mux inputs are connected > to actual clocks we could also describe the unconnected inputs as > ground clocks with rate 0. This way we describe something which > is really there instead of dummy clocks ;) > > Background to why it might be a good idea to connect a ground clock > to the unconnected input pins is that a driver has a chance to > successfully grab all clocks. Otherwise how does the driver distinguish > between an unconnected and an erroneous clock? Surely the presence of a name in "clock-names" tells you that a clock is connected, and if not listed it's not connected? If we don't have the infrastructure for telling apart unconnected clocks from inability to enable/configure clocks, that's a Linux issue rather than something that should be hacked up in the dt. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html