From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yan9n-0004a5-HV for qemu-devel@nongnu.org; Wed, 25 Mar 2015 11:23:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yan9k-0000oZ-8X for qemu-devel@nongnu.org; Wed, 25 Mar 2015 11:23:47 -0400 Received: from mout.web.de ([212.227.15.14]:55855) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yan9j-0000oD-VK for qemu-devel@nongnu.org; Wed, 25 Mar 2015 11:23:44 -0400 Message-ID: <5512D2F8.9010308@web.de> Date: Wed, 25 Mar 2015 16:23:36 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1427292969-30929-1-git-send-email-armbru@redhat.com> <1427292969-30929-3-git-send-email-armbru@redhat.com> In-Reply-To: <1427292969-30929-3-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 for-2.3 2/5] hw: Mark devices picking up char backends actively FIXME List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, Peter Crosthwaite , Michael Walle , Antony Pavlov , "Edgar E. Iglesias" , pbonzini@redhat.com, Li Guang Am 25.03.2015 um 15:16 schrieb Markus Armbruster: > Character devices defined with -serial and -parallel are for board > initialization to wire up. Board code examines serial_hds[] and > parallel_hds[] to find them, and creates devices with their qdev > chardev properties set accordingly. > > Except a few devices go on a fishing expedition for a suitable backend > instead of exposing a chardev property for board code to set: they use > serial_hds[] (often via qemu_char_get_next_serial()) or parallel_hds[] > in their realize() or init() method to connect to a backend. > > Picking up backends that way works when the devices are created by > board code. But it's inappropriate for -device or device_add. Not > only is it inconsistent with how the other characrer device models > work (they connect to a backend explicitly identified by a "chardev" > property), it breaks when the backend has been picked up by the board > or a previous -device / device_add already. > > Example: > > $ qemu-system-ppc64 -M bamboo -S -device i82378 -device pc87312 -device pc87312 > qemu-system-ppc64: -device pc87312: Property 'isa-parallel.chardev' can't take value 'parallel0', it's in use > > Mark them with suitable FIXME comments. > > Cc: Li Guang > Cc: Peter Crosthwaite > Cc: Antony Pavlov > Cc: "Edgar E. Iglesias" > Cc: Michael Walle > Cc: Peter Crosthwaite > Cc: "Andreas Färber" > Signed-off-by: Markus Armbruster > --- > hw/arm/allwinner-a10.c | 1 + > hw/char/cadence_uart.c | 1 + > hw/char/digic-uart.c | 1 + > hw/char/etraxfs_ser.c | 1 + > hw/char/lm32_juart.c | 1 + > hw/char/lm32_uart.c | 1 + > hw/char/milkymist-uart.c | 1 + > hw/char/pl011.c | 1 + > hw/char/stm32f2xx_usart.c | 1 + > hw/char/xilinx_uartlite.c | 1 + > hw/isa/pc87312.c | 2 ++ > 11 files changed, 12 insertions(+) [...] > diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c > index 2849e8d..3b1fcec 100644 > --- a/hw/isa/pc87312.c > +++ b/hw/isa/pc87312.c > @@ -278,6 +278,7 @@ static void pc87312_realize(DeviceState *dev, Error **errp) > pc87312_hard_reset(s); > > if (is_parallel_enabled(s)) { > + /* FIXME use a qdev chardev prop instead of parallel_hds[] */ > chr = parallel_hds[0]; > if (chr == NULL) { > chr = qemu_chr_new("par0", "null", NULL); > @@ -296,6 +297,7 @@ static void pc87312_realize(DeviceState *dev, Error **errp) > > for (i = 0; i < 2; i++) { > if (is_uart_enabled(s, i)) { > + /* FIXME use a qdev chardev prop instead of serial_hds[] */ > chr = serial_hds[i]; > if (chr == NULL) { > snprintf(name, sizeof(name), "ser%d", i); Same question here: The isa-parallel and isa-serial sub-devices a few lines further below do have a chardev property, just not the container device. Andreas