linux-kernel - Re: [PATCH 1/3] ARM: at91: move at91 aic driver to drivers/irqchip

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]

Message-ID: <20130523115845.GA24476@game.jcrosoft.org>
Date:	Thu, 23 May 2013 13:58:45 +0200
From:	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
To:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Cc:	boris brezillon <b.brezillon@...rkiz.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	Andrew Victor <linux@...im.org.za>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] ARM: at91: move at91 aic driver to drivers/irqchip

On 13:22 Thu 23 May     , Thomas Petazzoni wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> On Thu, 23 May 2013 12:26:01 +0200, Jean-Christophe PLAGNIOL-VILLARD
> wrote:
> 
> > > Did you read what I write? In arch/arm/mach-at91/pm.c, the only
> > > accesses made to the AIC registers are for pr_debug() calls, i.e only
> > > to print some debugging messages. I believe those ones can simply be
> > > removed to decouple the pm.c code from the irq.c code.
> > 
> > yes I read and yes I do not want to move code before we do cleanup MANDATORY
> 
> Then please explain to our volunteer contributor what sort of cleanup
> is required. Simply NACKing without offering any insight as to how
> things should be done is completely useless, counter-productive, and a
> complete abuse of a maintainer's power over contributors.
> 
> > >  * PATCH 1: remove usage of AIC registers for debug messages in pm.c
> > NACK I do want this debug, very usefull to debug pm
> 
> For the cases where you need to debug this stuff, you can always re-add
> some instrumentation in this code to dump whatever you want.
> 
> Or, alternatively, we can make the irqchip driver show using
> pr_debug() the value of IMR in its ->suspend() hook, the value of IMR &
> IPR in its ->resume() hook, and the pm.c code can show the PMC_PCSR on
> suspend. This way, you have all the informations you need for debugging.

if you read the code as you said both of them are link PMC & AIC so you can
not hope to split it as both of the IP contain part of the information

so no I want the information right where it's
> 
> Having the PM driver and the irqchip driver step on each other register
> areas is anyway a bad idea, especially when the only reason to do so is
> for two debugging messages that can be implemented differently.

> 
> Notice that this solution can always be improved later on if needed,
> with further cleanups from you.
> 
> But please stop rejecting valid patches from contributors, just because
> the code is not coming from you. What Boris is proposing is perfectly
so If I read you non clean code can come mainline, wow this not the way to
work. If for you do half clean is enough to go mainline. You really need to
change your way of working.

Best Regards,
J.
--
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/