Hi
Commits
05cb6b2a66fa - mmc: sdhci-of-esdhc: add erratum eSDHC-A001 and A-008358
support
a46e42712596 - mmc: sdhci-of-esdhc: add erratum eSDHC5 support
seem a bit odd, in that they set bits from the SDHCI_* namespace in the
->quirks2 member:
host->quirks2 |= SDHCI_QUIRK_RESET_AFTER_REQUEST;
host->quirks2 |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
These are bits 10 and 12, so they are also known as
SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD and
SDHCI_QUIRK2_TUNING_WORK_AROUND. So if the fixes are correct, one should
use those names on the right-hand side, but it seems just as likely (I
can't figure that out from the commit messages) that the left-hand sides
should have been host->quirks ?
Rasmus
On 03/12/2019 11.15, Rasmus Villemoes wrote:
> Hi
>
> Commits
>
> 05cb6b2a66fa - mmc: sdhci-of-esdhc: add erratum eSDHC-A001 and A-008358
> support
> a46e42712596 - mmc: sdhci-of-esdhc: add erratum eSDHC5 support
>
> seem a bit odd, in that they set bits from the SDHCI_* namespace in the
> ->quirks2 member:
>
> host->quirks2 |= SDHCI_QUIRK_RESET_AFTER_REQUEST;
> host->quirks2 |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
FWIW, with something like the patch below, sparse (make C=1) would complain
drivers/mmc/host/sdhci-of-esdhc.c:1306:31: warning: invalid assignment: |=
drivers/mmc/host/sdhci-of-esdhc.c:1306:31: left side has type
restricted sdhci_quirk2_t
drivers/mmc/host/sdhci-of-esdhc.c:1306:31: right side has type
restricted sdhci_quirk_t
drivers/mmc/host/sdhci-of-esdhc.c:1307:31: warning: invalid assignment: |=
drivers/mmc/host/sdhci-of-esdhc.c:1307:31: left side has type
restricted sdhci_quirk2_t
drivers/mmc/host/sdhci-of-esdhc.c:1307:31: right side has type
restricted sdhci_quirk_t
But maybe that's too much churn, dunno.
Rasmus
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 2af445b8c325..41712b848a06 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -14,8 +14,8 @@
struct sdhci_pltfm_data {
const struct sdhci_ops *ops;
- unsigned int quirks;
- unsigned int quirks2;
+ sdhci_quirk_t quirks;
+ sdhci_quirk2_t quirks2;
};
struct sdhci_pltfm_host {
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3140fe2e5dba..7316c81026a3 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3613,10 +3613,10 @@ void __sdhci_read_caps(struct sdhci_host *host,
const u16 *ver,
host->read_caps = true;
if (debug_quirks)
- host->quirks = debug_quirks;
+ host->quirks = (__force sdhci_quirk_t)debug_quirks;
if (debug_quirks2)
- host->quirks2 = debug_quirks2;
+ host->quirks2 = (__force sdhci_quirk2_t)debug_quirks2;
sdhci_do_reset(host, SDHCI_RESET_ALL);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0ed3e0eaef5f..5a97a6d13abf 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -369,119 +369,122 @@ enum sdhci_cookie {
COOKIE_MAPPED, /* mapped by sdhci_prepare_data() */
};
+typedef unsigned int __bitwise sdhci_quirk_t;
+typedef unsigned int __bitwise sdhci_quirk2_t;
+
struct sdhci_host {
/* Data set by hardware interface driver */
const char *hw_name; /* Hardware bus name */
- unsigned int quirks; /* Deviations from spec. */
+ sdhci_quirk_t quirks; /* Deviations from spec. */
/* Controller doesn't honor resets unless we touch the clock register */
-#define SDHCI_QUIRK_CLOCK_BEFORE_RESET (1<<0)
+#define SDHCI_QUIRK_CLOCK_BEFORE_RESET ((__force sdhci_quirk_t)(1<<0))
/* Controller has bad caps bits, but really supports DMA */
-#define SDHCI_QUIRK_FORCE_DMA (1<<1)
+#define SDHCI_QUIRK_FORCE_DMA ((__force sdhci_quirk_t)(1<<1))
/* Controller doesn't like to be reset when there is no card inserted. */
-#define SDHCI_QUIRK_NO_CARD_NO_RESET (1<<2)
+#define SDHCI_QUIRK_NO_CARD_NO_RESET ((__force sdhci_quirk_t)(1<<2))
/* Controller doesn't like clearing the power reg before a change */
-#define SDHCI_QUIRK_SINGLE_POWER_WRITE (1<<3)
+#define SDHCI_QUIRK_SINGLE_POWER_WRITE ((__force sdhci_quirk_t)(1<<3))
/* Controller has flaky internal state so reset it on each ios change */
-#define SDHCI_QUIRK_RESET_CMD_DATA_ON_IOS (1<<4)
+#define SDHCI_QUIRK_RESET_CMD_DATA_ON_IOS ((__force sdhci_quirk_t)(1<<4))
/* Controller has an unusable DMA engine */
-#define SDHCI_QUIRK_BROKEN_DMA (1<<5)
+#define SDHCI_QUIRK_BROKEN_DMA ((__force sdhci_quirk_t)(1<<5))
/* Controller has an unusable ADMA engine */
-#define SDHCI_QUIRK_BROKEN_ADMA (1<<6)
+#define SDHCI_QUIRK_BROKEN_ADMA ((__force sdhci_quirk_t)(1<<6))
/* Controller can only DMA from 32-bit aligned addresses */
-#define SDHCI_QUIRK_32BIT_DMA_ADDR (1<<7)
+#define SDHCI_QUIRK_32BIT_DMA_ADDR ((__force sdhci_quirk_t)(1<<7))
/* Controller can only DMA chunk sizes that are a multiple of 32 bits */
-#define SDHCI_QUIRK_32BIT_DMA_SIZE (1<<8)
+#define SDHCI_QUIRK_32BIT_DMA_SIZE ((__force sdhci_quirk_t)(1<<8))
/* Controller can only ADMA chunks that are a multiple of 32 bits */
-#define SDHCI_QUIRK_32BIT_ADMA_SIZE (1<<9)
+#define SDHCI_QUIRK_32BIT_ADMA_SIZE ((__force sdhci_quirk_t)(1<<9))
/* Controller needs to be reset after each request to stay stable */
-#define SDHCI_QUIRK_RESET_AFTER_REQUEST (1<<10)
+#define SDHCI_QUIRK_RESET_AFTER_REQUEST ((__force sdhci_quirk_t)(1<<10))
/* Controller needs voltage and power writes to happen separately */
-#define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER (1<<11)
+#define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER ((__force
sdhci_quirk_t)(1<<11))
/* Controller provides an incorrect timeout value for transfers */
-#define SDHCI_QUIRK_BROKEN_TIMEOUT_VAL (1<<12)
+#define SDHCI_QUIRK_BROKEN_TIMEOUT_VAL ((__force sdhci_quirk_t)(1<<12))
/* Controller has an issue with buffer bits for small transfers */
-#define SDHCI_QUIRK_BROKEN_SMALL_PIO (1<<13)
+#define SDHCI_QUIRK_BROKEN_SMALL_PIO ((__force sdhci_quirk_t)(1<<13))
/* Controller does not provide transfer-complete interrupt when not busy */
-#define SDHCI_QUIRK_NO_BUSY_IRQ (1<<14)
+#define SDHCI_QUIRK_NO_BUSY_IRQ ((__force sdhci_quirk_t)(1<<14))
/* Controller has unreliable card detection */
-#define SDHCI_QUIRK_BROKEN_CARD_DETECTION (1<<15)
+#define SDHCI_QUIRK_BROKEN_CARD_DETECTION ((__force sdhci_quirk_t)(1<<15))
/* Controller reports inverted write-protect state */
-#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT (1<<16)
+#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT ((__force
sdhci_quirk_t)(1<<16))
/* Controller does not like fast PIO transfers */
-#define SDHCI_QUIRK_PIO_NEEDS_DELAY (1<<18)
+#define SDHCI_QUIRK_PIO_NEEDS_DELAY ((__force sdhci_quirk_t)(1<<18))
/* Controller does not have a LED */
-#define SDHCI_QUIRK_NO_LED (1<<19)
+#define SDHCI_QUIRK_NO_LED ((__force sdhci_quirk_t)(1<<19))
/* Controller has to be forced to use block size of 2048 bytes */
-#define SDHCI_QUIRK_FORCE_BLK_SZ_2048 (1<<20)
+#define SDHCI_QUIRK_FORCE_BLK_SZ_2048 ((__force sdhci_quirk_t)(1<<20))
/* Controller cannot do multi-block transfers */
-#define SDHCI_QUIRK_NO_MULTIBLOCK (1<<21)
+#define SDHCI_QUIRK_NO_MULTIBLOCK ((__force sdhci_quirk_t)(1<<21))
/* Controller can only handle 1-bit data transfers */
-#define SDHCI_QUIRK_FORCE_1_BIT_DATA (1<<22)
+#define SDHCI_QUIRK_FORCE_1_BIT_DATA ((__force sdhci_quirk_t)(1<<22))
/* Controller needs 10ms delay between applying power and clock */
-#define SDHCI_QUIRK_DELAY_AFTER_POWER (1<<23)
+#define SDHCI_QUIRK_DELAY_AFTER_POWER ((__force sdhci_quirk_t)(1<<23))
/* Controller uses SDCLK instead of TMCLK for data timeouts */
-#define SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK (1<<24)
+#define SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK ((__force
sdhci_quirk_t)(1<<24))
/* Controller reports wrong base clock capability */
-#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN (1<<25)
+#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN ((__force sdhci_quirk_t)(1<<25))
/* Controller cannot support End Attribute in NOP ADMA descriptor */
-#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC (1<<26)
+#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC ((__force sdhci_quirk_t)(1<<26))
/* Controller is missing device caps. Use caps provided by host */
-#define SDHCI_QUIRK_MISSING_CAPS (1<<27)
+#define SDHCI_QUIRK_MISSING_CAPS ((__force sdhci_quirk_t)(1<<27))
/* Controller uses Auto CMD12 command to stop the transfer */
-#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 (1<<28)
+#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 ((__force
sdhci_quirk_t)(1<<28))
/* Controller doesn't have HISPD bit field in HI-SPEED SD card */
-#define SDHCI_QUIRK_NO_HISPD_BIT (1<<29)
+#define SDHCI_QUIRK_NO_HISPD_BIT ((__force sdhci_quirk_t)(1<<29))
/* Controller treats ADMA descriptors with length 0000h incorrectly */
-#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC (1<<30)
+#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC ((__force
sdhci_quirk_t)(1<<30))
/* The read-only detection via SDHCI_PRESENT_STATE register is unstable */
-#define SDHCI_QUIRK_UNSTABLE_RO_DETECT (1<<31)
+#define SDHCI_QUIRK_UNSTABLE_RO_DETECT ((__force sdhci_quirk_t)(1<<31))
- unsigned int quirks2; /* More deviations from spec. */
+ sdhci_quirk2_t quirks2; /* More deviations from spec. */
-#define SDHCI_QUIRK2_HOST_OFF_CARD_ON (1<<0)
-#define SDHCI_QUIRK2_HOST_NO_CMD23 (1<<1)
+#define SDHCI_QUIRK2_HOST_OFF_CARD_ON ((__force sdhci_quirk2_t)(1<<0))
+#define SDHCI_QUIRK2_HOST_NO_CMD23 ((__force sdhci_quirk2_t)(1<<1))
/* The system physically doesn't support 1.8v, even if the host does */
-#define SDHCI_QUIRK2_NO_1_8_V (1<<2)
-#define SDHCI_QUIRK2_PRESET_VALUE_BROKEN (1<<3)
-#define SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON (1<<4)
+#define SDHCI_QUIRK2_NO_1_8_V ((__force sdhci_quirk2_t)(1<<2))
+#define SDHCI_QUIRK2_PRESET_VALUE_BROKEN ((__force sdhci_quirk2_t)(1<<3))
+#define SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON ((__force sdhci_quirk2_t)(1<<4))
/* Controller has a non-standard host control register */
-#define SDHCI_QUIRK2_BROKEN_HOST_CONTROL (1<<5)
+#define SDHCI_QUIRK2_BROKEN_HOST_CONTROL ((__force sdhci_quirk2_t)(1<<5))
/* Controller does not support HS200 */
-#define SDHCI_QUIRK2_BROKEN_HS200 (1<<6)
+#define SDHCI_QUIRK2_BROKEN_HS200 ((__force sdhci_quirk2_t)(1<<6))
/* Controller does not support DDR50 */
-#define SDHCI_QUIRK2_BROKEN_DDR50 (1<<7)
+#define SDHCI_QUIRK2_BROKEN_DDR50 ((__force sdhci_quirk2_t)(1<<7))
/* Stop command (CMD12) can set Transfer Complete when not using
MMC_RSP_BUSY */
-#define SDHCI_QUIRK2_STOP_WITH_TC (1<<8)
+#define SDHCI_QUIRK2_STOP_WITH_TC ((__force sdhci_quirk2_t)(1<<8))
/* Controller does not support 64-bit DMA */
-#define SDHCI_QUIRK2_BROKEN_64_BIT_DMA (1<<9)
+#define SDHCI_QUIRK2_BROKEN_64_BIT_DMA ((__force sdhci_quirk2_t)(1<<9))
/* need clear transfer mode register before send cmd */
-#define SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD (1<<10)
+#define SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD ((__force
sdhci_quirk2_t)(1<<10))
/* Capability register bit-63 indicates HS400 support */
-#define SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 (1<<11)
+#define SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 ((__force
sdhci_quirk2_t)(1<<11))
/* forced tuned clock */
-#define SDHCI_QUIRK2_TUNING_WORK_AROUND (1<<12)
+#define SDHCI_QUIRK2_TUNING_WORK_AROUND ((__force sdhci_quirk2_t)(1<<12))
/* disable the block count for single block transactions */
-#define SDHCI_QUIRK2_SUPPORT_SINGLE (1<<13)
+#define SDHCI_QUIRK2_SUPPORT_SINGLE ((__force sdhci_quirk2_t)(1<<13))
/* Controller broken with using ACMD23 */
-#define SDHCI_QUIRK2_ACMD23_BROKEN (1<<14)
+#define SDHCI_QUIRK2_ACMD23_BROKEN ((__force sdhci_quirk2_t)(1<<14))
/* Broken Clock divider zero in controller */
-#define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN (1<<15)
+#define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN ((__force
sdhci_quirk2_t)(1<<15))
/* Controller has CRC in 136 bit Command Response */
-#define SDHCI_QUIRK2_RSP_136_HAS_CRC (1<<16)
+#define SDHCI_QUIRK2_RSP_136_HAS_CRC ((__force sdhci_quirk2_t)(1<<16))
/*
* Disable HW timeout if the requested timeout is more than the maximum
* obtainable timeout.
*/
-#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT (1<<17)
+#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT ((__force sdhci_quirk2_t)(1<<17))
/*
* 32-bit block count may not support eMMC where upper bits of CMD23
are used
* for other purposes. Consequently we support 16-bit block count by
default.
* Otherwise, SDHCI_QUIRK2_USE_32BIT_BLK_CNT can be selected to use 32-bit
* block count.
*/
-#define SDHCI_QUIRK2_USE_32BIT_BLK_CNT (1<<18)
+#define SDHCI_QUIRK2_USE_32BIT_BLK_CNT ((__force sdhci_quirk2_t)(1<<18))
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
On 3/12/19 12:49 pm, Rasmus Villemoes wrote:
> On 03/12/2019 11.15, Rasmus Villemoes wrote:
>> Hi
>>
>> Commits
>>
>> 05cb6b2a66fa - mmc: sdhci-of-esdhc: add erratum eSDHC-A001 and A-008358
>> support
>> a46e42712596 - mmc: sdhci-of-esdhc: add erratum eSDHC5 support
>>
>> seem a bit odd, in that they set bits from the SDHCI_* namespace in the
>> ->quirks2 member:
>>
>> host->quirks2 |= SDHCI_QUIRK_RESET_AFTER_REQUEST;
>> host->quirks2 |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> FWIW, with something like the patch below, sparse (make C=1) would complain
>
> drivers/mmc/host/sdhci-of-esdhc.c:1306:31: warning: invalid assignment: |=
> drivers/mmc/host/sdhci-of-esdhc.c:1306:31: left side has type
> restricted sdhci_quirk2_t
> drivers/mmc/host/sdhci-of-esdhc.c:1306:31: right side has type
> restricted sdhci_quirk_t
> drivers/mmc/host/sdhci-of-esdhc.c:1307:31: warning: invalid assignment: |=
> drivers/mmc/host/sdhci-of-esdhc.c:1307:31: left side has type
> restricted sdhci_quirk2_t
> drivers/mmc/host/sdhci-of-esdhc.c:1307:31: right side has type
> restricted sdhci_quirk_t
>
> But maybe that's too much churn, dunno.
Presumably testing would have caught this, so I think it would be better if
people tested their patches.