Re: [PATCH v2 05/10] mailbox: tegra-hsp: Add suspend/resume support

From: Thierry Reding <thierry.reding@gmail.com>
To: Jon Hunter <jonathanh@nvidia.com>
Cc: devicetree@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Mika Liljeberg <mliljeberg@nvidia.com>,
	Mikko Perttunen <mperttunen@nvidia.com>,
	Timo Alho <talho@nvidia.com>,
	linux-serial@vger.kernel.org, Jiri Slaby <jslaby@suse.com>,
	linux-tegra@vger.kernel.org, Pekka Pessi <ppessi@nvidia.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 05/10] mailbox: tegra-hsp: Add suspend/resume support
Date: Mon, 10 Dec 2018 10:58:16 +0100	[thread overview]
Message-ID: <20181210095816.GB15154@ulmo> (raw)
In-Reply-To: <9a7a55fa-9989-21cd-f981-f43d3b0f2d6c@nvidia.com>

[-- Attachment #1.1: Type: text/plain, Size: 2233 bytes --]

On Tue, Nov 13, 2018 at 11:17:58AM +0000, Jon Hunter wrote:
> 
> On 12/11/2018 15:18, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Upon resuming from a system sleep state, the interrupts for all active
> > shared mailboxes need to be reenabled, otherwise they will not work.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/mailbox/tegra-hsp.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> > index 0100a974149b..1259abf3542f 100644
> > --- a/drivers/mailbox/tegra-hsp.c
> > +++ b/drivers/mailbox/tegra-hsp.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/pm.h>
> >  #include <linux/slab.h>
> >  
> >  #include <dt-bindings/mailbox/tegra186-hsp.h>
> > @@ -817,6 +818,23 @@ static int tegra_hsp_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +static int tegra_hsp_resume(struct device *dev)
> > +{
> > +	struct tegra_hsp *hsp = dev_get_drvdata(dev);
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < hsp->num_sm; i++) {
> > +		struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
> > +
> > +		if (mb->channel.chan->cl)
> > +			tegra_hsp_mailbox_startup(mb->channel.chan);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(tegra_hsp_pm_ops, NULL, tegra_hsp_resume);
> 
> Is it worth disabling interrupts on suspend to avoid any in-flight
> interrupt triggering a bad access on entering suspend? I assume that the
> context of the mailbox registers get cleared/lost at some point and so I
> was not sure if there is a time where they have a bad state or are
> inaccessible?

Theoretically I think we'd have to do that. However, since we end up
having to busy-loop for any current use-cases (i.e. console) anyway,
we'll never end up in a situation where an interrupt could occur at
this point. The console will long have been suspended when we reach
this point. I'll take a look if a use-case can be constructed where
such an in-flight interrupt could be produced.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel