Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2018-05-30 Thread Georgi Djakov 
Hi Vijay,

On 05/30/2018 10:11 AM, Vijay Viswanath wrote:
> Hi Georgi,
> 
> Thanks for testing the patch on 8096 and pointing out this issue.
> The issue is coming because, when card is removed, the HOST_CONTROL2
> register is retaining the 1.8V Signalling enable bit till SDHCI reset
> happens after a new card is inserted.
> 
> Adding the change you suggested can avoid this wait, but I feel a better
> solution is to clear the 1.8V signalling bit when the card is removed.
> When a new card is inserted, we shouldn't be keeping the 1.8V bit set
> until we send cmd11 to the SD card. A new SD card should start with 3V.
> 
> One solution is to explicitly clear the HOST_CONTROL2 register when card
> is removed.
> 
> Other way is to revert the commit:
> 9718f84b85396e090ca42fafa730410d286d61e3 "mmc: sdhci-msm: Do not reset
> the controller if no card in the slot"
> 
> The sdhci-msm doesn't require "SDHCI_QUIRK_NO_CARD_NO_RESET". The issue
> which above commit is trying to avoid is fixed by the pwr-irq patches.
> Resetting the controller will clear the HOST_CONTROL2 register and avoid
> this issue.
> 
> Can you please try this ? I tested reverting the QUIRK on two platforms:
> db410c(8916) and sdm845. SD card insert/remove worked fine after that
> and I didn't get any "Reset 0x1 never completed" error during card
> insert/remove or shutdown.

Thank you! I have submitted a patch to remove the quirk and tested it on
db410c and db820c.

BR,
Georgi

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2018-05-30 Thread Georgi Djakov 
Hi Vijay,

On 05/30/2018 10:11 AM, Vijay Viswanath wrote:
> Hi Georgi,
> 
> Thanks for testing the patch on 8096 and pointing out this issue.
> The issue is coming because, when card is removed, the HOST_CONTROL2
> register is retaining the 1.8V Signalling enable bit till SDHCI reset
> happens after a new card is inserted.
> 
> Adding the change you suggested can avoid this wait, but I feel a better
> solution is to clear the 1.8V signalling bit when the card is removed.
> When a new card is inserted, we shouldn't be keeping the 1.8V bit set
> until we send cmd11 to the SD card. A new SD card should start with 3V.
> 
> One solution is to explicitly clear the HOST_CONTROL2 register when card
> is removed.
> 
> Other way is to revert the commit:
> 9718f84b85396e090ca42fafa730410d286d61e3 "mmc: sdhci-msm: Do not reset
> the controller if no card in the slot"
> 
> The sdhci-msm doesn't require "SDHCI_QUIRK_NO_CARD_NO_RESET". The issue
> which above commit is trying to avoid is fixed by the pwr-irq patches.
> Resetting the controller will clear the HOST_CONTROL2 register and avoid
> this issue.
> 
> Can you please try this ? I tested reverting the QUIRK on two platforms:
> db410c(8916) and sdm845. SD card insert/remove worked fine after that
> and I didn't get any "Reset 0x1 never completed" error during card
> insert/remove or shutdown.

Thank you! I have submitted a patch to remove the quirk and tested it on
db410c and db820c.

BR,
Georgi

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2018-05-30 Thread Vijay Viswanath 

Hi Georgi,

Thanks for testing the patch on 8096 and pointing out this issue.
The issue is coming because, when card is removed, the HOST_CONTROL2 
register is retaining the 1.8V Signalling enable bit till SDHCI reset 
happens after a new card is inserted.


Adding the change you suggested can avoid this wait, but I feel a better 
solution is to clear the 1.8V signalling bit when the card is removed. 
When a new card is inserted, we shouldn't be keeping the 1.8V bit set 
until we send cmd11 to the SD card. A new SD card should start with 3V.


One solution is to explicitly clear the HOST_CONTROL2 register when card 
is removed.


Other way is to revert the commit: 
9718f84b85396e090ca42fafa730410d286d61e3 "mmc: sdhci-msm: Do not reset 
the controller if no card in the slot"


The sdhci-msm doesn't require "SDHCI_QUIRK_NO_CARD_NO_RESET". The issue 
which above commit is trying to avoid is fixed by the pwr-irq patches. 
Resetting the controller will clear the HOST_CONTROL2 register and avoid 
this issue.


Can you please try this ? I tested reverting the QUIRK on two platforms: 
db410c(8916) and sdm845. SD card insert/remove worked fine after that 
and I didn't get any "Reset 0x1 never completed" error during card 
insert/remove or shutdown.


Thanks,
Vijay

On 5/29/2018 5:49 PM, Georgi Djakov wrote:

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 
Signed-off-by: Vijay Viswanath 
---
  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
+

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2018-05-30 Thread Vijay Viswanath 

Hi Georgi,

Thanks for testing the patch on 8096 and pointing out this issue.
The issue is coming because, when card is removed, the HOST_CONTROL2 
register is retaining the 1.8V Signalling enable bit till SDHCI reset 
happens after a new card is inserted.


Adding the change you suggested can avoid this wait, but I feel a better 
solution is to clear the 1.8V signalling bit when the card is removed. 
When a new card is inserted, we shouldn't be keeping the 1.8V bit set 
until we send cmd11 to the SD card. A new SD card should start with 3V.


One solution is to explicitly clear the HOST_CONTROL2 register when card 
is removed.


Other way is to revert the commit: 
9718f84b85396e090ca42fafa730410d286d61e3 "mmc: sdhci-msm: Do not reset 
the controller if no card in the slot"


The sdhci-msm doesn't require "SDHCI_QUIRK_NO_CARD_NO_RESET". The issue 
which above commit is trying to avoid is fixed by the pwr-irq patches. 
Resetting the controller will clear the HOST_CONTROL2 register and avoid 
this issue.


Can you please try this ? I tested reverting the QUIRK on two platforms: 
db410c(8916) and sdm845. SD card insert/remove worked fine after that 
and I didn't get any "Reset 0x1 never completed" error during card 
insert/remove or shutdown.


Thanks,
Vijay

On 5/29/2018 5:49 PM, Georgi Djakov wrote:

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 
Signed-off-by: Vijay Viswanath 
---
  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
+

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2018-05-29 Thread Georgi Djakov 
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 
> Signed-off-by: Vijay Viswanath 
> ---
>  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

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2018-05-29 Thread Georgi Djakov 
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 
> Signed-off-by: Vijay Viswanath 
> ---
>  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

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-20 Thread Bjorn Andersson 
On Fri 20 Oct 03:54 PDT 2017, Vijay Viswanath wrote:

> On 10/14/2017 1:01 PM, Bjorn Andersson wrote:
> > On Tue 26 Sep 22:34 PDT 2017, Vijay Viswanath wrote:
> > 
> > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> > [..]
> > > + 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);
> > 
> > Bumped my MSM8974AB device to latest linux-next and found this patch; I
> > see this print every five seconds during boot and the eMMC never comes
> > up.
> > 
> > Regards,
> > Bjorn
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> Hi Bjorn,
> 

Hi Vijay,

> I couldn't get one 8974 device. I tested the latest linux-next on db410c and
> its not showing any issue. 8974 is an older msm and has some differences in
> the controller like your "mmc: sdhci-msm: Enable delay circuit calibration
> clocks" patch.
> 

In particular there seems to be some quirks that appeared in 8974pro
(which this is).

> I am currently going through the programming guide to see what could be the
> reason. Can you please share the full logs from the device with me so that I
> can focus my search?
> 

Of course, sorry for the drive-by report without much to go on. I did
some more testing and you can find an extract of the kernel log below.
I was apparently not patient enough before, after 124 seconds we're
through all the timeouts and my eMMC is "functional" - so things are
working, but we're getting stuck waiting for the timeout every time
we're waiting for the pwr irq.


As you can see below we get sdhci_msm_handle_pwr_irq() with status
BUS_ON, so we set io-level HIGH. Then we don't get any more pwr
interrupts, so the io_level remains "high" - although I believe we're
actually low (vqmmc is always 1.8V on this board).

Worth noting is the large comment in sdhci_msm_check_power_status()
related to !host->pwr, host->pwr is always 0 on this board.

# dmesg |grep mmc0
[1.734573] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[1.857611] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[1.889400] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.141634] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.163023] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.238875] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.509617] mmc0: sdhci_msm_check_power_status: request 2 curr_pwr_state 0 
curr_io_level 0
[2.509710] mmc0: sdhci_msm_pwr_irq()
[2.509723] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(30), irq_status=0x2, 
ack=0x1
[2.536468] mmc0: sdhci_msm_check_power_status: request 2 done
[2.546363] mmc0: sdhci_msm_check_power_status: request 8 curr_pwr_state 2 
curr_io_level 8
[2.551220] mmc0: do not wait for power IRQ that never comes, req_type: 8
[2.559502] sdhci_msm f9824900.sdhci: mmc0: clock=0 uhs=0 ctrl_2=0x0
[2.572777] mmc0: sdhci_msm_check_power_status: request 8 curr_pwr_state 2 
curr_io_level 8
[2.577353] mmc0: do not wait for power IRQ that never comes, req_type: 8
[2.592564] mmc0: sdhci_msm_check_power_status: request 8 curr_pwr_state 2 
curr_io_level 8
[2.597230] mmc0: do not wait for power IRQ that never comes, req_type: 8
[2.605589] mmc0: Switching to 3.3V signalling voltage failed
[2.618172] mmc0: sdhci_msm_check_power_status: request 4 curr_pwr_state 2 
curr_io_level 8
[7.687764] sdhci_msm f9824900.sdhci: mmc0: pwr_irq for req: (4) timed out
[7.687795] mmc0: sdhci_msm_check_power_status: request 4 done
[7.717799] mmc0: Setting clock at rate 40 at timing 0
[7.717887] mmc0: sdhci_msm_check_power_status: request 2 curr_pwr_state 2 
curr_io_level 8
[7.722170] mmc0: sdhci_msm_check_power_status: request 2 done
[7.736229] mmc0: sdhci_msm_check_power_status: request 4 curr_pwr_state 2 
curr_io_level 8
[   12.807757] sdhci_msm f9824900.sdhci: mmc0: pwr_irq for req: (4) timed out
[   12.807785] mmc0: sdhci_msm_check_power_status: request 4 done
[   12.813531] sdhci_msm f9824900.sdhci: mmc0: clock=40 uhs=0 ctrl_2=0x8
[   12.826189] mmc0: sdhci_msm_check_power_status: request 4 curr_pwr_state 2 
curr_io_level 8
[   17.847757] sdhci_msm f9824900.sdhci: mmc0: pwr_irq for req: (4) timed out
[   17.847784] mmc0: sdhci_msm_check_power_status: request 4 done
[   17.853520] mmc0: Setting clock at rate 40 at timing 0
[   17.887757] mmc0: SDHCI controller on f9824900.sdhci [f9824900.sdhci] using

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-20 Thread Bjorn Andersson 
On Fri 20 Oct 03:54 PDT 2017, Vijay Viswanath wrote:

> On 10/14/2017 1:01 PM, Bjorn Andersson wrote:
> > On Tue 26 Sep 22:34 PDT 2017, Vijay Viswanath wrote:
> > 
> > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> > [..]
> > > + 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);
> > 
> > Bumped my MSM8974AB device to latest linux-next and found this patch; I
> > see this print every five seconds during boot and the eMMC never comes
> > up.
> > 
> > Regards,
> > Bjorn
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> Hi Bjorn,
> 

Hi Vijay,

> I couldn't get one 8974 device. I tested the latest linux-next on db410c and
> its not showing any issue. 8974 is an older msm and has some differences in
> the controller like your "mmc: sdhci-msm: Enable delay circuit calibration
> clocks" patch.
> 

In particular there seems to be some quirks that appeared in 8974pro
(which this is).

> I am currently going through the programming guide to see what could be the
> reason. Can you please share the full logs from the device with me so that I
> can focus my search?
> 

Of course, sorry for the drive-by report without much to go on. I did
some more testing and you can find an extract of the kernel log below.
I was apparently not patient enough before, after 124 seconds we're
through all the timeouts and my eMMC is "functional" - so things are
working, but we're getting stuck waiting for the timeout every time
we're waiting for the pwr irq.


As you can see below we get sdhci_msm_handle_pwr_irq() with status
BUS_ON, so we set io-level HIGH. Then we don't get any more pwr
interrupts, so the io_level remains "high" - although I believe we're
actually low (vqmmc is always 1.8V on this board).

Worth noting is the large comment in sdhci_msm_check_power_status()
related to !host->pwr, host->pwr is always 0 on this board.

# dmesg |grep mmc0
[1.734573] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[1.857611] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[1.889400] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.141634] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.163023] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.238875] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(0), irq_status=0x0, 
ack=0x0
[2.509617] mmc0: sdhci_msm_check_power_status: request 2 curr_pwr_state 0 
curr_io_level 0
[2.509710] mmc0: sdhci_msm_pwr_irq()
[2.509723] mmc0: sdhci_msm_handle_pwr_irq: Handled IRQ(30), irq_status=0x2, 
ack=0x1
[2.536468] mmc0: sdhci_msm_check_power_status: request 2 done
[2.546363] mmc0: sdhci_msm_check_power_status: request 8 curr_pwr_state 2 
curr_io_level 8
[2.551220] mmc0: do not wait for power IRQ that never comes, req_type: 8
[2.559502] sdhci_msm f9824900.sdhci: mmc0: clock=0 uhs=0 ctrl_2=0x0
[2.572777] mmc0: sdhci_msm_check_power_status: request 8 curr_pwr_state 2 
curr_io_level 8
[2.577353] mmc0: do not wait for power IRQ that never comes, req_type: 8
[2.592564] mmc0: sdhci_msm_check_power_status: request 8 curr_pwr_state 2 
curr_io_level 8
[2.597230] mmc0: do not wait for power IRQ that never comes, req_type: 8
[2.605589] mmc0: Switching to 3.3V signalling voltage failed
[2.618172] mmc0: sdhci_msm_check_power_status: request 4 curr_pwr_state 2 
curr_io_level 8
[7.687764] sdhci_msm f9824900.sdhci: mmc0: pwr_irq for req: (4) timed out
[7.687795] mmc0: sdhci_msm_check_power_status: request 4 done
[7.717799] mmc0: Setting clock at rate 40 at timing 0
[7.717887] mmc0: sdhci_msm_check_power_status: request 2 curr_pwr_state 2 
curr_io_level 8
[7.722170] mmc0: sdhci_msm_check_power_status: request 2 done
[7.736229] mmc0: sdhci_msm_check_power_status: request 4 curr_pwr_state 2 
curr_io_level 8
[   12.807757] sdhci_msm f9824900.sdhci: mmc0: pwr_irq for req: (4) timed out
[   12.807785] mmc0: sdhci_msm_check_power_status: request 4 done
[   12.813531] sdhci_msm f9824900.sdhci: mmc0: clock=40 uhs=0 ctrl_2=0x8
[   12.826189] mmc0: sdhci_msm_check_power_status: request 4 curr_pwr_state 2 
curr_io_level 8
[   17.847757] sdhci_msm f9824900.sdhci: mmc0: pwr_irq for req: (4) timed out
[   17.847784] mmc0: sdhci_msm_check_power_status: request 4 done
[   17.853520] mmc0: Setting clock at rate 40 at timing 0
[   17.887757] mmc0: SDHCI controller on f9824900.sdhci [f9824900.sdhci] using

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-20 Thread Vijay Viswanath 

On 10/14/2017 1:01 PM, Bjorn Andersson wrote:

On Tue 26 Sep 22:34 PDT 2017, Vijay Viswanath wrote:


diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c

[..]

+   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);


Bumped my MSM8974AB device to latest linux-next and found this patch; I
see this print every five seconds during boot and the eMMC never comes
up.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



Hi Bjorn,

I couldn't get one 8974 device. I tested the latest linux-next on db410c 
and its not showing any issue. 8974 is an older msm and has some 
differences in the controller like your "mmc: sdhci-msm: Enable delay 
circuit calibration clocks" patch.


I am currently going through the programming guide to see what could be 
the reason. Can you please share the full logs from the device with me 
so that I can focus my search?


Thanks,
Vijay

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-20 Thread Vijay Viswanath 

On 10/14/2017 1:01 PM, Bjorn Andersson wrote:

On Tue 26 Sep 22:34 PDT 2017, Vijay Viswanath wrote:


diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c

[..]

+   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);


Bumped my MSM8974AB device to latest linux-next and found this patch; I
see this print every five seconds during boot and the eMMC never comes
up.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



Hi Bjorn,

I couldn't get one 8974 device. I tested the latest linux-next on db410c 
and its not showing any issue. 8974 is an older msm and has some 
differences in the controller like your "mmc: sdhci-msm: Enable delay 
circuit calibration clocks" patch.


I am currently going through the programming guide to see what could be 
the reason. Can you please share the full logs from the device with me 
so that I can focus my search?


Thanks,
Vijay

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-14 Thread Bjorn Andersson 
On Tue 26 Sep 22:34 PDT 2017, Vijay Viswanath wrote:

> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
[..]
> + 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);

Bumped my MSM8974AB device to latest linux-next and found this patch; I
see this print every five seconds during boot and the eMMC never comes
up.

Regards,
Bjorn

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-14 Thread Bjorn Andersson 
On Tue 26 Sep 22:34 PDT 2017, Vijay Viswanath wrote:

> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
[..]
> + 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);

Bumped my MSM8974AB device to latest linux-next and found this patch; I
see this print every five seconds during boot and the eMMC never comes
up.

Regards,
Bjorn

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-03 Thread Adrian Hunter 
On 27/09/17 08:34, 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 
> Signed-off-by: Vijay Viswanath 

Acked-by: Adrian Hunter 

> ---
>  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
> index 42a65ab..b72a487 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -123,6 +123,10 @@
>  #define CMUX_SHIFT_PHASE_MASK(7 << CMUX_SHIFT_PHASE_SHIFT)
>  
>  #define MSM_MMC_AUTOSUSPEND_DELAY_MS 50
> +
> +/* Timeout value to avoid infinite waiting for pwr_irq */
> +#define MSM_PWR_IRQ_TIMEOUT_MS 5000
> +
>  struct sdhci_msm_host {
>   struct platform_device *pdev;
>   void __iomem *core_mem; /* MSM SDCC mapped address */
> @@ -138,6 +142,10 @@ struct sdhci_msm_host {
>   bool calibration_done;
>   u8 saved_tuning_phase;
>   bool use_cdclp533;
> + u32 curr_pwr_state;
> + u32 curr_io_level;
> + wait_queue_head_t pwr_irq_wait;
> + bool pwr_irq_flag;
>  };
>  
>  static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
> @@ -995,6 +1003,73 @@ static void sdhci_msm_set_uhs_signaling(struct 
> sdhci_host *host,
>   sdhci_msm_hs400(host, >ios);
>  }
>  
> +static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host 
> *msm_host)
> +{
> + init_waitqueue_head(_host->pwr_irq_wait);
> +}
> +
> +static inline void sdhci_msm_complete_pwr_irq_wait(
> + struct sdhci_msm_host *msm_host)
> +{
> + wake_up(_host->pwr_irq_wait);
> +}
> +
> +/*
> + * 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

Re: [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-10-03 Thread Adrian Hunter 
On 27/09/17 08:34, 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 
> Signed-off-by: Vijay Viswanath 

Acked-by: Adrian Hunter 

> ---
>  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
> index 42a65ab..b72a487 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -123,6 +123,10 @@
>  #define CMUX_SHIFT_PHASE_MASK(7 << CMUX_SHIFT_PHASE_SHIFT)
>  
>  #define MSM_MMC_AUTOSUSPEND_DELAY_MS 50
> +
> +/* Timeout value to avoid infinite waiting for pwr_irq */
> +#define MSM_PWR_IRQ_TIMEOUT_MS 5000
> +
>  struct sdhci_msm_host {
>   struct platform_device *pdev;
>   void __iomem *core_mem; /* MSM SDCC mapped address */
> @@ -138,6 +142,10 @@ struct sdhci_msm_host {
>   bool calibration_done;
>   u8 saved_tuning_phase;
>   bool use_cdclp533;
> + u32 curr_pwr_state;
> + u32 curr_io_level;
> + wait_queue_head_t pwr_irq_wait;
> + bool pwr_irq_flag;
>  };
>  
>  static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
> @@ -995,6 +1003,73 @@ static void sdhci_msm_set_uhs_signaling(struct 
> sdhci_host *host,
>   sdhci_msm_hs400(host, >ios);
>  }
>  
> +static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host 
> *msm_host)
> +{
> + init_waitqueue_head(_host->pwr_irq_wait);
> +}
> +
> +static inline void sdhci_msm_complete_pwr_irq_wait(
> + struct sdhci_msm_host *msm_host)
> +{
> + wake_up(_host->pwr_irq_wait);
> +}
> +
> +/*
> + * 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

[PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-09-26 Thread Vijay Viswanath 
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 
Signed-off-by: Vijay Viswanath 
---
 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
index 42a65ab..b72a487 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -123,6 +123,10 @@
 #define CMUX_SHIFT_PHASE_MASK  (7 << CMUX_SHIFT_PHASE_SHIFT)
 
 #define MSM_MMC_AUTOSUSPEND_DELAY_MS   50
+
+/* Timeout value to avoid infinite waiting for pwr_irq */
+#define MSM_PWR_IRQ_TIMEOUT_MS 5000
+
 struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -138,6 +142,10 @@ struct sdhci_msm_host {
bool calibration_done;
u8 saved_tuning_phase;
bool use_cdclp533;
+   u32 curr_pwr_state;
+   u32 curr_io_level;
+   wait_queue_head_t pwr_irq_wait;
+   bool pwr_irq_flag;
 };
 
 static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
@@ -995,6 +1003,73 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host 
*host,
sdhci_msm_hs400(host, >ios);
 }
 
+static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
+{
+   init_waitqueue_head(_host->pwr_irq_wait);
+}
+
+static inline void sdhci_msm_complete_pwr_irq_wait(
+   struct sdhci_msm_host *msm_host)
+{
+   wake_up(_host->pwr_irq_wait);
+}
+
+/*
+ * 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,
+

[PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq

2017-09-26 Thread Vijay Viswanath 
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 
Signed-off-by: Vijay Viswanath 
---
 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
index 42a65ab..b72a487 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -123,6 +123,10 @@
 #define CMUX_SHIFT_PHASE_MASK  (7 << CMUX_SHIFT_PHASE_SHIFT)
 
 #define MSM_MMC_AUTOSUSPEND_DELAY_MS   50
+
+/* Timeout value to avoid infinite waiting for pwr_irq */
+#define MSM_PWR_IRQ_TIMEOUT_MS 5000
+
 struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -138,6 +142,10 @@ struct sdhci_msm_host {
bool calibration_done;
u8 saved_tuning_phase;
bool use_cdclp533;
+   u32 curr_pwr_state;
+   u32 curr_io_level;
+   wait_queue_head_t pwr_irq_wait;
+   bool pwr_irq_flag;
 };
 
 static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
@@ -995,6 +1003,73 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host 
*host,
sdhci_msm_hs400(host, >ios);
 }
 
+static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
+{
+   init_waitqueue_head(_host->pwr_irq_wait);
+}
+
+static inline void sdhci_msm_complete_pwr_irq_wait(
+   struct sdhci_msm_host *msm_host)
+{
+   wake_up(_host->pwr_irq_wait);
+}
+
+/*
+ * 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,
+