From: sandeen@sandeen.net (Eric Sandeen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: Iomega ix4-200d support
Date: Fri, 13 Nov 2009 09:42:07 -0600 [thread overview]
Message-ID: <4AFD7E4F.5040909@sandeen.net> (raw)
In-Reply-To: <20091113085107.GJ13296@deprecation.cyrius.com>
Martin Michlmayr wrote:
> * Eric Sandeen <sandeen@redhat.com> [2009-11-13 00:04]:
>> +config MACH_IX4_88F6281_NAS
>> + bool "Iomega ix4-200d NAS"
>
> If you look at Kconfig, you'll see that putting the CPU id in the MACH
> name is quite unusual for NAS devices (it's only used for dev boards).
> I'd make this IX4200D or NASIX400D.
Ok
> Also, the Kconfig entry should be at the end and not in the middle of
> some reference boards.
ok, not sure how it ended up there ;)
>> --- /dev/null
>> +++ b/arch/arm/mach-kirkwood/ix4-88f6281-setup.c
>
> nasix4200d.c or ix4200d.c
>
>> +/*
>> + * Marvell RD-88F6281 Reference Board Setup
>
> That's obviously copy&paste...
>
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>
> You should add your Copyright. And probably the Copyright of the file
> on which yours is baed.
I'll have to find it :)
>> +
>> +/* #ifdef IX4_BOARD MV_BOARD_INFO db88f6281AInfo db88f6281AInfoBoardMacInfo */
>
> What's that?
sorry, leftovers ...
>> +MACHINE_START(NASIX4200D, "Iomega NAS ix4-200d")
>> + /* Maintainer: Saeed Bishara <saeed@marvell.com> */
>
> That should be you.
:) aye.
Argh, sorry, should have read this better before I sent it, it was a
quick hack - will clean this all up & resend.
>> --- a/arch/arm/tools/mach-types
>> +++ b/arch/arm/tools/mach-types
>> @@ -2132,6 +2132,7 @@ apollo MACH_APOLLO APOLLO 2141
>> at91cap9stk MACH_AT91CAP9STK AT91CAP9STK 2142
>> spc300 MACH_SPC300 SPC300 2143
>> eko MACH_EKO EKO 2144
>> +nasix4200d MACH_IX4_88F6281_NAS NASIX4200D 2145
>
> That's a big no. First of all, you have to register your own machine
> ID at http://www.arm.linux.org.uk/developer/machines/ Second, when
> you submit a patch, don't include mach-types since this is
> auto-generated.
OK
> Finally, a question: it seems that the buttons and LEDs are not
> supported by your patch. Do you know how they are hooked up?
I can try to find out.
> Thanks for working on this
Sure, sorry it's such a mess on the first pass, thanks for the review. :)
-Eric
prev parent reply other threads:[~2009-11-13 15:42 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-13 6:04 [PATCH] arm: Iomega ix4-200d support Eric Sandeen
2009-11-13 8:51 ` Martin Michlmayr
2009-11-13 15:42 ` Eric Sandeen [this message]
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=4AFD7E4F.5040909@sandeen.net \
--to=sandeen@sandeen.net \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.