linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 7/8] powerpc/5200: Refactor mpc5200 interrupt controller driver
Date: Mon, 26 Jan 2009 07:26:36 +0100	[thread overview]
Message-ID: <20090126062636.D3375832E416@gemini.denx.de> (raw)
In-Reply-To: <fa686aa40901251622j542b6029u83c510f5b4e2f92@mail.gmail.com>

Dear Grant Likely,

In message <fa686aa40901251622j542b6029u83c510f5b4e2f92@mail.gmail.com> you wrote:
>
> >> -     case IRQF_TRIGGER_HIGH:
> >> -             type = 0;
> >> -             break;
> >> -     case IRQF_TRIGGER_RISING:
> >> -             type = 1;
> >> -             break;
> >> -     case IRQF_TRIGGER_FALLING:
> >> -             type = 2;
> >> -             break;
> >> -     case IRQF_TRIGGER_LOW:
> >> -             type = 3;
> >> -             break;
> >> +     case IRQF_TRIGGER_HIGH: type = 0; break;
> >> +     case IRQF_TRIGGER_RISING: type = 1; handler = handle_edge_irq; break;
> >> +     case IRQF_TRIGGER_FALLING: type = 2; handler = handle_edge_irq; break;
> >> +     case IRQF_TRIGGER_LOW: type = 3; break;
> >
> > The Linux coding style tells us not to do that:
...
> In principle I agree and follow that rule most of the time, but I have
> good reason for not choosing to do so here.
> 
> The whole point of coding style is to promote
> readability/manageability.  ie. the 80 column rule is a very strong
> guideline, but there are places where breaking that rule makes for
> more readable code than breaking things up and it is left to the
> discretion of the coder and the maintainer.

It's not so much the line length, IMO.

It's "Don't put multiple statements on a single line", plus
readability.

I think the new version is more difficult to read. It's plain ugly,
and inconsistent.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
It is common sense to take a method and try it. If it fails, admit it
frankly and try another. But above all, try something.
                                              - Franklin D. Roosevelt

  reply	other threads:[~2009-01-26  6:26 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-21 20:55 [PATCH 1/8] powerpc/5200: update device tree binding documentation Grant Likely
2009-01-21 20:55 ` [PATCH 2/8] powerpc/5200: Stop using device_type and port-number properties Grant Likely
2009-01-21 21:13   ` Juergen Beisert
2009-01-22  4:46     ` Grant Likely
2009-01-29 21:15   ` Wolfram Sang
2009-01-21 20:55 ` [PATCH 3/8] powerpc/5200: Trim cruft from device trees Grant Likely
2009-01-29 21:21   ` Wolfram Sang
2009-01-21 20:55 ` [PATCH 4/8] powerpc/5200: Add support for the Media5200 board from Freescale Grant Likely
2009-01-21 20:55 ` [PATCH 5/8] powerpc/5200: Don't specify IRQF_SHARED in PSC UART driver Grant Likely
2009-01-29 21:24   ` Wolfram Sang
2009-01-21 20:55 ` [PATCH 6/8] powerpc/5200: Remove pr_debug() from hot paths in irq driver Grant Likely
2009-01-29 21:29   ` Wolfram Sang
2009-01-21 20:55 ` [PATCH 7/8] powerpc/5200: Refactor mpc5200 interrupt controller driver Grant Likely
2009-01-25 20:06   ` Wolfgang Grandegger
2009-01-26  0:22     ` Grant Likely
2009-01-26  6:26       ` Wolfgang Denk [this message]
2009-01-26  8:31       ` Wolfgang Grandegger
2009-01-26 17:58         ` Matt Sealey
2009-01-26  9:22       ` Wolfram Sang
2009-01-27 17:52   ` Matt Sealey
2009-01-27 17:54     ` Grant Likely
2009-01-29 21:33   ` Wolfram Sang
2009-01-21 20:55 ` [PATCH 8/8] powerpc/5200: Rework GPT driver to also be an IRQ controller Grant Likely
2009-01-27 12:23   ` Wolfram Sang
2009-01-25 19:48 ` [PATCH 1/8] powerpc/5200: update device tree binding documentation Wolfram Sang
2009-01-26  1:46   ` Grant Likely
2009-01-26  9:26     ` Wolfram Sang
2009-01-27 16:25       ` Grant Likely
2009-01-27 18:00 ` Matt Sealey
2009-01-27 18:06   ` Grant Likely
2009-01-29 21:09 ` Wolfram Sang

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=20090126062636.D3375832E416@gemini.denx.de \
    --to=wd@denx.de \
    --cc=grant.likely@secretlab.ca \
    --cc=linuxppc-dev@ozlabs.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).