From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45618) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJ1FP-00025y-2W for qemu-devel@nongnu.org; Fri, 01 Jul 2016 12:24:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bJ1FM-0004dW-Tf for qemu-devel@nongnu.org; Fri, 01 Jul 2016 12:24:54 -0400 Received: from mail-vk0-x22b.google.com ([2607:f8b0:400c:c05::22b]:32895) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJ1FM-0004d7-9e for qemu-devel@nongnu.org; Fri, 01 Jul 2016 12:24:52 -0400 Received: by mail-vk0-x22b.google.com with SMTP id k68so72998523vkb.0 for ; Fri, 01 Jul 2016 09:24:52 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1467138270-32481-8-git-send-email-clg@kaod.org> References: <1467138270-32481-1-git-send-email-clg@kaod.org> <1467138270-32481-8-git-send-email-clg@kaod.org> From: Peter Maydell Date: Fri, 1 Jul 2016 17:24:32 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 7/9] ast2400: add SPI flash slaves List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?C=C3=A9dric_Le_Goater?= Cc: Peter Crosthwaite , QEMU Developers , qemu-arm , Kevin Wolf , Markus Armbruster , Andrew Jeffery On 28 June 2016 at 19:24, C=C3=A9dric Le Goater wrote: > Each controller on the ast2400 has a memory range on which it maps its > flash module slaves. Each slave is assigned a memory segment for its > mapping that can be changed at bootime with the Segment Address > Register. This is not supported in the current implementation so we > are using the defaults provided by the specs. > > Each SPI flash slave can then be accessed in two modes: Command and > User. When in User mode, accesses to the memory segment of the slaves > are translated in SPI transfers. When in Command mode, the HW > generates the SPI commands automatically and the memory segment is > accessed as if doing a MMIO. Other SPI controllers call that mode > linear addressing mode. > > For this purpose, we are adding below each crontoller an array of > structs gathering for each SPI flash module, a segment rank, a > MemoryRegion to handle the memory accesses and the associated SPI > slave device, which should be a m25p80. > > Only the User mode is supported for now but we are preparing ground > for the Command mode. The framework is sufficient to support Linux. > > Signed-off-by: C=C3=A9dric Le Goater > +/* > + * Default segments mapping addresses and size for each slave per > + * controller. These can be changed when board is initialized with the > + * Segment Address Registers but they don't seem do be used on the > + * field. > + */ > +static const AspeedSegments aspeed_segments_legacy[] =3D { > + { 0x10000000, 32 * 1024 * 1024 }, > +}; > + > +static const AspeedSegments aspeed_segments_fmc[] =3D { > + { 0x20000000, 64 * 1024 * 1024 }, > + { 0x24000000, 32 * 1024 * 1024 }, > + { 0x26000000, 32 * 1024 * 1024 }, > + { 0x28000000, 32 * 1024 * 1024 }, > + { 0x2A000000, 32 * 1024 * 1024 } > +}; > + > +static const AspeedSegments aspeed_segments_spi[] =3D { > + { 0x30000000, 64 * 1024 * 1024 }, > +}; If I understand this correctly, and the Segment Address Registers are part of the SMC controller, then eventually if we want to implement making these writable then the correct approach is probably to have a container memory region which the SMC controller provides to the board (and which the board then maps into the system memory space), and then the controller is responsible for mapping and unmapping the individual memory regions inside that container. This is basically how we do PCI controllers, which also allow guests to write to PCI BARs to map and unmap bits of memory. This will be fine for now, though. > static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs= ) > @@ -237,6 +353,8 @@ static void aspeed_smc_realize(DeviceState *dev, Erro= r **errp) > AspeedSMCState *s =3D ASPEED_SMC(dev); > AspeedSMCClass *mc =3D ASPEED_SMC_GET_CLASS(s); > int i; > + char name[32]; > + hwaddr offset =3D 0; > > s->ctrl =3D mc->ctrl; > > @@ -270,6 +388,32 @@ static void aspeed_smc_realize(DeviceState *dev, Err= or **errp) > memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s, > s->ctrl->name, ASPEED_SMC_R_MAX * 4); > sysbus_init_mmio(sbd, &s->mmio); > + > + /* > + * Memory region where flash modules are remapped > + */ > + snprintf(name, sizeof(name), "%s.flash", s->ctrl->name); > + > + memory_region_init_io(&s->mmio_flash, OBJECT(s), > + &aspeed_smc_flash_default_ops, s, name, > + s->ctrl->mapping_window_size); > + sysbus_init_mmio(sbd, &s->mmio_flash); > + > + s->flashes =3D g_malloc0(sizeof(AspeedSMCFlash) * s->num_cs); This should be g_new0(AspeedSMCFlash, s->num_cs); -- multiplying in a g_malloc0() is usually a sign you should use g_new0 instead. Otherwise Reviewed-by: Peter Maydell so I'll just fix that when I put the series in target-arm.next. thanks -- PMM