svn commit: r245450 - in head/sys: arm/allwinner arm/conf boot/fdt/dts

[Ian Lepore]

On Wed, 2013-01-16 at 09:22 +0800, Ganbold Tsagaankhuu wrote:
> On Wed, Jan 16, 2013 at 12:21 AM, Wojciech A. Koszek
> <wkoszek at 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 at 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
> 

Actually these are console uart routines, which have to work before the
bus_space system is available, hence the direct hardware access.

-- Ian


> >
> >> +#
> >> +#
> >> +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 at FreeBSD.czest.pl
> > http://FreeBSD.czest.pl/~wkoszek/