2016-11-07 13:21:59

by Alexandre Bailon

[permalink] [raw]
Subject: [PATCH v5 0/4] usb: musb: da8xx: Fix few issues

Currently, the USB OTG of the da8xx doesn't work.
This series intend to fix them.

Change in v2:
* Fix the error path da8xx_musb_init()

Changes in v3:
* Remove the host workaround that was not working on every platform
* Add a property to the devicetree node of phy to force the phy in a specific
mode (host or device).
* Only use dr_mode to configure the controller mode and let the phy in OTG mode.
The main goal of this change is to prevent the phy to be set in host or
device mode because these modes have some issues.

Changes in v4:
* Enable VBUS sense and session end comparator during probe of phy.
* Add a quirk flag to skip the phy set mode made during the init of musb.
In that way, we can still can configure the phy mode by using mode file in
sysfs and configure the controller mode by using dr_mode or platform_data.
* Init the phy in otg mode in da8xx glue driver.

Changes in v5:
* Add the init argument to musb_platform_set_mode() and set_mode() callback.
* Remove the quirk
* Configure the DA8xx phy in OTG mode if da8xx_musb_set_mode() is called from
init.

Alexandre Bailon (4):
usb: musb: da8xx: Call earlier clk_prepare_enable()
phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
usb: musb: Add a new argument to musb_platform_set_mode()
usb: musb: da8xx: Set phy in OTG mode by default

drivers/phy/phy-da8xx-usb.c | 5 +++++
drivers/usb/musb/am35x.c | 2 +-
drivers/usb/musb/blackfin.c | 2 +-
drivers/usb/musb/da8xx.c | 26 ++++++++++++++++----------
drivers/usb/musb/davinci.c | 2 +-
drivers/usb/musb/musb_core.c | 12 ++++++------
drivers/usb/musb/musb_core.h | 6 +++---
drivers/usb/musb/musb_dsps.c | 2 +-
drivers/usb/musb/sunxi.c | 2 +-
drivers/usb/musb/tusb6010.c | 2 +-
10 files changed, 36 insertions(+), 25 deletions(-)

--
2.7.3


2016-11-07 13:17:42

by Alexandre Bailon

[permalink] [raw]
Subject: [PATCH v5 4/4] usb: musb: da8xx: Set phy in OTG mode by default

The DA8xx OTG PHY has some issues when it is forced in host or
peripheral mode. Actually, most of the time, OTG is the best mode
because host or peripheral mode are only required for hardware that
miss some circuitry.
Init the PHY mode OTG mode by default.

Signed-off-by: Alexandre Bailon <[email protected]>
---
drivers/usb/musb/da8xx.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index ac0c2f7..a7e7abe 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -340,6 +340,13 @@ static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
enum phy_mode phy_mode;

+ /*
+ * The PHY has some issues when it is forced in device or host mode.
+ * Unless the user request another mode, configure the PHY in OTG mode.
+ */
+ if (init)
+ return phy_set_mode(glue->phy, PHY_MODE_USB_OTG);
+
switch (musb_mode) {
case MUSB_HOST: /* Force VBUS valid, ID = 0 */
phy_mode = PHY_MODE_USB_HOST;
--
2.7.3

2016-11-07 13:19:48

by Alexandre Bailon

[permalink] [raw]
Subject: [PATCH v5 3/4] usb: musb: Add a new argument to musb_platform_set_mode()

During the init, the driver will use musb_platform_set_mode()
to configure the controller mode and the PHY mode.
The PHY of DA8xx has some issues when the PHY is forced in host or device,
so we want to keep it in OTG mode unless the user request a specific mode
by using the sysfs.
Add the init argument to musb_platform_set_mode() in order to let the
callback change its behavior if it is called during the init.

Signed-off-by: Alexandre Bailon <[email protected]>
---
drivers/usb/musb/am35x.c | 2 +-
drivers/usb/musb/blackfin.c | 2 +-
drivers/usb/musb/da8xx.c | 2 +-
drivers/usb/musb/davinci.c | 2 +-
drivers/usb/musb/musb_core.c | 12 ++++++------
drivers/usb/musb/musb_core.h | 6 +++---
drivers/usb/musb/musb_dsps.c | 2 +-
drivers/usb/musb/sunxi.c | 2 +-
drivers/usb/musb/tusb6010.c | 2 +-
9 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 50ca805..7136888 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -332,7 +332,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
return ret;
}

-static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode)
+static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
{
struct device *dev = musb->controller;
struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
index 310238c..544e98f 100644
--- a/drivers/usb/musb/blackfin.c
+++ b/drivers/usb/musb/blackfin.c
@@ -356,7 +356,7 @@ static int bfin_musb_vbus_status(struct musb *musb)
return 0;
}

-static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode)
+static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
{
return -EIO;
}
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 6749aa1..ac0c2f7 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -335,7 +335,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
return ret;
}

-static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
+static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
{
struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
enum phy_mode phy_mode;
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index cee61a5..d12b902 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -369,7 +369,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void *__hci)
return retval;
}

-static int davinci_musb_set_mode(struct musb *musb, u8 mode)
+static int davinci_musb_set_mode(struct musb *musb, u8 mode, bool init)
{
/* EVM can't do this (right?) */
return -EIO;
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 27dadc0..4a8d394 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1730,11 +1730,11 @@ musb_mode_store(struct device *dev, struct device_attribute *attr,

spin_lock_irqsave(&musb->lock, flags);
if (sysfs_streq(buf, "host"))
- status = musb_platform_set_mode(musb, MUSB_HOST);
+ status = musb_platform_set_mode(musb, MUSB_HOST, false);
else if (sysfs_streq(buf, "peripheral"))
- status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+ status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, false);
else if (sysfs_streq(buf, "otg"))
- status = musb_platform_set_mode(musb, MUSB_OTG);
+ status = musb_platform_set_mode(musb, MUSB_OTG, false);
else
status = -EINVAL;
spin_unlock_irqrestore(&musb->lock, flags);
@@ -2261,13 +2261,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
status = musb_host_setup(musb, plat->power);
if (status < 0)
goto fail3;
- status = musb_platform_set_mode(musb, MUSB_HOST);
+ status = musb_platform_set_mode(musb, MUSB_HOST, true);
break;
case MUSB_PORT_MODE_GADGET:
status = musb_gadget_setup(musb);
if (status < 0)
goto fail3;
- status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+ status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, true);
break;
case MUSB_PORT_MODE_DUAL_ROLE:
status = musb_host_setup(musb, plat->power);
@@ -2278,7 +2278,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
musb_host_cleanup(musb);
goto fail3;
}
- status = musb_platform_set_mode(musb, MUSB_OTG);
+ status = musb_platform_set_mode(musb, MUSB_OTG, true);
break;
default:
dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 2cb88a49..1bfc8fa 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -203,7 +203,7 @@ struct musb_platform_ops {
struct dma_controller *
(*dma_init) (struct musb *musb, void __iomem *base);
void (*dma_exit)(struct dma_controller *c);
- int (*set_mode)(struct musb *musb, u8 mode);
+ int (*set_mode)(struct musb *musb, u8 mode, bool init);
void (*try_idle)(struct musb *musb, unsigned long timeout);
int (*recover)(struct musb *musb);

@@ -558,12 +558,12 @@ static inline void musb_platform_disable(struct musb *musb)
musb->ops->disable(musb);
}

-static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
+static inline int musb_platform_set_mode(struct musb *musb, u8 mode, bool init)
{
if (!musb->ops->set_mode)
return 0;

- return musb->ops->set_mode(musb, mode);
+ return musb->ops->set_mode(musb, mode, init);
}

static inline void musb_platform_try_idle(struct musb *musb,
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 0f17d21..a889679 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -462,7 +462,7 @@ static int dsps_musb_exit(struct musb *musb)
return 0;
}

-static int dsps_musb_set_mode(struct musb *musb, u8 mode)
+static int dsps_musb_set_mode(struct musb *musb, u8 mode, bool init)
{
struct device *dev = musb->controller;
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 1408245..04bf763 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -346,7 +346,7 @@ static void sunxi_musb_dma_controller_destroy(struct dma_controller *c)
{
}

-static int sunxi_musb_set_mode(struct musb *musb, u8 mode)
+static int sunxi_musb_set_mode(struct musb *musb, u8 mode, bool init)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
enum phy_mode new_mode;
diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index df7c9f4..8a74587 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -628,7 +628,7 @@ static void tusb_musb_set_vbus(struct musb *musb, int is_on)
* Note that if a mini-A cable is plugged in the ID line will stay down as
* the weak ID pull-up is not able to pull the ID up.
*/
-static int tusb_musb_set_mode(struct musb *musb, u8 musb_mode)
+static int tusb_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
{
void __iomem *tbase = musb->ctrl_base;
u32 otg_stat, phy_otg_ctrl, phy_otg_ena, dev_conf;
--
2.7.3

2016-11-07 13:20:37

by Alexandre Bailon

[permalink] [raw]
Subject: [PATCH v5 1/4] usb: musb: da8xx: Call earlier clk_prepare_enable()

The first attempt to read a register may fail because the clock may not
be enabled, and then the probe of musb driver will fail.
Call clk_prepare_enable() before the first register read.

Signed-off-by: Alexandre Bailon <[email protected]>
---
drivers/usb/musb/da8xx.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 210b7e4..6749aa1 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -366,6 +366,12 @@ static int da8xx_musb_init(struct musb *musb)

musb->mregs += DA8XX_MENTOR_CORE_OFFSET;

+ ret = clk_prepare_enable(glue->clk);
+ if (ret) {
+ dev_err(glue->dev, "failed to enable clock\n");
+ return ret;
+ }
+
/* Returns zero if e.g. not clocked */
rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG);
if (!rev)
@@ -377,12 +383,6 @@ static int da8xx_musb_init(struct musb *musb)
goto fail;
}

- ret = clk_prepare_enable(glue->clk);
- if (ret) {
- dev_err(glue->dev, "failed to enable clock\n");
- goto fail;
- }
-
setup_timer(&otg_workaround, otg_timer, (unsigned long)musb);

/* Reset the controller */
@@ -392,7 +392,7 @@ static int da8xx_musb_init(struct musb *musb)
ret = phy_init(glue->phy);
if (ret) {
dev_err(glue->dev, "Failed to init phy.\n");
- goto err_phy_init;
+ goto fail;
}

ret = phy_power_on(glue->phy);
@@ -412,9 +412,8 @@ static int da8xx_musb_init(struct musb *musb)

err_phy_power_on:
phy_exit(glue->phy);
-err_phy_init:
- clk_disable_unprepare(glue->clk);
fail:
+ clk_disable_unprepare(glue->clk);
return ret;
}

--
2.7.3

2016-11-07 13:20:32

by Alexandre Bailon

[permalink] [raw]
Subject: [PATCH v5 2/4] phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround

If we configure the da8xx OTG phy in OTG mode, neither device or host
mode will work. That is because the PHY is not able to detect and notify
the driver that value of ID pin changed.
To work despite this hardware limitation, the da8xx glue implement a
workaround.
But to work, the workaround require the VBUS sense and the session end
comparator to enabled.
Enable them if the phy is configured in OTG mode.

Signed-off-by: Alexandre Bailon <[email protected]>
---
drivers/phy/phy-da8xx-usb.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index 32ae78c..089c13b 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -23,6 +23,8 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>

+#define PHY_INIT_BITS (CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN)
+
struct da8xx_usb_phy {
struct phy_provider *phy_provider;
struct phy *usb11_phy;
@@ -207,6 +209,9 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
dev_warn(dev, "Failed to create usb20 phy lookup\n");
}

+ regmap_write_bits(d_phy->regmap, CFGCHIP(2),
+ PHY_INIT_BITS, PHY_INIT_BITS);
+
return 0;
}

--
2.7.3

2016-11-14 17:37:12

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH v5 3/4] usb: musb: Add a new argument to musb_platform_set_mode()

Hi,

On Mon, Nov 07, 2016 at 02:05:07PM +0100, Alexandre Bailon wrote:
> During the init, the driver will use musb_platform_set_mode()
> to configure the controller mode and the PHY mode.
> The PHY of DA8xx has some issues when the PHY is forced in host or device,
> so we want to keep it in OTG mode unless the user request a specific mode
> by using the sysfs.
> Add the init argument to musb_platform_set_mode() in order to let the
> callback change its behavior if it is called during the init.

Tony's patch set which fixes musb pm regression (but has not been merged
yet) introduces musb->is_initialized. You might want to use this new
variable in da8xx_musb_set_mode(), instead of adding this init flag.

Regards,
-Bin.

>
> Signed-off-by: Alexandre Bailon <[email protected]>
> ---
> drivers/usb/musb/am35x.c | 2 +-
> drivers/usb/musb/blackfin.c | 2 +-
> drivers/usb/musb/da8xx.c | 2 +-
> drivers/usb/musb/davinci.c | 2 +-
> drivers/usb/musb/musb_core.c | 12 ++++++------
> drivers/usb/musb/musb_core.h | 6 +++---
> drivers/usb/musb/musb_dsps.c | 2 +-
> drivers/usb/musb/sunxi.c | 2 +-
> drivers/usb/musb/tusb6010.c | 2 +-
> 9 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
> index 50ca805..7136888 100644
> --- a/drivers/usb/musb/am35x.c
> +++ b/drivers/usb/musb/am35x.c
> @@ -332,7 +332,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
> return ret;
> }
>
> -static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
> {
> struct device *dev = musb->controller;
> struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
> diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
> index 310238c..544e98f 100644
> --- a/drivers/usb/musb/blackfin.c
> +++ b/drivers/usb/musb/blackfin.c
> @@ -356,7 +356,7 @@ static int bfin_musb_vbus_status(struct musb *musb)
> return 0;
> }
>
> -static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
> {
> return -EIO;
> }
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index 6749aa1..ac0c2f7 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -335,7 +335,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
> return ret;
> }
>
> -static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
> {
> struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
> enum phy_mode phy_mode;
> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
> index cee61a5..d12b902 100644
> --- a/drivers/usb/musb/davinci.c
> +++ b/drivers/usb/musb/davinci.c
> @@ -369,7 +369,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void *__hci)
> return retval;
> }
>
> -static int davinci_musb_set_mode(struct musb *musb, u8 mode)
> +static int davinci_musb_set_mode(struct musb *musb, u8 mode, bool init)
> {
> /* EVM can't do this (right?) */
> return -EIO;
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 27dadc0..4a8d394 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1730,11 +1730,11 @@ musb_mode_store(struct device *dev, struct device_attribute *attr,
>
> spin_lock_irqsave(&musb->lock, flags);
> if (sysfs_streq(buf, "host"))
> - status = musb_platform_set_mode(musb, MUSB_HOST);
> + status = musb_platform_set_mode(musb, MUSB_HOST, false);
> else if (sysfs_streq(buf, "peripheral"))
> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> + status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, false);
> else if (sysfs_streq(buf, "otg"))
> - status = musb_platform_set_mode(musb, MUSB_OTG);
> + status = musb_platform_set_mode(musb, MUSB_OTG, false);
> else
> status = -EINVAL;
> spin_unlock_irqrestore(&musb->lock, flags);
> @@ -2261,13 +2261,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> status = musb_host_setup(musb, plat->power);
> if (status < 0)
> goto fail3;
> - status = musb_platform_set_mode(musb, MUSB_HOST);
> + status = musb_platform_set_mode(musb, MUSB_HOST, true);
> break;
> case MUSB_PORT_MODE_GADGET:
> status = musb_gadget_setup(musb);
> if (status < 0)
> goto fail3;
> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> + status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, true);
> break;
> case MUSB_PORT_MODE_DUAL_ROLE:
> status = musb_host_setup(musb, plat->power);
> @@ -2278,7 +2278,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> musb_host_cleanup(musb);
> goto fail3;
> }
> - status = musb_platform_set_mode(musb, MUSB_OTG);
> + status = musb_platform_set_mode(musb, MUSB_OTG, true);
> break;
> default:
> dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index 2cb88a49..1bfc8fa 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -203,7 +203,7 @@ struct musb_platform_ops {
> struct dma_controller *
> (*dma_init) (struct musb *musb, void __iomem *base);
> void (*dma_exit)(struct dma_controller *c);
> - int (*set_mode)(struct musb *musb, u8 mode);
> + int (*set_mode)(struct musb *musb, u8 mode, bool init);
> void (*try_idle)(struct musb *musb, unsigned long timeout);
> int (*recover)(struct musb *musb);
>
> @@ -558,12 +558,12 @@ static inline void musb_platform_disable(struct musb *musb)
> musb->ops->disable(musb);
> }
>
> -static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> +static inline int musb_platform_set_mode(struct musb *musb, u8 mode, bool init)
> {
> if (!musb->ops->set_mode)
> return 0;
>
> - return musb->ops->set_mode(musb, mode);
> + return musb->ops->set_mode(musb, mode, init);
> }
>
> static inline void musb_platform_try_idle(struct musb *musb,
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 0f17d21..a889679 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -462,7 +462,7 @@ static int dsps_musb_exit(struct musb *musb)
> return 0;
> }
>
> -static int dsps_musb_set_mode(struct musb *musb, u8 mode)
> +static int dsps_musb_set_mode(struct musb *musb, u8 mode, bool init)
> {
> struct device *dev = musb->controller;
> struct dsps_glue *glue = dev_get_drvdata(dev->parent);
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 1408245..04bf763 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -346,7 +346,7 @@ static void sunxi_musb_dma_controller_destroy(struct dma_controller *c)
> {
> }
>
> -static int sunxi_musb_set_mode(struct musb *musb, u8 mode)
> +static int sunxi_musb_set_mode(struct musb *musb, u8 mode, bool init)
> {
> struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
> enum phy_mode new_mode;
> diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
> index df7c9f4..8a74587 100644
> --- a/drivers/usb/musb/tusb6010.c
> +++ b/drivers/usb/musb/tusb6010.c
> @@ -628,7 +628,7 @@ static void tusb_musb_set_vbus(struct musb *musb, int is_on)
> * Note that if a mini-A cable is plugged in the ID line will stay down as
> * the weak ID pull-up is not able to pull the ID up.
> */
> -static int tusb_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int tusb_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
> {
> void __iomem *tbase = musb->ctrl_base;
> u32 otg_stat, phy_otg_ctrl, phy_otg_ena, dev_conf;
> --
> 2.7.3
>

2016-11-14 17:37:53

by Alexandre Bailon

[permalink] [raw]
Subject: Re: [PATCH v5 3/4] usb: musb: Add a new argument to musb_platform_set_mode()

On 11/14/2016 06:36 PM, Bin Liu wrote:
> Hi,
>
> On Mon, Nov 07, 2016 at 02:05:07PM +0100, Alexandre Bailon wrote:
>> During the init, the driver will use musb_platform_set_mode()
>> to configure the controller mode and the PHY mode.
>> The PHY of DA8xx has some issues when the PHY is forced in host or device,
>> so we want to keep it in OTG mode unless the user request a specific mode
>> by using the sysfs.
>> Add the init argument to musb_platform_set_mode() in order to let the
>> callback change its behavior if it is called during the init.
>
> Tony's patch set which fixes musb pm regression (but has not been merged
> yet) introduces musb->is_initialized. You might want to use this new
> variable in da8xx_musb_set_mode(), instead of adding this init flag.
OK. I will take a look and update the series.
>
> Regards,
> -Bin.
>
>>
>> Signed-off-by: Alexandre Bailon <[email protected]>
>> ---
>> drivers/usb/musb/am35x.c | 2 +-
>> drivers/usb/musb/blackfin.c | 2 +-
>> drivers/usb/musb/da8xx.c | 2 +-
>> drivers/usb/musb/davinci.c | 2 +-
>> drivers/usb/musb/musb_core.c | 12 ++++++------
>> drivers/usb/musb/musb_core.h | 6 +++---
>> drivers/usb/musb/musb_dsps.c | 2 +-
>> drivers/usb/musb/sunxi.c | 2 +-
>> drivers/usb/musb/tusb6010.c | 2 +-
>> 9 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
>> index 50ca805..7136888 100644
>> --- a/drivers/usb/musb/am35x.c
>> +++ b/drivers/usb/musb/am35x.c
>> @@ -332,7 +332,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
>> return ret;
>> }
>>
>> -static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>> {
>> struct device *dev = musb->controller;
>> struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
>> diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
>> index 310238c..544e98f 100644
>> --- a/drivers/usb/musb/blackfin.c
>> +++ b/drivers/usb/musb/blackfin.c
>> @@ -356,7 +356,7 @@ static int bfin_musb_vbus_status(struct musb *musb)
>> return 0;
>> }
>>
>> -static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>> {
>> return -EIO;
>> }
>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>> index 6749aa1..ac0c2f7 100644
>> --- a/drivers/usb/musb/da8xx.c
>> +++ b/drivers/usb/musb/da8xx.c
>> @@ -335,7 +335,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
>> return ret;
>> }
>>
>> -static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>> {
>> struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
>> enum phy_mode phy_mode;
>> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
>> index cee61a5..d12b902 100644
>> --- a/drivers/usb/musb/davinci.c
>> +++ b/drivers/usb/musb/davinci.c
>> @@ -369,7 +369,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void *__hci)
>> return retval;
>> }
>>
>> -static int davinci_musb_set_mode(struct musb *musb, u8 mode)
>> +static int davinci_musb_set_mode(struct musb *musb, u8 mode, bool init)
>> {
>> /* EVM can't do this (right?) */
>> return -EIO;
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 27dadc0..4a8d394 100644
>> --- a/drivers/usb/musb/musb_core.c
>> +++ b/drivers/usb/musb/musb_core.c
>> @@ -1730,11 +1730,11 @@ musb_mode_store(struct device *dev, struct device_attribute *attr,
>>
>> spin_lock_irqsave(&musb->lock, flags);
>> if (sysfs_streq(buf, "host"))
>> - status = musb_platform_set_mode(musb, MUSB_HOST);
>> + status = musb_platform_set_mode(musb, MUSB_HOST, false);
>> else if (sysfs_streq(buf, "peripheral"))
>> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
>> + status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, false);
>> else if (sysfs_streq(buf, "otg"))
>> - status = musb_platform_set_mode(musb, MUSB_OTG);
>> + status = musb_platform_set_mode(musb, MUSB_OTG, false);
>> else
>> status = -EINVAL;
>> spin_unlock_irqrestore(&musb->lock, flags);
>> @@ -2261,13 +2261,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>> status = musb_host_setup(musb, plat->power);
>> if (status < 0)
>> goto fail3;
>> - status = musb_platform_set_mode(musb, MUSB_HOST);
>> + status = musb_platform_set_mode(musb, MUSB_HOST, true);
>> break;
>> case MUSB_PORT_MODE_GADGET:
>> status = musb_gadget_setup(musb);
>> if (status < 0)
>> goto fail3;
>> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
>> + status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, true);
>> break;
>> case MUSB_PORT_MODE_DUAL_ROLE:
>> status = musb_host_setup(musb, plat->power);
>> @@ -2278,7 +2278,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>> musb_host_cleanup(musb);
>> goto fail3;
>> }
>> - status = musb_platform_set_mode(musb, MUSB_OTG);
>> + status = musb_platform_set_mode(musb, MUSB_OTG, true);
>> break;
>> default:
>> dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
>> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
>> index 2cb88a49..1bfc8fa 100644
>> --- a/drivers/usb/musb/musb_core.h
>> +++ b/drivers/usb/musb/musb_core.h
>> @@ -203,7 +203,7 @@ struct musb_platform_ops {
>> struct dma_controller *
>> (*dma_init) (struct musb *musb, void __iomem *base);
>> void (*dma_exit)(struct dma_controller *c);
>> - int (*set_mode)(struct musb *musb, u8 mode);
>> + int (*set_mode)(struct musb *musb, u8 mode, bool init);
>> void (*try_idle)(struct musb *musb, unsigned long timeout);
>> int (*recover)(struct musb *musb);
>>
>> @@ -558,12 +558,12 @@ static inline void musb_platform_disable(struct musb *musb)
>> musb->ops->disable(musb);
>> }
>>
>> -static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
>> +static inline int musb_platform_set_mode(struct musb *musb, u8 mode, bool init)
>> {
>> if (!musb->ops->set_mode)
>> return 0;
>>
>> - return musb->ops->set_mode(musb, mode);
>> + return musb->ops->set_mode(musb, mode, init);
>> }
>>
>> static inline void musb_platform_try_idle(struct musb *musb,
>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>> index 0f17d21..a889679 100644
>> --- a/drivers/usb/musb/musb_dsps.c
>> +++ b/drivers/usb/musb/musb_dsps.c
>> @@ -462,7 +462,7 @@ static int dsps_musb_exit(struct musb *musb)
>> return 0;
>> }
>>
>> -static int dsps_musb_set_mode(struct musb *musb, u8 mode)
>> +static int dsps_musb_set_mode(struct musb *musb, u8 mode, bool init)
>> {
>> struct device *dev = musb->controller;
>> struct dsps_glue *glue = dev_get_drvdata(dev->parent);
>> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
>> index 1408245..04bf763 100644
>> --- a/drivers/usb/musb/sunxi.c
>> +++ b/drivers/usb/musb/sunxi.c
>> @@ -346,7 +346,7 @@ static void sunxi_musb_dma_controller_destroy(struct dma_controller *c)
>> {
>> }
>>
>> -static int sunxi_musb_set_mode(struct musb *musb, u8 mode)
>> +static int sunxi_musb_set_mode(struct musb *musb, u8 mode, bool init)
>> {
>> struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
>> enum phy_mode new_mode;
>> diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
>> index df7c9f4..8a74587 100644
>> --- a/drivers/usb/musb/tusb6010.c
>> +++ b/drivers/usb/musb/tusb6010.c
>> @@ -628,7 +628,7 @@ static void tusb_musb_set_vbus(struct musb *musb, int is_on)
>> * Note that if a mini-A cable is plugged in the ID line will stay down as
>> * the weak ID pull-up is not able to pull the ID up.
>> */
>> -static int tusb_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int tusb_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>> {
>> void __iomem *tbase = musb->ctrl_base;
>> u32 otg_stat, phy_otg_ctrl, phy_otg_ena, dev_conf;
>> --
>> 2.7.3
>>

2016-11-15 13:13:12

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround



On Monday 07 November 2016 06:35 PM, Alexandre Bailon wrote:
> If we configure the da8xx OTG phy in OTG mode, neither device or host
> mode will work. That is because the PHY is not able to detect and notify
> the driver that value of ID pin changed.
> To work despite this hardware limitation, the da8xx glue implement a
> workaround.
> But to work, the workaround require the VBUS sense and the session end
> comparator to enabled.
> Enable them if the phy is configured in OTG mode.
>
> Signed-off-by: Alexandre Bailon <[email protected]>

merged this to phy -next.

Thanks
Kishon
> ---
> drivers/phy/phy-da8xx-usb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
> index 32ae78c..089c13b 100644
> --- a/drivers/phy/phy-da8xx-usb.c
> +++ b/drivers/phy/phy-da8xx-usb.c
> @@ -23,6 +23,8 @@
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
>
> +#define PHY_INIT_BITS (CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN)
> +
> struct da8xx_usb_phy {
> struct phy_provider *phy_provider;
> struct phy *usb11_phy;
> @@ -207,6 +209,9 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
> dev_warn(dev, "Failed to create usb20 phy lookup\n");
> }
>
> + regmap_write_bits(d_phy->regmap, CFGCHIP(2),
> + PHY_INIT_BITS, PHY_INIT_BITS);
> +
> return 0;
> }
>
>