On Thu, 24 Mar 2022 at 10:00, Dylan Jhong <dy...@andestech.com> wrote: > > On Wed, Mar 23, 2022 at 05:37:10PM +0800, Peter Maydell wrote: > > On Wed, 23 Mar 2022 at 09:20, Dylan Jhong <dy...@andestech.com> wrote: > > > > > > Although the "wakeup" parameter is declared in SerialState, > > > but there is no function actually setting it up. > > > Support "wakeup" as parameter in serial_mm_init(). > > > > This patch seems to provide a new argument which every > > caller passes the same value for, unless I missed one > > somewhere. What's the reason for this change?
> First of all, thank you for your review. > The purpose of this variable is to allow users to specify their own wakeup > reason id. > > At present, there are only 4 wakeup reasons provided by QEMU[*1]. > Take uart as an example, which are classified as QEMU_WAKEUP_REASON_OTHER, > so there is no way to distinguish the source of the wakeup reason when > designing a custom power manager device. > > Indeed, as you can see, currently there is no device that supports the use of > "wakeup_reason". > But it will be used on the board we are going to upstream later. None of this tells me why different UARTs would want different wakeup reasons, or how, if I'm writing a new UART device model, I should select the wakeup reason, though. If the intention of this change is as an initial step in the addition of a new board model, then you should provide it as part of the patchseries that's upstreaming that board, so that we can see in context how the proposed new API is going to be used. Also, the patch changes the wakeup reason for every user of serial_mm_init() from "OTHER" to "NONE", which doesn't seem right. Since serial_mm_init() is now just a legacy wrapper for creating a device and setting its properties, consider just having your new board model create and configure the device directly, including the new wakeup-reason property, rather than using serial_mm_init() and forcing all the other callers of it to change. (Doing that is bette modern QEMU coding style anyway, usually.) thanks -- PMM
