sunxi: musb: Return early on VBUS GPIO error instead of on a positive value

musb might be used in gadget mode on sunxi, so we don't want to assume anything related to host mode, especially USB keyboard support.
However, in case gadget mode is not explicitly used, fallback to host mode.

Signed-off-by Paul Kocialkowski <[email protected]>

---
  board/sunxi/Kconfig            | 2 +-
  include/configs/sunxi-common.h | 5 ++++-
  2 files changed, 5 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +0 -0

index 9d0eb91..0b6a492 100644

include/configs/sunxi-common.h +1 -0

index 1f7a1cb..ffd9f5c 100644
@@ -297,13 +297,16 @@ extern int soft_i2c_gpio_scl;
  #endif
 
  #ifdef CONFIG_USB_MUSB_SUNXI
+#ifndef CONFIG_MUSB_GADGET

Hi,

Quoted 15 lines · Paul Kocialkowski

musb might be used in gadget mode on sunxi, so we don't want to assume anything related to host mode, especially USB keyboard support.
However, in case gadget mode is not explicitly used, fallback to host mode.

Signed-off-by Paul Kocialkowski <[email protected]>

---
  board/sunxi/Kconfig            | 2 +-
  include/configs/sunxi-common.h | 5 ++++-
  2 files changed, 5 insertions(+), 2 deletions(-)

board/sunxi/Kconfig +0 -0

index 9d0eb91..0b6a492 100644

board/sunxi/Kconfig config USB_MUSB_SUNXI · line 442

Quoted 8 lines

board/sunxi/Kconfig +1 -1

@@ -436,7 +436,7 @@ config USB_MUSB_SUNXI
 
  config USB_KEYBOARD
  	boolean "Enable USB keyboard support"
-	default y
+	default n
  	---help---
  	Say Y here to add support for using a USB keyboard (typically used
  	in combination with a graphical console).

You've just disabled the usb keyboard support pretty much everywhere, please do not do that.

Quoted 14 lines

include/configs/sunxi-common.h +2 -0

index 1f7a1cb..ffd9f5c 100644
@@ -297,13 +297,16 @@ extern int soft_i2c_gpio_scl;
  #endif
 
  #ifdef CONFIG_USB_MUSB_SUNXI
+#ifndef CONFIG_MUSB_GADGET
  #define CONFIG_MUSB_HOST
+#endif
  #define CONFIG_MUSB_PIO_ONLY
  #endif
 

include/configs/sunxi-common.h extern int soft_i2c_gpio_scl; · line 309

Quoted 5 lines

include/configs/sunxi-common.h +2 -1

-#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI
+#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST
  #define CONFIG_CMD_USB
  #define CONFIG_USB_STORAGE
+#define CONFIG_USB_KEYBOARD

And here you are overriding something set by Kconfig which is a big no no, if you want to disable the keyboard support on boards where you use the oth port in gadget mode jsut add:
CONFIG_USB_KEYBOARD=n to the (def)config.

VBUS detection could be needed not only by the musb code (to prevent host mode), but also by e.g. gadget drivers to start only when a cable is connected.

In addition, this allows more flexibility in vbus detection, as it could easily be extended to other USBC indexes. Eventually, this would help making musb support independent from a hardcoded USB controller index (0).

Signed-off-by Paul Kocialkowski <[email protected]>

arch/arm/cpu/armv7/sunxi/usbc.c +4 -0

+	}
+	return -1;
+}
+

arch/arm/cpu/armv7/sunxi/usbc.c +23 -0

+	if (use_axp_vbus_detect(index)) {
+		err = axp_get_vbus();
+		if (err < 0)
+			return err;
+	} else {
+#endif
+		if (sunxi_usbc->gpio_vbus_det == -1) {
+			eprintf("Error: invalid vbus detection pin\n");
+			return -1;
+		}
+
+		err = gpio_direction_input(sunxi_usbc->gpio_vbus_det);
+		if (err)
+			return err;
+
+		err = gpio_get_value(sunxi_usbc->gpio_vbus_det);
+		if (err < 0)
+			return err;
+#ifdef AXP_VBUS_DETECT
+	}
+#endif
+
+	return err;

Quoted 9 lines

VBUS detection could be needed not only by the musb code (to prevent host mode), but also by e.g. gadget drivers to start only when a cable is connected.

In addition, this allows more flexibility in vbus detection, as it could easily be extended to other USBC indexes. Eventually, this would help making musb support independent from a hardcoded USB controller index (0).

Signed-off-by Paul Kocialkowski <[email protected]>

Hi!

[...]

Quoted 4 lines

arch/arm/cpu/armv7/sunxi/usbc.c +0 -0

@@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
 	return -1;
 }
 

arch/arm/cpu/armv7/sunxi/usbc.c static int get_vbus_gpio(int index) · line 129

Quoted 7 lines

arch/arm/cpu/armv7/sunxi/usbc.c +7 -0

+static int get_vbus_detect_gpio(int index)
+{
+	if (use_axp_vbus_detect(index))
+		return -1;
+
+	switch (index) {
+	case 0: return sunxi_name_to_gpio(CONFIG_USB0_VBUS_DET);

What happens if the index != 0 here ? Default branch with some error report might be a good idea here.

Quoted 4 lines

arch/arm/cpu/armv7/sunxi/usbc.c +4 -0

+	}
+	return -1;
+}
+

[...]

arch/arm/cpu/armv7/sunxi/usbc.c void sunxi_usbc_vbus_disable(int index) · line 306

Quoted 11 lines

arch/arm/cpu/armv7/sunxi/usbc.c +7 -0

@@ -270,3 +297,34 @@ void sunxi_usbc_vbus_disable(int index)
 	if (sunxi_usbc->gpio_vbus != -1)
 		gpio_direction_output(sunxi_usbc->gpio_vbus, 0);
 }
+
+int sunxi_usbc_vbus_detect(int index)
+{
+	struct sunxi_usbc_hcd *sunxi_usbc = &sunxi_usbc_hcd[index];
+	int err;
+
+#ifdef AXP_VBUS_DETECT

Can't you just wrap this ifdef into use_axp_vbus_detect(), so the code isn't so riddled with the ifdeffery ?

Quoted 17 lines

arch/arm/cpu/armv7/sunxi/usbc.c +17 -0

+	if (use_axp_vbus_detect(index)) {
+		err = axp_get_vbus();
+		if (err < 0)
+			return err;
+	} else {
+#endif
+		if (sunxi_usbc->gpio_vbus_det == -1) {
+			eprintf("Error: invalid vbus detection pin\n");
+			return -1;
+		}
+
+		err = gpio_direction_input(sunxi_usbc->gpio_vbus_det);
+		if (err)
+			return err;
+
+		err = gpio_get_value(sunxi_usbc->gpio_vbus_det);
+		if (err < 0)

Quoted 6 lines

arch/arm/cpu/armv7/sunxi/usbc.c +6 -0

+			return err;
+#ifdef AXP_VBUS_DETECT
+	}
+#endif
+
+	return err;

[...]

Le dimanche 15 mars 2015 ? 19:19 +0100, Marek Vasut a ?crit :

Quoted 22 lines · Paul Kocialkowski

Quoted 10 lines

VBUS detection could be needed not only by the musb code (to prevent host mode), but also by e.g. gadget drivers to start only when a cable is connected.

In addition, this allows more flexibility in vbus detection, as it could easily be extended to other USBC indexes. Eventually, this would help making musb support independent from a hardcoded USB controller index (0).

Signed-off-by Paul Kocialkowski <[email protected]>

Hi!

[...]

Quoted 6 lines

(quoted diff) +0 -0

@@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
 
 	return -1;
 
 }
 

Quoted 10 lines

arch/arm/cpu/armv7/sunxi/usbc.c static int get_vbus_gpio(int index) · line 129

Quoted 7 lines

arch/arm/cpu/armv7/sunxi/usbc.c +7 -0

+static int get_vbus_detect_gpio(int index)
+{
+	if (use_axp_vbus_detect(index))
+		return -1;
+
+	switch (index) {
+	case 0: return sunxi_name_to_gpio(CONFIG_USB0_VBUS_DET);

What happens if the index != 0 here ? Default branch with some error report might be a good idea here.

Well, I'm going to have a very stupid answer here, that is: I copied what was already being done nearby, namely in get_vbus_gpio for this one. So I think that we should modify both functions accordingly.

I don't think it makes so much sense to print an error here because this will never be called directly, but instead through sunxi_usbc_request_resources which will make noise when it fails.

We could perhaps add some error printing in sunxi_usbc_request_resources to make it clear what failed, but I'm not really for it.

VBUS detection could be needed not only by the musb code (to prevent host mode), but also by e.g. gadget drivers to start only when a cable is connected.

In addition, this allows more flexibility in vbus detection, as it could easily be extended to other USBC indexes. Eventually, this would help making musb support independent from a hardcoded USB controller index (0).

Signed-off-by Paul Kocialkowski <[email protected]>

This allows printing the error message when VBUS is detected, as it would with AXP VBUS detect.

Signed-off-by Paul Kocialkowski <[email protected]>

---
  drivers/usb/musb-new/sunxi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/musb-new/sunxi.c +0 -0

index 4d8c15a..90aaec6 100644