* [PATCH] mmc: core: resume not check card exist before powerup
@ 2024-03-27 2:45 Ricky Wu
2024-03-28 12:38 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Ricky Wu @ 2024-03-27 2:45 UTC (permalink / raw)
To: ulf.hansson, ricky_wu, wenchao.chen, ricardo, marex, linux-mmc,
linux-kernel
_mmc_sd_resume
add get_cd before call powerup, make sure the card exist
Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
---
drivers/mmc/core/sd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 1c8148cdda50..35e036672cfb 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1750,6 +1750,9 @@ static int _mmc_sd_resume(struct mmc_host *host)
if (!mmc_card_suspended(host->card))
goto out;
+ if (host->ops->get_cd && !host->ops->get_cd(host))
+ goto out;
+
mmc_power_up(host, host->card->ocr);
err = mmc_sd_init_card(host, host->card->ocr, host->card);
mmc_card_clr_suspended(host->card);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: resume not check card exist before powerup
2024-03-27 2:45 [PATCH] mmc: core: resume not check card exist before powerup Ricky Wu
@ 2024-03-28 12:38 ` Ulf Hansson
2024-03-29 9:03 ` Ricky WU
2024-04-19 6:53 ` Ricky WU
0 siblings, 2 replies; 6+ messages in thread
From: Ulf Hansson @ 2024-03-28 12:38 UTC (permalink / raw)
To: Ricky Wu; +Cc: wenchao.chen, ricardo, marex, linux-mmc, linux-kernel
On Wed, 27 Mar 2024 at 03:46, Ricky Wu <ricky_wu@realtek.com> wrote:
>
> _mmc_sd_resume
> add get_cd before call powerup, make sure the card exist
Please elaborate more on what problem you are trying to solve, the
above doesn't make much sense to me, sorry.
Yes, the card may be removed while the system is suspended. Then, even
if ->get_cd() indicates that there is no card inserted in the
SD-card-slot, we may still have the card being registered - and then
we need to unregister it.
That said, we need to call mmc_power_up() in order to try to
communicate with the card again. At least that is what the
mmc_rescan() work assumes when it runs the bus_ops->detect() callback
at some point later in time.
>
> Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> ---
> drivers/mmc/core/sd.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 1c8148cdda50..35e036672cfb 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1750,6 +1750,9 @@ static int _mmc_sd_resume(struct mmc_host *host)
> if (!mmc_card_suspended(host->card))
> goto out;
>
> + if (host->ops->get_cd && !host->ops->get_cd(host))
> + goto out;
> +
> mmc_power_up(host, host->card->ocr);
> err = mmc_sd_init_card(host, host->card->ocr, host->card);
> mmc_card_clr_suspended(host->card);
> --
> 2.25.1
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] mmc: core: resume not check card exist before powerup
2024-03-28 12:38 ` Ulf Hansson
@ 2024-03-29 9:03 ` Ricky WU
2024-04-19 6:53 ` Ricky WU
1 sibling, 0 replies; 6+ messages in thread
From: Ricky WU @ 2024-03-29 9:03 UTC (permalink / raw)
To: Ulf Hansson
Cc: wenchao.chen@unisoc.com, ricardo@marliere.net, marex@denx.de,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
> On Wed, 27 Mar 2024 at 03:46, Ricky Wu <ricky_wu@realtek.com> wrote:
> >
> > _mmc_sd_resume
> > add get_cd before call powerup, make sure the card exist
>
> Please elaborate more on what problem you are trying to solve, the
> above doesn't make much sense to me, sorry.
>
> Yes, the card may be removed while the system is suspended. Then, even
> if ->get_cd() indicates that there is no card inserted in the
> SD-card-slot, we may still have the card being registered - and then
> we need to unregister it.
> That said, we need to call mmc_power_up() in order to try to
> communicate with the card again. At least that is what the
> mmc_rescan() work assumes when it runs the bus_ops->detect() callback
> at some point later in time.
>
We saw the power up in a short time from waveform when removing the card,
So we saw mmc_sd_resume call the power up did not check the card exist.
We think this the short time power up maybe cause OCP if no card exist.
And this power up we think is unnecessary when no card exist.
> >
> > Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> > ---
> > drivers/mmc/core/sd.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> > index 1c8148cdda50..35e036672cfb 100644
> > --- a/drivers/mmc/core/sd.c
> > +++ b/drivers/mmc/core/sd.c
> > @@ -1750,6 +1750,9 @@ static int _mmc_sd_resume(struct mmc_host
> *host)
> > if (!mmc_card_suspended(host->card))
> > goto out;
> >
> > + if (host->ops->get_cd && !host->ops->get_cd(host))
> > + goto out;
> > +
> > mmc_power_up(host, host->card->ocr);
> > err = mmc_sd_init_card(host, host->card->ocr, host->card);
> > mmc_card_clr_suspended(host->card);
> > --
> > 2.25.1
> >
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] mmc: core: resume not check card exist before powerup
2024-03-28 12:38 ` Ulf Hansson
2024-03-29 9:03 ` Ricky WU
@ 2024-04-19 6:53 ` Ricky WU
2024-04-25 16:59 ` Ulf Hansson
1 sibling, 1 reply; 6+ messages in thread
From: Ricky WU @ 2024-04-19 6:53 UTC (permalink / raw)
To: Ulf Hansson
Cc: wenchao.chen@unisoc.com, ricardo@marliere.net, marex@denx.de,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Soft reminder
If this solution, you still have concert on sd.c.
can we just do this check card flow in mmc host power up call_back?
> > >
> > > _mmc_sd_resume
> > > add get_cd before call powerup, make sure the card exist
> >
> > Please elaborate more on what problem you are trying to solve, the
> > above doesn't make much sense to me, sorry.
> >
> > Yes, the card may be removed while the system is suspended. Then, even
> > if ->get_cd() indicates that there is no card inserted in the
> > SD-card-slot, we may still have the card being registered - and then
> > we need to unregister it.
> > That said, we need to call mmc_power_up() in order to try to
> > communicate with the card again. At least that is what the
> > mmc_rescan() work assumes when it runs the bus_ops->detect() callback
> > at some point later in time.
> >
>
> We saw the power up in a short time from waveform when removing the card,
> So we saw mmc_sd_resume call the power up did not check the card exist.
>
> We think this the short time power up maybe cause OCP if no card exist.
> And this power up we think is unnecessary when no card exist.
>
>
> > >
> > > Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> > > ---
> > > drivers/mmc/core/sd.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> > > 1c8148cdda50..35e036672cfb 100644
> > > --- a/drivers/mmc/core/sd.c
> > > +++ b/drivers/mmc/core/sd.c
> > > @@ -1750,6 +1750,9 @@ static int _mmc_sd_resume(struct mmc_host
> > *host)
> > > if (!mmc_card_suspended(host->card))
> > > goto out;
> > >
> > > + if (host->ops->get_cd && !host->ops->get_cd(host))
> > > + goto out;
> > > +
> > > mmc_power_up(host, host->card->ocr);
> > > err = mmc_sd_init_card(host, host->card->ocr, host->card);
> > > mmc_card_clr_suspended(host->card);
> > > --
> > > 2.25.1
> > >
> >
> > Kind regards
> > Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: resume not check card exist before powerup
2024-04-19 6:53 ` Ricky WU
@ 2024-04-25 16:59 ` Ulf Hansson
2024-04-29 6:56 ` Ricky WU
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2024-04-25 16:59 UTC (permalink / raw)
To: Ricky WU
Cc: wenchao.chen@unisoc.com, ricardo@marliere.net, marex@denx.de,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
On Fri, 19 Apr 2024 at 08:53, Ricky WU <ricky_wu@realtek.com> wrote:
>
> Soft reminder
>
> If this solution, you still have concert on sd.c.
> can we just do this check card flow in mmc host power up call_back?
>
> > > >
> > > > _mmc_sd_resume
> > > > add get_cd before call powerup, make sure the card exist
> > >
> > > Please elaborate more on what problem you are trying to solve, the
> > > above doesn't make much sense to me, sorry.
> > >
> > > Yes, the card may be removed while the system is suspended. Then, even
> > > if ->get_cd() indicates that there is no card inserted in the
> > > SD-card-slot, we may still have the card being registered - and then
> > > we need to unregister it.
> > > That said, we need to call mmc_power_up() in order to try to
> > > communicate with the card again. At least that is what the
> > > mmc_rescan() work assumes when it runs the bus_ops->detect() callback
> > > at some point later in time.
> > >
> >
> > We saw the power up in a short time from waveform when removing the card,
> > So we saw mmc_sd_resume call the power up did not check the card exist.
Well, that doesn't really tell me if that is a problem or not?
Doesn't the first command sent to the SD card (that has been removed)
fail if there is no card inserted anyway?
> >
> > We think this the short time power up maybe cause OCP if no card exist.
OCP?
> > And this power up we think is unnecessary when no card exist.
It may not be necessary, but in what way does it hurt?
> >
> >
> > > >
> > > > Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> > > > ---
> > > > drivers/mmc/core/sd.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> > > > 1c8148cdda50..35e036672cfb 100644
> > > > --- a/drivers/mmc/core/sd.c
> > > > +++ b/drivers/mmc/core/sd.c
> > > > @@ -1750,6 +1750,9 @@ static int _mmc_sd_resume(struct mmc_host
> > > *host)
> > > > if (!mmc_card_suspended(host->card))
> > > > goto out;
> > > >
> > > > + if (host->ops->get_cd && !host->ops->get_cd(host))
> > > > + goto out;
> > > > +
> > > > mmc_power_up(host, host->card->ocr);
> > > > err = mmc_sd_init_card(host, host->card->ocr, host->card);
> > > > mmc_card_clr_suspended(host->card);
> > > > --
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] mmc: core: resume not check card exist before powerup
2024-04-25 16:59 ` Ulf Hansson
@ 2024-04-29 6:56 ` Ricky WU
0 siblings, 0 replies; 6+ messages in thread
From: Ricky WU @ 2024-04-29 6:56 UTC (permalink / raw)
To: Ulf Hansson
Cc: wenchao.chen@unisoc.com, ricardo@marliere.net, marex@denx.de,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
> >
> > Soft reminder
> >
> > If this solution, you still have concert on sd.c.
> > can we just do this check card flow in mmc host power up call_back?
> >
> > > > >
> > > > > _mmc_sd_resume
> > > > > add get_cd before call powerup, make sure the card exist
> > > >
> > > > Please elaborate more on what problem you are trying to solve, the
> > > > above doesn't make much sense to me, sorry.
> > > >
> > > > Yes, the card may be removed while the system is suspended. Then, even
> > > > if ->get_cd() indicates that there is no card inserted in the
> > > > SD-card-slot, we may still have the card being registered - and then
> > > > we need to unregister it.
> > > > That said, we need to call mmc_power_up() in order to try to
> > > > communicate with the card again. At least that is what the
> > > > mmc_rescan() work assumes when it runs the bus_ops->detect() callback
> > > > at some point later in time.
> > > >
> > >
> > > We saw the power up in a short time from waveform when removing the
> card,
> > > So we saw mmc_sd_resume call the power up did not check the card exist.
>
> Well, that doesn't really tell me if that is a problem or not?
>
> Doesn't the first command sent to the SD card (that has been removed)
> fail if there is no card inserted anyway?
>
> > >
> > > We think this the short time power up maybe cause OCP if no card exist.
>
> OCP?
OCP: over current protection
We think this short time power up has short circuit risk when no card exist
That is why we want to check card exist or not when power up
>
> > > And this power up we think is unnecessary when no card exist.
>
> It may not be necessary, but in what way does it hurt?
>
> > >
> > >
> > > > >
> > > > > Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> > > > > ---
> > > > > drivers/mmc/core/sd.c | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> > > > > 1c8148cdda50..35e036672cfb 100644
> > > > > --- a/drivers/mmc/core/sd.c
> > > > > +++ b/drivers/mmc/core/sd.c
> > > > > @@ -1750,6 +1750,9 @@ static int _mmc_sd_resume(struct
> mmc_host
> > > > *host)
> > > > > if (!mmc_card_suspended(host->card))
> > > > > goto out;
> > > > >
> > > > > + if (host->ops->get_cd && !host->ops->get_cd(host))
> > > > > + goto out;
> > > > > +
> > > > > mmc_power_up(host, host->card->ocr);
> > > > > err = mmc_sd_init_card(host, host->card->ocr, host->card);
> > > > > mmc_card_clr_suspended(host->card);
> > > > > --
>
> Kind regards
> Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-29 6:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 2:45 [PATCH] mmc: core: resume not check card exist before powerup Ricky Wu
2024-03-28 12:38 ` Ulf Hansson
2024-03-29 9:03 ` Ricky WU
2024-04-19 6:53 ` Ricky WU
2024-04-25 16:59 ` Ulf Hansson
2024-04-29 6:56 ` Ricky WU
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.