| Message ID | 20160626072838.28082-16-stephen.boyd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
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 --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);
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(-)