linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: plagnioj@jcrosoft.com (Jean-Christophe PLAGNIOL-VILLARD)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] ARM: at91: move at91 aic driver to drivers/irqchip
Date: Thu, 23 May 2013 13:58:45 +0200	[thread overview]
Message-ID: <20130523115845.GA24476@game.jcrosoft.org> (raw)
In-Reply-To: <20130523132255.3acf4431@skate>

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.

  reply	other threads:[~2013-05-23 11:58 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-23  9:05 [RESEND PATCH 0/3] ARM: at91: move aic driver to drivers/irqchip Boris BREZILLON
2013-05-23  9:05 ` [PATCH 1/3] ARM: at91: move at91 " Boris BREZILLON
2013-05-23  9:06   ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23  9:51     ` boris brezillon
2013-05-23 10:09       ` Thomas Petazzoni
2013-05-23 10:09         ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 10:20           ` Thomas Petazzoni
2013-05-23 10:26             ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 11:22               ` Thomas Petazzoni
2013-05-23 11:58                 ` Jean-Christophe PLAGNIOL-VILLARD [this message]
2013-05-23 12:09                   ` Thomas Petazzoni
2013-05-23 13:00                 ` Nicolas Ferre
2013-05-23 10:18   ` Russell King - ARM Linux
2013-05-23 11:59     ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 12:13     ` Thomas Petazzoni
2013-05-23 12:42       ` Boris BREZILLON
2013-05-23 12:44       ` boris brezillon
2013-05-23  9:05 ` [PATCH 2/3] ARM: at91: use IRQCHIP_DECLARE instead of machine specific init_irq Boris BREZILLON
2013-05-23  9:05 ` [PATCH 3/3] ARM: at91: use set_handle_irq instead of machine specific handle_irq Boris BREZILLON
  -- strict thread matches above, loose matches on Subject: below --
2013-05-23  8:49 [PATCH 0/3] ARM: at91: move aic driver to drivers/irqchip Boris BREZILLON
2013-05-23  8:49 ` [PATCH 1/3] ARM: at91: move at91 " Boris BREZILLON

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130523115845.GA24476@game.jcrosoft.org \
    --to=plagnioj@jcrosoft.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).