Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

[Hans de Goede <j.w.r.degoede@gmail.com>]

Hi,

On 02-03-18 18:41, Mark Brown wrote:
> On Fri, Mar 02, 2018 at 01:58:53PM +0100, Hans de Goede wrote:
>> On 02-03-18 13:18, Mark Brown wrote:
> 
>>> What makes you claim that?  Any board with jack detection wired up will
>>> call that function.  Not all boards have that configured of course but
>>> there's absolutely nothing Intel specific about that interface.
> 
>> Right I'm not claiming the interface is Intel specific, what I'm trying
>> to say is that the need for some code outside of the codec driver
>> to create the jack means that adding jack-support to DT platforms
>> cannot be done through just adding DT properties (once this patch
>> is merged), it will still require platform code changes.
> 
> On DT the situation with generic drivers is much better than it is on
> ACPI so we're actually able to create jacks purely from DT with both the
> simple and graph cards.  This was what drove the creation of the generic
> jack operation rather than device specific function calls, Bard realized
> that the device specific calls were all very similar and adding the call
> allowed the generic cards to work with jacks.
> 
>>> No, that's really not a good idea.  Any jacks in the system are part of
>>> the system specific wiring, they're not something that's intrinsic to
>>> the CODEC.  Even with fairly basic setups you've got options like having
>>> a headset jack or separate microphone and headphone jacks.
> 
>> OK, so if doing the jack creation in the machine-driver / platform
>> code is by design and you want to keep things that way, then I
>> assume that what needs changing for v3 of this patch is:
> 
>> 1) Split the patch in separate codec + machine-drv patches
>> 2) Add comments to make the ordering requirements clear to
>> both the codec- and machine-driver.
> 
>> Correct?
> 
> Yes, I think so.

Actually I've come up with a nicer way to deal with the ordering issues
around setting the device-properties from the machine-driver.

If we attach the properties in the machine-driver *before* calling
snd_soc_register_card() and check them from the codec/component driver's
probe function (rather then from the i2c_driver.probe function), then
there is no need for a codec private apply_properties() function to
get the ordering right, since the codec/component driver's probe
function is called from snd_soc_register_card().

This still needs some comments in both the codec and machine driver
to make sure that future changes don't cause issues, but it gets rid
of the ugly apply_properties() function call from the machine-driver.

I'm currently preparing a v3 with this change + other requested changes.
I need to re-run a bunch of tests before posting, so I hope to post the
v3 series tomorrow.

Regards,

Hans