Re: [PATCH 2/4] usb: dwc3: qcom: Configure TCSR phy mux register

[Felipe Balbi]

Hi,

Andy Gross  writes:
> This patch adds automatic configuration of the TCSR phy mux register based on
> the syscon-tcsr devicetree entry.  This configuration is optional, as some
> platforms may not require the mux selection.
>
> Signed-off-by: Andy Gross 

just when I find a way to make a generic dwc3-of-simple.c glue layer :-p

I can, certainly drop my patches but I need more details on the syscon
usage below.

> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 0880260..fcf264c 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  struct dwc3_qcom {
>   struct device   *dev;
> @@ -30,6 +32,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  {
>   struct device_node *node = pdev->dev.of_node;
>   struct dwc3_qcom *qdwc;
> + struct regmap *regmap;
> + u32 mux_offset;
> + u32 mux_bit;
>   int ret;
>  
>   qdwc = devm_kzalloc(>dev, sizeof(*qdwc), GFP_KERNEL);
> @@ -58,6 +63,26 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>   qdwc->sleep_clk = NULL;
>   }
>  
> + /* look for tcsr and if present, provision it */
> + regmap = syscon_regmap_lookup_by_phandle(node, "syscon-tcsr");
> + if (!IS_ERR(regmap)) {
> + if (of_property_read_u32_index(node, "syscon-tcsr", 1,
> +_offset)) {
> + dev_err(qdwc->dev, "missing USB TCSR mux offset\n");
> + return -EINVAL;
> + }
> + if (of_property_read_u32_index(node, "syscon-tcsr", 2,
> +_bit)) {
> + dev_err(qdwc->dev, "missing USB TCSR mux bit\n");
> + return -EINVAL;
> + }
> +
> + regmap_update_bits(regmap, mux_offset, BIT(mux_bit),
> +BIT(mux_bit));

what is tcsr and what does it ? It also seems to be optional, why's that ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 4/4] Documentation: usb: dwc3: qcom: Add TCSR mux usage

[Felipe Balbi]

Hi,

Andy Gross  writes:
> This patch adds documentation for the optional syscon-tcsr property in the
> Qualcomm DWC3 node.  The syscon-tcsr specifies the register and bit used to
> configure the TCSR USB phy mux register.
>
> Signed-off-by: Andy Gross 
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.txt | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt 
> b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> index ca164e7..dfa222d 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> @@ -8,6 +8,10 @@ Required properties:
>"core" Master/Core clock, have to be >= 125 MHz for SS
>   operation and >= 60MHz for HS operation
>  
> +Optional properties:
> +- syscon-tcsrSpecifies TCSR handle, register offset, and bit 
> position for
> + configuring the phy mux setting.

oh, it's a PHY mux ? I don't think it should be part of any dwc3-* glue
layer then. By the time we reach dwc3, the mux should be properly
configured.

Kishon, any ideas ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 2/2] usb: phy: msm: fix connect/disconnect bug for dragonboard OTG port

[Felipe Balbi]

Hi,

Tim Bird <tim.b...@sonymobile.com> writes:
> On 11/16/2015 09:21 AM, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Peter Chen <peter.c...@freescale.com> writes:
>>> On Wed, Nov 11, 2015 at 09:48:00AM -0800, Tim Bird wrote:
>>>>
>>>>
>>>> On 11/10/2015 07:14 PM, Peter Chen wrote:
>>>>> On Tue, Nov 10, 2015 at 04:46:51PM -0800, Tim Bird wrote:
>>>>>> This fixes a bug where if you disconnect and re-connect the USB cable,
>>>>>> the gadget driver stops working.
>>>>>>
>>>>>> Add support for async_irq to wake up driver from low power mode.
>>>>>> Without this, the power management code never calls resume.
>>>>>> Also, have the phy driver kick the gadget driver (chipidea otg)
>>>>>> by having the chipidea driver register with it, for vbus connect
>>>>>> notifications.
>>>>>>
>>>>>> Signed-off-by: Tim Bird <tim.b...@sonymobile.com>
>>>>>> ---
>>>>>>  drivers/usb/chipidea/udc.c|  6 ++
>>>>>>  drivers/usb/phy/phy-msm-usb.c | 16 
>>>>>>  include/linux/usb/msm_hsusb.h |  1 +
>>>>>>  3 files changed, 23 insertions(+)
>> 
>> I just wanna know how you guys want this to be handled ? Through my tree
>> or chipidea's ? Or do we break the dependencies between the changes ?
>
> I'm fine with splitting it up.  I'm sending a new series with 3 patches
> right after this message.  Do both trees go to linux-next?

I have my fixes and next branches both on next. Not sure about chipidea.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 2/2] usb: phy: msm: fix connect/disconnect bug for dragonboard OTG port

[Felipe Balbi]

Hi,

Peter Chen  writes:
> On Wed, Nov 11, 2015 at 09:48:00AM -0800, Tim Bird wrote:
>> 
>> 
>> On 11/10/2015 07:14 PM, Peter Chen wrote:
>> > On Tue, Nov 10, 2015 at 04:46:51PM -0800, Tim Bird wrote:
>> >> This fixes a bug where if you disconnect and re-connect the USB cable,
>> >> the gadget driver stops working.
>> >>
>> >> Add support for async_irq to wake up driver from low power mode.
>> >> Without this, the power management code never calls resume.
>> >> Also, have the phy driver kick the gadget driver (chipidea otg)
>> >> by having the chipidea driver register with it, for vbus connect
>> >> notifications.
>> >>
>> >> Signed-off-by: Tim Bird 
>> >> ---
>> >>  drivers/usb/chipidea/udc.c|  6 ++
>> >>  drivers/usb/phy/phy-msm-usb.c | 16 
>> >>  include/linux/usb/msm_hsusb.h |  1 +
>> >>  3 files changed, 23 insertions(+)

I just wanna know how you guys want this to be handled ? Through my tree
or chipidea's ? Or do we break the dependencies between the changes ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: phy: msm: Add D+/D- lines route control

[Felipe Balbi]

On Wed, Jul 08, 2015 at 12:45:54PM +0300, Ivan T. Ivanov wrote:
 apq8016-sbc board is using Dual SPDT USB Switch (TC7USB40MU),
 witch is controlled by GPIO to de/multiplex D+/D- USB lines to
 USB2513B Hub and uB connector. Add support for this.
 
 Signed-off-by: Ivan T. Ivanov ivan.iva...@linaro.org

doesn't apply:

checking file Documentation/devicetree/bindings/usb/msm-hsusb.txt
checking file drivers/usb/phy/phy-msm-usb.c
Hunk #5 succeeded at 1632 (offset 2 lines).
Hunk #6 succeeded at 1809 (offset 2 lines).
Hunk #7 FAILED at 1844.
1 out of 7 hunks FAILED
checking file include/linux/usb/msm_hsusb.h

Please rebase on my testing/next

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 4/5] usb: phy: phy-msm-usb: Rename regulator_set_optimum_mode

[Felipe Balbi]

On Wed, Feb 11, 2015 at 07:35:30PM -0800, Bjorn Andersson wrote:
 The function regulator_set_optimum_mode() is changing name to
 regulator_set_load(), so update the code accordingly.
 
 Signed-off-by: Bjorn Andersson bjorn.anders...@sonymobile.com

since this depends on the rest of the series:

Acked-by: Felipe Balbi ba...@ti.com

 ---
  drivers/usb/phy/phy-msm-usb.c | 15 +--
  1 file changed, 5 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index 000fd89..6ed67ea 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -142,27 +142,22 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg, 
 int on)
   int ret = 0;
  
   if (on) {
 - ret = regulator_set_optimum_mode(motg-v1p8,
 - USB_PHY_1P8_HPM_LOAD);
 + ret = regulator_set_load(motg-v1p8, USB_PHY_1P8_HPM_LOAD);
   if (ret  0) {
   pr_err(Could not set HPM for v1p8\n);
   return ret;
   }
 - ret = regulator_set_optimum_mode(motg-v3p3,
 - USB_PHY_3P3_HPM_LOAD);
 + ret = regulator_set_load(motg-v3p3, USB_PHY_3P3_HPM_LOAD);
   if (ret  0) {
   pr_err(Could not set HPM for v3p3\n);
 - regulator_set_optimum_mode(motg-v1p8,
 - USB_PHY_1P8_LPM_LOAD);
 + regulator_set_load(motg-v1p8, USB_PHY_1P8_LPM_LOAD);
   return ret;
   }
   } else {
 - ret = regulator_set_optimum_mode(motg-v1p8,
 - USB_PHY_1P8_LPM_LOAD);
 + ret = regulator_set_load(motg-v1p8, USB_PHY_1P8_LPM_LOAD);
   if (ret  0)
   pr_err(Could not set LPM for v1p8\n);
 - ret = regulator_set_optimum_mode(motg-v3p3,
 - USB_PHY_3P3_LPM_LOAD);
 + ret = regulator_set_load(motg-v3p3, USB_PHY_3P3_LPM_LOAD);
   if (ret  0)
   pr_err(Could not set LPM for v3p3\n);
   }
 -- 
 1.8.2.2
 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 3/5] usb: phy: ab8500-usb: Rename regulator_set_optimum_mode

[Felipe Balbi]

On Wed, Feb 11, 2015 at 07:35:29PM -0800, Bjorn Andersson wrote:
 The function regulator_set_optimum_mode() is changing name to
 regulator_set_load(), so update the code accordingly.
 
 Signed-off-by: Bjorn Andersson bjorn.anders...@sonymobile.com

since this depends on the rest of the series:

Acked-by: Felipe Balbi ba...@ti.com

 ---
  drivers/usb/phy/phy-ab8500-usb.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-ab8500-usb.c 
 b/drivers/usb/phy/phy-ab8500-usb.c
 index 0b1bd23..f5b3b92 100644
 --- a/drivers/usb/phy/phy-ab8500-usb.c
 +++ b/drivers/usb/phy/phy-ab8500-usb.c
 @@ -277,7 +277,7 @@ static void ab8500_usb_regulator_enable(struct ab8500_usb 
 *ab)
   dev_err(ab-dev, Failed to set the Vintcore to 1.3V, 
 ret=%d\n,
   ret);
  
 - ret = regulator_set_optimum_mode(ab-v_ulpi, 28000);
 + ret = regulator_set_load(ab-v_ulpi, 28000);
   if (ret  0)
   dev_err(ab-dev, Failed to set optimum mode 
 (ret=%d)\n,
   ret);
 @@ -317,7 +317,7 @@ static void ab8500_usb_regulator_disable(struct 
 ab8500_usb *ab)
   ab-saved_v_ulpi, ret);
   }
  
 - ret = regulator_set_optimum_mode(ab-v_ulpi, 0);
 + ret = regulator_set_load(ab-v_ulpi, 0);
   if (ret  0)
   dev_err(ab-dev, Failed to set optimum mode 
 (ret=%d)\n,
   ret);
 -- 
 1.8.2.2
 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] i2c: qup: Fix order of runtime pm initialization

[Felipe Balbi]

On Mon, Sep 29, 2014 at 05:00:51PM -0500, Andy Gross wrote:
 The runtime pm calls need to be done before populating the children via the
 i2c_add_adapter call.  If this is not done, a child can run into issues trying
 to do i2c read/writes due to the pm_runtime_sync failing.
 
 Signed-off-by: Andy Gross agr...@codeaurora.org

Reviewed-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Felipe Balbi]

Hi,

On Sat, Sep 13, 2014 at 12:16:01PM +0530, Kishon Vijay Abraham I wrote:
 On Saturday 13 September 2014 12:58 AM, Andy Gross wrote:
  This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on 
  some
  Qualcomm platforms.  This driver uses the generic PHY framework and will
  interact with the DWC3 controller.
 
 Do you have dt documentation for this driver?

see patch 1

  +static inline void qcom_dwc3_phy_write_readback(
  +   struct qcom_dwc3_usb_phy *phy_dwc3, u32 offset,
  +   const u32 mask, u32 val)
  +{
  +   u32 write_val, tmp = readl(phy_dwc3-base + offset);
  +
  +   tmp = ~mask;   /* retain other bits */
  +   write_val = tmp | val;
  +
  +   writel(write_val, phy_dwc3-base + offset);
  +
  +   /* Read back to see if val was written */
 
 Does it fail sometime? I'm not sure if this should be present in the
 driver since this looks more of a debug code.

this was mentioned before. Silicon bug.

  +   writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
  +   phy_dwc3-base + SSUSB_PHY_CTRL_REG);
  +   usleep_range(2000, 2200);
 
 use msleep here..

why ? usleep_range() gives the scheduler oportunity to group timers.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v8 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

Hi,

On Fri, Sep 12, 2014 at 12:29:45PM -0500, Andy Gross wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 DWC3 glue layer is hardware layer around Synopsys DesignWare
 USB3 core. Its purpose is to supply Synopsys IP with required
 clocks, voltages and interface it with the rest of the SoC.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 Signed-off-by: Andy Gross agr...@codeaurora.org
 ---
  drivers/usb/dwc3/Kconfig |9 +++
  drivers/usb/dwc3/Makefile|1 +
  drivers/usb/dwc3/dwc3-qcom.c |  133 
 ++
  3 files changed, 143 insertions(+)
  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
 
 diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
 index 785510a..e9f258e 100644
 --- a/drivers/usb/dwc3/Kconfig
 +++ b/drivers/usb/dwc3/Kconfig
 @@ -80,6 +80,15 @@ config USB_DWC3_KEYSTONE
 Support of USB2/3 functionality in TI Keystone2 platforms.
 Say 'Y' or 'M' here if you have one such device
  
 +config USB_DWC3_QCOM
 + tristate Qualcomm Platforms
 + depends on ARCH_QCOM || COMPILE_TEST
 + default USB_DWC3
 + select PHY_QCOM_DWC3

I would rather steer away from selecting the PHY as it prevents having
PHYs as modules. Also, this will add a Kconfig warning for selecting a
symbol that doesn't exist (yet).

If it's all the same with you, I can clean this up when applying.

 diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
 new file mode 100644
 index 000..2067c04
 --- /dev/null
 +++ b/drivers/usb/dwc3/dwc3-qcom.c
 @@ -0,0 +1,133 @@
 +/* Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 and
 + * only version 2 as published by the Free Software Foundation.
 + *
 + * 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.
 + */
 +
 +#include linux/clk.h
 +#include linux/err.h
 +#include linux/io.h
 +#include linux/module.h
 +#include linux/of.h
 +#include linux/of_platform.h
 +#include linux/platform_device.h
 +#include linux/regulator/consumer.h
 +#include linux/usb/phy.h

doesn't look like you need regulator/consumer.h and usb/phy.h. I can
clean it up when applying.

 +struct dwc3_qcom {
 + struct device   *dev;
 +
 + struct clk  *core_clk;
 + struct clk  *iface_clk;
 + struct clk  *sleep_clk;
 +};
 +
 +static int dwc3_qcom_probe(struct platform_device *pdev)
 +{
 + struct device_node *node = pdev-dev.of_node;
 + struct dwc3_qcom *qdwc;
 + int ret = 0;
 +
 + qdwc = devm_kzalloc(pdev-dev, sizeof(*qdwc), GFP_KERNEL);
 + if (!qdwc)
 + return -ENOMEM;
 +
 + platform_set_drvdata(pdev, qdwc);
 +
 + qdwc-dev = pdev-dev;
 +
 + qdwc-core_clk = devm_clk_get(qdwc-dev, core);
 + if (IS_ERR(qdwc-core_clk)) {
 + dev_dbg(qdwc-dev, failed to get core clock\n);

this one would be a dev_err() since core can't work without its clock. I
can fix when applying.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v8 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Felipe Balbi]

Hi,

On Fri, Sep 12, 2014 at 12:29:46PM -0500, Andy Gross wrote:
 This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
 Qualcomm platforms.  This driver uses the generic PHY framework and will
 interact with the DWC3 controller.
 
 Signed-off-by: Andy Gross agr...@codeaurora.org

Kishon, this looks good to my eyes. It has no direct dependency with
dwc3 glue layer. Can you still grab this for v3.18 merge window ?


Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Felipe Balbi ba...@ti.com

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

On Sat, Sep 13, 2014 at 01:44:25AM +0530, Pramod Gurav wrote:
 Andy,
 Couple of minor comments.
 
 On Sat, Sep 13, 2014 at 12:58 AM, Andy Gross agr...@codeaurora.org wrote:
 
  From: Ivan T. Ivanov iiva...@mm-sol.com
 
  DWC3 glue layer is hardware layer around Synopsys DesignWare
  USB3 core. Its purpose is to supply Synopsys IP with required
  clocks, voltages and interface it with the rest of the SoC.
 
  Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
  Signed-off-by: Andy Gross agr...@codeaurora.org
  ---
   drivers/usb/dwc3/Kconfig |8 +++
   drivers/usb/dwc3/Makefile|1 +
   drivers/usb/dwc3/dwc3-qcom.c |  131
  ++
   3 files changed, 140 insertions(+)
   create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
 
 
 ..
 
 
  +#include linux/platform_device.h
  +
  +struct dwc3_qcom {
  +   struct device   *dev;
  +
 
 Extra new line here.

that's not an issue however.

  +   struct clk  *core_clk;
  +   struct clk  *iface_clk;
  +   struct clk  *sleep_clk;
  +};
  +
  +static int dwc3_qcom_probe(struct platform_device *pdev)
  +{
  +   struct device_node *node = pdev-dev.of_node;
  +   struct dwc3_qcom *qdwc;
  +   int ret = 0;
 
 Initialization not required.

I'll fix this one as I'm already applying this patch.

  +
  +   qdwc = devm_kzalloc(pdev-dev, sizeof(*qdwc), GFP_KERNEL);
  +   if (!qdwc)
  +   return -ENOMEM;
  +
  +   platform_set_drvdata(pdev, qdwc);
  +
  +   qdwc-dev = pdev-dev;
  +
  +   qdwc-core_clk = devm_clk_get(qdwc-dev, core);
  +   if (IS_ERR(qdwc-core_clk)) {
  +   dev_err(qdwc-dev, failed to get core clock\n);
  +   return PTR_ERR(qdwc-core_clk);
  +   }
  +
  +   qdwc-iface_clk = devm_clk_get(qdwc-dev, iface);
  +   if (IS_ERR(qdwc-iface_clk)) {
  +   dev_dbg(qdwc-dev, failed to get optional iface clock\n);
  +   qdwc-iface_clk = NULL;
  +   }
  +
  +   qdwc-sleep_clk = devm_clk_get(qdwc-dev, sleep);
  +   if (IS_ERR(qdwc-sleep_clk)) {
  +   dev_dbg(qdwc-dev, failed to get optional sleep clock\n);
  +   qdwc-sleep_clk = NULL;
  +   }
  +
  +   ret = clk_prepare_enable(qdwc-core_clk);
  +   if (ret) {
  +   dev_err(qdwc-dev, failed to enable core clock\n);
  +   goto err_core;
  +   }
  +
  +   ret = clk_prepare_enable(qdwc-iface_clk);
 
 Should not we check if  qdwc-iface_clk is valid?

read the sources luke.

  +err_clks:
  +   clk_disable_unprepare(qdwc-sleep_clk);
 
 IS_ERR check before above statement not needed as we have continued with
 probe even after failure og devm_clk_get?

read more carefully, there's a detail which you're missing.

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

Hi,

On Sat, Sep 13, 2014 at 01:55:50AM +0530, Pramod Gurav wrote:
  +   qdwc = devm_kzalloc(pdev-dev, sizeof(*qdwc), GFP_KERNEL);
  +   if (!qdwc)
  +   return -ENOMEM;
  +
  +   platform_set_drvdata(pdev, qdwc);
  +
  +   qdwc-dev = pdev-dev;
  +
  +   qdwc-core_clk = devm_clk_get(qdwc-dev, core);
  +   if (IS_ERR(qdwc-core_clk)) {
  +   dev_err(qdwc-dev, failed to get core clock\n);
  +   return PTR_ERR(qdwc-core_clk);
  +   }
  +
  +   qdwc-iface_clk = devm_clk_get(qdwc-dev, iface);
  +   if (IS_ERR(qdwc-iface_clk)) {
  +   dev_dbg(qdwc-dev, failed to get optional iface 
  clock\n);
  +   qdwc-iface_clk = NULL;
  +   }
  +
  +   qdwc-sleep_clk = devm_clk_get(qdwc-dev, sleep);
  +   if (IS_ERR(qdwc-sleep_clk)) {
  +   dev_dbg(qdwc-dev, failed to get optional sleep 
  clock\n);
  +   qdwc-sleep_clk = NULL;
  +   }
  +
  +   ret = clk_prepare_enable(qdwc-core_clk);
  +   if (ret) {
  +   dev_err(qdwc-dev, failed to enable core clock\n);
  +   goto err_core;
  +   }
  +
  +   ret = clk_prepare_enable(qdwc-iface_clk);
 
  Should not we check if  qdwc-iface_clk is valid?
  
  read the sources luke.
 Now I read that its initialized to NULL in fail case but should we call
 prepare_enable at all if its NULL?

now read the source of clk_enable() and clk_prepare() ;-) NULL is a
valid clock, it just returns 0. This is better than sprinkling IS_ERR()
all over the place.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: phy: msm: Make phy_reset clk and reset line optional.

[Felipe Balbi]

On Thu, Jul 17, 2014 at 09:16:40PM +0100, Srinivas Kandagatla wrote:
 This patch makes the phy reset clk and reset line optional as this clk
 is not available on boards like IFC6410 with APQ8064.
 
 phy-reset clk is only used as argument to the mach level callbacks, so
 this patch adds condition before clk_get calls so that the driver
 wouldn't fail on SOCs which do not have this support.
 
 Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
 ---
 Hi Felipe,
 
 With this new patch now the error message is only printed if the SOC actually 
 supports
 the phy reset clk, for SOCs like APQ8064 where there is no phy reset clock or
 the callback which takes it there is no point in doing a clk_get call in the 
 first place.

doesn't apply. Please rebase on top of v3.17-rc1

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH RESEND] usb: chipidea: msm: Use USB PHY API to control PHY state

[Felipe Balbi]

On Fri, Aug 15, 2014 at 12:21:19PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 PHY drivers keep track of the current state of the hardware,
 so don't change PHY settings under it.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

looks correct to me from a PHY API perspective, so:

Acked-by: Felipe Balbi ba...@ti.com

However, it doesn't look like msm_phy_init() is equivalent to the lines
removes. Care to comment ?

 ---
  drivers/usb/chipidea/ci_hdrc_msm.c | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c 
 b/drivers/usb/chipidea/ci_hdrc_msm.c
 index d72b9d2..81de834 100644
 --- a/drivers/usb/chipidea/ci_hdrc_msm.c
 +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
 @@ -20,13 +20,11 @@
  static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
  {
   struct device *dev = ci-gadget.dev.parent;
 - int val;
  
   switch (event) {
   case CI_HDRC_CONTROLLER_RESET_EVENT:
   dev_dbg(dev, CI_HDRC_CONTROLLER_RESET_EVENT received\n);
 - writel(0, USB_AHBBURST);
 - writel(0, USB_AHBMODE);
 + usb_phy_init(ci-transceiver);
   break;
   case CI_HDRC_CONTROLLER_STOPPED_EVENT:
   dev_dbg(dev, CI_HDRC_CONTROLLER_STOPPED_EVENT received\n);
 @@ -34,10 +32,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, 
 unsigned event)
* Put the transceiver in non-driving mode. Otherwise host
* may not detect soft-disconnection.
*/
 - val = usb_phy_io_read(ci-transceiver, ULPI_FUNC_CTRL);
 - val = ~ULPI_FUNC_CTRL_OPMODE_MASK;
 - val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
 - usb_phy_io_write(ci-transceiver, val, ULPI_FUNC_CTRL);
 + usb_phy_notify_disconnect(ci-transceiver, USB_SPEED_UNKNOWN);
   break;
   default:
   dev_dbg(dev, unknown ci_hdrc event\n);
 -- 
 1.8.3.2
 

-- 
balbi


signature.asc
Description: Digital signature

Re: Fwd: Status of chipidea msm USB reset patch

[Felipe Balbi]

Hi,

On Thu, Aug 14, 2014 at 09:53:10AM -0700, Tim Bird wrote:
 Ping.  Anybody know the status of this patch?  Is it queued in someone's tree?
 Without it the USB driver for the Qualcomm 8974 (hsusb phy) doesn't
 work (at least for me).
 It looks like it got dropped from Ivan's original patch series, back in May.

I don't maintain chipidea, Peter's the guy you want

 
 -- Forwarded message --
 From: Tim Bird tbird...@gmail.com
 Date: Fri, Jul 25, 2014 at 2:38 PM
 Subject: Status of chipidea msm USB reset patch
 To: linux-arm-msm@vger.kernel.org, ba...@ti.com, Ivan T. Ivanov
 iiva...@mm-sol.com
 
 
 Ivan and Felipe,
 
 Do you know the status of the patch below?  It was part of Ivan's
 USB patch set, which got mainlined recently.  However, this patch
 did not show up in Linus' tree.
 
 Is it in another tree on it's way, or does it need a re-submission?
 
 I was recently testing the MSM USB gadget driver on the Dragonboard 800
 (with a Qualcomm 8974 processor), and without this patch the USB hardware
 does not come up properly.
 
 Thanks,
  -- Tim
 
 
 Subject: [PATCH] usb: chipidea: msm: Use USB PHY API to control PHY state
 
 PHY drivers keep track of the current state of the hardware,
 so don't change PHY settings under it.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  drivers/usb/chipidea/ci_hdrc_msm.c | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
 b/drivers/usb/chipidea/ci_hdrc_msm.c
 index d72b9d2..81de834 100644
 --- a/drivers/usb/chipidea/ci_hdrc_msm.c
 +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
 @@ -20,13 +20,11 @@
  static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
  {
   struct device *dev = ci-gadget.dev.parent;
 - int val;
 
   switch (event) {
   case CI_HDRC_CONTROLLER_RESET_EVENT:
   dev_dbg(dev, CI_HDRC_CONTROLLER_RESET_EVENT received\n);
 - writel(0, USB_AHBBURST);
 - writel(0, USB_AHBMODE);
 + usb_phy_init(ci-transceiver);
   break;
   case CI_HDRC_CONTROLLER_STOPPED_EVENT:
   dev_dbg(dev, CI_HDRC_CONTROLLER_STOPPED_EVENT received\n);
 @@ -34,10 +32,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc
 *ci, unsigned event)
   * Put the transceiver in non-driving mode. Otherwise host
   * may not detect soft-disconnection.
   */
 - val = usb_phy_io_read(ci-transceiver, ULPI_FUNC_CTRL);
 - val = ~ULPI_FUNC_CTRL_OPMODE_MASK;
 - val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
 - usb_phy_io_write(ci-transceiver, val, ULPI_FUNC_CTRL);
 + usb_phy_notify_disconnect(ci-transceiver, USB_SPEED_UNKNOWN);
   break;
   default:
   dev_dbg(dev, unknown ci_hdrc event\n);
 --
 1.8.2.2

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

[Felipe Balbi]

Hi,

On Wed, Jul 02, 2014 at 02:48:17PM +0200, Arnd Bergmann wrote:
 On Wednesday 02 July 2014 11:43:25 Ivan T. Ivanov wrote:
snip
   
 +
 +   ranges;
 +
 +   status = disabled;
 +
 +   dwc3@1100 {
 +   compatible = snps,dwc3;
   
This sub-node is just wrong. Why can't you have a single node with '
qcom,dwc3, snps,dwc3 ' for the compatible property? All you are
adding here is clocks. Does the Synopsys block have no clocks?
   
I guess this is copied from other broken dwc3 bindings... That doesn't
mean you have to copy it.
   
The dwc3 core does not deal with clocks.  That is why everyone has a 
wrapper.

everyone has a wrapper for several others reasons. PM from the point of
view of the Synopsys IP is *completely* different from PM from the point
of view the $current_soc. The wrapper helps abstract that. Currently,
there's little PM implemented in the driver, but when we get to
implementing the nice features Synopsys has given us (hibernation, for
instance) it will start to be aparent why the driver was split like
that.

That, in addition to pm, has to be handled from the wrapper.  That's my 
take
anyway.  I am sure Felipe can speak more to this.
   
   That is a Linux driver issue which is irrelevant to the binding.
  
  DWC3 IP core from Synopsys is the same across SoC's (OMAP, Exynos..),
  vendors are adding glue IP to provide necessary clocks and voltages. 
  I think that the DT bindings properly reflect this fact.
 
 Not really. The proper way to do this is to have a single node that
 lists multiple compatible strings from the most specific (per SoC variant)
 to most generic (the underlying IP core) and have all properties in it.
 
 We have seen many cases before where it later turned out that whatever
 feature people thought was SoC specific actually was common to all
 of them and that we later want to change the code to handle it in a
 common way, and to reflect it in the common binding. The clocks that
 Rob mentioned are one example of that.
 
 If you have a binding that separates one IP block into two device nodes,
 you cannot later adapt the common driver to be more generic and handle
 all sorts of SoCs. See the usb-ehci.txt for an example: it started out
 really simple but the generic driver has been extended multiple times
 to the point where it handles a lot of SoCs by default.

The fact is that you _DO_ have *TWO* IP blocks. The synopsys DWC3 is one
IP and the wrapper is another IP which adapts the synopsys IP to the
SoC's integration context.

From the SoC point of view, the device only needs one (or maybe two)
clocks, an IRQ line, etc... The wrapper then, sometimes, enables that
particular memory region, enables IRQs and generates several other
clocks which are needed for proper operation of the IP.

Having this sort of bridge or glue driver also helps *HUGELY* in
different PM methodologies different SoCs use. I certainly don't want
any clock API calls in my dwc3 driver when I'm running on top of a PCI
bus on an intel platform. Likewise, I don't want any pci_enable_device()
calls in my dwc3 driver when I'm running on OMAP/Exynos/Qcom/etc.

Now, you guys just decided to give crap to the authors for no aparent
reason. If you really want to describe the HW then every single clock
node and every single voltage rail would have to be described but that
would just create a whole bunch of fixed clocks and fixed regulators
which have no way of being controlled whatsoever and just waste memory
due to several other instances of such drivers being needed.

It's pointless to continue discussing if we should have one clock node
or two clock nodes. If the wrapper doesn't need an interface clock, it
just doesn't need an interface clock and it's not up to us to start
*GUESSING* that it maybe has both clock inputs tied to the same clock
node if that's not what's written in the documentation. Sure, Qcom folks
could ask their HW designers, but what would that give us in practice ?
Nothing, not a single damn benefit.

So can we just move on from this pointless discussion now ?

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

Hi,

On Mon, Jun 30, 2014 at 11:03:51AM -0500, Andy Gross wrote:
 +static int dwc3_qcom_probe(struct platform_device *pdev)
 +{
 + struct device_node *node = pdev-dev.of_node;
 + struct dwc3_qcom *qdwc;
 + int ret = 0;
 +
 + qdwc = devm_kzalloc(pdev-dev, sizeof(*qdwc), GFP_KERNEL);
 + if (!qdwc)
 + return -ENOMEM;
 +
 + platform_set_drvdata(pdev, qdwc);
 +
 + qdwc-dev = pdev-dev;
 +
 + qdwc-gdsc = devm_regulator_get(qdwc-dev, gdsc);
 +
 + qdwc-core_clk = devm_clk_get(qdwc-dev, core);
 + if (IS_ERR(qdwc-core_clk)) {
 + dev_dbg(qdwc-dev, failed to get core clock\n);
 + return PTR_ERR(qdwc-core_clk);
 + }
 +
 + qdwc-iface_clk = devm_clk_get(qdwc-dev, iface);
 + if (IS_ERR(qdwc-iface_clk)) {
 + dev_dbg(qdwc-dev, failed to get iface clock, skipping\n);
 + qdwc-iface_clk = NULL;

so this clock and sleep_clk are optional, that's fine...

 + }
 +
 + qdwc-sleep_clk = devm_clk_get(qdwc-dev, sleep);
 + if (IS_ERR(qdwc-sleep_clk)) {
 + dev_dbg(qdwc-dev, failed to get sleep clock, skipping\n);
 + qdwc-sleep_clk = NULL;
 + }
 +
 + if (!IS_ERR(qdwc-gdsc)) {
 + ret = regulator_enable(qdwc-gdsc);
 + if (ret)
 + dev_err(qdwc-dev, cannot enable gdsc\n);
 + }
 +
 + clk_prepare_enable(qdwc-core_clk);
 +
 + if (qdwc-iface_clk)
 + clk_prepare_enable(qdwc-iface_clk);

... right here you can drop the NULL check because clk_prepare_enable()
is safe against NULL pointers.

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers

[Felipe Balbi]

Hi,

first of all, since this is a brand new PHY driver, could you guys use
the generic phy framework instead ? (drivers/phy)

On Mon, Jun 30, 2014 at 11:03:52AM -0500, Andy Gross wrote:
 diff --git a/drivers/usb/phy/phy-qcom-hsusb.c 
 b/drivers/usb/phy/phy-qcom-hsusb.c
 new file mode 100644
 index 000..7a44b13
 --- /dev/null
 +++ b/drivers/usb/phy/phy-qcom-hsusb.c
 @@ -0,0 +1,348 @@
 +/* Copyright (c) 2013-2014, Code Aurora Forum. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 and
 + * only version 2 as published by the Free Software Foundation.
 + *
 + * 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.
 + */
 +
 +#include linux/clk.h
 +#include linux/err.h
 +#include linux/io.h
 +#include linux/module.h
 +#include linux/of.h
 +#include linux/platform_device.h
 +#include linux/regulator/consumer.h
 +#include linux/usb/phy.h
 +
 +/**
 + *  USB QSCRATCH Hardware registers
 + */
 +#define QSCRATCH_CTRL_REG(0x04)
 +#define QSCRATCH_GENERAL_CFG (0x08)
 +#define PHY_CTRL_REG (0x10)
 +#define PARAMETER_OVERRIDE_X_REG (0x14)
 +#define CHARGING_DET_CTRL_REG(0x18)
 +#define CHARGING_DET_OUTPUT_REG  (0x1c)
 +#define ALT_INTERRUPT_EN_REG (0x20)
 +#define PHY_IRQ_STAT_REG (0x24)
 +#define CGCTL_REG(0x28)
 +
 +#define PHY_3P3_VOL_MIN  305 /* uV */
 +#define PHY_3P3_VOL_MAX  330 /* uV */
 +#define PHY_3P3_HPM_LOAD 16000   /* uA */
 +
 +#define PHY_1P8_VOL_MIN  180 /* uV */
 +#define PHY_1P8_VOL_MAX  180 /* uV */
 +#define PHY_1P8_HPM_LOAD 19000   /* uA */
 +
 +/* TODO: these are suspicious */
 +#define USB_VDDCX_NO 1   /* index */
 +#define USB_VDDCX_MIN5   /* index */
 +#define USB_VDDCX_MAX7   /* index */
 +
 +/* PHY_CTRL_REG */
 +#define HSUSB_CTRL_DMSEHV_CLAMP  BIT(24)
 +#define HSUSB_CTRL_USB2_SUSPEND  BIT(23)
 +#define HSUSB_CTRL_UTMI_CLK_EN   BIT(21)
 +#define  HSUSB_CTRL_UTMI_OTG_VBUS_VALID  BIT(20)
 +#define HSUSB_CTRL_USE_CLKCORE   BIT(18)
 +#define HSUSB_CTRL_DPSEHV_CLAMP  BIT(17)
 +#define HSUSB_CTRL_COMMONONN BIT(11)
 +#define HSUSB_CTRL_ID_HV_CLAMP   BIT(9)
 +#define HSUSB_CTRL_OTGSESSVLD_CLAMP  BIT(8)
 +#define HSUSB_CTRL_CLAMP_EN  BIT(7)
 +#define HSUSB_CTRL_RETENABLENBIT(1)
 +#define HSYSB_CTRL_POR   BIT(0)
 +
 +
 +/* QSCRATCH_GENERAL_CFG */
 +#define HSUSB_GCFG_XHCI_REV  BIT(2)
 +
 +struct qcom_dwc3_hs_phy {
 + struct usb_phy  phy;
 + void __iomem*base;
 + struct device   *dev;
 +
 + struct clk  *xo_clk;
 + struct clk  *utmi_clk;
 +
 + struct regulator*v3p3;
 + struct regulator*v1p8;
 + struct regulator*vddcx;
 + struct regulator*vbus;
 +};
 +
 +#define  phy_to_dw_phy(x)container_of((x), struct 
 qcom_dwc3_hs_phy, phy)
 +
 +/**
 + * Write register.
 + *
 + * @base - QCOM DWC3 PHY base virtual address.
 + * @offset - register offset.
 + * @val - value to write.
 + */
 +static inline void qcom_dwc3_hs_write(void __iomem *base, u32 offset, u32 
 val)
 +{
 + writel(val, base + offset);
 +}
 +
 +/**
 + * Write register and read back masked value to confirm it is written
 + *
 + * @base - QCOM DWC3 PHY base virtual address.
 + * @offset - register offset.
 + * @mask - register bitmask specifying what should be updated
 + * @val - value to write.
 + */
 +static inline void qcom_dwc3_hs_write_readback(void __iomem *base, u32 
 offset,
 + const u32 mask, u32 val)
 +{
 + u32 write_val, tmp = readl(base + offset);
 +
 + tmp = ~mask;   /* retain other bits */
 + write_val = tmp | val;
 +
 + writel(write_val, base + offset);
 +
 + /* Read back to see if val was written */
 + tmp = readl(base + offset);
 + tmp = mask;/* clear other bits */
 +
 + if (tmp != val)
 + pr_err(write: %x to QSCRATCH: %x FAILED\n, val, offset);

really nit-picking - and I'm not even sure I care - but passing a dev
pointer to use dev_err() here might be a good idea.

 +}
 +
 +static void qcom_dwc3_hs_phy_shutdown(struct usb_phy *x)
 +{
 + struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
 + int ret;
 +
 + ret = regulator_set_voltage(phy-v3p3, 0, 

Re: [RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.

[Felipe Balbi]

Hi,

On Wed, Jun 18, 2014 at 06:00:01PM +0100, Srinivas Kandagatla wrote:
 While testing usb host on Qualcomm APQ8064, I encountered few issues.
 These patches fixes those issues.
 
 Without these patches USB is not functional on AQ8064.
 All the patches are tested on IFC6410.

please resend without RFC so I can apply.

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Felipe Balbi]

On Mon, Jun 30, 2014 at 11:23:43AM +0100, Srinivas Kandagatla wrote:
 Hi Felipe,
 
 On 27/06/14 16:54, Felipe Balbi wrote:
 Hi,
 
 On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote:
 Use case is when the phy is configured in host mode and a usb device is
 attached to board before bootup. On bootup, with the existing code and
 runtime pm enabled, the driver would decrement the pm usage count
 without checking the current state of the phy. This pm usage count
 decrement would trigger the runtime pm which than would abort the
 usb enumeration which was in progress. In my case a usb stick gets
 detected and then immediatly the driver goes to low power mode which is
 not correct.
 
 log:
 [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
 [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, 
 assigned bus number 1
 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
 [1.659473] hub 1-0:1.0: USB hub found
 [1.663415] hub 1-0:1.0: 1 port detected
 ...
 [1.973352] usb 1-1: new high-speed USB device number 2 using 
 msm_hsusb_host
 [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
 [2.108993] scsi0 : usb-storage 1-1:1.0
 [2.678341] msm_otg 1252.phy: USB in low power mode
 [3.168977] usb 1-1: USB disconnect, device number 2
 
 This issue was detected on IFC6410 board.
 
 This patch fixes the intial runtime pm trigger by checking the phy
 state and decrementing the pm use count only when the phy state is IDLE.
 
 Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
 ---
   drivers/usb/phy/phy-msm-usb.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index 3bb559d..78cc870 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
 motg-chg_state = USB_CHG_STATE_UNDEFINED;
 motg-chg_type = USB_INVALID_CHARGER;
 }
 -   pm_runtime_put_sync(otg-phy-dev);
 +
 +   if (otg-phy-state == OTG_STATE_B_IDLE)
 +   pm_runtime_put_sync(otg-phy-dev);
 
 instead, you might as well just use autosuspend.
 
 autosuspend is a good idea and will provide a delay before rumtime suspend,
 however the bug which is fixed here still needs to be fixed anyway.
 
 runtime PM in this driver is not that great, the driver just increments the
 pm use count if the phy is configured in host or deivce mode. This patch
 fixes a bug in its state-machine which decrements pm use-count without
 checking the state.
 
 As this is a PHY driver, am not sure moving to autosuspend means we would
 end up just adding some static inactivity delay? Is it really going to add
 any value to this PHY driver?

yeah, perhaps not... I guess we can apply this as a fix; you're right.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v1 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs

[Felipe Balbi]

On Mon, Jun 30, 2014 at 06:29:34PM +0100, Srinivas Kandagatla wrote:
 This patch makes the msm ehci driver available to use on QCOM SOCs,
 which have the same IP.
 
 Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org

Alan, this is ok to me:

Acked-by: Felipe Balbi ba...@ti.com

 ---
  drivers/usb/host/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
 index 61b7817..03314f8 100644
 --- a/drivers/usb/host/Kconfig
 +++ b/drivers/usb/host/Kconfig
 @@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91
  
  config USB_EHCI_MSM
   tristate Support for Qualcomm QSD/MSM on-chip EHCI USB controller
 - depends on ARCH_MSM
 + depends on ARCH_MSM || ARCH_QCOM
   select USB_EHCI_ROOT_HUB_TT
   ---help---
 Enables support for the USB Host controller present on the
 -- 
 1.9.1
 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v1 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Felipe Balbi]

On Mon, Jun 30, 2014 at 06:29:57PM +0100, Srinivas Kandagatla wrote:
 Use case is when the phy is configured in host mode and a usb device is
 attached to board before bootup. On bootup, with the existing code and
 runtime pm enabled, the driver would decrement the pm usage count
 without checking the current state of the phy. This pm usage count
 decrement would trigger the runtime pm which than would abort the
 usb enumeration which was in progress. In my case a usb stick gets
 detected and then immediatly the driver goes to low power mode which is
 not correct.
 
 log:
 [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
 [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
 bus number 1
 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
 [1.659473] hub 1-0:1.0: USB hub found
 [1.663415] hub 1-0:1.0: 1 port detected
 ...
 [1.973352] usb 1-1: new high-speed USB device number 2 using 
 msm_hsusb_host
 [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
 [2.108993] scsi0 : usb-storage 1-1:1.0
 [2.678341] msm_otg 1252.phy: USB in low power mode
 [3.168977] usb 1-1: USB disconnect, device number 2
 
 This issue was detected on IFC6410 board.
 
 This patch fixes the intial runtime pm trigger by checking the phy
 state and decrementing the pm use count only when the phy state is IDLE.
 
 Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
 ---
  drivers/usb/phy/phy-msm-usb.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index 3bb559d..78cc870 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
   motg-chg_state = USB_CHG_STATE_UNDEFINED;
   motg-chg_type = USB_INVALID_CHARGER;
   }
 - pm_runtime_put_sync(otg-phy-dev);
 +
 + if (otg-phy-state == OTG_STATE_B_IDLE)
 + pm_runtime_put_sync(otg-phy-dev);
   break;
   case OTG_STATE_B_PERIPHERAL:
   dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n);

does this need to go on this -rc or can it wait until v3.17 merge
window? Also, how far back should this be backported ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Felipe Balbi]

Hi,

On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote:
 Use case is when the phy is configured in host mode and a usb device is
 attached to board before bootup. On bootup, with the existing code and
 runtime pm enabled, the driver would decrement the pm usage count
 without checking the current state of the phy. This pm usage count
 decrement would trigger the runtime pm which than would abort the
 usb enumeration which was in progress. In my case a usb stick gets
 detected and then immediatly the driver goes to low power mode which is
 not correct.
 
 log:
 [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
 [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
 bus number 1
 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
 [1.659473] hub 1-0:1.0: USB hub found
 [1.663415] hub 1-0:1.0: 1 port detected
 ...
 [1.973352] usb 1-1: new high-speed USB device number 2 using 
 msm_hsusb_host
 [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
 [2.108993] scsi0 : usb-storage 1-1:1.0
 [2.678341] msm_otg 1252.phy: USB in low power mode
 [3.168977] usb 1-1: USB disconnect, device number 2
 
 This issue was detected on IFC6410 board.
 
 This patch fixes the intial runtime pm trigger by checking the phy
 state and decrementing the pm use count only when the phy state is IDLE.
 
 Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
 ---
  drivers/usb/phy/phy-msm-usb.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index 3bb559d..78cc870 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
   motg-chg_state = USB_CHG_STATE_UNDEFINED;
   motg-chg_type = USB_INVALID_CHARGER;
   }
 - pm_runtime_put_sync(otg-phy-dev);
 +
 + if (otg-phy-state == OTG_STATE_B_IDLE)
 + pm_runtime_put_sync(otg-phy-dev);

instead, you might as well just use autosuspend.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 1/3] usb: phy: msm: cast to unsigned long int

[Felipe Balbi]

On Sat, May 03, 2014 at 09:56:32AM +0300, Ivan T. Ivanov wrote:
 On Wed, 2014-04-30 at 11:38 -0500, Felipe Balbi wrote:
  this solves the following build warning found when
  running compile tests.
  
  drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_read_dt’:
  drivers/usb/phy/phy-msm-usb.c:1459:20: warning: cast from pointer \
  to integer of different size [-Wpointer-to-int-cast]
pdata-phy_type = (int) id-data;
  ^
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
  
  all patches are on top of Ivan's 20 patch series.
  
   drivers/usb/phy/phy-msm-usb.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
  index 9dc7918..c9963c8 100644
  --- a/drivers/usb/phy/phy-msm-usb.c
  +++ b/drivers/usb/phy/phy-msm-usb.c
  @@ -1456,7 +1456,7 @@ static int msm_otg_read_dt(struct platform_device 
  *pdev, struct msm_otg *motg)
  motg-pdata = pdata;
   
  id = of_match_device(msm_otg_dt_match, pdev-dev);
  -   pdata-phy_type = (int) id-data;
  +   pdata-phy_type = (unsigned long int) id-data;
 
 Probably cast to enum msm_usb_phy_type will be better.

will do.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 1/3] usb: phy: msm: cast to unsigned long int

[Felipe Balbi]

On Mon, May 05, 2014 at 10:03:31AM -0500, Felipe Balbi wrote:
 On Sat, May 03, 2014 at 09:56:32AM +0300, Ivan T. Ivanov wrote:
  On Wed, 2014-04-30 at 11:38 -0500, Felipe Balbi wrote:
   this solves the following build warning found when
   running compile tests.
   
   drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_read_dt’:
   drivers/usb/phy/phy-msm-usb.c:1459:20: warning: cast from pointer \
 to integer of different size [-Wpointer-to-int-cast]
 pdata-phy_type = (int) id-data;
   ^
   Signed-off-by: Felipe Balbi ba...@ti.com
   ---
   
   all patches are on top of Ivan's 20 patch series.
   
drivers/usb/phy/phy-msm-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
   index 9dc7918..c9963c8 100644
   --- a/drivers/usb/phy/phy-msm-usb.c
   +++ b/drivers/usb/phy/phy-msm-usb.c
   @@ -1456,7 +1456,7 @@ static int msm_otg_read_dt(struct platform_device 
   *pdev, struct msm_otg *motg)
 motg-pdata = pdata;

 id = of_match_device(msm_otg_dt_match, pdev-dev);
   - pdata-phy_type = (int) id-data;
   + pdata-phy_type = (unsigned long int) id-data;
  
  Probably cast to enum msm_usb_phy_type will be better.
 
 will do.

commit fe8ef54c824e5b01192462ef82861e65f57d299c
Author: Felipe Balbi ba...@ti.com
Date:   Wed Apr 30 11:33:04 2014 -0500

usb: phy: msm: cast to unsigned long int

this solves the following build warning found when
running compile tests.

drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_read_dt’:
drivers/usb/phy/phy-msm-usb.c:1459:20: warning: cast from pointer \
to integer of different size [-Wpointer-to-int-cast]
  pdata-phy_type = (int) id-data;
^
Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 9dc7918..591b406 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1456,7 +1456,7 @@ static int msm_otg_read_dt(struct platform_device *pdev, 
struct msm_otg *motg)
motg-pdata = pdata;
 
id = of_match_device(msm_otg_dt_match, pdev-dev);
-   pdata-phy_type = (int) id-data;
+   pdata-phy_type = (enum msm_usb_phy_type) id-data;
 
motg-link_rst = devm_reset_control_get(pdev-dev, link);
if (IS_ERR(motg-link_rst))

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v7 16/20] usb: phy: msm: Fix PTS definitions for MSM USB controller

[Felipe Balbi]

On Fri, May 02, 2014 at 08:49:05AM +0300, Ivan T. Ivanov wrote:
 On Wed, 2014-04-30 at 11:27 -0500, Felipe Balbi wrote:
  On Thu, Apr 24, 2014 at 06:48:11PM +0300, Ivan T. Ivanov wrote:
   From: Tim Bird tbird...@gmail.com
   
   Fix the value used for Parallel Transceiver Select (PTS) for the MSM USB
   controller.  This is a standard chipidea PORTSC definition, where
   a PHY_TYPE of 10b (30) is ULPI and 11b (30) is SERIAL.
   Fix the definitions and use them correctly in the driver code.
   
   Signed-off-by: Tim Bird tim.b...@sonymobile.com
  
  since you're the one sending the patch, you must add your signed-off-by
  too. If you reply with your SoB I'll add to proper patches.
  
 
 Sure. 
 
 Signed-of-by: Ivan T. Ivanov iiva...@mm-sol.com

Thanks, I also need you to SoB the other patch from Tim.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v8 14/20] usb: phy: msm: Add support for secondary PHY control

[Felipe Balbi]

Hi,

On Mon, Apr 28, 2014 at 04:34:17PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Allow support to use 2nd HSPHY with USB2 Core.
 Some platforms may have configuration to allow USB controller
 work with any of the two HSPHYs present. By default driver
 configures USB core to use primary HSPHY. Add support to allow
 user select 2nd HSPHY using DT parameter.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 Cc: Manu Gautam mgau...@codeaurora.org
 ---
  .../devicetree/bindings/usb/msm-hsusb.txt  |  6 ++
  drivers/usb/phy/phy-msm-usb.c  | 24 
 --
  include/linux/usb/msm_hsusb.h  |  1 +
  include/linux/usb/msm_hsusb_hw.h   |  1 +
  4 files changed, 30 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/usb/msm-hsusb.txt 
 b/Documentation/devicetree/bindings/usb/msm-hsusb.txt
 index ee4123d..0669667 100644
 --- a/Documentation/devicetree/bindings/usb/msm-hsusb.txt
 +++ b/Documentation/devicetree/bindings/usb/msm-hsusb.txt
 @@ -59,6 +59,12 @@ Optional properties:
  For example: qcom,phy-init-sequence =  -1 0x63 ;
  Will update only value at address ULPI_EXT_VENDOR_SPECIFIC + 
 1.
 
 +- qcom,phy-num: Select number of pyco-phy to use, can be one of
 +0 - PHY one, default
 +1 - Second PHY
 +Some platforms may have configuration to allow USB
 +controller work with any of the two HSPHYs present.

I have a feeling there's a better to do this with of alias. But it would
be good if Kumar or any of the DT folks could pitch in.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v8 17/20] usb: phy: msm: Select secondary PHY via TCSR

[Felipe Balbi]

On Mon, Apr 28, 2014 at 04:34:20PM +0300, Ivan T. Ivanov wrote:
 From: Tim Bird tbird...@gmail.com
 
 Select the secondary PHY using the TCSR register, if phy-num=1
 in the DTS (or phy_number is set in the platform data).  The
 SOC has 2 PHYs which can be used with the OTG port, and this
 code allows configuring the correct one.
 
 Note: This resolves the problem I was seeing where I couldn't
 get the USB driver working at all on a dragonboard, from cold
 boot.  This patch depends on patch 5/14 from Ivan's msm USB
 patch set.  It does not use DT for the register address, as
 there's no evidence that this address changes between SoC
 versions.
 
 Signed-off-by: Tim Bird tim.b...@sonymobile.com
 ---
  drivers/usb/phy/phy-msm-usb.c| 14 ++
  include/linux/usb/msm_hsusb_hw.h |  3 +++
  2 files changed, 17 insertions(+)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index db8d963..9437bcf 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -1489,6 +1489,7 @@ static int msm_otg_probe(struct platform_device *pdev)
   struct resource *res;
   struct msm_otg *motg;
   struct usb_phy *phy;
 + void __iomem *phy_select;
 
   motg = devm_kzalloc(pdev-dev, sizeof(struct msm_otg), GFP_KERNEL);
   if (!motg) {
 @@ -1553,6 +1554,19 @@ static int msm_otg_probe(struct platform_device *pdev)
   if (IS_ERR(motg-regs))
   return PTR_ERR(motg-regs);
 
 + /*
 +  * NOTE: The PHYs can be multiplexed between the chipidea controller
 +  * and the dwc3 controller, using a single bit. It is important that
 +  * the dwc3 driver does not set this bit in an incompatible way.

why would dwc3 access the PHY's address space ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v7 16/20] usb: phy: msm: Fix PTS definitions for MSM USB controller

[Felipe Balbi]

On Thu, Apr 24, 2014 at 06:48:11PM +0300, Ivan T. Ivanov wrote:
 From: Tim Bird tbird...@gmail.com
 
 Fix the value used for Parallel Transceiver Select (PTS) for the MSM USB
 controller.  This is a standard chipidea PORTSC definition, where
 a PHY_TYPE of 10b (30) is ULPI and 11b (30) is SERIAL.
 Fix the definitions and use them correctly in the driver code.
 
 Signed-off-by: Tim Bird tim.b...@sonymobile.com

since you're the one sending the patch, you must add your signed-off-by
too. If you reply with your SoB I'll add to proper patches.

-- 
balbi


signature.asc
Description: Digital signature

[PATCH 2/3] usb: phy: msm: switch over to writel()

[Felipe Balbi]

Remove that single instance of writel_relaxed()
call which is only available on ARM architecture.

This will let us build test this driver on all
different architectures.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/phy/phy-msm-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index c9963c8..13b59ad 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1600,7 +1600,7 @@ static int msm_otg_probe(struct platform_device *pdev)
if (IS_ERR(phy_select))
return PTR_ERR(phy_select);
/* Enable second PHY with the OTG port */
-   writel_relaxed(0x1, phy_select);
+   writel(0x1, phy_select);
}
 
dev_info(pdev-dev, OTG regs = %p\n, motg-regs);
-- 
2.0.0.rc1

--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: msm: cast to unsigned long int

[Felipe Balbi]

this solves the following build warning found when
running compile tests.

drivers/usb/phy/phy-msm-usb.c: In function ‘msm_otg_read_dt’:
drivers/usb/phy/phy-msm-usb.c:1459:20: warning: cast from pointer \
to integer of different size [-Wpointer-to-int-cast]
  pdata-phy_type = (int) id-data;
^
Signed-off-by: Felipe Balbi ba...@ti.com
---

all patches are on top of Ivan's 20 patch series.

 drivers/usb/phy/phy-msm-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 9dc7918..c9963c8 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1456,7 +1456,7 @@ static int msm_otg_read_dt(struct platform_device *pdev, 
struct msm_otg *motg)
motg-pdata = pdata;
 
id = of_match_device(msm_otg_dt_match, pdev-dev);
-   pdata-phy_type = (int) id-data;
+   pdata-phy_type = (unsigned long int) id-data;
 
motg-link_rst = devm_reset_control_get(pdev-dev, link);
if (IS_ERR(motg-link_rst))
-- 
2.0.0.rc1

--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] usb: phy: msm: enable build on other architectures

[Felipe Balbi]

By adding COMPILE_TEST to the list of dependencies
we can build test this driver on all other architectures
which is very valuable for maintainers applying patches
and to find silly mistakes during development.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/phy/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 0c668a3..fbbced8 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -172,7 +172,7 @@ config USB_ISP1301
 
 config USB_MSM_OTG
tristate Qualcomm on-chip USB OTG controller support
-   depends on (USB || USB_GADGET)  (ARCH_MSM || ARCH_QCOM)
+   depends on (USB || USB_GADGET)  (ARCH_MSM || ARCH_QCOM || 
COMPILE_TEST)
select USB_PHY
help
  Enable this to support the USB OTG transceiver on Qualcomm chips. It
-- 
2.0.0.rc1

--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 16/20] usb: phy: msm: Fix PTS definitions for MSM USB controller

[Felipe Balbi]

Hi,

On Wed, Apr 30, 2014 at 08:07:05PM +0200, Bird, Tim wrote:
 Signed-off-by:Tim Bird tim.b...@sonymobile.com

thanks Tim but I already had your SoB in the patch, I need Ivan's :-)

 
 Sent from my Sony Xperia™ smartphone
 
  Felipe Balbi wrote 
 
 On Thu, Apr 24, 2014 at 06:48:11PM +0300, Ivan T. Ivanov wrote:
  From: Tim Bird tbird...@gmail.com
  
  Fix the value used for Parallel Transceiver Select (PTS) for the MSM USB
  controller.  This is a standard chipidea PORTSC definition, where
  a PHY_TYPE of 10b (30) is ULPI and 11b (30) is SERIAL.
  Fix the definitions and use them correctly in the driver code.
  
  Signed-off-by: Tim Bird tim.b...@sonymobile.com
 
 since you're the one sending the patch, you must add your signed-off-by
 too. If you reply with your SoB I'll add to proper patches.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v6 01/19] usb: phy: msm: Make driver selectable on ARCH_QCOM

[Felipe Balbi]

On Tue, Apr 22, 2014 at 12:20:20PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Controller could be found on APQ and MSM platforms,
 make configuration description more generic.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  drivers/usb/phy/Kconfig | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
 index 416e0c8..0c668a3 100644
 --- a/drivers/usb/phy/Kconfig
 +++ b/drivers/usb/phy/Kconfig
 @@ -171,11 +171,11 @@ config USB_ISP1301
 module will be called phy-isp1301.
 
  config USB_MSM_OTG
 - tristate OTG support for Qualcomm on-chip USB controller
 - depends on (USB || USB_GADGET)  ARCH_MSM
 + tristate Qualcomm on-chip USB OTG controller support
 + depends on (USB || USB_GADGET)  (ARCH_MSM || ARCH_QCOM)

I would actually drop USB || USB_GADGET dependency here just make it
easier to enable the driver on Kconfig, other you have to enable
USB_SUPPORT, then enable USB, go back one menu level, go down to PHY
menu, and choose this driver.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v6 03/19] usb: phy: msm: Move global regulators variables to driver state

[Felipe Balbi]

On Tue, Apr 22, 2014 at 06:12:19PM +0300, Ivan T. Ivanov wrote:
 On Tue, 2014-04-22 at 09:57 -0500, Felipe Balbi wrote:
  On Tue, Apr 22, 2014 at 12:20:22PM +0300, Ivan T. Ivanov wrote:
   From: Ivan T. Ivanov iiva...@mm-sol.com
  
  #define ENOLOG  ENOCOMMIT
  
  return -ENOLOG;
  
 
 #if SIMPLE_CHANGE
 #define ENOLOG0
 #endif
 
 Will fix it :-)

#undef ENOLOG
#define THAT_DOESNT_REALLY_WORK_FOR_ME

hehehe :-) Thanks for updating ;-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v6 01/19] usb: phy: msm: Make driver selectable on ARCH_QCOM

[Felipe Balbi]

On Tue, Apr 22, 2014 at 06:16:35PM +0300, Ivan T. Ivanov wrote:
 
 Hi, 
 
 On Tue, 2014-04-22 at 09:57 -0500, Felipe Balbi wrote:
  On Tue, Apr 22, 2014 at 12:20:20PM +0300, Ivan T. Ivanov wrote:
   From: Ivan T. Ivanov iiva...@mm-sol.com
   
   Controller could be found on APQ and MSM platforms,
   make configuration description more generic.
   
   Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
   ---
drivers/usb/phy/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
   
   diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
   index 416e0c8..0c668a3 100644
   --- a/drivers/usb/phy/Kconfig
   +++ b/drivers/usb/phy/Kconfig
   @@ -171,11 +171,11 @@ config USB_ISP1301
   module will be called phy-isp1301.
   
config USB_MSM_OTG
   - tristate OTG support for Qualcomm on-chip USB controller
   - depends on (USB || USB_GADGET)  ARCH_MSM
   + tristate Qualcomm on-chip USB OTG controller support
   + depends on (USB || USB_GADGET)  (ARCH_MSM || ARCH_QCOM)
  
  I would actually drop USB || USB_GADGET dependency here just make it
  easier to enable the driver on Kconfig, other you have to enable
  USB_SUPPORT, then enable USB, go back one menu level, go down to PHY
  menu, and choose this driver.
  
 
 Because phy directory already depends on USB_SUPPORT?

right

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb: phy: msm: Select secondary PHY via TCSR

[Felipe Balbi]

Hi,

On Fri, Apr 04, 2014 at 03:18:11PM -0700, Tim Bird wrote:
 Select the secondary PHY using the TCSR register, if phy-num=1
 in the DTS (or phy_number is set in the platform data).  The
 SOC has 2 PHYs which can be used with the OTG port, and this
 code allows configuring the correct one.
 
 Note: This resolves the problem I was seeing where I couldn't
 get the USB driver working at all on a dragonboard, from cold
 boot.  This patch depends on patch 5/14 from Ivan's msm USB
 patch set.  It does not use DT for the register address, as
 there's no evidence that this address changes between SoC
 versions.
 
 Signed-off-by: Tim Bird tim.b...@sonymobile.com

doesn't apply:

checking file drivers/usb/phy/phy-msm-usb.c
Hunk #1 succeeded at 1412 with fuzz 2 (offset -106 lines).
Hunk #2 FAILED at 1581.
1 out of 2 hunks FAILED
checking file include/linux/usb/msm_hsusb_hw.h

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block

[Felipe Balbi]

Hi,

On Tue, Feb 25, 2014 at 06:02:18PM +0100, Michal Nazarewicz wrote:
 This reworks the way SuperSpeed descriptors are added and instead of
 having a magick after full and high speed descriptors, it reworks the
 whole descriptors block to include a flags field which lists which
 descriptors are present and makes future extensions possible.
 
 Signed-off-by: Michal Nazarewicz min...@mina86.com
 ---
  drivers/usb/gadget/f_fs.c   | 136 
 +++-
  drivers/usb/gadget/u_fs.h   |  12 ++--
  include/uapi/linux/usb/functionfs.h |  49 -
  3 files changed, 94 insertions(+), 103 deletions(-)
 
 All right, with some fidling with my fan and limiting CPU frequency to
 the mininimum, I've managed to compile this patch and fixed all the
 typos.
 
 Manu, if you could include it in your series, adjust your user space
 client and test it, it would be wonderful. :]

I'll wait for that then. Note that I want to close my tree next
wednesday tops.

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v7 2/2] dmaengine: add Qualcomm BAM dma driver

[Felipe Balbi]

Hi,

On Mon, Feb 24, 2014 at 05:11:40PM -0600, Andy Gross wrote:
 diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
 index 605b016..f87cef9 100644
 --- a/drivers/dma/Kconfig
 +++ b/drivers/dma/Kconfig
 @@ -401,4 +401,13 @@ config DMATEST
  config DMA_ENGINE_RAID
   bool
  
 +config QCOM_BAM_DMA
 + tristate QCOM BAM DMA support
 + depends on ARCH_QCOM || (COMPILE_TEST  OF  ARM)

do you really want to make it depend on ARM even when COMPILE_TEST=y ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [Patch v7 2/2] dmaengine: add Qualcomm BAM dma driver

[Felipe Balbi]

On Tue, Feb 25, 2014 at 12:05:00PM +0900, Mark Brown wrote:
 On Mon, Feb 24, 2014 at 06:09:13PM -0600, Felipe Balbi wrote:
 
   + depends on ARCH_QCOM || (COMPILE_TEST  OF  ARM)
 
  do you really want to make it depend on ARM even when COMPILE_TEST=y ?
 
 writel_relaxed() is unfortunately not generally available so it'd fail
 to build on platforms like x86.

bummer

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb: phy: msm: fix possible build error

[Felipe Balbi]

Hi,

On Tue, Feb 18, 2014 at 10:28:06PM -0800, Stephen Boyd wrote:
 On 02/18, Felipe Balbi wrote:
  This will fail builds on configs where
  CONFIG_PM_RUNTIME=y and CONFIG_PM_SLEEP=n.
  
  Following build error will show up:
  
   drivers/usb/phy/phy-msm-usb.c: In function ???msm_otg_runtime_suspend???:
   drivers/usb/phy/phy-msm-usb.c:1693:2: error: implicit declaration of \
  function ???msm_otg_suspend??? [-Werror=implicit-function-declaration]
 return msm_otg_suspend(motg);
 ^
   drivers/usb/phy/phy-msm-usb.c: In function ???msm_otg_runtime_resume???:
   drivers/usb/phy/phy-msm-usb.c:1701:2: error: implicit declaration of \
  function ???msm_otg_resume??? [-Werror=implicit-function-declaration]
 return msm_otg_resume(motg);
 ^
  
  This patch fixes the error by defining msm_otg_{suspend,resume}
  whenever CONFIG_PM=y.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
 
 I'm lost. Didn't Josh send a patch for this to you already?
 
 [1] https://patchwork.kernel.org/patch/3673401/

true, dropped.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP

[Felipe Balbi]

On Fri, Jan 17, 2014 at 12:26:50PM -0600, Josh Cartwright wrote:
 On Fri, Jan 17, 2014 at 11:58:51AM -0600, Josh Cartwright wrote:
  Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
  msm_otg_{suspend,resume} routines, however these routines are only being
  built when CONFIG_PM_SLEEP.  In addition, msm_otg_{suspend,resume} also
  depends on msm_hsusb_config_vddcx(), which is only built when
  CONFIG_PM_SLEEP.
  
  Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
  preprocessor conditional, and moving msm_hsusb_config_vddcx().
  
  While we're here, eliminate the CONFIG_PM conditional for setting
  up the dev_pm_ops.
  
  This address the following errors Russell King has hit doing randconfig
  builds:
  
  drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
  drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of 
  function 'msm_otg_suspend'
  drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
  drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of 
  function 'msm_otg_resume'
  
  Cc: Ivan T. Ivanov iiva...@mm-sol.com
  Reported-by: Russell King rmk+ker...@arm.linux.org.uk
  Signed-off-by: Josh Cartwright jo...@codeaurora.org
  ---
  v1-v2: Change conditional to simply CONFIG_PM (thanks ccov and khilman!)
  
   drivers/usb/phy/phy-msm-usb.c | 57 
  ---
   1 file changed, 26 insertions(+), 31 deletions(-)
  
  diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
  index 8546c8d..5b169a7 100644
  --- a/drivers/usb/phy/phy-msm-usb.c
  +++ b/drivers/usb/phy/phy-msm-usb.c
 [..]
  @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
   #define PHY_SUSPEND_TIMEOUT_USEC   (500 * 1000)
   #define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
   
  -#ifdef CONFIG_PM_SLEEP
  +#if CONFIG_PM
 
 *sigh*.  This, of course, should have been #ifdef CONFIG_PM.  Fixed
 v3 below.

sorry, please git send-email it properly.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP

[Felipe Balbi]

On Tue, Feb 18, 2014 at 10:33:21AM -0600, Josh Cartwright wrote:
 On Tue, Feb 18, 2014 at 10:24:16AM -0600, Felipe Balbi wrote:
  On Fri, Jan 17, 2014 at 12:26:50PM -0600, Josh Cartwright wrote:
   On Fri, Jan 17, 2014 at 11:58:51AM -0600, Josh Cartwright wrote:
Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
msm_otg_{suspend,resume} routines, however these routines are only being
built when CONFIG_PM_SLEEP.  In addition, msm_otg_{suspend,resume} also
depends on msm_hsusb_config_vddcx(), which is only built when
CONFIG_PM_SLEEP.

Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
preprocessor conditional, and moving msm_hsusb_config_vddcx().

While we're here, eliminate the CONFIG_PM conditional for setting
up the dev_pm_ops.

This address the following errors Russell King has hit doing randconfig
builds:

drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of 
function 'msm_otg_suspend'
drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of 
function 'msm_otg_resume'

Cc: Ivan T. Ivanov iiva...@mm-sol.com
Reported-by: Russell King rmk+ker...@arm.linux.org.uk
Signed-off-by: Josh Cartwright jo...@codeaurora.org
---
v1-v2: Change conditional to simply CONFIG_PM (thanks ccov and 
khilman!)

 drivers/usb/phy/phy-msm-usb.c | 57 
---
 1 file changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c 
b/drivers/usb/phy/phy-msm-usb.c
index 8546c8d..5b169a7 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
   [..]
@@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
 #define PHY_SUSPEND_TIMEOUT_USEC   (500 * 1000)
 #define PHY_RESUME_TIMEOUT_USEC(100 * 1000)
 
-#ifdef CONFIG_PM_SLEEP
+#if CONFIG_PM
   
   *sigh*.  This, of course, should have been #ifdef CONFIG_PM.  Fixed
   v3 below.
  
  sorry, please git send-email it properly.
 
 No problem, will do.  FWIW, it's applicable with git am --scissors.

ahaa, I didn't know about that option. Thanks :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v5 1/6] spmi: Linux driver framework for SPMI

[Felipe Balbi]

Hi,

On Mon, Feb 03, 2014 at 05:05:33PM -0600, Josh Cartwright wrote:

[ snip ]

 +#ifdef CONFIG_PM_RUNTIME
 +static int spmi_runtime_suspend(struct device *dev)
 +{
 + struct spmi_device *sdev = to_spmi_device(dev);
 + int err;
 +
 + err = pm_generic_runtime_suspend(dev);
 + if (err)
 + return err;
 +
 + return spmi_command_sleep(sdev);

shouldn't this too calls be swapped ? I mean, some pm_runtime
implementations could be gating clocks at the driver's
-runtime_suspend() callback.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 01/15] usb: phy: msm: Move mach dependent code to platform data

[Felipe Balbi]

Hi,

On Tue, Nov 12, 2013 at 04:51:36PM +0200, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 This patch fix compilation error when driver is compiled
 in multi-platform builds.
 
 drivers/built-in.o: In function `msm_otg_link_clk_reset':
 ./drivers/usb/phy/phy-msm-usb.c:314: undefined reference to `clk_reset'
 ./drivers/usb/phy/phy-msm-usb.c:318: undefined reference to `clk_reset'
 
 Use platform data supplied reset handlers and adjust error
 messages reported when reset sequence fail.
 
 This is an intermediate step before adding support for reset
 framework and newer targets.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 Acked-by: David Brown dav...@codeaurora.org
 Cc: Daniel Walker dwal...@fifo99.com
 Cc: Felipe Balbi ba...@ti.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org

this really looks like you should be using reset framework
(drivers/reset/), then your phy driver would simply reset_assert() and
reset_deassert().

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 01/15] usb: phy: msm: Move mach dependent code to platform data

[Felipe Balbi]

On Fri, Dec 27, 2013 at 10:23:10AM -0800, Stephen Boyd wrote:
 On 12/27/13 10:10, Felipe Balbi wrote:
  Hi,
 
  On Tue, Nov 12, 2013 at 04:51:36PM +0200, Ivan T. Ivanov wrote:
  From: Ivan T. Ivanov iiva...@mm-sol.com
 
  This patch fix compilation error when driver is compiled
  in multi-platform builds.
 
  drivers/built-in.o: In function `msm_otg_link_clk_reset':
  ./drivers/usb/phy/phy-msm-usb.c:314: undefined reference to `clk_reset'
  ./drivers/usb/phy/phy-msm-usb.c:318: undefined reference to `clk_reset'
 
  Use platform data supplied reset handlers and adjust error
  messages reported when reset sequence fail.
 
  This is an intermediate step before adding support for reset
  framework and newer targets.
 
  Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
  Acked-by: David Brown dav...@codeaurora.org
  Cc: Daniel Walker dwal...@fifo99.com
  Cc: Felipe Balbi ba...@ti.com
  Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
  this really looks like you should be using reset framework
  (drivers/reset/), then your phy driver would simply reset_assert() and
  reset_deassert().
 
 
 Unfortunately the reset framework is a DT only framework and there are
 still non-DT platforms within mach-msm. Arnd suggested we push the
 non-DT reset code down into the mach directory in the meantime. We're in
 the process of adding the reset framework to DT enabled MSM platforms,
 hopefully those get merged in 3.14.

And this is why the ARM port is in such a messy situation. It's always
better to push things into the mach- directory than improving existing
frameworks to cope with wild ARM SoCs.

fell free to push this through your tree. It _does_ make the PHY driver
slightly better and probably buildable on other arches with
COMPILE_TEST.

Still, I *really* want to see this switching over to reset framework on
v3.16.

cheers

ps: for this patch only you can have my

Acked-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Felipe Balbi]

On Mon, Dec 23, 2013 at 05:18:42PM +0530, Manu Gautam wrote:
 Allow userspace to pass SuperSpeed descriptors and
 handle them in the driver accordingly.
 This change doesn't modify existing desc_header and thereby
 keeps the ABI changes backward compatible i.e. existing
 userspace drivers compiled with old header (functionfs.h)
 would continue to work with the updated kernel.
 
 Change-Id: Ic27035fdef2a83828024348d75be1518e9f8c5c6

when sending patches upstream, please remove gerrit garbage from commit
log ;-)

I'll wait for Michal to review this one.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 00/15] usb: phy: msm: Fixes, cleanups and DT support

[Felipe Balbi]

On Thu, Dec 19, 2013 at 03:03:15PM -0800, David Brown wrote:
 On Tue, Nov 26, 2013 at 10:38:46AM -0600, Felipe Balbi wrote:
 Hi,
 
 On Tue, Nov 12, 2013 at 04:51:35PM +0200, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Hi,
 
 Patches have been tested on top of Stephen's clock controller
 patches[1] and recent fixes for chipidea msm glue layer driver
 posted here[2]. Hardware platform AP8074 DragonBoard. Only gadget
 mode utilized for now.
 
 CV Test Suite engine Chapter 9 tests are passing except
 Halt Endpoint Test.
 
 usbtest driver report following failure:
 test 13 -- 32 (Broken pipe)ep 81 couldn't set halt, -32
 
 This will be investigated further.
 Patches could be applied cleanly on top of the usb-3.13-rc1.
 
 Regards,
 Ivan
 
 [1] http://comments.gmane.org/gmane.linux.ports.arm.msm/5375
 [2] https://lkml.org/lkml/2013/11/11/310
 
 I wonder how will this be taken upstream provided there are
 interdependencies between arch/ and drivers/ patches.
 
 I can Ack patches but I'd really like to avoid merging of topic branches
 all over the place :-s
 
 Ivan's patches are the only ones in this window that touch these board
 files in MSM.  It's probably easiest if I ack them, and you can bring
 the whole series through your tree.

it's getting quite late for me. I still want to leave my stuff soaking
in linux-next for a while. I'll try my best, though, if you ack it ASAP
;-)

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 00/15] usb: phy: msm: Fixes, cleanups and DT support

[Felipe Balbi]

Hi,

On Tue, Nov 12, 2013 at 04:51:35PM +0200, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Hi, 
 
 Patches have been tested on top of Stephen's clock controller
 patches[1] and recent fixes for chipidea msm glue layer driver
 posted here[2]. Hardware platform AP8074 DragonBoard. Only gadget
 mode utilized for now. 
 
 CV Test Suite engine Chapter 9 tests are passing except
 Halt Endpoint Test.
 
 usbtest driver report following failure:
 test 13 -- 32 (Broken pipe)  ep 81 couldn't set halt, -32
 
 This will be investigated further. 
 Patches could be applied cleanly on top of the usb-3.13-rc1.
 
 Regards,
 Ivan
 
 [1] http://comments.gmane.org/gmane.linux.ports.arm.msm/5375
 [2] https://lkml.org/lkml/2013/11/11/310

I wonder how will this be taken upstream provided there are
interdependencies between arch/ and drivers/ patches.

I can Ack patches but I'd really like to avoid merging of topic branches
all over the place :-s

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Felipe Balbi]

Hi,

On Tue, Oct 08, 2013 at 09:52:55AM +0530, Manu Gautam wrote:
 On 10/2/2013 10:06 AM, Manu Gautam wrote:
  On 10/1/2013 8:07 PM, Felipe Balbi wrote:
  Hi,
 
  On Mon, Sep 30, 2013 at 02:31:50PM +0530, Manu Gautam wrote:
  On 9/28/2013 1:52 AM, Paul Zimmerman wrote:
  From: linux-usb-ow...@vger.kernel.org 
  [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Manu Gautam
  Sent: Thursday, September 26, 2013 12:08 AM
 
  On 9/26/2013 2:10 AM, Felipe Balbi wrote:
 
  On Tue, Sep 24, 2013 at 03:00:20PM +0530, Manu Gautam wrote:
  Hi Felipe,
 
  I wanted to mention one point with respect to this patch: Below
  changes in the functionfs.h to add ss_count (super speed descriptors
  count) in desc_header (which is passed from userspace) make the driver
  incompatible with existing userspace applications compiled against old
  header file. Let me know if that is acceptable.  We are using this
  driver with Android for adbd (android debug bridge) and these changes
  are required to support adb over Super Speed controllers e.g. DWC3
  along with changed in adbd to pass SS EP and companion descriptors.
 
  Good you mentioned, it saves me the trouble of reviewing this patch :-)
 
  It's not acceptable to break userspace ABI at all. If you want
  SuperSpeed support on function fs, we need to figure out a way to do so
  without breaking userspace.
 
  This might mean adding a separate userspace interface to be used with
  superspeed. While at that, we might want to add a few bytes of 
  reserved,
  unused space in our structures for situations where we need to add more
  data into it, just to make it slightly future proof.
 
 
  Thanks for your reply.
  As you suggested we can have a different interface for super speed
  which would be optional to workaround ABI compatibility issue.
  Let me know if below interface looks fine to you, I will then implement
  accordingly:
 
  Just a suggestion: Instead of a new interface for SuperSpeed, why not
  just add the new fields to the end of the ffs_data struct? And have the
  functions that copy the struct to/from userspace check the 'len' value
  passed in, and only handle the SuperSpeed stuff if the length indicates
  it is new userspace?
 
 
  Initially I thought on similar lines but then adding a new interface for
  SS looked cleaner to me. But, your suggestion also make sense as we can
  avoid extra system call and the same interface can be extended later.
  One more thing we can do is to add a magic number after hs_desc (i.e. at
  the end of existing ffs_data) to specify that ss_descriptors are 
  following.
  This can be used by kernel driver to check if userspace is trying pass
  ss desc only or some invalid data. 
  Felipe: Your recommendation on this?
 
  We need to have some more people look at this. I remember there were
  always some concerns about Chris architecture when doing such changes.
 
  
  Can you please add appropriate folks to this thread who can check this as
  well?
  
 
 I have CC'ed  Michal and Andrzej as they have contributed to this driver.
 Followed is the interface enhancement that I am suggesting:
 
 diff --git a/include/uapi/linux/usb/functionfs.h 
 b/include/uapi/linux/usb/functionfs.h
 index d6b0128..0f8f7be 100644
 --- a/include/uapi/linux/usb/functionfs.h
 +++ b/include/uapi/linux/usb/functionfs.h
 @@ -13,6 +13,7 @@ enum {
 FUNCTIONFS_STRINGS_MAGIC = 2
  };
 
 +#define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C
 
  #ifndef __KERNEL__
 
 @@ -50,7 +51,11 @@ struct usb_functionfs_descs_head {
   * |  12 | hs_count  | LE32 | number of high-speed descriptors |
   * |  16 | fs_descrs | Descriptor[] | list of full-speed descriptors   |
   * | | hs_descrs | Descriptor[] | list of high-speed descriptors   |
 + * | | ss_magic  | LE32 | FUNCTIONFS_SS_DESC_MAGIC |
 + * | | ss_count  | LE32 | number of super-speed descriptors|
 + * | | ss_descrs | Descriptor[] | list of super-speed descriptors  |
   *
 + * ss_magic: if present then it implies that SS_DESCs are also present
   * descs are just valid USB descriptors and have the following format:
   *
   * | off | name| type | description  |

any comments here ? nobody ? Manu, if nobody complains in another week,
please send this hunk as a formal patch.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v3 01/13] usb: phy: msm: Move mach depndend code to platform data

[Felipe Balbi]

Hi,

On Mon, Oct 14, 2013 at 06:24:28PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 This patch fix compilation error and is an intermediate step
 before the addition of DeviceTree support for newer targets.
 
 Cc: David Brown dav...@codeaurora.org
 Cc: Daniel Walker dwal...@fifo99.com
 Cc: Felipe Balbi ba...@ti.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  arch/arm/mach-msm/board-msm7x30.c |   35 +++
  arch/arm/mach-msm/board-qsd8x50.c |   35 +++
  drivers/usb/phy/phy-msm-usb.c |   34 ++
  include/linux/usb/msm_hsusb.h |3 +++
  4 files changed, 87 insertions(+), 20 deletions(-)
 
 diff --git a/arch/arm/mach-msm/board-msm7x30.c 
 b/arch/arm/mach-msm/board-msm7x30.c
 index f9af5a4..46de789 100644
 --- a/arch/arm/mach-msm/board-msm7x30.c
 +++ b/arch/arm/mach-msm/board-msm7x30.c
 @@ -30,6 +30,7 @@
  #include asm/memory.h
  #include asm/setup.h
  
 +#include mach/clk.h
  #include mach/msm_iomap.h
  #include mach/dma.h
  
 @@ -60,10 +61,44 @@ static int hsusb_phy_init_seq[] = {
   -1
  };
  
 +static int hsusb_link_clk_reset(struct clk *link_clk, bool assert)
 +{
 + int ret;
 +
 + if (assert) {
 + ret = clk_reset(link_clk, CLK_RESET_ASSERT);
 + if (ret)
 + pr_err(usb hs_clk assert failed\n);
 + } else {
 + ret = clk_reset(link_clk, CLK_RESET_DEASSERT);
 + if (ret)
 + pr_err(usb hs_clk deassert failed\n);
 + }
 + return ret;
 +}
 +
 +static int hsusb_phy_clk_reset(struct clk *phy_clk)
 +{
 + int ret;
 +
 + ret = clk_reset(phy_clk, CLK_RESET_ASSERT);
 + if (ret) {
 + pr_err(usb phy clk assert failed\n);
 + return ret;
 + }
 + usleep_range(1, 12000);
 + ret = clk_reset(phy_clk, CLK_RESET_DEASSERT);
 + if (ret)
 + pr_err(usb phy clk deassert failed\n);
 + return ret;
 +}
 +
  static struct msm_otg_platform_data msm_otg_pdata = {
   .phy_init_seq   = hsusb_phy_init_seq,
   .mode   = USB_PERIPHERAL,
   .otg_control= OTG_PHY_CONTROL,
 + .link_clk_reset = hsusb_link_clk_reset,
 + .phy_clk_reset  = hsusb_phy_clk_reset,
  };
  
  struct msm_gpiomux_config msm_gpiomux_configs[GPIOMUX_NGPIOS] = {
 diff --git a/arch/arm/mach-msm/board-qsd8x50.c 
 b/arch/arm/mach-msm/board-qsd8x50.c
 index 5f933bc..9169ec3 100644
 --- a/arch/arm/mach-msm/board-qsd8x50.c
 +++ b/arch/arm/mach-msm/board-qsd8x50.c
 @@ -31,6 +31,7 @@
  #include mach/irqs.h
  #include mach/sirc.h
  #include mach/vreg.h
 +#include mach/clk.h
  #include linux/platform_data/mmc-msm_sdcc.h
  
  #include devices.h
 @@ -81,10 +82,44 @@ static int hsusb_phy_init_seq[] = {
   -1
  };
  
 +static int hsusb_link_clk_reset(struct clk *link_clk, bool assert)
 +{
 + int ret;
 +
 + if (assert) {
 + ret = clk_reset(link_clk, CLK_RESET_ASSERT);
 + if (ret)
 + pr_err(usb hs_clk assert failed\n);
 + } else {
 + ret = clk_reset(link_clk, CLK_RESET_DEASSERT);
 + if (ret)
 + pr_err(usb hs_clk deassert failed\n);
 + }
 + return ret;
 +}
 +
 +static int hsusb_phy_clk_reset(struct clk *phy_clk)
 +{
 + int ret;
 +
 + ret = clk_reset(phy_clk, CLK_RESET_ASSERT);
 + if (ret) {
 + pr_err(usb phy clk assert failed\n);
 + return ret;
 + }
 + usleep_range(1, 12000);
 + ret = clk_reset(phy_clk, CLK_RESET_DEASSERT);
 + if (ret)
 + pr_err(usb phy clk deassert failed\n);
 + return ret;
 +}
 +
  static struct msm_otg_platform_data msm_otg_pdata = {
   .phy_init_seq   = hsusb_phy_init_seq,
   .mode   = USB_PERIPHERAL,
   .otg_control= OTG_PHY_CONTROL,
 + .link_clk_reset = hsusb_link_clk_reset,
 + .phy_clk_reset  = hsusb_phy_clk_reset,
  };
  
  static struct platform_device *devices[] __initdata = {
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index e9d4cd9..118798d 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -40,8 +40,6 @@
  #include linux/usb/msm_hsusb_hw.h
  #include linux/regulator/consumer.h
  
 -#include mach/clk.h
 -
  #define MSM_USB_BASE (motg-regs)
  #define DRIVER_NAME  msm_otg
  
 @@ -308,33 +306,29 @@ static void ulpi_init(struct msm_otg *motg)
  
  static int msm_otg_link_clk_reset(struct msm_otg *motg, bool assert)
  {
 - int ret;
 + int ret = 0;
 +
 + if (!motg-pdata-link_clk_reset)
 + return ret;
 +
 + ret = motg-pdata-link_clk_reset(motg-clk, assert);
 + if (ret)
 + dev_err(motg-phy.dev, usb link clk reset failed\n);
  
 - if (assert) {
 - ret

Re: [PATCH v3 02/13] usb: phy: msm: Move global regulators variables to driver state

[Felipe Balbi]

On Mon, Oct 14, 2013 at 06:24:29PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

this one looks good.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v3 12/13] usb: phy: msm: Properly check core interrupt number

[Felipe Balbi]

On Mon, Oct 14, 2013 at 06:24:39PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 IRQ with number 0 is valid case, so check for negative

not entirelly correct... IRQ 0 isn't supposed to be used as a linux IRQ
number IIRC.

 numbers instead.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  drivers/usb/phy/phy-msm-usb.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index ca2abe6..f34c8a9 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -1415,7 +1415,7 @@ static int __init msm_otg_probe(struct platform_device 
 *pdev)
   dev_info(pdev-dev, OTG regs = %p\n, motg-regs);
  
   motg-irq = platform_get_irq(pdev, 0);
 - if (!motg-irq) {
 + if (motg-irq  0) {

this check is correct though, since platform_get_irq() will return
-ENXIO if it doesn't find IRQ resource.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v5 1/3] usb: dwc3: msm: Add device tree binding information

[Felipe Balbi]

On Wed, Aug 21, 2013 at 04:29:44PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
 (SNPS) and HS, SS PHY's control and configuration registers.
 
 It could operate in device mode (SS, HS, FS) and host
 mode (SS, HS, FS, LS).
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

these patches were sent *ages* and nobody from DT mailing list has
reviewed. Unless someone steps up now, I'll be queueing these patches
next week.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Felipe Balbi]

Hi,

On Mon, Sep 30, 2013 at 02:31:50PM +0530, Manu Gautam wrote:
 On 9/28/2013 1:52 AM, Paul Zimmerman wrote:
  From: linux-usb-ow...@vger.kernel.org 
  [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Manu Gautam
  Sent: Thursday, September 26, 2013 12:08 AM
 
  On 9/26/2013 2:10 AM, Felipe Balbi wrote:
 
  On Tue, Sep 24, 2013 at 03:00:20PM +0530, Manu Gautam wrote:
  Hi Felipe,
 
  I wanted to mention one point with respect to this patch: Below
  changes in the functionfs.h to add ss_count (super speed descriptors
  count) in desc_header (which is passed from userspace) make the driver
  incompatible with existing userspace applications compiled against old
  header file. Let me know if that is acceptable.  We are using this
  driver with Android for adbd (android debug bridge) and these changes
  are required to support adb over Super Speed controllers e.g. DWC3
  along with changed in adbd to pass SS EP and companion descriptors.
 
  Good you mentioned, it saves me the trouble of reviewing this patch :-)
 
  It's not acceptable to break userspace ABI at all. If you want
  SuperSpeed support on function fs, we need to figure out a way to do so
  without breaking userspace.
 
  This might mean adding a separate userspace interface to be used with
  superspeed. While at that, we might want to add a few bytes of reserved,
  unused space in our structures for situations where we need to add more
  data into it, just to make it slightly future proof.
 
 
  Thanks for your reply.
  As you suggested we can have a different interface for super speed
  which would be optional to workaround ABI compatibility issue.
  Let me know if below interface looks fine to you, I will then implement
  accordingly:
  
  Just a suggestion: Instead of a new interface for SuperSpeed, why not
  just add the new fields to the end of the ffs_data struct? And have the
  functions that copy the struct to/from userspace check the 'len' value
  passed in, and only handle the SuperSpeed stuff if the length indicates
  it is new userspace?
  
 
 Initially I thought on similar lines but then adding a new interface for
 SS looked cleaner to me. But, your suggestion also make sense as we can
 avoid extra system call and the same interface can be extended later.
 One more thing we can do is to add a magic number after hs_desc (i.e. at
 the end of existing ffs_data) to specify that ss_descriptors are following.
 This can be used by kernel driver to check if userspace is trying pass
 ss desc only or some invalid data. 
 Felipe: Your recommendation on this?

We need to have some more people look at this. I remember there were
always some concerns about Chris architecture when doing such changes.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Felipe Balbi]

Hi,

(please avoid top-posting)

On Tue, Sep 24, 2013 at 03:00:20PM +0530, Manu Gautam wrote:
 Hi Felipe,
 
 I wanted to mention one point with respect to this patch: Below
 changes in the functionfs.h to add ss_count (super speed descriptors
 count) in desc_header (which is passed from userspace) make the driver
 incompatible with existing userspace applications compiled against old
 header file. Let me know if that is acceptable.  We are using this
 driver with Android for adbd (android debug bridge) and these changes
 are required to support adb over Super Speed controllers e.g. DWC3
 along with changed in adbd to pass SS EP and companion descriptors.

Good you mentioned, it saves me the trouble of reviewing this patch :-)

It's not acceptable to break userspace ABI at all. If you want
SuperSpeed support on function fs, we need to figure out a way to do so
without breaking userspace.

This might mean adding a separate userspace interface to be used with
superspeed. While at that, we might want to add a few bytes of reserved,
unused space in our structures for situations where we need to add more
data into it, just to make it slightly future proof.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 1/3] usb: dwc3: msm: Add device tree binding information

[Felipe Balbi]

Hi,

On Tue, Aug 20, 2013 at 12:56:03PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
 (SNPS) and HS, SS PHY's control and configuration registers.
 
 It could operate in device mode (SS, HS, FS) and host
 mode (SS, HS, FS, LS).
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

Any acks for the DT part ? This patch has been pending forever.

 ---
  .../devicetree/bindings/usb/msm-ssusb.txt  |  104 
 
  1 file changed, 104 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt
 
 diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt 
 b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
 new file mode 100644
 index 000..cacbd3b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
 @@ -0,0 +1,104 @@
 +MSM SuperSpeed DWC3 USB SoC controller
 +
 +
 +DWC3 Highspeed USB PHY
 +==
 +Required properities :
 +- compatible : sould be qcom,dwc3-hsphy;
 +- reg : offset and length of the register set in the memory map
 +- clocks : phandles to clock instances of the device tree nodes
 +- clock-names :
 + xo : External reference clock 19 MHz
 + sleep_a : Sleep clock, used when USB3 core goes into low
 + power mode (U3).
 +supply-name-supply : phandle to the regulator device tree node
 +Required supply-name are:
 + v1p8 : 1.8v supply for HSPHY
 + v3p3 : 3.3v supply for HSPHY
 + vbus : vbus supply for host mode
 + vddcx : vdd supply for HS-PHY digital circuit operation
 +
 +DWC3 Superspeed USB PHY
 +===
 +Required properities :
 +- compatible : sould be qcom,dwc3-ssphy;
 +- reg : offset and length of the register set in the memory map
 +- clocks : phandles to clock instances of the device tree nodes
 +- clock-names :
 + xo : External reference clock 19 MHz
 + ref : Reference clock - used in host mode.
 +supply-name-supply : phandle to the regulator device tree node
 +Required supply-name are:
 + v1p8 : 1.8v supply for SS-PHY
 + vddcx : vdd supply for SS-PHY digital circuit operation
 +
 +DWC3 controller wrapper
 +===
 +Required properties :
 +- compatible : should be qcom,dwc3
 +- reg : offset and length of the register set in the memory map
 + offset and length of the TCSR register for routing USB
 + signals to either picoPHY0 or picoPHY1.
 +- clocks : phandles to clock instances of the device tree nodes
 +- clock-names :
 + core : Master/Core clock, have to be = 125 MHz for SS
 + operation and = 60MHz for HS operation
 + iface : System bus AXI clock
 + sleep : Sleep clock, used when USB3 core goes into low
 + power mode (U3).
 + utmi : Generated by HS-PHY. Used to clock the low power
 + parts of thr HS Link layer.
 +Optional properties :
 +- gdsc-supply : phandle to the globally distributed switch controller
 +  regulator node to the USB controller.
 +Required child node:
 +A child node must exist to represent the core DWC3 IP block. The name of
 +the node is not important. The content of the node is defined in dwc3.txt.
 +
 +Example device nodes:
 +
 + dwc3_hsphy: phy@f92f8800 {
 + compatible = qcom,dwc3-hsphy;
 + reg = 0xf92f8800 0x30;
 +
 + clocks = cxo, usb2a_phy_sleep_cxc;
 + clock-names = xo, sleep_a;
 +
 + vbus-supply = supply;
 + vddcx-supply = supply;
 + v1p8-supply = supply;
 + v3p3-supply = supply;
 + };
 +
 + dwc3_ssphy: phy@f92f8830 {
 + compatible = qcom,dwc3-ssphy;
 + reg = 0xf92f8830 0x30;
 +
 + clocks = cxo, usb30_mock_utmi_cxc;
 + clock-names = xo, ref;
 +
 + vddcx-supply = supply;
 + v1p8-supply = supply;
 + };
 +
 + usb@fd4ab000 {
 + compatible = qcom,dwc3;
 + #address-cells = 1;
 + #size-cells = 1;
 + reg = 0xfd4ab000 0x4;
 +
 + clocks = usb30_master_cxc, sys_noc_usb3_axi_cxc,
 + usb30_sleep_cxc, usb30_mock_utmi_cxc;
 + clock-names = core, iface, sleep, utmi;
 +
 + gdsc-supply = supply;
 +
 + ranges;
 + dwc3@f920 {
 + compatible = snps,dwc3;
 + reg = 0xf920 0xcd00;
 + interrupts = 0 131 0;
 + usb-phy = dwc3_hsphy, dwc3_ssphy;
 + tx-fifo-resize;
 + };
 + };
 -- 
 1.7.9.5
 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v5 1/3] usb: dwc3: msm: Add device tree binding information

[Felipe Balbi]

Hi,

On Wed, Aug 21, 2013 at 04:29:44PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
 (SNPS) and HS, SS PHY's control and configuration registers.
 
 It could operate in device mode (SS, HS, FS) and host
 mode (SS, HS, FS, LS).
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

and here's a new version from same patch

 ---
  .../devicetree/bindings/usb/msm-ssusb.txt  |  104 
 
  1 file changed, 104 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/usb/msm-ssusb.txt
 
 diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt 
 b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
 new file mode 100644
 index 000..f57ba8d
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/usb/msm-ssusb.txt
 @@ -0,0 +1,104 @@
 +MSM SuperSpeed DWC3 USB SoC controller
 +
 +
 +MSM DW Highspeed USB PHY
 +
 +Required properities :
 +- compatible : sould be qcom,dw-hsphy;
 +- reg : offset and length of the register set in the memory map
 +- clocks : phandles to clock instances of the device tree nodes
 +- clock-names :
 + xo : External reference clock 19 MHz
 + sleep_a : Sleep clock, used when USB3 core goes into low
 + power mode (U3).
 +supply-name-supply : phandle to the regulator device tree node
 +Required supply-name are:
 + v1p8 : 1.8v supply for HSPHY
 + v3p3 : 3.3v supply for HSPHY
 + vbus : vbus supply for host mode
 + vddcx : vdd supply for HS-PHY digital circuit operation
 +
 +MSM DW Superspeed USB PHY
 +=
 +Required properities :
 +- compatible : sould be qcom,dw-ssphy;
 +- reg : offset and length of the register set in the memory map
 +- clocks : phandles to clock instances of the device tree nodes
 +- clock-names :
 + xo : External reference clock 19 MHz
 + ref : Reference clock - used in host mode.
 +supply-name-supply : phandle to the regulator device tree node
 +Required supply-name are:
 + v1p8 : 1.8v supply for SS-PHY
 + vddcx : vdd supply for SS-PHY digital circuit operation
 +
 +MSM DWC3 controller wrapper
 +===
 +Required properties :
 +- compatible : should be qcom,dwc3
 +- reg : offset and length of the register set in the memory map
 + offset and length of the TCSR register for routing USB
 + signals to either picoPHY0 or picoPHY1.
 +- clocks : phandles to clock instances of the device tree nodes
 +- clock-names :
 + core : Master/Core clock, have to be = 125 MHz for SS
 + operation and = 60MHz for HS operation
 + iface : System bus AXI clock
 + sleep : Sleep clock, used when USB3 core goes into low
 + power mode (U3).
 + utmi : Generated by HS-PHY. Used to clock the low power
 + parts of thr HS Link layer.
 +Optional properties :
 +- gdsc-supply : phandle to the globally distributed switch controller
 +  regulator node to the USB controller.
 +Required child node:
 +A child node must exist to represent the core DWC3 IP block. The name of
 +the node is not important. The content of the node is defined in dwc3.txt.
 +
 +Example device nodes:
 +
 + dw_hsphy: phy@f92f8800 {
 + compatible = qcom,dw-hsphy;
 + reg = 0xf92f8800 0x30;
 +
 + clocks = cxo, usb2a_phy_sleep_cxc;
 + clock-names = xo, sleep_a;
 +
 + vbus-supply = supply;
 + vddcx-supply = supply;
 + v1p8-supply = supply;
 + v3p3-supply = supply;
 + };
 +
 + dw_ssphy: phy@f92f8830 {
 + compatible = qcom,dw-ssphy;
 + reg = 0xf92f8830 0x30;
 +
 + clocks = cxo, usb30_mock_utmi_cxc;
 + clock-names = xo, ref;
 +
 + vddcx-supply = supply;
 + v1p8-supply = supply;
 + };
 +
 + usb@fd4ab000 {
 + compatible = qcom,dwc3;
 + #address-cells = 1;
 + #size-cells = 1;
 + reg = 0xfd4ab000 0x4;
 +
 + clocks = usb30_master_cxc, sys_noc_usb3_axi_cxc,
 + usb30_sleep_cxc, usb30_mock_utmi_cxc;
 + clock-names = core, iface, sleep, utmi;
 +
 + gdsc-supply = supply;
 +
 + ranges;
 + dwc3@f920 {
 + compatible = snps,dwc3;
 + reg = 0xf920 0xcd00;
 + interrupts = 0 131 0;
 + usb-phy = dw_hsphy, dw_ssphy;
 + tx-fifo-resize;
 + };
 + };
 -- 
 1.7.9.5
 

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v5 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DW PHY's

[Felipe Balbi]

Hi,

On Wed, Aug 21, 2013 at 04:29:45PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 These drivers handles control and configuration of the HS
 and SS USB PHY transceivers. They are part of the driver
 which manage Synopsys DesignWare USB3 controller stack
 inside Qualcomm SoC's.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

I can take this one if DT folks agree with bindings proposed on previous
patch.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core

[Felipe Balbi]

Hi,

On Thu, Aug 22, 2013 at 09:24:49PM +, Paul Zimmerman wrote:
  From: Ivan T. Ivanov [mailto:iiva...@mm-sol.com]
  Sent: Tuesday, August 20, 2013 8:26 AM
  
  On Tue, 2013-08-20 at 10:01 -0500, Kumar Gala wrote:
   On Aug 20, 2013, at 9:54 AM, Ivan T. Ivanov wrote:
  
   
Hi,
   
On Tue, 2013-08-20 at 09:33 -0500, Felipe Balbi wrote:
On Tue, Aug 20, 2013 at 05:09:11PM +0300, Ivan T. Ivanov wrote:
   
On Tue, 2013-08-20 at 08:37 -0500, Felipe Balbi wrote:
   
On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote:
   
I think they are SNPS DesignWare PHY's, additionally
wrapped with Qualcomm logic. I could substitute dwc3
with just dw, which will be more correct.
   
alright, thank you. Let's add Paul to the loop since he might have 
very
good insight in the synopsys PHYs.
   
mental note: if any other platform shows up with Synopsys PHY, ask 
them
to use this driver instead :-)
   
I really doubt that this will bi possible. Control of the PHY's is
not directly trough ULPI, UTMI or PIPE3 interfaces, but trough
QSCRATCH registers, which of course is highly Qualcomm specific.
   
isn't it a memory mapped IP ? doesn't synopsys provide their own set of
registers ?
   
From what I see it is not directly mapped. How QSCRATCH write and
reads transactions are translated to DW IP is unclear to me.
  
  
   I think the question is how does SW access them?
  
  USB QSCRATCH Hardware registers don't ask me what is this :-)
  or like Pawel says: it depends on the SOC .
 
 To answer the question doesn't synopsys provide their own set of
 registers, we provide registers in our USB cores to access the PHYs
 through I2C, ULPI/UTMI, or PIPE3 interfaces. But if someone wants to use
 our PHY with some other controller that doesn't provide that, then they
 may need to implement their own register set, as Qualcomm has apparently
 done.

thanks for clarifying. that pretty much hinders writing any sort of
generic drivers for Synopsys' PHYs though :-s

But I guess that's alright.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core

[Felipe Balbi]

On Wed, Aug 21, 2013 at 10:13:28AM -0500, Kumar Gala wrote:
 
 On Aug 21, 2013, at 8:06 AM, Ivan T. Ivanov wrote:
 
  On Tue, 2013-08-20 at 18:01 +0100, Pawel Moll wrote: 
  On Tue, 2013-08-20 at 16:06 +0100, Pawel Moll wrote:
  On Tue, 2013-08-20 at 16:01 +0100, Kumar Gala wrote:
  On Aug 20, 2013, at 9:54 AM, Ivan T. Ivanov wrote:
  
  
  Hi, 
  
  On Tue, 2013-08-20 at 09:33 -0500, Felipe Balbi wrote: 
  On Tue, Aug 20, 2013 at 05:09:11PM +0300, Ivan T. Ivanov wrote:
  Hi,
  
  On Tue, 2013-08-20 at 08:37 -0500, Felipe Balbi wrote: 
  Hi,
  
  On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote:
  On Tue, Aug 20, 2013 at 12:56:04PM +0300, Ivan T. Ivanov wrote:
  From: Ivan T. Ivanov iiva...@mm-sol.com
  
  These drivers handles control and configuration of the HS
  and SS USB PHY transceivers. They are part of the driver
  which manage Synopsys DesignWare USB3 controller stack
  inside Qualcomm SoC's.
  
  Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
  ---
  drivers/usb/phy/Kconfig   |   11 ++
  drivers/usb/phy/Makefile  |2 +
  drivers/usb/phy/phy-msm-dwc3-hs.c |  327 
  
  drivers/usb/phy/phy-msm-dwc3-ss.c |  374 
  +
  
  please rename these PHY drivers, they have nothing to do with 
  DWC3. PHYs
  don't care about the USB controller.
  
  I think they are SNPS DesignWare PHY's, additionally
  wrapped with Qualcomm logic. I could substitute dwc3
  with just dw, which will be more correct.
  
  alright, thank you. Let's add Paul to the loop since he might have 
  very
  good insight in the synopsys PHYs.
  
  mental note: if any other platform shows up with Synopsys PHY, ask 
  them
  to use this driver instead :-)
  
  I really doubt that this will bi possible. Control of the PHY's is
  not directly trough ULPI, UTMI or PIPE3 interfaces, but trough
  QSCRATCH registers, which of course is highly Qualcomm specific.
  
  isn't it a memory mapped IP ? doesn't synopsys provide their own set of
  registers ?
  
  From what I see it is not directly mapped. How QSCRATCH write and
  reads transactions are translated to DW IP is unclear to me.
  
  
  I think the question is how does SW access them?
  
  I afraid the answer may be: it depends on the SOC. In my past I had to
  initialize a (SATA) PHY by implementing a software JTAG state machine,
  as the PHY's registers were not memory mapped *at all*. And the IP
  itself came from Synopsys, Cadence or yet another EDA company...
  
  Having said all that... If the PHY's spec at least defined layout of the
  registers in question and driver was using regmap API to talk to the
  device (initially regmap-mmio), it has some chances to become universal.
  The SOCs designed like my one would have to provide a custom regmap
  implementation.
  
  Sound reasonable. Unfortunately I don't have PHY's IP spec. 
 
 Looking into this it appears that the register wrapper around the IP
 is most likely highly specific to qualcomm as I'm not seeing a
 register interface around the DWC HS PHY.

Then it's set, we'll go with this driver :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core

[Felipe Balbi]

Hi,

On Tue, Aug 20, 2013 at 12:56:04PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 These drivers handles control and configuration of the HS
 and SS USB PHY transceivers. They are part of the driver
 which manage Synopsys DesignWare USB3 controller stack
 inside Qualcomm SoC's.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  drivers/usb/phy/Kconfig   |   11 ++
  drivers/usb/phy/Makefile  |2 +
  drivers/usb/phy/phy-msm-dwc3-hs.c |  327 
  drivers/usb/phy/phy-msm-dwc3-ss.c |  374 
 +

please rename these PHY drivers, they have nothing to do with DWC3. PHYs
don't care about the USB controller.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core

[Felipe Balbi]

Hi,

On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote:
  On Tue, Aug 20, 2013 at 12:56:04PM +0300, Ivan T. Ivanov wrote:
   From: Ivan T. Ivanov iiva...@mm-sol.com
   
   These drivers handles control and configuration of the HS
   and SS USB PHY transceivers. They are part of the driver
   which manage Synopsys DesignWare USB3 controller stack
   inside Qualcomm SoC's.
   
   Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
   ---
drivers/usb/phy/Kconfig   |   11 ++
drivers/usb/phy/Makefile  |2 +
drivers/usb/phy/phy-msm-dwc3-hs.c |  327 
drivers/usb/phy/phy-msm-dwc3-ss.c |  374 
   +
  
  please rename these PHY drivers, they have nothing to do with DWC3. PHYs
  don't care about the USB controller.
 
 I think they are SNPS DesignWare PHY's, additionally
 wrapped with Qualcomm logic. I could substitute dwc3
 with just dw, which will be more correct.

alright, thank you. Let's add Paul to the loop since he might have very
good insight in the synopsys PHYs.

mental note: if any other platform shows up with Synopsys PHY, ask them
to use this driver instead :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core

[Felipe Balbi]

On Tue, Aug 20, 2013 at 05:09:11PM +0300, Ivan T. Ivanov wrote:
 Hi,
 
 On Tue, 2013-08-20 at 08:37 -0500, Felipe Balbi wrote: 
  Hi,
  
  On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote:
On Tue, Aug 20, 2013 at 12:56:04PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 These drivers handles control and configuration of the HS
 and SS USB PHY transceivers. They are part of the driver
 which manage Synopsys DesignWare USB3 controller stack
 inside Qualcomm SoC's.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  drivers/usb/phy/Kconfig   |   11 ++
  drivers/usb/phy/Makefile  |2 +
  drivers/usb/phy/phy-msm-dwc3-hs.c |  327 
 
  drivers/usb/phy/phy-msm-dwc3-ss.c |  374 
 +

please rename these PHY drivers, they have nothing to do with DWC3. PHYs
don't care about the USB controller.
   
   I think they are SNPS DesignWare PHY's, additionally
   wrapped with Qualcomm logic. I could substitute dwc3
   with just dw, which will be more correct.
  
  alright, thank you. Let's add Paul to the loop since he might have very
  good insight in the synopsys PHYs.
  
  mental note: if any other platform shows up with Synopsys PHY, ask them
  to use this driver instead :-)
 
 I really doubt that this will bi possible. Control of the PHY's is
 not directly trough ULPI, UTMI or PIPE3 interfaces, but trough
 QSCRATCH registers, which of course is highly Qualcomm specific.

isn't it a memory mapped IP ? doesn't synopsys provide their own set of
registers ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

On Mon, Aug 12, 2013 at 02:52:33PM -0400, Alan Stern wrote:
 On Mon, 12 Aug 2013, Felipe Balbi wrote:
 
maybe a single callback for supporting 'testmodes' ? which receives the
test mode as argument ?
   
   I don't have a clear picture of how you would apply such an approach to 
   this case.  There would have to be a way to tell the HCD to insert a
   15-second delay between the Setup and Data stages of a particular
   control URB.  How would you do that?  Whatever method you choose,
  
  each test is started after enumerating a known Vid/Pid pair. Based on
  that, you *know* which test to run.
 
 That's not what I meant.  Yes, the test-device driver knows what test
 it wants to run, based on the VID/PID.  I was asking how it would
 communicate this knowledge to the HCD.
 
 For example, it doesn't make sense to have a callback that means 
 Insert a 15-second delay into the next URB that I submit, because the 
 HCD doesn't know where URBs come from.

static int ehci_test_mode(struct usb_hcd *hcd, unsigned int test)
{
struct ehci_hcd *ehci = to_ehci(hcd);



switch (test) {
case USB_TEST_SINGLE_STEP_GET_DESC:
start_test();
wait_15_seconds();
finish_test();
break;
...

default:
return -ENOTSUP;
}

return ret;
}

...

static struct hc_driver ehci_hcd_driver = {


.test_mode  = ehci_test_mode,

...
};

   What other test modes would you want to support?
  
  anything that USB[23]0CV supports today. There are even link layer tests
  for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
  of a large(-ish) test suite which needs to be supported.
 
 That's what I'm trying to find out.  What are the special features that 
 we would need to implement in order to support the entire test suite?

I haven't looked at USB??CV spec for a while, maybe Jack knows better ?

   Is it worth adding this support to the standard host controller
   drivers, or should there be a special version (a Kconfig option like
   CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
   in distribution kernels where it will never be used seems like a big
   waste.
  
  right, I think it should be hidden by Kconfig, not arguing against that.
 
 Therefore we both agree the $SUBJECT patch should not be accepted in
 its current form.  At the very least, the new code in ehci-hcd should
 be conditional on a CONFIG_USB_HCD_TEST_MODE option.  In addition, we
 may want some of the work (though at this point I'm not still clear on
 exactly what parts) to be moved into usbcore.

right

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

Hi,

On Fri, Aug 09, 2013 at 11:04:48AM -0400, Alan Stern wrote:
heh, it doesn't need to be entirely in the core. Core could have the
generic calls and HCDs could implement some callbacks, but I think quite
a bit of the code will be similar if we implement the same thing on all
HCDs.
   
   What generic calls and callbacks would you suggest?  I assume you want 
   enough to cover not just this one test but the entire USB-CV suite.
  
  maybe a single callback for supporting 'testmodes' ? which receives the
  test mode as argument ?
 
 I don't have a clear picture of how you would apply such an approach to 
 this case.  There would have to be a way to tell the HCD to insert a
 15-second delay between the Setup and Data stages of a particular
 control URB.  How would you do that?  Whatever method you choose,

each test is started after enumerating a known Vid/Pid pair. Based on
that, you *know* which test to run.

 implementing it in every HCD would be a huge amount of work.

sure, we can support different HCDs with time, but once SINGLE_STEP is
implemented in e.g. EHCI, it should be simple to port it to
OHCI/xHCI/MUSB/etc.

 What other test modes would you want to support?

anything that USB[23]0CV supports today. There are even link layer tests
for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one
of a large(-ish) test suite which needs to be supported.

 Is it worth adding this support to the standard host controller
 drivers, or should there be a special version (a Kconfig option like
 CONFIG_RCU_TORTURE_TEST) to enable it?  Putting a lot of testing code
 in distribution kernels where it will never be used seems like a big
 waste.

right, I think it should be hidden by Kconfig, not arguing against that.

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC PATCH v2 1/3] usb: dwc3: msm: Add device tree binding information

[Felipe Balbi]

On Fri, Aug 09, 2013 at 10:31:58AM -0500, Kumar Gala wrote:
 
 On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote:
 
  From: Ivan T. Ivanov iiva...@mm-sol.com
  
  MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS)
 
 probably good to spell out Synopsys rather than SNPS

Synopsys (the company) has always used snps in their bindings though.

  +Required properities :
  +- compatible : sould be qcom,dwc3-hsphy;
  +- reg : offset and length of the register set in the memory map
  +- clocks : phandles to clock instances of the device tree nodes
  +- clock-names :
  +   xo : External reference clock 19 MHz
  +   sleep_a_clk : Sleep clock, used when USB3 core goes into low
  +   power mode (U3).
  +supply-name-supply : phandle to the regulator device tree node
  +Required supply-name are:
  +   v1p8 : 1.8v supply for HSPHY
  +   v3p3 : 3.3v supply for HSPHY
  +   vbus : vbus supply for host mode
  +   vddcx : vdd supply for HS-PHY digital circuit operation

I believe these regulators belong to the PHY, not DWC3. Please write a
PHY driver.

  +Required properities :
  +- compatible : sould be qcom,dwc3-ssphy;
  +- reg : offset and length of the register set in the memory map
  +- clocks : phandles to clock instances of the device tree nodes
  +- clock-names :
  +   xo : External reference clock 19 MHz
  +   ref_clk : Reference clock - used in host mode.
  +supply-name-supply : phandle to the regulator device tree node
  +Required supply-name are:
  +   v1p8 : 1.8v supply for SS-PHY
  +   vddcx : vdd supply for SS-PHY digital circuit operation

likewise, these regulators should be moved to the PHY driver.

  +Required properties :
  +- compatible : should be qcom,dwc3
  +- reg : offset and length of the register set in the memory map
  +   offset and length of the TCSR register for routing USB
  +   signals to either picoPHY0 or picoPHY1.
  +- clocks : phandles to clock instances of the device tree nodes
  +- clock-names :
  +   core_clk : Master/Core clock, have to be = 125 MHz for SS
  +   operation and = 60MHz for HS operation
  +   iface_clk : System bus AXI clock
  +   sleep_clk : Sleep clock, used when USB3 core goes into low
  +   power mode (U3).
  +   utmi_clk : Generated by HS-PHY. Used to clock the low power
  +   parts of thr HS Link layer.
  +
  +Optional properties :
  +- gdsc-supply : phandle to the globally distributed switch controller
  +  regulator node to the USB controller.
  +
  +Sub nodes:
  +- Sub node for DWC3 USB3 controller.
  +  This sub node is required property for device node. The properties
  +  of this subnode are specified in dwc3.txt.
 
 Is tx-fifo-resize required for qcom,dwc3? if so we should call that
 out in the binding.

AFAICT that's only needed for OMAP5 ES1.0. Unless Qualcomm also screwed
up default TX FIFO sizes :-p

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC PATCH v2 3/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

On Fri, Aug 09, 2013 at 12:53:47PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 DWC3 glue layer is hardware layer around Synopsys DesignWare
 USB3 core. Its purpose is to supply Synopsys IP with required
 clocks, voltages and interface it with the rest of the SoC.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

same comments from previous version.

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

Hi,

On Thu, Jul 25, 2013 at 05:33:48PM -0400, Alan Stern wrote:
  On Thu, Jul 25, 2013 at 03:44:20PM -0400, Alan Stern wrote:
   On Thu, 25 Jul 2013, Greg KH wrote:
   
On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
 From: Manu Gautam mgau...@codeaurora.org
 
 The USB Embedded High-speed Host Electrical Test (EHSET) defines the
 SINGLE_STEP_SET_FEATURE test as follows:
 
 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
 2) The host sends the SETUP stage of a GetDescriptor(Device)
 3) The device ACKs the request
 4) The host issues SOFs for 15 seconds allowing the test operator to
raise the scope trigger just above the SOF voltage level
 5) The host sends the IN packet
 6) The device sends data in response, triggering the scope
 7) The host sends an ACK in response to the data
 
 This patch adds additional handling to the EHCI hub driver and allows
 the EHSET driver to initiate this test mode by issuing a a SetFeature
 request to the root hub with a Test Selector value of 0x06. From there
 it mimics ehci_urb_enqueue() but separately submits QTDs for the
 SETUP and DATA/STATUS stages in order to insert a delay in between.
 
 Signed-off-by: Manu Gautam mgau...@codeaurora.org
 Signed-off-by: Jack Pham ja...@codeaurora.org

Alan, any thoughts about this patch?
   
   Sorry, this slipped my mind.
   
   It looks okay.  I haven't tested it yet (and it's so specialized that
   it probably will never receive very much testing).  It is somewhat 
   fragile, in that it copies part of usbcore into ehci-hcd; updates to 
   the core will have to be mirrored in the driver.
   
   On the other hand, there's no real reason to reject it, and it could 
   end up helping people who want to test new USB devices.  So...
   
   Acked-by: Alan Stern st...@rowland.harvard.edu
  
  Wait a minute, didn't we discuss a while back that these test features
  should be built into usbcore so that we could have a usbcv clone for
  linux ?
 
 There's no way this can be built into the core.  This test requires the
 behavior of the host controller to be modified at a low level
 (introducing a 15-second delay between the Setup and Data stages of a
 control transfer).  Only the HCD can do that.

heh, it doesn't need to be entirely in the core. Core could have the
generic calls and HCDs could implement some callbacks, but I think quite
a bit of the code will be similar if we implement the same thing on all
HCDs.

 Besides, doesn't USB-CV install its own version of Windows' EHCI driver 
 while it runs its tests?

IIRC only USB20CV, the new USB30CV (which also tests USB2) doesn't do
that.

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

Hi,

On Fri, Aug 09, 2013 at 10:37:50AM -0400, Alan Stern wrote:
Wait a minute, didn't we discuss a while back that these test features
should be built into usbcore so that we could have a usbcv clone for
linux ?
   
   There's no way this can be built into the core.  This test requires the
   behavior of the host controller to be modified at a low level
   (introducing a 15-second delay between the Setup and Data stages of a
   control transfer).  Only the HCD can do that.
  
  heh, it doesn't need to be entirely in the core. Core could have the
  generic calls and HCDs could implement some callbacks, but I think quite
  a bit of the code will be similar if we implement the same thing on all
  HCDs.
 
 What generic calls and callbacks would you suggest?  I assume you want 
 enough to cover not just this one test but the entire USB-CV suite.

maybe a single callback for supporting 'testmodes' ? which receives the
test mode as argument ?

   Besides, doesn't USB-CV install its own version of Windows' EHCI driver 
   while it runs its tests?
  
  IIRC only USB20CV, the new USB30CV (which also tests USB2) doesn't do
  that.
 
 I don't have more than the foggiest notion of how these things really
 work under Windows.

and that's hardly necessary to understand...

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 7/7] usb: phy: msm: Lindent the code

[Felipe Balbi]

Hi,

On Thu, Jul 25, 2013 at 04:40:51PM +0300, Ivan T. Ivanov wrote:
   diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
   index 6d05085..111f454 100644
   --- a/drivers/usb/phy/phy-msm-usb.c
   +++ b/drivers/usb/phy/phy-msm-usb.c
   @@ -64,8 +64,8 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, 
   int init)

 if (init) {
 ret = regulator_set_voltage(motg-vddcx,
   - USB_PHY_VDD_DIG_VOL_MIN,
   - USB_PHY_VDD_DIG_VOL_MAX);
   + USB_PHY_VDD_DIG_VOL_MIN,
   + USB_PHY_VDD_DIG_VOL_MAX);
  
  like here, what's the point ?
 
 It makes indentation the same like the majority of the code.
 I could drop this patch if you like.

yeah, I don't think it's worth to have this patch. Just looked at the
driver and, at least to my eyes, it didn't look all that bad. Matter of
taste though

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET

[Felipe Balbi]

On Thu, Jul 25, 2013 at 03:44:20PM -0400, Alan Stern wrote:
 On Thu, 25 Jul 2013, Greg KH wrote:
 
  On Tue, Jul 02, 2013 at 08:13:52PM -0700, Jack Pham wrote:
   From: Manu Gautam mgau...@codeaurora.org
   
   The USB Embedded High-speed Host Electrical Test (EHSET) defines the
   SINGLE_STEP_SET_FEATURE test as follows:
   
   1) The host enumerates the test device with VID:0x1A0A, PID:0x0108
   2) The host sends the SETUP stage of a GetDescriptor(Device)
   3) The device ACKs the request
   4) The host issues SOFs for 15 seconds allowing the test operator to
  raise the scope trigger just above the SOF voltage level
   5) The host sends the IN packet
   6) The device sends data in response, triggering the scope
   7) The host sends an ACK in response to the data
   
   This patch adds additional handling to the EHCI hub driver and allows
   the EHSET driver to initiate this test mode by issuing a a SetFeature
   request to the root hub with a Test Selector value of 0x06. From there
   it mimics ehci_urb_enqueue() but separately submits QTDs for the
   SETUP and DATA/STATUS stages in order to insert a delay in between.
   
   Signed-off-by: Manu Gautam mgau...@codeaurora.org
   Signed-off-by: Jack Pham ja...@codeaurora.org
  
  Alan, any thoughts about this patch?
 
 Sorry, this slipped my mind.
 
 It looks okay.  I haven't tested it yet (and it's so specialized that
 it probably will never receive very much testing).  It is somewhat 
 fragile, in that it copies part of usbcore into ehci-hcd; updates to 
 the core will have to be mirrored in the driver.
 
 On the other hand, there's no real reason to reject it, and it could 
 end up helping people who want to test new USB devices.  So...
 
 Acked-by: Alan Stern st...@rowland.harvard.edu

Wait a minute, didn't we discuss a while back that these test features
should be built into usbcore so that we could have a usbcv clone for
linux ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 1/7] usb: phy: msm: Move mach dependent code to platform data

[Felipe Balbi]

On Mon, Jun 24, 2013 at 06:27:38PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 This patch fix compilation error and is an intermediate step
 before the addition of DeviceTree support for newer targets.
 Fix suggested here: https://lkml.org/lkml/2013/6/19/381
 
 Cc: David Brown dav...@codeaurora.org
 Cc: Daniel Walker dwal...@fifo99.com
 Cc: Bryan Huntsman bry...@codeaurora.org
 Cc: Russell King li...@arm.linux.org.uk
 Cc: Felipe Balbi ba...@ti.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 Cc: Stephen Boyd sb...@codeaurora.org
 Cc: linux-arm-ker...@lists.infradead.org
 Cc: linux-ker...@vger.kernel.org
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

I need Acks

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 7/7] usb: phy: msm: Lindent the code

[Felipe Balbi]

On Mon, Jun 24, 2013 at 06:27:44PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Cc: Felipe Balbi ba...@ti.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 Cc: linux-...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

I really don't like blind Lindent patches... sometimes it makes things
even worse.

 ---
  drivers/usb/phy/phy-msm-usb.c |   99 
 ++---
  1 file changed, 52 insertions(+), 47 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index 6d05085..111f454 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -64,8 +64,8 @@ static int msm_hsusb_init_vddcx(struct msm_otg *motg, int 
 init)
  
   if (init) {
   ret = regulator_set_voltage(motg-vddcx,
 - USB_PHY_VDD_DIG_VOL_MIN,
 - USB_PHY_VDD_DIG_VOL_MAX);
 + USB_PHY_VDD_DIG_VOL_MIN,
 + USB_PHY_VDD_DIG_VOL_MAX);

like here, what's the point ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/7] usb: phy: msm: Move mach depndend code to platform data

[Felipe Balbi]

On Tue, Jul 09, 2013 at 06:47:07PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 This patch fix compilation error and is an intermediate step
 before the addition of DeviceTree support for newer targets.
 Fix suggested here: https://lkml.org/lkml/2013/6/19/381
 
 Cc: David Brown dav...@codeaurora.org
 Cc: Daniel Walker dwal...@fifo99.com
 Cc: Bryan Huntsman bry...@codeaurora.org
 Cc: Stephen Boyd sb...@codeaurora.org
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com

I need Acks for arch/arm/*, I'll drop these from my TODO until it's
resent with proper Acks.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/7] usb: phy: msm: Move mach depndend code to platform data

[Felipe Balbi]

On Wed, Jul 24, 2013 at 03:34:43PM +0300, Felipe Balbi wrote:
 On Tue, Jul 09, 2013 at 06:47:07PM +0300, Ivan T. Ivanov wrote:
  From: Ivan T. Ivanov iiva...@mm-sol.com
  
  This patch fix compilation error and is an intermediate step
  before the addition of DeviceTree support for newer targets.
  Fix suggested here: https://lkml.org/lkml/2013/6/19/381
  
  Cc: David Brown dav...@codeaurora.org
  Cc: Daniel Walker dwal...@fifo99.com
  Cc: Bryan Huntsman bry...@codeaurora.org
  Cc: Stephen Boyd sb...@codeaurora.org
  
  Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 
 I need Acks for arch/arm/*, I'll drop these from my TODO until it's
 resent with proper Acks.

btw, you better start using drivers/reset for device-side reset
assertion-deassertion.

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 2/7] usb: phy: msm: Migrate to Managed Device Resource allocation

[Felipe Balbi]

Hi,

On Tue, Jul 09, 2013 at 06:47:08PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 Use managed device resources to clean up the probe/remove
 and get DT support for free.
 
 Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
 ---
  drivers/usb/phy/phy-msm-usb.c |   78 
 +++--
  1 file changed, 20 insertions(+), 58 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
 index ab1b880..cc37f5e 100644
 --- a/drivers/usb/phy/phy-msm-usb.c
 +++ b/drivers/usb/phy/phy-msm-usb.c
 @@ -1458,30 +1455,27 @@ static int __init msm_otg_probe(struct 
 platform_device *pdev)
* clock is introduced to remove the dependency on AXI
* bus frequency.
*/
 - motg-core_clk = clk_get(pdev-dev, usb_hs_core_clk);
 + motg-core_clk = devm_clk_get(pdev-dev, usb_hs_core_clk);
   if (IS_ERR(motg-core_clk))
   motg-core_clk = NULL;
  
   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   if (!res) {

no need to check for the resource when using devm_ioremap_resource()

   dev_err(pdev-dev, failed to get platform resource mem\n);
 - ret = -ENODEV;
 - goto put_core_clk;
 + return -ENODEV;
   }
  
 - motg-regs = ioremap(res-start, resource_size(res));
 + motg-regs = devm_ioremap_resource(pdev-dev, res);
   if (!motg-regs) {
   dev_err(pdev-dev, ioremap failed\n);

don't print error messages when using devm_ioremap_resource()

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 3/7] usb: phy: msm: Move regulator usage to managed resource allocation

[Felipe Balbi]

On Tue, Jul 09, 2013 at 06:47:09PM +0300, Ivan T. Ivanov wrote:
 From: Ivan T. Ivanov iiva...@mm-sol.com
 
 This patch move global regulators variables to driver state
 structire and move allocation of the regulators to be devm managed.

split into two patches please. One for moving the global regulators into
your structure and a separate patch to move to devm_*

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 03/12] usb: otg: msm: Convert to clk_prepare/unprepare

[Felipe Balbi]

Hi,

On Tue, Jun 04, 2013 at 12:25:29PM -0700, Stephen Boyd wrote:
 Add calls to clk_prepare and unprepare so that MSM can migrate to
 the common clock framework.
 
 Cc: Felipe Balbi ba...@ti.com
 Signed-off-by: Stephen Boyd sb...@codeaurora.org

it's best if this goes with the rest of the series, for that:


Acked-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH v4 0/3] UAS Gadget Driver.

[Felipe Balbi]

On Tue, Dec 06, 2011 at 09:09:55AM -0800, Greg KH wrote:
 On Tue, Dec 06, 2011 at 08:39:58AM -0800, Shimrit Malichi wrote:
  
   On Sun, Dec 04, 2011 at 10:18:14PM +0200, Shimrit Malichi wrote:
   This patch series implements the UAS gadget driver.
  
   It has been tested using the following:
   1. Linux UAS host driver
   2. Internaly developed unittests framework
  
   The device is functional. More stress tests are needed.
  
   TODO: Further testing
 De-register the gadget if the main thread dies.
  
   What about all of the review comments you seem to be ignoring that have
   been made for this driver in the past?
  
   greg k-h
  
  We addressed most of the comments. One of them was indeed to use the
  target framework, however when we asked an additional information about
  how to use this framework we didn't get a detailed reply. Sebastian gave
  an answer on how to use it yesterday. We will study his answer, and come
  up with a new code.
 
 No one is going to normally give you such a detailed answer as he did.
 You were told to go use that interface, and Sebastian did just that, why
 couldn't you have done that in the first place, saving you, and everyone
 else here, time and effort of telling you to go do that again?

other than that, it's just waste of time if you go write new code, now
that Sebastian already has that almost ready. Save yourself sometime and
just help Sebastian finishing what he's already done. Even testing is
welcome at this stage.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 11/13] ARM: gpio: consolidate trivial gpiolib implementations

[Felipe Balbi]

On Tue, Aug 09, 2011 at 09:08:01AM +0100, Russell King - ARM Linux wrote:
 Consolidate 24 trivial gpiolib implementions out of mach/gpio.h
 into asm/gpio.h.  This is basically the include of asm-generic/gpio.h
 and the definition of gpio_get_value, gpio_set_value, and gpio_cansleep
 as described in Documentation/gpio.txt
 
 Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
 ---
  arch/arm/include/asm/gpio.h |   10 ++
  arch/arm/mach-at91/include/mach/gpio.h  |5 +
  arch/arm/mach-ep93xx/include/mach/gpio.h|7 +--
  arch/arm/mach-exynos4/include/mach/gpio.h   |5 +
  arch/arm/mach-gemini/include/mach/gpio.h|5 +
  arch/arm/mach-ks8695/include/mach/gpio.h|   11 ++-
  arch/arm/mach-lpc32xx/include/mach/gpio.h   |   17 +
  arch/arm/mach-msm/include/mach/gpio.h   |5 +
  arch/arm/mach-mxs/include/mach/gpio.h   |6 +-
  arch/arm/mach-realview/include/mach/gpio.h  |6 +-
  arch/arm/mach-s3c2410/include/mach/gpio.h   |5 +
  arch/arm/mach-s3c64xx/include/mach/gpio.h   |6 +-
  arch/arm/mach-s5p64x0/include/mach/gpio.h   |6 +-
  arch/arm/mach-s5pc100/include/mach/gpio.h   |6 +-
  arch/arm/mach-s5pv210/include/mach/gpio.h   |6 +-
  arch/arm/mach-shmobile/include/mach/gpio.h  |   15 +--
  arch/arm/mach-tegra/include/mach/gpio.h |6 +-
  arch/arm/mach-versatile/include/mach/gpio.h |6 +-
  arch/arm/mach-vt8500/include/mach/gpio.h|6 +-
  arch/arm/mach-w90x900/include/mach/gpio.h   |5 +
  arch/arm/plat-mxc/include/mach/gpio.h   |5 +
  arch/arm/plat-nomadik/include/plat/gpio.h   |   11 +--
  arch/arm/plat-omap/include/plat/gpio.h  |   15 +--
  arch/arm/plat-orion/include/plat/gpio.h |6 ++
  arch/arm/plat-spear/include/plat/gpio.h |6 +-
  25 files changed, 36 insertions(+), 151 deletions(-)
 
 diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
 index 166a7a3..15e8970 100644
 --- a/arch/arm/include/asm/gpio.h
 +++ b/arch/arm/include/asm/gpio.h
 @@ -4,4 +4,14 @@
  /* not all ARM platforms necessarily support this API ... */
  #include mach/gpio.h
  
 +#ifdef __ARM_GPIOLIB_TRIVIAL
 +/* Note: this may rely upon the value of ARCH_NR_GPIOS set in mach/gpio.h */
 +#include asm-generic/gpio.h
 +
 +/* The trivial gpiolib dispatchers */
 +#define gpio_get_value  __gpio_get_value
 +#define gpio_set_value  __gpio_set_value
 +#define gpio_cansleep   __gpio_cansleep
 +#endif

could that be a selectable symbol ? Something like:

CONFIG_HAS_ARM_TRIVIAL_GPIO

then on Kconfig you just:

select HAS_ARM_TIVIAL_GPIO or something ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 11/13] ARM: gpio: consolidate trivial gpiolib implementations

[Felipe Balbi]

Hi,

On Thu, Aug 11, 2011 at 04:07:18PM +0100, Russell King - ARM Linux wrote:
 On Thu, Aug 11, 2011 at 05:15:31PM +0300, Felipe Balbi wrote:
  On Tue, Aug 09, 2011 at 09:08:01AM +0100, Russell King - ARM Linux wrote:
   diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
   index 166a7a3..15e8970 100644
   --- a/arch/arm/include/asm/gpio.h
   +++ b/arch/arm/include/asm/gpio.h
   @@ -4,4 +4,14 @@
/* not all ARM platforms necessarily support this API ... */
#include mach/gpio.h

   +#ifdef __ARM_GPIOLIB_TRIVIAL
   +/* Note: this may rely upon the value of ARCH_NR_GPIOS set in 
   mach/gpio.h */
   +#include asm-generic/gpio.h
   +
   +/* The trivial gpiolib dispatchers */
   +#define gpio_get_value  __gpio_get_value
   +#define gpio_set_value  __gpio_set_value
   +#define gpio_cansleep   __gpio_cansleep
   +#endif
  
  could that be a selectable symbol ? Something like:
  
  CONFIG_HAS_ARM_TRIVIAL_GPIO
  
  then on Kconfig you just:
  
  select HAS_ARM_TIVIAL_GPIO or something ?
 
 That makes things more complicated, because that involves digging through
 a lot of platform code in a couple of places to work out exactly when we
 need this - and it crosses the boundary to arch/sh too.
 
 So I'd prefer to keep this simple.
 
 The long-term goal is to remove that symbol entirely, but in order to do
 that we need to kill of the optimized on-board SoC stuff in those (few)
 gpio.h which don't have the symbol selected.  This is rather necessary to
 progress towards the consolidated kernel.  (Re-inventing gpiolib by moving
 them out of line isn't a good idea...)

Ok, I understand.

-- 
balbi


signature.asc
Description: Digital signature

Re: [RFC/PATCH 2/3] usb:gadget: Add SuperSpeed function power management

[Felipe Balbi]

Hi,

On Sun, Jul 03, 2011 at 05:29:32PM +0300, Amit Blay wrote:
 1. Add missing definitions in ch9.h for requests tarageted to an

typo... s/tarageted/targeted

 Interface: GET_STATUS  SET_FEATURE(FUNC_SUSPEND).

if it's targeted to an interface, why don't you just let the gadget
driver handle it ? composite.c tracks all functions already and we
should just call function-setup() to let the correct function handle
this.

 2. Add func_wakeup api to usb_gadget_ops.
 Super-Speed device must provide interface number to the UDC when
 it triggers remote wakeup (function wake).
 The func_wakeup api is used instead of the wakeup api, when the
 gadget is connected in Super-Speed. The wakeup api is left as is,
 and it is used when the gadget is connected in High-Speed. Therefore,
 no change is required in non Super-Speed UDCs.

first of all, this needs to be splitted. You shouldn't do more than one
thing in a patch ;-)

 Signed-off-by: Amit Blay ab...@codeaurora.org
 
 ---
  drivers/usb/gadget/udc-core.c |2 +-
  drivers/usb/gadget/zero.c |6 +-
  include/linux/usb/ch9.h   |8 
  include/linux/usb/gadget.h|   35 +++
  4 files changed, 41 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
 index 05ba472..beb7e89 100644
 --- a/drivers/usb/gadget/udc-core.c
 +++ b/drivers/usb/gadget/udc-core.c
 @@ -347,7 +347,7 @@ static ssize_t usb_udc_srp_store(struct device *dev,
   struct usb_udc  *udc = dev_get_drvdata(dev);
  
   if (sysfs_streq(buf, 1))
 - usb_gadget_wakeup(udc-gadget);
 + usb_gadget_wakeup(udc-gadget, 0);
  
   return n;
  }
 diff --git a/drivers/usb/gadget/zero.c b/drivers/usb/gadget/zero.c
 index 00e2fd2..a5d13c6 100644
 --- a/drivers/usb/gadget/zero.c
 +++ b/drivers/usb/gadget/zero.c
 @@ -239,7 +239,11 @@ static void zero_autoresume(unsigned long _c)
* because of some direct user request.
*/
   if (g-speed != USB_SPEED_UNKNOWN) {
 - int status = usb_gadget_wakeup(g);
 + /*
 +  * The single interface of the current configuration
 +  * triggers the wakeup.
 +  */
 + int status = usb_gadget_wakeup(g, 1);

no no, I think this should be handled by the function itself (f_*.c).

   INFO(cdev, %s -- %d\n, __func__, status);
   }
  }
 diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
 index 0fd3fbd..404c10e 100644
 --- a/include/linux/usb/ch9.h
 +++ b/include/linux/usb/ch9.h
 @@ -142,7 +142,13 @@
  #define USB_DEVICE_LTM_ENABLE50  /* dev may send LTM */
  #define USB_INTRF_FUNC_SUSPEND   0   /* function suspend */
  
 +/*
 + * Function Suspend Options as defined by USB 3.0
 + * See USB 3.0 spec Table 9-7
 + */
  #define USB_INTR_FUNC_SUSPEND_OPT_MASK   0xFF00
 +#define USB_INTR_FUNC_SUSPEND_SUSP   1 /* function suspend option */
 +#define USB_INTR_FUNC_SUSPEND_RWAKE_EN   2 /* function wake enabled 
 option */
  
  #define USB_ENDPOINT_HALT0   /* IN/OUT will STALL */
  
 @@ -150,6 +156,8 @@
  #define USB_DEV_STAT_U1_ENABLED  2   /* transition into U1 
 state */
  #define USB_DEV_STAT_U2_ENABLED  3   /* transition into U2 
 state */
  #define USB_DEV_STAT_LTM_ENABLED 4   /* Latency tolerance messages */
 +#define USB_INTR_STAT_RWAKE_CAP  0   /* Function wake 
 capable */
 +#define USB_INTR_STAT_RWAKE_EN   1   /* Function 
 wake enabled */
  
  /**
   * struct usb_ctrlrequest - SETUP data for a USB device control request
 diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
 index 087f4b9..3ebbc11 100644
 --- a/include/linux/usb/gadget.h
 +++ b/include/linux/usb/gadget.h
 @@ -458,7 +458,11 @@ struct usb_gadget_ops {
   int (*pullup) (struct usb_gadget *, int is_on);
   int (*ioctl)(struct usb_gadget *,
   unsigned code, unsigned long param);
 +
 + /* USB 3.0 additions */

this comment is not part of this patch ;-) (nitpicking, I know)

   void(*get_config_params)(struct usb_dcd_config_params *);
 + int (*func_wakeup)(struct usb_gadget *, int interface_id);
 +
   int (*udc_start)(struct usb_gadget *,
   struct usb_gadget_driver *);
   int (*udc_stop)(struct usb_gadget *,
 @@ -607,21 +611,36 @@ static inline int usb_gadget_frame_number(struct 
 usb_gadget *gadget)
  /**
   * usb_gadget_wakeup - tries to wake up the host connected to this gadget
   * @gadget: controller used to wake up the host
 + * @interface_id: id of the first interface of the function that
 + *   triggered the wake up
   *
   * Returns zero on success, else negative error code if the hardware
   * doesn't support such attempts, or its support has not been enabled
   * by the usb host.  Drivers must return device descriptors 

Re: [RFC/PATCH 3/3] usb:gadget: SuperSpeed function power management testing support

[Felipe Balbi]

On Sun, Jul 03, 2011 at 05:29:33PM +0300, Amit Blay wrote:
 1. Fix dummy_hcd to let the function handle GET_STATUS(Interface) request.
 2. Fix a bug in f_loopback Gadget which updated the bmAttribute of
 f_sourcesink instead of f_loopback.
 3. Update f_sourcesink Gadget to support function suspend  wakeup.
 This is done by adding get_status()  func_suspend() handlers.
 The current status of the function is controlable via debug FS:
 (wakeup capable, wakeup enabled, suspended).
 
 Signed-off-by: Amit Blay ab...@codeaurora.org
 
 ---
  drivers/usb/gadget/dummy_hcd.c|3 +-
  drivers/usb/gadget/f_loopback.c   |2 +-
  drivers/usb/gadget/f_sourcesink.c |  162 
 +
  3 files changed, 165 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
 index e755a9d..7f6a2d8 100644
 --- a/drivers/usb/gadget/dummy_hcd.c
 +++ b/drivers/usb/gadget/dummy_hcd.c
 @@ -1496,7 +1496,8 @@ static int handle_control_request(struct dummy_hcd 
 *dum_hcd, struct urb *urb,
  Dev_InRequest) {
   buf[0] = (u8)dum-devstatus;
   } else
 - buf[0] = 0;
 + /* Let the function handle this */
 + break;
   }
   if (urb-transfer_buffer_length  1)
   buf[1] = 0;
 diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
 index ca660d4..75bac6d 100644
 --- a/drivers/usb/gadget/f_loopback.c
 +++ b/drivers/usb/gadget/f_loopback.c
 @@ -427,7 +427,7 @@ int __init loopback_add(struct usb_composite_dev *cdev, 
 bool autoresume)
  
   /* support autoresume for remote wakeup testing */
   if (autoresume)
 - sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 + loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;

this change doesn't belong here.

 diff --git a/drivers/usb/gadget/f_sourcesink.c 
 b/drivers/usb/gadget/f_sourcesink.c
 index 5247f07..5c5da19 100644
 --- a/drivers/usb/gadget/f_sourcesink.c
 +++ b/drivers/usb/gadget/f_sourcesink.c
 @@ -59,6 +59,12 @@ struct f_sourcesink {
  
   struct usb_ep   *in_ep;
   struct usb_ep   *out_ep;
 +
 + /* Function Power Management */
 + boolfunc_suspended;
 + boolfunc_wakeup_capable;
 + boolfunc_wakeup_enabled;
 + struct  device dev;

this should *NOT* have a device of its own. And the way you tabified is
wrong.

  };
  
  static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
 @@ -191,6 +197,79 @@ static struct usb_gadget_strings *sourcesink_strings[] = 
 {
   NULL,
  };
  
 +/*** DEVICE ATTRIBUTES ***/
 +
 +static ssize_t f_sourcesink_show_func_suspend(struct device *dev,
 + struct device_attribute *attr,
 + char *buf)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
 + dev);
 + return sprintf(buf, %d\n, ss-func_suspended);
 +}
 +
 +static ssize_t f_sourcesink_show_func_wakeup_enabled(struct device *dev,
 + struct device_attribute *attr,
 + char *buf)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
 + dev);
 + return sprintf(buf, %d\n, ss-func_wakeup_enabled);
 +}
 +
 +static ssize_t f_sourcesink_show_func_wakeup_capable(struct device *dev,
 + struct device_attribute *attr,
 + char *buf)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
 + dev);
 + return sprintf(buf, %d\n, ss-func_wakeup_capable);
 +}
 +
 +static ssize_t f_sourcesink_store_func_wakeup_capable(struct device *dev,
 + struct device_attribute *attr, const char *buf, size_t count)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink, dev);
 + unsigned long func_wakeup_capable;
 +
 + /* Allows changing function wakeup capable field from the file system */
 + if (strict_strtoul(buf, 2, func_wakeup_capable))
 + return -EINVAL;
 + ss-func_wakeup_capable = (bool)func_wakeup_capable;
 + return count;
 +}
 +
 +static ssize_t f_sourcesink_store_func_wakeup_trigger(struct device *dev,
 + struct device_attribute *attr, const char *buf, size_t count)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink, dev);
 +
 + /* Allows trigerring function wakeup from the file system */
 + if (!ss-func_wakeup_capable || !ss-func_wakeup_enabled)
 + return -EINVAL;
 +
 + if (usb_gadget_wakeup(ss-function.config-cdev-gadget,
 +source_sink_intf.bInterfaceNumber)  0)
 + return -EINVAL;
 + return count;
 +}

why didn't you 

Re: [RFC/PATCH 1/3] usb: Add SuperSpeed support to g_zero gadget

[Felipe Balbi]

Hi,

On Sun, Jul 03, 2011 at 05:29:31PM +0300, Amit Blay wrote:
 This patch adds SuperSpeed descriptors to the g_zero gadget.
 The SuperSpeed descriptors were added both for f_soursesink and f_loopback
 configurations of the g_zero gadget.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
 Signed-off-by: Amit Blay ab...@codeaurora.org

applied this one alone. The others we need to dicuss a bit further. I'll
get to them after this merge window.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 2/2] usb: gadget: add SuperSpeed support to the Gadget Framework

[Felipe Balbi]

On Wed, Jun 29, 2011 at 03:34:56PM +0300, Tatyana Brokhman wrote:
 SuperSpeed USB has defined a new descriptor, called
 the Binary Device Object Store (BOS) Descriptor. It
 has also changed a bit the definition of SET_FEATURE
 and GET_STATUS requests to add USB3-specific details.
 
 This patch implements both changes to the Composite
 Gadget Framework.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

had already fixed this one too..

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] usb: gadget: add max_speed to usb_composite_driver

[Felipe Balbi]

Hi,

On Wed, Jun 29, 2011 at 03:45:35PM +0300, Tanya Brokhman wrote:
  I have already fixed this one myself ;-)
  
 
 Oh, ok :) Thank you!

it might take a while for kernel.org to replicate things, maybe that's
why you didn't see before ;-)

give it a few more minutes and check my gadget branch to see if it's all
good, I'm going to test it again on beagleboard to be sure nothing is
broken.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] usb: gadget: add max_speed to usb_composite_driver

[Felipe Balbi]

Hi,

On Wed, Jun 29, 2011 at 08:27:17PM +0300, Tanya Brokhman wrote:
  it might take a while for kernel.org to replicate things, maybe that's
  why you didn't see before ;-)
  
  give it a few more minutes and check my gadget branch to see if it's
  all good, I'm going to test it again on beagleboard to be sure nothing
  is broken.
  
 
 I went over the emails and all is ok. Thanks!
 One question: Greg/Alan mentioned that the compilation warning should be
 fixed by __maybe_unused. I wasn't aware such thing existed. Just looked it
 up. 
 Do you want me to send you a patch that fixes that or have you done it
 already and I missed it again :) ?

Oops, I didn't. You can send a differential patch (just this change in
particular) and I'll merge into the original.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RFC 2/5] usb:dummy_hcd: connect/disconnect test support

[Felipe Balbi]

Hi,

On Thu, Jun 16, 2011 at 11:06:47AM -0400, Alan Stern wrote:
 On Thu, 16 Jun 2011, Tatyana Brokhman wrote:
 
  This implementation adds a new proprietary device control requests (to be
  handled by the dummy_hcd) that initiates a connect/disconnect sequence.
  The bRequest value of the new control request is 0x52.
  It is used by the user-space Unit testing application.
 
 This is not a bad idea.  On the other hand, it is slightly peculiar -- 
 it you're testing with real UDC hardware, you would do the 
 disconnect/reconnect sequence by hand (unplug and replug the USB 
 cable), not in software.

actually, this is quite useful. Specially for the controllers which
_can_ do soft-connect by toggling data pullups. I would rather have
these sort of thing maybe in composite.c, so that we can build tests
with all gadget drivers/controllers, not only dummy_hcd.

That said, I think now it's not the time to fiddle too much with these
details right now. So best to keep this in dummy_hcd until we know the
test tools are actually good.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RFC 2/5] usb:dummy_hcd: connect/disconnect test support

[Felipe Balbi]

Hi,

On Thu, Jun 16, 2011 at 04:31:04PM +0300, Tatyana Brokhman wrote:
 @@ -1362,6 +1363,43 @@ static struct dummy_ep *find_endpoint (struct dummy 
 *dum, u8 address)
  #define Ep_Request   (USB_TYPE_STANDARD | USB_RECIP_ENDPOINT)
  #define Ep_InRequest (Ep_Request | USB_DIR_IN)
  
 +/**
 + * dummy_hcd_conn_disc_work() - performs a disconnect/connect sequence.
 + * @data: pointer to the sceduled work_struct
 + */
 +static void dummy_hcd_conn_disc_work(struct work_struct *data)
 +{
 + struct dummy_hcd *dum_hcd =
 + container_of(data, struct dummy_hcd, conn_disc_work);
 + int ret_val;
 +
 + if (!dum_hcd-dum-driver) {
 + dev_err(dummy_dev(dum_hcd),
 + dummy_hcd_conn_disc_work called without connected 
 + device\n);
 + return;
 + }
 +
 + dev_info(dummy_dev(dum_hcd), disconnecting device...\n);
 + ret_val = dummy_pullup(dum_hcd-dum-gadget, 0);
 + if (ret_val) {
 + dev_err(dummy_dev(dum_hcd), dummy_hcd_conn_disc_work:
 +  couldn't disconnect device:
 +  ret_val=%d\n,
 + ret_val);
 + return;
 + }
 +
 + dev_info(dummy_dev(dum_hcd), re-connecting device...\n);
 + /* We have to let the hub task to update the device disconnect state */
 + msleep_interruptible(1000);
 + ret_val = dummy_pullup(dum_hcd-dum-gadget, 1);
 + if (ret_val)
 + dev_err(dummy_dev(dum_hcd), dummy_hcd_conn_disc_work:
 +  couldn't re-connect device:
 +  ret_val=%d\n,
 + ret_val);

this print is quite ugly. How about something like:

dev_err(dummy_dev(dum_hcd), %s: couldn't re-connect -- %d\n,
__func__, ret_val);

???

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RFC 3/5] usb:g_zero: bulk in/out unittest support

[Felipe Balbi]

Hi,

On Thu, Jun 16, 2011 at 04:31:05PM +0300, Tatyana Brokhman wrote:
 This commit adds a new vendor specific request to be handled by the
 g_zero module in it's sourcesink configuration. The purpose if this
 request is to update the length of the BULK transfer to a given value.
 The bRequest value of the new control request is 0x5e.
 It is used by the user-space Unit testing application for bulk in/out
 tests.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
 
 ---
  drivers/usb/gadget/f_sourcesink.c |   12 
  1 files changed, 12 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/gadget/f_sourcesink.c 
 b/drivers/usb/gadget/f_sourcesink.c
 index caf2f95..d417b8a 100644
 --- a/drivers/usb/gadget/f_sourcesink.c
 +++ b/drivers/usb/gadget/f_sourcesink.c
 @@ -70,6 +70,7 @@ static unsigned pattern;
  module_param(pattern, uint, 0);
  MODULE_PARM_DESC(pattern, 0 = all zeroes, 1 = mod63 );
  
 +static struct f_sourcesink   *the_sourcesink;

this is a big no-no. f_*.c should always be able to be instantiated as
many times as we need.

  /*-*/
  
  static struct usb_interface_descriptor source_sink_intf = {
 @@ -197,6 +198,7 @@ static void
  sourcesink_unbind(struct usb_configuration *c, struct usb_function *f)
  {
   kfree(func_to_ss(f));
 + the_sourcesink = NULL;
  }
  
  /* optionally require specific source/sink data patterns  */
 @@ -425,6 +427,7 @@ static int __init sourcesink_bind_config(struct 
 usb_configuration *c)
   status = usb_add_function(c, ss-function);
   if (status)
   kfree(ss);
 + the_sourcesink = ss;
   return status;
  }
  
 @@ -472,6 +475,15 @@ static int sourcesink_setup(struct usb_configuration *c,
   value = w_length;
   break;
  
 + case 0x5e:
 + /*
 +  * Change bulk ep buffer size. buflen is the length of
 +  * the BULK transfer (req-length=buflen). Defined in g_zero.h
 +  */
 + disable_source_sink(the_sourcesink);
 + buflen = w_value;
 + value = enable_source_sink(c-cdev, the_sourcesink);
 + break;
   default:

I think this request, should be handled by the f_sourcesink interface.
Meaning you'd need to introduce another f_sourcesink_setup(); to be
added to the function, rather then the configuration.

Please change that.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] s3c2410_udc: udc command always disables pull-up instead of passing the command

[Felipe Balbi]

Hi,

On Wed, Jun 15, 2011 at 09:30:00AM +0200, Skacore Systems wrote:
 I don't care much about pushing this into mainline. It's a bug and
 it's up to kernel developers to fix that.
 I was aware that kernel devs are bunch of lazy dicks and I'm not going
 to do more work for one line patch.

You are the lazy dick here. All I asked was to follow our standards, do
you have any idea how many patches each kernel maintainer has to go
through ? It doesn't scale if we have to re-format the ugly crap you
send us.

Get over yourself and if you want this fixed, then send a proper patch.

It's just a small set of instructions we are asking you to follow, try
saying the same to your Income Tax office and see if they'll like that
you didn't follow their instructions.

Lazy dicks... hah... the one who can't get a simple set of instructions
right is calling us lazy dicks. What a prick!

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] usb:gadget: use min_t() macro instead of min()

[Felipe Balbi]

Hi,


On Sun, Jun 12, 2011 at 02:14:46PM +0300, Tatyana Brokhman wrote:
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

I need a sensible commit log for this. Why do we need to change all
min() to min_t() ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v3] usb:dummy_hcd: Force FS device connection according to module parameter

[Felipe Balbi]

On Fri, Jun 10, 2011 at 11:12:57AM -0400, Alan Stern wrote:
 On Fri, 10 Jun 2011, Tatyana Brokhman wrote:
 
  This patch adds a new module parameter to dummy_hcd: is_high_speed
  When set to false the connected device will be forced to operate in FS
  mode. The default of this parameter is true.
  
  Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
 
 Acked-by: Alan Stern st...@rowland.harvard.edu

applied, thanks

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RESEND v15 1/10] usb: Add usb_endpoint_descriptor to be part of the struct usb_ep

[Felipe Balbi]

Hi,

On Mon, Jun 06, 2011 at 02:40:45PM +0300, Tatyana Brokhman wrote:
 Change usb_ep_enable() prototype to use endpoint descriptor from usb_ep.
 This optimization spares the FDs from saving the endpoint chosen
 descriptor. This optimization is not full though. To fully exploit this
 change one needs to update all the UDCs as well since in the current
 implementation each of them saves the endpoint descriptor in it's
 internal (and extended) endpoint structure.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

now that I look at this patch carefully, won't this break all gadget
drivers ? I mean, if I apply this patch, all gadget drivers will be
using descriptor from struct usb_ep, but noone is actually assigning
that pointer.

A better approach would be to:

(a) add the field to struct usb_ep
(b) make each controller assign that pointer while still keeping the
last one (keep the old interface too, make one patch per
controller)
(c) make each gadget/function driver use the new interface (one patch
per driver)
(d) remove the old interface from all controller (one patch for all of
them)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RESEND v15 1/10] usb: Add usb_endpoint_descriptor to be part of the struct usb_ep

[Felipe Balbi]

Hi again,

On Thu, Jun 09, 2011 at 11:32:10AM +0300, Felipe Balbi wrote:
 On Mon, Jun 06, 2011 at 02:40:45PM +0300, Tatyana Brokhman wrote:
  Change usb_ep_enable() prototype to use endpoint descriptor from usb_ep.
  This optimization spares the FDs from saving the endpoint chosen
  descriptor. This optimization is not full though. To fully exploit this
  change one needs to update all the UDCs as well since in the current
  implementation each of them saves the endpoint descriptor in it's
  internal (and extended) endpoint structure.
  
  Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
 
 now that I look at this patch carefully, won't this break all gadget
 drivers ? I mean, if I apply this patch, all gadget drivers will be
 using descriptor from struct usb_ep, but noone is actually assigning
 that pointer.

actually, I was wrong. The gadget driver is setting it, not the
controller anymore. I'll take this series in.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RESEND v15 2/10] usb: Configure endpoint according to gadget speed.

[Felipe Balbi]

On Mon, Jun 06, 2011 at 02:40:46PM +0300, Tatyana Brokhman wrote:
 Add config_ep_by_speed() to configure the endpoint according to the gadget
 speed. Using this function will spare the FDs from handling the endpoint
 chosen descriptor.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

applied to gadget branch, thanks

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RESEND v15 5/10] usb: coding style fix

[Felipe Balbi]

On Mon, Jun 06, 2011 at 02:40:49PM +0300, Tatyana Brokhman wrote:
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

applied to gadget branch, thanks

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RESEND v15 7/10] usb:gadget: Add SuperSpeed support to the Gadget Framework

[Felipe Balbi]

On Mon, Jun 06, 2011 at 02:40:51PM +0300, Tatyana Brokhman wrote:
 This patch adds the SuperSpeed functionality to the gadget framework.
 Support for new SuperSpeed BOS descriptor was added.
 Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
 added.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

applied to gadget branch, thanks

-- 
balbi


signature.asc
Description: Digital signature