From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59987) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlKvV-0006NW-Ij for qemu-devel@nongnu.org; Thu, 12 Jan 2012 08:42:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RlKvN-0002n1-Dd for qemu-devel@nongnu.org; Thu, 12 Jan 2012 08:42:45 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:64452) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlKvM-0002mj-UZ for qemu-devel@nongnu.org; Thu, 12 Jan 2012 08:42:37 -0500 Received: from euspt2 (mailout2.w1.samsung.com [210.118.77.12]) by mailout2.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTP id <0LXO00LQLU2XLD@mailout2.w1.samsung.com> for qemu-devel@nongnu.org; Thu, 12 Jan 2012 13:42:33 +0000 (GMT) Received: from [106.109.8.162] by spt2.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0LXO002W2U2VK3@spt2.w1.samsung.com> for qemu-devel@nongnu.org; Thu, 12 Jan 2012 13:42:33 +0000 (GMT) Date: Thu, 12 Jan 2012 17:42:31 +0400 From: Mitsyanko Igor In-reply-to: <4F0EDB98.9050306@suse.de> Message-id: <4F0EE347.9000707@samsung.com> MIME-version: 1.0 Content-type: text/plain; format=flowed; charset=ISO-8859-1 Content-transfer-encoding: QUOTED-PRINTABLE References: <1326213943-878-1-git-send-email-mark.langsdorf@calxeda.com> <1326299490-10780-1-git-send-email-mark.langsdorf@calxeda.com> <1326299490-10780-6-git-send-email-mark.langsdorf@calxeda.com> <4F0ED646.1040206@samsung.com> <4F0EDB98.9050306@suse.de> Subject: Re: [Qemu-devel] [PATCH v9 5/6] arm: SoC model for Calxeda Highbank Reply-To: i.mitsyanko@samsung.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Andreas_F=E4rber?= Cc: peter.maydell@linaro.org, Mark Langsdorf , qemu-devel@nongnu.org, Rob Herring , Paul Brook , edgar.iglesias@gmail.com On 01/12/2012 05:09 PM, Andreas F=E4rber wrote: > Am 12.01.2012 13:47, schrieb Mitsyanko Igor: >> On 01/11/2012 08:31 PM, Mark Langsdorf wrote: >>> + sysram =3D g_new(MemoryRegion, 1); >>> + memory_region_init_ram(sysram, "highbank.sysram", 0x8000); >>> + memory_region_add_subregion(sysmem, 0xfff88000, sysram); >>> + if (bios_name !=3D NULL) { >>> + sysboot_filename =3D qemu_find_file(QEMU_FILE_TYPE_BIOS, >>> bios_name); >>> + if (sysboot_filename !=3D NULL) { >>> + uint32_t filesize =3D get_image_size(sysboot_filenam= e); >>> + if (load_image_targphys("sysram.bin", 0xfff88000, >>> filesize)< 0) { >>> + hw_error("Unable to load %s\n", bios_name); >>> + } >> >> Probably should be >> if (load_image_targphys(sysboot_filename, 0xfff88000, 0x8000)< 0= ) { >> and then you don't need "uint32_t filesize" at all. > > You need it either way; if you use 0x8000 there, you need to check = if > filesize is actually 0x8000. Doing it this way allows to load small= er > files; a check for larger files should be added though. Thanks for > making me aware. > Why do we need to check if filesize is 0x8000? load_image_targphys()= =20 will call get_image_size() and check that size is not more then 0x800= 0=20 automatically, so it would operate on any file with size<=3D0x8000 an= d=20 return error if size>0x8000, just like we need it to. Well, OK, I kno= w=20 load_image_targphys() is currently broken and doesn't use max_sz=20 argument, but recently I saw a patch in mailing list which fixes this= . >>> + dev =3D qdev_create(NULL, "l2x0"); >>> + qdev_init_nofail(dev); >>> + busdev =3D sysbus_from_qdev(dev); >>> + sysbus_mmio_map(busdev, 0, 0xfff12000); >> >>> + dev =3D qdev_create(NULL, "highbank-regs"); >>> + qdev_init_nofail(dev); >>> + busdev =3D sysbus_from_qdev(dev); >>> + sysbus_mmio_map(busdev, 0, 0xfff3c000); >>> + >> >> You can use sysbus_create_simple() here (of course, if you didn't = avoid >> it intentionally for some reason). > > Depends on how you read this: > > /* Legacy helper function for creating devices. */ > DeviceState *sysbus_create_varargs(const char *name, > target_phys_addr_t addr, ...); > DeviceState *sysbus_try_create_varargs(const char *name, > target_phys_addr_t addr, ..= .); > static inline DeviceState *sysbus_create_simple(const char *name, > target_phys_addr_t a= ddr, > qemu_irq irq) > { > return sysbus_create_varargs(name, addr, irq, NULL); > } > > I interpret it as sysbus_create_simple() using deprecated > sysbus_create_varargs() and therefore being deprecated, too. > > Andreas > Sorry, never paid attention that these functions are deprecated. --=20 Mitsyanko Igor ASWG, Moscow R&D center, Samsung Electronics email: i.mitsyanko@samsung.com