From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Rae Date: Thu, 29 May 2014 12:44:16 -0700 Subject: [U-Boot] [PATCH] mmc: add wrappers for MMC block_{read, write, erase} In-Reply-To: <538781B6.90703@wwwdotorg.org> References: <1401315346-30231-1-git-send-email-srae@broadcom.com> <53875F6D.5010708@wwwdotorg.org> <5387753E.9060106@broadcom.com> <538781B6.90703@wwwdotorg.org> Message-ID: <53878E10.4010609@broadcom.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 14-05-29 11:51 AM, Stephen Warren wrote: > On 05/29/2014 11:58 AM, Steve Rae wrote: >> Hi, Stephen >> >> On 14-05-29 09:25 AM, Stephen Warren wrote: >>> On 05/28/2014 04:15 PM, Steve Rae wrote: >>>> Each wrapper function: >>>> - switches to the specified physical partition, then >>>> - performs the original function, and then >>>> - switches back to the original physical partition >>>> where the physical partition (aka HW partition) is >>>> 0=User, 1=Boot1, 2=Boot2, etc. >>> >>> This feels wrong; why wouldn't mmc_get_dev() return a block_dev_desc_t >>> containing block_read/block_write functions that do the HW partition >>> switching. That way, this is all completely hidden, and all client code >>> only knows about block devices, rather than having to know about >>> MMC-specific mmc_block_read/write/erase_hwpart() functions. >>> >> This goes back to the initial discussion on this mailing list (which was >> never resolved): >> http://lists.denx.de/pipermail/u-boot/2014-April/178171.html >> This issue is that the three callback functions defined in >> 'block_desc_t' do not accept the "partition number" as an argument. >> It was suggested that we could overwrite those functions; but the >> "partition number" still needs to be passed in by: >> (1) overloading the "int dev_num" argument, or >> (2) adding another argument to the callback functions >> I assumed that neither of these was acceptable, so I have proposed these >> wrappers... > > Can't the data simply be stored in the block_desc_t itself? > If I understand this suggestion, are you proposing: - add an "unsigned int specified_hw_part" to the block_desc_t Then the usage would become: mmc->block_dev.specified_hw_part = 1; /* specify Boot1 partition */ mmc->block_dev.block_read(0, 0, 1, buf); /* read first block (from Boot1 partition) */ mmc->block_dev.specified_hw_part = 0; /* specify User partition */ mmc->block_dev.block_read(0, 0, 1, buf); /* read first block (from User partition) */ I don't think this is a good idea... Please clarify! Thanks, Steve