[Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case
Signed-off-by: Xiong Zhang --- 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
On Thu, 13 Aug 2015, Xiong Zhang wrote: > Signed-off-by: Xiong Zhang 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
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 > >> --- > >> 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
> 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 > > >> --- > > >> 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
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case
On 13.08.2015 13:36, Timo Aaltonen wrote: > On 13.08.2015 13:00, Xiong Zhang wrote: >> Signed-off-by: Xiong Zhang >> --- >> 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
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Adding break for one case
On 13.08.2015 13:00, Xiong Zhang wrote: > Signed-off-by: Xiong Zhang > --- > 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