Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 14 Jul 2021 16:56:24 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Shanker R Donthineni <sdonthineni@nvidia.com>
Cc:     Amey Narkhede <ameynarkhede03@gmail.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Raphael Norwitz <raphael.norwitz@nutanix.com>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <kw@linux.com>, Sinan Kaya <okaya@kernel.org>,
        Len Brown <lenb@kernel.org>,
        "Rafael J . Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v10 7/8] PCI: Add support for ACPI _RST reset method
Message-ID: <20210714165624.47272c2d.alex.williamson@redhat.com>
In-Reply-To: <e8f0b236-dfb3-c9cf-4683-c43e8bc0c0b4@nvidia.com>
References: <20210709123813.8700-1-ameynarkhede03@gmail.com>
        <20210709123813.8700-8-ameynarkhede03@gmail.com>
        <20210712170920.2a0868ac.alex.williamson@redhat.com>
        <e8f0b236-dfb3-c9cf-4683-c43e8bc0c0b4@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 12 Jul 2021 19:51:41 -0500
Shanker R Donthineni <sdonthineni@nvidia.com> wrote:

> Hi Alex,
> 
> On 7/12/21 6:09 PM, Alex Williamson wrote:
> >> +/**
> >> + * pci_dev_acpi_reset - do a function level reset using _RST method
> >> + * @dev: device to reset
> >> + * @probe: check if _RST method is included in the acpi_device context.
> >> + */
> >> +int pci_dev_acpi_reset(struct pci_dev *dev, int probe)
> >> +{
> >> +     acpi_handle handle = ACPI_HANDLE(&dev->dev);
> >> +
> >> +     if (!handle || !acpi_has_method(handle, "_RST"))
> >> +             return -ENOTTY;
> >> +
> >> +     if (probe)
> >> +             return 0;
> >> +
> >> +     if (ACPI_FAILURE(acpi_evaluate_object(handle, "_RST", NULL, NULL))) {
> >> +             pci_warn(dev, "ACPI _RST failed\n");
> >> +             return -EINVAL;  
> > Should we return -ENOTTY here instead to give a possible secondary
> > reset method a chance?  Thanks,  
> Thanks for reviewing patches.
> 
> ACPI/AML _RST method type is VOID. The only possibility of failure would be
> either system is running out of memory or bugs in ACPICA. There is no strong
> reason not to return -NOTTY.
> 
> I'll fix in the next version. Is there opportunity to include reset feature in v5.14-rc2?

Sounds good, it's a corner case but since we've got a series of methods
we can try and part of the point of Amey's series is giving the user
control of the order and methods to try, we might as well make use of
it.  I think there's also some precedence in the quirks that they can
fail and fall through to standard resets.

I'll leave any upstream timing questions to Bjorn, but we've passed the
v5.14 merge window when new functionality is generally accepted.

> Can I add your reviewed-by since no other comments to this patch?

Yeah, s/-EINVAL/-ENOTTY/

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>

Thanks!