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? > > 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. > > 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 -- 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
