* [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place
@ 2020-04-08 7:21 Masahiro Yamada
2020-04-08 7:21 ` [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks Masahiro Yamada
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Masahiro Yamada @ 2020-04-08 7:21 UTC (permalink / raw)
To: linux-mmc
Cc: linux-kernel, Ulf Hansson, masahiroy, Masahiro Yamada, Adrian Hunter
In the SDHCI specification, the Capabilities Register (Offset 0x40h)
is the 64-bit width register, but in Linux, it is represented as two
registers, SDHCI_CAPABILITIES and SDHCI_CAPABILITIES_1 so that drivers
can use 32-bit register accessors.
The upper 32-bit field is associated with SDHCI_CAPABILITIES_1.
Move the definition of SDHCI_CAPABILITIES_1 to the correct place.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/mmc/host/sdhci.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 79dffbb731d3..b786b68e0302 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -220,6 +220,7 @@
#define SDHCI_CAN_64BIT_V4 0x08000000
#define SDHCI_CAN_64BIT 0x10000000
+#define SDHCI_CAPABILITIES_1 0x44
#define SDHCI_SUPPORT_SDR50 0x00000001
#define SDHCI_SUPPORT_SDR104 0x00000002
#define SDHCI_SUPPORT_DDR50 0x00000004
@@ -236,8 +237,6 @@
#define SDHCI_CAN_DO_ADMA3 0x08000000
#define SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */
-#define SDHCI_CAPABILITIES_1 0x44
-
#define SDHCI_MAX_CURRENT 0x48
#define SDHCI_MAX_CURRENT_LIMIT 0xFF
#define SDHCI_MAX_CURRENT_330_MASK 0x0000FF
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks 2020-04-08 7:21 [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Masahiro Yamada @ 2020-04-08 7:21 ` Masahiro Yamada 2020-04-15 9:34 ` Adrian Hunter 2020-04-15 7:19 ` [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Adrian Hunter 2020-04-15 10:21 ` Ulf Hansson 2 siblings, 1 reply; 6+ messages in thread From: Masahiro Yamada @ 2020-04-08 7:21 UTC (permalink / raw) To: linux-mmc Cc: linux-kernel, Ulf Hansson, masahiroy, Masahiro Yamada, Adrian Hunter, Alexandre Belloni, Fabio Estevam, Ludovic Desroches, NXP Linux Team, Nicolas Ferre, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, linux-arm-kernel Use FIELD_GET and FIELD_PREP to get access to the register fields. Delete the shift macros. I used GENMASK() for touched macros. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- drivers/mmc/host/sdhci-esdhc-imx.c | 4 +++- drivers/mmc/host/sdhci-of-at91.c | 5 +++-- drivers/mmc/host/sdhci-pci-core.c | 8 ++------ drivers/mmc/host/sdhci.c | 19 +++++++------------ drivers/mmc/host/sdhci.h | 17 ++++++----------- 5 files changed, 21 insertions(+), 32 deletions(-) diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c index 5ec8e4bf1ac7..38cd83118082 100644 --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -8,6 +8,7 @@ * Author: Wolfram Sang <kernel@pengutronix.de> */ +#include <linux/bitfield.h> #include <linux/io.h> #include <linux/iopoll.h> #include <linux/delay.h> @@ -399,7 +400,8 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg) val = SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_USE_SDR50_TUNING - | (SDHCI_TUNING_MODE_3 << SDHCI_RETUNING_MODE_SHIFT); + | FIELD_PREP(SDHCI_RETUNING_MODE_MASK, + SDHCI_TUNING_MODE_3); if (imx_data->socdata->flags & ESDHC_FLAG_HS400) val |= SDHCI_SUPPORT_HS400; diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c index c79bff5e2280..25f4e0f4f53b 100644 --- a/drivers/mmc/host/sdhci-of-at91.c +++ b/drivers/mmc/host/sdhci-of-at91.c @@ -6,6 +6,7 @@ * 2015 Ludovic Desroches <ludovic.desroches@atmel.com> */ +#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/delay.h> #include <linux/err.h> @@ -179,9 +180,9 @@ static int sdhci_at91_set_clks_presets(struct device *dev) clk_mul = gck_rate / clk_base_rate - 1; caps0 &= ~SDHCI_CLOCK_V3_BASE_MASK; - caps0 |= (clk_base << SDHCI_CLOCK_BASE_SHIFT) & SDHCI_CLOCK_V3_BASE_MASK; + caps0 |= FIELD_PREP(SDHCI_CLOCK_V3_BASE_MASK, clk_base); caps1 &= ~SDHCI_CLOCK_MUL_MASK; - caps1 |= (clk_mul << SDHCI_CLOCK_MUL_SHIFT) & SDHCI_CLOCK_MUL_MASK; + caps1 |= FIELD_PREP(SDHCI_CLOCK_MUL_MASK, clk_mul); /* Set capabilities in r/w mode. */ writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, host->ioaddr + SDMMC_CACR); writel(caps0, host->ioaddr + SDHCI_CAPABILITIES); diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c index 525de2454a4d..a98c9476bfc8 100644 --- a/drivers/mmc/host/sdhci-pci-core.c +++ b/drivers/mmc/host/sdhci-pci-core.c @@ -249,12 +249,8 @@ static int ricoh_probe(struct sdhci_pci_chip *chip) static int ricoh_mmc_probe_slot(struct sdhci_pci_slot *slot) { slot->host->caps = - ((0x21 << SDHCI_TIMEOUT_CLK_SHIFT) - & SDHCI_TIMEOUT_CLK_MASK) | - - ((0x21 << SDHCI_CLOCK_BASE_SHIFT) - & SDHCI_CLOCK_BASE_MASK) | - + FIELD_PREP(SDHCI_TIMEOUT_CLK_MASK, 0x21) | + FIELD_PREP(SDHCI_CLOCK_BASE_MASK, 0x21) | SDHCI_TIMEOUT_CLK_UNIT | SDHCI_CAN_VDD_330 | SDHCI_CAN_DO_HISPD | diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 3f716466fcfd..344a7e0e33fe 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -4117,11 +4117,9 @@ int sdhci_setup_host(struct sdhci_host *host) } if (host->version >= SDHCI_SPEC_300) - host->max_clk = (host->caps & SDHCI_CLOCK_V3_BASE_MASK) - >> SDHCI_CLOCK_BASE_SHIFT; + host->max_clk = FIELD_GET(SDHCI_CLOCK_V3_BASE_MASK, host->caps); else - host->max_clk = (host->caps & SDHCI_CLOCK_BASE_MASK) - >> SDHCI_CLOCK_BASE_SHIFT; + host->max_clk = FIELD_GET(SDHCI_CLOCK_BASE_MASK, host->caps); host->max_clk *= 1000000; if (host->max_clk == 0 || host->quirks & @@ -4139,8 +4137,7 @@ int sdhci_setup_host(struct sdhci_host *host) * In case of Host Controller v3.00, find out whether clock * multiplier is supported. */ - host->clk_mul = (host->caps1 & SDHCI_CLOCK_MUL_MASK) >> - SDHCI_CLOCK_MUL_SHIFT; + host->clk_mul = FIELD_GET(SDHCI_CLOCK_MUL_MASK, host->caps1); /* * In case the value in Clock Multiplier is 0, then programmable @@ -4173,8 +4170,7 @@ int sdhci_setup_host(struct sdhci_host *host) mmc->f_max = max_clk; if (!(host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)) { - host->timeout_clk = (host->caps & SDHCI_TIMEOUT_CLK_MASK) >> - SDHCI_TIMEOUT_CLK_SHIFT; + host->timeout_clk = FIELD_GET(SDHCI_TIMEOUT_CLK_MASK, host->caps); if (host->caps & SDHCI_TIMEOUT_CLK_UNIT) host->timeout_clk *= 1000; @@ -4326,8 +4322,8 @@ int sdhci_setup_host(struct sdhci_host *host) mmc->caps |= MMC_CAP_DRIVER_TYPE_D; /* Initial value for re-tuning timer count */ - host->tuning_count = (host->caps1 & SDHCI_RETUNING_TIMER_COUNT_MASK) >> - SDHCI_RETUNING_TIMER_COUNT_SHIFT; + host->tuning_count = FIELD_GET(SDHCI_RETUNING_TIMER_COUNT_MASK, + host->caps1); /* * In case Re-tuning Timer is not disabled, the actual value of @@ -4337,8 +4333,7 @@ int sdhci_setup_host(struct sdhci_host *host) host->tuning_count = 1 << (host->tuning_count - 1); /* Re-tuning mode supported by the Host Controller */ - host->tuning_mode = (host->caps1 & SDHCI_RETUNING_MODE_MASK) >> - SDHCI_RETUNING_MODE_SHIFT; + host->tuning_mode = FIELD_GET(SDHCI_RETUNING_MODE_MASK, host->caps1); ocr_avail = 0; diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index b786b68e0302..d7f1441b0fc3 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -200,12 +200,10 @@ #define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000 #define SDHCI_CAPABILITIES 0x40 -#define SDHCI_TIMEOUT_CLK_MASK 0x0000003F -#define SDHCI_TIMEOUT_CLK_SHIFT 0 +#define SDHCI_TIMEOUT_CLK_MASK GENMASK(5, 0) #define SDHCI_TIMEOUT_CLK_UNIT 0x00000080 -#define SDHCI_CLOCK_BASE_MASK 0x00003F00 -#define SDHCI_CLOCK_V3_BASE_MASK 0x0000FF00 -#define SDHCI_CLOCK_BASE_SHIFT 8 +#define SDHCI_CLOCK_BASE_MASK GENMASK(13, 8) +#define SDHCI_CLOCK_V3_BASE_MASK GENMASK(15, 8) #define SDHCI_MAX_BLOCK_MASK 0x00030000 #define SDHCI_MAX_BLOCK_SHIFT 16 #define SDHCI_CAN_DO_8BIT 0x00040000 @@ -227,13 +225,10 @@ #define SDHCI_DRIVER_TYPE_A 0x00000010 #define SDHCI_DRIVER_TYPE_C 0x00000020 #define SDHCI_DRIVER_TYPE_D 0x00000040 -#define SDHCI_RETUNING_TIMER_COUNT_MASK 0x00000F00 -#define SDHCI_RETUNING_TIMER_COUNT_SHIFT 8 +#define SDHCI_RETUNING_TIMER_COUNT_MASK GENMASK(11, 8) #define SDHCI_USE_SDR50_TUNING 0x00002000 -#define SDHCI_RETUNING_MODE_MASK 0x0000C000 -#define SDHCI_RETUNING_MODE_SHIFT 14 -#define SDHCI_CLOCK_MUL_MASK 0x00FF0000 -#define SDHCI_CLOCK_MUL_SHIFT 16 +#define SDHCI_RETUNING_MODE_MASK GENMASK(15, 14) +#define SDHCI_CLOCK_MUL_MASK GENMASK(23, 16) #define SDHCI_CAN_DO_ADMA3 0x08000000 #define SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */ -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks 2020-04-08 7:21 ` [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks Masahiro Yamada @ 2020-04-15 9:34 ` Adrian Hunter 2020-04-15 10:21 ` Ulf Hansson 0 siblings, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2020-04-15 9:34 UTC (permalink / raw) To: Masahiro Yamada, linux-mmc Cc: linux-kernel, Ulf Hansson, masahiroy, Alexandre Belloni, Fabio Estevam, Ludovic Desroches, NXP Linux Team, Nicolas Ferre, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, linux-arm-kernel On 8/04/20 10:21 am, Masahiro Yamada wrote: > Use FIELD_GET and FIELD_PREP to get access to the register fields. > Delete the shift macros. I used GENMASK() for touched macros. That has the side-effect of changing the constants to 64-bit on 64-bit platforms, which needs to be mentioned in the commit message. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Apart from above: Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > > drivers/mmc/host/sdhci-esdhc-imx.c | 4 +++- > drivers/mmc/host/sdhci-of-at91.c | 5 +++-- > drivers/mmc/host/sdhci-pci-core.c | 8 ++------ > drivers/mmc/host/sdhci.c | 19 +++++++------------ > drivers/mmc/host/sdhci.h | 17 ++++++----------- > 5 files changed, 21 insertions(+), 32 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index 5ec8e4bf1ac7..38cd83118082 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -8,6 +8,7 @@ > * Author: Wolfram Sang <kernel@pengutronix.de> > */ > > +#include <linux/bitfield.h> > #include <linux/io.h> > #include <linux/iopoll.h> > #include <linux/delay.h> > @@ -399,7 +400,8 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg) > val = SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_SDR104 > | SDHCI_SUPPORT_SDR50 > | SDHCI_USE_SDR50_TUNING > - | (SDHCI_TUNING_MODE_3 << SDHCI_RETUNING_MODE_SHIFT); > + | FIELD_PREP(SDHCI_RETUNING_MODE_MASK, > + SDHCI_TUNING_MODE_3); > > if (imx_data->socdata->flags & ESDHC_FLAG_HS400) > val |= SDHCI_SUPPORT_HS400; > diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c > index c79bff5e2280..25f4e0f4f53b 100644 > --- a/drivers/mmc/host/sdhci-of-at91.c > +++ b/drivers/mmc/host/sdhci-of-at91.c > @@ -6,6 +6,7 @@ > * 2015 Ludovic Desroches <ludovic.desroches@atmel.com> > */ > > +#include <linux/bitfield.h> > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/err.h> > @@ -179,9 +180,9 @@ static int sdhci_at91_set_clks_presets(struct device *dev) > clk_mul = gck_rate / clk_base_rate - 1; > > caps0 &= ~SDHCI_CLOCK_V3_BASE_MASK; > - caps0 |= (clk_base << SDHCI_CLOCK_BASE_SHIFT) & SDHCI_CLOCK_V3_BASE_MASK; > + caps0 |= FIELD_PREP(SDHCI_CLOCK_V3_BASE_MASK, clk_base); > caps1 &= ~SDHCI_CLOCK_MUL_MASK; > - caps1 |= (clk_mul << SDHCI_CLOCK_MUL_SHIFT) & SDHCI_CLOCK_MUL_MASK; > + caps1 |= FIELD_PREP(SDHCI_CLOCK_MUL_MASK, clk_mul); > /* Set capabilities in r/w mode. */ > writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, host->ioaddr + SDMMC_CACR); > writel(caps0, host->ioaddr + SDHCI_CAPABILITIES); > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c > index 525de2454a4d..a98c9476bfc8 100644 > --- a/drivers/mmc/host/sdhci-pci-core.c > +++ b/drivers/mmc/host/sdhci-pci-core.c > @@ -249,12 +249,8 @@ static int ricoh_probe(struct sdhci_pci_chip *chip) > static int ricoh_mmc_probe_slot(struct sdhci_pci_slot *slot) > { > slot->host->caps = > - ((0x21 << SDHCI_TIMEOUT_CLK_SHIFT) > - & SDHCI_TIMEOUT_CLK_MASK) | > - > - ((0x21 << SDHCI_CLOCK_BASE_SHIFT) > - & SDHCI_CLOCK_BASE_MASK) | > - > + FIELD_PREP(SDHCI_TIMEOUT_CLK_MASK, 0x21) | > + FIELD_PREP(SDHCI_CLOCK_BASE_MASK, 0x21) | > SDHCI_TIMEOUT_CLK_UNIT | > SDHCI_CAN_VDD_330 | > SDHCI_CAN_DO_HISPD | > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 3f716466fcfd..344a7e0e33fe 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -4117,11 +4117,9 @@ int sdhci_setup_host(struct sdhci_host *host) > } > > if (host->version >= SDHCI_SPEC_300) > - host->max_clk = (host->caps & SDHCI_CLOCK_V3_BASE_MASK) > - >> SDHCI_CLOCK_BASE_SHIFT; > + host->max_clk = FIELD_GET(SDHCI_CLOCK_V3_BASE_MASK, host->caps); > else > - host->max_clk = (host->caps & SDHCI_CLOCK_BASE_MASK) > - >> SDHCI_CLOCK_BASE_SHIFT; > + host->max_clk = FIELD_GET(SDHCI_CLOCK_BASE_MASK, host->caps); > > host->max_clk *= 1000000; > if (host->max_clk == 0 || host->quirks & > @@ -4139,8 +4137,7 @@ int sdhci_setup_host(struct sdhci_host *host) > * In case of Host Controller v3.00, find out whether clock > * multiplier is supported. > */ > - host->clk_mul = (host->caps1 & SDHCI_CLOCK_MUL_MASK) >> > - SDHCI_CLOCK_MUL_SHIFT; > + host->clk_mul = FIELD_GET(SDHCI_CLOCK_MUL_MASK, host->caps1); > > /* > * In case the value in Clock Multiplier is 0, then programmable > @@ -4173,8 +4170,7 @@ int sdhci_setup_host(struct sdhci_host *host) > mmc->f_max = max_clk; > > if (!(host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)) { > - host->timeout_clk = (host->caps & SDHCI_TIMEOUT_CLK_MASK) >> > - SDHCI_TIMEOUT_CLK_SHIFT; > + host->timeout_clk = FIELD_GET(SDHCI_TIMEOUT_CLK_MASK, host->caps); > > if (host->caps & SDHCI_TIMEOUT_CLK_UNIT) > host->timeout_clk *= 1000; > @@ -4326,8 +4322,8 @@ int sdhci_setup_host(struct sdhci_host *host) > mmc->caps |= MMC_CAP_DRIVER_TYPE_D; > > /* Initial value for re-tuning timer count */ > - host->tuning_count = (host->caps1 & SDHCI_RETUNING_TIMER_COUNT_MASK) >> > - SDHCI_RETUNING_TIMER_COUNT_SHIFT; > + host->tuning_count = FIELD_GET(SDHCI_RETUNING_TIMER_COUNT_MASK, > + host->caps1); > > /* > * In case Re-tuning Timer is not disabled, the actual value of > @@ -4337,8 +4333,7 @@ int sdhci_setup_host(struct sdhci_host *host) > host->tuning_count = 1 << (host->tuning_count - 1); > > /* Re-tuning mode supported by the Host Controller */ > - host->tuning_mode = (host->caps1 & SDHCI_RETUNING_MODE_MASK) >> > - SDHCI_RETUNING_MODE_SHIFT; > + host->tuning_mode = FIELD_GET(SDHCI_RETUNING_MODE_MASK, host->caps1); > > ocr_avail = 0; > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index b786b68e0302..d7f1441b0fc3 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -200,12 +200,10 @@ > #define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000 > > #define SDHCI_CAPABILITIES 0x40 > -#define SDHCI_TIMEOUT_CLK_MASK 0x0000003F > -#define SDHCI_TIMEOUT_CLK_SHIFT 0 > +#define SDHCI_TIMEOUT_CLK_MASK GENMASK(5, 0) > #define SDHCI_TIMEOUT_CLK_UNIT 0x00000080 > -#define SDHCI_CLOCK_BASE_MASK 0x00003F00 > -#define SDHCI_CLOCK_V3_BASE_MASK 0x0000FF00 > -#define SDHCI_CLOCK_BASE_SHIFT 8 > +#define SDHCI_CLOCK_BASE_MASK GENMASK(13, 8) > +#define SDHCI_CLOCK_V3_BASE_MASK GENMASK(15, 8) > #define SDHCI_MAX_BLOCK_MASK 0x00030000 > #define SDHCI_MAX_BLOCK_SHIFT 16 > #define SDHCI_CAN_DO_8BIT 0x00040000 > @@ -227,13 +225,10 @@ > #define SDHCI_DRIVER_TYPE_A 0x00000010 > #define SDHCI_DRIVER_TYPE_C 0x00000020 > #define SDHCI_DRIVER_TYPE_D 0x00000040 > -#define SDHCI_RETUNING_TIMER_COUNT_MASK 0x00000F00 > -#define SDHCI_RETUNING_TIMER_COUNT_SHIFT 8 > +#define SDHCI_RETUNING_TIMER_COUNT_MASK GENMASK(11, 8) > #define SDHCI_USE_SDR50_TUNING 0x00002000 > -#define SDHCI_RETUNING_MODE_MASK 0x0000C000 > -#define SDHCI_RETUNING_MODE_SHIFT 14 > -#define SDHCI_CLOCK_MUL_MASK 0x00FF0000 > -#define SDHCI_CLOCK_MUL_SHIFT 16 > +#define SDHCI_RETUNING_MODE_MASK GENMASK(15, 14) > +#define SDHCI_CLOCK_MUL_MASK GENMASK(23, 16) > #define SDHCI_CAN_DO_ADMA3 0x08000000 > #define SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */ > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks 2020-04-15 9:34 ` Adrian Hunter @ 2020-04-15 10:21 ` Ulf Hansson 0 siblings, 0 replies; 6+ messages in thread From: Ulf Hansson @ 2020-04-15 10:21 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-mmc, Linux Kernel Mailing List, masahiroy, Alexandre Belloni, Fabio Estevam, Adrian Hunter, Ludovic Desroches, NXP Linux Team, Nicolas Ferre, Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, Linux ARM On Wed, 15 Apr 2020 at 11:35, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 8/04/20 10:21 am, Masahiro Yamada wrote: > > Use FIELD_GET and FIELD_PREP to get access to the register fields. > > Delete the shift macros. I used GENMASK() for touched macros. > > That has the side-effect of changing the constants to 64-bit on 64-bit > platforms, which needs to be mentioned in the commit message. > > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > Apart from above: > > Acked-by: Adrian Hunter <adrian.hunter@intel.com> Applied for next, by amending the changelog according to the suggestion by Adrian. Kind regards Uffe > > > --- > > > > drivers/mmc/host/sdhci-esdhc-imx.c | 4 +++- > > drivers/mmc/host/sdhci-of-at91.c | 5 +++-- > > drivers/mmc/host/sdhci-pci-core.c | 8 ++------ > > drivers/mmc/host/sdhci.c | 19 +++++++------------ > > drivers/mmc/host/sdhci.h | 17 ++++++----------- > > 5 files changed, 21 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > > index 5ec8e4bf1ac7..38cd83118082 100644 > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > > @@ -8,6 +8,7 @@ > > * Author: Wolfram Sang <kernel@pengutronix.de> > > */ > > > > +#include <linux/bitfield.h> > > #include <linux/io.h> > > #include <linux/iopoll.h> > > #include <linux/delay.h> > > @@ -399,7 +400,8 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg) > > val = SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_SDR104 > > | SDHCI_SUPPORT_SDR50 > > | SDHCI_USE_SDR50_TUNING > > - | (SDHCI_TUNING_MODE_3 << SDHCI_RETUNING_MODE_SHIFT); > > + | FIELD_PREP(SDHCI_RETUNING_MODE_MASK, > > + SDHCI_TUNING_MODE_3); > > > > if (imx_data->socdata->flags & ESDHC_FLAG_HS400) > > val |= SDHCI_SUPPORT_HS400; > > diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c > > index c79bff5e2280..25f4e0f4f53b 100644 > > --- a/drivers/mmc/host/sdhci-of-at91.c > > +++ b/drivers/mmc/host/sdhci-of-at91.c > > @@ -6,6 +6,7 @@ > > * 2015 Ludovic Desroches <ludovic.desroches@atmel.com> > > */ > > > > +#include <linux/bitfield.h> > > #include <linux/clk.h> > > #include <linux/delay.h> > > #include <linux/err.h> > > @@ -179,9 +180,9 @@ static int sdhci_at91_set_clks_presets(struct device *dev) > > clk_mul = gck_rate / clk_base_rate - 1; > > > > caps0 &= ~SDHCI_CLOCK_V3_BASE_MASK; > > - caps0 |= (clk_base << SDHCI_CLOCK_BASE_SHIFT) & SDHCI_CLOCK_V3_BASE_MASK; > > + caps0 |= FIELD_PREP(SDHCI_CLOCK_V3_BASE_MASK, clk_base); > > caps1 &= ~SDHCI_CLOCK_MUL_MASK; > > - caps1 |= (clk_mul << SDHCI_CLOCK_MUL_SHIFT) & SDHCI_CLOCK_MUL_MASK; > > + caps1 |= FIELD_PREP(SDHCI_CLOCK_MUL_MASK, clk_mul); > > /* Set capabilities in r/w mode. */ > > writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, host->ioaddr + SDMMC_CACR); > > writel(caps0, host->ioaddr + SDHCI_CAPABILITIES); > > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c > > index 525de2454a4d..a98c9476bfc8 100644 > > --- a/drivers/mmc/host/sdhci-pci-core.c > > +++ b/drivers/mmc/host/sdhci-pci-core.c > > @@ -249,12 +249,8 @@ static int ricoh_probe(struct sdhci_pci_chip *chip) > > static int ricoh_mmc_probe_slot(struct sdhci_pci_slot *slot) > > { > > slot->host->caps = > > - ((0x21 << SDHCI_TIMEOUT_CLK_SHIFT) > > - & SDHCI_TIMEOUT_CLK_MASK) | > > - > > - ((0x21 << SDHCI_CLOCK_BASE_SHIFT) > > - & SDHCI_CLOCK_BASE_MASK) | > > - > > + FIELD_PREP(SDHCI_TIMEOUT_CLK_MASK, 0x21) | > > + FIELD_PREP(SDHCI_CLOCK_BASE_MASK, 0x21) | > > SDHCI_TIMEOUT_CLK_UNIT | > > SDHCI_CAN_VDD_330 | > > SDHCI_CAN_DO_HISPD | > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 3f716466fcfd..344a7e0e33fe 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -4117,11 +4117,9 @@ int sdhci_setup_host(struct sdhci_host *host) > > } > > > > if (host->version >= SDHCI_SPEC_300) > > - host->max_clk = (host->caps & SDHCI_CLOCK_V3_BASE_MASK) > > - >> SDHCI_CLOCK_BASE_SHIFT; > > + host->max_clk = FIELD_GET(SDHCI_CLOCK_V3_BASE_MASK, host->caps); > > else > > - host->max_clk = (host->caps & SDHCI_CLOCK_BASE_MASK) > > - >> SDHCI_CLOCK_BASE_SHIFT; > > + host->max_clk = FIELD_GET(SDHCI_CLOCK_BASE_MASK, host->caps); > > > > host->max_clk *= 1000000; > > if (host->max_clk == 0 || host->quirks & > > @@ -4139,8 +4137,7 @@ int sdhci_setup_host(struct sdhci_host *host) > > * In case of Host Controller v3.00, find out whether clock > > * multiplier is supported. > > */ > > - host->clk_mul = (host->caps1 & SDHCI_CLOCK_MUL_MASK) >> > > - SDHCI_CLOCK_MUL_SHIFT; > > + host->clk_mul = FIELD_GET(SDHCI_CLOCK_MUL_MASK, host->caps1); > > > > /* > > * In case the value in Clock Multiplier is 0, then programmable > > @@ -4173,8 +4170,7 @@ int sdhci_setup_host(struct sdhci_host *host) > > mmc->f_max = max_clk; > > > > if (!(host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)) { > > - host->timeout_clk = (host->caps & SDHCI_TIMEOUT_CLK_MASK) >> > > - SDHCI_TIMEOUT_CLK_SHIFT; > > + host->timeout_clk = FIELD_GET(SDHCI_TIMEOUT_CLK_MASK, host->caps); > > > > if (host->caps & SDHCI_TIMEOUT_CLK_UNIT) > > host->timeout_clk *= 1000; > > @@ -4326,8 +4322,8 @@ int sdhci_setup_host(struct sdhci_host *host) > > mmc->caps |= MMC_CAP_DRIVER_TYPE_D; > > > > /* Initial value for re-tuning timer count */ > > - host->tuning_count = (host->caps1 & SDHCI_RETUNING_TIMER_COUNT_MASK) >> > > - SDHCI_RETUNING_TIMER_COUNT_SHIFT; > > + host->tuning_count = FIELD_GET(SDHCI_RETUNING_TIMER_COUNT_MASK, > > + host->caps1); > > > > /* > > * In case Re-tuning Timer is not disabled, the actual value of > > @@ -4337,8 +4333,7 @@ int sdhci_setup_host(struct sdhci_host *host) > > host->tuning_count = 1 << (host->tuning_count - 1); > > > > /* Re-tuning mode supported by the Host Controller */ > > - host->tuning_mode = (host->caps1 & SDHCI_RETUNING_MODE_MASK) >> > > - SDHCI_RETUNING_MODE_SHIFT; > > + host->tuning_mode = FIELD_GET(SDHCI_RETUNING_MODE_MASK, host->caps1); > > > > ocr_avail = 0; > > > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > > index b786b68e0302..d7f1441b0fc3 100644 > > --- a/drivers/mmc/host/sdhci.h > > +++ b/drivers/mmc/host/sdhci.h > > @@ -200,12 +200,10 @@ > > #define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000 > > > > #define SDHCI_CAPABILITIES 0x40 > > -#define SDHCI_TIMEOUT_CLK_MASK 0x0000003F > > -#define SDHCI_TIMEOUT_CLK_SHIFT 0 > > +#define SDHCI_TIMEOUT_CLK_MASK GENMASK(5, 0) > > #define SDHCI_TIMEOUT_CLK_UNIT 0x00000080 > > -#define SDHCI_CLOCK_BASE_MASK 0x00003F00 > > -#define SDHCI_CLOCK_V3_BASE_MASK 0x0000FF00 > > -#define SDHCI_CLOCK_BASE_SHIFT 8 > > +#define SDHCI_CLOCK_BASE_MASK GENMASK(13, 8) > > +#define SDHCI_CLOCK_V3_BASE_MASK GENMASK(15, 8) > > #define SDHCI_MAX_BLOCK_MASK 0x00030000 > > #define SDHCI_MAX_BLOCK_SHIFT 16 > > #define SDHCI_CAN_DO_8BIT 0x00040000 > > @@ -227,13 +225,10 @@ > > #define SDHCI_DRIVER_TYPE_A 0x00000010 > > #define SDHCI_DRIVER_TYPE_C 0x00000020 > > #define SDHCI_DRIVER_TYPE_D 0x00000040 > > -#define SDHCI_RETUNING_TIMER_COUNT_MASK 0x00000F00 > > -#define SDHCI_RETUNING_TIMER_COUNT_SHIFT 8 > > +#define SDHCI_RETUNING_TIMER_COUNT_MASK GENMASK(11, 8) > > #define SDHCI_USE_SDR50_TUNING 0x00002000 > > -#define SDHCI_RETUNING_MODE_MASK 0x0000C000 > > -#define SDHCI_RETUNING_MODE_SHIFT 14 > > -#define SDHCI_CLOCK_MUL_MASK 0x00FF0000 > > -#define SDHCI_CLOCK_MUL_SHIFT 16 > > +#define SDHCI_RETUNING_MODE_MASK GENMASK(15, 14) > > +#define SDHCI_CLOCK_MUL_MASK GENMASK(23, 16) > > #define SDHCI_CAN_DO_ADMA3 0x08000000 > > #define SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */ > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place 2020-04-08 7:21 [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Masahiro Yamada 2020-04-08 7:21 ` [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks Masahiro Yamada @ 2020-04-15 7:19 ` Adrian Hunter 2020-04-15 10:21 ` Ulf Hansson 2 siblings, 0 replies; 6+ messages in thread From: Adrian Hunter @ 2020-04-15 7:19 UTC (permalink / raw) To: Masahiro Yamada, linux-mmc; +Cc: linux-kernel, Ulf Hansson, masahiroy On 8/04/20 10:21 am, Masahiro Yamada wrote: > In the SDHCI specification, the Capabilities Register (Offset 0x40h) > is the 64-bit width register, but in Linux, it is represented as two > registers, SDHCI_CAPABILITIES and SDHCI_CAPABILITIES_1 so that drivers > can use 32-bit register accessors. > > The upper 32-bit field is associated with SDHCI_CAPABILITIES_1. > > Move the definition of SDHCI_CAPABILITIES_1 to the correct place. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > > drivers/mmc/host/sdhci.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 79dffbb731d3..b786b68e0302 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -220,6 +220,7 @@ > #define SDHCI_CAN_64BIT_V4 0x08000000 > #define SDHCI_CAN_64BIT 0x10000000 > > +#define SDHCI_CAPABILITIES_1 0x44 > #define SDHCI_SUPPORT_SDR50 0x00000001 > #define SDHCI_SUPPORT_SDR104 0x00000002 > #define SDHCI_SUPPORT_DDR50 0x00000004 > @@ -236,8 +237,6 @@ > #define SDHCI_CAN_DO_ADMA3 0x08000000 > #define SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */ > > -#define SDHCI_CAPABILITIES_1 0x44 > - > #define SDHCI_MAX_CURRENT 0x48 > #define SDHCI_MAX_CURRENT_LIMIT 0xFF > #define SDHCI_MAX_CURRENT_330_MASK 0x0000FF > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place 2020-04-08 7:21 [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Masahiro Yamada 2020-04-08 7:21 ` [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks Masahiro Yamada 2020-04-15 7:19 ` [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Adrian Hunter @ 2020-04-15 10:21 ` Ulf Hansson 2 siblings, 0 replies; 6+ messages in thread From: Ulf Hansson @ 2020-04-15 10:21 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-mmc, Linux Kernel Mailing List, masahiroy, Adrian Hunter On Wed, 8 Apr 2020 at 09:21, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > In the SDHCI specification, the Capabilities Register (Offset 0x40h) > is the 64-bit width register, but in Linux, it is represented as two > registers, SDHCI_CAPABILITIES and SDHCI_CAPABILITIES_1 so that drivers > can use 32-bit register accessors. > > The upper 32-bit field is associated with SDHCI_CAPABILITIES_1. > > Move the definition of SDHCI_CAPABILITIES_1 to the correct place. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Applied for next, thanks! Kind regards Uffe > --- > > drivers/mmc/host/sdhci.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 79dffbb731d3..b786b68e0302 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -220,6 +220,7 @@ > #define SDHCI_CAN_64BIT_V4 0x08000000 > #define SDHCI_CAN_64BIT 0x10000000 > > +#define SDHCI_CAPABILITIES_1 0x44 > #define SDHCI_SUPPORT_SDR50 0x00000001 > #define SDHCI_SUPPORT_SDR104 0x00000002 > #define SDHCI_SUPPORT_DDR50 0x00000004 > @@ -236,8 +237,6 @@ > #define SDHCI_CAN_DO_ADMA3 0x08000000 > #define SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */ > > -#define SDHCI_CAPABILITIES_1 0x44 > - > #define SDHCI_MAX_CURRENT 0x48 > #define SDHCI_MAX_CURRENT_LIMIT 0xFF > #define SDHCI_MAX_CURRENT_330_MASK 0x0000FF > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-15 10:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-08 7:21 [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Masahiro Yamada 2020-04-08 7:21 ` [PATCH 2/2] mmc: sdhci: use FIELD_GET/PREP for capabilities bit masks Masahiro Yamada 2020-04-15 9:34 ` Adrian Hunter 2020-04-15 10:21 ` Ulf Hansson 2020-04-15 7:19 ` [PATCH 1/2] mmc: sdhci: move SDHCI_CAPABILITIES_1 to a more suitable place Adrian Hunter 2020-04-15 10:21 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®