[U-Boot] [PATCH] dm: pci: Change to config device according to Header Type

[Simon Glass]

Hi Hou,

On Mon, 5 Aug 2019 at 02:05, Hou Zhiqiang <Zhiqiang.Hou at nxp.com> wrote:
>
> The configuration space layouts of normal device, bridge and
> CardBus bridge are distinct, and it is indicated by Header
> Type instead of Class Code. So change it to config device
> according to Header Type.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> ---
>  drivers/pci/pci_auto.c | 60 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> index 1a3bf70..06621d5 100644
> --- a/drivers/pci/pci_auto.c
> +++ b/drivers/pci/pci_auto.c
> @@ -311,6 +311,7 @@ int dm_pciauto_config_device(struct udevice *dev)
>         struct pci_region *pci_io;
>         unsigned int sub_bus = PCI_BUS(dm_pci_get_bdf(dev));
>         unsigned short class;
> +       u8 header_type;
>         bool enum_only = false;
>         struct udevice *ctlr = pci_get_controller(dev);
>         struct pci_controller *ctlr_hose = dev_get_uclass_priv(ctlr);
> @@ -325,9 +326,33 @@ int dm_pciauto_config_device(struct udevice *dev)
>         pci_io = ctlr_hose->pci_io;
>
>         dm_pci_read_config16(dev, PCI_CLASS_DEVICE, &class);
> +       dm_pci_read_config8(dev, PCI_HEADER_TYPE, &header_type);
> +
> +       switch (header_type & 0x7f) {
> +       case PCI_HEADER_TYPE_BRIDGE:
> +               if (class == PCI_CLASS_BRIDGE_OTHER) {
> +#if defined(CONFIG_PCIAUTO_SKIP_HOST_BRIDGE)
> +                       debug("PCI Autoconfig: Skipping bridge device %d\n",
> +                             PCI_DEV(dm_pci_get_bdf(dev)));
> +                       break;
> +#endif
> +#if defined(CONFIG_ARCH_MPC834X) && !defined(CONFIG_TARGET_VME8349) && \
> +                       !defined(CONFIG_TARGET_CADDY2)

We cannot put board-specific code in generic code. You could fix this by:

1. adding a 'quirks' value to pci
2. Adding a Kconfig CONFIG_PCI_QUIRKS to enable checking quirks
3. Enabling that for your board
4. Checking the quirk here, e.g. if (ctrl->quirk & PCI_QUIRK_NAME_HERE)
5. Turn on the quirk using a device-tree property in your PCI driver.

Is that code was already there, but it is still wrong :-)

> +                       /*
> +                        * The host/PCI bridge 1 seems broken in 8349 - it
> +                        * presents itself as 'PCI_CLASS_BRIDGE_OTHER' and
> +                        * appears as an _agent_ device claiming resources
> +                        * io/mem/irq.. we only allow for the PIMMR window
> +                        * to be allocated (BAR0 - 1MB size)
> +                        */
> +                       debug("PCI Autoconfig: Broken bridge found, only minimal config\n");
> +                       dm_pciauto_setup_device(dev, 0, hose->pci_mem,
> +                                               hose->pci_prefetch,
> +                                               hose->pci_io, enum_only);
> +                       break;
> +#endif
> +               }
>
> -       switch (class) {
> -       case PCI_CLASS_BRIDGE_PCI:
>                 debug("PCI Autoconfig: Found P2P bridge, device %d\n",
>                       PCI_DEV(dm_pci_get_bdf(dev)));
>
> @@ -340,7 +365,7 @@ int dm_pciauto_config_device(struct udevice *dev)
>                 sub_bus = (unsigned int)n;
>                 break;
>
> -       case PCI_CLASS_BRIDGE_CARDBUS:
> +       case PCI_HEADER_TYPE_CARDBUS:
>                 /*
>                  * just do a minimal setup of the bridge,
>                  * let the OS take care of the rest
> @@ -353,35 +378,14 @@ int dm_pciauto_config_device(struct udevice *dev)
>
>                 break;
>
> -#if defined(CONFIG_PCIAUTO_SKIP_HOST_BRIDGE)
> -       case PCI_CLASS_BRIDGE_OTHER:
> -               debug("PCI Autoconfig: Skipping bridge device %d\n",
> -                     PCI_DEV(dm_pci_get_bdf(dev)));
> -               break;
> -#endif
> -#if defined(CONFIG_ARCH_MPC834X) && !defined(CONFIG_TARGET_VME8349) && \
> -               !defined(CONFIG_TARGET_CADDY2)
> -       case PCI_CLASS_BRIDGE_OTHER:
> -               /*
> -                * The host/PCI bridge 1 seems broken in 8349 - it presents
> -                * itself as 'PCI_CLASS_BRIDGE_OTHER' and appears as an _agent_
> -                * device claiming resources io/mem/irq.. we only allow for
> -                * the PIMMR window to be allocated (BAR0 - 1MB size)
> -                */
> -               debug("PCI Autoconfig: Broken bridge found, only minimal config\n");
> -               dm_pciauto_setup_device(dev, 0, hose->pci_mem,
> -                                       hose->pci_prefetch, hose->pci_io,
> +       case PCI_HEADER_TYPE_NORMAL:
> +               dm_pciauto_setup_device(dev, 6, pci_mem, pci_prefetch, pci_io,
>                                         enum_only);
>                 break;
> -#endif
> -
> -       case PCI_CLASS_PROCESSOR_POWERPC: /* an agent or end-point */
> -               debug("PCI AutoConfig: Found PowerPC device\n");
> -               /* fall through */
>
>         default:
> -               dm_pciauto_setup_device(dev, 6, pci_mem, pci_prefetch, pci_io,
> -                                       enum_only);
> +               printf("Unknown header type %02x, ignoring device\n",
> +                      header_type);
>                 break;
>         }
>
> --
> 2.9.5
>

Regards,
Simon