Hello? Thanks for comments. Some patches will be posted for your requests. Also I wrote my opinion for some comments.
>> diff --git a/drivers/block/Makefile b/drivers/block/Makefile >> index 59388d9..eccefc1 100644 >> --- a/drivers/block/Makefile >> +++ b/drivers/block/Makefile >> @@ -25,13 +25,14 @@ include $(TOPDIR)/config.mk >> >> LIB := $(obj)libblock.a >> >> -COBJS-$(CONFIG_SCSI_AHCI) += ahci.o > why? This list badly sorted before I add. I just resort it to alphabetical order and add new option. Nothing removed. >> COBJS-$(CONFIG_ATA_PIIX) += ata_piix.o >> +COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o >> COBJS-$(CONFIG_FSL_SATA) += fsl_sata.o >> +COBJS-$(CONFIG_IDE_SIL680) += sil680.o >> COBJS-$(CONFIG_LIBATA) += libata.o >> COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o >> COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o >> -COBJS-$(CONFIG_IDE_SIL680) += sil680.o >> +COBJS-$(CONFIG_SCSI_AHCI) += ahci.o >> COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o >> COBJS-$(CONFIG_SYSTEMACE) += systemace.o >> >> diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c >> new file mode 100644 >> index 0000000..4454fca >> --- /dev/null >> +++ b/drivers/block/mg_disk.c >> + >> +#define MG_BASE (host.drv_data->base) > an inline function will be better This isn't a function. >> + >> + >> +/* >> + * copy src to dest, skipping leading and trailing blanks and null >> + * terminate the string >> + * "len" is the size of available memory including the terminating '\0' >> + */ >> +static void mg_ident_cpy (unsigned char *dst, unsigned char *src, >> + unsigned int len) >> +{ >> + unsigned char *end, *last; >> + >> + last = dst; >> + end = src + len - 1; >> + >> + /* reserve space for '\0' */ >> + if (len < 2) >> + goto OUT; >> + >> + /* skip leading white space */ >> + while ((*src) && (src<end) && (*src==' ')) > please add a space before and after '<' '==' & co >> + ++src; >> + >> + /* copy string, omitting trailing white space */ >> + while ((*src) && (src<end)) { >> + *dst++ = *src; >> + if (*src++ != ' ') >> + last = dst; >> + } >> +OUT: >> + *last = '\0'; >> +} > why do you need this? for parsing string parts of ATAID >> +static unsigned int mg_do_read_sects(void *buff, u32 sect_num, u32 sect_cnt) >> +{ >> + u32 i, j, err; >> + u8 *buff_ptr = buff; >> + >> + err = mg_out(sect_num, sect_cnt, MG_CMD_RD); >> + if (err) >> + return err; >> + >> + for (i = 0; i < sect_cnt; i++) { >> + err = mg_wait(MG_REG_STATUS_BIT_DATA_REQ, 3000); >> + if (err) >> + return err; >> + >> + /* TODO : u16 unaligned case */ >> + for(j = 0; j < MG_SECTOR_SIZE >> 1; j++) { >> + *(u16 *)buff_ptr = >> + readw(MG_BASE + MG_BUFF_OFFSET + (j << 1)); > ?? I can't guess your intention. Please write more clearly >> + buff_ptr += 2; >> + } >> + >> + writeb(MG_CMD_RD_CONF, MG_BASE + MG_REG_COMMAND); >> + >> + MG_DBG("%u (0x%8.8x) sector read", sect_num + i, >> + (sect_num + i) * MG_SECTOR_SIZE); >> + } >> + >> + return err; >> +} >> +unsigned int mg_disk_read_sects(void *buff, u32 sect_num, u32 sect_cnt) >> +{ >> + u32 quotient, residue, i, err; >> + u8 *buff_ptr = buff; >> + >> + quotient = sect_cnt >> 8; >> + residue = sect_cnt % 256; >> + >> + for (i = 0; i < quotient; i++) { >> + MG_DBG("sect num : %u buff : 0x%8.8x", sect_num, (u32)buff_ptr); >> + err = mg_do_read_sects(buff_ptr, sect_num, 256); >> + if (err) >> + return err; >> + sect_num += 256; >> + buff_ptr += 256 * MG_SECTOR_SIZE; >> + } >> + >> + if (residue) { >> + MG_DBG("sect num : %u buff : %8.8x", sect_num, (u32)buff_ptr); > please use debug(x) MG_DBG prints lots of messages. I think changing MG_DBG to debug might confuse other driver's debug message. If u-boot policy only allow debug(x), I'll follow. >> + err = mg_do_read_sects(buff_ptr, sect_num, residue); >> + } >> + >> + return err; >> +} >> +/* must override this function */ >> +struct mg_drv_data * __attribute__((weak)) mg_get_drv_data (void) >> +{ >> + puts ("### WARNING ### port mg_get_drv_data function\n"); >> + return NULL; >> +} > please do an compile error not a run time error IMHO, compile error (or warning) is not a good choice for this case. Compile time error always generate error even though override function exist. Also mg_disk_init() function will generate run time error when weak function used. Typically, users will be read README.mflash that explains how to override this function and finally port appropriately. Regards, unsik Kim _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot