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

[Manu Gautam]

Hi Kishon,

Please review this so that I can re-submit patch-set based on this approach.

On 10/9/2017 1:33 PM, Manu Gautam wrote:
> Hi Kishon
>
> On 10/5/2017 2:38 PM, Manu Gautam wrote:
>> Kishon,
>> What would you suggest here?
>> Should we add new calls e.g. phy_get/set_current_speed like::
>>
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>> index 78bb0d7..41d9ec2 100644
>> --- a/include/linux/phy/phy.h
>> +++ b/include/linux/phy/phy.h
>> @@ -29,6 +29,14 @@ enum phy_mode {
>>     PHY_MODE_USB_OTG,
>>  };
>>
>> +enum phy_speed {
>> +   PHY_SPEED_INVALID,
>> +   PHY_SPEED_USB_LS,
>> +   PHY_SPEED_USB_FS_HS,
>> +   PHY_SPEED_USB_SS,
>> +};
>> +
>>  /**
>>   * struct phy_ops - set of function pointers for performing phy operations
>>   * @init: operation to be performed for initializing phy
>> @@ -45,6 +53,7 @@ struct phy_ops {
>>     int (*power_on)(struct phy *phy);
>>     int (*power_off)(struct phy *phy);
>>     int (*set_mode)(struct phy *phy, enum phy_mode mode);
>> +   int (*set_speed)(struct phy *phy, enum phy_speed speed);
>>     int (*reset)(struct phy *phy);
>>     struct module *owner;
>>  };
>>
> @Kishon,
> Let me know if we can add set_speed to phy_ops. We need this for glue
> driver to notify PHY of current connection speed to enable appropriate
> wakeup interrupts.
>
>

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

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

[Manu Gautam]

Hi Kishon,

Please review this so that I can re-submit patch-set based on this approach.

On 10/9/2017 1:33 PM, Manu Gautam wrote:
> Hi Kishon
>
> On 10/5/2017 2:38 PM, Manu Gautam wrote:
>> Kishon,
>> What would you suggest here?
>> Should we add new calls e.g. phy_get/set_current_speed like::
>>
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>> index 78bb0d7..41d9ec2 100644
>> --- a/include/linux/phy/phy.h
>> +++ b/include/linux/phy/phy.h
>> @@ -29,6 +29,14 @@ enum phy_mode {
>>     PHY_MODE_USB_OTG,
>>  };
>>
>> +enum phy_speed {
>> +   PHY_SPEED_INVALID,
>> +   PHY_SPEED_USB_LS,
>> +   PHY_SPEED_USB_FS_HS,
>> +   PHY_SPEED_USB_SS,
>> +};
>> +
>>  /**
>>   * struct phy_ops - set of function pointers for performing phy operations
>>   * @init: operation to be performed for initializing phy
>> @@ -45,6 +53,7 @@ struct phy_ops {
>>     int (*power_on)(struct phy *phy);
>>     int (*power_off)(struct phy *phy);
>>     int (*set_mode)(struct phy *phy, enum phy_mode mode);
>> +   int (*set_speed)(struct phy *phy, enum phy_speed speed);
>>     int (*reset)(struct phy *phy);
>>     struct module *owner;
>>  };
>>
> @Kishon,
> Let me know if we can add set_speed to phy_ops. We need this for glue
> driver to notify PHY of current connection speed to enable appropriate
> wakeup interrupts.
>
>

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

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

[Manu Gautam]

Hi Kishon

On 10/5/2017 2:38 PM, Manu Gautam wrote:
> Hi Jack,
>
> On 9/28/2017 10:23 PM, Jack Pham wrote:
>>
>> +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?
> it looks like newer QUSB2 PHY doesn't have a register to read linestate.
> QSCRATCH is the only option.
> However, setting dp/dm wakeup interrupt polarity based on current linestate
> isn't perfect either. It could race with any change in linestate while it was 
> being
> read, resulting in missed wakeup interrupt.
> Same is the case with QMP PHY when trying to check for RX terminations on
> suspend.
> IMO PHY driver should get this info from platform glue or controller driver.
> E.g. current speed -> SS, HS/FS, LS or NONE (if not in session).
>
> Kishon,
> What would you suggest here?
> Should we add new calls e.g. phy_get/set_current_speed like::
>
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 78bb0d7..41d9ec2 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -29,6 +29,14 @@ enum phy_mode {
>     PHY_MODE_USB_OTG,
>  };
>
> +enum phy_speed {
> +   PHY_SPEED_INVALID,
> +   PHY_SPEED_USB_LS,
> +   PHY_SPEED_USB_FS_HS,
> +   PHY_SPEED_USB_SS,
> +};
> +
>  /**
>   * struct phy_ops - set of function pointers for performing phy operations
>   * @init: operation to be performed for initializing phy
> @@ -45,6 +53,7 @@ struct phy_ops {
>     int (*power_on)(struct phy *phy);
>     int (*power_off)(struct phy *phy);
>     int (*set_mode)(struct phy *phy, enum phy_mode mode);
> +   int (*set_speed)(struct phy *phy, enum phy_speed speed);
>     int (*reset)(struct phy *phy);
>     struct module *owner;
>  };
>

@Kishon,
Let me know if we can add set_speed to phy_ops. We need this for glue
driver to notify PHY of current connection speed to enable appropriate
wakeup interrupts.


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

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

[Manu Gautam]

Hi Kishon

On 10/5/2017 2:38 PM, Manu Gautam wrote:
> Hi Jack,
>
> On 9/28/2017 10:23 PM, Jack Pham wrote:
>>
>> +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?
> it looks like newer QUSB2 PHY doesn't have a register to read linestate.
> QSCRATCH is the only option.
> However, setting dp/dm wakeup interrupt polarity based on current linestate
> isn't perfect either. It could race with any change in linestate while it was 
> being
> read, resulting in missed wakeup interrupt.
> Same is the case with QMP PHY when trying to check for RX terminations on
> suspend.
> IMO PHY driver should get this info from platform glue or controller driver.
> E.g. current speed -> SS, HS/FS, LS or NONE (if not in session).
>
> Kishon,
> What would you suggest here?
> Should we add new calls e.g. phy_get/set_current_speed like::
>
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 78bb0d7..41d9ec2 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -29,6 +29,14 @@ enum phy_mode {
>     PHY_MODE_USB_OTG,
>  };
>
> +enum phy_speed {
> +   PHY_SPEED_INVALID,
> +   PHY_SPEED_USB_LS,
> +   PHY_SPEED_USB_FS_HS,
> +   PHY_SPEED_USB_SS,
> +};
> +
>  /**
>   * struct phy_ops - set of function pointers for performing phy operations
>   * @init: operation to be performed for initializing phy
> @@ -45,6 +53,7 @@ struct phy_ops {
>     int (*power_on)(struct phy *phy);
>     int (*power_off)(struct phy *phy);
>     int (*set_mode)(struct phy *phy, enum phy_mode mode);
> +   int (*set_speed)(struct phy *phy, enum phy_speed speed);
>     int (*reset)(struct phy *phy);
>     struct module *owner;
>  };
>

@Kishon,
Let me know if we can add set_speed to phy_ops. We need this for glue
driver to notify PHY of current connection speed to enable appropriate
wakeup interrupts.


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

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

[Manu Gautam]

Hi Jack,

On 9/28/2017 10:23 PM, Jack Pham wrote:
> 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?

it looks like newer QUSB2 PHY doesn't have a register to read linestate.
QSCRATCH is the only option.
However, setting dp/dm wakeup interrupt polarity based on current linestate
isn't perfect either. It could race with any change in linestate while it was 
being
read, resulting in missed wakeup interrupt.
Same is the case with QMP PHY when trying to check for RX terminations on
suspend.
IMO PHY driver should get this info from platform glue or controller driver.
E.g. current speed -> SS, HS/FS, LS or NONE (if not in session).

Kishon,
What would you suggest here?
Should we add new calls e.g. phy_get/set_current_speed like::

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 78bb0d7..41d9ec2 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -29,6 +29,14 @@ enum phy_mode {
    PHY_MODE_USB_OTG,
 };

+enum phy_speed {
+   PHY_SPEED_INVALID,
+   PHY_SPEED_USB_LS,
+   PHY_SPEED_USB_FS_HS,
+   PHY_SPEED_USB_SS,
+};
+
 /**
  * struct phy_ops - set of function pointers for performing phy operations
  * @init: operation to be performed for initializing phy
@@ -45,6 +53,7 @@ struct phy_ops {
    int (*power_on)(struct phy *phy);
    int (*power_off)(struct phy *phy);
    int (*set_mode)(struct phy *phy, enum phy_mode mode);
+   int (*set_speed)(struct phy *phy, enum phy_speed speed);
    int (*reset)(struct phy *phy);
    struct module *owner;
 };


 - 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, 

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

[Manu Gautam]

Hi Jack,

On 9/28/2017 10:23 PM, Jack Pham wrote:
> 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?

it looks like newer QUSB2 PHY doesn't have a register to read linestate.
QSCRATCH is the only option.
However, setting dp/dm wakeup interrupt polarity based on current linestate
isn't perfect either. It could race with any change in linestate while it was 
being
read, resulting in missed wakeup interrupt.
Same is the case with QMP PHY when trying to check for RX terminations on
suspend.
IMO PHY driver should get this info from platform glue or controller driver.
E.g. current speed -> SS, HS/FS, LS or NONE (if not in session).

Kishon,
What would you suggest here?
Should we add new calls e.g. phy_get/set_current_speed like::

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 78bb0d7..41d9ec2 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -29,6 +29,14 @@ enum phy_mode {
    PHY_MODE_USB_OTG,
 };

+enum phy_speed {
+   PHY_SPEED_INVALID,
+   PHY_SPEED_USB_LS,
+   PHY_SPEED_USB_FS_HS,
+   PHY_SPEED_USB_SS,
+};
+
 /**
  * struct phy_ops - set of function pointers for performing phy operations
  * @init: operation to be performed for initializing phy
@@ -45,6 +53,7 @@ struct phy_ops {
    int (*power_on)(struct phy *phy);
    int (*power_off)(struct phy *phy);
    int (*set_mode)(struct phy *phy, enum phy_mode mode);
+   int (*set_speed)(struct phy *phy, enum phy_speed speed);
    int (*reset)(struct phy *phy);
    struct module *owner;
 };


 - 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 

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]

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

[Manu Gautam]

Hi Jack,


On 9/28/2017 12:46 AM, Jack Pham wrote:
> 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.

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.

>> - 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.


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

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

[Manu Gautam]

Hi Jack,


On 9/28/2017 12:46 AM, Jack Pham wrote:
> 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.

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.

>> - 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.


-- 
The Qualcomm Innovation Center, Inc. is a member of the 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 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 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

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

[Manu Gautam]

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 
---
 drivers/phy/qualcomm/phy-qcom-qusb2.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c 
b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index bda1f4c..0e9d88b 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -68,6 +68,11 @@
 #defineQUSB2PHY_IMP_CTRL2  0x224
 #defineQUSB2PHY_CHG_CTRL2  0x23c
 
+/* QSCRATCH register bits */
+#define QSCRATCH_HS_PHY_CTRL   0x10
+#define UTMI_OTG_VBUS_VALIDBIT(20)
+#define SW_SESSVLD_SEL BIT(28)
+
 struct qusb2_phy_init_tbl {
unsigned int offset;
unsigned int val;
@@ -211,6 +216,7 @@ struct qusb2_phy_cfg {
  *
  * @phy: generic phy
  * @base: iomapped memory space for qubs2 phy
+ * @qscratch_base: iomapped memory space for qscratch region
  *
  * @cfg_ahb_clk: AHB2PHY interface clock
  * @ref_clk: phy reference clock
@@ -223,10 +229,12 @@ struct qusb2_phy_cfg {
  *
  * @cfg: phy config data
  * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
+ * @mode: indicate current PHY mode of operation e.g. HOST or DEVICE
  */
 struct qusb2_phy {
struct phy *phy;
void __iomem *base;
+   void __iomem *qscratch_base;
 
struct clk *cfg_ahb_clk;
struct clk *ref_clk;
@@ -239,6 +247,7 @@ struct qusb2_phy {
 
const struct qusb2_phy_cfg *cfg;
bool has_se_clk_scheme;
+   enum phy_mode mode;
 };
 
 static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
@@ -307,6 +316,25 @@ static void qusb2_phy_set_tune2_param(struct qusb2_phy 
*qphy)
qusb2_setbits(qphy->base, QUSB2PHY_PORT_TUNE2, val[0] << 0x4);
 }
 
+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);
+   }
+
+   return 0;
+}
+
 static int qusb2_phy_init(struct phy *phy)
 {
struct qusb2_phy *qphy = phy_get_drvdata(phy);
@@ -473,6 +501,7 @@ static int qusb2_phy_exit(struct phy *phy)
 static const struct phy_ops qusb2_phy_gen_ops = {
.init   = qusb2_phy_init,
.exit   = qusb2_phy_exit,
+   .set_mode   = qusb2_phy_set_mode,
.owner  = THIS_MODULE,
 };
 
@@ -507,6 +536,16 @@ static int qusb2_phy_probe(struct platform_device *pdev)
if (IS_ERR(qphy->base))
return PTR_ERR(qphy->base);
 
+   /* Check if platform uses qscratch wrapper */
+   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qscratch");
+   if (res) {
+   /* Can't request region as used by other phy and glue drivers */
+   qphy->qscratch_base = devm_ioremap(dev, res->start,
+  resource_size(res));
+   if (IS_ERR(qphy->qscratch_base))
+   return PTR_ERR(qphy->qscratch_base);
+   }
+
qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
if (IS_ERR(qphy->cfg_ahb_clk)) {
ret = PTR_ERR(qphy->cfg_ahb_clk);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Manu Gautam]

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 
---
 drivers/phy/qualcomm/phy-qcom-qusb2.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c 
b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index bda1f4c..0e9d88b 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -68,6 +68,11 @@
 #defineQUSB2PHY_IMP_CTRL2  0x224
 #defineQUSB2PHY_CHG_CTRL2  0x23c
 
+/* QSCRATCH register bits */
+#define QSCRATCH_HS_PHY_CTRL   0x10
+#define UTMI_OTG_VBUS_VALIDBIT(20)
+#define SW_SESSVLD_SEL BIT(28)
+
 struct qusb2_phy_init_tbl {
unsigned int offset;
unsigned int val;
@@ -211,6 +216,7 @@ struct qusb2_phy_cfg {
  *
  * @phy: generic phy
  * @base: iomapped memory space for qubs2 phy
+ * @qscratch_base: iomapped memory space for qscratch region
  *
  * @cfg_ahb_clk: AHB2PHY interface clock
  * @ref_clk: phy reference clock
@@ -223,10 +229,12 @@ struct qusb2_phy_cfg {
  *
  * @cfg: phy config data
  * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
+ * @mode: indicate current PHY mode of operation e.g. HOST or DEVICE
  */
 struct qusb2_phy {
struct phy *phy;
void __iomem *base;
+   void __iomem *qscratch_base;
 
struct clk *cfg_ahb_clk;
struct clk *ref_clk;
@@ -239,6 +247,7 @@ struct qusb2_phy {
 
const struct qusb2_phy_cfg *cfg;
bool has_se_clk_scheme;
+   enum phy_mode mode;
 };
 
 static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
@@ -307,6 +316,25 @@ static void qusb2_phy_set_tune2_param(struct qusb2_phy 
*qphy)
qusb2_setbits(qphy->base, QUSB2PHY_PORT_TUNE2, val[0] << 0x4);
 }
 
+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);
+   }
+
+   return 0;
+}
+
 static int qusb2_phy_init(struct phy *phy)
 {
struct qusb2_phy *qphy = phy_get_drvdata(phy);
@@ -473,6 +501,7 @@ static int qusb2_phy_exit(struct phy *phy)
 static const struct phy_ops qusb2_phy_gen_ops = {
.init   = qusb2_phy_init,
.exit   = qusb2_phy_exit,
+   .set_mode   = qusb2_phy_set_mode,
.owner  = THIS_MODULE,
 };
 
@@ -507,6 +536,16 @@ static int qusb2_phy_probe(struct platform_device *pdev)
if (IS_ERR(qphy->base))
return PTR_ERR(qphy->base);
 
+   /* Check if platform uses qscratch wrapper */
+   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qscratch");
+   if (res) {
+   /* Can't request region as used by other phy and glue drivers */
+   qphy->qscratch_base = devm_ioremap(dev, res->start,
+  resource_size(res));
+   if (IS_ERR(qphy->qscratch_base))
+   return PTR_ERR(qphy->qscratch_base);
+   }
+
qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
if (IS_ERR(qphy->cfg_ahb_clk)) {
ret = PTR_ERR(qphy->cfg_ahb_clk);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project