public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: ching2048@areca.com.tw
Cc: linux-scsi@vger.kernel.org
Subject: re: [SCSI] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284
Date: Wed, 28 Aug 2013 18:48:48 +0300	[thread overview]
Message-ID: <20130828154848.GA571@elgon.mountain> (raw)

Hello 黃清隆,

The patch 17628f3a062b: "[SCSI] arcmsr: Support Areca new SATA Raid
Adapter ARC1214/1224/1264/1284" from Aug 26, 2013, leads to the
following Smatch warning:
"drivers/scsi/arcmsr/arcmsr_hba.c:3580 arcmsr_hbaD_get_config()
	 warn: signedness bug returning '(-12)'"

drivers/scsi/arcmsr/arcmsr_hba.c
  3576          dma_coherent = dma_alloc_coherent(&pdev->dev, acb->uncache_size,
  3577          &dma_coherent_handle, GFP_KERNEL);
  3578          if (!dma_coherent) {
  3579                  pr_notice("DMA allocation failed...\n");
  3580                  return -ENOMEM;
                        ^^^^^^^^^^^^^^
This should be returning false.

  3581          }

Line 3577 has messed up indenting.

Also this patch says it adds support for new hardware but almost 900
lines out of this 3605 line patch are white space changes.  Do the
unrelated white space changes in a separate patch.

This patch also re-introduces a bug which I fixed in the mainline kernel
a year ago.

drivers/scsi/arcmsr/arcmsr_hba.c
  4525                          writel(0xD, &pmuC->write_sequence);
  4526                  } while ((((temp = readl(&pmuC->host_diagnostic)) |
                                                                          ^
This should be a '&' not a '|'.  Please fix this again back to the way
it was.

  4527                  ARCMSR_ARC1880_DiagWrite_ENABLE) == 0) &&
  4528                  (count < 5));

The indenting here is messed up as well.  This is a very low quality
patch.

I think you are not using git internally in your company and that is why
you are messing up so badly.  Please learn to use it.  Keep track of the
fixes which go into the mainline kernel.  Separate the white space
cleanups from the new features.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

             reply	other threads:[~2013-08-28 15:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-28 15:48 Dan Carpenter [this message]
2013-08-28 16:25 ` [SCSI] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284 James Bottomley
2013-08-29 12:01 ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130828154848.GA571@elgon.mountain \
    --to=dan.carpenter@oracle.com \
    --cc=ching2048@areca.com.tw \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox