[alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers

[Grant Likely]

On Tue, Apr 27, 2010 at 2:07 AM, Liam Girdwood <lrg at slimlogic.co.uk> wrote:
> On Tue, 2010-04-27 at 16:36 +1000, Benjamin Herrenschmidt wrote:
>> On Mon, 2010-04-26 at 15:49 -0500, Timur Tabi wrote:
>> > An upcoming change in the architecture of ALSA SoC (ASoC) will require the
>> > MPC8610 HPCD's ASoC fabric driver to register as a standard platform driver.
>> > Therefore, we need to call platform_device_register_simple() from the board's
>> > platform code.
>> >
>> > Signed-off-by: Timur Tabi <timur at freescale.com>
>> > ---
>>
>> Gross. Loses the linkage to device-tree etc... which is useful for audio
>> especially. You should at least make sure the device node follows for
>> the target driver to be able to use it :-) I'd like you to sync with
>> Grant work on that matter if you are going to convert of_devices into
>> platform_devices.
>
> Timur, please correct my device tree understanding form our IRC
> conversation if I'm wrong here.
>
> I think one of the difficulties encountered here is that the device tree
> root for sound in this case is the SSI (Digital Audio Interface)
> controller and not the sound card as in ASoC. So maybe the problem is
> how do we represent an ASoC sound card in the device tree ?

Most likely this is currently a problem, yes.  *however* the device
tree is just data.  It is convenient and useful to have a common
representation between similar kinds of devices, but it isn't
mandatory.  The device tree structure does *not* need to have a 1:1
correspondence to the ASoC device registrations.

So, the current 86xx device tree binding assumes a simple layout with
a node describing an DAI controller, and another node describing the
codec with a single phandle (pointer) from the DAI to the codec.  In
this configuration, it is completely reasonable for the DAI node to
trigger both the instantiation of the ASoC DAI controller device and
the sound card device.  Linux can treat them as separate even though
the current device tree has a simplistic representation.

> The sound card within ASoC is a compound device that is made up of the
> SSI, Codec and audio DMA engine components. The SSI/Codec/DMA components
> do not have to be the sound cards child devices wrt the driver model but
> do register with the ASoC core and are 'joined' with each other and the
> sound card driver to form a working audio device.

Right.  This makes sense to me.

> Now I don't know the mechanics of adding an ASoC sound card to the
> device tree, but the device tree should be able to define an ASoC sound
> card and also represent the relationships between the sound card and the
> SSI/Codec/DMA components. The components should also be represented in
> the device tree. Parsing the device tree should probe() all the ASoC
> components and sound card. The ASoC core would then instantiated them as
> a sound device.

I've tried very hard to maintain a distinction between device tree
binding (representation) and Linux kernel internal implementation
details.  The real question is whether or not the binding provides
sufficient detail for the operating system to figure out what to do.
In the extreme minimalist case, the audio driver could decide how to
configure itself solely on the board name property of the root node.
There is nothing wrong with that, but it also means that no data is
available to dynamically select common modules or modify connections;
it all has to be hard coded.

The 8610 device tree looks something like this right now:

	[...]
		cs4270:codec at 4f {
			compatible = "cirrus,cs4270";
			reg = <0x4f>;
			/* MCLK source is a stand-alone oscillator */
			clock-frequency = <12288000>;
		};
	[...]
	ssi at 16000 {
		compatible = "fsl,mpc8610-ssi";
		[...]
		fsl,mode = "i2s-slave";
		codec-handle = <&cs4270>;
		fsl,playback-dma = <&dma00>;
		fsl,capture-dma = <&dma01>;
		fsl,fifo-depth = <8>;
		sleep = <&pmc 0 0x08000000>;
	};
	[...]

(I've omitted the DMA nodes and some irrelevant details)  This is
enough information for a simplistic driver registration that probably
makes a lot of assumptions.  Such as the ssi represents a single
logical sound device.  It won't handle complex representations, but in
a lot of cases that may be just fine.

However, as you and Mark rightly point out, it completely fails to
represent complex sound devices with weird clocking and strange
routes.  Something like this is probably more appropriate:

	[...]
		codec1 :codec at 4f {
			compatible = "cirrus,cs4270";
			reg = <0x4f>;
			/* MCLK source is a stand-alone oscillator */
			clock-frequency = <12288000>;
		};
	[...]
	ssi1: ssi at 16000 {
		compatible = "fsl,mpc8610-ssi";
		[...]
		fsl,mode = "i2s-slave";
		fsl,playback-dma = <&dma00>;
		fsl,capture-dma = <&dma01>;
		fsl,fifo-depth = <8>;
		sleep = <&pmc 0 0x08000000>;
	};
	[...]
	sound {
		compatible = "fsl,mpc8610-hpcd-sound";
		/* maybe something like (totally off the top of my head) */
		dai-links = <&ssi1 0 &codec 0
		             &ssi1 1 &codec 1>;
		[...]
	};

Where the 'sound' node is now the starting point for representing a
logical sound device instead of the ssi node.  This binding probably
makes more sense (but I'm not committing to anything like this until I
see a real proposal for a real device).

The question for the drivers is more around how to deal with the data
provided.  I've got zero issues with platform specific code to handle
things that aren't easily described in a device tree.  The point is to
make the common cases data-driven so that common code will handle
them.  The complex corner cases are still complex corner cases that
need platform specific code.

Or, in other words, the device tree should *not* be used to describe
every possible detail and permutation.  It is best used to describe
the permutations that are common so that they don't need to be hard
coded for each and every board.

I would solve the problem this way: In the ssi driver, if the
codec-node property is present, then call a function to instantiate a
simple or platform specific sound card instance that makes the
assumptions listed above.  If not, then just register the ssi and
exit, which leaves the ssi available for a separate driver to pick it
up.  I wouldn't do this for new platforms, but it gracefully makes use
of the data provided in the current 8610 device tree.

BTW Timur, there is nothing wrong with registering multiple devices
that all have the of_node pointer set to the same node.

g.