Re: [PATCH 1/4] soc/sound: tegra_max98090: do not invert headphone jack

On 02/08/2016 11:59 AM, Stephen Warren wrote:

On 02/04/2016 12:05 PM, Jonathan Tinkham wrote:

quoted

On 02/04/2016 09:36 AM, Stephen Warren wrote:

quoted

On 02/03/2016 10:30 PM, Jonathan Tinkham wrote:

quoted

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.

quoted

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).

quoted

On 02/03/2016 10:31 PM, Jonathan Tinkham wrote:

quoted

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"?