On Thu, Oct 03, 2019 at 08:48:59AM +0200, Heinrich Schuchardt wrote:
> On 10/2/19 11:11 PM, Patrick Wildt wrote:
> > This adds a device path node for NVMe block devices.  For that
> > nvme_get_namespace_id() is added to return the privately stored
> > namespace identifier.
> 
> Thanks a lot for looking into this.
> 
> The structures and constants that you define are correct. I would prefer
> if we could use the real value for the EUI instead of a dummy value of 0.
> 
> Both NVMe namespaces and EUIs can be discovered by issuing the
> 'Identify' command to the NVMe controller. See U-Boot function
> nvme_identify().
> 
> All NVMe revisions support 'Controller or Namespace Structure (CNS)'
> value 0x00. With this value you will retrieve the 'Identity Namespace'
> data structure. This call is already done in nvme_blk_probe().
> 
> Looking at the code I guess that you just have to add the EUI64 field to
> the NVMe private data (struct nvme_dev) and copy it from id->eui64 in
> nvme_blk_probe().
> 
> It would be great if you could also provide a patch adding the NVMe node
> to the device path to text protocol.
> 
> Unfortunately the patch was not addressed to me. You can use
> scripts/get_maintainer.pl to identify maintainers.
> 
> Best regards
> 
> Heinrich

Hi Heinrich,

that maintainer script is really useful, thank you!

I will follow up with a v2 that addresses your points in a second.

Best regards,
Patrick

> > 
> > Signed-off-by: Patrick Wildt <patr...@blueri.se>
> > 
> > diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> > index 561f757772..0a72fe2b75 100644
> > --- a/drivers/nvme/nvme.c
> > +++ b/drivers/nvme/nvme.c
> > @@ -627,6 +627,13 @@ static int nvme_get_info_from_identify(struct nvme_dev 
> > *dev)
> >     return 0;
> >   }
> > 
> > +u32
> > +nvme_get_namespace_id(struct udevice *udev)
> > +{
> > +   struct nvme_ns *ns = dev_get_priv(udev);
> > +   return ns->ns_id;
> > +}
> > +
> >   int nvme_scan_namespace(void)
> >   {
> >     struct uclass *uc;
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 37e56da460..0000b4ab1a 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -422,6 +422,7 @@ struct efi_device_path_acpi_path {
> >   #  define DEVICE_PATH_SUB_TYPE_MSG_USB            0x05
> >   #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR       0x0b
> >   #  define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS      0x0f
> > +#  define DEVICE_PATH_SUB_TYPE_MSG_NVME            0x17
> >   #  define DEVICE_PATH_SUB_TYPE_MSG_SD             0x1a
> >   #  define DEVICE_PATH_SUB_TYPE_MSG_MMC            0x1d
> > 
> > @@ -459,6 +460,12 @@ struct efi_device_path_usb_class {
> >     u8 device_protocol;
> >   } __packed;
> > 
> > +struct efi_device_path_nvme {
> > +   struct efi_device_path dp;
> > +   u32 nsid;
> > +   u64 eui64;
> > +} __packed;
> > +
> >   struct efi_device_path_sd_mmc_path {
> >     struct efi_device_path dp;
> >     u8 slot_number;
> > diff --git a/include/nvme.h b/include/nvme.h
> > index 2c3d14d241..95193c0334 100644
> > --- a/include/nvme.h
> > +++ b/include/nvme.h
> > @@ -78,4 +78,14 @@ int nvme_scan_namespace(void);
> >    */
> >   int nvme_print_info(struct udevice *udev);
> > 
> > +/**
> > + * nvme_get_namespace_id - return namespace identifier
> > + *
> > + * This returns the namespace identifier.
> > + *
> > + * @udev:  NVMe controller device
> > + * @return:        namespace identifier
> > + */
> > +u32 nvme_get_namespace_id(struct udevice *udev);
> > +
> >   #endif /* __NVME_H__ */
> > diff --git a/lib/efi_loader/efi_device_path.c 
> > b/lib/efi_loader/efi_device_path.c
> > index 86297bb7c1..89ad80c7bd 100644
> > --- a/lib/efi_loader/efi_device_path.c
> > +++ b/lib/efi_loader/efi_device_path.c
> > @@ -10,6 +10,7 @@
> >   #include <dm.h>
> >   #include <usb.h>
> >   #include <mmc.h>
> > +#include <nvme.h>
> >   #include <efi_loader.h>
> >   #include <part.h>
> >   #include <sandboxblockdev.h>
> > @@ -451,6 +452,11 @@ static unsigned dp_size(struct udevice *dev)
> >                     return dp_size(dev->parent) +
> >                             sizeof(struct efi_device_path_sd_mmc_path);
> >   #endif
> > +#if defined(CONFIG_NVME)
> > +           case UCLASS_NVME:
> > +                   return dp_size(dev->parent) +
> > +                           sizeof(struct efi_device_path_nvme);
> > +#endif
> >   #ifdef CONFIG_SANDBOX
> >             case UCLASS_ROOT:
> >                      /*
> > @@ -583,6 +589,19 @@ static void *dp_fill(void *buf, struct udevice *dev)
> >                     sddp->slot_number = dev->seq;
> >                     return &sddp[1];
> >                     }
> > +#endif
> > +#if defined(CONFIG_NVME)
> > +           case UCLASS_NVME: {
> > +                   struct efi_device_path_nvme *dp =
> > +                           dp_fill(buf, dev->parent);
> > +
> > +                   dp->dp.type     = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> > +                   dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_NVME;
> > +                   dp->dp.length   = sizeof(*dp);
> > +                   dp->nsid        = nvme_get_namespace_id(dev);
> > +                   dp->eui64       = 0;
> > +                   return &dp[1];
> > +                   }
> >   #endif
> >             default:
> >                     debug("%s(%u) %s: unhandled parent class: %s (%u)\n",
> > 
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to