[alsa-devel] [PATCH] ASoC: twl6040: Support for DT

[Mark Brown]

On Fri, May 11, 2012 at 05:44:19PM +0200, Cousson, Benoit wrote:
> On 5/11/2012 3:08 PM, Mark Brown wrote:

> >This binding doesn't do anything to move towards that goal given that
> >the only information it includes about the contents of the chip is the
> >name.

> But it does not have to expose everything. And what will be your
> definition of everything in that case?

A useful binding that abstracted things sufficiently to allow multiple
devices would be one which contained some information about the contents
of the chip.

> >Writing the name out in separate CODEC and vibra nodes really
> >isn't going to accomplish much to promote reuse that can't trivially be
> >achieved by parsing the name in the MFD driver.

> Maybe, but... so what? This is not because you can do it with MFD
> that you cannot make it more flexible with DT.

The issue is the tastefulness.  Just looking at the binding it's
immediately clear that the only thing that the extra levels of
indirection are adding is the removal of the mfd_add_cells() call from
the MFD driver which isn't particularly helping anything and is
basically Linux specific.

> Preferring to hard code that in MFD is similar to keeping all the
> static C board files we are all trying to remove. It is possible,
> sure, but there are now better way to describe HW modules than
> hard-coding that in a C file.

Sure, but this binding doesn't do anything like that.  If it said things
like "there's a PDM speaker driver with register map X at address Y"
then you'd have a block you could easily pick up and reuse when defining
another device but it doesn't do that at all.

> Moreover, there is not an unique way to describe the HW. So for sure
> we will cheat a little bit and make some assumption about what the
> SW will really use. But otherwise you will end up putting the full
> RTL inside the DT node.

This isn't a "cheat a little bit" type thing - it's really not talking
about blocks that are likely to reappear elsewhere in exactly the same
form on a device that's different enough that you'd be able to add some
useful reuse via the device tree.

With CODECs the IP blocks you see are generally things like audio
interfaces, mixers, PLLs, amplifiers and so on.  New devices that are
meaningfully different to software will generally be some new
combination of that sort of stuff rather than a straight clone of the
chip top level which is what this binding is talking about.

> It is very similar to the discussion we had for the clock tree. For
> sure we can describe the full clock tree in DT, but that's huge and
> useless. So we are taking some assumption and expose only the ones
> that have to be exposed because dependent of the board or needed by
> the devices.

The difference I'm seeing here is that the split in the device tree here
is at such that it's the equivalent of having a single device tree node
for the entire clock tree of a given OMAP model - it's just completely
redundant, it's not adding any additional information beyond the top
level information about which chip this is.

> It is up to the driver owner to assess the level of information he'd
> like to expose to DT based on the number of chip variants that
> already exist.

This code handles exactly one chip and looking at it it's *extremely*
hard to see it as a useful abstraction for multiple chips.  There's also
the fact that with things like this where the chip has to be hooked up
to other pieces of the system it makes much more of a difference than it
might otherwise since it's much more externally visible.  In this
particular case for example we might want to split the clocking out
differently as the generic clock API comes along, and there's extcon
too.  All of which should be entirely Linux internal things.

> Both formats are valid.
> It is just a matter of flexibility / personal choice / mood of the day.

> The important point is that this is not a black or white kind of
> decision, we can be in the grey area and use a little bit of both
> approach.

If I could see how this would allow reuse I'd probably agree with you
but I really can't - it's immediately obvious looking at what's there
that all that's happening is that a bit of the Linux internals that's
not particularly great as a generic abstraction dropped straight into
the device tree.

Plus there's the fact that from both a device tree and a code point of
view it's utterly trivial to not land ourselves with this in the device
tree, it's just registering two devices and using a different of_node
for reading properties in code and removing a layer of indentation in
the DT.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20120511/813e0634/attachment-0001.sig