RE: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Jaehoon
Our driver use the poll mode (mmc_rescan will be call every 1 second).
I don't say we can't use the callback function, I just to say if the get_cd is supported, we use it. If no, we still use CMD13 to detect the card status.
 
About performance degrade, we can answer it from theory easily:
We use CMD13 to detect the card. Every time, interrupts will be generated for this command, which will consume CPU time. Then, the time for other task will be decreased, which will degrade its performance.
And I made this conclusion through pressure test on Freescale's board with the tool Iozne (or FIO).

I don't understand you "your commit message isn't reasonable", could you point it out?

Best Regards
Jerry Huang


> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@xxxxxxxxxxx]
> Sent: Friday, March 22, 2013 2:24 PM
> To: Huang Changming-R66093
> Cc: Ulf Hansson; linux-mmc@xxxxxxxxxxxxxxx; Chris Ball
> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
> 
> Hi Jerry,
> 
> Well, i didn't know which SoC you used and how do your controller detect
> the card.
> if we will use callback function, then every driver can prevent the
> performance degrade?
> I didn't think so..
> 
> I think your commit message isn't reasonable.
> 
> Best Regards,
> Jaehoon Chung
> 
> On 03/22/2013 03:11 PM, Huang Changming-R66093 wrote:
> > Hi,
> > If use the non-removable (eMMC), driver does not send the CMD13.
> > But the most case is removable (SD, SDHC or MMC), driver will send
> CMD13 to get the card status.
> > I don't know if your driver sdhci-s3c.c is non-removable.
> > But for removable card, this degrade will be observed when pressure
> test (fio or Iozone).
> >
> > Best Regards
> > Jerry Huang
> >
> >> -----Original Message-----
> >> From: Jaehoon Chung [mailto:jh80.chung@xxxxxxxxxxx]
> >> Sent: Friday, March 22, 2013 1:44 PM
> >> To: Huang Changming-R66093
> >> Cc: Ulf Hansson; linux-mmc@xxxxxxxxxxxxxxx; Chris Ball
> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
> >> card
> >>
> >> Hi,
> >>
> >> As my understanding, You means that controller is sending CMD13 to
> >> detect the card at every time.
> >> Right?
> >> If card is nonremovable (eMMC), need not to send the CMD13 to card
> >> detect and actually didn't send the command.
> >> Then how do you implement the code in get_cd?
> >>
> >> Actually, i didn't see the any performance degrade with sdhci-s3c.c
> >>
> >> Best Regards,
> >> Jaehoon Chung
> >>
> >> On 03/21/2013 12:28 PM, Huang Changming-R66093 wrote:
> >>> In the current codes, SDHCI use the CMD13 to detect the card status.
> >>> So
> >> there are many interrupt for this command, regardless of the card is
> >> absent or present.
> >>> When the card is removed, these interrupts are still generated,
> >>> which
> >> will cause the performance degrade (such as Iozone for SATA I/O
> >> pressure test).
> >>>
> >>> And in the current SD/MMC stack, there is one callback "get_cd"
> >>> which
> >> is used on some platforms.
> >>> So I introduce it to SDHCI, if the driver support this callback, we
> >> first use it, instead of CMD13.
> >>>
> >>> Best Regards
> >>> Jerry Huang
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx]
> >>>> Sent: Wednesday, March 20, 2013 5:29 PM
> >>>> To: Huang Changming-R66093
> >>>> Cc: linux-mmc@xxxxxxxxxxxxxxx; Chris Ball
> >>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
> >>>> detect card
> >>>>
> >>>> On 20 March 2013 10:04, Huang Changming-R66093
> >>>> <r66093@xxxxxxxxxxxxx>
> >>>> wrote:
> >>>>> They have different function, Kevin's patch is for slowly removed,
> >>>>> it
> >>>> can't resolve the performance issue.
> >>>>
> >>>> Actually I am not sure what your patch is trying to solve. Kevin's
> >>>> patch makes sense.
> >>>>
> >>>> Can you elaborate a bit on what performance issues that needs to be
> >>>> considered during card removal?
> >>>>
> >>>>> First, we should use get_cd to get the card status, if it is not
> >>>> supported, then use the alive to send CMD13.
> >>>>> For the driver that supports get_cd, the system performance can
> >> upgrade.
> >>>>> Maybe Kevin can base on this patch.
> >>>>>
> >>>>> Best Regards
> >>>>> Jerry Huang
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx]
> >>>>>> Sent: Wednesday, March 20, 2013 4:43 PM
> >>>>>> To: Huang Changming-R66093
> >>>>>> Cc: linux-mmc@xxxxxxxxxxxxxxx; Chris Ball
> >>>>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
> >>>>>> detect card
> >>>>>>
> >>>>>> On 20 March 2013 06:41,  <Chang-Ming.Huang@xxxxxxxxxxxxx> wrote:
> >>>>>>> From: Jerry Huang <Chang-Ming.Huang@xxxxxxxxxxxxx>
> >>>>>>>
> >>>>>>> In order to check whether the card has been removed, the
> >>>>>>> function
> >>>>>>> mmc_send_status() will send command CMD13 to card and ask the
> >>>>>>> card to send its status register to sdhc driver, which will
> >>>>>>> generate many interrupts repeatedly and make the system
> performance bad.
> >>>>>>> From the performance test on Freescale's board (such as Iozone
> >>>>>>> for SD), the performance will degrade about 4~6%.
> >>>>>>>
> >>>>>>> There is one another way to get this information, which is to
> >>>>>>> read the register PRSSTAT and check the bit CDPL or CINS.
> >>>>>>> If the card is present, these two bit will set to one.
> >>>>>>> Therefore, add callback function get_cd() to check whether the
> >>>>>>> card has been inserted/removed when the driver supports this
> >> feature.
> >>>>>>> If the card is present, 0 will return, if the card is absent, 1
> >>>>>>> will
> >>>>>> return.
> >>>>>>> If the controller will not support this feature, -ENOSYS will
> >> return.
> >>>>>>>
> >>>>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@xxxxxxxxxxxxx>
> >>>>>>> Reviewed-by: Johan Rudholm <johan.rudholm@xxxxxxxxxxxxxx>
> >>>>>>> Reviewed-by: Anton Vorontsov <cbouatmailru@xxxxxxxxx>
> >>>>>>> CC: Chris Ball <cjb@xxxxxxxxxx>
> >>>>>>> ---
> >>>>>>> changes for v2:
> >>>>>>>         - when controller don't support get_cd, return -ENOSYS
> >>>>>>>         - add the CC
> >>>>>>> changes for v3:
> >>>>>>>         - enalbe the controller clock in platform, instead of
> >>>>>>> core changes for v4:
> >>>>>>>         - move the detect code to core.c according to the new
> >>>>>>> structure changes for v5:
> >>>>>>>         - reviewed by Anton and Johan, add the reviewed-by.
> >>>>>>> changes for v6:
> >>>>>>>         - add more comment.
> >>>>>>>
> >>>>>>>  drivers/mmc/core/core.c |   16 ++++++++++++++--
> >>>>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >>>>>>> index 08a3cf2..e225deb 100644
> >>>>>>> --- a/drivers/mmc/core/core.c
> >>>>>>> +++ b/drivers/mmc/core/core.c
> >>>>>>> @@ -2280,7 +2280,7 @@ static int mmc_rescan_try_freq(struct
> >>>>>>> mmc_host *host, unsigned freq)
> >>>>>>>
> >>>>>>>  int _mmc_detect_card_removed(struct mmc_host *host)  {
> >>>>>>> -       int ret;
> >>>>>>> +       int ret = -ENOSYS;
> >>>>>>>
> >>>>>>>         if ((host->caps & MMC_CAP_NONREMOVABLE) ||
> >>>>>>> !host->bus_ops-
> >>>>>>> alive)
> >>>>>>>                 return 0;
> >>>>>>> @@ -2288,7 +2288,19 @@ int _mmc_detect_card_removed(struct
> >>>>>>> mmc_host
> >>>>>> *host)
> >>>>>>>         if (!host->card || mmc_card_removed(host->card))
> >>>>>>>                 return 1;
> >>>>>>>
> >>>>>>> -       ret = host->bus_ops->alive(host);
> >>>>>>> +       /* First, try host-controller's card detect callback */
> >>>>>>> +       if (host->ops->get_cd) {
> >>>>>>> +               ret = host->ops->get_cd(host);
> >>>>>>> +               /*
> >>>>>>> +                * The return value from get_cd: 1 for present,
> >>>>>>> + 0 for
> >>>>>> absent,
> >>>>>>> +                * we need to convert it to: 1 for absent, 0 for
> >>>>>> present.
> >>>>>>> +                */
> >>>>>>> +               if (ret >= 0)
> >>>>>>> +                       ret = !ret;
> >>>>>>> +       }
> >>>>>>> +       /* If failed, back to the bus_ops alive() callback */
> >>>>>>> +       if (ret < 0)
> >>>>>>> +               ret = host->bus_ops->alive(host);
> >>>>>>>         if (ret) {
> >>>>>>>                 mmc_card_set_removed(host->card);
> >>>>>>>                 pr_debug("%s: card remove detected\n",
> >>>>>> mmc_hostname(host));
> >>>>>>
> >>>>>> I guess this already has been fixed by Kevin's patch below.
> >>>>>>
> >>>>>> http://article.gmane.org/gmane.linux.kernel.mmc/19481
> >>>>>>
> >>>>>>> --
> >>>>>>> 1.7.9.5
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> 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
> >>>>>>
> >>>>>> 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
> >>>
> >>
> >
> >
> >
> 


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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux