[PATCH] ARM: mxs: Add initial support for Bluegiga APX4 Development Kit
Uwe Kleine-König
u.kleine-koenig at pengutronix.de
Tue Oct 18 03:39:54 EDT 2011
On Tue, Oct 18, 2011 at 10:27:04AM +0300, Lauri Hintsala wrote:
> 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.
Yeah, so you have two symbols that are =y for your board. One of them
doesn't do anything apart from being there and having a value. Just
removing config MODULE_APX4, putting all the selects into
MACH_APX4DEVKIT and remove the select MODULE_APX4 from it doesn't break
anything.
MODULE_TX28 at least has a separate file that is compiled for it and
that might be useful for maschines that don't necessarily use
mach-tx28.c .
> >> config MACH_TX28
> >> bool "Ka-Ro TX28 module"
> >> select MODULE_TX28
> >>
> >>+config MACH_APX4DEVKIT
> >>+ bool "Support Bluegiga APX4 Development Kit"
> >>+ select MODULE_APX4
> >>+
> >> endif
> >>+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?
sscanf("%pM", ...) would be straight forward, but I'd be surprised if
that worked. I'd ask on netdev at vger.kernel.org.
> 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.
Yeah, so a generic solution would be great!
> >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.
fec.macaddr should work already today.
> >>+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?
yes.
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
More information about the linux-arm-kernel
mailing list