qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] char: cadence: check baud rate generator and


From: Alistair Francis
Subject: Re: [Qemu-devel] [PATCH v2] char: cadence: check baud rate generator and divider values
Date: Tue, 25 Oct 2016 10:22:17 -0700

On Mon, Oct 24, 2016 at 6:22 AM, P J P <address@hidden> wrote:
> From: Prasad J Pandit <address@hidden>
>
> The Cadence UART device emulator calculates speed by dividing the
> baud rate by a 'baud rate generator' & 'baud rate divider' value.
> The device specification defines these register values to be
> non-zero and within certain limits. Add checks for these limits
> to avoid errors like divide by zero.
>
> Reported-by: Huawei PSIRT <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  hw/char/cadence_uart.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> Update: set register values as per the specification
>   -> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04931.html
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index e3bc52f..c176446 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -1,5 +1,6 @@
>  /*
>   * Device model for Cadence UART
> + *  -> 
> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf

Can you say what page/section the UART spec is in the Xilinx TRM?

I think it might also be worth noting that the datasheet is a Xilinx
datasheet that covers the Cadence UART. Others might be using the IP
as well and might get confused why you are referring to a Xilinx
datasheet.

>   *
>   * Copyright (c) 2010 Xilinx Inc.
>   * Copyright (c) 2012 Peter A.G. Crosthwaite (address@hidden)
> @@ -410,6 +411,18 @@ static void uart_write(void *opaque, hwaddr offset,
>              break;
>          }
>          break;
> +    case R_BRGR: /* Baud rate generator */
> +        s->r[offset] = 0x028B; /* default reset value */

Is this the correct behavior, or should the write just be ignored?
pg.587 of the TRM doesn't really make this clear, did you find this
somewhere else?

> +        if (value >= 0x01 && value <= 0xFFFF) {
> +            s->r[offset] = value;
> +        }
> +        break;
> +    case R_BDIV:    /* Baud rate divider */
> +        s->r[offset] = 0x0F;

Same here.

Thanks,

Alistair

> +        if (value >= 0x04 && value <= 0xFF) {
> +            s->r[offset] = value;
> +        }
> +        break;
>      default:
>          s->r[offset] = value;
>      }
> --
> 2.7.4
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]