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

[Ganbold Tsagaankhuu]

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

>
>> +#
>> +#
>> +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/