Hi Cedric, > From: Cédric Le Goater <c...@kaod.org> > Subject: Re: [SPAM] Re: [PATCH v4 09/16] aspeed/smc: Add AST2700 support > > >>>> @@ -670,7 +670,7 @@ static const MemoryRegionOps > >> aspeed_smc_flash_ops > >>>> = { > >>>> .endianness = DEVICE_LITTLE_ENDIAN, > >>>> .valid = { > >>>> .min_access_size = 1, > >>>> - .max_access_size = 4, > >>>> + .max_access_size = 8, > >>> > >>> Is this a bugfix? If so, please use a separate patch. Otherwise > >>> please mention why it is OK to widen access for AST2600 & AST10x0. > >> > > According the design of SPI drivers, it uses this "memcpy_fromio" KERNEL API > for SPI calibration. > > > https://github.com/AspeedTech-BMC/linux/blob/1062a07420f9aed4ed7dc9deb > > 3429b8e7828f5cf/drivers/spi/spi-aspeed-smc.c#L1832 > > AST2700 is a 64 bits quad core cpus(Cortex-a35), so kernel API use 64 bits > > for > data access. > > https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/a > > rm64/kernel/io.c#L25 I simply set the max_access_size to 8 for AST2700 > > support. > > AST2500, AST2600 and AST10x0 are all 32bits CPUS, that was why this > max_access_size 8 did not impact these models. > > https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/a > > rm/kernel/io.c#L45 > > Yes. I think we are safe on that side. > > > If you have any suggestion about this patch modification, please let me > > know. > > I am going to re-send v5 patch for AST2700 support. > > Please move this change in its own commit explaining the reason and add a > TODO comment in the code. > > The aspeed_smc_flash_ops MemoryRegionOps should be copied in _realize() > to set a different width for the AST2700 SoC. You could do that too. > > Thanks, > > C. I will do the following changes. Could you give me any suggestion?
1. add asc->max_access_size = 8 in aspeed_2700_fmc_class_init, aspeed_2700_spi0_class_init, aspeed_2700_spi1_class_init and aspeed_2700_spi2_class_init 2. Update aspeed_smc_flash_realize as below static void aspeed_smc_flash_realize(DeviceState *dev, Error **errp) { ------ s->asc = ASPEED_SMC_GET_CLASS(s->controller) if (s->asc->max_access_size ==8) --> check max_access_size aspeed_smc_flash_ops.valid.max_access_size = s->asc->max_access --> update max_access_size /* * Use the default segment value to size the memory region. This * can be changed by FW at runtime. */ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_flash_ops, s, name, s->asc->segments[s->cs].size); ------ } Thanks-Jamin