Hi Heinrich, On Mon, 27 Mar 2023 at 22:31, Heinrich Schuchardt <[email protected]> wrote: > > On 3/27/23 10:24, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 27 Mar 2023 at 19:13, Heinrich Schuchardt > > <[email protected]> wrote: > >> > >> > >> > >> On 3/27/23 06:00, Simon Glass wrote: > >>> Hi Heinrich, > >>> > >>> On Mon, 27 Mar 2023 at 06:27, Heinrich Schuchardt > >>> <[email protected]> wrote: > >>>> > >>>> Generate a Ctrl() node for block devices. > >>>> > >>>> Signed-off-by: Heinrich Schuchardt <[email protected]> > >>>> --- > >>>> drivers/block/blk-uclass.c | 56 ++++++++++++++++++++++++++++++++++++++ > >>>> 1 file changed, 56 insertions(+) > >>> > >>> Can this go in drivers/block/blk_efi.c ? > >>> > >>> Can you create a function which returns the path given a device? Then > >> > >> That function already exists in lib/efi_loader/device_path. > > > > Yes, but I mean one for each media type. > > > >> > >>> we are discuss the get_dp_node() member or moving to an event. I > >>> favour the event for now. > >> > >> If for a device we already have an EFI handle with a device-path > >> installed, the device-path of its children has to be constructed by > >> adding a uclass specific device path node. > >> > >> Otherwise we have to recurse to the root node to collect all device path > >> nodes. > > > > OK > > > >> > >>> > >>>> > >>>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c > >>>> index c69fc4d518..08202aaaba 100644 > >>>> --- a/drivers/block/blk-uclass.c > >>>> +++ b/drivers/block/blk-uclass.c > >>>> @@ -9,6 +9,7 @@ > >>>> #include <common.h> > >>>> #include <blk.h> > >>>> #include <dm.h> > >>>> +#include <efi_loader.h> > >>>> #include <log.h> > >>>> #include <malloc.h> > >>>> #include <part.h> > >>>> @@ -779,9 +780,64 @@ static int blk_post_probe(struct udevice *dev) > >>>> return 0; > >>>> } > >>>> > >>>> +#if CONFIG_IS_ENABLED(EFI_LOADER) > >>>> +__maybe_unused > >>>> +static struct efi_device_path *blk_scsi_get_dp_node(struct udevice *dev) > >>>> +{ > >>>> + struct efi_device_path_scsi *dp; > >>>> + struct blk_desc *desc = dev_get_uclass_plat(dev); > >>>> + > >>>> + dp = efi_alloc(sizeof(struct efi_device_path_scsi)); > >>>> + if (!dp) > >>>> + return NULL; > >>>> + > >>>> + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; > >>>> + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI; > >>>> + dp->dp.length = sizeof(*dp); > >>>> + dp->logical_unit_number = desc->lun; > >>>> + dp->target_id = desc->target; > >>>> + > >>>> + return &dp->dp; > >>>> +} > >>>> + > >>>> +static struct efi_device_path *blk_get_dp_node(struct udevice *dev) > >>>> +{ > >>>> + struct efi_device_path_controller *dp; > >>>> + struct blk_desc *desc = dev_get_uclass_plat(dev); > >>>> + u32 controller_number; > >>>> + > >>>> + switch (device_get_uclass_id(dev->parent)) { > >>>> +#if CONFIG_IS_ENABLED(SCSI) > >>>> + case UCLASS_SCSI: > >>>> + return blk_scsi_get_dp_node(dev); > >>>> +#endif > >>>> + case UCLASS_MMC: > >>>> + dp->controller_number = desc->hwpart; > >>>> + break; > >>>> + default: > >>> > >>> Since this is checking the parent class, it seems to me that this info > >>> should be attacked there (the 'media' device) instead of the block > >>> device. > >> > >> For MMC we have these layers: > >> > >> * MMC controller - UCLASS_MMC > >> * MMC slot (or channel) - missing uclass > >> * hardware partition - UCLASS_BLK > >> > >> See https://www.sage-micro.com/us/portfolio-items/s881/ > >> > >> Currently in the DM tree we don't model the MMC controller and the MMC > >> slot separately. I think this is a gap that should be closed. > >> > >> A single UCLASS_MMC (= MMC slot) device can have multiple UCLASS_BLK > >> children. One for each hardware partition. > >> > >> An MMC controller in EFI is modelled as VenHW() node, > >> a slot as SD() or eMMC() node. > >> > >> An eMMC() or SD() node has a field for the slot number but none for the > >> hardware partition. Hence, we cannot attach the hardware partition > >> information to the UCLASS_MMC device. For the UCLASS_BLK device I use a > >> Ctrl() node. We could also consider using a Unit() node but Unit() nodes > >> are meant to be used for LUNs. > >> > >> We have a similar situation for USB and SCSI LUNs. > > > > The current model is to add multiple BLK children for each media > > device, if needed. That seems to work OK for now. What do you think? > > There is nothing in DM tree we need to change now. As I understood your > first reply it was not clear to you which EFI device-path node type with > which information matches which DM tree node: > > MMC controller/MMC slot (UCLASS_MMC) -> SD(slot) or MMC(slot) > Hardware partition (UCLASS_BLK) -> Ctrl() > > SCSI drive (UCLASS_SCSI) -> VenHw() > SCSI LUN (UClASS_BLK) -> Scsi(PUN, LUN)
OK, so this is explaining is why you must attach the functions to the BLK device rather than the storage device? If so, I understand. What are VenHw and Ctrl ? Regrads, Simon
