Re: [PATCH] hw/misc/pfsoc: add fabric clocks to ioscb

[Conor Dooley]

On Thu, Nov 10, 2022 at 12:18:44AM +0100, Philippe Mathieu-Daudé wrote:
> On 9/11/22 20:08, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > On PolarFire SoC, some peripherals (eg the PCI root port) are clocked by
> > "Clock Conditioning Circuitry" in the FPGA. The specific clock depends
> > on the FPGA bitstream & can be locked to one particular {D,P}LL - in the
> > Icicle Kit Reference Design v2022.09 or later this is/will be the case.
> > 
> > Linux v6.1+ will have a driver for this peripheral and devicetrees that
> > previously relied on "fixed-frequency" clock nodes have been switched
> > over to clock-controller nodes. The IOSCB region is represented in QEMU,
> > but the specific region of it that the CCCs occupy has not so v6.1-rcN
> > kernels fail to boot in QEMU.
> > 
> > Add the regions as unimplemented so that the status-quo in terms of boot
> > is maintained.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > The last line there is a white lie. v6.1-rcN has both v2022.09 and
> > v2022.10 reference design changes. This patch only accounts for the
> > v2022.09 changes. The FPGA design is a moving target and I am not
> > really sure how to handle that in QEMU. For v2022.10 a bunch of stuff
> > got changed, including the addresses that DDR lies at which I am not
> > sure how to handle yet.
> > 
> > That puts my todo list of broken things to:
> > - MMC (only direct kernel boot works), pre v2022.09 reference issue
> 
> How do you start without 'direct kernel boot'?

You used to be able to load the "bios" etc and follow the boot flow [0].
This no longer works, and has not for at least a year. I assume it still
works if you check out the (fossilised) versions mentioned in that doc.
Think I said it last time I sent patches, but we had some floating around
internally that I know /did/ work at some point about this time last year
but I was never able to figure out the correct alignment of the stars to
get working myself. It required pretty decent changes to the sdhci driver,
which, I'm hoping cease to be required with the v2022.10 reference
design that I mentioned else where in this patch.
But one problem at a time ;)

0 - https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html

> > - PCI root port address, address changed in v2022.09 but from a cursory
> >    check, I didn't see any PCI support in the first place. It's connected
> >    to a FIC, so I think it can just be made into an unimplemented region.
> > - DDR address changes, 2022.10 issue. Looks like a straightforward
> >    change to hw/riscv/pfsoc.c but I don't think it'll be backwards
> >    compatible.
> > - hwrng breaks boot. Tipping away at this one, hopefully I'll have a fix
> >    for it soon. Need to implement the irq side of the mailbox for it.
> > 
> > I'll send some more patches as I work through them.
> > 
> >   hw/misc/mchp_pfsoc_ioscb.c         | 6 ++++++
> >   include/hw/misc/mchp_pfsoc_ioscb.h | 1 +
> >   2 files changed, 7 insertions(+)
> > 
> > diff --git a/hw/misc/mchp_pfsoc_ioscb.c b/hw/misc/mchp_pfsoc_ioscb.c
> > index f4fd55a0e5..f976e42f72 100644
> > --- a/hw/misc/mchp_pfsoc_ioscb.c
> > +++ b/hw/misc/mchp_pfsoc_ioscb.c
> > @@ -33,6 +33,7 @@
> >    */
> >   #define IOSCB_WHOLE_REG_SIZE        0x10000000
> >   #define IOSCB_SUBMOD_REG_SIZE       0x1000
> > +#define IOSCB_CCC_REG_SIZE          0x2000000
> >   /*
> >    * There are many sub-modules in the IOSCB module.
> > @@ -45,6 +46,7 @@
> >   #define IOSCB_LANE23_BASE           0x06510000
> >   #define IOSCB_CTRL_BASE             0x07020000
> >   #define IOSCB_CFG_BASE              0x07080000
> > +#define IOSCB_CCC_BASE              0x08000000
> >   #define IOSCB_PLL_MSS_BASE          0x0E001000
> >   #define IOSCB_CFM_MSS_BASE          0x0E002000
> >   #define IOSCB_PLL_DDR_BASE          0x0E010000
> > @@ -168,6 +170,10 @@ static void mchp_pfsoc_ioscb_realize(DeviceState *dev, 
> > Error **errp)
> >                             "mchp.pfsoc.ioscb.cfg", IOSCB_SUBMOD_REG_SIZE);
> >       memory_region_add_subregion(&s->container, IOSCB_CFG_BASE, &s->cfg);
> > +    memory_region_init_io(&s->ccc, OBJECT(s), &mchp_pfsoc_dummy_ops, s,
> > +                          "mchp.pfsoc.ioscb.ccc", IOSCB_CCC_REG_SIZE);
> > +    memory_region_add_subregion(&s->container, IOSCB_CCC_BASE, &s->ccc);
> 
> Unrelated but using the TYPE_UNIMPLEMENTED_DEVICE would ease tracing all
> these block accesses, as the block name would appear before the
> address/size. See for example aspeed_mmio_map_unimplemented();

I just copy pasted what was already here. Follow on patch for the
conversions since most of what's in this file is effectively
unimplemented?

> >       memory_region_init_io(&s->pll_mss, OBJECT(s), &mchp_pfsoc_pll_ops, s,
> >                             "mchp.pfsoc.ioscb.pll_mss", 
> > IOSCB_SUBMOD_REG_SIZE);
> >       memory_region_add_subregion(&s->container, IOSCB_PLL_MSS_BASE, 
> > &s->pll_mss);
> > diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h 
> > b/include/hw/misc/mchp_pfsoc_ioscb.h
> > index 9235523e33..687b213742 100644
> > --- a/include/hw/misc/mchp_pfsoc_ioscb.h
> > +++ b/include/hw/misc/mchp_pfsoc_ioscb.h
> > @@ -30,6 +30,7 @@ typedef struct MchpPfSoCIoscbState {
> >       MemoryRegion lane23;
> >       MemoryRegion ctrl;
> >       MemoryRegion cfg;
> > +    MemoryRegion ccc;
> >       MemoryRegion pll_mss;
> >       MemoryRegion cfm_mss;
> >       MemoryRegion pll_ddr;
>