[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 10:32:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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.
>>>
>>
>
>