[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: |
Guenter Roeck |
Subject: |
Re: [PATCH 1/2] sd: sdhci: Implement basic vendor specific register support |
Date: |
Tue, 2 Jun 2020 23:58:58 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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.
>>
>