Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

2015-08-14 Thread Jani Nikula
On Thu, 13 Aug 2015, Xiong Zhang xiong.y.zh...@intel.com wrote:
 Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com

Even for a small patch like this, your commit message is inadequate.

First, it's obvious from the code that you're adding a break for one
case. Instead, you should explain what bug you fix. If someone hits this
bug and is looking for a fix, he's not going to find your patch with
this title.

Second, that break was omitted from or removed by some commit, and you
should find out which one it was, and reference it in your commit
message. It's helpful for everyone, and particularly for maintainers and
backporters who need to find that out anyway to decide where this fix
should be applied.

Thanks,
Jani.

 ---
  drivers/gpu/drm/i915/intel_display.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 65cc5b1..801187c 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct drm_i915_private 
 *dev_priv,
   break;
   case PORT_E:
   bit = SDE_PORTE_HOTPLUG_SPT;
 + break;
   default:
   return true;
   }
 -- 
 2.1.4

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

2015-08-14 Thread Daniel Vetter
On Thu, Aug 13, 2015 at 01:37:49PM +0300, Timo Aaltonen wrote:
 On 13.08.2015 13:36, Timo Aaltonen wrote:
  On 13.08.2015 13:00, Xiong Zhang wrote:
  Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c | 1 +
   1 file changed, 1 insertion(+)
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 65cc5b1..801187c 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct 
  drm_i915_private *dev_priv,
 break;
 case PORT_E:
 bit = SDE_PORTE_HOTPLUG_SPT;
  +  break;
 default:
 return true;
 }
 
  
  shouldn't this belong to [5/6]?
 
 Nevermind, I see now that it got merged already.

I dropped that patch again so that we can rectify this properly. Jani's
complaint about the sub-par commit message still holds though, like why
was this not caught in testing?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

2015-08-14 Thread Zhang, Xiong Y
 On Thu, Aug 13, 2015 at 01:37:49PM +0300, Timo Aaltonen wrote:
  On 13.08.2015 13:36, Timo Aaltonen wrote:
   On 13.08.2015 13:00, Xiong Zhang wrote:
   Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com
   ---
drivers/gpu/drm/i915/intel_display.c | 1 +
1 file changed, 1 insertion(+)
  
   diff --git a/drivers/gpu/drm/i915/intel_display.c
   b/drivers/gpu/drm/i915/intel_display.c
   index 65cc5b1..801187c 100644
   --- a/drivers/gpu/drm/i915/intel_display.c
   +++ b/drivers/gpu/drm/i915/intel_display.c
   @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct
 drm_i915_private *dev_priv,
break;
case PORT_E:
bit = SDE_PORTE_HOTPLUG_SPT;
   +break;
default:
return true;
}
  
  
   shouldn't this belong to [5/6]?
 
  Nevermind, I see now that it got merged already.
 
 I dropped that patch again so that we can rectify this properly. Jani's 
 complaint
 about the sub-par commit message still holds though, like why was this not
 caught in testing?
[Zhang, Xiong Y] Yes, it's better to drop it. I will explain it the commit 
message and resent the patch.
 -Daniel

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

2015-08-13 Thread Xiong Zhang
Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 65cc5b1..801187c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct drm_i915_private 
*dev_priv,
break;
case PORT_E:
bit = SDE_PORTE_HOTPLUG_SPT;
+   break;
default:
return true;
}
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

2015-08-13 Thread Timo Aaltonen
On 13.08.2015 13:00, Xiong Zhang wrote:
 Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 65cc5b1..801187c 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct drm_i915_private 
 *dev_priv,
   break;
   case PORT_E:
   bit = SDE_PORTE_HOTPLUG_SPT;
 + break;
   default:
   return true;
   }
 

shouldn't this belong to [5/6]?


-- 
t
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case

2015-08-13 Thread Timo Aaltonen
On 13.08.2015 13:36, Timo Aaltonen wrote:
 On 13.08.2015 13:00, Xiong Zhang wrote:
 Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 65cc5b1..801187c 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -1100,6 +1100,7 @@ bool ibx_digital_port_connected(struct 
 drm_i915_private *dev_priv,
  break;
  case PORT_E:
  bit = SDE_PORTE_HOTPLUG_SPT;
 +break;
  default:
  return true;
  }

 
 shouldn't this belong to [5/6]?

Nevermind, I see now that it got merged already.


-- 
t
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx