Trivial nits/questions. In subject: s/Introdude/Introduce/ s/ / / (remove double space) s/\.// (remove trailing period, also appears in patches 4, 5, 6) On Wed, Sep 26, 2018 at 09:52:36AM +0530, Suganath Prabu S wrote: > The code for getting shost and IOC is redundant so > moved that to function "scsih_get_shost_and_ioc". > Also checks for NULL are added to IOC and shost. > > Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@xxxxxxxxxxxx> Per [1], it's good to use full names, e.g., spell out "S" when that makes sense. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n456 > --- > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 98 ++++++++++++++++++++++++++++++------ > 1 file changed, 82 insertions(+), 16 deletions(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > index 566a550..f6e92eb 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > @@ -9809,6 +9809,35 @@ _scsih_ir_shutdown(struct MPT3SAS_ADAPTER *ioc) > } > > /** > + * _scsih_get_shost_and_ioc - get shost and ioc > + * and verify whether they are NULL or not > + * @pdev: PCI device struct > + * @shost: address of scsi host pointer There seems to be a convention to capitalize "IOC" and "SCSI" when used in English text, i.e., when they're not variable names or part of a function name. > + * @ioc: address of HBA adapter pointer > + * > + * Return zero if *shost and *ioc are not NULL otherwise return error number. > + */ > +static int > +_scsih_get_shost_and_ioc(struct pci_dev *pdev, > + struct Scsi_Host **shost, struct MPT3SAS_ADAPTER **ioc) > +{ > + *shost = pci_get_drvdata(pdev); > + if (*shost == NULL) { > + dev_err(&pdev->dev, "pdev's driver data is null\n"); I don't see any other uses of dev_printk() in this file; existing output seems to use sdev_printk(), pr_info(), etc. Just pointing this out to make sure dev_printk() is really what you want here. > + return -ENXIO; > + } Bjorn
