diff for duplicates of <20130823214144.8231.2039@quantum> diff --git a/a/1.txt b/N1/1.txt index d97acb7..0b90b5f 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -4,197 +4,301 @@ Quoting Sascha Hauer (2013-08-23 07:01:28) > > > On Fri, Aug 23, 2013 at 12:49:19AM +0200, Tomasz Figa wrote: > > > > On Thursday 22 of August 2013 15:43:31 Mike Turquette wrote: > > > > > Quoting Sascha Hauer (2013-08-22 14:00:35) -> > > > > +> > > > > = + > > > > > > 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: +> > > > > > > On Thu, Aug 22, 2013 at 08:19:10AM +0100, Mike Turquette wrot= +e: > > > > > > > > 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 +> > > > > > > > = + +> > > > > > > > > On Wednesday 21 of August 2013 09:50:15 Mark Rutland wrot= +e: +> > > > > > > > > > On Tue, Aug 20, 2013 at 01:06:25AM +0100, Mike Turquett= +e = + > > > > wrote: > > > > > > > > > > > Quoting Mark Rutland (2013-08-19 02:35:43) -> > > > > > > > > > > -> > > > > > > > > > > > On Sat, Aug 17, 2013 at 04:17:18PM +0100, Tomasz Figa +> > > > > > > > > > > = + +> > > > > > > > > > > > On Sat, Aug 17, 2013 at 04:17:18PM +0100, Tomasz Fi= +ga = + > > > > wrote: -> > > > > > > > > > > > > On Saturday 17 of August 2013 16:53:16 Sascha Hauer +> > > > > > > > > > > > > On Saturday 17 of August 2013 16:53:16 Sascha Hau= +er = + > > > > wrote: -> > > > > > > > > > > > > > On Sat, Aug 17, 2013 at 02:28:04PM +0200, Tomasz Figa +> > > > > > > > > > > > > > On Sat, Aug 17, 2013 at 02:28:04PM +0200, Tomas= +z Figa = + > > > > wrote: -> > > > > > > > > > > > > > > > > > Also I would make this option required. Use a +> > > > > > > > > > > > > > > > > > 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 +> > > > > > > > > > > > > > > > > > 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 +> > > > > > > > > > > > > > > > > so in most cases we can't provide a phand= +le 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 +> > > > > > > > > > > > > > > > > looks more flexible to me and a default o= +ne is > > > > > > > > > > > > > > > > > ensured > > > > > > > > > > > > > > > > > even if > > > > > > > > > > > > > > > > > it's > > > > > > > > > > > > > > > > > missing. -> > > > > > > > > > > > > > > > -> > > > > > > > > > > > > > > > <&clks 0> is the dummy clock. This can be used for +> > > > > > > > > > > > > > > > = + +> > > > > > > > > > > > > > > > <&clks 0> is the dummy clock. This can be u= +sed 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 -> > > > > > > > > > > > > +> > > > > > > > > > > > > > = + +> > > > > > > > > > > > > > This is how all i.MX clock bindings currently a= +re. See +> > > > > > > > > > > > > > Documentation/devicetree/bindings/clock/imx*-cl= +ock.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 +> > > > > > > > > > > > > I think we want some consistency on this, don't w= +e? +> > > > > > > > > > > > > = + +> > > > > > > > > > > > > If we really need a dummy clock, then we might al= +so want > > > > > > > > > > > > > a generic > > > > > > > > > > > > > way to specify it. -> > > > > > > > > > > > -> > > > > > > > > > > > What do we actually mean by a "dummy clock"? We already +> > > > > > > > > > > > = + +> > > > > > > > > > > > What do we actually mean by a "dummy clock"? We alr= +eady > > > > > > > > > > > > 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 +> > > > > > > > > > > basically doesn't do anything. This is analogous to t= +he > > > > > > > > > > > dummy > > > > > > > > > > > regulator -> > > > > > > > > > > implementation. A central one could be registered by the +> > > > > > > > > > > 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 +> > > > > > > > > > = + +> > > > > > > > > > When you say some platforms, you presumably mean the pl= +atform > > > > > > > > > > code in -> > > > > > > > > > Linux? A dummy clock sounds like a completely Linux-specific -> > > > > > > > > > abstraction rather than a description of the hardware, and I +> > > > > > > > > > Linux? A dummy clock sounds like a completely Linux-spe= +cific +> > > > > > > > > > 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 +> > > > > > > > > > = + +> > > > > > > > > > * 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. -> > > > > > > > > > +> > > > > > > > > > = + +> > > > > > > > > > * If no clock is wired up, then we should be able to de= +scribe +> > > > > > > > > > that. If a driver believes that a clock is required whe= +n it +> > > > > > > > > > isn't (for some level of functionality), then that driv= +er +> > > > > > > > > > should be fixed up to support the clock as being option= +al. +> > > > > > > > > > = + > > > > > > > > > > Am I missing something? -> > > > > > > > > +> > > > > > > > > = + > > > > > > > > > I second that. -> > > > > > > > > -> > > > > > > > > Moreover, I don't think that device tree should deal with dummy +> > > > > > > > > = + +> > > > > > > > > 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 +> > > > > > > > = + +> > > > > > > > I wasn't clear. The dummy clock IS a completely Linux-speci= +fic > > > > > > > > abstraction. -> > > > > > > > +> > > > > > > > = + > > > > > > > > I'm not advocating a dummy clock in DT. I am advocating -> > > > > > > > consolidation of the implementation of a clock that does nothing +> > > > > > > > consolidation of the implementation of a clock that does no= +thing > > > > > > > > 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 +> > > > > > > = + +> > > > > > > > I'm guessing one of the reasons for such a clock are driver= +s do > > > > > > > > not -> > > > > > > > honor the clk.h api and they freak out when clk_get gives them a +> > > > > > > > honor the clk.h api and they freak out when clk_get gives t= +hem 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 +> > > > > > = + +> > > > > > The original reason introducing the dummy clocks in the i.MX dt= +bs > > > > > > 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 +> > > > > > devices are on several SoCs, but not on all of them all clocks = +have > > > > > > a bit to en/disable them. -> > > > > > -> > > > > > Anyway, to accomplish this we don't need dummy clocks. We can just +> > > > > > = + +> > > > > > Anyway, to accomplish this we don't need dummy clocks. We can j= +ust > > > > > > describe the real clocks. -> > > > > -> > > > > You could use a dummy clk for the Linux implementation, but the downside -> > > > > is that a dummy clock has a rate of 0 always and a your clocks likely +> > > > > = + +> > > > > You could use a dummy clk for the Linux implementation, but the d= +ownside +> > > > > is that a dummy clock has a rate of 0 always and a your clocks li= +kely > > > > > have non-zero rates. -> > > > > -> > > > > It is probably better for you define a clock which only implements the -> > > > > .recalc_rate callback. If the rate of this clock changes without Linux +> > > > > = + +> > > > > It is probably better for you define a clock which only implement= +s the +> > > > > .recalc_rate callback. If the rate of this clock changes without = +Linux > > > > > having knowledge of it you can use the CLK_GET_RATE_NOCACHE flag. -> > > > -> > > > I doubt that rate of a dummy clock could ever change... unless it is a +> > > > = + +> > > > I doubt that rate of a dummy clock could ever change... unless it i= +s a = + > > > > rather smart dummy. -> > > > -> > > > > > 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 +> > > > = + +> > > > > > BTW with the S/PDIF core on which not all mux inputs are connec= +ted +> > > > > > 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 ;) -> > > > > -> > > > > Again you could use a dummy clock for this OR a fixed-rate clock with a +> > > > > = + +> > > > > Again you could use a dummy clock for this OR a fixed-rate clock = +with a > > > > > rate of zero from the perspective of the Linux implementation. -> > > > > -> > > > > Do you think it worthwhile to have a DT binding for a grounded clock? +> > > > > = + +> > > > > Do you think it worthwhile to have a DT binding for a grounded cl= +ock? > > > > > That is not an entirely uncommon case. -> > > > -> > > > Well, how would that differ from skipping a clock from clocks list, i.e. +> > > > = + +> > > > Well, how would that differ from skipping a clock from clocks list,= + i.e. = + > > > > not specifying it in clock-names and clocks properties? -> > > +> > > = + > > > The difference is that you can successfully grab it in your driver. -> > +> > = + > > That's a driver-specific issue. The driver knows best which clocks it > > can live without (if it's poking only a subset of the hardware, it may > > not need some just yet, but could for extended functionality in future > > when support is extended), and could assign a dummy to those clocks it > > knows it doesn't need that aren't described. That doesn't need to be in > > the dt, and shouldn't be, because it's OS and driver specific. -> > -> > > -> > > > -> > > > > > Background to why it might be a good idea to connect a ground clock +> > = + +> > > = + +> > > > = + +> > > > > > Background to why it might be a good idea to connect a ground c= +lock > > > > > > 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? -> > > > > -> > > > > Sorry, I don't follow this last question. Do you mean how to distinguish +> > > > > = + +> > > > > Sorry, I don't follow this last question. Do you mean how to dist= +inguish > > > > > based on the value returned from clk_get? -> > > > -> > > > Hmm, in theory, a driver could want to distinguish an error case (e.g. -> > > > clock specified, but there is a problem with it) from no clock (e.g. clock -> > > > not specified in DT, because it is not available on particular board). -> > > +> > > > = + +> > > > Hmm, in theory, a driver could want to distinguish an error case (e= +.g. = + +> > > > clock specified, but there is a problem with it) from no clock (e.g= +. clock = + +> > > > not specified in DT, because it is not available on particular boar= +d). +> > > = + > > > Yes, that's what I meant. To illustrate the problem for this driver: -> > > -> > > for (i = 0; i < STC_TXCLK_SRC_MAX; i++) { +> > > = + +> > > for (i =3D 0; i < STC_TXCLK_SRC_MAX; i++) { > > > sprintf(tmp, "rxtx%d", i); -> > > clk = devm_clk_get(&pdev->dev, tmp); +> > > clk =3D devm_clk_get(&pdev->dev, tmp); > > > if (IS_ERR(clk)) { -> > +> > = + > > [...] -> > +> > = + > > /* > > * ERR_PTR(-ENOENT) returned when clock not > > * present in the dt (i.e. not wired up). We can @@ -204,18 +308,22 @@ Quoting Sascha Hauer (2013-08-23 07:01:28) > > * wrong, we'll get a different ERR_PTR value > > * and actually fail. > > */ -> > if (clk == ERR_PTR(-ENOENT) -> > clk = NULL; -> > +> > if (clk =3D=3D ERR_PTR(-ENOENT) +> > clk =3D NULL; +> > = + > > > } > > > } -> > > +> > > = + > > > This could be solved by always specifying all input clocks in the > > > devicetree. -> > +> > = + > > As far as I can see, the above is sufficient, and leaves the knowledge > > of skippable clocks in the driver, where I believe it should be. -> +> = + > You miss my point. Once a clock is specified in the devicetree it is no > longer optional. The driver currently can't find out whether a clock > hasn't been specified (and it's ok that it's not there) or a clock has @@ -229,11 +337,15 @@ optional-versus-not-optional logic up to the driver. Regards, Mike -> +> = + > Sascha -> -> -> -- +> = + +> = + +> -- = + > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | diff --git a/a/content_digest b/N1/content_digest index dd188c9..33ce9af 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -6,7 +6,7 @@ "ref\020130823125815.GG25856@e106331-lin.cambridge.arm.com\0" "ref\020130823140128.GI31036@pengutronix.de\0" "From\0Mike Turquette <mturquette@linaro.org>\0" - "Subject\0Re: [PATCH v5 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver\0" + "Subject\0Re: [alsa-devel] [PATCH v5 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver\0" "Date\0Fri, 23 Aug 2013 14:41:44 -0700\0" "To\0Sascha Hauer <s.hauer@pengutronix.de>" " Mark Rutland <mark.rutland@arm.com>\0" @@ -34,197 +34,301 @@ "> > > On Fri, Aug 23, 2013 at 12:49:19AM +0200, Tomasz Figa wrote:\n" "> > > > On Thursday 22 of August 2013 15:43:31 Mike Turquette wrote:\n" "> > > > > Quoting Sascha Hauer (2013-08-22 14:00:35)\n" - "> > > > > \n" + "> > > > > =\n" + "\n" "> > > > > > On Thu, Aug 22, 2013 at 01:09:31PM +0100, Mark Rutland wrote:\n" - "> > > > > > > On Thu, Aug 22, 2013 at 08:19:10AM +0100, Mike Turquette wrote:\n" + "> > > > > > > On Thu, Aug 22, 2013 at 08:19:10AM +0100, Mike Turquette wrot=\n" + "e:\n" "> > > > > > > > Quoting Tomasz Figa (2013-08-21 14:34:55)\n" - "> > > > > > > > \n" - "> > > > > > > > > On Wednesday 21 of August 2013 09:50:15 Mark Rutland wrote:\n" - "> > > > > > > > > > On Tue, Aug 20, 2013 at 01:06:25AM +0100, Mike Turquette \n" + "> > > > > > > > =\n" + "\n" + "> > > > > > > > > On Wednesday 21 of August 2013 09:50:15 Mark Rutland wrot=\n" + "e:\n" + "> > > > > > > > > > On Tue, Aug 20, 2013 at 01:06:25AM +0100, Mike Turquett=\n" + "e =\n" + "\n" "> > > > wrote:\n" "> > > > > > > > > > > Quoting Mark Rutland (2013-08-19 02:35:43)\n" - "> > > > > > > > > > > \n" - "> > > > > > > > > > > > On Sat, Aug 17, 2013 at 04:17:18PM +0100, Tomasz Figa \n" + "> > > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > > > On Sat, Aug 17, 2013 at 04:17:18PM +0100, Tomasz Fi=\n" + "ga =\n" + "\n" "> > > > wrote:\n" - "> > > > > > > > > > > > > On Saturday 17 of August 2013 16:53:16 Sascha Hauer \n" + "> > > > > > > > > > > > > On Saturday 17 of August 2013 16:53:16 Sascha Hau=\n" + "er =\n" + "\n" "> > > > wrote:\n" - "> > > > > > > > > > > > > > On Sat, Aug 17, 2013 at 02:28:04PM +0200, Tomasz Figa \n" + "> > > > > > > > > > > > > > On Sat, Aug 17, 2013 at 02:28:04PM +0200, Tomas=\n" + "z Figa =\n" + "\n" "> > > > wrote:\n" - "> > > > > > > > > > > > > > > > > > Also I would make this option required. Use a\n" + "> > > > > > > > > > > > > > > > > > Also I would make this option required.=\n" + " Use a\n" "> > > > > > > > > > > > > > > > > > dummy\n" "> > > > > > > > > > > > > > > > > > clock for\n" "> > > > > > > > > > > > > > > > > > mux\n" - "> > > > > > > > > > > > > > > > > > inputs that are grounded for a specific SoC.\n" - "> > > > > > > > > > > > > > > > > \n" - "> > > > > > > > > > > > > > > > > Some clocks are not from CCM and we haven't\n" + "> > > > > > > > > > > > > > > > > > inputs that are grounded for a specific=\n" + " SoC.\n" + "> > > > > > > > > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > > > > > > > > Some clocks are not from CCM and we haven=\n" + "'t\n" "> > > > > > > > > > > > > > > > > defined in\n" "> > > > > > > > > > > > > > > > > imx6q-clk.txt,\n" - "> > > > > > > > > > > > > > > > > so in most cases we can't provide a phandle for\n" + "> > > > > > > > > > > > > > > > > so in most cases we can't provide a phand=\n" + "le for\n" "> > > > > > > > > > > > > > > > > them, eg:\n" "> > > > > > > > > > > > > > > > > spdif_ext.\n" "> > > > > > > > > > > > > > > > > I think it's a bit hard to force it to be\n" "> > > > > > > > > > > > > > > > > 'required'. An\n" "> > > > > > > > > > > > > > > > > 'optional'\n" - "> > > > > > > > > > > > > > > > > looks more flexible to me and a default one is\n" + "> > > > > > > > > > > > > > > > > looks more flexible to me and a default o=\n" + "ne is\n" "> > > > > > > > > > > > > > > > > ensured\n" "> > > > > > > > > > > > > > > > > even if\n" "> > > > > > > > > > > > > > > > > it's\n" "> > > > > > > > > > > > > > > > > missing.\n" - "> > > > > > > > > > > > > > > > \n" - "> > > > > > > > > > > > > > > > <&clks 0> is the dummy clock. This can be used for\n" + "> > > > > > > > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > > > > > > > <&clks 0> is the dummy clock. This can be u=\n" + "sed for\n" "> > > > > > > > > > > > > > > > all input\n" "> > > > > > > > > > > > > > > > clocks\n" "> > > > > > > > > > > > > > > > not\n" "> > > > > > > > > > > > > > > > defined by the SoC.\n" - "> > > > > > > > > > > > > > > \n" + "> > > > > > > > > > > > > > > =\n" + "\n" "> > > > > > > > > > > > > > > Where does this assumption come from? Is it\n" "> > > > > > > > > > > > > > > documented\n" "> > > > > > > > > > > > > > > anywhere?\n" - "> > > > > > > > > > > > > > \n" - "> > > > > > > > > > > > > > This is how all i.MX clock bindings currently are. See\n" - "> > > > > > > > > > > > > > Documentation/devicetree/bindings/clock/imx*-clock.txt\n" - "> > > > > > > > > > > > > \n" + "> > > > > > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > > > > > This is how all i.MX clock bindings currently a=\n" + "re. See\n" + "> > > > > > > > > > > > > > Documentation/devicetree/bindings/clock/imx*-cl=\n" + "ock.txt\n" + "> > > > > > > > > > > > > =\n" + "\n" "> > > > > > > > > > > > > OK, thanks.\n" - "> > > > > > > > > > > > > \n" + "> > > > > > > > > > > > > =\n" + "\n" "> > > > > > > > > > > > > I guess we need some discussion on dummy clocks vs\n" "> > > > > > > > > > > > > skipped clocks.\n" - "> > > > > > > > > > > > > I think we want some consistency on this, don't we?\n" - "> > > > > > > > > > > > > \n" - "> > > > > > > > > > > > > If we really need a dummy clock, then we might also want\n" + "> > > > > > > > > > > > > I think we want some consistency on this, don't w=\n" + "e?\n" + "> > > > > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > > > > If we really need a dummy clock, then we might al=\n" + "so want\n" "> > > > > > > > > > > > > a generic\n" "> > > > > > > > > > > > > way to specify it.\n" - "> > > > > > > > > > > > \n" - "> > > > > > > > > > > > What do we actually mean by a \"dummy clock\"? We already\n" + "> > > > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > > > What do we actually mean by a \"dummy clock\"? We alr=\n" + "eady\n" "> > > > > > > > > > > > have\n" "> > > > > > > > > > > > bindings\n" "> > > > > > > > > > > > for \"fixed-clock\" and co friends describe relatively\n" "> > > > > > > > > > > > simple\n" "> > > > > > > > > > > > preconfigured clocks.\n" - "> > > > > > > > > > > \n" + "> > > > > > > > > > > =\n" + "\n" "> > > > > > > > > > > Some platforms have a fake clock which defines noops\n" "> > > > > > > > > > > callbacks and\n" - "> > > > > > > > > > > basically doesn't do anything. This is analogous to the\n" + "> > > > > > > > > > > basically doesn't do anything. This is analogous to t=\n" + "he\n" "> > > > > > > > > > > dummy\n" "> > > > > > > > > > > regulator\n" - "> > > > > > > > > > > implementation. A central one could be registered by the\n" + "> > > > > > > > > > > implementation. A central one could be registered by =\n" + "the\n" "> > > > > > > > > > > clock core,\n" "> > > > > > > > > > > as\n" "> > > > > > > > > > > is done by the regulator core.\n" - "> > > > > > > > > > \n" - "> > > > > > > > > > When you say some platforms, you presumably mean the platform\n" + "> > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > When you say some platforms, you presumably mean the pl=\n" + "atform\n" "> > > > > > > > > > code in\n" - "> > > > > > > > > > Linux? A dummy clock sounds like a completely Linux-specific\n" - "> > > > > > > > > > abstraction rather than a description of the hardware, and I\n" + "> > > > > > > > > > Linux? A dummy clock sounds like a completely Linux-spe=\n" + "cific\n" + "> > > > > > > > > > abstraction rather than a description of the hardware, =\n" + "and I\n" "> > > > > > > > > > don't see why we need that in the DT:\n" - "> > > > > > > > > > \n" - "> > > > > > > > > > * If a clock is wired up and running (as presumably the dummy\n" - "> > > > > > > > > > clock is), then surely it's a fixed-clock (it's running, we\n" - "> > > > > > > > > > and we have no control over it, but we presumably know its\n" + "> > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > * If a clock is wired up and running (as presumably the=\n" + " dummy\n" + "> > > > > > > > > > clock is), then surely it's a fixed-clock (it's running=\n" + ", we\n" + "> > > > > > > > > > and we have no control over it, but we presumably know =\n" + "its\n" "> > > > > > > > > > rate) and can be described as such?\n" - "> > > > > > > > > > \n" - "> > > > > > > > > > * If no clock is wired up, then we should be able to describe\n" - "> > > > > > > > > > that. If a driver believes that a clock is required when it\n" - "> > > > > > > > > > isn't (for some level of functionality), then that driver\n" - "> > > > > > > > > > should be fixed up to support the clock as being optional.\n" - "> > > > > > > > > > \n" + "> > > > > > > > > > =\n" + "\n" + "> > > > > > > > > > * If no clock is wired up, then we should be able to de=\n" + "scribe\n" + "> > > > > > > > > > that. If a driver believes that a clock is required whe=\n" + "n it\n" + "> > > > > > > > > > isn't (for some level of functionality), then that driv=\n" + "er\n" + "> > > > > > > > > > should be fixed up to support the clock as being option=\n" + "al.\n" + "> > > > > > > > > > =\n" + "\n" "> > > > > > > > > > Am I missing something?\n" - "> > > > > > > > > \n" + "> > > > > > > > > =\n" + "\n" "> > > > > > > > > I second that.\n" - "> > > > > > > > > \n" - "> > > > > > > > > Moreover, I don't think that device tree should deal with dummy\n" + "> > > > > > > > > =\n" + "\n" + "> > > > > > > > > Moreover, I don't think that device tree should deal with=\n" + " dummy\n" "> > > > > > > > > anything. It should be able to describe hardware that is\n" "> > > > > > > > > available on given system, not list what hardware is not\n" "> > > > > > > > > available.\n" - "> > > > > > > > \n" - "> > > > > > > > I wasn't clear. The dummy clock IS a completely Linux-specific\n" + "> > > > > > > > =\n" + "\n" + "> > > > > > > > I wasn't clear. The dummy clock IS a completely Linux-speci=\n" + "fic\n" "> > > > > > > > abstraction.\n" - "> > > > > > > > \n" + "> > > > > > > > =\n" + "\n" "> > > > > > > > I'm not advocating a dummy clock in DT. I am advocating\n" - "> > > > > > > > consolidation of the implementation of a clock that does nothing\n" + "> > > > > > > > consolidation of the implementation of a clock that does no=\n" + "thing\n" "> > > > > > > > into the clock core. This code could easily live in\n" "> > > > > > > > drivers/clk/clk.c instead of having everyone open-code it.\n" - "> > > > > > > > \n" + "> > > > > > > > =\n" + "\n" "> > > > > > > > As far as specifying a dummy clock in DT? I dunno. DT should\n" "> > > > > > > > describe\n" "> > > > > > > > real hardware so there isn't much use for a dummy clock.\n" - "> > > > > > > \n" + "> > > > > > > =\n" + "\n" "> > > > > > > Sorry, I misunderstood. Good to hear we're on the same page :)\n" - "> > > > > > > \n" - "> > > > > > > > I'm guessing one of the reasons for such a clock are drivers do\n" + "> > > > > > > =\n" + "\n" + "> > > > > > > > I'm guessing one of the reasons for such a clock are driver=\n" + "s do\n" "> > > > > > > > not\n" - "> > > > > > > > honor the clk.h api and they freak out when clk_get gives them a\n" + "> > > > > > > > honor the clk.h api and they freak out when clk_get gives t=\n" + "hem a\n" "> > > > > > > > NULL\n" "> > > > > > > > pointer?\n" - "> > > > > > > \n" + "> > > > > > > =\n" + "\n" "> > > > > > > I'm not sure. Sascha, could you shed some light on the matter?\n" - "> > > > > > \n" - "> > > > > > The original reason introducing the dummy clocks in the i.MX dtbs\n" + "> > > > > > =\n" + "\n" + "> > > > > > The original reason introducing the dummy clocks in the i.MX dt=\n" + "bs\n" "> > > > > > was to provide devices a clock which the driver requests but is\n" "> > > > > > not software controllable. We often have the case where the same\n" - "> > > > > > devices are on several SoCs, but not on all of them all clocks have\n" + "> > > > > > devices are on several SoCs, but not on all of them all clocks =\n" + "have\n" "> > > > > > a bit to en/disable them.\n" - "> > > > > > \n" - "> > > > > > Anyway, to accomplish this we don't need dummy clocks. We can just\n" + "> > > > > > =\n" + "\n" + "> > > > > > Anyway, to accomplish this we don't need dummy clocks. We can j=\n" + "ust\n" "> > > > > > describe the real clocks.\n" - "> > > > > \n" - "> > > > > You could use a dummy clk for the Linux implementation, but the downside\n" - "> > > > > is that a dummy clock has a rate of 0 always and a your clocks likely\n" + "> > > > > =\n" + "\n" + "> > > > > You could use a dummy clk for the Linux implementation, but the d=\n" + "ownside\n" + "> > > > > is that a dummy clock has a rate of 0 always and a your clocks li=\n" + "kely\n" "> > > > > have non-zero rates.\n" - "> > > > > \n" - "> > > > > It is probably better for you define a clock which only implements the\n" - "> > > > > .recalc_rate callback. If the rate of this clock changes without Linux\n" + "> > > > > =\n" + "\n" + "> > > > > It is probably better for you define a clock which only implement=\n" + "s the\n" + "> > > > > .recalc_rate callback. If the rate of this clock changes without =\n" + "Linux\n" "> > > > > having knowledge of it you can use the CLK_GET_RATE_NOCACHE flag.\n" - "> > > > \n" - "> > > > I doubt that rate of a dummy clock could ever change... unless it is a \n" + "> > > > =\n" + "\n" + "> > > > I doubt that rate of a dummy clock could ever change... unless it i=\n" + "s a =\n" + "\n" "> > > > rather smart dummy.\n" - "> > > > \n" - "> > > > > > BTW with the S/PDIF core on which not all mux inputs are connected\n" - "> > > > > > to actual clocks we could also describe the unconnected inputs as\n" + "> > > > =\n" + "\n" + "> > > > > > BTW with the S/PDIF core on which not all mux inputs are connec=\n" + "ted\n" + "> > > > > > to actual clocks we could also describe the unconnected inputs =\n" + "as\n" "> > > > > > ground clocks with rate 0. This way we describe something which\n" "> > > > > > is really there instead of dummy clocks ;)\n" - "> > > > > \n" - "> > > > > Again you could use a dummy clock for this OR a fixed-rate clock with a\n" + "> > > > > =\n" + "\n" + "> > > > > Again you could use a dummy clock for this OR a fixed-rate clock =\n" + "with a\n" "> > > > > rate of zero from the perspective of the Linux implementation.\n" - "> > > > > \n" - "> > > > > Do you think it worthwhile to have a DT binding for a grounded clock?\n" + "> > > > > =\n" + "\n" + "> > > > > Do you think it worthwhile to have a DT binding for a grounded cl=\n" + "ock?\n" "> > > > > That is not an entirely uncommon case.\n" - "> > > > \n" - "> > > > Well, how would that differ from skipping a clock from clocks list, i.e. \n" + "> > > > =\n" + "\n" + "> > > > Well, how would that differ from skipping a clock from clocks list,=\n" + " i.e. =\n" + "\n" "> > > > not specifying it in clock-names and clocks properties?\n" - "> > > \n" + "> > > =\n" + "\n" "> > > The difference is that you can successfully grab it in your driver.\n" - "> > \n" + "> > =\n" + "\n" "> > That's a driver-specific issue. The driver knows best which clocks it\n" "> > can live without (if it's poking only a subset of the hardware, it may\n" "> > not need some just yet, but could for extended functionality in future\n" "> > when support is extended), and could assign a dummy to those clocks it\n" "> > knows it doesn't need that aren't described. That doesn't need to be in\n" "> > the dt, and shouldn't be, because it's OS and driver specific.\n" - "> > \n" - "> > > \n" - "> > > > \n" - "> > > > > > Background to why it might be a good idea to connect a ground clock\n" + "> > =\n" + "\n" + "> > > =\n" + "\n" + "> > > > =\n" + "\n" + "> > > > > > Background to why it might be a good idea to connect a ground c=\n" + "lock\n" "> > > > > > to the unconnected input pins is that a driver has a chance to\n" "> > > > > > successfully grab all clocks. Otherwise how does the driver\n" "> > > > > > distinguish\n" "> > > > > > between an unconnected and an erroneous clock?\n" - "> > > > > \n" - "> > > > > Sorry, I don't follow this last question. Do you mean how to distinguish\n" + "> > > > > =\n" + "\n" + "> > > > > Sorry, I don't follow this last question. Do you mean how to dist=\n" + "inguish\n" "> > > > > based on the value returned from clk_get?\n" - "> > > > \n" - "> > > > Hmm, in theory, a driver could want to distinguish an error case (e.g. \n" - "> > > > clock specified, but there is a problem with it) from no clock (e.g. clock \n" - "> > > > not specified in DT, because it is not available on particular board).\n" - "> > > \n" + "> > > > =\n" + "\n" + "> > > > Hmm, in theory, a driver could want to distinguish an error case (e=\n" + ".g. =\n" + "\n" + "> > > > clock specified, but there is a problem with it) from no clock (e.g=\n" + ". clock =\n" + "\n" + "> > > > not specified in DT, because it is not available on particular boar=\n" + "d).\n" + "> > > =\n" + "\n" "> > > Yes, that's what I meant. To illustrate the problem for this driver:\n" - "> > > \n" - "> > > for (i = 0; i < STC_TXCLK_SRC_MAX; i++) {\n" + "> > > =\n" + "\n" + "> > > for (i =3D 0; i < STC_TXCLK_SRC_MAX; i++) {\n" "> > > sprintf(tmp, \"rxtx%d\", i);\n" - "> > > clk = devm_clk_get(&pdev->dev, tmp);\n" + "> > > clk =3D devm_clk_get(&pdev->dev, tmp);\n" "> > > if (IS_ERR(clk)) {\n" - "> > \n" + "> > =\n" + "\n" "> > [...]\n" - "> > \n" + "> > =\n" + "\n" "> > /*\n" "> > * ERR_PTR(-ENOENT) returned when clock not\n" "> > * present in the dt (i.e. not wired up). We can\n" @@ -234,18 +338,22 @@ "> > * wrong, we'll get a different ERR_PTR value\n" "> > * and actually fail.\n" "> > */\n" - "> > if (clk == ERR_PTR(-ENOENT)\n" - "> > clk = NULL;\n" - "> > \n" + "> > if (clk =3D=3D ERR_PTR(-ENOENT)\n" + "> > clk =3D NULL;\n" + "> > =\n" + "\n" "> > > }\n" "> > > }\n" - "> > > \n" + "> > > =\n" + "\n" "> > > This could be solved by always specifying all input clocks in the\n" "> > > devicetree.\n" - "> > \n" + "> > =\n" + "\n" "> > As far as I can see, the above is sufficient, and leaves the knowledge\n" "> > of skippable clocks in the driver, where I believe it should be.\n" - "> \n" + "> =\n" + "\n" "> You miss my point. Once a clock is specified in the devicetree it is no\n" "> longer optional. The driver currently can't find out whether a clock\n" "> hasn't been specified (and it's ok that it's not there) or a clock has\n" @@ -259,14 +367,18 @@ "Regards,\n" "Mike\n" "\n" - "> \n" + "> =\n" + "\n" "> Sascha\n" - "> \n" - "> \n" - "> -- \n" + "> =\n" + "\n" + "> =\n" + "\n" + "> -- =\n" + "\n" "> Pengutronix e.K. | |\n" "> Industrial Linux Solutions | http://www.pengutronix.de/ |\n" "> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |\n" > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -00dc779a6300e7f60df2e3f147161dee9572e4201025fb70693a440054a06e5a +426e92e5cce78f1fdf25321016003c01879ab6a4fab85babb05bc671c21fed18
diff --git a/a/content_digest b/N2/content_digest index dd188c9..d5e02e3 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -6,7 +6,7 @@ "ref\020130823125815.GG25856@e106331-lin.cambridge.arm.com\0" "ref\020130823140128.GI31036@pengutronix.de\0" "From\0Mike Turquette <mturquette@linaro.org>\0" - "Subject\0Re: [PATCH v5 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver\0" + "Subject\0Re: [alsa-devel] [PATCH v5 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver\0" "Date\0Fri, 23 Aug 2013 14:41:44 -0700\0" "To\0Sascha Hauer <s.hauer@pengutronix.de>" " Mark Rutland <mark.rutland@arm.com>\0" @@ -16,7 +16,7 @@ ian.campbell@citrix.com <ian.campbell@citrix.com> Pawel Moll <Pawel.Moll@arm.com> swarren@wwwdotorg.org <swarren@wwwdotorg.org> - linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org> + festevam@gmail.com <festevam@gmail.com> Nicolin Chen <b42378@freescale.com> Tomasz Figa <tomasz.figa@gmail.com> rob.herring@calxeda.com <rob.herring@calxeda.com> @@ -25,7 +25,7 @@ p.zabel@pengutronix.de <p.zabel@pengutronix.de> galak@codeaurora.org <galak@codeaurora.org> shawn.guo@linaro.org <shawn.guo@linaro.org> - " festevam@gmail.com <festevam@gmail.com>\0" + " linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>\0" "\00:1\0" "b\0" "Quoting Sascha Hauer (2013-08-23 07:01:28)\n" @@ -269,4 +269,4 @@ "> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |\n" > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -00dc779a6300e7f60df2e3f147161dee9572e4201025fb70693a440054a06e5a +5e44ee529a2c8015bea2fe5240e3a62cd2ff7465ef22d9fd6f6997e8abff8331
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.