Hi Simon,

On Thu, May 19, 2022 at 02:11:02PM +0900, AKASHI Takahiro wrote:
> Hi Simon,
> 
> On Mon, May 09, 2022 at 02:33:10PM +0900, AKASHI Takahiro wrote:
> > Simon,
> > 
> > On Thu, Apr 28, 2022 at 12:43:03PM +0200, Heinrich Schuchardt wrote:
> > > On 4/28/22 06:49, AKASHI Takahiro wrote:
> > > > While GPT partition is mandated in UEFI specification, CONFIG_PARTITION 
> > > > is
> > > > seen optional under the current implementation.
> > > > So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
> > > > 
> > > > Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of 
> > > > blk_desc")
> > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> > > > ---
> > > >   lib/efi_loader/efi_disk.c | 29 +++++++++++++++--------------
> > > >   1 file changed, 15 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > > > index 8fb5b2363c45..f5b462fb164a 100644
> > > > --- a/lib/efi_loader/efi_disk.c
> > > > +++ b/lib/efi_loader/efi_disk.c
> > > > @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct 
> > > > efi_block_io *this,
> > > >         if (buffer_size & (blksz - 1))
> > > >                 return EFI_BAD_BUFFER_SIZE;
> > > > 
> > > > -#if CONFIG_IS_ENABLED(PARTITIONS)
> > > > -       if (direction == EFI_DISK_READ)
> > > > -               n = dev_read(diskobj->dev, lba, blocks, buffer);
> > > > -       else
> > > > -               n = dev_write(diskobj->dev, lba, blocks, buffer);
> > > > -#else
> > > > -       /* dev is always a block device (UCLASS_BLK) */
> > > > -       struct blk_desc *desc;
> > > > +       if (CONFIG_IS_ENABLED(PARTITIONS) &&
> > > > +           device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {
> > > > +               if (direction == EFI_DISK_READ)
> > > > +                       n = dev_read(diskobj->dev, lba, blocks, buffer);
> > > > +               else
> > > > +                       n = dev_write(diskobj->dev, lba, blocks, 
> > > > buffer);
> > > 
> > > Thanks for the patch. It solves the problem with the block IO protocol.
> > > 
> > > Why should dev_read() and dev_write only work for UCLASS_PARTITION?
> > 
> > I don't remember well, but I have expected that even a whole disk be
> > represented as a partition (0).
> > 
> > > Can't we make it work for any block device? I think the treatment of
> > > different types of block devices should be moved to disk/disk-uclass.c.
> > 
> > @Simon, what are your thoughts?
> 
> Gentle ping. Any comment here?

Ping again.
I hope that you have some opinion here as the maintainer of DM.

-Takahiro Akashi

> -Takahiro Akashi
> 
> > 
> > -Takahiro Akashi
> > 
> > > I will pull this patch as a fast fix. But this should not be the final
> > > design.
> > > 
> > > Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de>
> > > 
> > > > +       } else {
> > > > +               /* dev is a block device (UCLASS_BLK) */
> > > > +               struct blk_desc *desc;
> > > > 
> > > > -       desc = dev_get_uclass_plat(diskobj->dev);
> > > > -       if (direction == EFI_DISK_READ)
> > > > -               n = blk_dread(desc, lba, blocks, buffer);
> > > > -       else
> > > > -               n = blk_dwrite(desc, lba, blocks, buffer);
> > > > -#endif
> > > > +               desc = dev_get_uclass_plat(diskobj->dev);
> > > > +               if (direction == EFI_DISK_READ)
> > > > +                       n = blk_dread(desc, lba, blocks, buffer);
> > > > +               else
> > > > +                       n = blk_dwrite(desc, lba, blocks, buffer);
> > > > +       }
> > > > 
> > > >         /* We don't do interrupts, so check for timers cooperatively */
> > > >         efi_timer_check();
> > > 

Reply via email to