Hi Cedric, Philippe
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Tuesday, May 28, 2024 3:03 PM
> To: Philippe Mathieu-Daudé <phi...@linaro.org>; Jamin Lin
> <jamin_...@aspeedtech.com>; Peter Maydell <peter.mayd...@linaro.org>;
> Andrew Jeffery <and...@codeconstruct.com.au>; Joel Stanley
> <j...@jms.id.au>; Alistair Francis <alist...@alistair23.me>; Cleber Rosa
> <cr...@redhat.com>; Wainer dos Santos Moschetta <waine...@redhat.com>;
> Beraldo Leal <bl...@redhat.com>; open list:ASPEED BMCs
> <qemu-...@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_...@aspeedtech.com>; Yunlin Tang
> <yunlin.t...@aspeedtech.com>
> 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 <troy_...@aspeedtech.com>
> >> Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
> >> Reviewed-by: Cédric Le Goater <c...@kaod.org>
> >> ---
> >> 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.
>
>
>
