Date: Wed, 16 Jan 2013 09:22:26 +0800 From: Ganbold Tsagaankhuu <ganbold@gmail.com> To: "Wojciech A. Koszek" <wkoszek@freebsd.org> Cc: svn-src-head@freebsd.org, Ganbold Tsagaankhuu <ganbold@freebsd.org>, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r245450 - in head/sys: arm/allwinner arm/conf boot/fdt/dts Message-ID: <CAGtf9xNCMwXa%2BeUfm396eFdDCbPQK-EPgocL21jDxwPqCSx54Q@mail.gmail.com> In-Reply-To: <20130115162116.GI20538@FreeBSD.org> References: <201301150826.r0F8QGJr044600@svn.freebsd.org> <20130115162116.GI20538@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 16, 2013 at 12:21 AM, Wojciech A. Koszek <wkoszek@freebsd.org> wrote: > On Tue, Jan 15, 2013 at 08:26:16AM +0000, Ganbold Tsagaankhuu wrote: >> Author: ganbold (doc committer) >> Date: Tue Jan 15 08:26:16 2013 >> New Revision: 245450 >> URL: http://svnweb.freebsd.org/changeset/base/245450 >> >> Log: >> Initial support for Allwinner A10 SoC (Cubieboard) >> Add simple console driver >> Add interrupt handling and timer codes >> Add kernel config file >> Add dts file >> Approved by: gonzo > > Ganbold, > > Thanks for this commit. Comments below. > >> >> Added: head/sys/arm/allwinner/a10_machdep.c >> ============================================================================== >> --- /dev/null 00:00:00 1970 (empty, because file is newly added) >> +++ head/sys/arm/allwinner/a10_machdep.c Tue Jan 15 08:26:16 2013 (r245450) >> @@ -0,0 +1,122 @@ >> +/*- >> + * Copyright (c) 2012 Ganbold Tsagaankhuu. <ganbold@gmail.com> > > Dot '.' after name isn't necessary. > > I'd really appreciate having PDF filename of the documentation from which > the code was derived in the comments, e.g.: > > "This file is derived from X.PDF, ver1.0, date 2012/Y/Z" > >> + * >> + * from: FreeBSD: //depot/projects/arm/src/sys/arm/ti/ti_machdep.c >> + */ >> + > > [..] > >> +int >> +platform_devmap_init(void) >> +{ >> + int i = 0; >> + >> + fdt_devmap[i].pd_va = 0xE1C00000; >> + fdt_devmap[i].pd_pa = 0x01C00000; >> + fdt_devmap[i].pd_size = 0x00400000; /* 4 MB */ > > Do you think you could comment on what these mean (or pages in the PDF where > can I find them) next to these variables? > >> >> Added: head/sys/arm/allwinner/console.c >> ============================================================================== >> --- /dev/null 00:00:00 1970 (empty, because file is newly added) >> +++ head/sys/arm/allwinner/console.c Tue Jan 15 08:26:16 2013 (r245450) >> @@ -0,0 +1,146 @@ > [..] >> +#ifndef A10_UART_BASE >> +#define A10_UART_BASE 0xe1c28000 /* UART0 */ >> +#endif >> + >> +int reg_shift = 2; > > Could you make it static and move it below defines? > >> +#define UART_DLL 0 /* Out: Divisor Latch Low */ >> +#define UART_DLM 1 /* Out: Divisor Latch High */ >> +#define UART_FCR 2 /* Out: FIFO Control Register */ >> +#define UART_LCR 3 /* Out: Line Control Register */ >> +#define UART_MCR 4 /* Out: Modem Control Register */ >> +#define UART_LSR 5 /* In: Line Status Register */ >> +#define UART_LSR_THRE 0x20 /* Transmit-hold-register empty */ >> +#define UART_LSR_DR 0x01 /* Receiver data ready */ >> +#define UART_MSR 6 /* In: Modem Status Register */ >> +#define UART_SCR 7 /* I/O: Scratch Register */ >> + >> + >> +/* >> + * uart related funcs >> + */ >> +static u_int32_t >> +uart_getreg(u_int32_t *bas) >> +{ >> + return *((volatile u_int32_t *)(bas)) & 0xff; >> +} >> + >> +static void >> +uart_setreg(u_int32_t *bas, u_int32_t val) >> +{ >> + *((volatile u_int32_t *)(bas)) = (u_int32_t)val; >> +} >> + >> +static int >> +ub_getc(void) >> +{ >> + while ((uart_getreg((u_int32_t *)(A10_UART_BASE + >> + (UART_LSR << reg_shift))) & UART_LSR_DR) == 0); >> + __asm __volatile("nop"); >> + >> + return (uart_getreg((u_int32_t *)A10_UART_BASE) & 0xff); >> +} >> + >> +static void >> +ub_putc(unsigned char c) >> +{ >> + if (c == '\n') >> + ub_putc('\r'); >> + >> + while ((uart_getreg((u_int32_t *)(A10_UART_BASE + >> + (UART_LSR << reg_shift))) & UART_LSR_THRE) == 0) >> + __asm __volatile("nop"); >> + >> + uart_setreg((u_int32_t *)A10_UART_BASE, c); >> +} > > Why aren't bus_* methods used here for accessing memory? This is just initial support of the SoC so eventually I hope it will be improved. sorry for the noise and thanks for the comments, Ganbold > >> +# >> +# >> +options PHYSADDR=0x40000000 >> + >> +makeoptions KERNPHYSADDR=0x40200000 > > Two tabs? > >> +options KERNPHYSADDR=0x40200000 >> +makeoptions KERNVIRTADDR=0xc0200000 >> +options KERNVIRTADDR=0xc0200000 >> + >> +options STARTUP_PAGETABLE_ADDR=0x48000000 >> + >> +files "../allwinner/files.a10" > > > >> *** DIFF OUTPUT TRUNCATED AT 1000 LINES *** > > -- > Wojciech A. Koszek > wkoszek@FreeBSD.czest.pl > http://FreeBSD.czest.pl/~wkoszek/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGtf9xNCMwXa%2BeUfm396eFdDCbPQK-EPgocL21jDxwPqCSx54Q>