diff mbox

[15/21] usb: chipidea: msm: Mux over secondary phy at the right time

Message ID 20160626072838.28082-16-stephen.boyd@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Boyd June 26, 2016, 7:28 a.m. UTC
We need to pick the correct phy at runtime based on how the SoC
has been wired onto the board. If the secondary phy is used, take
it out of reset and mux over to it by writing into the TCSR
register. Make sure to do this on reset too, because this
register is reset to the default value (primary phy) after the
RESET bit is set in USBCMD.

Cc: Peter Chen <peter.chen@nxp.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/usb/chipidea/ci_hdrc_msm.c | 78 +++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 5 deletions(-)

Comments

Bjorn Andersson June 28, 2016, 4:51 a.m. UTC | #1
On Sun 26 Jun 00:28 PDT 2016, Stephen Boyd wrote:

> We need to pick the correct phy at runtime based on how the SoC
> has been wired onto the board. If the secondary phy is used, take
> it out of reset and mux over to it by writing into the TCSR
> register. Make sure to do this on reset too, because this
> register is reset to the default value (primary phy) after the
> RESET bit is set in USBCMD.
> 
> Cc: Peter Chen <peter.chen@nxp.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/usb/chipidea/ci_hdrc_msm.c | 78 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 73 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
[..]
>  
> +static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> +			       struct platform_device *pdev)
> +{
> +	struct regmap *regmap;
> +	struct device_node *syscon;
> +	struct device *dev = &pdev->dev;
> +	u32 off, val;
> +	int ret;
> +
> +	syscon = of_parse_phandle(dev->of_node, "phy-select", 0);
> +	if (!syscon)
> +		return 0;
> +
> +	regmap = syscon_node_to_regmap(syscon);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	ret = of_property_read_u32_index(dev->of_node, "phy-select", 1, &off);
> +	if (ret < 0) {
> +		dev_err(dev, "no offset in syscon\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32_index(dev->of_node, "phy-select", 2, &val);
> +	if (ret < 0) {
> +		dev_err(dev, "no value in syscon\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_write(regmap, off, val);

I recently found out (thanks to a comment from Srinivas) that you can
drop the last two error checks by using
of_parse_phandle_with_fixed_args() as in:

  struct of_phandle_args args;

  ret = of_parse_phandle_with_fixed_args(dev->of_node, "phy-select", 2, 0, &args);
  if (ret < 0)
	...

  regmap = syscon_node_to_regmap(args.np);
  of_node_put(args.np);
  if (IS_ERR(regmap))
	...

  ret = regmap_write(regmap, args.args[0], args.args[1]);

> +	if (ret)
> +		return ret;
> +
> +	ci->secondary_phy = !!val;
> +	if (ci->secondary_phy) {
> +		val = readl_relaxed(ci->base + HS_PHY_SEC_CTRL);
> +		val |= HS_PHY_DIG_CLAMP_N;
> +		writel_relaxed(val, ci->base + HS_PHY_SEC_CTRL);
> +	}
> +
> +	return 0;
> +}
> +
>  static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  {
>  	struct ci_hdrc_msm *ci;
>  	struct platform_device *plat_ci;
>  	struct clk *clk;
>  	struct reset_control *reset;
> +	struct resource *res;
> +	void __iomem *base;

Doesn't look like you need "base".

> +	resource_size_t size;
>  	int ret;
>  
>  	dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> @@ -76,6 +132,15 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  	if (IS_ERR(clk))
>  		return PTR_ERR(clk);
>  
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	size = resource_size(res);
> +	ci->base = base = devm_ioremap(&pdev->dev, res->start, size);
> +	if (!base)
> +		return -ENOMEM;

Replace these two snippets with:

  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  ci->base = devm_ioremap_resource(&pdev->dev, res);
  if (IS_ERR(ci->base))
	return PTR_ERR(ci->base);

> +
>  	reset_control_assert(reset);
>  	usleep_range(10000, 12000);
>  	reset_control_deassert(reset);

Regards,
Bjorn
Stephen Boyd June 28, 2016, 8:39 a.m. UTC | #2
Quoting Bjorn Andersson (2016-06-27 21:51:37)
> On Sun 26 Jun 00:28 PDT 2016, Stephen Boyd wrote:
> > +static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> > +                            struct platform_device *pdev)
> > +{
> > +     struct regmap *regmap;
> > +     struct device_node *syscon;
> > +     struct device *dev = &pdev->dev;
> > +     u32 off, val;
> > +     int ret;
> > +
> > +     syscon = of_parse_phandle(dev->of_node, "phy-select", 0);
> > +     if (!syscon)
> > +             return 0;
> > +
> > +     regmap = syscon_node_to_regmap(syscon);
> > +     if (IS_ERR(regmap))
> > +             return PTR_ERR(regmap);
> > +
> > +     ret = of_property_read_u32_index(dev->of_node, "phy-select", 1, &off);
> > +     if (ret < 0) {
> > +             dev_err(dev, "no offset in syscon\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = of_property_read_u32_index(dev->of_node, "phy-select", 2, &val);
> > +     if (ret < 0) {
> > +             dev_err(dev, "no value in syscon\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = regmap_write(regmap, off, val);
> 
> I recently found out (thanks to a comment from Srinivas) that you can
> drop the last two error checks by using
> of_parse_phandle_with_fixed_args() as in:
> 
>   struct of_phandle_args args;
> 
>   ret = of_parse_phandle_with_fixed_args(dev->of_node, "phy-select", 2, 0, &args);
>   if (ret < 0)
>         ...
> 
>   regmap = syscon_node_to_regmap(args.np);
>   of_node_put(args.np);
>   if (IS_ERR(regmap))
>         ...
> 
>   ret = regmap_write(regmap, args.args[0], args.args[1]);

Awesome, thanks. I'll fold this in.

> > +     resource_size_t size;
> >       int ret;
> >  
> >       dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> > @@ -76,6 +132,15 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> >       if (IS_ERR(clk))
> >               return PTR_ERR(clk);
> >  
> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (!res)
> > +             return -ENODEV;
> > +
> > +     size = resource_size(res);
> > +     ci->base = base = devm_ioremap(&pdev->dev, res->start, size);
> > +     if (!base)
> > +             return -ENOMEM;
> 
> Replace these two snippets with:
> 
>   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   ci->base = devm_ioremap_resource(&pdev->dev, res);
>   if (IS_ERR(ci->base))
>         return PTR_ERR(ci->base);

Unfortunately I can't do that. I'm mapping the base here without
ioremap_resource() because the ci core is mapping it with
devm_ioremap_resource() and that doesn't allow two callers to map the
same address space. If the ci core was a library and not written as a
sub device driver this could be made to work assuming there was some API
to setup the mapping and then another API to do the rest of the ci core
probe.

Or I can add another event like SETUP that would allow me to mux the phy
over early during the ci device probe. I would still need to get a
handle on the registers for the extcon handler though, so that would
need a think.
Peter Chen June 29, 2016, 8:08 a.m. UTC | #3
On Sun, Jun 26, 2016 at 12:28:32AM -0700, Stephen Boyd wrote:
> We need to pick the correct phy at runtime based on how the SoC
> has been wired onto the board. If the secondary phy is used, take
> it out of reset and mux over to it by writing into the TCSR
> register. Make sure to do this on reset too, because this
> register is reset to the default value (primary phy) after the
> RESET bit is set in USBCMD.
> 

I am curious when you need the secondary phy?

> Cc: Peter Chen <peter.chen@nxp.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/usb/chipidea/ci_hdrc_msm.c | 78 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 73 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 40249b0e3e93..df0f8b31db4f 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -8,30 +8,40 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> -#include <linux/usb/gadget.h>
>  #include <linux/usb/chipidea.h>
>  #include <linux/clk.h>
>  #include <linux/reset.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/io.h>
>  
>  #include "ci.h"
>  
>  #define HS_PHY_AHB_MODE			0x0098
> +#define HS_PHY_SEC_CTRL			0x0278
> +# define HS_PHY_DIG_CLAMP_N		BIT(16)
>  

One space at the beginning, and keep alignment.

>  struct ci_hdrc_msm {
>  	struct platform_device *ci;
>  	struct clk *core_clk;
>  	struct clk *iface_clk;
> +	bool secondary_phy;
> +	void __iomem *base;
>  };
>  
>  static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
>  {
> -	struct device *dev = ci->gadget.dev.parent;
> +	struct device *dev = ci->dev->parent;
> +	struct ci_hdrc_msm *msm_ci = dev_get_drvdata(dev);
>  
>  	switch (event) {
>  	case CI_HDRC_CONTROLLER_RESET_EVENT:
>  		dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
>  		/* use AHB transactor, allow posted data writes */
>  		hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0xffffffff, 0x8);
> +		if (msm_ci->secondary_phy)
> +			hw_write_id_reg(ci, HS_PHY_SEC_CTRL, HS_PHY_DIG_CLAMP_N,
> +					HS_PHY_DIG_CLAMP_N);
>  		break;
>  	default:
>  		dev_dbg(dev, "unknown ci_hdrc event\n");
> @@ -49,12 +59,58 @@ static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = {
>  	.notify_event		= ci_hdrc_msm_notify_event,
>  };
>  
> +static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> +			       struct platform_device *pdev)
> +{
> +	struct regmap *regmap;
> +	struct device_node *syscon;
> +	struct device *dev = &pdev->dev;
> +	u32 off, val;
> +	int ret;
> +
> +	syscon = of_parse_phandle(dev->of_node, "phy-select", 0);
> +	if (!syscon)
> +		return 0;
> +
> +	regmap = syscon_node_to_regmap(syscon);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	ret = of_property_read_u32_index(dev->of_node, "phy-select", 1, &off);
> +	if (ret < 0) {
> +		dev_err(dev, "no offset in syscon\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32_index(dev->of_node, "phy-select", 2, &val);
> +	if (ret < 0) {
> +		dev_err(dev, "no value in syscon\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_write(regmap, off, val);
> +	if (ret)
> +		return ret;
> +
> +	ci->secondary_phy = !!val;
> +	if (ci->secondary_phy) {
> +		val = readl_relaxed(ci->base + HS_PHY_SEC_CTRL);
> +		val |= HS_PHY_DIG_CLAMP_N;
> +		writel_relaxed(val, ci->base + HS_PHY_SEC_CTRL);
> +	}
> +
> +	return 0;
> +}
> +
>  static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  {
>  	struct ci_hdrc_msm *ci;
>  	struct platform_device *plat_ci;
>  	struct clk *clk;
>  	struct reset_control *reset;
> +	struct resource *res;
> +	void __iomem *base;
> +	resource_size_t size;
>  	int ret;
>  
>  	dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
> @@ -76,6 +132,15 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  	if (IS_ERR(clk))
>  		return PTR_ERR(clk);
>  
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	size = resource_size(res);
> +	ci->base = base = devm_ioremap(&pdev->dev, res->start, size);
> +	if (!base)
> +		return -ENOMEM;
> +

The core will do the ioremap too, you can't remap io address two times.
The offset larger than 0x200 is vendor specific, you can map it as
the second io region.


>  	reset_control_assert(reset);
>  	usleep_range(10000, 12000);
>  	reset_control_deassert(reset);
> @@ -88,9 +153,12 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_iface;
>  
> -	plat_ci = ci_hdrc_add_device(&pdev->dev,
> -				pdev->resource, pdev->num_resources,
> -				&ci_hdrc_msm_platdata);
> +	ret = ci_hdrc_msm_mux_phy(ci, pdev);
> +	if (ret)
> +		goto err_mux;
> +
> +	plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> +				     pdev->num_resources, &ci_hdrc_msm_platdata);
>  	if (IS_ERR(plat_ci)) {
>  		dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
>  		ret = PTR_ERR(plat_ci);
> -- 
> 2.9.0.rc2.8.ga28705d
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Stephen Boyd June 30, 2016, 1:35 a.m. UTC | #4
Quoting Peter Chen (2016-06-29 01:08:52)
> On Sun, Jun 26, 2016 at 12:28:32AM -0700, Stephen Boyd wrote:
> > We need to pick the correct phy at runtime based on how the SoC
> > has been wired onto the board. If the secondary phy is used, take
> > it out of reset and mux over to it by writing into the TCSR
> > register. Make sure to do this on reset too, because this
> > register is reset to the default value (primary phy) after the
> > RESET bit is set in USBCMD.
> > 
> 
> I am curious when you need the secondary phy?

The msm8974 SoC has two phys that sit behind a mux. The primary phy is
typically used by the dwc3 instance for usb3, but it can also be used by
the chipidea controller if this mux is set to zero instead of one. It's
a board designer decision to use either the primary phy or the secondary
phy for the chipidea controller, but dwc3 can only use the primary phy
as it's the only HS phy it's connected to. I suppose if there's a board
out there that doesn't want to use the dwc3 instance because they don't
want USB3 they can use the primary phy for the chipidea instance and
keep the secondary phy powered off.
diff mbox

Patch

diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
index 40249b0e3e93..df0f8b31db4f 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -8,30 +8,40 @@ 
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
-#include <linux/usb/gadget.h>
 #include <linux/usb/chipidea.h>
 #include <linux/clk.h>
 #include <linux/reset.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/io.h>
 
 #include "ci.h"
 
 #define HS_PHY_AHB_MODE			0x0098
+#define HS_PHY_SEC_CTRL			0x0278
+# define HS_PHY_DIG_CLAMP_N		BIT(16)
 
 struct ci_hdrc_msm {
 	struct platform_device *ci;
 	struct clk *core_clk;
 	struct clk *iface_clk;
+	bool secondary_phy;
+	void __iomem *base;
 };
 
 static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
 {
-	struct device *dev = ci->gadget.dev.parent;
+	struct device *dev = ci->dev->parent;
+	struct ci_hdrc_msm *msm_ci = dev_get_drvdata(dev);
 
 	switch (event) {
 	case CI_HDRC_CONTROLLER_RESET_EVENT:
 		dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
 		/* use AHB transactor, allow posted data writes */
 		hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0xffffffff, 0x8);
+		if (msm_ci->secondary_phy)
+			hw_write_id_reg(ci, HS_PHY_SEC_CTRL, HS_PHY_DIG_CLAMP_N,
+					HS_PHY_DIG_CLAMP_N);
 		break;
 	default:
 		dev_dbg(dev, "unknown ci_hdrc event\n");
@@ -49,12 +59,58 @@  static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = {
 	.notify_event		= ci_hdrc_msm_notify_event,
 };
 
+static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
+			       struct platform_device *pdev)
+{
+	struct regmap *regmap;
+	struct device_node *syscon;
+	struct device *dev = &pdev->dev;
+	u32 off, val;
+	int ret;
+
+	syscon = of_parse_phandle(dev->of_node, "phy-select", 0);
+	if (!syscon)
+		return 0;
+
+	regmap = syscon_node_to_regmap(syscon);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	ret = of_property_read_u32_index(dev->of_node, "phy-select", 1, &off);
+	if (ret < 0) {
+		dev_err(dev, "no offset in syscon\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_index(dev->of_node, "phy-select", 2, &val);
+	if (ret < 0) {
+		dev_err(dev, "no value in syscon\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_write(regmap, off, val);
+	if (ret)
+		return ret;
+
+	ci->secondary_phy = !!val;
+	if (ci->secondary_phy) {
+		val = readl_relaxed(ci->base + HS_PHY_SEC_CTRL);
+		val |= HS_PHY_DIG_CLAMP_N;
+		writel_relaxed(val, ci->base + HS_PHY_SEC_CTRL);
+	}
+
+	return 0;
+}
+
 static int ci_hdrc_msm_probe(struct platform_device *pdev)
 {
 	struct ci_hdrc_msm *ci;
 	struct platform_device *plat_ci;
 	struct clk *clk;
 	struct reset_control *reset;
+	struct resource *res;
+	void __iomem *base;
+	resource_size_t size;
 	int ret;
 
 	dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
@@ -76,6 +132,15 @@  static int ci_hdrc_msm_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	size = resource_size(res);
+	ci->base = base = devm_ioremap(&pdev->dev, res->start, size);
+	if (!base)
+		return -ENOMEM;
+
 	reset_control_assert(reset);
 	usleep_range(10000, 12000);
 	reset_control_deassert(reset);
@@ -88,9 +153,12 @@  static int ci_hdrc_msm_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_iface;
 
-	plat_ci = ci_hdrc_add_device(&pdev->dev,
-				pdev->resource, pdev->num_resources,
-				&ci_hdrc_msm_platdata);
+	ret = ci_hdrc_msm_mux_phy(ci, pdev);
+	if (ret)
+		goto err_mux;
+
+	plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
+				     pdev->num_resources, &ci_hdrc_msm_platdata);
 	if (IS_ERR(plat_ci)) {
 		dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
 		ret = PTR_ERR(plat_ci);