Re: [PATCH v4 05/12] hw/arm: Add NPCM730 and NPCM750 SoC models

[Havard Skinnemoen]

On Wed, Jul 8, 2020 at 10:56 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 7/8/20 7:31 PM, Philippe Mathieu-Daudé wrote:
> > Hi Havard,
> >
> > On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
> >> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> >> Management Controllers in servers. While the family includes four SoCs,
> >> this patch implements limited support for two of them: NPCM730 (targeted
> >> for Data Center applications) and NPCM750 (targeted for Enterprise
> >> applications).
> >>
> >> This patch includes little more than the bare minimum needed to boot a
> >> Linux kernel built with NPCM7xx support in direct-kernel mode:
> >>
> >>   - Two Cortex-A9 CPU cores with built-in periperhals.
> >>   - Global Configuration Registers.
> >>   - Clock Management.
> >>   - 3 Timer Modules with 5 timers each.
> >>   - 4 serial ports.
> >>
> >> The chips themselves have a lot more features, some of which will be
> >> added to the model at a later stage.
> [...]
>
> >> +static void npcm7xx_realize(DeviceState *dev, Error **errp)
> >> +{
> >> +    NPCM7xxState *s = NPCM7XX(dev);
> >> +    NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
> >> +    Error *err = NULL;
> >> +    int i;
> >> +
> >> +    /* I/O space -- unimplemented unless overridden below. */
> >> +    create_unimplemented_device("npcm7xx.io", NPCM7XX_MMIO_BA, 
> >> NPCM7XX_MMIO_SZ);
> >
> > I still insist this is not the best, but as "The data sheet for these
> > SoCs is not generally available" there is not much I can suggest to
> > improve.
>
> From your other comment I found:
>
> https://github.com/Nuvoton-Israel/bootblock/blob/master/SWC_HAL/Chips/npcm750/npcm750.h
>
> In particular:
>
> #define AHB1_BASE_ADDR                  0xF0000000      /* AHB1
> allocation (Including APB allocations)  */
> #define AHB18_BASE_ADDR                 0x80000000      /* AHB18
> allocation  */
> #define AHB3_BASE_ADDR                  0xA0000000      /* AHB3
> allocation  */
> #define XBUSR_BASE_ADDR                 0xC0002000      /* XBUS
> registers  */
> #define AHB14_BASE_ADDR                 0xE0000000      /* AHB14
> Allocation  */
> #define APB14_BASE_ADDR                 0xE0000000      /* APB14
> Allocation  */
> #define VDMX_BASE_ADDR                  0xE0800000      /* VDMX  */
>
> XBUS doesn't seem important.
>
> If SPI flashes aren't connected, returning bus transaction sounds
> correct:
>
> #define SPI0CS0_BASE_ADDR               0x80000000      /* SPI0 direct
> access CS0  */
> #define SPI0CS1_BASE_ADDR               0x88000000      /* SPI0 direct
> access CS1  */
> #define SPI0CS2_BASE_ADDR               0x90000000      /* SPI0 direct
> access CS2  */
> #define SPI0CS3_BASE_ADDR               0x98000000      /* SPI0 direct
> access CS3  */
>
> #define SPI3CS0_BASE_ADDR               0xA0000000      /* SPI3 direct
> access CS0  */
> #define SPI3CS1_BASE_ADDR               0xA8000000      /* SPI3 direct
> access CS1  */
> #define SPI3CS2_BASE_ADDR               0xB0000000      /* SPI3 direct
> access CS2  */
> #define SPI3CS3_BASE_ADDR               0xB8000000      /* SPI3 direct
> access CS3  */
>
> So I'd prefer you use:
>
>   create_unimplemented_device("npcm7xx.AHB1",  0xf0000000, 256 * MiB);
>
> Maybe for the PCI root complex:
>
>   create_unimplemented_device("npcm7xx.AHB14", 0xe0000000, 256 * MiB);
>
> What do you think?

I went ahead and added them all since they are all defined in that
public file. It does make the -d unimp output a lot more helpful.

I'll send v5 tonight. Not sure if I got the DRAM stuff 100% right.
Please let me know what you think.

Havard