On Wed, 15 Jul 2026 10:50:35 -0500, Mikail Sadic <[email protected]> wrote:
> Add an emulation of the AST2700 UFS host controller
> (aspeed,ufshc-m31-16nm) and wire it into the AST2700 SoC.
> Implement the functionality needed by U-Boot and Linux to enumerate the
> device and access a BlockBackend using the UFS protocol, allowing the
> AST2700 machine to boot from a UFS image.
> 

Hi Mikail,

This patch duplicates a large amount of logic that already exists in
hw/ufs/. aspeed_ufs.c reimplements the UFSHCI register interface,
UTRL and doorbell handling, UPIU processing, Query handling, and SCSI
command handling, much of which is already implemented in hw/ufs/ufs.c
and hw/ufs/lu.c. It also introduces register and UPIU/UTRD definitions
that overlap with definitions already present in include/block/ufs.h.

For these reasons, I don't think we should add a second independent UFS
implementation under hw/ufs/. This needs to be based on a shared UFS
core instead.

If the motivation for the separate implementation is that the existing
UFS device, TYPE_UFS, is a PCI device, the solution should be to
refactor the existing implementation rather than reimplement it. The
common core, including the register block, UTRL, UPIU, Query, and
ufs-lu command handling, should be separated from the PCI-specific
glue so that both PCI and sysbus frontends can be built on top of it.

The PCI coupling in ufs.c appears to be relatively thin: DMA access,
interrupt handling, and PCI realization and BAR setup. QEMU already has
well-established examples of this structure, such as SDHCI, with a
common core and PCI/sysbus frontends, and XHCI, with hcd-xhci.c shared
by its PCI and sysbus wrappers.

The ASPEED controller should then be implemented as a sysbus frontend
on top of the shared UFS core, with only the ASPEED-specific integration
kept in the frontend.

Thanks,

-- 
Jeuk Kim <[email protected]>

Reply via email to