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

2015-03-21 Thread 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 cont...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-03-15 Thread 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 cont...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-03-15 Thread 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 cont...@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

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

2015-03-15 Thread 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 cont...@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;
 
 [...]



signature.asc
Description: This is a digitally signed message part
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot