[PATCH] sm501fb: Silence a coccinelle script warning.

According to surrounding goto, it is likely that 'goto err_probed_panel' is
expected here.
This change is just done in order to silence some coccinelle scripts
which try to detect wrongly ordered goto.

If 'info->fb[HEAD_PANEL]' and' 'info->fb[HEAD_CRT]' are both NULL, this
means that no 'framebuffer_alloc' has been performed, so 'goto err_alloc'
looks fine.
Anyway, it is also harmless in this case to call 'framebuffer_release'.
The code looks more straight forward.

Signed-off-by: Christophe JAILLET <redacted>
---
 drivers/video/fbdev/sm501fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index 67e314fdd947..4d89b045ce40 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c

@@ -1990,7 +1990,7 @@ static int sm501fb_probe(struct platform_device *pdev)
 	    info->fb[HEAD_CRT] = NULL) {
 		dev_err(dev, "no framebuffers found\n");
 		ret = -ENODEV;
-		goto err_alloc;
+		goto err_probed_panel;
 	}
 
 	/* get the resources for both of the framebuffers */

-- 
2.11.0

Hi,

On Monday, May 08, 2017 08:11:16 AM Christophe JAILLET wrote:

According to surrounding goto, it is likely that 'goto err_probed_panel' is
expected here.
This change is just done in order to silence some coccinelle scripts
which try to detect wrongly ordered goto.

If 'info->fb[HEAD_PANEL]' and' 'info->fb[HEAD_CRT]' are both NULL, this
means that no 'framebuffer_alloc' has been performed, so 'goto err_alloc'
looks fine.
Anyway, it is also harmless in this case to call 'framebuffer_release'.
The code looks more straight forward.

Signed-off-by: Christophe JAILLET <redacted>

It seems that the code for supporting only selected framebuffers
(only HEAD_PANEL fb or only HEAD_CRT fb) is broken anyway as at least
the suspend/resume support assumes that both framebuffers are always
present. Also all sm501fb driver users always try try to initialize
both framebuffers. Therefore I would prefer the removal of non-working
individual framebuffers support (the code that your patch modifies
would be removed as well). Could you please look into it?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Le 14/06/2017 à 12:54, Bartlomiej Zolnierkiewicz a écrit :

Hi,

On Monday, May 08, 2017 08:11:16 AM Christophe JAILLET wrote:

quoted

According to surrounding goto, it is likely that 'goto err_probed_panel' is
expected here.
This change is just done in order to silence some coccinelle scripts
which try to detect wrongly ordered goto.

If 'info->fb[HEAD_PANEL]' and' 'info->fb[HEAD_CRT]' are both NULL, this
means that no 'framebuffer_alloc' has been performed, so 'goto err_alloc'
looks fine.
Anyway, it is also harmless in this case to call 'framebuffer_release'.
The code looks more straight forward.

Signed-off-by: Christophe JAILLET <redacted>

It seems that the code for supporting only selected framebuffers
(only HEAD_PANEL fb or only HEAD_CRT fb) is broken anyway as at least
the suspend/resume support assumes that both framebuffers are always
present. Also all sm501fb driver users always try try to initialize
both framebuffers. Therefore I would prefer the removal of non-working
individual framebuffers support (the code that your patch modifies
would be removed as well). Could you please look into it?

Thx for the feedback, but no, I won't be able to go further.
I don't know the code enough in this area.
Sorry.

Best regards,
CJ

Hi Bartlomiej,

On Wed, Jun 14, 2017 at 12:54:58PM +0200, Bartlomiej Zolnierkiewicz wrote:

Hi,

On Monday, May 08, 2017 08:11:16 AM Christophe JAILLET wrote:

quoted

According to surrounding goto, it is likely that 'goto err_probed_panel' is
expected here.
This change is just done in order to silence some coccinelle scripts
which try to detect wrongly ordered goto.

If 'info->fb[HEAD_PANEL]' and' 'info->fb[HEAD_CRT]' are both NULL, this
means that no 'framebuffer_alloc' has been performed, so 'goto err_alloc'
looks fine.
Anyway, it is also harmless in this case to call 'framebuffer_release'.
The code looks more straight forward.

Signed-off-by: Christophe JAILLET <redacted>

It seems that the code for supporting only selected framebuffers
(only HEAD_PANEL fb or only HEAD_CRT fb) is broken anyway as at least
the suspend/resume support assumes that both framebuffers are always
present. Also all sm501fb driver users always try try to initialize
both framebuffers. Therefore I would prefer the removal of non-working
individual framebuffers support (the code that your patch modifies
would be removed as well). Could you please look into it?

Looks like it will be easier to fix the suspend/resume functions and
the part of probe which creates the device files for both.
Let me have a look and I will try to send you a patch over the weekend.


--
Regards
Sudip