[PATCH 5/6] MIPS: malta: add support for PCI driver model

Daniel Schwierzeck daniel.schwierzeck at gmail.com
Tue Jul 13 03:17:18 CEST 2021


Hi Simon,

Am Samstag, den 10.07.2021, 18:00 -0600 schrieb Simon Glass:
> () Hi Daniel,
> 
> On Tue, 6 Jul 2021 at 08:22, Daniel Schwierzeck
> <daniel.schwierzeck at gmail.com> wrote:
> > As almost all peripherals are connected via PCI dependent on the
> > used core card, PCI setup is always required. Thus run pci_init()
> > including PCI scanning and probing and core card specific setups
> > in board_early_init_r().
> > 
> > Also prepare support for dynamically managing the status of the
> > different PCI DT nodes dependent on used core card via option
> > CONFIG_OF_BOARD_FIXUP. Before this feature can be enabled,
> > the call order of the fix_fdt() init hook in board_init_f
> > needs to be changed. Otherwise rw_fdt_blob points to a read-only
> > NOR flash address. Thus this options needs to stay disabled
> > until the board_init_f problem could be solved. This breaks
> > running the default U-Boot image on real HW using the FPGA core
> > card but Qemu emulation still works. Currently Qemu is more
> > important as MIPS CI tests depend on Malta and the deadline
> > for PCI DM conversion will be enforced soon.
> > 
> > Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> > ---
> > 
> >  board/imgtec/malta/malta.c | 84
> > +++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 83 insertions(+), 1 deletion(-)
> > 
> > diff --git a/board/imgtec/malta/malta.c
> > b/board/imgtec/malta/malta.c
> > index c04f727961..e78d5a7fbc 100644
> > --- a/board/imgtec/malta/malta.c
> > +++ b/board/imgtec/malta/malta.c
> > @@ -4,7 +4,8 @@
> >   * Copyright (C) 2013 Imagination Technologies
> >   */
> > 
> > -#include <common.h>
> > +#include <config.h>
> > +#include <fdt_support.h>
> >  #include <ide.h>
> >  #include <init.h>
> >  #include <net.h>
> > @@ -24,6 +25,9 @@
> > 
> >  DECLARE_GLOBAL_DATA_PTR;
> > 
> > +#define MALTA_GT_PATH   "/pci0 at 1be00000"
> > +#define MALTA_MSC_PATH  "/pci0 at 1bd00000"
> > +
> >  enum core_card {
> >         CORE_UNKNOWN,
> >         CORE_LV,
> > @@ -120,10 +124,12 @@ int checkboard(void)
> >         return 0;
> >  }
> > 
> > +#if !IS_ENABLED(CONFIG_DM_ETH)
> >  int board_eth_init(struct bd_info *bis)
> >  {
> >         return pci_eth_init(bis);
> >  }
> > +#endif
> > 
> >  void _machine_restart(void)
> >  {
> > @@ -167,6 +173,81 @@ int misc_init_r(void)
> >         return 0;
> >  }
> > 
> > +#if IS_ENABLED(CONFIG_OF_BOARD_FIXUP)
> > +/*
> > + * TODO: currently doesn't work because rw_fdt_blob points to a
> > + * NOR flash address. This needs some changes in board_init_f.
> > + */
> > +int board_fix_fdt(void *rw_fdt_blob)
> > +{
> > +       int node = -1;
> > +
> > +       switch (malta_sys_con()) {
> > +       case SYSCON_GT64120:
> > +               node =  fdt_path_offset(rw_fdt_blob,
> > MALTA_GT_PATH);
> > +               break;
> > +       default:
> > +       case SYSCON_MSC01:
> > +               node =  fdt_path_offset(rw_fdt_blob,
> );
> > +               break;
> > +       }
> > +
> > +       return fdt_status_okay(rw_fdt_blob, node);
> > +}
> > +#endif
> > +
> > +#if IS_ENABLED(CONFIG_DM_PCI)
> > +int board_early_init_r(void)
> > +{
> > +       struct udevice *dev;
> > +       u32 val32;
> > +       u8 val8;
> > +       int ret;
> > +
> > +       pci_init();
> > +
> > +       ret = dm_pci_find_device(PCI_VENDOR_ID_INTEL,
> > +                                PCI_DEVICE_ID_INTEL_82371AB_0, 0,
> > &dev);
> 
> This feels a bit wonky to me. How about a sysinfo driver for your
> board which does this init? Then you could just probe it.
> 
> As to finding PCI devices, your sysinfo driver could have a few
> phandle properties to point to the two devices you need.
> 
> But if these are really SYSCON devices, why not use the SYSCON
> devices, give them a unique ID (perhaps you already have with
> SYSCON_MSC01) and then call syscon_get_regmap_by_driver_data() ?

This is just a reimplementation of the original pci_init_board()
function. I wanted to keep the changes small and to keep the original
PCI init behaviour to get the PCI DM conversion done and get this patch
series merged in this merge window ;) The Malta board is the reference
board for MIPS Qemu and essential for testing and U-Boot CI. Therefore
it would be very bad if it gets removed due to the PCI DM conversion
deadline.

I already tried to add PIIX4 IRQ and ATA drivers for this init code.
But those drivers were just bound and I didn't have an idea how to
trigger a probe. I will look into it again with the next patch series
for removing all non-DM code.
> 
> 
> > +       if (ret)
> > +               panic("Failed to find PIIX4 PCI bridge\n");
> > +
> > +       /* setup PCI interrupt routing */
> > +       dm_pci_write_config8(dev, PCI_CFG_PIIX4_PIRQRCA, 10);
> > +       dm_pci_write_config8(dev, PCI_CFG_PIIX4_PIRQRCB, 10);
> > +       dm_pci_write_config8(dev, PCI_CFG_PIIX4_PIRQRCC, 11);
> > +       dm_pci_write_config8(dev, PCI_CFG_PIIX4_PIRQRCD, 11);
> 
> This feels like it should be in DT.
> 
> I don't know if this is at all similar to x86, but see
> create_pirq_routing_table()  and intel,irq-router.txt for some ideas.
> 
> > +
> > +       /* mux SERIRQ onto SERIRQ pin */
> > +       dm_pci_read_config32(dev, PCI_CFG_PIIX4_GENCFG, &val32);
> > +       val32 |= PCI_CFG_PIIX4_GENCFG_SERIRQ;
> > +       dm_pci_write_config32(dev, PCI_CFG_PIIX4_GENCFG, val32);
> > +
> 
> dm_pci_clrset_config32() can do these three in one line, similar
> below.

I'll fix it

> 
> 
> > +       /* enable SERIRQ - Linux currently depends upon this */
> > +       dm_pci_read_config8(dev, PCI_CFG_PIIX4_SERIRQC, &val8);
> > +       val8 |= PCI_CFG_PIIX4_SERIRQC_EN |
> > PCI_CFG_PIIX4_SERIRQC_CONT;
> > +       dm_pci_write_config8(dev, PCI_CFG_PIIX4_SERIRQC, val8);
> > +
> > +       ret = dm_pci_find_device(PCI_VENDOR_ID_INTEL,
> > +                                PCI_DEVICE_ID_INTEL_82371AB, 0,
> > &dev);
> > +       if (ret)
> > +               panic("Failed to find PIIX4 IDE controller\n");
> > +
> > +       /* enable bus master & IO access */
> > +       val32 |= PCI_COMMAND_MASTER | PCI_COMMAND_IO;
> > +       dm_pci_write_config32(dev, PCI_COMMAND, val32);
> > +
> > +       /* set latency */
> > +       dm_pci_write_config8(dev, PCI_LATENCY_TIMER, 0x40);
> > +
> > +       /* enable IDE/ATA */
> > +       dm_pci_write_config32(dev, PCI_CFG_PIIX4_IDETIM_PRI,
> > +                             PCI_CFG_PIIX4_IDETIM_IDE);
> > +       dm_pci_write_config32(dev, PCI_CFG_PIIX4_IDETIM_SEC,
> > +                             PCI_CFG_PIIX4_IDETIM_IDE);
> > +
> > +       return 0;
> > +}
> > +#else
> >  void pci_init_board(void)
> >  {
> >         pci_dev_t bdf;
> > @@ -231,3 +312,4 @@ void pci_init_board(void)
> >         pci_write_config_dword(bdf, PCI_CFG_PIIX4_IDETIM_SEC,
> >                                PCI_CFG_PIIX4_IDETIM_IDE);
> >  }
> > +#endif
> > --
> > 2.32.0
> > 
> 
> Regards,
> Simon
-- 
- Daniel



More information about the U-Boot mailing list