Hi Sean,
On Sat, 20 Jul 2024 at 16:44, Sean Anderson seanga2@gmail.com wrote:
On 7/20/24 02:16, Simon Glass wrote:
The code makes quite a few uses of __func__ which puts the function name into the resulting SPL image. Use the log subsystem instead, to reduce size.
The CONFIG_LOGF_FUNC option can be used to enable the function name.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/mmc/sdhci.c | 46 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 560b7e889c7..24998233543 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -32,8 +32,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask) sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET); while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) { if (timeout == 0) {
printf("%s: Reset 0x%x never completed.\n",__func__, (int)mask);
log_warning("Reset %x never completed\n", (int)mask);I think this cast can be removed too
return; } timeout--;@@ -139,8 +138,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data) do { stat = sdhci_readl(host, SDHCI_INT_STATUS); if (stat & SDHCI_INT_ERROR) {
pr_debug("%s: Error detected in status(0x%X)!\n",__func__, stat);
log_debug("Error detected in status(%x)!\n", stat); return -EIO; } if (!transfer_done && (stat & rdy)) {@@ -173,7 +171,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data) if (timeout-- > 0) udelay(10); else {
printf("%s: Transfer data timeout\n", __func__);
log_err("Transfer data timeout\n"); return -ETIMEDOUT; } } while (!(stat & SDHCI_INT_DATA_END));@@ -232,7 +230,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) { if (time >= cmd_timeout) {
printf("%s: MMC: %d busy ", __func__, mmc_dev);
log_warning("MMC: %d busy ", mmc_dev);Maybe this should be "mmc%d busy\n"
if (2 * cmd_timeout <= SDHCI_CMD_MAX_TIMEOUT) { cmd_timeout += cmd_timeout; printf("timeout increasing to: %u ms.\n",@@ -316,8 +314,8 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, }
if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {
printf("%s: Timeout for status update: %08x %08x\n",__func__, stat, mask);
log_warning("Timeout for status update: %08x %08x\n",stat, mask); return -ETIMEDOUT; } } while ((stat & mask) != mask);@@ -358,7 +356,7 @@ static int sdhci_execute_tuning(struct udevice *dev, uint opcode) struct mmc *mmc = mmc_get_mmc_dev(dev); struct sdhci_host *host = mmc->priv;
debug("%s\n", __func__);
log_debug("tuning\n");I think this one should be left as-is. Or at least given something more descriptive.
I'll add sdhci in there.
if (host->ops && host->ops->platform_execute_tuning) { err = host->ops->platform_execute_tuning(mmc, opcode);@@ -380,8 +378,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock) while (sdhci_readl(host, SDHCI_PRESENT_STATE) & (SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT)) { if (timeout == 0) {
printf("%s: Timeout to wait cmd & data inhibit\n",__func__);
log_err("Timeout to wait cmd & data inhibit\n");Might as well fix the grammar while we're at it
log_err("Timeout waiting for cmd & data inhibit\n");
return -EBUSY; }@@ -397,7 +394,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock) if (host->ops && host->ops->set_delay) { ret = host->ops->set_delay(host); if (ret) {
printf("%s: Error while setting tap delay\n", __func__);
log_err("Error while setting tap delay\n"); return ret; } }@@ -405,7 +402,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock) if (host->ops && host->ops->config_dll) { ret = host->ops->config_dll(host, clock, false); if (ret) {
printf("%s: Error while configuring dll\n", __func__);
log_err("Error configuring dll\n");To distinguish these, can we say "Error while disabling dll"
return ret; } }@@ -456,7 +453,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock) if (host->ops && host->ops->config_dll) { ret = host->ops->config_dll(host, clock, true); if (ret) {
printf("%s: Error while configuring dll\n", __func__);
log_err("Error configuring dll\n");and "Error while enabling dll"
I know this grows .data a bit...
I'll drop the 'while' since error messages mostly assume that.
return ret; } }@@ -472,8 +469,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock) while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL)) & SDHCI_CLOCK_INT_STABLE)) { if (timeout == 0) {
printf("%s: Internal clock never stabilised.\n",__func__);
log_err("Internal clock never stabilised.\n"); return -EBUSY; } timeout--;@@ -738,8 +734,7 @@ static int sdhci_init(struct mmc *mmc) if (host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) { host->align_buffer = memalign(8, 512 * 1024); if (!host->align_buffer) {
printf("%s: Aligned buffer alloc failed!!!\n",__func__);
log_err("Aligned buffer alloc failed\n"); return -ENOMEM; } }@@ -881,20 +876,18 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, #else caps = sdhci_readl(host, SDHCI_CAPABILITIES); #endif
debug("%s, caps: 0x%x\n", __func__, caps);
log_debug("caps: 0x%x\n", caps);#if CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) if ((caps & SDHCI_CAN_DO_SDMA)) { host->flags |= USE_SDMA; } else {
debug("%s: Your controller doesn't support SDMA!!\n",__func__);
log_debug("Controller doesn't support SDMA\n"); }
printf("%s: Your controller doesn't support ADMA!!\n",__func__);
log_err("Controller doesn't support ADMA\n"); return -EINVAL; } if (!host->adma_desc_table) {@@ -927,7 +920,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, #else caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1); #endif
debug("%s, caps_1: 0x%x\n", __func__, caps_1);
log_debug("caps_1: %x\n", caps_1); host->clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;@@ -953,8 +946,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, host->max_clk *= host->clk_mul; } if (host->max_clk == 0) {
printf("%s: Hardware doesn't specify base clock frequency\n",__func__);
log_err("Hardware doesn't specify base clock frequency\n"); return -EINVAL; } if (f_max && (f_max < host->max_clk))@@ -1047,7 +1039,7 @@ int add_sdhci(struct sdhci_host *host, u32 f_max, u32 f_min)
host->mmc = mmc_create(&host->cfg, host); if (host->mmc == NULL) {
printf("%s: mmc create fail!\n", __func__);
log_err("mmc create fail\n"); return -ENOMEM; }
Regards, Simon