From: lauri.hintsala@bluegiga•com (Lauri Hintsala)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] ARM: mxs: Add initial support for Bluegiga APX4 Development Kit
Date: Tue, 18 Oct 2011 10:27:04 +0300 [thread overview]
Message-ID: <4E9D2A48.9060605@bluegiga.com> (raw)
In-Reply-To: <20111017094434.GA21504@pengutronix.de>
Hi Uwe,
On 10/17/2011 12:44 PM, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Mon, Oct 17, 2011 at 11:08:36AM +0300, Lauri Hintsala wrote:
>> Added initial support for Bluegiga APX4 module and Development Kit.
>> Patches are based on Linux v3.1-rc9.
> Specifying the base isn't helpful in the commit log. You can better
> write things like these after the tripple dash below.
I'll drop the version information from commit log.
>> +config MODULE_APX4
>> + bool
>> + select SOC_IMX28
>> + select LEDS_GPIO_REGISTER
>> + select MXS_HAVE_AMBA_DUART
>> + select MXS_HAVE_PLATFORM_AUART
>> + select MXS_HAVE_PLATFORM_FEC
>> + select MXS_HAVE_PLATFORM_MXS_I2C
> broken indention
Thanks for catching it.
>> + select MXS_HAVE_PLATFORM_MXS_MMC
>> + select MXS_OCOTP
>> +
> MODULE_APX4 is unused, do you plan to use it later?
It is already used. MODULE_APX4 is selected by MACH_APX4DEVKIT.
>> config MACH_TX28
>> bool "Ka-Ro TX28 module"
>> select MODULE_TX28
>>
>> +config MACH_APX4DEVKIT
>> + bool "Support Bluegiga APX4 Development Kit"
>> + select MODULE_APX4
>> +
>> endif
>> diff --git a/arch/arm/mach-mxs/Makefile b/arch/arm/mach-mxs/Makefile
>> index 6c38262..1838466 100644
>> --- a/arch/arm/mach-mxs/Makefile
>> +++ b/arch/arm/mach-mxs/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_SOC_IMX28) += clock-mx28.o mm-mx28.o
>> obj-$(CONFIG_MACH_STMP378X_DEVB) += mach-stmp378x_devb.o
>> obj-$(CONFIG_MACH_MX23EVK) += mach-mx23evk.o
>> obj-$(CONFIG_MACH_MX28EVK) += mach-mx28evk.o
>> +obj-$(CONFIG_MACH_APX4DEVKIT) += mach-apx4devkit.o
>> obj-$(CONFIG_MODULE_TX28) += module-tx28.o
>> obj-$(CONFIG_MACH_TX28) += mach-tx28.o
>>
>> diff --git a/arch/arm/mach-mxs/include/mach/mxs.h b/arch/arm/mach-mxs/include/mach/mxs.h
>> index 35a89dd..209698c 100644
>> --- a/arch/arm/mach-mxs/include/mach/mxs.h
>> +++ b/arch/arm/mach-mxs/include/mach/mxs.h
>> @@ -33,6 +33,7 @@
>> 0)
>> #define cpu_is_mx28() ( \
>> machine_is_mx28evk() || \
>> + machine_is_apx4devkit() || \
>> machine_is_tx28() || \
> please keep this list sorted alphabetically.
Thanks. I'll fix it.
>> +static struct fec_platform_data mx28_fec_pdata __initdata = {
>> + .phy = PHY_INTERFACE_MODE_RMII,
>> +};
>> +
>> +static int __init apx4devkit_fec_get_mac(char *macstr)
>> +{
>> + int i, h, l;
>> +
>> + macstr++;
>> +
>> + for (i = 0; i< 6; i++) {
>> + if (i != 5&& *(macstr + 2) != ':')
>> + goto error;
>> +
>> + h = hex_to_bin(*macstr++);
>> + if (h == -1)
>> + goto error;
>> +
>> + l = hex_to_bin(*macstr++);
>> + if (l == -1)
>> + goto error;
>> +
>> + macstr++;
>> + mx28_fec_pdata.mac[i] = (h<< 4) + l;
>> + }
>> + return 0;
> I wonder if there isn't a more generic way to parse a mac address.
Maybe it is better to use sscanf for parsing mac address, is it. I
didn't found any generic function for this purpose. Is there any?
Other boards seems to use similar methods for parsing mac address:
arch/arm/mach-orion5x/dns323-setup.c: dns323_read_mac_addr
arch/arm/mach-orion5x/tsx09-common.c: qnap_tsx09_check_mac_addr
arch/mips/rb532/devices.c: parse_mac_addr
I dont't mean it is right/best way to do parsing but it seems to be
generic problem.
> Other machines put the mac into the otp.
We do not want to use otps for mac.
>> +error:
>> + pr_err("%s: invalid mac address\n", __func__);
>> + return -EINVAL;
>> +}
>> +
>> +__setup("ethaddr", apx4devkit_fec_get_mac);
> the name is IMHO too generic for a board specific parameter. Think about
> a kernel that runs on all mxs based machines. ethaddr=... only affects
> your machine type. And it's not possible to add the same parameter to a
> different board.
I could rename the parameter name.
>> +static void __init apx4devkit_timer_init(void)
>> +{
>> + mx28_clocks_init();
>> +}
>> +
>> +static struct sys_timer apx4devkit_timer = {
>> + .init = apx4devkit_timer_init,
>> +};
>> +
>> +MACHINE_START(APX4DEVKIT, "Bluegiga APX4 Development Kit")
>> + .map_io = mx28_map_io,
>> + .init_irq = mx28_init_irq,
>> + .init_machine = apx4devkit_init,
>> + .timer =&apx4devkit_timer,
>> +MACHINE_END
> please read http://article.gmane.org/gmane.linux.ports.arm.omap/50721
Do you mean init_machine and timer are in wrong order?
Regards,
Lauri Hintsala
next prev parent reply other threads:[~2011-10-18 7:27 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-17 8:08 [PATCH] ARM: mxs: Add initial support for Bluegiga APX4 Development Kit Lauri Hintsala
2011-10-17 9:44 ` Uwe Kleine-König
2011-10-17 10:39 ` Baruch Siach
2011-10-18 7:45 ` Lauri Hintsala
2011-10-18 7:58 ` Baruch Siach
2011-10-18 9:34 ` Lauri Hintsala
2011-10-18 12:54 ` Baruch Siach
2011-10-18 7:27 ` Lauri Hintsala [this message]
2011-10-18 7:39 ` Uwe Kleine-König
2011-10-18 8:01 ` Lauri Hintsala
2011-10-18 8:12 ` Uwe Kleine-König
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=4E9D2A48.9060605@bluegiga.com \
--to=lauri.hintsala@bluegiga$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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