Date:   Mon, 29 Nov 2021 21:45:00 +0800
From:   Shawn Guo <shawn.guo@linaro.org>
To:     Maulik Shah <quic_mkshah@quicinc.com>
Cc:     Marc Zyngier <maz@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Loic Poulain <loic.poulain@linaro.org>,
        devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] irqchip: Add Qualcomm MPM controller driver
Message-ID: <20211129134459.GC10105@dragon>
References: <20211126093529.31661-1-shawn.guo@linaro.org>
 <20211126093529.31661-3-shawn.guo@linaro.org>
 <56a5820e-9cd7-aa49-7ce8-9547f355986e@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <56a5820e-9cd7-aa49-7ce8-9547f355986e@quicinc.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Mon, Nov 29, 2021 at 12:53:39PM +0530, Maulik Shah wrote:
> Hi Shawn,

Hi Maulik,

Thanks for the comment!

> 
> On 11/26/2021 3:05 PM, Shawn Guo wrote:
> > +static int __maybe_unused qcom_mpm_suspend_late(struct device *dev)
> 
> why maybe unused?
> 
> > +{
> > +	struct qcom_mpm_priv *priv = dev_get_drvdata(dev);
> > +	int i, ret;
> > +
> > +	for (i = 0; i < priv->reg_stride; i++)
> > +		qcom_mpm_write(priv, MPM_REG_STATUS, i, 0);
> > +
> > +	/* Notify RPM to write vMPM into HW */
> > +	ret = mbox_send_message(priv->mbox_chan, NULL);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused qcom_mpm_resume_early(struct device *dev)
> > +{
> > +	/* Nothing to do for now */
> > +	return 0;
> > +}
> > +
> > +static const struct dev_pm_ops qcom_mpm_pm_ops = {
> > +	SET_LATE_SYSTEM_SLEEP_PM_OPS(qcom_mpm_suspend_late,
> > +				     qcom_mpm_resume_early)
> > +};
> 
> This is not limited to suspend, you will need to notify RPM during deepest
> cpu idle state entry as well, since MPM may be monitoring interrupts in that
> case too.

Yeah, I was trying to test this MPM driver with cpuidle, but failed to
see the SoC get into vlow/vmin state from cpuidle.  Do you have any
suggestion how I should test it properly?

> This may be handled for both suspend/CPUidle using cpu pm notifications
> where in last cpu entering deepest idle may notify RPM (something similar to
> what rpmh-rsc.c does)

Thanks for the pointer!  I will take a look.

Shawn