On 24/02/2022 15:41, Pierre-Louis Bossart wrote:
static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
@@ -1197,12 +1224,23 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
static int swrm_reg_show(struct seq_file *s_file, void *data)
{
struct qcom_swrm_ctrl *swrm = s_file->private;
- int reg, reg_val;
+ int reg, reg_val, ret;
+
+ ret = pm_runtime_get_sync(swrm->dev);
+ if (ret < 0 && ret != -EACCES) {
+ dev_err_ratelimited(swrm->dev,
+ "pm_runtime_get_sync failed in %s, ret %d\n",
+ __func__, ret);
+ pm_runtime_put_noidle(swrm->dev);
+ }
for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
swrm->reg_read(swrm, reg, ®_val);
seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
}
+ pm_runtime_mark_last_busy(swrm->dev);
+ pm_runtime_put_autosuspend(swrm->dev);
+
question: is there a reason why this specific set of reg_read() is
surrounded pm_runtime stuff? Is this saying that in all other case where
the callback is used, the controller is already resumed and fully
operational? That's be worthy of a comment.
controller register reads require clk to be ON, which might not be
always ON. In suspended case we switch off the clocks.
Other places so far that I have seen is that controller is either
already resumed or clk is on (interrupt case) and resume case.
struct qcom_swrm_ctrl *swrm
struct qcom_swrm_ctrl *ctrl
nit-pick: it helps reviewers when the same variable name is used
consistently.
Yes, I did notice this, but for some reason I forgot to fix it.
+static int __maybe_unused swrm_runtime_suspend(struct device *dev)
+{
+ struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
+ int ret;
+
+ if (!ctrl->clock_stop_not_supported) {
+ /* Mask bus clash interrupt */
+ ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
+ ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
+ ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
+ }
+ /* Prepare slaves for clock stop */
+ ret = sdw_bus_prep_clk_stop(&ctrl->bus);
+ if (ret < 0) {
if (ret < 0 && ret != -ENODATA) {
?
+ dev_err(dev, "prepare clock stop failed %d", ret);
+ return ret;
+ }
+
+ ret = sdw_bus_clk_stop(&ctrl->bus);
+ if (ret < 0 && ret != -ENODATA) {
+ dev_err(dev, "bus clock stop failed %d", ret);
+ return ret;
+ }
+
+ clk_disable_unprepare(ctrl->hclk);
+
+ usleep_range(300, 305);
+
+ return 0;
+}
