[PATCH V3] mmc: sdhci-msm: Ensure SD card power isn't ON when card removed

[PATCH V3] mmc: sdhci-msm: Ensure SD card power isn't ON when card removed

[Sarthak Garg]

Many mobile phones feature multi-card tray designs, where the same
tray is used for both SD and SIM cards. If the SD card is placed
at the outermost location in the tray, the SIM card may come in
contact with SD card power-supply while removing the tray, possibly
resulting in SIM damage.

To prevent that, make sure the SD card is really inserted by reading
the Card Detect pin state. If it's not, turn off the power in
sdhci_msm_check_power_status() and also set the BUS_FAIL power state
on the controller as part of pwr_irq handling for BUS_ON request.

Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 Changes from v2:
 As per Konrad Dybcio's comment :
 - Updated commit text
 - Removed READ_ONCE as mmc->ops will be present always
 - Passed the parameter directly to msm_host_writel
 As per Adrian Hunter's comment :
 - Removed get_cd function as not much use now

 Changes from v1:
 As per Adrian Hunter's comment :
 - Removed unrelated changes
 - Created a separate function get_cd for cleaner code
 - Used READ_ONCE when getting mmc->ops to handle card removal cases
 - Reordered if check conditions
---
 drivers/mmc/host/sdhci-msm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index bf91cb96a0ea..f99fb3b096d6 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1579,6 +1579,7 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct mmc_host *mmc = host->mmc;
 	bool done = false;
 	u32 val = SWITCHABLE_SIGNALING_VOLTAGE;
 	const struct sdhci_msm_offset *msm_offset =
@@ -1636,6 +1637,12 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
 				 "%s: pwr_irq for req: (%d) timed out\n",
 				 mmc_hostname(host->mmc), req_type);
 	}
+
+	if ((req_type & REQ_BUS_ON) && mmc->card && !mmc->ops->get_cd(mmc)) {
+		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
+		host->pwr = 0;
+	}
+
 	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
 			__func__, req_type);
 }
@@ -1694,6 +1701,13 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 		udelay(10);
 	}
 
+	if ((irq_status & CORE_PWRCTL_BUS_ON) && mmc->card &&
+	    !mmc->ops->get_cd(mmc)) {
+		msm_host_writel(msm_host, CORE_PWRCTL_BUS_FAIL, host,
+				msm_offset->core_pwrctl_ctl);
+		return;
+	}
+
 	/* Handle BUS ON/OFF*/
 	if (irq_status & CORE_PWRCTL_BUS_ON) {
 		pwr_state = REQ_BUS_ON;
-- 
2.34.1

Re: [PATCH V3] mmc: sdhci-msm: Ensure SD card power isn't ON when card removed

[Dmitry Baryshkov]

On Tue, Jul 01, 2025 at 03:36:59PM +0530, Sarthak Garg wrote:
> Many mobile phones feature multi-card tray designs, where the same
> tray is used for both SD and SIM cards. If the SD card is placed
> at the outermost location in the tray, the SIM card may come in
> contact with SD card power-supply while removing the tray, possibly
> resulting in SIM damage.
> 
> To prevent that, make sure the SD card is really inserted by reading
> the Card Detect pin state. If it's not, turn off the power in
> sdhci_msm_check_power_status() and also set the BUS_FAIL power state
> on the controller as part of pwr_irq handling for BUS_ON request.
> 
> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Should this be handled by the MMC core instead?

> ---
>  Changes from v2:
>  As per Konrad Dybcio's comment :
>  - Updated commit text
>  - Removed READ_ONCE as mmc->ops will be present always
>  - Passed the parameter directly to msm_host_writel
>  As per Adrian Hunter's comment :
>  - Removed get_cd function as not much use now
> 
>  Changes from v1:
>  As per Adrian Hunter's comment :
>  - Removed unrelated changes
>  - Created a separate function get_cd for cleaner code
>  - Used READ_ONCE when getting mmc->ops to handle card removal cases
>  - Reordered if check conditions
> ---
>  drivers/mmc/host/sdhci-msm.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index bf91cb96a0ea..f99fb3b096d6 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1579,6 +1579,7 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	struct mmc_host *mmc = host->mmc;
>  	bool done = false;
>  	u32 val = SWITCHABLE_SIGNALING_VOLTAGE;
>  	const struct sdhci_msm_offset *msm_offset =
> @@ -1636,6 +1637,12 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
>  				 "%s: pwr_irq for req: (%d) timed out\n",
>  				 mmc_hostname(host->mmc), req_type);
>  	}
> +
> +	if ((req_type & REQ_BUS_ON) && mmc->card && !mmc->ops->get_cd(mmc)) {
> +		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
> +		host->pwr = 0;
> +	}
> +
>  	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
>  			__func__, req_type);
>  }
> @@ -1694,6 +1701,13 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  		udelay(10);
>  	}
>  
> +	if ((irq_status & CORE_PWRCTL_BUS_ON) && mmc->card &&
> +	    !mmc->ops->get_cd(mmc)) {
> +		msm_host_writel(msm_host, CORE_PWRCTL_BUS_FAIL, host,
> +				msm_offset->core_pwrctl_ctl);
> +		return;
> +	}
> +
>  	/* Handle BUS ON/OFF*/
>  	if (irq_status & CORE_PWRCTL_BUS_ON) {
>  		pwr_state = REQ_BUS_ON;
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH V3] mmc: sdhci-msm: Ensure SD card power isn't ON when card removed

[Konrad Dybcio]

On 01-Jul-25 12:27, Dmitry Baryshkov wrote:
> On Tue, Jul 01, 2025 at 03:36:59PM +0530, Sarthak Garg wrote:
>> Many mobile phones feature multi-card tray designs, where the same
>> tray is used for both SD and SIM cards. If the SD card is placed
>> at the outermost location in the tray, the SIM card may come in
>> contact with SD card power-supply while removing the tray, possibly
>> resulting in SIM damage.
>>
>> To prevent that, make sure the SD card is really inserted by reading
>> the Card Detect pin state. If it's not, turn off the power in
>> sdhci_msm_check_power_status() and also set the BUS_FAIL power state
>> on the controller as part of pwr_irq handling for BUS_ON request.
>>
>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Should this be handled by the MMC core instead?

If possible, yes, but if it only works because our hw-specific
sdhci_msm_check_power_status() behaves a certain way, it's good
to go as is. I have no clue if that's the case.

Konrad

Re: [PATCH V3] mmc: sdhci-msm: Ensure SD card power isn't ON when card removed

[Ulf Hansson]

On Tue, 1 Jul 2025 at 14:09, Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
>
>
> On 01-Jul-25 12:27, Dmitry Baryshkov wrote:
> > On Tue, Jul 01, 2025 at 03:36:59PM +0530, Sarthak Garg wrote:
> >> Many mobile phones feature multi-card tray designs, where the same
> >> tray is used for both SD and SIM cards. If the SD card is placed
> >> at the outermost location in the tray, the SIM card may come in
> >> contact with SD card power-supply while removing the tray, possibly
> >> resulting in SIM damage.
> >>
> >> To prevent that, make sure the SD card is really inserted by reading
> >> the Card Detect pin state. If it's not, turn off the power in
> >> sdhci_msm_check_power_status() and also set the BUS_FAIL power state
> >> on the controller as part of pwr_irq handling for BUS_ON request.
> >>
> >> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> >> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> >
> > Should this be handled by the MMC core instead?

Ideally, yes, but honestly I am not sure how.

The current support by the core, detects that cards are being removed
and then it schedules the actual removal/power-off to be managed from
a punted work. This seems too slow for this kind of use-case.

If the core would try to inform the host at an earlier stage, as soon
as the mmc core detects that the card is being removed, would that be
soon enough?

>
> If possible, yes, but if it only works because our hw-specific
> sdhci_msm_check_power_status() behaves a certain way, it's good
> to go as is. I have no clue if that's the case.
>
> Konrad

I agree, sdhci_msm_check_power_status() is certainly HW specific.

That said, I decided to apply this as is, for next, thanks!

Kind regards
Uffe