Re: Broken CONFIG_SPL_SATA_SUPPORT=y

From: Simon Glass <sjg@chromium.org>
To: "Pali Rohár" <pali@kernel.org>
Cc: "Stefan Roese" <sr@denx.de>,
	"Masami Hiramatsu" <masami.hiramatsu@linaro.org>,
	"Frank Wunderlich" <frank-w@public-files.de>,
	"Marek Behún" <marek.behun@nic.cz>,
	"U-Boot Mailing List" <u-boot@lists.denx.de>
Subject: Re: Broken CONFIG_SPL_SATA_SUPPORT=y
Date: Mon, 2 Aug 2021 13:21:51 -0600	[thread overview]
Message-ID: <CAPnjgZ1zi5_pc8mtD5cf9ZsehsfmUf54314k-akqotaXR0JmFw@mail.gmail.com> (raw)
In-Reply-To: <20210801122516.dl6vykk22aumzjfd@pali>

Hi Pali,

On Sun, 1 Aug 2021 at 06:25, Pali Rohár <pali@kernel.org> wrote:
>
> Hello!
>
> Option CONFIG_SPL_SATA_SUPPORT=y is currently broken in u-boot master
> branch. If I try to enable it for A38x platform I'm getting following
> compiler error:
>
>     LD      spl/u-boot-spl
>   arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.o: in function `ahci_probe_scsi_pci':
>   drivers/ata/ahci.c:1205: undefined reference to `dm_pci_map_bar'
>   arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.c:1215: undefined reference to `dm_pci_read_config16'
>   arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.c:1216: undefined reference to `dm_pci_read_config16'
>   arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci.c:1220: undefined reference to `dm_pci_map_bar'
>   make[1]: *** [scripts/Makefile.spl:512: spl/u-boot-spl] Error 1
>   make: *** [Makefile:1977: spl/u-boot-spl] Error 2
>
> You can reproduce it by running following commands:
>
>   $ make turris_omnia_defconfig
>   $ echo CONFIG_SPL_SATA_SUPPORT=y >> .config
>   $ make CROSS_COMPILE=arm-linux-gnueabihf-
>
> I workaround it by following patch:
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index d4047c04f5d0..6bad72e4cfa4 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1196,7 +1196,7 @@ int ahci_probe_scsi(struct udevice *ahci_dev, ulong base)
>         return 0;
>  }
>
> -#ifdef CONFIG_DM_PCI
> +#if CONFIG_IS_ENABLED(DM_PCI)
>  int ahci_probe_scsi_pci(struct udevice *ahci_dev)
>  {
>         ulong base;
>
> It fixed this particular problem. So it looks like that CONFIG_DM_PCI is
> defined also when building SPL even when it is not enabled for SPL.
> Whole PCI is disabled in SPL.
>
> But then I got another compile error:
>
>     LD      spl/u-boot-spl
>   arm-linux-gnueabihf-ld.bfd: drivers/ata/ahci-pci.o: in function `ahci_pci_probe':
>   drivers/ata/ahci-pci.c:21: undefined reference to `ahci_probe_scsi_pci'
>   make[1]: *** [scripts/Makefile.spl:512: spl/u-boot-spl] Error 1
>   make: *** [Makefile:1977: spl/u-boot-spl] Error 2
>
> Seems that u-boot is trying to compile and link ahci-pci.o into SPL
> binary even when it is not enabled nor used. PCI is completed disabled
> in SPL for this case.
>
> I workaround it by putting whole ahci-pci.c file into one big #idef:
>
> diff --git a/drivers/ata/ahci-pci.c b/drivers/ata/ahci-pci.c
> index b1d231e0f9e1..34afebd2f87f 100644
> --- a/drivers/ata/ahci-pci.c
> +++ b/drivers/ata/ahci-pci.c
> @@ -9,6 +9,8 @@
>  #include <dm.h>
>  #include <pci.h>
>
> +#if CONFIG_IS_ENABLED(DM_PCI)
> +
>  static int ahci_pci_bind(struct udevice *dev)
>  {
>         struct udevice *scsi_dev;
> @@ -42,3 +44,5 @@ static struct pci_device_id ahci_pci_supported[] = {
>  };
>
>  U_BOOT_PCI_DEVICE(ahci_pci, ahci_pci_supported);
> +
> +#endif
>
> And then finally U-Boot produced final target image u-boot-spl.kwb:
>
>   LD      spl/u-boot-spl
>   OBJCOPY spl/u-boot-spl-nodtb.bin
>   SYM     spl/u-boot-spl.sym
>   CAT     spl/u-boot-spl-dtb.bin
>   COPY    spl/u-boot-spl.bin
>   MKIMAGE u-boot-spl.kwb
>
> So this looks like a bug in Kconfig or Makefile dependences that build
> system is trying to compile and link also files which should not be
> linked at all.

I sent a series to drop DM_PCI:

http://patchwork.ozlabs.org/project/uboot/list/?series=256181

Can you give that a try and see if it helps?

Regards,
Simon