Re: [PATCH v1] spi: spi-topcliff-pch: use generic power management

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Bjorn Helgaas <bjorn@helgaas.com>,
	Vaibhav Gupta <vaibhav.varodek@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v1] spi: spi-topcliff-pch: use generic power management
Date: Fri, 24 Jul 2020 23:16:55 +0300	[thread overview]
Message-ID: <CAHp75Vdo22ofbCktupFYbfYy6PQ609fsk5B6u2b3FpfKxs8OQg@mail.gmail.com> (raw)
In-Reply-To: <20200724151601.GA3642@gmail.com>

On Fri, Jul 24, 2020 at 6:17 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote:
> On Fri, Jul 24, 2020 at 01:51:49PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 20, 2020 at 7:31 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote:

...

> > > +       device_wakeup_disable(dev);
> >
> > Here I left a result. Care to explain (and perhaps send a follow up
> > fix) where is the counterpart to this call?
> >
> Hello Andy,
> I didn't quite understand what you are trying to point at. And the result part.

I emphasized the line by surrounding it with two blank lines followed
by my comment.

> Yes, it seem I forgot to put device_wakeup_disable() in .suspend() when I
> removed pci_enable_wake(pdev, PCI_D3hot, 0); from there. It doesn't seem that
> .suspend() wants to enable-wake the device as the bool value passed to
> pci_enable_wake() is zero.

> Am I missing something else?

At least above. Either you need to drop the current call, or explain
how it works.
Since you have no hardware to test, I would rather ask to drop an
extra call or revert the change.

-- 
With Best Regards,
Andy Shevchenko