* [2/4] usb: chipidea: imx: add HSIC support
@ 2018-10-16 5:01 Peter Chen
0 siblings, 0 replies; 5+ messages in thread
From: Peter Chen @ 2018-10-16 5:01 UTC (permalink / raw)
To: linux-usb@vger.kernel.org, frieder.schrempf@exceet.de
Cc: dl-linux-imx, robh+dt@kernel.org, devicetree@vger.kernel.org,
Peter Chen
To support imx HSIC, there are some special requirement:
- The HSIC pad is 1.2v, it may need to supply from external
- The data/strobe pin needs to be pulled down first, and after
host mode is initialized, the strobe pin needs to be pulled up
- During the USB suspend/resume, special setting is needed
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 153 +++++++++++++++++++++++++++++++++----
drivers/usb/chipidea/ci_hdrc_imx.h | 9 ++-
drivers/usb/chipidea/usbmisc_imx.c | 131 +++++++++++++++++++++++++++++++
3 files changed, 274 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 09b37c0d075d..d566771fc77a 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -85,6 +85,9 @@ struct ci_hdrc_imx_data {
bool supports_runtime_pm;
bool override_phy_control;
bool in_lpm;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *pinctrl_hsic_active;
+ struct regulator *hsic_pad_regulator;
/* SoC before i.mx6 (except imx23/imx28) needs three clks */
bool need_three_clks;
struct clk *clk_ipg;
@@ -245,19 +248,58 @@ static void imx_disable_unprepare_clks(struct device *dev)
}
}
+static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
+{
+ struct device *dev = ci->dev->parent;
+ struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+ int ret = 0;
+
+ switch (event) {
+ case CI_HDRC_IMX_HSIC_ACTIVE_EVENT:
+ if (!IS_ERR(data->pinctrl) &&
+ !IS_ERR(data->pinctrl_hsic_active)) {
+ ret = pinctrl_select_state(data->pinctrl,
+ data->pinctrl_hsic_active);
+ if (ret)
+ dev_err(dev,
+ "hsic_active select failed, err=%d\n",
+ ret);
+ return ret;
+ }
+ break;
+ case CI_HDRC_IMX_HSIC_SUSPEND_EVENT:
+ if (data->usbmisc_data) {
+ ret = imx_usbmisc_hsic_set_connect(data->usbmisc_data);
+ if (ret)
+ dev_err(dev,
+ "hsic_set_connect failed, err=%d\n",
+ ret);
+ return ret;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
static int ci_hdrc_imx_probe(struct platform_device *pdev)
{
struct ci_hdrc_imx_data *data;
struct ci_hdrc_platform_data pdata = {
.name = dev_name(&pdev->dev),
.capoffset = DEF_CAPOFFSET,
+ .notify_event = ci_hdrc_imx_notify_event,
};
int ret;
const struct of_device_id *of_id;
const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct pinctrl_state *pinctrl_hsic_idle;
- of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
+ of_id = of_match_device(ci_hdrc_imx_dt_ids, dev);
if (!of_id)
return -ENODEV;
@@ -268,19 +310,48 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, data);
- data->usbmisc_data = usbmisc_get_init_data(&pdev->dev);
+ data->usbmisc_data = usbmisc_get_init_data(dev);
if (IS_ERR(data->usbmisc_data))
return PTR_ERR(data->usbmisc_data);
- ret = imx_get_clks(&pdev->dev);
+ data->pinctrl = devm_pinctrl_get(dev);
+ if (IS_ERR(data->pinctrl)) {
+ dev_dbg(dev, "pinctrl get failed, err=%ld\n",
+ PTR_ERR(data->pinctrl));
+ } else {
+ pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
+ if (IS_ERR(pinctrl_hsic_idle)) {
+ dev_dbg(dev,
+ "pinctrl_hsic_idle lookup failed, err=%ld\n",
+ PTR_ERR(pinctrl_hsic_idle));
+ } else {
+ ret = pinctrl_select_state(data->pinctrl,
+ pinctrl_hsic_idle);
+ if (ret) {
+ dev_err(dev,
+ "hsic_idle select failed, err=%d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
+ "active");
+ if (IS_ERR(data->pinctrl_hsic_active))
+ dev_dbg(dev,
+ "pinctrl_hsic_active lookup failed, err=%ld\n",
+ PTR_ERR(data->pinctrl_hsic_active));
+ }
+
+ ret = imx_get_clks(dev);
if (ret)
return ret;
- ret = imx_prepare_enable_clks(&pdev->dev);
+ ret = imx_prepare_enable_clks(dev);
if (ret)
return ret;
- data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
+ data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0);
if (IS_ERR(data->phy)) {
ret = PTR_ERR(data->phy);
/* Return -EINVAL if no usbphy is available */
@@ -303,42 +374,72 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
data->supports_runtime_pm = true;
+ if (of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC) {
+ pdata.flags |= CI_HDRC_IMX_IS_HSIC;
+ data->usbmisc_data->hsic = 1;
+ data->hsic_pad_regulator = devm_regulator_get(dev, "hsic");
+ if (PTR_ERR(data->hsic_pad_regulator) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_clk;
+ } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV) {
+ /* no pad regualator is needed */
+ data->hsic_pad_regulator = NULL;
+ } else if (IS_ERR(data->hsic_pad_regulator)) {
+ dev_err(dev, "Get hsic pad regulator error: %ld\n",
+ PTR_ERR(data->hsic_pad_regulator));
+ ret = PTR_ERR(data->hsic_pad_regulator);
+ goto err_clk;
+ }
+
+ if (data->hsic_pad_regulator) {
+ ret = regulator_enable(data->hsic_pad_regulator);
+ if (ret) {
+ dev_err(dev,
+ "Fail to enable hsic pad regulator\n");
+ goto err_clk;
+ }
+ }
+ }
+
ret = imx_usbmisc_init(data->usbmisc_data);
if (ret) {
- dev_err(&pdev->dev, "usbmisc init failed, ret=%d\n", ret);
- goto err_clk;
+ dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
+ goto disable_hsic_regulator;
}
- data->ci_pdev = ci_hdrc_add_device(&pdev->dev,
+ data->ci_pdev = ci_hdrc_add_device(dev,
pdev->resource, pdev->num_resources,
&pdata);
if (IS_ERR(data->ci_pdev)) {
ret = PTR_ERR(data->ci_pdev);
if (ret != -EPROBE_DEFER)
- dev_err(&pdev->dev,
- "ci_hdrc_add_device failed, err=%d\n", ret);
- goto err_clk;
+ dev_err(dev, "ci_hdrc_add_device failed, err=%d\n",
+ ret);
+ goto disable_hsic_regulator;
}
ret = imx_usbmisc_init_post(data->usbmisc_data);
if (ret) {
- dev_err(&pdev->dev, "usbmisc post failed, ret=%d\n", ret);
+ dev_err(dev, "usbmisc post failed, ret=%d\n", ret);
goto disable_device;
}
if (data->supports_runtime_pm) {
- pm_runtime_set_active(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
}
- device_set_wakeup_capable(&pdev->dev, true);
+ device_set_wakeup_capable(dev, true);
return 0;
disable_device:
ci_hdrc_remove_device(data->ci_pdev);
+disable_hsic_regulator:
+ if (data->hsic_pad_regulator)
+ ret = regulator_disable(data->hsic_pad_regulator);
err_clk:
- imx_disable_unprepare_clks(&pdev->dev);
+ imx_disable_unprepare_clks(dev);
return ret;
}
@@ -355,6 +456,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
if (data->override_phy_control)
usb_phy_shutdown(data->phy);
imx_disable_unprepare_clks(&pdev->dev);
+ if (data->hsic_pad_regulator)
+ regulator_disable(data->hsic_pad_regulator);
return 0;
}
@@ -367,9 +470,19 @@ static void ci_hdrc_imx_shutdown(struct platform_device *pdev)
static int __maybe_unused imx_controller_suspend(struct device *dev)
{
struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+ int ret = 0;
dev_dbg(dev, "at %s\n", __func__);
+ if (data->usbmisc_data) {
+ ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, false);
+ if (ret) {
+ dev_err(dev,
+ "usbmisc hsic_set_clk failed, ret=%d\n", ret);
+ return ret;
+ }
+ }
+
imx_disable_unprepare_clks(dev);
data->in_lpm = true;
@@ -400,8 +513,16 @@ static int __maybe_unused imx_controller_resume(struct device *dev)
goto clk_disable;
}
+ ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, true);
+ if (ret) {
+ dev_err(dev, "usbmisc hsic_set_clk failed, ret=%d\n", ret);
+ goto hsic_set_clk_fail;
+ }
+
return 0;
+hsic_set_clk_fail:
+ imx_usbmisc_set_wakeup(data->usbmisc_data, true);
clk_disable:
imx_disable_unprepare_clks(dev);
return ret;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 204275f47573..fcecab478934 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -14,10 +14,13 @@ struct imx_usbmisc_data {
unsigned int oc_polarity:1; /* over current polarity if oc enabled */
unsigned int evdo:1; /* set external vbus divider option */
unsigned int ulpi:1; /* connected to an ULPI phy */
+ unsigned int hsic:1; /* HSIC controlller */
};
-int imx_usbmisc_init(struct imx_usbmisc_data *);
-int imx_usbmisc_init_post(struct imx_usbmisc_data *);
-int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *, bool);
+int imx_usbmisc_init(struct imx_usbmisc_data *data);
+int imx_usbmisc_init_post(struct imx_usbmisc_data *data);
+int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled);
+int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data);
+int imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on);
#endif /* __DRIVER_USB_CHIPIDEA_CI_HDRC_IMX_H */
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index def80ff547e4..a66a15075200 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -64,10 +64,22 @@
#define MX6_BM_OVER_CUR_DIS BIT(7)
#define MX6_BM_OVER_CUR_POLARITY BIT(8)
#define MX6_BM_WAKEUP_ENABLE BIT(10)
+#define MX6_BM_UTMI_ON_CLOCK BIT(13)
#define MX6_BM_ID_WAKEUP BIT(16)
#define MX6_BM_VBUS_WAKEUP BIT(17)
#define MX6SX_BM_DPDM_WAKEUP_EN BIT(29)
#define MX6_BM_WAKEUP_INTR BIT(31)
+
+#define MX6_USB_HSIC_CTRL_OFFSET 0x10
+/* Send resume signal without 480Mhz PHY clock */
+#define MX6SX_BM_HSIC_AUTO_RESUME BIT(23)
+/* set before portsc.suspendM = 1 */
+#define MX6_BM_HSIC_DEV_CONN BIT(21)
+/* HSIC enable */
+#define MX6_BM_HSIC_EN BIT(12)
+/* Force HSIC module 480M clock on, even when in Host is in suspend mode */
+#define MX6_BM_HSIC_CLK_ON BIT(11)
+
#define MX6_USB_OTG1_PHY_CTRL 0x18
/* For imx6dql, it is host-only controller, for later imx6, it is otg's */
#define MX6_USB_OTG2_PHY_CTRL 0x1c
@@ -94,6 +106,10 @@ struct usbmisc_ops {
int (*post)(struct imx_usbmisc_data *data);
/* It's called when we need to enable/disable usb wakeup */
int (*set_wakeup)(struct imx_usbmisc_data *data, bool enabled);
+ /* It's called before setting portsc.suspendM */
+ int (*hsic_set_connect)(struct imx_usbmisc_data *data);
+ /* It's called during suspend/resume */
+ int (*hsic_set_clk)(struct imx_usbmisc_data *data, bool enabled);
};
struct imx_usbmisc {
@@ -353,6 +369,18 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
writel(reg | MX6_BM_NON_BURST_SETTING,
usbmisc->base + data->index * 4);
+ /* For HSIC controller */
+ if (data->hsic) {
+ reg = readl(usbmisc->base + data->index * 4);
+ writel(reg | MX6_BM_UTMI_ON_CLOCK,
+ usbmisc->base + data->index * 4);
+ reg = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET
+ + (data->index - 2) * 4);
+ reg |= MX6_BM_HSIC_EN | MX6_BM_HSIC_CLK_ON;
+ writel(reg, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET
+ + (data->index - 2) * 4);
+ }
+
spin_unlock_irqrestore(&usbmisc->lock, flags);
usbmisc_imx6q_set_wakeup(data, false);
@@ -360,6 +388,70 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
return 0;
}
+static int usbmisc_imx6_hsic_set_connect(struct imx_usbmisc_data *data)
+{
+ unsigned long flags;
+ u32 val, offset;
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ int ret = 0;
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ if (data->index == 2 || data->index == 3) {
+ offset = (data->index - 2) * 4;
+ } else if (data->index == 0) {
+ /*
+ * For controllers later than imx7d (imx7d is included),
+ * each controller has its own non core register region.
+ * And the controllers before than imx7d, the 1st controller
+ * is not HSIC controller.
+ */
+ offset = 0;
+ } else {
+ dev_err(data->dev, "index is error for usbmisc\n");
+ offset = 0;
+ ret = -EINVAL;
+ }
+
+ val = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
+ if (!(val & MX6_BM_HSIC_DEV_CONN))
+ writel(val | MX6_BM_HSIC_DEV_CONN,
+ usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ return ret;
+}
+
+static int usbmisc_imx6_hsic_set_clk(struct imx_usbmisc_data *data, bool on)
+{
+ unsigned long flags;
+ u32 val, offset;
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ int ret = 0;
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ if (data->index == 2 || data->index == 3) {
+ offset = (data->index - 2) * 4;
+ } else if (data->index == 0) {
+ offset = 0;
+ } else {
+ dev_err(data->dev, "index is error for usbmisc\n");
+ offset = 0;
+ ret = -EINVAL;
+ }
+
+ val = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
+ val |= MX6_BM_HSIC_EN | MX6_BM_HSIC_CLK_ON;
+ if (on)
+ val |= MX6_BM_HSIC_CLK_ON;
+ else
+ val &= ~MX6_BM_HSIC_CLK_ON;
+ writel(val, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ return 0;
+}
+
+
static int usbmisc_imx6sx_init(struct imx_usbmisc_data *data)
{
void __iomem *reg = NULL;
@@ -385,6 +477,13 @@ static int usbmisc_imx6sx_init(struct imx_usbmisc_data *data)
spin_unlock_irqrestore(&usbmisc->lock, flags);
}
+ /* For HSIC controller */
+ if (data->hsic) {
+ val = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET);
+ val |= MX6SX_BM_HSIC_AUTO_RESUME;
+ writel(val, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET);
+ }
+
return 0;
}
@@ -454,6 +553,7 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID,
usbmisc->base + MX7D_USBNC_USB_CTRL2);
+
spin_unlock_irqrestore(&usbmisc->lock, flags);
usbmisc_imx7d_set_wakeup(data, false);
@@ -481,6 +581,8 @@ static const struct usbmisc_ops imx53_usbmisc_ops = {
static const struct usbmisc_ops imx6q_usbmisc_ops = {
.set_wakeup = usbmisc_imx6q_set_wakeup,
.init = usbmisc_imx6q_init,
+ .hsic_set_connect = usbmisc_imx6_hsic_set_connect,
+ .hsic_set_clk = usbmisc_imx6_hsic_set_clk,
};
static const struct usbmisc_ops vf610_usbmisc_ops = {
@@ -490,6 +592,8 @@ static const struct usbmisc_ops vf610_usbmisc_ops = {
static const struct usbmisc_ops imx6sx_usbmisc_ops = {
.set_wakeup = usbmisc_imx6q_set_wakeup,
.init = usbmisc_imx6sx_init,
+ .hsic_set_connect = usbmisc_imx6_hsic_set_connect,
+ .hsic_set_clk = usbmisc_imx6_hsic_set_clk,
};
static const struct usbmisc_ops imx7d_usbmisc_ops = {
@@ -546,6 +650,33 @@ int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled)
}
EXPORT_SYMBOL_GPL(imx_usbmisc_set_wakeup);
+int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data)
+{
+ struct imx_usbmisc *usbmisc;
+
+ if (!data)
+ return 0;
+
+ usbmisc = dev_get_drvdata(data->dev);
+ if (!usbmisc->ops->hsic_set_connect || !data->hsic)
+ return 0;
+ return usbmisc->ops->hsic_set_connect(data);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_hsic_set_connect);
+
+int imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on)
+{
+ struct imx_usbmisc *usbmisc;
+
+ if (!data)
+ return 0;
+
+ usbmisc = dev_get_drvdata(data->dev);
+ if (!usbmisc->ops->hsic_set_clk || !data->hsic)
+ return 0;
+ return usbmisc->ops->hsic_set_clk(data, on);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_hsic_set_clk);
static const struct of_device_id usbmisc_imx_dt_ids[] = {
{
.compatible = "fsl,imx25-usbmisc",
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [2/4] usb: chipidea: imx: add HSIC support
@ 2018-10-16 5:52 kbuild test robot
0 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2018-10-16 5:52 UTC (permalink / raw)
To: Peter Chen
Cc: kbuild-all, linux-usb@vger.kernel.org, frieder.schrempf@exceet.de,
dl-linux-imx, robh+dt@kernel.org, devicetree@vger.kernel.org
Hi Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on peter.chen-usb/ci-for-usb-next]
[also build test ERROR on v4.19-rc8 next-20181012]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Peter-Chen/usb-chipidea-imx-add-HSIC-support/20181016-130840
base: https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git ci-for-usb-next
config: x86_64-randconfig-x017-201841 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/usb/chipidea/ci_hdrc_imx.c: In function 'ci_hdrc_imx_notify_event':
>> drivers/usb/chipidea/ci_hdrc_imx.c:261:10: error: implicit declaration of function 'pinctrl_select_state'; did you mean 'inc_node_state'? [-Werror=implicit-function-declaration]
ret = pinctrl_select_state(data->pinctrl,
^~~~~~~~~~~~~~~~~~~~
inc_node_state
drivers/usb/chipidea/ci_hdrc_imx.c: In function 'ci_hdrc_imx_probe':
drivers/usb/chipidea/ci_hdrc_imx.c:317:18: error: implicit declaration of function 'devm_pinctrl_get'; did you mean 'devm_clk_get'? [-Werror=implicit-function-declaration]
data->pinctrl = devm_pinctrl_get(dev);
^~~~~~~~~~~~~~~~
devm_clk_get
drivers/usb/chipidea/ci_hdrc_imx.c:317:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
data->pinctrl = devm_pinctrl_get(dev);
^
>> drivers/usb/chipidea/ci_hdrc_imx.c:322:23: error: implicit declaration of function 'pinctrl_lookup_state'; did you mean 'inc_node_state'? [-Werror=implicit-function-declaration]
pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
^~~~~~~~~~~~~~~~~~~~
inc_node_state
drivers/usb/chipidea/ci_hdrc_imx.c:322:21: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
^
drivers/usb/chipidea/ci_hdrc_imx.c:338:29: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
^
cc1: some warnings being treated as errors
vim +261 drivers/usb/chipidea/ci_hdrc_imx.c
250
251 static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
252 {
253 struct device *dev = ci->dev->parent;
254 struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
255 int ret = 0;
256
257 switch (event) {
258 case CI_HDRC_IMX_HSIC_ACTIVE_EVENT:
259 if (!IS_ERR(data->pinctrl) &&
260 !IS_ERR(data->pinctrl_hsic_active)) {
> 261 ret = pinctrl_select_state(data->pinctrl,
262 data->pinctrl_hsic_active);
263 if (ret)
264 dev_err(dev,
265 "hsic_active select failed, err=%d\n",
266 ret);
267 return ret;
268 }
269 break;
270 case CI_HDRC_IMX_HSIC_SUSPEND_EVENT:
271 if (data->usbmisc_data) {
272 ret = imx_usbmisc_hsic_set_connect(data->usbmisc_data);
273 if (ret)
274 dev_err(dev,
275 "hsic_set_connect failed, err=%d\n",
276 ret);
277 return ret;
278 }
279 break;
280 default:
281 break;
282 }
283
284 return ret;
285 }
286
287 static int ci_hdrc_imx_probe(struct platform_device *pdev)
288 {
289 struct ci_hdrc_imx_data *data;
290 struct ci_hdrc_platform_data pdata = {
291 .name = dev_name(&pdev->dev),
292 .capoffset = DEF_CAPOFFSET,
293 .notify_event = ci_hdrc_imx_notify_event,
294 };
295 int ret;
296 const struct of_device_id *of_id;
297 const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
298 struct device_node *np = pdev->dev.of_node;
299 struct device *dev = &pdev->dev;
300 struct pinctrl_state *pinctrl_hsic_idle;
301
302 of_id = of_match_device(ci_hdrc_imx_dt_ids, dev);
303 if (!of_id)
304 return -ENODEV;
305
306 imx_platform_flag = of_id->data;
307
308 data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
309 if (!data)
310 return -ENOMEM;
311
312 platform_set_drvdata(pdev, data);
313 data->usbmisc_data = usbmisc_get_init_data(dev);
314 if (IS_ERR(data->usbmisc_data))
315 return PTR_ERR(data->usbmisc_data);
316
> 317 data->pinctrl = devm_pinctrl_get(dev);
318 if (IS_ERR(data->pinctrl)) {
319 dev_dbg(dev, "pinctrl get failed, err=%ld\n",
320 PTR_ERR(data->pinctrl));
321 } else {
> 322 pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
323 if (IS_ERR(pinctrl_hsic_idle)) {
324 dev_dbg(dev,
325 "pinctrl_hsic_idle lookup failed, err=%ld\n",
326 PTR_ERR(pinctrl_hsic_idle));
327 } else {
328 ret = pinctrl_select_state(data->pinctrl,
329 pinctrl_hsic_idle);
330 if (ret) {
331 dev_err(dev,
332 "hsic_idle select failed, err=%d\n",
333 ret);
334 return ret;
335 }
336 }
337
338 data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
339 "active");
340 if (IS_ERR(data->pinctrl_hsic_active))
341 dev_dbg(dev,
342 "pinctrl_hsic_active lookup failed, err=%ld\n",
343 PTR_ERR(data->pinctrl_hsic_active));
344 }
345
346 ret = imx_get_clks(dev);
347 if (ret)
348 return ret;
349
350 ret = imx_prepare_enable_clks(dev);
351 if (ret)
352 return ret;
353
354 data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0);
355 if (IS_ERR(data->phy)) {
356 ret = PTR_ERR(data->phy);
357 /* Return -EINVAL if no usbphy is available */
358 if (ret == -ENODEV)
359 ret = -EINVAL;
360 goto err_clk;
361 }
362
363 pdata.usb_phy = data->phy;
364
365 if ((of_device_is_compatible(np, "fsl,imx53-usb") ||
366 of_device_is_compatible(np, "fsl,imx51-usb")) && pdata.usb_phy &&
367 of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
368 pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
369 data->override_phy_control = true;
370 usb_phy_init(pdata.usb_phy);
371 }
372
373 pdata.flags |= imx_platform_flag->flags;
374 if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
375 data->supports_runtime_pm = true;
376
377 if (of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC) {
378 pdata.flags |= CI_HDRC_IMX_IS_HSIC;
379 data->usbmisc_data->hsic = 1;
380 data->hsic_pad_regulator = devm_regulator_get(dev, "hsic");
381 if (PTR_ERR(data->hsic_pad_regulator) == -EPROBE_DEFER) {
382 ret = -EPROBE_DEFER;
383 goto err_clk;
384 } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV) {
385 /* no pad regualator is needed */
386 data->hsic_pad_regulator = NULL;
387 } else if (IS_ERR(data->hsic_pad_regulator)) {
388 dev_err(dev, "Get hsic pad regulator error: %ld\n",
389 PTR_ERR(data->hsic_pad_regulator));
390 ret = PTR_ERR(data->hsic_pad_regulator);
391 goto err_clk;
392 }
393
394 if (data->hsic_pad_regulator) {
395 ret = regulator_enable(data->hsic_pad_regulator);
396 if (ret) {
397 dev_err(dev,
398 "Fail to enable hsic pad regulator\n");
399 goto err_clk;
400 }
401 }
402 }
403
404 ret = imx_usbmisc_init(data->usbmisc_data);
405 if (ret) {
406 dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
407 goto disable_hsic_regulator;
408 }
409
410 data->ci_pdev = ci_hdrc_add_device(dev,
411 pdev->resource, pdev->num_resources,
412 &pdata);
413 if (IS_ERR(data->ci_pdev)) {
414 ret = PTR_ERR(data->ci_pdev);
415 if (ret != -EPROBE_DEFER)
416 dev_err(dev, "ci_hdrc_add_device failed, err=%d\n",
417 ret);
418 goto disable_hsic_regulator;
419 }
420
421 ret = imx_usbmisc_init_post(data->usbmisc_data);
422 if (ret) {
423 dev_err(dev, "usbmisc post failed, ret=%d\n", ret);
424 goto disable_device;
425 }
426
427 if (data->supports_runtime_pm) {
428 pm_runtime_set_active(dev);
429 pm_runtime_enable(dev);
430 }
431
432 device_set_wakeup_capable(dev, true);
433
434 return 0;
435
436 disable_device:
437 ci_hdrc_remove_device(data->ci_pdev);
438 disable_hsic_regulator:
439 if (data->hsic_pad_regulator)
440 ret = regulator_disable(data->hsic_pad_regulator);
441 err_clk:
442 imx_disable_unprepare_clks(dev);
443 return ret;
444 }
445
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
* [2/4] usb: chipidea: imx: add HSIC support
@ 2018-10-16 6:07 Peter Chen
0 siblings, 0 replies; 5+ messages in thread
From: Peter Chen @ 2018-10-16 6:07 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all@01.org, linux-usb@vger.kernel.org,
frieder.schrempf@exceet.de, dl-linux-imx, robh+dt@kernel.org,
devicetree@vger.kernel.org
> Hi Peter,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on peter.chen-usb/ci-for-usb-next] [also build test ERROR
> on v4.19-rc8 next-20181012] [if your patch is applied to the wrong git tree, please
> drop us a note to help improve the system]
>
> url:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com
> %2F0day-ci%2Flinux%2Fcommits%2FPeter-Chen%2Fusb-chipidea-imx-add-HSIC-
> support%2F20181016-
> 130840&data=02%7C01%7Cpeter.chen%40nxp.com%7C50f2cf2d6d5341bf72
> cc08d6332ba794%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63675
> 2662551616821&sdata=clbCGj%2BrgTz56IKDM0DVVyL3e4q79FsCv3vn%2F7
> TSrQI%3D&reserved=0
> base:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.o
> rg%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fpeter.chen%2Fusb.git&data
> =02%7C01%7Cpeter.chen%40nxp.com%7C50f2cf2d6d5341bf72cc08d6332ba794
> %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636752662551616821
> &sdata=6Bp7JvbXAsJ%2FGZCOn0VmtZQSnWUCYhgqlbG8ZrbzBoE%3D&a
> mp;reserved=0 ci-for-usb-next
> config: x86_64-randconfig-x017-201841 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
Will fix it by adding #include <linux/pinctrl/consumer.h>, it is strange I did not meet this error.
Peter
> All errors (new ones prefixed by >>):
>
> drivers/usb/chipidea/ci_hdrc_imx.c: In function 'ci_hdrc_imx_notify_event':
> >> drivers/usb/chipidea/ci_hdrc_imx.c:261:10: error: implicit
> >> declaration of function 'pinctrl_select_state'; did you mean
> >> 'inc_node_state'? [-Werror=implicit-function-declaration]
> ret = pinctrl_select_state(data->pinctrl,
> ^~~~~~~~~~~~~~~~~~~~
> inc_node_state
> drivers/usb/chipidea/ci_hdrc_imx.c: In function 'ci_hdrc_imx_probe':
> drivers/usb/chipidea/ci_hdrc_imx.c:317:18: error: implicit declaration of function
> 'devm_pinctrl_get'; did you mean 'devm_clk_get'? [-Werror=implicit-function-
> declaration]
> data->pinctrl = devm_pinctrl_get(dev);
> ^~~~~~~~~~~~~~~~
> devm_clk_get
> drivers/usb/chipidea/ci_hdrc_imx.c:317:16: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
> data->pinctrl = devm_pinctrl_get(dev);
> ^
> >> drivers/usb/chipidea/ci_hdrc_imx.c:322:23: error: implicit
> >> declaration of function 'pinctrl_lookup_state'; did you mean
> >> 'inc_node_state'? [-Werror=implicit-function-declaration]
> pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
> ^~~~~~~~~~~~~~~~~~~~
> inc_node_state
> drivers/usb/chipidea/ci_hdrc_imx.c:322:21: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
> pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
> ^
> drivers/usb/chipidea/ci_hdrc_imx.c:338:29: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
> data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
> ^
> cc1: some warnings being treated as errors
>
> vim +261 drivers/usb/chipidea/ci_hdrc_imx.c
>
> 250
> 251 static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> 252 {
> 253 struct device *dev = ci->dev->parent;
> 254 struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> 255 int ret = 0;
> 256
> 257 switch (event) {
> 258 case CI_HDRC_IMX_HSIC_ACTIVE_EVENT:
> 259 if (!IS_ERR(data->pinctrl) &&
> 260 !IS_ERR(data->pinctrl_hsic_active)) {
> > 261 ret = pinctrl_select_state(data->pinctrl,
> 262 data->pinctrl_hsic_active);
> 263 if (ret)
> 264 dev_err(dev,
> 265 "hsic_active select failed, err=%d\n",
> 266 ret);
> 267 return ret;
> 268 }
> 269 break;
> 270 case CI_HDRC_IMX_HSIC_SUSPEND_EVENT:
> 271 if (data->usbmisc_data) {
> 272 ret = imx_usbmisc_hsic_set_connect(data-
> >usbmisc_data);
> 273 if (ret)
> 274 dev_err(dev,
> 275 "hsic_set_connect failed, err=%d\n",
> 276 ret);
> 277 return ret;
> 278 }
> 279 break;
> 280 default:
> 281 break;
> 282 }
> 283
> 284 return ret;
> 285 }
> 286
> 287 static int ci_hdrc_imx_probe(struct platform_device *pdev)
> 288 {
> 289 struct ci_hdrc_imx_data *data;
> 290 struct ci_hdrc_platform_data pdata = {
> 291 .name = dev_name(&pdev->dev),
> 292 .capoffset = DEF_CAPOFFSET,
> 293 .notify_event = ci_hdrc_imx_notify_event,
> 294 };
> 295 int ret;
> 296 const struct of_device_id *of_id;
> 297 const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
> 298 struct device_node *np = pdev->dev.of_node;
> 299 struct device *dev = &pdev->dev;
> 300 struct pinctrl_state *pinctrl_hsic_idle;
> 301
> 302 of_id = of_match_device(ci_hdrc_imx_dt_ids, dev);
> 303 if (!of_id)
> 304 return -ENODEV;
> 305
> 306 imx_platform_flag = of_id->data;
> 307
> 308 data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> 309 if (!data)
> 310 return -ENOMEM;
> 311
> 312 platform_set_drvdata(pdev, data);
> 313 data->usbmisc_data = usbmisc_get_init_data(dev);
> 314 if (IS_ERR(data->usbmisc_data))
> 315 return PTR_ERR(data->usbmisc_data);
> 316
> > 317 data->pinctrl = devm_pinctrl_get(dev);
> 318 if (IS_ERR(data->pinctrl)) {
> 319 dev_dbg(dev, "pinctrl get failed, err=%ld\n",
> 320 PTR_ERR(data->pinctrl));
> 321 } else {
> > 322 pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
> 323 if (IS_ERR(pinctrl_hsic_idle)) {
> 324 dev_dbg(dev,
> 325 "pinctrl_hsic_idle lookup failed, err=%ld\n",
> 326 PTR_ERR(pinctrl_hsic_idle));
> 327 } else {
> 328 ret = pinctrl_select_state(data->pinctrl,
> 329 pinctrl_hsic_idle);
> 330 if (ret) {
> 331 dev_err(dev,
> 332 "hsic_idle select failed, err=%d\n",
> 333 ret);
> 334 return ret;
> 335 }
> 336 }
> 337
> 338 data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
> 339 "active");
> 340 if (IS_ERR(data->pinctrl_hsic_active))
> 341 dev_dbg(dev,
> 342 "pinctrl_hsic_active lookup failed, err=%ld\n",
> 343 PTR_ERR(data->pinctrl_hsic_active));
> 344 }
> 345
> 346 ret = imx_get_clks(dev);
> 347 if (ret)
> 348 return ret;
> 349
> 350 ret = imx_prepare_enable_clks(dev);
> 351 if (ret)
> 352 return ret;
> 353
> 354 data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0);
> 355 if (IS_ERR(data->phy)) {
> 356 ret = PTR_ERR(data->phy);
> 357 /* Return -EINVAL if no usbphy is available */
> 358 if (ret == -ENODEV)
> 359 ret = -EINVAL;
> 360 goto err_clk;
> 361 }
> 362
> 363 pdata.usb_phy = data->phy;
> 364
> 365 if ((of_device_is_compatible(np, "fsl,imx53-usb") ||
> 366 of_device_is_compatible(np, "fsl,imx51-usb")) && pdata.usb_phy
> &&
> 367 of_usb_get_phy_mode(np) ==
> USBPHY_INTERFACE_MODE_ULPI) {
> 368 pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> 369 data->override_phy_control = true;
> 370 usb_phy_init(pdata.usb_phy);
> 371 }
> 372
> 373 pdata.flags |= imx_platform_flag->flags;
> 374 if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> 375 data->supports_runtime_pm = true;
> 376
> 377 if (of_usb_get_phy_mode(dev->of_node) ==
> USBPHY_INTERFACE_MODE_HSIC) {
> 378 pdata.flags |= CI_HDRC_IMX_IS_HSIC;
> 379 data->usbmisc_data->hsic = 1;
> 380 data->hsic_pad_regulator = devm_regulator_get(dev, "hsic");
> 381 if (PTR_ERR(data->hsic_pad_regulator) == -
> EPROBE_DEFER) {
> 382 ret = -EPROBE_DEFER;
> 383 goto err_clk;
> 384 } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV)
> {
> 385 /* no pad regualator is needed */
> 386 data->hsic_pad_regulator = NULL;
> 387 } else if (IS_ERR(data->hsic_pad_regulator)) {
> 388 dev_err(dev, "Get hsic pad regulator error: %ld\n",
> 389 PTR_ERR(data->hsic_pad_regulator));
> 390 ret = PTR_ERR(data->hsic_pad_regulator);
> 391 goto err_clk;
> 392 }
> 393
> 394 if (data->hsic_pad_regulator) {
> 395 ret = regulator_enable(data->hsic_pad_regulator);
> 396 if (ret) {
> 397 dev_err(dev,
> 398 "Fail to enable hsic pad regulator\n");
> 399 goto err_clk;
> 400 }
> 401 }
> 402 }
> 403
> 404 ret = imx_usbmisc_init(data->usbmisc_data);
> 405 if (ret) {
> 406 dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> 407 goto disable_hsic_regulator;
> 408 }
> 409
> 410 data->ci_pdev = ci_hdrc_add_device(dev,
> 411 pdev->resource, pdev->num_resources,
> 412 &pdata);
> 413 if (IS_ERR(data->ci_pdev)) {
> 414 ret = PTR_ERR(data->ci_pdev);
> 415 if (ret != -EPROBE_DEFER)
> 416 dev_err(dev, "ci_hdrc_add_device failed, err=%d\n",
> 417 ret);
> 418 goto disable_hsic_regulator;
> 419 }
> 420
> 421 ret = imx_usbmisc_init_post(data->usbmisc_data);
> 422 if (ret) {
> 423 dev_err(dev, "usbmisc post failed, ret=%d\n", ret);
> 424 goto disable_device;
> 425 }
> 426
> 427 if (data->supports_runtime_pm) {
> 428 pm_runtime_set_active(dev);
> 429 pm_runtime_enable(dev);
> 430 }
> 431
> 432 device_set_wakeup_capable(dev, true);
> 433
> 434 return 0;
> 435
> 436 disable_device:
> 437 ci_hdrc_remove_device(data->ci_pdev);
> 438 disable_hsic_regulator:
> 439 if (data->hsic_pad_regulator)
> 440 ret = regulator_disable(data->hsic_pad_regulator);
> 441 err_clk:
> 442 imx_disable_unprepare_clks(dev);
> 443 return ret;
> 444 }
> 445
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org
> %2Fpipermail%2Fkbuild-
> all&data=02%7C01%7Cpeter.chen%40nxp.com%7C50f2cf2d6d5341bf72cc08
> d6332ba794%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636752662
> 551616821&sdata=FeXt%2BW7Jnl%2B%2BihbZjlVj5vZddN2C%2Fuf63PeBJv
> MQ6TA%3D&reserved=0 Intel Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
* [2/4] usb: chipidea: imx: add HSIC support
@ 2018-10-17 7:03 Frieder Schrempf
0 siblings, 0 replies; 5+ messages in thread
From: Frieder Schrempf @ 2018-10-17 7:03 UTC (permalink / raw)
To: Peter Chen, linux-usb@vger.kernel.org
Cc: dl-linux-imx, robh+dt@kernel.org, devicetree@vger.kernel.org
Hi Peter,
please see my comments below. For reference I also pushed the changes
here: https://github.com/fschrempf/linux/commits/usb-hsic-test
On 16.10.18 07:01, Peter Chen wrote:
> To support imx HSIC, there are some special requirement:
> - The HSIC pad is 1.2v, it may need to supply from external
> - The data/strobe pin needs to be pulled down first, and after
> host mode is initialized, the strobe pin needs to be pulled up
> - During the USB suspend/resume, special setting is needed
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 153 +++++++++++++++++++++++++++++++++----
> drivers/usb/chipidea/ci_hdrc_imx.h | 9 ++-
> drivers/usb/chipidea/usbmisc_imx.c | 131 +++++++++++++++++++++++++++++++
> 3 files changed, 274 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 09b37c0d075d..d566771fc77a 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -85,6 +85,9 @@ struct ci_hdrc_imx_data {
> bool supports_runtime_pm;
> bool override_phy_control;
> bool in_lpm;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pinctrl_hsic_active;
> + struct regulator *hsic_pad_regulator;
> /* SoC before i.mx6 (except imx23/imx28) needs three clks */
> bool need_three_clks;
> struct clk *clk_ipg;
> @@ -245,19 +248,58 @@ static void imx_disable_unprepare_clks(struct device *dev)
> }
> }
>
> +static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> +{
> + struct device *dev = ci->dev->parent;
> + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + switch (event) {
> + case CI_HDRC_IMX_HSIC_ACTIVE_EVENT:
> + if (!IS_ERR(data->pinctrl) &&
> + !IS_ERR(data->pinctrl_hsic_active)) {
If we make the pinctrl mandatory in case of HSIC as proposed below, we
don't need the checks here.
> + ret = pinctrl_select_state(data->pinctrl,
> + data->pinctrl_hsic_active);
> + if (ret)
> + dev_err(dev,
> + "hsic_active select failed, err=%d\n",
> + ret);
> + return ret;
> + }
> + break;
> + case CI_HDRC_IMX_HSIC_SUSPEND_EVENT:
> + if (data->usbmisc_data) {
> + ret = imx_usbmisc_hsic_set_connect(data->usbmisc_data);
> + if (ret)
> + dev_err(dev,
> + "hsic_set_connect failed, err=%d\n",
> + ret);
> + return ret;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> +}
> +
> static int ci_hdrc_imx_probe(struct platform_device *pdev)
> {
> struct ci_hdrc_imx_data *data;
> struct ci_hdrc_platform_data pdata = {
> .name = dev_name(&pdev->dev),
> .capoffset = DEF_CAPOFFSET,
> + .notify_event = ci_hdrc_imx_notify_event,
> };
> int ret;
> const struct of_device_id *of_id;
> const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
> struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct pinctrl_state *pinctrl_hsic_idle;
>
> - of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
> + of_id = of_match_device(ci_hdrc_imx_dt_ids, dev);
> if (!of_id)
> return -ENODEV;
>
> @@ -268,19 +310,48 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> platform_set_drvdata(pdev, data);
> - data->usbmisc_data = usbmisc_get_init_data(&pdev->dev);
> + data->usbmisc_data = usbmisc_get_init_data(dev);
> if (IS_ERR(data->usbmisc_data))
> return PTR_ERR(data->usbmisc_data);
>
> - ret = imx_get_clks(&pdev->dev);
> + data->pinctrl = devm_pinctrl_get(dev);
> + if (IS_ERR(data->pinctrl)) {
> + dev_dbg(dev, "pinctrl get failed, err=%ld\n",
> + PTR_ERR(data->pinctrl));
> + } else {
> + pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
> + if (IS_ERR(pinctrl_hsic_idle)) {
> + dev_dbg(dev,
> + "pinctrl_hsic_idle lookup failed, err=%ld\n",
> + PTR_ERR(pinctrl_hsic_idle));
> + } else {
> + ret = pinctrl_select_state(data->pinctrl,
> + pinctrl_hsic_idle);
> + if (ret) {
> + dev_err(dev,
> + "hsic_idle select failed, err=%d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
> + "active");
> + if (IS_ERR(data->pinctrl_hsic_active))
> + dev_dbg(dev,
> + "pinctrl_hsic_active lookup failed, err=%ld\n",
> + PTR_ERR(data->pinctrl_hsic_active));
> + }
In the paragraph above, I think we should make the pinctrl settings
mandatory in case of HSIC and error out if one of them is missing.
Also I think we could make the code more readable by removing the nested
conditions.
Maybe something like this would be better?
if (of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC) {
data->pinctrl = devm_pinctrl_get(dev);
if (IS_ERR(data->pinctrl)) {
dev_err(dev, "failed to get HSIC pinctrl, err=%ld\n",
PTR_ERR(data->pinctrl));
return PTR_ERR(data->pinctrl);
}
pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
if (IS_ERR(pinctrl_hsic_idle)) {
dev_err(dev, "failed to get HSIC idle pinctrl,"
"err=%ld\n", PTR_ERR(pinctrl_hsic_idle));
return PTR_ERR(pinctrl_hsic_idle);
}
ret = pinctrl_select_state(data->pinctrl, pinctrl_hsic_idle);
if (ret) {
dev_err(dev, "failed to select HSIC idle pinctrl,"
"err=%d\n", ret);
return ret;
}
data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
"active");
if (IS_ERR(data->pinctrl_hsic_active)) {
dev_err(dev, "failed to get HSIC active pinctrl,"
"err=%ld\n",
PTR_ERR(data->pinctrl_hsic_active));
return PTR_ERR(data->pinctrl_hsic_active);
}
}
> +
> + ret = imx_get_clks(dev);
> if (ret)
> return ret;
>
> - ret = imx_prepare_enable_clks(&pdev->dev);
> + ret = imx_prepare_enable_clks(dev);
> if (ret)
> return ret;
>
> - data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
> + data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0);
> if (IS_ERR(data->phy)) {
> ret = PTR_ERR(data->phy);
> /* Return -EINVAL if no usbphy is available */
> @@ -303,42 +374,72 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> data->supports_runtime_pm = true;
>
> + if (of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC) {
> + pdata.flags |= CI_HDRC_IMX_IS_HSIC;
> + data->usbmisc_data->hsic = 1;
> + data->hsic_pad_regulator = devm_regulator_get(dev, "hsic");
> + if (PTR_ERR(data->hsic_pad_regulator) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto err_clk;
> + } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV) {
> + /* no pad regualator is needed */
> + data->hsic_pad_regulator = NULL;
> + } else if (IS_ERR(data->hsic_pad_regulator)) {
> + dev_err(dev, "Get hsic pad regulator error: %ld\n",
> + PTR_ERR(data->hsic_pad_regulator));
> + ret = PTR_ERR(data->hsic_pad_regulator);
> + goto err_clk;
> + }
> +
> + if (data->hsic_pad_regulator) {
> + ret = regulator_enable(data->hsic_pad_regulator);
> + if (ret) {
> + dev_err(dev,
> + "Fail to enable hsic pad regulator\n");
> + goto err_clk;
> + }
> + }
> + }
> +
> ret = imx_usbmisc_init(data->usbmisc_data);
> if (ret) {
> - dev_err(&pdev->dev, "usbmisc init failed, ret=%d\n", ret);
> - goto err_clk;
> + dev_err(dev, "usbmisc init failed, ret=%d\n", ret);
> + goto disable_hsic_regulator;
> }
>
> - data->ci_pdev = ci_hdrc_add_device(&pdev->dev,
> + data->ci_pdev = ci_hdrc_add_device(dev,
> pdev->resource, pdev->num_resources,
> &pdata);
> if (IS_ERR(data->ci_pdev)) {
> ret = PTR_ERR(data->ci_pdev);
> if (ret != -EPROBE_DEFER)
> - dev_err(&pdev->dev,
> - "ci_hdrc_add_device failed, err=%d\n", ret);
> - goto err_clk;
> + dev_err(dev, "ci_hdrc_add_device failed, err=%d\n",
> + ret);
> + goto disable_hsic_regulator;
> }
>
> ret = imx_usbmisc_init_post(data->usbmisc_data);
> if (ret) {
> - dev_err(&pdev->dev, "usbmisc post failed, ret=%d\n", ret);
> + dev_err(dev, "usbmisc post failed, ret=%d\n", ret);
> goto disable_device;
> }
>
> if (data->supports_runtime_pm) {
> - pm_runtime_set_active(&pdev->dev);
> - pm_runtime_enable(&pdev->dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> }
>
> - device_set_wakeup_capable(&pdev->dev, true);
> + device_set_wakeup_capable(dev, true);
>
> return 0;
>
> disable_device:
> ci_hdrc_remove_device(data->ci_pdev);
> +disable_hsic_regulator:
> + if (data->hsic_pad_regulator)
> + ret = regulator_disable(data->hsic_pad_regulator);
> err_clk:
> - imx_disable_unprepare_clks(&pdev->dev);
> + imx_disable_unprepare_clks(dev);
> return ret;
> }
>
> @@ -355,6 +456,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
> if (data->override_phy_control)
> usb_phy_shutdown(data->phy);
> imx_disable_unprepare_clks(&pdev->dev);
> + if (data->hsic_pad_regulator)
> + regulator_disable(data->hsic_pad_regulator);
>
> return 0;
> }
> @@ -367,9 +470,19 @@ static void ci_hdrc_imx_shutdown(struct platform_device *pdev)
> static int __maybe_unused imx_controller_suspend(struct device *dev)
> {
> struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> + int ret = 0;
>
> dev_dbg(dev, "at %s\n", __func__);
>
> + if (data->usbmisc_data) {
Why do we have this check here, but not in imx_controller_resume()?
> + ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, false);
> + if (ret) {
> + dev_err(dev,
> + "usbmisc hsic_set_clk failed, ret=%d\n", ret);
> + return ret;
> + }
> + }
> +
> imx_disable_unprepare_clks(dev);
> data->in_lpm = true;
>
> @@ -400,8 +513,16 @@ static int __maybe_unused imx_controller_resume(struct device *dev)
> goto clk_disable;
> }
>
Why don't we have a check for data->usbmisc_data here, but in
imx_controller_suspend() we have one?
> + ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, true);
> + if (ret) {
> + dev_err(dev, "usbmisc hsic_set_clk failed, ret=%d\n", ret);
> + goto hsic_set_clk_fail;
> + }
> +
> return 0;
>
> +hsic_set_clk_fail:
> + imx_usbmisc_set_wakeup(data->usbmisc_data, true);
> clk_disable:
> imx_disable_unprepare_clks(dev);
> return ret;
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
> index 204275f47573..fcecab478934 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -14,10 +14,13 @@ struct imx_usbmisc_data {
> unsigned int oc_polarity:1; /* over current polarity if oc enabled */
> unsigned int evdo:1; /* set external vbus divider option */
> unsigned int ulpi:1; /* connected to an ULPI phy */
> + unsigned int hsic:1; /* HSIC controlller */
> };
>
> -int imx_usbmisc_init(struct imx_usbmisc_data *);
> -int imx_usbmisc_init_post(struct imx_usbmisc_data *);
> -int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *, bool);
> +int imx_usbmisc_init(struct imx_usbmisc_data *data);
> +int imx_usbmisc_init_post(struct imx_usbmisc_data *data);
> +int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled);
> +int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data);
> +int imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on);
>
> #endif /* __DRIVER_USB_CHIPIDEA_CI_HDRC_IMX_H */
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index def80ff547e4..a66a15075200 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -64,10 +64,22 @@
> #define MX6_BM_OVER_CUR_DIS BIT(7)
> #define MX6_BM_OVER_CUR_POLARITY BIT(8)
> #define MX6_BM_WAKEUP_ENABLE BIT(10)
> +#define MX6_BM_UTMI_ON_CLOCK BIT(13)
> #define MX6_BM_ID_WAKEUP BIT(16)
> #define MX6_BM_VBUS_WAKEUP BIT(17)
> #define MX6SX_BM_DPDM_WAKEUP_EN BIT(29)
> #define MX6_BM_WAKEUP_INTR BIT(31)
> +
> +#define MX6_USB_HSIC_CTRL_OFFSET 0x10
> +/* Send resume signal without 480Mhz PHY clock */
> +#define MX6SX_BM_HSIC_AUTO_RESUME BIT(23)
> +/* set before portsc.suspendM = 1 */
> +#define MX6_BM_HSIC_DEV_CONN BIT(21)
> +/* HSIC enable */
> +#define MX6_BM_HSIC_EN BIT(12)
> +/* Force HSIC module 480M clock on, even when in Host is in suspend mode */
> +#define MX6_BM_HSIC_CLK_ON BIT(11)
> +
> #define MX6_USB_OTG1_PHY_CTRL 0x18
> /* For imx6dql, it is host-only controller, for later imx6, it is otg's */
> #define MX6_USB_OTG2_PHY_CTRL 0x1c
> @@ -94,6 +106,10 @@ struct usbmisc_ops {
> int (*post)(struct imx_usbmisc_data *data);
> /* It's called when we need to enable/disable usb wakeup */
> int (*set_wakeup)(struct imx_usbmisc_data *data, bool enabled);
> + /* It's called before setting portsc.suspendM */
> + int (*hsic_set_connect)(struct imx_usbmisc_data *data);
> + /* It's called during suspend/resume */
> + int (*hsic_set_clk)(struct imx_usbmisc_data *data, bool enabled);
> };
>
> struct imx_usbmisc {
> @@ -353,6 +369,18 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
> writel(reg | MX6_BM_NON_BURST_SETTING,
> usbmisc->base + data->index * 4);
>
> + /* For HSIC controller */
> + if (data->hsic) {
> + reg = readl(usbmisc->base + data->index * 4);
> + writel(reg | MX6_BM_UTMI_ON_CLOCK,
> + usbmisc->base + data->index * 4);
> + reg = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET
> + + (data->index - 2) * 4);
> + reg |= MX6_BM_HSIC_EN | MX6_BM_HSIC_CLK_ON;
> + writel(reg, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET
> + + (data->index - 2) * 4);
> + }
> +
> spin_unlock_irqrestore(&usbmisc->lock, flags);
>
> usbmisc_imx6q_set_wakeup(data, false);
> @@ -360,6 +388,70 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
> return 0;
> }
>
> +static int usbmisc_imx6_hsic_set_connect(struct imx_usbmisc_data *data)
> +{
> + unsigned long flags;
> + u32 val, offset;
> + struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> + int ret = 0;
> +
> + spin_lock_irqsave(&usbmisc->lock, flags);
> + if (data->index == 2 || data->index == 3) {
> + offset = (data->index - 2) * 4;
> + } else if (data->index == 0) {
> + /*
> + * For controllers later than imx7d (imx7d is included),
"For SOCs like i.MX7D and later, ..."
> + * each controller has its own non core register region.
> + * And the controllers before than imx7d, the 1st controller
> + * is not HSIC controller.
"For SOCs before i.MX7D, the first USB controller is non-HSIC."
Thanks,
Frieder
> + */
> + offset = 0;
> + } else {
> + dev_err(data->dev, "index is error for usbmisc\n");
> + offset = 0;
> + ret = -EINVAL;
> + }
> +
> + val = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
> + if (!(val & MX6_BM_HSIC_DEV_CONN))
> + writel(val | MX6_BM_HSIC_DEV_CONN,
> + usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
> + spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> + return ret;
> +}
> +
> +static int usbmisc_imx6_hsic_set_clk(struct imx_usbmisc_data *data, bool on)
> +{
> + unsigned long flags;
> + u32 val, offset;
> + struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> + int ret = 0;
> +
> + spin_lock_irqsave(&usbmisc->lock, flags);
> + if (data->index == 2 || data->index == 3) {
> + offset = (data->index - 2) * 4;
> + } else if (data->index == 0) {
> + offset = 0;
> + } else {
> + dev_err(data->dev, "index is error for usbmisc\n");
> + offset = 0;
> + ret = -EINVAL;
> + }
> +
> + val = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
> + val |= MX6_BM_HSIC_EN | MX6_BM_HSIC_CLK_ON;
> + if (on)
> + val |= MX6_BM_HSIC_CLK_ON;
> + else
> + val &= ~MX6_BM_HSIC_CLK_ON;
> + writel(val, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET + offset);
> + spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> + return 0;
> +}
> +
> +
> static int usbmisc_imx6sx_init(struct imx_usbmisc_data *data)
> {
> void __iomem *reg = NULL;
> @@ -385,6 +477,13 @@ static int usbmisc_imx6sx_init(struct imx_usbmisc_data *data)
> spin_unlock_irqrestore(&usbmisc->lock, flags);
> }
>
> + /* For HSIC controller */
> + if (data->hsic) {
> + val = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET);
> + val |= MX6SX_BM_HSIC_AUTO_RESUME;
> + writel(val, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET);
> + }
> +
> return 0;
> }
>
> @@ -454,6 +553,7 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
> reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
> writel(reg | MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID,
> usbmisc->base + MX7D_USBNC_USB_CTRL2);
> +
> spin_unlock_irqrestore(&usbmisc->lock, flags);
>
> usbmisc_imx7d_set_wakeup(data, false);
> @@ -481,6 +581,8 @@ static const struct usbmisc_ops imx53_usbmisc_ops = {
> static const struct usbmisc_ops imx6q_usbmisc_ops = {
> .set_wakeup = usbmisc_imx6q_set_wakeup,
> .init = usbmisc_imx6q_init,
> + .hsic_set_connect = usbmisc_imx6_hsic_set_connect,
> + .hsic_set_clk = usbmisc_imx6_hsic_set_clk,
> };
>
> static const struct usbmisc_ops vf610_usbmisc_ops = {
> @@ -490,6 +592,8 @@ static const struct usbmisc_ops vf610_usbmisc_ops = {
> static const struct usbmisc_ops imx6sx_usbmisc_ops = {
> .set_wakeup = usbmisc_imx6q_set_wakeup,
> .init = usbmisc_imx6sx_init,
> + .hsic_set_connect = usbmisc_imx6_hsic_set_connect,
> + .hsic_set_clk = usbmisc_imx6_hsic_set_clk,
> };
>
> static const struct usbmisc_ops imx7d_usbmisc_ops = {
> @@ -546,6 +650,33 @@ int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled)
> }
> EXPORT_SYMBOL_GPL(imx_usbmisc_set_wakeup);
>
> +int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data)
> +{
> + struct imx_usbmisc *usbmisc;
> +
> + if (!data)
> + return 0;
> +
> + usbmisc = dev_get_drvdata(data->dev);
> + if (!usbmisc->ops->hsic_set_connect || !data->hsic)
> + return 0;
> + return usbmisc->ops->hsic_set_connect(data);
> +}
> +EXPORT_SYMBOL_GPL(imx_usbmisc_hsic_set_connect);
> +
> +int imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on)
> +{
> + struct imx_usbmisc *usbmisc;
> +
> + if (!data)
> + return 0;
> +
> + usbmisc = dev_get_drvdata(data->dev);
> + if (!usbmisc->ops->hsic_set_clk || !data->hsic)
> + return 0;
> + return usbmisc->ops->hsic_set_clk(data, on);
> +}
> +EXPORT_SYMBOL_GPL(imx_usbmisc_hsic_set_clk);
> static const struct of_device_id usbmisc_imx_dt_ids[] = {
> {
> .compatible = "fsl,imx25-usbmisc",
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [2/4] usb: chipidea: imx: add HSIC support
@ 2018-10-18 1:18 Peter Chen
0 siblings, 0 replies; 5+ messages in thread
From: Peter Chen @ 2018-10-18 1:18 UTC (permalink / raw)
To: Frieder Schrempf, linux-usb@vger.kernel.org
Cc: dl-linux-imx, robh+dt@kernel.org, devicetree@vger.kernel.org
> > +static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int
> > +event) {
> > + struct device *dev = ci->dev->parent;
> > + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > + int ret = 0;
> > +
> > + switch (event) {
> > + case CI_HDRC_IMX_HSIC_ACTIVE_EVENT:
> > + if (!IS_ERR(data->pinctrl) &&
> > + !IS_ERR(data->pinctrl_hsic_active)) {
>
> If we make the pinctrl mandatory in case of HSIC as proposed below, we don't need
> the checks here.
>
Will delete them
> > +
> > + data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
> > + "active");
> > + if (IS_ERR(data->pinctrl_hsic_active))
> > + dev_dbg(dev,
> > + "pinctrl_hsic_active lookup failed, err=%ld\n",
> > + PTR_ERR(data->pinctrl_hsic_active));
> > + }
>
> In the paragraph above, I think we should make the pinctrl settings mandatory in
> case of HSIC and error out if one of them is missing.
>
> Also I think we could make the code more readable by removing the nested
> conditions.
>
> Maybe something like this would be better?
>
> if (of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC)
> {
> data->pinctrl = devm_pinctrl_get(dev);
> if (IS_ERR(data->pinctrl)) {
> dev_err(dev, "failed to get HSIC pinctrl, err=%ld\n",
> PTR_ERR(data->pinctrl));
> return PTR_ERR(data->pinctrl);
> }
>
> pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
> if (IS_ERR(pinctrl_hsic_idle)) {
> dev_err(dev, "failed to get HSIC idle pinctrl,"
> "err=%ld\n", PTR_ERR(pinctrl_hsic_idle));
> return PTR_ERR(pinctrl_hsic_idle);
> }
>
> ret = pinctrl_select_state(data->pinctrl, pinctrl_hsic_idle);
> if (ret) {
> dev_err(dev, "failed to select HSIC idle pinctrl,"
> "err=%d\n", ret);
> return ret;
> }
>
> data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl,
> "active");
> if (IS_ERR(data->pinctrl_hsic_active)) {
> dev_err(dev, "failed to get HSIC active pinctrl,"
> "err=%ld\n",
> PTR_ERR(data->pinctrl_hsic_active));
> return PTR_ERR(data->pinctrl_hsic_active);
> }
> }
>
Good idea.
> > }
> > @@ -367,9 +470,19 @@ static void ci_hdrc_imx_shutdown(struct
> platform_device *pdev)
> > static int __maybe_unused imx_controller_suspend(struct device *dev)
> > {
> > struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > + int ret = 0;
> >
> > dev_dbg(dev, "at %s\n", __func__);
> >
> > + if (data->usbmisc_data) {
>
> Why do we have this check here, but not in imx_controller_resume()?
>
I will delete check here since there is NULL point check at imx_usbmisc_hsic_set_clk too.
> > + ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, false);
> > + if (ret) {
> > + dev_err(dev,
> > + "usbmisc hsic_set_clk failed, ret=%d\n", ret);
> > + return ret;
> > + }
> > + }
> > +
> > imx_disable_unprepare_clks(dev);
> > data->in_lpm = true;
> >
> > @@ -400,8 +513,16 @@ static int __maybe_unused
> imx_controller_resume(struct device *dev)
> > goto clk_disable;
> > }
> >
>
> Why don't we have a check for data->usbmisc_data here, but in
> imx_controller_suspend() we have one?
>
Yes, not need.
> > + ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, true);
> > + if (ret) {
> > + dev_err(dev, "usbmisc hsic_set_clk failed, ret=%d\n", ret);
> > + goto hsic_set_clk_fail;
> > + }
> > +
> > return 0;
> >
> > +hsic_set_clk_fail:
> > + imx_usbmisc_set_wakeup(data->usbmisc_data, true);
> > clk_disable:
> > imx_disable_unprepare_clks(dev);
> > return ret;
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h
> > b/drivers/usb/chipidea/ci_hdrc_imx.h
> > index 204275f47573..fcecab478934 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> > @@ -14,10 +14,13 @@ struct imx_usbmisc_data {
> > unsigned int oc_polarity:1; /* over current polarity if oc enabled */
> > unsigned int evdo:1; /* set external vbus divider option */
> > unsigned int ulpi:1; /* connected to an ULPI phy */
> > + unsigned int hsic:1; /* HSIC controlller */
> > };
> >
> > -int imx_usbmisc_init(struct imx_usbmisc_data *); -int
> > imx_usbmisc_init_post(struct imx_usbmisc_data *); -int
> > imx_usbmisc_set_wakeup(struct imx_usbmisc_data *, bool);
> > +int imx_usbmisc_init(struct imx_usbmisc_data *data); int
> > +imx_usbmisc_init_post(struct imx_usbmisc_data *data); int
> > +imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled);
> > +int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data); int
> > +imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on);
> >
> > #endif /* __DRIVER_USB_CHIPIDEA_CI_HDRC_IMX_H */ diff --git
> > a/drivers/usb/chipidea/usbmisc_imx.c
> > b/drivers/usb/chipidea/usbmisc_imx.c
> > index def80ff547e4..a66a15075200 100644
> > --- a/drivers/usb/chipidea/usbmisc_imx.c
> > +++ b/drivers/usb/chipidea/usbmisc_imx.c
> > @@ -64,10 +64,22 @@
> > #define MX6_BM_OVER_CUR_DIS BIT(7)
> > #define MX6_BM_OVER_CUR_POLARITY BIT(8)
> > #define MX6_BM_WAKEUP_ENABLE BIT(10)
> > +#define MX6_BM_UTMI_ON_CLOCK BIT(13)
> > #define MX6_BM_ID_WAKEUP BIT(16)
> > #define MX6_BM_VBUS_WAKEUP BIT(17)
> > #define MX6SX_BM_DPDM_WAKEUP_EN BIT(29)
> > #define MX6_BM_WAKEUP_INTR BIT(31)
> > +
> > +#define MX6_USB_HSIC_CTRL_OFFSET 0x10
> > +/* Send resume signal without 480Mhz PHY clock */
> > +#define MX6SX_BM_HSIC_AUTO_RESUME BIT(23)
> > +/* set before portsc.suspendM = 1 */
> > +#define MX6_BM_HSIC_DEV_CONN BIT(21)
> > +/* HSIC enable */
> > +#define MX6_BM_HSIC_EN BIT(12)
> > +/* Force HSIC module 480M clock on, even when in Host is in suspend mode */
> > +#define MX6_BM_HSIC_CLK_ON BIT(11)
> > +
> > #define MX6_USB_OTG1_PHY_CTRL 0x18
> > /* For imx6dql, it is host-only controller, for later imx6, it is otg's */
> > #define MX6_USB_OTG2_PHY_CTRL 0x1c
> > @@ -94,6 +106,10 @@ struct usbmisc_ops {
> > int (*post)(struct imx_usbmisc_data *data);
> > /* It's called when we need to enable/disable usb wakeup */
> > int (*set_wakeup)(struct imx_usbmisc_data *data, bool enabled);
> > + /* It's called before setting portsc.suspendM */
> > + int (*hsic_set_connect)(struct imx_usbmisc_data *data);
> > + /* It's called during suspend/resume */
> > + int (*hsic_set_clk)(struct imx_usbmisc_data *data, bool enabled);
> > };
> >
> > struct imx_usbmisc {
> > @@ -353,6 +369,18 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data
> *data)
> > writel(reg | MX6_BM_NON_BURST_SETTING,
> > usbmisc->base + data->index * 4);
> >
> > + /* For HSIC controller */
> > + if (data->hsic) {
> > + reg = readl(usbmisc->base + data->index * 4);
> > + writel(reg | MX6_BM_UTMI_ON_CLOCK,
> > + usbmisc->base + data->index * 4);
> > + reg = readl(usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET
> > + + (data->index - 2) * 4);
> > + reg |= MX6_BM_HSIC_EN | MX6_BM_HSIC_CLK_ON;
> > + writel(reg, usbmisc->base + MX6_USB_HSIC_CTRL_OFFSET
> > + + (data->index - 2) * 4);
> > + }
> > +
> > spin_unlock_irqrestore(&usbmisc->lock, flags);
> >
> > usbmisc_imx6q_set_wakeup(data, false); @@ -360,6 +388,70 @@ static
> > int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
> > return 0;
> > }
> >
> > +static int usbmisc_imx6_hsic_set_connect(struct imx_usbmisc_data
> > +*data) {
> > + unsigned long flags;
> > + u32 val, offset;
> > + struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
> > + int ret = 0;
> > +
> > + spin_lock_irqsave(&usbmisc->lock, flags);
> > + if (data->index == 2 || data->index == 3) {
> > + offset = (data->index - 2) * 4;
> > + } else if (data->index == 0) {
> > + /*
> > + * For controllers later than imx7d (imx7d is included),
>
> "For SOCs like i.MX7D and later, ..."
>
> > + * each controller has its own non core register region.
> > + * And the controllers before than imx7d, the 1st controller
> > + * is not HSIC controller.
>
> "For SOCs before i.MX7D, the first USB controller is non-HSIC."
>
I will change both.
Thanks.
Peter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-18 1:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-17 7:03 [2/4] usb: chipidea: imx: add HSIC support Frieder Schrempf
-- strict thread matches above, loose matches on Subject: below --
2018-10-18 1:18 Peter Chen
2018-10-16 6:07 Peter Chen
2018-10-16 5:52 kbuild test robot
2018-10-16 5:01 Peter Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).