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:   Sat, 13 Mar 2021 00:10:38 +0530
From:   Amey Narkhede <ameynarkhede03@gmail.com>
To:     Alex Williamson <alex.williamson@redhat.com>
Cc:     bhelgaas@google.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, raphael.norwitz@nutanix.com
Subject: Re: [PATCH 0/4] Expose and manage PCI device reset
Message-ID: <20210312184038.to3g3px6ep4xfavn@archlinux>
References: <20210312173452.3855-1-ameynarkhede03@gmail.com>
 <20210312112043.3f2954e3@omen.home.shazbot.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210312112043.3f2954e3@omen.home.shazbot.org>
Precedence: bulk

On 21/03/12 11:20AM, Alex Williamson wrote:
> On Fri, 12 Mar 2021 23:04:48 +0530
> ameynarkhede03@gmail.com wrote:
>
> > From: Amey Narkhede <ameynarkhede03@gmail.com>
> >
> > PCI and PCIe devices may support a number of possible reset mechanisms
> > for example Function Level Reset (FLR) provided via Advanced Feature or
> > PCIe capabilities, Power Management reset, bus reset, or device specific reset.
> > Currently the PCI subsystem creates a policy prioritizing these reset methods
> > which provides neither visibility nor control to userspace.
> >
> > Expose the reset methods available per device to userspace, via sysfs
> > and allow an administrative user or device owner to have ability to
> > manage per device reset method priorities or exclusions.
> > This feature aims to allow greater control of a device for use cases
> > as device assignment, where specific device or platform issues may
> > interact poorly with a given reset method, and for which device specific
> > quirks have not been developed.
> >
> > Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> > Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
> > Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
>
> Reviews/Acks/Sign-off-by from others (aside from Tested/Reported-by)
> really need to be explicit, IMO.  This is a common issue for new
> developers, but it really needs to be more formal.  I wouldn't claim to
> be able to speak for Raphael and interpret his comments so far as his
> final seal of approval.
>
> Also in the patches, all Sign-offs/Reviews/Acks need to be above the
> triple dash '---' line.  Anything between that line and the beginning
> of the diff is discarded by tools.  People will often use that for
> difference between version since it will be discarded on commit.
> Likewise, the cover letter is not committed, so Review-by there are
> generally not done.  I generally make my Sign-off last in the chain and
> maintainers will generally add theirs after that.  This makes for a
> chain where someone can read up from the bottom to see how this commit
> entered the kernel.  Reviews, Acks, and whatnot will therefore usually
> be collected above the author posting the patch.
>
> Since this is a v1 patch and it's likely there will be more revisions,
> rather than send a v2 immediately with corrections, I'd probably just
> reply to the cover letter retracting Raphael's Review-by for him to
> send his own and noting that you'll fix the commit reviews formatting,
> but will wait for a bit for further comments before sending a new
> version.
>
> No big deal, nice work getting it sent out.  Thanks,
>
> Alex
>
Raphael sent me the email with
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> that
is why I included it.
So basically in v2 I should reorder tags such that Sign-off will be
the last. Did I get that right? Or am I missing something?

Thanks,
Amey

> > Amey Narkhede (4):
> >   PCI: Refactor pcie_flr to follow calling convention of other reset
> >     methods
> >   PCI: Add new bitmap for keeping track of supported reset mechanisms
> >   PCI: Remove reset_fn field from pci_dev
> >   PCI/sysfs: Allow userspace to query and set device reset mechanism
> >
> >  Documentation/ABI/testing/sysfs-bus-pci       |  15 ++
> >  drivers/crypto/cavium/nitrox/nitrox_main.c    |   4 +-
> >  drivers/crypto/qat/qat_common/adf_aer.c       |   2 +-
> >  drivers/infiniband/hw/hfi1/chip.c             |   4 +-
> >  drivers/net/ethernet/broadcom/bnxt/bnxt.c     |   2 +-
> >  .../ethernet/cavium/liquidio/lio_vf_main.c    |   4 +-
> >  .../ethernet/cavium/liquidio/octeon_mailbox.c |   2 +-
> >  drivers/net/ethernet/freescale/enetc/enetc.c  |   2 +-
> >  .../ethernet/freescale/enetc/enetc_pci_mdio.c |   2 +-
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   4 +-
> >  drivers/pci/pci-sysfs.c                       |  68 +++++++-
> >  drivers/pci/pci.c                             | 160 ++++++++++--------
> >  drivers/pci/pci.h                             |  11 +-
> >  drivers/pci/pcie/aer.c                        |  12 +-
> >  drivers/pci/probe.c                           |   4 +-
> >  drivers/pci/quirks.c                          |  17 +-
> >  include/linux/pci.h                           |  17 +-
> >  17 files changed, 213 insertions(+), 117 deletions(-)
> >
> > --
> > 2.30.2
> >
>