Re: [Qemu-devel] [PATCH 2/2] exynos4210: UART: Added using of CMU-callback functionality

[Blue Swirl]

On Fri, Jun 29, 2012 at 4:04 PM, Maksim Kozlov <address@hidden> wrote:
> Add using of functionality provided by CMU - get_rate and 
> register_clock_handler.
>
> Signed-off-by: Maksim Kozlov <address@hidden>
> ---
>  hw/exynos4210_uart.c |   36 +++++++++++++++++++++++++++++++++++-
>  1 files changed, 35 insertions(+), 1 deletions(-)
>
> diff --git a/hw/exynos4210_uart.c b/hw/exynos4210_uart.c
> index ccc4780..2a5eb28 100644
> --- a/hw/exynos4210_uart.c
> +++ b/hw/exynos4210_uart.c
> @@ -307,6 +307,7 @@ static void 
> exynos4210_uart_update_parameters(Exynos4210UartState *s)
>     uint64_t uclk_rate;
>
>     if (s->reg[I_(UBRDIV)] == 0) {
> +        PRINT_DEBUG("Baud rate division value is 0\n");
>         return;
>     }
>
> @@ -332,7 +333,19 @@ static void 
> exynos4210_uart_update_parameters(Exynos4210UartState *s)
>
>     frame_size += data_bits + stop_bits;
>
> -    uclk_rate = 24000000;
> +    switch (s->channel) {
> +    case 0:
> +        uclk_rate = exynos4210_cmu_get_rate(EXYNOS4210_SCLK_UART0); break;

Please put the break statement to a separate line.

> +    case 1:
> +        uclk_rate = exynos4210_cmu_get_rate(EXYNOS4210_SCLK_UART1); break;
> +    case 2:
> +        uclk_rate = exynos4210_cmu_get_rate(EXYNOS4210_SCLK_UART2); break;
> +    case 3:
> +        uclk_rate = exynos4210_cmu_get_rate(EXYNOS4210_SCLK_UART3); break;
> +    default:
> +        hw_error("%s: Incorrect UART channel: %d\n",
> +                 __func__, s->channel);
> +    }
>
>     speed = uclk_rate / ((16 * (s->reg[I_(UBRDIV)]) & 0xffff) +
>             (s->reg[I_(UFRACVAL)] & 0x7) + 16);
> @@ -348,6 +361,15 @@ static void 
> exynos4210_uart_update_parameters(Exynos4210UartState *s)
>                 s->channel, speed, parity, data_bits, stop_bits);
>  }
>
> +static void uclk_rate_changed(void *opaque)
> +{
> +    Exynos4210UartState *s = (Exynos4210UartState *)opaque;

The cast is useless, this is C.

> +
> +    PRINT_DEBUG("Clock sclk_uart%d was changed\n", s->channel);
> +
> +    exynos4210_uart_update_parameters(s);
> +}
> +
>  static void exynos4210_uart_write(void *opaque, target_phys_addr_t offset,
>                                uint64_t val, unsigned size)
>  {
> @@ -542,6 +564,7 @@ static void exynos4210_uart_reset(DeviceState *dev)
>             container_of(dev, Exynos4210UartState, busdev.qdev);
>     int regs_number = sizeof(exynos4210_uart_regs)/sizeof(Exynos4210UartReg);
>     int i;
> +    Exynos4210Clock clock_id;
>
>     for (i = 0; i < regs_number; i++) {
>         s->reg[I_(exynos4210_uart_regs[i].offset)] =
> @@ -551,6 +574,17 @@ static void exynos4210_uart_reset(DeviceState *dev)
>     fifo_reset(&s->rx);
>     fifo_reset(&s->tx);
>
> +    switch (s->channel) {
> +    case 0: clock_id = EXYNOS4210_SCLK_UART0; break;

Also here the case label and statements belong to separate lines.

> +    case 1: clock_id = EXYNOS4210_SCLK_UART1; break;
> +    case 2: clock_id = EXYNOS4210_SCLK_UART2; break;
> +    case 3: clock_id = EXYNOS4210_SCLK_UART3; break;
> +    default:
> +        hw_error("Wrong channel number: %d.\n", s->channel);
> +    }
> +
> +    exynos4210_register_clock_handler(uclk_rate_changed, clock_id, s);
> +
>     PRINT_DEBUG("UART%d: Rx FIFO size: %d\n", s->channel, s->rx.size);
>  }
>
> --
> 1.7.5.4
>
>