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