Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dev][uart][pl011] Move to common driver #274

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions dev/pl011/include/dev/pl011.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#pragma once
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the standard license header here.


void pl011_uart_init(int irq, int nr, uintptr_t base);
void pl011_uart_init_early(int nr, uintptr_t base);
7 changes: 7 additions & 0 deletions dev/pl011/rules.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
LOCAL_DIR := $(GET_LOCAL_DIR)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please name this dev/uart/pl011 instead of at the top level of dev.


MODULE := $(LOCAL_DIR)

MODULE_SRCS += $(LOCAL_DIR)/uart.c

include make/module.mk
61 changes: 30 additions & 31 deletions platform/qemu-virt-arm/uart.c → dev/pl011/uart.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include <kernel/thread.h>
#include <platform/interrupts.h>
#include <platform/debug.h>
#include <platform/qemu-virt.h>
#include <target/debugconfig.h>
#include <assert.h>

/* PL011 implementation */
#define UART_DR (0x00)
Expand All @@ -37,24 +37,22 @@
#define NUM_UART 1

static cbuf_t uart_rx_buf[NUM_UART];
static uintptr_t uart_base[NUM_UART];

static inline uintptr_t uart_to_ptr(unsigned int n) {
switch (n) {
default:
case 0:
return UART_BASE;
}
return uart_base[n];
}

static enum handler_return uart_irq(void *arg) {
bool resched = false;
uint port = (uintptr_t)arg;
uint port = (uint)arg;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an issue with 64bit targets, since its down casting to a smaller size. Perhaps cast it through uintptr_t first, then down to uint.

uintptr_t base = uart_to_ptr(port);

/* read interrupt status and mask */
uint32_t isr = UARTREG(base, UART_TMIS);

if (isr & (1<<4)) { // rxmis
if (isr & ((1<<6) | (1<<4))) { // rtmis, rxmis
UARTREG(base, UART_ICR) = (1<<4);
cbuf_t *rxbuf = &uart_rx_buf[port];

/* while fifo is not empty, read chars out of it */
Expand Down Expand Up @@ -83,37 +81,38 @@ static enum handler_return uart_irq(void *arg) {
return resched ? INT_RESCHEDULE : INT_NO_RESCHEDULE;
}

void uart_init(void) {
for (size_t i = 0; i < NUM_UART; i++) {
uintptr_t base = uart_to_ptr(i);
void pl011_uart_init(int irq, int nr, uintptr_t base) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To generally be consistent with existing apis, put the number first.

assert(nr < NUM_UART);
uart_base[nr] = base;
// create circular buffer to hold received data
cbuf_initialize(&uart_rx_buf[nr], RXBUF_SIZE);

// create circular buffer to hold received data
cbuf_initialize(&uart_rx_buf[i], RXBUF_SIZE);
// assumes interrupts are contiguous
register_int_handler(irq, &uart_irq, (void *)nr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the (void *) is a warning on arm64. Perhaps as before, cast it through uintptr_t first.


// assumes interrupts are contiguous
register_int_handler(UART0_INT + i, &uart_irq, (void *)i);
// clear all irqs
UARTREG(base, UART_ICR) = 0x3ff;

// clear all irqs
UARTREG(base, UART_ICR) = 0x3ff;
UARTREG(base, UART_LCRH) = (3<<5) // 8bit mode
| (1<<4); // fifo enable

// set fifo trigger level
UARTREG(base, UART_IFLS) = 0; // 1/8 rxfifo, 1/8 txfifo
// set fifo trigger level
UARTREG(base, UART_IFLS) = 4 << 3; // 7/8 rxfifo, 1/8 txfifo

// enable rx interrupt
UARTREG(base, UART_IMSC) = (1<<4); // rxim
// enable rx interrupt
UARTREG(base, UART_IMSC) = (1<<6)|(1<<4); // rtim, rxim

// enable receive
UARTREG(base, UART_CR) |= (1<<9); // rxen
// enable receive
UARTREG(base, UART_CR) |= (1<<9)|(1<<8)|(1<<0); // rxen, tx_enable, uarten

// enable interrupt
unmask_interrupt(UART0_INT + i);
}
// enable interrupt
unmask_interrupt(irq);
}

void uart_init_early(void) {
for (size_t i = 0; i < NUM_UART; i++) {
UARTREG(uart_to_ptr(i), UART_CR) = (1<<8)|(1<<0); // tx_enable, uarten
}
void pl011_uart_init_early(int nr, uintptr_t base) {
assert(nr < NUM_UART);
uart_base[nr] = base;
UARTREG(uart_to_ptr(nr), UART_CR) = (1<<8)|(1<<0); // tx_enable, uarten
}

int uart_putc(int port, char c) {
Expand All @@ -132,7 +131,7 @@ int uart_getc(int port, bool wait) {

char c;
if (cbuf_read_char(rxbuf, &c, wait) == 1) {
UARTREG(uart_to_ptr(port), UART_IMSC) = (1<<4); // rxim
UARTREG(uart_to_ptr(port), UART_IMSC) |= (1<<4); // rxim
return c;
}

Expand Down
17 changes: 17 additions & 0 deletions platform/bcm28xx/include/platform/bcm28xx.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,15 @@
#define PCM_CLOCK_BASE (BCM_PERIPH_BASE_VIRT + 0x101098)
#define RNG_BASE (BCM_PERIPH_BASE_VIRT + 0x104000)
#define GPIO_BASE (BCM_PERIPH_BASE_VIRT + 0x200000)

// all PL011 uarts, 2-5 only exist on BCM2828/pi4/VC6
// all share the same INTERRUPT_VC_UART
#define UART0_BASE (BCM_PERIPH_BASE_VIRT + 0x201000)
#define UART2_BASE (BCM_PERIPH_BASE_VIRT + 0x201400)
#define UART3_BASE (BCM_PERIPH_BASE_VIRT + 0x201600)
#define UART4_BASE (BCM_PERIPH_BASE_VIRT + 0x201800)
#define UART5_BASE (BCM_PERIPH_BASE_VIRT + 0x201a00)

#define MMCI0_BASE (BCM_PERIPH_BASE_VIRT + 0x202000)
#define I2S_BASE (BCM_PERIPH_BASE_VIRT + 0x203000)
#define SPI0_BASE (BCM_PERIPH_BASE_VIRT + 0x204000)
Expand Down Expand Up @@ -175,6 +183,15 @@

/* GPIO */

#define GPIO_IN 0
#define GPIO_OUT 1
#define GPIO_ALT5 2
#define GPIO_ALT4 3
#define GPIO_ALT0 4
#define GPIO_ALT1 5
#define GPIO_ALT2 6
#define GPIO_ALT3 7

#define GPIO_GPFSEL0 (GPIO_BASE + 0x00)
#define GPIO_GPFSEL1 (GPIO_BASE + 0x04)
#define GPIO_GPFSEL2 (GPIO_BASE + 0x08)
Expand Down
12 changes: 10 additions & 2 deletions platform/bcm28xx/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <lk/trace.h>

#include <dev/uart.h>
#include <dev/pl011.h>
#include <arch.h>
#include <lk/init.h>
#include <kernel/vm.h>
Expand Down Expand Up @@ -108,7 +109,7 @@ void platform_init_mmu_mappings(void) {
}

void platform_early_init(void) {
uart_init_early();
pl011_uart_init_early(0, UART0_BASE);

intc_init();

Expand Down Expand Up @@ -195,7 +196,14 @@ void platform_early_init(void) {
}

void platform_init(void) {
uart_init();
pl011_uart_init(INTERRUPT_VC_UART, 0, UART0_BASE);
#if BCM2838
pl011_uart_init(INTERRUPT_VC_UART, 2, UART2_BASE);
pl011_uart_init(INTERRUPT_VC_UART, 3, UART3_BASE);
pl011_uart_init(INTERRUPT_VC_UART, 4, UART4_BASE);
pl011_uart_init(INTERRUPT_VC_UART, 5, UART5_BASE);
#endif

#if BCM2837
init_framebuffer();
#endif
Expand Down
4 changes: 2 additions & 2 deletions platform/bcm28xx/rules.mk
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ SMP_CPU_ID_BITS := 8
GLOBAL_DEFINES += \
BCM2836=1

MODULE_SRCS += \
$(LOCAL_DIR)/uart.c
MODULES += \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use MODULE_DEPS instead of MODULES. MODULES sometimes works but the _DEPS is the correct solution.

dev/pl011

else ifeq ($(TARGET),rpi3)
ARCH := arm64
Expand Down
135 changes: 0 additions & 135 deletions platform/bcm28xx/uart.c

This file was deleted.

5 changes: 3 additions & 2 deletions platform/qemu-virt-arm/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <dev/interrupt/arm_gic.h>
#include <dev/timer/arm_generic.h>
#include <dev/uart.h>
#include <dev/pl011.h>
#include <dev/virtio.h>
#include <dev/virtio/net.h>
#include <lib/fdtwalk.h>
Expand Down Expand Up @@ -104,7 +105,7 @@ void platform_early_init(void) {

arm_generic_timer_init(ARM_GENERIC_TIMER_PHYSICAL_INT, 0);

uart_init_early();
pl011_uart_init_early(0, UART_BASE);

int cpu_count = 0;
bool found_mem = false;
Expand Down Expand Up @@ -152,7 +153,7 @@ void platform_early_init(void) {
}

void platform_init(void) {
uart_init();
pl011_uart_init(UART0_INT, 0, UART_BASE);

/* detect any virtio devices */
uint virtio_irqs[NUM_VIRTIO_TRANSPORTS];
Expand Down
2 changes: 1 addition & 1 deletion platform/qemu-virt-arm/rules.mk
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ MODULE_SRCS += \
$(LOCAL_DIR)/debug.c \
$(LOCAL_DIR)/platform.c \
$(LOCAL_DIR)/secondary_boot.S \
$(LOCAL_DIR)/uart.c

MEMBASE := 0x40000000
MEMSIZE ?= 0x08000000 # 512MB
Expand All @@ -33,6 +32,7 @@ MODULE_DEPS += \
dev/virtio/block \
dev/virtio/gpu \
dev/virtio/net \
dev/pl011 \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent, sort the list of the devs.


GLOBAL_DEFINES += \
MEMBASE=$(MEMBASE) \
Expand Down
Empty file.
Empty file.