[alsa-devel] [PATCH v3 03/22] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede]

Hi,

On 07-03-18 13:34, Mark Brown wrote:
> On Sun, Mar 04, 2018 at 03:35:51PM +0100, Hans de Goede wrote:
> 
>> +/*
>> + * Note these MUST match the values from the DT binding:
>> + * Documentation/devicetree/bindings/sound/rt5651.txt
>> + */
>>   enum rt5651_jd_src {
>>   	RT5651_JD_NULL,
>>   	RT5651_JD1_1,
> 
> It's normal to put values like this in a header in include/dt-bindings
> so it's easy to get things right in actual DT bindings (and possibly
> ACPI tables as well if the machinery for that does the right thing) -
> can you send a followup doing that please?

Good point, yes I will send a follow-up patch.

>> -static const struct dmi_system_id rt5651_quirk_table[] = {
>> -	{
>> -		.callback = rt5651_quirk_cb,
>> -		.matches = {
>> -			DMI_MATCH(DMI_SYS_VENDOR, "KIANO"),
>> -			DMI_MATCH(DMI_PRODUCT_NAME, "KIANO SlimNote 14.2"),
>> -		},
>> -		.driver_data = (unsigned long *) RT5651_JD1_1,
>> -	},
>> -	{}
>> -};
>> -
> 
> I'm still not thrilled about this bit but oh well :(

Hmm, I'm planning on doing something very similar for the rt5640 code.

That is I'm working on jack-detect for the rt5640 too, and that will
need some platform data like which pin to use and OVCD settings,
I plan to once again use device-properties for this to have a
platform agnostic solution at the codec driver level.

And then in the Intel machine driver attach the device properties
based on dmi quirks (like bytcr_rt5651 the machine driver already
has DMI based quirks for a bunch of boards, which I will re-use).

Although I agree that ideally we would be able to get all this
directly from the firmware, to me fixing the x86 specific problem
of that info missing using quirks in the Intel machine driver seems
like the right place to fix this, so that we are not "poluting"
generic code with these Intel specific workarounds.

I know that second best would be some generic mechanism in the
kernel to attach extra device-properties based on the board we're
booting on, but alas currently that does not exist. One advantage
of using device-properties for this as this series is doing, is
that if such a mechanism gets added moving the info over should
be easy.

TL;DR: I know this is not the prettiest thing ever, but as you
said: "oh well".

So as said I plan to do something very similar for the rt5640 code,
if you've any objections against that I would like to hear those
objections sooner rather then later to avoid throwing away work.

Regards,

Hans