答复: [PATCH] hw/arm/aspeed: fix connect_serial_hds_to_uarts

Commit Message

kenneth_jia--- via Dec. 13, 2024, 9:48 a.m. UTC

We plans to use a relatively large number of UART connections, and currently test all UARTs that can be used. 
When we use qumu for simulation, the problem was discovered.
 The current code does not allocate char devices with index 5 (label serial5).
 And even if there are more serial devices, the last one (UAR12)  is not connected to serial.

Add debug information to watch:

Test command
        ~/office_qemu/build/qemu-system-arm -m 1G -M ast2600-evb,fmc-model=mx66l1g45g -nographic \
        -drive file=./${evb}.bin,format=raw,if=mtd,unit=0 \
        -drive file=./emmc1.bin,format=raw,if=sd,unit=2 \
        -net nic -net nic -net nic -net nic \
        -net user,hostfwd=::2443-:443,hostfwd=::2242-:22,hostfwd=udp::2623-:623 \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty \
        -serial pty

Output Log: 

QEMU 9.2.50 monitor - type 'help' for more information
char device redirected to /dev/pts/6 (label serial0)
char device redirected to /dev/pts/7 (label serial1)
char device redirected to /dev/pts/8 (label serial2)
char device redirected to /dev/pts/9 (label serial3)
char device redirected to /dev/pts/10 (label serial4)
char device redirected to /dev/pts/11 (label serial5)
char device redirected to /dev/pts/12 (label serial6)
char device redirected to /dev/pts/13 (label serial7)
char device redirected to /dev/pts/14 (label serial8)
char device redirected to /dev/pts/15 (label serial9)
char device redirected to /dev/pts/16 (label serial10)
char device redirected to /dev/pts/17 (label serial11)
char device redirected to /dev/pts/19 (label serial12)
char device redirected to /dev/pts/21 (label serial13)
char device redirected to /dev/pts/22 (label serial14)
uart 0 char index 1 name pty:/dev/pts/7
uart 1 char index 2 name pty:/dev/pts/8
uart 2 char index 3 name pty:/dev/pts/9
uart 3 char index 4 name pty:/dev/pts/10
uart 5 char index 6 name pty:/dev/pts/12
uart 6 char index 7 name pty:/dev/pts/13
uart 7 char index 8 name pty:/dev/pts/14
uart 8 char index 9 name pty:/dev/pts/15
uart 9 char index 10 name pty:/dev/pts/16
uart 10 char index 11 name pty:/dev/pts/17
uart 11 char index 12 name pty:/dev/pts/19
(qemu)

Issue: 
/dev/pts/17 (label serial11) is not connected to a UART,
UART12 is not connect to a serial device.

-----邮件原件-----
发件人: Cédric Le Goater <[e-mail]> 
发送时间: 2024年12月13日 15:19
收件人: Kenneth Jia(贾纪东_华硕上海) <[e-mail]>; '[e-mail]' <[e-mail]>
抄送: 'Cédric Le Goater' <[e-mail]>; 'Philippe Mathieu-Daudé' <philmd@linaro.o
> 
> Subject: [PATCH]    hw/arm/aspeed: fix connect_serial_hds_to_uarts
> 
>     In the loop, we need ignore the index increase when uart == 
> uart_chosen
> 
>     We should increase the index only after we allocate a serial.
> 
> Signed-off-by: Kenneth Jia <[e-mail]>

Could you please tell us a bit on your test environment ? I am interested to know how you found the issue.

Also, please use "git send-email" to send patches.

Anyhow,

Fixes: d2b3eaefb4d7 ("aspeed: Refactor UART init for multi-SoC machines")
Reviewed-by: Cédric Le Goater <[e-mail]>

Thanks,

C.



> 
> ---
> 
> hw/arm/aspeed.c | 4 ++--
> 
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> 
> index 556498f2a0..d8cb2d1429 100644
> 
> --- a/hw/arm/aspeed.c
> 
> +++ b/hw/arm/aspeed.c
> 
> @@ -364,11 +364,11 @@ static void 
> connect_serial_hds_to_uarts(AspeedMachineState *bmc)
> 
>       int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : 
> amc->uart_default;
> 
>       aspeed_soc_uart_set_chr(s, uart_chosen, serial_hd(0));
> 
> -    for (int i = 1, uart = sc->uarts_base; i < sc->uarts_num; i++, 
> uart++) {
> 
> +    for (int i = 1, uart = sc->uarts_base; i < sc->uarts_num; uart++) 
> +{
> 
>           if (uart == uart_chosen) {
> 
>               continue;
> 
>           }
> 
> -        aspeed_soc_uart_set_chr(s, uart, serial_hd(i));
> 
> +        aspeed_soc_uart_set_chr(s, uart, serial_hd(i++));
> 
>       }
> 
> }
> 
> --
> 
> 2.34.1
> 


<p></p>

Comments

Cédric Le Goater Dec. 13, 2024, 9:56 a.m. UTC | #1

On 12/13/24 10:48, Kenneth Jia(贾纪东_华硕上海) wrote:
>   We plans to use a relatively large number of UART connections, and currently test all UARTs that can be used.
> When we use qumu for simulation, the problem was discovered.

Which QEMU machine are you using ?

Thanks,

C.

kenneth_jia--- via Dec. 13, 2024, 10:11 a.m. UTC | #2

> Which QEMU machine are you using ?
   ast2600-evb


Thanks,

Kenneth

-----邮件原件-----
发件人: Cédric Le Goater <[e-mail]> 
发送时间: 2024年12月13日 17:57
收件人: Kenneth Jia(贾纪东_华硕上海) <[e-mail]>; '[e-mail]' <[e-mail]>
抄送: 'Cédric Le Goater' <[e-mail]>; 'Philippe Mathieu-Daudé' <[e-mail]>; 'Jamin Lin' <[e-mail]>; 'Andrew Jeffery' <[e-mail]>; 'Gavin Shan' <[e-mail]>; 'open list:All patches CC here' <[e-mail]>
主题: Re: 答复: [PATCH] hw/arm/aspeed: fix connect_serial_hds_to_uarts

External email : Ensure your email is secure before opening links and attachments.

On 12/13/24 10:48, Kenneth Jia(贾纪东_华硕上海) wrote:
>   We plans to use a relatively large number of UART connections, and currently test all UARTs that can be used.
> When we use qumu for simulation, the problem was discovered.

Which QEMU machine are you using ?

Thanks,

C.


<p></p>

Patch

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 556498f2a0..786df450ed 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -369,6 +369,7 @@  static void connect_serial_hds_to_uarts(AspeedMachineState *bmc)
             continue;
         }
         aspeed_soc_uart_set_chr(s, uart, serial_hd(i));
+       printf("uart %d char index %d name %s\n", uart-sc->uarts_base, i, serial_hd(i)?serial_hd(i)->filename:"null");
     }
 }