? 2016/5/26 11:59, Jaehoon Chung ??: > On 05/26/2016 11:23 AM, Shawn Lin wrote: >> Hi Jaehoon, >> >> On 2016/5/19 21:07, Jaehoon Chung wrote: >>> On 05/19/2016 08:31 PM, Shawn Lin wrote: >>>> Hi, >>>> >>>> On 2016/5/19 1:37, Doug Anderson wrote: >>>>> Hi, >>>>> >>>>> On Wed, May 18, 2016 at 2:14 AM, Shawn Lin <shawn.lin at rock-chips.com> wrote: >>>>>> Hi >>>>>> >>>>>> >>>>>> On 2016-5-18 12:12, Doug Anderson wrote: >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> On Tue, May 17, 2016 at 6:59 PM, Shawn Lin >>>>>>> <shawn.lin at kernel-upstream.org> wrote: >>>>>>>> >>>>>>>> Could you try this patch to see if you can still find HLE? >>>>>>>> >>>>>>>> @@ -2356,12 +2356,22 @@ static void dw_mci_cmd_interrupt(struct dw_mci >>>>>>>> *host, u32 status) >>>>>>>> static void dw_mci_handle_cd(struct dw_mci *host) >>>>>>>> { >>>>>>>> int i; >>>>>>>> + int present; >>>>>>>> >>>>>>>> for (i = 0; i < host->num_slots; i++) { >>>>>>>> struct dw_mci_slot *slot = host->slot[i]; >>>>>>>> >>>>>>>> if (!slot) >>>>>>>> continue; >>>>>>>> >>>>>>>> + present = !(mci_readl(slot->host, CDETECT) & (1 << >>>>>>>> slot->id)); >>>>>>>> + if (present) >>>>>>>> + set_bit(DW_MMC_CARD_PRESENT, &slot->flags); >>>>>>>> + else >>>>>>>> + clear_bit(DW_MMC_CARD_PRESENT, &slot->flags); >>>>>>> >>>>>>> >>>>>>> No, because we don't use the builtin card detect on veyron. ;) >>>>>>> >>>>>>> We use GPIO card detect because we didn't like the way JTAG and SD >>>>>>> interacted. Also on rk3288 the builtin card detect line had the wrong >>>>>>> voltage domain (you couldn't detect a card when the IO lines were >>>>>>> powered off). The builtin card detect line is always driven low on >>>>>>> veyron. >>>>>> >>>>>> >>>>>> Okay, I see. >>>>>> >>>>>>> >>>>>>> >>>>>>> I'm nearly certain that the root cause of my HLE errors is actually >>>>>>> related to the same problem addressed by the commit 7c5209c315ea >>>>>>> ("mmc: core: Increase delay for voltage to stabilize from 3.3V to >>>>>>> 1.8V"). I think that on minnie we're still on the hairy edge and >>>>>>> sometimes the line doesn't transition fast enough. >>>>>> >>>>>> >>>>>> Things are not so simple from your details. >>>>>> >>>>>> I was not enabling SD3.0 support, then I also found HLE sometimes. >>>>>> So it seems commit 7c5209c315ea does not contibute to this phenomenon. >>>>> >>>>> Just to clarify, in my case commit 7c5209c315ea didn't make the >>>>> problem worse, but made it better. Just not better enough. ;) >>>>> >>>>> >>>>>> The scenario looks like: >>>>>> remove sd-card -> mmc_sd_detect -> send status(CMD13) ->power_off -> >>>>>> set_ios -> setup_bus -> disabled clk , then HLE irq storm coming >>>>>> >>>>>> From the code of dw_mci_prepare_command: >>>>>> SDMMC_CMD_PRV_DAT_WAIT will not be used for CMD13, so we don't >>>>>> wait_busy here, then cmd code is loding into queue of dw_mmc but >>>>>> still failing send out because it's in busy? >>>>>> >>>>>> With my patch, things go well: >>>>>> remove sd-card -> clear bit of DW_MMC_CARD_PRESENT -> send >>>>>> status(CMD13) return directly -> power_off -> set_ios -> setup_bus -> >>>>>> disable clk >>>>>> >>>>>> So why should we allow inquiry of card status if we sure the card is >>>>>> removed? I mean no any further cmds should be delivered. >>>>> >>>>> Quite honestly just dealing with the HLE error (my patch or >>>>> equivalent) might be a sane solution for the problem you describe. >>>> >>>> Yes, your patch looks good to me, so it should be merged firstly. :) >>>> Then let's push it a bit further more that when HLEs are coming, >>>> somethings must be wrong(currently I don't see a obvious clue from >>>> the code itself although, I'm prone to think it belongs to the >>>> software issue). >>> >>> We don't know what's main cause for HLE..But i also think it's relevant to SW issue. >>> But we need to consider all possibilities.. >>> >>>> >>>> >>>>> >>>>> dw_mmc needs to be able to work with an external card detect GPIO. >>>>> It's been part of the dw_mmc driver for a long time and is (in fact) >>>>> in use upstream at least by rk3288-veyron. Any solution that only >>>>> works for internal card detect is not enough. Just handling the HLE >>>>> error to deal with the interrupt storm and then letting Linux remove >>>>> the card (because of the card detect interrupt) seems totally OK to >>>>> me. >>>>> >>>> >>>> Sure, some of rockchip Socs use gpio for CD because they don't >>>> have a internal CD, such as RK3036, right? >>>> >>>>> Note: I'd be very curious if your problems get better if you disable >>>> >>>> Not at all. >>>> >>>>> the "grf_force_jtag" bit in the GRF. If you're using the builtin card >>>>> detect and you use the boot default of "grf_force_jtag" then your pins >>>>> will be unmuxed behind your back when the card is ejected. This could >>>>> be causing the dw_mmc controller to get confused. >>>> >>>> Right, grf_force_jtag is also not a friend of mine. :) >>>> So I had disabled this function before I was debugging it. >>>> >>>>> >>>>> >>>>>> And another question: should we wait busy for cmd13? >>>>> >>>>> I don't think so. As I understand it CMD13 uses only the CMD line for >>>>> communication and it should be appropriate to send this when the bus >>>>> is "busy" (which means that the DATA lines are low). >>>> >>>> Ahh... take back my question.. I was just considering a wired situation >>>> that pins are unmuxed on the background(cmd line as well) when cmd13 is >>>> delivering.... >>>> >>>> >>>>> >>>>> Also: it seems odd that the HLE IRQ storm didn't come right after the >>>>> CMD 13 in your description above. Are you sure it was the CMD 13 that >>>>> caused the HLEs, or could it has been something else? >>>> >>>> Actually no. Any cmds be issued can trigger HLEs, I think, after sd card is removed When I hacked mmc_sd_detecd to send other cmds intead >>>> of cmd13. >>>> >>>> From dw_mmc databook v270a(7.2.3 Clock Programming) we can see: >>>> The DWC_mobile_storage loads each of these registers only when the >>>> start_cmd bit and the Update_clk_regs_only bit in the CMD register are >>>> set. When a command is successfully loaded, the DWC_mobile_storage >>>> clears this bit, unless the DWC_mobile_storage already has another >>>> command in the queue, at which point it gives an HLE (Hardware Locked >>>> Error); for details on HLEs, refer to ?Error Handling? on page 233. >>>> Software should look for the start_cmd and the Update_clk_regs_only >>>> bits, and should also set the wait_prvdata_complete bit to ensure that >>>> clock parameters do not change during data transfer. >>>> >>>> Maybe the cmd is trying to load(or somethings wrong with the >>>> controller?) when we disable the clk? That may explain my observation >>>> that HLEs came after disabling clk. >>> >>> I agreed. >>> >>> To Disable clock, it sends cmd with update_clk_regs_only and wait_prvdata_complete bit. >>> I think it's problem..(waiting for prvdata..) >>> >>> If there are ongoing some data(read/write), then before disabling clock, waiting for completing previous data. >>> (But card was already removed, and it couldn't do anything.) >>> It's difficult to analyze the HLE..So After applying first, then we can solve this problem, step by step. >>> >> >> I saw you send a PR for v4.7-fix to Ulf which didn't include this one. >> Do you plan to add it into 4.8 materials? :) > > Yes, I think good that this is prepared for next. (Will apply on this weekend.) > Do you have other opinion? :) No, thanks. I just wanna make sure this one will be merged. :) > > If you have other opinion, i will reflect yours. Thanks! > > Best Regards, > Jaehoon Chung > >> >>> Best Regards, >>> Jaehoon Chung >>> >>>> >>>> >>>>> >>>>> >>>>> -Doug >>>>> >>>>> >>>>> >>>> >>>> >>> >>> >>> >>> >> >> > > > > -- Best Regards Shawn Lin