linux-mips
[Top] [All Lists]

Re: [PATCH] ide: Add tx4939ide driver (v3)

To: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Subject: Re: [PATCH] ide: Add tx4939ide driver (v3)
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date: Thu, 16 Oct 2008 21:23:07 +0400
Cc: linux-mips@linux-mips.org, linux-ide@vger.kernel.org, bzolnier@gmail.com, ralf@linux-mips.org
In-reply-to: <20081017.013101.128619577.anemo@mba.ocn.ne.jp>
Organization: MontaVista Software Inc.
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20081003.000838.27954527.anemo@mba.ocn.ne.jp> <48F7391D.8050109@ru.mvista.com> <20081017.013101.128619577.anemo@mba.ocn.ne.jp>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.2) Gecko/20040803
Hello.

Atsushi Nemoto wrote:

This is the driver for the Toshiba TX4939 SoC ATA controller.

  Mostly ACK but there's still a few minor nits...

Welcome back!

   Indeed, it turned out to be hard to drown in the Dead see. :-)

 I will address all of your points except for followings.

+                       xcount = bcount & 0xffff;
+                       if (xcount == 0x0000) {

Hm, I'm not sure this is necessary here... although I didn't see an explicit mention that zero count means 64 KB in the datasheet -- which it must mean if the BMIDE spec. was followed).
In ide-dma.c this check was added because of CS5530's brain damage...

Hmm, if I could test this case easily I will drop this.  Otherwise I
will keep it as is for future investigation.

   OK.

+       if ((dma_stat & 7) == 0 &&
+           (ctl & (TX4939IDE_INT_XFEREND | TX4939IDE_INT_HOST)) ==
+           (TX4939IDE_INT_XFEREND | TX4939IDE_INT_HOST))

  Parens around & and | are hardly needed...

You mean more parens are needed?

   I mean less. :-)

---
Atsushi Nemoto

MBR, Sergei

<Prev in Thread] Current Thread [Next in Thread>