| Message ID | 20201111100244.15823-2-wsa+renesas@sang-engineering.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] mmc: renesas_sdhi: improve HOST_MODE usage | expand |
Hi Wolfram, Thanks for your patch. On 2020-11-11 11:02:42 +0100, Wolfram Sang wrote: > HOST_MODE should have a CTL_ prefix, too. This makes the code more > readable because we immediately know what it is. Also, remove the > hardcoded values with something readable, too. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/mmc/host/renesas_sdhi_core.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index 25c6a1993f8e..b3eb0182c4af 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -39,7 +39,15 @@ > #include "renesas_sdhi.h" > #include "tmio_mmc.h" > > -#define HOST_MODE 0xe4 > +#define CTL_HOST_MODE 0xe4 > +#define HOST_MODE_GEN2_SDR50_WMODE BIT(0) > +#define HOST_MODE_GEN2_SDR104_WMODE BIT(0) > +#define HOST_MODE_GEN3_WMODE BIT(0) > +#define HOST_MODE_GEN3_BUSWIDTH BIT(8) > + > +#define HOST_MODE_GEN3_16BIT HOST_MODE_GEN3_WMODE > +#define HOST_MODE_GEN3_32BIT (HOST_MODE_GEN3_WMODE | HOST_MODE_GEN3_BUSWIDTH) > +#define HOST_MODE_GEN3_64BIT 0 > > #define SDHI_VER_GEN2_SDR50 0x490c > #define SDHI_VER_RZ_A1 0x820b > @@ -60,26 +68,26 @@ static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width) > */ > switch (sd_ctrl_read16(host, CTL_VERSION)) { > case SDHI_VER_GEN2_SDR50: > - val = (width == 32) ? 0x0001 : 0x0000; > + val = (width == 32) ? HOST_MODE_GEN2_SDR50_WMODE : 0; > break; > case SDHI_VER_GEN2_SDR104: > - val = (width == 32) ? 0x0000 : 0x0001; > + val = (width == 32) ? 0 : HOST_MODE_GEN2_SDR104_WMODE; > break; > case SDHI_VER_GEN3_SD: > case SDHI_VER_GEN3_SDMMC: > if (width == 64) > - val = 0x0000; > + val = HOST_MODE_GEN3_64BIT; > else if (width == 32) > - val = 0x0101; > + val = HOST_MODE_GEN3_32BIT; > else > - val = 0x0001; > + val = HOST_MODE_GEN3_16BIT; > break; > default: > /* nothing to do */ > return; > } > > - sd_ctrl_write16(host, HOST_MODE, val); > + sd_ctrl_write16(host, CTL_HOST_MODE, val); > } > > static int renesas_sdhi_clk_enable(struct tmio_mmc_host *host) > @@ -795,7 +803,7 @@ static int renesas_sdhi_write16_hook(struct tmio_mmc_host *host, int addr) > case CTL_SD_MEM_CARD_OPT: > case CTL_TRANSACTION_CTL: > case CTL_DMA_ENABLE: > - case HOST_MODE: > + case CTL_HOST_MODE: > if (host->pdata->flags & TMIO_MMC_HAVE_CBSY) > bit = TMIO_STAT_CMD_BUSY; > fallthrough; > -- > 2.28.0 > -- Regards, Niklas Söderlund
Hi Wolfram-san, > From: Wolfram Sang, Sent: Wednesday, November 11, 2020 7:03 PM > > HOST_MODE should have a CTL_ prefix, too. This makes the code more > readable because we immediately know what it is. Also, remove the > hardcoded values with something readable, too. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Thank you for the patch! Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 25c6a1993f8e..b3eb0182c4af 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -39,7 +39,15 @@ #include "renesas_sdhi.h" #include "tmio_mmc.h" -#define HOST_MODE 0xe4 +#define CTL_HOST_MODE 0xe4 +#define HOST_MODE_GEN2_SDR50_WMODE BIT(0) +#define HOST_MODE_GEN2_SDR104_WMODE BIT(0) +#define HOST_MODE_GEN3_WMODE BIT(0) +#define HOST_MODE_GEN3_BUSWIDTH BIT(8) + +#define HOST_MODE_GEN3_16BIT HOST_MODE_GEN3_WMODE +#define HOST_MODE_GEN3_32BIT (HOST_MODE_GEN3_WMODE | HOST_MODE_GEN3_BUSWIDTH) +#define HOST_MODE_GEN3_64BIT 0 #define SDHI_VER_GEN2_SDR50 0x490c #define SDHI_VER_RZ_A1 0x820b @@ -60,26 +68,26 @@ static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width) */ switch (sd_ctrl_read16(host, CTL_VERSION)) { case SDHI_VER_GEN2_SDR50: - val = (width == 32) ? 0x0001 : 0x0000; + val = (width == 32) ? HOST_MODE_GEN2_SDR50_WMODE : 0; break; case SDHI_VER_GEN2_SDR104: - val = (width == 32) ? 0x0000 : 0x0001; + val = (width == 32) ? 0 : HOST_MODE_GEN2_SDR104_WMODE; break; case SDHI_VER_GEN3_SD: case SDHI_VER_GEN3_SDMMC: if (width == 64) - val = 0x0000; + val = HOST_MODE_GEN3_64BIT; else if (width == 32) - val = 0x0101; + val = HOST_MODE_GEN3_32BIT; else - val = 0x0001; + val = HOST_MODE_GEN3_16BIT; break; default: /* nothing to do */ return; } - sd_ctrl_write16(host, HOST_MODE, val); + sd_ctrl_write16(host, CTL_HOST_MODE, val); } static int renesas_sdhi_clk_enable(struct tmio_mmc_host *host) @@ -795,7 +803,7 @@ static int renesas_sdhi_write16_hook(struct tmio_mmc_host *host, int addr) case CTL_SD_MEM_CARD_OPT: case CTL_TRANSACTION_CTL: case CTL_DMA_ENABLE: - case HOST_MODE: + case CTL_HOST_MODE: if (host->pdata->flags & TMIO_MMC_HAVE_CBSY) bit = TMIO_STAT_CMD_BUSY; fallthrough;
HOST_MODE should have a CTL_ prefix, too. This makes the code more readable because we immediately know what it is. Also, remove the hardcoded values with something readable, too. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- drivers/mmc/host/renesas_sdhi_core.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)