Hi Quentin,
On Tue, 6 Aug 2024 at 08:10, Quentin Schulz <quentin.sch...@cherry.de> wrote:
>
> Hi Simon,
>
> On 7/21/24 5:25 PM, Simon Glass wrote:
> > Use the log subsystem instead of dev, to avoid including function names
> > in the code.
> >
> > The CONFIG_LOGF_FUNC option can be used to enable the function name.
> >
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > Reviewed-by: Sean Anderson <sean...@gmail.com>
> > ---
> >
> > (no changes since v1)
> >
> > drivers/mmc/mmc.c | 49 ++++++++++++++++++++++++-----------------------
> > 1 file changed, 25 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index b18dc331f78..b0105afe5d6 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -294,7 +294,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
> >
> > if (status & MMC_STATUS_MASK) {
> > #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> > - pr_err("Status Error: 0x%08x\n", status);
> > + log_err("Status Error: %08x\n", status);
>
> Please don't remove the 0x prefix (will not comment on other patches in
> this series if it happens again, please check :) ).
OK...hex is the default though.
>
> > #endif
> > return -ECOMM;
> > }
> > @@ -307,7 +307,7 @@ int mmc_poll_for_busy(struct mmc *mmc, int timeout_ms)
> >
> > if (timeout_ms <= 0) {
> > #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> > - pr_err("Timeout waiting card ready\n");
> > + log_err("Timeout waiting card ready\n");
> > #endif
> > return -ETIMEDOUT;
> > }
> > @@ -449,7 +449,7 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst,
> > lbaint_t start,
> > if (blkcnt > 1) {
> > if (mmc_send_stop_transmission(mmc, false)) {
> > #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> > - pr_err("mmc fail to send stop cmd\n");
> > + log_err("mmc fail to send stop cmd\n");
> > #endif
> > return 0;
> > }
> > @@ -500,8 +500,8 @@ ulong mmc_bread(struct blk_desc *block_dev, lbaint_t
> > start, lbaint_t blkcnt,
> >
> > if ((start + blkcnt) > block_dev->lba) {
> > #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> > - pr_err("MMC: block number 0x" LBAF " exceeds max(0x" LBAF
> > ")\n",
> > - start + blkcnt, block_dev->lba);
> > + log_err("MMC: block number " LBAF " exceeds max(" LBAF ")\n",
>
> DOn't remove the 0x prefix please.
>
> > + start + blkcnt, block_dev->lba);
> > #endif
> > return 0;
> > }
> > @@ -997,7 +997,7 @@ static int mmc_get_capabilities(struct mmc *mmc)
> > return 0;
> >
> > if (!ext_csd) {
> > - pr_err("No ext_csd found!\n"); /* this should enver happen */
> > + log_err("No ext_csd found!\n"); /* this should enver happen */
> > return -ENOTSUPP;
> > }
> >
> > @@ -1109,17 +1109,17 @@ int mmc_hwpart_config(struct mmc *mmc,
> > return -EINVAL;
> >
> > if (IS_SD(mmc) || (mmc->version < MMC_VERSION_4_41)) {
> > - pr_err("eMMC >= 4.4 required for enhanced user data area\n");
> > + log_err("eMMC >= 4.4 required for enhanced user data area\n");
> > return -EMEDIUMTYPE;
> > }
> >
> > if (!(mmc->part_support & PART_SUPPORT)) {
> > - pr_err("Card does not support partitioning\n");
> > + log_err("Card does not support partitioning\n");
> > return -EMEDIUMTYPE;
> > }
> >
> > if (!mmc->hc_wp_grp_size) {
> > - pr_err("Card does not define HC WP group size\n");
> > + log_err("Card does not define HC WP group size\n");
> > return -EMEDIUMTYPE;
> > }
> >
> > @@ -1127,8 +1127,7 @@ int mmc_hwpart_config(struct mmc *mmc,
> > if (conf->user.enh_size) {
> > if (conf->user.enh_size % mmc->hc_wp_grp_size ||
> > conf->user.enh_start % mmc->hc_wp_grp_size) {
> > - pr_err("User data enhanced area not HC WP group "
> > - "size aligned\n");
> > + log_err("User data enhanced area not HC WP group size
> > aligned\n");
> > return -EINVAL;
> > }
> > part_attrs |= EXT_CSD_ENH_USR;
> > @@ -1146,8 +1145,8 @@ int mmc_hwpart_config(struct mmc *mmc,
> >
> > for (pidx = 0; pidx < 4; pidx++) {
> > if (conf->gp_part[pidx].size % mmc->hc_wp_grp_size) {
> > - pr_err("GP%i partition not HC WP group size "
> > - "aligned\n", pidx+1);
> > + log_err("GP%i partition not HC WP group-size
> > aligned\n",
> > + pidx + 1);
> > return -EINVAL;
> > }
> > gp_size_mult[pidx] = conf->gp_part[pidx].size /
> > mmc->hc_wp_grp_size;
> > @@ -1158,7 +1157,7 @@ int mmc_hwpart_config(struct mmc *mmc,
> > }
> >
> > if (part_attrs && ! (mmc->part_support & ENHNCD_SUPPORT)) {
> > - pr_err("Card does not support enhanced attribute\n");
> > + log_err("Card does not support enhanced attribute\n");
> > return -EMEDIUMTYPE;
> > }
> >
> > @@ -1171,8 +1170,8 @@ int mmc_hwpart_config(struct mmc *mmc,
> > (ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT+1] << 8) +
> > ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT];
> > if (tot_enh_size_mult > max_enh_size_mult) {
> > - pr_err("Total enhanced size exceeds maximum (%u > %u)\n",
> > - tot_enh_size_mult, max_enh_size_mult);
> > + log_err("Total enhanced size exceeds maximum (%x > %x)\n",
>
> If you change the type (why?) to hex, please add 0x prefix to explicit
> the base.
>
> > + tot_enh_size_mult, max_enh_size_mult);
> > return -EMEDIUMTYPE;
> > }
> >
> > @@ -1205,7 +1204,7 @@ int mmc_hwpart_config(struct mmc *mmc,
> >
> > if (ext_csd[EXT_CSD_PARTITION_SETTING] &
> > EXT_CSD_PARTITION_SETTING_COMPLETED) {
> > - pr_err("Card already partitioned\n");
> > + log_err("Card already partitioned\n");
> > return -EPERM;
> > }
> >
> > @@ -1876,7 +1875,7 @@ error:
> > }
> > }
> >
> > - pr_err("unable to select a mode\n");
> > + log_err("unable to select a mode\n");
> > return -ENOTSUPP;
> > }
> >
> > @@ -2254,7 +2253,7 @@ error:
> > }
> > }
> >
> > - pr_err("unable to select a mode : %d\n", err);
> > + log_err("unable to select a mode: %d\n", err);
> >
> > return -ENOTSUPP;
> > }
> > @@ -2921,8 +2920,10 @@ retry:
> >
> > if (err) {
> > #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> > - if (!quiet)
> > - pr_err("Card did not respond to voltage
> > select! : %d\n", err);
> > + if (!quiet) {
> > + log_err("Card did not respond to voltage
> > select! : %d\n",
> > + err);
> > + }
>
> We still don't need the curly brackets here.
>
> Looking good otherwise,
Regards,
Simon
