| Message ID | 1439460012-17545-1-git-send-email-xiong.y.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13.08.2015 13:00, Xiong Zhang wrote: > Signed-off-by: Xiong Zhang <xiong.y.zhang@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]?
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.zhang@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.
On Thu, 13 Aug 2015, Xiong Zhang <xiong.y.zhang@intel.com> wrote: > Signed-off-by: Xiong Zhang <xiong.y.zhang@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
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.zhang@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
> 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.zhang@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
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; }
Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 1 + 1 file changed, 1 insertion(+)