[PATCH v3] bootstd: sata: Add bootstd support for ahci sata

[Simon Glass]

Hi Tony,

On Fri, 8 Sept 2023 at 13:10, Tony Dinh <mibodhi at gmail.com> wrote:
>
> Add ahci sata bootdev and corresponding hunting function.
>
> Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> ---
>
> Changes in v3:
> - Correct drivers/ata/Makefile to compile sata_bootdev only if
> ahci sata is enabled.
>
> Changes in v2:
> - set devtype to sata in bootmeth_script for non-scsi SATA device.
>
>  boot/bootmeth_script.c     | 12 ++++++--
>  drivers/ata/Makefile       |  2 +-
>  drivers/ata/sata.c         | 25 +++++++++++++++
>  drivers/ata/sata_bootdev.c | 62 ++++++++++++++++++++++++++++++++++++++
>  include/sata.h             |  1 +
>  5 files changed, 99 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/ata/sata_bootdev.c
>
> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> index 58c57a2d4b..0269e0f9b0 100644
> --- a/boot/bootmeth_script.c
> +++ b/boot/bootmeth_script.c
> @@ -190,10 +190,18 @@ static int script_boot(struct udevice *dev, struct bootflow *bflow)
>         ulong addr;
>         int ret;
>
> -       if (desc->uclass_id == UCLASS_USB)
> +       if (desc->uclass_id == UCLASS_USB) {
>                 ret = env_set("devtype", "usb");
> -       else
> +       } else {
>                 ret = env_set("devtype", blk_get_devtype(bflow->blk));
> +
> +               /* If the parent uclass is AHCI, but the driver is ATA
> +                * (not scsi), set devtype to sata
> +                */
> +               if (!ret && IS_ENABLED(CONFIG_SATA) &&
> +                   !strcmp("ahci", env_get("devtype")))

Can you not use desc->uclass_id to do the right thing here? It seems
odd to compare with the value of the env var you just set up above.

> +                       ret = env_set("devtype", "sata");
> +       }
>         if (!ret)
>                 ret = env_set_hex("devnum", desc->devnum);
>         if (!ret)
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 6e30180b8b..0b6f91098a 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -10,7 +10,7 @@ obj-$(CONFIG_SCSI_AHCI) += ahci.o
>  obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o
>  obj-$(CONFIG_FSL_SATA) += fsl_sata.o
>  obj-$(CONFIG_LIBATA) += libata.o
> -obj-$(CONFIG_SATA) += sata.o
> +obj-$(CONFIG_SATA) += sata.o sata_bootdev.o
>  obj-$(CONFIG_SATA_CEVA) += sata_ceva.o
>  obj-$(CONFIG_SATA_MV) += sata_mv.o
>  obj-$(CONFIG_SATA_SIL) += sata_sil.o
> diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
> index ce3e9b5a40..9da7218564 100644
> --- a/drivers/ata/sata.c
> +++ b/drivers/ata/sata.c
> @@ -15,6 +15,8 @@
>  #include <dm.h>
>  #include <part.h>
>  #include <sata.h>
> +#include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
>
>  #ifndef CONFIG_AHCI
>  struct blk_desc sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
> @@ -50,6 +52,29 @@ int sata_scan(struct udevice *dev)
>         return ops->scan(dev);
>  }
>
> +int sata_rescan(bool verbose)
> +{
> +       struct udevice *dev;
> +       int devnum = 0;
> +       int ret;
> +
> +       /* Find and probing the SATA controller */
> +       ret = uclass_get_device(UCLASS_AHCI, devnum, &dev);

This does not actually rescan...if the device has already been probed,
it won't be probed again. You will need to use the logic like in
sata_remove().

Also, please avoid using devnum. You should probably just use
uclass_probe_all(UCLASS_AHCI)

> +
> +       /* Sanity check */
> +       if (ret)
> +               ret = uclass_find_first_device(UCLASS_AHCI, &dev);

That just finds it, but does not probe it. See uclass_first_device_err()

> +       if (ret) {
> +               printf("Cannot probe SATA device %d (err=%d)\n", devnum, ret);
> +               return -ENOSYS;

return ret

> +       }
> +       if (!dev) {
> +               printf("No SATA device found!\n");
> +               return -ENOSYS;
> +       }
> +       return 0;
> +}
> +
>  #ifndef CONFIG_AHCI
>  #ifdef CONFIG_PARTITIONS
>  struct blk_desc *sata_get_dev(int dev)
> diff --git a/drivers/ata/sata_bootdev.c b/drivers/ata/sata_bootdev.c
> new file mode 100644
> index 0000000000..f638493ce0
> --- /dev/null
> +++ b/drivers/ata/sata_bootdev.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Bootdev for sata
> + *
> + * Copyright 2023 Tony Dinh <mibodhi at gmail.com>
> + */
> +
> +#include <common.h>
> +#include <ahci.h>
> +#include <bootdev.h>
> +#include <dm.h>
> +#include <init.h>
> +#include <sata.h>
> +
> +static int sata_bootdev_bind(struct udevice *dev)
> +{
> +       struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev);
> +
> +       ucp->prio = BOOTDEVP_4_SCAN_FAST;
> +
> +       return 0;
> +}
> +
> +static int sata_bootdev_hunt(struct bootdev_hunter *info, bool show)
> +{
> +       int ret;
> +
> +       if (IS_ENABLED(CONFIG_PCI)) {
> +               ret = pci_init();
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       ret = sata_rescan(true);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +struct bootdev_ops sata_bootdev_ops = {
> +};
> +
> +static const struct udevice_id sata_bootdev_ids[] = {
> +       { .compatible = "u-boot,bootdev-sata" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(sata_bootdev) = {
> +       .name           = "sata_bootdev",
> +       .id             = UCLASS_BOOTDEV,
> +       .ops            = &sata_bootdev_ops,
> +       .bind           = sata_bootdev_bind,
> +       .of_match       = sata_bootdev_ids,
> +};
> +
> +BOOTDEV_HUNTER(sata_bootdev_hunter) = {
> +       .prio           = BOOTDEVP_4_SCAN_FAST,
> +       .uclass         = UCLASS_AHCI,
> +       .hunt           = sata_bootdev_hunt,
> +       .drv            = DM_DRIVER_REF(sata_bootdev),
> +};
> diff --git a/include/sata.h b/include/sata.h
> index d89f7a8a29..0495744bad 100644
> --- a/include/sata.h
> +++ b/include/sata.h
> @@ -20,5 +20,6 @@ extern struct blk_desc sata_dev_desc[];
>
>  int sata_probe(int devnum);
>  int sata_remove(int devnum);
> +int sata_rescan(bool verbose);

Please add a comment

>
>  #endif
> --
> 2.39.2
>

Regards,
Simon