All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Olof Johansson <olof@lixom.net>
Cc: Jeff Moyer <jmoyer@redhat.com>,
	OS Engineering <osengineering@stec-inc.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Akhil Bhansali <abhansali@stec-inc.com>,
	Ramprasad Chinthekindi <rchinthekindi@stec-inc.com>,
	Amit Phansalkar <aphansalkar@stec-inc.com>
Subject: Re: [PATCH] block: Device driver for sTec's PCIe Kronos Card.
Date: Tue, 17 Sep 2013 14:20:55 -0600	[thread overview]
Message-ID: <5238B9A7.6040002@kernel.dk> (raw)
In-Reply-To: <CAOesGMhF42=qQLyMyF4-tsTjCxwX0bb10rTQv9sLooyzrEOpHA@mail.gmail.com>

On 09/17/2013 01:04 PM, Olof Johansson wrote:
> On Thu, Sep 5, 2013 at 7:12 AM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 09/05/2013 06:00 AM, Jeff Moyer wrote:
>>> OS Engineering <osengineering@stec-inc.com> writes:
>>>
>>>> Hi Jeff,
>>>>
>>>> Thank you for reviewing the patch.
>>>
>>> No problem.  Jens, any objection to queueing this up for 3.12?
>>
>> I'll give it a look-over, but usually I'm pretty lax when it comes to
>> new drivers. So no, I'd be surprised if we can't queue this up for 3.12.
> 
> I came across this driver because it was spewing a lot of really
> trivial and easy to fix compiler warnings. Silly stuff such as
> printing u32 with %lu.
> 
> From a quick look at the code, several things are immediately apparent:
> 
> First, checkpatch says, on the currently existing file in -next:
> 
> total: 3 errors, 61 warnings, 5817 lines checked
> 
> Code like this looks _really_ confused:
> 
>                 barrier();
>                 val = readl(skdev->mem_map[1] + offset);
>                 barrier();
> 
> There are also some crazy long functions that should be refactored,
> such as skd_request_fn().
> 
> So, it looks like this driver needs a bunch of work before it's ready
> to go in. Or, maybe it's better to submit it with a TODO list for the
> staging tree instead?

Not disagreeing with you, it definitely needs a bit of cleaning. And so
far stec has not been very responsive in fixing those issues.

-- 
Jens Axboe


  reply	other threads:[~2013-09-17 20:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-30 12:36 [PATCH] block: Device driver for sTec's PCIe Kronos Card OS Engineering
2013-08-30 13:18 ` Jeff Moyer
2013-09-05  5:31   ` OS Engineering
2013-09-05 12:00     ` Jeff Moyer
2013-09-05 14:12       ` Jens Axboe
2013-09-17 19:04         ` Olof Johansson
2013-09-17 20:20           ` Jens Axboe [this message]
2013-09-20 22:31             ` Andrew Morton
2013-09-21 20:01               ` Jens Axboe
2013-09-22 23:02                 ` Olof Johansson
2013-09-22 23:12                   ` Jens Axboe
2013-09-23 11:09                     ` Amit Phansalkar

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=5238B9A7.6040002@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=abhansali@stec-inc.com \
    --cc=aphansalkar@stec-inc.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olof@lixom.net \
    --cc=osengineering@stec-inc.com \
    --cc=rchinthekindi@stec-inc.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.