[U-Boot] [PATCH 1/4] serial: Add Dragonfire serial driver

Michal Simek
2012-08-14T17:17:17Z

On 08/14/2012 06:45 PM, Joe Hershberger wrote:
> Hi Michal,
>
> On Tue, Aug 14, 2012 at 11:38 AM, Michal Simek <monstr@monstr.eu> wrote:
>> On 08/14/2012 04:09 PM, Joe Hershberger wrote:
>>>
>>> Hi Michal,
>>>
>>> On Tue, Aug 14, 2012 at 6:42 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>>
>>>> The driver is used on Xilinx Zynq platform.
>>>>
>>>> Signed-off-by: Michal Simek <monstr@monstr.eu>
>>>> ---
>>>> drivers/serial/Makefile | 1 +
>>>> drivers/serial/serial_xpssuart.c | 218
>>>> ++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 219 insertions(+), 0 deletions(-)
>>>> create mode 100644 drivers/serial/serial_xpssuart.c
>>>
>>>
>>> I think this would be clearer if it was named serial_zynq.c
>>>
>>>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>>>> index 65d0f23..81350d0 100644
>>>> --- a/drivers/serial/Makefile
>>>> +++ b/drivers/serial/Makefile
>>>> @@ -56,6 +56,7 @@ COBJS-$(CONFIG_S3C44B0_SERIAL) += serial_s3c44b0.o
>>>> COBJS-$(CONFIG_XILINX_UARTLITE) += serial_xuartlite.o
>>>> COBJS-$(CONFIG_SANDBOX_SERIAL) += sandbox.o
>>>> COBJS-$(CONFIG_SCIF_CONSOLE) += serial_sh.o
>>>> +COBJS-$(CONFIG_XPSS_SERIAL) += serial_xpssuart.o
>>>
>>>
>>> Replace every reference to "XPSS" with "ZYNQ".
>>>
>>>> ifndef CONFIG_SPL_BUILD
>>>> COBJS-$(CONFIG_USB_TTY) += usbtty.o
>>>> diff --git a/drivers/serial/serial_xpssuart.c
>>>> b/drivers/serial/serial_xpssuart.c
>>>> new file mode 100644
>>>> index 0000000..3c6d838
>>>> --- /dev/null
>>>> +++ b/drivers/serial/serial_xpssuart.c
>>>> @@ -0,0 +1,218 @@
>>>> +/*
>>>> + * U-Boot driver for Xilinx Dragonfire UART.
>>>
>>>
>>> Use the released name "Zynq" not the old codename "Dragonfire".
>>
>>
>> ok.
>>
>>
>>>
>>>> + *
>>>> + * Copyright (C) 2012 Michal Simek <monstr@monstr.eu>
>>>> + * Copyright (C) 2011-2012 Xilinx, Inc. All rights reserved.
>>>> + *
>>>> + * See file CREDITS for list of people who contributed to this
>>>> + * project.
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU General Public License as
>>>> + * published by the Free Software Foundation; either version 2 of
>>>> + * the License, or (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public License
>>>> + * along with this program; if not, write to the Free Software
>>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>>>> + * MA 02111-1307 USA
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <watchdog.h>
>>>> +#include <asm/io.h>
>>>> +#include <serial.h>
>>>> +
>>>> +#define XDFUART_SR_TXFULL 0x00000010 /* TX FIFO full */
>>>> +#define XDFUART_SR_RXEMPTY 0x00000002 /* RX FIFO empty */
>>>
>>>
>>> Replace all references to "XDFUART" with "ZYNQ_UART".
>>
>>
>> agree.
>>
>>
>>>
>>>> +
>>>> +#define XDFUART_CR_TX_EN 0x00000010 /* TX enabled */
>>>> +#define XDFUART_CR_RX_EN 0x00000004 /* RX enabled */
>>>> +#define XDFUART_CR_TXRST 0x00000002 /* TX logic reset */
>>>> +#define XDFUART_CR_RXRST 0x00000001 /* RX logic reset */
>>>> +
>>>> +#define XDFUART_MR_PARITY_NONE 0x00000020 /* No parity mode */
>>>> +
>>>> +/* Some clock/baud constants */
>>>> +#define XDFUART_BDIV 15 /* Default/reset BDIV value */
>>>> +#define XDFUART_BASECLK 3125000L /* master / (bdiv + 1) */
>>>> +
>>>> +struct xdfuart {
>>>> + u32 control; /* Control Register [8:0] */
>>>> + u32 mode; /* Mode Register [10:0] */
>>>> + u32 reserved1[4];
>>>> + u32 baud_rate_gen; /* Baud Rate Generator [15:0] */
>>>> + u32 reserved2[4];
>>>> + u32 channel_sts; /* Channel Status [11:0] */
>>>> + u32 tx_rx_fifo; /* FIFO [15:0] or [7:0] */
>>>> + u32 baud_rate_divider; /* Baud Rate Divider [7:0] */
>>>> +};
>>>> +
>>>> +static struct xdfuart *xdf_ports[4] = {
>>>> +#ifdef CONFIG_XPSS_SERIAL_BASEADDR0
>>>> + [0] = (struct xdfuart *)CONFIG_XPSS_SERIAL_BASEADDR0,
>>>> +#endif
>>>> +#ifdef CONFIG_XPSS_SERIAL_BASEADDR1
>>>> + [1] = (struct xdfuart *)CONFIG_XPSS_SERIAL_BASEADDR1,
>>>> +#endif
>>>
>>>
>>> There are 2 UARTS in hard silicon.
>>
>>
>> My fault.
>>
>>
>> They should be supported with
>>>
>>> their known base addresses (0xE0000000 and 0xE0001000) here without
>>> pushing that into the config header.
>>
>>
>> I am not sure that hardcoding addresses here is the right thing to do.
>> The main reason is that none has tested option that hard IPs can be
>> also used from programmable logic. It means that this driver
>> could be possible to use from Microblaze with address translation.
>>
>> No problem to setup these addresses in config file.
>
> I accept that you can access these from microblaze, but I think that
> will be the 1% use-case. You can make the base address overridable,
> but use the ARM core address space by default and have them here
> instead of copied to every config.

None has shown any table with use cases that why you can't say it will be 1% use-cases.

+ I don't think that hardcode IP address in generic driver is right way to go.
For example we are using SERIAL0 as console and in EDK you can setup which
IP it is. It means that sometimes SERIAL0 is at 0xe0001000 and sometimes at 0xe0000000.
This setting is also done for Linux that on serial0 is also Linux console which
simplify configuration.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian