* [PATCH v4 1/3] usb: chipidea: imx: Change switch order
2016-09-19 11:45 [PATCH v4 0/3] usb: chipidea: imx: Add USB configuration for imx53 Fabien Lahoudere
@ 2016-09-19 11:45 ` Fabien Lahoudere
2016-09-21 6:57 ` Peter Chen
2016-09-19 11:45 ` [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy Fabien Lahoudere
2016-09-19 11:45 ` [PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock Fabien Lahoudere
2 siblings, 1 reply; 9+ messages in thread
From: Fabien Lahoudere @ 2016-09-19 11:45 UTC (permalink / raw)
To: hzpeterchen; +Cc: linux-usb, linux-kernel, Fabien Lahoudere
Each USB controller have different behaviour, so in order to avoid to have
several "swicth(data->index)" and lock/unlock, we prefer to get the index
switch and then test for features if they exist for this index.
This patch also remove useless test of reg and val. Those two values cannot
be NULL.
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
drivers/usb/chipidea/usbmisc_imx.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 20d02a5..9549821 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
val |= MX53_USB_PLL_DIV_24_MHZ;
writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET);
- if (data->disable_oc) {
- spin_lock_irqsave(&usbmisc->lock, flags);
- switch (data->index) {
- case 0:
+ spin_lock_irqsave(&usbmisc->lock, flags);
+
+ switch (data->index) {
+ case 0:
+ if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
- break;
- case 1:
+ writel(val, reg);
+ }
+ break;
+ case 1:
+ if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
- break;
- case 2:
+ writel(val, reg);
+ }
+ break;
+ case 2:
+ if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
- break;
- case 3:
+ writel(val, reg);
+ }
+ break;
+ case 3:
+ if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
- break;
- }
- if (reg && val)
writel(val, reg);
- spin_unlock_irqrestore(&usbmisc->lock, flags);
+ }
+ break;
}
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 1/3] usb: chipidea: imx: Change switch order
2016-09-19 11:45 ` [PATCH v4 1/3] usb: chipidea: imx: Change switch order Fabien Lahoudere
@ 2016-09-21 6:57 ` Peter Chen
2016-09-21 7:51 ` Fabien Lahoudere
0 siblings, 1 reply; 9+ messages in thread
From: Peter Chen @ 2016-09-21 6:57 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: linux-usb, linux-kernel
On Mon, Sep 19, 2016 at 01:45:38PM +0200, Fabien Lahoudere wrote:
> Each USB controller have different behaviour, so in order to avoid to have
> several "swicth(data->index)" and lock/unlock, we prefer to get the index
> switch and then test for features if they exist for this index.
> This patch also remove useless test of reg and val. Those two values cannot
> be NULL.
Sorry, I can't see the benefits of this change.
Considering certain controller doesn't have oc feature, with current
code it only needs one comparison (flag disable_oc), but with your
changes, it needs two comparisons.
Peter
>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
> drivers/usb/chipidea/usbmisc_imx.c | 38 ++++++++++++++++++++++++--------------
> 1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 20d02a5..9549821 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> val |= MX53_USB_PLL_DIV_24_MHZ;
> writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET);
>
> - if (data->disable_oc) {
> - spin_lock_irqsave(&usbmisc->lock, flags);
> - switch (data->index) {
> - case 0:
> + spin_lock_irqsave(&usbmisc->lock, flags);
> +
> + switch (data->index) {
> + case 0:
> + if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
> - break;
> - case 1:
> + writel(val, reg);
> + }
> + break;
> + case 1:
> + if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
> - break;
> - case 2:
> + writel(val, reg);
> + }
> + break;
> + case 2:
> + if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> - break;
> - case 3:
> + writel(val, reg);
> + }
> + break;
> + case 3:
> + if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> - break;
> - }
> - if (reg && val)
> writel(val, reg);
> - spin_unlock_irqrestore(&usbmisc->lock, flags);
> + }
> + break;
> }
>
> + spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> return 0;
> }
>
> --
> 2.1.4
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v4 1/3] usb: chipidea: imx: Change switch order
2016-09-21 6:57 ` Peter Chen
@ 2016-09-21 7:51 ` Fabien Lahoudere
0 siblings, 0 replies; 9+ messages in thread
From: Fabien Lahoudere @ 2016-09-21 7:51 UTC (permalink / raw)
To: Peter Chen; +Cc: linux-usb, linux-kernel
Hi,
On 21/09/16 08:57, Peter Chen wrote:
> On Mon, Sep 19, 2016 at 01:45:38PM +0200, Fabien Lahoudere wrote:
>> Each USB controller have different behaviour, so in order to avoid to have
>> several "swicth(data->index)" and lock/unlock, we prefer to get the index
>> switch and then test for features if they exist for this index.
>> This patch also remove useless test of reg and val. Those two values cannot
>> be NULL.
>
> Sorry, I can't see the benefits of this change.
> Considering certain controller doesn't have oc feature, with current
> code it only needs one comparison (flag disable_oc), but with your
> changes, it needs two comparisons.
The benefits are visible with next patches only but you ask me to split
it, so I do.
Thanks
Fabien
>
> Peter
>>
>> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
>> ---
>> drivers/usb/chipidea/usbmisc_imx.c | 38 ++++++++++++++++++++++++--------------
>> 1 file changed, 24 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
>> index 20d02a5..9549821 100644
>> --- a/drivers/usb/chipidea/usbmisc_imx.c
>> +++ b/drivers/usb/chipidea/usbmisc_imx.c
>> @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>> val |= MX53_USB_PLL_DIV_24_MHZ;
>> writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET);
>>
>> - if (data->disable_oc) {
>> - spin_lock_irqsave(&usbmisc->lock, flags);
>> - switch (data->index) {
>> - case 0:
>> + spin_lock_irqsave(&usbmisc->lock, flags);
>> +
>> + switch (data->index) {
>> + case 0:
>> + if (data->disable_oc) {
>> reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
>> val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
>> - break;
>> - case 1:
>> + writel(val, reg);
>> + }
>> + break;
>> + case 1:
>> + if (data->disable_oc) {
>> reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
>> val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
>> - break;
>> - case 2:
>> + writel(val, reg);
>> + }
>> + break;
>> + case 2:
>> + if (data->disable_oc) {
>> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
>> - break;
>> - case 3:
>> + writel(val, reg);
>> + }
>> + break;
>> + case 3:
>> + if (data->disable_oc) {
>> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
>> - break;
>> - }
>> - if (reg && val)
>> writel(val, reg);
>> - spin_unlock_irqrestore(&usbmisc->lock, flags);
>> + }
>> + break;
>> }
>>
>> + spin_unlock_irqrestore(&usbmisc->lock, flags);
>> +
>> return 0;
>> }
>>
>> --
>> 2.1.4
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy
2016-09-19 11:45 [PATCH v4 0/3] usb: chipidea: imx: Add USB configuration for imx53 Fabien Lahoudere
2016-09-19 11:45 ` [PATCH v4 1/3] usb: chipidea: imx: Change switch order Fabien Lahoudere
@ 2016-09-19 11:45 ` Fabien Lahoudere
2016-09-21 7:12 ` Peter Chen
2016-09-19 11:45 ` [PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock Fabien Lahoudere
2 siblings, 1 reply; 9+ messages in thread
From: Fabien Lahoudere @ 2016-09-19 11:45 UTC (permalink / raw)
To: hzpeterchen; +Cc: linux-usb, linux-kernel, Fabien Lahoudere
In order to use ULPI phy with usb host 2 and 3, we need to configure
controller register to enable ULPI features.
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
drivers/usb/chipidea/usbmisc_imx.c | 37 +++++++++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 0991794..96c0e33 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -18,6 +18,7 @@
#include <linux/pm_runtime.h>
#include <linux/dma-mapping.h>
#include <linux/usb/chipidea.h>
+#include <linux/usb/of.h>
#include <linux/clk.h>
#include "ci.h"
@@ -146,6 +147,10 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
if (of_find_property(np, "external-vbus-divider", NULL))
data->evdo = 1;
+
+ if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
+ data->ulpi = 1;
+
return data;
}
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 409aa5ca8..d666c9f 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -19,6 +19,7 @@ struct imx_usbmisc_data {
unsigned int disable_oc:1; /* over current detect disabled */
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 */
};
int imx_usbmisc_init(struct imx_usbmisc_data *);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 9549821..11f51bd 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -46,11 +46,20 @@
#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
#define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c
+#define MX53_USB_CTRL_1_OFFSET 0x10
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
+#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
+#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
#define MX53_USB_UH2_CTRL_OFFSET 0x14
#define MX53_USB_UH3_CTRL_OFFSET 0x18
#define MX53_BM_OVER_CUR_DIS_H1 BIT(5)
#define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
#define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
+#define MX53_USB_CTRL_1_UH2_ULPI_EN BIT(26)
+#define MX53_USB_CTRL_1_UH3_ULPI_EN BIT(27)
+#define MX53_USB_UHx_CTRL_WAKE_UP_EN BIT(7)
+#define MX53_USB_UHx_CTRL_ULPI_INT_EN BIT(8)
#define MX53_USB_PHYCTRL1_PLLDIV_MASK 0x3
#define MX53_USB_PLL_DIV_24_MHZ 0x01
@@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
}
break;
case 2:
+ if (data->ulpi) {
+ /* set USBH2 into ULPI-mode. */
+ reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
+ val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
+ /* select ULPI clock */
+ val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
+ val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
+ writel(val, reg);
+ /* Set interrupt wake up enable */
+ reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
+ val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
+ | MX53_USB_UHx_CTRL_ULPI_INT_EN;
+ writel(val, reg);
+ }
if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
@@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
}
break;
case 3:
+ if (data->ulpi) {
+ /* set USBH3 into ULPI-mode. */
+ reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
+ val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
+ /* select ULPI clock */
+ val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
+ val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
+ writel(val, reg);
+ /* Set interrupt wake up enable */
+ reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
+ val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
+ | MX53_USB_UHx_CTRL_ULPI_INT_EN;
+ writel(val, reg);
+ }
if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy
2016-09-19 11:45 ` [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy Fabien Lahoudere
@ 2016-09-21 7:12 ` Peter Chen
2016-09-21 7:54 ` Fabien Lahoudere
0 siblings, 1 reply; 9+ messages in thread
From: Peter Chen @ 2016-09-21 7:12 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: linux-usb, linux-kernel
On Mon, Sep 19, 2016 at 01:45:39PM +0200, Fabien Lahoudere wrote:
> In order to use ULPI phy with usb host 2 and 3, we need to configure
> controller register to enable ULPI features.
>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
> drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
> drivers/usb/chipidea/usbmisc_imx.c | 37 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 0991794..96c0e33 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -18,6 +18,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/dma-mapping.h>
> #include <linux/usb/chipidea.h>
> +#include <linux/usb/of.h>
> #include <linux/clk.h>
>
> #include "ci.h"
> @@ -146,6 +147,10 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
> if (of_find_property(np, "external-vbus-divider", NULL))
> data->evdo = 1;
>
> +
> + if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
> + data->ulpi = 1;
> +
> return data;
> }
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
> index 409aa5ca8..d666c9f 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -19,6 +19,7 @@ struct imx_usbmisc_data {
> unsigned int disable_oc:1; /* over current detect disabled */
> 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 */
> };
>
> int imx_usbmisc_init(struct imx_usbmisc_data *);
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 9549821..11f51bd 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -46,11 +46,20 @@
>
> #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
> #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c
> +#define MX53_USB_CTRL_1_OFFSET 0x10
> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
> #define MX53_USB_UH2_CTRL_OFFSET 0x14
> #define MX53_USB_UH3_CTRL_OFFSET 0x18
> #define MX53_BM_OVER_CUR_DIS_H1 BIT(5)
> #define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
> #define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
> +#define MX53_USB_CTRL_1_UH2_ULPI_EN BIT(26)
> +#define MX53_USB_CTRL_1_UH3_ULPI_EN BIT(27)
> +#define MX53_USB_UHx_CTRL_WAKE_UP_EN BIT(7)
> +#define MX53_USB_UHx_CTRL_ULPI_INT_EN BIT(8)
> #define MX53_USB_PHYCTRL1_PLLDIV_MASK 0x3
> #define MX53_USB_PLL_DIV_24_MHZ 0x01
>
> @@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> }
> break;
> case 2:
> + if (data->ulpi) {
> + /* set USBH2 into ULPI-mode. */
> + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
> + val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
> + /* select ULPI clock */
> + val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
> + val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
> + writel(val, reg);
> + /* Set interrupt wake up enable */
> + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
> + | MX53_USB_UHx_CTRL_ULPI_INT_EN;
> + writel(val, reg);
> + }
Ok, it seems your 1st patch is just let code structure change be easy
in this one, and I may give the wrong comments that you had improved
disable oc code before.
If you agree with me, would you squash these two into one, and send
again. (delete "reg && val" is necessary).
Peter
> if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> @@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> }
> break;
> case 3:
> + if (data->ulpi) {
> + /* set USBH3 into ULPI-mode. */
> + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
> + val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
> + /* select ULPI clock */
> + val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
> + val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
> + writel(val, reg);
> + /* Set interrupt wake up enable */
> + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
> + | MX53_USB_UHx_CTRL_ULPI_INT_EN;
> + writel(val, reg);
> + }
> if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> --
> 2.1.4
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy
2016-09-21 7:12 ` Peter Chen
@ 2016-09-21 7:54 ` Fabien Lahoudere
0 siblings, 0 replies; 9+ messages in thread
From: Fabien Lahoudere @ 2016-09-21 7:54 UTC (permalink / raw)
To: Peter Chen; +Cc: linux-usb, linux-kernel
Hi,
On 21/09/16 09:12, Peter Chen wrote:
> On Mon, Sep 19, 2016 at 01:45:39PM +0200, Fabien Lahoudere wrote:
>> In order to use ULPI phy with usb host 2 and 3, we need to configure
>> controller register to enable ULPI features.
>>
>> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
>> ---
>> drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
>> drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
>> drivers/usb/chipidea/usbmisc_imx.c | 37 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
>> index 0991794..96c0e33 100644
>> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
>> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
>> @@ -18,6 +18,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/usb/chipidea.h>
>> +#include <linux/usb/of.h>
>> #include <linux/clk.h>
>>
>> #include "ci.h"
>> @@ -146,6 +147,10 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
>> if (of_find_property(np, "external-vbus-divider", NULL))
>> data->evdo = 1;
>>
>> +
>> + if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
>> + data->ulpi = 1;
>> +
>> return data;
>> }
>>
>> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
>> index 409aa5ca8..d666c9f 100644
>> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
>> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
>> @@ -19,6 +19,7 @@ struct imx_usbmisc_data {
>> unsigned int disable_oc:1; /* over current detect disabled */
>> 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 */
>> };
>>
>> int imx_usbmisc_init(struct imx_usbmisc_data *);
>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
>> index 9549821..11f51bd 100644
>> --- a/drivers/usb/chipidea/usbmisc_imx.c
>> +++ b/drivers/usb/chipidea/usbmisc_imx.c
>> @@ -46,11 +46,20 @@
>>
>> #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
>> #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c
>> +#define MX53_USB_CTRL_1_OFFSET 0x10
>> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
>> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
>> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
>> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
>> #define MX53_USB_UH2_CTRL_OFFSET 0x14
>> #define MX53_USB_UH3_CTRL_OFFSET 0x18
>> #define MX53_BM_OVER_CUR_DIS_H1 BIT(5)
>> #define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
>> #define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
>> +#define MX53_USB_CTRL_1_UH2_ULPI_EN BIT(26)
>> +#define MX53_USB_CTRL_1_UH3_ULPI_EN BIT(27)
>> +#define MX53_USB_UHx_CTRL_WAKE_UP_EN BIT(7)
>> +#define MX53_USB_UHx_CTRL_ULPI_INT_EN BIT(8)
>> #define MX53_USB_PHYCTRL1_PLLDIV_MASK 0x3
>> #define MX53_USB_PLL_DIV_24_MHZ 0x01
>>
>> @@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>> }
>> break;
>> case 2:
>> + if (data->ulpi) {
>> + /* set USBH2 into ULPI-mode. */
>> + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
>> + val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
>> + /* select ULPI clock */
>> + val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
>> + val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
>> + writel(val, reg);
>> + /* Set interrupt wake up enable */
>> + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>> + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
>> + | MX53_USB_UHx_CTRL_ULPI_INT_EN;
>> + writel(val, reg);
>> + }
>
> Ok, it seems your 1st patch is just let code structure change be easy
> in this one, and I may give the wrong comments that you had improved
> disable oc code before.
>
> If you agree with me, would you squash these two into one, and send
> again. (delete "reg && val" is necessary).
ok I will squash. =-D
>
> Peter
>> if (data->disable_oc) {
>> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
>> @@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>> }
>> break;
>> case 3:
>> + if (data->ulpi) {
>> + /* set USBH3 into ULPI-mode. */
>> + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
>> + val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
>> + /* select ULPI clock */
>> + val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
>> + val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
>> + writel(val, reg);
>> + /* Set interrupt wake up enable */
>> + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>> + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
>> + | MX53_USB_UHx_CTRL_ULPI_INT_EN;
>> + writel(val, reg);
>> + }
>> if (data->disable_oc) {
>> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
>> --
>> 2.1.4
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
2016-09-19 11:45 [PATCH v4 0/3] usb: chipidea: imx: Add USB configuration for imx53 Fabien Lahoudere
2016-09-19 11:45 ` [PATCH v4 1/3] usb: chipidea: imx: Change switch order Fabien Lahoudere
2016-09-19 11:45 ` [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy Fabien Lahoudere
@ 2016-09-19 11:45 ` Fabien Lahoudere
2016-09-21 7:19 ` Peter Chen
2 siblings, 1 reply; 9+ messages in thread
From: Fabien Lahoudere @ 2016-09-19 11:45 UTC (permalink / raw)
To: hzpeterchen; +Cc: linux-usb, linux-kernel, Fabien Lahoudere
This binding allow to disable the internal 60Mhz clock for USB host2 and
host3.
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++
drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
drivers/usb/chipidea/usbmisc_imx.c | 13 +++++++++++++
3 files changed, 16 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 96c0e33..89a9d98 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -147,6 +147,8 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
if (of_find_property(np, "external-vbus-divider", NULL))
data->evdo = 1;
+ if (of_find_property(np, "disable-int60ck", NULL))
+ data->disable_int60ck = 1;
if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
data->ulpi = 1;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index d666c9f..43bafae 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -20,6 +20,7 @@ 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 disable_int60ck:1; /* disable 60 MHZ clock */
};
int imx_usbmisc_init(struct imx_usbmisc_data *);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 11f51bd..a781f87 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -53,6 +53,9 @@
#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
#define MX53_USB_UH2_CTRL_OFFSET 0x14
#define MX53_USB_UH3_CTRL_OFFSET 0x18
+#define MX53_USB_CLKONOFF_CTRL_OFFSET 0x24
+#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21)
+#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22)
#define MX53_BM_OVER_CUR_DIS_H1 BIT(5)
#define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
#define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
@@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
| MX53_USB_UHx_CTRL_ULPI_INT_EN;
writel(val, reg);
}
+ if (data->disable_int60ck) {
+ reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
+ val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF;
+ writel(val, reg);
+ }
if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
@@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
| MX53_USB_UHx_CTRL_ULPI_INT_EN;
writel(val, reg);
}
+ if (data->disable_int60ck) {
+ reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
+ val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF;
+ writel(val, reg);
+ }
if (data->disable_oc) {
reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
2016-09-19 11:45 ` [PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock Fabien Lahoudere
@ 2016-09-21 7:19 ` Peter Chen
0 siblings, 0 replies; 9+ messages in thread
From: Peter Chen @ 2016-09-21 7:19 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: linux-usb, linux-kernel
On Mon, Sep 19, 2016 at 01:45:40PM +0200, Fabien Lahoudere wrote:
> This binding allow to disable the internal 60Mhz clock for USB host2 and
> host3.
>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++
> drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
> drivers/usb/chipidea/usbmisc_imx.c | 13 +++++++++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 96c0e33..89a9d98 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -147,6 +147,8 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
> if (of_find_property(np, "external-vbus-divider", NULL))
> data->evdo = 1;
>
> + if (of_find_property(np, "disable-int60ck", NULL))
> + data->disable_int60ck = 1;
>
> if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
> data->ulpi = 1;
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
> index d666c9f..43bafae 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -20,6 +20,7 @@ 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 disable_int60ck:1; /* disable 60 MHZ clock */
> };
>
> int imx_usbmisc_init(struct imx_usbmisc_data *);
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 11f51bd..a781f87 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -53,6 +53,9 @@
> #define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
> #define MX53_USB_UH2_CTRL_OFFSET 0x14
> #define MX53_USB_UH3_CTRL_OFFSET 0x18
> +#define MX53_USB_CLKONOFF_CTRL_OFFSET 0x24
> +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21)
> +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22)
> #define MX53_BM_OVER_CUR_DIS_H1 BIT(5)
> #define MX53_BM_OVER_CUR_DIS_OTG BIT(8)
> #define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
> @@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> | MX53_USB_UHx_CTRL_ULPI_INT_EN;
> writel(val, reg);
> }
> + if (data->disable_int60ck) {
> + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF;
> + writel(val, reg);
> + }
> if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> @@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> | MX53_USB_UHx_CTRL_ULPI_INT_EN;
> writel(val, reg);
> }
> + if (data->disable_int60ck) {
> + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET;
> + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF;
> + writel(val, reg);
> + }
> if (data->disable_oc) {
> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> --
> 2.1.4
>
Like I commented before, you need to have binding-doc update
(ocumentation/devicetree/bindings/usb/ci-hdrc-usb2.txt)
Besides, try to run ./scripts/get_maintainer.pl to get the
necessary maintainers and reviewers. You may get some
useful tips for reading Documentation/SubmittingPatches
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 9+ messages in thread