[alsa-devel] [PATCH 01/11] ASoC: rt5651: Avoid duplicating DMI quirks between codec and machine driver

[Mark Brown]

On Wed, Feb 21, 2018 at 08:23:01PM +0100, Hans de Goede wrote:
> On 21-02-18 12:54, Mark Brown wrote:

> > It's not clear to me that this is a great idea.  The goal in general is
> > to keep as much of the CODEC configuration in the CODEC driver as
> > possible so we can move more towards generic machine drivers, this is
> > moving us in the opposite drection quite dramatically.

> Not really, instead of having Intel/ACPI specific quirks in the codec
> driver it moves everything over to pdata, so in a way this unifies the
> device-tree and ACPI paths.

Bear in mind that ACPI isn't x86 specific any more, and on x86 those
Intel drivers really are Intel specific rather than x86 specific - AMD
are doing their own completely different thing.  Much ACPI, so
standards, sadly :(

> Also to me it seems that the Intel specific machine driver is the
> logical place to put quirks for Intel platforms rather then poluting the
> platform-agnostic codec driver with this.

> > It is really unfortunate that ACPI based systems aren't embracing
> > standards here and using this mess of open coded quirks but it feels
> > like the way to do this is to keep the code as close as possible to well
> > designed firmware interfaces and hope that they come round.

> Both devicetree and ACPI code-paths are filling pdata and that is the
> only thing the codec driver cares about after this patch. so this is
> using a well designed interface (but a non firmware one). I can understand

It's not just platform data, it's also putting the platform data in
through a regular driver outside of the usual device model stuff.

> if you would prefer to use the functions from include/linux/property.h
> for this, but those simply do not work for ACPI platforms atm.

Yeah, they work fine with DSD which is the official ACPI way to pass
device specific data through.

> One possible solution here would be to add device-properties to the codec
> i2c-device from the machine-driver using device_add_properties() and make
> the codec driver consume those.

> This has probe-ordering issues, but those can be worked around by exporting
> something like the rt5651_apply_pdata() function from this patch and make
> the machine driver call that after it has attached the properties.

The ordering is part of the issue, yeah.  It increases fragility if we
have to wait to parse the platform data until later on in init, but only
on some systems.

> I'm not sure if this extra level of indirection buys us much though, the
> DT binding maintainers will not accept binding docs for these kind of
> kernel internal only device-properties until there are actual DT users
> of them, so for now this would mainly add a bunch of extra code for
> little gain.

I'm not aware of any such policy from the DT people, and in any case
the individual driver bindings go through subsystems so...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20180222/27b458a4/attachment-0001.sig>