[alsa-devel] [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack

[Stephen Warren]

On 02/12/2016 07:10 PM, Jonathan Tinkham wrote:
> On 02/08/2016 11:59 AM, Stephen Warren wrote:
>> On 02/04/2016 12:05 PM, Jonathan Tinkham wrote:
>>> On 02/04/2016 09:36 AM, Stephen Warren wrote:
>>>> On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:
>>>>> The headphone jack should not be inverted
>>>>
>>>> Have you tested this on Venice2 as well as Nyan? I'm pretty sure
>>>> Venice2 was tested when this driver was written, and whoever added
>>>> Nyan support to the kernel simply assumed it would work. As such, my
>>>> suspicion is that this series will break Venice2 even as it fixes Nyan.
>>>
>>> I have not tested this on Venice2, only on Nyan. That seems like a
>>> plausible cause and reasonable suspicion.
>>>
>>>> Why doesn't user-space expect what the kernel actually implements? The
>>>> kernel should be defining the control naming.
>>>>
>>>> Which user-space are you using specifically, and which part of it
>>>> expects particular naming?
>>>>
>>>> Perhaps this series needs to be parametrized based on a flag in DT,
>>>> rather than switching the hard-coded values, so that only Venice2 can
>>>> be affected?
>>>
>>> Specifically pulse-audio and alsa under Arch Linux.
>>>
>>> I was referencing 'Documentation/sound/alsa/soc/dapm.txt' with regards
>>> to control names. While it is possible to add another entry into the
>>> user-space configuration, I took this documentation as a definition of
>>> kernel control naming schemes, and thought the driver had used a
>>> non-standard naming scheme (or at least, not a consistent one).
>>>
>>>> On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:
>>>>> Update device-tree bindings to reflect the rename of the board's
>>>>> headphone jack.
>>>>
>>>> This looks like an incompatible change to the DT. While you've fixed
>>>> the DT, which will fix new installations, old DTs now won't work. This
>>>> breaks DT ABI. Any DT change needs to be backwards compatible, i.e.
>>>> the old name should still work and be documented as a legacy value.
>>>
>>> I see that now. If the inversion behavior differs between venice2 and
>>> nyan, then another compatible string would need to be added anyways,
>>> correct? As you mentioned above, this might need to be done anyways for
>>> the rename.
>>>
>>> Something like:
>>> - "compatible = nvidia,tegra-audio-max98090" implements old inversion
>>> behavior and leaves the switch as "Headphones" to avoid breaking
>>> older DTs
>>> - "compatible = nvidia,tegra-{venice2,nyan}-audio-max98090" that
>>> separates the inversion behavior and also introduces the rename
>>
>> It'd be preferable to key this off an separate flag rather than the
>> compatible value. That would more directly represent the data in
>> question, and allow any future boards to be added without having to
>> edit the driver to know whether those new boards neded HP DET
>> inversion or not.
>>
>> However, do note that each of the 3 boards using this binding has a
>> board-specific compatible value present already:
>>
>> nvidia,tegra-audio-max98090-venice2
>> nvidia,tegra-audio-max98090-nyan-blaze
>> nvidia,tegra-audio-max98090-nyan-big
>
> Indeed, I noticed those compatible strings after I sent that message.
>
> A property seems the more ideal/desired way to go. However, to avoid
> breaking older DTs, does that mean it must be implemented as assuming
> inversion is default, and set nyan and other boards to "hd-invert = 0"?

The default (if no property is present in the DT) should indeed match 
whatever the current behaviour is in order to maintain compatibility.