2013/4/5 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: > On 25 March 2013 16:27, Kevin Liu <keyuan.liu@xxxxxxxxx> wrote: >> 2013/3/25 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: >>> On 22 March 2013 16:27, Kevin Liu <keyuan.liu@xxxxxxxxx> wrote: >>>> 2013/3/22 Ulf Hansson <ulf.hansson@xxxxxxxxxx>: >>>>> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@xxxxxxxxxxxxx> wrote: >>>>>> >>>>>> <r66093@xxxxxxxxxxxxx>: >>>>>>> >> >> Jerry, >>>>>>> >> >> >>>>>>> >> >> Function _mmc_detect_card_removed should only be called when card >>>>>>> >> >> insert/removal or i/o error occur unless POLLING is used(called >>>>>>> >> >> once per second). So it should _not_ impact performance much. >>>>>>> >> > [jerry] >>>>>>> >> > No, your understanding is wrong. >>>>>>> >> > Function "_mmc_detect_card_removed " is called to check if our card >>>>>>> >> > has >>>>>>> >> been removed when driver run the function "mmc_rescan" every time. >>>>>>> >> > >>>>>>> >> >>>>>>> >> But when will mmc_rescan be called? >>>>>>> >> Besides boot up, only when card status change it will be called. >>>>>>> >> So usually it should _not_ be called frequently. >>>>>>> > [jerry] >>>>>>> > For poll mode, how to know card status changed? >>>>>>> > The answer is "mmc_rescan". >>>>>>> > maybe you could read the last two lines of this function ago. >>>>>>> > >>>>>>> >>>>>>> I mentioned this in my first response "unless POLLING is used(called once >>>>>>> per second)". I was a bit confused firstly. >>>>>>> I think slot-gpio is a better solution, with which you don't need the >>>>>>> every second polling. >>>>>> [jerry] >>>>>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it. >>>>>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards. >>>>>> >>>>>>> >> >> POLLING is poor for performance, why not change to slot-gpio if >>>>>>> >> >> host is broken for card detect. >>>>>>> >> > [jerry] >>>>>>> >> > Some our boards can't support interrupt mode to detect card >>>>>>> >> insert/removable, so we unify the poll mode. >>>>>>> >> > >>>>>>> >> >>>>>>> >> Not support gpio interrupt? >>>>>>> >> I think you only don't support sdh host interrupt. >>>>>>> > >>>>>>> -- >>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >>>>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at >>>>>>> http://vger.kernel.org/majordomo-info.html >>>>>> >>>>>> >>>>> >>>>> A suggestion; the poll time out is 1s today. We could make some more >>>>> intelligent update to the timeout value so we decrease the number of >>>>> timeouts to happen. In other words minimize the number of mmc_rescan >>>>> to be executed. >>>>> >>>>> 1. When no card is inserted, use 1 s. >>>>> 2. When card is inserted, switch to 30 s timeout. The card removal >>>>> will be detected anyway when a blk err occurs, due to that >>>>> mmc_detect_card_removed will be called from the block layer at error >>>>> handling path. >>>>> >>>> >>>> Ulf, >>>> >>>> Sounds good! But I have one concern: >>>> If the card is removed when there is no i/o access. Then the >>>> mmc_rescan won't run until 30 seconds later (just imagine the worst >>>> case). If within 30 seconds, the card is inserted again or another >>>> different card is inserted, then issue may occur. >>> >>> True! >>> >>> Do you think a timeout of say 10 s could be more more acceptable? >>> >> >> I prefer below solution. >> >>>> >>>> Then I have a suggestion as below: >>>> 1. keep current code as 1 second in mmc_rescan for polling. >>>> 2. When an i/o request come, we can cancel current delay work and >>>> reschedule the delay detect work after 1 second from that point. >>>> 3. If next i/o request come within 1 second, we can reschedule the >>>> detect work after 1 second from that point again. >>> >>> Well, I actually thought of such a solution as well, but I kind of >>> dropped it because it felt a bit messy. Although, I guess you are >>> right, that is probably the most proper way of doing it. >>> >>> Giving it some more thoughts, I guess we should utilize the runtime pm >>> callbacks for the sd card bus_ops somehow. I pushed a skeleton >>> patchset for this a while ago. If it gets merged of course. >>> >>> 1.mmc_rescan could check the runtime status, if active, skip the rescan. >>> 2. Or, make the runtime supend callback schedule a rescan work. >>> >> >> Good idea! >> Agree with either of above two. Maybe the second is better. Cancel the >> rescan work in runtime resume callback while schedule the rescan in >> runtime suspend. >> I think above should can handle normal cases. >> To handle the special case that card is removed during i/o operations, >> can remove MMC_CAP2_DETECT_ON_ERR and use MMC_CAP_NEEDS_POLL in >> mmc_detect_card_removed when error occur as your patch did. >> >>>> >>>> So during continuous i/o operations, there will be no mmc_rescan execution. >>>> Once i/o operation stopped, the mmc_rescan will be called every 1 second. >>>> This way can solve above issue. >>>> How do you think? >>>> >>>> Thanks >>>> Kevin >>>> >>>>> Any thoughts? >>>>> >>>>> Kind regards >>>>> Ulf Hansson > > Hi Kevin, > > For reference, I have sent a patch which removes the > MMC_CAP2_DETECT_ON_ERR and instead use MMC_CAP_NEEDS_POLL for the same > code. I will send another patch for the second part, if the runtime PM > patchset gets merged. > Ulf, I noticed that patch and added my review. Thanks for the reminder. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
