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

John Williams
2012-08-14T22:40:41Z

On Wed, Aug 15, 2012 at 3:17 AM, Michal Simek <monstr@monstr.eu> wrote:

> 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.

I agree - coding base addresses in drivers, rather than machines/platforms,
sounds like a bad idea.

There is a somewhat open question in both Linux and u-boot about what to do
if the MIOs enable PS7_UART1 but not PS7_UART0. Should UART1 enumerate as
u-boot's first serial device? In Linux as /dev/ttyS0?

Let's keep addresses and decisions like this in platform files, not device
drivers.

John
--
John Williams, PhD, B. Eng, B. IT
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663 f: +61-7-30090663