On Wed, 14 Nov 2018, Mark Brown wrote:
> On Wed, Nov 14, 2018 at 06:54:02PM +0000, Schulman, James wrote:
>> On Tue, 13 Nov 2018, Mark Brown wrote:
>>> On Tue, Nov 13, 2018 at 12:49:16PM -0600, James Schulman wrote:
>
>>>> +static const struct snd_kcontrol_new cs35l36_boost_mux[] = {
>>>> + SOC_DAPM_ENUM("Boost Enable", boost_enum),
>>>> +};
>
>>> Simple on/off conttrols should be _SINGLE controls with Switch at the
>>> end of their name. It's not super clear why this is in DAPM as a widget
>>> at all, the routing around it is:
>
>>> + {"BOOST Mux", "On", "Channel Mux"},
>>> + {"CLASS H", NULL, "BOOST Mux"},
>
>>> which is a bit perplexing. Possibly this should be handled in the event
>>> for the main amplifier?
>
>> We used _ENUM instead of _SINGLE because we wanted the default value to be
>> "On" instead of "Off". We don't want the default assumption to have boost
>> turned off. However, we're happy to change this if you would rather have
>> consitency with the _SINGLE controls.
>
> I don't follow this at all. Setting a default value for a control is
> not related to the type of the control.
>
Ok we will switch to a _SINGLE control.
>>>> + }
>>>> +
>>>> + return IRQ_HANDLED;
>
>>> This unconditionally reports that it handled the interrupt, even if it
>>> didn't. This will stop the interrupt core handling for faulty interrupt
>>> lines working and will disrupt any support that gets added for handling
>>> shared threaded interrupts.
>
>> Earlier in the handler, we have a check to see if any unmasked bits are
>> set. If not we return IRQ_NONE. I figured this would be enough to ensure
>> we don't unconditionally report IRQ_HANDLED.
>
> What if something you didn't expect got unmasked by some bug?
>
We make the assumption that this is not the case. This handler is modeled
after our other upstreamed drivers (cs35l35.c). If there is a better
example please let me know and we can try to be more consistent.A
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
