Hi,
On 02-03-18 13:18, Mark Brown wrote:
On Fri, Mar 02, 2018 at 10:32:25AM +0100, Hans de Goede wrote:
On 01-03-18 23:35, Hans de Goede wrote:
It is not _that_ fragile, but I agree that it would be good to add
a comment about not moving the property-parsing to the codec driver.
So one other solution which comes to mind here is to move the
snd_soc_card_jack_new() call into the codec driver's
rt5651_apply_properties() function (conditional on a jack src being
set in the properties).
This would make the machine driver code look like this:
props[cnt++] = PROPERTY_ENTRY_...
props[cnt++] = PROPERTY_ENTRY_...
props[cnt++] = PROPERTY_ENTRY_...
props[cnt++] = PROPERTY_ENTRY_...
ret = device_add_properties(codec->dev, props);
if (ret)
return ret;
ret = rt5651_apply_properties(codec);
if (ret)
return ret;
Which makes the ordering really clear without needing any
comments.
It's still unusual to have something outside the driver trigger property
parsing, though the EXPORT_SYMBOL_GPL() would make that apparent.
This will also make the jack-detect device-properties work with
devicetree platforms without any changes to their platform code,
which currently is not the case since the non Intel platform-code
does not call snd_soc_component_set_jack().
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.
That is fine, I was just thinking it would be convenient if DT
platforms could lift on the jack-detect work being done for
the rt5651 driver now with just some DT changes and no other
changes required.
Thinking more about this I believe that doing the
snd_soc_card_jack_new() call inside the codec driver based on
device-properties is a better solution then doing it in the
machine driver.
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?
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
