Hi Paul
On Sep 6, 2013, at 3:51 PM, Paul Burton wrote:
>
> On 06/09/13 13:48, Pantelis Antoniou wrote:
>> Hi Paul
>>
>> On Sep 4, 2013, at 6:12 PM, Paul Burton wrote:
>>
>>> If we don't have CONFIG_SPL_LIBCOMMON_SUPPORT defined then stdio
>>> & *printf functions are unavailable & calling them will cause a link
>>> failure.
>>>
>>> Signed-off-by: Paul Burton <paul.bur...@imgtec.com>
>>> ---
>>> drivers/mmc/mmc.c | 36 ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 36 insertions(+)
>>>
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 5502675..30a985b 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -135,8 +135,10 @@ static int mmc_send_status(struct mmc *mmc, int
>>> timeout)
>>> MMC_STATE_PRG)
>>> break;
>>> else if (cmd.response[0] & MMC_STATUS_MASK) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("Status Error: 0x%08X\n",
>>> cmd.response[0]);
>>> +#endif
>>> return COMM_ERR;
>>> }
>>> } else if (--retries < 0)
>>> @@ -151,7 +153,9 @@ static int mmc_send_status(struct mmc *mmc, int timeout)
>>> printf("CURR STATE:%d\n", status);
>>> #endif
>>> if (timeout <= 0) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("Timeout waiting card ready\n");
>>> +#endif
>>> return TIMEOUT;
>>> }
>>>
>>> @@ -181,7 +185,9 @@ struct mmc *find_mmc_device(int dev_num)
>>> return m;
>>> }
>>>
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("MMC Device %d not found\n", dev_num);
>>> +#endif
>>>
>>> return NULL;
>>> }
>>> @@ -233,7 +239,9 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start,
>>> lbaint_t blkcnt)
>>> return 0;
>>>
>>> err_out:
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> puts("mmc erase failed\n");
>>> +#endif
>>> return err;
>>> }
>>>
>>> @@ -248,6 +256,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
>>> if (!mmc)
>>> return -1;
>>>
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> if ((start % mmc->erase_grp_size) || (blkcnt % mmc->erase_grp_size))
>>> printf("\n\nCaution! Your devices Erase group is 0x%x\n"
>>> "The erase range would be change to "
>>> @@ -255,6 +264,7 @@ mmc_berase(int dev_num, lbaint_t start, lbaint_t blkcnt)
>>> mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
>>> ((start + blkcnt + mmc->erase_grp_size)
>>> & ~(mmc->erase_grp_size - 1)) - 1);
>>> +#endif
>>>
>>> while (blk < blkcnt) {
>>> blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
>>> @@ -281,8 +291,10 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>>> lbaint_t blkcnt, const void*sr
>>> int timeout = 1000;
>>>
>>> if ((start + blkcnt) > mmc->block_dev.lba) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
>>> start + blkcnt, mmc->block_dev.lba);
>>> +#endif
>>> return 0;
>>> }
>>>
>>> @@ -306,7 +318,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>>> lbaint_t blkcnt, const void*sr
>>> data.flags = MMC_DATA_WRITE;
>>>
>>> if (mmc_send_cmd(mmc, &cmd, &data)) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("mmc write failed\n");
>>> +#endif
>>> return 0;
>>> }
>>>
>>> @@ -318,7 +332,9 @@ mmc_write_blocks(struct mmc *mmc, lbaint_t start,
>>> lbaint_t blkcnt, const void*sr
>>> cmd.cmdarg = 0;
>>> cmd.resp_type = MMC_RSP_R1b;
>>> if (mmc_send_cmd(mmc, &cmd, NULL)) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("mmc fail to send stop cmd\n");
>>> +#endif
>>> return 0;
>>> }
>>> }
>>> @@ -385,7 +401,9 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst,
>>> lbaint_t start,
>>> cmd.cmdarg = 0;
>>> cmd.resp_type = MMC_RSP_R1b;
>>> if (mmc_send_cmd(mmc, &cmd, NULL)) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("mmc fail to send stop cmd\n");
>>> +#endif
>>> return 0;
>>> }
>>> }
>>> @@ -405,8 +423,10 @@ static ulong mmc_bread(int dev_num, lbaint_t start,
>>> lbaint_t blkcnt, void *dst)
>>> return 0;
>>>
>>> if ((start + blkcnt) > mmc->block_dev.lba) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("MMC: block number 0x" LBAF " exceeds max(0x" LBAF ")\n",
>>> start + blkcnt, mmc->block_dev.lba);
>>> +#endif
>>> return 0;
>>> }
>> The idea is sound, but I don't like peppering the source with #ifdefs here.
>>
>> Why not create a varargs orintf macro and use it instead
>>
>> i.e.
>>
>>
>> #if !defined(CONFIG_SPL_BUILD_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>> #define mmc_printf(...) ...
>> #else
>> #define mmc_printf(...) /* nothing */
>> #endif
> That would work too, I'm fine with changing to that if people prefer it.
>
>
Speaking of which this isn't really mmc specific, more like SPL specific;
perhaps what needs to
be done is have by default printf defined to empty on SPL and have an
foo_printf that always outputs.
Tom, what do you think?
>>> @@ -1268,6 +1288,7 @@ static int mmc_startup(struct mmc *mmc)
>>> mmc->block_dev.blksz = mmc->read_bl_len;
>>> mmc->block_dev.log2blksz = LOG2(mmc->block_dev.blksz);
>>> mmc->block_dev.lba = lldiv(mmc->capacity, mmc->read_bl_len);
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> sprintf(mmc->block_dev.vendor, "Man %06x Snr %04x%04x",
>>> mmc->cid[0] >> 24, (mmc->cid[2] & 0xffff),
>>> (mmc->cid[3] >> 16) & 0xffff);
>>> @@ -1277,6 +1298,11 @@ static int mmc_startup(struct mmc *mmc)
>>> (mmc->cid[2] >> 24) & 0xff);
>>> sprintf(mmc->block_dev.revision, "%d.%d", (mmc->cid[2] >> 20) & 0xf,
>>> (mmc->cid[2] >> 16) & 0xf);
>>> +#else
>>> + mmc->block_dev.vendor[0] = 0;
>>> + mmc->block_dev.product[0] = 0;
>>> + mmc->block_dev.revision[0] = 0;
>>> +#endif
>> ^^^
>> What goes on here?
> Thanks for pointing that out, perhaps I should comment on it in the source.
> Basically it needs to avoid sprintf, and since there's no possibility that
> the information will ever be displayed in this case (and it has no other uses
> than display) I figured it would be fine to just have empty strings.
>
Drop this please then, and put it in another patch if need be. I still don't
like it.
>>> #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)
>>> init_part(&mmc->block_dev);
>>> #endif
>>> @@ -1343,7 +1369,9 @@ int mmc_start_init(struct mmc *mmc)
>>>
>>> if (mmc_getcd(mmc) == 0) {
>>> mmc->has_init = 0;
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("MMC: no card present\n");
>>> +#endif
>>> return NO_CARD_ERR;
>>> }
>>>
>>> @@ -1378,7 +1406,9 @@ int mmc_start_init(struct mmc *mmc)
>>> err = mmc_send_op_cond(mmc);
>>>
>>> if (err && err != IN_PROGRESS) {
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> printf("Card did not respond to voltage select!\n");
>>> +#endif
>>> return UNUSABLE_ERR;
>>> }
>>> }
>>> @@ -1434,6 +1464,8 @@ static int __def_mmc_init(bd_t *bis)
>>> int cpu_mmc_init(bd_t *bis) __attribute__((weak, alias("__def_mmc_init")));
>>> int board_mmc_init(bd_t *bis) __attribute__((weak,
>>> alias("__def_mmc_init")));
>>>
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>> +
>>> void print_mmc_devices(char separator)
>>> {
>>> struct mmc *m;
>>> @@ -1451,6 +1483,10 @@ void print_mmc_devices(char separator)
>>> printf("\n");
>>> }
>>>
>>> +#else
>>> +void print_mmc_devices(char separator) { }
>>> +#endif
>>> +
>>> int get_mmc_num(void)
>>> {
>>> return cur_dev_num;
>>> --
>>> 1.8.3.4
>>>
>>>
>> CCing Tom Rini on this.
>>
>> Regards
>>
>> -- Pantelis
>>
>
> Thanks,
> Paul
>
Regards
-- Pantelis
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
