On Fri, Apr 29, 2016 at 04:15:17PM +0800, John Hsu wrote:
@@ -614,8 +623,10 @@ int nau8825_enable_jack_detect(struct snd_soc_codec *codec, NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L, NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L);
- regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_EJECT_EN, 0);
- /* Change jack type detection interruption to non-clock architecture
* for power saving less 1mW. Move these configuration about inter-* ruption at auto mode to auto irq setup function.*/
This comment is about the change you're making to the code rather than something that should be in the code.
- /* Clear all interruption status */
- nau8825_int_status_clear_all(regmap);
- /* Mask all interruptions except jack insertion interruption */
- regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0xfffe);
So if any other interrupts occur then things will break...
- regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq);
- if (regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq)) {
dev_err(nau8825->dev, "failed to clear interruption\n");return IRQ_NONE;- }
This is a read, not a write, and it's better to report the error code if the read failed. This should probably be a separate patch.
static const struct regmap_config nau8825_regmap_config = {
- .val_bits = 16,
- .reg_bits = 16,
- .val_bits = NAU8825_REG_DATA_LEN,
- .reg_bits = NAU8825_REG_ADDR_LEN,
This appears to be an unrelated change which it's not clear helps the reader, these defines only seem to be used in htis one place.
@@ -1134,7 +1244,26 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id, struct regmap *regmap = nau8825->regmap; int ret;
- if (!nau8825_is_jack_inserted(nau8825->regmap) &&
clk_id != NAU8825_CLK_DIS) {/* For power saving less 1mW, the jack type detection inter-* ruption changes to non-clock architecture. Therefore, the* clock should be disabled and not allowed to config any clock* when no headset connected.*/dev_warn(nau8825->dev, "Souldn't enable any clock when no headset connected\n");return 0;- }
This is ignoring the attempt to set up a clock but returning success which is going to break things, printing the warning is dubious (a system could be built without detection for example, or a speaker driver connected) but probably OK in itself but the fact that we don't tell the caller may make things worse.
nau8825_eject_jack(nau8825);snd_soc_jack_report(nau8825->jack, 0, SND_JACK_HEADSET);- }
- enable_irq(nau8825->irq);
The interrupt is optional (that bug appears to be already present in the driver but should be fixed).