Hi Cedric, Philippe > From: Cédric Le Goater <[email protected]> > Sent: Tuesday, May 28, 2024 3:03 PM > To: Philippe Mathieu-Daudé <[email protected]>; Jamin Lin > <[email protected]>; Peter Maydell <[email protected]>; > Andrew Jeffery <[email protected]>; Joel Stanley > <[email protected]>; Alistair Francis <[email protected]>; Cleber Rosa > <[email protected]>; Wainer dos Santos Moschetta <[email protected]>; > Beraldo Leal <[email protected]>; open list:ASPEED BMCs > <[email protected]>; open list:All patches CC here > <[email protected]> > Cc: Troy Lee <[email protected]>; Yunlin Tang > <[email protected]> > Subject: Re: [SPAM] Re: [PATCH v4 09/16] aspeed/smc: Add AST2700 support > > On 5/27/24 17:58, Philippe Mathieu-Daudé wrote: > > Hi, > > > > On 27/5/24 10:02, Jamin Lin wrote: > >> AST2700 fmc/spi controller's address decoding unit is 64KB and only > >> bits [31:16] are used for decoding. Introduce seg_to_reg and > >> reg_to_seg handlers for ast2700 fmc/spi controller. > >> In addition, adds ast2700 fmc, spi0, spi1, and spi2 class init handler. > >> > >> Signed-off-by: Troy Lee <[email protected]> > >> Signed-off-by: Jamin Lin <[email protected]> > >> Reviewed-by: Cédric Le Goater <[email protected]> > >> --- > >> hw/ssi/aspeed_smc.c | 222 > >> +++++++++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 220 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index > >> df0c63469c..b4006c8339 100644 > >> --- a/hw/ssi/aspeed_smc.c > >> +++ b/hw/ssi/aspeed_smc.c > >> @@ -185,7 +185,7 @@ > >> * 0: 4 bytes > >> * 0x1FFFFFC: 32M bytes > >> * > >> - * DMA length is from 1 byte to 32MB (AST2600, AST10x0) > >> + * DMA length is from 1 byte to 32MB (AST2600, AST10x0 and AST2700) > >> * 0: 1 byte > >> * 0x1FFFFFF: 32M bytes > >> */ > >> @@ -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
If you have any suggestion about this patch modification, please let me know. I am going to re-send v5 patch for AST2700 support. Thanks-Jamin > Ah I missed that. I wonder how we could set different access width tough on > the model ? > > Should we allocate a MemoryRegionOps in the realize() handler and set the > width depending on the SoC ? > > > Thanks, > > C. > > >
