[PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Patrick DELAUNAY-2
On some board, the ID pin is not connected so the B session must be
overridden with "u-boot,force_b_session_valid" but the VBus sensing
must continue to be handle.

To managed it, this patch adds a new DT field
"u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"

Signed-off-by: Patrick Delaunay <[hidden email]>
---

 drivers/usb/gadget/dwc2_udc_otg.c      | 59 +++++++++++++++++---------
 drivers/usb/gadget/dwc2_udc_otg_regs.h |  2 +
 include/usb/dwc2_udc.h                 |  1 +
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c
index eaa5dcb9b1..d20ce6147e 100644
--- a/drivers/usb/gadget/dwc2_udc_otg.c
+++ b/drivers/usb/gadget/dwc2_udc_otg.c
@@ -1014,6 +1014,9 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev)
  platdata->force_b_session_valid =
  dev_read_bool(dev, "u-boot,force-b-session-valid");
 
+ platdata->force_vbus_detection =
+ dev_read_bool(dev, "u-boot,force-vbus-detection");
+
  /* force platdata according compatible */
  drvdata = dev_get_driver_data(dev);
  if (drvdata) {
@@ -1106,31 +1109,45 @@ static int dwc2_udc_otg_probe(struct udevice *dev)
  if (ret)
  return ret;
 
- if (CONFIG_IS_ENABLED(DM_REGULATOR) &&
-    platdata->activate_stm_id_vb_detection &&
-    !platdata->force_b_session_valid) {
- ret = device_get_supply_regulator(dev, "usb33d-supply",
-  &priv->usb33d_supply);
- if (ret) {
- dev_err(dev, "can't get voltage level detector supply\n");
- return ret;
+ if (platdata->activate_stm_id_vb_detection) {
+ if (CONFIG_IS_ENABLED(DM_REGULATOR) &&
+    (!platdata->force_b_session_valid ||
+     platdata->force_vbus_detection)) {
+ ret = device_get_supply_regulator(dev, "usb33d-supply",
+  &priv->usb33d_supply);
+ if (ret) {
+ dev_err(dev, "can't get voltage level detector supply\n");
+ return ret;
+ }
+ ret = regulator_set_enable(priv->usb33d_supply, true);
+ if (ret) {
+ dev_err(dev, "can't enable voltage level detector supply\n");
+ return ret;
+ }
  }
- ret = regulator_set_enable(priv->usb33d_supply, true);
- if (ret) {
- dev_err(dev, "can't enable voltage level detector supply\n");
- return ret;
+
+ if (platdata->force_b_session_valid &&
+    !platdata->force_vbus_detection) {
+ /* Override VBUS detection: enable then value*/
+ setbits_le32(&usbotg_reg->gotgctl, VB_VALOEN);
+ setbits_le32(&usbotg_reg->gotgctl, VB_VALOVAL);
+ } else {
+ /* Enable VBUS sensing */
+ setbits_le32(&usbotg_reg->ggpio,
+     GGPIO_STM32_OTG_GCCFG_VBDEN);
+ }
+ if (platdata->force_b_session_valid) {
+ /* Override B session bits: enable then value */
+ setbits_le32(&usbotg_reg->gotgctl, A_VALOEN | B_VALOEN);
+ setbits_le32(&usbotg_reg->gotgctl,
+     A_VALOVAL | B_VALOVAL);
+ } else {
+ /* Enable ID detection */
+ setbits_le32(&usbotg_reg->ggpio,
+     GGPIO_STM32_OTG_GCCFG_IDEN);
  }
- /* Enable vbus sensing */
- setbits_le32(&usbotg_reg->ggpio,
-     GGPIO_STM32_OTG_GCCFG_VBDEN |
-     GGPIO_STM32_OTG_GCCFG_IDEN);
  }
 
- if (platdata->force_b_session_valid)
- /* Override B session bits : value and enable */
- setbits_le32(&usbotg_reg->gotgctl,
-     A_VALOEN | A_VALOVAL | B_VALOEN | B_VALOVAL);
-
  ret = dwc2_udc_probe(platdata);
  if (ret)
  return ret;
diff --git a/drivers/usb/gadget/dwc2_udc_otg_regs.h b/drivers/usb/gadget/dwc2_udc_otg_regs.h
index 2eda5c3720..9ca6f42375 100644
--- a/drivers/usb/gadget/dwc2_udc_otg_regs.h
+++ b/drivers/usb/gadget/dwc2_udc_otg_regs.h
@@ -94,6 +94,8 @@ struct dwc2_usbotg_reg {
 #define B_VALOEN BIT(6)
 #define A_VALOVAL BIT(5)
 #define A_VALOEN BIT(4)
+#define VB_VALOVAL BIT(3)
+#define VB_VALOEN BIT(2)
 
 /* DWC2_UDC_OTG_GOTINT */
 #define GOTGINT_SES_END_DET (1<<2)
diff --git a/include/usb/dwc2_udc.h b/include/usb/dwc2_udc.h
index a2af381a66..aa37e957b4 100644
--- a/include/usb/dwc2_udc.h
+++ b/include/usb/dwc2_udc.h
@@ -28,6 +28,7 @@ struct dwc2_plat_otg_data {
  unsigned int tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
  unsigned char   tx_fifo_sz_nb;
  bool force_b_session_valid;
+ bool force_vbus_detection;
  bool activate_stm_id_vb_detection;
 };
 
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Marek Vasut-3
On 10/15/20 2:49 PM, Patrick Delaunay wrote:
> On some board, the ID pin is not connected so the B session must be
> overridden with "u-boot,force_b_session_valid" but the VBus sensing
> must continue to be handle.
>
> To managed it, this patch adds a new DT field
> "u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"

How is this solved in Linux ?

btw can you do something about that huge change in indent ? Is it
necessary ?
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Patrick DELAUNAY-2
Hi Marek,

> From: Marek Vasut <[hidden email]>
> Sent: jeudi 15 octobre 2020 15:08
>
> On 10/15/20 2:49 PM, Patrick Delaunay wrote:
> > On some board, the ID pin is not connected so the B session must be
> > overridden with "u-boot,force_b_session_valid" but the VBus sensing
> > must continue to be handle.
> >
> > To managed it, this patch adds a new DT field
> > "u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"
>
> How is this solved in Linux ?

It is managed by Linux DWC2 driver: it is a real OTG driver, with dual mode
support and by usb framework

Throught the properties
&usbotg_hs {
        usb-role-switch;
};

a glue treat the session and the sensing management
see linux/drivers/usb/dwc2/drd.c in linux-next

PS: activate_stm_id_vb_detection is also used in driver = hsotg->params.

As ID pin / vbus is completly managed by the USB TYPE driver C
(STUSB1600 for STMicroelectronics board) and DWC2 driver with dual role
stack (host/gadget).

I don't found a better solution than device tree property for this task in U-Boot as DWC2
driver don't support dual role and U-Boot don't have framework for USB type C controller.

>
> btw can you do something about that huge change in indent ? Is it necessary ?

I move all this code under activate_stm_id_vb_detection (linked to compatible "st,stm32mp1-hsotg")
to avoid impact on other platform as this "sensing" properties are only support for STM32MP15X
(it is linked to USB block detection integrated in SOC)

And after I need to check the
1/ The usb33d-supply is required of vbus or IDPIN sensing
2/ manage Vbus sensing or override (according dt)
3/ manage IDPIN or override (according dt)

I add a new property to be backward compatible (even it the combinaison is less clear)
I protect regulator function call to avoid compilation issue for other platform.

PS: after reading the refmanual, I also split VALOEN and VALOVAL bit update as it is required.

So yes I think it is needed but I can split the patch to simplify the review.

Patrick
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Marek Vasut-3
On 10/15/20 6:52 PM, Patrick DELAUNAY wrote:

Hi,

[...]

>> On 10/15/20 2:49 PM, Patrick Delaunay wrote:
>>> On some board, the ID pin is not connected so the B session must be
>>> overridden with "u-boot,force_b_session_valid" but the VBus sensing
>>> must continue to be handle.
>>>
>>> To managed it, this patch adds a new DT field
>>> "u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"
>>
>> How is this solved in Linux ?
>
> It is managed by Linux DWC2 driver: it is a real OTG driver, with dual mode
> support and by usb framework
>
> Throught the properties
> &usbotg_hs {
> usb-role-switch;
> };
>
> a glue treat the session and the sensing management
> see linux/drivers/usb/dwc2/drd.c in linux-next
>
> PS: activate_stm_id_vb_detection is also used in driver = hsotg->params.
>
> As ID pin / vbus is completly managed by the USB TYPE driver C
> (STUSB1600 for STMicroelectronics board) and DWC2 driver with dual role
> stack (host/gadget).
>
> I don't found a better solution than device tree property for this task in U-Boot as DWC2
> driver don't support dual role and U-Boot don't have framework for USB type C controller.
>
>>
>> btw can you do something about that huge change in indent ? Is it necessary ?
>
> I move all this code under activate_stm_id_vb_detection (linked to compatible "st,stm32mp1-hsotg")
> to avoid impact on other platform as this "sensing" properties are only support for STM32MP15X
> (it is linked to USB block detection integrated in SOC)
>
> And after I need to check the
> 1/ The usb33d-supply is required of vbus or IDPIN sensing
> 2/ manage Vbus sensing or override (according dt)
> 3/ manage IDPIN or override (according dt)
>
> I add a new property to be backward compatible (even it the combinaison is less clear)
> I protect regulator function call to avoid compilation issue for other platform.
>
> PS: after reading the refmanual, I also split VALOEN and VALOVAL bit update as it is required.
>
> So yes I think it is needed but I can split the patch to simplify the review.

I presume you don't feel like implementing proper OTG support, right ?
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Patrick DELAUNAY-2
Hi Marek,

> From: Marek Vasut <[hidden email]>
> Sent: jeudi 15 octobre 2020 19:39
>
> On 10/15/20 6:52 PM, Patrick DELAUNAY wrote:
>
> Hi,
>
> [...]
>
> >> On 10/15/20 2:49 PM, Patrick Delaunay wrote:
> >>> On some board, the ID pin is not connected so the B session must be
> >>> overridden with "u-boot,force_b_session_valid" but the VBus sensing
> >>> must continue to be handle.
> >>>
> >>> To managed it, this patch adds a new DT field
> >>> "u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"
> >>
> >> How is this solved in Linux ?
> >
> > It is managed by Linux DWC2 driver: it is a real OTG driver, with dual
> > mode support and by usb framework
> >
> > Throught the properties
> > &usbotg_hs {
> > usb-role-switch;
> > };
> >
> > a glue treat the session and the sensing management see
> > linux/drivers/usb/dwc2/drd.c in linux-next
> >
> > PS: activate_stm_id_vb_detection is also used in driver = hsotg->params.
> >
> > As ID pin / vbus is completly managed by the USB TYPE driver C
> > (STUSB1600 for STMicroelectronics board) and DWC2 driver with dual
> > role stack (host/gadget).
> >
> > I don't found a better solution than device tree property for this
> > task in U-Boot as DWC2 driver don't support dual role and U-Boot don't have
> framework for USB type C controller.
> >
> >>
> >> btw can you do something about that huge change in indent ? Is it necessary ?
> >
> > I move all this code under activate_stm_id_vb_detection (linked to
> > compatible "st,stm32mp1-hsotg") to avoid impact on other platform as
> > this "sensing" properties are only support for STM32MP15X (it is
> > linked to USB block detection integrated in SOC)
> >
> > And after I need to check the
> > 1/ The usb33d-supply is required of vbus or IDPIN sensing 2/ manage
> > Vbus sensing or override (according dt) 3/ manage IDPIN or override
> > (according dt)
> >
> > I add a new property to be backward compatible (even it the
> > combinaison is less clear) I protect regulator function call to avoid compilation
> issue for other platform.
> >
> > PS: after reading the refmanual, I also split VALOEN and VALOVAL bit update
> as it is required.
> >
> > So yes I think it is needed but I can split the patch to simplify the review.
>
> I presume you don't feel like implementing proper OTG support, right ?

Yes, I am afraid of this task.

Patrick
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Marek Vasut-3
On 10/16/20 6:32 PM, Patrick DELAUNAY wrote:
> Hi Marek,

Hi,

[...]

>>>> On 10/15/20 2:49 PM, Patrick Delaunay wrote:
>>>>> On some board, the ID pin is not connected so the B session must be
>>>>> overridden with "u-boot,force_b_session_valid" but the VBus sensing
>>>>> must continue to be handle.
>>>>>
>>>>> To managed it, this patch adds a new DT field
>>>>> "u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"
>>>>
>>>> How is this solved in Linux ?
>>>
>>> It is managed by Linux DWC2 driver: it is a real OTG driver, with dual
>>> mode support and by usb framework
>>>
>>> Throught the properties
>>> &usbotg_hs {
>>> usb-role-switch;
>>> };
>>>
>>> a glue treat the session and the sensing management see
>>> linux/drivers/usb/dwc2/drd.c in linux-next
>>>
>>> PS: activate_stm_id_vb_detection is also used in driver = hsotg->params.
>>>
>>> As ID pin / vbus is completly managed by the USB TYPE driver C
>>> (STUSB1600 for STMicroelectronics board) and DWC2 driver with dual
>>> role stack (host/gadget).
>>>
>>> I don't found a better solution than device tree property for this
>>> task in U-Boot as DWC2 driver don't support dual role and U-Boot don't have
>> framework for USB type C controller.
>>>
>>>>
>>>> btw can you do something about that huge change in indent ? Is it necessary ?
>>>
>>> I move all this code under activate_stm_id_vb_detection (linked to
>>> compatible "st,stm32mp1-hsotg") to avoid impact on other platform as
>>> this "sensing" properties are only support for STM32MP15X (it is
>>> linked to USB block detection integrated in SOC)
>>>
>>> And after I need to check the
>>> 1/ The usb33d-supply is required of vbus or IDPIN sensing 2/ manage
>>> Vbus sensing or override (according dt) 3/ manage IDPIN or override
>>> (according dt)
>>>
>>> I add a new property to be backward compatible (even it the
>>> combinaison is less clear) I protect regulator function call to avoid compilation
>> issue for other platform.
>>>
>>> PS: after reading the refmanual, I also split VALOEN and VALOVAL bit update
>> as it is required.
>>>
>>> So yes I think it is needed but I can split the patch to simplify the review.
>>
>> I presume you don't feel like implementing proper OTG support, right ?
>
> Yes, I am afraid of this task.

Can you take a look ?

I will pick this patch into next for now.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] usb: dwc2: add "u-boot,force-vbus-detection" for stm32

Patrice Chotard
In reply to this post by Patrick DELAUNAY-2
Hi Patrick

On 10/15/20 2:49 PM, Patrick Delaunay wrote:

> On some board, the ID pin is not connected so the B session must be
> overridden with "u-boot,force_b_session_valid" but the VBus sensing
> must continue to be handle.
>
> To managed it, this patch adds a new DT field
> "u-boot,force-vbus-detection" to use with "u-boot,force_b_session_valid"
>
> Signed-off-by: Patrick Delaunay <[hidden email]>
> ---
>
>  drivers/usb/gadget/dwc2_udc_otg.c      | 59 +++++++++++++++++---------
>  drivers/usb/gadget/dwc2_udc_otg_regs.h |  2 +
>  include/usb/dwc2_udc.h                 |  1 +
>  3 files changed, 41 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c
> index eaa5dcb9b1..d20ce6147e 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg.c
> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
> @@ -1014,6 +1014,9 @@ static int dwc2_udc_otg_ofdata_to_platdata(struct udevice *dev)
>   platdata->force_b_session_valid =
>   dev_read_bool(dev, "u-boot,force-b-session-valid");
>  
> + platdata->force_vbus_detection =
> + dev_read_bool(dev, "u-boot,force-vbus-detection");
> +
>   /* force platdata according compatible */
>   drvdata = dev_get_driver_data(dev);
>   if (drvdata) {
> @@ -1106,31 +1109,45 @@ static int dwc2_udc_otg_probe(struct udevice *dev)
>   if (ret)
>   return ret;
>  
> - if (CONFIG_IS_ENABLED(DM_REGULATOR) &&
> -    platdata->activate_stm_id_vb_detection &&
> -    !platdata->force_b_session_valid) {
> - ret = device_get_supply_regulator(dev, "usb33d-supply",
> -  &priv->usb33d_supply);
> - if (ret) {
> - dev_err(dev, "can't get voltage level detector supply\n");
> - return ret;
> + if (platdata->activate_stm_id_vb_detection) {
> + if (CONFIG_IS_ENABLED(DM_REGULATOR) &&
> +    (!platdata->force_b_session_valid ||
> +     platdata->force_vbus_detection)) {
> + ret = device_get_supply_regulator(dev, "usb33d-supply",
> +  &priv->usb33d_supply);
> + if (ret) {
> + dev_err(dev, "can't get voltage level detector supply\n");
> + return ret;
> + }
> + ret = regulator_set_enable(priv->usb33d_supply, true);
> + if (ret) {
> + dev_err(dev, "can't enable voltage level detector supply\n");
> + return ret;
> + }
>   }
> - ret = regulator_set_enable(priv->usb33d_supply, true);
> - if (ret) {
> - dev_err(dev, "can't enable voltage level detector supply\n");
> - return ret;
> +
> + if (platdata->force_b_session_valid &&
> +    !platdata->force_vbus_detection) {
> + /* Override VBUS detection: enable then value*/
> + setbits_le32(&usbotg_reg->gotgctl, VB_VALOEN);
> + setbits_le32(&usbotg_reg->gotgctl, VB_VALOVAL);
> + } else {
> + /* Enable VBUS sensing */
> + setbits_le32(&usbotg_reg->ggpio,
> +     GGPIO_STM32_OTG_GCCFG_VBDEN);
> + }
> + if (platdata->force_b_session_valid) {
> + /* Override B session bits: enable then value */
> + setbits_le32(&usbotg_reg->gotgctl, A_VALOEN | B_VALOEN);
> + setbits_le32(&usbotg_reg->gotgctl,
> +     A_VALOVAL | B_VALOVAL);
> + } else {
> + /* Enable ID detection */
> + setbits_le32(&usbotg_reg->ggpio,
> +     GGPIO_STM32_OTG_GCCFG_IDEN);
>   }
> - /* Enable vbus sensing */
> - setbits_le32(&usbotg_reg->ggpio,
> -     GGPIO_STM32_OTG_GCCFG_VBDEN |
> -     GGPIO_STM32_OTG_GCCFG_IDEN);
>   }
>  
> - if (platdata->force_b_session_valid)
> - /* Override B session bits : value and enable */
> - setbits_le32(&usbotg_reg->gotgctl,
> -     A_VALOEN | A_VALOVAL | B_VALOEN | B_VALOVAL);
> -
>   ret = dwc2_udc_probe(platdata);
>   if (ret)
>   return ret;
> diff --git a/drivers/usb/gadget/dwc2_udc_otg_regs.h b/drivers/usb/gadget/dwc2_udc_otg_regs.h
> index 2eda5c3720..9ca6f42375 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg_regs.h
> +++ b/drivers/usb/gadget/dwc2_udc_otg_regs.h
> @@ -94,6 +94,8 @@ struct dwc2_usbotg_reg {
>  #define B_VALOEN BIT(6)
>  #define A_VALOVAL BIT(5)
>  #define A_VALOEN BIT(4)
> +#define VB_VALOVAL BIT(3)
> +#define VB_VALOEN BIT(2)
>  
>  /* DWC2_UDC_OTG_GOTINT */
>  #define GOTGINT_SES_END_DET (1<<2)
> diff --git a/include/usb/dwc2_udc.h b/include/usb/dwc2_udc.h
> index a2af381a66..aa37e957b4 100644
> --- a/include/usb/dwc2_udc.h
> +++ b/include/usb/dwc2_udc.h
> @@ -28,6 +28,7 @@ struct dwc2_plat_otg_data {
>   unsigned int tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
>   unsigned char   tx_fifo_sz_nb;
>   bool force_b_session_valid;
> + bool force_vbus_detection;
>   bool activate_stm_id_vb_detection;
>  };
>  

Reviewed-by: Patrice Chotard <[hidden email]>

Thanks