@@ -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/1062a07420f9aed4ed7dc9deb3429b8e7828f5cf/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/arm64/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/arm/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.


Reply via email to