| Message ID | 20181002142443.30976-9-damien.hedde@greensocs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clock framework API. | expand |
Hi Damien, On 10/2/18 4:24 PM, Damien Hedde wrote: > Add bus interface and uart reference clock inputs. > > Note: it is hard to find out from the doc what is the behavior when only one > of the clock is disabled. > > The implemented behaviour is that register access needs both clock being active. > > The bus interface control the mmios visibility This sentence sounds odd. > > The reference clock controls the baudrate generation. If it disabled, > any input characters and events are ignored. Also register accesses are > conditioned to the clock being enabled (but is it really the case in > reality ?) and a guest error is triggerred if that is not the case. > > If theses clocks remains unconnected, the uart behaves as before > (default to 50MHz ref clock). > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > include/hw/char/cadence_uart.h | 3 ++ > hw/char/cadence_uart.c | 92 ++++++++++++++++++++++++++++++++-- > hw/char/trace-events | 3 ++ > 3 files changed, 93 insertions(+), 5 deletions(-) > > diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h > index 118e3f10de..fd1d4725f4 100644 > --- a/include/hw/char/cadence_uart.h > +++ b/include/hw/char/cadence_uart.h > @@ -21,6 +21,7 @@ > #include "hw/sysbus.h" > #include "chardev/char-fe.h" > #include "qemu/timer.h" > +#include "hw/clock-port.h" > > #define CADENCE_UART_RX_FIFO_SIZE 16 > #define CADENCE_UART_TX_FIFO_SIZE 16 > @@ -47,6 +48,8 @@ typedef struct { > CharBackend chr; > qemu_irq irq; > QEMUTimer *fifo_trigger_handle; > + ClockIn *refclk; > + ClockIn *busclk; > } CadenceUARTState; > > static inline DeviceState *cadence_uart_create(hwaddr addr, > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c > index fbdbd463bb..feb5cee4d7 100644 > --- a/hw/char/cadence_uart.c > +++ b/hw/char/cadence_uart.c > @@ -28,6 +28,7 @@ > #include "qemu/timer.h" > #include "qemu/log.h" > #include "hw/char/cadence_uart.h" > +#include "trace.h" > > #ifdef CADENCE_UART_ERR_DEBUG > #define DB_PRINT(...) do { \ > @@ -94,7 +95,7 @@ > #define LOCAL_LOOPBACK (0x2 << UART_MR_CHMODE_SH) > #define REMOTE_LOOPBACK (0x3 << UART_MR_CHMODE_SH) > > -#define UART_INPUT_CLK 50000000 > +#define UART_DEFAULT_REF_CLK (50 * 1000 * 1000) > > #define R_CR (0x00/4) > #define R_MR (0x04/4) > @@ -165,15 +166,30 @@ static void uart_send_breaks(CadenceUARTState *s) > &break_enabled); > } > > +static unsigned int uart_input_clk(CadenceUARTState *s) > +{ > + return clock_get_frequency(s->refclk); > +} > + > static void uart_parameters_setup(CadenceUARTState *s) > { > QEMUSerialSetParams ssp; > unsigned int baud_rate, packet_size; > > baud_rate = (s->r[R_MR] & UART_MR_CLKS) ? > - UART_INPUT_CLK / 8 : UART_INPUT_CLK; > + uart_input_clk(s) / 8 : uart_input_clk(s); > + baud_rate /= (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); Safe because s->r[R_BRGR] >= 1, OK. > + trace_cadence_uart_baudrate(baud_rate); > + > + ssp.speed = baud_rate; > + if (ssp.speed == 0) { > + /* > + * Avoid division-by-zero below. > + * TODO: find something better > + */ > + ssp.speed = 1; > + } > > - ssp.speed = baud_rate / (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); > packet_size = 1; > > switch (s->r[R_MR] & UART_MR_PAR) { > @@ -337,6 +353,11 @@ static void uart_receive(void *opaque, const uint8_t *buf, int size) > CadenceUARTState *s = opaque; > uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE; > > + /* ignore characters when unclocked */ > + if (!clock_is_enabled(s->refclk)) { > + return; > + } > + > if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) { > uart_write_rx_fifo(opaque, buf, size); > } > @@ -350,6 +371,11 @@ static void uart_event(void *opaque, int event) > CadenceUARTState *s = opaque; > uint8_t buf = '\0'; > > + /* ignore events when unclocked */ > + if (!clock_is_enabled(s->refclk)) { > + return; > + } > + > if (event == CHR_EVENT_BREAK) { > uart_write_rx_fifo(opaque, &buf, 1); > } > @@ -382,6 +408,14 @@ static void uart_write(void *opaque, hwaddr offset, > { > CadenceUARTState *s = opaque; > > + /* ignore accesses when bus or ref clock is disabled */ > + if (!(clock_is_enabled(s->busclk) && clock_is_enabled(s->refclk))) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "cadence_uart: Trying to write register 0x%x" > + " while clock is disabled\n", (unsigned) offset); > + return; > + } > + > DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value); > offset >>= 2; > if (offset >= CADENCE_UART_R_MAX) { > @@ -440,6 +474,14 @@ static uint64_t uart_read(void *opaque, hwaddr offset, > CadenceUARTState *s = opaque; > uint32_t c = 0; > > + /* ignore accesses when bus or ref clock is disabled */ > + if (!(clock_is_enabled(s->busclk) && clock_is_enabled(s->refclk))) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "cadence_uart: Trying to read register 0x%x" > + " while clock is disabled\n", (unsigned) offset); > + return 0; > + } > + > offset >>= 2; > if (offset >= CADENCE_UART_R_MAX) { > c = 0; > @@ -488,6 +530,14 @@ static void cadence_uart_realize(DeviceState *dev, Error **errp) > uart_event, NULL, s, NULL, true); > } > > +static void cadence_uart_refclk_update(void *opaque) > +{ > + CadenceUARTState *s = opaque; > + > + /* recompute uart's speed on clock change */ > + uart_parameters_setup(s); > +} > + > static void cadence_uart_init(Object *obj) > { > SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > @@ -497,9 +547,26 @@ static void cadence_uart_init(Object *obj) > sysbus_init_mmio(sbd, &s->iomem); > sysbus_init_irq(sbd, &s->irq); > > + s->refclk = qdev_init_clock_in(DEVICE(obj), "refclk", > + cadence_uart_refclk_update, s); > + /* initialize the frequency in case the clock remains unconnected */ > + clock_init_frequency(s->refclk, UART_DEFAULT_REF_CLK); > + s->busclk = qdev_init_clock_in(DEVICE(obj), "busclk", NULL, NULL); > + /* initialize the frequency to non-zero in case it remains unconnected */ > + clock_init_frequency(s->busclk, 100 * 1000 * 1000); #define INPUT_BUS_CLK_FREQUENCY (100 * 1000 * 1000) > + > s->char_tx_time = (NANOSECONDS_PER_SECOND / 9600) * 10; > } > > +static int cadence_uart_pre_load(void *opaque) > +{ > + CadenceUARTState *s = opaque; > + > + clock_init_frequency(s->refclk, UART_DEFAULT_REF_CLK); > + clock_init_frequency(s->busclk, 100 * 1000 * 1000); INPUT_BUS_CLK_FREQUENCY > + return 0; > +} > + > static int cadence_uart_post_load(void *opaque, int version_id) > { > CadenceUARTState *s = opaque; > @@ -516,10 +583,22 @@ static int cadence_uart_post_load(void *opaque, int version_id) > return 0; > } > > +static const VMStateDescription vmstate_cadence_uart_clocks = { > + .name = "cadence_uart_clocks", > + .version_id = 0, > + .minimum_version_id = 0, > + .fields = (VMStateField[]) { > + VMSTATE_CLOCKIN(refclk, CadenceUARTState), > + VMSTATE_CLOCKIN(busclk, CadenceUARTState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_cadence_uart = { > .name = "cadence_uart", > .version_id = 2, > .minimum_version_id = 2, > + .pre_load = cadence_uart_pre_load, > .post_load = cadence_uart_post_load, > .fields = (VMStateField[]) { > VMSTATE_UINT32_ARRAY(r, CadenceUARTState, CADENCE_UART_R_MAX), > @@ -532,7 +611,10 @@ static const VMStateDescription vmstate_cadence_uart = { > VMSTATE_UINT32(rx_wpos, CadenceUARTState), > VMSTATE_TIMER_PTR(fifo_trigger_handle, CadenceUARTState), > VMSTATE_END_OF_LIST() > - } > + }, > + .subsections = (const VMStateDescription * []) { > + &vmstate_cadence_uart_clocks, > + }, > }; > > static Property cadence_uart_properties[] = { > @@ -548,7 +630,7 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data) > dc->vmsd = &vmstate_cadence_uart; > dc->reset = cadence_uart_reset; > dc->props = cadence_uart_properties; > - } > +} > > static const TypeInfo cadence_uart_info = { > .name = TYPE_CADENCE_UART, > diff --git a/hw/char/trace-events b/hw/char/trace-events > index b64213d4dd..2ea25d1ea1 100644 > --- a/hw/char/trace-events > +++ b/hw/char/trace-events > @@ -73,3 +73,6 @@ cmsdk_apb_uart_receive(uint8_t c) "CMSDK APB UART: got character 0x%x from backe > cmsdk_apb_uart_tx_pending(void) "CMSDK APB UART: character send to backend pending" > cmsdk_apb_uart_tx(uint8_t c) "CMSDK APB UART: character 0x%x sent to backend" > cmsdk_apb_uart_set_params(int speed) "CMSDK APB UART: params set to %d 8N1" > + > +# hw/char/cadence_uart.c > +cadence_uart_baudrate(unsigned baudrate) "baudrate %u" > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Except migration: Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Hi Philippe, On 10/3/18 1:26 AM, Philippe Mathieu-Daudé wrote: > Hi Damien, > > On 10/2/18 4:24 PM, Damien Hedde wrote: >> Add bus interface and uart reference clock inputs. >> >> Note: it is hard to find out from the doc what is the behavior when only one >> of the clock is disabled. >> >> The implemented behaviour is that register access needs both clock being active. >> >> The bus interface control the mmios visibility > > This sentence sounds odd. Indeed, and also outdated. I removed the part switching on/off the mmio visibility according to the bus clock : accesses were silently ignored in that case, which would probably have made some user mad at some point. I replaced it by a check on every access and a LOG_GUEST_ERROR in case the access is dropped. > >> >> The reference clock controls the baudrate generation. If it disabled, >> any input characters and events are ignored. Also register accesses are >> conditioned to the clock being enabled (but is it really the case in >> reality ?) and a guest error is triggerred if that is not the case. >> >> If theses clocks remains unconnected, the uart behaves as before >> (default to 50MHz ref clock). >> >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >> --- >> include/hw/char/cadence_uart.h | 3 ++ >> hw/char/cadence_uart.c | 92 ++++++++++++++++++++++++++++++++-- >> hw/char/trace-events | 3 ++ >> 3 files changed, 93 insertions(+), 5 deletions(-) >> >> diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h >> index 118e3f10de..fd1d4725f4 100644 >> --- a/include/hw/char/cadence_uart.h >> +++ b/include/hw/char/cadence_uart.h >> @@ -21,6 +21,7 @@ >> #include "hw/sysbus.h" >> #include "chardev/char-fe.h" >> #include "qemu/timer.h" >> +#include "hw/clock-port.h" >> >> #define CADENCE_UART_RX_FIFO_SIZE 16 >> #define CADENCE_UART_TX_FIFO_SIZE 16 >> @@ -47,6 +48,8 @@ typedef struct { >> CharBackend chr; >> qemu_irq irq; >> QEMUTimer *fifo_trigger_handle; >> + ClockIn *refclk; >> + ClockIn *busclk; >> } CadenceUARTState; >> >> static inline DeviceState *cadence_uart_create(hwaddr addr, >> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c >> index fbdbd463bb..feb5cee4d7 100644 >> --- a/hw/char/cadence_uart.c >> +++ b/hw/char/cadence_uart.c >> @@ -28,6 +28,7 @@ >> #include "qemu/timer.h" >> #include "qemu/log.h" >> #include "hw/char/cadence_uart.h" >> +#include "trace.h" >> >> #ifdef CADENCE_UART_ERR_DEBUG >> #define DB_PRINT(...) do { \ >> @@ -94,7 +95,7 @@ >> #define LOCAL_LOOPBACK (0x2 << UART_MR_CHMODE_SH) >> #define REMOTE_LOOPBACK (0x3 << UART_MR_CHMODE_SH) >> >> -#define UART_INPUT_CLK 50000000 >> +#define UART_DEFAULT_REF_CLK (50 * 1000 * 1000) >> >> #define R_CR (0x00/4) >> #define R_MR (0x04/4) >> @@ -165,15 +166,30 @@ static void uart_send_breaks(CadenceUARTState *s) >> &break_enabled); >> } >> >> +static unsigned int uart_input_clk(CadenceUARTState *s) >> +{ >> + return clock_get_frequency(s->refclk); >> +} >> + >> static void uart_parameters_setup(CadenceUARTState *s) >> { >> QEMUSerialSetParams ssp; >> unsigned int baud_rate, packet_size; >> >> baud_rate = (s->r[R_MR] & UART_MR_CLKS) ? >> - UART_INPUT_CLK / 8 : UART_INPUT_CLK; >> + uart_input_clk(s) / 8 : uart_input_clk(s); >> + baud_rate /= (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); > > Safe because s->r[R_BRGR] >= 1, OK. > >> + trace_cadence_uart_baudrate(baud_rate); >> + >> + ssp.speed = baud_rate; >> + if (ssp.speed == 0) { >> + /* >> + * Avoid division-by-zero below. >> + * TODO: find something better >> + */ >> + ssp.speed = 1; >> + } >> >> - ssp.speed = baud_rate / (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); >> packet_size = 1; >> >> switch (s->r[R_MR] & UART_MR_PAR) { >> @@ -337,6 +353,11 @@ static void uart_receive(void *opaque, const uint8_t *buf, int size) >> CadenceUARTState *s = opaque; >> uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE; >> >> + /* ignore characters when unclocked */ >> + if (!clock_is_enabled(s->refclk)) { >> + return; >> + } >> + >> if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) { >> uart_write_rx_fifo(opaque, buf, size); >> } >> @@ -350,6 +371,11 @@ static void uart_event(void *opaque, int event) >> CadenceUARTState *s = opaque; >> uint8_t buf = '\0'; >> >> + /* ignore events when unclocked */ >> + if (!clock_is_enabled(s->refclk)) { >> + return; >> + } >> + >> if (event == CHR_EVENT_BREAK) { >> uart_write_rx_fifo(opaque, &buf, 1); >> } >> @@ -382,6 +408,14 @@ static void uart_write(void *opaque, hwaddr offset, >> { >> CadenceUARTState *s = opaque; >> >> + /* ignore accesses when bus or ref clock is disabled */ >> + if (!(clock_is_enabled(s->busclk) && clock_is_enabled(s->refclk))) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "cadence_uart: Trying to write register 0x%x" >> + " while clock is disabled\n", (unsigned) offset); >> + return; >> + } >> + >> DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value); >> offset >>= 2; >> if (offset >= CADENCE_UART_R_MAX) { >> @@ -440,6 +474,14 @@ static uint64_t uart_read(void *opaque, hwaddr offset, >> CadenceUARTState *s = opaque; >> uint32_t c = 0; >> >> + /* ignore accesses when bus or ref clock is disabled */ >> + if (!(clock_is_enabled(s->busclk) && clock_is_enabled(s->refclk))) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "cadence_uart: Trying to read register 0x%x" >> + " while clock is disabled\n", (unsigned) offset); >> + return 0; >> + } >> + >> offset >>= 2; >> if (offset >= CADENCE_UART_R_MAX) { >> c = 0; >> @@ -488,6 +530,14 @@ static void cadence_uart_realize(DeviceState *dev, Error **errp) >> uart_event, NULL, s, NULL, true); >> } >> >> +static void cadence_uart_refclk_update(void *opaque) >> +{ >> + CadenceUARTState *s = opaque; >> + >> + /* recompute uart's speed on clock change */ >> + uart_parameters_setup(s); >> +} >> + >> static void cadence_uart_init(Object *obj) >> { >> SysBusDevice *sbd = SYS_BUS_DEVICE(obj); >> @@ -497,9 +547,26 @@ static void cadence_uart_init(Object *obj) >> sysbus_init_mmio(sbd, &s->iomem); >> sysbus_init_irq(sbd, &s->irq); >> >> + s->refclk = qdev_init_clock_in(DEVICE(obj), "refclk", >> + cadence_uart_refclk_update, s); >> + /* initialize the frequency in case the clock remains unconnected */ >> + clock_init_frequency(s->refclk, UART_DEFAULT_REF_CLK); >> + s->busclk = qdev_init_clock_in(DEVICE(obj), "busclk", NULL, NULL); >> + /* initialize the frequency to non-zero in case it remains unconnected */ >> + clock_init_frequency(s->busclk, 100 * 1000 * 1000); > > #define INPUT_BUS_CLK_FREQUENCY (100 * 1000 * 1000) > >> + >> s->char_tx_time = (NANOSECONDS_PER_SECOND / 9600) * 10; >> } >> >> +static int cadence_uart_pre_load(void *opaque) >> +{ >> + CadenceUARTState *s = opaque; >> + >> + clock_init_frequency(s->refclk, UART_DEFAULT_REF_CLK); >> + clock_init_frequency(s->busclk, 100 * 1000 * 1000); > > INPUT_BUS_CLK_FREQUENCY > >> + return 0; >> +} >> + >> static int cadence_uart_post_load(void *opaque, int version_id) >> { >> CadenceUARTState *s = opaque; >> @@ -516,10 +583,22 @@ static int cadence_uart_post_load(void *opaque, int version_id) >> return 0; >> } >> >> +static const VMStateDescription vmstate_cadence_uart_clocks = { >> + .name = "cadence_uart_clocks", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .fields = (VMStateField[]) { >> + VMSTATE_CLOCKIN(refclk, CadenceUARTState), >> + VMSTATE_CLOCKIN(busclk, CadenceUARTState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> static const VMStateDescription vmstate_cadence_uart = { >> .name = "cadence_uart", >> .version_id = 2, >> .minimum_version_id = 2, >> + .pre_load = cadence_uart_pre_load, >> .post_load = cadence_uart_post_load, >> .fields = (VMStateField[]) { >> VMSTATE_UINT32_ARRAY(r, CadenceUARTState, CADENCE_UART_R_MAX), >> @@ -532,7 +611,10 @@ static const VMStateDescription vmstate_cadence_uart = { >> VMSTATE_UINT32(rx_wpos, CadenceUARTState), >> VMSTATE_TIMER_PTR(fifo_trigger_handle, CadenceUARTState), >> VMSTATE_END_OF_LIST() >> - } >> + }, >> + .subsections = (const VMStateDescription * []) { >> + &vmstate_cadence_uart_clocks, >> + }, >> }; >> >> static Property cadence_uart_properties[] = { >> @@ -548,7 +630,7 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data) >> dc->vmsd = &vmstate_cadence_uart; >> dc->reset = cadence_uart_reset; >> dc->props = cadence_uart_properties; >> - } >> +} >> >> static const TypeInfo cadence_uart_info = { >> .name = TYPE_CADENCE_UART, >> diff --git a/hw/char/trace-events b/hw/char/trace-events >> index b64213d4dd..2ea25d1ea1 100644 >> --- a/hw/char/trace-events >> +++ b/hw/char/trace-events >> @@ -73,3 +73,6 @@ cmsdk_apb_uart_receive(uint8_t c) "CMSDK APB UART: got character 0x%x from backe >> cmsdk_apb_uart_tx_pending(void) "CMSDK APB UART: character send to backend pending" >> cmsdk_apb_uart_tx(uint8_t c) "CMSDK APB UART: character 0x%x sent to backend" >> cmsdk_apb_uart_set_params(int speed) "CMSDK APB UART: params set to %d 8N1" >> + >> +# hw/char/cadence_uart.c >> +cadence_uart_baudrate(unsigned baudrate) "baudrate %u" >> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Except migration: > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> >
diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h index 118e3f10de..fd1d4725f4 100644 --- a/include/hw/char/cadence_uart.h +++ b/include/hw/char/cadence_uart.h @@ -21,6 +21,7 @@ #include "hw/sysbus.h" #include "chardev/char-fe.h" #include "qemu/timer.h" +#include "hw/clock-port.h" #define CADENCE_UART_RX_FIFO_SIZE 16 #define CADENCE_UART_TX_FIFO_SIZE 16 @@ -47,6 +48,8 @@ typedef struct { CharBackend chr; qemu_irq irq; QEMUTimer *fifo_trigger_handle; + ClockIn *refclk; + ClockIn *busclk; } CadenceUARTState; static inline DeviceState *cadence_uart_create(hwaddr addr, diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c index fbdbd463bb..feb5cee4d7 100644 --- a/hw/char/cadence_uart.c +++ b/hw/char/cadence_uart.c @@ -28,6 +28,7 @@ #include "qemu/timer.h" #include "qemu/log.h" #include "hw/char/cadence_uart.h" +#include "trace.h" #ifdef CADENCE_UART_ERR_DEBUG #define DB_PRINT(...) do { \ @@ -94,7 +95,7 @@ #define LOCAL_LOOPBACK (0x2 << UART_MR_CHMODE_SH) #define REMOTE_LOOPBACK (0x3 << UART_MR_CHMODE_SH) -#define UART_INPUT_CLK 50000000 +#define UART_DEFAULT_REF_CLK (50 * 1000 * 1000) #define R_CR (0x00/4) #define R_MR (0x04/4) @@ -165,15 +166,30 @@ static void uart_send_breaks(CadenceUARTState *s) &break_enabled); } +static unsigned int uart_input_clk(CadenceUARTState *s) +{ + return clock_get_frequency(s->refclk); +} + static void uart_parameters_setup(CadenceUARTState *s) { QEMUSerialSetParams ssp; unsigned int baud_rate, packet_size; baud_rate = (s->r[R_MR] & UART_MR_CLKS) ? - UART_INPUT_CLK / 8 : UART_INPUT_CLK; + uart_input_clk(s) / 8 : uart_input_clk(s); + baud_rate /= (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); + trace_cadence_uart_baudrate(baud_rate); + + ssp.speed = baud_rate; + if (ssp.speed == 0) { + /* + * Avoid division-by-zero below. + * TODO: find something better + */ + ssp.speed = 1; + } - ssp.speed = baud_rate / (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); packet_size = 1; switch (s->r[R_MR] & UART_MR_PAR) { @@ -337,6 +353,11 @@ static void uart_receive(void *opaque, const uint8_t *buf, int size) CadenceUARTState *s = opaque; uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE; + /* ignore characters when unclocked */ + if (!clock_is_enabled(s->refclk)) { + return; + } + if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) { uart_write_rx_fifo(opaque, buf, size); } @@ -350,6 +371,11 @@ static void uart_event(void *opaque, int event) CadenceUARTState *s = opaque; uint8_t buf = '\0'; + /* ignore events when unclocked */ + if (!clock_is_enabled(s->refclk)) { + return; + } + if (event == CHR_EVENT_BREAK) { uart_write_rx_fifo(opaque, &buf, 1); } @@ -382,6 +408,14 @@ static void uart_write(void *opaque, hwaddr offset, { CadenceUARTState *s = opaque; + /* ignore accesses when bus or ref clock is disabled */ + if (!(clock_is_enabled(s->busclk) && clock_is_enabled(s->refclk))) { + qemu_log_mask(LOG_GUEST_ERROR, + "cadence_uart: Trying to write register 0x%x" + " while clock is disabled\n", (unsigned) offset); + return; + } + DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value); offset >>= 2; if (offset >= CADENCE_UART_R_MAX) { @@ -440,6 +474,14 @@ static uint64_t uart_read(void *opaque, hwaddr offset, CadenceUARTState *s = opaque; uint32_t c = 0; + /* ignore accesses when bus or ref clock is disabled */ + if (!(clock_is_enabled(s->busclk) && clock_is_enabled(s->refclk))) { + qemu_log_mask(LOG_GUEST_ERROR, + "cadence_uart: Trying to read register 0x%x" + " while clock is disabled\n", (unsigned) offset); + return 0; + } + offset >>= 2; if (offset >= CADENCE_UART_R_MAX) { c = 0; @@ -488,6 +530,14 @@ static void cadence_uart_realize(DeviceState *dev, Error **errp) uart_event, NULL, s, NULL, true); } +static void cadence_uart_refclk_update(void *opaque) +{ + CadenceUARTState *s = opaque; + + /* recompute uart's speed on clock change */ + uart_parameters_setup(s); +} + static void cadence_uart_init(Object *obj) { SysBusDevice *sbd = SYS_BUS_DEVICE(obj); @@ -497,9 +547,26 @@ static void cadence_uart_init(Object *obj) sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->irq); + s->refclk = qdev_init_clock_in(DEVICE(obj), "refclk", + cadence_uart_refclk_update, s); + /* initialize the frequency in case the clock remains unconnected */ + clock_init_frequency(s->refclk, UART_DEFAULT_REF_CLK); + s->busclk = qdev_init_clock_in(DEVICE(obj), "busclk", NULL, NULL); + /* initialize the frequency to non-zero in case it remains unconnected */ + clock_init_frequency(s->busclk, 100 * 1000 * 1000); + s->char_tx_time = (NANOSECONDS_PER_SECOND / 9600) * 10; } +static int cadence_uart_pre_load(void *opaque) +{ + CadenceUARTState *s = opaque; + + clock_init_frequency(s->refclk, UART_DEFAULT_REF_CLK); + clock_init_frequency(s->busclk, 100 * 1000 * 1000); + return 0; +} + static int cadence_uart_post_load(void *opaque, int version_id) { CadenceUARTState *s = opaque; @@ -516,10 +583,22 @@ static int cadence_uart_post_load(void *opaque, int version_id) return 0; } +static const VMStateDescription vmstate_cadence_uart_clocks = { + .name = "cadence_uart_clocks", + .version_id = 0, + .minimum_version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_CLOCKIN(refclk, CadenceUARTState), + VMSTATE_CLOCKIN(busclk, CadenceUARTState), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_cadence_uart = { .name = "cadence_uart", .version_id = 2, .minimum_version_id = 2, + .pre_load = cadence_uart_pre_load, .post_load = cadence_uart_post_load, .fields = (VMStateField[]) { VMSTATE_UINT32_ARRAY(r, CadenceUARTState, CADENCE_UART_R_MAX), @@ -532,7 +611,10 @@ static const VMStateDescription vmstate_cadence_uart = { VMSTATE_UINT32(rx_wpos, CadenceUARTState), VMSTATE_TIMER_PTR(fifo_trigger_handle, CadenceUARTState), VMSTATE_END_OF_LIST() - } + }, + .subsections = (const VMStateDescription * []) { + &vmstate_cadence_uart_clocks, + }, }; static Property cadence_uart_properties[] = { @@ -548,7 +630,7 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data) dc->vmsd = &vmstate_cadence_uart; dc->reset = cadence_uart_reset; dc->props = cadence_uart_properties; - } +} static const TypeInfo cadence_uart_info = { .name = TYPE_CADENCE_UART, diff --git a/hw/char/trace-events b/hw/char/trace-events index b64213d4dd..2ea25d1ea1 100644 --- a/hw/char/trace-events +++ b/hw/char/trace-events @@ -73,3 +73,6 @@ cmsdk_apb_uart_receive(uint8_t c) "CMSDK APB UART: got character 0x%x from backe cmsdk_apb_uart_tx_pending(void) "CMSDK APB UART: character send to backend pending" cmsdk_apb_uart_tx(uint8_t c) "CMSDK APB UART: character 0x%x sent to backend" cmsdk_apb_uart_set_params(int speed) "CMSDK APB UART: params set to %d 8N1" + +# hw/char/cadence_uart.c +cadence_uart_baudrate(unsigned baudrate) "baudrate %u"
Add bus interface and uart reference clock inputs. Note: it is hard to find out from the doc what is the behavior when only one of the clock is disabled. The implemented behaviour is that register access needs both clock being active. The bus interface control the mmios visibility The reference clock controls the baudrate generation. If it disabled, any input characters and events are ignored. Also register accesses are conditioned to the clock being enabled (but is it really the case in reality ?) and a guest error is triggerred if that is not the case. If theses clocks remains unconnected, the uart behaves as before (default to 50MHz ref clock). Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- include/hw/char/cadence_uart.h | 3 ++ hw/char/cadence_uart.c | 92 ++++++++++++++++++++++++++++++++-- hw/char/trace-events | 3 ++ 3 files changed, 93 insertions(+), 5 deletions(-)