| Message ID | 287833f5-f1ad-b7f0-c614-d4c903b1c890@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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:
>> 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
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
>>> 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
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
> 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
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.
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
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: