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.
- /*
* Rev B0 has 2 versions* L36 is 10V* L37 is 12V
\o/
Yes we had 2 silicon spins for this revision >_< is there anything about this comment you want us to change?
You could more explicitly explain what happened here, revisions and spins are generally synonymous so it's confusing.
- }
- 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?
I think we will add an err_disable_regs jump statement to ensure we don't try to use the uninitialized reset_gpio handle. Once we do that, I think this will handle -EPROBE_DEFER cleanly?
Yes.