2/7 AU1100 MMC support
Subsystems (2)
PATCH 1/1 2/7 AU1100 MMC support
Protect code on "BCSR" device.
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
drivers/mmc/au1xmmc.c +29 -7
@@ -65,8 +65,8 @@ #endifconst struct {u32 iobase;u32 tx_devid, rx_devid; -u16 bcsrpwr; -u16 bcsrstatus; +u16 power; +u16 status;u16 wpstatus; } au1xmmc_card_table[] = { {SD0_BASE ,DSCR_CMD0_SDMS_TX0 ,DSCR_CMD0_SDMS_RX0 ,
@@ -138,24 +138,42 @@ static inline void SEND_STOP(struct au1xstatic 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 ); }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; }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; }static void au1xmmc_finish_request (struct au1xmmc_host *host)
@@ -175,7 +193,9 @@ static void au1xmmc_finish_request(struc host->status =HOST_S_IDLE ; +#if defined(CONFIG_MIPS_DB1200 ) bcsr->disk_leds |= (1 <<8 ); +#endif mmc_request_done (host->mmc, mrq); }
@@ -670,7 +690,9 @@ static void au1xmmc_request(struct mmc_h host->mrq = mrq; host->status =HOST_S_CMD ; +#if defined(CONFIG_MIPS_DB1200 ) bcsr->disk_leds &= ~(1 <<8 ); +#endif if (mrq->data) {FLUSH_FIFO (host);
Hello.
Quoted 27 lines · Rodolfo Giometti
Protect code on "BCSR" device.
Signed-off-by Rodolfo Giometti <[email protected]>------------------------------------------------------------------------
drivers/mmc/au1xmmc.c +3
-3@@ -65,8 +65,8 @@ #endifconst struct {u32 iobase;u32 tx_devid, rx_devid; -u16 bcsrpwr; -u16 bcsrstatus; +u16 power; +u16 status;u16 wpstatus; } au1xmmc_card_table[] = { {SD0_BASE ,DSCR_CMD0_SDMS_TX0 ,DSCR_CMD0_SDMS_RX0 ,@@ -138,24 +138,42 @@ static inline void SEND_STOP(struct au1xstatic void au1xmmc_set_power (struct au1xmmc_host *host,int state) { -u32 val = au1xmmc_card_table[host->id].bcsrpwr; +u32 val;
drivers/mmc/au1xmmc.c static inline void SEND_STOP(struct au1x · line 151
Quoted 10 lines
drivers/mmc/au1xmmc.c +4
-0+ 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)?
Quoted 4 lines
drivers/mmc/au1xmmc.c +0
-2static inline int au1xmmc_card_inserted (struct au1xmmc_host *host) { -return (bcsr->sig_status & au1xmmc_card_table[host->id].bcsrstatus) - ?1 :0 ;
drivers/mmc/au1xmmc.c static inline void SEND_STOP(struct au1x · line 164
Quoted 10 lines
drivers/mmc/au1xmmc.c +9
-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?
Quoted 4 lines
drivers/mmc/au1xmmc.c +0
-2static inline int au1xmmc_card_readonly (struct au1xmmc_host *host) { -return (bcsr->status & au1xmmc_card_table[host->id].wpstatus) - ?1 :0 ;
drivers/mmc/au1xmmc.c static inline void SEND_STOP(struct au1x · line 177
Quoted 10 lines
drivers/mmc/au1xmmc.c +9
-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.
WBR, Sergei
Quoted 13 lines · Sergei Shtylyov
Quoted 13 lines
static void
au1xmmc_set_power(struct au1xmmc_host *host, int state){drivers/mmc/au1xmmc.c +4
-1-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
Quoted 7 lines
Quoted 2 lines
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 inau1xmmc_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.
Quoted 9 lines
Quoted 9 lines
static inline int
au1xmmc_card_inserted(struct au1xmmc_host *host){drivers/mmc/au1xmmc.c +5
-2-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 )
Quoted 8 lines
Quoted 5 lines
drivers/mmc/au1xmmc.c +4
-0+ 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...
#endifMaybe 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.
Quoted 8 lines
Quoted 8 lines
static inline int
au1xmmc_card_readonly(struct au1xmmc_host *host){drivers/mmc/au1xmmc.c +4
-2-return (bcsr->status & au1xmmc_card_table[host->id].wpstatus) - ?1 :0 ; +u32 val, data =0 ; + + val = au1xmmc_card_table[host->id].wpstatus; +
Quoted 8 lines
Quoted 6 lines
drivers/mmc/au1xmmc.c +5
-0+#if defined(CONFIG_MIPS_DB1200 ) + data = bcsr->status & val; +#endif + +return !!data;}
Ditto.
The same as above. :)
Ciao,
Rodolfo
Signature · 4 lines
GNU/Linux Solutions e-mail: [email protected] Linux Device Driver [email protected] Embedded Systems [email protected] UNIX programming phone: +39 349 2432127
Hi Rodolfo,
Quoted 11 lines · Rodolfo Giometti
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... #endifMaybe 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.
FWIW, I think the whole Db/Pb board specific mess should be removed from the driver entirely and placed in the respective boards' code. The board should then pass on a struct to the driver with hooks into those functions (like in arch/arm/mach-pxa/corgi.c).
It is on my list of things to fix for a later time. If noone beats me to it, I'm going to do it, but it may take a while since I'm working on other things.
Thanks,
Signature · 1 line
ml.
Hello.
Quoted 16 lines · Rodolfo Giometti
Quoted 15 lines
Quoted 15 lines
static void
au1xmmc_set_power(struct au1xmmc_host *host, int state){drivers/mmc/au1xmmc.c +4
-1-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); }
Quoted 6 lines
Quoted 2 lines
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 inau1xmmc_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.
Quoted 27 lines
Quoted 9 lines
Quoted 9 lines
static inline int
au1xmmc_card_inserted(struct au1xmmc_host *host){drivers/mmc/au1xmmc.c +5
-2-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 )Quoted 8 lines
Quoted 5 lines
drivers/mmc/au1xmmc.c +4
-0+ 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
Quoted 3 lines
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...
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
static inline int
au1xmmc_card_readonly(struct au1xmmc_host *host){drivers/mmc/au1xmmc.c +4
-2-return (bcsr->status & au1xmmc_card_table[host->id].wpstatus) - ?1 :0 ; +u32 val, data =0 ; + + val = au1xmmc_card_table[host->id].wpstatus; +Quoted 8 lines
Quoted 6 lines
drivers/mmc/au1xmmc.c +5
-0+#if defined(CONFIG_MIPS_DB1200 ) + data = bcsr->status & val; +#endif + +return !!data;}
Ditto.
Quoted 1 line
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.
Quoted 1 line
Ciao,
WBR, Sergei