Re: [PATCH v5 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

On Tue, Aug 20, 2013 at 01:06:25AM +0100, Mike Turquette wrote:

Quoting Mark Rutland (2013-08-19 02:35:43)

quoted

On Sat, Aug 17, 2013 at 04:17:18PM +0100, Tomasz Figa wrote:

quoted

On Saturday 17 of August 2013 16:53:16 Sascha Hauer wrote:

quoted

On Sat, Aug 17, 2013 at 02:28:04PM +0200, Tomasz Figa wrote:

quoted

quoted

quoted

quoted

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?

Thanks,
Mark.