* UHS-1 cards with iMX6Q
@ 2014-02-07 15:29 Russell King - ARM Linux
2014-02-07 15:59 ` Shawn Guo
0 siblings, 1 reply; 8+ messages in thread
From: Russell King - ARM Linux @ 2014-02-07 15:29 UTC (permalink / raw)
To: Dong Aisheng, Shawn Guo, Chris Ball; +Cc: linux-mmc
Hi,
I have a Samsung 8GB UHS-1 card which I'm trying with iMX6Q, and it's
not behaving very well with the voltage switch. With MMC debugging
enabled - and augmented with additional debug for the ESDHC_VENDOR
register, I'm seeing this with 3.14-rc1:
[ 2.771270] mmc1: starting CMD11 arg 00000000 flags 00000015
[ 2.771575] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000080
[ 2.771613] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000001
[ 2.771632] mmc1: req done (CMD11): 0: 00000320 00000000 00000000 00000000
[ 2.772669] mmc1: clock 0Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
[ 2.772679] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
[ 2.772687] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
[ 2.772695] mmc1: clearing ESDHC_VENDOR_SPEC_VSELECT
[ 2.772703] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
[ 2.772713] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
[ 2.772719] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
[ 2.772729] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
[ 2.772735] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
[ 2.778276] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
[ 2.786482] mmc1: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
[ 2.786502] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
[ 2.786511] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
[ 2.787515] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
[ 2.787522] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
[ 2.787529] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
[ 2.787536] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
[ 2.787548] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
[ 2.787554] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
[ 2.789560] mmc1: card failed to indicate switch to low voltage mode
This appears to correspond with the sequence:
CMD11 -> clock off -> set vselect -> clock on -> clock off ->
set pinctrl -> clock on -> test D[3:0]
which appears not to be the expected sequence - the expected sequence
should be:
CMD11 -> clock off -> set vselect -> clock on -> test D[3:0]
maybe with the setting of the IOMUX settings somewhere in there -
probably at the point where the clock is off - the additional clock
off/clock on step looks to me incorrect.
In any case, I've also augmented the other failure paths in
mmc_set_signal_voltage(), and it's always this one (the last step)
which fails:
if (host->ops->card_busy && host->ops->card_busy(host)) {
pr_debug("%s: card failed to indicate switch to low voltage mode\n",
mmc_hostname(host));
err = -EAGAIN;
}
Any ideas?
Thanks.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-07 15:29 UHS-1 cards with iMX6Q Russell King - ARM Linux
@ 2014-02-07 15:59 ` Shawn Guo
2014-02-07 16:16 ` Russell King - ARM Linux
2014-02-09 15:14 ` Russell King - ARM Linux
0 siblings, 2 replies; 8+ messages in thread
From: Shawn Guo @ 2014-02-07 15:59 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Dong Aisheng, Chris Ball, linux-mmc
On Fri, Feb 07, 2014 at 03:29:51PM +0000, Russell King - ARM Linux wrote:
> Hi,
>
> I have a Samsung 8GB UHS-1 card which I'm trying with iMX6Q, and it's
> not behaving very well with the voltage switch. With MMC debugging
> enabled - and augmented with additional debug for the ESDHC_VENDOR
> register, I'm seeing this with 3.14-rc1:
>
> [ 2.771270] mmc1: starting CMD11 arg 00000000 flags 00000015
> [ 2.771575] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000080
> [ 2.771613] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000001
> [ 2.771632] mmc1: req done (CMD11): 0: 00000320 00000000 00000000 00000000
> [ 2.772669] mmc1: clock 0Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
> [ 2.772679] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> [ 2.772687] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
> [ 2.772695] mmc1: clearing ESDHC_VENDOR_SPEC_VSELECT
> [ 2.772703] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> [ 2.772713] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
> [ 2.772719] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> [ 2.772729] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
> [ 2.772735] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
> [ 2.778276] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
> [ 2.786482] mmc1: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
> [ 2.786502] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
> [ 2.786511] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> [ 2.787515] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
> [ 2.787522] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
> [ 2.787529] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> [ 2.787536] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
> [ 2.787548] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
> [ 2.787554] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> [ 2.789560] mmc1: card failed to indicate switch to low voltage mode
>
> This appears to correspond with the sequence:
>
> CMD11 -> clock off -> set vselect -> clock on -> clock off ->
> set pinctrl -> clock on -> test D[3:0]
>
> which appears not to be the expected sequence - the expected sequence
> should be:
>
> CMD11 -> clock off -> set vselect -> clock on -> test D[3:0]
>
> maybe with the setting of the IOMUX settings somewhere in there -
> probably at the point where the clock is off - the additional clock
> off/clock on step looks to me incorrect.
>
> In any case, I've also augmented the other failure paths in
> mmc_set_signal_voltage(), and it's always this one (the last step)
> which fails:
>
> if (host->ops->card_busy && host->ops->card_busy(host)) {
> pr_debug("%s: card failed to indicate switch to low voltage mode\n",
> mmc_hostname(host));
> err = -EAGAIN;
> }
>
> Any ideas?
I'm not sure if it's the cause or you have it set up or not, but I
remember that we need to have 3 pinctrl states for 50MHz, 100MHz and
200MHz to get UHS card to work. You can look at
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi for example.
Shawn
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-07 15:59 ` Shawn Guo
@ 2014-02-07 16:16 ` Russell King - ARM Linux
2014-02-08 13:55 ` Dong Aisheng
2014-02-09 15:14 ` Russell King - ARM Linux
1 sibling, 1 reply; 8+ messages in thread
From: Russell King - ARM Linux @ 2014-02-07 16:16 UTC (permalink / raw)
To: Shawn Guo; +Cc: Dong Aisheng, Chris Ball, linux-mmc
On Fri, Feb 07, 2014 at 11:59:23PM +0800, Shawn Guo wrote:
> On Fri, Feb 07, 2014 at 03:29:51PM +0000, Russell King - ARM Linux wrote:
> > Hi,
> >
> > I have a Samsung 8GB UHS-1 card which I'm trying with iMX6Q, and it's
> > not behaving very well with the voltage switch. With MMC debugging
> > enabled - and augmented with additional debug for the ESDHC_VENDOR
> > register, I'm seeing this with 3.14-rc1:
> >
> > [ 2.771270] mmc1: starting CMD11 arg 00000000 flags 00000015
> > [ 2.771575] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000080
> > [ 2.771613] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000001
> > [ 2.771632] mmc1: req done (CMD11): 0: 00000320 00000000 00000000 00000000
> > [ 2.772669] mmc1: clock 0Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
> > [ 2.772679] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> > [ 2.772687] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
> > [ 2.772695] mmc1: clearing ESDHC_VENDOR_SPEC_VSELECT
> > [ 2.772703] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> > [ 2.772713] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
> > [ 2.772719] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> > [ 2.772729] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
> > [ 2.772735] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
> > [ 2.778276] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
> > [ 2.786482] mmc1: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
> > [ 2.786502] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
> > [ 2.786511] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> > [ 2.787515] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
> > [ 2.787522] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
> > [ 2.787529] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> > [ 2.787536] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
> > [ 2.787548] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
> > [ 2.787554] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
> > [ 2.789560] mmc1: card failed to indicate switch to low voltage mode
> >
> > This appears to correspond with the sequence:
> >
> > CMD11 -> clock off -> set vselect -> clock on -> clock off ->
> > set pinctrl -> clock on -> test D[3:0]
> >
> > which appears not to be the expected sequence - the expected sequence
> > should be:
> >
> > CMD11 -> clock off -> set vselect -> clock on -> test D[3:0]
> >
> > maybe with the setting of the IOMUX settings somewhere in there -
> > probably at the point where the clock is off - the additional clock
> > off/clock on step looks to me incorrect.
> >
> > In any case, I've also augmented the other failure paths in
> > mmc_set_signal_voltage(), and it's always this one (the last step)
> > which fails:
> >
> > if (host->ops->card_busy && host->ops->card_busy(host)) {
> > pr_debug("%s: card failed to indicate switch to low voltage mode\n",
> > mmc_hostname(host));
> > err = -EAGAIN;
> > }
> >
> > Any ideas?
>
> I'm not sure if it's the cause or you have it set up or not, but I
> remember that we need to have 3 pinctrl states for 50MHz, 100MHz and
> 200MHz to get UHS card to work. You can look at
> arch/arm/boot/dts/imx6qdl-sabreauto.dtsi for example.
Yes, you need those before the voltage switch is even attempted - I have
all that in place. Lack of the pinctrl states disables 1.8V support:
/* sdr50 and sdr104 needs work on 1.8v signal voltage */
if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data)) {
imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
ESDHC_PINCTRL_STATE_100MHZ);
imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
ESDHC_PINCTRL_STATE_200MHZ);
if (IS_ERR(imx_data->pins_100mhz) ||
IS_ERR(imx_data->pins_200mhz)) {
dev_warn(mmc_dev(host->mmc),
"could not get ultra high speed state, work on $ /* fall back to not support uhs by specify no 1.8v quir$ host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
}
} else {
host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
}
Here's the latest extract from DT for this:
&iomuxc {
cubox_i {
pinctrl_cubox_i_usdhc2_aux: cubox-i-usdhc2-aux {
fsl,pins = <
MX6QDL_PAD_GPIO_4__GPIO1_IO04 0x1f071
MX6QDL_PAD_KEY_ROW1__SD2_VSELECT 0x1b071
>;
};
pinctrl_cubox_i_usdhc2: cubox-i-usdhc2 {
fsl,pins = <
MX6QDL_PAD_SD2_CMD__SD2_CMD 0x17059
MX6QDL_PAD_SD2_CLK__SD2_CLK 0x10059
MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x17059
MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x17059
MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x17059
MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x17059
>;
};
pinctrl_cubox_i_usdhc2_100mhz: cubox-i-usdhc2-100mhz {
fsl,pins = <
MX6QDL_PAD_SD2_CMD__SD2_CMD 0x170b9
MX6QDL_PAD_SD2_CLK__SD2_CLK 0x100b9
MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x170b9
MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x170b9
MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x170b9
MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x170b9
>;
};
pinctrl_cubox_i_usdhc2_200mhz: cubox-i-usdhc2-200mhz {
fsl,pins = <
MX6QDL_PAD_SD2_CMD__SD2_CMD 0x170f9
MX6QDL_PAD_SD2_CLK__SD2_CLK 0x100f9
MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x170f9
MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x170f9
MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x170f9
MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x170f9
>;
};
};
};
&usdhc2 {
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_cubox_i_usdhc2_aux &pinctrl_cubox_i_usdhc2>;
pinctrl-1 = <&pinctrl_cubox_i_usdhc2_aux &pinctrl_cubox_i_usdhc2_100mhz>;
pinctrl-2 = <&pinctrl_cubox_i_usdhc2_aux &pinctrl_cubox_i_usdhc2_200mhz>;
vmmc-supply = <®_3p3v>;
cd-gpios = <&gpio1 4 0>;
status = "okay";
};
The VSELECT pin drives an external switch which supplies either 3.3V or
1.8V to the card and NVCC_SD2.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-07 16:16 ` Russell King - ARM Linux
@ 2014-02-08 13:55 ` Dong Aisheng
0 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2014-02-08 13:55 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Shawn Guo, Dong Aisheng, Chris Ball, linux-mmc@vger.kernel.org
On Sat, Feb 8, 2014 at 12:16 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Feb 07, 2014 at 11:59:23PM +0800, Shawn Guo wrote:
>> On Fri, Feb 07, 2014 at 03:29:51PM +0000, Russell King - ARM Linux wrote:
>> > Hi,
>> >
>> > I have a Samsung 8GB UHS-1 card which I'm trying with iMX6Q, and it's
>> > not behaving very well with the voltage switch. With MMC debugging
>> > enabled - and augmented with additional debug for the ESDHC_VENDOR
>> > register, I'm seeing this with 3.14-rc1:
>> >
>> > [ 2.771270] mmc1: starting CMD11 arg 00000000 flags 00000015
>> > [ 2.771575] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000080
>> > [ 2.771613] sdhci [sdhci_irq()]: *** mmc1 got interrupt: 0x00000001
>> > [ 2.771632] mmc1: req done (CMD11): 0: 00000320 00000000 00000000 00000000
>> > [ 2.772669] mmc1: clock 0Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
>> > [ 2.772679] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
>> > [ 2.772687] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
>> > [ 2.772695] mmc1: clearing ESDHC_VENDOR_SPEC_VSELECT
>> > [ 2.772703] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
>> > [ 2.772713] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
>> > [ 2.772719] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
>> > [ 2.772729] mmc1: ESDHC_VENDOR_SPEC_VSELECT is clear
>> > [ 2.772735] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
>> > [ 2.778276] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
>> > [ 2.786482] mmc1: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 0 timing 0
>> > [ 2.786502] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
>> > [ 2.786511] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
>> > [ 2.787515] mmc1: ESDHC_VENDOR_SPEC_VSELECT is sett
>> > [ 2.787522] mmc1: setting ESDHC_VENDOR_SPEC_VSELECT
>> > [ 2.787529] mmc1: clearing ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
>> > [ 2.787536] sdhci-esdhc-imx 2194000.usdhc: change pinctrl state for uhs 0
>> > [ 2.787548] sdhci-esdhc-imx 2194000.usdhc: desired SD clock: 400000, actual: 386718
>> > [ 2.787554] mmc1: setting ESDHC_VENDOR_SPEC_FRC_SDCLK_ON
>> > [ 2.789560] mmc1: card failed to indicate switch to low voltage mode
>> >
>> > This appears to correspond with the sequence:
>> >
>> > CMD11 -> clock off -> set vselect -> clock on -> clock off ->
>> > set pinctrl -> clock on -> test D[3:0]
>> >
>> > which appears not to be the expected sequence - the expected sequence
>> > should be:
>> >
>> > CMD11 -> clock off -> set vselect -> clock on -> test D[3:0]
>> >
>> > maybe with the setting of the IOMUX settings somewhere in there -
>> > probably at the point where the clock is off - the additional clock
>> > off/clock on step looks to me incorrect.
>> >
The clock on is done by mmc_set_ios(host), from the code,
in sdhci_do_set_ios, the clock will be off and on during set_uhs_signaling.
That could be the reason why you see one more clock off and on before
test D[3:0].
I'm not sure this could be the issue for card to do voltage switch,
the sequence is a bit different from the spec.
Up till now my tests of 3 SD3.0 cards seemed work well on this sequence.
If you think it may be the problem of your card, you can simply remove
that one more
clock disable code to see if it becomes working.
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f892757..7c3d6a4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1550,11 +1550,6 @@ static void sdhci_do_set_ios(struct sdhci_host
*host, struct mmc_ios *ios)
}
- /* Reset SD Clock Enable */
- clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
- clk &= ~SDHCI_CLOCK_CARD_EN;
- sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
-
if (host->ops->set_uhs_signaling)
host->ops->set_uhs_signaling(host, ios->timing);
else {
@@ -1588,9 +1583,6 @@ static void sdhci_do_set_ios(struct sdhci_host
*host, struct mmc_ios *ios)
ios->drv_type = (preset & SDHCI_PRESET_DRV_MASK)
>> SDHCI_PRESET_DRV_SHIFT;
}
-
- /* Re-enable SD Clock */
- sdhci_update_clock(host);
} else
sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>> > In any case, I've also augmented the other failure paths in
>> > mmc_set_signal_voltage(), and it's always this one (the last step)
>> > which fails:
>> >
>> > if (host->ops->card_busy && host->ops->card_busy(host)) {
>> > pr_debug("%s: card failed to indicate switch to low voltage mode\n",
>> > mmc_hostname(host));
>> > err = -EAGAIN;
>> > }
>> >
>> > Any ideas?
>>
>> I'm not sure if it's the cause or you have it set up or not, but I
>> remember that we need to have 3 pinctrl states for 50MHz, 100MHz and
>> 200MHz to get UHS card to work. You can look at
>> arch/arm/boot/dts/imx6qdl-sabreauto.dtsi for example.
>
> Yes, you need those before the voltage switch is even attempted - I have
> all that in place. Lack of the pinctrl states disables 1.8V support:
>
Signal voltage switch is earlier than tunning so the pad setting may not be
the cause of this issue.
> /* sdr50 and sdr104 needs work on 1.8v signal voltage */
> if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data)) {
> imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
> ESDHC_PINCTRL_STATE_100MHZ);
> imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
> ESDHC_PINCTRL_STATE_200MHZ);
> if (IS_ERR(imx_data->pins_100mhz) ||
> IS_ERR(imx_data->pins_200mhz)) {
> dev_warn(mmc_dev(host->mmc),
> "could not get ultra high speed state, work on $ /* fall back to not support uhs by specify no 1.8v quir$ host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> }
> } else {
> host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> }
>
> Here's the latest extract from DT for this:
>
> &iomuxc {
> cubox_i {
> pinctrl_cubox_i_usdhc2_aux: cubox-i-usdhc2-aux {
> fsl,pins = <
> MX6QDL_PAD_GPIO_4__GPIO1_IO04 0x1f071
> MX6QDL_PAD_KEY_ROW1__SD2_VSELECT 0x1b071
> >;
> };
>
> pinctrl_cubox_i_usdhc2: cubox-i-usdhc2 {
> fsl,pins = <
> MX6QDL_PAD_SD2_CMD__SD2_CMD 0x17059
> MX6QDL_PAD_SD2_CLK__SD2_CLK 0x10059
> MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x17059
> MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x17059
> MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x17059
> MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x17059
> >;
> };
>
> pinctrl_cubox_i_usdhc2_100mhz: cubox-i-usdhc2-100mhz {
> fsl,pins = <
> MX6QDL_PAD_SD2_CMD__SD2_CMD 0x170b9
> MX6QDL_PAD_SD2_CLK__SD2_CLK 0x100b9
> MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x170b9
> MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x170b9
> MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x170b9
> MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x170b9
> >;
> };
>
> pinctrl_cubox_i_usdhc2_200mhz: cubox-i-usdhc2-200mhz {
> fsl,pins = <
> MX6QDL_PAD_SD2_CMD__SD2_CMD 0x170f9
> MX6QDL_PAD_SD2_CLK__SD2_CLK 0x100f9
> MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x170f9
> MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x170f9
> MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x170f9
> MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x170f9
> >;
> };
> };
> };
>
> &usdhc2 {
> pinctrl-names = "default", "state_100mhz", "state_200mhz";
> pinctrl-0 = <&pinctrl_cubox_i_usdhc2_aux &pinctrl_cubox_i_usdhc2>;
> pinctrl-1 = <&pinctrl_cubox_i_usdhc2_aux &pinctrl_cubox_i_usdhc2_100mhz>;
> pinctrl-2 = <&pinctrl_cubox_i_usdhc2_aux &pinctrl_cubox_i_usdhc2_200mhz>;
> vmmc-supply = <®_3p3v>;
> cd-gpios = <&gpio1 4 0>;
> status = "okay";
> };
>
> The VSELECT pin drives an external switch which supplies either 3.3V or
> 1.8V to the card and NVCC_SD2.
>
Actually VSELECT pin should only affect the signal voltage, card voltage
should be always 3.3v for SD 3.0 cards.
Not sure if this could be the issue of your boards.
Did you have an imx6q sabreauto board?
If yes, you could give a quick try on that board for your cards to if
it works well.
I just tried a Toshiba SD3.0 cards and a Sandisk SD3.0 cards, both works well
on imx6q sabreauto boards.
Regards
Dong Aisheng
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-07 15:59 ` Shawn Guo
2014-02-07 16:16 ` Russell King - ARM Linux
@ 2014-02-09 15:14 ` Russell King - ARM Linux
2014-02-10 3:31 ` Dong Aisheng
1 sibling, 1 reply; 8+ messages in thread
From: Russell King - ARM Linux @ 2014-02-09 15:14 UTC (permalink / raw)
To: Shawn Guo, Dong Aisheng; +Cc: Chris Ball, linux-mmc
Okay, I have this working now - there was a hardware problem. However,
it's obvious that this has never been tested with lockdep enabled beacuse
of the following:
disable_irq(host->irq);
spin_lock(&host->lock);
host->mrq = &mrq;
sdhci_send_command(host, mrq.cmd);
spin_unlock(&host->lock);
enable_irq(host->irq);
You can't "work around" stuff like this. Use spin_lock_irq()..
spin_unlock_irq(). You can't just disable the interrupt line and
expect lockdep to know that it's safe.
[ 2.290584] =================================
[ 2.308140] [ INFO: inconsistent lock state ]
[ 2.308146] 3.14.0-rc1+ #490 Not tainted
[ 2.308148] ---------------------------------
[ 2.308152] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
[ 2.308158] kworker/u8:0/6 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 2.308191] (&(&host->lock)->rlock#2){?.-...}, at: [<c04b57a4>] esdhc_send_tuning_cmd+0x104/0x14c
[ 2.308193] {IN-HARDIRQ-W} state was registered at:
[ 2.308215] [<c00652fc>] mark_lock+0x15c/0x6f8
[ 2.308224] [<c0066354>] __lock_acquire+0xabc/0x1ca0
[ 2.308231] [<c0067ad8>] lock_acquire+0xa0/0x130
[ 2.308251] [<c0697a44>] _raw_spin_lock+0x34/0x44
[ 2.308258] [<c04b0dbc>] sdhci_irq+0x20/0xa40
[ 2.308272] [<c0071b1c>] handle_irq_event_percpu+0x74/0x284
[ 2.308278] [<c0071d70>] handle_irq_event+0x44/0x64
[ 2.308289] [<c0074db8>] handle_fasteoi_irq+0xac/0x140
[ 2.308296] [<c007147c>] generic_handle_irq+0x28/0x38
[ 2.308311] [<c000efd4>] handle_IRQ+0x40/0x98
[ 2.308317] [<c0008584>] gic_handle_irq+0x30/0x64
[ 2.308324] [<c0013144>] __irq_svc+0x44/0x58
[ 2.308336] [<c0028fc8>] irq_exit+0xc0/0x120
[ 2.308343] [<c000efd8>] handle_IRQ+0x44/0x98
[ 2.308349] [<c0008584>] gic_handle_irq+0x30/0x64
[ 2.308355] [<c0013144>] __irq_svc+0x44/0x58
[ 2.308363] [<c068f398>] printk+0x3c/0x44
[ 2.308378] [<c03191d0>] _regulator_get+0x1b4/0x1e0
[ 2.308385] [<c031924c>] regulator_get+0x18/0x1c
[ 2.308397] [<c049fbc4>] mmc_add_host+0x30/0x1c0
[ 2.308404] [<c04b2e10>] sdhci_add_host+0x804/0xbbc
[ 2.308411] [<c04b5318>] sdhci_esdhc_imx_probe+0x380/0x674
[ 2.308427] [<c036d530>] platform_drv_probe+0x20/0x50
[ 2.308435] [<c036b948>] driver_probe_device+0x120/0x234
[ 2.308441] [<c036baf8>] __driver_attach+0x9c/0xa0
[ 2.308448] [<c036a04c>] bus_for_each_dev+0x5c/0x90
[ 2.308455] [<c036b418>] driver_attach+0x24/0x28
[ 2.308462] [<c036b018>] bus_add_driver+0xe4/0x1d8
[ 2.308468] [<c036c1b0>] driver_register+0x80/0xfc
[ 2.308477] [<c036ce28>] __platform_driver_register+0x50/0x64
[ 2.308497] [<c093706c>] sdhci_esdhc_imx_driver_init+0x18/0x20
[ 2.308504] [<c0008834>] do_one_initcall+0x3c/0x164
[ 2.308519] [<c0901c94>] kernel_init_freeable+0x104/0x1d0
[ 2.308537] [<c068c45c>] kernel_init+0x10/0x118
[ 2.308546] [<c000e768>] ret_from_fork+0x14/0x2c
[ 2.308549] irq event stamp: 5933
[ 2.308560] hardirqs last enabled at (5933): [<c069813c>] _raw_spin_unlock_irqrestore+0x38/0x4c
[ 2.308569] hardirqs last disabled at (5932): [<c0697b04>] _raw_spin_lock_irqsave+0x24/0x60
[ 2.308577] softirqs last enabled at (5914): [<c0028ba0>] __do_softirq+0x260/0x360
[ 2.308584] softirqs last disabled at (5909): [<c0028fc8>] irq_exit+0xc0/0x120
[ 2.308586]
[ 2.308586] other info that might help us debug this:
[ 2.308588] Possible unsafe locking scenario:
[ 2.308588]
[ 2.308590] CPU0
[ 2.308592] ----
[ 2.308598] lock(&(&host->lock)->rlock#2);
[ 2.308600] <Interrupt>
[ 2.308606] lock(&(&host->lock)->rlock#2);
[ 2.308607]
[ 2.308607] *** DEADLOCK ***
[ 2.308607]
[ 2.308611] 2 locks held by kworker/u8:0/6:
[ 2.308626] #0: (kmmcd){.+.+.+}, at: [<c003d890>] process_one_work+0x134/0x4e8
[ 2.308638] #1: ((&(&host->detect)->work)){+.+.+.}, at: [<c003d890>] process_one_work+0x134/0x4e8
[ 2.308640]
[ 2.308640] stack backtrace:
[ 2.308649] CPU: 2 PID: 6 Comm: kworker/u8:0 Not tainted 3.14.0-rc1+ #490
[ 2.308659] Workqueue: kmmcd mmc_rescan
[ 2.308663] Backtrace:
[ 2.308679] [<c00124a0>] (dump_backtrace) from [<c0012640>] (show_stack+0x18/0x1c)
[ 2.308689] r6:ea07b3e0 r5:ea07af80 r4:00000000 r3:ea07af80
[ 2.308699] [<c0012628>] (show_stack) from [<c069164c>] (dump_stack+0x70/0x8c)
[ 2.308707] [<c06915dc>] (dump_stack) from [<c068f080>] (print_usage_bug+0x274/0x2e4)
[ 2.308713] r4:c0a8a2c0 r3:ea07af80
[ 2.308723] [<c068ee0c>] (print_usage_bug) from [<c0065774>] (mark_lock+0x5d4/0x6f8)
[ 2.308736] r10:ea07af80 r8:ea07af80 r7:00000000 r6:c0a8a2c0 r5:ea07b3e0 r4:00000002
[ 2.308745] [<c00651a0>] (mark_lock) from [<c0065e6c>] (__lock_acquire+0x5d4/0x1ca0)
[ 2.308758] r10:ea07af80 r9:00000000 r8:ea07b3e0 r7:e988cd28 r6:00000002 r5:00000460
[ 2.308762] r4:ea07af80
[ 2.308770] [<c0065898>] (__lock_acquire) from [<c0067ad8>] (lock_acquire+0xa0/0x130)
[ 2.308783] r10:00000000 r9:00000002 r8:00000000 r7:00000000 r6:e988cd28 r5:ea094000
[ 2.308787] r4:00000000
[ 2.308797] [<c0067a38>] (lock_acquire) from [<c0697a44>] (_raw_spin_lock+0x34/0x44)
[ 2.308809] r10:00000006 r9:00000002 r8:00000035 r7:e988cd18 r6:ea095bf8 r5:00000002
[ 2.308813] r4:e988cd18
[ 2.308823] [<c0697a10>] (_raw_spin_lock) from [<c04b57a4>] (esdhc_send_tuning_cmd+0x104/0x14c)
[ 2.308829] r5:e988cc40 r4:00000000
[ 2.308838] [<c04b56a0>] (esdhc_send_tuning_cmd) from [<c04b582c>] (esdhc_executing_tuning+0x40/0x100)
[ 2.308849] r8:e9878800 r7:00000013 r6:e988cd18 r5:e988cc40 r4:00000000
[ 2.308858] [<c04b57ec>] (esdhc_executing_tuning) from [<c04afa54>] (sdhci_execute_tuning+0xcc/0x754)
[ 2.308867] r7:00000000 r6:e988cd18 r5:00000000 r4:e988c800
[ 2.308879] [<c04af988>] (sdhci_execute_tuning) from [<c04a4684>] (mmc_sd_init_card+0x65c/0x694)
[ 2.308891] r10:00000006 r9:00000002 r8:e9878800 r7:00000000 r6:e982e700 r5:00000000
[ 2.308895] r4:e988c800
[ 2.308903] [<c04a4028>] (mmc_sd_init_card) from [<c04a48f0>] (mmc_attach_sd+0xb0/0x184)
[ 2.308915] r10:ea094000 r8:00000000 r7:c0703e20 r6:e988c800 r5:00000000 r4:e988c800
[ 2.308923] [<c04a4840>] (mmc_attach_sd) from [<c049eb28>] (mmc_rescan+0x26c/0x2e8)
[ 2.308929] r5:c0703e14 r4:e988cb08
[ 2.308936] [<c049e8bc>] (mmc_rescan) from [<c003d914>] (process_one_work+0x1b8/0x4e8)
[ 2.308946] r7:ea190200 r6:ea00dc00 r5:ea02d980 r4:e988cb08
[ 2.308953] [<c003d75c>] (process_one_work) from [<c003e090>] (worker_thread+0x13c/0x3f8)
[ 2.308965] r10:ea094000 r9:00000089 r8:ea02d998 r7:ea094000 r6:ea00dc30 r5:ea00dc00
[ 2.308969] r4:ea02d980
[ 2.308984] [<c003df54>] (worker_thread) from [<c00449bc>] (kthread+0xcc/0xe8)
[ 2.308996] r10:00000000 r9:00000000 r8:00000000 r7:c003df54 r6:ea02d980 r5:ea040700
[ 2.309000] r4:00000000
[ 2.309010] [<c00448f0>] (kthread) from [<c000e768>] (ret_from_fork+0x14/0x2c)
[ 2.309020] r7:00000000 r6:00000000 r5:c00448f0 r4:ea040700
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-09 15:14 ` Russell King - ARM Linux
@ 2014-02-10 3:31 ` Dong Aisheng
2014-02-10 10:33 ` Russell King - ARM Linux
0 siblings, 1 reply; 8+ messages in thread
From: Dong Aisheng @ 2014-02-10 3:31 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Shawn Guo, Dong Aisheng, Chris Ball, linux-mmc@vger.kernel.org
On Sun, Feb 9, 2014 at 11:14 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Okay, I have this working now - there was a hardware problem. However,
> it's obvious that this has never been tested with lockdep enabled beacuse
> of the following:
>
> disable_irq(host->irq);
> spin_lock(&host->lock);
> host->mrq = &mrq;
>
> sdhci_send_command(host, mrq.cmd);
>
> spin_unlock(&host->lock);
> enable_irq(host->irq);
>
> You can't "work around" stuff like this. Use spin_lock_irq()..
> spin_unlock_irq(). You can't just disable the interrupt line and
> expect lockdep to know that it's safe.
>
Yes, i missed this.
Such kind of code was originally referenced from sdhci_execute_tuning.
It was formerly fixed in commit 2b35bd83.
But i did miss sdhci-esdhci-imx also has such issue.
Thanks for the reminder.
You can try the following fix and I will send out a patch for it later.
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index b841bb7..0372baf 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -720,6 +720,7 @@ static int esdhc_send_tuning_cmd(struct sdhci_host
*host, u32 opcode)
struct mmc_data data = {0};
struct scatterlist sg;
char tuning_pattern[ESDHC_TUNING_BLOCK_PATTERN_LEN];
+ unsigned long flags;
cmd.opcode = opcode;
cmd.arg = 0;
@@ -742,14 +743,12 @@ static int esdhc_send_tuning_cmd(struct
sdhci_host *host, u32 opcode)
mrq.done = esdhc_request_done;
init_completion(&(mrq.completion));
- disable_irq(host->irq);
- spin_lock(&host->lock);
+ spin_lock_irqsave(&host->lock, flags);
host->mrq = &mrq;
sdhci_send_command(host, mrq.cmd);
- spin_unlock(&host->lock);
- enable_irq(host->irq);
+ spin_unlock_irqrestore(&host->lock, flags);
wait_for_completion(&mrq.completion);
Regards
Dong Aisheng
> [ 2.290584] =================================
> [ 2.308140] [ INFO: inconsistent lock state ]
> [ 2.308146] 3.14.0-rc1+ #490 Not tainted
> [ 2.308148] ---------------------------------
> [ 2.308152] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
> [ 2.308158] kworker/u8:0/6 [HC0[0]:SC0[0]:HE1:SE1] takes:
> [ 2.308191] (&(&host->lock)->rlock#2){?.-...}, at: [<c04b57a4>] esdhc_send_tuning_cmd+0x104/0x14c
> [ 2.308193] {IN-HARDIRQ-W} state was registered at:
> [ 2.308215] [<c00652fc>] mark_lock+0x15c/0x6f8
> [ 2.308224] [<c0066354>] __lock_acquire+0xabc/0x1ca0
> [ 2.308231] [<c0067ad8>] lock_acquire+0xa0/0x130
> [ 2.308251] [<c0697a44>] _raw_spin_lock+0x34/0x44
> [ 2.308258] [<c04b0dbc>] sdhci_irq+0x20/0xa40
> [ 2.308272] [<c0071b1c>] handle_irq_event_percpu+0x74/0x284
> [ 2.308278] [<c0071d70>] handle_irq_event+0x44/0x64
> [ 2.308289] [<c0074db8>] handle_fasteoi_irq+0xac/0x140
> [ 2.308296] [<c007147c>] generic_handle_irq+0x28/0x38
> [ 2.308311] [<c000efd4>] handle_IRQ+0x40/0x98
> [ 2.308317] [<c0008584>] gic_handle_irq+0x30/0x64
> [ 2.308324] [<c0013144>] __irq_svc+0x44/0x58
> [ 2.308336] [<c0028fc8>] irq_exit+0xc0/0x120
> [ 2.308343] [<c000efd8>] handle_IRQ+0x44/0x98
> [ 2.308349] [<c0008584>] gic_handle_irq+0x30/0x64
> [ 2.308355] [<c0013144>] __irq_svc+0x44/0x58
> [ 2.308363] [<c068f398>] printk+0x3c/0x44
> [ 2.308378] [<c03191d0>] _regulator_get+0x1b4/0x1e0
> [ 2.308385] [<c031924c>] regulator_get+0x18/0x1c
> [ 2.308397] [<c049fbc4>] mmc_add_host+0x30/0x1c0
> [ 2.308404] [<c04b2e10>] sdhci_add_host+0x804/0xbbc
> [ 2.308411] [<c04b5318>] sdhci_esdhc_imx_probe+0x380/0x674
> [ 2.308427] [<c036d530>] platform_drv_probe+0x20/0x50
> [ 2.308435] [<c036b948>] driver_probe_device+0x120/0x234
> [ 2.308441] [<c036baf8>] __driver_attach+0x9c/0xa0
> [ 2.308448] [<c036a04c>] bus_for_each_dev+0x5c/0x90
> [ 2.308455] [<c036b418>] driver_attach+0x24/0x28
> [ 2.308462] [<c036b018>] bus_add_driver+0xe4/0x1d8
> [ 2.308468] [<c036c1b0>] driver_register+0x80/0xfc
> [ 2.308477] [<c036ce28>] __platform_driver_register+0x50/0x64
> [ 2.308497] [<c093706c>] sdhci_esdhc_imx_driver_init+0x18/0x20
> [ 2.308504] [<c0008834>] do_one_initcall+0x3c/0x164
> [ 2.308519] [<c0901c94>] kernel_init_freeable+0x104/0x1d0
> [ 2.308537] [<c068c45c>] kernel_init+0x10/0x118
> [ 2.308546] [<c000e768>] ret_from_fork+0x14/0x2c
> [ 2.308549] irq event stamp: 5933
> [ 2.308560] hardirqs last enabled at (5933): [<c069813c>] _raw_spin_unlock_irqrestore+0x38/0x4c
> [ 2.308569] hardirqs last disabled at (5932): [<c0697b04>] _raw_spin_lock_irqsave+0x24/0x60
> [ 2.308577] softirqs last enabled at (5914): [<c0028ba0>] __do_softirq+0x260/0x360
> [ 2.308584] softirqs last disabled at (5909): [<c0028fc8>] irq_exit+0xc0/0x120
> [ 2.308586]
> [ 2.308586] other info that might help us debug this:
> [ 2.308588] Possible unsafe locking scenario:
> [ 2.308588]
> [ 2.308590] CPU0
> [ 2.308592] ----
> [ 2.308598] lock(&(&host->lock)->rlock#2);
> [ 2.308600] <Interrupt>
> [ 2.308606] lock(&(&host->lock)->rlock#2);
> [ 2.308607]
> [ 2.308607] *** DEADLOCK ***
> [ 2.308607]
> [ 2.308611] 2 locks held by kworker/u8:0/6:
> [ 2.308626] #0: (kmmcd){.+.+.+}, at: [<c003d890>] process_one_work+0x134/0x4e8
> [ 2.308638] #1: ((&(&host->detect)->work)){+.+.+.}, at: [<c003d890>] process_one_work+0x134/0x4e8
> [ 2.308640]
> [ 2.308640] stack backtrace:
> [ 2.308649] CPU: 2 PID: 6 Comm: kworker/u8:0 Not tainted 3.14.0-rc1+ #490
> [ 2.308659] Workqueue: kmmcd mmc_rescan
> [ 2.308663] Backtrace:
> [ 2.308679] [<c00124a0>] (dump_backtrace) from [<c0012640>] (show_stack+0x18/0x1c)
> [ 2.308689] r6:ea07b3e0 r5:ea07af80 r4:00000000 r3:ea07af80
> [ 2.308699] [<c0012628>] (show_stack) from [<c069164c>] (dump_stack+0x70/0x8c)
> [ 2.308707] [<c06915dc>] (dump_stack) from [<c068f080>] (print_usage_bug+0x274/0x2e4)
> [ 2.308713] r4:c0a8a2c0 r3:ea07af80
> [ 2.308723] [<c068ee0c>] (print_usage_bug) from [<c0065774>] (mark_lock+0x5d4/0x6f8)
> [ 2.308736] r10:ea07af80 r8:ea07af80 r7:00000000 r6:c0a8a2c0 r5:ea07b3e0 r4:00000002
> [ 2.308745] [<c00651a0>] (mark_lock) from [<c0065e6c>] (__lock_acquire+0x5d4/0x1ca0)
> [ 2.308758] r10:ea07af80 r9:00000000 r8:ea07b3e0 r7:e988cd28 r6:00000002 r5:00000460
> [ 2.308762] r4:ea07af80
> [ 2.308770] [<c0065898>] (__lock_acquire) from [<c0067ad8>] (lock_acquire+0xa0/0x130)
> [ 2.308783] r10:00000000 r9:00000002 r8:00000000 r7:00000000 r6:e988cd28 r5:ea094000
> [ 2.308787] r4:00000000
> [ 2.308797] [<c0067a38>] (lock_acquire) from [<c0697a44>] (_raw_spin_lock+0x34/0x44)
> [ 2.308809] r10:00000006 r9:00000002 r8:00000035 r7:e988cd18 r6:ea095bf8 r5:00000002
> [ 2.308813] r4:e988cd18
> [ 2.308823] [<c0697a10>] (_raw_spin_lock) from [<c04b57a4>] (esdhc_send_tuning_cmd+0x104/0x14c)
> [ 2.308829] r5:e988cc40 r4:00000000
> [ 2.308838] [<c04b56a0>] (esdhc_send_tuning_cmd) from [<c04b582c>] (esdhc_executing_tuning+0x40/0x100)
> [ 2.308849] r8:e9878800 r7:00000013 r6:e988cd18 r5:e988cc40 r4:00000000
> [ 2.308858] [<c04b57ec>] (esdhc_executing_tuning) from [<c04afa54>] (sdhci_execute_tuning+0xcc/0x754)
> [ 2.308867] r7:00000000 r6:e988cd18 r5:00000000 r4:e988c800
> [ 2.308879] [<c04af988>] (sdhci_execute_tuning) from [<c04a4684>] (mmc_sd_init_card+0x65c/0x694)
> [ 2.308891] r10:00000006 r9:00000002 r8:e9878800 r7:00000000 r6:e982e700 r5:00000000
> [ 2.308895] r4:e988c800
> [ 2.308903] [<c04a4028>] (mmc_sd_init_card) from [<c04a48f0>] (mmc_attach_sd+0xb0/0x184)
> [ 2.308915] r10:ea094000 r8:00000000 r7:c0703e20 r6:e988c800 r5:00000000 r4:e988c800
> [ 2.308923] [<c04a4840>] (mmc_attach_sd) from [<c049eb28>] (mmc_rescan+0x26c/0x2e8)
> [ 2.308929] r5:c0703e14 r4:e988cb08
> [ 2.308936] [<c049e8bc>] (mmc_rescan) from [<c003d914>] (process_one_work+0x1b8/0x4e8)
> [ 2.308946] r7:ea190200 r6:ea00dc00 r5:ea02d980 r4:e988cb08
> [ 2.308953] [<c003d75c>] (process_one_work) from [<c003e090>] (worker_thread+0x13c/0x3f8)
> [ 2.308965] r10:ea094000 r9:00000089 r8:ea02d998 r7:ea094000 r6:ea00dc30 r5:ea00dc00
> [ 2.308969] r4:ea02d980
> [ 2.308984] [<c003df54>] (worker_thread) from [<c00449bc>] (kthread+0xcc/0xe8)
> [ 2.308996] r10:00000000 r9:00000000 r8:00000000 r7:c003df54 r6:ea02d980 r5:ea040700
> [ 2.309000] r4:00000000
> [ 2.309010] [<c00448f0>] (kthread) from [<c000e768>] (ret_from_fork+0x14/0x2c)
> [ 2.309020] r7:00000000 r6:00000000 r5:c00448f0 r4:ea040700
>
>
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-10 3:31 ` Dong Aisheng
@ 2014-02-10 10:33 ` Russell King - ARM Linux
2014-02-10 12:11 ` Dong Aisheng
0 siblings, 1 reply; 8+ messages in thread
From: Russell King - ARM Linux @ 2014-02-10 10:33 UTC (permalink / raw)
To: Dong Aisheng
Cc: Shawn Guo, Dong Aisheng, Chris Ball, linux-mmc@vger.kernel.org
On Mon, Feb 10, 2014 at 11:31:40AM +0800, Dong Aisheng wrote:
> On Sun, Feb 9, 2014 at 11:14 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Okay, I have this working now - there was a hardware problem. However,
> > it's obvious that this has never been tested with lockdep enabled beacuse
> > of the following:
> >
> > disable_irq(host->irq);
> > spin_lock(&host->lock);
> > host->mrq = &mrq;
> >
> > sdhci_send_command(host, mrq.cmd);
> >
> > spin_unlock(&host->lock);
> > enable_irq(host->irq);
> >
> > You can't "work around" stuff like this. Use spin_lock_irq()..
> > spin_unlock_irq(). You can't just disable the interrupt line and
> > expect lockdep to know that it's safe.
> >
>
> Yes, i missed this.
> Such kind of code was originally referenced from sdhci_execute_tuning.
> It was formerly fixed in commit 2b35bd83.
> But i did miss sdhci-esdhci-imx also has such issue.
> Thanks for the reminder.
>
> You can try the following fix and I will send out a patch for it later.
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index b841bb7..0372baf 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -720,6 +720,7 @@ static int esdhc_send_tuning_cmd(struct sdhci_host
> *host, u32 opcode)
> struct mmc_data data = {0};
> struct scatterlist sg;
> char tuning_pattern[ESDHC_TUNING_BLOCK_PATTERN_LEN];
> + unsigned long flags;
>
> cmd.opcode = opcode;
> cmd.arg = 0;
> @@ -742,14 +743,12 @@ static int esdhc_send_tuning_cmd(struct
> sdhci_host *host, u32 opcode)
> mrq.done = esdhc_request_done;
> init_completion(&(mrq.completion));
>
> - disable_irq(host->irq);
> - spin_lock(&host->lock);
> + spin_lock_irqsave(&host->lock, flags);
> host->mrq = &mrq;
>
> sdhci_send_command(host, mrq.cmd);
>
> - spin_unlock(&host->lock);
> - enable_irq(host->irq);
> + spin_unlock_irqrestore(&host->lock, flags);
>
> wait_for_completion(&mrq.completion);
You don't need to use irqsave/irqrestore here - IRQs must be enabled here
otherwise wait_for_completion() will scream - scheduling with interrupts
disabled is not allowed.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: UHS-1 cards with iMX6Q
2014-02-10 10:33 ` Russell King - ARM Linux
@ 2014-02-10 12:11 ` Dong Aisheng
0 siblings, 0 replies; 8+ messages in thread
From: Dong Aisheng @ 2014-02-10 12:11 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Shawn Guo, Dong Aisheng, Chris Ball, linux-mmc@vger.kernel.org
On Mon, Feb 10, 2014 at 6:33 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Feb 10, 2014 at 11:31:40AM +0800, Dong Aisheng wrote:
>> On Sun, Feb 9, 2014 at 11:14 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > Okay, I have this working now - there was a hardware problem. However,
>> > it's obvious that this has never been tested with lockdep enabled beacuse
>> > of the following:
>> >
>> > disable_irq(host->irq);
>> > spin_lock(&host->lock);
>> > host->mrq = &mrq;
>> >
>> > sdhci_send_command(host, mrq.cmd);
>> >
>> > spin_unlock(&host->lock);
>> > enable_irq(host->irq);
>> >
>> > You can't "work around" stuff like this. Use spin_lock_irq()..
>> > spin_unlock_irq(). You can't just disable the interrupt line and
>> > expect lockdep to know that it's safe.
>> >
>>
>> Yes, i missed this.
>> Such kind of code was originally referenced from sdhci_execute_tuning.
>> It was formerly fixed in commit 2b35bd83.
>> But i did miss sdhci-esdhci-imx also has such issue.
>> Thanks for the reminder.
>>
>> You can try the following fix and I will send out a patch for it later.
>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
>> b/drivers/mmc/host/sdhci-esdhc-imx.c
>> index b841bb7..0372baf 100644
>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> @@ -720,6 +720,7 @@ static int esdhc_send_tuning_cmd(struct sdhci_host
>> *host, u32 opcode)
>> struct mmc_data data = {0};
>> struct scatterlist sg;
>> char tuning_pattern[ESDHC_TUNING_BLOCK_PATTERN_LEN];
>> + unsigned long flags;
>>
>> cmd.opcode = opcode;
>> cmd.arg = 0;
>> @@ -742,14 +743,12 @@ static int esdhc_send_tuning_cmd(struct
>> sdhci_host *host, u32 opcode)
>> mrq.done = esdhc_request_done;
>> init_completion(&(mrq.completion));
>>
>> - disable_irq(host->irq);
>> - spin_lock(&host->lock);
>> + spin_lock_irqsave(&host->lock, flags);
>> host->mrq = &mrq;
>>
>> sdhci_send_command(host, mrq.cmd);
>>
>> - spin_unlock(&host->lock);
>> - enable_irq(host->irq);
>> + spin_unlock_irqrestore(&host->lock, flags);
>>
>> wait_for_completion(&mrq.completion);
>
> You don't need to use irqsave/irqrestore here - IRQs must be enabled here
> otherwise wait_for_completion() will scream - scheduling with interrupts
> disabled is not allowed.
>
Right.
Thanks for the suggestion.
Regards
Dong Aisheng
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-02-10 12:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-07 15:29 UHS-1 cards with iMX6Q Russell King - ARM Linux
2014-02-07 15:59 ` Shawn Guo
2014-02-07 16:16 ` Russell King - ARM Linux
2014-02-08 13:55 ` Dong Aisheng
2014-02-09 15:14 ` Russell King - ARM Linux
2014-02-10 3:31 ` Dong Aisheng
2014-02-10 10:33 ` Russell King - ARM Linux
2014-02-10 12:11 ` Dong Aisheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox