Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1487165 > unrolled thread

[PATCH 0/6] GPU-DRM-GMA500: Fine-tuning for two function implementations

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2016-09-20 11:00 +0200
Last post2016-09-20 14:50 +0200
Articles 17 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/6] GPU-DRM-GMA500: Fine-tuning for two function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 11:00 +0200
    [PATCH 2/6] GPU-DRM-GMA500: Rename a jump label in  mid_get_vbt_data_r10() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 11:00 +0200
      Re: [PATCH 2/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data_r10() Jani Nikula <jani.nikula@linux.intel.com> - 2016-09-20 12:10 +0200
    [PATCH 3/6] GPU-DRM-GMA500: Move a variable assignment in  mid_get_vbt_data_r10() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 11:00 +0200
    [PATCH 4/6] GPU-DRM-GMA500: Fix indentation for a function call  parameter in mid_get_vbt_data_r10() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 11:10 +0200
    [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision  mismatch in mid_get_vbt_data() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 11:10 +0200
      Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data() Jani Nikula <jani.nikula@linux.intel.com> - 2016-09-20 12:10 +0200
        Re: GPU-DRM-GMA500: One error message less for a GCT revision  mismatch in mid_get_vbt_data() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 12:40 +0200
        Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT  revision mismatch in mid_get_vbt_data() Dan Carpenter <dan.carpenter@oracle.com> - 2016-09-20 13:00 +0200
          Re: GPU-DRM-GMA500: One error message less for a GCT revision  mismatch in mid_get_vbt_data() Dan Carpenter <dan.carpenter@oracle.com> - 2016-09-20 13:20 +0200
            Re: GPU-DRM-GMA500: One error message less for a GCT revision  mismatch in mid_get_vbt_data() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 13:40 +0200
          Re: GPU-DRM-GMA500: One error message less for a GCT revision  mismatch in mid_get_vbt_data() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 13:20 +0200
          Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data() Jani Nikula <jani.nikula@linux.intel.com> - 2016-09-20 14:10 +0200
            Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT  revision mismatch in mid_get_vbt_data() Patrik Jakobsson <patrik.r.jakobsson@gmail.com> - 2016-09-20 22:30 +0200
    [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-20 11:10 +0200
      Re: [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data() Jani Nikula <jani.nikula@linux.intel.com> - 2016-09-20 12:10 +0200
        Re: [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in  mid_get_vbt_data() Dan Carpenter <dan.carpenter@oracle.com> - 2016-09-20 14:50 +0200

#1487165 — [PATCH 0/6] GPU-DRM-GMA500: Fine-tuning for two function implementations

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 11:00 +0200
Subject[PATCH 0/6] GPU-DRM-GMA500: Fine-tuning for two function implementations
Message-ID<sjp4d-EF-5@gated-at.bofh.it>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 20 Sep 2016 10:48:04 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (6):
  Use kmalloc_array() in mid_get_vbt_data_r10()
  Rename a jump label in mid_get_vbt_data_r10()
  Move a variable assignment in mid_get_vbt_data_r10()
  Fix indentation for a function call parameter in mid_get_vbt_data_r10()
  One error message less for a GCT revision mismatch in mid_get_vbt_data()
  Rename a jump label in mid_get_vbt_data()

 drivers/gpu/drm/gma500/mid_bios.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

-- 
2.10.0

[toc] | [next] | [standalone]

#1487166 — [PATCH 2/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data_r10()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 11:00 +0200
Subject[PATCH 2/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data_r10()
Message-ID<sjp4d-EF-7@gated-at.bofh.it>
In reply to#1487165
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 20 Sep 2016 09:09:10 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/gma500/mid_bios.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
index a833568..cf4e605 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -242,7 +242,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
 	gct_virtual = ioremap(addr + sizeof(vbt),
 			sizeof(*gct) * vbt.panel_count);
 	if (!gct_virtual)
-		goto out;
+		goto free_gct;
 	memcpy_fromio(gct, gct_virtual, sizeof(*gct));
 	iounmap(gct_virtual);
 
@@ -270,7 +270,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
 	dp_ti->vsync_pulse_width_lo = ti->vsync_pulse_width_lo;
 
 	ret = 0;
-out:
+ free_gct:
 	kfree(gct);
 	return ret;
 }
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1487199 — Re: [PATCH 2/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data_r10()

FromJani Nikula <jani.nikula@linux.intel.com>
Date2016-09-20 12:10 +0200
SubjectRe: [PATCH 2/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data_r10()
Message-ID<sjq9X-1yF-5@gated-at.bofh.it>
In reply to#1487166
On Tue, 20 Sep 2016, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 20 Sep 2016 09:09:10 +0200
>
> Adjust a jump label according to the current Linux coding style convention.

Generally, please don't send patches to fix checkpatch issues in
existing code. Moreover, this particular "convention" is just something
someone sneaked into CodingStyle, it's subjective, and it's probably
going to be removed soon.

NAK.

BR,
Jani.

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/gma500/mid_bios.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
> index a833568..cf4e605 100644
> --- a/drivers/gpu/drm/gma500/mid_bios.c
> +++ b/drivers/gpu/drm/gma500/mid_bios.c
> @@ -242,7 +242,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
>  	gct_virtual = ioremap(addr + sizeof(vbt),
>  			sizeof(*gct) * vbt.panel_count);
>  	if (!gct_virtual)
> -		goto out;
> +		goto free_gct;
>  	memcpy_fromio(gct, gct_virtual, sizeof(*gct));
>  	iounmap(gct_virtual);
>  
> @@ -270,7 +270,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
>  	dp_ti->vsync_pulse_width_lo = ti->vsync_pulse_width_lo;
>  
>  	ret = 0;
> -out:
> + free_gct:
>  	kfree(gct);
>  	return ret;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1487170 — [PATCH 3/6] GPU-DRM-GMA500: Move a variable assignment in mid_get_vbt_data_r10()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 11:00 +0200
Subject[PATCH 3/6] GPU-DRM-GMA500: Move a variable assignment in mid_get_vbt_data_r10()
Message-ID<sjp4e-EF-27@gated-at.bofh.it>
In reply to#1487165
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 20 Sep 2016 10:32:12 +0200

One local variable was set to an error code before a concrete
error situation was detected. Thus move the corresponding assignment into
an if branch to indicate a software failure there.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/gma500/mid_bios.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
index cf4e605..3caee42 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -230,7 +230,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
 	struct gct_r10 *gct;
 	struct oaktrail_timing_info *dp_ti = &dev_priv->gct_data.DTD;
 	struct gct_r10_timing_info *ti;
-	int ret = -1;
+	int ret;
 
 	if (read_vbt_r10(addr, &vbt))
 		return -1;
@@ -241,8 +241,10 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
 
 	gct_virtual = ioremap(addr + sizeof(vbt),
 			sizeof(*gct) * vbt.panel_count);
-	if (!gct_virtual)
+	if (!gct_virtual) {
+		ret = -1;
 		goto free_gct;
+	}
 	memcpy_fromio(gct, gct_virtual, sizeof(*gct));
 	iounmap(gct_virtual);
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1487171 — [PATCH 4/6] GPU-DRM-GMA500: Fix indentation for a function call parameter in mid_get_vbt_data_r10()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 11:10 +0200
Subject[PATCH 4/6] GPU-DRM-GMA500: Fix indentation for a function call parameter in mid_get_vbt_data_r10()
Message-ID<sjpdT-WX-1@gated-at.bofh.it>
In reply to#1487165
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 20 Sep 2016 10:34:28 +0200

Adjust the indentation for a single function call parameter here.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/gma500/mid_bios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
index 3caee42..9004d30 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -240,7 +240,7 @@ static int mid_get_vbt_data_r10(struct drm_psb_private *dev_priv, u32 addr)
 		return -1;
 
 	gct_virtual = ioremap(addr + sizeof(vbt),
-			sizeof(*gct) * vbt.panel_count);
+			      sizeof(*gct) * vbt.panel_count);
 	if (!gct_virtual) {
 		ret = -1;
 		goto free_gct;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1487172 — [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 11:10 +0200
Subject[PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjpdT-WX-13@gated-at.bofh.it>
In reply to#1487165
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 20 Sep 2016 10:36:19 +0200

A single error message should be sufficient to inform about
the detection of an unknown GCT revision at the end.
Thus return after the logging call in this case directly.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/gma500/mid_bios.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
index 9004d30..e5cece0 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -320,6 +320,7 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
 		break;
 	default:
 		dev_err(dev->dev, "Unknown revision of GCT!\n");
+		return;
 	}
 
 out:
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1487200 — Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromJani Nikula <jani.nikula@linux.intel.com>
Date2016-09-20 12:10 +0200
SubjectRe: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjq9X-1yF-9@gated-at.bofh.it>
In reply to#1487172
On Tue, 20 Sep 2016, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 20 Sep 2016 10:36:19 +0200
>
> A single error message should be sufficient to inform about
> the detection of an unknown GCT revision at the end.
> Thus return after the logging call in this case directly.

Did you test this?

BR,
Jani.

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/gma500/mid_bios.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
> index 9004d30..e5cece0 100644
> --- a/drivers/gpu/drm/gma500/mid_bios.c
> +++ b/drivers/gpu/drm/gma500/mid_bios.c
> @@ -320,6 +320,7 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
>  		break;
>  	default:
>  		dev_err(dev->dev, "Unknown revision of GCT!\n");
> +		return;
>  	}
>  
>  out:

-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1487218 — Re: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 12:40 +0200
SubjectRe: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjqCZ-1My-1@gated-at.bofh.it>
In reply to#1487200
>> A single error message should be sufficient to inform about
>> the detection of an unknown GCT revision at the end.
>> Thus return after the logging call in this case directly.
> 
> Did you test this?

What is your software development opinion for this update suggestion?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1487235 — Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromDan Carpenter <dan.carpenter@oracle.com>
Date2016-09-20 13:00 +0200
SubjectRe: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjqWl-1To-9@gated-at.bofh.it>
In reply to#1487200
On Tue, Sep 20, 2016 at 01:07:35PM +0300, Jani Nikula wrote:
> On Tue, 20 Sep 2016, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Tue, 20 Sep 2016 10:36:19 +0200
> >
> > A single error message should be sufficient to inform about
> > the detection of an unknown GCT revision at the end.
> > Thus return after the logging call in this case directly.
> 
> Did you test this?
> 

Don't be a dummy...  This is easy to review an it fixes a bug.

I'm fine with you NAKing all these patches based on who they are from.
I mostly just delete these without responding because the guy has
history of introducing bugs and never listens to feedback.  But asking
pointless rhetorical questions is not helpful.

A lot of people are CC'd and you're wasting everyone's time by asking
questions where you know the answer.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1487241 — Re: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromDan Carpenter <dan.carpenter@oracle.com>
Date2016-09-20 13:20 +0200
SubjectRe: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjrfH-2eQ-7@gated-at.bofh.it>
In reply to#1487235
On Tue, Sep 20, 2016 at 01:03:06PM +0200, SF Markus Elfring wrote:
> Are you looking for any special information to improve
> a corresponding discussion?

If you restricted yourself to only sending bug fixes and not sending
any more cleanups that would be good.

Please stop sending clean up patches.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1487264 — Re: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 13:40 +0200
SubjectRe: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjrz5-2lw-49@gated-at.bofh.it>
In reply to#1487241
> If you restricted yourself to only sending bug fixes and not sending
> any more cleanups that would be good.

Thanks for another bit of constructive feedback.


> Please stop sending clean up patches.

This will not happen for a while.

I am in the process of informing various developers about some software
update opportunities. The proposed changes will belong to a mixture of error
categories as you observe them so far usually.

The involved static source code analysis will point more details out
for further considerations, won't it?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1487251 — Re: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 13:20 +0200
SubjectRe: GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjrfH-2eQ-9@gated-at.bofh.it>
In reply to#1487235
>>> A single error message should be sufficient to inform about
>>> the detection of an unknown GCT revision at the end.
>>> Thus return after the logging call in this case directly.
>>
>> Did you test this?
>>
> 
> Don't be a dummy...  This is easy to review an it fixes a bug.

Thanks for this kind of constructive feedback.


> I'm fine with you NAKing all these patches based on who they are from.

Would you like to clarify such an information a bit more?


> I mostly just delete these without responding because the guy has
> history of introducing bugs and never listens to feedback.

I admit that I'll stumble on programming mistakes again occasionally
as another ordinary free software developer who is struggling various open issues.

I am listening to various feedback. My responses might not be pleasing enough
for you. Are you looking for any special information to improve
a corresponding discussion?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1487275 — Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromJani Nikula <jani.nikula@linux.intel.com>
Date2016-09-20 14:10 +0200
SubjectRe: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjs26-2Lm-1@gated-at.bofh.it>
In reply to#1487235
On Tue, 20 Sep 2016, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Don't be a dummy...  This is easy to review an it fixes a bug.
>
> I'm fine with you NAKing all these patches based on who they are from.
> I mostly just delete these without responding because the guy has
> history of introducing bugs and never listens to feedback.  But asking
> pointless rhetorical questions is not helpful.
>
> A lot of people are CC'd and you're wasting everyone's time by asking
> questions where you know the answer.

Fair enough, sorry for the noise.

To be honest, I did only look at the patches, not who they were from. We
have CI for drm/i915, but I don't think it's constructive to keep
changing drivers like this where the upstream isn't actively developed
and tested. But I digress. It's up to Patrik anyway.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1487615 — Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

FromPatrik Jakobsson <patrik.r.jakobsson@gmail.com>
Date2016-09-20 22:30 +0200
SubjectRe: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()
Message-ID<sjzPY-7R6-13@gated-at.bofh.it>
In reply to#1487275
On Tue, Sep 20, 2016 at 2:08 PM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Tue, 20 Sep 2016, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> Don't be a dummy...  This is easy to review an it fixes a bug.

In this particular case it might not be clear that an unknown GCT
version causes a complete GCT failure so both messages are useful.

>>
>> I'm fine with you NAKing all these patches based on who they are from.
>> I mostly just delete these without responding because the guy has
>> history of introducing bugs and never listens to feedback.  But asking
>> pointless rhetorical questions is not helpful.
>>
>> A lot of people are CC'd and you're wasting everyone's time by asking
>> questions where you know the answer.
>
> Fair enough, sorry for the noise.
>
> To be honest, I did only look at the patches, not who they were from. We
> have CI for drm/i915, but I don't think it's constructive to keep
> changing drivers like this where the upstream isn't actively developed
> and tested. But I digress. It's up to Patrik anyway.

Nothing in this series is very helpful so NAK. In general I'm not fond
of trivial changes like this since it's hard to say what motivates the
author. In theory it shouldn't matter but so far it's been directly
related to the quality of the patches. I can help test changes for
gma500 if needed but please make it worth my while.

Best regards
Patrik

>
> BR,
> Jani.
>
>
>
> --
> Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1487174 — [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-09-20 11:10 +0200
Subject[PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data()
Message-ID<sjpdT-WX-5@gated-at.bofh.it>
In reply to#1487165
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 20 Sep 2016 10:40:22 +0200

* Adjust a jump target.

* Delete the explicit initialisation for the local variable "ret"
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/gma500/mid_bios.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
index e5cece0..602d16f 100644
--- a/drivers/gpu/drm/gma500/mid_bios.c
+++ b/drivers/gpu/drm/gma500/mid_bios.c
@@ -284,7 +284,7 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
 	u8 __iomem *vbt_virtual;
 	struct mid_vbt_header vbt_header;
 	struct pci_dev *pci_gfx_root = pci_get_bus_and_slot(0, PCI_DEVFN(2, 0));
-	int ret = -1;
+	int ret;
 
 	/* Get the address of the platform config vbt */
 	pci_read_config_dword(pci_gfx_root, 0xFC, &addr);
@@ -293,18 +293,18 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
 	dev_dbg(dev->dev, "drm platform config address is %x\n", addr);
 
 	if (!addr)
-		goto out;
+		goto report_failure;
 
 	/* get the virtual address of the vbt */
 	vbt_virtual = ioremap(addr, sizeof(vbt_header));
 	if (!vbt_virtual)
-		goto out;
+		goto report_failure;
 
 	memcpy_fromio(&vbt_header, vbt_virtual, sizeof(vbt_header));
 	iounmap(vbt_virtual);
 
 	if (memcmp(&vbt_header.signature, "$GCT", 4))
-		goto out;
+		goto report_failure;
 
 	dev_dbg(dev->dev, "GCT revision is %02x\n", vbt_header.revision);
 
@@ -322,9 +322,8 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
 		dev_err(dev->dev, "Unknown revision of GCT!\n");
 		return;
 	}
-
-out:
 	if (ret)
+ report_failure:
 		dev_err(dev->dev, "Unable to read GCT!");
 	else
 		dev_priv->has_gct = true;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1487205 — Re: [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data()

FromJani Nikula <jani.nikula@linux.intel.com>
Date2016-09-20 12:10 +0200
SubjectRe: [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data()
Message-ID<sjq9X-1yF-25@gated-at.bofh.it>
In reply to#1487174
On Tue, 20 Sep 2016, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 20 Sep 2016 10:40:22 +0200
>
> * Adjust a jump target.

Please don't.

BR,
Jani.

>
> * Delete the explicit initialisation for the local variable "ret"
>   which became unnecessary with this refactoring.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/gma500/mid_bios.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mid_bios.c b/drivers/gpu/drm/gma500/mid_bios.c
> index e5cece0..602d16f 100644
> --- a/drivers/gpu/drm/gma500/mid_bios.c
> +++ b/drivers/gpu/drm/gma500/mid_bios.c
> @@ -284,7 +284,7 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
>  	u8 __iomem *vbt_virtual;
>  	struct mid_vbt_header vbt_header;
>  	struct pci_dev *pci_gfx_root = pci_get_bus_and_slot(0, PCI_DEVFN(2, 0));
> -	int ret = -1;
> +	int ret;
>  
>  	/* Get the address of the platform config vbt */
>  	pci_read_config_dword(pci_gfx_root, 0xFC, &addr);
> @@ -293,18 +293,18 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
>  	dev_dbg(dev->dev, "drm platform config address is %x\n", addr);
>  
>  	if (!addr)
> -		goto out;
> +		goto report_failure;
>  
>  	/* get the virtual address of the vbt */
>  	vbt_virtual = ioremap(addr, sizeof(vbt_header));
>  	if (!vbt_virtual)
> -		goto out;
> +		goto report_failure;
>  
>  	memcpy_fromio(&vbt_header, vbt_virtual, sizeof(vbt_header));
>  	iounmap(vbt_virtual);
>  
>  	if (memcmp(&vbt_header.signature, "$GCT", 4))
> -		goto out;
> +		goto report_failure;
>  
>  	dev_dbg(dev->dev, "GCT revision is %02x\n", vbt_header.revision);
>  
> @@ -322,9 +322,8 @@ static void mid_get_vbt_data(struct drm_psb_private *dev_priv)
>  		dev_err(dev->dev, "Unknown revision of GCT!\n");
>  		return;
>  	}
> -
> -out:
>  	if (ret)
> + report_failure:
>  		dev_err(dev->dev, "Unable to read GCT!");
>  	else
>  		dev_priv->has_gct = true;

-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1487305 — Re: [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data()

FromDan Carpenter <dan.carpenter@oracle.com>
Date2016-09-20 14:50 +0200
SubjectRe: [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label in mid_get_vbt_data()
Message-ID<sjsEO-31h-15@gated-at.bofh.it>
In reply to#1487205
On Tue, Sep 20, 2016 at 01:08:12PM +0300, Jani Nikula wrote:
> On Tue, 20 Sep 2016, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Tue, 20 Sep 2016 10:40:22 +0200
> >
> > * Adjust a jump target.
> 
> Please don't.
> 

Also there is nothing in CodingStyle that prohibits out: labels.  I
wrote that section and I wrote it in a deliberately way because no one
wants to see a bunch of "cleanup" patches that change the label names
for no reason.

People have been complaining about this in the kernel-summit mailing
list as if CodingStyle bans out labels and that it's my fault.  Neither
of these complaints are accurate.

regards,
dan carpenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web