| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 29 May 2018 15:19:34 +0300
From: Georgi Djakov <georgi.djakov@...aro.org>
To: Vijay Viswanath <vviswana@...eaurora.org>
Cc: adrian.hunter@...el.com, ulf.hansson@...aro.org,
will.deacon@....com, linux-arm-kernel@...ts.infradead.org,
linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, asutoshd@...eaurora.org,
stummala@...eaurora.org, riteshh@...eaurora.org,
subhashj@...eaurora.org,
Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs
which wait for pwr irq
Hello Vijay,
On 09/27/2017 08:34 AM, Vijay Viswanath wrote:
> Register writes which change voltage of IO lines or turn the IO bus
> on/off require controller to be ready before progressing further. When
> the controller is ready, it will generate a power irq which needs to be
> handled. The thread which initiated the register write should wait for
> power irq to complete. This will be done through the new sdhc msm write
> APIs which will check whether the particular write can trigger a power
> irq and wait for it with a timeout if it is expected.
> The SDHC core power control IRQ gets triggered when -
> * There is a state change in power control bit (bit 0)
> of SDHCI_POWER_CONTROL register.
> * There is a state change in 1.8V enable bit (bit 3) of
> SDHCI_HOST_CONTROL2 register.
> * Bit 1 of SDHCI_SOFTWARE_RESET is set.
>
> Also add support APIs which are used by sdhc msm write APIs to check
> if power irq is expected to be generated and wait for the power irq
> to come and complete if the irq is expected.
>
> This patch requires CONFIG_MMC_SDHCI_IO_ACCESSORS to be enabled.
>
> Signed-off-by: Sahitya Tummala <stummala@...eaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@...eaurora.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 173 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 171 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
[..]
> +/*
> + * sdhci_msm_check_power_status API should be called when registers writes
> + * which can toggle sdhci IO bus ON/OFF or change IO lines HIGH/LOW happens.
> + * To what state the register writes will change the IO lines should be passed
> + * as the argument req_type. This API will check whether the IO line's state
> + * is already the expected state and will wait for power irq only if
> + * power irq is expected to be trigerred based on the current IO line state
> + * and expected IO line state.
> + */
> +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);
> + bool done = false;
> +
> + pr_debug("%s: %s: request %d curr_pwr_state %x curr_io_level %x\n",
> + mmc_hostname(host->mmc), __func__, req_type,
> + msm_host->curr_pwr_state, msm_host->curr_io_level);
> +
> + /*
> + * The IRQ for request type IO High/LOW will be generated when -
> + * there is a state change in 1.8V enable bit (bit 3) of
> + * SDHCI_HOST_CONTROL2 register. The reset state of that bit is 0
> + * which indicates 3.3V IO voltage. So, when MMC core layer tries
> + * to set it to 3.3V before card detection happens, the
> + * IRQ doesn't get triggered as there is no state change in this bit.
> + * The driver already handles this case by changing the IO voltage
> + * level to high as part of controller power up sequence. Hence, check
> + * for host->pwr to handle a case where IO voltage high request is
> + * issued even before controller power up.
> + */
> + if ((req_type & REQ_IO_HIGH) && !host->pwr) {
> + pr_debug("%s: do not wait for power IRQ that never comes, req_type: %d\n",
> + mmc_hostname(host->mmc), req_type);
> + return;
> + }
> + if ((req_type & msm_host->curr_pwr_state) ||
> + (req_type & msm_host->curr_io_level))
> + done = true;
> + /*
> + * This is needed here to handle cases where register writes will
> + * not change the current bus state or io level of the controller.
> + * In this case, no power irq will be triggerred and we should
> + * not wait.
> + */
> + if (!done) {
> + if (!wait_event_timeout(msm_host->pwr_irq_wait,
> + msm_host->pwr_irq_flag,
> + msecs_to_jiffies(MSM_PWR_IRQ_TIMEOUT_MS)))
> + __WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
> + mmc_hostname(host->mmc), req_type);
I am seeing the above error message on a db820c device (apq8096). When i
unplug the SD card and then plug it back in, there is a 5 sec card
detection delay and a timeout. Here is a log:
[ 50.253997] mmc0: card 59b4 removed
[ 50.381874] mmc0: sdhci_msm_check_power_status: request 1
curr_pwr_state 2 curr_io_level 4 sdhci_host_ctrl2 b
[ 50.382656] mmc0: sdhci_msm_check_power_status: request 1 done
[ 50.392493] mmc0: sdhci_msm_check_power_status: request 4
curr_pwr_state 1 curr_io_level 4 sdhci_host_ctrl2 b
[ 50.398258] mmc0: sdhci_msm_check_power_status: request 4 done
[ 50.408728] mmc0: sdhci_msm_check_power_status: request 4
curr_pwr_state 1 curr_io_level 4 sdhci_host_ctrl2 8
[ 50.413865] mmc0: sdhci_msm_check_power_status: request 4 done
[ 54.966316] mmc0: sdhci_msm_check_power_status: request 2
curr_pwr_state 1 curr_io_level 4 sdhci_host_ctrl2 8 <-- card is plugged
[ 54.967756] mmc0: sdhci_msm_check_power_status: request 2 done
[ 54.976822] mmc0: sdhci_msm_check_power_status: request 4
curr_pwr_state 2 curr_io_level 8 sdhci_host_ctrl2 8
[ 60.132253] sdhci_msm 74a4900.sdhci: mmc0: pwr_irq for req: (4) timed out
[ 60.132782] mmc0: sdhci_msm_check_power_status: request 4 done
[ 60.139888] mmc0: sdhci_msm_check_power_status: request 4
curr_pwr_state 2 curr_io_level 8 sdhci_host_ctrl2 8
[ 65.252497] sdhci_msm 74a4900.sdhci: mmc0: pwr_irq for req: (4) timed out
The following patch seem to "fix" it.
-- >8 --
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index b2d875afae5f..ca8ad4edcf80 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1049,6 +1049,11 @@ static void sdhci_msm_check_power_status(struct
sdhci_host *host, u32 req_type)
return;
}
+ if (req_type & REQ_IO_LOW &&
+ msm_host->curr_pwr_state & REQ_BUS_ON &&
+ msm_host->curr_io_level & REQ_IO_HIGH &&
+ !host->pwr)
+ done = true;
/*
* The IRQ for request type IO High/LOW will be generated when -
* there is a state change in 1.8V enable bit (bit 3) of
-- >8 --
Reverting this patch series or applying the above patch makes the issue
go away. According to the log, there is no change in the
sdhci_host_ctrl2 register. What do you think?
Thanks,
Georgi
Powered by blists - more mailing lists