* [PATCH 0/2] sd: sdhci: Implement basic vendor specific register support
@ 2020-06-03 5:24 Guenter Roeck
2020-06-03 5:24 ` [PATCH 1/2] " Guenter Roeck
2020-06-03 5:24 ` [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations Guenter Roeck
0 siblings, 2 replies; 8+ messages in thread
From: Guenter Roeck @ 2020-06-03 5:24 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrey Smirnov, qemu-devel, Jean-Christophe Dubois, qemu-arm,
Philippe Mathieu-Daudé,
Guenter Roeck
The Linux kernel's IMX code now uses vendor specific commands.
This results in endless warnings when booting the Linux kernel.
sdhci-esdhc-imx 2194000.usdhc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
Implement support for the vendor specific command implemented in IMX
SDHCI hardware to be able to avoid this warning.
Patch 1/2 implements vendor specific command support in the SDHCI core
code. At this time, only IMX vendor command support is implemented,
but the implementation is written with expandability in mind.
Patch 2/2 enables IMX SDHCI vendor extensions for all affected emulations.
----------------------------------------------------------------
Guenter Roeck (2):
sd: sdhci: Implement basic vendor specific register support
hw: arm: Set vendor property for IMX SDHCI emulations
hw/arm/fsl-imx25.c | 2 ++
hw/arm/fsl-imx6.c | 2 ++
hw/arm/fsl-imx6ul.c | 2 ++
hw/arm/fsl-imx7.c | 2 ++
hw/sd/sdhci-internal.h | 5 +++++
hw/sd/sdhci.c | 18 +++++++++++++++++-
include/hw/sd/sdhci.h | 5 +++++
7 files changed, 35 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] sd: sdhci: Implement basic vendor specific register support
2020-06-03 5:24 [PATCH 0/2] sd: sdhci: Implement basic vendor specific register support Guenter Roeck
@ 2020-06-03 5:24 ` Guenter Roeck
2020-06-03 6:37 ` Philippe Mathieu-Daudé
2020-06-03 5:24 ` [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations Guenter Roeck
1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2020-06-03 5:24 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrey Smirnov, qemu-devel, Jean-Christophe Dubois, qemu-arm,
Philippe Mathieu-Daudé,
Guenter Roeck
The Linux kernel's IMX code now uses vendor specific commands.
This results in endless warnings when booting the Linux kernel.
sdhci-esdhc-imx 2194000.usdhc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
Implement support for the vendor specific command implemented in IMX hardware
to be able to avoid this warning.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
hw/sd/sdhci-internal.h | 5 +++++
hw/sd/sdhci.c | 18 +++++++++++++++++-
include/hw/sd/sdhci.h | 5 +++++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index e7c8a523b5..e8c753d6d1 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -75,6 +75,7 @@
#define SDHC_CMD_INHIBIT 0x00000001
#define SDHC_DATA_INHIBIT 0x00000002
#define SDHC_DAT_LINE_ACTIVE 0x00000004
+#define SDHC_IMX_CLOCK_GATE_OFF 0x00000080
#define SDHC_DOING_WRITE 0x00000100
#define SDHC_DOING_READ 0x00000200
#define SDHC_SPACE_AVAILABLE 0x00000400
@@ -289,7 +290,10 @@ extern const VMStateDescription sdhci_vmstate;
#define ESDHC_MIX_CTRL 0x48
+
#define ESDHC_VENDOR_SPEC 0xc0
+#define ESDHC_IMX_FRC_SDCLK_ON (1 << 8)
+
#define ESDHC_DLL_CTRL 0x60
#define ESDHC_TUNING_CTRL 0xcc
@@ -326,6 +330,7 @@ extern const VMStateDescription sdhci_vmstate;
#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
+ DEFINE_PROP_UINT8("vendor", _state, vendor, SDHCI_VENDOR_NONE), \
\
/* Capabilities registers provide information on supported
* features of this specific host controller implementation */ \
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1b75d7bab9..eb2be6529e 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1569,11 +1569,13 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
}
break;
+ case ESDHC_VENDOR_SPEC:
+ ret = s->vendor_spec;
+ break;
case ESDHC_DLL_CTRL:
case ESDHC_TUNE_CTRL_STATUS:
case ESDHC_UNDOCUMENTED_REG27:
case ESDHC_TUNING_CTRL:
- case ESDHC_VENDOR_SPEC:
case ESDHC_MIX_CTRL:
case ESDHC_WTMK_LVL:
ret = 0;
@@ -1596,7 +1598,21 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
case ESDHC_UNDOCUMENTED_REG27:
case ESDHC_TUNING_CTRL:
case ESDHC_WTMK_LVL:
+ break;
+
case ESDHC_VENDOR_SPEC:
+ s->vendor_spec = value;
+ switch (s->vendor) {
+ case SDHCI_VENDOR_IMX:
+ if (value & ESDHC_IMX_FRC_SDCLK_ON) {
+ s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
+ } else {
+ s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
+ }
+ break;
+ default:
+ break;
+ }
break;
case SDHC_HOSTCTL:
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index c6868c9699..5d9275f3d6 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -74,6 +74,7 @@ typedef struct SDHCIState {
uint16_t acmd12errsts; /* Auto CMD12 error status register */
uint16_t hostctl2; /* Host Control 2 */
uint64_t admasysaddr; /* ADMA System Address Register */
+ uint16_t vendor_spec; /* Vendor specific register */
/* Read-only registers */
uint64_t capareg; /* Capabilities Register */
@@ -96,8 +97,12 @@ typedef struct SDHCIState {
uint32_t quirks;
uint8_t sd_spec_version;
uint8_t uhs_mode;
+ uint8_t vendor; /* For vendor specific functionality */
} SDHCIState;
+#define SDHCI_VENDOR_NONE 0
+#define SDHCI_VENDOR_IMX 1
+
/*
* Controller does not provide transfer-complete interrupt when not
* busy.
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations
2020-06-03 5:24 [PATCH 0/2] sd: sdhci: Implement basic vendor specific register support Guenter Roeck
2020-06-03 5:24 ` [PATCH 1/2] " Guenter Roeck
@ 2020-06-03 5:24 ` Guenter Roeck
2020-06-03 6:35 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2020-06-03 5:24 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrey Smirnov, qemu-devel, Jean-Christophe Dubois, qemu-arm,
Philippe Mathieu-Daudé,
Guenter Roeck
Set vendor property to IMX to enable IMX specific functionality
in sdhci code.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
hw/arm/fsl-imx25.c | 2 ++
hw/arm/fsl-imx6.c | 2 ++
hw/arm/fsl-imx6ul.c | 2 ++
hw/arm/fsl-imx7.c | 2 ++
4 files changed, 8 insertions(+)
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index cdaa79c26b..2cbd985e93 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -274,6 +274,8 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
&err);
object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES,
"capareg", &err);
+ object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
+ "vendor", &err);
object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index f58c85aa8c..8e9a94e4d7 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -350,6 +350,8 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
&err);
object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES,
"capareg", &err);
+ object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
+ "vendor", &err);
object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index 3ecb212da6..ce1462927c 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -505,6 +505,8 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
FSL_IMX6UL_USDHC2_IRQ,
};
+ object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX,
+ "vendor", &error_abort);
object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized",
&error_abort);
diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 89c3b64c06..dbf16b2814 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -416,6 +416,8 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
FSL_IMX7_USDHC3_IRQ,
};
+ object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX,
+ "vendor", &error_abort);
object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized",
&error_abort);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations
2020-06-03 5:24 ` [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations Guenter Roeck
@ 2020-06-03 6:35 ` Philippe Mathieu-Daudé
2020-06-03 7:02 ` Guenter Roeck
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-03 6:35 UTC (permalink / raw)
To: Guenter Roeck, Peter Maydell
Cc: Andrey Smirnov, qemu-arm, qemu-devel, Jean-Christophe Dubois
Hi Guenter,
On 6/3/20 7:24 AM, Guenter Roeck wrote:
> Set vendor property to IMX to enable IMX specific functionality
> in sdhci code.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> hw/arm/fsl-imx25.c | 2 ++
> hw/arm/fsl-imx6.c | 2 ++
> hw/arm/fsl-imx6ul.c | 2 ++
> hw/arm/fsl-imx7.c | 2 ++
> 4 files changed, 8 insertions(+)
>
> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
> index cdaa79c26b..2cbd985e93 100644
> --- a/hw/arm/fsl-imx25.c
> +++ b/hw/arm/fsl-imx25.c
> @@ -274,6 +274,8 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
> &err);
> object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES,
> "capareg", &err);
> + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
> + "vendor", &err);
Either check &err, or use &error_abort.
You can see a fix here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg695544.html
Otherwise:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> index f58c85aa8c..8e9a94e4d7 100644
> --- a/hw/arm/fsl-imx6.c
> +++ b/hw/arm/fsl-imx6.c
> @@ -350,6 +350,8 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
> &err);
> object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES,
> "capareg", &err);
> + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
> + "vendor", &err);
> object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 3ecb212da6..ce1462927c 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -505,6 +505,8 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
> FSL_IMX6UL_USDHC2_IRQ,
> };
>
> + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX,
> + "vendor", &error_abort);
> object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized",
> &error_abort);
>
> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> index 89c3b64c06..dbf16b2814 100644
> --- a/hw/arm/fsl-imx7.c
> +++ b/hw/arm/fsl-imx7.c
> @@ -416,6 +416,8 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
> FSL_IMX7_USDHC3_IRQ,
> };
>
> + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX,
> + "vendor", &error_abort);
> object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized",
> &error_abort);
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] sd: sdhci: Implement basic vendor specific register support
2020-06-03 5:24 ` [PATCH 1/2] " Guenter Roeck
@ 2020-06-03 6:37 ` Philippe Mathieu-Daudé
2020-06-03 6:58 ` Guenter Roeck
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-03 6:37 UTC (permalink / raw)
To: Guenter Roeck, Peter Maydell
Cc: Andrey Smirnov, qemu-arm, qemu-devel, Jean-Christophe Dubois
Hi Guenter,
On 6/3/20 7:24 AM, Guenter Roeck wrote:
> The Linux kernel's IMX code now uses vendor specific commands.
> This results in endless warnings when booting the Linux kernel.
>
> sdhci-esdhc-imx 2194000.usdhc: esdhc_wait_for_card_clock_gate_off:
> card clock still not gate off in 100us!.
>
> Implement support for the vendor specific command implemented in IMX hardware
> to be able to avoid this warning.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> hw/sd/sdhci-internal.h | 5 +++++
> hw/sd/sdhci.c | 18 +++++++++++++++++-
> include/hw/sd/sdhci.h | 5 +++++
> 3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index e7c8a523b5..e8c753d6d1 100644
> --- a/hw/sd/sdhci-internal.h
> +++ b/hw/sd/sdhci-internal.h
> @@ -75,6 +75,7 @@
> #define SDHC_CMD_INHIBIT 0x00000001
> #define SDHC_DATA_INHIBIT 0x00000002
> #define SDHC_DAT_LINE_ACTIVE 0x00000004
> +#define SDHC_IMX_CLOCK_GATE_OFF 0x00000080
> #define SDHC_DOING_WRITE 0x00000100
> #define SDHC_DOING_READ 0x00000200
> #define SDHC_SPACE_AVAILABLE 0x00000400
> @@ -289,7 +290,10 @@ extern const VMStateDescription sdhci_vmstate;
>
>
> #define ESDHC_MIX_CTRL 0x48
> +
> #define ESDHC_VENDOR_SPEC 0xc0
> +#define ESDHC_IMX_FRC_SDCLK_ON (1 << 8)
I searched for the datasheet but couldn't find any, so I suppose it is
only available under NDA. I can not review much (in particular I wanted
to check the register sizes), anyway the overall looks OK:
Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Also:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> +
> #define ESDHC_DLL_CTRL 0x60
>
> #define ESDHC_TUNING_CTRL 0xcc
> @@ -326,6 +330,7 @@ extern const VMStateDescription sdhci_vmstate;
> #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
> DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
> DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
> + DEFINE_PROP_UINT8("vendor", _state, vendor, SDHCI_VENDOR_NONE), \
> \
> /* Capabilities registers provide information on supported
> * features of this specific host controller implementation */ \
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 1b75d7bab9..eb2be6529e 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1569,11 +1569,13 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
> }
> break;
>
> + case ESDHC_VENDOR_SPEC:
> + ret = s->vendor_spec;
> + break;
> case ESDHC_DLL_CTRL:
> case ESDHC_TUNE_CTRL_STATUS:
> case ESDHC_UNDOCUMENTED_REG27:
> case ESDHC_TUNING_CTRL:
> - case ESDHC_VENDOR_SPEC:
> case ESDHC_MIX_CTRL:
> case ESDHC_WTMK_LVL:
> ret = 0;
> @@ -1596,7 +1598,21 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
> case ESDHC_UNDOCUMENTED_REG27:
> case ESDHC_TUNING_CTRL:
> case ESDHC_WTMK_LVL:
> + break;
> +
> case ESDHC_VENDOR_SPEC:
> + s->vendor_spec = value;
> + switch (s->vendor) {
> + case SDHCI_VENDOR_IMX:
> + if (value & ESDHC_IMX_FRC_SDCLK_ON) {
> + s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
> + } else {
> + s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
> + }
> + break;
> + default:
> + break;
> + }
> break;
>
> case SDHC_HOSTCTL:
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index c6868c9699..5d9275f3d6 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -74,6 +74,7 @@ typedef struct SDHCIState {
> uint16_t acmd12errsts; /* Auto CMD12 error status register */
> uint16_t hostctl2; /* Host Control 2 */
> uint64_t admasysaddr; /* ADMA System Address Register */
> + uint16_t vendor_spec; /* Vendor specific register */
>
> /* Read-only registers */
> uint64_t capareg; /* Capabilities Register */
> @@ -96,8 +97,12 @@ typedef struct SDHCIState {
> uint32_t quirks;
> uint8_t sd_spec_version;
> uint8_t uhs_mode;
> + uint8_t vendor; /* For vendor specific functionality */
> } SDHCIState;
>
> +#define SDHCI_VENDOR_NONE 0
> +#define SDHCI_VENDOR_IMX 1
> +
> /*
> * Controller does not provide transfer-complete interrupt when not
> * busy.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] sd: sdhci: Implement basic vendor specific register support
2020-06-03 6:37 ` Philippe Mathieu-Daudé
@ 2020-06-03 6:58 ` Guenter Roeck
2020-06-03 8:32 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2020-06-03 6:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: Andrey Smirnov, qemu-arm, qemu-devel, Jean-Christophe Dubois
On 6/2/20 11:37 PM, Philippe Mathieu-Daudé wrote:
> Hi Guenter,
>
> On 6/3/20 7:24 AM, Guenter Roeck wrote:
>> The Linux kernel's IMX code now uses vendor specific commands.
>> This results in endless warnings when booting the Linux kernel.
>>
>> sdhci-esdhc-imx 2194000.usdhc: esdhc_wait_for_card_clock_gate_off:
>> card clock still not gate off in 100us!.
>>
>> Implement support for the vendor specific command implemented in IMX hardware
>> to be able to avoid this warning.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> hw/sd/sdhci-internal.h | 5 +++++
>> hw/sd/sdhci.c | 18 +++++++++++++++++-
>> include/hw/sd/sdhci.h | 5 +++++
>> 3 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
>> index e7c8a523b5..e8c753d6d1 100644
>> --- a/hw/sd/sdhci-internal.h
>> +++ b/hw/sd/sdhci-internal.h
>> @@ -75,6 +75,7 @@
>> #define SDHC_CMD_INHIBIT 0x00000001
>> #define SDHC_DATA_INHIBIT 0x00000002
>> #define SDHC_DAT_LINE_ACTIVE 0x00000004
>> +#define SDHC_IMX_CLOCK_GATE_OFF 0x00000080
>> #define SDHC_DOING_WRITE 0x00000100
>> #define SDHC_DOING_READ 0x00000200
>> #define SDHC_SPACE_AVAILABLE 0x00000400
>> @@ -289,7 +290,10 @@ extern const VMStateDescription sdhci_vmstate;
>>
>>
>> #define ESDHC_MIX_CTRL 0x48
>> +
>> #define ESDHC_VENDOR_SPEC 0xc0
>> +#define ESDHC_IMX_FRC_SDCLK_ON (1 << 8)
>
> I searched for the datasheet but couldn't find any, so I suppose it is
> only available under NDA. I can not review much (in particular I wanted
> to check the register sizes), anyway the overall looks OK:
>
Actually, I only had to register an account to be able to download
the datasheets from NXP. Register width is 32 bit.
> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Also:
>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
Thanks!
Guenter
>> +
>> #define ESDHC_DLL_CTRL 0x60
>>
>> #define ESDHC_TUNING_CTRL 0xcc
>> @@ -326,6 +330,7 @@ extern const VMStateDescription sdhci_vmstate;
>> #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>> DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
>> DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
>> + DEFINE_PROP_UINT8("vendor", _state, vendor, SDHCI_VENDOR_NONE), \
>> \
>> /* Capabilities registers provide information on supported
>> * features of this specific host controller implementation */ \
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index 1b75d7bab9..eb2be6529e 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -1569,11 +1569,13 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>> }
>> break;
>>
>> + case ESDHC_VENDOR_SPEC:
>> + ret = s->vendor_spec;
>> + break;
>> case ESDHC_DLL_CTRL:
>> case ESDHC_TUNE_CTRL_STATUS:
>> case ESDHC_UNDOCUMENTED_REG27:
>> case ESDHC_TUNING_CTRL:
>> - case ESDHC_VENDOR_SPEC:
>> case ESDHC_MIX_CTRL:
>> case ESDHC_WTMK_LVL:
>> ret = 0;
>> @@ -1596,7 +1598,21 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>> case ESDHC_UNDOCUMENTED_REG27:
>> case ESDHC_TUNING_CTRL:
>> case ESDHC_WTMK_LVL:
>> + break;
>> +
>> case ESDHC_VENDOR_SPEC:
>> + s->vendor_spec = value;
>> + switch (s->vendor) {
>> + case SDHCI_VENDOR_IMX:
>> + if (value & ESDHC_IMX_FRC_SDCLK_ON) {
>> + s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
>> + } else {
>> + s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> break;
>>
>> case SDHC_HOSTCTL:
>> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
>> index c6868c9699..5d9275f3d6 100644
>> --- a/include/hw/sd/sdhci.h
>> +++ b/include/hw/sd/sdhci.h
>> @@ -74,6 +74,7 @@ typedef struct SDHCIState {
>> uint16_t acmd12errsts; /* Auto CMD12 error status register */
>> uint16_t hostctl2; /* Host Control 2 */
>> uint64_t admasysaddr; /* ADMA System Address Register */
>> + uint16_t vendor_spec; /* Vendor specific register */
>>
>> /* Read-only registers */
>> uint64_t capareg; /* Capabilities Register */
>> @@ -96,8 +97,12 @@ typedef struct SDHCIState {
>> uint32_t quirks;
>> uint8_t sd_spec_version;
>> uint8_t uhs_mode;
>> + uint8_t vendor; /* For vendor specific functionality */
>> } SDHCIState;
>>
>> +#define SDHCI_VENDOR_NONE 0
>> +#define SDHCI_VENDOR_IMX 1
>> +
>> /*
>> * Controller does not provide transfer-complete interrupt when not
>> * busy.
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations
2020-06-03 6:35 ` Philippe Mathieu-Daudé
@ 2020-06-03 7:02 ` Guenter Roeck
0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2020-06-03 7:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: Andrey Smirnov, qemu-arm, qemu-devel, Jean-Christophe Dubois
On 6/2/20 11:35 PM, Philippe Mathieu-Daudé wrote:
> Hi Guenter,
>
> On 6/3/20 7:24 AM, Guenter Roeck wrote:
>> Set vendor property to IMX to enable IMX specific functionality
>> in sdhci code.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> hw/arm/fsl-imx25.c | 2 ++
>> hw/arm/fsl-imx6.c | 2 ++
>> hw/arm/fsl-imx6ul.c | 2 ++
>> hw/arm/fsl-imx7.c | 2 ++
>> 4 files changed, 8 insertions(+)
>>
>> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
>> index cdaa79c26b..2cbd985e93 100644
>> --- a/hw/arm/fsl-imx25.c
>> +++ b/hw/arm/fsl-imx25.c
>> @@ -274,6 +274,8 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
>> &err);
>> object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES,
>> "capareg", &err);
>> + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
>> + "vendor", &err);
>
> Either check &err, or use &error_abort.
>
Ok, I'll follow the guidance from the patch pointed to below
and add the error check.
Thanks,
Guenter
> You can see a fix here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg695544.html
>
> Otherwise:
>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
>> if (err) {
>> error_propagate(errp, err);
>> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
>> index f58c85aa8c..8e9a94e4d7 100644
>> --- a/hw/arm/fsl-imx6.c
>> +++ b/hw/arm/fsl-imx6.c
>> @@ -350,6 +350,8 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>> &err);
>> object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES,
>> "capareg", &err);
>> + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
>> + "vendor", &err);
>> object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
>> if (err) {
>> error_propagate(errp, err);
>> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
>> index 3ecb212da6..ce1462927c 100644
>> --- a/hw/arm/fsl-imx6ul.c
>> +++ b/hw/arm/fsl-imx6ul.c
>> @@ -505,6 +505,8 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
>> FSL_IMX6UL_USDHC2_IRQ,
>> };
>>
>> + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX,
>> + "vendor", &error_abort);
>> object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized",
>> &error_abort);
>>
>> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
>> index 89c3b64c06..dbf16b2814 100644
>> --- a/hw/arm/fsl-imx7.c
>> +++ b/hw/arm/fsl-imx7.c
>> @@ -416,6 +416,8 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
>> FSL_IMX7_USDHC3_IRQ,
>> };
>>
>> + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX,
>> + "vendor", &error_abort);
>> object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized",
>> &error_abort);
>>
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] sd: sdhci: Implement basic vendor specific register support
2020-06-03 6:58 ` Guenter Roeck
@ 2020-06-03 8:32 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-03 8:32 UTC (permalink / raw)
To: Guenter Roeck, Peter Maydell
Cc: Andrey Smirnov, qemu-arm, qemu-devel, Jean-Christophe Dubois
On 6/3/20 8:58 AM, Guenter Roeck wrote:
> On 6/2/20 11:37 PM, Philippe Mathieu-Daudé wrote:
>> Hi Guenter,
>>
>> On 6/3/20 7:24 AM, Guenter Roeck wrote:
>>> The Linux kernel's IMX code now uses vendor specific commands.
>>> This results in endless warnings when booting the Linux kernel.
>>>
>>> sdhci-esdhc-imx 2194000.usdhc: esdhc_wait_for_card_clock_gate_off:
>>> card clock still not gate off in 100us!.
>>>
>>> Implement support for the vendor specific command implemented in IMX hardware
>>> to be able to avoid this warning.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> hw/sd/sdhci-internal.h | 5 +++++
>>> hw/sd/sdhci.c | 18 +++++++++++++++++-
>>> include/hw/sd/sdhci.h | 5 +++++
>>> 3 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
>>> index e7c8a523b5..e8c753d6d1 100644
>>> --- a/hw/sd/sdhci-internal.h
>>> +++ b/hw/sd/sdhci-internal.h
>>> @@ -75,6 +75,7 @@
>>> #define SDHC_CMD_INHIBIT 0x00000001
>>> #define SDHC_DATA_INHIBIT 0x00000002
>>> #define SDHC_DAT_LINE_ACTIVE 0x00000004
>>> +#define SDHC_IMX_CLOCK_GATE_OFF 0x00000080
>>> #define SDHC_DOING_WRITE 0x00000100
>>> #define SDHC_DOING_READ 0x00000200
>>> #define SDHC_SPACE_AVAILABLE 0x00000400
>>> @@ -289,7 +290,10 @@ extern const VMStateDescription sdhci_vmstate;
>>>
>>>
>>> #define ESDHC_MIX_CTRL 0x48
>>> +
>>> #define ESDHC_VENDOR_SPEC 0xc0
>>> +#define ESDHC_IMX_FRC_SDCLK_ON (1 << 8)
>>
>> I searched for the datasheet but couldn't find any, so I suppose it is
>> only available under NDA. I can not review much (in particular I wanted
>> to check the register sizes), anyway the overall looks OK:
>>
>
> Actually, I only had to register an account to be able to download
> the datasheets from NXP. Register width is 32 bit.
Yes, thanks for the tip!
"10.3.8.28 Vendor Specific Register (uSDHCx_VEND_SPEC)"
>
>> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ this can be changed by:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Also:
>>
>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
> Thanks!
>
> Guenter
>
>>> +
>>> #define ESDHC_DLL_CTRL 0x60
>>>
>>> #define ESDHC_TUNING_CTRL 0xcc
>>> @@ -326,6 +330,7 @@ extern const VMStateDescription sdhci_vmstate;
>>> #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>>> DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
>>> DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
>>> + DEFINE_PROP_UINT8("vendor", _state, vendor, SDHCI_VENDOR_NONE), \
>>> \
>>> /* Capabilities registers provide information on supported
>>> * features of this specific host controller implementation */ \
>>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>>> index 1b75d7bab9..eb2be6529e 100644
>>> --- a/hw/sd/sdhci.c
>>> +++ b/hw/sd/sdhci.c
>>> @@ -1569,11 +1569,13 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, unsigned size)
>>> }
>>> break;
>>>
>>> + case ESDHC_VENDOR_SPEC:
>>> + ret = s->vendor_spec;
>>> + break;
>>> case ESDHC_DLL_CTRL:
>>> case ESDHC_TUNE_CTRL_STATUS:
>>> case ESDHC_UNDOCUMENTED_REG27:
>>> case ESDHC_TUNING_CTRL:
>>> - case ESDHC_VENDOR_SPEC:
>>> case ESDHC_MIX_CTRL:
>>> case ESDHC_WTMK_LVL:
>>> ret = 0;
>>> @@ -1596,7 +1598,21 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>>> case ESDHC_UNDOCUMENTED_REG27:
>>> case ESDHC_TUNING_CTRL:
>>> case ESDHC_WTMK_LVL:
>>> + break;
>>> +
>>> case ESDHC_VENDOR_SPEC:
>>> + s->vendor_spec = value;
>>> + switch (s->vendor) {
>>> + case SDHCI_VENDOR_IMX:
>>> + if (value & ESDHC_IMX_FRC_SDCLK_ON) {
>>> + s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
>>> + } else {
>>> + s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
>>> + }
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> break;
>>>
>>> case SDHC_HOSTCTL:
>>> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
>>> index c6868c9699..5d9275f3d6 100644
>>> --- a/include/hw/sd/sdhci.h
>>> +++ b/include/hw/sd/sdhci.h
>>> @@ -74,6 +74,7 @@ typedef struct SDHCIState {
>>> uint16_t acmd12errsts; /* Auto CMD12 error status register */
>>> uint16_t hostctl2; /* Host Control 2 */
>>> uint64_t admasysaddr; /* ADMA System Address Register */
>>> + uint16_t vendor_spec; /* Vendor specific register */
>>>
>>> /* Read-only registers */
>>> uint64_t capareg; /* Capabilities Register */
>>> @@ -96,8 +97,12 @@ typedef struct SDHCIState {
>>> uint32_t quirks;
>>> uint8_t sd_spec_version;
>>> uint8_t uhs_mode;
>>> + uint8_t vendor; /* For vendor specific functionality */
>>> } SDHCIState;
>>>
>>> +#define SDHCI_VENDOR_NONE 0
>>> +#define SDHCI_VENDOR_IMX 1
>>> +
>>> /*
>>> * Controller does not provide transfer-complete interrupt when not
>>> * busy.
>>>
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-06-03 8:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 5:24 [PATCH 0/2] sd: sdhci: Implement basic vendor specific register support Guenter Roeck
2020-06-03 5:24 ` [PATCH 1/2] " Guenter Roeck
2020-06-03 6:37 ` Philippe Mathieu-Daudé
2020-06-03 6:58 ` Guenter Roeck
2020-06-03 8:32 ` Philippe Mathieu-Daudé
2020-06-03 5:24 ` [PATCH 2/2] hw: arm: Set vendor property for IMX SDHCI emulations Guenter Roeck
2020-06-03 6:35 ` Philippe Mathieu-Daudé
2020-06-03 7:02 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).