[PATCH 0/3] usb: dwc2: fill in gadget caps, configure it for stm32mp15

[PATCH 0/3] usb: dwc2: fill in gadget caps, configure it for stm32mp15

[Fabrice Gasnier]

This patchset fills in 'otg_caps' of the usb_gadget structure, and configures it
on stm32mp15.

When dwc2 is configured as dual role (OTG), the USB gadget descriptors (device mode)
are configured via configfs. This lead in calling usb_otg_descriptor_init().
In usb_otg_descriptor_init() (drivers/usb/gadget/config.c):
- If otg caps structure is provided -> use it
- If otg caps structure isn't provided -> HNP and SRP are enabled by default

This could lead to a configuration mismatch beetween:
- OTG controller: HNP and SRP aren't enabled
- gadget descriptors: HNP and SRP are advertised

Fabrice Gasnier (3):
  usb: dwc2: add otg_rev and otg_caps information for gadget driver
  usb: dwc2: stm32mp15: set otg_rev
  ARM: dts: stm32: set otg-rev on stm32mp151

 arch/arm/boot/dts/stm32mp151.dtsi |  1 +
 drivers/usb/dwc2/core.h           |  7 +++++++
 drivers/usb/dwc2/gadget.c         |  1 +
 drivers/usb/dwc2/params.c         | 16 ++++++++++++++++
 4 files changed, 25 insertions(+)

-- 
2.7.4

[PATCH 1/3] usb: dwc2: add otg_rev and otg_caps information for gadget driver

[Fabrice Gasnier]

Currently the dwc2 doesn't fill in the 'otg_caps' of usb_gadget structure.
When registering a gadget device (e.g. via configfs), the
usb_otg_descriptor_init() checks the 'otg_caps' and 'otg_rev'. It defaults
to HNP and SRP bmAttributes if unspecified. There may be a mismatch with
what's being set in dwc2 params structure. This result in the descriptors
to be miss-configured in this case.

So add an option to setup 'otg_caps' and 'otg_rev' in the params. It's
then provided to the gadget struct. These parameters can then be tuned
for each platform. In case it's not set, it will default to current
behavior.
Also add option to setup these from the device tree by calling
of_usb_update_otg_caps(). This provides support for standard properties
such as "otg-rev", "hnp-disable" and "srp-disable" (see usb-drd.yaml).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/usb/dwc2/core.h   |  7 +++++++
 drivers/usb/dwc2/gadget.c |  1 +
 drivers/usb/dwc2/params.c | 14 ++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index cb9059a..68d0967 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -243,6 +243,9 @@ enum dwc2_ep0_state {
  *                       1 - SRP Only capable
  *                       2 - No HNP/SRP capable (always available)
  *                      Defaults to best available option (0, 1, then 2)
+ * @otg_rev:		The OTG revision number the device is compliant with,
+ *			in binary-coded decimal (i.e. 2.0 is 0200H).
+ *			(see struct usb_otg_caps)
  * @host_dma:           Specifies whether to use slave or DMA mode for accessing
  *                      the data FIFOs. The driver will automatically detect the
  *                      value for this parameter if none is specified.
@@ -458,6 +461,7 @@ struct dwc2_core_params {
 #define DWC2_CAP_PARAM_SRP_ONLY_CAPABLE		1
 #define DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE	2
 
+	u16 otg_rev;
 	u8 phy_type;
 #define DWC2_PHY_TYPE_PARAM_FS		0
 #define DWC2_PHY_TYPE_PARAM_UTMI	1
@@ -1054,6 +1058,8 @@ struct dwc2_hregs_backup {
  * @new_connection:	Used in host mode. True if there are new connected
  *			device
  * @enabled:		Indicates the enabling state of controller
+ * @dw_otg_caps:	OTG caps from the platform parameters, used to setup the
+ *			gadget structure.
  *
  */
 struct dwc2_hsotg {
@@ -1220,6 +1226,7 @@ struct dwc2_hsotg {
 	unsigned int remote_wakeup_allowed:1;
 	struct dwc2_hsotg_ep *eps_in[MAX_EPS_CHANNELS];
 	struct dwc2_hsotg_ep *eps_out[MAX_EPS_CHANNELS];
+	struct usb_otg_caps dw_otg_caps;
 #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
 };
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 837237e..4dc2e9e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4945,6 +4945,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
 	hsotg->gadget.max_speed = USB_SPEED_HIGH;
 	hsotg->gadget.ops = &dwc2_hsotg_gadget_ops;
 	hsotg->gadget.name = dev_name(dev);
+	hsotg->gadget.otg_caps = &hsotg->dw_otg_caps;
 	hsotg->remote_wakeup_allowed = 0;
 
 	if (hsotg->params.lpm)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 59e1193..f8ab211 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -36,6 +36,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/usb/of.h>
 
 #include "core.h"
 
@@ -246,18 +247,25 @@ static void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg)
 	switch (hsotg->hw_params.op_mode) {
 	case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE:
 		val = DWC2_CAP_PARAM_HNP_SRP_CAPABLE;
+		hsotg->dw_otg_caps.hnp_support = true;
+		hsotg->dw_otg_caps.srp_support = true;
 		break;
 	case GHWCFG2_OP_MODE_SRP_ONLY_CAPABLE:
 	case GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE:
 	case GHWCFG2_OP_MODE_SRP_CAPABLE_HOST:
 		val = DWC2_CAP_PARAM_SRP_ONLY_CAPABLE;
+		hsotg->dw_otg_caps.hnp_support = false;
+		hsotg->dw_otg_caps.srp_support = true;
 		break;
 	default:
 		val = DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE;
+		hsotg->dw_otg_caps.hnp_support = false;
+		hsotg->dw_otg_caps.srp_support = false;
 		break;
 	}
 
 	hsotg->params.otg_cap = val;
+	hsotg->dw_otg_caps.otg_rev = hsotg->params.otg_rev;
 }
 
 static void dwc2_set_param_phy_type(struct dwc2_hsotg *hsotg)
@@ -465,6 +473,9 @@ static void dwc2_get_device_properties(struct dwc2_hsotg *hsotg)
 		}
 	}
 
+	if (hsotg->dr_mode == USB_DR_MODE_OTG)
+		of_usb_update_otg_caps(hsotg->dev->of_node, &hsotg->dw_otg_caps);
+
 	if (of_find_property(hsotg->dev->of_node, "disable-over-current", NULL))
 		p->oc_disable = true;
 }
@@ -484,6 +495,7 @@ static void dwc2_check_param_otg_cap(struct dwc2_hsotg *hsotg)
 		case GHWCFG2_OP_MODE_SRP_ONLY_CAPABLE:
 		case GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE:
 		case GHWCFG2_OP_MODE_SRP_CAPABLE_HOST:
+			hsotg->dw_otg_caps.hnp_support = false;
 			break;
 		default:
 			valid = 0;
@@ -492,6 +504,8 @@ static void dwc2_check_param_otg_cap(struct dwc2_hsotg *hsotg)
 		break;
 	case DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE:
 		/* always valid */
+		hsotg->dw_otg_caps.hnp_support = false;
+		hsotg->dw_otg_caps.srp_support = false;
 		break;
 	default:
 		valid = 0;
-- 
2.7.4

Re: [PATCH 1/3] usb: dwc2: add otg_rev and otg_caps information for gadget driver

[Minas Harutyunyan]

On 9/15/2021 11:54 AM, Fabrice Gasnier wrote:
> Currently the dwc2 doesn't fill in the 'otg_caps' of usb_gadget structure.
> When registering a gadget device (e.g. via configfs), the
> usb_otg_descriptor_init() checks the 'otg_caps' and 'otg_rev'. It defaults
> to HNP and SRP bmAttributes if unspecified. There may be a mismatch with
> what's being set in dwc2 params structure. This result in the descriptors
> to be miss-configured in this case.
> 
> So add an option to setup 'otg_caps' and 'otg_rev' in the params. It's
> then provided to the gadget struct. These parameters can then be tuned
> for each platform. In case it's not set, it will default to current
> behavior.
> Also add option to setup these from the device tree by calling
> of_usb_update_otg_caps(). This provides support for standard properties
> such as "otg-rev", "hnp-disable" and "srp-disable" (see usb-drd.yaml).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>

Re: [PATCH 1/3] usb: dwc2: add otg_rev and otg_caps information for gadget driver

[gregkh]

On Fri, Oct 01, 2021 at 11:10:19AM +0000, Minas Harutyunyan wrote:
> On 9/15/2021 11:54 AM, Fabrice Gasnier wrote:
> > Currently the dwc2 doesn't fill in the 'otg_caps' of usb_gadget structure.
> > When registering a gadget device (e.g. via configfs), the
> > usb_otg_descriptor_init() checks the 'otg_caps' and 'otg_rev'. It defaults
> > to HNP and SRP bmAttributes if unspecified. There may be a mismatch with
> > what's being set in dwc2 params structure. This result in the descriptors
> > to be miss-configured in this case.
> > 
> > So add an option to setup 'otg_caps' and 'otg_rev' in the params. It's
> > then provided to the gadget struct. These parameters can then be tuned
> > for each platform. In case it's not set, it will default to current
> > behavior.
> > Also add option to setup these from the device tree by calling
> > of_usb_update_otg_caps(). This provides support for standard properties
> > such as "otg-rev", "hnp-disable" and "srp-disable" (see usb-drd.yaml).
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
> 

Looks like this commit breaks the build on some configurations:
	https://lore.kernel.org/r/000000000000b01f1505cda8e03c@google.com

So I'll go drop this and the 2/3 patch from my tree.  Please fix up and
resend.

thanks,

greg k-h

[PATCH 2/3] usb: dwc2: stm32mp15: set otg_rev

[Fabrice Gasnier]

STM32MP15 complies with the OTG 2.0. Set OTG revision accordingly. It is
useful for the of_usb_update_otg_caps() routine to check and update
otg_rev to the lower value between DT and provided params.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/usb/dwc2/params.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index f8ab211..fe2e88f 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -170,6 +170,7 @@ static void dwc2_set_stm32mp15_fsotg_params(struct dwc2_hsotg *hsotg)
 	struct dwc2_core_params *p = &hsotg->params;
 
 	p->otg_cap = DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE;
+	p->otg_rev = 0x200;
 	p->speed = DWC2_SPEED_PARAM_FULL;
 	p->host_rx_fifo_size = 128;
 	p->host_nperio_tx_fifo_size = 96;
@@ -190,6 +191,7 @@ static void dwc2_set_stm32mp15_hsotg_params(struct dwc2_hsotg *hsotg)
 	struct dwc2_core_params *p = &hsotg->params;
 
 	p->otg_cap = DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE;
+	p->otg_rev = 0x200;
 	p->activate_stm_id_vb_detection = !device_property_read_bool(hsotg->dev, "usb-role-switch");
 	p->host_rx_fifo_size = 440;
 	p->host_nperio_tx_fifo_size = 256;
-- 
2.7.4

Re: [PATCH 2/3] usb: dwc2: stm32mp15: set otg_rev

[Minas Harutyunyan]

On 9/15/2021 11:54 AM, Fabrice Gasnier wrote:
> STM32MP15 complies with the OTG 2.0. Set OTG revision accordingly. It is
> useful for the of_usb_update_otg_caps() routine to check and update
> otg_rev to the lower value between DT and provided params.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>

[PATCH 3/3] ARM: dts: stm32: set otg-rev on stm32mp151

[Fabrice Gasnier]

STM32MP151 complies with the OTG 2.0. Set it with otg-rev dt property.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 arch/arm/boot/dts/stm32mp151.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index bd289bf..14e3cdf 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1085,6 +1085,7 @@
 			g-np-tx-fifo-size = <32>;
 			g-tx-fifo-size = <256 16 16 16 16 16 16 16>;
 			dr_mode = "otg";
+			otg-rev = <0x200>;
 			usb33d-supply = <&usb33>;
 			status = "disabled";
 		};
-- 
2.7.4