Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1431357 > unrolled thread
| Started by | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| First post | 2016-06-26 09:30 +0200 |
| Last post | 2016-07-05 21:40 +0200 |
| Articles | 20 on this page of 62 — 9 participants |
Back to article view | Back to linux.kernel
[PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:30 +0200
[PATCH 13/21] usb: chipidea: msm: Allow core to get usb phy Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:30 +0200
Re: [PATCH 13/21] usb: chipidea: msm: Allow core to get usb phy Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 09:00 +0200
Re: [PATCH 13/21] usb: chipidea: msm: Allow core to get usb phy Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 13:50 +0200
Re: [PATCH 13/21] usb: chipidea: msm: Allow core to get usb phy Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 04:00 +0200
[PATCH 09/21] usb: chipidea: Add support for ULPI PHY bus Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 09/21] usb: chipidea: Add support for ULPI PHY bus Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 08:40 +0200
[PATCH 14/21] usb: chipidea: msm: Add proper clk and reset support Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 14/21] usb: chipidea: msm: Add proper clk and reset support Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 09:10 +0200
[PATCH 07/21] usb: chipidea: Notify of reset when switching into host mode Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
[PATCH 06/21] usb: chipidea: Initialize and reinitialize phy later Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 06/21] usb: chipidea: Initialize and reinitialize phy later Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 04:40 +0200
Re: [PATCH 06/21] usb: chipidea: Initialize and reinitialize phy later Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 03:30 +0200
[PATCH 19/21] usb: chipidea: msm: Be silent on probe defer errors Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 19/21] usb: chipidea: msm: Be silent on probe defer errors Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 03:30 +0200
[PATCH 03/21] usb: ulpi: Avoid reading/writing in device creation with OF devices Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
[PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy Neil Armstrong <narmstrong@baylibre.com> - 2016-06-28 10:50 +0200
Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-29 00:10 +0200
Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy Neil Armstrong <narmstrong@baylibre.com> - 2016-06-29 11:20 +0200
Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-29 21:00 +0200
[PATCH 05/21] usb: chipidea: Handle extcon events properly Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 05/21] usb: chipidea: Handle extcon events properly Peter Chen <hzpeterchen@gmail.com> - 2016-06-28 12:10 +0200
[PATCH 08/21] usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 08/21] usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 05:20 +0200
Re: [PATCH 08/21] usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 03:40 +0200
RE: [PATCH 08/21] usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon Jun Li <jun.li@nxp.com> - 2016-06-30 04:10 +0200
[PATCH 18/21] usb: chipidea: msm: Add reset controller for PHY POR bit Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 18/21] usb: chipidea: msm: Add reset controller for PHY POR bit kbuild test robot <lkp@intel.com> - 2016-06-27 05:50 +0200
Re: [PATCH 18/21] usb: chipidea: msm: Add reset controller for PHY POR bit kbuild test robot <lkp@intel.com> - 2016-06-27 07:00 +0200
Re: [PATCH 18/21] usb: chipidea: msm: Add reset controller for PHY POR bit kbuild test robot <lkp@intel.com> - 2016-06-27 10:00 +0200
Re: [PATCH 18/21] usb: chipidea: msm: Add reset controller for PHY POR bit Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 14:00 +0200
[PATCH 02/21] usb: ulpi: Support device discovery via DT Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT kbuild test robot <lkp@intel.com> - 2016-06-27 06:20 +0200
Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-27 16:40 +0200
Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-28 13:50 +0200
Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 04:10 +0200
Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT Rob Herring <robh@kernel.org> - 2016-06-28 23:00 +0200
Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT Rob Herring <robh@kernel.org> - 2016-07-01 03:10 +0200
[PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-06-28 07:00 +0200
Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 10:20 +0200
Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 04:00 +0200
[PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 10:40 +0200
Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 11:10 +0200
Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-30 18:30 +0200
[PATCH 11/21] usb: chipidea: msm: Use hw_write_id_reg() instead of writel directly Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 11/21] usb: chipidea: msm: Use hw_write_id_reg() instead of writel directly Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 08:50 +0200
[PATCH 21/21] phy: Add support for Qualcomm's USB HS phy Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
[PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global Peter Chen <hzpeterchen@gmail.com> - 2016-06-29 13:40 +0200
Re: [PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global Peter Chen <hzpeterchen@gmail.com> - 2016-06-30 11:20 +0200
[PATCH 01/21] of: device: Support loading a module with OF based modalias Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
Re: [PATCH 01/21] of: device: Support loading a module with OF based modalias Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-06-28 06:20 +0200
[PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-26 09:40 +0200
RE: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place Jun Li <jun.li@nxp.com> - 2016-06-27 10:10 +0200
RE: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place Stephen Boyd <stephen.boyd@linaro.org> - 2016-06-27 21:10 +0200
Re: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place Peter Chen <hzpeterchen@gmail.com> - 2016-06-28 11:50 +0200
Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support John Stultz <john.stultz@linaro.org> - 2016-06-28 05:10 +0200
Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support John Stultz <john.stultz@linaro.org> - 2016-07-02 08:10 +0200
Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support John Stultz <john.stultz@linaro.org> - 2016-07-05 21:40 +0200
Page 3 of 4 — ← Prev page 1 2 [3] 4 Next page →
| From | Bjorn Andersson <bjorn.andersson@linaro.org> |
|---|---|
| Date | 2016-06-28 07:00 +0200 |
| Subject | Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time |
| Message-ID | <rOThT-25m-19@gated-at.bofh.it> |
| In reply to | #1431370 |
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
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-29 10:20 +0200 |
| Subject | Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time |
| Message-ID | <rPiT0-1Lg-29@gated-at.bofh.it> |
| In reply to | #1431370 |
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
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-30 04:00 +0200 |
| Subject | Re: [PATCH 15/21] usb: chipidea: msm: Mux over secondary phy at the right time |
| Message-ID | <rPzqO-3jh-19@gated-at.bofh.it> |
| In reply to | #1433543 |
On Wed, Jun 29, 2016 at 12:28:58PM -0700, Stephen Boyd wrote:
> 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?
> >
> > > 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.
>
> You can ioremap an address twice, but it's not a great solution. On ARM,
> at least, we detect the double mapping and return the same virtual
> address the second time.
>
> > The offset larger than 0x200 is vendor specific, you can map it as
> > the second io region.
>
> Ok. That would mean we need to adjust the binding then to have to reg
> properties?
Just using two regs at dts, the second one is dedicated for glue layer.
> Or we can limit the size of the resource in the core and the
> size of the resource here can be bumped up by 0x200. So DT still says
> one resource for the entire ci address space but we don't map anything
> more than what we use. Something like this?
>
> ---8<----
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 4c70fa6fdc34..1121cf3e3fdc 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -190,8 +190,9 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> if (!res)
> return -ENODEV;
>
> - size = resource_size(res);
> - ci->base = base = devm_ioremap(&pdev->dev, res->start, size);
> + res->start += 0x200;
> + res->end -= 0x200;
> + ci->base = base = devm_ioremap_resource(&pdev->dev, res);
> if (!base)
> return -ENOMEM;
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 298029a9ffce..8f5782913b11 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -887,6 +887,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (resource_size(res) > 0x200)
> + res->end = res->start + 0x200;
> base = devm_ioremap_resource(dev, res);
> if (IS_ERR(base))
> return PTR_ERR(base);
>
No, don't do it. Keep the core unchanging, and map the second regs at
glue layer.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-26 09:40 +0200 |
| Subject | [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset |
| Message-ID | <rOcPE-8ip-25@gated-at.bofh.it> |
| In reply to | #1431357 |
When the RESET bit is set in the USBCMD register it resets quite
a few of the wrapper's registers to their reset state. This
includes the GENCONFIG and GENCONFIG2 registers. Currently this
is done by the usb phy and ehci-msm drivers writing into the
controller wrapper's MMIO address space. Let's consolidate the
register writes into the wrapper driver instead so that we
clearly split the wrapper from the phys.
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 | 46 ++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
index df0f8b31db4f..cc6f9b0df9d5 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -14,6 +14,8 @@
#include <linux/mfd/syscon.h>
#include <linux/regmap.h>
#include <linux/io.h>
+#include <linux/extcon.h>
+#include <linux/of.h>
#include "ci.h"
@@ -21,11 +23,22 @@
#define HS_PHY_SEC_CTRL 0x0278
# define HS_PHY_DIG_CLAMP_N BIT(16)
+#define HS_PHY_GENCONFIG 0x009c
+# define HS_PHY_TXFIFO_IDLE_FORCE_DIS BIT(4)
+
+#define HS_PHY_GENCONFIG_2 0x00a0
+# define HS_PHY_SESS_VLD_CTRL_EN BIT(7)
+# define HS_PHY_ULPI_TX_PKT_EN_CLR_FIX BIT(19)
+
+#define HSPHY_SESS_VLD_CTRL BIT(25)
+
struct ci_hdrc_msm {
struct platform_device *ci;
struct clk *core_clk;
struct clk *iface_clk;
+ struct extcon_dev *vbus_edev;
bool secondary_phy;
+ bool hsic;
void __iomem *base;
};
@@ -39,9 +52,26 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned 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);
+ /* workaround for rx buffer collision issue */
+ hw_write_id_reg(ci, HS_PHY_GENCONFIG,
+ HS_PHY_TXFIFO_IDLE_FORCE_DIS, 0);
+
if (msm_ci->secondary_phy)
hw_write_id_reg(ci, HS_PHY_SEC_CTRL, HS_PHY_DIG_CLAMP_N,
HS_PHY_DIG_CLAMP_N);
+
+ if (!msm_ci->hsic)
+ hw_write_id_reg(ci, HS_PHY_GENCONFIG_2,
+ HS_PHY_ULPI_TX_PKT_EN_CLR_FIX, 0);
+
+ if (msm_ci->vbus_edev) {
+ hw_write_id_reg(ci, HS_PHY_GENCONFIG_2,
+ HS_PHY_SESS_VLD_CTRL_EN,
+ HS_PHY_SESS_VLD_CTRL_EN);
+ hw_write(ci, OP_USBCMD, HSPHY_SESS_VLD_CTRL,
+ HSPHY_SESS_VLD_CTRL);
+
+ }
break;
default:
dev_dbg(dev, "unknown ci_hdrc event\n");
@@ -112,6 +142,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
void __iomem *base;
resource_size_t size;
int ret;
+ struct device_node *ulpi_node, *phy_node;
dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
@@ -141,6 +172,13 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
if (!base)
return -ENOMEM;
+ ci->vbus_edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
+ if (IS_ERR(ci->vbus_edev)) {
+ if (PTR_ERR(ci->vbus_edev) != -ENODEV)
+ return PTR_ERR(ci->vbus_edev);
+ ci->vbus_edev = NULL;
+ }
+
reset_control_assert(reset);
usleep_range(10000, 12000);
reset_control_deassert(reset);
@@ -157,6 +195,14 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
if (ret)
goto err_mux;
+ ulpi_node = of_find_node_by_name(pdev->dev.of_node, "ulpi");
+ if (ulpi_node) {
+ phy_node = of_get_next_available_child(ulpi_node, NULL);
+ ci->hsic = of_device_is_compatible(phy_node, "qcom,usb-hsic-phy");
+ of_node_put(phy_node);
+ }
+ of_node_put(ulpi_node);
+
plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
pdev->num_resources, &ci_hdrc_msm_platdata);
if (IS_ERR(plat_ci)) {
--
2.9.0.rc2.8.ga28705d
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-29 10:40 +0200 |
| Subject | Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset |
| Message-ID | <rPjcm-1RY-39@gated-at.bofh.it> |
| In reply to | #1431371 |
On Sun, Jun 26, 2016 at 12:28:33AM -0700, Stephen Boyd wrote:
> When the RESET bit is set in the USBCMD register it resets quite
> a few of the wrapper's registers to their reset state. This
> includes the GENCONFIG and GENCONFIG2 registers. Currently this
> is done by the usb phy and ehci-msm drivers writing into the
> controller wrapper's MMIO address space. Let's consolidate the
> register writes into the wrapper driver instead so that we
> clearly split the wrapper from the phys.
>
> 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 | 46 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index df0f8b31db4f..cc6f9b0df9d5 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -14,6 +14,8 @@
> #include <linux/mfd/syscon.h>
> #include <linux/regmap.h>
> #include <linux/io.h>
> +#include <linux/extcon.h>
> +#include <linux/of.h>
>
> #include "ci.h"
>
> @@ -21,11 +23,22 @@
> #define HS_PHY_SEC_CTRL 0x0278
> # define HS_PHY_DIG_CLAMP_N BIT(16)
>
> +#define HS_PHY_GENCONFIG 0x009c
> +# define HS_PHY_TXFIFO_IDLE_FORCE_DIS BIT(4)
> +
> +#define HS_PHY_GENCONFIG_2 0x00a0
> +# define HS_PHY_SESS_VLD_CTRL_EN BIT(7)
> +# define HS_PHY_ULPI_TX_PKT_EN_CLR_FIX BIT(19)
> +
> +#define HSPHY_SESS_VLD_CTRL BIT(25)
> +
Keep alignment please.
> struct ci_hdrc_msm {
> struct platform_device *ci;
> struct clk *core_clk;
> struct clk *iface_clk;
> + struct extcon_dev *vbus_edev;
> bool secondary_phy;
> + bool hsic;
> void __iomem *base;
> };
>
> @@ -39,9 +52,26 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned 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);
> + /* workaround for rx buffer collision issue */
> + hw_write_id_reg(ci, HS_PHY_GENCONFIG,
> + HS_PHY_TXFIFO_IDLE_FORCE_DIS, 0);
> +
> if (msm_ci->secondary_phy)
> hw_write_id_reg(ci, HS_PHY_SEC_CTRL, HS_PHY_DIG_CLAMP_N,
> HS_PHY_DIG_CLAMP_N);
> +
> + if (!msm_ci->hsic)
> + hw_write_id_reg(ci, HS_PHY_GENCONFIG_2,
> + HS_PHY_ULPI_TX_PKT_EN_CLR_FIX, 0);
> +
> + if (msm_ci->vbus_edev) {
> + hw_write_id_reg(ci, HS_PHY_GENCONFIG_2,
> + HS_PHY_SESS_VLD_CTRL_EN,
> + HS_PHY_SESS_VLD_CTRL_EN);
> + hw_write(ci, OP_USBCMD, HSPHY_SESS_VLD_CTRL,
> + HSPHY_SESS_VLD_CTRL);
> +
> + }
> break;
> default:
> dev_dbg(dev, "unknown ci_hdrc event\n");
> @@ -112,6 +142,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> void __iomem *base;
> resource_size_t size;
> int ret;
> + struct device_node *ulpi_node, *phy_node;
>
> dev_dbg(&pdev->dev, "ci_hdrc_msm_probe\n");
>
> @@ -141,6 +172,13 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> if (!base)
> return -ENOMEM;
>
> + ci->vbus_edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> + if (IS_ERR(ci->vbus_edev)) {
> + if (PTR_ERR(ci->vbus_edev) != -ENODEV)
> + return PTR_ERR(ci->vbus_edev);
> + ci->vbus_edev = NULL;
> + }
> +
Why not using ci->platdata->vbus_extcon directly?
> reset_control_assert(reset);
> usleep_range(10000, 12000);
> reset_control_deassert(reset);
> @@ -157,6 +195,14 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> if (ret)
> goto err_mux;
>
> + ulpi_node = of_find_node_by_name(pdev->dev.of_node, "ulpi");
> + if (ulpi_node) {
> + phy_node = of_get_next_available_child(ulpi_node, NULL);
> + ci->hsic = of_device_is_compatible(phy_node, "qcom,usb-hsic-phy");
> + of_node_put(phy_node);
> + }
> + of_node_put(ulpi_node);
> +
Just confirm with you that ci->platdata->phy_mode is not enough?
> plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> pdev->num_resources, &ci_hdrc_msm_platdata);
> if (IS_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
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-30 11:10 +0200 |
| Subject | Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset |
| Message-ID | <rPG8V-7It-3@gated-at.bofh.it> |
| In reply to | #1433567 |
On Wed, Jun 29, 2016 at 12:13:45PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-06-29 01:26:48)
> > On Sun, Jun 26, 2016 at 12:28:33AM -0700, Stephen Boyd wrote:
> > > When the RESET bit is set in the USBCMD register it resets quite
> > > @@ -21,11 +23,22 @@
> > > #define HS_PHY_SEC_CTRL 0x0278
> > > # define HS_PHY_DIG_CLAMP_N BIT(16)
> > >
> > > +#define HS_PHY_GENCONFIG 0x009c
> > > +# define HS_PHY_TXFIFO_IDLE_FORCE_DIS BIT(4)
> > > +
> > > +#define HS_PHY_GENCONFIG_2 0x00a0
> > > +# define HS_PHY_SESS_VLD_CTRL_EN BIT(7)
> > > +# define HS_PHY_ULPI_TX_PKT_EN_CLR_FIX BIT(19)
> > > +
> > > +#define HSPHY_SESS_VLD_CTRL BIT(25)
> > > +
> >
> > Keep alignment please.
>
> I take it this means it should look like:
>
> #define HS_PHY_GENCONFIG
> #define HS_PHY_TXFIFO_IDLE_FORCE_DIS
>
> ?
>
Yes
> > > @@ -141,6 +172,13 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> > > if (!base)
> > > return -ENOMEM;
> > >
> > > + ci->vbus_edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> > > + if (IS_ERR(ci->vbus_edev)) {
> > > + if (PTR_ERR(ci->vbus_edev) != -ENODEV)
> > > + return PTR_ERR(ci->vbus_edev);
> > > + ci->vbus_edev = NULL;
> > > + }
> > > +
> >
> > Why not using ci->platdata->vbus_extcon directly?
>
> Because ci->platdata->vbus_extcon is assigned after the child platform
> driver probes, and we have no idea when that will happen from the
> ci_hdrc_msm driver probe. If we try after ci_hdrc_add_device() we'll
> race with the driver probe and only get the pointer sometimes.
ci->platdata->vbus_extcon->edev is assigned at ci_get_platdata which is
called before ci core device is created.
>
> >
> > > reset_control_assert(reset);
> > > usleep_range(10000, 12000);
> > > reset_control_deassert(reset);
> > > @@ -157,6 +195,14 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> > > if (ret)
> > > goto err_mux;
> > >
> > > + ulpi_node = of_find_node_by_name(pdev->dev.of_node, "ulpi");
> > > + if (ulpi_node) {
> > > + phy_node = of_get_next_available_child(ulpi_node, NULL);
> > > + ci->hsic = of_device_is_compatible(phy_node, "qcom,usb-hsic-phy");
> > > + of_node_put(phy_node);
> > > + }
> > > + of_node_put(ulpi_node);
> > > +
> >
> > Just confirm with you that ci->platdata->phy_mode is not enough?
>
> Right. The phy_mode is never set to HSIC. It's always ULPI.
Ok
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-30 18:30 +0200 |
| Subject | Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset |
| Message-ID | <rPN0K-3nZ-31@gated-at.bofh.it> |
| In reply to | #1434391 |
On 30 June 2016 at 01:54, Peter Chen <hzpeterchen@gmail.com> wrote: > On Wed, Jun 29, 2016 at 12:13:45PM -0700, Stephen Boyd wrote: >> Quoting Peter Chen (2016-06-29 01:26:48 >> > Why not using ci->platdata->vbus_extcon directly? >> >> Because ci->platdata->vbus_extcon is assigned after the child platform >> driver probes, and we have no idea when that will happen from the >> ci_hdrc_msm driver probe. If we try after ci_hdrc_add_device() we'll >> race with the driver probe and only get the pointer sometimes. > > ci->platdata->vbus_extcon->edev is assigned at ci_get_platdata which is > called before ci core device is created. > Ok.
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-26 09:40 +0200 |
| Subject | [PATCH 11/21] usb: chipidea: msm: Use hw_write_id_reg() instead of writel directly |
| Message-ID | <rOcPE-8ip-27@gated-at.bofh.it> |
| In reply to | #1431357 |
The MSM_USB_BASE macro trick is not very clear, and we're using
it for only one register write so let's just move to using
hw_write_id_reg() and passing the ci pointer instead. That
clearly shows what offset we're using and avoids needing to
include the msm_hsusb_hw.h file when we're going to delete that
file soon.
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 | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
index 37591a4b1346..520c85e701ef 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -8,14 +8,12 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
-#include <linux/usb/msm_hsusb_hw.h>
-#include <linux/usb/ulpi.h>
#include <linux/usb/gadget.h>
#include <linux/usb/chipidea.h>
#include "ci.h"
-#define MSM_USB_BASE (ci->hw_bank.abs)
+#define HS_PHY_AHB_MODE 0x0098
static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
{
@@ -25,7 +23,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
case CI_HDRC_CONTROLLER_RESET_EVENT:
dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
/* use AHB transactor, allow posted data writes */
- writel(0x8, USB_AHBMODE);
+ hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0xffffffff, 0x8);
usb_phy_init(ci->usb_phy);
break;
case CI_HDRC_CONTROLLER_STOPPED_EVENT:
--
2.9.0.rc2.8.ga28705d
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-29 08:50 +0200 |
| Subject | Re: [PATCH 11/21] usb: chipidea: msm: Use hw_write_id_reg() instead of writel directly |
| Message-ID | <rPhtT-NA-1@gated-at.bofh.it> |
| In reply to | #1431372 |
On Sun, Jun 26, 2016 at 12:28:28AM -0700, Stephen Boyd wrote:
> The MSM_USB_BASE macro trick is not very clear, and we're using
> it for only one register write so let's just move to using
> hw_write_id_reg() and passing the ci pointer instead. That
> clearly shows what offset we're using and avoids needing to
> include the msm_hsusb_hw.h file when we're going to delete that
> file soon.
>
> 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 | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 37591a4b1346..520c85e701ef 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -8,14 +8,12 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> -#include <linux/usb/msm_hsusb_hw.h>
> -#include <linux/usb/ulpi.h>
> #include <linux/usb/gadget.h>
> #include <linux/usb/chipidea.h>
>
> #include "ci.h"
>
> -#define MSM_USB_BASE (ci->hw_bank.abs)
> +#define HS_PHY_AHB_MODE 0x0098
>
> static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> {
> @@ -25,7 +23,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> case CI_HDRC_CONTROLLER_RESET_EVENT:
> dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
> /* use AHB transactor, allow posted data writes */
> - writel(0x8, USB_AHBMODE);
> + hw_write_id_reg(ci, HS_PHY_AHB_MODE, 0xffffffff, 0x8);
> usb_phy_init(ci->usb_phy);
> break;
> case CI_HDRC_CONTROLLER_STOPPED_EVENT:
> --
Acked-by: Peter Chen <peter.chen@nxp.com>
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-26 09:40 +0200 |
| Subject | [PATCH 21/21] phy: Add support for Qualcomm's USB HS phy |
| Message-ID | <rOcPE-8ip-31@gated-at.bofh.it> |
| In reply to | #1431357 |
The high-speed phy on qcom SoCs is controlled via the ULPI
viewport.
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
.../devicetree/bindings/phy/qcom,usb-hs-phy.txt | 71 ++++++
drivers/phy/Kconfig | 8 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-qcom-usb-hs.c | 283 +++++++++++++++++++++
4 files changed, 363 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
create mode 100644 drivers/phy/phy-qcom-usb-hs.c
diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
new file mode 100644
index 000000000000..2bd22c53cee0
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
@@ -0,0 +1,71 @@
+Qualcomm's USB HS PHY
+
+PROPERTIES
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: Should contain "qcom,usb-hs-phy"
+
+- #phy-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: Should contain 0
+
+- clocks:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Should contain clock specifier for the reference and sleep
+ clocks
+
+- clock-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain "ref" and "sleep" for the reference and sleep
+ clocks respectively
+
+- resets:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Should contain the phy and POR resets
+
+- reset-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain "phy" and "por" for the phy and POR resets
+ respectively
+
+- v3p3-supply:
+ Usage: required
+ Value type: <phandle>
+ Definition: Should contain a reference to the 3.3V supply
+
+- v1p8-supply:
+ Usage: required
+ Value type: <phandle>
+ Definition: Should contain a reference to the 1.8V supply
+
+- qcom,init-seq:
+ Usage: optional
+ Value type: <u8 array>
+ Definition: Should contain a sequence of ULPI register and address pairs to
+ program into the ULPI_EXT_VENDOR_SPECIFIC area. This is related
+ to Device Mode Eye Diagram test.
+
+EXAMPLE
+
+otg: usb-controller {
+ ulpi {
+ phy {
+ compatible = "qcom,usb-hs-phy";
+ #phy-cells = <0>;
+ clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
+ clock-names = "ref", "sleep";
+ resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;
+ reset-names = "phy", "por";
+ v3p3-supply = <&pm8941_l24>;
+ v1p8-supply = <&pm8941_l6>;
+ qcom,init-seq = /bits/ 8 <0x81 0x63>;
+ };
+ };
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a2866949dc97..cfb3ded0896d 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -406,6 +406,14 @@ config PHY_QCOM_UFS
help
Support for UFS PHY on QCOM chipsets.
+config PHY_QCOM_USB_HS
+ tristate "Qualcomm USB HS PHY module"
+ depends on USB_ULPI_BUS
+ select GENERIC_PHY
+ help
+ Support for the USB high-speed ULPI compliant phy on Qualcomm
+ chipsets.
+
config PHY_QCOM_USB_HSIC
tristate "Qualcomm USB HSIC ULPI PHY module"
depends on USB_ULPI_BUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 982e84a290ec..21435fc0b656 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o
obj-$(CONFIG_PHY_QCOM_USB_HSIC) += phy-qcom-usb-hsic.o
obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o
obj-$(CONFIG_PHY_BRCM_SATA) += phy-brcm-sata.o
diff --git a/drivers/phy/phy-qcom-usb-hs.c b/drivers/phy/phy-qcom-usb-hs.c
new file mode 100644
index 000000000000..8be83100ecd9
--- /dev/null
+++ b/drivers/phy/phy-qcom-usb-hs.c
@@ -0,0 +1,283 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/ulpi/driver.h>
+#include <linux/ulpi/regs.h>
+#include <linux/clk.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
+#include <linux/reset.h>
+#include <linux/extcon.h>
+#include <linux/notifier.h>
+#include <linux/usb/of.h>
+
+#include "ulpi_phy.h"
+
+#define ULPI_PWR_CLK_MNG_REG 0x88
+# define ULPI_PWR_OTG_COMP_DISABLE BIT(0)
+
+#define ULPI_MISC_A 0x96
+# define ULPI_MISC_A_VBUSVLDEXTSEL BIT(1)
+# define ULPI_MISC_A_VBUSVLDEXT BIT(0)
+
+
+struct ulpi_seq {
+ u8 addr;
+ u8 val;
+};
+
+struct qcom_usb_hs_phy {
+ struct ulpi *ulpi;
+ struct phy *phy;
+ struct clk *ref_clk;
+ struct clk *sleep_clk;
+ struct regulator *v1p8;
+ struct regulator *v3p3;
+ struct reset_control *reset;
+ struct ulpi_seq *init_seq;
+ struct notifier_block vbus_notify;
+ struct extcon_dev *vbus_edev;
+ struct extcon_dev *id_edev;
+ enum usb_dr_mode dr_mode;
+};
+
+static int
+qcom_usb_hs_phy_vbus_notifier(struct notifier_block *nb, unsigned long event,
+ void *ptr)
+{
+ struct qcom_usb_hs_phy *uphy;
+ int is_host;
+ u8 addr;
+
+ uphy = container_of(nb, struct qcom_usb_hs_phy, vbus_notify);
+ is_host = extcon_get_cable_state_(uphy->id_edev, EXTCON_USB_HOST);
+ if (is_host < 0)
+ is_host = 0; /* No id event means always a peripheral */
+
+ if (event && !is_host)
+ addr = ULPI_SET(ULPI_MISC_A);
+ else
+ addr = ULPI_CLR(ULPI_MISC_A);
+
+ return ulpi_write(uphy->ulpi, addr,
+ ULPI_MISC_A_VBUSVLDEXTSEL | ULPI_MISC_A_VBUSVLDEXT);
+}
+
+static int qcom_usb_hs_phy_power_on(struct phy *phy)
+{
+ struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ struct ulpi *ulpi = uphy->ulpi;
+ const struct ulpi_seq *seq;
+ int ret, state;
+
+ ret = clk_prepare_enable(uphy->ref_clk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(uphy->sleep_clk);
+ if (ret)
+ goto err_sleep;
+
+ ret = regulator_set_voltage(uphy->v1p8, 1800000, 1800000);
+ if (ret)
+ goto err_1p8;
+ ret = regulator_set_load(uphy->v1p8, 50000);
+ if (ret < 0)
+ goto err_1p8;
+
+ ret = regulator_enable(uphy->v1p8);
+ if (ret)
+ goto err_1p8;
+
+ ret = regulator_set_voltage_triplet(uphy->v3p3, 3050000, 3300000,
+ 3300000);
+ if (ret)
+ goto err_3p3;
+
+ ret = regulator_set_load(uphy->v3p3, 50000);
+ if (ret < 0)
+ goto err_3p3;
+
+ ret = regulator_enable(uphy->v3p3);
+ if (ret)
+ goto err_3p3;
+
+ for (seq = uphy->init_seq; seq->addr; seq++) {
+ ret = ulpi_write(ulpi, seq->addr, seq->val);
+ if (ret)
+ goto err_ulpi;
+ }
+
+ if (uphy->reset) {
+ ret = reset_control_reset(uphy->reset);
+ if (ret)
+ goto err_ulpi;
+ }
+
+ if (uphy->vbus_edev) {
+ ulpi_write(ulpi, ULPI_SET(ULPI_PWR_CLK_MNG_REG),
+ ULPI_PWR_OTG_COMP_DISABLE);
+ state = extcon_get_cable_state_(uphy->vbus_edev, EXTCON_USB);
+ /* setup initial state */
+ qcom_usb_hs_phy_vbus_notifier(&uphy->vbus_notify, state,
+ uphy->vbus_edev);
+ } else {
+ u8 val;
+
+ switch (uphy->dr_mode) {
+ case USB_DR_MODE_OTG:
+ val = ULPI_INT_IDGRD;
+ case USB_DR_MODE_PERIPHERAL:
+ val |= ULPI_INT_SESS_VALID;
+ break;
+ default:
+ val = 0;
+ }
+
+ ret = ulpi_write(ulpi, ULPI_USB_INT_EN_RISE, val);
+ if (ret)
+ goto err_ulpi;
+ ret = ulpi_write(ulpi, ULPI_USB_INT_EN_FALL, val);
+ if (ret)
+ goto err_ulpi;
+ }
+
+ return 0;
+err_ulpi:
+ regulator_disable(uphy->v3p3);
+err_3p3:
+ regulator_disable(uphy->v1p8);
+err_1p8:
+ clk_disable_unprepare(uphy->sleep_clk);
+err_sleep:
+ clk_disable_unprepare(uphy->ref_clk);
+ return ret;
+}
+
+static int qcom_usb_hs_phy_power_off(struct phy *phy)
+{
+ struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+
+ regulator_disable(uphy->v3p3);
+ regulator_disable(uphy->v1p8);
+ clk_disable_unprepare(uphy->sleep_clk);
+ clk_disable_unprepare(uphy->ref_clk);
+
+ return 0;
+}
+
+static const struct phy_ops qcom_usb_hs_phy_ops = {
+ .power_on = qcom_usb_hs_phy_power_on,
+ .power_off = qcom_usb_hs_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
+{
+ struct qcom_usb_hs_phy *uphy;
+ struct phy_provider *p;
+ struct clk *clk;
+ struct regulator *reg;
+ struct reset_control *reset;
+ int size;
+ int ret;
+
+ uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
+ if (!uphy)
+ return -ENOMEM;
+ ulpi_set_drvdata(ulpi, uphy);
+ uphy->ulpi = ulpi;
+ uphy->dr_mode = of_usb_get_dr_mode_by_phy(ulpi->dev.of_node);
+
+ size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
+ if (size < 0)
+ size = 0;
+ uphy->init_seq = devm_kmalloc_array(&ulpi->dev, (size / 2) + 1,
+ sizeof(*uphy->init_seq), GFP_KERNEL);
+ if (!uphy->init_seq)
+ return -ENOMEM;
+ ret = of_property_read_u8_array(ulpi->dev.of_node, "qcom,init-seq",
+ (u8 *)uphy->init_seq, size);
+ if (ret && size)
+ return ret;
+ /* NUL terminate */
+ uphy->init_seq[size / 2].addr = uphy->init_seq[size / 2].val = 0;
+
+ uphy->ref_clk = clk = devm_clk_get(&ulpi->dev, "ref");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->sleep_clk = clk = devm_clk_get(&ulpi->dev, "sleep");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->v1p8 = reg = devm_regulator_get(&ulpi->dev, "v1p8");
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+
+ uphy->v3p3 = reg = devm_regulator_get(&ulpi->dev, "v3p3");
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+
+ uphy->reset = reset = devm_reset_control_get(&ulpi->dev, "por");
+ if (IS_ERR(reset)) {
+ if (PTR_ERR(reset) == -EPROBE_DEFER)
+ return PTR_ERR(reset);
+ uphy->reset = NULL;
+ }
+
+ uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
+ &qcom_usb_hs_phy_ops);
+ if (IS_ERR(uphy->phy))
+ return PTR_ERR(uphy->phy);
+
+ uphy->vbus_edev = extcon_get_edev_by_phandle(&ulpi->dev, 0);
+ if (IS_ERR(uphy->vbus_edev)) {
+ if (PTR_ERR(uphy->vbus_edev) != -ENODEV)
+ return PTR_ERR(uphy->vbus_edev);
+ uphy->vbus_edev = NULL;
+ }
+
+ uphy->id_edev = extcon_get_edev_by_phandle(&ulpi->dev, 1);
+ if (IS_ERR(uphy->id_edev)) {
+ if (PTR_ERR(uphy->id_edev) != -ENODEV)
+ return PTR_ERR(uphy->id_edev);
+ uphy->id_edev = NULL;
+ }
+
+ if (uphy->vbus_edev) {
+ uphy->vbus_notify.notifier_call = qcom_usb_hs_phy_vbus_notifier;
+ ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
+ &uphy->vbus_notify);
+ if (ret)
+ return ret;
+ }
+
+ phy_set_drvdata(uphy->phy, uphy);
+
+ p = devm_of_phy_provider_register(&ulpi->dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(p);
+}
+
+static const struct of_device_id qcom_usb_hs_phy_match[] = {
+ { .compatible = "qcom,usb-hs-phy", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, qcom_usb_hs_phy_match);
+
+static struct ulpi_driver qcom_usb_hs_phy_driver = {
+ .probe = qcom_usb_hs_phy_probe,
+ .driver = {
+ .name = "qcom_usb_hs_phy",
+ .of_match_table = qcom_usb_hs_phy_match
+ },
+};
+module_ulpi_driver(qcom_usb_hs_phy_driver);
+
+MODULE_DESCRIPTION("Qualcomm USB HS phy");
+MODULE_LICENSE("GPL v2");
--
2.9.0.rc2.8.ga28705d
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-26 09:40 +0200 |
| Subject | [PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global |
| Message-ID | <rOcPE-8ip-29@gated-at.bofh.it> |
| In reply to | #1431357 |
If two devices are probed with this same driver, they'll share
the same platform data structure, while the chipidea core layer
writes and modifies it. This can lead to interesting results
especially if one device is an OTG type chipidea controller and
another is a host. Let's create a copy of this structure per each
device instance so that odd things don't happen.
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 | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
index cc6f9b0df9d5..fb4340f02c16 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -37,6 +37,7 @@ struct ci_hdrc_msm {
struct clk *core_clk;
struct clk *iface_clk;
struct extcon_dev *vbus_edev;
+ struct ci_hdrc_platform_data pdata;
bool secondary_phy;
bool hsic;
void __iomem *base;
@@ -79,16 +80,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
}
}
-static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = {
- .name = "ci_hdrc_msm",
- .capoffset = DEF_CAPOFFSET,
- .flags = CI_HDRC_REGS_SHARED |
- CI_HDRC_DISABLE_STREAMING |
- CI_HDRC_OVERRIDE_AHB_BURST,
-
- .notify_event = ci_hdrc_msm_notify_event,
-};
-
static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
struct platform_device *pdev)
{
@@ -151,6 +142,12 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, ci);
+ ci->pdata.name = "ci_hdrc_msm";
+ ci->pdata.capoffset = DEF_CAPOFFSET;
+ ci->pdata.flags = CI_HDRC_REGS_SHARED | CI_HDRC_DISABLE_STREAMING |
+ CI_HDRC_OVERRIDE_AHB_BURST;
+ ci->pdata.notify_event = ci_hdrc_msm_notify_event;
+
reset = devm_reset_control_get(&pdev->dev, "core");
if (IS_ERR(reset))
return PTR_ERR(reset);
@@ -204,7 +201,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
of_node_put(ulpi_node);
plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
- pdev->num_resources, &ci_hdrc_msm_platdata);
+ pdev->num_resources, &ci->pdata);
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
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-29 13:40 +0200 |
| Subject | Re: [PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global |
| Message-ID | <rPm0x-3Cd-1@gated-at.bofh.it> |
| In reply to | #1431374 |
On Sun, Jun 26, 2016 at 12:28:34AM -0700, Stephen Boyd wrote:
> If two devices are probed with this same driver, they'll share
> the same platform data structure, while the chipidea core layer
> writes and modifies it. This can lead to interesting results
> especially if one device is an OTG type chipidea controller and
> another is a host. Let's create a copy of this structure per each
> device instance so that odd things don't happen.
>
> 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 | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index cc6f9b0df9d5..fb4340f02c16 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -37,6 +37,7 @@ struct ci_hdrc_msm {
> struct clk *core_clk;
> struct clk *iface_clk;
> struct extcon_dev *vbus_edev;
> + struct ci_hdrc_platform_data pdata;
> bool secondary_phy;
> bool hsic;
> void __iomem *base;
> @@ -79,16 +80,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> }
> }
>
> -static struct ci_hdrc_platform_data ci_hdrc_msm_platdata = {
> - .name = "ci_hdrc_msm",
> - .capoffset = DEF_CAPOFFSET,
> - .flags = CI_HDRC_REGS_SHARED |
> - CI_HDRC_DISABLE_STREAMING |
> - CI_HDRC_OVERRIDE_AHB_BURST,
> -
> - .notify_event = ci_hdrc_msm_notify_event,
> -};
> -
> static int ci_hdrc_msm_mux_phy(struct ci_hdrc_msm *ci,
> struct platform_device *pdev)
> {
> @@ -151,6 +142,12 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> return -ENOMEM;
> platform_set_drvdata(pdev, ci);
>
> + ci->pdata.name = "ci_hdrc_msm";
> + ci->pdata.capoffset = DEF_CAPOFFSET;
> + ci->pdata.flags = CI_HDRC_REGS_SHARED | CI_HDRC_DISABLE_STREAMING |
> + CI_HDRC_OVERRIDE_AHB_BURST;
> + ci->pdata.notify_event = ci_hdrc_msm_notify_event;
> +
> reset = devm_reset_control_get(&pdev->dev, "core");
> if (IS_ERR(reset))
> return PTR_ERR(reset);
> @@ -204,7 +201,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> of_node_put(ulpi_node);
>
> plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> - pdev->num_resources, &ci_hdrc_msm_platdata);
> + pdev->num_resources, &ci->pdata);
> if (IS_ERR(plat_ci)) {
> dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> ret = PTR_ERR(plat_ci);
You can do something like ci_hdrc_usb2.c, it looks simpler.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-30 11:20 +0200 |
| Subject | Re: [PATCH 17/21] usb: chipidea: msm: Make platform data driver local instead of global |
| Message-ID | <rPGiC-7LV-15@gated-at.bofh.it> |
| In reply to | #1433687 |
On Wed, Jun 29, 2016 at 12:17:12PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-06-29 04:29:25)
> > On Sun, Jun 26, 2016 at 12:28:34AM -0700, Stephen Boyd wrote:
> > > @@ -204,7 +201,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> > > of_node_put(ulpi_node);
> > >
> > > plat_ci = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> > > - pdev->num_resources, &ci_hdrc_msm_platdata);
> > > + pdev->num_resources, &ci->pdata);
> > > if (IS_ERR(plat_ci)) {
> > > dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> > > ret = PTR_ERR(plat_ci);
> >
> > You can do something like ci_hdrc_usb2.c, it looks simpler.
> >
>
> Do what exactly? I'd rather not do a structure copy because that wastes
> some memory for a structure that is just a template. We add some more
> code here to assign values directly, but that is smaller size wise than
> the large platdata structure that only has a few values set in it.
But you add one struct ci_hdrc_platform_data pdata entry at struct
ci_hdrc_msm which needs to allocate the memory too. Anyway, it is not a
big problem.
Acked-by: Peter Chen <peter.chen@nxp.com>
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-26 09:40 +0200 |
| Subject | [PATCH 01/21] of: device: Support loading a module with OF based modalias |
| Message-ID | <rOcPE-8ip-33@gated-at.bofh.it> |
| In reply to | #1431357 |
In the case of ULPI devices, we want to be able to load the
driver before registering the device so that we don't get stuck
in a loop waiting for the phy module to appear and failing usb
controller probe. Currently we request the ulpi module via the
ulpi ids, but in the DT case we might need to request it with the
OF based modalias instead. Add a common function that allows
anyone to request a module with the OF based modalias.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
drivers/of/device.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_device.h | 6 ++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index fd5cfad7c403..f275e5beb736 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -226,6 +226,56 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
return tsize;
}
+static ssize_t of_device_modalias_size(struct device *dev)
+{
+ const char *compat;
+ int cplen, i;
+ ssize_t csize;
+
+ if ((!dev) || (!dev->of_node))
+ return -ENODEV;
+
+ /* Name & Type */
+ csize = 5 + strlen(dev->of_node->name) + strlen(dev->of_node->type);
+
+ /* Get compatible property if any */
+ compat = of_get_property(dev->of_node, "compatible", &cplen);
+ if (!compat)
+ return csize;
+
+ /* Find true end (we tolerate multiple \0 at the end */
+ for (i = (cplen - 1); i >= 0 && !compat[i]; i--)
+ cplen--;
+ if (!cplen)
+ return csize;
+ cplen++;
+
+ /* Check space (need cplen+1 chars including final \0) */
+ return csize + cplen;
+}
+
+int of_device_request_module(struct device *dev)
+{
+ char *str;
+ ssize_t size;
+ int ret;
+
+ size = of_device_modalias_size(dev);
+ if (size < 0)
+ return size;
+
+ str = kmalloc(size + 1, GFP_KERNEL);
+ if (!str)
+ return -ENOMEM;
+
+ of_device_get_modalias(dev, str, size);
+ str[size] = '\0';
+ ret = request_module(str);
+ kfree(str);
+
+ return ret;
+}
+
/**
* of_device_uevent - Display OF related uevent information
*/
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687a89d..e9afbcc8de12 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -37,6 +37,7 @@ extern const void *of_device_get_match_data(const struct device *dev);
extern ssize_t of_device_get_modalias(struct device *dev,
char *str, ssize_t len);
+extern int of_device_request_module(struct device *dev);
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
@@ -78,6 +79,11 @@ static inline int of_device_get_modalias(struct device *dev,
return -ENODEV;
}
+static inline int of_device_request_module(struct device *dev)
+{
+ return -ENODEV;
+}
+
static inline int of_device_uevent_modalias(struct device *dev,
struct kobj_uevent_env *env)
{
--
2.9.0.rc2.8.ga28705d
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Andersson <bjorn.andersson@linaro.org> |
|---|---|
| Date | 2016-06-28 06:20 +0200 |
| Subject | Re: [PATCH 01/21] of: device: Support loading a module with OF based modalias |
| Message-ID | <rOSFb-1N2-1@gated-at.bofh.it> |
| In reply to | #1431375 |
On Sun 26 Jun 00:28 PDT 2016, Stephen Boyd wrote:
> In the case of ULPI devices, we want to be able to load the
> driver before registering the device so that we don't get stuck
> in a loop waiting for the phy module to appear and failing usb
> controller probe. Currently we request the ulpi module via the
> ulpi ids, but in the DT case we might need to request it with the
> OF based modalias instead. Add a common function that allows
> anyone to request a module with the OF based modalias.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> drivers/of/device.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of_device.h | 6 ++++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index fd5cfad7c403..f275e5beb736 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -226,6 +226,56 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
> return tsize;
> }
>
> +static ssize_t of_device_modalias_size(struct device *dev)
> +{
> + const char *compat;
> + int cplen, i;
> + ssize_t csize;
> +
> + if ((!dev) || (!dev->of_node))
> + return -ENODEV;
> +
> + /* Name & Type */
> + csize = 5 + strlen(dev->of_node->name) + strlen(dev->of_node->type);
It would be clearer if you replaced 5 with strlen("of:NT"), but...
> +
> + /* Get compatible property if any */
> + compat = of_get_property(dev->of_node, "compatible", &cplen);
> + if (!compat)
> + return csize;
> +
> + /* Find true end (we tolerate multiple \0 at the end */
> + for (i = (cplen - 1); i >= 0 && !compat[i]; i--)
> + cplen--;
> + if (!cplen)
> + return csize;
> + cplen++;
> +
> + /* Check space (need cplen+1 chars including final \0) */
> + return csize + cplen;
> +}
...if I understand of_device_get_modalias() correctly you should be able
to replace this function with:
size = of_device_get_modalias(dev, NULL, 0);
snprintf() will not write to NULL, csize will be larger than 0 so tsize
will be returned before it will memcpy() to the buffer.
> +
> +int of_device_request_module(struct device *dev)
> +{
> + char *str;
> + ssize_t size;
> + int ret;
> +
> + size = of_device_modalias_size(dev);
> + if (size < 0)
> + return size;
> +
> + str = kmalloc(size + 1, GFP_KERNEL);
> + if (!str)
> + return -ENOMEM;
> +
> + of_device_get_modalias(dev, str, size);
> + str[size] = '\0';
> + ret = request_module(str);
> + kfree(str);
> +
> + return ret;
> +}
> +
Regards,
Bjorn
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-26 09:40 +0200 |
| Subject | [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place |
| Message-ID | <rOcPE-8ip-35@gated-at.bofh.it> |
| In reply to | #1431357 |
With the id and vbus detection done via extcon we need to make
sure we poll the status of OTGSC properly by considering what the
extcon is saying, and not just what the register is saying. Let's
move this hw_wait_reg() function to the only place it's used and
simplify it for polling the OTGSC register. Then we can make
certain we only use the hw_read_otgsc() API to read OTGSC, which
will make sure we properly handle extcon events.
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ivan T. Ivanov" <iivanov.xz@gmail.com>
Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID detect")
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
drivers/usb/chipidea/core.c | 32 --------------------------------
drivers/usb/chipidea/otg.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e644d17..01390e02ee53 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)
return 0;
}
-/**
- * hw_wait_reg: wait the register value
- *
- * Sometimes, it needs to wait register value before going on.
- * Eg, when switch to device mode, the vbus value should be lower
- * than OTGSC_BSV before connects to host.
- *
- * @ci: the controller
- * @reg: register index
- * @mask: mast bit
- * @value: the bit value to wait
- * @timeout_ms: timeout in millisecond
- *
- * This function returns an error code if timeout
- */
-int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
- u32 value, unsigned int timeout_ms)
-{
- unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms);
-
- while (hw_read(ci, reg, mask) != value) {
- if (time_after(jiffies, elapse)) {
- dev_err(ci->dev, "timeout waiting for %08x in %d\n",
- mask, reg);
- return -ETIMEDOUT;
- }
- msleep(20);
- }
-
- return 0;
-}
-
static irqreturn_t ci_irq(int irq, void *data)
{
struct ci_hdrc *ci = data;
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 03b6743461d1..763a8332b009 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -104,7 +104,32 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
usb_gadget_vbus_disconnect(&ci->gadget);
}
-#define CI_VBUS_STABLE_TIMEOUT_MS 5000
+/**
+ * Sometimes, it needs to wait register value before going on.
+ * Eg, when switch to device mode, the vbus value should be lower
+ * than OTGSC_BSV before connects to host.
+ *
+ * @ci: the controller
+ *
+ * This function returns an error code if timeout
+ */
+static int hw_wait_otgsc_bsv(struct ci_hdrc *ci)
+{
+ unsigned long elapse = jiffies + msecs_to_jiffies(5000);
+ u32 mask = OTGSC_BSV;
+
+ while (!hw_read_otgsc(ci, mask)) {
+ if (time_after(jiffies, elapse)) {
+ dev_err(ci->dev, "timeout waiting for %08x in OTGSC\n",
+ mask);
+ return -ETIMEDOUT;
+ }
+ msleep(20);
+ }
+
+ return 0;
+}
+
static void ci_handle_id_switch(struct ci_hdrc *ci)
{
enum ci_role role = ci_otg_role(ci);
@@ -116,9 +141,11 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
ci_role_stop(ci);
if (role == CI_ROLE_GADGET)
- /* wait vbus lower than OTGSC_BSV */
- hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
- CI_VBUS_STABLE_TIMEOUT_MS);
+ /*
+ * wait vbus lower than OTGSC_BSV before connecting
+ * to host
+ */
+ hw_wait_otgsc_bsv(ci);
ci_role_start(ci, role);
}
--
2.9.0.rc2.8.ga28705d
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-06-27 10:10 +0200 |
| Subject | RE: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place |
| Message-ID | <rOzMe-62T-41@gated-at.bofh.it> |
| In reply to | #1431376 |
Hi
> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Stephen Boyd
> Sent: Sunday, June 26, 2016 3:28 PM
> To: linux-usb@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> linux-arm-msm@vger.kernel.org; Andy Gross <andy.gross@linaro.org>; Bjorn
> Andersson <bjorn.andersson@linaro.org>; Neil Armstrong
> <narmstrong@baylibre.com>; Arnd Bergmann <arnd@arndb.de>; Felipe Balbi
> <balbi@kernel.org>; Peter Chen <peter.chen@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Ivan T. Ivanov <iivanov.xz@gmail.com>
> Subject: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place
>
> With the id and vbus detection done via extcon we need to make sure we
> poll the status of OTGSC properly by considering what the extcon is saying,
> and not just what the register is saying. Let's move this hw_wait_reg()
> function to the only place it's used and simplify it for polling the OTGSC
> register. Then we can make certain we only use the hw_read_otgsc() API to
> read OTGSC, which will make sure we properly handle extcon events.
>
> Cc: Peter Chen <peter.chen@nxp.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Ivan T. Ivanov" <iivanov.xz@gmail.com>
> Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID
> detect")
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> drivers/usb/chipidea/core.c | 32 --------------------------------
> drivers/usb/chipidea/otg.c | 35 +++++++++++++++++++++++++++++++----
> 2 files changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e644d17..01390e02ee53 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)
> return 0;
> }
>
> -/**
> - * hw_wait_reg: wait the register value
> - *
> - * Sometimes, it needs to wait register value before going on.
> - * Eg, when switch to device mode, the vbus value should be lower
> - * than OTGSC_BSV before connects to host.
> - *
> - * @ci: the controller
> - * @reg: register index
> - * @mask: mast bit
> - * @value: the bit value to wait
> - * @timeout_ms: timeout in millisecond
> - *
> - * This function returns an error code if timeout
> - */
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms)
> -{
> - unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms);
> -
> - while (hw_read(ci, reg, mask) != value) {
> - if (time_after(jiffies, elapse)) {
> - dev_err(ci->dev, "timeout waiting for %08x in %d\n",
> - mask, reg);
> - return -ETIMEDOUT;
> - }
> - msleep(20);
> - }
> -
> - return 0;
> -}
> -
> static irqreturn_t ci_irq(int irq, void *data) {
> struct ci_hdrc *ci = data;
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> 03b6743461d1..763a8332b009 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -104,7 +104,32 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
> usb_gadget_vbus_disconnect(&ci->gadget);
> }
>
> -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
> +/**
> + * Sometimes, it needs to wait register value before going on.
> + * Eg, when switch to device mode, the vbus value should be lower
> + * than OTGSC_BSV before connects to host.
This should be updated since this API is dedicated for BSV now.
> + *
> + * @ci: the controller
> + *
> + * This function returns an error code if timeout */ static int
> +hw_wait_otgsc_bsv(struct ci_hdrc *ci) {
> + unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> + u32 mask = OTGSC_BSV;
> +
> + while (!hw_read_otgsc(ci, mask)) {
Reverse logic, should be:
while (hw_read_otgsc(ci, mask)) {
Li Jun
> + if (time_after(jiffies, elapse)) {
> + dev_err(ci->dev, "timeout waiting for %08x in OTGSC\n",
> + mask);
> + return -ETIMEDOUT;
> + }
> + msleep(20);
> + }
> +
> + return 0;
> +}
> +
> static void ci_handle_id_switch(struct ci_hdrc *ci) {
> enum ci_role role = ci_otg_role(ci);
> @@ -116,9 +141,11 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
> ci_role_stop(ci);
>
> if (role == CI_ROLE_GADGET)
> - /* wait vbus lower than OTGSC_BSV */
> - hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
> - CI_VBUS_STABLE_TIMEOUT_MS);
> + /*
> + * wait vbus lower than OTGSC_BSV before connecting
> + * to host
> + */
> + hw_wait_otgsc_bsv(ci);
>
> ci_role_start(ci, role);
> }
> --
> 2.9.0.rc2.8.ga28705d
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Stephen Boyd <stephen.boyd@linaro.org> |
|---|---|
| Date | 2016-06-27 21:10 +0200 |
| Subject | RE: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place |
| Message-ID | <rOK4W-4o3-37@gated-at.bofh.it> |
| In reply to | #1431796 |
Quoting Jun Li (2016-06-27 01:04:39)
> > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> > 03b6743461d1..763a8332b009 100644
> > --- a/drivers/usb/chipidea/otg.c
> > +++ b/drivers/usb/chipidea/otg.c
> > @@ -104,7 +104,32 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
> > usb_gadget_vbus_disconnect(&ci->gadget);
> > }
> >
> > -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
> > +/**
> > + * Sometimes, it needs to wait register value before going on.
> > + * Eg, when switch to device mode, the vbus value should be lower
> > + * than OTGSC_BSV before connects to host.
>
> This should be updated since this API is dedicated for BSV now.
Ok I've updated it to say:
When we switch to device mode, the vbus value should be lower
than OTGSC_BSV before connecting to host.
>
> > + *
> > + * @ci: the controller
> > + *
> > + * This function returns an error code if timeout */ static int
> > +hw_wait_otgsc_bsv(struct ci_hdrc *ci) {
> > + unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> > + u32 mask = OTGSC_BSV;
> > +
> > + while (!hw_read_otgsc(ci, mask)) {
>
> Reverse logic, should be:
> while (hw_read_otgsc(ci, mask)) {
>
Good catch! Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Peter Chen <hzpeterchen@gmail.com> |
|---|---|
| Date | 2016-06-28 11:50 +0200 |
| Subject | Re: [PATCH 04/21] usb: chipidea: Only read/write OTGSC from one place |
| Message-ID | <rOXOy-593-7@gated-at.bofh.it> |
| In reply to | #1432231 |
On Mon, Jun 27, 2016 at 12:07:54PM -0700, Stephen Boyd wrote:
> Quoting Jun Li (2016-06-27 01:04:39)
> > > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> > > 03b6743461d1..763a8332b009 100644
> > > --- a/drivers/usb/chipidea/otg.c
> > > +++ b/drivers/usb/chipidea/otg.c
> > > @@ -104,7 +104,32 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
> > > usb_gadget_vbus_disconnect(&ci->gadget);
> > > }
> > >
> > > -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
> > > +/**
> > > + * Sometimes, it needs to wait register value before going on.
> > > + * Eg, when switch to device mode, the vbus value should be lower
> > > + * than OTGSC_BSV before connects to host.
> >
> > This should be updated since this API is dedicated for BSV now.
>
> Ok I've updated it to say:
>
> When we switch to device mode, the vbus value should be lower
> than OTGSC_BSV before connecting to host.
>
> >
> > > + *
> > > + * @ci: the controller
> > > + *
> > > + * This function returns an error code if timeout */ static int
> > > +hw_wait_otgsc_bsv(struct ci_hdrc *ci) {
> > > + unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> > > + u32 mask = OTGSC_BSV;
> > > +
> > > + while (!hw_read_otgsc(ci, mask)) {
> >
> > Reverse logic, should be:
> > while (hw_read_otgsc(ci, mask)) {
> >
>
> Good catch! Thanks.
Besides above, please delete the declaration at ci.h.
--
Best Regards,
Peter Chen
[toc] | [prev] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-06-28 05:10 +0200 |
| Message-ID | <rORzs-165-3@gated-at.bofh.it> |
| In reply to | #1431357 |
On Sun, Jun 26, 2016 at 12:28 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote: > The state of USB ChipIdea support on Qualcomm's platforms is not great. > The DT description of these devices requires up to three different nodes > for what amounts to be the same hardware block, when there should really > only be one. Furthermore, the "phy" driver that is in mainline (phy-msm-usb.c) > duplicates the OTG state machine and touches the ci controller wrapper > registers when it should really be focused on the phy and the ULPI accesses > needed to get the phy working. There's also a slimmed down phy driver for > the msm8916 platform, but really the phy hardware is the same as other MSMs, > so we have two drivers doing pretty much the same thing. This leads to a > situtaion where we have the chipidea core driver, the "phy" driver, and > sometimes the ehci-msm.c driver operating the same device all at the same > time with very little coordination. This just isn't very safe and is > confusing from a driver perspective when trying to figure out who does what. > Finally, there isn't any HSIC support on platforms like apq8074 so we > should add that. > > This patch series updates the ChipIdea driver and the MSM wrapper > (ci_hdrc_msm.c) to properly handle the PHY and wrapper bits at the right > times in the right places. To get there, we update the ChipIdea core to > have support for the ULPI phy bus introduced by Heikki. Along the way > we fix bugs with the extcon handling for peripheral and OTG mode controllers > and move the parts of phy-usb-msm.c that are touching the CI controller > wrapper into the wrapper driver (ci_hdrc_msm.c). Finally we add support > for the HSIC phy based on the ULPI bus and rewrite the HS phy driver > (phy-usb-msm.c) as a standard ULPI phy driver. > > Once this series is accepted, we should be able to delete the phy-usb-msm.c > phy-qcom-8x16-usb.c, and ehci-msm.c drivers from the tree and use the ULPI > based phy driver (which also lives in drivers/phy/ instead of drivers/usb/phy/) > and the chipidea host core instead. > > I've also sent seperate patches for other minor pieces to make this > all work. The full tree can be found here[3], hacks and all to get > things working. I've tested this on the db410c, apq8074 dragonboard, > and ifc6410 with configfs gadgets and otg cables. ... > [3] https://git.linaro.org/people/stephen.boyd/linux.git/shortlog/refs/heads/usb-hsic-8074 Very excited to see this moving upstream! Just a heads up, trying to build with this branch gives me: drivers/usb/Kconfig:39:error: recursive dependency detected! For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/input/mouse/Kconfig:187: symbol MOUSE_APPLETOUCH depends on INPUT For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/input/Kconfig:8: symbol INPUT is selected by VT For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/tty/Kconfig:12: symbol VT is selected by FB_STI For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:674: symbol FB_STI depends on FB For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:42: symbol DRM_KMS_FB_HELPER is selected by DRM_KMS_CMA_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:98: symbol DRM_KMS_CMA_HELPER is selected by DRM_IMX For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/imx/Kconfig:1: symbol DRM_IMX depends on IMX_IPUV3_CORE For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/ipu-v3/Kconfig:1: symbol IMX_IPUV3_CORE depends on RESET_CONTROLLER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/reset/Kconfig:4: symbol RESET_CONTROLLER is selected by USB_CHIPIDEA For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/usb/chipidea/Kconfig:1: symbol USB_CHIPIDEA depends on USB_EHCI_HCD For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/usb/host/Kconfig:84: symbol USB_EHCI_HCD depends on USB drivers/usb/chipidea/otg.c: In function ‘hw_write_otgsc’: drivers/usb/chipidea/otg.c:120:2: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘long unsigned int’ [-Wformat] drivers/usb/chipidea/otg.c:120:2: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘long unsigned int’ [-Wformat] I haven't yet been able to test with this, as I need some other fixes it seems too to deal with some of the iommu changes in my flo-WIP tree (it can't find of_dma_configure), but will let you know how things work once I have all that sorted. thanks -john
[toc] | [prev] | [next] | [standalone]
Page 3 of 4 — ← Prev page 1 2 [3] 4 Next page →
Back to top | Article view | linux.kernel
csiph-web