Re: [PATCH] usb: gadget: Stall OS descriptor request for unsupported functions

[Jack Pham]

Hi Wesley,

On Mon, Mar 22, 2021 at 06:50:17PM -0700, Wesley Cheng wrote:
> From: Chandana Kishori Chiluveru 
> 
> Hosts which request "OS descriptors" from gadgets do so during
> the enumeration phase and before the configuration is set with
> SET_CONFIGURATION. Composite driver supports OS descriptor
> handling in composite_setup function. This requires to pass
> signature field, vendor code, compatibleID and subCompatibleID
> from user space.
> 
> For USB compositions that contain functions which don't implement os
> descriptors, Windows is sending vendor specific requests for os
> descriptors and composite driver handling this request with invalid
> data. With this invalid info host resetting the bus and never
> selecting the configuration and leading enumeration issue.
> 
> Fix this by bailing out from the OS descriptor setup request
> handling if the functions does not have OS descriptors compatibleID.
> 
> Signed-off-by: Chandana Kishori Chiluveru 
> Signed-off-by: Wesley Cheng 
> ---
>  drivers/usb/gadget/composite.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 72a9797..473edda6 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1945,6 +1945,12 @@ composite_setup(struct usb_gadget *gadget, const 
> struct usb_ctrlrequest *ctrl)
>   buf[6] = w_index;
>   /* Number of ext compat interfaces */
>   count = count_ext_compat(os_desc_cfg);
> + /*
> +  * Bailout if device does not
> +  * have ext_compat interfaces.
> +  */
> + if (count == 0)
> + break;
>   buf[8] = count;
>   count *= 24; /* 24 B/ext compat desc */
>   count += 16; /* header */

Do we still need this fix? IIRC we had this change in our downstream
kernel to fix the case when dynamically re-configuring ConfigFS, i.e.
changing the composition of functions wherein none of the interfaces
support OS Descriptors, so this causes count_ext_compat() to return
0 and results in the issue described in $SUBJECT.

But I think this is more of a problem of an improperly configured
ConfigFS gadget. If userspace instead removes the config from the
gadget's os_desc subdirectory that should cause cdev->os_desc_config to
be set to NULL and hence composite_setup() should never enter this
handling at all, right?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usb: dwc3: gadget: Prevent EP queuing while stopping transfers

[Jack Pham]

Hi Wesley,

On Wed, Mar 10, 2021 at 03:02:10AM -0800, Wesley Cheng wrote:
> In the situations where the DWC3 gadget stops active transfers, once
> calling the dwc3_gadget_giveback(), there is a chance where a function
> driver can queue a new USB request in between the time where the dwc3
> lock has been released and re-aquired.  This occurs after we've already
> issued an ENDXFER command.  When the stop active transfers continues
> to remove USB requests from all dep lists, the newly added request will
> also be removed, while controller still has an active TRB for it.
> This can lead to the controller accessing an unmapped memory address.
> 
> Fix this by ensuring parameters to prevent EP queuing are set before
> calling the stop active transfers API.

Is it correct to say this Fixes: ae7e86108b12 ("usb: dwc3: Stop active
transfers before halting the controller") ?

Jack

> Signed-off-by: Wesley Cheng 
> ---
>  drivers/usb/dwc3/gadget.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 4780983..4d98fbf 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -783,8 +783,6 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>  
>   trace_dwc3_gadget_ep_disable(dep);
>  
> - dwc3_remove_requests(dwc, dep);
> -
>   /* make sure HW endpoint isn't stalled */
>   if (dep->flags & DWC3_EP_STALL)
>   __dwc3_gadget_ep_set_halt(dep, 0, false);
> @@ -803,6 +801,8 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>   dep->endpoint.desc = NULL;
>   }
>  
> + dwc3_remove_requests(dwc, dep);
> +
>   return 0;
>  }
>  
> @@ -1617,7 +1617,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
> struct dwc3_request *req)
>  {
>   struct dwc3 *dwc = dep->dwc;
>  
> - if (!dep->endpoint.desc || !dwc->pullups_connected) {
> + if (!dep->endpoint.desc || !dwc->pullups_connected || !dwc->connected) {
>   dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n",
>   dep->name);
>   return -ESHUTDOWN;
> @@ -2247,6 +2247,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int 
> is_on)
>   if (!is_on) {
>   u32 count;
>  
> + dwc->connected = false;
>   /*
>* In the Synopsis DesignWare Cores USB3 Databook Rev. 3.30a
>* Section 4.1.8 Table 4-7, it states that for a 
> device-initiated
> @@ -3329,8 +3330,6 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 
> *dwc)
>  {
>   u32 reg;
>  
> - dwc->connected = true;
> -
>   /*
>* WORKAROUND: DWC3 revisions <1.88a have an issue which
>* would cause a missing Disconnect Event if there's a
> @@ -3370,6 +3369,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 
> *dwc)
>* transfers."
>*/
>   dwc3_stop_active_transfers(dwc);
> + dwc->connected = true;
>  
>   reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>   reg &= ~DWC3_DCTL_TSTCTRL_MASK;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/6] arm64: dts: qcom: sm8350: add USB and PHY device nodes

[Jack Pham]

Hi Vinod,

On Thu, Feb 04, 2021 at 10:39:03PM +0530, Vinod Koul wrote:
> From: Jack Pham 
> 
> Add device nodes for the two instances each of USB3 controllers,
> QMP SS PHYs and SNPS HS PHYs.
> 
> Signed-off-by: Jack Pham 
> Message-Id: <20210116013802.1609-2-ja...@codeaurora.org>
> Signed-off-by: Vinod Koul 
> ---
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 179 +++
>  1 file changed, 179 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi 
> b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index e3597e2a22ab..e51d9ca0210c 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -592,6 +592,185 @@ rpmhcc: clock-controller {
>   };
>  
>   };
> +
> + usb_1_hsphy: phy@88e3000 {
> + compatible = "qcom,sm8350-usb-hs-phy",
> +  "qcom,usb-snps-hs-7nm-phy";
> + reg = <0 0x088e3000 0 0x400>;
> + status = "disabled";
> + #phy-cells = <0>;
> +
> + clocks = < RPMH_CXO_CLK>;
> + clock-names = "ref";
> +
> + resets = < 20>;

Shouldn't this (and all the other gcc phandles below) use the
dt-bindings macros from here?
https://patchwork.kernel.org/project/linux-arm-msm/patch/20210118044321.2571775-5-vk...@kernel.org/

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/4] phy: qcom-qmp: Add UFS v4 registers found in SM8350

[Jack Pham]

Hi Vinod,

On Mon, Jan 25, 2021 at 03:39:05PM +0530, Vinod Koul wrote:
> Add the registers for few new registers found in SM8350. Also the UFS
> phy used in SM8350 seems to have different offsets than V4 phy, although
> it claims it is v4 phy, so add the new offsets with SM8350 tag instead
> of V4 tag.

Actually I believe SM8350 UFS PHY is on V5, as the internal IP revision
shows 5.0.0. So IMO some of the below definitions should just be using
the V5 macros for consistency with the ones I recently added for USB3.

And like USB3 QMP, it seems we have mixed usage of V4/V5 macros in the
sequence tables, mainly wherever the offsets are identical between IP
revisions. Hope this doesn't turn out to be a maintenance nightmare...

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.h | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.h 
> b/drivers/phy/qualcomm/phy-qcom-qmp.h
> index dff7be5a1cc1..bba1d5e3eb73 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.h
> @@ -451,6 +451,7 @@
>  #define QSERDES_V4_TX_RES_CODE_LANE_OFFSET_RX0x40
>  #define QSERDES_V4_TX_LANE_MODE_10x84
>  #define QSERDES_V4_TX_LANE_MODE_20x88
> +#define QSERDES_V4_TX_LANE_MODE_30x8C
>  #define QSERDES_V4_TX_RCV_DETECT_LVL_2   0x9c
>  #define QSERDES_V4_TX_PWM_GEAR_1_DIVIDER_BAND0_1 0xd8
>  #define QSERDES_V4_TX_PWM_GEAR_2_DIVIDER_BAND0_1 0xdC
> @@ -459,6 +460,13 @@
>  #define QSERDES_V4_TX_TRAN_DRVR_EMP_EN   0xb8
>  #define QSERDES_V4_TX_PI_QEC_CTRL0x104
>  
> +/* Only for SM8350 QMP V4 Phy TX offsets different from V4 */
> +#define QSERDES_SM8350_TX_PWM_GEAR_1_DIVIDER_BAND0_1 0x178
> +#define QSERDES_SM8350_TX_PWM_GEAR_2_DIVIDER_BAND0_1 0x17c
> +#define QSERDES_SM8350_TX_PWM_GEAR_3_DIVIDER_BAND0_1 0x180
> +#define QSERDES_SM8350_TX_PWM_GEAR_4_DIVIDER_BAND0_1 0x184
> +#define QSERDES_SM8350_TX_TRAN_DRVR_EMP_EN   0xc0

These could be augmented to the V5 TX defintions? Although they are not
present for USB, so not sure if you want to add "V5_UFS" to the prefix.
But since they are at offsets past the end of the USB TX bank it should
also be ok to share in the QSERDES_V5_TX namespace.

> +
>  /* Only for QMP V4 PHY - RX registers */
>  #define QSERDES_V4_RX_UCDR_FO_GAIN   0x008
>  #define QSERDES_V4_RX_UCDR_SO_GAIN   0x014
> @@ -514,6 +522,24 @@
>  #define QSERDES_V4_RX_DCC_CTRL1  0x1bc
>  #define QSERDES_V4_RX_VTH_CODE   0x1c4
>  
> +/* Only for SM8350 QMP V4 Phy RX offsets different from V4 */
> +#define QSERDES_SM8350_RX_RX_MODE_00_LOW 0x15c
> +#define QSERDES_SM8350_RX_RX_MODE_00_HIGH0x160
> +#define QSERDES_SM8350_RX_RX_MODE_00_HIGH2   0x164
> +#define QSERDES_SM8350_RX_RX_MODE_00_HIGH3   0x168
> +#define QSERDES_SM8350_RX_RX_MODE_00_HIGH4   0x16c
> +#define QSERDES_SM8350_RX_RX_MODE_01_LOW 0x170
> +#define QSERDES_SM8350_RX_RX_MODE_01_HIGH0x174
> +#define QSERDES_SM8350_RX_RX_MODE_01_HIGH2   0x178
> +#define QSERDES_SM8350_RX_RX_MODE_01_HIGH3   0x17c
> +#define QSERDES_SM8350_RX_RX_MODE_01_HIGH4   0x180
> +#define QSERDES_SM8350_RX_RX_MODE_10_LOW 0x184
> +#define QSERDES_SM8350_RX_RX_MODE_10_HIGH0x188
> +#define QSERDES_SM8350_RX_RX_MODE_10_HIGH2   0x18c
> +#define QSERDES_SM8350_RX_RX_MODE_10_HIGH3   0x190
> +#define QSERDES_SM8350_RX_RX_MODE_10_HIGH4   0x194
> +#define QSERDES_SM8350_RX_DCC_CTRL1  0x1a8

These are identical to the "V5" offsets I had added for SM8350 USB.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v6 1/4] usb: gadget: udc: core: Introduce check_config to verify USB configuration

[Jack Pham]

Hi Wesley,

On Thu, Jan 21, 2021 at 08:01:37PM -0800, Wesley Cheng wrote:
> Some UDCs may have constraints on how many high bandwidth endpoints it can
> support in a certain configuration.  This API allows for the composite
> driver to pass down the total number of endpoints to the UDC so it can verify
> it has the required resources to support the configuration.
> 
> Signed-off-by: Wesley Cheng 
> ---
>  drivers/usb/gadget/udc/core.c | 9 +
>  include/linux/usb/gadget.h| 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 4173acd..469962f 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1003,6 +1003,15 @@ int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
>  
> +int usb_gadget_check_config(struct usb_gadget *gadget, unsigned long ep_map)

You should probably add a kernel-doc for this function.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3] dt-bindings: usb: qcom,dwc3: Add bindings for SM8150, SM8250, SM8350

[Jack Pham]

Add compatible strings for the USB DWC3 controller on QCOM SM8150,
SM8250 and SM8350 SoCs.

Note the SM8150 & SM8250 compatibles are already being used in the
dts but was missing from the documentation.

Acked-by: Felipe Balbi 
Signed-off-by: Jack Pham 
---
v3: Resend of #4/4 of 
https://lore.kernel.org/linux-usb/20210115174723.7424-1-ja...@codeaurora.org
added Felipe's Ack & rebased on gregkh/usb-testing

 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml 
b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index dd1d8bcd9254..c3cbd1fa9944 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -18,6 +18,9 @@ properties:
   - qcom,sc7180-dwc3
   - qcom,sdm845-dwc3
   - qcom,sdx55-dwc3
+  - qcom,sm8150-dwc3
+  - qcom,sm8250-dwc3
+  - qcom,sm8350-dwc3
   - const: qcom,dwc3
 
   reg:
-- 
2.24.0

[PATCH v2 3/4] dt-bindings: phy: qcom,usb-snps-femto-v2: Add SM8250 and SM8350 bindings

[Jack Pham]

Add the compatible strings for the USB2 PHYs found on QCOM
SM8250 & SM8350 SoCs.

Note that the SM8250 compatible is already in use in the dts and
driver implementation but was missing from the documentation.

Signed-off-by: Jack Pham 
---
 .../devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml 
b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
index 4949a2851532..ee77c6458326 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
@@ -17,6 +17,8 @@ properties:
 enum:
   - qcom,usb-snps-hs-7nm-phy
   - qcom,sm8150-usb-hs-phy
+  - qcom,sm8250-usb-hs-phy
+  - qcom,sm8350-usb-hs-phy
   - qcom,usb-snps-femto-v2-phy
 
   reg:
-- 
2.24.0

[PATCH v2 2/4] phy: qcom-qmp: Add SM8350 USB QMP PHYs

[Jack Pham]

Add support for the USB DP & UNI PHYs found on SM8350. These use
version 5.0.0 of the QMP PHY IP and thus require new "V5"
definitions of the register offset macros for the QSERDES RX
and TX blocks. The QSERDES common and QPHY PCS blocks' register
offsets are largely unchanged from V4 so some of the existing
macros can be reused.

Signed-off-by: Jack Pham 
---
v2: Added missing to sm8350_usb3_uniphy entry to qcom_qmp_phy_of_match_table

 drivers/phy/qualcomm/phy-qcom-qmp.c | 212 
 drivers/phy/qualcomm/phy-qcom-qmp.h | 100 +
 2 files changed, 312 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index f103b14f983e..d7f0d47f774f 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -216,6 +216,15 @@ static const unsigned int 
qmp_v4_usb3_uniphy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR]  = 0x614,
 };
 
+static const unsigned int sm8350_usb3_uniphy_regs_layout[QPHY_LAYOUT_SIZE] = {
+   [QPHY_SW_RESET] = 0x00,
+   [QPHY_START_CTRL]   = 0x44,
+   [QPHY_PCS_STATUS]   = 0x14,
+   [QPHY_PCS_POWER_DOWN_CONTROL]   = 0x40,
+   [QPHY_PCS_AUTONOMOUS_MODE_CTRL] = 0x1008,
+   [QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR]  = 0x1014,
+};
+
 static const unsigned int sdm845_ufsphy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_START_CTRL]   = 0x00,
[QPHY_PCS_READY_STATUS] = 0x160,
@@ -2025,6 +2034,144 @@ static const struct qmp_phy_init_tbl 
sdx55_usb3_uniphy_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V4_RX_GM_CAL, 0x1f),
 };
 
+static const struct qmp_phy_init_tbl sm8350_usb3_tx_tbl[] = {
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_TX, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_RX, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_OFFSET_TX, 0x16),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_OFFSET_RX, 0x0e),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_1, 0x35),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_3, 0x3f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_4, 0x7f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_5, 0x3f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RCV_DETECT_LVL_2, 0x12),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_PI_QEC_CTRL, 0x21),
+};
+
+static const struct qmp_phy_init_tbl sm8350_usb3_rx_tbl[] = {
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FO_GAIN, 0x0a),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SO_GAIN, 0x05),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FASTLOCK_FO_GAIN, 0x2f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x7f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FASTLOCK_COUNT_LOW, 0xff),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FASTLOCK_COUNT_HIGH, 0x0f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_PI_CONTROLS, 0x99),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_THRESH1, 0x08),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_THRESH2, 0x08),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_GAIN1, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_GAIN2, 0x04),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_VGA_CAL_CNTRL1, 0x54),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_VGA_CAL_CNTRL2, 0x0f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQU_ADAPTOR_CNTRL2, 0x0f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4a),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQU_ADAPTOR_CNTRL4, 0x0a),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_IDAC_TSETTLE_LOW, 0xc0),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_IDAC_TSETTLE_HIGH, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x47),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_SIGDET_CNTRL, 0x04),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_LOW, 0xbb),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_HIGH, 0x7b),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_HIGH2, 0xbb),
+   QMP_PHY_INIT_CFG_LANE(QSERDES_V5_RX_RX_MODE_00_HIGH3, 0x3d, 1),
+   QMP_PHY_INIT_CFG_LANE(QSERDES_V5_RX_RX_MODE_00_HIGH3, 0x3c, 2),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_HIGH4, 0xdb),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_LOW, 0x64),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH, 0x24),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH2, 0xd2),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH3, 0x13),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH4, 0xa9),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_DFE_EN_TIMER, 0x04),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_DFE_CTLE_POST_CAL_OFFSET, 0x38),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_AUX_DATA_TCOARSE_TFINE, 0xa0),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_DCC_CTRL1, 0x0c),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_GM_CAL, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_VTH_CODE, 0x10),
+};
+
+static const struct qmp_phy_init_tbl sm835

[PATCH v2 4/4] dt-bindings: usb: qcom,dwc3: Add bindings for SM8150, SM8250, SM8350

[Jack Pham]

Add compatible strings for the USB DWC3 controller on QCOM SM8150,
SM8250 and SM8350 SoCs.

Note the SM8150 & SM8250 compatibles are already being used in the
dts but was missing from the documentation.

Signed-off-by: Jack Pham 
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml 
b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 2cf525d21e05..da47f43d6b04 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -17,6 +17,9 @@ properties:
   - qcom,msm8998-dwc3
   - qcom,sc7180-dwc3
   - qcom,sdm845-dwc3
+  - qcom,sm8150-dwc3
+  - qcom,sm8250-dwc3
+  - qcom,sm8350-dwc3
   - const: qcom,dwc3
 
   reg:
-- 
2.24.0

[PATCH v2 1/4] dt-bindings: phy: qcom,qmp: Add SM8150, SM8250 and SM8350 USB PHY bindings

[Jack Pham]

Add the compatible strings for the USB3 PHYs found on SM8150, SM8250
and SM8350 SoCs. These require separate subschemas due to the different
required clock entries.

Note the SM8150 and SM8250 compatibles have already been in place in
the dts as well as the driver implementation but were missing from
the documentation.

Signed-off-by: Jack Pham 
---
 .../devicetree/bindings/phy/qcom,qmp-phy.yaml | 67 +++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml 
b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
index 390df23b82e7..841c72863b4f 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
@@ -30,15 +30,24 @@ properties:
   - qcom,sdm845-qmp-ufs-phy
   - qcom,sdm845-qmp-usb3-uni-phy
   - qcom,sm8150-qmp-ufs-phy
+  - qcom,sm8150-qmp-usb3-phy
+  - qcom,sm8150-qmp-usb3-uni-phy
   - qcom,sm8250-qmp-ufs-phy
   - qcom,sm8250-qmp-gen3x1-pcie-phy
   - qcom,sm8250-qmp-gen3x2-pcie-phy
   - qcom,sm8250-qmp-modem-pcie-phy
+  - qcom,sm8250-qmp-usb3-phy
+  - qcom,sm8250-qmp-usb3-uni-phy
+  - qcom,sm8350-qmp-usb3-phy
+  - qcom,sm8350-qmp-usb3-uni-phy
   - qcom,sdx55-qmp-usb3-uni-phy
 
   reg:
+minItems: 1
+maxItems: 2
 items:
   - description: Address and length of PHY's common serdes block.
+  - description: Address and length of PHY's DP_COM control block.
 
   "#clock-cells":
 enum: [ 1, 2 ]
@@ -287,6 +296,64 @@ allOf:
 reset-names:
   items:
 - const: phy
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - qcom,sm8150-qmp-usb3-phy
+  - qcom,sm8150-qmp-usb3-uni-phy
+  - qcom,sm8250-qmp-usb3-uni-phy
+  - qcom,sm8350-qmp-usb3-uni-phy
+then:
+  properties:
+clocks:
+  items:
+- description: Phy aux clock.
+- description: 19.2 MHz ref clk source.
+- description: 19.2 MHz ref clk.
+- description: Phy common block aux clock.
+clock-names:
+  items:
+- const: aux
+- const: ref_clk_src
+- const: ref
+- const: com_aux
+resets:
+  items:
+- description: reset of phy block.
+- description: phy common block reset.
+reset-names:
+  items:
+- const: phy
+- const: common
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - qcom,sm8250-qmp-usb3-phy
+  - qcom,sm8350-qmp-usb3-phy
+then:
+  properties:
+clocks:
+  items:
+- description: Phy aux clock.
+- description: 19.2 MHz ref clk.
+- description: Phy common block aux clock.
+clock-names:
+  items:
+- const: aux
+- const: ref_clk_src
+- const: com_aux
+resets:
+  items:
+- description: reset of phy block.
+- description: phy common block reset.
+reset-names:
+  items:
+- const: phy
+- const: common
 
 examples:
   - |
-- 
2.24.0

[PATCH v2 0/4] SM8350 USB and dt-bindings updates

[Jack Pham]

This series adds support for the SM8350 USB PHY to the QMP PHY driver
as well as adds the documentation for the QMP, SNPS PHY and DWC3
controller bindings. This also adds the bindings for SM8150 and SM8250
to the same docs which had not been added previously even though they
are in use now.

v2: Reordered Patches 1 & 2; added missing entry to match_table

Jack Pham (4):
  dt-bindings: phy: qcom,qmp: Add SM8150, SM8250 and SM8350 USB PHY
bindings
  phy: qcom-qmp: Add SM8350 USB QMP PHYs
  dt-bindings: phy: qcom,usb-snps-femto-v2: Add SM8250 and SM8350
bindings
  dt-bindings: usb: qcom,dwc3: Add bindings for SM8150, SM8250, SM8350

 .../devicetree/bindings/phy/qcom,qmp-phy.yaml |  67 ++
 .../bindings/phy/qcom,usb-snps-femto-v2.yaml  |   2 +
 .../devicetree/bindings/usb/qcom,dwc3.yaml|   3 +
 drivers/phy/qualcomm/phy-qcom-qmp.c   | 209 ++
 drivers/phy/qualcomm/phy-qcom-qmp.h   | 100 +
 5 files changed, 381 insertions(+)

-- 
2.24.0

Re: [PATCH 1/4] phy: qcom-qmp: Add SM8350 USB QMP PHYs

[Jack Pham]

Hi Vinod,

On Fri, Jan 15, 2021 at 06:17:36PM +0530, Vinod Koul wrote:
> On 15-01-21, 12:54, Konrad Dybcio wrote:
> > I might be wrong but it looks as if you forgot to add a compatible
> > for the "sm8350_usb3_uniphy_cfg" configuration.

I believe Konrad was referring to the driver in which I had neglected to
add the compatible to the qcom_qmp_phy_of_match_table. My mistake.

> It seems to be documented in patch 2, ideally we should have the
> bindings patches first and this as patch 3...

Ok. I think driver change would be patch 2 rather, with the bindings in
patch 1? Patch 3 and 4 are dt-bindings updates to the SNPS Femto PHY and
DWC3 QCOM docs respectively.

Will send v2, thanks.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/4] phy: qcom-qmp: Add SM8350 USB QMP PHYs

[Jack Pham]

On Fri, Jan 15, 2021 at 12:54:26PM +0100, Konrad Dybcio wrote:
> I might be wrong but it looks as if you forgot to add a compatible for
> the "sm8350_usb3_uniphy_cfg" configuration.

Not wrong at all! My mistake, will add it in v2.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/4] dt-bindings: phy: qcom,qmp: Add SM8150, SM8250 and SM8350 USB PHY bindings

[Jack Pham]

Add the compatible strings for the USB3 PHYs found on SM8150, SM8250
and SM8350 SoCs.

Note the SM8150 and SM8250 compatibles have already been in place in
the dts as well as the driver implementation but were missing from
the documentation.

Signed-off-by: Jack Pham 
---
 .../devicetree/bindings/phy/qcom,qmp-phy.yaml | 67 +++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml 
b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
index 390df23b82e7..841c72863b4f 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
@@ -30,15 +30,24 @@ properties:
   - qcom,sdm845-qmp-ufs-phy
   - qcom,sdm845-qmp-usb3-uni-phy
   - qcom,sm8150-qmp-ufs-phy
+  - qcom,sm8150-qmp-usb3-phy
+  - qcom,sm8150-qmp-usb3-uni-phy
   - qcom,sm8250-qmp-ufs-phy
   - qcom,sm8250-qmp-gen3x1-pcie-phy
   - qcom,sm8250-qmp-gen3x2-pcie-phy
   - qcom,sm8250-qmp-modem-pcie-phy
+  - qcom,sm8250-qmp-usb3-phy
+  - qcom,sm8250-qmp-usb3-uni-phy
+  - qcom,sm8350-qmp-usb3-phy
+  - qcom,sm8350-qmp-usb3-uni-phy
   - qcom,sdx55-qmp-usb3-uni-phy
 
   reg:
+minItems: 1
+maxItems: 2
 items:
   - description: Address and length of PHY's common serdes block.
+  - description: Address and length of PHY's DP_COM control block.
 
   "#clock-cells":
 enum: [ 1, 2 ]
@@ -287,6 +296,64 @@ allOf:
 reset-names:
   items:
 - const: phy
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - qcom,sm8150-qmp-usb3-phy
+  - qcom,sm8150-qmp-usb3-uni-phy
+  - qcom,sm8250-qmp-usb3-uni-phy
+  - qcom,sm8350-qmp-usb3-uni-phy
+then:
+  properties:
+clocks:
+  items:
+- description: Phy aux clock.
+- description: 19.2 MHz ref clk source.
+- description: 19.2 MHz ref clk.
+- description: Phy common block aux clock.
+clock-names:
+  items:
+- const: aux
+- const: ref_clk_src
+- const: ref
+- const: com_aux
+resets:
+  items:
+- description: reset of phy block.
+- description: phy common block reset.
+reset-names:
+  items:
+- const: phy
+- const: common
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - qcom,sm8250-qmp-usb3-phy
+  - qcom,sm8350-qmp-usb3-phy
+then:
+  properties:
+clocks:
+  items:
+- description: Phy aux clock.
+- description: 19.2 MHz ref clk.
+- description: Phy common block aux clock.
+clock-names:
+  items:
+- const: aux
+- const: ref_clk_src
+- const: com_aux
+resets:
+  items:
+- description: reset of phy block.
+- description: phy common block reset.
+reset-names:
+  items:
+- const: phy
+- const: common
 
 examples:
   - |
-- 
2.24.0

[PATCH 3/4] dt-bindings: phy: qcom,usb-snps-femto-v2: Add SM8250 and SM8350 bindings

[Jack Pham]

Add the compatible strings for the USB2 PHYs found on QCOM
SM8250 & SM8350 SoCs.

Note that the SM8250 compatible is already in use in the dts and
driver implementation but was missing from the documentation.

Signed-off-by: Jack Pham 
---
 .../devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml 
b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
index 4949a2851532..ee77c6458326 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
@@ -17,6 +17,8 @@ properties:
 enum:
   - qcom,usb-snps-hs-7nm-phy
   - qcom,sm8150-usb-hs-phy
+  - qcom,sm8250-usb-hs-phy
+  - qcom,sm8350-usb-hs-phy
   - qcom,usb-snps-femto-v2-phy
 
   reg:
-- 
2.24.0

[PATCH 0/4] SM8350 USB and dt-bindings updates

[Jack Pham]

This series adds support for the SM8350 USB PHY to the QMP PHY driver
as well as adds the documentation for the QMP, SNPS PHY and DWC3
controller bindings. This also adds the bindings for SM8150 and SM8250
to the same docs which had not been added previously even though they
are in use now.

Jack Pham (4):
  phy: qcom-qmp: Add SM8350 USB QMP PHYs
  dt-bindings: phy: qcom,qmp: Add SM8150, SM8250 and SM8350 USB PHY
bindings
  dt-bindings: phy: qcom,usb-snps-femto-v2: Add SM8250 and SM8350
bindings
  dt-bindings: usb: qcom,dwc3: Add bindings for SM8150, SM8250, SM8350

 .../devicetree/bindings/phy/qcom,qmp-phy.yaml |  67 ++
 .../bindings/phy/qcom,usb-snps-femto-v2.yaml  |   2 +
 .../devicetree/bindings/usb/qcom,dwc3.yaml|   3 +
 drivers/phy/qualcomm/phy-qcom-qmp.c   | 209 ++
 drivers/phy/qualcomm/phy-qcom-qmp.h   | 100 +
 5 files changed, 381 insertions(+)

-- 
2.24.0

[PATCH 4/4] dt-bindings: usb: qcom,dwc3: Add bindings for SM8150, SM8250, SM8350

[Jack Pham]

Add compatible strings for the USB DWC3 controller on QCOM SM8150,
SM8250 and SM8350 SoCs.

Note the SM8150 & SM8250 compatibles are already being used in the
dts and driver implementation but was missing from the documentation.

Signed-off-by: Jack Pham 
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml 
b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 2cf525d21e05..da47f43d6b04 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -17,6 +17,9 @@ properties:
   - qcom,msm8998-dwc3
   - qcom,sc7180-dwc3
   - qcom,sdm845-dwc3
+  - qcom,sm8150-dwc3
+  - qcom,sm8250-dwc3
+  - qcom,sm8350-dwc3
   - const: qcom,dwc3
 
   reg:
-- 
2.24.0

[PATCH 1/4] phy: qcom-qmp: Add SM8350 USB QMP PHYs

[Jack Pham]

Add support for the USB DP & UNI PHYs found on SM8350. These use
version 5.0.0 of the QMP PHY IP and thus require new "V5"
definitions of the register offset macros for the QSERDES RX
and TX blocks. The QSERDES common and QPHY PCS blocks' register
offsets are largely unchanged from V4 so some of the existing
macros can be reused.

Signed-off-by: Jack Pham 
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 209 
 drivers/phy/qualcomm/phy-qcom-qmp.h | 100 +
 2 files changed, 309 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index f103b14f983e..b2f90bf5c212 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -216,6 +216,15 @@ static const unsigned int 
qmp_v4_usb3_uniphy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR]  = 0x614,
 };
 
+static const unsigned int sm8350_usb3_uniphy_regs_layout[QPHY_LAYOUT_SIZE] = {
+   [QPHY_SW_RESET] = 0x00,
+   [QPHY_START_CTRL]   = 0x44,
+   [QPHY_PCS_STATUS]   = 0x14,
+   [QPHY_PCS_POWER_DOWN_CONTROL]   = 0x40,
+   [QPHY_PCS_AUTONOMOUS_MODE_CTRL] = 0x1008,
+   [QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR]  = 0x1014,
+};
+
 static const unsigned int sdm845_ufsphy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_START_CTRL]   = 0x00,
[QPHY_PCS_READY_STATUS] = 0x160,
@@ -2025,6 +2034,144 @@ static const struct qmp_phy_init_tbl 
sdx55_usb3_uniphy_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V4_RX_GM_CAL, 0x1f),
 };
 
+static const struct qmp_phy_init_tbl sm8350_usb3_tx_tbl[] = {
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_TX, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_RX, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_OFFSET_TX, 0x16),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RES_CODE_LANE_OFFSET_RX, 0x0e),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_1, 0x35),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_3, 0x3f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_4, 0x7f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_LANE_MODE_5, 0x3f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_RCV_DETECT_LVL_2, 0x12),
+   QMP_PHY_INIT_CFG(QSERDES_V5_TX_PI_QEC_CTRL, 0x21),
+};
+
+static const struct qmp_phy_init_tbl sm8350_usb3_rx_tbl[] = {
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FO_GAIN, 0x0a),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SO_GAIN, 0x05),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FASTLOCK_FO_GAIN, 0x2f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x7f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FASTLOCK_COUNT_LOW, 0xff),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_FASTLOCK_COUNT_HIGH, 0x0f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_PI_CONTROLS, 0x99),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_THRESH1, 0x08),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_THRESH2, 0x08),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_GAIN1, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_UCDR_SB2_GAIN2, 0x04),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_VGA_CAL_CNTRL1, 0x54),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_VGA_CAL_CNTRL2, 0x0f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQU_ADAPTOR_CNTRL2, 0x0f),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4a),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQU_ADAPTOR_CNTRL4, 0x0a),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_IDAC_TSETTLE_LOW, 0xc0),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_IDAC_TSETTLE_HIGH, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x47),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_SIGDET_CNTRL, 0x04),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_LOW, 0xbb),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_HIGH, 0x7b),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_HIGH2, 0xbb),
+   QMP_PHY_INIT_CFG_LANE(QSERDES_V5_RX_RX_MODE_00_HIGH3, 0x3d, 1),
+   QMP_PHY_INIT_CFG_LANE(QSERDES_V5_RX_RX_MODE_00_HIGH3, 0x3c, 2),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_00_HIGH4, 0xdb),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_LOW, 0x64),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH, 0x24),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH2, 0xd2),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH3, 0x13),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_RX_MODE_01_HIGH4, 0xa9),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_DFE_EN_TIMER, 0x04),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_DFE_CTLE_POST_CAL_OFFSET, 0x38),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_AUX_DATA_TCOARSE_TFINE, 0xa0),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_DCC_CTRL1, 0x0c),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_GM_CAL, 0x00),
+   QMP_PHY_INIT_CFG(QSERDES_V5_RX_VTH_CODE, 0x10),
+};
+
+static const struct qmp_phy_init_tbl sm8350_usb3_pcs_tbl[] = {
+   QMP_PHY_INIT_CFG(QPHY_V5_PCS_USB3_RCVR_DTCT_

Re: [PATCH 2/3] usb: gadget: composite: Split composite reset and disconnect

[Jack Pham]

Hi Thinh,

On Fri, Jan 08, 2021 at 02:19:30AM +, Thinh Nguyen wrote:
> Hi Wesley,
> 
> Felipe Balbi wrote:
> > Hi,
> >
> > Wesley Cheng  writes:
> >> +void composite_reset(struct usb_gadget *gadget)
> >> +{
> >> +  /*
> >> +   * Section 1.4.13 Standard Downstream Port of the USB battery charging
> >> +   * specification v1.2 states that a device connected on a SDP shall only
> >> +   * draw at max 100mA while in a connected, but unconfigured state.
> > The requirements are different, though. I think OTG spec has some extra
> > requirements where only 8mA can be drawn max. You need to check for the
> > otg flag. Moreover, USB3+ spec has units of 150mA meaning the device
> > can't draw 100mA (IIRC).
> >
> 
> We see issue with this patch series. For our device running at SSP, the
> device couldn't recover from a port reset and remained in eSS.Inactive
> state.
> 
> This patch series is already in Greg's usb-testing. Please review and
> help fix it.
> 
> We can see the failure once the patch "usb: gadget: configfs: Add a
> specific configFS reset callback" is introduced.

Hmm. Does your device use a legacy USB2 PHY (not generic PHY) driver,
and does it implement the usb_phy_set_power() callback? Because
otherwise this new configfs_composite_reset() callback would not have
changed from the original behavior since the newly introduced (in patch
1/3) dwc3_gadget_vbus_draw() callback would simply be a no-op if
dwc->usb2_phy is not present.

If it does turn out to be something with your PHY driver's set_power(),
it's still puzzling since it's directed to only the usb2_phy, so I'm
curious how telling it to draw 100mA could affect SSP operation at all.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req

[Jack Pham]

Hi Greg and Jerome,

On Mon, Dec 28, 2020 at 04:01:46PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 21, 2020 at 06:35:30PM +0100, Jerome Brunet wrote:
> > 'struct uac_req' purpose is to link 'struct usb_request' to the
> > corresponding 'struct uac_rtd_params'. However member req is never
> > used. Using the context of the usb request, we can keep track of the
> > corresponding 'struct uac_rtd_params' just as well, without allocating
> > extra memory.
> > 
> > Signed-off-by: Jerome Brunet 
> > ---
> >  drivers/usb/gadget/function/u_audio.c | 58 ---
> >  1 file changed, 26 insertions(+), 32 deletions(-)
> 
> This patch doesn't apply, so I can't apply patches 3 or 4 of this series
> :(
> 
> Can you rebase against my usb-testing branch and resend?

>From the cover letter:

On Mon, Dec 21, 2020 at 06:35:27PM +0100, Jerome Brunet wrote:
> The series depends on this fix [0] by Jack Pham to apply cleanly
> 
> [0]: 
> https://lore.kernel.org/linux-usb/20201029175949.6052-1-ja...@codeaurora.org/

My patch hadn't been picked up by Felipe, so it's not in your tree
either, Greg. Should I just resend it to you first?  Or shall I invite
Jerome to just include it in v2 of this series? 

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 4/5] USB: gadget: f_fs: add SuperSpeed Plus support

[Jack Pham]

On Fri, Nov 27, 2020 at 03:05:58PM +0100, Greg Kroah-Hartman wrote:
> From: "taehyun.cho" 
> 
> Setup the descriptors for SuperSpeed Plus for f_fs. This allows the
> gadget to work properly without crashing at SuperSpeed rates.
> 
> Cc: Felipe Balbi 
> Cc: stable 
> Signed-off-by: taehyun.cho 
> Signed-off-by: Will McVicker 
> Reviewed-by: Peter Chen 
> Signed-off-by: Greg Kroah-Hartman 
> ---
>  drivers/usb/gadget/function/f_fs.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c 
> b/drivers/usb/gadget/function/f_fs.c
> index 046f770a76da..a34a7c96a1ab 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1327,6 +1327,7 @@ static long ffs_epfile_ioctl(struct file *file, 
> unsigned code,
>   struct usb_endpoint_descriptor *desc;
>  
>   switch (epfile->ffs->gadget->speed) {
> + case USB_SPEED_SUPER_PLUS:
>   case USB_SPEED_SUPER:
>   desc_idx = 2;
>   break;
> @@ -3222,6 +3223,10 @@ static int _ffs_func_bind(struct usb_configuration *c,
>   func->function.os_desc_n =
>   c->cdev->use_os_string ? ffs->interfaces_count : 0;
>  
> + if (likely(super)) {
> + func->function.ssp_descriptors =
> + usb_copy_descriptors(func->function.ss_descriptors);
> + }
>   /* And we're done */
>   ffs_event_add(ffs, FUNCTIONFS_BIND);
>   return 0;
> -- 

Hi Greg,

FWIW I had sent a very similar patch[1] a while back (twice in fact)
but got no response about it. Looks like Taehyun's patch already went
through Google for this, I assume it must be working on their Android
kernels so I've no problem with you or Felipe taking this instead.

Only one difference with my patch though is mine additionally clears the
func->function.ssp_descriptors member to NULL upon ffs_func_unbind() as
otherwise it could lead to a dangling reference in case the function is
re-bound and userspace does not issue SS descriptors the next time.
Realistically I don't think that's possible, except maybe when fuzzing?

[1] 
https://patchwork.kernel.org/project/linux-usb/patch/20201027230731.9073-1-ja...@codeaurora.org/

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 6/6] arm64: boot: dts: qcom: pm8150b: Add DTS node for PMIC VBUS booster

[Jack Pham]

Hey Wesley,

On Wed, Jun 17, 2020 at 11:02:09AM -0700, Wesley Cheng wrote:
> Add the required DTS node for the USB VBUS output regulator, which is
> available on PM8150B.  This will provide the VBUS source to connected
> peripherals.
> 
> Signed-off-by: Wesley Cheng 
> ---
>  arch/arm64/boot/dts/qcom/pm8150b.dtsi   | 6 ++
>  arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 7 +++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8150b.dtsi 
> b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
> index ec44a8bc2f84..b7274d9d7341 100644
> --- a/arch/arm64/boot/dts/qcom/pm8150b.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
> @@ -22,6 +22,12 @@ power-on@800 {
>   status = "disabled";
>   };
>  
> + qcom,dcdc@1100 {
> + compatible = "qcom,pm8150b-vbus-reg";
> + status = "disabled";
> + reg = <0x1100>;
> + };
> +
>   qcom,typec@1500 {
>   compatible = "qcom,pm8150b-usb-typec";
>   status = "disabled";

Don't you also need a "usb_vbus-supply" property here under the Type-C
node pointing to the phandle of the vbus reg?

Jack

> diff --git a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts 
> b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> index 6c6325c3af59..3845d19893eb 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> @@ -426,6 +426,13 @@ _1 {
>   status = "okay";
>  };
>  
> +_bus {
> + pmic@2 {
> + qcom,dcdc@1100 {
> + status = "okay";
> + };
> +};
> +
>  _1_dwc3 {
>   dr_mode = "peripheral";
>  };
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH 1/3] usb: typec: Add QCOM PMIC typec detection driver

[Jack Pham]

Hi Wesley,

On Tue, Jun 09, 2020 at 01:58:49PM -0700, Wesley Cheng wrote:
> The QCOM SPMI typec driver handles the role and orientation detection, and
> notifies client drivers using the USB role switch framework.   It registers
> as a typec port, so orientation can be communicated using the typec switch
> APIs.  The driver also registers the VBUS output regulator, so client

Doesn't look like it.. As we discussed in earlier revisions we decided
to drop the regulator.

> drivers can enable the VBUS source when acting as a source/host.
> 
> Signed-off-by: Wesley Cheng 
> ---
>  drivers/usb/typec/Kconfig   |  11 ++
>  drivers/usb/typec/Makefile  |   1 +
>  drivers/usb/typec/qcom-pmic-typec.c | 278 
> 
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/usb/typec/qcom-pmic-typec.c
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 559dd06..8de2520 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -73,6 +73,17 @@ config TYPEC_TPS6598X
> If you choose to build this driver as a dynamically linked module, the
> module will be called tps6598x.ko.
>  
> +config TYPEC_QCOM_PMIC
> + tristate "Qualcomm PMIC USB typec driver"
> + depends on ARCH_QCOM
> + help
> +   Driver for supporting role switch over the Qualcomm PMIC.  This will
> +   handle the type C role and orientation detection reported by the QCOM
> +   PMIC if the PMIC has the capability to handle type C detection.
> +
> +   It will also enable the VBUS output to connected devices when a
> +   DFP connection is made.
> +
>  source "drivers/usb/typec/mux/Kconfig"
>  
>  source "drivers/usb/typec/altmodes/Kconfig"
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 7753a5c3..cceffd9 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_TYPEC_TCPM)  += tcpm/
>  obj-$(CONFIG_TYPEC_UCSI) += ucsi/
>  obj-$(CONFIG_TYPEC_HD3SS3220)+= hd3ss3220.o
>  obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
> +obj-$(CONFIG_TYPEC_QCOM_PMIC)+= qcom-pmic-typec.o
>  obj-$(CONFIG_TYPEC)  += mux/
> diff --git a/drivers/usb/typec/qcom-pmic-typec.c 
> b/drivers/usb/typec/qcom-pmic-typec.c
> new file mode 100644
> index 000..ce6319c
> --- /dev/null
> +++ b/drivers/usb/typec/qcom-pmic-typec.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DCDC_BASE0x1100

along with USB_BASE @ 0x1300, is it ok to allow this driver to access
registers outside of its 'reg' base (0x1500 according to the DT
bindings)?

> +#define CMD_OTG  (DCDC_BASE + 0x40)
> +#define OTG_EN   BIT(0)
> +#define OTG_CFG  (DCDC_BASE + 0x53)
> +#define OTG_EN_SRC_CFG   BIT(1)
> +
> +#define USB_BASE 0x1300
> +#define TYPE_C_CFG_REG   (USB_BASE + 0x58)
> +#define BC12_START_ON_CC BIT(7)
> +
> +#define TYPEC_BASE   0x1500
> +#define TYPEC_MISC_STATUS(TYPEC_BASE + 0xb)
> +#define CC_ATTACHED  BIT(0)
> +#define CC_ORIENTATION   BIT(1)
> +#define SNK_SRC_MODE BIT(6)
> +#define TYPEC_MODE_CFG   (TYPEC_BASE + 0x44)
> +#define TYPEC_DISABLE_CMDBIT(0)
> +#define EN_SNK_ONLY  BIT(1)
> +#define EN_SRC_ONLY  BIT(2)
> +#define EN_TRY_SNK   BIT(4)
> +#define TYPEC_VCONN_CONTROL  (TYPEC_BASE + 0x46)
> +#define VCONN_EN_SRC BIT(0)
> +#define VCONN_EN_VAL BIT(1)
> +#define TYPEC_EXIT_STATE_CFG (TYPEC_BASE + 0x50)
> +#define SEL_SRC_UPPER_REFBIT(2)
> +#define TYPEC_INTR_EN_CFG_1  (TYPEC_BASE + 0x5e)
> +#define TYPEC_INTR_EN_CFG_1_MASK GENMASK(0, 7)
> +
> +struct qcom_pmic_typec {
> + struct device   *dev;
> + struct fwnode_handle*fwnode;
> + struct regmap   *regmap;
> + struct work_struct  bh_work;
> +
> + struct typec_capability *cap;
> + struct typec_port   *port;
> + struct usb_role_switch *role_sw;
> +
> + struct regulator_desc usb_vbus_rdesc;
> + struct regulator_dev *usb_vbus_reg;

These aren't used...leftovers from earlier revisions?

> +};
> +
> +static int qcom_pmic_typec_vbus_enable(struct qcom_pmic_typec *qcom_usb)
> +{
> + int rc;
> +
> + rc = regmap_update_bits(qcom_usb->regmap, CMD_OTG, OTG_EN, OTG_EN);
> + if (rc)
> + dev_err(qcom_usb->dev, "failed to update 

Re: [RFC v3 1/3] usb: dwc3: Resize TX FIFOs to meet EP bursting requirements

[Jack Pham]

Hi Wesley,

On Wed, May 27, 2020 at 06:46:01PM -0700, Wesley Cheng wrote:
> Some devices have USB compositions which may require multiple endpoints
> that support EP bursting.  HW defined TX FIFO sizes may not always be
> sufficient for these compositions.  By utilizing flexible TX FIFO
> allocation, this allows for endpoints to request the required FIFO depth to
> achieve higher bandwidth.  With some higher bMaxBurst configurations, using
> a larger TX FIFO size results in better TX throughput.
> 
> Ensure that one TX FIFO is reserved for every IN endpoint.  This allows for
> the FIFO logic to prevent running out of FIFO space.
> 
> Signed-off-by: Wesley Cheng 
> ---



> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 00746c2..9b09528 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -540,6 +540,117 @@ static int dwc3_gadget_start_config(struct dwc3_ep *dep)
>   return 0;
>  }
>  
> +/*
> + * dwc3_gadget_resize_tx_fifos - reallocate fifo spaces for current use-case
> + * @dwc: pointer to our context structure
> + *
> + * This function will a best effort FIFO allocation in order
> + * to improve FIFO usage and throughput, while still allowing
> + * us to enable as many endpoints as possible.
> + *
> + * Keep in mind that this operation will be highly dependent
> + * on the configured size for RAM1 - which contains TxFifo -,
> + * the amount of endpoints enabled on coreConsultant tool, and
> + * the width of the Master Bus.
> + *
> + * In general, FIFO depths are represented with the following equation:
> + *
> + * fifo_size = mult * ((max_packet + mdwidth)/mdwidth + 1) + 1
> + *
> + * Conversions can be done to the equation to derive the number of packets 
> that
> + * will fit to a particular FIFO size value.
> + */
> +static int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc, struct dwc3_ep *dep)

The 'dep' param should be sufficient; we can just get 'dwc' from
dep->dwc. That will make it more clear this function operates on each
endpoint that needs resizing.

> +{
> + int ram1_depth, mdwidth, fifo_0_start, tmp, num_in_ep;
> + int min_depth, remaining, fifo_size, mult = 1, fifo, max_packet = 1024;
> +
> + if (!dwc->needs_fifo_resize)
> + return 0;
> +
> + /* resize IN endpoints except ep0 */
> + if (!usb_endpoint_dir_in(dep->endpoint.desc) || dep->number <= 1)
> + return 0;
> +
> + /* Don't resize already resized IN endpoint */
> + if (dep->fifo_depth)
> + return 0;
> +
> + ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
> + mdwidth = DWC3_MDWIDTH(dwc->hwparams.hwparams0);
> + /* MDWIDTH is represented in bits, we need it in bytes */
> + mdwidth >>= 3;
> +
> + if (((dep->endpoint.maxburst > 1) &&
> + usb_endpoint_xfer_bulk(dep->endpoint.desc))
> + || usb_endpoint_xfer_isoc(dep->endpoint.desc))
> + mult = 3;
> +
> + if ((dep->endpoint.maxburst > 6) &&
> + usb_endpoint_xfer_bulk(dep->endpoint.desc)
> + && dwc3_is_usb31(dwc))
> + mult = 6;
> +
> + /* FIFO size for a single buffer */
> + fifo = (max_packet + mdwidth)/mdwidth;
> + fifo++;
> +
> + /* Calculate the number of remaining EPs w/o any FIFO */
> + num_in_ep = dwc->num_eps/2;
> + num_in_ep -= dwc->num_ep_resized;
> + /* Ignore EP0 IN */
> + num_in_ep--;
> +
> + /* Reserve at least one FIFO for the number of IN EPs */
> + min_depth = num_in_ep * (fifo+1);
> + remaining = ram1_depth - min_depth - dwc->last_fifo_depth;
> +
> + /* We've already reserved 1 FIFO per EP, so check what we can fit in
> +  * addition to it.  If there is not enough remaining space, allocate
> +  * all the remaining space to the EP.
> +  */
> + fifo_size = (mult-1) * fifo;
> + if (remaining < fifo_size) {
> + if (remaining > 0)
> + fifo_size = remaining;
> + else
> + fifo_size = 0;
> + }
> +
> + fifo_size += fifo;
> + fifo_size++;
> + dep->fifo_depth = fifo_size;
> +
> + /* Check if TXFIFOs start at non-zero addr */
> + tmp = dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(0));
> + fifo_0_start = DWC3_GTXFIFOSIZ_TXFSTADDR(tmp);
> +
> + fifo_size |= (fifo_0_start + (dwc->last_fifo_depth << 16));
> + if (dwc3_is_usb31(dwc))
> + dwc->last_fifo_depth += DWC31_GTXFIFOSIZ_TXFDEP(fifo_size);
> + else
> + dwc->last_fifo_depth += DWC3_GTXFIFOSIZ_TXFDEP(fifo_size);
> +
> + /* Check fifo size allocation doesn't exceed available RAM size. */
> + if (dwc->last_fifo_depth >= ram1_depth) {
> + dev_err(dwc->dev, "Fifosize(%d) > RAM size(%d) %s depth:%d\n",
> + (dwc->last_fifo_depth * mdwidth), ram1_depth,
> + dep->endpoint.name, fifo_size);

Use dev_WARN() here and eliminate the 

Re: [PATCH] usb: dwc3: gadget: Correct the logic for finding last SG entry

[Jack Pham]

Hi Anurag,

On Fri, Jun 07, 2019 at 09:49:59AM +0300, Felipe Balbi wrote:
> Anurag Kumar Vulisha  writes:
> >>> The dma_map_sg() merges sg1 & sg2 memory regions into sg1-
> >>>dma_address.
> >>> Similarly sg3 & sg4 into sg2->dma_address, sg5 & sg6 into the
> >>> sg3->dma_address and sg6 & sg8 into sg4->dma_address. Here the
> >>memory
> >>> regions are merged but the page_link properties like SG_END are not
> >>> retained into the merged sgs.
> >>
> >>isn't this a bug in the scatterlist mapping code? Why doesn't it keep
> >>SG_END?
> >>
> >
> > Thanks for providing your comment.
> >
> > I don't think it is a bug, instead I feel some enhancement needs to be done 
> > in
> > dma-mapping code.
> >
> > SG_END represents the last sg entry in the sglist and it is correctly 
> > getting
> > set to the last sg entry.
> >
> > The issue happens only when 2 or more sg entry pages are merged into
> > contiguous dma-able address and sg_is_last() is used to find the last sg 
> > entry
> > with valid dma address.
> 
> Right, and that's something that's bound to happen. I'm arguing that, perhaps,
> dma API should move SG_END in case entries are merged.
> 
> > I think that along with sg_is_last() a new flag (SG_DMA_END) and function
> > (something like sg_dma_is_last() ) needs to be added into dma-mapping code 
> > for
> > identifying the last valid sg entry with valid dma address. So that we can
> > make use of that function instead of sg_is_last().
> 
> Sure, propose a patch to DMA API.

I'm curious if this was ever resolved. I just ran into this exact issue
with Android ADB which uses 16KB buffers, along with f_fs supporting
S/G since 5.0, combined with our IOMMU which performs this merging
behavior, so it resulted in a single TRB getting queued with CHN=1 and
LST=0 and thus the transfer never completes. Your initial patch resolves
the issue for me, but upon revisiting this discussion I couldn't tell if
you had attempted to patch DMA API instead as per Felipe's suggestion.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 3/3] phy: qcom-qmp: Add SM8150 QMP UFS PHY support

[Jack Pham]

Hi Vinod,

On Fri, Sep 06, 2019 at 10:40:17AM +0530, Vinod Koul wrote:
> SM8150 UFS PHY is v4 of QMP phy. Add support for V4 QMP phy register
> defines and support for SM8150 QMP UFS PHY.
> 
> Signed-off-by: Vinod Koul 
> Reviewed-by: Bjorn Andersson 
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 125 
>  drivers/phy/qualcomm/phy-qcom-qmp.h |  96 +
>  2 files changed, 221 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 34ff6434da8f..92d3048f2b36 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -164,6 +164,11 @@ static const unsigned int sdm845_ufsphy_regs_layout[] = {
>   [QPHY_PCS_READY_STATUS] = 0x160,
>  };
>  
> +static const unsigned int sm8150_ufsphy_regs_layout[] = {
> + [QPHY_START_CTRL]   = 0x00,
> + [QPHY_PCS_READY_STATUS] = 0x180,
> +};
> +
>  static const struct qmp_phy_init_tbl msm8996_pcie_serdes_tbl[] = {
>   QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x1c),
>   QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
> @@ -878,6 +883,93 @@ static const struct qmp_phy_init_tbl 
> msm8998_usb3_pcs_tbl[] = {
>   QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_RUN_TIME, 0x13),
>  };
>  
> +static const struct qmp_phy_init_tbl sm8150_ufsphy_serdes_tbl[] = {
> + QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_COM_V4_SYSCLK_EN_SEL, 0xd9),

QSERDES_V4_COM? See below.



> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.h 
> b/drivers/phy/qualcomm/phy-qcom-qmp.h
> index 335ea5d7ef40..0eefd8621669 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.h
> @@ -313,4 +313,100 @@
>  #define QPHY_V3_PCS_MISC_OSC_DTCT_MODE2_CONFIG4  0x5c
>  #define QPHY_V3_PCS_MISC_OSC_DTCT_MODE2_CONFIG5  0x60
>  
> +/* Only for QMP V4 PHY - QSERDES COM registers */
> +#define QSERDES_COM_V4_SYSCLK_EN_SEL 0x094

Should these rather be prefixed as QSERDES_V4_COM? There are already
QSERDES_V3_COM_* in this header so the convention appears to be
Q{SERDES,PHY}_VX_{COM,TX,RX,PCS}.

> +#define QSERDES_COM_V4_HSCLK_SEL 0x158
> +#define QSERDES_COM_V4_HSCLK_HS_SWITCH_SEL   0x15C
> +#define QSERDES_COM_V4_LOCK_CMP_EN   0x0A4
> +#define QSERDES_COM_V4_VCO_TUNE_MAP  0x10C

Nit: sort in ascending offset order, and make the hex values lowercase?



> +/* Only for QMP V4 PHY - PCS registers */
> +#define QPHY_V4_PHY_START0x000
> +#define QPHY_V4_POWER_DOWN_CONTROL   0x004
> +#define QPHY_V4_SW_RESET 0x008
> +#define QPHY_V4_PCS_READY_STATUS 0x180
> +#define QPHY_V4_LINECFG_DISABLE  0x148
> +#define QPHY_V4_MULTI_LANE_CTRL1 0x1E0
> +#define QPHY_V4_RX_SIGDET_CTRL2  0x158
> +#define QPHY_V4_TX_LARGE_AMP_DRV_LVL 0x030
> +#define QPHY_V4_TX_SMALL_AMP_DRV_LVL 0x038
> +#define QPHY_V4_TX_MID_TERM_CTRL10x1D8
> +#define QPHY_V4_DEBUG_BUS_CLKSEL 0x124
> +#define QPHY_V4_PLL_CNTL 0x02C
> +#define QPHY_V4_TIMER_20US_CORECLK_STEPS_MSB 0x00C
> +#define QPHY_V4_TIMER_20US_CORECLK_STEPS_LSB 0x010
> +#define QPHY_V4_TX_PWM_GEAR_BAND 0x160
> +#define QPHY_V4_TX_HS_GEAR_BAND  0x168
> +#define QPHY_V4_TX_HSGEAR_CAPABILITY 0x074
> +#define QPHY_V4_RX_HSGEAR_CAPABILITY 0x0B4
> +#define QPHY_V4_RX_MIN_HIBERN8_TIME  0x150
> +#define QPHY_V4_BIST_FIXED_PAT_CTRL  0x060

Interesting. These offsets appear to be valid only for the UFS instance
of the QMP PHY. For PCIe and USB the PCS layout is completely different.
Wonder if we need to add _UFS_ to  the prefix to differentiate them? Or
can this be deferred to when PCIe/USB PHY driver support for SM8150 gets
added?

I was thinking of taking a stab at USB if I get time, not sure if that's
already on your or somebody's (Bjorn?) radar.

Thanks
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC][PATCH v2 2/5] usb: dwc3: Execute GCTL Core Soft Reset while switch mdoe for Hisilicon Kirin Soc

[Jack Pham]

Hi John, Yu, Felipe,

On Mon, Oct 07, 2019 at 05:55:50PM +, John Stultz wrote:
> From: Yu Chen 
> 
> A GCTL soft reset should be executed when switch mode for dwc3 core
> of Hisilicon Kirin Soc.
> 
> Cc: Greg Kroah-Hartman 
> Cc: Felipe Balbi 
> Cc: Andy Shevchenko 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: Yu Chen 
> Cc: Matthias Brugger 
> Cc: Chunfeng Yun 
> Cc: linux-...@vger.kernel.org
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Yu Chen 
> Signed-off-by: John Stultz 
> ---
>  drivers/usb/dwc3/core.c | 20 
>  drivers/usb/dwc3/core.h |  3 +++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 999ce5e84d3c..440261432421 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -112,6 +112,19 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
>   dwc->current_dr_role = mode;
>  }
>  
> +static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
> +{
> + u32 reg;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg |= DWC3_GCTL_CORESOFTRESET;
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg &= ~DWC3_GCTL_CORESOFTRESET;
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +}
> +
>  static void __dwc3_set_mode(struct work_struct *work)
>  {
>   struct dwc3 *dwc = work_to_dwc(work);
> @@ -156,6 +169,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>  
>   dwc3_set_prtcap(dwc, dwc->desired_dr_role);
>  
> + /* Execute a GCTL Core Soft Reset when switch mode */
> + if (dwc->gctl_reset_quirk)
> + dwc3_gctl_core_soft_reset(dwc);
> +

In fact it is mentioned in the Synopsys databook to perform a GCTL
CoreSoftReset when changing the PrtCapDir between device & host modes.
So I think this should apply generally without a quirk. Further, it
states to do this *prior* to writing PrtCapDir, so should it go before
dwc3_set_prtcap() instead?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: Add Qualcomm USB SuperSpeed PHY bindings

[Jack Pham]

Hi Jorge, Bjorn,

On Thu, Sep 05, 2019 at 09:18:57AM +0200, Jorge Ramirez wrote:
> On 9/4/19 01:34, Bjorn Andersson wrote:
> > On Tue 03 Sep 14:45 PDT 2019, Stephen Boyd wrote:
> > 
> >> Quoting Jack Pham (2019-09-03 10:39:24)
> >>> On Mon, Sep 02, 2019 at 08:23:04AM +0200, Jorge Ramirez wrote:
> >>>> On 8/30/19 20:28, Stephen Boyd wrote:
> >>>>> Quoting Bjorn Andersson (2019-08-30 09:45:20)
> >>>>>> On Fri 30 Aug 09:01 PDT 2019, Stephen Boyd wrote:
> >>>>>>
> >>>>>>>>>
> >>>>>>>>> The USB-C connector is attached both to the HS and SS PHYs, so I 
> >>>>>>>>> think
> >>>>>>>>> you should represent this external to this node and use of_graph to
> >>>>>>>>> query it.
> >>>>>>>>
> >>>>>>>> but AFAICS we wont be able to retrieve the vbux-supply from an 
> >>>>>>>> external
> >>>>>>>> node (that interface does not exist).
> >>>>>>>>
> >>>>>>>> rob, do you have a suggestion?
> >>>>>>>
> >>>>>>> Shouldn't the vbus supply be in the phy? Or is this a situation where
> >>>>>>> the phy itself doesn't have the vbus supply going to it because the 
> >>>>>>> PMIC
> >>>>>>> gets in the way and handles the vbus for the connector by having the 
> >>>>>>> SoC
> >>>>>>> communicate with the PMIC about when to turn the vbus on and off, etc?
> >>>>>>>
> >>>>>>
> >>>>>> That's correct, the VBUS comes out of the PMIC and goes directly to the
> >>>>>> connector.
> >>>>>>
> >>>>>> The additional complicating factor here is that the connector is wired
> >>>>>> to a USB2 phy as well, so we need to wire up detection and vbus control
> >>>>>> to both of them - but I think this will be fine, if we can only figure
> >>>>>> out a sane way of getting hold of the vbus-supply.
> >>>>>>
> >>>>>
> >>>>> Does it really matter to describe this situation though? Maybe it's
> >>>>> simpler to throw the vbus supply into the phy and control it from the
> >>>>> phy driver, even if it never really goes there. Or put it into the
> >>>>> toplevel usb controller?
> >>>>>
> >>>> that would work for me - the connector definition seemed a better way to
> >>>> explain the connectivity but since we cant retrieve the supply from the
> >>>> external node is not of much functional use.
> >>>>
> >>>> but please let me know how to proceed. shall I add the supply back to
> >>>> the phy?
> >>
> >> So does the vbus actually go to the phy? I thought it never went there
> >> and the power for the phy was different (and possibly lower in voltage).
> >>
> > 
> > No, the PHYs use different - lower voltage - supplies to operate. VBUS
> > is coming from a 5V supply straight to the connector and plug-detect
> > logic (which is passive in this design).
> > 
> >>>
> >>> Putting it in the toplevel usb node makes sense to me, since that's
> >>> usually the driver that knows when it's switching into host mode and
> >>> needs to turn on VBUS. The dwc3-qcom driver & bindings currently don't 
> >>> do this but there's precedent in a couple of the other dwc3 "glues"--see
> >>> Documentation/devicetree/bindings/usb/{amlogic\,dwc3,omap-usb}.txt
> >>>
> >>> One exception is if the PMIC is also USB-PD capable and can do power
> >>> role swap, in which case the VBUS control needs to be done by the TCPM,
> >>> so that'd be a case where having vbus-supply in the connector node might
> >>> make more sense.
> >>>
> >>
> >> The other way is to implement the code to get the vbus supply out of a
> >> connector. Then any driver can do the work if it knows it needs to and
> >> we don't have to care that the vbus isn't going somewhere. I suppose
> >> that would need an of_regulator_get() sort of API that can get the
> >> regulator out of there? Or to make the connector into a struct device
> >> that can get the regulator out per s

Re: [PATCH v4 3/4] dt-bindings: Add Qualcomm USB SuperSpeed PHY bindings

[Jack Pham]

On Mon, Sep 02, 2019 at 08:23:04AM +0200, Jorge Ramirez wrote:
> On 8/30/19 20:28, Stephen Boyd wrote:
> > Quoting Bjorn Andersson (2019-08-30 09:45:20)
> >> On Fri 30 Aug 09:01 PDT 2019, Stephen Boyd wrote:
> >>
> >>> Quoting Jorge Ramirez (2019-08-29 00:03:48)
>  On 2/23/19 17:52, Bjorn Andersson wrote:
> > On Thu 07 Feb 03:17 PST 2019, Jorge Ramirez-Ortiz wrote:
> >> +
> >> +Required child nodes:
> >> +
> >> +- usb connector node as defined in 
> >> bindings/connector/usb-connector.txt
> >> +  containing the property vbus-supply.
> >> +
> >> +Example:
> >> +
> >> +usb3_phy: usb3-phy@78000 {
> >> +compatible = "qcom,snps-usb-ssphy";
> >> +reg = <0x78000 0x400>;
> >> +#phy-cells = <0>;
> >> +clocks = < RPM_SMD_LN_BB_CLK>,
> >> + < GCC_USB_HS_PHY_CFG_AHB_CLK>,
> >> + < GCC_USB3_PHY_PIPE_CLK>;
> >> +clock-names = "ref", "phy", "pipe";
> >> +resets = < GCC_USB3_PHY_BCR>,
> >> + < GCC_USB3PHY_PHY_BCR>;
> >> +reset-names = "com", "phy";
> >> +vdd-supply = <_l3_1p05>;
> >> +vdda1p8-supply = <_l5_1p8>;
> >> +usb3_c_connector: usb3-c-connector {
> >>>
> >>> Node name should be 'connector', not usb3-c-connector.
> >>>
> >>
> >> It probably has to be usb-c-connector, because we have a
> >> micro-usb-connector on the same board.
> > 
> > Ok. Or connector@1 and connector@2? Our toplevel node container story is
> > sort of sad because we have to play tricks with node names. But in the
> > example, just connector I presume? 
> > 
> >>
> >
> > The USB-C connector is attached both to the HS and SS PHYs, so I think
> > you should represent this external to this node and use of_graph to
> > query it.
> 
>  but AFAICS we wont be able to retrieve the vbux-supply from an external
>  node (that interface does not exist).
> 
>  rob, do you have a suggestion?
> >>>
> >>> Shouldn't the vbus supply be in the phy? Or is this a situation where
> >>> the phy itself doesn't have the vbus supply going to it because the PMIC
> >>> gets in the way and handles the vbus for the connector by having the SoC
> >>> communicate with the PMIC about when to turn the vbus on and off, etc?
> >>>
> >>
> >> That's correct, the VBUS comes out of the PMIC and goes directly to the
> >> connector.
> >>
> >> The additional complicating factor here is that the connector is wired
> >> to a USB2 phy as well, so we need to wire up detection and vbus control
> >> to both of them - but I think this will be fine, if we can only figure
> >> out a sane way of getting hold of the vbus-supply.
> >>
> > 
> > Does it really matter to describe this situation though? Maybe it's
> > simpler to throw the vbus supply into the phy and control it from the
> > phy driver, even if it never really goes there. Or put it into the
> > toplevel usb controller?
> > 
> that would work for me - the connector definition seemed a better way to
> explain the connectivity but since we cant retrieve the supply from the
> external node is not of much functional use.
> 
> but please let me know how to proceed. shall I add the supply back to
> the phy?

Putting it in the toplevel usb node makes sense to me, since that's
usually the driver that knows when it's switching into host mode and
needs to turn on VBUS. The dwc3-qcom driver & bindings currently don't 
do this but there's precedent in a couple of the other dwc3 "glues"--see
Documentation/devicetree/bindings/usb/{amlogic\,dwc3,omap-usb}.txt

One exception is if the PMIC is also USB-PD capable and can do power
role swap, in which case the VBUS control needs to be done by the TCPM,
so that'd be a case where having vbus-supply in the connector node might
make more sense.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2] usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup

[Jack Pham]

OUT endpoint requests may somtimes have this flag set when
preparing to be submitted to HW indicating that there is an
additional TRB chained to the request for alignment purposes.
If that request is removed before the controller can execute the
transfer (e.g. ep_dequeue/ep_disable), the request will not go
through the dwc3_gadget_ep_cleanup_completed_request() handler
and will not have its needs_extra_trb flag cleared when
dwc3_gadget_giveback() is called.  This same request could be
later requeued for a new transfer that does not require an
extra TRB and if it is successfully completed, the cleanup
and TRB reclamation will incorrectly process the additional TRB
which belongs to the next request, and incorrectly advances the
TRB dequeue pointer, thereby messing up calculation of the next
requeust's actual/remaining count when it completes.

The right thing to do here is to ensure that the flag is cleared
before it is given back to the function driver.  A good place
to do that is in dwc3_gadget_del_and_unmap_request().

Fixes: c6267a51639b ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
Cc: sta...@vger.kernel.org
Signed-off-by: Jack Pham 
---
v2: Added Fixes tag and Cc: stable

Felipe, as I mentioned in the cover for v1, for stable (from 4.11 where
c6267a51639b first landed through 4.20), the fix needs to be modified to
assign to the separate req->unaligned and req->zero flags in lieu of
needs_extra_trb which appeared in 5.0-rc1 in:

commit 1a22ec643580626f439c8583edafdcc73798f2fb
Author: Felipe Balbi 
Date:   Wed Aug 1 13:15:05 2018 +0300

usb: dwc3: gadget: combine unaligned and zero flags

Do I need to send a separate patch for <= 4.20 or will you handle it?
It's straightforward really, the code change should instead be

+   req->unaligned = false;
+   req->zero = false;

Thanks,
Jack

 drivers/usb/dwc3/gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2ecde30ad0b7..e97b14f444c8 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -177,6 +177,7 @@ static void dwc3_gadget_del_and_unmap_request(struct 
dwc3_ep *dep,
req->started = false;
list_del(>list);
req->remaining = 0;
+   req->needs_extra_trb = false;
 
if (req->request.status == -EINPROGRESS)
req->request.status = status;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] dt-bindings: Add Qualcomm USB Super-Speed PHY bindings

[Jack Pham]

Hi Jorge,

Sorry for the late reply as I was out during the holiday break.

On Fri, Dec 28, 2018 at 01:38:59PM +0100, Jorge Ramirez wrote:
> On 12/20/18 18:37, Jack Pham wrote:
> >Hi Rob, Jorge,
> >
> >On Thu, Dec 20, 2018 at 11:05:31AM -0600, Rob Herring wrote:
> >>On Fri, Dec 07, 2018 at 10:55:57AM +0100, Jorge Ramirez-Ortiz wrote:
> >>>Binding description for Qualcomm's Synopsys 1.0.0 super-speed PHY
> >>>controller embedded in QCS404.
> >>>
> >>>Based on Sriharsha Allenki's  original
> >>>definitions.
> >>>
> >>>Signed-off-by: Jorge Ramirez-Ortiz 
> >>>Reviewed-by: Vinod Koul 
> >>>---
> >>>  .../devicetree/bindings/usb/qcom,usb-ssphy.txt | 78 
> >>> ++
> >>>  1 file changed, 78 insertions(+)
> >>>  create mode 100644 
> >>> Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt
> >>>
> >>>diff --git a/Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt 
> >>>b/Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt
> >>>new file mode 100644
> >>>index 000..fcf4e01
> >>>--- /dev/null
> >>>+++ b/Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt
> >>>@@ -0,0 +1,78 @@
> >>>+Qualcomm Synopsys 1.0.0 SS phy controller
> >>>+===
> >>>+
> >>>+Synopsys 1.0.0 ss phy controller supports SS usb connectivity on Qualcomm
> >>>+chipsets
> >>>+
> >>>+Required properties:
> >>>+
> >>>+- compatible:
> >>>+Value type: 
> >>>+Definition: Should contain "qcom,usb-ssphy".
> >>
> >>What is "qcom,dwc3-ss-usb-phy" which already exists then?
> >
> >Uh, apparently only the bindings doc is there but the driver never
> >landed. I guess it fell through the cracks nearly 4 years ago.
> >
> >https://lore.kernel.org/patchwork/patch/499502/
> >
> >Jorge, does Andy's version of this driver at all resemble what can be
> >used for QCS404?
> 
> on close inspection I cant see any similitudes between the drivers.
> Unfortunately I don't have access to documentation yet but the
> control register offsets and the control bits in the drivers do not
> match.
> 
> because of the above I'd like to go ahead with our separate drivers
> -already tested and validated- for HS (Shawn's) and SS (mine).
> 
> if that is acceptable, should we reuse the upstream bindings for
> our implementation? or perhaps Shawn Guo will do for his HS version
> of the driver and I go ahead and create a new one? what would you
> suggest?

I'm not really sure. My understanding of the driver Andy submitted
were for some of the older MSM and IPQ SoCs that implemented the PHY
controls as part of the DWC3 controller's "QScratch" registers, which is
why the bindings doc and the compatible string reference "dwc3" in both
the compatible and the docs filename. Is the SNPS PHY on QCS404
architected similarly in this regard? Either way, the existing bindings
doc for the non-existent driver looks incomplete for QCS404, so you'd
have to update it anyway. My feeling is that there should just be one
document describing all variants of SNPS PHYs on Qualcomm chips.

Maybe we should also just delete the "qcom,dwc3-ss-usb-phy" binding
unless there is a plan to resurrect Andy's driver.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 3/6] phy: qcom-qusb2: Add QUSB2 PHY support for msm8998

[Jack Pham]

Hi Jeff,

Spotted a typo below:

On Fri, Jan 04, 2019 at 09:50:29AM -0700, Jeffrey Hugo wrote:
> MSM8998 contains one QUSB2 PHY which is very similar to the existing
> sdm845 support.
> 
> Signed-off-by: Jeffrey Hugo 
> ---
>  .../devicetree/bindings/phy/qcom-qusb2-phy.txt |  1 +
>  drivers/phy/qualcomm/phy-qcom-qusb2.c  | 41 
> ++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt 
> b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> index 03025d9..3976847 100644
> --- a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> @@ -6,6 +6,7 @@ QUSB2 controller supports LS/FS/HS usb connectivity on 
> Qualcomm chipsets.
>  Required properties:
>   - compatible: compatible list, contains
>  "qcom,msm8996-qusb2-phy" for 14nm PHY on msm8996,
> +"qcom,msm8998-qusb2-phy" for 10nm PHY on msm8996,
   
should be 8998.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup

[Jack Pham]

OUT endpoint requests may somtimes have this flag set when
preparing to be submitted to HW indicating that there is an
additional TRB chained to the request for alignment purposes.
If that request is removed before the controller can execute the
transfer (e.g. ep_dequeue/ep_disable), the request will not go
through the dwc3_gadget_ep_cleanup_completed_request() handler
and will not have its needs_extra_trb flag cleared when
dwc3_gadget_giveback() is called.  This same request could be
later requeued for a new transfer that does not require an
extra TRB and if it is successfully completed, the cleanup
and TRB reclamation will incorrectly process the additional TRB
which belongs to the next request, and incorrectly advances the
TRB dequeue pointer, thereby messing up calculation of the next
requeust's actual/remaining count when it completes.

The right thing to do here is to ensure that the flag is cleared
before it is given back to the function driver.  A good place
to do that is in dwc3_gadget_del_and_unmap_request().

Signed-off-by: Jack Pham 
---
Hi Felipe,

There's probably zero chance this is making it to 4.20, so if you take
this after 4.21-rc1 so be it. But should this be Cc: stable? If so it
needs to be sent separately for <= 4.19 as needs_extra_trb was previously
req->unaligned and req->zero.

Thanks,
Jack

 drivers/usb/dwc3/gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2ecde30ad0b7..e97b14f444c8 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -177,6 +177,7 @@ static void dwc3_gadget_del_and_unmap_request(struct 
dwc3_ep *dep,
req->started = false;
list_del(>list);
req->remaining = 0;
+   req->needs_extra_trb = false;
 
if (req->request.status == -EINPROGRESS)
req->request.status = status;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] dt-bindings: Add Qualcomm USB Super-Speed PHY bindings

[Jack Pham]

Hi Rob, Jorge,

On Thu, Dec 20, 2018 at 11:05:31AM -0600, Rob Herring wrote:
> On Fri, Dec 07, 2018 at 10:55:57AM +0100, Jorge Ramirez-Ortiz wrote:
> > Binding description for Qualcomm's Synopsys 1.0.0 super-speed PHY
> > controller embedded in QCS404.
> > 
> > Based on Sriharsha Allenki's  original
> > definitions.
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz 
> > Reviewed-by: Vinod Koul 
> > ---
> >  .../devicetree/bindings/usb/qcom,usb-ssphy.txt | 78 
> > ++
> >  1 file changed, 78 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt 
> > b/Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt
> > new file mode 100644
> > index 000..fcf4e01
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/qcom,usb-ssphy.txt
> > @@ -0,0 +1,78 @@
> > +Qualcomm Synopsys 1.0.0 SS phy controller
> > +===
> > +
> > +Synopsys 1.0.0 ss phy controller supports SS usb connectivity on Qualcomm
> > +chipsets
> > +
> > +Required properties:
> > +
> > +- compatible:
> > +Value type: 
> > +Definition: Should contain "qcom,usb-ssphy".
> 
> What is "qcom,dwc3-ss-usb-phy" which already exists then?

Uh, apparently only the bindings doc is there but the driver never
landed. I guess it fell through the cracks nearly 4 years ago.

https://lore.kernel.org/patchwork/patch/499502/

Jorge, does Andy's version of this driver at all resemble what can be
used for QCS404?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/2] phy: qualcomm: Add Synopsys High-Speed USB PHY driver

[Jack Pham]

Hi Shawn,

On Tue, Nov 27, 2018 at 06:07:22PM +0800, Shawn Guo wrote:
> It adds Synopsys 28nm Femto High-Speed USB PHY driver support, which
> is usually paired with Synopsys DWC3 USB controllers on Qualcomm SoCs.
> 
> Signed-off-by: Shawn Guo 
> ---
>  drivers/phy/qualcomm/Kconfig  |  10 +
>  drivers/phy/qualcomm/Makefile |   1 +
>  .../phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c  | 529 ++

Just caught this...
s/snsp/snps/ in the file name? The bindings doc in Patch 1 is named
correctly.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/2] phy: qualcomm: Add Synopsys High-Speed USB PHY driver

[Jack Pham]

Hi Shawn, Kishon,

On Wed, Dec 19, 2018 at 09:07:36AM +0800, Shawn Guo wrote:
> On Tue, Dec 18, 2018 at 07:46:13PM +0530, Kishon Vijay Abraham I wrote:
> > 
> > 
> > On 17/12/18 6:04 AM, Shawn Guo wrote:
> > > On Tue, Dec 04, 2018 at 02:01:07PM +0800, Shawn Guo wrote:
> > >> Hi Kishon,
> > >>
> > >> On Tue, Dec 04, 2018 at 10:38:19AM +0530, Kishon Vijay Abraham I wrote:
> > >>> Hi,
> > >>>
> > >>> On 27/11/18 3:37 PM, Shawn Guo wrote:
> >  It adds Synopsys 28nm Femto High-Speed USB PHY driver support, which
> >  is usually paired with Synopsys DWC3 USB controllers on Qualcomm SoCs.
> > >>>
> > >>> Is this Synopsys PHY specific to Qualcomm or could it be used by other 
> > >>> vendors
> > >>> (with just changing tuning parameters)? If it could be used by other 
> > >>> vendors
> > >>> then it would make sense to add this PHY driver in synopsys directory.
> > >>
> > >> My knowledge is that this Synopsys PHY is specific to Qualcomm SoCs.
> > >> @Sriharsha, correct me if I'm wrong.
> > > 
> > > Kishon,
> > > 
> > > Do you have any further comments on the patches?  We are close the 4.21
> > > merge window, and I really hope they can hit mainline with 4.21 release.
> > > Thanks.
> > 
> > Aren't we waiting for feedback from Sriharsha?
> 
> I think no correction means that I'm right :)

I'm sorry Sriharsha has not yet responded but I can confirm that this
PHY driver would only be specific to Qualcomm SoCs simply because of our
custom register mapping used to control and initialize it. These are
only defined as as HW signals by the Synopsys IP, which have been
synthesized to map to our own IO register space. So really it can be
treated as just a "Qualcomm USB PHY" with acknowledgement that the core
design is licensed from SNPS.

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Jack Pham]

On Fri, Apr 13, 2018 at 11:32:04PM +0530, Manu Gautam wrote:
> On 4/13/2018 11:03 PM, Jack Pham wrote:
> > Are the extcon phandles bound to the glue node? I don't see the
> > description in the bindings doc in PATCH 1/3. And if so, would it be
> > a duplicate of the child node's extcon binding? Then again, the
> > alternative would be to grab it directly from the child (i.e.
> > qcom->dwc3->dev.of_node) which I'm not sure is ok to do or not.
> >
> 
> Yes these are bound to glue node. I missed to add it to documentation, will do
> so.

Ok thanks.

> I kept it separate for couple of reasons - one is to not peek too-much into 
> child
> node. Another reason is that doing so allows to have extcon in "peripheral"
> only mode as well (not just drd mode which is the case with dwc3 core).
> It allows to notify h/w when vbus is not there in device mode which IMO is
> right thing to do.

Ah, makes sense. extcon for the child is only needed for dual-role.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Jack Pham]

On Fri, Apr 13, 2018 at 11:32:04PM +0530, Manu Gautam wrote:
> On 4/13/2018 11:03 PM, Jack Pham wrote:
> > Are the extcon phandles bound to the glue node? I don't see the
> > description in the bindings doc in PATCH 1/3. And if so, would it be
> > a duplicate of the child node's extcon binding? Then again, the
> > alternative would be to grab it directly from the child (i.e.
> > qcom->dwc3->dev.of_node) which I'm not sure is ok to do or not.
> >
> 
> Yes these are bound to glue node. I missed to add it to documentation, will do
> so.

Ok thanks.

> I kept it separate for couple of reasons - one is to not peek too-much into 
> child
> node. Another reason is that doing so allows to have extcon in "peripheral"
> only mode as well (not just drd mode which is the case with dwc3 core).
> It allows to notify h/w when vbus is not there in device mode which IMO is
> right thing to do.

Ah, makes sense. extcon for the child is only needed for dual-role.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Jack Pham]

Hi Manu,

On Fri, Apr 13, 2018 at 10:21:23PM +0530, Manu Gautam wrote:
> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
> Some of its uses are described below resulting in need to
> have a separate glue driver instead of using dwc3-of-simple:
>  - It exposes register interface to override vbus-override
>and lane0-pwr-present signals going to hardware. These
>must be updated in peripheral mode for DWC3 if vbus lines
>are not connected to hardware block. Otherwise RX termination
>in SS mode or DP pull-up is not applied by device controller.
>  - pwr_events_irq_stat support to check if USB2 PHY is in L2 state
>before glue driver proceeds with suspend.
>  - Support for wakeup interrupts lines that are asserted whenever
>there is any wakeup event on USB3 or USB2 bus.
>  - Support to replace pip3 clock going to DWC3 with utmi clock
>for hardware configuration where SSPHY is not used with DWC3.
> 
> Signed-off-by: Manu Gautam 



> +static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
> +{
> + struct device   *dev = qcom->dev;
> + struct extcon_dev   *host_edev;
> + int ret;
> +
> + if (!of_property_read_bool(dev->of_node, "extcon"))
> + return 0;
> +
> + qcom->edev = extcon_get_edev_by_phandle(dev, 0);

Are the extcon phandles bound to the glue node? I don't see the
description in the bindings doc in PATCH 1/3. And if so, would it be
a duplicate of the child node's extcon binding? Then again, the
alternative would be to grab it directly from the child (i.e.
qcom->dwc3->dev.of_node) which I'm not sure is ok to do or not.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Jack Pham]

Hi Manu,

On Fri, Apr 13, 2018 at 10:21:23PM +0530, Manu Gautam wrote:
> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
> Some of its uses are described below resulting in need to
> have a separate glue driver instead of using dwc3-of-simple:
>  - It exposes register interface to override vbus-override
>and lane0-pwr-present signals going to hardware. These
>must be updated in peripheral mode for DWC3 if vbus lines
>are not connected to hardware block. Otherwise RX termination
>in SS mode or DP pull-up is not applied by device controller.
>  - pwr_events_irq_stat support to check if USB2 PHY is in L2 state
>before glue driver proceeds with suspend.
>  - Support for wakeup interrupts lines that are asserted whenever
>there is any wakeup event on USB3 or USB2 bus.
>  - Support to replace pip3 clock going to DWC3 with utmi clock
>for hardware configuration where SSPHY is not used with DWC3.
> 
> Signed-off-by: Manu Gautam 



> +static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
> +{
> + struct device   *dev = qcom->dev;
> + struct extcon_dev   *host_edev;
> + int ret;
> +
> + if (!of_property_read_bool(dev->of_node, "extcon"))
> + return 0;
> +
> + qcom->edev = extcon_get_edev_by_phandle(dev, 0);

Are the extcon phandles bound to the glue node? I don't see the
description in the bindings doc in PATCH 1/3. And if so, would it be
a duplicate of the child node's extcon binding? Then again, the
alternative would be to grab it directly from the child (i.e.
qcom->dwc3->dev.of_node) which I'm not sure is ok to do or not.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usb: gadget: f_fs: get the correct address of comp_desc

[Jack Pham]

Hi William,

On Mon, Feb 05, 2018 at 07:33:38PM +0800, William Wu wrote:
> Refer to the USB 3.0 spec '9.6.7 SuperSpeed Endpoint Companion',
> the companion descriptor follows the standard endpoint descriptor.
> This descriptor is only defined for SuperSpeed endpoints. The
> f_fs driver gets the address of the companion descriptor via
> 'ds + USB_DT_ENDPOINT_SIZE', and actually, the ds variable is
> a pointer to the struct usb_endpoint_descriptor, so the offset
> of the companion descriptor which we get is USB_DT_ENDPOINT_SIZE *
> sizeof(struct usb_endpoint_descriptor), the wrong offset is 63
> bytes. This cause out-of-bound with the following error log if
> CONFIG_KASAN and CONFIG_SLUB_DEBUG is enabled on Rockchip RK3399
> Evaluation Board.
> 
> android_work: sent uevent USB_STATE=CONNECTED
> configfs-gadget gadget: super-speed config #1: b
> ==
> BUG: KASAN: slab-out-of-bounds in ffs_func_set_alt+0x230/0x398
> Read of size 1 at addr ffc0ce2d0b10 by task irq/224-dwc3/364

> Memory state around the buggy address:
>  ffc0ce2d0a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffc0ce2d0a80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  >ffc0ce2d0b00: 00 04 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  ^
>  ffc0ce2d0b80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  ffc0ce2d0c00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==
> Disabling lock debugging due to kernel taint
> android_work: sent uevent USB_STATE=CONFIGURED
> 
> This patch adds struct usb_endpoint_descriptor * -> u8 * type conversion
> for ds variable, then we can get the correct address of comp_desc
> with offset USB_DT_ENDPOINT_SIZE bytes.
> 
> Signed-off-by: William Wu 
> ---
>  drivers/usb/gadget/function/f_fs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c 
> b/drivers/usb/gadget/function/f_fs.c
> index 6756472..f13ead0 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1882,8 +1882,8 @@ static int ffs_func_eps_enable(struct ffs_function 
> *func)
>   ep->ep->desc = ds;
>  
>   if (needs_comp_desc) {
> - comp_desc = (struct usb_ss_ep_comp_descriptor *)(ds +
> - USB_DT_ENDPOINT_SIZE);
> + comp_desc = (struct usb_ss_ep_comp_descriptor *)
> +  ((u8 *)ds + USB_DT_ENDPOINT_SIZE);
>   ep->ep->maxburst = comp_desc->bMaxBurst + 1;
>   ep->ep->comp_desc = comp_desc;
>   }

Please see my alternative fix for this. I proposed changing this
function to use config_ep_by_speed() instead.

https://www.spinics.net/lists/linux-usb/msg165149.html

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usb: gadget: f_fs: get the correct address of comp_desc

[Jack Pham]

Hi William,

On Mon, Feb 05, 2018 at 07:33:38PM +0800, William Wu wrote:
> Refer to the USB 3.0 spec '9.6.7 SuperSpeed Endpoint Companion',
> the companion descriptor follows the standard endpoint descriptor.
> This descriptor is only defined for SuperSpeed endpoints. The
> f_fs driver gets the address of the companion descriptor via
> 'ds + USB_DT_ENDPOINT_SIZE', and actually, the ds variable is
> a pointer to the struct usb_endpoint_descriptor, so the offset
> of the companion descriptor which we get is USB_DT_ENDPOINT_SIZE *
> sizeof(struct usb_endpoint_descriptor), the wrong offset is 63
> bytes. This cause out-of-bound with the following error log if
> CONFIG_KASAN and CONFIG_SLUB_DEBUG is enabled on Rockchip RK3399
> Evaluation Board.
> 
> android_work: sent uevent USB_STATE=CONNECTED
> configfs-gadget gadget: super-speed config #1: b
> ==
> BUG: KASAN: slab-out-of-bounds in ffs_func_set_alt+0x230/0x398
> Read of size 1 at addr ffc0ce2d0b10 by task irq/224-dwc3/364

> Memory state around the buggy address:
>  ffc0ce2d0a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffc0ce2d0a80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  >ffc0ce2d0b00: 00 04 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  ^
>  ffc0ce2d0b80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  ffc0ce2d0c00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==
> Disabling lock debugging due to kernel taint
> android_work: sent uevent USB_STATE=CONFIGURED
> 
> This patch adds struct usb_endpoint_descriptor * -> u8 * type conversion
> for ds variable, then we can get the correct address of comp_desc
> with offset USB_DT_ENDPOINT_SIZE bytes.
> 
> Signed-off-by: William Wu 
> ---
>  drivers/usb/gadget/function/f_fs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c 
> b/drivers/usb/gadget/function/f_fs.c
> index 6756472..f13ead0 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1882,8 +1882,8 @@ static int ffs_func_eps_enable(struct ffs_function 
> *func)
>   ep->ep->desc = ds;
>  
>   if (needs_comp_desc) {
> - comp_desc = (struct usb_ss_ep_comp_descriptor *)(ds +
> - USB_DT_ENDPOINT_SIZE);
> + comp_desc = (struct usb_ss_ep_comp_descriptor *)
> +  ((u8 *)ds + USB_DT_ENDPOINT_SIZE);
>   ep->ep->maxburst = comp_desc->bMaxBurst + 1;
>   ep->ep->comp_desc = comp_desc;
>   }

Please see my alternative fix for this. I proposed changing this
function to use config_ep_by_speed() instead.

https://www.spinics.net/lists/linux-usb/msg165149.html

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/3] usb: phy: remove phy-msm-usb.c

[Jack Pham]

Hi Alex,

On Tue, Oct 31, 2017 at 08:11:30AM -0500, Alex Elder wrote:
> No Qualcomm SoC requires the "phy-msm-usb.c" USB phy driver support
> any more, so remove the code.
> 
> Suggested-by: Stephen Boyd 
> Signed-off-by: Alex Elder 
> Acked-by: Bjorn Andersson 
> Acked-by: Andy Gross 
> ---


> -#include 

This file and ehci-msm.c were the last consumers of this header, so it
too can now be deleted. Has your series been picked up yet? If so I or
someone can send a follow-up patch.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/3] usb: phy: remove phy-msm-usb.c

[Jack Pham]

Hi Alex,

On Tue, Oct 31, 2017 at 08:11:30AM -0500, Alex Elder wrote:
> No Qualcomm SoC requires the "phy-msm-usb.c" USB phy driver support
> any more, so remove the code.
> 
> Suggested-by: Stephen Boyd 
> Signed-off-by: Alex Elder 
> Acked-by: Bjorn Andersson 
> Acked-by: Andy Gross 
> ---


> -#include 

This file and ehci-msm.c were the last consumers of this header, so it
too can now be deleted. Has your series been picked up yet? If so I or
someone can send a follow-up patch.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 1/7] typec: tcpm: Add PD Rev 3.0 definitions to PD header

[Jack Pham]

Hi Adam,

On Wed, Nov 01, 2017 at 05:03:09PM +, Adam Thomson wrote:
> This commit adds definitions for PD Rev 3.0 messages, including
> APDO PPS and extended message support for TCPM.
> 
> Signed-off-by: Adam Thomson 
> ---
>  include/linux/usb/pd.h | 162 
> +
>  1 file changed, 151 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index e00051c..77c6cd6 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h



>  #define PD_REV10 0x0
>  #define PD_REV20 0x1
> +#define PD_REV30 0x2
>  
> +#define PD_HEADER_EXT_HDRBIT(15)
>  #define PD_HEADER_CNT_SHIFT  12
>  #define PD_HEADER_CNT_MASK   0x7
>  #define PD_HEADER_ID_SHIFT   9
> @@ -59,18 +91,19 @@ enum pd_data_msg_type {
>  #define PD_HEADER_REV_MASK   0x3
>  #define PD_HEADER_DATA_ROLE  BIT(5)
>  #define PD_HEADER_TYPE_SHIFT 0
> -#define PD_HEADER_TYPE_MASK  0xf
> +#define PD_HEADER_TYPE_MASK  0x1f
>  
> -#define PD_HEADER(type, pwr, data, id, cnt)  \
> +#define PD_HEADER(type, pwr, data, id, cnt, ext_hdr) \
>   type) & PD_HEADER_TYPE_MASK) << PD_HEADER_TYPE_SHIFT) | \
>((pwr) == TYPEC_SOURCE ? PD_HEADER_PWR_ROLE : 0) | \
>((data) == TYPEC_HOST ? PD_HEADER_DATA_ROLE : 0) | \
> -  (PD_REV20 << PD_HEADER_REV_SHIFT) |\
> +  (PD_REV30 << PD_HEADER_REV_SHIFT) |\

You are making a hardcoded change for the Spec Rev field of every
outgoing message to be 3.0. However, this needs to be flexible in order
to support backwards compatibility when communicating with a 2.0 peer.
The revision "negotiation" would need to be done at the time the first
Request is sent such that both source & sink settle on the highest
supported revision of both. (PD 3.0 spec section 6.2.1.1.5)

>(((id) & PD_HEADER_ID_MASK) << PD_HEADER_ID_SHIFT) |   \
> -  (((cnt) & PD_HEADER_CNT_MASK) << PD_HEADER_CNT_SHIFT))
> +  (((cnt) & PD_HEADER_CNT_MASK) << PD_HEADER_CNT_SHIFT) |\
> +  ((ext_hdr) ? PD_HEADER_EXT_HDR : 0))
>  
>  #define PD_HEADER_LE(type, pwr, data, id, cnt) \
> - cpu_to_le16(PD_HEADER((type), (pwr), (data), (id), (cnt)))
> + cpu_to_le16(PD_HEADER((type), (pwr), (data), (id), (cnt), (0)))
>  
>  static inline unsigned int pd_header_cnt(u16 header)
>  {
> @@ -102,16 +135,66 @@ static inline unsigned int pd_header_msgid_le(__le16 
> header)
>   return pd_header_msgid(le16_to_cpu(header));
>  }
>  
> +#define PD_EXT_HDR_CHUNKED   BIT(15)
> +#define PD_EXT_HDR_CHUNK_NUM_SHIFT   11
> +#define PD_EXT_HDR_CHUNK_NUM_MASK0xf
> +#define PD_EXT_HDR_REQ_CHUNK BIT(10)
> +#define PD_EXT_HDR_DATA_SIZE_SHIFT   0
> +#define PD_EXT_HDR_DATA_SIZE_MASK0x1ff
> +
> +#define PD_EXT_HDR(data_size, req_chunk, chunk_num, chunked) 
> \
> + data_size) & PD_EXT_HDR_DATA_SIZE_MASK) << 
> PD_EXT_HDR_DATA_SIZE_SHIFT) |\
> +  ((req_chunk) ? PD_EXT_HDR_REQ_CHUNK : 0) | 
> \
> +  (((chunk_num) & PD_EXT_HDR_CHUNK_NUM_MASK) << 
> PD_EXT_HDR_CHUNK_NUM_SHIFT) |\
> +  ((chunked) ? PD_EXT_HDR_CHUNKED : 0))
> +
> +#define PD_EXT_HDR_LE(data_size, req_chunk, chunk_num, chunked) \
> + cpu_to_le16(PD_EXT_HDR((data_size), (req_chunk), (chunk_num), 
> (chunked)))
> +
> +static inline unsigned int pd_ext_header_chunk_num(u16 ext_header)
> +{
> + return (ext_header >> PD_EXT_HDR_CHUNK_NUM_SHIFT) &
> + PD_EXT_HDR_CHUNK_NUM_MASK;
> +}
> +
> +static inline unsigned int pd_ext_header_data_size(u16 ext_header)
> +{
> + return (ext_header >> PD_EXT_HDR_DATA_SIZE_SHIFT) &
> + PD_EXT_HDR_DATA_SIZE_MASK;
> +}
> +
> +static inline unsigned int pd_ext_header_data_size_le(__le16 ext_header)
> +{
> + return pd_ext_header_data_size(le16_to_cpu(ext_header));
> +}
> +
>  #define PD_MAX_PAYLOAD   7
> +#define PD_EXT_MAX_LEGACY_DATA   26
> +#define PD_EXT_MAX_CHUNK_DATA26
> +#define PD_EXT_MAX_DATA  260
>  
>  /**
> - * struct pd_message - PD message as seen on wire
> - * @header:  PD message header
> - * @payload: PD message payload
> - */
> +  * struct pd_ext_message_data - PD extended message data as seen on wire
> +  * @header:PD extended message header
> +  * @data:  PD extended message data
> +  */
> +struct pd_ext_message_data {
> + __le16 header;
> + u8 data[PD_EXT_MAX_DATA];
> +} __packed;
> +
> +/**
> +  * struct pd_message - PD message as seen on wire
> +  * @header:PD message header
> +  * @payload:   PD message payload
> +  * @ext_msg:   PD message extended message data
> +  */
>  struct pd_message {
>   __le16 header;
> - __le32 payload[PD_MAX_PAYLOAD];
> + union {
> + __le32 payload[PD_MAX_PAYLOAD];
> + struct pd_ext_message_data 

Re: [RFC PATCH 1/7] typec: tcpm: Add PD Rev 3.0 definitions to PD header

[Jack Pham]

Hi Adam,

On Wed, Nov 01, 2017 at 05:03:09PM +, Adam Thomson wrote:
> This commit adds definitions for PD Rev 3.0 messages, including
> APDO PPS and extended message support for TCPM.
> 
> Signed-off-by: Adam Thomson 
> ---
>  include/linux/usb/pd.h | 162 
> +
>  1 file changed, 151 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index e00051c..77c6cd6 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h



>  #define PD_REV10 0x0
>  #define PD_REV20 0x1
> +#define PD_REV30 0x2
>  
> +#define PD_HEADER_EXT_HDRBIT(15)
>  #define PD_HEADER_CNT_SHIFT  12
>  #define PD_HEADER_CNT_MASK   0x7
>  #define PD_HEADER_ID_SHIFT   9
> @@ -59,18 +91,19 @@ enum pd_data_msg_type {
>  #define PD_HEADER_REV_MASK   0x3
>  #define PD_HEADER_DATA_ROLE  BIT(5)
>  #define PD_HEADER_TYPE_SHIFT 0
> -#define PD_HEADER_TYPE_MASK  0xf
> +#define PD_HEADER_TYPE_MASK  0x1f
>  
> -#define PD_HEADER(type, pwr, data, id, cnt)  \
> +#define PD_HEADER(type, pwr, data, id, cnt, ext_hdr) \
>   type) & PD_HEADER_TYPE_MASK) << PD_HEADER_TYPE_SHIFT) | \
>((pwr) == TYPEC_SOURCE ? PD_HEADER_PWR_ROLE : 0) | \
>((data) == TYPEC_HOST ? PD_HEADER_DATA_ROLE : 0) | \
> -  (PD_REV20 << PD_HEADER_REV_SHIFT) |\
> +  (PD_REV30 << PD_HEADER_REV_SHIFT) |\

You are making a hardcoded change for the Spec Rev field of every
outgoing message to be 3.0. However, this needs to be flexible in order
to support backwards compatibility when communicating with a 2.0 peer.
The revision "negotiation" would need to be done at the time the first
Request is sent such that both source & sink settle on the highest
supported revision of both. (PD 3.0 spec section 6.2.1.1.5)

>(((id) & PD_HEADER_ID_MASK) << PD_HEADER_ID_SHIFT) |   \
> -  (((cnt) & PD_HEADER_CNT_MASK) << PD_HEADER_CNT_SHIFT))
> +  (((cnt) & PD_HEADER_CNT_MASK) << PD_HEADER_CNT_SHIFT) |\
> +  ((ext_hdr) ? PD_HEADER_EXT_HDR : 0))
>  
>  #define PD_HEADER_LE(type, pwr, data, id, cnt) \
> - cpu_to_le16(PD_HEADER((type), (pwr), (data), (id), (cnt)))
> + cpu_to_le16(PD_HEADER((type), (pwr), (data), (id), (cnt), (0)))
>  
>  static inline unsigned int pd_header_cnt(u16 header)
>  {
> @@ -102,16 +135,66 @@ static inline unsigned int pd_header_msgid_le(__le16 
> header)
>   return pd_header_msgid(le16_to_cpu(header));
>  }
>  
> +#define PD_EXT_HDR_CHUNKED   BIT(15)
> +#define PD_EXT_HDR_CHUNK_NUM_SHIFT   11
> +#define PD_EXT_HDR_CHUNK_NUM_MASK0xf
> +#define PD_EXT_HDR_REQ_CHUNK BIT(10)
> +#define PD_EXT_HDR_DATA_SIZE_SHIFT   0
> +#define PD_EXT_HDR_DATA_SIZE_MASK0x1ff
> +
> +#define PD_EXT_HDR(data_size, req_chunk, chunk_num, chunked) 
> \
> + data_size) & PD_EXT_HDR_DATA_SIZE_MASK) << 
> PD_EXT_HDR_DATA_SIZE_SHIFT) |\
> +  ((req_chunk) ? PD_EXT_HDR_REQ_CHUNK : 0) | 
> \
> +  (((chunk_num) & PD_EXT_HDR_CHUNK_NUM_MASK) << 
> PD_EXT_HDR_CHUNK_NUM_SHIFT) |\
> +  ((chunked) ? PD_EXT_HDR_CHUNKED : 0))
> +
> +#define PD_EXT_HDR_LE(data_size, req_chunk, chunk_num, chunked) \
> + cpu_to_le16(PD_EXT_HDR((data_size), (req_chunk), (chunk_num), 
> (chunked)))
> +
> +static inline unsigned int pd_ext_header_chunk_num(u16 ext_header)
> +{
> + return (ext_header >> PD_EXT_HDR_CHUNK_NUM_SHIFT) &
> + PD_EXT_HDR_CHUNK_NUM_MASK;
> +}
> +
> +static inline unsigned int pd_ext_header_data_size(u16 ext_header)
> +{
> + return (ext_header >> PD_EXT_HDR_DATA_SIZE_SHIFT) &
> + PD_EXT_HDR_DATA_SIZE_MASK;
> +}
> +
> +static inline unsigned int pd_ext_header_data_size_le(__le16 ext_header)
> +{
> + return pd_ext_header_data_size(le16_to_cpu(ext_header));
> +}
> +
>  #define PD_MAX_PAYLOAD   7
> +#define PD_EXT_MAX_LEGACY_DATA   26
> +#define PD_EXT_MAX_CHUNK_DATA26
> +#define PD_EXT_MAX_DATA  260
>  
>  /**
> - * struct pd_message - PD message as seen on wire
> - * @header:  PD message header
> - * @payload: PD message payload
> - */
> +  * struct pd_ext_message_data - PD extended message data as seen on wire
> +  * @header:PD extended message header
> +  * @data:  PD extended message data
> +  */
> +struct pd_ext_message_data {
> + __le16 header;
> + u8 data[PD_EXT_MAX_DATA];
> +} __packed;
> +
> +/**
> +  * struct pd_message - PD message as seen on wire
> +  * @header:PD message header
> +  * @payload:   PD message payload
> +  * @ext_msg:   PD message extended message data
> +  */
>  struct pd_message {
>   __le16 header;
> - __le32 payload[PD_MAX_PAYLOAD];
> + union {
> + __le32 payload[PD_MAX_PAYLOAD];
> + struct pd_ext_message_data ext_msg;
> + };
>  } __packed;

It 

Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

[Jack Pham]

Hi Manu,

On Thu, Sep 28, 2017 at 09:30:38AM +0530, Manu Gautam wrote:
> On 9/28/2017 12:46 AM, Jack Pham wrote:
> > On Wed, Sep 27, 2017 at 10:57:41AM -0700, Jack Pham wrote:
> >> On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> >>> VBUS signal coming from PHY must be asserted in device for
> >>> controller to start operation or assert pull-up. For some
> >>> platforms where VBUS line is not connected to PHY there is
> >>> HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> >>> by software to override VBUS signal going to controller.
> >>>
> >>> Signed-off-by: Manu Gautam <mgau...@codeaurora.org>
> >>> ---
> >>>  
> >>> +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> >>> +{
> >>> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
> >>> +
> >>> + qphy->mode = mode;
> >>> +
> >>> + /* Update VBUS override in qscratch register */
> >>> + if (qphy->qscratch_base) {
> >>> + if (mode == PHY_MODE_USB_DEVICE)
> >>> + qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> >>> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> >>> + else
> >>> + qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> >>> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> >> Wouldn't this be better off handled in the controller glue driver? Two
> >> reasons I think this patch is unattractive:
> >>
> >> - qscratch_base is part of the controller's register space. Your later
> >>   patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
> >>   device mode") does a similar thing and hence both drivers have to
> >>   ioremap() the same register resource while at the same time avoiding
> >>   request_mem_region() (called by devm_ioremap_resource) to allow it to
> >>   be mapped in both places.
> 
> Right. There is one more reason why qusb2 driver needs qscratch:
> - During runtime suspend, it has to check linestate to set correct  polarity 
> for dp/dm
>   wakeup interrupt in order to detect disconnect/resume ion LS and FS/HS 
> modes.

Ugh, oh yeah. The way I understand we did it in our downstream driver
is still to have the controller driver read the linestate but then pass
the information via additional set_mode() flags which the PHY driver
could use to correctly arm the interrupt trigger polarity.

An alternative would be to access a couple of the debug QUSB2PHY
registers that also provide a reading of the current UTMI linestate. The
HPG mentions them vaguely, and I can't remember if we tested that
interface or not. Assuming it works, would that be preferable to reading
a non-PHY register here?

> >> - VBUS override bit becomes asserted simply because the mode is changed
> >>   to device mode but this is irrespective of the actual VBUS state. This
> >>   could break some test setups which perform a logical disconnect by
> >>   switching off/on VBUS while leaving data lines connected. Controller
> >>   would go merrily along thinking it is still attached to the host.
> >>
> >> Instead maybe this could be tied to EXTCON_USB handling in the glue
> >> driver; though it would need to be an additional notifier on top of
> >> dwc3/drd.c which already handles extcon for host/device mode.
> 
> Yes, dwc3/drd.c currently deals with only EXTCON_USB_HOST. So, for platforms
> where role swap happens using only Vbus or single GPIO this should take care 
> of.
> 
> 
> > That is to say, we'd probably need to split out dwc3-qcom from
> > dwc3-of-simple.c into its own driver (again) in order to add this.
> >
> > Jack
> 
> However, I agree that more appropriate place for lane0-pwr-present and
> vbus override update is dwc3 glue driver. Since we don't have one right now,
> 
> IMO once we have dwc3-qcom driver in place, this handling can be moved from
> PHY to glue driver. Until then we can use this approach to get USB device mode
> working on qcom platforms which are using dwc3-of-simple.c e.g. sdm820
> dragonboard.

Could that be done in this series too? IMO better to get it right in one
shot. Is this aimed for 4.15?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

[Jack Pham]

Hi Manu,

On Thu, Sep 28, 2017 at 09:30:38AM +0530, Manu Gautam wrote:
> On 9/28/2017 12:46 AM, Jack Pham wrote:
> > On Wed, Sep 27, 2017 at 10:57:41AM -0700, Jack Pham wrote:
> >> On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> >>> VBUS signal coming from PHY must be asserted in device for
> >>> controller to start operation or assert pull-up. For some
> >>> platforms where VBUS line is not connected to PHY there is
> >>> HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> >>> by software to override VBUS signal going to controller.
> >>>
> >>> Signed-off-by: Manu Gautam 
> >>> ---
> >>>  
> >>> +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> >>> +{
> >>> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
> >>> +
> >>> + qphy->mode = mode;
> >>> +
> >>> + /* Update VBUS override in qscratch register */
> >>> + if (qphy->qscratch_base) {
> >>> + if (mode == PHY_MODE_USB_DEVICE)
> >>> + qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> >>> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> >>> + else
> >>> + qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> >>> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> >> Wouldn't this be better off handled in the controller glue driver? Two
> >> reasons I think this patch is unattractive:
> >>
> >> - qscratch_base is part of the controller's register space. Your later
> >>   patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
> >>   device mode") does a similar thing and hence both drivers have to
> >>   ioremap() the same register resource while at the same time avoiding
> >>   request_mem_region() (called by devm_ioremap_resource) to allow it to
> >>   be mapped in both places.
> 
> Right. There is one more reason why qusb2 driver needs qscratch:
> - During runtime suspend, it has to check linestate to set correct  polarity 
> for dp/dm
>   wakeup interrupt in order to detect disconnect/resume ion LS and FS/HS 
> modes.

Ugh, oh yeah. The way I understand we did it in our downstream driver
is still to have the controller driver read the linestate but then pass
the information via additional set_mode() flags which the PHY driver
could use to correctly arm the interrupt trigger polarity.

An alternative would be to access a couple of the debug QUSB2PHY
registers that also provide a reading of the current UTMI linestate. The
HPG mentions them vaguely, and I can't remember if we tested that
interface or not. Assuming it works, would that be preferable to reading
a non-PHY register here?

> >> - VBUS override bit becomes asserted simply because the mode is changed
> >>   to device mode but this is irrespective of the actual VBUS state. This
> >>   could break some test setups which perform a logical disconnect by
> >>   switching off/on VBUS while leaving data lines connected. Controller
> >>   would go merrily along thinking it is still attached to the host.
> >>
> >> Instead maybe this could be tied to EXTCON_USB handling in the glue
> >> driver; though it would need to be an additional notifier on top of
> >> dwc3/drd.c which already handles extcon for host/device mode.
> 
> Yes, dwc3/drd.c currently deals with only EXTCON_USB_HOST. So, for platforms
> where role swap happens using only Vbus or single GPIO this should take care 
> of.
> 
> 
> > That is to say, we'd probably need to split out dwc3-qcom from
> > dwc3-of-simple.c into its own driver (again) in order to add this.
> >
> > Jack
> 
> However, I agree that more appropriate place for lane0-pwr-present and
> vbus override update is dwc3 glue driver. Since we don't have one right now,
> 
> IMO once we have dwc3-qcom driver in place, this handling can be moved from
> PHY to glue driver. Until then we can use this approach to get USB device mode
> working on qcom platforms which are using dwc3-of-simple.c e.g. sdm820
> dragonboard.

Could that be done in this series too? IMO better to get it right in one
shot. Is this aimed for 4.15?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

[Jack Pham]

On Wed, Sep 27, 2017 at 10:57:41AM -0700, Jack Pham wrote:
> Hi Manu,
> 
> On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> > VBUS signal coming from PHY must be asserted in device for
> > controller to start operation or assert pull-up. For some
> > platforms where VBUS line is not connected to PHY there is
> > HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> > by software to override VBUS signal going to controller.
> > 
> > Signed-off-by: Manu Gautam <mgau...@codeaurora.org>
> > ---
> >  
> > +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> > +{
> > +   struct qusb2_phy *qphy = phy_get_drvdata(phy);
> > +
> > +   qphy->mode = mode;
> > +
> > +   /* Update VBUS override in qscratch register */
> > +   if (qphy->qscratch_base) {
> > +   if (mode == PHY_MODE_USB_DEVICE)
> > +   qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> > + UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> > +   else
> > +   qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> > + UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> 
> Wouldn't this be better off handled in the controller glue driver? Two
> reasons I think this patch is unattractive:
> 
> - qscratch_base is part of the controller's register space. Your later
>   patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
>   device mode") does a similar thing and hence both drivers have to
>   ioremap() the same register resource while at the same time avoiding
>   request_mem_region() (called by devm_ioremap_resource) to allow it to
>   be mapped in both places.
> 
> - VBUS override bit becomes asserted simply because the mode is changed
>   to device mode but this is irrespective of the actual VBUS state. This
>   could break some test setups which perform a logical disconnect by
>   switching off/on VBUS while leaving data lines connected. Controller
>   would go merrily along thinking it is still attached to the host.
> 
> Instead maybe this could be tied to EXTCON_USB handling in the glue
> driver; though it would need to be an additional notifier on top of
> dwc3/drd.c which already handles extcon for host/device mode.

That is to say, we'd probably need to split out dwc3-qcom from
dwc3-of-simple.c into its own driver (again) in order to add this.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

[Jack Pham]

On Wed, Sep 27, 2017 at 10:57:41AM -0700, Jack Pham wrote:
> Hi Manu,
> 
> On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> > VBUS signal coming from PHY must be asserted in device for
> > controller to start operation or assert pull-up. For some
> > platforms where VBUS line is not connected to PHY there is
> > HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> > by software to override VBUS signal going to controller.
> > 
> > Signed-off-by: Manu Gautam 
> > ---
> >  
> > +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> > +{
> > +   struct qusb2_phy *qphy = phy_get_drvdata(phy);
> > +
> > +   qphy->mode = mode;
> > +
> > +   /* Update VBUS override in qscratch register */
> > +   if (qphy->qscratch_base) {
> > +   if (mode == PHY_MODE_USB_DEVICE)
> > +   qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> > + UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> > +   else
> > +   qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> > + UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> 
> Wouldn't this be better off handled in the controller glue driver? Two
> reasons I think this patch is unattractive:
> 
> - qscratch_base is part of the controller's register space. Your later
>   patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
>   device mode") does a similar thing and hence both drivers have to
>   ioremap() the same register resource while at the same time avoiding
>   request_mem_region() (called by devm_ioremap_resource) to allow it to
>   be mapped in both places.
> 
> - VBUS override bit becomes asserted simply because the mode is changed
>   to device mode but this is irrespective of the actual VBUS state. This
>   could break some test setups which perform a logical disconnect by
>   switching off/on VBUS while leaving data lines connected. Controller
>   would go merrily along thinking it is still attached to the host.
> 
> Instead maybe this could be tied to EXTCON_USB handling in the glue
> driver; though it would need to be an additional notifier on top of
> dwc3/drd.c which already handles extcon for host/device mode.

That is to say, we'd probably need to split out dwc3-qcom from
dwc3-of-simple.c into its own driver (again) in order to add this.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 13/17] phy: qcom-qmp: Add support for QMP V3 USB3 PHY

[Jack Pham]

On Wed, Sep 27, 2017 at 02:29:09PM +0530, Manu Gautam wrote:
> QMP V3 USB3 PHY is a DP USB combo PHY with
> dual RX/TX lanes to support type-c. There is a
> separate block DP_COM for configuration related
> to type-c or DP. Add support for dp_com region
> and secondary rx/tx lanes initialization.

Clarify "DP" as DisplayPort here?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 13/17] phy: qcom-qmp: Add support for QMP V3 USB3 PHY

[Jack Pham]

On Wed, Sep 27, 2017 at 02:29:09PM +0530, Manu Gautam wrote:
> QMP V3 USB3 PHY is a DP USB combo PHY with
> dual RX/TX lanes to support type-c. There is a
> separate block DP_COM for configuration related
> to type-c or DP. Add support for dp_com region
> and secondary rx/tx lanes initialization.

Clarify "DP" as DisplayPort here?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

[Jack Pham]

Hi Manu,

On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> VBUS signal coming from PHY must be asserted in device for
> controller to start operation or assert pull-up. For some
> platforms where VBUS line is not connected to PHY there is
> HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> by software to override VBUS signal going to controller.
> 
> Signed-off-by: Manu Gautam 
> ---
>  
> +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> +{
> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
> +
> + qphy->mode = mode;
> +
> + /* Update VBUS override in qscratch register */
> + if (qphy->qscratch_base) {
> + if (mode == PHY_MODE_USB_DEVICE)
> + qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> + else
> + qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);

Wouldn't this be better off handled in the controller glue driver? Two
reasons I think this patch is unattractive:

- qscratch_base is part of the controller's register space. Your later
  patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
  device mode") does a similar thing and hence both drivers have to
  ioremap() the same register resource while at the same time avoiding
  request_mem_region() (called by devm_ioremap_resource) to allow it to
  be mapped in both places.

- VBUS override bit becomes asserted simply because the mode is changed
  to device mode but this is irrespective of the actual VBUS state. This
  could break some test setups which perform a logical disconnect by
  switching off/on VBUS while leaving data lines connected. Controller
  would go merrily along thinking it is still attached to the host.

Instead maybe this could be tied to EXTCON_USB handling in the glue
driver; though it would need to be an additional notifier on top of
dwc3/drd.c which already handles extcon for host/device mode.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

[Jack Pham]

Hi Manu,

On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> VBUS signal coming from PHY must be asserted in device for
> controller to start operation or assert pull-up. For some
> platforms where VBUS line is not connected to PHY there is
> HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> by software to override VBUS signal going to controller.
> 
> Signed-off-by: Manu Gautam 
> ---
>  
> +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> +{
> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
> +
> + qphy->mode = mode;
> +
> + /* Update VBUS override in qscratch register */
> + if (qphy->qscratch_base) {
> + if (mode == PHY_MODE_USB_DEVICE)
> + qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> + else
> + qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> +   UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);

Wouldn't this be better off handled in the controller glue driver? Two
reasons I think this patch is unattractive:

- qscratch_base is part of the controller's register space. Your later
  patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
  device mode") does a similar thing and hence both drivers have to
  ioremap() the same register resource while at the same time avoiding
  request_mem_region() (called by devm_ioremap_resource) to allow it to
  be mapped in both places.

- VBUS override bit becomes asserted simply because the mode is changed
  to device mode but this is irrespective of the actual VBUS state. This
  could break some test setups which perform a logical disconnect by
  switching off/on VBUS while leaving data lines connected. Controller
  would go merrily along thinking it is still attached to the host.

Instead maybe this could be tied to EXTCON_USB handling in the glue
driver; though it would need to be an additional notifier on top of
dwc3/drd.c which already handles extcon for host/device mode.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usb: gadget: pch_udc: add checks for dma mapping errors

[Jack Pham]

On Thu, Aug 24, 2017 at 01:47:08AM +0300, Alexey Khoroshilov wrote:
> There are no checks for dma mapping errors in pch_udc.
> Tha patch adds the checks and error handling code.
> Compile tested only.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov 
> ---
>  drivers/usb/gadget/udc/pch_udc.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/pch_udc.c 
> b/drivers/usb/gadget/udc/pch_udc.c
> index 84dcbcd756f0..a305f8392082 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -1767,7 +1767,7 @@ static struct usb_request *pch_udc_alloc_request(struct 
> usb_ep *usbep,
>   req->req.dma = DMA_ADDR_INVALID;
>   req->dma = DMA_ADDR_INVALID;
>   INIT_LIST_HEAD(>queue);
> - if (!ep->dev->dma_addr)
> + if (ep->dev->dma_addr != DMA_ADDR_INVALID)
>   return >req;
>   /* ep0 in requests are allocated from data pool here */
>   dma_desc = dma_pool_alloc(ep->dev->data_requests, gfp,
> @@ -1879,6 +1879,13 @@ static int pch_udc_pcd_queue(struct usb_ep *usbep, 
> struct usb_request *usbreq,
> usbreq->length,
> DMA_FROM_DEVICE);
>   }
> + if (dma_mapping_error(>pdev->dev, req->dma)) {
> + req->dma = DMA_ADDR_INVALID;
> + retval = -ENOMEM;
> + if ((unsigned long)(usbreq->buf) & 0x03)
> + kfree(req->buf);
> + goto probe_end;
> + }
>   req->dma_mapped = 1;
>   }
>   if (usbreq->length > 0) {
> @@ -2961,6 +2968,10 @@ static int init_dma_pools(struct pch_udc_dev *dev)
>   dev->dma_addr = dma_map_single(>pdev->dev, ep0out_buf,
>  UDC_EP0OUT_BUFF_SIZE * 4,
>  DMA_FROM_DEVICE);
> + if (dma_mapping_error(>pdev->dev, dev->dma_addr)) {
> + dev->dma_addr = DMA_ADDR_INVALID;
> + return -ENOMEM;
> + }

Wouldn't this driver be better off using the
usb_gadget_{map,unmap}_request() functions provided by UDC core.c?
dma_mapping_error() is provided for free that way.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usb: gadget: pch_udc: add checks for dma mapping errors

[Jack Pham]

On Thu, Aug 24, 2017 at 01:47:08AM +0300, Alexey Khoroshilov wrote:
> There are no checks for dma mapping errors in pch_udc.
> Tha patch adds the checks and error handling code.
> Compile tested only.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov 
> ---
>  drivers/usb/gadget/udc/pch_udc.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/pch_udc.c 
> b/drivers/usb/gadget/udc/pch_udc.c
> index 84dcbcd756f0..a305f8392082 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -1767,7 +1767,7 @@ static struct usb_request *pch_udc_alloc_request(struct 
> usb_ep *usbep,
>   req->req.dma = DMA_ADDR_INVALID;
>   req->dma = DMA_ADDR_INVALID;
>   INIT_LIST_HEAD(>queue);
> - if (!ep->dev->dma_addr)
> + if (ep->dev->dma_addr != DMA_ADDR_INVALID)
>   return >req;
>   /* ep0 in requests are allocated from data pool here */
>   dma_desc = dma_pool_alloc(ep->dev->data_requests, gfp,
> @@ -1879,6 +1879,13 @@ static int pch_udc_pcd_queue(struct usb_ep *usbep, 
> struct usb_request *usbreq,
> usbreq->length,
> DMA_FROM_DEVICE);
>   }
> + if (dma_mapping_error(>pdev->dev, req->dma)) {
> + req->dma = DMA_ADDR_INVALID;
> + retval = -ENOMEM;
> + if ((unsigned long)(usbreq->buf) & 0x03)
> + kfree(req->buf);
> + goto probe_end;
> + }
>   req->dma_mapped = 1;
>   }
>   if (usbreq->length > 0) {
> @@ -2961,6 +2968,10 @@ static int init_dma_pools(struct pch_udc_dev *dev)
>   dev->dma_addr = dma_map_single(>pdev->dev, ep0out_buf,
>  UDC_EP0OUT_BUFF_SIZE * 4,
>  DMA_FROM_DEVICE);
> + if (dma_mapping_error(>pdev->dev, dev->dma_addr)) {
> + dev->dma_addr = DMA_ADDR_INVALID;
> + return -ENOMEM;
> + }

Wouldn't this driver be better off using the
usb_gadget_{map,unmap}_request() functions provided by UDC core.c?
dma_mapping_error() is provided for free that way.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v12 6/9] usb: xhci: use bus->sysdev for DMA configuration

[Jack Pham]

Hi Peter, Sriram, Arnd,

On Mon, Feb 06, 2017 at 05:13:38PM +0800, Peter Chen wrote:
> From: Arnd Bergmann 
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices. So, set
> the dma for xhci from sysdev. sysdev is pointing to device that
> is known to the system firmware or hardware.
> 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Sriram Dash 
> Tested-by: Baolin Wang 
> Tested-by: Vivek Gautam 
> Tested-by: Alexander Sverdlin 
> Signed-off-by: Mathias Nyman 
> ---
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 6d33b42..7a9c860 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c

> - hcd = usb_create_hcd(driver, >dev, dev_name(>dev));
> + hcd = __usb_create_hcd(driver, sysdev, >dev,
> +dev_name(>dev), NULL);

As mentioned already in [1], usb_create_shared_hcd() is called to create
the second bus, however it also needs to be converted.

Not exactly as Roger's suggestion but this worked for me:

-   xhci->shared_hcd = usb_create_shared_hcd(driver, >dev,
+   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, >dev,
dev_name(>dev), hcd);
if (!xhci->shared_hcd) {
ret = -ENOMEM;

Without this, SuperSpeed devices fail to enumerate:

 usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 usb 2-1: new SuperSpeed USB device number 3 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 usb 2-1: new SuperSpeed USB device number 3 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 

Thanks,
Jack

[1] https://lkml.org/lkml/2016/12/9/240
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v12 6/9] usb: xhci: use bus->sysdev for DMA configuration

[Jack Pham]

Hi Peter, Sriram, Arnd,

On Mon, Feb 06, 2017 at 05:13:38PM +0800, Peter Chen wrote:
> From: Arnd Bergmann 
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices. So, set
> the dma for xhci from sysdev. sysdev is pointing to device that
> is known to the system firmware or hardware.
> 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Sriram Dash 
> Tested-by: Baolin Wang 
> Tested-by: Vivek Gautam 
> Tested-by: Alexander Sverdlin 
> Signed-off-by: Mathias Nyman 
> ---
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 6d33b42..7a9c860 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c

> - hcd = usb_create_hcd(driver, >dev, dev_name(>dev));
> + hcd = __usb_create_hcd(driver, sysdev, >dev,
> +dev_name(>dev), NULL);

As mentioned already in [1], usb_create_shared_hcd() is called to create
the second bus, however it also needs to be converted.

Not exactly as Roger's suggestion but this worked for me:

-   xhci->shared_hcd = usb_create_shared_hcd(driver, >dev,
+   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, >dev,
dev_name(>dev), hcd);
if (!xhci->shared_hcd) {
ret = -ENOMEM;

Without this, SuperSpeed devices fail to enumerate:

 usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 usb 2-1: new SuperSpeed USB device number 3 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 usb 2-1: new SuperSpeed USB device number 3 using xhci-hcd
 usb 2-1: device descriptor read/8, error -11
 

Thanks,
Jack

[1] https://lkml.org/lkml/2016/12/9/240
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] regmap: spmi: Fix regmap_spmi_ext_read in multi-byte case

[Jack Pham]

Specifically for the case of reads that use the Extended Register
Read Long command, a multi-byte read operation is broken up into
8-byte chunks.  However the call to spmi_ext_register_readl() is
incorrectly passing 'val_size', which if greater than 8 will
always fail.  The argument should instead be 'len'.

Fixes: c9afbb05a9ff ("regmap: spmi: support base and extended register spaces")
Signed-off-by: Jack Pham <ja...@codeaurora.org>
---
 drivers/base/regmap/regmap-spmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-spmi.c 
b/drivers/base/regmap/regmap-spmi.c
index 7e58f65..4a36e41 100644
--- a/drivers/base/regmap/regmap-spmi.c
+++ b/drivers/base/regmap/regmap-spmi.c
@@ -142,7 +142,7 @@ static int regmap_spmi_ext_read(void *context,
while (val_size) {
len = min_t(size_t, val_size, 8);
 
-   err = spmi_ext_register_readl(context, addr, val, val_size);
+   err = spmi_ext_register_readl(context, addr, val, len);
if (err)
goto err_out;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] regmap: spmi: Fix regmap_spmi_ext_read in multi-byte case

[Jack Pham]

Specifically for the case of reads that use the Extended Register
Read Long command, a multi-byte read operation is broken up into
8-byte chunks.  However the call to spmi_ext_register_readl() is
incorrectly passing 'val_size', which if greater than 8 will
always fail.  The argument should instead be 'len'.

Fixes: c9afbb05a9ff ("regmap: spmi: support base and extended register spaces")
Signed-off-by: Jack Pham 
---
 drivers/base/regmap/regmap-spmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-spmi.c 
b/drivers/base/regmap/regmap-spmi.c
index 7e58f65..4a36e41 100644
--- a/drivers/base/regmap/regmap-spmi.c
+++ b/drivers/base/regmap/regmap-spmi.c
@@ -142,7 +142,7 @@ static int regmap_spmi_ext_read(void *context,
while (val_size) {
len = min_t(size_t, val_size, 8);
 
-   err = spmi_ext_register_readl(context, addr, val, val_size);
+   err = spmi_ext_register_readl(context, addr, val, len);
if (err)
goto err_out;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:08PM -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 
> ---
>  drivers/phy/Kconfig |   11 +
>  drivers/phy/Makefile|1 +
>  drivers/phy/phy-qcom-dwc3.c |  483 
> +++
>  3 files changed, 495 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-dwc3.c

What happened to this patch? Looks like [1/3] & [2/3] made it in during
the 3.18 merge, but did this one not get picked up by Kishon?

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:08PM -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
 ---
  drivers/phy/Kconfig |   11 +
  drivers/phy/Makefile|1 +
  drivers/phy/phy-qcom-dwc3.c |  483 
 +++
  3 files changed, 495 insertions(+)
  create mode 100644 drivers/phy/phy-qcom-dwc3.c

What happened to this patch? Looks like [1/3]  [2/3] made it in during
the 3.18 merge, but did this one not get picked up by Kishon?

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:08PM -0500, Andy Gross wrote:
> +static int qcom_dwc3_hs_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
> +{
> + u32 val;
> +
> + /*
> +  * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
> +  * enable clamping, and disable RETENTION (power-on default is ENABLED)
> +  */
> + val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
> + HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
> + HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
> + HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
> + HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
> +
> + /* use core clock if external reference is not present */
> + if (!phy_dwc3->xo_clk)
> + val |= HSUSB_CTRL_USE_CLKCORE;
> +
> + writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
> + usleep_range(2000, 2200);
> +
> + /* Disable (bypass) VBUS and ID filters */
> + writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);

Is this comment accurate? I believe this bit forces the IP to behave in
XHCI rev 1.0. In which case, shouldn't it be done in the glue driver?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:06PM -0500, Andy Gross wrote:
> +Example device nodes:
> +
> + hs_phy: phy@100f8800 {
> + compatible = "qcom,dwc3-hs-usb-phy";
> + reg = <0x100f8800 0x30>;

Just wanted to point out that in our downstream code, the glue
device/driver, i.e. "qcom,dwc3", does claim an entire register region
that encompasses the same registers used by these PHYs. I noticed you're
not adding a reg resource to the glue node below in this patchset. In
order to allow multiple devices to use the overlapping regions, we avoid
calling devm_ioremap_resource() and instead call devm_ioremap_nocache()
directly which bypasses the request_mem_region() call, which I don't
think is an entirely correct thing to do.

On the other hand I'm trying to think of use cases on our other SOCs
(MSM8974, APQ8084) where the glue actually would need access to the same
or adjacent IO registers that couldn't just be handled directly by these
PHY drivers. Should we accommodate for the potential of overlapping
regions or should we just hold the line here and maybe only expose with
finer granularity the registers that will actually be needed by the PHYs
& glue respectively?

> + clocks = < USB30_0_UTMI_CLK>;
> + clock-names = "ref";
> + #phy-cells = <0>;
> +
> + status = "ok";
> + };
> +
> + ss_phy: phy@100f8830 {
> + compatible = "qcom,dwc3-ss-usb-phy";
> + reg = <0x100f8830 0x30>;
> + clocks = < USB30_0_MASTER_CLK>;
> + clock-names = "ref";
> + #phy-cells = <0>;
> +
> + status = "ok";
> + };
> +
> + usb3_0: usb30@0 {
> + compatible = "qcom,dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = < USB30_0_MASTER_CLK>;
> + clock-names = "core";
> +
> + ranges;
> +
> + status = "ok";
> +
> + dwc3@1000 {
> + compatible = "snps,dwc3";
> + reg = <0x1000 0xcd00>;
> + interrupts = <0 205 0x4>;
> + phys = <_phy>, <_phy>;
> + phy-names = "usb2-phy", "usb3-phy";
> + tx-fifo-resize;
> + dr_mode = "host";
> + };
> + };
> +

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:06PM -0500, Andy Gross wrote:
 +Example device nodes:
 +
 + hs_phy: phy@100f8800 {
 + compatible = qcom,dwc3-hs-usb-phy;
 + reg = 0x100f8800 0x30;

Just wanted to point out that in our downstream code, the glue
device/driver, i.e. qcom,dwc3, does claim an entire register region
that encompasses the same registers used by these PHYs. I noticed you're
not adding a reg resource to the glue node below in this patchset. In
order to allow multiple devices to use the overlapping regions, we avoid
calling devm_ioremap_resource() and instead call devm_ioremap_nocache()
directly which bypasses the request_mem_region() call, which I don't
think is an entirely correct thing to do.

On the other hand I'm trying to think of use cases on our other SOCs
(MSM8974, APQ8084) where the glue actually would need access to the same
or adjacent IO registers that couldn't just be handled directly by these
PHY drivers. Should we accommodate for the potential of overlapping
regions or should we just hold the line here and maybe only expose with
finer granularity the registers that will actually be needed by the PHYs
 glue respectively?

 + clocks = gcc USB30_0_UTMI_CLK;
 + clock-names = ref;
 + #phy-cells = 0;
 +
 + status = ok;
 + };
 +
 + ss_phy: phy@100f8830 {
 + compatible = qcom,dwc3-ss-usb-phy;
 + reg = 0x100f8830 0x30;
 + clocks = gcc USB30_0_MASTER_CLK;
 + clock-names = ref;
 + #phy-cells = 0;
 +
 + status = ok;
 + };
 +
 + usb3_0: usb30@0 {
 + compatible = qcom,dwc3;
 + #address-cells = 1;
 + #size-cells = 1;
 + clocks = gcc USB30_0_MASTER_CLK;
 + clock-names = core;
 +
 + ranges;
 +
 + status = ok;
 +
 + dwc3@1000 {
 + compatible = snps,dwc3;
 + reg = 0x1000 0xcd00;
 + interrupts = 0 205 0x4;
 + phys = hs_phy, ss_phy;
 + phy-names = usb2-phy, usb3-phy;
 + tx-fifo-resize;
 + dr_mode = host;
 + };
 + };
 +

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:08PM -0500, Andy Gross wrote:
 +static int qcom_dwc3_hs_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
 +{
 + u32 val;
 +
 + /*
 +  * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
 +  * enable clamping, and disable RETENTION (power-on default is ENABLED)
 +  */
 + val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
 + HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
 + HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
 + HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
 + HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
 +
 + /* use core clock if external reference is not present */
 + if (!phy_dwc3-xo_clk)
 + val |= HSUSB_CTRL_USE_CLKCORE;
 +
 + writel(val, phy_dwc3-base + HSUSB_PHY_CTRL_REG);
 + usleep_range(2000, 2200);
 +
 + /* Disable (bypass) VBUS and ID filters */
 + writel(HSUSB_GCFG_XHCI_REV, phy_dwc3-base + QSCRATCH_GENERAL_CFG);

Is this comment accurate? I believe this bit forces the IP to behave in
XHCI rev 1.0. In which case, shouldn't it be done in the glue driver?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] usb: dwc3: debugfs: fix regdump offset

[Jack Pham]

As with dwc_readl/writel, the global registers are specified as
offsets starting from the beginning of the xHCI address space,
but the memory region pointed to by dwc->regs already maps to
the start of the global addresses. Fix by offsetting each of the
regs relative to DWC3_GLOBALS_REGS_START.

Signed-off-by: Jack Pham 
---
 drivers/usb/dwc3/debugfs.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index d4a30f1..be4eff7 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -56,7 +56,7 @@
 #define dump_register(nm)  \
 {  \
.name   = __stringify(nm),  \
-   .offset = DWC3_ ##nm,   \
+   .offset = DWC3_ ##nm - DWC3_GLOBALS_REGS_START, \
 }
 
 static const struct debugfs_reg32 dwc3_regs[] = {
-- 
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] usb: dwc3: debugfs: fix regdump offset

[Jack Pham]

As with dwc_readl/writel, the global registers are specified as
offsets starting from the beginning of the xHCI address space,
but the memory region pointed to by dwc-regs already maps to
the start of the global addresses. Fix by offsetting each of the
regs relative to DWC3_GLOBALS_REGS_START.

Signed-off-by: Jack Pham ja...@codeaurora.org
---
 drivers/usb/dwc3/debugfs.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index d4a30f1..be4eff7 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -56,7 +56,7 @@
 #define dump_register(nm)  \
 {  \
.name   = __stringify(nm),  \
-   .offset = DWC3_ ##nm,   \
+   .offset = DWC3_ ##nm - DWC3_GLOBALS_REGS_START, \
 }
 
 static const struct debugfs_reg32 dwc3_regs[] = {
-- 
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/