[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] sd: sdhci: Implement basic vendor specific register supp
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 1/2] sd: sdhci: Implement basic vendor specific register support |
Date: |
Wed, 3 Jun 2020 08:37:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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.
>