linux-kernel - Re: [PATCH 10/11] spi: add more debug prints in s3c64xx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 4 Jun 2015 12:52:19 +0200
From:	Michal Suchanek <hramrach@...il.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	Kukjin Kim <kgene@...nel.org>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Vinod Koul <vinod.koul@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	Han Xu <han.xu@...escale.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Marek Vasut <marex@...x.de>,
	Rafał Miłecki <zajec5@...il.com>,
	Alison Chaiken <alison_chaiken@...tor.com>,
	Huang Shijie <b32955@...escale.com>,
	Ben Hutchings <ben@...adent.org.uk>,
	Knut Wohlrab <knut.wohlrab@...bosch.com>,
	Bean Huo 霍斌斌 (beanhuo) 
	<beanhuo@...ron.com>, "grmoore@...era.com" <grmoore@...era.com>,
	devicetree <devicetree@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
	dmaengine <dmaengine@...r.kernel.org>,
	MTD Maling List <linux-mtd@...ts.infradead.org>,
	linux-spi <linux-spi@...r.kernel.org>
Subject: Re: [PATCH 10/11] spi: add more debug prints in s3c64xx

On 4 June 2015 at 12:26, Mark Brown <broonie@...nel.org> wrote:
> On Thu, Jun 04, 2015 at 11:33:37AM +0200, Michal Suchanek wrote:
>> On 4 June 2015 at 11:16, Mark Brown <broonie@...nel.org> 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.

Yes, this patch should be meaningful on its own.

The ultimate purpose of this patch series is to make the SPI NOR flash
chip usable on boards of which I have 1 sample. If the results on
boards other than mine happen to be different the debug prints will be
useful for users of this flash chip.

There is no code interdependency with the other patches so if it's
preferred I can send patches like this separately.

>
>> 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.

As has been pointed out public interface already exists for getting
the clock name so the issue here is cosmetic.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/