Re: [PATCH v3 0/6] mpt3sas: Hot-Plug Surprise removal support on IOC.

Re: [PATCH v3 0/6] mpt3sas: Hot-Plug Surprise removal support on IOC.

[Andy Shevchenko]

On Mon, Sep 24, 2018 at 9:36 AM Suganath Prabu S
<suganath-prabu.subramani@broadcom.com> wrote:
>
> Posting below set of patches to support PCIe Hot Plug surprise removal,
> and few defect fixes.
>
> This is NOT the normal PCIe Hot Plug support, whereby the user informs the
> OS that a hot removal is desired, the OS does an orderly shutdown of the
> driver on the device, special hot plug circuitry removes power from the
> PCIe slot, then the user can remove the device and replace it
> (where orderly bring-up of the device is done).
>
> With a true surprise removal (just removing HBA from a slot)
> there is a possibility to get all kinds of PCIe transaction errors,
> Below patches addresses those issues and remove HBA without bringing
> the system down.
>
> For surprise removal detection, driver does a PCI
> read of IOC's vendor field in IOC's PCI configuration space.
> If the read value is 0xFFFFFFFF this indicates that the device
> might have hot removed and the device will be removed from driver.
>

AFAIR Lukas asked you to Cc new versions to Linux PCI mailing list
and, if I'm not mistaken, to him.

> V1 changes:
> In Patch 0001 - unlock mutex, if active reset is in progress.
>
> V2 changes:
> Replaced mpt3sas_base_pci_device_is_unplugged with
> pci_device_is_present.
>
> V3 Change Set:
> Simplified function "mpt3sas_base_pci_device_is_available" and
> made inline
>
> Suganath Prabu S (6):
>   mpt3sas: Introduce      mpt3sas_base_pci_device_is_available
>   mpt3sas: Separate out      mpt3sas_wait_for_ioc_to_operational
>   mpt3sas: Introdude  _scsih_get_shost_and_ioc.
>   mpt3sas: Fix Sync cache command failure during driver      unload.
>   mpt3sas: Fix driver modifying NVRAM/persistent data.
>   mpt3sas: Bump driver version to 27.100.00.00.
>
>  drivers/scsi/mpt3sas/mpt3sas_base.c      | 135 +++++++++++++++-------
>  drivers/scsi/mpt3sas/mpt3sas_base.h      |  11 +-
>  drivers/scsi/mpt3sas/mpt3sas_config.c    |  32 +-----
>  drivers/scsi/mpt3sas/mpt3sas_ctl.c       |  26 +----
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 189 +++++++++++++++++++++++++++----
>  drivers/scsi/mpt3sas/mpt3sas_transport.c |  82 +++-----------
>  6 files changed, 298 insertions(+), 177 deletions(-)
>
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

[Suganath Prabu Subramani]

Hi Andy,

I forgot to add Lukas in last patch CC list, But i have sent a note
regarding the updated patch to him.
 Also added linux-pci

On Tue, Sep 25, 2018 at 2:10 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Sep 24, 2018 at 9:36 AM Suganath Prabu S
> <suganath-prabu.subramani@broadcom.com> wrote:
> >
> > * Driver uses "pci_device_is_present" to check whether
> > If Hot unplugged:
> > the outstanding IOs with 'DID_NO_CONNECT' before removing the drives
> > attached to the HBA.
> > "DID_NO_CONNECT" status and free the smid, if driver detects that
> > HBA is hot unplugged.
> >
> > * In the hard reset flush out all the outstanding IOs even if diag reset
> > fails and also if driver detects that HBA is hot unplugged.
>
> > +       if (!mpt3sas_base_pci_device_is_available(ioc)) {
>
> > +               pr_err(MPT3SAS_FMT
> > +                   "%s: pci error recovery reset or"
> > +                   " pci device unplug occurred\n",
>
> This should be just one line.
Above line crosses over 80 characters, so it is in two lines.
>
> > +                   ioc->name, __func__);
> > +               return;
> > +       }
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v3 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

[Andy Shevchenko]

On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
<suganath-prabu.subramani@broadcom.com> wrote:

> > > +               pr_err(MPT3SAS_FMT
> > > +                   "%s: pci error recovery reset or"
> > > +                   " pci device unplug occurred\n",
> >
> > This should be just one line.
> Above line crosses over 80 characters, so it is in two lines.

For years there is no such requirement for string literals.
Please, don't split string literals in ugly way like this.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

[Suganath Prabu Subramani]

On Tue, Sep 25, 2018 at 3:22 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
> <suganath-prabu.subramani@broadcom.com> wrote:
>
> > > > +               pr_err(MPT3SAS_FMT
> > > > +                   "%s: pci error recovery reset or"
> > > > +                   " pci device unplug occurred\n",
> > >
> > > This should be just one line.
> > Above line crosses over 80 characters, so it is in two lines.
>
> For years there is no such requirement for string literals.
Below is the one i was mentioning,
WARNING: line over 80 characters
#30: FILE: drivers/scsi/mpt3sas/mpt3sas_base.c:6861:
+ pr_err(MPT3SAS_FMT "%s: pci error recovery reset or pci device
unplug occurred\n", ioc->name, __func__);

Warning comes for both characters over 80 chars and string split.
Do you want us to ignore warning and repost like above in single line ?
> Please, don't split string literals in ugly way like this.
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v3 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available

[Andy Shevchenko]

On Tue, Sep 25, 2018 at 1:29 PM Suganath Prabu Subramani
<suganath-prabu.subramani@broadcom.com> wrote:
>
> On Tue, Sep 25, 2018 at 3:22 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Tue, Sep 25, 2018 at 12:46 PM Suganath Prabu Subramani
> > <suganath-prabu.subramani@broadcom.com> wrote:
> >
> > > > > +               pr_err(MPT3SAS_FMT
> > > > > +                   "%s: pci error recovery reset or"
> > > > > +                   " pci device unplug occurred\n",
> > > >
> > > > This should be just one line.
> > > Above line crosses over 80 characters, so it is in two lines.
> >
> > For years there is no such requirement for string literals.
> Below is the one i was mentioning,
> WARNING: line over 80 characters
> #30: FILE: drivers/scsi/mpt3sas/mpt3sas_base.c:6861:
> + pr_err(MPT3SAS_FMT "%s: pci error recovery reset or pci device
> unplug occurred\n", ioc->name, __func__);
>
> Warning comes for both characters over 80 chars and string split.
> Do you want us to ignore warning and repost like above in single line ?
> > Please, don't split string literals in ugly way like this.

I didn't tell anything about other parameters to be on the same line.
So, please, try again.

-- 
With Best Regards,
Andy Shevchenko