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

Faiz Abbas faiz_abbas at ti.com
Wed Sep 11 08:06:28 UTC 2019


Hi Vignesh,

On 10/09/19 9:18 AM, Vignesh Raghavendra wrote:
> 
> 
> 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?

I did write that in the ufs.c file description. Will also add it here.

> 
>> Also register this host controller as a SCSI host controller.
>>
>> Signed-off-by: Faiz Abbas <faiz_abbas at 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?

Other scsi implementations (like ahci) have their own folders so I'm
just following that convention.

> 
> 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 at ti.com>
>> +S:	Maintained
>> +F:	drivers/ufs/
>> +
>>  USB
>>  M:	Marek Vasut <marex at 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.

Makes sense. Will modify the API.

> 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

Ok.

Thanks,
Faiz


More information about the U-Boot mailing list