Re: [PATCH 10/11] spi: add more debug prints in s3c64xx
From: Mark Brown
Date: Thu Jun 04 2015 - 06:27:26 EST
On Thu, Jun 04, 2015 at 11:33:37AM +0200, Michal Suchanek wrote:
> On 4 June 2015 at 11:16, Mark Brown <broonie@xxxxxxxxxx> wrote:
> > Also for this patch (which just adds some trace) there isn't any clear
> > reason for it to be sent as part of the series at all, it doesn't help
> > deliver the functionality and doesn't depend on the rest of the series.
> I used this patch to add missing information to dmesg output so I
> could diagnose the SPI failures so I think it is relevant.
The fact that you used this to debug problems is not relevant to any fix
you might have developed for those problems; the fix can be explained
without needing to know how you got there. Similarly the debugging is
hopefully useful in general without needing to know which particular
problem prompted it's creation.
> To print a clock name you AFAICT need this header. I think this is an
> abstraction problem in the clock framework. Fixing the abstraction
> problem with clock framework would only grow the list of recipients
> which is already so long you complain about it.
This is a bit of a warning sign that the series isn't very focused.
It's also just not good practice, sending patches with obvious problems
(especially obvious problems that aren't clearly flagged up as something
temporary) will frustrate reviewers and can often lead to other issues
being obscured.
Attachment:
signature.asc
Description: Digital signature