Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Lee Jones
On Wed, 21 Jan 2015, Roger Tseng wrote:

> On Tue, 2015-01-20 at 16:07 +, Lee Jones wrote:
> > On Tue, 20 Jan 2015, Roger Tseng wrote:
> > 
> > > On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
> > > > On Thu, 15 Jan 2015, Roger Tseng wrote:
> > > > 
> > > > > sd_set_power_mode() in derived module 
> > > > > drivers/mmc/host/rtsx_usb_sdmmc.c
> > > > > acquires dev_mutex and then calls pm_runtime_get_sync() to make sure 
> > > > > the
> > > > > device is awake while initializing a newly inserted card. Once it is
> > > > > called during suspending state and explicitly before 
> > > > > rtsx_usb_suspend()
> > > > > acquires the same dev_mutex, both routine deadlock and further hang 
> > > > > the
> > > > > driver because pm_runtime_get_sync() waits the pending PM operations.
> > > > > 
> > > > > Fix this by using an empty suspend method. mmc_core always turns the
> > > > > LED off after a request is done and thus it is ok to remove the only
> > > > > rtsx_usb_turn_off_led() here.
> > > > > 
> > > > > Cc:  # v3.16+
> > > > > Fixes: 730876be2566 ("mfd: Add realtek USB card reader driver")
> > > > > Signed-off-by: Roger Tseng 
> > > > > ---
> > > > >  drivers/mfd/rtsx_usb.c | 9 -
> > > > >  1 file changed, 9 deletions(-)
> > > > 
> > > > Applied, thanks.
> > > > 
> > > I'm sorry but build bot from Intel just reported me that I forgot to
> > > delete an unused variable "ucr" between two commits. My bad.
> > > 
> > > Do I have a chance to send v2?
> > 
> > You're lucky I'm in a good mood. ;)
> > 
> > I fixed it already.
> > 
> That's great, thanks! And thus patch 2/2 also needs to be changed
> accordingly.
> 
> By the way, the build bot reported again about an undefined variable
> build error in 2/2(I'm sorry for this again). I put the overall updated
> content of 2/2 here for you if you're going to fix it, or I can also
> re-send it individually.

Please re-send this patch and prefix it with [OVERWRITE].

In future make sure you test your submission thoroughly prior to
sending, as my time is very limited and with all due respect I could
make better use of it than this.

> diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> index 210d1f85679e..ede50244f265 100644
> --- a/drivers/mfd/rtsx_usb.c
> +++ b/drivers/mfd/rtsx_usb.c
> @@ -681,9 +681,27 @@ static void rtsx_usb_disconnect(struct
> usb_interface *intf)
>  #ifdef CONFIG_PM
>  static int rtsx_usb_suspend(struct usb_interface *intf, pm_message_t
> message)
>  {
> + struct rtsx_ucr *ucr =
> + (struct rtsx_ucr *)usb_get_intfdata(intf);
> + u16 val = 0;
> +
>   dev_dbg(>dev, "%s called with pm message 0x%04x\n",
>   __func__, message.event);
>  
> + if (PMSG_IS_AUTO(message)) {
> + if (mutex_trylock(>dev_mutex)) {
> + rtsx_usb_get_card_status(ucr, );
> + mutex_unlock(>dev_mutex);
> +
> + /* Defer the autosuspend if card exists */
> + if (val & (SD_CD | MS_CD))
> + return -EAGAIN;
> + } else {
> + /* There is an ongoing operation*/
> + return -EAGAIN;
> + }
> + }
> +
>   return 0;
>  }
> 
> 
> 
> > > > > diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> > > > > index dbdd0faeb6ce..076694126e5d 100644
> > > > > --- a/drivers/mfd/rtsx_usb.c
> > > > > +++ b/drivers/mfd/rtsx_usb.c
> > > > > @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
> > > > > *intf, pm_message_t message)
> > > > >   dev_dbg(>dev, "%s called with pm message 0x%04x\n",
> > > > >   __func__, message.event);
> > > > >  
> > > > > - /*
> > > > > -  * Call to make sure LED is off during suspend to save more 
> > > > > power.
> > > > > -  * It is NOT a permanent state and could be turned on anytime 
> > > > > later.
> > > > > -  * Thus no need to call turn_on when resunming.
> > > > > -  */
> > > > > - mutex_lock(>dev_mutex);
> > > > > - rtsx_usb_turn_off_led(ucr);
> > > > > - mutex_unlock(>dev_mutex);
> > > > > -
> > > > >   return 0;
> > > > >  }
> > > > >  
> > > > 
> > > 
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Roger Tseng
On Tue, 2015-01-20 at 16:07 +, Lee Jones wrote:
> On Tue, 20 Jan 2015, Roger Tseng wrote:
> 
> > On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
> > > On Thu, 15 Jan 2015, Roger Tseng wrote:
> > > 
> > > > sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
> > > > acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
> > > > device is awake while initializing a newly inserted card. Once it is
> > > > called during suspending state and explicitly before rtsx_usb_suspend()
> > > > acquires the same dev_mutex, both routine deadlock and further hang the
> > > > driver because pm_runtime_get_sync() waits the pending PM operations.
> > > > 
> > > > Fix this by using an empty suspend method. mmc_core always turns the
> > > > LED off after a request is done and thus it is ok to remove the only
> > > > rtsx_usb_turn_off_led() here.
> > > > 
> > > > Cc:  # v3.16+
> > > > Fixes: 730876be2566 ("mfd: Add realtek USB card reader driver")
> > > > Signed-off-by: Roger Tseng 
> > > > ---
> > > >  drivers/mfd/rtsx_usb.c | 9 -
> > > >  1 file changed, 9 deletions(-)
> > > 
> > > Applied, thanks.
> > > 
> > I'm sorry but build bot from Intel just reported me that I forgot to
> > delete an unused variable "ucr" between two commits. My bad.
> > 
> > Do I have a chance to send v2?
> 
> You're lucky I'm in a good mood. ;)
> 
> I fixed it already.
> 
That's great, thanks! And thus patch 2/2 also needs to be changed
accordingly.

By the way, the build bot reported again about an undefined variable
build error in 2/2(I'm sorry for this again). I put the overall updated
content of 2/2 here for you if you're going to fix it, or I can also
re-send it individually.

diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index 210d1f85679e..ede50244f265 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -681,9 +681,27 @@ static void rtsx_usb_disconnect(struct
usb_interface *intf)
 #ifdef CONFIG_PM
 static int rtsx_usb_suspend(struct usb_interface *intf, pm_message_t
message)
 {
+   struct rtsx_ucr *ucr =
+   (struct rtsx_ucr *)usb_get_intfdata(intf);
+   u16 val = 0;
+
dev_dbg(>dev, "%s called with pm message 0x%04x\n",
__func__, message.event);
 
+   if (PMSG_IS_AUTO(message)) {
+   if (mutex_trylock(>dev_mutex)) {
+   rtsx_usb_get_card_status(ucr, );
+   mutex_unlock(>dev_mutex);
+
+   /* Defer the autosuspend if card exists */
+   if (val & (SD_CD | MS_CD))
+   return -EAGAIN;
+   } else {
+   /* There is an ongoing operation*/
+   return -EAGAIN;
+   }
+   }
+
return 0;
 }



> > > > diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> > > > index dbdd0faeb6ce..076694126e5d 100644
> > > > --- a/drivers/mfd/rtsx_usb.c
> > > > +++ b/drivers/mfd/rtsx_usb.c
> > > > @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
> > > > *intf, pm_message_t message)
> > > > dev_dbg(>dev, "%s called with pm message 0x%04x\n",
> > > > __func__, message.event);
> > > >  
> > > > -   /*
> > > > -* Call to make sure LED is off during suspend to save more 
> > > > power.
> > > > -* It is NOT a permanent state and could be turned on anytime 
> > > > later.
> > > > -* Thus no need to call turn_on when resunming.
> > > > -*/
> > > > -   mutex_lock(>dev_mutex);
> > > > -   rtsx_usb_turn_off_led(ucr);
> > > > -   mutex_unlock(>dev_mutex);
> > > > -
> > > > return 0;
> > > >  }
> > > >  
> > > 
> > 
> 

-- 
Best regards,
Roger Tseng
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Lee Jones
On Tue, 20 Jan 2015, Roger Tseng wrote:

> On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
> > On Thu, 15 Jan 2015, Roger Tseng wrote:
> > 
> > > sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
> > > acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
> > > device is awake while initializing a newly inserted card. Once it is
> > > called during suspending state and explicitly before rtsx_usb_suspend()
> > > acquires the same dev_mutex, both routine deadlock and further hang the
> > > driver because pm_runtime_get_sync() waits the pending PM operations.
> > > 
> > > Fix this by using an empty suspend method. mmc_core always turns the
> > > LED off after a request is done and thus it is ok to remove the only
> > > rtsx_usb_turn_off_led() here.
> > > 
> > > Cc:  # v3.16+
> > > Fixes: 730876be2566 ("mfd: Add realtek USB card reader driver")
> > > Signed-off-by: Roger Tseng 
> > > ---
> > >  drivers/mfd/rtsx_usb.c | 9 -
> > >  1 file changed, 9 deletions(-)
> > 
> > Applied, thanks.
> > 
> I'm sorry but build bot from Intel just reported me that I forgot to
> delete an unused variable "ucr" between two commits. My bad.
> 
> Do I have a chance to send v2?

You're lucky I'm in a good mood. ;)

I fixed it already.

> > > diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> > > index dbdd0faeb6ce..076694126e5d 100644
> > > --- a/drivers/mfd/rtsx_usb.c
> > > +++ b/drivers/mfd/rtsx_usb.c
> > > @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
> > > *intf, pm_message_t message)
> > >   dev_dbg(>dev, "%s called with pm message 0x%04x\n",
> > >   __func__, message.event);
> > >  
> > > - /*
> > > -  * Call to make sure LED is off during suspend to save more power.
> > > -  * It is NOT a permanent state and could be turned on anytime later.
> > > -  * Thus no need to call turn_on when resunming.
> > > -  */
> > > - mutex_lock(>dev_mutex);
> > > - rtsx_usb_turn_off_led(ucr);
> > > - mutex_unlock(>dev_mutex);
> > > -
> > >   return 0;
> > >  }
> > >  
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Roger Tseng
On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
> On Thu, 15 Jan 2015, Roger Tseng wrote:
> 
> > sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
> > acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
> > device is awake while initializing a newly inserted card. Once it is
> > called during suspending state and explicitly before rtsx_usb_suspend()
> > acquires the same dev_mutex, both routine deadlock and further hang the
> > driver because pm_runtime_get_sync() waits the pending PM operations.
> > 
> > Fix this by using an empty suspend method. mmc_core always turns the
> > LED off after a request is done and thus it is ok to remove the only
> > rtsx_usb_turn_off_led() here.
> > 
> > Cc:  # v3.16+
> > Fixes: 730876be2566 ("mfd: Add realtek USB card reader driver")
> > Signed-off-by: Roger Tseng 
> > ---
> >  drivers/mfd/rtsx_usb.c | 9 -
> >  1 file changed, 9 deletions(-)
> 
> Applied, thanks.
> 
I'm sorry but build bot from Intel just reported me that I forgot to
delete an unused variable "ucr" between two commits. My bad.

Do I have a chance to send v2?

> > diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> > index dbdd0faeb6ce..076694126e5d 100644
> > --- a/drivers/mfd/rtsx_usb.c
> > +++ b/drivers/mfd/rtsx_usb.c
> > @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
> > *intf, pm_message_t message)
> > dev_dbg(>dev, "%s called with pm message 0x%04x\n",
> > __func__, message.event);
> >  
> > -   /*
> > -* Call to make sure LED is off during suspend to save more power.
> > -* It is NOT a permanent state and could be turned on anytime later.
> > -* Thus no need to call turn_on when resunming.
> > -*/
> > -   mutex_lock(>dev_mutex);
> > -   rtsx_usb_turn_off_led(ucr);
> > -   mutex_unlock(>dev_mutex);
> > -
> > return 0;
> >  }
> >  
> 

-- 
Best regards,
Roger Tseng


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Roger Tseng
On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
 On Thu, 15 Jan 2015, Roger Tseng wrote:
 
  sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
  acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
  device is awake while initializing a newly inserted card. Once it is
  called during suspending state and explicitly before rtsx_usb_suspend()
  acquires the same dev_mutex, both routine deadlock and further hang the
  driver because pm_runtime_get_sync() waits the pending PM operations.
  
  Fix this by using an empty suspend method. mmc_core always turns the
  LED off after a request is done and thus it is ok to remove the only
  rtsx_usb_turn_off_led() here.
  
  Cc: sta...@vger.kernel.org # v3.16+
  Fixes: 730876be2566 (mfd: Add realtek USB card reader driver)
  Signed-off-by: Roger Tseng rogera...@realtek.com
  ---
   drivers/mfd/rtsx_usb.c | 9 -
   1 file changed, 9 deletions(-)
 
 Applied, thanks.
 
I'm sorry but build bot from Intel just reported me that I forgot to
delete an unused variable ucr between two commits. My bad.

Do I have a chance to send v2?

  diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
  index dbdd0faeb6ce..076694126e5d 100644
  --- a/drivers/mfd/rtsx_usb.c
  +++ b/drivers/mfd/rtsx_usb.c
  @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
  *intf, pm_message_t message)
  dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
  __func__, message.event);
   
  -   /*
  -* Call to make sure LED is off during suspend to save more power.
  -* It is NOT a permanent state and could be turned on anytime later.
  -* Thus no need to call turn_on when resunming.
  -*/
  -   mutex_lock(ucr-dev_mutex);
  -   rtsx_usb_turn_off_led(ucr);
  -   mutex_unlock(ucr-dev_mutex);
  -
  return 0;
   }
   
 

-- 
Best regards,
Roger Tseng


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Lee Jones
On Tue, 20 Jan 2015, Roger Tseng wrote:

 On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
  On Thu, 15 Jan 2015, Roger Tseng wrote:
  
   sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
   acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
   device is awake while initializing a newly inserted card. Once it is
   called during suspending state and explicitly before rtsx_usb_suspend()
   acquires the same dev_mutex, both routine deadlock and further hang the
   driver because pm_runtime_get_sync() waits the pending PM operations.
   
   Fix this by using an empty suspend method. mmc_core always turns the
   LED off after a request is done and thus it is ok to remove the only
   rtsx_usb_turn_off_led() here.
   
   Cc: sta...@vger.kernel.org # v3.16+
   Fixes: 730876be2566 (mfd: Add realtek USB card reader driver)
   Signed-off-by: Roger Tseng rogera...@realtek.com
   ---
drivers/mfd/rtsx_usb.c | 9 -
1 file changed, 9 deletions(-)
  
  Applied, thanks.
  
 I'm sorry but build bot from Intel just reported me that I forgot to
 delete an unused variable ucr between two commits. My bad.
 
 Do I have a chance to send v2?

You're lucky I'm in a good mood. ;)

I fixed it already.

   diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
   index dbdd0faeb6ce..076694126e5d 100644
   --- a/drivers/mfd/rtsx_usb.c
   +++ b/drivers/mfd/rtsx_usb.c
   @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
   *intf, pm_message_t message)
 dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
 __func__, message.event);

   - /*
   -  * Call to make sure LED is off during suspend to save more power.
   -  * It is NOT a permanent state and could be turned on anytime later.
   -  * Thus no need to call turn_on when resunming.
   -  */
   - mutex_lock(ucr-dev_mutex);
   - rtsx_usb_turn_off_led(ucr);
   - mutex_unlock(ucr-dev_mutex);
   -
 return 0;
}

  
 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Lee Jones
On Wed, 21 Jan 2015, Roger Tseng wrote:

 On Tue, 2015-01-20 at 16:07 +, Lee Jones wrote:
  On Tue, 20 Jan 2015, Roger Tseng wrote:
  
   On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
On Thu, 15 Jan 2015, Roger Tseng wrote:

 sd_set_power_mode() in derived module 
 drivers/mmc/host/rtsx_usb_sdmmc.c
 acquires dev_mutex and then calls pm_runtime_get_sync() to make sure 
 the
 device is awake while initializing a newly inserted card. Once it is
 called during suspending state and explicitly before 
 rtsx_usb_suspend()
 acquires the same dev_mutex, both routine deadlock and further hang 
 the
 driver because pm_runtime_get_sync() waits the pending PM operations.
 
 Fix this by using an empty suspend method. mmc_core always turns the
 LED off after a request is done and thus it is ok to remove the only
 rtsx_usb_turn_off_led() here.
 
 Cc: sta...@vger.kernel.org # v3.16+
 Fixes: 730876be2566 (mfd: Add realtek USB card reader driver)
 Signed-off-by: Roger Tseng rogera...@realtek.com
 ---
  drivers/mfd/rtsx_usb.c | 9 -
  1 file changed, 9 deletions(-)

Applied, thanks.

   I'm sorry but build bot from Intel just reported me that I forgot to
   delete an unused variable ucr between two commits. My bad.
   
   Do I have a chance to send v2?
  
  You're lucky I'm in a good mood. ;)
  
  I fixed it already.
  
 That's great, thanks! And thus patch 2/2 also needs to be changed
 accordingly.
 
 By the way, the build bot reported again about an undefined variable
 build error in 2/2(I'm sorry for this again). I put the overall updated
 content of 2/2 here for you if you're going to fix it, or I can also
 re-send it individually.

Please re-send this patch and prefix it with [OVERWRITE].

In future make sure you test your submission thoroughly prior to
sending, as my time is very limited and with all due respect I could
make better use of it than this.

 diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
 index 210d1f85679e..ede50244f265 100644
 --- a/drivers/mfd/rtsx_usb.c
 +++ b/drivers/mfd/rtsx_usb.c
 @@ -681,9 +681,27 @@ static void rtsx_usb_disconnect(struct
 usb_interface *intf)
  #ifdef CONFIG_PM
  static int rtsx_usb_suspend(struct usb_interface *intf, pm_message_t
 message)
  {
 + struct rtsx_ucr *ucr =
 + (struct rtsx_ucr *)usb_get_intfdata(intf);
 + u16 val = 0;
 +
   dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
   __func__, message.event);
  
 + if (PMSG_IS_AUTO(message)) {
 + if (mutex_trylock(ucr-dev_mutex)) {
 + rtsx_usb_get_card_status(ucr, val);
 + mutex_unlock(ucr-dev_mutex);
 +
 + /* Defer the autosuspend if card exists */
 + if (val  (SD_CD | MS_CD))
 + return -EAGAIN;
 + } else {
 + /* There is an ongoing operation*/
 + return -EAGAIN;
 + }
 + }
 +
   return 0;
  }
 
 
 
 diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
 index dbdd0faeb6ce..076694126e5d 100644
 --- a/drivers/mfd/rtsx_usb.c
 +++ b/drivers/mfd/rtsx_usb.c
 @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
 *intf, pm_message_t message)
   dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
   __func__, message.event);
  
 - /*
 -  * Call to make sure LED is off during suspend to save more 
 power.
 -  * It is NOT a permanent state and could be turned on anytime 
 later.
 -  * Thus no need to call turn_on when resunming.
 -  */
 - mutex_lock(ucr-dev_mutex);
 - rtsx_usb_turn_off_led(ucr);
 - mutex_unlock(ucr-dev_mutex);
 -
   return 0;
  }
  

   
  
 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-20 Thread Roger Tseng
On Tue, 2015-01-20 at 16:07 +, Lee Jones wrote:
 On Tue, 20 Jan 2015, Roger Tseng wrote:
 
  On Mon, 2015-01-19 at 09:45 +, Lee Jones wrote:
   On Thu, 15 Jan 2015, Roger Tseng wrote:
   
sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
device is awake while initializing a newly inserted card. Once it is
called during suspending state and explicitly before rtsx_usb_suspend()
acquires the same dev_mutex, both routine deadlock and further hang the
driver because pm_runtime_get_sync() waits the pending PM operations.

Fix this by using an empty suspend method. mmc_core always turns the
LED off after a request is done and thus it is ok to remove the only
rtsx_usb_turn_off_led() here.

Cc: sta...@vger.kernel.org # v3.16+
Fixes: 730876be2566 (mfd: Add realtek USB card reader driver)
Signed-off-by: Roger Tseng rogera...@realtek.com
---
 drivers/mfd/rtsx_usb.c | 9 -
 1 file changed, 9 deletions(-)
   
   Applied, thanks.
   
  I'm sorry but build bot from Intel just reported me that I forgot to
  delete an unused variable ucr between two commits. My bad.
  
  Do I have a chance to send v2?
 
 You're lucky I'm in a good mood. ;)
 
 I fixed it already.
 
That's great, thanks! And thus patch 2/2 also needs to be changed
accordingly.

By the way, the build bot reported again about an undefined variable
build error in 2/2(I'm sorry for this again). I put the overall updated
content of 2/2 here for you if you're going to fix it, or I can also
re-send it individually.

diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index 210d1f85679e..ede50244f265 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -681,9 +681,27 @@ static void rtsx_usb_disconnect(struct
usb_interface *intf)
 #ifdef CONFIG_PM
 static int rtsx_usb_suspend(struct usb_interface *intf, pm_message_t
message)
 {
+   struct rtsx_ucr *ucr =
+   (struct rtsx_ucr *)usb_get_intfdata(intf);
+   u16 val = 0;
+
dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
__func__, message.event);
 
+   if (PMSG_IS_AUTO(message)) {
+   if (mutex_trylock(ucr-dev_mutex)) {
+   rtsx_usb_get_card_status(ucr, val);
+   mutex_unlock(ucr-dev_mutex);
+
+   /* Defer the autosuspend if card exists */
+   if (val  (SD_CD | MS_CD))
+   return -EAGAIN;
+   } else {
+   /* There is an ongoing operation*/
+   return -EAGAIN;
+   }
+   }
+
return 0;
 }



diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index dbdd0faeb6ce..076694126e5d 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface 
*intf, pm_message_t message)
dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
__func__, message.event);
 
-   /*
-* Call to make sure LED is off during suspend to save more 
power.
-* It is NOT a permanent state and could be turned on anytime 
later.
-* Thus no need to call turn_on when resunming.
-*/
-   mutex_lock(ucr-dev_mutex);
-   rtsx_usb_turn_off_led(ucr);
-   mutex_unlock(ucr-dev_mutex);
-
return 0;
 }
 
   
  
 

-- 
Best regards,
Roger Tseng
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-19 Thread Lee Jones
On Thu, 15 Jan 2015, Roger Tseng wrote:

> sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
> acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
> device is awake while initializing a newly inserted card. Once it is
> called during suspending state and explicitly before rtsx_usb_suspend()
> acquires the same dev_mutex, both routine deadlock and further hang the
> driver because pm_runtime_get_sync() waits the pending PM operations.
> 
> Fix this by using an empty suspend method. mmc_core always turns the
> LED off after a request is done and thus it is ok to remove the only
> rtsx_usb_turn_off_led() here.
> 
> Cc:  # v3.16+
> Fixes: 730876be2566 ("mfd: Add realtek USB card reader driver")
> Signed-off-by: Roger Tseng 
> ---
>  drivers/mfd/rtsx_usb.c | 9 -
>  1 file changed, 9 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> index dbdd0faeb6ce..076694126e5d 100644
> --- a/drivers/mfd/rtsx_usb.c
> +++ b/drivers/mfd/rtsx_usb.c
> @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface *intf, 
> pm_message_t message)
>   dev_dbg(>dev, "%s called with pm message 0x%04x\n",
>   __func__, message.event);
>  
> - /*
> -  * Call to make sure LED is off during suspend to save more power.
> -  * It is NOT a permanent state and could be turned on anytime later.
> -  * Thus no need to call turn_on when resunming.
> -  */
> - mutex_lock(>dev_mutex);
> - rtsx_usb_turn_off_led(ucr);
> - mutex_unlock(>dev_mutex);
> -
>   return 0;
>  }
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-19 Thread Lee Jones
On Thu, 15 Jan 2015, Roger Tseng wrote:

 sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
 acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
 device is awake while initializing a newly inserted card. Once it is
 called during suspending state and explicitly before rtsx_usb_suspend()
 acquires the same dev_mutex, both routine deadlock and further hang the
 driver because pm_runtime_get_sync() waits the pending PM operations.
 
 Fix this by using an empty suspend method. mmc_core always turns the
 LED off after a request is done and thus it is ok to remove the only
 rtsx_usb_turn_off_led() here.
 
 Cc: sta...@vger.kernel.org # v3.16+
 Fixes: 730876be2566 (mfd: Add realtek USB card reader driver)
 Signed-off-by: Roger Tseng rogera...@realtek.com
 ---
  drivers/mfd/rtsx_usb.c | 9 -
  1 file changed, 9 deletions(-)

Applied, thanks.

 diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
 index dbdd0faeb6ce..076694126e5d 100644
 --- a/drivers/mfd/rtsx_usb.c
 +++ b/drivers/mfd/rtsx_usb.c
 @@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface *intf, 
 pm_message_t message)
   dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
   __func__, message.event);
  
 - /*
 -  * Call to make sure LED is off during suspend to save more power.
 -  * It is NOT a permanent state and could be turned on anytime later.
 -  * Thus no need to call turn_on when resunming.
 -  */
 - mutex_lock(ucr-dev_mutex);
 - rtsx_usb_turn_off_led(ucr);
 - mutex_unlock(ucr-dev_mutex);
 -
   return 0;
  }
  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-14 Thread Roger Tseng
sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
device is awake while initializing a newly inserted card. Once it is
called during suspending state and explicitly before rtsx_usb_suspend()
acquires the same dev_mutex, both routine deadlock and further hang the
driver because pm_runtime_get_sync() waits the pending PM operations.

Fix this by using an empty suspend method. mmc_core always turns the
LED off after a request is done and thus it is ok to remove the only
rtsx_usb_turn_off_led() here.

Cc:  # v3.16+
Fixes: 730876be2566 ("mfd: Add realtek USB card reader driver")
Signed-off-by: Roger Tseng 
---
 drivers/mfd/rtsx_usb.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index dbdd0faeb6ce..076694126e5d 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface *intf, 
pm_message_t message)
dev_dbg(>dev, "%s called with pm message 0x%04x\n",
__func__, message.event);
 
-   /*
-* Call to make sure LED is off during suspend to save more power.
-* It is NOT a permanent state and could be turned on anytime later.
-* Thus no need to call turn_on when resunming.
-*/
-   mutex_lock(>dev_mutex);
-   rtsx_usb_turn_off_led(ucr);
-   mutex_unlock(>dev_mutex);
-
return 0;
 }
 
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] mfd: rtsx_usb: Fix runtime PM deadlock

2015-01-14 Thread Roger Tseng
sd_set_power_mode() in derived module drivers/mmc/host/rtsx_usb_sdmmc.c
acquires dev_mutex and then calls pm_runtime_get_sync() to make sure the
device is awake while initializing a newly inserted card. Once it is
called during suspending state and explicitly before rtsx_usb_suspend()
acquires the same dev_mutex, both routine deadlock and further hang the
driver because pm_runtime_get_sync() waits the pending PM operations.

Fix this by using an empty suspend method. mmc_core always turns the
LED off after a request is done and thus it is ok to remove the only
rtsx_usb_turn_off_led() here.

Cc: sta...@vger.kernel.org # v3.16+
Fixes: 730876be2566 (mfd: Add realtek USB card reader driver)
Signed-off-by: Roger Tseng rogera...@realtek.com
---
 drivers/mfd/rtsx_usb.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index dbdd0faeb6ce..076694126e5d 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -687,15 +687,6 @@ static int rtsx_usb_suspend(struct usb_interface *intf, 
pm_message_t message)
dev_dbg(intf-dev, %s called with pm message 0x%04x\n,
__func__, message.event);
 
-   /*
-* Call to make sure LED is off during suspend to save more power.
-* It is NOT a permanent state and could be turned on anytime later.
-* Thus no need to call turn_on when resunming.
-*/
-   mutex_lock(ucr-dev_mutex);
-   rtsx_usb_turn_off_led(ucr);
-   mutex_unlock(ucr-dev_mutex);
-
return 0;
 }
 
-- 
2.1.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/