答复: [PATCH v3 1/5] scsi: ufs: add Hisilicon ufs driver code
Hi, Arnd
Thanks for your suggestions, I hope you'll reply again:
-邮件原件-
发件人: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] 代表 Arnd Bergmann
发送时间: 2017年9月7日 6:47
收件人: liwei (CM)
抄送: Rob Herring; Mark Rutland; xuwei (O); Catalin Marinas; Will Deacon; Vinayak
Holikatti; James E.J. Bottomley; Martin K. Petersen; Kevin Hilman; Gregory
CLEMENT; Thomas Petazzoni; Masahiro Yamada; Riku Voipio; Thierry Reding;
Krzysztof Kozlowski; Eric Anholt; devicet...@vger.kernel.org; Linux Kernel
Mailing List; Linux ARM; linux-scsi; Guodong Xu; Fengbaopeng (kevin, Kirin
Solution Dept)
主题: Re: [PATCH v3 1/5] scsi: ufs: add Hisilicon ufs driver code
On Tue, Aug 29, 2017 at 10:41 AM, Li Wei wrote:
itel(host, UFS_ARESET, PERRSTDIS3_OFFSET);
> +
> + /* disable lp_reset_n */
> + ufs_sys_ctrl_set_bits(host, BIT_SYSCTRL_LP_RESET_N, RESET_CTRL_EN);
> + mdelay(1);
> +
> + if (gpio_is_valid(host->reset_gpio))
> + gpio_direction_output(host->reset_gpio, 1);
> +
> + ufs_sys_ctrl_writel(host, MASK_UFS_DEVICE_RESET |
> BIT_UFS_DEVICE_RESET,
> + UFS_DEVICE_RESET_CTRL);
> +
> + mdelay(20);
Could those mdelay() be turned into msleep() functions?
I will fix it in patch v4.
> +static int ufs_hisi_get_resource(struct ufs_hisi_host *host) {
> + struct resource *mem_res;
> + struct device_node *np = NULL;
> + struct device *dev = host->hba->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + /* get resource of ufs sys ctrl */
> + mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + host->ufs_sys_ctrl = devm_ioremap_resource(dev, mem_res);
> + if (IS_ERR(host->ufs_sys_ctrl))
> + return PTR_ERR(host->ufs_sys_ctrl);
> +
> + np = of_find_compatible_node(NULL, NULL,
> + "hisilicon,hi3660-crgctrl");
It's generally not a good idea to look up one device by its "compatible"
string. What is the "crgctrl"? Does it have a proper DT binding?
Maybe there should be a driver for it, or you could make it a "syscon"
device and look it up by phandle instead.
ok, crgctrl is our common register, if look up device by its "compatible" is
not appropriate,
I will add a properties in ufs node, like this:
ufs: ufs@ff3b {
compatible = "jedec,ufs-1.1", "hisilicon,hi3660-ufs";
/* 0: HCI standard */
/* 1: UFS SYS CTRL */
reg = <0x0 0xff3b 0x0 0x1000>,
<0x0 0xff3b1000 0x0 0x1000>;
interrupt-parent = <&gic>;
interrupts = ;
clocks = <&crg_ctrl HI3660_CLK_GATE_UFSIO_REF>,
<&crg_ctrl HI3660_CLK_GATE_UFSPHY_CFG>;
clock-names = "clk_ref", "clk_phy";
freq-table-hz = <0 0>, <0 0>;
+ /* offset: 0x84; bit: 12 */
+ /* offset: 0x84; bit: 7 */
+ resets = <&crg_rst 0x84 12>,
+ <&crg_rst 0x84 7>;
+ reset-names = "rst", "assert";
}
And find that by, is it OK?
+ host->rst = devm_reset_control_get(dev, "rst");
+ host->assert = devm_reset_control_get(dev, "assert");
> diff --git a/drivers/scsi/ufs/ufs-hisi.h b/drivers/scsi/ufs/ufs-hisi.h
> new file mode 100644 index ..52430a2aca90
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-hisi.h
If the header is only used in one file, you don't need it, just move
the definitions
into the other file.
Currently only one file use ufs-hisi.h, but I think so many definitions are
defined in a .h file is more clearer, like ufs-qcom.h.
If you think it isn't necessary, I will move it into ufs-hisi.c?
Arnd
Re: [PATCH v3 1/5] scsi: ufs: add Hisilicon ufs driver code
On Tue, Aug 29, 2017 at 10:41 AM, Li Wei wrote:
itel(host, UFS_ARESET, PERRSTDIS3_OFFSET);
> +
> + /* disable lp_reset_n */
> + ufs_sys_ctrl_set_bits(host, BIT_SYSCTRL_LP_RESET_N, RESET_CTRL_EN);
> + mdelay(1);
> +
> + if (gpio_is_valid(host->reset_gpio))
> + gpio_direction_output(host->reset_gpio, 1);
> +
> + ufs_sys_ctrl_writel(host, MASK_UFS_DEVICE_RESET |
> BIT_UFS_DEVICE_RESET,
> + UFS_DEVICE_RESET_CTRL);
> +
> + mdelay(20);
Could those mdelay() be turned into msleep() functions?
> +static int ufs_hisi_get_resource(struct ufs_hisi_host *host)
> +{
> + struct resource *mem_res;
> + struct device_node *np = NULL;
> + struct device *dev = host->hba->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + /* get resource of ufs sys ctrl */
> + mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + host->ufs_sys_ctrl = devm_ioremap_resource(dev, mem_res);
> + if (IS_ERR(host->ufs_sys_ctrl))
> + return PTR_ERR(host->ufs_sys_ctrl);
> +
> + np = of_find_compatible_node(NULL, NULL, "hisilicon,hi3660-crgctrl");
It's generally not a good idea to look up one device by its "compatible"
string. What is the "crgctrl"? Does it have a proper DT binding?
Maybe there should be a driver for it, or you could make it a "syscon"
device and look it up by phandle instead.
> diff --git a/drivers/scsi/ufs/ufs-hisi.h b/drivers/scsi/ufs/ufs-hisi.h
> new file mode 100644
> index ..52430a2aca90
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-hisi.h
If the header is only used in one file, you don't need it, just move
the definitions
into the other file.
Arnd
[PATCH v3 1/5] scsi: ufs: add Hisilicon ufs driver code
add Hisilicon ufs driver code
Signed-off-by: Li Wei
Signed-off-by: Geng Jianfeng
Signed-off-by: Zang Leigang
Signed-off-by: Yu Jianfeng
---
drivers/scsi/ufs/Kconfig| 9 +
drivers/scsi/ufs/Makefile | 1 +
drivers/scsi/ufs/ufs-hisi.c | 634
drivers/scsi/ufs/ufs-hisi.h | 163
4 files changed, 807 insertions(+)
create mode 100644 drivers/scsi/ufs/ufs-hisi.c
create mode 100644 drivers/scsi/ufs/ufs-hisi.h
diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index e27b4d4e6ae2..bf2ff5628b15 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -80,6 +80,15 @@ config SCSI_UFSHCD_PLATFORM
If unsure, say N.
+config SCSI_UFS_HISI
+ tristate "Hisilicon specific hooks to UFS controller platform driver"
+ depends on (ARCH_HISI || COMPILE_TEST) && SCSI_UFSHCD_PLATFORM
+ ---help---
+ This selects the Hisilicon specific additions to UFSHCD platform
driver.
+
+ Select this if you have UFS controller on Hisilicon chipset.
+ If unsure, say N.
+
config SCSI_UFS_DWC_TC_PLATFORM
tristate "DesignWare platform support using a G210 Test Chip"
depends on SCSI_UFSHCD_PLATFORM
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 6e77cb0bfee9..9f2c17029a38 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o
tc-dwc-g210.o
obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
+obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
new file mode 100644
index ..7c3ff7060428
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -0,0 +1,634 @@
+/*
+ *
+ * HiSilicon Hi UFS Driver
+ *
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ufshcd.h"
+#include "ufshcd-pltfrm.h"
+#include "unipro.h"
+#include "ufs-hisi.h"
+#include "ufshci.h"
+
+static int ufs_hisi_check_hibern8(struct ufs_hba *hba)
+{
+ int err;
+ u32 tx_fsm_val_0;
+ u32 tx_fsm_val_1;
+ unsigned long timeout = jiffies + msecs_to_jiffies(HBRN8_POLL_TOUT_MS);
+
+ do {
+ err = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 0),
+ &tx_fsm_val_0);
+ err |= ufshcd_dme_get(hba,
+ UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 1), &tx_fsm_val_1);
+ if (err || (tx_fsm_val_0 == TX_FSM_HIBERN8 &&
+ tx_fsm_val_1 == TX_FSM_HIBERN8))
+ break;
+
+ /* sleep for max. 200us */
+ usleep_range(100, 200);
+ } while (time_before(jiffies, timeout));
+
+ /*
+* we might have scheduled out for long during polling so
+* check the state again.
+*/
+ if (time_after(jiffies, timeout)) {
+ err = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 0),
+&tx_fsm_val_0);
+ err |= ufshcd_dme_get(hba,
+UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, 1), &tx_fsm_val_1);
+ }
+
+ if (err) {
+ dev_err(hba->dev, "%s: unable to get TX_FSM_STATE, err %d\n",
+ __func__, err);
+ } else if (tx_fsm_val_0 != TX_FSM_HIBERN8 ||
+tx_fsm_val_1 != TX_FSM_HIBERN8) {
+ err = -1;
+ dev_err(hba->dev, "%s: invalid TX_FSM_STATE, lane0 = %d, lane1
= %d\n",
+ __func__, tx_fsm_val_0, tx_fsm_val_1);
+ }
+
+ return err;
+}
+
+static void ufs_hisi_clk_init(struct ufs_hba *hba)
+{
+ struct ufs_hisi_host *host = ufshcd_get_variant(hba);
+
+ ufs_sys_ctrl_clr_bits(host, BIT_SYSCTRL_REF_CLOCK_EN, PHY_CLK_CTRL);
+ if (ufs_sys_ctrl_readl(host, PHY_CLK_CTRL) & BIT_SYSCTRL_REF_CLOCK_EN)
+ mdelay(1);
+ /* use abb clk */
+ ufs_sys_ctrl_clr_bits(host, BIT_UFS_REFCLK_SRC_SEl, UFS_SYSCTRL);
+ ufs_sys_ctrl_clr_bits(host, BIT_UFS_REFCLK_ISO_EN, PHY_ISO_EN);
+ /* open mphy ref clk */
+ ufs_sys_ctrl_set_bits(host, BIT_SYSCTRL_REF_CLOCK_EN, PHY_CLK_CTRL);
+}
+
+static void ufs_hisi_soc_init(struct ufs_hba *hba)
+{
+ struct ufs_hisi_host *host = ufshcd_get_variant(h
