s5p-mfc: Remove unreachable code

PATCH In review Mauro Carvalho Chehab 3 participants 4 messages Sep 25, 2012 Media (V4L/DVB)

lore

Share & tools

lore

Mauro Carvalho Chehab

author

2012-09-25 13:28 UTC #0 lore raw

PATCH 1/1 s5p-mfc: Remove unreachable code

Hi Sylwester,

Please review.

Thanks!
Mauro

Forwarded message:

Date: Fri, 14 Sep 2012 14:50:17 +0530
From: Sachin Kamat <[email protected]>
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: [PATCH] [media] s5p-mfc: Remove unreachable code

Code after return statement never gets executed.
Hence can be deleted.

Signed-off-by Sachin Kamat <[email protected]>

---
 drivers/media/platform/s5p-mfc/s5p_mfc.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)

drivers/media/platform/s5p-mfc/s5p_mfc.c +1 ~~-20~~

index e3e616d..56876be 100644
@@ -1144,30 +1144,11 @@ static int s5p_mfc_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct s5p_mfc_dev *m_dev = platform_get_drvdata(pdev);
-	int ret;
 
 	if (m_dev->num_inst == 0)
 		return 0;
-	return s5p_mfc_sleep(m_dev);
-	if (test_and_set_bit(0, &m_dev->enter_suspend) != 0) {
-		mfc_err("Error: going to suspend for a second time\n");
-		return -EIO;
-	}
 
-	/* Check if we're processing then wait if it necessary. */
-	while (test_and_set_bit(0, &m_dev->hw_lock) != 0) {
-		/* Try and lock the HW */
-		/* Wait on the interrupt waitqueue */
-		ret = wait_event_interruptible_timeout(m_dev->queue,
-			m_dev->int_cond || m_dev->ctx[m_dev->curr_ctx]->int_cond,
-			msecs_to_jiffies(MFC_INT_TIMEOUT));
-
-		if (ret == 0) {
-			mfc_err("Waiting for hardware to finish timed out\n");
-			return -EIO;
-		}
-	}
-	return 0;
+	return s5p_mfc_sleep(m_dev);
 }
 
 static int s5p_mfc_resume(struct device *dev)

Signature · 11 lines

1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Regards,
Mauro

Sylwester Nawrocki

2012-09-25 14:07 UTC #1 lore raw

Hi Mauro, Sachin,

Quoted 3 lines · Mauro Carvalho Chehab

Hi Sylwester,
Please review.

I checked it with Kamil and it seems the code being removed is required for proper driver operation. Thus please hold on with this patch. We will prepare a different fix after testing.

Sachin, thanks for reporting that issue. The code below is needed to make sure the MFC is fully suspended after s5p_mfc_suspend() returns. The "return s5p_mfc_sleep(m_dev);" probably just needs to be moved after the while loop.

Quoted 46 lines

Thanks!
Mauro

Forwarded message:

Code after return statement never gets executed.
Hence can be deleted.

Signed-off-by Sachin Kamat <[email protected]>

---
 drivers/media/platform/s5p-mfc/s5p_mfc.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)

drivers/media/platform/s5p-mfc/s5p_mfc.c +0 ~~-14~~

index e3e616d..56876be 100644
@@ -1144,30 +1144,11 @@ static int s5p_mfc_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct s5p_mfc_dev *m_dev = platform_get_drvdata(pdev);
-	int ret;
 
 	if (m_dev->num_inst == 0)
 		return 0;
-	return s5p_mfc_sleep(m_dev);
-	if (test_and_set_bit(0, &m_dev->enter_suspend) != 0) {
-		mfc_err("Error: going to suspend for a second time\n");
-		return -EIO;
-	}
 
-	/* Check if we're processing then wait if it necessary. */
-	while (test_and_set_bit(0, &m_dev->hw_lock) != 0) {
-		/* Try and lock the HW */
-		/* Wait on the interrupt waitqueue */
-		ret = wait_event_interruptible_timeout(m_dev->queue,
-			m_dev->int_cond || m_dev->ctx[m_dev->curr_ctx]->int_cond,
-			msecs_to_jiffies(MFC_INT_TIMEOUT));
-

drivers/media/platform/s5p-mfc/s5p_mfc.c static int s5p_mfc_suspend(struct device *dev) · line 1154

Quoted 10 lines

drivers/media/platform/s5p-mfc/s5p_mfc.c +1 -6

-		if (ret == 0) {
-			mfc_err("Waiting for hardware to finish timed out\n");
-			return -EIO;
-		}
-	}
-	return 0;
+	return s5p_mfc_sleep(m_dev);
 }
 
 static int s5p_mfc_resume(struct device *dev)

Thanks,
Sylwester

Sachin Kamat

2012-09-25 15:25 UTC #2 lore raw

Hi Sylwester,

Quoted 7 lines · Sylwester Nawrocki

Hi Mauro, Sachin,
Quoted 3 lines · Mauro Carvalho Chehab
Hi Sylwester,
Please review.

Quoted 7 lines

I checked it with Kamil and it seems the code being removed is required for proper driver operation. Thus please hold on with this patch. We will prepare a different fix after testing.
Sachin, thanks for reporting that issue. The code below is needed to make sure the MFC is fully suspended after s5p_mfc_suspend() returns.

OK.

The "return s5p_mfc_sleep(m_dev);" probably just needs

Quoted 1 line

to be moved after the while loop.

Do you want me to make this change and re-send the patch?

Quoted 62 lines

Quoted 47 lines

Thanks!
Mauro

Forwarded message:

Code after return statement never gets executed.
Hence can be deleted.

Signed-off-by Sachin Kamat <[email protected]>

---
 drivers/media/platform/s5p-mfc/s5p_mfc.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)

drivers/media/platform/s5p-mfc/s5p_mfc.c +0 ~~-14~~

index e3e616d..56876be 100644
@@ -1144,30 +1144,11 @@ static int s5p_mfc_suspend(struct device *dev)
 {
      struct platform_device *pdev = to_platform_device(dev);
      struct s5p_mfc_dev *m_dev = platform_get_drvdata(pdev);
-     int ret;
 
      if (m_dev->num_inst == 0)
              return 0;
-     return s5p_mfc_sleep(m_dev);
-     if (test_and_set_bit(0, &m_dev->enter_suspend) != 0) {
-             mfc_err("Error: going to suspend for a second time\n");
-             return -EIO;
-     }
 
-     /* Check if we're processing then wait if it necessary. */
-     while (test_and_set_bit(0, &m_dev->hw_lock) != 0) {
-             /* Try and lock the HW */
-             /* Wait on the interrupt waitqueue */
-             ret = wait_event_interruptible_timeout(m_dev->queue,
-                     m_dev->int_cond || m_dev->ctx[m_dev->curr_ctx]->int_cond,
-                     msecs_to_jiffies(MFC_INT_TIMEOUT));
-

drivers/media/platform/s5p-mfc/s5p_mfc.c static int s5p_mfc_suspend(struct device *dev) · line 1154

Quoted 10 lines

drivers/media/platform/s5p-mfc/s5p_mfc.c +1 -6

-             if (ret == 0) {
-                     mfc_err("Waiting for hardware to finish timed out\n");
-                     return -EIO;
-             }
-     }
-     return 0;
+     return s5p_mfc_sleep(m_dev);
 }
 
 static int s5p_mfc_resume(struct device *dev)

Thanks,
Sylwester

Signature · 2 lines

With warm regards,
Sachin

Sylwester Nawrocki

2012-09-25 15:55 UTC #3 lore raw

Hi Sachin,

Quoted 1 line · Sachin Kamat

Do you want me to make this change and re-send the patch?

No, there is no need. Kamil is investigating it and will prepare required patch. Right now I'm not sure if the change suggested is correct or not.

Thanks,
Sylwester