Re: [PATCH v4 2/6] dt-bindings: audio-graph-card: Add plls and sysclks properties

From: Mark Brown <broonie@kernel.org>
To: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Rob Herring <robh@kernel.org>,
	kuninori.morimoto.gx@renesas.com, nsaenzjulienne@suse.de,
	f.fainelli@gmail.com, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	patches@opensource.cirrus.com,
	bcm-kernel-feedback-list@broadcom.com,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 2/6] dt-bindings: audio-graph-card: Add plls and sysclks properties
Date: Fri, 15 Jan 2021 18:35:27 +0000	[thread overview]
Message-ID: <20210115183527.GG4384@sirena.org.uk> (raw)
In-Reply-To: <d67f805f-2813-14e9-0c4f-5948ec73f7b0@opensource.cirrus.com>

[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]

On Fri, Jan 15, 2021 at 04:15:21PM +0000, Richard Fitzgerald wrote:

> If I do:
>  	sound {
>  		clocks = <&clock>;
>  	};
> 
> 	clock: clock {
> 		compatible = "fixed-clock";
> 		clock-frequency = <98304000>;
> 	};
> 
> I can clk_bulk_get_all().
> But if I remove the 'compatible' from the clock node, clk_bulk_get_all()
> will return -EPROBE_DEFER and log:

OK, so if this is only supposed to represent a fixed clock on the board
separate to the CODEC then yes, of course you do need to instantiate a
driver for it like you do for every device on the board.  However it
shouldn't be a subdevice of the CODEC as you had it originally, it
should be a distinct device as the above has it since that is what
physically exists.  This obviously won't configure the FLL at all though
(which was what the binding you were proposing was for, the above is
definitely not a direct substitute for the binding you originally
proposed).

> > When we say to use the clock binding what we are saying is to use the
> > actual clock bindings to describe the clocks, not make a custom binding
> > that looks kind of like them - making a custom binding doesn't address
> > the problem.

> But I don't know what you mean by "use the actual clock bindings to
> describe the clocks".

> What is not clear to me is how you want me to use a clock binding to
> describe something that isn't a clk-framework clk. If you know what you
> want, then please.. an example would help explain.

The concept of a clock framework is an implementation detail of Linux
which should not affect how the DT bindings for a device or system are
written, DT bindings should be clear and idiomatic as DT bindings.  The
goal is to represent the system in a clear and standardized fashion
which is useful to OSs in general, not just something convenient for
Linux as it happens to be implemented right now.  Current Linux
internals are not a constraint for DT bindings.

In this case if you can't figure out how to parse clock bindings without
moving the clocks over to standard Linux clock APIs (which seems likely)
then it follows that if you want to describe the clock configuration in
DT then the driver support for these clocks should use the standard
Linux clock framework.  This seems like a good idea in general anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]