Re: [PATCH v5 2/3] scsi: ufs: spacemit: k3: Add UFS Host Controller driver

[Yixun Lan <dlan@kernel.org>]

Hi Aurelien, 

On 18:26 Wed 16 Sep     , Aurelien Jarno wrote:
> Hi Yixun,
> 
> On 2026-09-16 02:59, Yixun Lan wrote:
> > SpacemiT K3 SoC consist of UFS (Universal Flash Storage) Host Controller
> > which has features compatible with JEDEC UFS 2.2, MIPI UniPro v1.61 and
> > M-PHY v3.0 standard.
> > 
> > Tested-by: Aurelien Jarno <aurelien@aurel32.net>
> > Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
> > Signed-off-by: Yixun Lan <dlan@kernel.org>
> > ---
> >  drivers/ufs/host/Kconfig        |  12 +
> >  drivers/ufs/host/Makefile       |   1 +
> >  drivers/ufs/host/ufs-spacemit.c | 688 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/ufs/host/ufs-spacemit.h |  79 +++++
> >  4 files changed, 780 insertions(+)
> > 
> > diff --git a/drivers/ufs/host/Kconfig b/drivers/ufs/host/Kconfig
> > index ff170c0b6da0..6146872fcecc 100644
> > --- a/drivers/ufs/host/Kconfig
> > +++ b/drivers/ufs/host/Kconfig
> > @@ -132,6 +132,18 @@ config SCSI_UFS_VARIABLE_SG_ENTRY_SIZE
> >  	bool
> >  	default y if SCSI_UFS_EXYNOS && SCSI_UFS_CRYPTO
> >  
> > +config SCSI_UFS_SPACEMIT
> > +	tristate "SpacemiT UFS controller driver"
> > +	depends on SCSI_UFSHCD_PLATFORM && (ARCH_SPACEMIT || COMPILE_TEST)
> > +	help
> > +	  This selects the SpacemiT specific additions to UFSHCD platform driver.
> > +	  UFS host on SpacemiT needs some vendor specific configuration before
> > +	  accessing the hardware which includes PHY configuration and vendor
> > +	  specific registers.
> > +
> > +	  Select this if you have UFS controller on SpacemiT chipset.
> > +	  If unsure, say N.
> > +
> >  config SCSI_UFS_SPRD
> >  	tristate "Unisoc specific hooks to UFS controller platform driver"
> >  	depends on SCSI_UFSHCD_PLATFORM && (ARCH_SPRD || COMPILE_TEST)
> > diff --git a/drivers/ufs/host/Makefile b/drivers/ufs/host/Makefile
> > index 7d8db67eb23c..a3c447b8fc21 100644
> > --- a/drivers/ufs/host/Makefile
> > +++ b/drivers/ufs/host/Makefile
> > @@ -13,6 +13,7 @@ obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
> >  obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
> >  obj-$(CONFIG_SCSI_UFS_RENESAS) += ufs-renesas.o
> >  obj-$(CONFIG_SCSI_UFS_ROCKCHIP) += ufs-rockchip.o
> > +obj-$(CONFIG_SCSI_UFS_SPACEMIT) += ufs-spacemit.o
> >  obj-$(CONFIG_SCSI_UFS_SPRD) += ufs-sprd.o
> >  obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
> >  obj-$(CONFIG_SCSI_UFS_AMD_VERSAL2) += ufs-amd-versal2.o ufshcd-dwc.o
> > diff --git a/drivers/ufs/host/ufs-spacemit.c b/drivers/ufs/host/ufs-spacemit.c
> > new file mode 100644
> > index 000000000000..179e6edb2ac2
> > --- /dev/null
> > +++ b/drivers/ufs/host/ufs-spacemit.c
> > @@ -0,0 +1,688 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2026 SpacemiT (Hangzhou) Technology Co. Ltd
> > + */
> > +
> > +
...
> > +/*
> > + * Keep UFS ACLK at a lower parent rate (409.6MHz) for stable init.
> > + * This mirrors the "ufs-low-aclk-freq" change from the other environment.
> > + */
> > +#define UFS_ACLK_LOW_FREQ_HZ		409600000UL
> 
> You probably want to rename that to AXI, although here it's not that 
> important.
> 
Ok, will do

...
> > +static u32 ufs_spacemit_get_sys1clk_1us(struct ufs_hba *hba)
> > +{
> > +	struct ufs_clk_info *clki, *ufs_aclk = NULL;
> > +	struct list_head *head = &hba->clk_list_head;
> > +	unsigned long rate_hz = 0;
> > +
> > +	if (!list_empty(head)) {
> > +		list_for_each_entry(clki, head, list) {
> > +			if (clki->name && !strcmp(clki->name, "aclk") && clki->clk) {
> 
> On the other hand, you definitely want to rename this one to "axi" to 
> match the device tree change. Right now the whole patchset just doesn't 
> work.
> 
Oops, you right, will fix it

-- 
Yixun Lan (dlan)