[PATCH 1/2] usb: chipidea: imx: Cleanup ci_hdrc_imx_platform_flag

[PATCH 1/2] usb: chipidea: imx: Cleanup ci_hdrc_imx_platform_flag

[Sebastian Reichel]

Some trivial cleanups, that do not change functionality.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 3b45c25f296e..de155c80eb70 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -20,7 +20,6 @@
 
 struct ci_hdrc_imx_platform_flag {
 	unsigned int flags;
-	bool runtime_pm;
 };
 
 static const struct ci_hdrc_imx_platform_flag imx23_usb_data = {
@@ -29,7 +28,7 @@ static const struct ci_hdrc_imx_platform_flag imx23_usb_data = {
 };
 
 static const struct ci_hdrc_imx_platform_flag imx27_usb_data = {
-		CI_HDRC_DISABLE_STREAMING,
+	.flags = CI_HDRC_DISABLE_STREAMING,
 };
 
 static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
-- 
2.15.1

[1/2] usb: chipidea: imx: Cleanup ci_hdrc_imx_platform_flag

[Sebastian Reichel]

Some trivial cleanups, that do not change functionality.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 3b45c25f296e..de155c80eb70 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -20,7 +20,6 @@
 
 struct ci_hdrc_imx_platform_flag {
 	unsigned int flags;
-	bool runtime_pm;
 };
 
 static const struct ci_hdrc_imx_platform_flag imx23_usb_data = {
@@ -29,7 +28,7 @@ static const struct ci_hdrc_imx_platform_flag imx23_usb_data = {
 };
 
 static const struct ci_hdrc_imx_platform_flag imx27_usb_data = {
-		CI_HDRC_DISABLE_STREAMING,
+	.flags = CI_HDRC_DISABLE_STREAMING,
 };
 
 static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {

[PATCH 2/2] usb: chipidea: imx: Fix ULPI on imx53

[Sebastian Reichel]

Traditionally, PORTSC should be set before initializing ULPI phys. But
setting PORTSC before powering on the phy results in a kernel freeze
on imx53 based GE PPD. As a workaround this initializes the phy early
in the imx platform code and disables phy power management from the
core.

Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index de155c80eb70..e431c5aafe35 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
 	struct clk *clk;
 	struct imx_usbmisc_data *usbmisc_data;
 	bool supports_runtime_pm;
+	bool override_phy_control;
 	bool in_lpm;
 	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
 	bool need_three_clks;
@@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	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;
 
 	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
 	if (!of_id)
@@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	}
 
 	pdata.usb_phy = data->phy;
+
+	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
+	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
+		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
+		data->override_phy_control = true;
+		usb_phy_init(pdata.usb_phy);
+	}
+
 	pdata.flags |= imx_platform_flag->flags;
 	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
 		data->supports_runtime_pm = true;
@@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
 		pm_runtime_put_noidle(&pdev->dev);
 	}
 	ci_hdrc_remove_device(data->ci_pdev);
+	if (data->override_phy_control)
+		usb_phy_shutdown(data->phy);
 	imx_disable_unprepare_clks(&pdev->dev);
 
 	return 0;
-- 
2.15.1

[2/2] usb: chipidea: imx: Fix ULPI on imx53

[Sebastian Reichel]

Traditionally, PORTSC should be set before initializing ULPI phys. But
setting PORTSC before powering on the phy results in a kernel freeze
on imx53 based GE PPD. As a workaround this initializes the phy early
in the imx platform code and disables phy power management from the
core.

Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index de155c80eb70..e431c5aafe35 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
 	struct clk *clk;
 	struct imx_usbmisc_data *usbmisc_data;
 	bool supports_runtime_pm;
+	bool override_phy_control;
 	bool in_lpm;
 	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
 	bool need_three_clks;
@@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	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;
 
 	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
 	if (!of_id)
@@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	}
 
 	pdata.usb_phy = data->phy;
+
+	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
+	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
+		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
+		data->override_phy_control = true;
+		usb_phy_init(pdata.usb_phy);
+	}
+
 	pdata.flags |= imx_platform_flag->flags;
 	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
 		data->supports_runtime_pm = true;
@@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
 		pm_runtime_put_noidle(&pdev->dev);
 	}
 	ci_hdrc_remove_device(data->ci_pdev);
+	if (data->override_phy_control)
+		usb_phy_shutdown(data->phy);
 	imx_disable_unprepare_clks(&pdev->dev);
 
 	return 0;

Re: [PATCH 2/2] usb: chipidea: imx: Fix ULPI on imx53

[Peter Chen]

On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> Traditionally, PORTSC should be set before initializing ULPI phys. But
> setting PORTSC before powering on the phy results in a kernel freeze
> on imx53 based GE PPD. As a workaround this initializes the phy early
> in the imx platform code and disables phy power management from the
> core.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index de155c80eb70..e431c5aafe35 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
>  	struct clk *clk;
>  	struct imx_usbmisc_data *usbmisc_data;
>  	bool supports_runtime_pm;
> +	bool override_phy_control;
>  	bool in_lpm;
>  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
>  	bool need_three_clks;
> @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	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;
>  
>  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
>  	if (!of_id)
> @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	}
>  
>  	pdata.usb_phy = data->phy;
> +
> +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> +		data->override_phy_control = true;
> +		usb_phy_init(pdata.usb_phy);
> +	}
> +
>  	pdata.flags |= imx_platform_flag->flags;
>  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
>  		data->supports_runtime_pm = true;
> @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
>  		pm_runtime_put_noidle(&pdev->dev);
>  	}
>  	ci_hdrc_remove_device(data->ci_pdev);
> +	if (data->override_phy_control)
> +		usb_phy_shutdown(data->phy);
>  	imx_disable_unprepare_clks(&pdev->dev);
>  
>  	return 0;
> -- 

Applied both, thanks.

-- 

Best Regards,
Peter Chen

[2/2] usb: chipidea: imx: Fix ULPI on imx53

[Peter Chen]

On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> Traditionally, PORTSC should be set before initializing ULPI phys. But
> setting PORTSC before powering on the phy results in a kernel freeze
> on imx53 based GE PPD. As a workaround this initializes the phy early
> in the imx platform code and disables phy power management from the
> core.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index de155c80eb70..e431c5aafe35 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
>  	struct clk *clk;
>  	struct imx_usbmisc_data *usbmisc_data;
>  	bool supports_runtime_pm;
> +	bool override_phy_control;
>  	bool in_lpm;
>  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
>  	bool need_three_clks;
> @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	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;
>  
>  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
>  	if (!of_id)
> @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	}
>  
>  	pdata.usb_phy = data->phy;
> +
> +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> +		data->override_phy_control = true;
> +		usb_phy_init(pdata.usb_phy);
> +	}
> +
>  	pdata.flags |= imx_platform_flag->flags;
>  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
>  		data->supports_runtime_pm = true;
> @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
>  		pm_runtime_put_noidle(&pdev->dev);
>  	}
>  	ci_hdrc_remove_device(data->ci_pdev);
> +	if (data->override_phy_control)
> +		usb_phy_shutdown(data->phy);
>  	imx_disable_unprepare_clks(&pdev->dev);
>  
>  	return 0;
> -- 

Applied both, thanks.

Re: [PATCH 2/2] usb: chipidea: imx: Fix ULPI on imx53

[Peter Chen]

On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> Traditionally, PORTSC should be set before initializing ULPI phys. But
> setting PORTSC before powering on the phy results in a kernel freeze
> on imx53 based GE PPD. As a workaround this initializes the phy early
> in the imx platform code and disables phy power management from the
> core.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index de155c80eb70..e431c5aafe35 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
>  	struct clk *clk;
>  	struct imx_usbmisc_data *usbmisc_data;
>  	bool supports_runtime_pm;
> +	bool override_phy_control;
>  	bool in_lpm;
>  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
>  	bool need_three_clks;
> @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	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;
>  
>  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
>  	if (!of_id)
> @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	}
>  
>  	pdata.usb_phy = data->phy;
> +
> +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> +		data->override_phy_control = true;
> +		usb_phy_init(pdata.usb_phy);
> +	}
> +
>  	pdata.flags |= imx_platform_flag->flags;
>  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
>  		data->supports_runtime_pm = true;
> @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
>  		pm_runtime_put_noidle(&pdev->dev);
>  	}
>  	ci_hdrc_remove_device(data->ci_pdev);
> +	if (data->override_phy_control)
> +		usb_phy_shutdown(data->phy);
>  	imx_disable_unprepare_clks(&pdev->dev);
>  

Sebastian, I have a question, do you have any USB or generic PHY drivers
for ULPI bus, any power controls are needed for your ULPI peripheral?

-- 

Best Regards,
Peter Chen

[2/2] usb: chipidea: imx: Fix ULPI on imx53

[Peter Chen]

On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> Traditionally, PORTSC should be set before initializing ULPI phys. But
> setting PORTSC before powering on the phy results in a kernel freeze
> on imx53 based GE PPD. As a workaround this initializes the phy early
> in the imx platform code and disables phy power management from the
> core.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index de155c80eb70..e431c5aafe35 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
>  	struct clk *clk;
>  	struct imx_usbmisc_data *usbmisc_data;
>  	bool supports_runtime_pm;
> +	bool override_phy_control;
>  	bool in_lpm;
>  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
>  	bool need_three_clks;
> @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	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;
>  
>  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
>  	if (!of_id)
> @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	}
>  
>  	pdata.usb_phy = data->phy;
> +
> +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> +		data->override_phy_control = true;
> +		usb_phy_init(pdata.usb_phy);
> +	}
> +
>  	pdata.flags |= imx_platform_flag->flags;
>  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
>  		data->supports_runtime_pm = true;
> @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
>  		pm_runtime_put_noidle(&pdev->dev);
>  	}
>  	ci_hdrc_remove_device(data->ci_pdev);
> +	if (data->override_phy_control)
> +		usb_phy_shutdown(data->phy);
>  	imx_disable_unprepare_clks(&pdev->dev);
>  

Sebastian, I have a question, do you have any USB or generic PHY drivers
for ULPI bus, any power controls are needed for your ULPI peripheral?

Re: [PATCH 2/2] usb: chipidea: imx: Fix ULPI on imx53

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3867 bytes --]

Hi Peter,

On Mon, Jan 29, 2018 at 11:33:15AM +0800, Peter Chen wrote:
> On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> > Traditionally, PORTSC should be set before initializing ULPI phys. But
> > setting PORTSC before powering on the phy results in a kernel freeze
> > on imx53 based GE PPD. As a workaround this initializes the phy early
> > in the imx platform code and disables phy power management from the
> > core.
> > 
> > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index de155c80eb70..e431c5aafe35 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
> >  	struct clk *clk;
> >  	struct imx_usbmisc_data *usbmisc_data;
> >  	bool supports_runtime_pm;
> > +	bool override_phy_control;
> >  	bool in_lpm;
> >  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
> >  	bool need_three_clks;
> > @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  	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;
> >  
> >  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
> >  	if (!of_id)
> > @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	pdata.usb_phy = data->phy;
> > +
> > +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> > +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> > +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> > +		data->override_phy_control = true;
> > +		usb_phy_init(pdata.usb_phy);
> > +	}
> > +
> >  	pdata.flags |= imx_platform_flag->flags;
> >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> >  		data->supports_runtime_pm = true;
> > @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
> >  		pm_runtime_put_noidle(&pdev->dev);
> >  	}
> >  	ci_hdrc_remove_device(data->ci_pdev);
> > +	if (data->override_phy_control)
> > +		usb_phy_shutdown(data->phy);
> >  	imx_disable_unprepare_clks(&pdev->dev);
> >  
> 
> Sebastian, I have a question, do you have any USB or generic PHY drivers
> for ULPI bus, any power controls are needed for your ULPI peripheral?

The devicetree for GE PPD is available in the mainline kernel:

$ grep -A9 "usbphy[23] {" arch/arm/boot/dts/imx53-ppd.dts
	usbphy2: usbphy2 {
		compatible = "usb-nop-xceiv";
		reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>;
		clock-names = "main_clk";
		clock-frequency = <24000000>;
		clocks = <&clks IMX5_CLK_CKO2>;
		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
	};

	usbphy3: usbphy3 {
		compatible = "usb-nop-xceiv";
		reset-gpios = <&gpio2 19 GPIO_ACTIVE_LOW>;
		clock-names = "main_clk";

		clock-frequency = <24000000>;
		clocks = <&clks IMX5_CLK_CKO2>;
		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
	};

So currently the machine only uses drivers/usb/phy/phy-generic.c. Both
USB phys are actually SMSC USB3315, which is also detected by the kernel:

root@csmon :~# cat /sys/bus/ulpi/devices/ci_hdrc.*.ulpi/uevent 
DEVTYPE=ulpi_device
MODALIAS=ulpi:v0424p0006
DEVTYPE=ulpi_device
MODALIAS=ulpi:v0424p0006

So maybe drivers/usb/phy/phy-ulpi.c should be used, but I don't see
a simple way to do so and using the generic PHY works.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[2/2] usb: chipidea: imx: Fix ULPI on imx53

[Sebastian Reichel]

Hi Peter,

On Mon, Jan 29, 2018 at 11:33:15AM +0800, Peter Chen wrote:
> On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> > Traditionally, PORTSC should be set before initializing ULPI phys. But
> > setting PORTSC before powering on the phy results in a kernel freeze
> > on imx53 based GE PPD. As a workaround this initializes the phy early
> > in the imx platform code and disables phy power management from the
> > core.
> > 
> > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index de155c80eb70..e431c5aafe35 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
> >  	struct clk *clk;
> >  	struct imx_usbmisc_data *usbmisc_data;
> >  	bool supports_runtime_pm;
> > +	bool override_phy_control;
> >  	bool in_lpm;
> >  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
> >  	bool need_three_clks;
> > @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  	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;
> >  
> >  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
> >  	if (!of_id)
> > @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	pdata.usb_phy = data->phy;
> > +
> > +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> > +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> > +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> > +		data->override_phy_control = true;
> > +		usb_phy_init(pdata.usb_phy);
> > +	}
> > +
> >  	pdata.flags |= imx_platform_flag->flags;
> >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> >  		data->supports_runtime_pm = true;
> > @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
> >  		pm_runtime_put_noidle(&pdev->dev);
> >  	}
> >  	ci_hdrc_remove_device(data->ci_pdev);
> > +	if (data->override_phy_control)
> > +		usb_phy_shutdown(data->phy);
> >  	imx_disable_unprepare_clks(&pdev->dev);
> >  
> 
> Sebastian, I have a question, do you have any USB or generic PHY drivers
> for ULPI bus, any power controls are needed for your ULPI peripheral?

The devicetree for GE PPD is available in the mainline kernel:

$ grep -A9 "usbphy[23] {" arch/arm/boot/dts/imx53-ppd.dts
	usbphy2: usbphy2 {
		compatible = "usb-nop-xceiv";
		reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>;
		clock-names = "main_clk";
		clock-frequency = <24000000>;
		clocks = <&clks IMX5_CLK_CKO2>;
		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
	};

	usbphy3: usbphy3 {
		compatible = "usb-nop-xceiv";
		reset-gpios = <&gpio2 19 GPIO_ACTIVE_LOW>;
		clock-names = "main_clk";

		clock-frequency = <24000000>;
		clocks = <&clks IMX5_CLK_CKO2>;
		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
	};

So currently the machine only uses drivers/usb/phy/phy-generic.c. Both
USB phys are actually SMSC USB3315, which is also detected by the kernel:

root@csmon :~# cat /sys/bus/ulpi/devices/ci_hdrc.*.ulpi/uevent 
DEVTYPE=ulpi_device
MODALIAS=ulpi:v0424p0006
DEVTYPE=ulpi_device
MODALIAS=ulpi:v0424p0006

So maybe drivers/usb/phy/phy-ulpi.c should be used, but I don't see
a simple way to do so and using the generic PHY works.

-- Sebastian

Re: [PATCH 2/2] usb: chipidea: imx: Fix ULPI on imx53

[Peter Chen]

On Tue, Feb 06, 2018 at 04:50:41PM +0100, Sebastian Reichel wrote:
> Hi Peter,
> 
> On Mon, Jan 29, 2018 at 11:33:15AM +0800, Peter Chen wrote:
> > On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> > > Traditionally, PORTSC should be set before initializing ULPI phys. But
> > > setting PORTSC before powering on the phy results in a kernel freeze
> > > on imx53 based GE PPD. As a workaround this initializes the phy early
> > > in the imx platform code and disables phy power management from the
> > > core.
> > > 
> > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > > ---
> > >  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > index de155c80eb70..e431c5aafe35 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
> > >  	struct clk *clk;
> > >  	struct imx_usbmisc_data *usbmisc_data;
> > >  	bool supports_runtime_pm;
> > > +	bool override_phy_control;
> > >  	bool in_lpm;
> > >  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
> > >  	bool need_three_clks;
> > > @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  	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;
> > >  
> > >  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
> > >  	if (!of_id)
> > > @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  	}
> > >  
> > >  	pdata.usb_phy = data->phy;
> > > +
> > > +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> > > +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> > > +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> > > +		data->override_phy_control = true;
> > > +		usb_phy_init(pdata.usb_phy);
> > > +	}
> > > +
> > >  	pdata.flags |= imx_platform_flag->flags;
> > >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> > >  		data->supports_runtime_pm = true;
> > > @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
> > >  		pm_runtime_put_noidle(&pdev->dev);
> > >  	}
> > >  	ci_hdrc_remove_device(data->ci_pdev);
> > > +	if (data->override_phy_control)
> > > +		usb_phy_shutdown(data->phy);
> > >  	imx_disable_unprepare_clks(&pdev->dev);
> > >  
> > 
> > Sebastian, I have a question, do you have any USB or generic PHY drivers
> > for ULPI bus, any power controls are needed for your ULPI peripheral?
> 
> The devicetree for GE PPD is available in the mainline kernel:
> 
> $ grep -A9 "usbphy[23] {" arch/arm/boot/dts/imx53-ppd.dts
> 	usbphy2: usbphy2 {
> 		compatible = "usb-nop-xceiv";
> 		reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>;
> 		clock-names = "main_clk";
> 		clock-frequency = <24000000>;
> 		clocks = <&clks IMX5_CLK_CKO2>;
> 		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
> 		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
> 	};
> 
> 	usbphy3: usbphy3 {
> 		compatible = "usb-nop-xceiv";
> 		reset-gpios = <&gpio2 19 GPIO_ACTIVE_LOW>;
> 		clock-names = "main_clk";
> 
> 		clock-frequency = <24000000>;
> 		clocks = <&clks IMX5_CLK_CKO2>;
> 		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
> 		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
> 	};
> 
> So currently the machine only uses drivers/usb/phy/phy-generic.c. Both
> USB phys are actually SMSC USB3315, which is also detected by the kernel:
> 
> root@csmon :~# cat /sys/bus/ulpi/devices/ci_hdrc.*.ulpi/uevent 
> DEVTYPE=ulpi_device
> MODALIAS=ulpi:v0424p0006
> DEVTYPE=ulpi_device
> MODALIAS=ulpi:v0424p0006
> 
> So maybe drivers/usb/phy/phy-ulpi.c should be used, but I don't see
> a simple way to do so and using the generic PHY works.
> 

It is correct you use phy-generic.c if it can let your design
work, thanks.

-- 

Best Regards,
Peter Chen

[2/2] usb: chipidea: imx: Fix ULPI on imx53

[Peter Chen]

On Tue, Feb 06, 2018 at 04:50:41PM +0100, Sebastian Reichel wrote:
> Hi Peter,
> 
> On Mon, Jan 29, 2018 at 11:33:15AM +0800, Peter Chen wrote:
> > On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> > > Traditionally, PORTSC should be set before initializing ULPI phys. But
> > > setting PORTSC before powering on the phy results in a kernel freeze
> > > on imx53 based GE PPD. As a workaround this initializes the phy early
> > > in the imx platform code and disables phy power management from the
> > > core.
> > > 
> > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > > ---
> > >  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > index de155c80eb70..e431c5aafe35 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
> > >  	struct clk *clk;
> > >  	struct imx_usbmisc_data *usbmisc_data;
> > >  	bool supports_runtime_pm;
> > > +	bool override_phy_control;
> > >  	bool in_lpm;
> > >  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
> > >  	bool need_three_clks;
> > > @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  	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;
> > >  
> > >  	of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
> > >  	if (!of_id)
> > > @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
> > >  	}
> > >  
> > >  	pdata.usb_phy = data->phy;
> > > +
> > > +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> > > +	    of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) {
> > > +		pdata.flags |= CI_HDRC_OVERRIDE_PHY_CONTROL;
> > > +		data->override_phy_control = true;
> > > +		usb_phy_init(pdata.usb_phy);
> > > +	}
> > > +
> > >  	pdata.flags |= imx_platform_flag->flags;
> > >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> > >  		data->supports_runtime_pm = true;
> > > @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
> > >  		pm_runtime_put_noidle(&pdev->dev);
> > >  	}
> > >  	ci_hdrc_remove_device(data->ci_pdev);
> > > +	if (data->override_phy_control)
> > > +		usb_phy_shutdown(data->phy);
> > >  	imx_disable_unprepare_clks(&pdev->dev);
> > >  
> > 
> > Sebastian, I have a question, do you have any USB or generic PHY drivers
> > for ULPI bus, any power controls are needed for your ULPI peripheral?
> 
> The devicetree for GE PPD is available in the mainline kernel:
> 
> $ grep -A9 "usbphy[23] {" arch/arm/boot/dts/imx53-ppd.dts
> 	usbphy2: usbphy2 {
> 		compatible = "usb-nop-xceiv";
> 		reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>;
> 		clock-names = "main_clk";
> 		clock-frequency = <24000000>;
> 		clocks = <&clks IMX5_CLK_CKO2>;
> 		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
> 		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
> 	};
> 
> 	usbphy3: usbphy3 {
> 		compatible = "usb-nop-xceiv";
> 		reset-gpios = <&gpio2 19 GPIO_ACTIVE_LOW>;
> 		clock-names = "main_clk";
> 
> 		clock-frequency = <24000000>;
> 		clocks = <&clks IMX5_CLK_CKO2>;
> 		assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
> 		assigned-clock-parents = <&clks IMX5_CLK_OSC>;
> 	};
> 
> So currently the machine only uses drivers/usb/phy/phy-generic.c. Both
> USB phys are actually SMSC USB3315, which is also detected by the kernel:
> 
> root@csmon :~# cat /sys/bus/ulpi/devices/ci_hdrc.*.ulpi/uevent 
> DEVTYPE=ulpi_device
> MODALIAS=ulpi:v0424p0006
> DEVTYPE=ulpi_device
> MODALIAS=ulpi:v0424p0006
> 
> So maybe drivers/usb/phy/phy-ulpi.c should be used, but I don't see
> a simple way to do so and using the generic PHY works.
> 

It is correct you use phy-generic.c if it can let your design
work, thanks.