RE: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi,
> On Thu, Oct 23, 2014 at 10:18:41PM -0500, Felipe Balbi wrote:
> > HI,
> >
> > On Fri, Oct 24, 2014 at 10:14:19AM +0900, Anton Tikhomirov wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 23 Oct 2014, Alan Stern wrote:
> > > > On Thu, 23 Oct 2014, Felipe Balbi wrote:
> > > >
> > > > > here's v2:
> > > > >
> > > > > 8<-
> -
> > > > >
> > > > > From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17
> 00:00:00
> > > > 2001
> > > > > From: Felipe Balbi
> > > > > Date: Tue, 30 Sep 2014 10:39:14 -0500
> > > > > Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer
> sizes
> > > > aligned to
> > > > > wMaxPacketSize
> > > > >
> > > > > According to Section 8.5.3.2 of the USB 2.0 specification,
> > > > > a USB device must terminate a Data Phase with either a
> > > > > short packet or a ZLP (if the previous transfer was
> > > > > a multiple of wMaxPacketSize).
> > > > >
> > > > > For reference, here's what the USB 2.0 specification, section
> > > > > 8.5.3.2 says:
> > > > >
> > > > > "
> > > > > 8.5.3.2 Variable-length Data Stage
> > > > >
> > > > > A control pipe may have a variable-length data phase
> > > > > in which the host requests more data than is contained
> > > > > in the specified data structure. When all of the data
> > > > > structure is returned to the host, the function should
> > > > > indicate that the Data stage is ended by returning a
> > > > > packet that is shorter than the MaxPacketSize for the
> > > > > pipe. If the data structure is an exact multiple of
> > > > > wMaxPacketSize for the pipe, the function will return
> > > > > a zero-length packet to indicate the end of the Data
> > > > > stage.
> > > > > "
> > > > >
> > > > > Signed-off-by: Felipe Balbi
> > > > > ---
> > > > > drivers/usb/dwc3/ep0.c | 19 +--
> > > > > 1 file changed, 13 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> > > > > index a47cc1e..ce6b0c7 100644
> > > > > --- a/drivers/usb/dwc3/ep0.c
> > > > > +++ b/drivers/usb/dwc3/ep0.c
> > > > > @@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct
> dwc3
> > > > *dwc,
> > > > >
> > > > > dwc3_ep0_stall_and_restart(dwc);
> > > > > } else {
> > > > > - /*
> > > > > - * handle the case where we have to send a zero
> packet.
> > > > This
> > > > > - * seems to be case when req.length > maxpacket.
> Could it
> > > > be?
> > > > > - */
> > > > > - if (r)
> > > > > - dwc3_gadget_giveback(ep0, r, 0);
> > > > > + dwc3_gadget_giveback(ep0, r, 0);
> > > >
> > > > Don't you want to wait until the ZLP has completed before doing
> the
> > > > giveback?
> > > >
> > > > In fact, shouldn't all this ZLP code run when the transfer is
> > > > submitted, rather than when it completes? The idea is that you
> get a
> > > > request, you queue up all the necessary TRBs or whatever, and if
> an
> > > > extra ZLP is needed then you queue up an extra TRB.
> > >
> > > You may want to check my patch one more time. I sent it for review
> 10
> > > monthes ago:
> > >
> > > [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
> > >
> > > It works just fine for us in our custom kernel.
> >
> > you also said you'd send another version (see [1]) and never did. 10
> > months passed and I had even forgotten about this issue until I
> decided
> > to run all gadget drivers through USB[23]0CV and found that g_ncm
> falls
> > into this same bug, so I wrote the patch above.
I'm sorry about this. I was busy with current project at that time and
finally forgot about this issue too.
> >
> > considering you never sent another version even after 10 months, I'll
> > just go ahead with this one and work on improving TRB handling on
> this
> > driver so that when req->zero is true we can actually allocate a
> > separate TRB (as has been discussed on this and previous threads).
> >
> > I'll add a note to commit log stating that you provided the original
> > patch but failed to provide a follow up.
>
> actually, I can't do that anymore as I have already moved this commit
> to
> my 'fixes' branch.
It's ok
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi,
On Thu, 23 Oct 2014, Alan Stern wrote:
> On Thu, 23 Oct 2014, Felipe Balbi wrote:
>
> > here's v2:
> >
> > 8<--
> >
> > From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00
> 2001
> > From: Felipe Balbi
> > Date: Tue, 30 Sep 2014 10:39:14 -0500
> > Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes
> aligned to
> > wMaxPacketSize
> >
> > According to Section 8.5.3.2 of the USB 2.0 specification,
> > a USB device must terminate a Data Phase with either a
> > short packet or a ZLP (if the previous transfer was
> > a multiple of wMaxPacketSize).
> >
> > For reference, here's what the USB 2.0 specification, section
> > 8.5.3.2 says:
> >
> > "
> > 8.5.3.2 Variable-length Data Stage
> >
> > A control pipe may have a variable-length data phase
> > in which the host requests more data than is contained
> > in the specified data structure. When all of the data
> > structure is returned to the host, the function should
> > indicate that the Data stage is ended by returning a
> > packet that is shorter than the MaxPacketSize for the
> > pipe. If the data structure is an exact multiple of
> > wMaxPacketSize for the pipe, the function will return
> > a zero-length packet to indicate the end of the Data
> > stage.
> > "
> >
> > Signed-off-by: Felipe Balbi
> > ---
> > drivers/usb/dwc3/ep0.c | 19 +--
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> > index a47cc1e..ce6b0c7 100644
> > --- a/drivers/usb/dwc3/ep0.c
> > +++ b/drivers/usb/dwc3/ep0.c
> > @@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3
> *dwc,
> >
> > dwc3_ep0_stall_and_restart(dwc);
> > } else {
> > - /*
> > -* handle the case where we have to send a zero packet.
> This
> > -* seems to be case when req.length > maxpacket. Could it
> be?
> > -*/
> > - if (r)
> > - dwc3_gadget_giveback(ep0, r, 0);
> > + dwc3_gadget_giveback(ep0, r, 0);
>
> Don't you want to wait until the ZLP has completed before doing the
> giveback?
>
> In fact, shouldn't all this ZLP code run when the transfer is
> submitted, rather than when it completes? The idea is that you get a
> request, you queue up all the necessary TRBs or whatever, and if an
> extra ZLP is needed then you queue up an extra TRB.
You may want to check my patch one more time. I sent it for review 10
monthes ago:
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
It works just fine for us in our custom kernel.
>
> > +
> > + if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket) &&
> > + ur->zero) {
>
> Is this correct in the case where ur->length is 0? When that happens,
> there should be only one ZLP, not two.
>
> > + int ret;
> > +
> > + dwc->ep0_next_event = DWC3_EP0_COMPLETE;
> > +
> > + ret = dwc3_ep0_start_trans(dwc, epnum,
> > + dwc->ctrl_req_addr, 0,
> > + DWC3_TRBCTL_CONTROL_DATA);
> > + WARN_ON(ret < 0);
> > + }
> > }
> > }
>
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi, > On Mon, 20 Oct 2014, Paul Zimmerman wrote: > > > > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of David Laight > > > Sent: Monday, October 20, 2014 2:48 AM > > > > > > From: Felipe Balbi > > > > According to Section 8.5.3.2 of the USB 2.0 specification, > > > > a USB device must terminate a Data Phase with either a > > > > short packet or a ZLP (if the previous transfer was > > > > a multiple of wMaxPacketSize). > > > > > > > > For reference, here's what the USB 2.0 specification, section > > > > 8.5.3.2 says: > > > > > > > > " > > > > 8.5.3.2 Variable-length Data Stage > > > > > > > > A control pipe may have a variable-length data phase > > > > in which the host requests more data than is contained > > > > in the specified data structure. When all of the data > > > > structure is returned to the host, the function should > > > > indicate that the Data stage is ended by returning a > > > > packet that is shorter than the MaxPacketSize for the > > > > pipe. If the data structure is an exact multiple of > > > > wMaxPacketSize for the pipe, the function will return > > > > a zero-length packet to indicate the end of the Data > > > > stage. > > > > " > > > > > > If that is the same as my understanding of the USB3 spec then the > > > requirement for a ZLP isn't unconditional. > > > > > > In particular if the data phase isn't variable length then a ZLP > > > must not be added. > > > > Also, in the USB 3.0 spec, the corresponding section has been > modified > > a bit. The last sentence has been changed to this: > > > > "Note that if the amount of data in the data structure that is > returned > > to the host *is less than the amount requested by the host* and is an > > exact multiple of maximum packet size then a control endpoint shall > send > > a zero length DP to terminate the data stage." (emphasis mine) > > > > So I think you also need to take into account the wLength field of > the > > request, and only send the ZLP if the amount of data returned is less > > than wLength. > > That's right, and it's true for USB-2 as well. A ZLP is needed only in > cases where the host otherwise wouldn't know the transfer is over, > i.e., when the transfer length is a nonzero multiple of the maxpacket > size and is smaller than wLength. Shall we use/check struct usb_request's zero flag for this? > > It's not clear what a "variable length" control transfer is supposed to > be. I guess it means that sometimes the device will send back less > than wLength bytes of data. > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7
Hello, > Hi Anton, > > On 13.10.2014 06:54, Anton Tikhomirov wrote: > > Hi Vivek, > > > >> Exynos7 also has a separate special gate clock going to the IP > >> apart from the usual AHB clock. So add support for the same. > > > > As we discussed before, Exynos7 SoCs have 7 clocks to be controlled > > by the driver. Adding only sclk is not enough. > > > > I'm quite interested in this discussion. Has it happened on mailing > lists? No, we used company messenger for the discussion. > > In general, previous SoCs also gave the possibility of controlling all > the bus clocks separately, in addition to bulk gates, but there was no correct > real advantage in using those, while burdening the clock tree with > numerous clocks. Isn't Exynos7 similar in this aspect? Exynos7 doesn't have "Gating all clocks for USBDRD30" bit. The clocks should be controlled separately. > > Best regards, > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 3/4] phy: exynos5-usbdrd: Add facility for VBUS-BOOST-5V supply
Hi Vivek,
> Some Exynos SoCs have a separate regulator controlling a
I guess you meant the Exynos based *boards* instead of SoCs,
since Exynos SoCs don't have any boost regulators.
> Boost 5V supply which goes as input for VBUS regulator.
> So adding a control for the same in driver, to enable
> vbus supply on the port.
>
> Signed-off-by: Vivek Gautam
> ---
> drivers/phy/phy-exynos5-usbdrd.c | 30 --
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-
> exynos5-usbdrd.c
> index 013ee84..57e8a0a 100644
> --- a/drivers/phy/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/phy-exynos5-usbdrd.c
> @@ -176,6 +176,7 @@ struct exynos5_usbdrd_phy {
> u32 extrefclk;
> struct clk *ref_clk;
> struct regulator *vbus;
> + struct regulator *vbus_boost;
> };
>
> static inline
> @@ -455,11 +456,20 @@ static int exynos5_usbdrd_phy_power_on(struct phy
> *phy)
> clk_prepare_enable(phy_drd->ref_clk);
>
> /* Enable VBUS supply */
> + if (phy_drd->vbus_boost) {
> + ret = regulator_enable(phy_drd->vbus_boost);
> + if (ret) {
> + dev_err(phy_drd->dev,
> + "Failed to enable VBUS boost supply\n");
> + goto fail_vbus;
> + }
> + }
> +
> if (phy_drd->vbus) {
> ret = regulator_enable(phy_drd->vbus);
> if (ret) {
> dev_err(phy_drd->dev, "Failed to enable VBUS
> supply\n");
> - goto fail_vbus;
> + goto fail_vbus_boost;
> }
> }
>
> @@ -468,6 +478,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy
> *phy)
>
> return 0;
>
> +fail_vbus_boost:
> + if (phy_drd->vbus_boost)
> + regulator_disable(phy_drd->vbus_boost);
> +
> fail_vbus:
> clk_disable_unprepare(phy_drd->ref_clk);
> clk_disable_unprepare(phy_drd->pipeclk);
> @@ -489,6 +503,8 @@ static int exynos5_usbdrd_phy_power_off(struct phy
> *phy)
> /* Disable VBUS supply */
> if (phy_drd->vbus)
> regulator_disable(phy_drd->vbus);
> + if (phy_drd->vbus_boost)
> + regulator_disable(phy_drd->vbus_boost);
>
> clk_disable_unprepare(phy_drd->ref_clk);
> clk_disable_unprepare(phy_drd->pipeclk);
> @@ -644,7 +660,7 @@ static int exynos5_usbdrd_phy_probe(struct
> platform_device *pdev)
> break;
> }
>
> - /* Get Vbus regulator */
> + /* Get Vbus regulators */
> phy_drd->vbus = devm_regulator_get(dev, "vbus");
> if (IS_ERR(phy_drd->vbus)) {
> ret = PTR_ERR(phy_drd->vbus);
> @@ -655,6 +671,16 @@ static int exynos5_usbdrd_phy_probe(struct
> platform_device *pdev)
> phy_drd->vbus = NULL;
> }
>
> + phy_drd->vbus_boost = devm_regulator_get(dev, "vbus-boost");
> + if (IS_ERR(phy_drd->vbus_boost)) {
> + ret = PTR_ERR(phy_drd->vbus_boost);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + dev_warn(dev, "Failed to get VBUS boost supply
> regulator\n");
> + phy_drd->vbus_boost = NULL;
> + }
> +
> dev_vdbg(dev, "Creating usbdrd_phy phy\n");
>
> for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 2/4] phy: exynos5-usbdrd: Add pipe-clk and utmi-clk support
Hi Vivek,
> Exynos7 SoC has now separate gate control for 125MHz pipe3 phy
> clock, as well as 60MHz utmi phy clock.
> So get the same and control in the phy-exynos5-usbdrd driver.
In case of the PHY the situation is pretty much the same as with
DWC3 core. Here we should control 6 clocks to make Exynos7 USB DRD
PHY working.
By the way, the driver name phy-exynos5-usbdrd.c doesn't imply it
supports Exynos7 :)
>
> Signed-off-by: Vivek Gautam
> ---
> .../devicetree/bindings/phy/samsung-phy.txt|4
> drivers/phy/phy-exynos5-usbdrd.c | 22
>
> 2 files changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 15e0f2c..c2bc9dc 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -138,6 +138,10 @@ Required properties:
> PHY operations, associated by phy name. It is used to
> determine bit values for clock settings register.
> For Exynos5420 this is given as 'sclk_usbphy30' in CMU.
> + - optional clocks: Exynos7 SoC has now following additional
> +gate clocks available:
> +- phy_pipe: for PIPE3 phy
> +- phy_utmi: for UTMI+ phy
> - samsung,pmu-syscon: phandle for PMU system controller interface,
> used to
> control pmu registers for power isolation.
> - #phy-cells : from the generic PHY bindings, must be 1;
> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-
> exynos5-usbdrd.c
> index f756aca..013ee84 100644
> --- a/drivers/phy/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/phy-exynos5-usbdrd.c
> @@ -148,6 +148,8 @@ struct exynos5_usbdrd_phy_drvdata {
> * @dev: pointer to device instance of this platform device
> * @reg_phy: usb phy controller register memory base
> * @clk: phy clock for register access
> + * @pipeclk: clock for pipe3 phy
> + * @utmiclk: clock for utmi+ phy
> * @drv_data: pointer to SoC level driver data structure
> * @phys[]: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY
> * instances each with its 'phy' and 'phy_cfg'.
> @@ -161,6 +163,8 @@ struct exynos5_usbdrd_phy {
> struct device *dev;
> void __iomem *reg_phy;
> struct clk *clk;
> + struct clk *pipeclk;
> + struct clk *utmiclk;
> const struct exynos5_usbdrd_phy_drvdata *drv_data;
> struct phy_usb_instance {
> struct phy *phy;
> @@ -446,6 +450,8 @@ static int exynos5_usbdrd_phy_power_on(struct phy
> *phy)
>
> dev_dbg(phy_drd->dev, "Request to power_on usbdrd_phy phy\n");
>
> + clk_prepare_enable(phy_drd->utmiclk);
> + clk_prepare_enable(phy_drd->pipeclk);
> clk_prepare_enable(phy_drd->ref_clk);
>
> /* Enable VBUS supply */
> @@ -464,6 +470,8 @@ static int exynos5_usbdrd_phy_power_on(struct phy
> *phy)
>
> fail_vbus:
> clk_disable_unprepare(phy_drd->ref_clk);
> + clk_disable_unprepare(phy_drd->pipeclk);
> + clk_disable_unprepare(phy_drd->utmiclk);
>
> return ret;
> }
> @@ -483,6 +491,8 @@ static int exynos5_usbdrd_phy_power_off(struct phy
> *phy)
> regulator_disable(phy_drd->vbus);
>
> clk_disable_unprepare(phy_drd->ref_clk);
> + clk_disable_unprepare(phy_drd->pipeclk);
> + clk_disable_unprepare(phy_drd->utmiclk);
>
> return 0;
> }
> @@ -582,6 +592,18 @@ static int exynos5_usbdrd_phy_probe(struct
> platform_device *pdev)
> return PTR_ERR(phy_drd->clk);
> }
>
> + phy_drd->pipeclk = devm_clk_get(dev, "phy_pipe");
> + if (IS_ERR(phy_drd->pipeclk)) {
> + dev_info(dev, "PIPE3 phy operational clock not
> specified\n");
> + phy_drd->pipeclk = NULL;
> + }
> +
> + phy_drd->utmiclk = devm_clk_get(dev, "phy_utmi");
> + if (IS_ERR(phy_drd->utmiclk)) {
> + dev_info(dev, "UTMI phy operational clock not specified\n");
> + phy_drd->utmiclk = NULL;
> + }
> +
> phy_drd->ref_clk = devm_clk_get(dev, "ref");
> if (IS_ERR(phy_drd->ref_clk)) {
> dev_err(dev, "Failed to get reference clock of usbdrd
> phy\n");
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7
Hi Vivek,
> Exynos7 also has a separate special gate clock going to the IP
> apart from the usual AHB clock. So add support for the same.
As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
by the driver. Adding only sclk is not enough.
>
> Signed-off-by: Vivek Gautam
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> exynos.c
> index 3951a65..7dc6a98 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -35,6 +35,7 @@ struct dwc3_exynos {
> struct device *dev;
>
> struct clk *clk;
The clock "clk" in Exynos5 just gated all that above 7 clocks, which
we should control separately now in Exynos7.
> + struct clk *sclk;
> struct regulator*vdd33;
> struct regulator*vdd10;
> };
> @@ -139,10 +140,21 @@ static int dwc3_exynos_probe(struct
> platform_device *pdev)
> return -EINVAL;
> }
>
> + /*
> + * Exynos7 has a special gate clock going to this IP,
> + * which in earlier SoCs was probably concealed.
> + */
> + exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
> + if (IS_ERR(exynos->sclk)) {
> + dev_info(dev, "no sclk specified\n");
> + exynos->sclk = NULL;
> + }
> +
> exynos->dev = dev;
> exynos->clk = clk;
>
> clk_prepare_enable(exynos->clk);
> + clk_prepare_enable(exynos->sclk);
>
> exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> if (IS_ERR(exynos->vdd33)) {
> @@ -185,6 +197,7 @@ err4:
> err3:
> regulator_disable(exynos->vdd33);
> err2:
> + clk_disable_unprepare(exynos->sclk);
> clk_disable_unprepare(clk);
> return ret;
> }
> @@ -197,6 +210,7 @@ static int dwc3_exynos_remove(struct
> platform_device *pdev)
> platform_device_unregister(exynos->usb2_phy);
> platform_device_unregister(exynos->usb3_phy);
>
> + clk_disable_unprepare(exynos->sclk);
> clk_disable_unprepare(exynos->clk);
>
> regulator_disable(exynos->vdd33);
> @@ -218,6 +232,7 @@ static int dwc3_exynos_suspend(struct device *dev)
> {
> struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>
> + clk_disable(exynos->sclk);
> clk_disable(exynos->clk);
>
> regulator_disable(exynos->vdd33);
> @@ -243,6 +258,7 @@ static int dwc3_exynos_resume(struct device *dev)
> }
>
> clk_enable(exynos->clk);
> + clk_enable(exynos->sclk);
>
> /* runtime set active to reflect active state. */
> pm_runtime_disable(dev);
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: exynos5-drd: Fix PHYPARAM1_PCS_TXDEEMPH definition
According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 register is 6bits wide, so mask value should be 0x3f instead of 0x1f. Additionally, this patch renames the macro to correctly reflect the field name which we see in SoC documentation. Signed-off-by: Anton Tikhomirov --- drivers/phy/phy-exynos5-usbdrd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5- usbdrd.c index 392101c..216bbf8 100644 --- a/drivers/phy/phy-exynos5-usbdrd.c +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -99,8 +99,8 @@ #define EXYNOS5_DRD_PHYPARAM1 0x20 -#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f << 0) -#define PHYPARAM1_PCS_TXDEEMPH (0x1c) +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK (0x3f << 0) +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB (0x1c) #define EXYNOS5_DRD_PHYTERM0x24 @@ -309,8 +309,8 @@ static void exynos5_usbdrd_pipe3_init(struct exynos5_usbdrd_phy *phy_drd) reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); /* Set Tx De-Emphasis level */ - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; - reg |= PHYPARAM1_PCS_TXDEEMPH; + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); @@ -330,8 +330,8 @@ static void exynos5_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd) reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); /* Set Tx De-Emphasis level */ - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; - reg |= PHYPARAM1_PCS_TXDEEMPH; + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); /* UTMI Power Control */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] phy: exynos5-drd: Fix PCS_TXDEEMPH mask
Hi Vivek, > Hi Anton, > > > On Fri, Sep 19, 2014 at 1:05 PM, Anton Tikhomirov > wrote: > > According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 > > register is 6bits wide, so mask value should be 0x3f instead > > of 0x1f. > > > > Signed-off-by: Anton Tikhomirov > > --- > > drivers/phy/phy-exynos5-usbdrd.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy- > exynos5- > > usbdrd.c > > index 392101c..f421d16 100644 > > --- a/drivers/phy/phy-exynos5-usbdrd.c > > +++ b/drivers/phy/phy-exynos5-usbdrd.c > > @@ -99,7 +99,7 @@ > > > > #define EXYNOS5_DRD_PHYPARAM1 0x20 > > > > -#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f << 0) > > +#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x3f << 0) > > use a "_3P5DB" suffix for the macro name ? may sound similar to how UM > says. Agree. Please drop this patch. I will make new one. > anyways verified from Exynos5420 user manual. > > with that change, > Reviewed-by: Vivek Gautam > > > [snip] > > > -- > Best Regards > Vivek Gautam > Samsung R&D Institute, Bangalore > India > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: exynos5-drd: Fix PCS_TXDEEMPH mask
According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 register is 6bits wide, so mask value should be 0x3f instead of 0x1f. Signed-off-by: Anton Tikhomirov --- drivers/phy/phy-exynos5-usbdrd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5- usbdrd.c index 392101c..f421d16 100644 --- a/drivers/phy/phy-exynos5-usbdrd.c +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -99,7 +99,7 @@ #define EXYNOS5_DRD_PHYPARAM1 0x20 -#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f << 0) +#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x3f << 0) #define PHYPARAM1_PCS_TXDEEMPH (0x1c) #define EXYNOS5_DRD_PHYTERM0x24 -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] usb: ohci-exynos: Make provision for vdd regulators
Hi,
> Hi,
>
> > -Original Message-
> > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> > ow...@vger.kernel.org] On Behalf Of Vivek Gautam
> > Sent: Monday, April 21, 2014 9:17 PM
> >
> > Facilitate getting required 3.3V and 1.0V VDD supply for
> > OHCI controller on Exynos.
> >
> > With patches for regulators' nodes merged in 3.15:
> > c8c253f ARM: dts: Add regulator entries to smdk5420
> > 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
> >
> > certain perripherals will now need to ensure that,
> > they request VDD regulators in their drivers, and enable
> > them so as to make them working.
> >
> > Signed-off-by: Vivek Gautam
> > Cc: Jingoo Han
> > ---
> >
> > Based on 'usb-next' branch of Greg's usb tree.
> >
> > drivers/usb/host/ohci-exynos.c | 47
> >
> > 1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-
> > exynos.c
> > index 68588d8..e2e72a8 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -18,6 +18,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -37,6 +38,8 @@ struct exynos_ohci_hcd {
> > struct clk *clk;
> > struct usb_phy *phy;
> > struct usb_otg *otg;
> > + struct regulator *vdd33;
> > + struct regulator *vdd10;
> > };
> >
> > static void exynos_ohci_phy_enable(struct platform_device *pdev)
> > @@ -98,6 +101,28 @@ static int exynos_ohci_probe(struct
> platform_device
> > *pdev)
> > exynos_ohci->otg = phy->otg;
> > }
> >
> > + exynos_ohci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
> > + if (IS_ERR(exynos_ohci->vdd33)) {
> > + err = PTR_ERR(exynos_ohci->vdd33);
> > + goto fail_regulator1;
> > + }
> > + err = regulator_enable(exynos_ohci->vdd33);
> > + if (err) {
> > + dev_err(&pdev->dev, "Failed to enable VDD33 supply\n");
> > + goto fail_regulator1;
> > + }
> > +
> > + exynos_ohci->vdd10 = devm_regulator_get(&pdev->dev, "vdd10");
> > + if (IS_ERR(exynos_ohci->vdd10)) {
> > + err = PTR_ERR(exynos_ohci->vdd10);
> > + goto fail_regulator2;
> > + }
> > + err = regulator_enable(exynos_ohci->vdd10);
> > + if (err) {
> > + dev_err(&pdev->dev, "Failed to enable VDD10 supply\n");
> > + goto fail_regulator2;
> > + }
> > +
>
> Do we need to skip regulator settings together with PHY configuration
> in case of exynos5440?
>
> > skip_phy:
> > exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
> >
> > @@ -154,6 +179,10 @@ fail_add_hcd:
> > fail_io:
> > clk_disable_unprepare(exynos_ohci->clk);
> > fail_clk:
> > + regulator_disable(exynos_ohci->vdd10);
> > +fail_regulator2:
> > + regulator_disable(exynos_ohci->vdd33);
> > +fail_regulator1:
> > usb_put_hcd(hcd);
> > return err;
> > }
> > @@ -172,6 +201,9 @@ static int exynos_ohci_remove(struct
> > platform_device *pdev)
> >
> > clk_disable_unprepare(exynos_ohci->clk);
> >
> > + regulator_disable(exynos_ohci->vdd10);
> > + regulator_disable(exynos_ohci->vdd33);
> > +
> > usb_put_hcd(hcd);
> >
> > return 0;
> > @@ -208,6 +240,9 @@ static int exynos_ohci_suspend(struct device *dev)
> >
> > clk_disable_unprepare(exynos_ohci->clk);
> >
> > + regulator_disable(exynos_ohci->vdd10);
> > + regulator_disable(exynos_ohci->vdd33);
> > +
> > spin_unlock_irqrestore(&ohci->lock, flags);
> >
> > return 0;
> > @@ -218,6 +253,18 @@ static int exynos_ohci_resume(struct device *dev)
> > struct usb_hcd *hcd = dev_get_drvdata(dev);
> > struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> > struct platform_device *pdev= to_platform_device(dev);
> > + int ret;
> > +
> > + ret = regulator_enable(exynos_ohci->vdd33);
> > + if (ret) {
> > + dev_err(dev, "Failed to enable VDD33 supply\n");
> > + return ret;
>
> Not sure, but shall we continue resuming and do everything
> we can in case of error? At least it will avoid
> WARN_ON(clk->enable_count == 0) on next system suspend.
On the other hand, if power is not applied to the controller,
register access in ohci_resume() may lead to undefined behavior.
What's your opinion?
>
> > + }
> > + ret = regulator_enable(exynos_ohci->vdd10);
> > + if (ret) {
> > + dev_err(dev, "Failed to enable VDD10 supply\n");
> > + return ret;
> > + }
> >
> > clk_prepare_enable(exynos_ohci->clk);
> >
> > --
>
> Thanks
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] usb: ohci-exynos: Make provision for vdd regulators
Hi,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Vivek Gautam
> Sent: Monday, April 21, 2014 9:17 PM
>
> Facilitate getting required 3.3V and 1.0V VDD supply for
> OHCI controller on Exynos.
>
> With patches for regulators' nodes merged in 3.15:
> c8c253f ARM: dts: Add regulator entries to smdk5420
> 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
>
> certain perripherals will now need to ensure that,
> they request VDD regulators in their drivers, and enable
> them so as to make them working.
>
> Signed-off-by: Vivek Gautam
> Cc: Jingoo Han
> ---
>
> Based on 'usb-next' branch of Greg's usb tree.
>
> drivers/usb/host/ohci-exynos.c | 47
>
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-
> exynos.c
> index 68588d8..e2e72a8 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -37,6 +38,8 @@ struct exynos_ohci_hcd {
> struct clk *clk;
> struct usb_phy *phy;
> struct usb_otg *otg;
> + struct regulator *vdd33;
> + struct regulator *vdd10;
> };
>
> static void exynos_ohci_phy_enable(struct platform_device *pdev)
> @@ -98,6 +101,28 @@ static int exynos_ohci_probe(struct platform_device
> *pdev)
> exynos_ohci->otg = phy->otg;
> }
>
> + exynos_ohci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
> + if (IS_ERR(exynos_ohci->vdd33)) {
> + err = PTR_ERR(exynos_ohci->vdd33);
> + goto fail_regulator1;
> + }
> + err = regulator_enable(exynos_ohci->vdd33);
> + if (err) {
> + dev_err(&pdev->dev, "Failed to enable VDD33 supply\n");
> + goto fail_regulator1;
> + }
> +
> + exynos_ohci->vdd10 = devm_regulator_get(&pdev->dev, "vdd10");
> + if (IS_ERR(exynos_ohci->vdd10)) {
> + err = PTR_ERR(exynos_ohci->vdd10);
> + goto fail_regulator2;
> + }
> + err = regulator_enable(exynos_ohci->vdd10);
> + if (err) {
> + dev_err(&pdev->dev, "Failed to enable VDD10 supply\n");
> + goto fail_regulator2;
> + }
> +
Do we need to skip regulator settings together with PHY configuration
in case of exynos5440?
> skip_phy:
> exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
>
> @@ -154,6 +179,10 @@ fail_add_hcd:
> fail_io:
> clk_disable_unprepare(exynos_ohci->clk);
> fail_clk:
> + regulator_disable(exynos_ohci->vdd10);
> +fail_regulator2:
> + regulator_disable(exynos_ohci->vdd33);
> +fail_regulator1:
> usb_put_hcd(hcd);
> return err;
> }
> @@ -172,6 +201,9 @@ static int exynos_ohci_remove(struct
> platform_device *pdev)
>
> clk_disable_unprepare(exynos_ohci->clk);
>
> + regulator_disable(exynos_ohci->vdd10);
> + regulator_disable(exynos_ohci->vdd33);
> +
> usb_put_hcd(hcd);
>
> return 0;
> @@ -208,6 +240,9 @@ static int exynos_ohci_suspend(struct device *dev)
>
> clk_disable_unprepare(exynos_ohci->clk);
>
> + regulator_disable(exynos_ohci->vdd10);
> + regulator_disable(exynos_ohci->vdd33);
> +
> spin_unlock_irqrestore(&ohci->lock, flags);
>
> return 0;
> @@ -218,6 +253,18 @@ static int exynos_ohci_resume(struct device *dev)
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
> struct platform_device *pdev= to_platform_device(dev);
> + int ret;
> +
> + ret = regulator_enable(exynos_ohci->vdd33);
> + if (ret) {
> + dev_err(dev, "Failed to enable VDD33 supply\n");
> + return ret;
Not sure, but shall we continue resuming and do everything
we can in case of error? At least it will avoid
WARN_ON(clk->enable_count == 0) on next system suspend.
> + }
> + ret = regulator_enable(exynos_ohci->vdd10);
> + if (ret) {
> + dev_err(dev, "Failed to enable VDD10 supply\n");
> + return ret;
> + }
>
> clk_prepare_enable(exynos_ohci->clk);
>
> --
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] usb: dwc3-exynos: Make provision for vdd regulators
Hi,
> Hi Anton,
>
>
> On Wed, Apr 23, 2014 at 2:56 PM, Anton Tikhomirov
> wrote:
> > Hello,
> >
> >> -Original Message-
> >> From: Vivek Gautam [mailto:gautamvivek1...@gmail.com] On Behalf Of
> >> Vivek Gautam
> >> Sent: Monday, April 21, 2014 9:17 PM
> >> To: linux-usb@vger.kernel.org; linux-samsung-...@vger.kernel.org
> >> Cc: linux-ker...@vger.kernel.org; linux-o...@vger.kernel.org; linux-
> >> arm-ker...@lists.infradead.org; gre...@linuxfoundation.org;
> >> st...@rowland.harvard.edu; ba...@ti.com; kgene@samsung.com;
> Vivek
> >> Gautam; Anton Tikhomirov
> >> Subject: [PATCH 3/3] usb: dwc3-exynos: Make provision for vdd
> >> regulators
> >>
> >> Facilitate getting required 3.3V and 1.0V VDD supply for
> >> DWC3 controller on Exynos.
> >>
> >> With patches for regulators' nodes merged in 3.15:
> >> c8c253f ARM: dts: Add regulator entries to smdk5420
> >> 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
> >>
> >> certain perripherals will now need to ensure that,
> >> they request VDD regulators in their drivers, and enable
> >> them so as to make them working.
> >>
> >> Signed-off-by: Vivek Gautam
> >> Cc: Anton Tikhomirov
> >> ---
> >>
> >> Based on 'usb-next' branch of Greg's USB tree.
> >> Also cleanly applies on 'next' branch of Balbi's USB tree.
> >>
> >> drivers/usb/dwc3/dwc3-exynos.c | 51
> >> ++--
> >> 1 file changed, 49 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> >> exynos.c
> >> index 28c8ad7..c9d9102 100644
> >> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >> @@ -27,6 +27,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >>
> >> struct dwc3_exynos {
> >> struct platform_device *usb2_phy;
> >> @@ -34,6 +35,8 @@ struct dwc3_exynos {
> >> struct device *dev;
> >>
> >> struct clk *clk;
> >> + struct regulator*vdd33;
> >> + struct regulator*vdd10;
> >> };
> >>
> >> static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
> >> @@ -144,20 +147,46 @@ static int dwc3_exynos_probe(struct
> >> platform_device *pdev)
> >>
> >> clk_prepare_enable(exynos->clk);
> >>
> >> + exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> >> + if (IS_ERR(exynos->vdd33)) {
> >> + ret = PTR_ERR(exynos->vdd33);
> >> + goto err2;
> >
> > Is regulator property mandatory for dwc3-exynos? If it is not
> > and device tree doesn't provide it, dwc3-exynos driver probe
> shouldn't
> > fail here.
>
> These are the VDD regulators (from PMIC ldo supplies), in absence of
> which the controller will not be powered up.
> So doesn't it make sense to stop the probe when these are not supplied
> by device tree ?
Agree. Just curious, is there special reason for this change except making
things right?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] usb: dwc3-exynos: Make provision for vdd regulators
Hello,
> -Original Message-
> From: Vivek Gautam [mailto:gautamvivek1...@gmail.com] On Behalf Of
> Vivek Gautam
> Sent: Monday, April 21, 2014 9:17 PM
> To: linux-usb@vger.kernel.org; linux-samsung-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; linux-o...@vger.kernel.org; linux-
> arm-ker...@lists.infradead.org; gre...@linuxfoundation.org;
> st...@rowland.harvard.edu; ba...@ti.com; kgene@samsung.com; Vivek
> Gautam; Anton Tikhomirov
> Subject: [PATCH 3/3] usb: dwc3-exynos: Make provision for vdd
> regulators
>
> Facilitate getting required 3.3V and 1.0V VDD supply for
> DWC3 controller on Exynos.
>
> With patches for regulators' nodes merged in 3.15:
> c8c253f ARM: dts: Add regulator entries to smdk5420
> 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
>
> certain perripherals will now need to ensure that,
> they request VDD regulators in their drivers, and enable
> them so as to make them working.
>
> Signed-off-by: Vivek Gautam
> Cc: Anton Tikhomirov
> ---
>
> Based on 'usb-next' branch of Greg's USB tree.
> Also cleanly applies on 'next' branch of Balbi's USB tree.
>
> drivers/usb/dwc3/dwc3-exynos.c | 51
> ++--
> 1 file changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> exynos.c
> index 28c8ad7..c9d9102 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
>
> struct dwc3_exynos {
> struct platform_device *usb2_phy;
> @@ -34,6 +35,8 @@ struct dwc3_exynos {
> struct device *dev;
>
> struct clk *clk;
> + struct regulator*vdd33;
> + struct regulator*vdd10;
> };
>
> static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
> @@ -144,20 +147,46 @@ static int dwc3_exynos_probe(struct
> platform_device *pdev)
>
> clk_prepare_enable(exynos->clk);
>
> + exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> + if (IS_ERR(exynos->vdd33)) {
> + ret = PTR_ERR(exynos->vdd33);
> + goto err2;
Is regulator property mandatory for dwc3-exynos? If it is not
and device tree doesn't provide it, dwc3-exynos driver probe shouldn't
fail here.
> + }
> + ret = regulator_enable(exynos->vdd33);
> + if (ret) {
> + dev_err(dev, "Failed to enable VDD33 supply\n");
> + goto err2;
> + }
> +
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
Hi,
> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
>
> Hi,
>
> On Thursday 06 March 2014 02:49 PM, Anton Tikhomirov wrote:
> > Hi,
> >
> >> Subject: RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
> >>
> >> Hi,
> >>
> >>> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
> >>>
> >>> Hi,
> >>>
> >>> On Thursday 06 March 2014 02:22 PM, Anton Tikhomirov wrote:
> >>>> Hello,
> >>>>
> >>>>> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY
> driver
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Thursday 06 March 2014 01:56 PM, Anton Tikhomirov wrote:
> >>>>>> Hi Kamil,
> >>>>>>
> >>>>>> ...
> >>>>>>
> >>>>>>> +| 3. Supporting SoCs
> >>>>>>> ++
> >>>>>>> +
> >>>>>>> +To support a new SoC a new file should be added to the
> >>> drivers/phy
> >>>>>>> +directory. Each SoC's configuration is stored in an instance
> of
> >>> the
> >>>>>>> +struct samsung_usb2_phy_config.
> >>>>>>> +
> >>>>>>> +struct samsung_usb2_phy_config {
> >>>>>>> + const struct samsung_usb2_common_phy *phys;
> >>>>>>> + unsigned int num_phys;
> >>>>>>> + bool has_mode_switch;
> >>>>>>
> >>>>>> You missed rate_to_clk here.
> >>>>>>
> >>>>>>> +};
> >>>>>>> +
> >>>>>>
> >>>>>> ...
> >>>>>>
> >>>>>>> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-
> >>>>> samsung-
> >>>>>>> usb2.c
> >>>>>>> new file mode 100644
> >>>>>>> index 000..c3b7719
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/drivers/phy/phy-samsung-usb2.c
> >>>>>>> @@ -0,0 +1,222 @@
> >>>>>>> +/*
> >>>>>>> + * Samsung SoC USB 1.1/2.0 PHY driver
> >>>>>>> + *
> >>>>>>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> >>>>>>> + * Author: Kamil Debski
> >>>>>>> + *
> >>>>>>> + * 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
> >>>>>>> +#include
> >>>>>>> +#include
> >>>>>>> +#include
> >>>>>>> +#include
> >>>>>>> +#include
> >>>>>>> +#include
> >>>>>>> +#include
> >>>>>>> +#include "phy-samsung-usb2.h"
> >>>>>>> +
> >>>>>>> +static int samsung_usb2_phy_power_on(struct phy *phy)
> >>>>>>> +{
> >>>>>>> + struct samsung_usb2_phy_instance *inst =
> >>> phy_get_drvdata(phy);
> >>>>>>> + struct samsung_usb2_phy_driver *drv = inst->drv;
> >>>>>>> + int ret;
> >>>>>>> +
> >>>>>>> + dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> >>>>>>> + inst->cfg->label);
> >>>>>>> + ret = clk_prepare_enable(drv->clk);
> >>>>>>
> >>>>>> clk_prepare_enable() can sleep, and therefore doesn't allow
> >>>>>> samusng_usb2_phy_power_on() to be used in atomic context
> >>>>>> (e.g. inside spin_lock-ed area), what sometimes may be desirable.
> >>>>>> What about to prepare clock in probe, and just enable it here
> >>>>>> (note: clk_enable() doesn't sleep).
> >>>>>
> >>>>> The PHY power-on callback is anyway called with mutex held, so I
> >>> guess
> >>>>> it's fine to have clk_prepare_enable() here.
> >>>>
> >>>> If we rely totally on generic PHY functions such as phy_power_on()
> >>>> and friends, why do we need to use locking in callbacks at all.
> >>>
> >>> Didn't get you.. We don't want to invoke power_on when init is
> >> getting
> >>> executed or you don't want power on or power off to get executed
> >>> simultaneously right? So we need to protect it.
> >>
> >> I mean callbacks such as samsung_usb2_phy_power_on() which uses
> >> spin_lock.
> >> It's already protected by mutex in phy_power_on().
> >
> > Well... phy_power_on() uses mutex to protect power_on() callback.
> > power_on() is samsung_usb2_phy_power_on() in our case.
> > samsung_usb2_phy_power_on() uses spinlock.
> > My question is why do we need to use spinlock _inside_ callback
> > if it is already protected by mutex.
>
> It is needed when the same PHY provider implements multiple PHYs.
> phy-core can protect phy-ops of same PHY. However if the PHY provider
> implements multiple PHYs, phy-core won't be able to protect.
Thank you Kishon. Now it's clear.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
Hi,
> Subject: RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
>
> Hi,
>
> > Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
> >
> > Hi,
> >
> > On Thursday 06 March 2014 02:22 PM, Anton Tikhomirov wrote:
> > > Hello,
> > >
> > >> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
> > >>
> > >>
> > >>
> > >> On Thursday 06 March 2014 01:56 PM, Anton Tikhomirov wrote:
> > >>> Hi Kamil,
> > >>>
> > >>> ...
> > >>>
> > >>>> +| 3. Supporting SoCs
> > >>>> ++
> > >>>> +
> > >>>> +To support a new SoC a new file should be added to the
> > drivers/phy
> > >>>> +directory. Each SoC's configuration is stored in an instance of
> > the
> > >>>> +struct samsung_usb2_phy_config.
> > >>>> +
> > >>>> +struct samsung_usb2_phy_config {
> > >>>> + const struct samsung_usb2_common_phy *phys;
> > >>>> + unsigned int num_phys;
> > >>>> + bool has_mode_switch;
> > >>>
> > >>> You missed rate_to_clk here.
> > >>>
> > >>>> +};
> > >>>> +
> > >>>
> > >>> ...
> > >>>
> > >>>> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-
> > >> samsung-
> > >>>> usb2.c
> > >>>> new file mode 100644
> > >>>> index 000..c3b7719
> > >>>> --- /dev/null
> > >>>> +++ b/drivers/phy/phy-samsung-usb2.c
> > >>>> @@ -0,0 +1,222 @@
> > >>>> +/*
> > >>>> + * Samsung SoC USB 1.1/2.0 PHY driver
> > >>>> + *
> > >>>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> > >>>> + * Author: Kamil Debski
> > >>>> + *
> > >>>> + * 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
> > >>>> +#include
> > >>>> +#include
> > >>>> +#include
> > >>>> +#include
> > >>>> +#include
> > >>>> +#include
> > >>>> +#include
> > >>>> +#include "phy-samsung-usb2.h"
> > >>>> +
> > >>>> +static int samsung_usb2_phy_power_on(struct phy *phy)
> > >>>> +{
> > >>>> + struct samsung_usb2_phy_instance *inst =
> > phy_get_drvdata(phy);
> > >>>> + struct samsung_usb2_phy_driver *drv = inst->drv;
> > >>>> + int ret;
> > >>>> +
> > >>>> + dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> > >>>> + inst->cfg->label);
> > >>>> + ret = clk_prepare_enable(drv->clk);
> > >>>
> > >>> clk_prepare_enable() can sleep, and therefore doesn't allow
> > >>> samusng_usb2_phy_power_on() to be used in atomic context
> > >>> (e.g. inside spin_lock-ed area), what sometimes may be desirable.
> > >>> What about to prepare clock in probe, and just enable it here
> > >>> (note: clk_enable() doesn't sleep).
> > >>
> > >> The PHY power-on callback is anyway called with mutex held, so I
> > guess
> > >> it's fine to have clk_prepare_enable() here.
> > >
> > > If we rely totally on generic PHY functions such as phy_power_on()
> > > and friends, why do we need to use locking in callbacks at all.
> >
> > Didn't get you.. We don't want to invoke power_on when init is
> getting
> > executed or you don't want power on or power off to get executed
> > simultaneously right? So we need to protect it.
>
> I mean callbacks such as samsung_usb2_phy_power_on() which uses
> spin_lock.
> It's already protected by mutex in phy_power_on().
Well... phy_power_on() uses mutex to protect power_on() callback.
power_on() is samsung_usb2_phy_power_on() in our case.
samsung_usb2_phy_power_on() uses spinlock.
My question is why do we need to use spinlock _inside_ callback
if it is already protected by mutex.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
Hi,
> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
>
> Hi,
>
> On Thursday 06 March 2014 02:22 PM, Anton Tikhomirov wrote:
> > Hello,
> >
> >> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
> >>
> >>
> >>
> >> On Thursday 06 March 2014 01:56 PM, Anton Tikhomirov wrote:
> >>> Hi Kamil,
> >>>
> >>> ...
> >>>
> >>>> +| 3. Supporting SoCs
> >>>> ++
> >>>> +
> >>>> +To support a new SoC a new file should be added to the
> drivers/phy
> >>>> +directory. Each SoC's configuration is stored in an instance of
> the
> >>>> +struct samsung_usb2_phy_config.
> >>>> +
> >>>> +struct samsung_usb2_phy_config {
> >>>> +const struct samsung_usb2_common_phy *phys;
> >>>> +unsigned int num_phys;
> >>>> +bool has_mode_switch;
> >>>
> >>> You missed rate_to_clk here.
> >>>
> >>>> +};
> >>>> +
> >>>
> >>> ...
> >>>
> >>>> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-
> >> samsung-
> >>>> usb2.c
> >>>> new file mode 100644
> >>>> index 000..c3b7719
> >>>> --- /dev/null
> >>>> +++ b/drivers/phy/phy-samsung-usb2.c
> >>>> @@ -0,0 +1,222 @@
> >>>> +/*
> >>>> + * Samsung SoC USB 1.1/2.0 PHY driver
> >>>> + *
> >>>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> >>>> + * Author: Kamil Debski
> >>>> + *
> >>>> + * 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
> >>>> +#include
> >>>> +#include
> >>>> +#include
> >>>> +#include
> >>>> +#include
> >>>> +#include
> >>>> +#include
> >>>> +#include "phy-samsung-usb2.h"
> >>>> +
> >>>> +static int samsung_usb2_phy_power_on(struct phy *phy)
> >>>> +{
> >>>> +struct samsung_usb2_phy_instance *inst =
> phy_get_drvdata(phy);
> >>>> +struct samsung_usb2_phy_driver *drv = inst->drv;
> >>>> +int ret;
> >>>> +
> >>>> +dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> >>>> +inst->cfg->label);
> >>>> +ret = clk_prepare_enable(drv->clk);
> >>>
> >>> clk_prepare_enable() can sleep, and therefore doesn't allow
> >>> samusng_usb2_phy_power_on() to be used in atomic context
> >>> (e.g. inside spin_lock-ed area), what sometimes may be desirable.
> >>> What about to prepare clock in probe, and just enable it here
> >>> (note: clk_enable() doesn't sleep).
> >>
> >> The PHY power-on callback is anyway called with mutex held, so I
> guess
> >> it's fine to have clk_prepare_enable() here.
> >
> > If we rely totally on generic PHY functions such as phy_power_on()
> > and friends, why do we need to use locking in callbacks at all.
>
> Didn't get you.. We don't want to invoke power_on when init is getting
> executed or you don't want power on or power off to get executed
> simultaneously right? So we need to protect it.
I mean callbacks such as samsung_usb2_phy_power_on() which uses spin_lock.
It's already protected by mutex in phy_power_on().
>
> Cheers
> Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
Hello,
> Subject: Re: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
>
>
>
> On Thursday 06 March 2014 01:56 PM, Anton Tikhomirov wrote:
> > Hi Kamil,
> >
> > ...
> >
> >> +| 3. Supporting SoCs
> >> ++
> >> +
> >> +To support a new SoC a new file should be added to the drivers/phy
> >> +directory. Each SoC's configuration is stored in an instance of the
> >> +struct samsung_usb2_phy_config.
> >> +
> >> +struct samsung_usb2_phy_config {
> >> + const struct samsung_usb2_common_phy *phys;
> >> + unsigned int num_phys;
> >> + bool has_mode_switch;
> >
> > You missed rate_to_clk here.
> >
> >> +};
> >> +
> >
> > ...
> >
> >> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-
> samsung-
> >> usb2.c
> >> new file mode 100644
> >> index 000..c3b7719
> >> --- /dev/null
> >> +++ b/drivers/phy/phy-samsung-usb2.c
> >> @@ -0,0 +1,222 @@
> >> +/*
> >> + * Samsung SoC USB 1.1/2.0 PHY driver
> >> + *
> >> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> >> + * Author: Kamil Debski
> >> + *
> >> + * 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
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include "phy-samsung-usb2.h"
> >> +
> >> +static int samsung_usb2_phy_power_on(struct phy *phy)
> >> +{
> >> + struct samsung_usb2_phy_instance *inst = phy_get_drvdata(phy);
> >> + struct samsung_usb2_phy_driver *drv = inst->drv;
> >> + int ret;
> >> +
> >> + dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> >> + inst->cfg->label);
> >> + ret = clk_prepare_enable(drv->clk);
> >
> > clk_prepare_enable() can sleep, and therefore doesn't allow
> > samusng_usb2_phy_power_on() to be used in atomic context
> > (e.g. inside spin_lock-ed area), what sometimes may be desirable.
> > What about to prepare clock in probe, and just enable it here
> > (note: clk_enable() doesn't sleep).
>
> The PHY power-on callback is anyway called with mutex held, so I guess
> it's fine to have clk_prepare_enable() here.
If we rely totally on generic PHY functions such as phy_power_on()
and friends, why do we need to use locking in callbacks at all.
>
> Thanks
> Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v9 3/4] phy: Add new Exynos USB 2.0 PHY driver
Hi Kamil,
...
> +| 3. Supporting SoCs
> ++
> +
> +To support a new SoC a new file should be added to the drivers/phy
> +directory. Each SoC's configuration is stored in an instance of the
> +struct samsung_usb2_phy_config.
> +
> +struct samsung_usb2_phy_config {
> + const struct samsung_usb2_common_phy *phys;
> + unsigned int num_phys;
> + bool has_mode_switch;
You missed rate_to_clk here.
> +};
> +
...
> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-
> usb2.c
> new file mode 100644
> index 000..c3b7719
> --- /dev/null
> +++ b/drivers/phy/phy-samsung-usb2.c
> @@ -0,0 +1,222 @@
> +/*
> + * Samsung SoC USB 1.1/2.0 PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Kamil Debski
> + *
> + * 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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "phy-samsung-usb2.h"
> +
> +static int samsung_usb2_phy_power_on(struct phy *phy)
> +{
> + struct samsung_usb2_phy_instance *inst = phy_get_drvdata(phy);
> + struct samsung_usb2_phy_driver *drv = inst->drv;
> + int ret;
> +
> + dev_dbg(drv->dev, "Request to power_on \"%s\" usb phy\n",
> + inst->cfg->label);
> + ret = clk_prepare_enable(drv->clk);
clk_prepare_enable() can sleep, and therefore doesn't allow
samusng_usb2_phy_power_on() to be used in atomic context
(e.g. inside spin_lock-ed area), what sometimes may be desirable.
What about to prepare clock in probe, and just enable it here
(note: clk_enable() doesn't sleep).
> + if (ret)
> + goto err_main_clk;
> + ret = clk_prepare_enable(drv->ref_clk);
> + if (ret)
> + goto err_instance_clk;
> + if (inst->cfg->power_on) {
> + spin_lock(&drv->lock);
> + ret = inst->cfg->power_on(inst);
> + spin_unlock(&drv->lock);
> + }
> +
> + return 0;
Thank you
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: phy: Fix double lock in OTG FSM
Mutex obtained at the beginning of the function should
be released at the end to avoid double locking.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsm-usb.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 6223872..65c3a72 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -357,7 +357,7 @@ int otg_statemachine(struct otg_fsm *fsm)
default:
break;
}
- mutex_lock(&fsm->lock);
+ mutex_unlock(&fsm->lock);
VDBG("quit statemachine, changed = %d\n", state_changed);
return state_changed;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi Felipe,
> > +
> > + /* handle the case where we have to send a zero packet */
> > + if ((epnum & 1) && ur->zero &&
> > + (ur->length % ep0->endpoint.maxpacket == 0)) {
> > + int ret;
> > +
> > + ret = dwc3_ep0_start_trans(dwc, epnum, dwc->ctrl_req_addr,
> 0,
> > + DWC3_TRBCTL_CONTROL_DATA);
> > + WARN_ON(ret < 0);
> > + dwc->ep0_zlp_sent = 1;
> > + return;
> > + }
>
> note that this causes a slight bug. Code expects to receive a
> NRDY_STATUS, but we're sending another CONTROL_DATA which means we will
> receive XFER_COMPLETE_DATA.
Even if ep0_next_event is set to DWC3_EP0_NRDY_STATUS at the beginning of
this function, in case of ZLP it will be reset back to DWC3_EP0_COMPLETE in
dwc3_ep0_start_trans(). So there is no mismatch between what code expects
to receive and actual event. Did I miss something?
>
> It's only working because Control(Data) lost its XferNotReady handling
> due to a silicon bug we found. If someone ever patches that handler,
> this will be a hard-to-track problem.
>
Thank you
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi, > On Thu, Dec 19, 2013 at 03:38:18PM +0900, Anton Tikhomirov wrote: > > Hi, > > > > > Hi, > > > > > > On Thu, Dec 19, 2013 at 02:54:31PM +0900, Anton Tikhomirov wrote: > > > > > WARN_ON(ret < 0); > > > > > > > > > > Regards > > > > > Pratyush > > > > > > > > By the way, chaining additional (auxiliary) TRB would allow > complying > > > with > > > > Buffer Size Rule on _UDC_driver_level_ for any type of OUT > endpoints, > > > when > > > > total size of a Buffer Descriptor must be a multiple of > MaxPacketSize: > > > > > > > > rem = request_length % MaxPacketSize > > > > > > > > general TRB size = request_length (CHN = 1) > > > > > > should be request_length - rem > > > > > > > aux TRB size = MaxPacketSize - rem (CHN = 0) > > > > > > should be rem > > > > In this case: > > > > Buffer_Descriptor_size = general_TRB_size + aux_TRB_size = > > (request_lenth - rem) + rem = request_length , > > > > which is not multiple of MaxPacketSize. > > the multiple of packet size thing is only for RX and in this case, he's > sending data (a config descriptor) to host. I mean OUT transfers here. Sorry, I had to create separate mail thread for this discussion. Thank you -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi, > > > > On Thu, Dec 19, 2013 at 11:32:08AM +0900, Anton Tikhomirov wrote: > > > > > Hi, > > > > > > > > > > > Hi, > > > > > > > > > > > > On Tue, Dec 17, 2013 at 03:59:31PM +0900, Anton Tikhomirov > wrote: > > > > > > > In accordance with specification, when sent data length is > > > > > > > > > > > > please mention section of specification. > > > > > > > > > > USB2.0 spec., 8.5.3.2 Variable-length Data Stage > > > > > > > > > > > > > > > > > > an exact multiple of wMaxPacketSize for the pipe and less > > > > > > > than requested by host, the function shall return a zero- > length > > > > > > > packet (ZLP) to indicate the end of the Data stage to a USB > host. > > > > > > > > > > > > hmm... so in USB3 mode that would be host requesting 513 > bytes and us > > > > > > sending only 512. > > > > > > > > > > Our customer reported this issue. In their case, Windows USB2.0 > host > > > > > requests Configuration descriptor with wLength = 255. Device > replies > > > > > with two 64 bytes IN transactions, and stalls EP0 on 3rd IN > transaction, > > > > > where it has to reply with ZLP. Unfortunately, we don’t have > full > > > > > picture of what's happening on their side and why host requests > > > > > more bytes than actual length of Configuration descriptor. > > > > > > > > that's a bug on the host side. > > > > > > Certainly not. The host doesn't know how big the Configuration > descriptor > > > is, so it always asks for 255 bytes (on Windows that is, I'm not > sure > > > about Linux). > > > > As far as I know, first time Windows host requests only 9 bytes of > > Configuration descriptor, which include total size of Configuration > > descriptor and its subordinates. Then, second time it requests > > Configuration descriptor again and uses total length obtained in > first > > step. Am I wrong? > > I think it does that for the Device descriptor, but not the > Configuration > descriptor, for some reason. It could depend on which version of > Windows > you have, too. > > My point was that it is not a bug for the host to ask for more bytes > than > the length of the descriptor, just as it is not a bug to ask for fewer > bytes than the length of the descriptor. The device must handle both > cases. Agree. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi, > > On Thu, Dec 19, 2013 at 11:32:08AM +0900, Anton Tikhomirov wrote: > > > Hi, > > > > > > > Hi, > > > > > > > > On Tue, Dec 17, 2013 at 03:59:31PM +0900, Anton Tikhomirov wrote: > > > > > In accordance with specification, when sent data length is > > > > > > > > please mention section of specification. > > > > > > USB2.0 spec., 8.5.3.2 Variable-length Data Stage > > > > > > > > > > > > an exact multiple of wMaxPacketSize for the pipe and less > > > > > than requested by host, the function shall return a zero-length > > > > > packet (ZLP) to indicate the end of the Data stage to a USB > host. > > > > > > > > hmm... so in USB3 mode that would be host requesting 513 bytes > and us > > > > sending only 512. > > > > > > Our customer reported this issue. In their case, Windows USB2.0 > host > > > requests Configuration descriptor with wLength = 255. Device > replies > > > with two 64 bytes IN transactions, and stalls EP0 on 3rd IN > transaction, > > > where it has to reply with ZLP. Unfortunately, we don’t have full > > > picture of what's happening on their side and why host requests > > > more bytes than actual length of Configuration descriptor. > > > > that's a bug on the host side. > > Certainly not. The host doesn't know how big the Configuration > descriptor > is, so it always asks for 255 bytes (on Windows that is, I'm not sure > about Linux). As far as I know, first time Windows host requests only 9 bytes of Configuration descriptor, which include total size of Configuration descriptor and its subordinates. Then, second time it requests Configuration descriptor again and uses total length obtained in first step. Am I wrong? > > The problem here sounds like the Configuration descriptor for this > particular device is exactly 128 bytes (two packets at HS), so a 0- Exactly! > length > packet is needed to terminate the transfer. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi, > Hi, > > On Thu, Dec 19, 2013 at 02:54:31PM +0900, Anton Tikhomirov wrote: > > > WARN_ON(ret < 0); > > > > > > Regards > > > Pratyush > > > > By the way, chaining additional (auxiliary) TRB would allow complying > with > > Buffer Size Rule on _UDC_driver_level_ for any type of OUT endpoints, > when > > total size of a Buffer Descriptor must be a multiple of MaxPacketSize: > > > > rem = request_length % MaxPacketSize > > > > general TRB size = request_length (CHN = 1) > > should be request_length - rem > > > aux TRB size = MaxPacketSize - rem (CHN = 0) > > should be rem In this case: Buffer_Descriptor_size = general_TRB_size + aux_TRB_size = (request_lenth - rem) + rem = request_length , which is not multiple of MaxPacketSize. The idea is to build Buffer Descriptor, whose total size is multiple of MaxPacketSize, using additional TRB. > > > Buffer for aux TRBs can be allocated at initialization time and used > > when necessary. > > send a patch and test it for weeks ;-) > > -- > Balbi Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi Pratyush, > Hi Anton, > > On Wed, Dec 18, 2013 at 12:11:33PM +0800, Felipe Balbi wrote: > > Hi, > > > > On Tue, Dec 17, 2013 at 03:59:31PM +0900, Anton Tikhomirov wrote: > > > In accordance with specification, when sent data length is > > > > please mention section of specification. > > > > > an exact multiple of wMaxPacketSize for the pipe and less > > > than requested by host, the function shall return a zero-length > > > packet (ZLP) to indicate the end of the Data stage to a USB host. > > > > hmm... so in USB3 mode that would be host requesting 513 bytes and us > > sending only 512. > > Curious, what was the use case when you encountered above situation? Please read my reply to Felipe. > > Wouldn't something simple as follows be able to resolve it? > > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c > index 95f7649..8b419aa 100644 > --- a/drivers/usb/dwc3/ep0.c > +++ b/drivers/usb/dwc3/ep0.c > @@ -924,6 +924,10 @@ static void __dwc3_ep0_do_control_data(struct dwc3 > *dwc, > > ret = dwc3_ep0_start_trans(dwc, dep->number, req- > >request.dma, > req->request.length, DWC3_TRBCTL_CONTROL_DATA); > + if (req->zero && !ret) > + ret = dwc3_ep0_start_trans(dwc, dep->number, > + dwc->ctrl_req_addr, 0, > + DWC3_TRBCTL_CONTROL_DATA); No. Second call to dwc3_ep0_start_trans() modifies EP0 TRB which is already owned by the core. More correct solution here is to use second zero-length TRB for ZLP. > } > > WARN_ON(ret < 0); > > Regards > Pratyush By the way, chaining additional (auxiliary) TRB would allow complying with Buffer Size Rule on _UDC_driver_level_ for any type of OUT endpoints, when total size of a Buffer Descriptor must be a multiple of MaxPacketSize: rem = request_length % MaxPacketSize general TRB size = request_length (CHN = 1) aux TRB size = MaxPacketSize - rem (CHN = 0) Buffer for aux TRBs can be allocated at initialization time and used when necessary. Thank you -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
Hi,
> Hi,
>
> On Tue, Dec 17, 2013 at 03:59:31PM +0900, Anton Tikhomirov wrote:
> > In accordance with specification, when sent data length is
>
> please mention section of specification.
USB2.0 spec., 8.5.3.2 Variable-length Data Stage
>
> > an exact multiple of wMaxPacketSize for the pipe and less
> > than requested by host, the function shall return a zero-length
> > packet (ZLP) to indicate the end of the Data stage to a USB host.
>
> hmm... so in USB3 mode that would be host requesting 513 bytes and us
> sending only 512.
Our customer reported this issue. In their case, Windows USB2.0 host
requests Configuration descriptor with wLength = 255. Device replies
with two 64 bytes IN transactions, and stalls EP0 on 3rd IN transaction,
where it has to reply with ZLP. Unfortunately, we don’t have full
picture of what's happening on their side and why host requests
more bytes than actual length of Configuration descriptor.
>
> > @@ -619,6 +619,7 @@ struct dwc3_scratchpad_array {
> > * @three_stage_setup: set if we perform a three phase setup
> > * @ep0_bounced: true when we used bounce buffer
> > * @ep0_expect_in: true when we expect a DATA IN transfer
> > + * @ep0_zlp_sent: true when ZLP was sent
>
> I would rather have a ep0_needs_zlp flag.
ok
>
> > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> > index 21a3520..cf72561 100644
> > --- a/drivers/usb/dwc3/ep0.c
> > +++ b/drivers/usb/dwc3/ep0.c
> > @@ -782,6 +782,9 @@ static void dwc3_ep0_complete_data(struct dwc3
> *dwc,
> > return;
> > }
> >
> > + if (dwc->ep0_zlp_sent)
> > + goto finish_zlp;
> > +
> > length = trb->size & DWC3_TRB_SIZE_MASK;
> >
> > if (dwc->ep0_bounced) {
> > @@ -802,14 +805,24 @@ static void dwc3_ep0_complete_data(struct dwc3
> *dwc,
> > /* for some reason we did not get everything out */
> >
> > dwc3_ep0_stall_and_restart(dwc);
> > - } else {
> > - /*
> > -* handle the case where we have to send a zero packet.
> This
> > -* seems to be case when req.length > maxpacket. Could it
> be?
> > -*/
> > - if (r)
By the way, why do we need this check? If r is NULL, we will have
panic above in this function, where ur is dereferenced.
> > - dwc3_gadget_giveback(ep0, r, 0);
> > + return;
> > }
> > +
> > + /* handle the case where we have to send a zero packet */
> > + if ((epnum & 1) && ur->zero &&
> > + (ur->length % ep0->endpoint.maxpacket == 0)) {
> > + int ret;
> > +
> > + ret = dwc3_ep0_start_trans(dwc, epnum, dwc->ctrl_req_addr,
> 0,
> > + DWC3_TRBCTL_CONTROL_DATA);
> > + WARN_ON(ret < 0);
> > + dwc->ep0_zlp_sent = 1;
> > + return;
> > + }
>
> note that this causes a slight bug. Code expects to receive a
> NRDY_STATUS, but we're sending another CONTROL_DATA which means we will
> receive XFER_COMPLETE_DATA.
>
> It's only working because Control(Data) lost its XferNotReady handling
> due to a silicon bug we found. If someone ever patches that handler,
> this will be a hard-to-track problem.
My bad, will fix this.
>
> how have you tested this ? Any changes to ep0 handling must come with a
> libusb-based testcase so I can make sure nothing else gets broken (yes,
> new requirement :-)
>
> Also make sure to run testusb for control endpoints and leave it
> running
> for weeks. You should be able to survive 4 weeks of stress test without
> any issues.
>
> Don't forget to run through USB30CV ch9 on USB3 and USB2 modes.
>
> Sorry dude, but I can't accept regressions and this code has been
> exercised pretty well.
Thank you for review. I will rework the patch and test carefully.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 3/9] phy: Add new Exynos USB PHY driver
Hi Kamil, > Hi Anton, > > > From: Anton Tikhomirov [mailto:av.tikhomi...@samsung.com] > > Sent: Tuesday, December 10, 2013 3:43 AM > > > > Hi Kamil, > > > > Same USB2.0 PHY may be used by several HCDs, for example EHCI and > OHCI. > > Consider the situation, when EHCI stops using the PHY and calls > > power_off, then OHCI becomes non-operational. In other words, PHY > > power_on and power_off calls must be balanced. > > > > Shall we handle it in your driver? (usage count?) > > Please look in the drivers/phy/phy-core.c file. Usage count is handled > there - see phy_power_on and phy_power_off functions. I understand that > after both EHCI and OHCI power on the phy, the usage count is 2. So > powering off one of them (EHCI for instance) the usage count is still > 1, so the OHCI should still work properly. Oops, sorry, missed that. > > [snip] > > Best wishes, > -- > Kamil Debski > Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
In accordance with specification, when sent data length is
an exact multiple of wMaxPacketSize for the pipe and less
than requested by host, the function shall return a zero-length
packet (ZLP) to indicate the end of the Data stage to a USB host.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/dwc3/core.h |2 ++
drivers/usb/dwc3/ep0.c| 27 ---
drivers/usb/dwc3/gadget.c |3 +++
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f8af8d4..fefbf2d 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -619,6 +619,7 @@ struct dwc3_scratchpad_array {
* @three_stage_setup: set if we perform a three phase setup
* @ep0_bounced: true when we used bounce buffer
* @ep0_expect_in: true when we expect a DATA IN transfer
+ * @ep0_zlp_sent: true when ZLP was sent
* @start_config_issued: true when StartConfig command has been issued
* @setup_packet_pending: true when there's a Setup Packet in FIFO. Workaround
* @needs_fifo_resize: not all users might want fifo resizing, flag it
@@ -700,6 +701,7 @@ struct dwc3 {
unsignedthree_stage_setup:1;
unsignedep0_bounced:1;
unsignedep0_expect_in:1;
+ unsignedep0_zlp_sent:1;
unsignedstart_config_issued:1;
unsignedsetup_packet_pending:1;
unsigneddelayed_status:1;
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 21a3520..cf72561 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -782,6 +782,9 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
return;
}
+ if (dwc->ep0_zlp_sent)
+ goto finish_zlp;
+
length = trb->size & DWC3_TRB_SIZE_MASK;
if (dwc->ep0_bounced) {
@@ -802,14 +805,24 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
/* for some reason we did not get everything out */
dwc3_ep0_stall_and_restart(dwc);
- } else {
- /*
-* handle the case where we have to send a zero packet. This
-* seems to be case when req.length > maxpacket. Could it be?
-*/
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ return;
}
+
+ /* handle the case where we have to send a zero packet */
+ if ((epnum & 1) && ur->zero &&
+ (ur->length % ep0->endpoint.maxpacket == 0)) {
+ int ret;
+
+ ret = dwc3_ep0_start_trans(dwc, epnum, dwc->ctrl_req_addr, 0,
+ DWC3_TRBCTL_CONTROL_DATA);
+ WARN_ON(ret < 0);
+ dwc->ep0_zlp_sent = 1;
+ return;
+ }
+
+finish_zlp:
+ if (r)
+ dwc3_gadget_giveback(ep0, r, 0);
}
static void dwc3_ep0_complete_status(struct dwc3 *dwc,
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 02e44fc..59ecd1e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -251,6 +251,9 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
dwc3_request *req,
usb_gadget_unmap_request(&dwc->gadget, &req->request,
req->direction);
+ if (dwc->ep0_zlp_sent && dep->number == 0)
+ dwc->ep0_zlp_sent = 0;
+
dev_dbg(dwc->dev, "request %p from %s completed %d/%d ===> %d\n",
req, dep->name, req->request.actual,
req->request.length, status);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] usb: dwc3: exynos: add pm_runtime support
Hi Felipe,
> On Fri, Dec 13, 2013 at 01:56:18PM -0600, Felipe Balbi wrote:
> > On Fri, Dec 13, 2013 at 02:01:32PM +0900, Anton Tikhomirov wrote:
> > > Hi Felipe,
> > >
> > > > -static int dwc3_exynos_suspend(struct device *dev)
> > > > +static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
> > > > {
> > > > - struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > > -
> > > > clk_disable(exynos->clk);
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > +static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
> > > > +{
> > > > + return clk_enable(exynos->clk);
> > > > +}
> > > > +
> > > > +static int dwc3_exynos_suspend(struct device *dev)
> > > > +{
> > > > + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > > +
> > > > + return __dwc3_exynos_suspend(exynos);
> > >
> > > If dwc3-exynos is runtime suspended, the clock will be disabled
> > > second time here (unbalanced clk_enable/clk_disable).
> >
> > I don't get what you mean but there is something that probably needs
> > fixing, I guess below makes it better:
> >
> > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> exynos.c
> > index c93919a..1e5720a 100644
> > --- a/drivers/usb/dwc3/dwc3-exynos.c
> > +++ b/drivers/usb/dwc3/dwc3-exynos.c
> > @@ -218,6 +218,9 @@ static int dwc3_exynos_suspend(struct device *dev)
> > {
> > struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> >
> > + if (pm_runtime_suspended(dev))
> > + return 0;
> > +
> > return __dwc3_exynos_suspend(exynos);
> > }
> >
> >
> > Is that what you meant ?
>
> note, however, that this is *not* a case where we would fall today. See
> that we pm_runtime_get() in probe and only pm_runtime_put() during
> remove. So there would never be a case where we would try system
> suspend
> while device was already runtime suspended.
You are right, while runtime PM is blocked by get_sync() in probe, this
check
doesn't matter.
>
> I have fixed all patches in my testing/next branch anyway, just to make
> sure we're "idiot-proof" when it comes to implementing real runtime pm
> later on :-)
>
> cheers
>
> --
> balbi
Thank you
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] usb: dwc3: exynos: add pm_runtime support
Hi Felipe,
> On Fri, Dec 13, 2013 at 02:01:32PM +0900, Anton Tikhomirov wrote:
> > Hi Felipe,
> >
> > > -static int dwc3_exynos_suspend(struct device *dev)
> > > +static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
> > > {
> > > - struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > -
> > > clk_disable(exynos->clk);
> > >
> > > return 0;
> > > }
> > >
> > > +static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
> > > +{
> > > + return clk_enable(exynos->clk);
> > > +}
> > > +
> > > +static int dwc3_exynos_suspend(struct device *dev)
> > > +{
> > > + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > +
> > > + return __dwc3_exynos_suspend(exynos);
> >
> > If dwc3-exynos is runtime suspended, the clock will be disabled
> > second time here (unbalanced clk_enable/clk_disable).
>
> I don't get what you mean but there is something that probably needs
> fixing, I guess below makes it better:
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> exynos.c
> index c93919a..1e5720a 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -218,6 +218,9 @@ static int dwc3_exynos_suspend(struct device *dev)
> {
> struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> return __dwc3_exynos_suspend(exynos);
> }
>
>
> Is that what you meant ?
Yes, this is exactly what I meant.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] usb: dwc3: exynos: add pm_runtime support
Hi Felipe,
> -static int dwc3_exynos_suspend(struct device *dev)
> +static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
> {
> - struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> -
> clk_disable(exynos->clk);
>
> return 0;
> }
>
> +static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
> +{
> + return clk_enable(exynos->clk);
> +}
> +
> +static int dwc3_exynos_suspend(struct device *dev)
> +{
> + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> +
> + return __dwc3_exynos_suspend(exynos);
If dwc3-exynos is runtime suspended, the clock will be disabled
second time here (unbalanced clk_enable/clk_disable).
> +}
> +
> static int dwc3_exynos_resume(struct device *dev)
> {
> struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> + int ret;
>
> - clk_enable(exynos->clk);
> + ret = __dwc3_exynos_resume(exynos);
> + if (ret)
> + return ret;
>
> /* runtime set active to reflect active state. */
> pm_runtime_disable(dev);
> @@ -207,8 +238,24 @@ static int dwc3_exynos_resume(struct device *dev)
> return 0;
> }
>
> +static int dwc3_exynos_runtime_suspend(struct device *dev)
> +{
> + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> +
> + return __dwc3_exynos_suspend(exynos);
> +}
> +
> +static int dwc3_exynos_runtime_resume(struct device *dev)
> +{
> + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> +
> + return __dwc3_exynos_resume(exynos);
> +}
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 3/9] phy: Add new Exynos USB PHY driver
Hi Kamil,
Same USB2.0 PHY may be used by several HCDs, for example EHCI and OHCI.
Consider the situation, when EHCI stops using the PHY and calls power_off,
then OHCI becomes non-operational. In other words, PHY power_on and
power_off calls must be balanced.
Shall we handle it in your driver? (usage count?)
> Add a new driver for the Exynos USB PHY. The new driver uses the
> generic
> PHY framework. The driver includes support for the Exynos 4x10 and 4x12
> SoC families.
>
> Signed-off-by: Kamil Debski
> Signed-off-by: Kyungmin Park
> ---
> .../devicetree/bindings/phy/samsung-usbphy.txt | 54
> drivers/phy/Kconfig| 20 ++
> drivers/phy/Makefile |3 +
> drivers/phy/phy-exynos4210-usb2.c | 264
> +
> drivers/phy/phy-exynos4212-usb2.c | 312
>
> drivers/phy/phy-samsung-usb2.c | 228
> ++
> drivers/phy/phy-samsung-usb2.h | 72 +
> 7 files changed, 953 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/samsung-
> usbphy.txt
> create mode 100644 drivers/phy/phy-exynos4210-usb2.c
> create mode 100644 drivers/phy/phy-exynos4212-usb2.c
> create mode 100644 drivers/phy/phy-samsung-usb2.c
> create mode 100644 drivers/phy/phy-samsung-usb2.h
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> new file mode 100644
> index 000..cadbf70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt
> @@ -0,0 +1,54 @@
> +Samsung S5P/EXYNOS SoC series USB PHY
> +-
> +
> +Required properties:
> +- compatible : should be one of the listed compatibles:
> + - "samsung,exynos4210-usb2-phy"
> + - "samsung,exynos4212-usb2-phy"
> +- reg : a list of registers used by phy driver
> + - first and obligatory is the location of phy modules registers
> +- samsung,sysreg-phandle - handle to syscon used to control the system
> registers
> +- samsung,pmureg-phandle - handle to syscon used to control PMU
> registers
> +- #phy-cells : from the generic phy bindings, must be 1;
> +- clocks and clock-names:
> + - the "phy" clocks is required by the phy module
> + - next for each of the phys a clock has to be assidned, this
> clock
> + will be used to determine clocking frequency for the phys
> + (the labels are specified in the paragraph below)
> +
> +The first phandle argument in the PHY specifier identifies the PHY,
> its
> +meaning is compatible dependent. For the currently supported SoCs
> (Exynos 4210
> +and Exynos 4212) it is as follows:
> + 0 - USB device ("device"),
> + 1 - USB host ("host"),
> + 2 - HSIC0 ("hsic0"),
> + 3 - HSIC1 ("hsic1"),
> +
> +Exynos 4210 and Exynos 4212 use mode switching and require that mode
> switch
> +register is supplied.
> +
> +Example:
> +
> +For Exynos 4412 (compatible with Exynos 4212):
> +
> +usbphy: phy@125B {
> + compatible = "samsung,exynos4212-usb2-phy";
> + reg = <0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4>;
> + clocks = <&clock 305>, <&clock 2>, <&clock 2>, <&clock 2>,
> + <&clock 2>;
> + clock-names = "phy", "device", "host", "hsic0", "hsic1";
> + status = "okay";
> + #phy-cells = <1>;
> + samsung,sysreg-phandle = <&sys_reg>;
> + samsung,pmureg-phandle = <&pmu_reg>;
> +};
> +
> +Then the PHY can be used in other nodes such as:
> +
> +phy-consumer@1234 {
> + phys = <&usbphy 2>;
> + phy-names = "phy";
> +};
> +
> +Refer to DT bindings documentation of particular PHY consumer devices
> for more
> +information about required PHYs and the way of specification.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d..b29018f 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -51,4 +51,24 @@ config PHY_EXYNOS_DP_VIDEO
> help
> Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +config PHY_SAMSUNG_USB2
> + tristate "Samsung USB 2.0 PHY driver"
> + help
> + Enable this to support Samsung USB phy helper driver for
> Samsung SoCs.
> + This driver provides common interface to interact, for Samsung
> + USB 2.0 PHY driver.
> +
> +config PHY_EXYNOS4210_USB2
> + bool "Support for Exynos 4210"
> + depends on PHY_SAMSUNG_USB2
> + depends on CPU_EXYNOS4210
> + help
> + Enable USB PHY support for Exynos 4210
> +
> +config PHY_EXYNOS4212_USB2
> + bool "Support for Exynos 4212"
> + depends on PHY_SAMSUNG_USB2
> + depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
> + help
> + Enable USB PHY support for Exynos 4212
> endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..9f4befd 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Ma
RE: [REWORKED PATCH 1/5] usb: phy: fsm: don't depend on indirect includes
Hi, > On Wed, Nov 27, 2013 at 11:14:56AM +0900, Anton Tikhomirov wrote: > > Hi Felipe, > > > > > Hi, > > > > > > On Tue, Nov 26, 2013 at 10:33:44AM -0600, Felipe Balbi wrote: > > > > this header uses spinlocks and errno values, so > > > > we must include and > > > > to avoid build errors. > > > > > > > > Signed-off-by: Felipe Balbi > > > > > > Anton, I had to rework your patch series quite a bit in order to > make > > > > Thank you. > > > > > it > > > acceptable. It seems unlikely that you compile-tested your series, > > > considering we would very clearly see build errors after moving the > > > header around. > > > > > > Your driver depends (or depended?) too much on indirect header > > > inclusions. > > > > Currently only Freescale OTG Transceiver Driver uses OTG FSM. After > applying > > patches it was compiled without errors. > > usually allmodconfig, allyesconfig, allnoconfig with different > architectures uncovers a bunch of errors. Thank you Felipe. I'll be more careful next time. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [REWORKED PATCH 1/5] usb: phy: fsm: don't depend on indirect includes
Hi Felipe, > Hi, > > On Tue, Nov 26, 2013 at 10:33:44AM -0600, Felipe Balbi wrote: > > this header uses spinlocks and errno values, so > > we must include and > > to avoid build errors. > > > > Signed-off-by: Felipe Balbi > > Anton, I had to rework your patch series quite a bit in order to make Thank you. > it > acceptable. It seems unlikely that you compile-tested your series, > considering we would very clearly see build errors after moving the > header around. > > Your driver depends (or depended?) too much on indirect header > inclusions. Currently only Freescale OTG Transceiver Driver uses OTG FSM. After applying patches it was compiled without errors. > Make sure to build test each and every patch in your series next time, Usually I do. > or they won't make it to mainline. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] USB: phy: Add OTG FSM configuration option
This patch removes dependency on Freescale USB UTG Transceiver driver and makes OTG FSM implementation selectable. Signed-off-by: Anton Tikhomirov --- drivers/usb/phy/Kconfig | 10 +- drivers/usb/phy/Makefile |4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 08e2f39..921a9f4 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -6,6 +6,14 @@ menu "USB Physical Layer drivers" config USB_PHY def_bool n +config USB_OTG_FSM + bool "USB 2.0 OTG FSM implementation" + select USB_OTG + select USB_PHY + help + Implements OTG Final State Machine as specified in On-The-Go + and Embedded Host Supplement to the USB Revision 2.0 Specification. + # # USB Transceiver Drivers # @@ -20,7 +28,7 @@ config AB8500_USB config FSL_USB2_OTG bool "Freescale USB OTG Transceiver Driver" - depends on USB_EHCI_FSL && USB_FSL_USB2 && PM_RUNTIME + depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM_RUNTIME select USB_OTG select USB_PHY help diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 022c1da..1bf6c08 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -3,12 +3,12 @@ # obj-$(CONFIG_USB_PHY) += phy.o obj-$(CONFIG_OF) += of.o +obj-$(CONFIG_USB_OTG_FSM) += phy-fsm-usb.o # transceiver drivers, keep the list sorted obj-$(CONFIG_AB8500_USB) += phy-ab8500-usb.o -phy-fsl-usb2-objs := phy-fsl-usb.o phy-fsm-usb.o -obj-$(CONFIG_FSL_USB2_OTG) += phy-fsl-usb2.o +obj-$(CONFIG_FSL_USB2_OTG) += phy-fsl-usb.o obj-$(CONFIG_ISP1301_OMAP) += phy-isp1301-omap.o obj-$(CONFIG_MV_U3D_PHY) += phy-mv-u3d-usb.o obj-$(CONFIG_NOP_USB_XCEIV)+= phy-generic.o -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] USB: phy: move OTG FSM header
Other USB drivers may want to use OTG final state machine
implementation, so make this header available for them.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.h |1 -
drivers/usb/phy/phy-fsm-usb.c |2 --
include/linux/usb/otg.h|1 +
.../phy-fsm-usb.h => include/linux/usb/otg_fsm.h |7 +++
4 files changed, 8 insertions(+), 3 deletions(-)
rename drivers/usb/phy/phy-fsm-usb.h => include/linux/usb/otg_fsm.h (97%)
diff --git a/drivers/usb/phy/phy-fsl-usb.h b/drivers/usb/phy/phy-fsl-usb.h
index 7365170..b8589c5 100644
--- a/drivers/usb/phy/phy-fsl-usb.h
+++ b/drivers/usb/phy/phy-fsl-usb.h
@@ -15,7 +15,6 @@
* 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-#include "phy-fsm-usb.h"
#include
#include
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 2817b04..e72dbee 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -29,8 +29,6 @@
#include
#include
-#include "phy-fsm-usb.h"
-
/* Change USB protocol when there is a protocol change */
static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
{
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index 154332b..ba02706 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -9,6 +9,7 @@
#ifndef __LINUX_USB_OTG_H
#define __LINUX_USB_OTG_H
+#include
#include
struct usb_otg {
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/include/linux/usb/otg_fsm.h
similarity index 97%
rename from drivers/usb/phy/phy-fsm-usb.h
rename to include/linux/usb/otg_fsm.h
index 12ece58..354e57d 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/include/linux/usb/otg_fsm.h
@@ -15,6 +15,9 @@
* 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#ifndef __LINUX_USB_OTG_FSM_H
+#define __LINUX_USB_OTG_FSM_H
+
#undef VERBOSE
#ifdef VERBOSE
@@ -51,6 +54,8 @@ enum otg_fsm_timer {
NUM_OTG_FSM_TIMERS,
};
+struct usb_otg;
+
/* OTG state machine according to the OTG spec */
struct otg_fsm {
/* Input */
@@ -234,3 +239,5 @@ static inline int otg_start_gadget(struct otg_fsm *fsm, int
on)
}
int otg_statemachine(struct otg_fsm *fsm);
+
+#endif /* __LINUX_USB_OTG_FSM_H */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] USB: phy: replace spinlock with mutex in OTG FSM
OTG Final State Machine calls functions which may sleep.
For example, start_gadget callback implementation can use
usb_gadget_vbus_connect(), whose context: can sleep.
If so, mutex should be used instead of spinlock.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.c |7 +++
drivers/usb/phy/phy-fsm-usb.c |7 +++
drivers/usb/phy/phy-fsm-usb.h |2 +-
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 7f3c73b..62d5af2 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -848,7 +848,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
pr_info("Couldn't init OTG timers\n");
goto err;
}
- spin_lock_init(&fsl_otg_tc->fsm.lock);
+ mutex_init(&fsl_otg_tc->fsm.lock);
/* Set OTG state machine operations */
fsl_otg_tc->fsm.ops = &fsl_otg_ops;
@@ -1017,10 +1017,9 @@ static int show_fsl_usb2_otg_state(struct device *dev,
struct otg_fsm *fsm = &fsl_otg_dev->fsm;
char *next = buf;
unsigned size = PAGE_SIZE;
- unsigned long flags;
int t;
- spin_lock_irqsave(&fsm->lock, flags);
+ mutex_lock(&fsm->lock);
/* basic driver infomation */
t = scnprintf(next, size,
@@ -1088,7 +1087,7 @@ static int show_fsl_usb2_otg_state(struct device *dev,
size -= t;
next += t;
- spin_unlock_irqrestore(&fsm->lock, flags);
+ mutex_unlock(&fsm->lock);
return PAGE_SIZE - size;
}
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 329c2d2..2817b04 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -23,7 +23,7 @@
#include
#include
-#include
+#include
#include
#include
#include
@@ -245,9 +245,8 @@ int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state
new_state)
int otg_statemachine(struct otg_fsm *fsm)
{
enum usb_otg_state state;
- unsigned long flags;
- spin_lock_irqsave(&fsm->lock, flags);
+ mutex_lock(&fsm->lock);
state = fsm->otg->phy->state;
state_changed = 0;
@@ -359,7 +358,7 @@ int otg_statemachine(struct otg_fsm *fsm)
default:
break;
}
- spin_unlock_irqrestore(&fsm->lock, flags);
+ mutex_lock(&fsm->lock);
VDBG("quit statemachine, changed = %d\n", state_changed);
return state_changed;
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 7441b46..12ece58 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -110,7 +110,7 @@ struct otg_fsm {
/* Current usb protocol used: 0:undefine; 1:host; 2:client */
int protocol;
- spinlock_t lock;
+ struct mutex lock;
};
struct otg_fsm_ops {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: phy: add include guard to OTG FSM header
This patchs adds include guard to avoid double inclusion of the header. Signed-off-by: Anton Tikhomirov --- include/linux/usb/otg_fsm.h |5 + 1 file changed, 5 insertions(+) diff --git a/include/linux/usb/otg_fsm.h b/include/linux/usb/otg_fsm.h index 12ece58..55de52e 100644 --- a/include/linux/usb/otg_fsm.h +++ b/include/linux/usb/otg_fsm.h @@ -15,6 +15,9 @@ * 675 Mass Ave, Cambridge, MA 02139, USA. */ +#ifndef __LINUX_USB_OTG_FSM_H +#define __LINUX_USB_OTG_FSM_H + #undef VERBOSE #ifdef VERBOSE @@ -234,3 +237,5 @@ static inline int otg_start_gadget(struct otg_fsm *fsm, int on) } int otg_statemachine(struct otg_fsm *fsm); + +#endif /* __LINUX_USB_OTG_FSM_H */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] USB: phy: replace spinlock with mutex in OTG FSM
OTG Final State Machine calls functions which may sleep.
For example, start_gadget callback implementation can use
usb_gadget_vbus_connect(), whose context: can sleep.
If so, mutex should be used instead of spinlock.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.c |7 +++
drivers/usb/phy/phy-fsm-usb.c |7 +++
drivers/usb/phy/phy-fsm-usb.h |2 +-
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 7f3c73b..62d5af2 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -848,7 +848,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
pr_info("Couldn't init OTG timers\n");
goto err;
}
- spin_lock_init(&fsl_otg_tc->fsm.lock);
+ mutex_init(&fsl_otg_tc->fsm.lock);
/* Set OTG state machine operations */
fsl_otg_tc->fsm.ops = &fsl_otg_ops;
@@ -1017,10 +1017,9 @@ static int show_fsl_usb2_otg_state(struct device *dev,
struct otg_fsm *fsm = &fsl_otg_dev->fsm;
char *next = buf;
unsigned size = PAGE_SIZE;
- unsigned long flags;
int t;
- spin_lock_irqsave(&fsm->lock, flags);
+ mutex_lock(&fsm->lock);
/* basic driver infomation */
t = scnprintf(next, size,
@@ -1088,7 +1087,7 @@ static int show_fsl_usb2_otg_state(struct device *dev,
size -= t;
next += t;
- spin_unlock_irqrestore(&fsm->lock, flags);
+ mutex_unlock(&fsm->lock);
return PAGE_SIZE - size;
}
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 329c2d2..2817b04 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -23,7 +23,7 @@
#include
#include
-#include
+#include
#include
#include
#include
@@ -245,9 +245,8 @@ int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state
new_state)
int otg_statemachine(struct otg_fsm *fsm)
{
enum usb_otg_state state;
- unsigned long flags;
- spin_lock_irqsave(&fsm->lock, flags);
+ mutex_lock(&fsm->lock);
state = fsm->otg->phy->state;
state_changed = 0;
@@ -359,7 +358,7 @@ int otg_statemachine(struct otg_fsm *fsm)
default:
break;
}
- spin_unlock_irqrestore(&fsm->lock, flags);
+ mutex_lock(&fsm->lock);
VDBG("quit statemachine, changed = %d\n", state_changed);
return state_changed;
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 7441b46..12ece58 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -110,7 +110,7 @@ struct otg_fsm {
/* Current usb protocol used: 0:undefine; 1:host; 2:client */
int protocol;
- spinlock_t lock;
+ struct mutex lock;
};
struct otg_fsm_ops {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] USB: phy: move OTG FSM header
Other USB drivers may want to use OTG final state machine
implementation, so make this header available for them.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.h |1 -
drivers/usb/phy/phy-fsm-usb.c |2 -
drivers/usb/phy/phy-fsm-usb.h | 236 -
include/linux/usb/otg.h |1 +
include/linux/usb/otg_fsm.h | 236 +
5 files changed, 237 insertions(+), 239 deletions(-)
delete mode 100644 drivers/usb/phy/phy-fsm-usb.h
create mode 100644 include/linux/usb/otg_fsm.h
diff --git a/drivers/usb/phy/phy-fsl-usb.h b/drivers/usb/phy/phy-fsl-usb.h
index 7365170..b8589c5 100644
--- a/drivers/usb/phy/phy-fsl-usb.h
+++ b/drivers/usb/phy/phy-fsl-usb.h
@@ -15,7 +15,6 @@
* 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-#include "phy-fsm-usb.h"
#include
#include
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 2817b04..e72dbee 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -29,8 +29,6 @@
#include
#include
-#include "phy-fsm-usb.h"
-
/* Change USB protocol when there is a protocol change */
static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
{
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
deleted file mode 100644
index 12ece58..000
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ /dev/null
@@ -1,236 +0,0 @@
-/* Copyright (C) 2007,2008 Freescale Semiconductor, Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s] " fmt , \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
-#define PROTO_UNDEF(0)
-#define PROTO_HOST (1)
-#define PROTO_GADGET (2)
-
-enum otg_fsm_timer {
- /* Standard OTG timers */
- A_WAIT_VRISE,
- A_WAIT_VFALL,
- A_WAIT_BCON,
- A_AIDL_BDIS,
- B_ASE0_BRST,
- A_BIDL_ADIS,
-
- /* Auxiliary timers */
- B_SE0_SRP,
- B_SRP_FAIL,
- A_WAIT_ENUM,
-
- NUM_OTG_FSM_TIMERS,
-};
-
-/* OTG state machine according to the OTG spec */
-struct otg_fsm {
- /* Input */
- int id;
- int adp_change;
- int power_up;
- int test_device;
- int a_bus_drop;
- int a_bus_req;
- int a_srp_det;
- int a_vbus_vld;
- int b_conn;
- int a_bus_resume;
- int a_bus_suspend;
- int a_conn;
- int b_bus_req;
- int b_se0_srp;
- int b_ssend_srp;
- int b_sess_vld;
- /* Auxilary inputs */
- int a_sess_vld;
- int b_bus_resume;
- int b_bus_suspend;
-
- /* Output */
- int data_pulse;
- int drv_vbus;
- int loc_conn;
- int loc_sof;
- int adp_prb;
- int adp_sns;
-
- /* Internal variables */
- int a_set_b_hnp_en;
- int b_srp_done;
- int b_hnp_enable;
- int a_clr_err;
-
- /* Informative variables */
- int a_bus_drop_inf;
- int a_bus_req_inf;
- int a_clr_err_inf;
- int b_bus_req_inf;
- /* Auxilary informative variables */
- int a_suspend_req_inf;
-
- /* Timeout indicator for timers */
- int a_wait_vrise_tmout;
- int a_wait_vfall_tmout;
- int a_wait_bcon_tmout;
- int a_aidl_bdis_tmout;
- int b_ase0_brst_tmout;
- int a_bidl_adis_tmout;
-
- struct otg_fsm_ops *ops;
- struct usb_otg *otg;
-
- /* Current usb protocol used: 0:undefine; 1:host; 2:client */
- int protocol;
- struct mutex lock;
-};
-
-struct otg_fsm_ops {
- void(*chrg_vbus)(struct otg_fsm *fsm, int on);
- void(*drv_vbus)(struct otg_fsm *fsm, int on);
- void(*loc_conn)(struct otg_fsm *fsm, int on);
- void(*loc_sof)(struct otg_fsm *fsm, int on);
- void(*start_pulse)(struct otg_fsm *fsm);
- void(*start_adp_prb)(struct otg_fsm *fsm);
- void(*start_adp_sns)(struct otg_fsm *fsm);
- void(*add_timer)(struct otg_fsm *fsm, enum otg_fsm_t
[PATCH 3/3] USB: phy: Add OTG FSM configuration option
This patch removes dependency on Freescale USB UTG Transceiver driver and makes OTG FSM implementation selectable. Signed-off-by: Anton Tikhomirov --- drivers/usb/phy/Kconfig | 10 +- drivers/usb/phy/Makefile |4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index d5589f9..9d5eaba 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -6,6 +6,14 @@ menu "USB Physical Layer drivers" config USB_PHY def_bool n +config USB_OTG_FSM + bool "USB 2.0 OTG FSM implementation" + select USB_OTG + select USB_PHY + help + Implements OTG Final State Machine as specified in On-The-Go + and Embedded Host Supplement to the USB Revision 2.0 Specification. + # # USB Transceiver Drivers # @@ -20,7 +28,7 @@ config AB8500_USB config FSL_USB2_OTG bool "Freescale USB OTG Transceiver Driver" - depends on USB_EHCI_FSL && USB_FSL_USB2 && PM_RUNTIME + depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM_RUNTIME select USB_OTG select USB_PHY help diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 2135e85..df274a1 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -3,12 +3,12 @@ # obj-$(CONFIG_USB_PHY) += phy.o obj-$(CONFIG_OF) += of.o +obj-$(CONFIG_USB_OTG_FSM) += phy-fsm-usb.o # transceiver drivers, keep the list sorted obj-$(CONFIG_AB8500_USB) += phy-ab8500-usb.o -phy-fsl-usb2-objs := phy-fsl-usb.o phy-fsm-usb.o -obj-$(CONFIG_FSL_USB2_OTG) += phy-fsl-usb2.o +obj-$(CONFIG_FSL_USB2_OTG) += phy-fsl-usb.o obj-$(CONFIG_ISP1301_OMAP) += phy-isp1301-omap.o obj-$(CONFIG_MV_U3D_PHY) += phy-mv-u3d-usb.o obj-$(CONFIG_NOP_USB_XCEIV)+= phy-generic.o -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: phy: samsung: Support multiple PHYs of same type
This patch removes limitation when only one PHY of specific type could be used. Signed-off-by: Anton Tikhomirov --- drivers/usb/phy/phy-samsung-usb2.c |3 ++- drivers/usb/phy/phy-samsung-usb3.c |3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/phy/phy-samsung-usb2.c b/drivers/usb/phy/phy-samsung-usb2.c index ff70e4b..b3ba866 100644 --- a/drivers/usb/phy/phy-samsung-usb2.c +++ b/drivers/usb/phy/phy-samsung-usb2.c @@ -411,6 +411,7 @@ static int samsung_usb2phy_probe(struct platform_device *pdev) sphy->drv_data = drv_data; sphy->phy.dev = sphy->dev; sphy->phy.label = "samsung-usb2phy"; + sphy->phy.type = USB_PHY_TYPE_USB2; sphy->phy.init = samsung_usb2phy_init; sphy->phy.shutdown = samsung_usb2phy_shutdown; @@ -426,7 +427,7 @@ static int samsung_usb2phy_probe(struct platform_device *pdev) platform_set_drvdata(pdev, sphy); - return usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2); + return usb_add_phy_dev(&sphy->phy); } static int samsung_usb2phy_remove(struct platform_device *pdev) diff --git a/drivers/usb/phy/phy-samsung-usb3.c b/drivers/usb/phy/phy-samsung-usb3.c index c6eb222..cc08192 100644 --- a/drivers/usb/phy/phy-samsung-usb3.c +++ b/drivers/usb/phy/phy-samsung-usb3.c @@ -271,6 +271,7 @@ static int samsung_usb3phy_probe(struct platform_device *pdev) sphy->clk = clk; sphy->phy.dev = sphy->dev; sphy->phy.label = "samsung-usb3phy"; + sphy->phy.type = USB_PHY_TYPE_USB3; sphy->phy.init = samsung_usb3phy_init; sphy->phy.shutdown = samsung_usb3phy_shutdown; sphy->drv_data = samsung_usbphy_get_driver_data(pdev); @@ -283,7 +284,7 @@ static int samsung_usb3phy_probe(struct platform_device *pdev) platform_set_drvdata(pdev, sphy); - return usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB3); + return usb_add_phy_dev(&sphy->phy); } static int samsung_usb3phy_remove(struct platform_device *pdev) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9] USB: phy: OTG FSM fixes
Hello, This patch set implements several fixes to USB2.0 OTG Final State Machine. All changes affect the only user of OTG FSM, Freescale USB OTG Transceiver Driver, as little as possible and (hope) keep it functional. Further changes to OTG FSM will require reworking of the driver. Anton Tikhomirov (9): USB: phy: Pass OTG FSM pointer to callback functions USB: phy: Check OTG FSM callback existance in helper functions USB: phy: Add and use missed helper functions USB: phy: Fix OTG FSM timer handling USB: phy: Add and use missed OTG FSM timers USB: phy: Rename OTG FSM informative variables USB: phy: Rename "B-device session end SRP" OTG FSM input USB: phy: Add and use missed OTG FSM input/outputs USB: phy: Reordering of OTG FSM variables drivers/usb/phy/phy-fsl-usb.c | 96 +- drivers/usb/phy/phy-fsl-usb.h |4 +- drivers/usb/phy/phy-fsm-usb.c | 72 ++--- drivers/usb/phy/phy-fsm-usb.h | 179 ++--- 4 files changed, 257 insertions(+), 94 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] USB: phy: Add and use missed helper functions
This patch implements missed helper functions for start_gadget() and
start_host() OTG FSM callbacks.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsm-usb.c |8
drivers/usb/phy/phy-fsm-usb.h | 14 ++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 7f45966..7898459 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -41,17 +41,17 @@ static int otg_set_protocol(struct otg_fsm *fsm, int
protocol)
fsm->protocol, protocol);
/* stop old protocol */
if (fsm->protocol == PROTO_HOST)
- ret = fsm->ops->start_host(fsm, 0);
+ ret = otg_start_host(fsm, 0);
else if (fsm->protocol == PROTO_GADGET)
- ret = fsm->ops->start_gadget(fsm, 0);
+ ret = otg_start_gadget(fsm, 0);
if (ret)
return ret;
/* start new protocol */
if (protocol == PROTO_HOST)
- ret = fsm->ops->start_host(fsm, 1);
+ ret = otg_start_host(fsm, 1);
else if (protocol == PROTO_GADGET)
- ret = fsm->ops->start_gadget(fsm, 1);
+ ret = otg_start_gadget(fsm, 1);
if (ret)
return ret;
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 0f3f7d8..157f106 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -160,6 +160,20 @@ static inline int otg_del_timer(struct otg_fsm *fsm, void
*timer)
return 0;
}
+static inline int otg_start_host(struct otg_fsm *fsm, int on)
+{
+ if (!fsm->ops->start_host)
+ return -EOPNOTSUPP;
+ return fsm->ops->start_host(fsm, on);
+}
+
+static inline int otg_start_gadget(struct otg_fsm *fsm, int on)
+{
+ if (!fsm->ops->start_gadget)
+ return -EOPNOTSUPP;
+ return fsm->ops->start_gadget(fsm, on);
+}
+
int otg_statemachine(struct otg_fsm *fsm);
/* Defined by device specific driver, for different timer implementation */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] USB: phy: Rename OTG FSM informative variables
Mark informative variables with suffix '_inf' to distinguish
them from other non-informative variables with the same name.
If such non-informative varialbes were missed, they are created.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.c |2 +-
drivers/usb/phy/phy-fsm-usb.c |6 +++---
drivers/usb/phy/phy-fsm-usb.h | 14 +-
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 6495861..d13ccd5 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -1113,7 +1113,7 @@ static long fsl_otg_ioctl(struct file *file, unsigned int
cmd,
break;
case SET_A_SUSPEND_REQ:
- fsl_otg_dev->fsm.a_suspend_req = arg;
+ fsl_otg_dev->fsm.a_suspend_req_inf = arg;
break;
case SET_A_BUS_DROP:
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 5e899ed..cb0367c 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -99,7 +99,7 @@ void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
old_state)
case OTG_STATE_A_SUSPEND:
otg_del_timer(fsm, A_AIDL_BDIS);
fsm->a_aidl_bdis_tmout = 0;
- fsm->a_suspend_req = 0;
+ fsm->a_suspend_req_inf = 0;
break;
case OTG_STATE_A_PERIPHERAL:
otg_del_timer(fsm, A_BIDL_ADIS);
@@ -192,7 +192,7 @@ int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state
new_state)
* When HNP is triggered while a_bus_req = 0, a_host will
* suspend too fast to complete a_set_b_hnp_en
*/
- if (!fsm->a_bus_req || fsm->a_suspend_req)
+ if (!fsm->a_bus_req || fsm->a_suspend_req_inf)
otg_add_timer(fsm, A_WAIT_ENUM);
break;
case OTG_STATE_A_SUSPEND:
@@ -307,7 +307,7 @@ int otg_statemachine(struct otg_fsm *fsm)
otg_set_state(fsm, OTG_STATE_A_WAIT_VFALL);
break;
case OTG_STATE_A_HOST:
- if ((!fsm->a_bus_req || fsm->a_suspend_req) &&
+ if ((!fsm->a_bus_req || fsm->a_suspend_req_inf) &&
fsm->otg->host->b_hnp_enable)
otg_set_state(fsm, OTG_STATE_A_SUSPEND);
else if (fsm->id || !fsm->b_conn || fsm->a_bus_drop)
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index a74e14a..4049e5c 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -54,9 +54,12 @@ enum otg_fsm_timer {
/* OTG state machine according to the OTG spec */
struct otg_fsm {
/* Input */
+ int a_bus_drop;
+ int a_bus_req;
int a_bus_resume;
int a_bus_suspend;
int a_conn;
+ int b_bus_req;
int a_sess_vld;
int a_srp_det;
int a_vbus_vld;
@@ -72,6 +75,7 @@ struct otg_fsm {
int a_set_b_hnp_en;
int b_srp_done;
int b_hnp_enable;
+ int a_clr_err;
/* Timeout indicator for timers */
int a_wait_vrise_tmout;
@@ -82,11 +86,11 @@ struct otg_fsm {
int a_bidl_adis_tmout;
/* Informative variables */
- int a_bus_drop;
- int a_bus_req;
- int a_clr_err;
- int a_suspend_req;
- int b_bus_req;
+ int a_bus_drop_inf;
+ int a_bus_req_inf;
+ int a_clr_err_inf;
+ int a_suspend_req_inf;
+ int b_bus_req_inf;
/* Output */
int drv_vbus;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] USB: phy: Add and use missed OTG FSM timers
a_bidl_adis_tmr and a_wait_vfall_tmr OTG timers missed in
current FSM implementation. This patch adds and makes use
of the timers as speicfied in OTG and EH supplement to USB2.0.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsm-usb.c | 12 +---
drivers/usb/phy/phy-fsm-usb.h |8
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index f8fe7ec..5e899ed 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -102,8 +102,12 @@ void otg_leave_state(struct otg_fsm *fsm, enum
usb_otg_state old_state)
fsm->a_suspend_req = 0;
break;
case OTG_STATE_A_PERIPHERAL:
+ otg_del_timer(fsm, A_BIDL_ADIS);
+ fsm->a_bidl_adis_tmout = 0;
break;
case OTG_STATE_A_WAIT_VFALL:
+ otg_del_timer(fsm, A_WAIT_VFALL);
+ fsm->a_wait_vfall_tmout = 0;
otg_del_timer(fsm, A_WAIT_VRISE);
break;
case OTG_STATE_A_VBUS_ERR:
@@ -204,12 +208,14 @@ int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state
new_state)
otg_loc_sof(fsm, 0);
otg_set_protocol(fsm, PROTO_GADGET);
otg_drv_vbus(fsm, 1);
+ otg_add_timer(fsm, A_BIDL_ADIS);
break;
case OTG_STATE_A_WAIT_VFALL:
otg_drv_vbus(fsm, 0);
otg_loc_conn(fsm, 0);
otg_loc_sof(fsm, 0);
otg_set_protocol(fsm, PROTO_HOST);
+ otg_add_timer(fsm, A_WAIT_VFALL);
break;
case OTG_STATE_A_VBUS_ERR:
otg_drv_vbus(fsm, 0);
@@ -324,14 +330,14 @@ int otg_statemachine(struct otg_fsm *fsm)
case OTG_STATE_A_PERIPHERAL:
if (fsm->id || fsm->a_bus_drop)
otg_set_state(fsm, OTG_STATE_A_WAIT_VFALL);
- else if (fsm->b_bus_suspend)
+ else if (fsm->a_bidl_adis_tmout || fsm->b_bus_suspend)
otg_set_state(fsm, OTG_STATE_A_WAIT_BCON);
else if (!fsm->a_vbus_vld)
otg_set_state(fsm, OTG_STATE_A_VBUS_ERR);
break;
case OTG_STATE_A_WAIT_VFALL:
- if (fsm->id || fsm->a_bus_req || (!fsm->a_sess_vld &&
- !fsm->b_conn))
+ if (fsm->a_wait_vfall_tmout || fsm->id || fsm->a_bus_req ||
+ (!fsm->a_sess_vld && !fsm->b_conn))
otg_set_state(fsm, OTG_STATE_A_IDLE);
break;
case OTG_STATE_A_VBUS_ERR:
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index b47b32c..a74e14a 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -35,13 +35,19 @@
#define PROTO_GADGET (2)
enum otg_fsm_timer {
+ /* Standard OTG timers */
A_WAIT_VRISE,
+ A_WAIT_VFALL,
A_WAIT_BCON,
A_AIDL_BDIS,
B_ASE0_BRST,
+ A_BIDL_ADIS,
+
+ /* Auxiliary timers */
B_SE0_SRP,
B_SRP_FAIL,
A_WAIT_ENUM,
+
NUM_OTG_FSM_TIMERS,
};
@@ -69,9 +75,11 @@ struct otg_fsm {
/* Timeout indicator for timers */
int a_wait_vrise_tmout;
+ int a_wait_vfall_tmout;
int a_wait_bcon_tmout;
int a_aidl_bdis_tmout;
int b_ase0_brst_tmout;
+ int a_bidl_adis_tmout;
/* Informative variables */
int a_bus_drop;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] USB: phy: Check OTG FSM callback existance in helper functions
Existence of callback must be checked to avoid NULL pointer
dereferncing.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsm-usb.h | 35 ---
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 75344be..0f3f7d8 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -95,48 +95,69 @@ struct otg_fsm_ops {
};
-static inline void otg_chrg_vbus(struct otg_fsm *fsm, int on)
+static inline int otg_chrg_vbus(struct otg_fsm *fsm, int on)
{
+ if (!fsm->ops->chrg_vbus)
+ return -EOPNOTSUPP;
fsm->ops->chrg_vbus(fsm, on);
+ return 0;
}
-static inline void otg_drv_vbus(struct otg_fsm *fsm, int on)
+static inline int otg_drv_vbus(struct otg_fsm *fsm, int on)
{
+ if (!fsm->ops->drv_vbus)
+ return -EOPNOTSUPP;
if (fsm->drv_vbus != on) {
fsm->drv_vbus = on;
fsm->ops->drv_vbus(fsm, on);
}
+ return 0;
}
-static inline void otg_loc_conn(struct otg_fsm *fsm, int on)
+static inline int otg_loc_conn(struct otg_fsm *fsm, int on)
{
+ if (!fsm->ops->loc_conn)
+ return -EOPNOTSUPP;
if (fsm->loc_conn != on) {
fsm->loc_conn = on;
fsm->ops->loc_conn(fsm, on);
}
+ return 0;
}
-static inline void otg_loc_sof(struct otg_fsm *fsm, int on)
+static inline int otg_loc_sof(struct otg_fsm *fsm, int on)
{
+ if (!fsm->ops->loc_sof)
+ return -EOPNOTSUPP;
if (fsm->loc_sof != on) {
fsm->loc_sof = on;
fsm->ops->loc_sof(fsm, on);
}
+ return 0;
}
-static inline void otg_start_pulse(struct otg_fsm *fsm)
+static inline int otg_start_pulse(struct otg_fsm *fsm)
{
+ if (!fsm->ops->start_pulse)
+ return -EOPNOTSUPP;
fsm->ops->start_pulse(fsm);
+ return 0;
}
-static inline void otg_add_timer(struct otg_fsm *fsm, void *timer)
+static inline int otg_add_timer(struct otg_fsm *fsm, void *timer)
{
+ if (!fsm->ops->add_timer)
+ return -EOPNOTSUPP;
fsm->ops->add_timer(fsm, timer);
+ return 0;
}
-static inline void otg_del_timer(struct otg_fsm *fsm, void *timer)
+static inline int otg_del_timer(struct otg_fsm *fsm, void *timer)
{
+ if (!fsm->ops->del_timer)
+ return -EOPNOTSUPP;
fsm->ops->del_timer(fsm, timer);
+ return 0;
}
int otg_statemachine(struct otg_fsm *fsm);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] USB: phy: Add and use missed OTG FSM inputs/outputs
Several input/output variables missed in current FSM implementation.
This patch adds and makes use of them as specified in OTG and EH
supplement to USB2.0.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsm-usb.c | 18 +++---
drivers/usb/phy/phy-fsm-usb.h | 36 +++-
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index d5c6db0..329c2d2 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -71,8 +71,11 @@ void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
old_state)
case OTG_STATE_B_IDLE:
otg_del_timer(fsm, B_SE0_SRP);
fsm->b_se0_srp = 0;
+ fsm->adp_sns = 0;
+ fsm->adp_prb = 0;
break;
case OTG_STATE_B_SRP_INIT:
+ fsm->data_pulse = 0;
fsm->b_srp_done = 0;
break;
case OTG_STATE_B_PERIPHERAL:
@@ -84,6 +87,7 @@ void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
old_state)
case OTG_STATE_B_HOST:
break;
case OTG_STATE_A_IDLE:
+ fsm->adp_prb = 0;
break;
case OTG_STATE_A_WAIT_VRISE:
otg_del_timer(fsm, A_WAIT_VRISE);
@@ -131,6 +135,11 @@ int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state
new_state)
otg_chrg_vbus(fsm, 0);
otg_loc_conn(fsm, 0);
otg_loc_sof(fsm, 0);
+ /*
+* Driver is responsible for starting ADP probing
+* if ADP sensing times out.
+*/
+ otg_start_adp_sns(fsm);
otg_set_protocol(fsm, PROTO_UNDEF);
otg_add_timer(fsm, B_SE0_SRP);
break;
@@ -167,6 +176,7 @@ int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state
new_state)
otg_chrg_vbus(fsm, 0);
otg_loc_conn(fsm, 0);
otg_loc_sof(fsm, 0);
+ otg_start_adp_prb(fsm);
otg_set_protocol(fsm, PROTO_HOST);
break;
case OTG_STATE_A_WAIT_VRISE:
@@ -256,7 +266,8 @@ int otg_statemachine(struct otg_fsm *fsm)
otg_set_state(fsm, OTG_STATE_A_IDLE);
else if (fsm->b_sess_vld && fsm->otg->gadget)
otg_set_state(fsm, OTG_STATE_B_PERIPHERAL);
- else if (fsm->b_bus_req && fsm->b_ssend_srp && fsm->b_se0_srp)
+ else if ((fsm->b_bus_req || fsm->adp_change || fsm->power_up) &&
+ fsm->b_ssend_srp && fsm->b_se0_srp)
otg_set_state(fsm, OTG_STATE_B_SRP_INIT);
break;
case OTG_STATE_B_SRP_INIT:
@@ -283,13 +294,14 @@ int otg_statemachine(struct otg_fsm *fsm)
case OTG_STATE_B_HOST:
if (!fsm->id || !fsm->b_sess_vld)
otg_set_state(fsm, OTG_STATE_B_IDLE);
- else if (!fsm->b_bus_req || !fsm->a_conn)
+ else if (!fsm->b_bus_req || !fsm->a_conn || fsm->test_device)
otg_set_state(fsm, OTG_STATE_B_PERIPHERAL);
break;
case OTG_STATE_A_IDLE:
if (fsm->id)
otg_set_state(fsm, OTG_STATE_B_IDLE);
- else if (!fsm->a_bus_drop && (fsm->a_bus_req || fsm->a_srp_det))
+ else if (!fsm->a_bus_drop && (fsm->a_bus_req ||
+ fsm->a_srp_det || fsm->adp_change || fsm->power_up))
otg_set_state(fsm, OTG_STATE_A_WAIT_VRISE);
break;
case OTG_STATE_A_WAIT_VRISE:
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 2f185ed..6ce3b3c 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -54,6 +54,9 @@ enum otg_fsm_timer {
/* OTG state machine according to the OTG spec */
struct otg_fsm {
/* Input */
+ int adp_change;
+ int power_up;
+ int test_device;
int a_bus_drop;
int a_bus_req;
int a_bus_resume;
@@ -93,9 +96,12 @@ struct otg_fsm {
int b_bus_req_inf;
/* Output */
+ int data_pulse;
int drv_vbus;
int loc_conn;
int loc_sof;
+ int adp_prb;
+ int adp_sns;
struct otg_fsm_ops *ops;
struct usb_otg *otg;
@@ -111,6 +117,8 @@ struct otg_fsm_ops {
void(*loc_conn)(struct otg_fsm *fsm, int on);
void(*loc_sof)(struct otg_fsm *fsm, int on);
void(*start_pulse)(struct otg_fsm *fsm);
+ void(*start_adp_prb)(struct otg_fsm *fsm);
+ void(*start_adp_sns)(struct otg_fsm *fsm);
void(*add_timer)(struct otg_fsm *fsm, enum o
[PATCH 9/9] USB: phy: Reordering of OTG FSM variables
Reorder variables in struct otg_fsm as they appear in OTG and EH
supplement to USB2.0.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsm-usb.h | 46 +
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 6ce3b3c..7441b46 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -54,25 +54,34 @@ enum otg_fsm_timer {
/* OTG state machine according to the OTG spec */
struct otg_fsm {
/* Input */
+ int id;
int adp_change;
int power_up;
int test_device;
int a_bus_drop;
int a_bus_req;
+ int a_srp_det;
+ int a_vbus_vld;
+ int b_conn;
int a_bus_resume;
int a_bus_suspend;
int a_conn;
int b_bus_req;
- int a_sess_vld;
- int a_srp_det;
- int a_vbus_vld;
- int b_bus_resume;
- int b_bus_suspend;
- int b_conn;
int b_se0_srp;
int b_ssend_srp;
int b_sess_vld;
- int id;
+ /* Auxilary inputs */
+ int a_sess_vld;
+ int b_bus_resume;
+ int b_bus_suspend;
+
+ /* Output */
+ int data_pulse;
+ int drv_vbus;
+ int loc_conn;
+ int loc_sof;
+ int adp_prb;
+ int adp_sns;
/* Internal variables */
int a_set_b_hnp_en;
@@ -80,6 +89,14 @@ struct otg_fsm {
int b_hnp_enable;
int a_clr_err;
+ /* Informative variables */
+ int a_bus_drop_inf;
+ int a_bus_req_inf;
+ int a_clr_err_inf;
+ int b_bus_req_inf;
+ /* Auxilary informative variables */
+ int a_suspend_req_inf;
+
/* Timeout indicator for timers */
int a_wait_vrise_tmout;
int a_wait_vfall_tmout;
@@ -88,21 +105,6 @@ struct otg_fsm {
int b_ase0_brst_tmout;
int a_bidl_adis_tmout;
- /* Informative variables */
- int a_bus_drop_inf;
- int a_bus_req_inf;
- int a_clr_err_inf;
- int a_suspend_req_inf;
- int b_bus_req_inf;
-
- /* Output */
- int data_pulse;
- int drv_vbus;
- int loc_conn;
- int loc_sof;
- int adp_prb;
- int adp_sns;
-
struct otg_fsm_ops *ops;
struct usb_otg *otg;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] USB: phy: Pass OTG FSM pointer to callback functions
struct otg_fsm may be embedded to device's context structure.
The callbacks may require pointer to struct otg_fsm to obtain
necessary data for its operation (example: regulator reference
for drv_vbus()).
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.c | 30 +++---
drivers/usb/phy/phy-fsl-usb.h |4 ++--
drivers/usb/phy/phy-fsm-usb.h | 28 ++--
3 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index fa7c9f9..8b34694 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -134,7 +134,7 @@ int write_ulpi(u8 addr, u8 data)
/* Operations that will be called from OTG Finite State Machine */
/* Charge vbus for vbus pulsing in SRP */
-void fsl_otg_chrg_vbus(int on)
+void fsl_otg_chrg_vbus(struct otg_fsm *fsm, int on)
{
u32 tmp;
@@ -170,7 +170,7 @@ void fsl_otg_dischrg_vbus(int on)
}
/* A-device driver vbus, controlled through PP bit in PORTSC */
-void fsl_otg_drv_vbus(int on)
+void fsl_otg_drv_vbus(struct otg_fsm *fsm, int on)
{
u32 tmp;
@@ -188,7 +188,7 @@ void fsl_otg_drv_vbus(int on)
* Pull-up D+, signalling connect by periperal. Also used in
* data-line pulsing in SRP
*/
-void fsl_otg_loc_conn(int on)
+void fsl_otg_loc_conn(struct otg_fsm *fsm, int on)
{
u32 tmp;
@@ -207,7 +207,7 @@ void fsl_otg_loc_conn(int on)
* port. In host mode, controller will automatically send SOF.
* Suspend will block the data on the port.
*/
-void fsl_otg_loc_sof(int on)
+void fsl_otg_loc_sof(struct otg_fsm *fsm, int on)
{
u32 tmp;
@@ -222,7 +222,7 @@ void fsl_otg_loc_sof(int on)
}
/* Start SRP pulsing by data-line pulsing, followed with v-bus pulsing. */
-void fsl_otg_start_pulse(void)
+void fsl_otg_start_pulse(struct otg_fsm *fsm)
{
u32 tmp;
@@ -235,7 +235,7 @@ void fsl_otg_start_pulse(void)
fsl_otg_loc_conn(1);
#endif
- fsl_otg_add_timer(b_data_pulse_tmr);
+ fsl_otg_add_timer(fsm, b_data_pulse_tmr);
}
void b_data_pulse_end(unsigned long foo)
@@ -252,14 +252,14 @@ void b_data_pulse_end(unsigned long foo)
void fsl_otg_pulse_vbus(void)
{
srp_wait_done = 0;
- fsl_otg_chrg_vbus(1);
+ fsl_otg_chrg_vbus(&fsl_otg_dev->fsm, 1);
/* start the timer to end vbus charge */
- fsl_otg_add_timer(b_vbus_pulse_tmr);
+ fsl_otg_add_timer(&fsl_otg_dev->fsm, b_vbus_pulse_tmr);
}
void b_vbus_pulse_end(unsigned long foo)
{
- fsl_otg_chrg_vbus(0);
+ fsl_otg_chrg_vbus(&fsl_otg_dev->fsm, 0);
/*
* As USB3300 using the same a_sess_vld and b_sess_vld voltage
@@ -267,7 +267,7 @@ void b_vbus_pulse_end(unsigned long foo)
* residual voltage of vbus pulsing and A device pull up
*/
fsl_otg_dischrg_vbus(1);
- fsl_otg_add_timer(b_srp_wait_tmr);
+ fsl_otg_add_timer(&fsl_otg_dev->fsm, b_srp_wait_tmr);
}
void b_srp_end(unsigned long foo)
@@ -289,7 +289,7 @@ void a_wait_enum(unsigned long foo)
{
VDBG("a_wait_enum timeout\n");
if (!fsl_otg_dev->phy.otg->host->b_hnp_enable)
- fsl_otg_add_timer(a_wait_enum_tmr);
+ fsl_otg_add_timer(&fsl_otg_dev->fsm, a_wait_enum_tmr);
else
otg_statemachine(&fsl_otg_dev->fsm);
}
@@ -376,7 +376,7 @@ void fsl_otg_uninit_timers(void)
}
/* Add timer to timer list */
-void fsl_otg_add_timer(void *gtimer)
+void fsl_otg_add_timer(struct otg_fsm *fsm, void *gtimer)
{
struct fsl_otg_timer *timer = gtimer;
struct fsl_otg_timer *tmp_timer;
@@ -395,7 +395,7 @@ void fsl_otg_add_timer(void *gtimer)
}
/* Remove timer from the timer list; clear timeout status */
-void fsl_otg_del_timer(void *gtimer)
+void fsl_otg_del_timer(struct otg_fsm *fsm, void *gtimer)
{
struct fsl_otg_timer *timer = gtimer;
struct fsl_otg_timer *tmp_timer, *del_tmp;
@@ -468,7 +468,7 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on)
retval = dev->driver->pm->resume(dev);
if (fsm->id) {
/* default-b */
- fsl_otg_drv_vbus(1);
+ fsl_otg_drv_vbus(fsm, 1);
/*
* Workaround: b_host can't driver
* vbus, but PP in PORTSC needs to
@@ -493,7 +493,7 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on)
retval = dev->driver->pm->suspend(dev);
if (fsm->id)
/* default-b */
- fsl_otg_drv_vbus(0);
+ fsl_ot
[PATCH 7/9] USB: phy: Rename "B-device session end SRP" OTG FSM input
In accordance with OTG and EH supplement, the correct name
of the FSM input is b_ssend_srp.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.c |4 ++--
drivers/usb/phy/phy-fsm-usb.c |2 +-
drivers/usb/phy/phy-fsm-usb.h |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index d13ccd5..7f3c73b 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -1067,7 +1067,7 @@ static int show_fsl_usb2_otg_state(struct device *dev,
"b_bus_suspend: %d\n"
"b_conn: %d\n"
"b_se0_srp: %d\n"
- "b_sess_end: %d\n"
+ "b_ssend_srp: %d\n"
"b_sess_vld: %d\n"
"id: %d\n",
fsm->a_bus_req,
@@ -1082,7 +1082,7 @@ static int show_fsl_usb2_otg_state(struct device *dev,
fsm->b_bus_suspend,
fsm->b_conn,
fsm->b_se0_srp,
- fsm->b_sess_end,
+ fsm->b_ssend_srp,
fsm->b_sess_vld,
fsm->id);
size -= t;
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index cb0367c..d5c6db0 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -256,7 +256,7 @@ int otg_statemachine(struct otg_fsm *fsm)
otg_set_state(fsm, OTG_STATE_A_IDLE);
else if (fsm->b_sess_vld && fsm->otg->gadget)
otg_set_state(fsm, OTG_STATE_B_PERIPHERAL);
- else if (fsm->b_bus_req && fsm->b_sess_end && fsm->b_se0_srp)
+ else if (fsm->b_bus_req && fsm->b_ssend_srp && fsm->b_se0_srp)
otg_set_state(fsm, OTG_STATE_B_SRP_INIT);
break;
case OTG_STATE_B_SRP_INIT:
diff --git a/drivers/usb/phy/phy-fsm-usb.h b/drivers/usb/phy/phy-fsm-usb.h
index 4049e5c..2f185ed 100644
--- a/drivers/usb/phy/phy-fsm-usb.h
+++ b/drivers/usb/phy/phy-fsm-usb.h
@@ -67,7 +67,7 @@ struct otg_fsm {
int b_bus_suspend;
int b_conn;
int b_se0_srp;
- int b_sess_end;
+ int b_ssend_srp;
int b_sess_vld;
int id;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] USB: phy: Fix OTG FSM timer handling
Get rid of using OTG driver specific timers by passing timer
type to corresponding callbacks.
Signed-off-by: Anton Tikhomirov
---
drivers/usb/phy/phy-fsl-usb.c | 60 +++--
drivers/usb/phy/phy-fsm-usb.c | 28 +--
drivers/usb/phy/phy-fsm-usb.h | 24 ++---
3 files changed, 87 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 8b34694..6495861 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -375,6 +375,40 @@ void fsl_otg_uninit_timers(void)
kfree(b_vbus_pulse_tmr);
}
+static struct fsl_otg_timer *fsl_otg_get_timer(enum otg_fsm_timer t)
+{
+ struct fsl_otg_timer *timer;
+
+ /* REVISIT: use array of pointers to timers instead */
+ switch (t) {
+ case A_WAIT_VRISE:
+ timer = a_wait_vrise_tmr;
+ break;
+ case A_WAIT_BCON:
+ timer = a_wait_vrise_tmr;
+ break;
+ case A_AIDL_BDIS:
+ timer = a_wait_vrise_tmr;
+ break;
+ case B_ASE0_BRST:
+ timer = a_wait_vrise_tmr;
+ break;
+ case B_SE0_SRP:
+ timer = a_wait_vrise_tmr;
+ break;
+ case B_SRP_FAIL:
+ timer = a_wait_vrise_tmr;
+ break;
+ case A_WAIT_ENUM:
+ timer = a_wait_vrise_tmr;
+ break;
+ default:
+ timer = NULL;
+ }
+
+ return timer;
+}
+
/* Add timer to timer list */
void fsl_otg_add_timer(struct otg_fsm *fsm, void *gtimer)
{
@@ -394,6 +428,17 @@ void fsl_otg_add_timer(struct otg_fsm *fsm, void *gtimer)
list_add_tail(&timer->list, &active_timers);
}
+static void fsl_otg_fsm_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer t)
+{
+ struct fsl_otg_timer *timer;
+
+ timer = fsl_otg_get_timer(t);
+ if (!timer)
+ return;
+
+ fsl_otg_add_timer(fsm, timer);
+}
+
/* Remove timer from the timer list; clear timeout status */
void fsl_otg_del_timer(struct otg_fsm *fsm, void *gtimer)
{
@@ -405,6 +450,17 @@ void fsl_otg_del_timer(struct otg_fsm *fsm, void *gtimer)
list_del(&timer->list);
}
+static void fsl_otg_fsm_del_timer(struct otg_fsm *fsm, enum otg_fsm_timer t)
+{
+ struct fsl_otg_timer *timer;
+
+ timer = fsl_otg_get_timer(t);
+ if (!timer)
+ return;
+
+ fsl_otg_del_timer(fsm, timer);
+}
+
/*
* Reduce timer count by 1, and find timeout conditions.
* Called by fsl_otg 1ms timer interrupt
@@ -757,8 +813,8 @@ static struct otg_fsm_ops fsl_otg_ops = {
.loc_sof = fsl_otg_loc_sof,
.start_pulse = fsl_otg_start_pulse,
- .add_timer = fsl_otg_add_timer,
- .del_timer = fsl_otg_del_timer,
+ .add_timer = fsl_otg_fsm_add_timer,
+ .del_timer = fsl_otg_fsm_del_timer,
.start_host = fsl_otg_start_host,
.start_gadget = fsl_otg_start_gadget,
diff --git a/drivers/usb/phy/phy-fsm-usb.c b/drivers/usb/phy/phy-fsm-usb.c
index 7898459..f8fe7ec 100644
--- a/drivers/usb/phy/phy-fsm-usb.c
+++ b/drivers/usb/phy/phy-fsm-usb.c
@@ -69,7 +69,7 @@ void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
old_state)
{
switch (old_state) {
case OTG_STATE_B_IDLE:
- otg_del_timer(fsm, b_se0_srp_tmr);
+ otg_del_timer(fsm, B_SE0_SRP);
fsm->b_se0_srp = 0;
break;
case OTG_STATE_B_SRP_INIT:
@@ -78,7 +78,7 @@ void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state
old_state)
case OTG_STATE_B_PERIPHERAL:
break;
case OTG_STATE_B_WAIT_ACON:
- otg_del_timer(fsm, b_ase0_brst_tmr);
+ otg_del_timer(fsm, B_ASE0_BRST);
fsm->b_ase0_brst_tmout = 0;
break;
case OTG_STATE_B_HOST:
@@ -86,25 +86,25 @@ void otg_leave_state(struct otg_fsm *fsm, enum
usb_otg_state old_state)
case OTG_STATE_A_IDLE:
break;
case OTG_STATE_A_WAIT_VRISE:
- otg_del_timer(fsm, a_wait_vrise_tmr);
+ otg_del_timer(fsm, A_WAIT_VRISE);
fsm->a_wait_vrise_tmout = 0;
break;
case OTG_STATE_A_WAIT_BCON:
- otg_del_timer(fsm, a_wait_bcon_tmr);
+ otg_del_timer(fsm, A_WAIT_BCON);
fsm->a_wait_bcon_tmout = 0;
break;
case OTG_STATE_A_HOST:
- otg_del_timer(fsm, a_wait_enum_tmr);
+ otg_del_timer(fsm, A_WAIT_ENUM);
break;
case OTG_STATE_A_SUSPEND:
- otg_del_timer(fsm, a_aidl_bdis_tmr);
+ otg_del_timer(fsm, A_AIDL_BDIS);
fsm->a_aidl_bdis_tmout = 0;
fsm->a_suspend_req = 0;
break;
case OTG_STATE_A_PERIPHERAL:
RE: Endpoint flushing is not safe against URB removal
Hello,
Sorry for my haste, I missed that. We have situation when CPU
stalls in usb_hcd_flush_endpoint(), so at first glance it looked
strange for me. Will analyze more.
Thank you.
> On Mon, 28 Jan 2013, Anton Tikhomirov wrote:
>
> > Hello,
> >
> > In drivers/usb/core/hcd.c: usb_hcd_flush_endpoint() uses
> > list_for_each_entry() to unlink URBs pending on
> > endpoint. At the same time unlink1() calls usb_rh_urb_dequeue()
> > where URB is removed from its endpoint queue:
> >
> > void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb)
> > {
> > /* clear all state linking urb to this dev (and hcd) */
> > spin_lock(&hcd_urb_list_lock);
> > list_del_init(&urb->urb_list);
> > spin_unlock(&hcd_urb_list_lock);
> > }
> >
> > Shall we use safe version of list_for_each_entry() for cancelling
> URBs?
>
> No. Read usb_hcd_flush_endpoint() more closely. After calling
> unlink1() it restarts the loop from the beginning.
>
> Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Endpoint flushing is not safe against URB removal
Hello,
In drivers/usb/core/hcd.c: usb_hcd_flush_endpoint() uses
list_for_each_entry() to unlink URBs pending on
endpoint. At the same time unlink1() calls usb_rh_urb_dequeue()
where URB is removed from its endpoint queue:
void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb)
{
/* clear all state linking urb to this dev (and hcd) */
spin_lock(&hcd_urb_list_lock);
list_del_init(&urb->urb_list);
spin_unlock(&hcd_urb_list_lock);
}
Shall we use safe version of list_for_each_entry() for cancelling URBs?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 4/5] ARM: S3C64XX: Enabling samsung-usbphy driver
Hi Praveen,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Praveen Paneri
> Sent: Wednesday, August 08, 2012 4:41 PM
> To: linux-usb@vger.kernel.org
> Cc: devicetree-disc...@lists.ozlabs.org; linux-arm-
> ker...@lists.infradead.org; linux-samsung-...@vger.kernel.org;
> kgene@samsung.com; ba...@ti.com; gre...@linuxfoundation.org;
> thomas.abra...@linaro.org; ben-li...@fluff.org;
> broo...@opensource.wolfsonmicro.com; l.majew...@samsung.com;
> kyungmin.p...@samsung.com; grant.lik...@secretlab.ca; he...@sntech.de
> Subject: [PATCH v3 4/5] ARM: S3C64XX: Enabling samsung-usbphy driver
>
> Adding platform device for samsung-usbphy driver. Enabling it for
> s3c64xx based machines using s3c-hsotg.
>
> Signed-off-by: Praveen Paneri
> ---
> arch/arm/mach-s3c64xx/include/mach/map.h |2 +
> arch/arm/mach-s3c64xx/mach-crag6410.c|4 +++
> arch/arm/mach-s3c64xx/mach-smartq.c |5
> arch/arm/mach-s3c64xx/mach-smdk6410.c|4 +++
> arch/arm/mach-s3c64xx/setup-usb-phy.c| 14 +++
> arch/arm/plat-samsung/devs.c | 33
++
> arch/arm/plat-samsung/include/plat/devs.h|1 +
> arch/arm/plat-samsung/include/plat/usb-phy.h |1 +
> 8 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-s3c64xx/include/mach/map.h b/arch/arm/mach-
> s3c64xx/include/mach/map.h
> index 8e2097b..dc482bb 100644
> --- a/arch/arm/mach-s3c64xx/include/mach/map.h
> +++ b/arch/arm/mach-s3c64xx/include/mach/map.h
> @@ -65,6 +65,7 @@
>
> #define S3C64XX_PA_NAND (0x7020)
> #define S3C64XX_PA_FB(0x7710)
> +#define S3C64XX_PA_USB_HSPHY (0x7C10)
> #define S3C64XX_PA_USB_HSOTG (0x7C00)
> #define S3C64XX_PA_WATCHDOG (0x7E004000)
> #define S3C64XX_PA_RTC (0x7E005000)
> @@ -113,6 +114,7 @@
> #define S3C_PA_FBS3C64XX_PA_FB
> #define S3C_PA_USBHOST S3C64XX_PA_USBHOST
> #define S3C_PA_USB_HSOTG S3C64XX_PA_USB_HSOTG
> +#define S3C_PA_USB_PHY S3C64XX_PA_USB_HSPHY
> #define S3C_PA_RTC S3C64XX_PA_RTC
> #define S3C_PA_WDT S3C64XX_PA_WATCHDOG
> #define S3C_PA_SPI0 S3C64XX_PA_SPI0
> diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-
> s3c64xx/mach-crag6410.c
> index b0f5baf..fa02e2f 100644
> --- a/arch/arm/mach-s3c64xx/mach-crag6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
> @@ -31,6 +31,7 @@
> #include
>
> #include
> +#include
>
> #include
>
> @@ -336,6 +337,7 @@ static struct platform_device wallvdd_device = {
> };
>
> static struct platform_device *crag6410_devices[] __initdata = {
> + &samsung_device_usbphy,
> &s3c_device_hsmmc0,
> &s3c_device_hsmmc2,
> &s3c_device_i2c0,
> @@ -765,6 +767,7 @@ static const struct gpio_led_platform_data
> gpio_leds_pdata = {
> .num_leds = ARRAY_SIZE(gpio_leds),
> };
>
> +static struct samsung_usbphy_data crag6410_usbphy_pdata;
Why don't you use __initdata for this structure? It will be duplicated
in s3c_set_platdata() and we don't need it in memory after booting.
I think you can even initialize it here (with s5p_usb_phy_pmu_isolation).
>
> static void __init crag6410_machine_init(void)
> {
> @@ -790,6 +793,7 @@ static void __init crag6410_machine_init(void)
> s3c_i2c0_set_platdata(&i2c0_pdata);
> s3c_i2c1_set_platdata(&i2c1_pdata);
> s3c_fb_set_platdata(&crag6410_lcd_pdata);
> + samsung_usbphy_set_pdata(&crag6410_usbphy_pdata);
>
> i2c_register_board_info(0, i2c_devs0, ARRAY_SIZE(i2c_devs0));
> i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
> diff --git a/arch/arm/mach-s3c64xx/mach-smartq.c b/arch/arm/mach-
> s3c64xx/mach-smartq.c
> index 7400da1..e0c4df8 100644
> --- a/arch/arm/mach-s3c64xx/mach-smartq.c
> +++ b/arch/arm/mach-s3c64xx/mach-smartq.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -234,6 +235,7 @@ static struct i2c_board_info smartq_i2c_devs[]
> __initdata = {
> };
>
> static struct platform_device *smartq_devices[] __initdata = {
> + &samsung_device_usbphy,
> &s3c_device_hsmmc1, /* Init iNAND first, ... */
> &s3c_device_hsmmc0, /* ... then the external SD card */
> &s3c_device_hsmmc2,
> @@ -380,9 +382,12 @@ void __init smartq_map_io(void)
> smartq_lcd_mode_set();
> }
>
> +static struct samsung_usbphy_data smartq_usbphy_pdata;
Same here
> +
> void __init smartq_machine_init(void)
> {
> s3c_i2c0_set_platdata(NULL);
> + samsung_usbphy_set_pdata(&smartq_usbphy_pdata);
> s3c_hwmon_set_platdata(&smartq_hwmon_pdata);
> s3c_sdhci1_set_platdata(&smartq_internal_hsmmc_pdata);
> s3c_sdhci2_set_platdata(&smartq_internal_hsmmc_pdata);
> diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c b/arch/arm/mach-
> s3c64xx/mach-smdk64
RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
> -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of Ido Shayevitz > Sent: Wednesday, August 01, 2012 11:44 PM > To: Anton Tikhomirov > Cc: 'Ido Shayevitz'; 'Felipe Balbi'; paul.zimmer...@synopsys.com; linux- > u...@vger.kernel.org > Subject: RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3 > > > Hi Anton, > > On Tue, July 31, 2012 11:27 pm, Anton Tikhomirov wrote: > > Hi Ido, > > > >> -Original Message- > >> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > >> ow...@vger.kernel.org] On Behalf Of Ido Shayevitz > >> Sent: Monday, July 30, 2012 10:16 PM > >> To: Anton Tikhomirov > >> Cc: 'Ido Shayevitz'; 'Felipe Balbi'; linux-usb@vger.kernel.org > >> Subject: RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3 > >> > >> Hi Anton, > >> > >> On Mon, July 30, 2012 5:25 am, Anton Tikhomirov wrote: > >> > Hi Ido, > >> > > >> >> > > >> >> > You activate sm only if gadget and host are ready. At the same > >> time, > >> >> > in dwc3_otg_interrupt() you schedule work if interrupt happens. In > >> >> > situation > >> >> > when host is not set yet, but cable is plugged, we will have > >> unwanted > >> >> sm > >> >> > activation > >> >> > (in interrupt handler) and, as a consequence, repeating error > >> "unable > >> >> to > >> >> > start A-device\n" > >> >> > in dwc3_otg_sm_work(). > >> >> > >> >> Host and gadget should set themselves to the otg on drivers probe, > in > >> >> boot > >> >> time. So cable connect happens later. > >> >> If the scenario you describe does happen it means that the xHCI > >> driver > >> >> was > >> >> not loaded into the kernel, but cable with micro-A was plugged into > >> your > >> >> device, so need add host support to the menuconfig. > >> >> It should be enough to select in the menuconfig CONFIG_USB and > >> >> CONFIG_USB_XHCI_HCD. > >> > > >> > Agree. But what if my controller supports DRD mode, but I _want_ to > >> keep > >> > this > >> > option (XHCI support) off, for any reason. By the way, it seems we > >> will > >> > have > >> > similar effect when micro-A cable is plugged after > > otg_set_host(phy->otg, > >> > NULL) > >> > call. Of course such situations are rare. > >> > > >> > >> OK, so I will avoid the re-schedule of the sm_work after this error > will > >> be printed (so will be printed once...) > >> Thanks... > > > > One more thing. Currently the work is first time scheduled in two cases: > > when > > interrupt happens or both, peripheral and host, were set. If host is not > > set > > (and probably won't be) the work will _not_ be scheduled (and peripheral > > will > > not start) till we connect and then disconnect micro-A cable. In other > > words: > > we should be able to use peripheral even if host is not supported (and > > vice > > versa?). What's your opinion? > > The idea behind schedule the work after both are set, is that we don't > want a case that we scheduled the work after only the gadget was set but > the ID pin is zero. In this case we will get wasted run of work state > machine. How about to stop the state machine after the first error "unable to start A-device" (in OTG_STATE_A_IDLE). It will be restarted (in irq handler) when user unplugs the micro-A cable. > If host is not set, we will start the peripheral on cable connect, as you What cable type do you mean? If type is micro-A, then we will have otg event (id 1->0) and we can start peripheral as soon as user unplugs the cable (id 0->1). In this case, it's a little bit strange, that user needs to plug micro-A cable to start peripheral (read machine). If type is micro-B, then how can we catch this event? (Do you remember: host is not set, preventing us to start machine, and no interrupt since there is no id transition). Now dwc3_gadget_run_stop() is called from dwc3_gadget_pullup() only if dwc->vbus_active is 1, and from dwc3_gadget_vbus_session() which won't be called (and consequently vbus_active won't be set) since machine is not running. > said, and if ID pin is 1. When we sta
RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
Hi Ido,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Ido Shayevitz
> Sent: Monday, July 30, 2012 10:16 PM
> To: Anton Tikhomirov
> Cc: 'Ido Shayevitz'; 'Felipe Balbi'; linux-usb@vger.kernel.org
> Subject: RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
>
> Hi Anton,
>
> On Mon, July 30, 2012 5:25 am, Anton Tikhomirov wrote:
> > Hi Ido,
> >
> >> >
> >> > You activate sm only if gadget and host are ready. At the same time,
> >> > in dwc3_otg_interrupt() you schedule work if interrupt happens. In
> >> > situation
> >> > when host is not set yet, but cable is plugged, we will have unwanted
> >> sm
> >> > activation
> >> > (in interrupt handler) and, as a consequence, repeating error "unable
> >> to
> >> > start A-device\n"
> >> > in dwc3_otg_sm_work().
> >>
> >> Host and gadget should set themselves to the otg on drivers probe, in
> >> boot
> >> time. So cable connect happens later.
> >> If the scenario you describe does happen it means that the xHCI driver
> >> was
> >> not loaded into the kernel, but cable with micro-A was plugged into
> your
> >> device, so need add host support to the menuconfig.
> >> It should be enough to select in the menuconfig CONFIG_USB and
> >> CONFIG_USB_XHCI_HCD.
> >
> > Agree. But what if my controller supports DRD mode, but I _want_ to keep
> > this
> > option (XHCI support) off, for any reason. By the way, it seems we will
> > have
> > similar effect when micro-A cable is plugged after
otg_set_host(phy->otg,
> > NULL)
> > call. Of course such situations are rare.
> >
>
> OK, so I will avoid the re-schedule of the sm_work after this error will
> be printed (so will be printed once...)
> Thanks...
One more thing. Currently the work is first time scheduled in two cases:
when
interrupt happens or both, peripheral and host, were set. If host is not set
(and probably won't be) the work will _not_ be scheduled (and peripheral
will
not start) till we connect and then disconnect micro-A cable. In other
words:
we should be able to use peripheral even if host is not supported (and vice
versa?). What's your opinion?
>
> >> But if your controller DWC_MODE (see core.c) does not support DRD, or
> >> the
> >> cable plugged in is micro-B then this error should not be printed
> >> eventhough the host was not set.
> >>
> >> >> >> +} else {
> >> >> >> +if (otg->phy->state == OTG_STATE_A_HOST) {
> >> >> >> +dwc3_otg_start_host(otg, 0);
> >> >> >> +otg->host = NULL;
> >> >> >> +otg->phy->state = OTG_STATE_UNDEFINED;
> >> >> >> +schedule_work(&dotg->sm_work);
> >> >> >> +} else {
> >> >> >> +otg->host = NULL;
> >> >> >> +}
> >> >> >> +}
> >> >> >> +
> >> >> >> +return 0;
> >> >> >> +}
> >> >> >> +
> >> >> >> +/**
> >> >> >> + * dwc3_otg_start_peripheral - bind/unbind the peripheral
> >> >> controller.
> >> >> >> + *
> >> >> >> + * @otg: Pointer to the otg_transceiver structure.
> >> >> >> + * @gadget: pointer to the usb_gadget structure.
> >> >> >
> >> >> > This comment doesn't match the function definition (@on, not
> > @gadget).
> >> >> >
> >> >> >> + *
> >> >> >> + * Returns 0 on success otherwise negative errno.
> >> >> >> + */
> >> >> >> +static int dwc3_otg_start_peripheral(struct usb_otg *otg, int
on)
> >> >> >> +{
> >> >> >> +struct dwc3_otg *dotg = container_of(otg, struct dwc3_otg,
> >> otg);
> >> >> >> +
> >> >> >> +if (!otg->gadget)
> >> >> >> +return -EINVAL;
> >> >> >> +
> >> >> >> +if (on) {
> >> >> >> +dev_dbg(otg->phy->dev, "%s: turn on gadget %s\n",
> >> >> >
RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
Hi Ido,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Ido Shayevitz
> Sent: Monday, July 30, 2012 7:53 PM
> To: Anton Tikhomirov
> Cc: 'Ido Shayevitz'; 'Felipe Balbi'; linux-usb@vger.kernel.org
> Subject: RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
>
>
> Hi Anton,
>
> On Sun, July 29, 2012 7:10 pm, Anton Tikhomirov wrote:
> > Hi Ido,
> >
> > Some more comments.
> >
> >> -Original Message-
> >> From: Ido Shayevitz [mailto:i...@codeaurora.org]
> >> Sent: Sunday, July 29, 2012 9:55 PM
> >> To: Anton Tikhomirov
> >> Subject: RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
> >> >> +
> >> >> +/**
> >> >> + * dwc3_otg_set_host - bind/unbind the host controller driver.
> >> >> + *
> >> >> + * @otg: Pointer to the otg_transceiver structure.
> >> >> + * @host: Pointer to the usb_bus structure.
> >> >> + *
> >> >> + * Returns 0 on success otherwise negative errno.
> >> >> + */
> >> >> +static int dwc3_otg_set_host(struct usb_otg *otg, struct usb_bus
> > *host)
> >> >> +{
> >> >> + struct dwc3_otg *dotg = container_of(otg, struct dwc3_otg,
> otg);
> >> >> +
> >> >> + if (host) {
> >> >> + dev_dbg(otg->phy->dev, "%s: set host %s\n",
> >> >> + __func__, host->bus_name);
> >> >> + otg->host = host;
> >> >> +
> >> >> + /*
> >> >> +* Only after both peripheral and host are set then
> check
> >> >> +* OTG sm. This prevents unnecessary activation of
the
> sm
> >> >> +* in case the ID is high.
> >> >> +*/
> >> >> + if (otg->gadget)
> >> >> + schedule_work(&dotg->sm_work);
> >
> > You activate sm only if gadget and host are ready. At the same time,
> > in dwc3_otg_interrupt() you schedule work if interrupt happens. In
> > situation
> > when host is not set yet, but cable is plugged, we will have unwanted sm
> > activation
> > (in interrupt handler) and, as a consequence, repeating error "unable to
> > start A-device\n"
> > in dwc3_otg_sm_work().
>
> Host and gadget should set themselves to the otg on drivers probe, in boot
> time. So cable connect happens later.
> If the scenario you describe does happen it means that the xHCI driver was
> not loaded into the kernel, but cable with micro-A was plugged into your
> device, so need add host support to the menuconfig.
> It should be enough to select in the menuconfig CONFIG_USB and
> CONFIG_USB_XHCI_HCD.
Agree. But what if my controller supports DRD mode, but I _want_ to keep
this
option (XHCI support) off, for any reason. By the way, it seems we will have
similar effect when micro-A cable is plugged after otg_set_host(phy->otg,
NULL)
call. Of course such situations are rare.
> But if your controller DWC_MODE (see core.c) does not support DRD, or the
> cable plugged in is micro-B then this error should not be printed
> eventhough the host was not set.
>
> >> >> + } else {
> >> >> + if (otg->phy->state == OTG_STATE_A_HOST) {
> >> >> + dwc3_otg_start_host(otg, 0);
> >> >> + otg->host = NULL;
> >> >> + otg->phy->state = OTG_STATE_UNDEFINED;
> >> >> + schedule_work(&dotg->sm_work);
> >> >> + } else {
> >> >> + otg->host = NULL;
> >> >> + }
> >> >> + }
> >> >> +
> >> >> + return 0;
> >> >> +}
> >> >> +
> >> >> +/**
> >> >> + * dwc3_otg_start_peripheral - bind/unbind the peripheral
> >> controller.
> >> >> + *
> >> >> + * @otg: Pointer to the otg_transceiver structure.
> >> >> + * @gadget: pointer to the usb_gadget structure.
> >> >
> >> > This comment doesn't match the function definition (@on, not
@gadget).
> >> >
> >> >> + *
> >> >> + * Returns 0 on success otherwise negative errno.
RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
Hi Ido,
Some more comments.
> -Original Message-
> From: Ido Shayevitz [mailto:i...@codeaurora.org]
> Sent: Sunday, July 29, 2012 9:55 PM
> To: Anton Tikhomirov
> Subject: RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
>
>
> Hi Anton,
>
> Thanks,
> Fixed and will be sent soon,
> Ido
>
> On Wed, July 25, 2012 7:33 pm, Anton Tikhomirov wrote:
> > Hi Ido,
> >
> >> -Original Message-
> >> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> >> ow...@vger.kernel.org] On Behalf Of Ido Shayevitz
> >> Sent: Wednesday, July 25, 2012 9:46 PM
> >> To: ba...@ti.com; av.tikhomi...@samsung.com
> >> Cc: linux-usb@vger.kernel.org; i...@codeaurora.org
> >> Subject: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
> >>
> >> This is first release of otg driver for the dwc3 Synopsys USB3 core.
> >> The otg driver implements the otg final state machine and control the
> >> activation of the device controller or host controller.
> >>
> >> In this first implementation, only simple DRD mode is implemented,
> >> determine if A or B device according to the ID pin as reflected in the
> >> OSTS.ConIDSts field.
> >>
> >> Signed-off-by: Ido Shayevitz
> >>
> >> ---
> >> drivers/usb/dwc3/Kconfig |6 +-
> >> drivers/usb/dwc3/Makefile|2 +
> >> drivers/usb/dwc3/core.c | 15 +-
> >> drivers/usb/dwc3/core.h | 54 +-
> >> drivers/usb/dwc3/dwc3_otg.c | 512
> >> ++
> >> drivers/usb/dwc3/dwc3_otg.h | 38 +++
> >> drivers/usb/dwc3/gadget.c| 63 +
> >> drivers/usb/host/xhci-plat.c | 21 ++
> >> drivers/usb/host/xhci.c | 13 +-
> >> 9 files changed, 711 insertions(+), 13 deletions(-)
> >> create mode 100644 drivers/usb/dwc3/dwc3_otg.c
> >> create mode 100644 drivers/usb/dwc3/dwc3_otg.h
> >>
> >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> >> index d13c60f..0cc108d 100644
> >> --- a/drivers/usb/dwc3/Kconfig
> >> +++ b/drivers/usb/dwc3/Kconfig
> >> @@ -1,9 +1,9 @@
> >> config USB_DWC3
> >>tristate "DesignWare USB3 DRD Core Support"
> >> - depends on (USB && USB_GADGET)
> >> + depends on (USB || USB_GADGET)
> >>select USB_OTG_UTILS
> >> - select USB_GADGET_DUALSPEED
> >> - select USB_GADGET_SUPERSPEED
> >> + select USB_GADGET_DUALSPEED if USB_GADGET
> >> + select USB_GADGET_SUPERSPEED if USB_GADGET
> >>select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
> >>help
> >> Say Y or M here if your system has a Dual Role SuperSpeed
> >> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> >> index d441fe4..ffb3f55 100644
> >> --- a/drivers/usb/dwc3/Makefile
> >> +++ b/drivers/usb/dwc3/Makefile
> >> @@ -1,11 +1,13 @@
> >> ccflags-$(CONFIG_USB_DWC3_DEBUG) := -DDEBUG
> >> ccflags-$(CONFIG_USB_DWC3_VERBOSE)+= -DVERBOSE_DEBUG
> >> +ccflags-y += -Idrivers/usb/host
> >>
> >> obj-$(CONFIG_USB_DWC3)+= dwc3.o
> >>
> >> dwc3-y:= core.o
> >> dwc3-y+= host.o
> >> dwc3-y+= gadget.o ep0.o
> >> +dwc3-y+= dwc3_otg.o
> >>
> >> ifneq ($(CONFIG_DEBUG_FS),)
> >>dwc3-y += debugfs.o
> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> index c34452a..5343e39 100644
> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -517,15 +517,24 @@ static int __devinit dwc3_probe(struct
> >> platform_device *pdev)
> >>break;
> >>case DWC3_MODE_DRD:
> >>dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> >> + ret = dwc3_otg_init(dwc);
> >> + if (ret) {
> >> + dev_err(dev, "failed to initialize otg\n");
> >> + goto err1;
> >> + }
> >> +
> >>ret = dwc3_host_init(dwc);
> >>if (ret) {
> >>dev_err(dev, "failed to initialize host\n");
> >> + dwc3_otg_exit(dwc);
> >>goto e
RE: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
Hi Ido,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Ido Shayevitz
> Sent: Wednesday, July 25, 2012 9:46 PM
> To: ba...@ti.com; av.tikhomi...@samsung.com
> Cc: linux-usb@vger.kernel.org; i...@codeaurora.org
> Subject: [RFC/PATCH v3] usb: dwc3: Introduce OTG driver for dwc3
>
> This is first release of otg driver for the dwc3 Synopsys USB3 core.
> The otg driver implements the otg final state machine and control the
> activation of the device controller or host controller.
>
> In this first implementation, only simple DRD mode is implemented,
> determine if A or B device according to the ID pin as reflected in the
> OSTS.ConIDSts field.
>
> Signed-off-by: Ido Shayevitz
>
> ---
> drivers/usb/dwc3/Kconfig |6 +-
> drivers/usb/dwc3/Makefile|2 +
> drivers/usb/dwc3/core.c | 15 +-
> drivers/usb/dwc3/core.h | 54 +-
> drivers/usb/dwc3/dwc3_otg.c | 512
> ++
> drivers/usb/dwc3/dwc3_otg.h | 38 +++
> drivers/usb/dwc3/gadget.c| 63 +
> drivers/usb/host/xhci-plat.c | 21 ++
> drivers/usb/host/xhci.c | 13 +-
> 9 files changed, 711 insertions(+), 13 deletions(-)
> create mode 100644 drivers/usb/dwc3/dwc3_otg.c
> create mode 100644 drivers/usb/dwc3/dwc3_otg.h
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index d13c60f..0cc108d 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -1,9 +1,9 @@
> config USB_DWC3
> tristate "DesignWare USB3 DRD Core Support"
> - depends on (USB && USB_GADGET)
> + depends on (USB || USB_GADGET)
> select USB_OTG_UTILS
> - select USB_GADGET_DUALSPEED
> - select USB_GADGET_SUPERSPEED
> + select USB_GADGET_DUALSPEED if USB_GADGET
> + select USB_GADGET_SUPERSPEED if USB_GADGET
> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
> help
> Say Y or M here if your system has a Dual Role SuperSpeed
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index d441fe4..ffb3f55 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -1,11 +1,13 @@
> ccflags-$(CONFIG_USB_DWC3_DEBUG) := -DDEBUG
> ccflags-$(CONFIG_USB_DWC3_VERBOSE) += -DVERBOSE_DEBUG
> +ccflags-y += -Idrivers/usb/host
>
> obj-$(CONFIG_USB_DWC3) += dwc3.o
>
> dwc3-y := core.o
> dwc3-y += host.o
> dwc3-y += gadget.o ep0.o
> +dwc3-y += dwc3_otg.o
>
> ifneq ($(CONFIG_DEBUG_FS),)
> dwc3-y += debugfs.o
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c34452a..5343e39 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -517,15 +517,24 @@ static int __devinit dwc3_probe(struct
> platform_device *pdev)
> break;
> case DWC3_MODE_DRD:
> dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> + ret = dwc3_otg_init(dwc);
> + if (ret) {
> + dev_err(dev, "failed to initialize otg\n");
> + goto err1;
> + }
> +
> ret = dwc3_host_init(dwc);
> if (ret) {
> dev_err(dev, "failed to initialize host\n");
> + dwc3_otg_exit(dwc);
> goto err1;
> }
>
> ret = dwc3_gadget_init(dwc);
> if (ret) {
> dev_err(dev, "failed to initialize gadget\n");
> + dwc3_host_exit(dwc);
> + dwc3_otg_exit(dwc);
> goto err1;
> }
> break;
> @@ -554,8 +563,9 @@ err2:
> dwc3_host_exit(dwc);
> break;
> case DWC3_MODE_DRD:
> - dwc3_host_exit(dwc);
> dwc3_gadget_exit(dwc);
> + dwc3_host_exit(dwc);
> + dwc3_otg_exit(dwc);
> break;
> default:
> /* do nothing */
> @@ -588,8 +598,9 @@ static int __devexit dwc3_remove(struct
> platform_device *pdev)
> dwc3_host_exit(dwc);
> break;
> case DWC3_MODE_DRD:
> - dwc3_host_exit(dwc);
> dwc3_gadget_exit(dwc);
> + dwc3_host_exit(dwc);
> + dwc3_otg_exit(dwc);
> break;
> default:
> /* do nothing */
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index c611d80..29a03e0 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -50,6 +50,8 @@
> #include
> #include
>
> +#include "dwc3_otg.h"
> +
> /* Global constants */
> #define DWC3_EP0_BOUNCE_SIZE 512
> #define DWC3_ENDPOINTS_NUM 32
> @@ -152,8 +154,9 @@
> /* OTG Registers */
> #define DWC3_OC
RE: [RFC/PATCH v2] usb: dwc3: Introduce OTG driver for dwc3
Hi,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Ido Shayevitz
> Sent: Thursday, July 12, 2012 1:24 AM
> To: ba...@ti.com
> Cc: linux-usb@vger.kernel.org; i...@codeaurora.org
> Subject: [RFC/PATCH v2] usb: dwc3: Introduce OTG driver for dwc3
>
> This is first release of otg driver for the dwc3 Synopsys USB3 core.
> The otg driver implements the otg final state machine and control the
> activation of the device controller or host controller.
>
> In this first implementation, only simple DRD mode is implemented,
> determine if A or B device according to the ID pin as reflected in the
> OSTS.ConIDSts field.
>
> Signed-off-by: Ido Shayevitz
>
> ---
> drivers/usb/dwc3/Kconfig |6 +-
> drivers/usb/dwc3/Makefile|2 +
> drivers/usb/dwc3/core.c | 15 +-
> drivers/usb/dwc3/core.h | 51 -
> drivers/usb/dwc3/dwc3_otg.c | 512
> ++
> drivers/usb/dwc3/dwc3_otg.h | 38 +++
> drivers/usb/dwc3/gadget.c| 63 +
> drivers/usb/host/xhci-plat.c | 21 ++
> drivers/usb/host/xhci.c | 13 +-
> 9 files changed, 708 insertions(+), 13 deletions(-)
> create mode 100644 drivers/usb/dwc3/dwc3_otg.c
> create mode 100644 drivers/usb/dwc3/dwc3_otg.h
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index d13c60f..0cc108d 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -1,9 +1,9 @@
> config USB_DWC3
> tristate "DesignWare USB3 DRD Core Support"
> - depends on (USB && USB_GADGET)
> + depends on (USB || USB_GADGET)
> select USB_OTG_UTILS
> - select USB_GADGET_DUALSPEED
> - select USB_GADGET_SUPERSPEED
> + select USB_GADGET_DUALSPEED if USB_GADGET
> + select USB_GADGET_SUPERSPEED if USB_GADGET
> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
> help
> Say Y or M here if your system has a Dual Role SuperSpeed
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index d441fe4..ffb3f55 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -1,11 +1,13 @@
> ccflags-$(CONFIG_USB_DWC3_DEBUG) := -DDEBUG
> ccflags-$(CONFIG_USB_DWC3_VERBOSE) += -DVERBOSE_DEBUG
> +ccflags-y += -Idrivers/usb/host
>
> obj-$(CONFIG_USB_DWC3) += dwc3.o
>
> dwc3-y := core.o
> dwc3-y += host.o
> dwc3-y += gadget.o ep0.o
> +dwc3-y += dwc3_otg.o
>
> ifneq ($(CONFIG_DEBUG_FS),)
> dwc3-y += debugfs.o
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c34452a..5343e39 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -517,15 +517,24 @@ static int __devinit dwc3_probe(struct
> platform_device *pdev)
> break;
> case DWC3_MODE_DRD:
> dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> + ret = dwc3_otg_init(dwc);
> + if (ret) {
> + dev_err(dev, "failed to initialize otg\n");
> + goto err1;
> + }
> +
> ret = dwc3_host_init(dwc);
> if (ret) {
> dev_err(dev, "failed to initialize host\n");
> + dwc3_otg_exit(dwc);
> goto err1;
> }
>
> ret = dwc3_gadget_init(dwc);
> if (ret) {
> dev_err(dev, "failed to initialize gadget\n");
> + dwc3_host_exit(dwc);
> + dwc3_otg_exit(dwc);
> goto err1;
> }
> break;
> @@ -554,8 +563,9 @@ err2:
> dwc3_host_exit(dwc);
> break;
> case DWC3_MODE_DRD:
> - dwc3_host_exit(dwc);
> dwc3_gadget_exit(dwc);
> + dwc3_host_exit(dwc);
> + dwc3_otg_exit(dwc);
> break;
> default:
> /* do nothing */
> @@ -588,8 +598,9 @@ static int __devexit dwc3_remove(struct
> platform_device *pdev)
> dwc3_host_exit(dwc);
> break;
> case DWC3_MODE_DRD:
> - dwc3_host_exit(dwc);
> dwc3_gadget_exit(dwc);
> + dwc3_host_exit(dwc);
> + dwc3_otg_exit(dwc);
> break;
> default:
> /* do nothing */
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 151eca8..793758b 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -50,6 +50,8 @@
> #include
> #include
>
> +#include "dwc3_otg.h"
> +
> /* Global constants */
> #define DWC3_EP0_BOUNCE_SIZE 512
> #define DWC3_ENDPOINTS_NUM 32
> @@ -152,8 +154,9 @@
> /* OTG Registers */
> #define DWC3_OCFG0xcc00
> #define D
