Re: [U-Boot] [PATCH 07/10] ufs: Add glue layer driver for TI J721E devices
Vignesh,
On 10/09/19 9:18 AM, Vignesh Raghavendra wrote:
>
>
> On 09/09/19 1:49 PM, Faiz Abbas wrote:
>> Add glue layer driver for the controller present on TI's J721E devices.
>>
>> Signed-off-by: Faiz Abbas
>> ---
>> drivers/ufs/Kconfig| 6 +++
>> drivers/ufs/Makefile | 1 +
>> drivers/ufs/ti-j721e-ufs.c | 75 ++
>> 3 files changed, 82 insertions(+)
>> create mode 100644 drivers/ufs/ti-j721e-ufs.c
>>
>> diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
>> index a320b4561f..4875e9448e 100644
>> --- a/drivers/ufs/Kconfig
>> +++ b/drivers/ufs/Kconfig
>> @@ -14,4 +14,10 @@ config CADENCE_UFS
>>This selects the platform driver for the Cadence UFS host
>>controller present on present TI's J721e devices.
>>
>> +config TI_J721E_UFS
>> +bool "Glue Layer driver for UFS on TI J721E devices"
>> +help
>> + This selects the glue layer driver for Cadence controller
>> + present on TI's J721E devices.
>> +
>> endmenu
>> diff --git a/drivers/ufs/Makefile b/drivers/ufs/Makefile
>> index 9262bd6cd0..62ed016608 100644
>> --- a/drivers/ufs/Makefile
>> +++ b/drivers/ufs/Makefile
>> @@ -5,3 +5,4 @@
>>
>> obj-$(CONFIG_UFS) += ufs.o ufs-uclass.o
>> obj-$(CONFIG_CADENCE_UFS) += cdns-platform.o
>> +obj-$(CONFIG_TI_J721E_UFS) += ti-j721e-ufs.o
>> diff --git a/drivers/ufs/ti-j721e-ufs.c b/drivers/ufs/ti-j721e-ufs.c
>> new file mode 100644
>> index 00..249d3796c0
>> --- /dev/null
>> +++ b/drivers/ufs/ti-j721e-ufs.c
>> @@ -0,0 +1,75 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>
> #include
>
> should be sufficient instead of two includes.
Replaced it.
>
>> +#include
>> +#include
>> +
>> +#define UFS_SS_CTRL 0x4
>> +#define UFS_SS_RST_N_PCSBIT(0)
>> +#define UFS_SS_CLK_26MHZBIT(4)
>> +
>> +static int ti_j721e_ufs_probe(struct udevice *dev)
>> +{
>> +struct power_domain ufs_pwrdmn;
>> +struct regmap *map;
>> +unsigned int clock;
>> +struct clk clk;
>> +u32 reg = 0;
>> +int ret;
>> +
>> +ret = power_domain_get(dev, _pwrdmn);
>> +if (ret) {
>> +dev_err(dev, "failed to get power domain %d\n", ret);
>> +return ret;
>> +}
>> +
>> +ret = power_domain_on(_pwrdmn);
>> +if (ret) {
>> +dev_err(dev, "Power domain on failed\n");
>> +return ret;
>> +}
>
> Core takes care of enabling power domain before calling probe. So this
> can be dropped
Ok.
>
>> +
>> +ret = regmap_init_mem(dev_ofnode(dev), );
>> +if (ret)
>> +return ret;
>> +
>
>
> I think regmap is an overkill here.. We are just accessing a single
> register and within dedicated wrapper space. Just use regular IO accessors.
Ok. Going back to writel/readl calls.
>
>
>
>> +ret = clk_get_by_index(dev, 0, );
>> +if (ret) {
>> +dev_err(dev, "failed to get M-PHY clock\n");
>> +return ret;
>> +}
>> +
>> +clock = clk_get_rate();
>> +if (IS_ERR_VALUE(clock)) {
>> +dev_err(dev, "failed to get rate\n");
>> +return ret;
>> +}
>> +
>> +if (clock == 2600)
>> +reg |= UFS_SS_CLK_26MHZ;
>> +/* Take UFS slave device out of reset */
>> +reg |= UFS_SS_RST_N_PCS;
>> +regmap_write(map, UFS_SS_CTRL, reg);
>> +
>
> To be safe, put the slave back to reset before jumping to kernel so that
> card does not see any glitches when kernel is doing any
> re-initialization of clocks etc.
> This can be done in driver remove callback (and needs DM_FLAG_OS_PREPARE
> flag to be set in driver declaration)
Implemented this for v2. Thanks.
Regards,
Faiz
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 07/10] ufs: Add glue layer driver for TI J721E devices
On 09/09/19 1:49 PM, Faiz Abbas wrote:
> Add glue layer driver for the controller present on TI's J721E devices.
>
> Signed-off-by: Faiz Abbas
> ---
> drivers/ufs/Kconfig| 6 +++
> drivers/ufs/Makefile | 1 +
> drivers/ufs/ti-j721e-ufs.c | 75 ++
> 3 files changed, 82 insertions(+)
> create mode 100644 drivers/ufs/ti-j721e-ufs.c
>
> diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
> index a320b4561f..4875e9448e 100644
> --- a/drivers/ufs/Kconfig
> +++ b/drivers/ufs/Kconfig
> @@ -14,4 +14,10 @@ config CADENCE_UFS
> This selects the platform driver for the Cadence UFS host
> controller present on present TI's J721e devices.
>
> +config TI_J721E_UFS
> + bool "Glue Layer driver for UFS on TI J721E devices"
> + help
> + This selects the glue layer driver for Cadence controller
> + present on TI's J721E devices.
> +
> endmenu
> diff --git a/drivers/ufs/Makefile b/drivers/ufs/Makefile
> index 9262bd6cd0..62ed016608 100644
> --- a/drivers/ufs/Makefile
> +++ b/drivers/ufs/Makefile
> @@ -5,3 +5,4 @@
>
> obj-$(CONFIG_UFS) += ufs.o ufs-uclass.o
> obj-$(CONFIG_CADENCE_UFS) += cdns-platform.o
> +obj-$(CONFIG_TI_J721E_UFS) += ti-j721e-ufs.o
> diff --git a/drivers/ufs/ti-j721e-ufs.c b/drivers/ufs/ti-j721e-ufs.c
> new file mode 100644
> index 00..249d3796c0
> --- /dev/null
> +++ b/drivers/ufs/ti-j721e-ufs.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#include
> +#include
> +#include
> +#include
#include
should be sufficient instead of two includes.
> +#include
> +#include
> +
> +#define UFS_SS_CTRL 0x4
> +#define UFS_SS_RST_N_PCSBIT(0)
> +#define UFS_SS_CLK_26MHZBIT(4)
> +
> +static int ti_j721e_ufs_probe(struct udevice *dev)
> +{
> + struct power_domain ufs_pwrdmn;
> + struct regmap *map;
> + unsigned int clock;
> + struct clk clk;
> + u32 reg = 0;
> + int ret;
> +
> + ret = power_domain_get(dev, _pwrdmn);
> + if (ret) {
> + dev_err(dev, "failed to get power domain %d\n", ret);
> + return ret;
> + }
> +
> + ret = power_domain_on(_pwrdmn);
> + if (ret) {
> + dev_err(dev, "Power domain on failed\n");
> + return ret;
> + }
Core takes care of enabling power domain before calling probe. So this
can be dropped
> +
> + ret = regmap_init_mem(dev_ofnode(dev), );
> + if (ret)
> + return ret;
> +
I think regmap is an overkill here.. We are just accessing a single
register and within dedicated wrapper space. Just use regular IO accessors.
> + ret = clk_get_by_index(dev, 0, );
> + if (ret) {
> + dev_err(dev, "failed to get M-PHY clock\n");
> + return ret;
> + }
> +
> + clock = clk_get_rate();
> + if (IS_ERR_VALUE(clock)) {
> + dev_err(dev, "failed to get rate\n");
> + return ret;
> + }
> +
> + if (clock == 2600)
> + reg |= UFS_SS_CLK_26MHZ;
> + /* Take UFS slave device out of reset */
> + reg |= UFS_SS_RST_N_PCS;
> + regmap_write(map, UFS_SS_CTRL, reg);
> +
To be safe, put the slave back to reset before jumping to kernel so that
card does not see any glitches when kernel is doing any
re-initialization of clocks etc.
This can be done in driver remove callback (and needs DM_FLAG_OS_PREPARE
flag to be set in driver declaration)
> + return dm_scan_fdt_dev(dev);
> +}
> +
> +static const struct udevice_id ti_j721e_ufs_ids[] = {
> + {
> + .compatible = "ti,j721e-ufs",
> + },
> + {},
> +};
> +
> +U_BOOT_DRIVER(ti_j721e_ufs) = {
> + .name = "ti-j721e-ufs",
> + .id = UCLASS_UFS,
> + .of_match = ti_j721e_ufs_ids,
> + .probe = ti_j721e_ufs_probe,
> +};
>
--
Regards
Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
