[PATCH 1/3] ufs: add support for DesignWare Controller
neil.armstrong at linaro.org
neil.armstrong at linaro.org
Thu Sep 19 17:44:02 CEST 2024
Hi,
On 19/09/2024 11:17, Venkatesh Yadav Abbarapu wrote:
> This patch has the goal to add support for DesignWare UFS Controller
> specific operations.
>
> This is based on linux kernel commit:
> "drivers/scsi/ufs/ufshcd-dwc.c: ufs: add support for DesignWare
> Controller" (sha1: 4b9ffb5a353bdee49f1f477ffe2b95ab3f9cbc0c)
> It is ported from linux kernel 6.11-rc1.
>
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
> ---
> drivers/ufs/ufs.h | 11 +++
> drivers/ufs/ufshcd-dwc.c | 141 +++++++++++++++++++++++++++++++++++++++
> drivers/ufs/ufshcd-dwc.h | 23 +++++++
> drivers/ufs/ufshci-dwc.h | 32 +++++++++
> 4 files changed, 207 insertions(+)
> create mode 100644 drivers/ufs/ufshcd-dwc.c
> create mode 100644 drivers/ufs/ufshcd-dwc.h
> create mode 100644 drivers/ufs/ufshci-dwc.h
>
> diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
> index e8a1441156..83837d5817 100644
> --- a/drivers/ufs/ufs.h
> +++ b/drivers/ufs/ufs.h
> @@ -850,6 +850,7 @@ struct ufs_hba {
> struct ufs_pwr_mode_info max_pwr_info;
>
> struct ufs_dev_cmd dev_cmd;
> + enum uic_link_state uic_link_state;
Will this be really used ?
> };
>
> static inline int ufshcd_ops_init(struct ufs_hba *hba)
> @@ -878,6 +879,14 @@ static inline int ufshcd_ops_link_startup_notify(struct ufs_hba *hba,
> return 0;
> }
>
> +static inline int ufshcd_vops_phy_initialization(struct ufs_hba *hba)
> +{
> + if (hba->ops && hba->ops->phy_initialization)
> + return hba->ops->phy_initialization(hba);
> +
> + return 0;
> +}
There's no users of hba->ops->phy_initialization,
and the patch 2 doesn't even add an user, so I think you can
also drop this....
> +
> /* Controller UFSHCI version */
> enum {
> UFSHCI_VERSION_10 = 0x00010000, /* 1.0 */
> @@ -1022,6 +1031,8 @@ enum {
> writel((val), (hba)->mmio_base + (reg))
> #define ufshcd_readl(hba, reg) \
> readl((hba)->mmio_base + (reg))
> +#define ufshcd_set_link_active(hba) ((hba)->uic_link_state = \
> + UIC_LINK_ACTIVE_STATE)
Same here, you set as active, but there's no users of uic_link_state
I think you can drop this
>
> /**
> * ufshcd_rmwl - perform read/modify/write for a controller register
> diff --git a/drivers/ufs/ufshcd-dwc.c b/drivers/ufs/ufshcd-dwc.c
> new file mode 100644
> index 0000000000..db51a3b807
> --- /dev/null
> +++ b/drivers/ufs/ufshcd-dwc.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * UFS Host driver for Synopsys Designware Core
> + *
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + */
> +#include <clk.h>
> +#include <dm.h>
> +#include <ufs.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/time.h>
> +
> +#include "ufs.h"
> +#include "ufshci-dwc.h"
> +#include "ufshcd-dwc.h"
> +
> +int ufshcd_dwc_dme_set_attrs(struct ufs_hba *hba,
> + const struct ufshcd_dme_attr_val *v, int n)
> +{
> + int ret = 0;
> + int attr_node = 0;
> +
> + for (attr_node = 0; attr_node < n; attr_node++) {
> + ret = ufshcd_dme_set_attr(hba, v[attr_node].attr_sel,
> + ATTR_SET_NOR, v[attr_node].mib_val, v[attr_node].peer);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * ufshcd_dwc_program_clk_div() - program clock divider.
> + * @hba: Private Structure pointer
> + * @divider_val: clock divider value to be programmed
> + *
> + */
> +static void ufshcd_dwc_program_clk_div(struct ufs_hba *hba, u32 divider_val)
> +{
> + ufshcd_writel(hba, divider_val, DWC_UFS_REG_HCLKDIV);
> +}
> +
> +/**
> + * ufshcd_dwc_link_is_up() - check if link is up.
> + * @hba: private structure pointer
> + *
> + * Return: 0 on success, non-zero value on failure.
> + */
> +static int ufshcd_dwc_link_is_up(struct ufs_hba *hba)
> +{
> + int dme_result = 0;
> +
> + ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
> +
> + if (dme_result == UFSHCD_LINK_IS_UP) {
> + ufshcd_set_link_active(hba);
You can drop this, we don't track and use the link state
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +/**
> + * ufshcd_dwc_connection_setup() - configure unipro attributes.
> + * @hba: pointer to drivers private data
> + *
> + * This function configures both the local side (host) and the peer side
> + * (device) unipro attributes to establish the connection to application/
> + * cport.
> + * This function is not required if the hardware is properly configured to
> + * have this connection setup on reset. But invoking this function does no
> + * harm and should be fine even working with any ufs device.
> + *
> + * Return: 0 on success non-zero value on failure.
> + */
> +static int ufshcd_dwc_connection_setup(struct ufs_hba *hba)
> +{
> + static const struct ufshcd_dme_attr_val setup_attrs[] = {
> + { UIC_ARG_MIB(T_CONNECTIONSTATE), 0, DME_LOCAL },
> + { UIC_ARG_MIB(N_DEVICEID), 0, DME_LOCAL },
> + { UIC_ARG_MIB(N_DEVICEID_VALID), 0, DME_LOCAL },
> + { UIC_ARG_MIB(T_PEERDEVICEID), 1, DME_LOCAL },
> + { UIC_ARG_MIB(T_PEERCPORTID), 0, DME_LOCAL },
> + { UIC_ARG_MIB(T_TRAFFICCLASS), 0, DME_LOCAL },
> + { UIC_ARG_MIB(T_CPORTFLAGS), 0x6, DME_LOCAL },
> + { UIC_ARG_MIB(T_CPORTMODE), 1, DME_LOCAL },
> + { UIC_ARG_MIB(T_CONNECTIONSTATE), 1, DME_LOCAL },
> + { UIC_ARG_MIB(T_CONNECTIONSTATE), 0, DME_PEER },
> + { UIC_ARG_MIB(N_DEVICEID), 1, DME_PEER },
> + { UIC_ARG_MIB(N_DEVICEID_VALID), 1, DME_PEER },
> + { UIC_ARG_MIB(T_PEERDEVICEID), 1, DME_PEER },
> + { UIC_ARG_MIB(T_PEERCPORTID), 0, DME_PEER },
> + { UIC_ARG_MIB(T_TRAFFICCLASS), 0, DME_PEER },
> + { UIC_ARG_MIB(T_CPORTFLAGS), 0x6, DME_PEER },
> + { UIC_ARG_MIB(T_CPORTMODE), 1, DME_PEER },
> + { UIC_ARG_MIB(T_CONNECTIONSTATE), 1, DME_PEER }
> + };
> + return ufshcd_dwc_dme_set_attrs(hba, setup_attrs, ARRAY_SIZE(setup_attrs));
> +}
> +
> +/**
> + * ufshcd_dwc_link_startup_notify() - program clock divider.
> + * @hba: private structure pointer
> + * @status: Callback notify status
> + *
> + * Return: 0 on success, non-zero value on failure.
> + */
> +int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
> + enum ufs_notify_change_status status)
> +{
> + int err = 0;
> +
> + if (status == PRE_CHANGE) {
> + ufshcd_dwc_program_clk_div(hba, DWC_UFS_REG_HCLKDIV_DIV_125);
> +
> + err = ufshcd_vops_phy_initialization(hba);
> + if (err) {
> + dev_err(hba->dev, "Phy setup failed (%d)\n", err);
> + return err;
> + }
Drop this call, it's dead code, and weird to call core ops from a dwc helper...
> + } else { /* POST_CHANGE */
> + err = ufshcd_dwc_link_is_up(hba);
> + if (err) {
> + dev_err(hba->dev, "Link is not up\n");
> + return err;
> + }
> +
> + err = ufshcd_dwc_connection_setup(hba);
> + if (err)
> + dev_err(hba->dev, "Connection setup failed (%d)\n",
> + err);
> + }
> +
> + return err;
> +}
> diff --git a/drivers/ufs/ufshcd-dwc.h b/drivers/ufs/ufshcd-dwc.h
> new file mode 100644
> index 0000000000..d997045d3c
> --- /dev/null
> +++ b/drivers/ufs/ufshcd-dwc.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * UFS Host driver for Synopsys Designware Core
> + *
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + * Authors: Joao Pinto <jpinto at synopsys.com>
> + */
> +
> +#ifndef _UFSHCD_DWC_H
> +#define _UFSHCD_DWC_H
> +
> +struct ufshcd_dme_attr_val {
> + u32 attr_sel;
> + u32 mib_val;
> + u8 peer;
> +};
> +
> +int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
> + enum ufs_notify_change_status status);
> +int ufshcd_dwc_dme_set_attrs(struct ufs_hba *hba,
> + const struct ufshcd_dme_attr_val *v, int n);
> +#endif /* End of Header */
> diff --git a/drivers/ufs/ufshci-dwc.h b/drivers/ufs/ufshci-dwc.h
> new file mode 100644
> index 0000000000..9e24c230c6
> --- /dev/null
> +++ b/drivers/ufs/ufshci-dwc.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * UFS Host driver for Synopsys Designware Core
> + *
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + * Authors: Joao Pinto <jpinto at synopsys.com>
> + */
> +
> +#ifndef _UFSHCI_DWC_H
> +#define _UFSHCI_DWC_H
> +
> +/* DWC HC UFSHCI specific Registers */
> +enum dwc_specific_registers {
> + DWC_UFS_REG_HCLKDIV = 0xFC,
> +};
> +
> +/* Clock Divider Values: Hex equivalent of frequency in MHz */
> +enum clk_div_values {
> + DWC_UFS_REG_HCLKDIV_DIV_62_5 = 0x3e,
> + DWC_UFS_REG_HCLKDIV_DIV_125 = 0x7d,
> + DWC_UFS_REG_HCLKDIV_DIV_200 = 0xc8,
> +};
> +
> +/* Selector Index */
> +enum selector_index {
> + SELIND_LN0_TX = 0x00,
> + SELIND_LN1_TX = 0x01,
> + SELIND_LN0_RX = 0x04,
> + SELIND_LN1_RX = 0x05,
> +};
> +#endif
Thanks,
Neil
More information about the U-Boot
mailing list