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

Quoting Tomasz Figa (2013-08-21 14:34:55)

On Wednesday 21 of August 2013 09:50:15 Mark Rutland wrote:

quoted

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

quoted

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

quoted

quoted

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.

=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

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.

=

quoted

quoted

quoted

quoted

quoted

quoted

quoted

<&clks 0> is the dummy clock. This can be used for all input
clocks
not
defined by the SoC.

=

quoted

quoted

quoted

quoted

quoted

quoted

Where does this assumption come from? Is it documented
anywhere?

=

quoted

quoted

quoted

quoted

quoted

This is how all i.MX clock bindings currently are. See
Documentation/devicetree/bindings/clock/imx*-clock.txt

=

quoted

quoted

quoted

quoted

OK, thanks.
=

quoted

quoted

quoted

quoted

I guess we need some discussion on dummy clocks vs skipped clocks.
I think we want some consistency on this, don't we?
=

quoted

quoted

quoted

quoted

If we really need a dummy clock, then we might also want a generic
way to specify it.

=

quoted

quoted

quoted

What do we actually mean by a "dummy clock"? We already have
bindings
for "fixed-clock" and co friends describe relatively simple
preconfigured clocks.

=

quoted

quoted

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.

=

quoted

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:
=

quoted

* 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?
=

quoted

* 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.
=

quoted

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.

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?

Regards,
Mike

=

Best regards,
Tomasz