[alsa-devel] [PATCH] sound/soc/tegra: tegra_max98090: Invert headphone by GPIO flag

[Jonathan Tinkham]

On 02/24/2016 09:45 AM, Stephen Warren wrote:
> On 02/24/2016 09:42 AM, Jonathan Tinkham wrote:
>> On 02/23/2016 10:35 AM, Stephen Warren wrote:
>>> On 02/23/2016 10:19 AM, Jonathan Tinkham wrote:
>>>
>>> > Subject: sound/soc/tegra: tegra_max98090: Invert headphone by GPIO
>>> > flag
>>>
>>> Mark has mentioned quite a few times that this patch subject is
>>> incorrect. ASoC patch subjects should start with "ASoC: ". You can see
>>> this by running:
>>>
>>> git log -- sound/soc/tegra/
>>>
>>> I'd suggest the following:
>>>
>>> AsoC: tegra_max98090: honor headphone detect GPIO polarity
>>>
>> My apologies, I finally grok what he was saying. Thank you.
>>>> Set the invert property for the headphone jack depending on the GPIO
>>>> flag in the
>>>> device-tree. This is similar to what is done for tegra_rt5640.
>>>>
>>>
>>>> diff --git a/sound/soc/tegra/tegra_max98090.c
>>>> b/sound/soc/tegra/tegra_max98090.c
>>>
>>>> @@ -155,6 +156,7 @@ static int tegra_max98090_asoc_init(struct
>>>> snd_soc_pcm_runtime *rtd)
>>>> ARRAY_SIZE(tegra_max98090_hp_jack_pins));
>>>>
>>>>           tegra_max98090_hp_jack_gpio.gpio = machine->gpio_hp_det;
>>>> +        tegra_max98090_hp_jack_gpio.invert =
>>>> (machine->gpio_hp_det_flags & OF_GPIO_ACTIVE_LOW);
>>>
>>> Did you run checkpatch? It should complain about > 80 columns, and I
>>> suspect about the unnecessary brackets around that expression. In
>>> fact, checkpatch indicates quite a few other warnings and errors.
>>>
>>> The logic in this patch looks OK. Do the relevant DT files all have
>>> the correct GPIO flags already? That'd be nice!
>>
>> The only board is the venice2, which doesn't have a 'nvidia,hp-det-gpio'
>> entry at all (how did this even work before?)
>
> I mean: Is ACTIVE_LOW/ACTIVE_HIGH flag in the existing 
> nvidia,hp-det-gpios already correctly set?

Yes. I also have cleaned up the warnings/errors from checkpatch, and can 
resubmit if acceptable.