| Message ID | 20200603052404.30788-2-linux@roeck-us.net |
|---|---|
| State | New |
| Headers | show |
| Series | sd: sdhci: Implement basic vendor specific register support | expand |
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. >
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. >> >
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. >>> >> > >
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.
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(-)