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

Simon Glass sjg at chromium.org
Tue Jul 13 22:17:00 CEST 2021


Hi Daniel,

On Mon, 12 Jul 2021 at 19:17, Daniel Schwierzeck
<daniel.schwierzeck at gmail.com> wrote:
>
> 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.

OK.


> >
> >
> > > +       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


More information about the U-Boot mailing list