linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: marek.vasut@gmail.com (Marek Vasut)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/4] pxa/spitz: Rework spitz
Date: Wed, 30 Jun 2010 16:09:04 +0200	[thread overview]
Message-ID: <201006301609.04907.marek.vasut@gmail.com> (raw)
In-Reply-To: <AANLkTinRkgIg1q8LfRF6jWOoxLNY__xMi3iJjEv1FhWL@mail.gmail.com>

Dne St 30. ?ervna 2010 08:57:57 Eric Miao napsal(a):
> On Mon, Jun 28, 2010 at 11:29 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dne Po 28. ?ervna 2010 14:19:46 Eric Miao napsal(a):
> >> On Mon, Jun 28, 2010 at 8:07 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> > Dne Po 28. ?ervna 2010 05:43:12 Eric Miao napsal(a):
> >> >> On Thu, Jun 24, 2010 at 10:02 PM, Marek Vasut <marek.vasut@gmail.com>
> > 
> > wrote:
> >> >> > This huge patch mostly shuffles code. The spitz.c file contained
> >> >> > terrible mess and needed a cleanup, here it is:
> >> >> > 
> >> >> > 1) Made every part modular, components are not built in if not
> >> >> > selected. 2) Removed loads of preprocessor goo, mostly "#ifdef
> >> >> > MACH_AKITA .... #endif" and similar code. The kernel size will grow
> >> >> > by a few kb now, but the file is much more readable.
> >> >> > 3) Reworked SD/CF power setting function and made it reentrant.
> >> >> > 4) Add ISL6271A regulator support
> >> >> > 5) Correctly register WM8750
> >> >> > 
> >> >> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >> >> 
> >> >> I'd prefer to see separate small patches for this?
> >> > 
> >> > This is nearly a replacement for the whole file ... maybe we could
> >> > merge spitz2.c or something ... or do it this way. I understand your
> >> > point about spliting it into smaller chunks, but I don't see a reason
> >> > in this case.
> >> 
> >> I guess the above 1, 2, 3, 4, 5 can be well separated into 5 patches,
> >> I don't mind merge many patches, but I'd really like to see small and
> >> consistent changes (make it regression test friendly BTW), ;-)
> > 
> > I said I get your point, but I don't see the benefit here.
> > 1) already does most of the rewrite of the file,
> > 2) only removes the #ifdef MACH_PLAT around MACHINE_START (as the rest
> > was done in 1) ),
> > 3) is actually a part of 1) as well
> > 4) ok, this could be separated out, but I see no benefit
> > 5) well this is already in if I recall well, but I put it here for
> > completeness
> 
> Several of other comments, though:
> 
> 1. It doesn't look necessary to me to change spitz_* to sharpslc_*. spitz_*
>    isn't a good prefix, neither is sharpslc_* (think about the corgi
> series, which is normally called Sharp SL-C7xx). And this is also going to
> save many unnecessary changes.

I know, but what prefix do you sugest then?
> 
> 2. Direct references of spitzscoop[12]_device are still there, which I'd
>    prefer a cleaner patch for this

I'd prefer rewriting the whole scoop driver and patching the pcmcia driver so 
all this crap can get lot.
> 
> 3. Others seem to be coding style changes - which can be separated

You're right.

  reply	other threads:[~2010-06-30 14:09 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-24 14:02 [PATCH 1/4] pxa/z2: Add regulator support Marek Vasut
2010-06-24 14:02 ` [PATCH 2/4] pxa/colibri320: Add UDC support Marek Vasut
2010-06-28  3:42   ` Eric Miao
2010-06-24 14:02 ` [PATCH 3/4] ARM/scoop: Add CPR register bit definitions Marek Vasut
2010-06-28  3:42   ` Eric Miao
2010-06-24 14:02 ` [PATCH 4/4] pxa/spitz: Rework spitz Marek Vasut
2010-06-28  3:43   ` Eric Miao
2010-06-28 12:07     ` Marek Vasut
2010-06-28 12:19       ` Eric Miao
2010-06-28 15:29         ` Marek Vasut
2010-06-30  6:57           ` Eric Miao
2010-06-30 14:09             ` Marek Vasut [this message]
2010-06-30 14:14               ` Eric Miao
2010-06-30 14:34                 ` Marek Vasut
2010-06-28  3:42 ` [PATCH 1/4] pxa/z2: Add regulator support Eric Miao

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=201006301609.04907.marek.vasut@gmail.com \
    --to=marek.vasut@gmail.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).