linux-mips
[Top] [All Lists]

Re: [PATCH] 2/7 AU1100 MMC support

To: Rodolfo Giometti <giometti@linux.it>
Subject: Re: [PATCH] 2/7 AU1100 MMC support
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date: Thu, 10 Aug 2006 20:12:42 +0400
Cc: linux-mips@linux-mips.org
In-reply-to: <20060810133658.GZ342@enneenne.com>
Organization: MontaVista Software Inc.
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20060809210843.GC13145@enneenne.com> <44DB34C2.3090302@ru.mvista.com> <20060810133658.GZ342@enneenne.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.2) Gecko/20040803
Hello.

Rodolfo Giometti wrote:

static void au1xmmc_set_power(struct au1xmmc_host *host, int state)
{

-       u32 val = au1xmmc_card_table[host->id].bcsrpwr;
+       u32 val;

+       val = au1xmmc_card_table[host->id].power;
+
+#if defined(CONFIG_MIPS_DB1200)
        bcsr->board &= ~val;
        if (state) bcsr->board |= val;
+#endif

        au_sync_delay(1);
}

If DBAu1100 doesn't allow to control slot power, then I don't think pretending it does is a good thing. Shouldn't these #ifdef's be in au1xmmc_set_ios() instead (the function is void anyway but that would allow us to save on the code size a bit more)?

Mmm. I proposed that solution but I don't know exaclty how several
DB1x00 boards work. I just protect the variable "bcsr" which is not
defined for my board.

That wasn't obvious from the patch that this is intended for some specific board and BCSRs (board control/status registers) seem to exist on all Alchemy development boards.

static inline int au1xmmc_card_inserted(struct au1xmmc_host *host)
{
-       return (bcsr->sig_status & au1xmmc_card_table[host->id].bcsrstatus)
-               ? 1 : 0;
+       u32 val, data = 1;
+
+       val = au1xmmc_card_table[host->id].status;
+
+#if defined(CONFIG_MIPS_DB1200)
+       data = bcsr->sig_status & val;
+#endif
+
+       return !!data;
}

Hrm, are you sure there's no way to sense that the card is *really* inserted or not?


Again as above. For my specific board I use:

   #if defined(CONFIG_MIPS_DB1200)
       data = bcsr->sig_status & val;
   #elif defined(CONFIG_MIPS_MYBOARD)
       specific code...
   #endif

Maybe we should modify my solution including other DB1x00 boards
ifdef. However, the important thing is to protect againt variable
"bcsr" if a specific board doesn't support it.

If you're not introducing the support for the new board, introducing those #ifdef's doesn't make much sense. Overall, the support for the boards other than Pb/DbAu1200 seems broken in that driver -- see below why...

static inline int au1xmmc_card_readonly(struct au1xmmc_host *host)
{
-       return (bcsr->status & au1xmmc_card_table[host->id].wpstatus)
-               ? 1 : 0;
+       u32 val, data = 0;
+
+       val = au1xmmc_card_table[host->id].wpstatus;
+
+#if defined(CONFIG_MIPS_DB1200)
+       data = bcsr->status & val;
+#endif
+
+       return !!data;
}

  Ditto.

The same as above. :)

Indeed, some modifications are needed for the other Alchemy boards -- there's some discrepancy with the "board" register in particular, for the boards older than Pb/DBAu1200 it was called "specific" having much the same format and purpose. This is not so however with "sig_status" and "status" registers used to sense card's presence and for write protecting. Judging on <asm/mach-db1x00/db1x00.h>, "specific" reg. should be used instead. There are MMC macros in that file BTW for detecting the cards and applying power to them.

Ciao,

WBR, Sergei

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