[v4,3/3] usb: dwc2: don't use ID/Vbus detection if usb-role-switch on STM32MP15 SoCs

Commit Message

Amelie Delaunay July 27, 2020, 9:23 a.m. UTC

If usb-role-switch is present in the device tree, it means that ID and Vbus
signals are not connected to the OTG controller but to an external
component (GPIOs, Type-C controller). In this configuration, usb role
switch is used to force valid sessions on STM32MP15 SoCs.

Acked-by: Minas Harutyunyan <[e-mail]>
Signed-off-by: Amelie Delaunay <[e-mail]>
---
No changes in v4.
---
 drivers/usb/dwc2/params.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Martin Blumenstingl July 27, 2020, 5:49 p.m. UTC | #1

Hi Amelie,

On Mon, Jul 27, 2020 at 11:23 AM Amelie Delaunay <[e-mail]> wrote:
[...]
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index a3611cdd1dea..65f8596f6795 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -183,9 +183,11 @@ static void dwc2_set_stm32mp15_fsotg_params(struct dwc2_hsotg *hsotg)
>  static void dwc2_set_stm32mp15_hsotg_params(struct dwc2_hsotg *hsotg)
>  {
>         struct dwc2_core_params *p = &hsotg->params;
> +       struct device_node *np = hsotg->dev->of_node;
>
>         p->otg_cap = DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE;
> -       p->activate_stm_id_vb_detection = true;
> +       p->activate_stm_id_vb_detection =
> +               !of_property_read_bool(np, "usb-role-switch");
the rest of params.c uses device_property_read_* instead of of_read_property_*
I thought I'd mention it so you can decide yourself whether this is
fine or needs to be changed


Martin

Andy Shevchenko July 27, 2020, 7:44 p.m. UTC | #2

On Mon, Jul 27, 2020 at 10:04 PM Martin Blumenstingl
<[e-mail]> wrote:
> On Mon, Jul 27, 2020 at 11:23 AM Amelie Delaunay <[e-mail]> wrote:

> > -       p->activate_stm_id_vb_detection = true;
> > +       p->activate_stm_id_vb_detection =
> > +               !of_property_read_bool(np, "usb-role-switch");
> the rest of params.c uses device_property_read_* instead of of_read_property_*
> I thought I'd mention it so you can decide yourself whether this is
> fine or needs to be changed

Better to change and leave all on one line.

Amelie Delaunay July 28, 2020, 7:27 a.m. UTC | #3

Hi,

On 7/27/20 9:44 PM, Andy Shevchenko wrote:
> On Mon, Jul 27, 2020 at 10:04 PM Martin Blumenstingl
> <[e-mail]> wrote:
>> On Mon, Jul 27, 2020 at 11:23 AM Amelie Delaunay <[e-mail]> wrote:
> 
>> > -       p->activate_stm_id_vb_detection = true;
>> > +       p->activate_stm_id_vb_detection =
>> > +               !of_property_read_bool(np, "usb-role-switch");
>> the rest of params.c uses device_property_read_* instead of of_read_property_*
>> I thought I'd mention it so you can decide yourself whether this is
>> fine or needs to be changed
> 
> Better to change and leave all on one line.
> 

Thank you both for your review. I'll change it in upcoming v5. I'm still 
not used to exceed the 80 columns :)

Regards,
Amelie

> -- 
> With Best Regards,
> Andy Shevchenko

Patch

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index a3611cdd1dea..65f8596f6795 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -183,9 +183,11 @@  static void dwc2_set_stm32mp15_fsotg_params(struct dwc2_hsotg *hsotg)
 static void dwc2_set_stm32mp15_hsotg_params(struct dwc2_hsotg *hsotg)
 {
 	struct dwc2_core_params *p = &hsotg->params;
+	struct device_node *np = hsotg->dev->of_node;
 
 	p->otg_cap = DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE;
-	p->activate_stm_id_vb_detection = true;
+	p->activate_stm_id_vb_detection =
+		!of_property_read_bool(np, "usb-role-switch");
 	p->host_rx_fifo_size = 440;
 	p->host_nperio_tx_fifo_size = 256;
 	p->host_perio_tx_fifo_size = 256;