From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753093AbcEZD7N (ORCPT ); Wed, 25 May 2016 23:59:13 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:42552 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752824AbcEZD7L (ORCPT ); Wed, 25 May 2016 23:59:11 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee690-f79056d000001327-bc-5746748bfaab Content-transfer-encoding: 8BIT Message-id: <5746748B.7080305@samsung.com> Date: Thu, 26 May 2016 12:59:07 +0900 From: Jaehoon Chung User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Shawn Lin Cc: Doug Anderson , Ulf Hansson , Alim Akhtar , Sonny Rao , Heiko Stuebner , Alexandru Stan , "open list:ARM/Rockchip SoC..." , "linux-arm-kernel@lists.infradead.org" , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] mmc: dw_mmc: Consider HLE errors to be data and command errors References: <1426002490-2014-1-git-send-email-dianders@chromium.org> <5502CA4E.9060401@samsung.com> <5506707D.40708@samsung.com> <55189F04.8000404@samsung.com> <573BCC8D.5090606@kernel-upstream.org> <573C3283.1040606@rock-chips.com> <10b3dc0e-aebf-664b-b36b-c54692cd9983@rock-chips.com> <573DBAA5.2070706@samsung.com> <4d8f3992-a1d3-6010-4964-8ca13cddc1c5@rock-chips.com> In-reply-to: <4d8f3992-a1d3-6010-4964-8ca13cddc1c5@rock-chips.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrLIsWRmVeSWpSXmKPExsWyRsSkRLe7xC3c4NlzTYsH87axWTS8mMRq cXbZQTaL/49es1psenyN1eLyrjlsFkf+9zNafHrwn9nizpP1rBZPzsxktDi+NtyB22N2w0UW jzvX9rB5bF5S7/F31n4Wj74tqxg9tl+bx+zxeZNcAHsUl01Kak5mWWqRvl0CV8amGXPZC/7Z VMz42s3YwPhcv4uRk0NCwETi8u7/bBC2mMSFe+uBbC4OIYEVjBKLn11nhil6du4uI0RiKaPE 17ctYB28AoISPybfY+li5OBgFpCXOHIpG8JUl5gyJRei/AGjxMMl+xghyrUkLv6ZwwJiswio Sry5d5wJxGYT0JHY/g3CFhUIk3iwbi8riC0ioCFx4+x1sIOYBf4wS3xd3QaWEBYIlWj50QF1 6Q42icNn2thBNnMKOEk8vckHEpcQ+Msu8WDrTmaIbQIS3yYfAjtUQkBWYtMBqMckJQ6uuMEy gVFsFpJ3ZiG8MwvhnQWMzKsYRVMLkguKk9KLTPSKE3OLS/PS9ZLzczcxAmP19L9nE3Yw3jtg fYhRgINRiYf3hIFbuBBrYllxZe4hRlOgGyYyS4km5wMTQl5JvKGxmZGFqYmpsZG5pZmSOO9r qZ/BQgLpiSWp2ampBalF8UWlOanFhxiZODilGhjbuXJaPErPRrsYvnjk9nFpXGGdsHgk08Q9 W4p5NhbWu23SdNU0t5xfcn37krMvnnydflQ7PuKP+t+Igg3/907tuLH5eFjluvY2fpueu3sX PdohH7zs5pmTzmpsloJyFV/Ttnft2+37Y+Ydr+X7xa9e2L1h7nHN6FMyyxZ8WmKgu/i26e4w ieszlFiKMxINtZiLihMBgu/PKNACAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupileLIzCtJLcpLzFFi42I5/e+xgG53iVu4QfNseYsH87axWTS8mMRq cXbZQTaL/49es1psenyN1eLyrjlsFkf+9zNafHrwn9nizpP1rBZPzsxktDi+NtyB22N2w0UW jzvX9rB5bF5S7/F31n4Wj74tqxg9tl+bx+zxeZNcAHtUA6NNRmpiSmqRQmpecn5KZl66rZJ3 cLxzvKmZgaGuoaWFuZJCXmJuqq2Si0+ArltmDtCNSgpliTmlQKGAxOJiJX07TBNCQ9x0LWAa I3R9Q4LgeowM0EDCGsaMTTPmshf8s6mY8bWbsYHxuX4XIyeHhICJxLNzdxkhbDGJC/fWs3Ux cnEICSxllPj6toUNJMErICjxY/I9li5GDg5mAXmJI5eyIUx1iSlTciHKHzBKPFyyjxGiXEvi 4p85LCA2i4CqxJt7x5lAbDYBHYnt3yBsUYEwiQfr9rKC2CICGhI3zl4H28ss8IdZ4uvqNrCE sECoRMuPDqiDdrBJHD7Txg6ymVPASeLpTb4JjAKzkJw3C+G8WQjnLWBkXsUokVqQXFCclJ5r mJdarlecmFtcmpeul5yfu4kRnA6eSe1gPLjL/RCjAAejEg/vBw63cCHWxLLiytxDjBIczEoi vOb5QCHelMTKqtSi/Pii0pzU4kOMpkD/TWSWEk3OB6aqvJJ4Q2MTMyNLI3NDCyNjcyVx3sf/ 14UJCaQnlqRmp6YWpBbB9DFxcEo1MMYvX/07bdO21Qsn8E6xevbDMqhO53eR2cWZe32PxgVp qPxa63dFWPNy4vGYrb8qZQql2Lby9p62+PzWLM9La45U54bEP9UzeyefTNPmkwu14JhzXGp7 smd22cnK/9ZvK1lvTNir0PdjTs1r2RusReynPa9GNTplH71u2Jq09Ow//gZ7m10HbiqxFGck GmoxFxUnAgADE88hHQMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 wrote: >>>>> Hi >>>>> >>>>> >>>>> On 2016-5-18 12:12, Doug Anderson wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> On Tue, May 17, 2016 at 6:59 PM, Shawn Lin >>>>>> 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? :) If you have other opinion, i will reflect yours. Thanks! Best Regards, Jaehoon Chung > >> Best Regards, >> Jaehoon Chung >> >>> >>> >>>> >>>> >>>> -Doug >>>> >>>> >>>> >>> >>> >> >> >> >> > >