[U-Boot] [PATCH 1/4] sunxi: musb: Return early on VBUS GPIO error instead of on a positive value

[U-Boot] [PATCH 1/4] sunxi: musb: Return early on VBUS GPIO error instead of on a positive value

[Paul Kocialkowski]

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

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/usb/musb-new/sunxi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index 4d8c15a..90aaec6 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -261,7 +261,7 @@ static int sunxi_musb_init(struct musb *musb)
 			}
 
 			err = gpio_get_value(vbus_det);
-			if (err) {
+			if (err < 0) {
 				gpio_free(vbus_det);
 				return -EIO;
 			}
-- 
1.9.1

[U-Boot] [PATCH 2/4] sunxi: Avoid any assumption between musb gadget and host but fallback to host

[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 <contact@paulk.fr>
---
 board/sunxi/Kconfig            | 2 +-
 include/configs/sunxi-common.h | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 9d0eb91..0b6a492 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -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).
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 1f7a1cb..ffd9f5c 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -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
 
-#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
 #endif
 
 #ifdef CONFIG_USB_KEYBOARD
-- 
1.9.1

[U-Boot] [PATCH 3/4] sunxi: common VBUS detection logic in usbc

[Paul Kocialkowski]

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 <contact@paulk.fr>
---
 arch/arm/cpu/armv7/sunxi/usbc.c        | 66 +++++++++++++++++++++++++++++++---
 arch/arm/include/asm/arch-sunxi/usbc.h |  1 +
 board/sunxi/Kconfig                    |  1 -
 drivers/usb/musb-new/sunxi.c           | 45 ++++-------------------
 4 files changed, 69 insertions(+), 44 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/usbc.c b/arch/arm/cpu/armv7/sunxi/usbc.c
index 14de9f9..5c85e4a 100644
--- a/arch/arm/cpu/armv7/sunxi/usbc.c
+++ b/arch/arm/cpu/armv7/sunxi/usbc.c
@@ -41,6 +41,7 @@ static struct sunxi_usbc_hcd {
 	int usb_rst_mask;
 	int ahb_clk_mask;
 	int gpio_vbus;
+	int gpio_vbus_det;
 	int irq;
 	int id;
 } sunxi_usbc_hcd[] = {
@@ -86,6 +87,12 @@ static bool use_axp_drivebus(int index)
 	       strcmp(CONFIG_USB0_VBUS_PIN, "axp_drivebus") == 0;
 }
 
+static bool use_axp_vbus_detect(int index)
+{
+	return index == 0 &&
+	       strcmp(CONFIG_USB0_VBUS_DET, "axp_vbus_detect") == 0;
+}
+
 void *sunxi_usbc_get_io_base(int index)
 {
 	switch (index) {
@@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
 	return -1;
 }
 
+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);
+	}
+	return -1;
+}
+
 static void usb_phy_write(struct sunxi_usbc_hcd *sunxi_usbc, int addr,
 			  int data, int len)
 {
@@ -185,22 +203,31 @@ static void sunxi_usb_passby(struct sunxi_usbc_hcd *sunxi_usbc, int enable)
 int sunxi_usbc_request_resources(int index)
 {
 	struct sunxi_usbc_hcd *sunxi_usbc = &sunxi_usbc_hcd[index];
+	int ret = 0;
 
 	sunxi_usbc->gpio_vbus = get_vbus_gpio(index);
 	if (sunxi_usbc->gpio_vbus != -1)
-		return gpio_request(sunxi_usbc->gpio_vbus, "usbc_vbus");
+		ret |= gpio_request(sunxi_usbc->gpio_vbus, "usbc_vbus");
+
+	sunxi_usbc->gpio_vbus_det = get_vbus_detect_gpio(index);
+	if (sunxi_usbc->gpio_vbus_det != -1)
+		ret |= gpio_request(sunxi_usbc->gpio_vbus_det, "usbc_vbus_det");
 
-	return 0;
+	return ret;
 }
 
 int sunxi_usbc_free_resources(int index)
 {
 	struct sunxi_usbc_hcd *sunxi_usbc = &sunxi_usbc_hcd[index];
+	int ret = 0;
 
 	if (sunxi_usbc->gpio_vbus != -1)
-		return gpio_free(sunxi_usbc->gpio_vbus);
+		ret |= gpio_free(sunxi_usbc->gpio_vbus);
+
+	if (sunxi_usbc->gpio_vbus_det != -1)
+		ret |= gpio_free(sunxi_usbc->gpio_vbus_det);
 
-	return 0;
+	return ret;
 }
 
 void sunxi_usbc_enable(int index)
@@ -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
+	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;
+}
diff --git a/arch/arm/include/asm/arch-sunxi/usbc.h b/arch/arm/include/asm/arch-sunxi/usbc.h
index cb538cd..67281ec 100644
--- a/arch/arm/include/asm/arch-sunxi/usbc.h
+++ b/arch/arm/include/asm/arch-sunxi/usbc.h
@@ -20,3 +20,4 @@ void sunxi_usbc_enable(int index);
 void sunxi_usbc_disable(int index);
 void sunxi_usbc_vbus_enable(int index);
 void sunxi_usbc_vbus_disable(int index);
+int sunxi_usbc_vbus_detect(int index);
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 0b6a492..c8dbfbe 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -225,7 +225,6 @@ config USB0_VBUS_PIN
 
 config USB0_VBUS_DET
 	string "Vbus detect pin for usb0 (otg)"
-	depends on USB_MUSB_SUNXI
 	default ""
 	---help---
 	Set the Vbus detect pin for usb0 (otg). This takes a string in the
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index 90aaec6..c9a6a16 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -235,52 +235,19 @@ static int sunxi_musb_init(struct musb *musb)
 
 	pr_debug("%s():\n", __func__);
 
-	if (is_host_enabled(musb)) {
-		int vbus_det = sunxi_name_to_gpio(CONFIG_USB0_VBUS_DET);
-
-#ifdef AXP_VBUS_DETECT
-		if (!strcmp(CONFIG_USB0_VBUS_DET, "axp_vbus_detect")) {
-			err = axp_get_vbus();
-			if (err < 0)
-				return err;
-		} else {
-#endif
-			if (vbus_det == -1) {
-				eprintf("Error invalid Vusb-det pin\n");
-				return -EINVAL;
-			}
-
-			err = gpio_request(vbus_det, "vbus0_det");
-			if (err)
-				return err;
-
-			err = gpio_direction_input(vbus_det);
-			if (err) {
-				gpio_free(vbus_det);
-				return err;
-			}
-
-			err = gpio_get_value(vbus_det);
-			if (err < 0) {
-				gpio_free(vbus_det);
-				return -EIO;
-			}
-
-			gpio_free(vbus_det);
-#ifdef AXP_VBUS_DETECT
-		}
-#endif
+	err = sunxi_usbc_request_resources(0);
+	if (err)
+		return err;
 
+	if (is_host_enabled(musb)) {
+		err = sunxi_usbc_vbus_detect(0);
 		if (err) {
 			eprintf("Error: A charger is plugged into the OTG\n");
+			sunxi_usbc_free_resources(0);
 			return -EIO;
 		}
 	}
 
-	err = sunxi_usbc_request_resources(0);
-	if (err)
-		return err;
-
 	musb->isr = sunxi_musb_interrupt;
 	sunxi_usbc_enable(0);
 
-- 
1.9.1

[U-Boot] [PATCH 4/4] board: sunxi: USB download gadget cable detection

[Paul Kocialkowski]

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 board/sunxi/board.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index e1891d1..becdc8b 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -241,6 +241,13 @@ static struct musb_hdrc_platform_data musb_plat = {
 };
 #endif
 
+#ifdef CONFIG_USB_GADGET
+int g_dnl_board_usb_cable_connected(void)
+{
+	return sunxi_usbc_vbus_detect(0);
+}
+#endif
+
 #ifdef CONFIG_MISC_INIT_R
 int misc_init_r(void)
 {
-- 
1.9.1

[U-Boot] [PATCH 3/4] sunxi: common VBUS detection logic in usbc

[Marek Vasut]

On Sunday, March 15, 2015 at 06:27:46 PM, Paul Kocialkowski wrote:
> 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 <contact@paulk.fr>

Hi!

[...]

> @@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
>  	return -1;
>  }
> 
> +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.

> +	}
> +	return -1;
> +}
> +

[...]

> @@ -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 ?

> +	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;

[...]

[U-Boot] [PATCH 3/4] sunxi: common VBUS detection logic in usbc

[Paul Kocialkowski]

Le dimanche 15 mars 2015 ? 19:19 +0100, Marek Vasut a ?crit :
> On Sunday, March 15, 2015 at 06:27:46 PM, Paul Kocialkowski wrote:
> > 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 <contact@paulk.fr>
> 
> Hi!
> 
> [...]
> 
> > @@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
> >  	return -1;
> >  }
> > 
> > +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.

> > +	}
> > +	return -1;
> > +}
> > +
> 
> [...]
> 
> > @@ -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 ?

I agree those ifdefs are not used consistently and should be moved to
one place. Your suggestion looks fine to me.

> > +	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;
> 
> [...]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150315/1d7fa5a1/attachment.sig>

[U-Boot] [PATCH 3/4] sunxi: common VBUS detection logic in usbc

[Marek Vasut]

On Sunday, March 15, 2015 at 10:54:05 PM, Paul Kocialkowski wrote:
> Le dimanche 15 mars 2015 ? 19:19 +0100, Marek Vasut a ?crit :
> > On Sunday, March 15, 2015 at 06:27:46 PM, Paul Kocialkowski wrote:
> > > 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 <contact@paulk.fr>
> > 
> > Hi!
> > 
> > [...]
> > 
> > > @@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
> > > 
> > >  	return -1;
> > >  
> > >  }
> > > 
> > > +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.

Hi

this makes sense, but if you're planning to clean this up, please also
wait for some feedback from Hans. Also, it'd be nice if the cleanup patch
could be applied before a feature-addition patch like this one.

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

OK

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

Well if it spits in some sensible way somewhere, that'd be OK. If it doesn't,
subsequent patch which fixes this would make sense.

btw. you might want to return proper values from errno.h here instead of -1.

[...]

Thanks!

[U-Boot] [PATCH 1/4] sunxi: musb: Return early on VBUS GPIO error instead of on a positive value

[Hans de Goede]

Hi,

On 15-03-15 18:27, Paul Kocialkowski wrote:
> This allows printing the error message when VBUS is detected, as it would with
> AXP VBUS detect.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>   drivers/usb/musb-new/sunxi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
> index 4d8c15a..90aaec6 100644
> --- a/drivers/usb/musb-new/sunxi.c
> +++ b/drivers/usb/musb-new/sunxi.c
> @@ -261,7 +261,7 @@ static int sunxi_musb_init(struct musb *musb)
>   			}
>
>   			err = gpio_get_value(vbus_det);
> -			if (err) {
> +			if (err < 0) {
>   				gpio_free(vbus_det);
>   				return -EIO;
>   			}
>

Good catch, thanks I've send a pull-req with this fix upstream,
please base your next patch-set on u-boot-sunxi/master, or
at least keep this patch as a base before doing the axp gpio
cleanups I asked for in my previous mail.

Regards,

Hans

[U-Boot] [PATCH 2/4] sunxi: Avoid any assumption between musb gadget and host but fallback to host

[Hans de Goede]

Hi,

On 15-03-15 18:27, Paul Kocialkowski wrote:
> 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 <contact@paulk.fr>
> ---
>   board/sunxi/Kconfig            | 2 +-
>   include/configs/sunxi-common.h | 5 ++++-
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index 9d0eb91..0b6a492 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -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.

> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 1f7a1cb..ffd9f5c 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -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
>
> -#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.

Regards,

Hans

[U-Boot] [PATCH 3/4] sunxi: common VBUS detection logic in usbc

[Hans de Goede]

Hi,

On 15-03-15 18:27, Paul Kocialkowski wrote:
> 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 <contact@paulk.fr>

Thanks for working on this, please respin this and the next patch once the axp vbus
detect / enable cleanups have been done.

Regards,

Hans

[U-Boot] [PATCH 2/4] sunxi: Avoid any assumption between musb gadget and host but fallback to host

[Paul Kocialkowski]

Le samedi 21 mars 2015 ? 14:14 +0100, Hans de Goede a ?crit :
> Hi,
> 
> On 15-03-15 18:27, Paul Kocialkowski wrote:
> > 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 <contact@paulk.fr>
> > ---
> >   board/sunxi/Kconfig            | 2 +-
> >   include/configs/sunxi-common.h | 5 ++++-
> >   2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> > index 9d0eb91..0b6a492 100644
> > --- a/board/sunxi/Kconfig
> > +++ b/board/sunxi/Kconfig
> > @@ -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.
> 
> > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> > index 1f7a1cb..ffd9f5c 100644
> > --- a/include/configs/sunxi-common.h
> > +++ b/include/configs/sunxi-common.h
> > @@ -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
> >
> > -#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.

I understand that the logic is flawed here (disable a Kconfig option in
Kconfig and enable it back in the config header, which is weaker), but I
really don't think it should always be enabled.

IMO the best solution would be to remove USB_KEYBOARD from Kconfig
entirely and enable it in sunxi-common.h as I suggested. This way, we
don't even have to explicitely disable it for devices that don't have
USB support (MSI_Primo81/MSI_Primo73): it will only get enabled when
EHCI or host MUSB support is enabled.

I fail to see the point of using Kconfig here anyway, since nothing
depends on it (in the Kconfig sense) and other platforms have been using
CONFIG_USB_KEYBOARD in the config files without ever needing to have it
in Kconfig. In addition, it doesn't seem very relevant to me to have
that Kconfig option defined in the sunxi part of the code, while
USB_KEYBOARD is also used on other SoCs.

Please let me know what you think here, I'd be glad to make a patch
fixing this behaviour.

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.

Website: http://www.replicant.us/
Blog: http://blog.replicant.us/
Wiki/tracker/forums: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150322/1f4047cd/attachment.sig>