Re: [PATCH 08/10] ASoC: cs35l56: Add driver for Cirrus Logic CS35L56

+static int cs35l56_sdw_interrupt(struct sdw_slave *peripheral,

• 		 struct sdw_slave_intr_status *status)
  

+{

• struct cs35l56_private *cs35l56 = dev_get_drvdata(&peripheral->dev);
• /* SoundWire core holds our pm_runtime when calling this function. */
• dev_dbg(cs35l56->dev, "int control_port=%#x\n", status->control_port);
• if ((status->control_port & SDW_SCP_INT1_IMPL_DEF) == 0)

• return 0;
  

• /* Prevent host controller suspending before we handle the interrupt */
• pm_runtime_get_noresume(cs35l56->dev);

can this happen that the manager suspends in this function?

Or is this needed because of the queued work which the manager has no knowledge of?

• /*

• * Mask and clear until it has been handled. The read of GEN_INT_STAT_1
  

• * is required as per the SoundWire spec for interrupt status bits
  

• * to clear. GEN_INT_MASK_1 masks the _inputs_ to GEN_INT_STAT1.
  

• * None of the interrupts are time-critical so use the
  

• * power-efficient queue.
  

• */
  

• sdw_write_no_pm(peripheral, CS35L56_SDW_GEN_INT_MASK_1, 0);
• sdw_read_no_pm(peripheral, CS35L56_SDW_GEN_INT_STAT_1);
• sdw_write_no_pm(peripheral, CS35L56_SDW_GEN_INT_STAT_1, 0xFF);
• queue_work(system_power_efficient_wq, &cs35l56->sdw_irq_work);
• return 0;

+}

+static int __maybe_unused cs35l56_sdw_handle_unattach(struct cs35l56_private *cs35l56) +{

• struct sdw_slave *peripheral = cs35l56->sdw_peripheral;
• if (peripheral->unattach_request) {

• /* Cannot access registers until master re-attaches. */
  

not sure what the comment means, the manager does not attach. did you mean resume the bus?

• dev_dbg(cs35l56->dev, "Wait for initialization_complete\n");
  

• if (!wait_for_completion_timeout(&peripheral->initialization_complete,
  

• 				 msecs_to_jiffies(5000))) {
  

• 	dev_err(cs35l56->dev, "initialization_complete timed out\n");
  

• 	return -ETIMEDOUT;
  

• }
  

• peripheral->unattach_request = 0;
  

• /*
  

•  * Don't call regcache_mark_dirty(), we can't be sure that the
  

•  * Manager really did issue a Bus Reset.
  

•  */
  

• }
• return 0;

+}

...

+static void cs35l56_dsp_work(struct work_struct *work) +{

• struct cs35l56_private *cs35l56 = container_of(work,

• 				       struct cs35l56_private,
  

• 				       dsp_work);
  

• unsigned int reg;
• unsigned int val;
• int ret = 0;
• if (!wait_for_completion_timeout(&cs35l56->init_completion,

• 			 msecs_to_jiffies(5000))) {
  

• dev_err(cs35l56->dev, "%s: init_completion timed out\n", __func__);
  

• goto complete;
  

• }
• if (!cs35l56->init_done || cs35l56->removing)

• goto complete;
  

• cs35l56->dsp.part = devm_kasprintf(cs35l56->dev, GFP_KERNEL, "cs35l56%s-%02x",

• 			   cs35l56->secured ? "s" : "", cs35l56->rev);
  

• if (!cs35l56->dsp.part)

• goto complete;
  

• pm_runtime_get_sync(cs35l56->dev);

test that this is successful?

• /*

• * Disable SoundWire interrupts to prevent race with IRQ work.
  

• * Setting sdw_irq_no_unmask prevents the handler re-enabling
  

• * the SoundWire interrupt.
  

• */
  

• if (cs35l56->sdw_peripheral) {

• cs35l56->sdw_irq_no_unmask = true;
  

• cancel_work_sync(&cs35l56->sdw_irq_work);
  

• sdw_write_no_pm(cs35l56->sdw_peripheral, CS35L56_SDW_GEN_INT_MASK_1, 0);
  

• sdw_read_no_pm(cs35l56->sdw_peripheral, CS35L56_SDW_GEN_INT_STAT_1);
  

• sdw_write_no_pm(cs35l56->sdw_peripheral, CS35L56_SDW_GEN_INT_STAT_1, 0xFF);
  

• }
• ret = cs35l56_mbox_send(cs35l56, CS35L56_MBOX_CMD_SHUTDOWN);
• if (ret) {

• dev_dbg(cs35l56->dev, "%s: CS35L56_MBOX_CMD_SHUTDOWN ret %d\n", __func__, ret);
  

• goto err;
  

• }
• if (cs35l56->rev < CS35L56_REVID_B0)

• reg = CS35L56_DSP1_PM_CUR_STATE_A1;
  

• else

• reg = CS35L56_DSP1_PM_CUR_STATE;
  

• ret = regmap_read_poll_timeout(cs35l56->regmap, reg,

• 		       val, (val == CS35L56_HALO_STATE_SHUTDOWN),
  

• 		       CS35L56_HALO_STATE_POLL_US,
  

• 		       CS35L56_HALO_STATE_TIMEOUT_US);
  

• if (ret < 0)

• dev_err(cs35l56->dev, "Failed to poll PM_CUR_STATE to 1 is %d (ret %d)\n",
  

• 	val, ret);
  

• /* Use wm_adsp to load and apply the firmware patch and coefficient files */
• ret = wm_adsp_power_up(&cs35l56->dsp);
• if (ret) {

• dev_dbg(cs35l56->dev, "%s: wm_adsp_power_up ret %d\n", __func__, ret);
  

• goto err;
  

• }
• if (cs35l56->removing)

• goto err;
  

• mutex_lock(&cs35l56->irq_lock);
• init_completion(&cs35l56->init_completion);
• cs35l56_system_reset(cs35l56);
• if (cs35l56->sdw_peripheral) {

• if (!wait_for_completion_timeout(&cs35l56->init_completion,
  

• 				 msecs_to_jiffies(5000))) {
  

• 	dev_err(cs35l56->dev, "%s: init_completion timed out (SDW)\n", __func__);
  

shouldn't do the same routine as for a regular pm_runtime resume, including re-synching regmaps?

• 	goto err_unlock;
  

• }
  

• } else {

• if (cs35l56_init(cs35l56))
  

• 	goto err_unlock;
  

• }
• cs35l56->fw_patched = true;

+err_unlock:

• mutex_unlock(&cs35l56->irq_lock);

+err:

• pm_runtime_mark_last_busy(cs35l56->dev);
• pm_runtime_put_autosuspend(cs35l56->dev);
• /* Re-enable SoundWire interrupts */
• if (cs35l56->sdw_peripheral) {

• cs35l56->sdw_irq_no_unmask = false;
  

• sdw_write_no_pm(cs35l56->sdw_peripheral, CS35L56_SDW_GEN_INT_MASK_1,
  

• 		CS35L56_SDW_INT_MASK_CODEC_IRQ);
  

• }

+complete:

• complete_all(&cs35l56->dsp_ready_completion);

+}