On Thu, Jan 10, 2019 at 08:30:13AM +0100, Alexander Graf wrote: > > > On 10.01.19 08:26, AKASHI Takahiro wrote: > > Alex, > > > > On Thu, Jan 10, 2019 at 07:21:12AM +0100, Alexander Graf wrote: > >> > >> > >> On 10.01.19 03:13, AKASHI Takahiro wrote: > >>> Alex, > >>> > >>> On Wed, Jan 09, 2019 at 10:06:16AM +0100, Alexander Graf wrote: > >>>> > >>>> > >>>> On 13.12.18 08:58, AKASHI Takahiro wrote: > >>>>> Heinrich, > >>>>> > >>>>> On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote: > >>>>>> On 11/15/18 5:58 AM, AKASHI Takahiro wrote: > >>>>>>> Currently, efi_init_obj_list() scan disk devices only once, and never > >>>>>>> change a list of efi disk devices. This will possibly result in > >>>>>>> failing > >>>>>>> to find a removable storage which may be added later on. See [1]. > >>>>>>> > >>>>>>> In this patch, called is efi_disk_update() which is responsible for > >>>>>>> re-scanning UCLASS_BLK devices and removing/adding efi disks if > >>>>>>> necessary. > >>>>>>> > >>>>>>> For example, > >>>>>>> > >>>>>>> => efishell devices > >>>>>>> Scanning disk pci_mmc.blk... > >>>>>>> Found 3 disks > >>>>>>> Device Name > >>>>>>> ============================================ > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>>>>>> => usb start > >>>>>>> starting USB... > >>>>>>> USB0: USB EHCI 1.00 > >>>>>>> scanning bus 0 for devices... 3 USB Device(s) found > >>>>>>> scanning usb for storage devices... 1 Storage Device(s) found > >>>>>>> => efishell devices > >>>>>>> Scanning disk usb_mass_storage.lun0... > >>>>>>> Device Name > >>>>>>> ============================================ > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800) > >>>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c) > >>>>>>> > >>>>>>> Without this patch, the last device, USB mass storage, won't show up. > >>>>>>> > >>>>>>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >>>>>>> > >>>>>>> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > >>>>>> > >>>>>> Why should we try to fix something in the EFI subsystems that goes > >>>>>> wrong > >>>>>> in the handling of device enumeration. > >>>>> > >>>>> No. > >>>>> This is a natural result from how efi disks are currently implemented > >>>>> on u-boot. > >>>>> Do you want to totally re-write/re-implement efi disks? > >>>> > >>>> Could we just make this event based for now? Call a hook from the > >>>> storage dm subsystem when a new u-boot block device gets created to > >>>> issue a sync of that in the efi subsystem? > >>> > >>> If I correctly understand you, your suggestion here corresponds > >>> with my proposal#3 in [1] while my current approach is #2. > >>> > >>> [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html > >> > >> Yes, I think so. > >> > >>> So we will call, say, efi_disk_create(struct udevice *) in > >>> blk_create_device() and efi_dsik_delete() in blk_unbind_all(). > >> > >> I would prefer if we didn't call them directly, but through an event > >> mechanism. So the efi_disk subsystem registers an event with the dm > >> block subsystem and that will just call all events when block devices > >> get created which will automatically also include the efi disk creation > >> callback. Same for reverse. > > > > Do you mean efi event by "event?" > > (I don't think there is any generic event interface on DM side.) > > > > Whatever an "event" is or whether we call efi_disk_create() directly > > or indirectly via an event, there is one (big?) issue in this approach > > (while I've almost finished prototyping): > > > > We cannot call efi_disk_create() within blk_create_device() because > > some data fields of struct blk_desc, which are to be used by efi disk, > > are initialized *after* blk_create_device() in driver side. > > > > So we need to add a hook at/after every occurrence of blk_create_device() > > on driver side. For example, > > > > === drivers/scsi/scsi.c === > > int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose) > > { > > ... > > ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1, > > bd.blksz, bd.lba, &bdev); > > ... > > bdesc = dev_get_uclass_platdata(bdev); > > bdesc->target = id; > > bdesc->lun = lun; > > ... > > > > /* > > * We need have efi_disk_create() called here because bdesc->target > > * and lun will be used by dp helpers in efi_disk_add_dev(). > > */ > > efi_disk_create(bdev); > > } > > > > int scsi_scan_dev(struct udevice *dev, bool verbose) > > { > > for (i = 0; i < uc_plat->max_id; i++) > > for (lun = 0; lun < uc_plat->max_lun; lun++) > > do_scsi_scan_one(dev, i, lun, verbose); > > ... > > } > > > > int scsi_scan(bool verbose) > > { > > ret = uclass_get(UCLASS_SCSI, &uc); > > ... > > uclass_foreach_dev(dev, uc) > > ret = scsi_scan_dev(dev, verbose); > > ... > > } > > === === > > > > Since scsn_scan() can be directly called by "scsi rescan" command, > > There seems to be no generic hook, or event, available in order to > > call efi_disk_create(). > > > > Do I miss anything? > > Could the event handler that gets called from somewhere around > blk_create_device() just put it into an efi internal "todo list" which > we then process using an efi event? > > EFI events will only get triggered on the next entry to efi land, so by > then we should be safe.
I think I now understand your suggestion; we are going to invent a specialized event-queuing mechanism so that we can take any actions later at appropriate time (probably in efi_init_obj_list()?). But if so, it's not much different from my current approach where a list of efi disks are updated in efi_init_obj_list() :) -Takahiro Akashi > > Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
