Hi, some trivial comments below. On Fri, Oct 06, 2017 at 05:51:33PM +0200, srinivas.kandagatla@linaro.org wrote: > From: Sagar Dharia > > Per slimbus specification, a reconfiguration sequence known as > 'clock pause' needs to be broadcast over the bus while entering low- > power mode. Clock-pause is initiated by the controller driver. > To exit clock-pause, controller typically wakes up the framer device. > Since wakeup precedure is controller-specific, framework calls it via > controller's function pointer to invoke it. > > Signed-off-by: Sagar Dharia > Signed-off-by: Srinivas Kandagatla > --- [...] > @@ -429,6 +444,14 @@ void slim_return_tx(struct slim_controller *ctrl, int err) > cur.cb(cur.ctx, err); > > up(&ctrl->tx_sem); > + if (!cur.clk_pause && (!cur.need_tid || err)) { > + /** This isn't really a kerneldoc comment. > + * remove runtime-pm vote if this was TX only, or > + * if there was error during this transaction > + */ > + pm_runtime_mark_last_busy(ctrl->dev.parent); > + pm_runtime_put_autosuspend(ctrl->dev.parent); > + } > } > EXPORT_SYMBOL_GPL(slim_return_tx); > [...] > +/** > + * slim_ctrl_clk_pause: Called by slimbus controller to enter/exit 'clock pause' > + * Slimbus specification needs this sequence to turn-off clocks for the bus. > + * The sequence involves sending 3 broadcast messages (reconfiguration > + * sequence) to inform all devices on the bus. > + * To exit clock-pause, controller typically wakes up active framer device. > + * @ctrl: controller requesting bus to be paused or woken up > + * @wakeup: Wakeup this controller from clock pause. > + * @restart: Restart time value per spec used for clock pause. This value > + * isn't used when controller is to be woken up. > + * This API executes clock pause reconfiguration sequence if wakeup is false. > + * If wakeup is true, controller's wakeup is called. > + * For entering clock-pause, -EBUSY is returned if a message txn in pending. > + */ > +int slim_ctrl_clk_pause(struct slim_controller *ctrl, bool wakeup, u8 restart) > +{ > + int i, ret = 0; > + unsigned long flags; > + struct slim_sched *sched = &ctrl->sched; > + struct slim_val_inf msg = {0, 0, NULL, NULL, NULL, NULL}; > + > + DEFINE_SLIM_BCAST_TXN(txn, SLIM_MSG_MC_BEGIN_RECONFIGURATION, > + 3, SLIM_LA_MANAGER, &msg); > + > + if (wakeup == false && restart > SLIM_CLK_UNSPECIFIED) > + return -EINVAL; > + > + mutex_lock(&sched->m_reconf); > + if (wakeup) { > + if (sched->clk_state == SLIM_CLK_ACTIVE) { > + mutex_unlock(&sched->m_reconf); > + return 0; > + } > + > + /** ditto > + * Fine-tune calculation based on clock gear, > + * message-bandwidth after bandwidth management > + */ > + ret = wait_for_completion_timeout(&sched->pause_comp, > + msecs_to_jiffies(100)); > + if (!ret) { > + mutex_unlock(&sched->m_reconf); > + pr_err("Previous clock pause did not finish"); > + return -ETIMEDOUT; > + } > + ret = 0; > + > + /** ditto > + * Slimbus framework will call controller wakeup > + * Controller should make sure that it sets active framer > + * out of clock pause > + */ > + if (sched->clk_state == SLIM_CLK_PAUSED && ctrl->wakeup) > + ret = ctrl->wakeup(ctrl); > + if (!ret) > + sched->clk_state = SLIM_CLK_ACTIVE; > + mutex_unlock(&sched->m_reconf); > + > + return ret; > + } [...] Thanks, Jonathan Neuschäfer