[PATCH v2 1/2] mmc: tmio: Check 'addr' width before checking for 64bit limitation

The 64bit limitation check is compiled and optimized out on 32bit platforms, but generates a type width warning:
drivers/mmc/tmio-common.c: In function ‘tmio_sd_addr_is_dmaable’: drivers/mmc/tmio-common.c:376:26: warning: right shift count >= width of type [-Wshift-count-overflow] 376 | if (addr >> 32) | ^~
Fix the warning by checking the addr type width to see whether the shift even makes sense in the first place. The width check is also optimized out at compile time.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com --- V2: New patch --- drivers/mmc/tmio-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c index e9c7d3a2e00..0b24a5a7bdb 100644 --- a/drivers/mmc/tmio-common.c +++ b/drivers/mmc/tmio-common.c @@ -373,7 +373,7 @@ static bool tmio_sd_addr_is_dmaable(struct mmc_data *data) if (!(data->flags & MMC_DATA_READ) && !IS_ALIGNED(addr, 128)) return false; /* Gen3 DMA has 32bit limit */ - if (addr >> 32) + if (sizeof(addr) > 4 && addr >> 32) return false; #endif

Instead of #if and #ifdef, use IS_ENABLED and CONFIG_IS_ENABLED macros. This improves build test coverage. The CONFIG_SPL_BUILD must remain an ifdef, as CONFIG_SPL_STACK may not always be defined, e.g. in U-Boot proper build. No functional change.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com --- V2: - Rebase on 1/2 - Drop DM_REGULATOR check from struct tmio_sd_priv in tmio-common.h --- drivers/mmc/tmio-common.c | 59 +++++++++++++++++++-------------------- drivers/mmc/tmio-common.h | 2 -- 2 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c index 0b24a5a7bdb..d8b6a4a8821 100644 --- a/drivers/mmc/tmio-common.c +++ b/drivers/mmc/tmio-common.c @@ -369,22 +369,23 @@ static bool tmio_sd_addr_is_dmaable(struct mmc_data *data) if (!IS_ALIGNED(addr, TMIO_SD_DMA_MINALIGN)) return false;
-#if defined(CONFIG_RCAR_GEN3) - if (!(data->flags & MMC_DATA_READ) && !IS_ALIGNED(addr, 128)) - return false; - /* Gen3 DMA has 32bit limit */ - if (sizeof(addr) > 4 && addr >> 32) - return false; -#endif + if (IS_ENABLED(CONFIG_RCAR_GEN3)) { + if (!(data->flags & MMC_DATA_READ) && !IS_ALIGNED(addr, 128)) + return false; + /* Gen3 DMA has 32bit limit */ + if (sizeof(addr) > 4 && addr >> 32) + return false; + }
-#if defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARM64) && \ - defined(CONFIG_SPL_BUILD) - /* - * For UniPhier ARMv7 SoCs, the stack is allocated in the locked ways - * of L2, which is unreachable from the DMA engine. - */ - if (addr < CONFIG_SPL_STACK) - return false; +#ifdef CONFIG_SPL_BUILD + if (IS_ENABLED(CONFIG_ARCH_UNIPHIER) && !CONFIG_IS_ENABLED(CONFIG_ARM64)) { + /* + * For UniPhier ARMv7 SoCs, the stack is allocated in locked + * ways of L2, which is unreachable from the DMA engine. + */ + if (addr < CONFIG_SPL_STACK) + return false; + } #endif
return true; @@ -622,25 +623,22 @@ static void tmio_sd_set_clk_rate(struct tmio_sd_priv *priv, struct mmc *mmc) static void tmio_sd_set_pins(struct udevice *dev) { __maybe_unused struct mmc *mmc = mmc_get_mmc_dev(dev); - -#ifdef CONFIG_DM_REGULATOR struct tmio_sd_priv *priv = dev_get_priv(dev);
- if (priv->vqmmc_dev) { + if (CONFIG_IS_ENABLED(DM_REGULATOR) && priv->vqmmc_dev) { if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180) regulator_set_value(priv->vqmmc_dev, 1800000); else regulator_set_value(priv->vqmmc_dev, 3300000); regulator_set_enable(priv->vqmmc_dev, true); } -#endif
-#ifdef CONFIG_PINCTRL - if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180) - pinctrl_select_state(dev, "state_uhs"); - else - pinctrl_select_state(dev, "default"); -#endif + if (CONFIG_IS_ENABLED(PINCTRL)) { + if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180) + pinctrl_select_state(dev, "state_uhs"); + else + pinctrl_select_state(dev, "default"); + } }
int tmio_sd_set_ios(struct udevice *dev) @@ -734,11 +732,12 @@ int tmio_sd_probe(struct udevice *dev, u32 quirks) if (!priv->regbase) return -ENOMEM;
-#ifdef CONFIG_DM_REGULATOR - device_get_supply_regulator(dev, "vqmmc-supply", &priv->vqmmc_dev); - if (priv->vqmmc_dev) - regulator_set_value(priv->vqmmc_dev, 3300000); -#endif + if (CONFIG_IS_ENABLED(DM_REGULATOR)) { + device_get_supply_regulator(dev, "vqmmc-supply", + &priv->vqmmc_dev); + if (priv->vqmmc_dev) + regulator_set_value(priv->vqmmc_dev, 3300000); + }
ret = mmc_of_parse(dev, &plat->cfg); if (ret < 0) { diff --git a/drivers/mmc/tmio-common.h b/drivers/mmc/tmio-common.h index 4d717d85dec..f489fb70766 100644 --- a/drivers/mmc/tmio-common.h +++ b/drivers/mmc/tmio-common.h @@ -133,9 +133,7 @@ struct tmio_sd_priv { #define TMIO_SD_CAP_RCAR_UHS BIT(7) /* Renesas RCar UHS/SDR modes */ #define TMIO_SD_CAP_RCAR \ (TMIO_SD_CAP_RCAR_GEN2 | TMIO_SD_CAP_RCAR_GEN3) -#ifdef CONFIG_DM_REGULATOR struct udevice *vqmmc_dev; -#endif #if CONFIG_IS_ENABLED(CLK) struct clk clk; struct clk clkh;

On Tue, 28 Feb 2023 at 14:18, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
Instead of #if and #ifdef, use IS_ENABLED and CONFIG_IS_ENABLED macros. This improves build test coverage. The CONFIG_SPL_BUILD must remain an ifdef, as CONFIG_SPL_STACK may not always be defined, e.g. in U-Boot proper build. No functional change.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
V2: - Rebase on 1/2 - Drop DM_REGULATOR check from struct tmio_sd_priv in tmio-common.h
drivers/mmc/tmio-common.c | 59 +++++++++++++++++++-------------------- drivers/mmc/tmio-common.h | 2 -- 2 files changed, 29 insertions(+), 32 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 3/1/23 06:18, Marek Vasut wrote:
Instead of #if and #ifdef, use IS_ENABLED and CONFIG_IS_ENABLED macros. This improves build test coverage. The CONFIG_SPL_BUILD must remain an ifdef, as CONFIG_SPL_STACK may not always be defined, e.g. in U-Boot proper build. No functional change.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
V2: - Rebase on 1/2 - Drop DM_REGULATOR check from struct tmio_sd_priv in tmio-common.h
drivers/mmc/tmio-common.c | 59 +++++++++++++++++++-------------------- drivers/mmc/tmio-common.h | 2 -- 2 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c index 0b24a5a7bdb..d8b6a4a8821 100644 --- a/drivers/mmc/tmio-common.c +++ b/drivers/mmc/tmio-common.c @@ -369,22 +369,23 @@ static bool tmio_sd_addr_is_dmaable(struct mmc_data *data) if (!IS_ALIGNED(addr, TMIO_SD_DMA_MINALIGN)) return false;
-#if defined(CONFIG_RCAR_GEN3)
- if (!(data->flags & MMC_DATA_READ) && !IS_ALIGNED(addr, 128))
return false;
- /* Gen3 DMA has 32bit limit */
- if (sizeof(addr) > 4 && addr >> 32)
return false;
-#endif
- if (IS_ENABLED(CONFIG_RCAR_GEN3)) {
if (!(data->flags & MMC_DATA_READ) && !IS_ALIGNED(addr, 128))
return false;
/* Gen3 DMA has 32bit limit */
if (sizeof(addr) > 4 && addr >> 32)
return false;
- }
-#if defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARM64) && \
- defined(CONFIG_SPL_BUILD)
- /*
* For UniPhier ARMv7 SoCs, the stack is allocated in the locked ways
* of L2, which is unreachable from the DMA engine.
*/
- if (addr < CONFIG_SPL_STACK)
return false;
+#ifdef CONFIG_SPL_BUILD
- if (IS_ENABLED(CONFIG_ARCH_UNIPHIER) && !CONFIG_IS_ENABLED(CONFIG_ARM64)) {
/*
* For UniPhier ARMv7 SoCs, the stack is allocated in locked
* ways of L2, which is unreachable from the DMA engine.
*/
if (addr < CONFIG_SPL_STACK)
return false;
- }
#endif
return true; @@ -622,25 +623,22 @@ static void tmio_sd_set_clk_rate(struct tmio_sd_priv *priv, struct mmc *mmc) static void tmio_sd_set_pins(struct udevice *dev) { __maybe_unused struct mmc *mmc = mmc_get_mmc_dev(dev);
-#ifdef CONFIG_DM_REGULATOR struct tmio_sd_priv *priv = dev_get_priv(dev);
- if (priv->vqmmc_dev) {
- if (CONFIG_IS_ENABLED(DM_REGULATOR) && priv->vqmmc_dev) { if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180) regulator_set_value(priv->vqmmc_dev, 1800000); else regulator_set_value(priv->vqmmc_dev, 3300000); regulator_set_enable(priv->vqmmc_dev, true); }
-#endif
-#ifdef CONFIG_PINCTRL
- if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180)
pinctrl_select_state(dev, "state_uhs");
- else
pinctrl_select_state(dev, "default");
-#endif
- if (CONFIG_IS_ENABLED(PINCTRL)) {
if (mmc->signal_voltage == MMC_SIGNAL_VOLTAGE_180)
pinctrl_select_state(dev, "state_uhs");
else
pinctrl_select_state(dev, "default");
- }
}
int tmio_sd_set_ios(struct udevice *dev) @@ -734,11 +732,12 @@ int tmio_sd_probe(struct udevice *dev, u32 quirks) if (!priv->regbase) return -ENOMEM;
-#ifdef CONFIG_DM_REGULATOR
- device_get_supply_regulator(dev, "vqmmc-supply", &priv->vqmmc_dev);
- if (priv->vqmmc_dev)
regulator_set_value(priv->vqmmc_dev, 3300000);
-#endif
if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
device_get_supply_regulator(dev, "vqmmc-supply",
&priv->vqmmc_dev);
if (priv->vqmmc_dev)
regulator_set_value(priv->vqmmc_dev, 3300000);
}
ret = mmc_of_parse(dev, &plat->cfg); if (ret < 0) {
diff --git a/drivers/mmc/tmio-common.h b/drivers/mmc/tmio-common.h index 4d717d85dec..f489fb70766 100644 --- a/drivers/mmc/tmio-common.h +++ b/drivers/mmc/tmio-common.h @@ -133,9 +133,7 @@ struct tmio_sd_priv { #define TMIO_SD_CAP_RCAR_UHS BIT(7) /* Renesas RCar UHS/SDR modes */ #define TMIO_SD_CAP_RCAR \ (TMIO_SD_CAP_RCAR_GEN2 | TMIO_SD_CAP_RCAR_GEN3) -#ifdef CONFIG_DM_REGULATOR struct udevice *vqmmc_dev; -#endif #if CONFIG_IS_ENABLED(CLK) struct clk clk; struct clk clkh;

On 3/1/23 06:18, Marek Vasut wrote:
The 64bit limitation check is compiled and optimized out on 32bit platforms, but generates a type width warning:
drivers/mmc/tmio-common.c: In function ‘tmio_sd_addr_is_dmaable’: drivers/mmc/tmio-common.c:376:26: warning: right shift count >= width of type [-Wshift-count-overflow] 376 | if (addr >> 32) | ^~
Fix the warning by checking the addr type width to see whether the shift even makes sense in the first place. The width check is also optimized out at compile time.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
V2: New patch
drivers/mmc/tmio-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c index e9c7d3a2e00..0b24a5a7bdb 100644 --- a/drivers/mmc/tmio-common.c +++ b/drivers/mmc/tmio-common.c @@ -373,7 +373,7 @@ static bool tmio_sd_addr_is_dmaable(struct mmc_data *data) if (!(data->flags & MMC_DATA_READ) && !IS_ALIGNED(addr, 128)) return false; /* Gen3 DMA has 32bit limit */
- if (addr >> 32)
- if (sizeof(addr) > 4 && addr >> 32) return false;
#endif
participants (3)
-
Jaehoon Chung
-
Marek Vasut
-
Simon Glass