[U-Boot] [PATCH 05/10] ufs: Add Initial Support for UFS subsystem

From: Vignesh Raghavendra <vigneshr@ti.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 05/10] ufs: Add Initial Support for UFS subsystem
Date: Tue, 10 Sep 2019 09:18:00 +0530	[thread overview]
Message-ID: <2ce49313-cbc8-89b9-6ee3-5153474fc4c1@ti.com> (raw)
In-Reply-To: <20190909081959.16173-6-faiz_abbas@ti.com>

On 09/09/19 1:49 PM, Faiz Abbas wrote:
> Add Support for UFS Host Controller Interface (UFSHCI) for communicating
> with Universal Flash Storage (UFS) devices. The steps to initialize the
> host controller interface are the following:
> 
> - Initiate the Host Controller Initialization process by writing to the
> Host controller enable register.
> - Configure the Host Controller base address registers by allocating a
> host memory space and related data structures.
> - Unipro link startup procedure
> - Check for connected device
> - Configure UFS host controller to process requests
> 

I am guessing code is derived from Linux kernel? Could you add kernel
version from which this code is borrowed from?

> Also register this host controller as a SCSI host controller.
> 
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
>  MAINTAINERS              |    5 +
>  drivers/Kconfig          |    2 +
>  drivers/Makefile         |    1 +
>  drivers/ufs/Kconfig      |    9 +
>  drivers/ufs/Makefile     |    6 +
>  drivers/ufs/ufs-uclass.c |   16 +
>  drivers/ufs/ufs.c        | 1973 ++++++++++++++++++++++++++++++++++++++
>  drivers/ufs/ufs.h        |  918 ++++++++++++++++++
>  drivers/ufs/unipro.h     |  270 ++++++

Should UFS reside under SCSI framework given that UFS follows SAM?
driver/scsi/ufs?

Regards
Vignesh

>  include/dm/uclass-id.h   |    1 +
>  include/ufs.h            |    7 +
>  11 files changed, 3208 insertions(+)
>  create mode 100644 drivers/ufs/Kconfig
>  create mode 100644 drivers/ufs/Makefile
>  create mode 100644 drivers/ufs/ufs-uclass.c
>  create mode 100644 drivers/ufs/ufs.c
>  create mode 100644 drivers/ufs/ufs.h
>  create mode 100644 drivers/ufs/unipro.h
>  create mode 100644 include/ufs.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36625795a4..ed3a4c352c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -772,6 +772,11 @@ S:	Maintained
>  T:	git git://git.denx.de/u-boot-ubi.git
>  F:	drivers/mtd/ubi/
>  
> +UFS
> +M:	Faiz Abbas <faiz_abbas@ti.com>
> +S:	Maintained
> +F:	drivers/ufs/
> +
>  USB
>  M:	Marek Vasut <marex@denx.de>
>  S:	Maintained
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 96ff4f566a..61bbe88d6c 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -118,6 +118,8 @@ source "drivers/tpm/Kconfig"
>  
>  source "drivers/usb/Kconfig"
>  
> +source "drivers/ufs/Kconfig"
> +
>  source "drivers/video/Kconfig"
>  
>  source "drivers/virtio/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 6635dabd2c..2794bef18a 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -111,6 +111,7 @@ obj-y += soc/
>  obj-y += thermal/
>  obj-$(CONFIG_TEE) += tee/
>  obj-y += axi/
> +obj-y += ufs/
>  obj-$(CONFIG_W1) += w1/
>  obj-$(CONFIG_W1_EEPROM) += w1-eeprom/
>  
> diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
> new file mode 100644
> index 0000000000..538aad8cd9
> --- /dev/null
> +++ b/drivers/ufs/Kconfig
> @@ -0,0 +1,9 @@
> +menu "UFS Host Controller Support"
> +
> +config UFS
> +	bool "Support UFS controllers"
> +	select DM_SCSI

DM_SCSI has further dependencies, so its preferred not to use select

	depends on DM_SCSI

Also if this is moved under drivers/scsi/ then this Kconfig file can be
sourced conditionally

> +	help
> +	  This selects support for Universal Flash Subsystem (UFS).
> +	  Say Y here if you want UFS Support.
> +endmenu
> diff --git a/drivers/ufs/Makefile b/drivers/ufs/Makefile
> new file mode 100644
> index 0000000000..b8df759f66
> --- /dev/null
> +++ b/drivers/ufs/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
> +#
> +
> +obj-$(CONFIG_UFS) += ufs.o ufs-uclass.o
> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> new file mode 100644
> index 0000000000..920bfa64e1
> --- /dev/null
> +++ b/drivers/ufs/ufs-uclass.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * ufs-uclass.c - Universal Flash Subsystem (UFS) Uclass driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
> + */
> +
> +#include <common.h>
> +#include "ufs.h"
> +#include <dm.h>
> +
> +UCLASS_DRIVER(ufs) = {
> +	.id	= UCLASS_UFS,
> +	.name	= "ufs",
> +	.per_device_auto_alloc_size = sizeof(struct ufs_hba),
> +};

[...]

> +#endif /* _UNIPRO_H_ */
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 09e0ad5391..a606b8c41b 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -96,6 +96,7 @@ enum uclass_id {
>  	UCLASS_THERMAL,		/* Thermal sensor */
>  	UCLASS_TIMER,		/* Timer device */
>  	UCLASS_TPM,		/* Trusted Platform Module TIS interface */
> +	UCLASS_UFS,		/* Universale Flash Storage */

s/Universale/Universal

>  	UCLASS_USB,		/* USB bus */
>  	UCLASS_USB_DEV_GENERIC,	/* USB generic device */
>  	UCLASS_USB_HUB,		/* USB hub */
> diff --git a/include/ufs.h b/include/ufs.h
> new file mode 100644
> index 0000000000..2245838b3c
> --- /dev/null
> +++ b/include/ufs.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef _UFS_H
> +#define _UFS_H
> +
> +int ufs_probe(void);

This is fine, but you may want to provide helper to initialize a single
instance of controller so that we don't end up in a situation like MMC
where all the instances are initialized always.
Probably modify ufs_probe() to take an int arg that indicates instance
to be probed and -1 to probe all instances.
Irrespective of this, please add API documentation before the function
declaration

> +int ufs_scsi_bind(struct udevice *scsi_dev, struct udevice **devp);

Please document the API

> +#endif
> 

-- 
Regards
Vignesh