ARM: shmobile: Lager USBHS update

PATCH New Magnus Damm 4 participants 19 messages Jan 29, 2014 – Feb 10, 2014

Subsystems (5)

Arm (32-bit) Arm sub-architectures SuperH Arm/RISC-V/Renesas architecture +1 more subsystems

lore

Share & tools

lore

Magnus Damm

author maintainer

2014-01-29 23:09 UTC #0 lore raw

cover ARM: shmobile: Lager USBHS update

ARM: shmobile: Lager USBHS update

[PATCH 01/03] ARM: shmobile: Remove Lager USBHS UDC ifdefs [PATCH 02/03] pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS [PATCH 03/03] ARM: shmobile: Lager USB0 cable detection workaround

Update the Lager USB0 code to let the USBHS device always be present and check for DIP switch configuration. To be able to check the DIP switch configuration the r8a7790 PINCTRL bits need to be updated too.

With this series USB0 on Lager will be fixed to USB Function via the USBHS driver. In theory the SoC can also use USB Host on this port either via USBHS or PCI USB, but since cable detection is missing it is possible for the user to misconfigure USB Host and drive VBUS with the wrong cable. Because of that USB0 will be forced to stick to Function-only in the Lager board code.

Signed-off-by Magnus Damm <[email protected]>

---

Written against renesas.git tag renesas-devel-v3.13-20140127

 arch/arm/mach-shmobile/board-lager.c |   29 ++++++++++++++++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 2 files changed, 31 insertions(+), 7 deletions(-)

Magnus Damm

author maintainer

2014-01-29 23:10 UTC #1 lore raw

PATCH 1/3 ARM: shmobile: Remove Lager USBHS UDC ifdefs

From: Magnus Damm <[email protected]>

Remove ifdefs to make the Lager USBHS device always present. This makes it more like other devices, no need to be special.

Also, these ifdefs by themselves do not hurt much, but combined with USB Host device ifdefs that were proposed earlier we could basically end up with a kernel that drives VBUS incorrectly depending on the kernel configuration - lets not do that.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/arm/mach-shmobile/board-lager.c |    4 ----
 1 file changed, 4 deletions(-)

arch/arm/mach-shmobile/board-lager.c +0 -4

@@ -368,7 +368,6 @@ static const struct platform_device_info
 };
 
 /* USBHS */
-#if IS_ENABLED(CONFIG_USB_RENESAS_USBHS_UDC)
 static const struct resource usbhs_resources[] __initconst = {
 	DEFINE_RES_MEM(0xe6590000, 0x100),
 	DEFINE_RES_IRQ(gic_spi(107)),

@@ -478,9 +477,6 @@ static void __init lager_register_usbhs(
 					  &usbhs_priv.info,
 					  sizeof(usbhs_priv.info));
 }
-#else	/* CONFIG_USB_RENESAS_USBHS_UDC */
-static inline void lager_register_usbhs(void) { }
-#endif	/* CONFIG_USB_RENESAS_USBHS_UDC */
 
 /* USBHS PHY */
 static const struct rcar_gen2_phy_platform_data usbhs_phy_pdata __initconst = {

Magnus Damm

author maintainer

2014-01-29 23:10 UTC #2 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)

drivers/pinctrl/sh-pfc/pfc-r8a7790.c +9 -0

@@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
 static const unsigned int usb0_mux[] = {
 	USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
 };
+static const unsigned int usb0_ovc_vbus_pins[] = {
+	/* OVC/VBUS */
+	RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+	USB0_OVC_VBUS_MARK,
+};
 /* - USB1 ------------------------------------------------------------------- */
 static const unsigned int usb1_pins[] = {
 	/* PWEN, OVC */

@@ -3789,6 +3796,7 @@ static const struct sh_pfc_pin_group pin
 	SH_PFC_PIN_GROUP(tpu0_to2),
 	SH_PFC_PIN_GROUP(tpu0_to3),
 	SH_PFC_PIN_GROUP(usb0),
+	SH_PFC_PIN_GROUP(usb0_ovc_vbus),
 	SH_PFC_PIN_GROUP(usb1),
 	SH_PFC_PIN_GROUP(usb2),
 	VIN_DATA_PIN_GROUP(vin0_data, 24),

@@ -4134,6 +4142,7 @@ static const char * const tpu0_groups[]
 
 static const char * const usb0_groups[] = {
 	"usb0",
+	"usb0_ovc_vbus",
 };
 
 static const char * const usb1_groups[] = {

Laurent Pinchart

2014-01-31 01:17 UTC #3 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Hi Magnus,

Thank you for the patch.

drivers/pinctrl/sh-pfc/pfc-r8a7790.c

Quoted 13 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:

From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7790.c +0 -0

10:23:32.000000000

drivers/pinctrl/sh-pfc/pfc-r8a7790.c static const unsigned int usb0_pins[] = · line 3240

Quoted 11 lines

+0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
static const unsigned int usb0_mux[] = {
drivers/pinctrl/sh-pfc/pfc-r8a7790.c +7 -0
 	USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
 };
+static const unsigned int usb0_ovc_vbus_pins[] = {
+	/* OVC/VBUS */
+	RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+	USB0_OVC_VBUS_MARK,
+};

Another option would have been to split the existing usb0 group in usb0_pwen and usb0_ovc. I'm not sure which is better though, I'd just like to know if you had given it a thought.

Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus to keep names short ?

Quoted 20 lines

/* - USB1
------------------------------------------------------------------- */ static const unsigned int usb1_pins[] = {

drivers/pinctrl/sh-pfc/pfc-r8a7790.c +2 -0

 	/* PWEN, OVC */

@@ -3789,6 +3796,7 @@ static const struct sh_pfc_pin_group pin
 	SH_PFC_PIN_GROUP(tpu0_to2),
 	SH_PFC_PIN_GROUP(tpu0_to3),
 	SH_PFC_PIN_GROUP(usb0),
+	SH_PFC_PIN_GROUP(usb0_ovc_vbus),
 	SH_PFC_PIN_GROUP(usb1),
 	SH_PFC_PIN_GROUP(usb2),
 	VIN_DATA_PIN_GROUP(vin0_data, 24),

@@ -4134,6 +4142,7 @@ static const char * const tpu0_groups[]
 
 static const char * const usb0_groups[] = {
 	"usb0",
+	"usb0_ovc_vbus",
 };
 
 static const char * const usb1_groups[] = {

Signature · 3 lines

Regards,

Laurent Pinchart

Magnus Damm

author maintainer

2014-01-31 03:10 UTC #4 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Hi Laurent,

Quoted 19 lines · Laurent Pinchart

Hi Magnus,
Thank you for the patch.
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 13 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +0 -0
10:23:32.000000000

Quoted 15 lines

drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 11 lines
+0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
 static const unsigned int usb0_mux[] = {
      USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
};
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +7 -0
+static const unsigned int usb0_ovc_vbus_pins[] = {
+     /* OVC/VBUS */
+     RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+     USB0_OVC_VBUS_MARK,
+};
Another option would have been to split the existing usb0 group in usb0_pwen and usb0_ovc. I'm not sure which is better though, I'd just like to know if you had given it a thought.

I actually did just that in my first local attempt, but I decided not to since it will only cause potential breakage.

Quoted 2 lines

Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus to keep names short ?

Is there any particular reason why you want shorter names?

Quoted 1 line

From my side, I prefer to keep the names in sync with the data sheet.

In this particular case it is a shared pin so OVC is used for Host while VBUS is used for gadget, so if you're proposing to ditch VBUS then this feels somewhat inconsistent with the current gadget use case. =)

Thanks,

/ magnus

Simon Horman

2014-02-06 06:24 UTC #5 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Hi Laurent,

Quoted 45 lines · Magnus Damm

Hi Laurent,
Quoted 19 lines · Laurent Pinchart
Hi Magnus,
Thank you for the patch.
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 13 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +0 -0
10:23:32.000000000
Quoted 15 lines
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 11 lines
+0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
 static const unsigned int usb0_mux[] = {
      USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
};
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +7 -0
+static const unsigned int usb0_ovc_vbus_pins[] = {
+     /* OVC/VBUS */
+     RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+     USB0_OVC_VBUS_MARK,
+};
Another option would have been to split the existing usb0 group in usb0_pwen and usb0_ovc. I'm not sure which is better though, I'd just like to know if you had given it a thought.
I actually did just that in my first local attempt, but I decided not to since it will only cause potential breakage.
Quoted 2 lines
Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus to keep names short ?

Quoted 7 lines

Is there any particular reason why you want shorter names?
Quoted 1 line
From my side, I prefer to keep the names in sync with the data sheet.
In this particular case it is a shared pin so OVC is used for Host while VBUS is used for gadget, so if you're proposing to ditch VBUS then this feels somewhat inconsistent with the current gadget use case. =)

Hi Laurent,

I would like to move this patch forwards somehow.
If you are happy with it as-is could you consider merging it? Otherwise, could you let me know what changes you would like made so I can see about making it so?

Thanks

Laurent Pinchart

2014-02-06 11:01 UTC #6 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

In reply to Magnus Damm

Hi Magnus,

Quoted 32 lines · On Friday 31 January 2014 12:10:05 Magnus Damm wrote:

Quoted 15 lines · Laurent Pinchart
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 13 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +0 -0
10:23:32.000000000
Quoted 15 lines
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 11 lines
+0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
 static const unsigned int usb0_mux[] = {
      USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
};
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +7 -0
+static const unsigned int usb0_ovc_vbus_pins[] = {
+     /* OVC/VBUS */
+     RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+     USB0_OVC_VBUS_MARK,
+};
Another option would have been to split the existing usb0 group in usb0_pwen and usb0_ovc. I'm not sure which is better though, I'd just like to know if you had given it a thought.

Quoted 2 lines

I actually did just that in my first local attempt, but I decided not to since it will only cause potential breakage.

OK. I assume that using PWEN without OVC/VBUS doesn't make sense, right ?

Quoted 4 lines

Quoted 2 lines
Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus to keep names short ?
Is there any particular reason why you want shorter names?

When it doesn't reduce clarity I prefer to keep names short, as that makes the code easier to read and write, and (slightly) lowers the memory footprint.

Quoted 4 lines

From my side, I prefer to keep the names in sync with the data sheet. In this particular case it is a shared pin so OVC is used for Host while VBUS is used for gadget, so if you're proposing to ditch VBUS then this feels somewhat inconsistent with the current gadget use case. =)

I thought the pin was used for over current detection only, but that doesn't make sense for function mode, you're right. Let's keep the name as-is then.

Provided PWEN without OVC/VBUS doesn't make sense and won't be needed,

Acked-by Laurent Pinchart <[email protected]>

Signature · 3 lines

Regards,

Laurent Pinchart

Magnus Damm

author maintainer

2014-02-06 13:34 UTC #7 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Hi Laurent,

Quoted 35 lines · Laurent Pinchart

Hi Magnus,
Quoted 32 lines · On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
Quoted 30 lines · Laurent Pinchart
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 13 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +0 -0
10:23:32.000000000
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 11 lines
+0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
 static const unsigned int usb0_mux[] = {
      USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
};
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +7 -0
+static const unsigned int usb0_ovc_vbus_pins[] = {
+     /* OVC/VBUS */
+     RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+     USB0_OVC_VBUS_MARK,
+};
Another option would have been to split the existing usb0 group in usb0_pwen and usb0_ovc. I'm not sure which is better though, I'd just like to know if you had given it a thought.

Quoted 4 lines

Quoted 2 lines
I actually did just that in my first local attempt, but I decided not to since it will only cause potential breakage.
OK. I assume that using PWEN without OVC/VBUS doesn't make sense, right ?

Correct!

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus to keep names short ?
Is there any particular reason why you want shorter names?
When it doesn't reduce clarity I prefer to keep names short, as that makes the code easier to read and write, and (slightly) lowers the memory footprint.

That sounds sane. =)

Quoted 11 lines

Quoted 4 lines
From my side, I prefer to keep the names in sync with the data sheet. In this particular case it is a shared pin so OVC is used for Host while VBUS is used for gadget, so if you're proposing to ditch VBUS then this feels somewhat inconsistent with the current gadget use case. =)
I thought the pin was used for over current detection only, but that doesn't make sense for function mode, you're right. Let's keep the name as-is then.
Provided PWEN without OVC/VBUS doesn't make sense and won't be needed,
Acked-by Laurent Pinchart <[email protected]>

Thanks,

/ magnus

Simon Horman

2014-02-07 00:15 UTC #8 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Quoted 61 lines · Magnus Damm

Hi Laurent,
Quoted 35 lines · Laurent Pinchart
Hi Magnus,
Quoted 32 lines · On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
Quoted 30 lines · Laurent Pinchart
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 13 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +0 -0
10:23:32.000000000
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
Quoted 11 lines
+0900 @@ -3231,6 +3231,13 @@ static const unsigned int usb0_pins[] =
 static const unsigned int usb0_mux[] = {
      USB0_PWEN_MARK, USB0_OVC_VBUS_MARK,
};
drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24 renamed from drivers/pinctrl/sh-pfc/pfc-r8a7790.c +7 -0
+static const unsigned int usb0_ovc_vbus_pins[] = {
+     /* OVC/VBUS */
+     RCAR_GP_PIN(5, 19),
+};
+static const unsigned int usb0_ovc_vbus_mux[] = {
+     USB0_OVC_VBUS_MARK,
+};
Another option would have been to split the existing usb0 group in usb0_pwen and usb0_ovc. I'm not sure which is better though, I'd just like to know if you had given it a thought.
Quoted 4 lines
Quoted 2 lines
I actually did just that in my first local attempt, but I decided not to since it will only cause potential breakage.
OK. I assume that using PWEN without OVC/VBUS doesn't make sense, right ?
Correct!
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Regardless, what about naming the new group usb0_ovc instead of usb0_ovc_bus to keep names short ?
Is there any particular reason why you want shorter names?
When it doesn't reduce clarity I prefer to keep names short, as that makes the code easier to read and write, and (slightly) lowers the memory footprint.
That sounds sane. =)
Quoted 5 lines
Quoted 4 lines
From my side, I prefer to keep the names in sync with the data sheet. In this particular case it is a shared pin so OVC is used for Host while VBUS is used for gadget, so if you're proposing to ditch VBUS then this feels somewhat inconsistent with the current gadget use case. =)

Quoted 8 lines

Quoted 6 lines
I thought the pin was used for over current detection only, but that doesn't make sense for function mode, you're right. Let's keep the name as-is then.
Provided PWEN without OVC/VBUS doesn't make sense and won't be needed,
Acked-by Laurent Pinchart <[email protected]>
Thanks,

Thanks Magnus.

Laurent, with that in mind could you pick up this patch?

Laurent Pinchart

2014-02-07 00:36 UTC #9 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Hi Simon,

Quoted 11 lines · On Friday 07 February 2014 09:15:44 Simon Horman wrote:

Quoted 10 lines · Magnus Damm
Quoted 9 lines · Laurent Pinchart
Quoted 8 lines · On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
Quoted 7 lines · Laurent Pinchart
Quoted 6 lines · On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>

[snip]

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
Acked-by Laurent Pinchart <[email protected]>
Thanks,
Thanks Magnus.
Laurent, with that in mind could you pick up this patch?

I was thinking about letting Linus pick up the PFC patches again now that the flood is over. Of course, if it can help, I can still pick the patches up and submit pull requests to Linus.

Linus, what's your opinion on this ? Would you rather pick the patches directly after I've acked them, or process pull requests ?

Signature · 3 lines

Regards,

Laurent Pinchart

Simon Horman

2014-02-07 01:39 UTC #10 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Quoted 8 lines · Laurent Pinchart

Hi Simon,
Quoted 5 lines · On Friday 07 February 2014 09:15:44 Simon Horman wrote:
Quoted 4 lines · Magnus Damm
Quoted 3 lines · Laurent Pinchart
Quoted 2 lines · On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
Quoted 1 line · Laurent Pinchart
On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:

Quoted 11 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>
[snip]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Acked-by Laurent Pinchart <[email protected]>

Acked-by Simon Horman <[email protected]>

Quoted 5 lines

Quoted 5 lines
Quoted 2 lines
Thanks,
Thanks Magnus.

Quoted 8 lines

Quoted 1 line
Laurent, with that in mind could you pick up this patch?
I was thinking about letting Linus pick up the PFC patches again now that the flood is over. Of course, if it can help, I can still pick the patches up and submit pull requests to Linus.
Linus, what's your opinion on this ? Would you rather pick the patches directly after I've acked them, or process pull requests ?

Either way is fine for me :)

Linus Walleij

2014-02-10 09:17 UTC #11 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

In reply to Laurent Pinchart

Quoted 6 lines · Laurent Pinchart

I was thinking about letting Linus pick up the PFC patches again now that the flood is over. Of course, if it can help, I can still pick the patches up and submit pull requests to Linus.
Linus, what's your opinion on this ? Would you rather pick the patches directly after I've acked them, or process pull requests ?

As long as it's reasonable traffic I'll take it. :-)

I'll queue this one then, I guess only patch 2/3?

Yours,
Linus Walleij

Laurent Pinchart

2014-02-10 12:08 UTC #12 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

Hi Linus,

Quoted 11 lines · On Monday 10 February 2014 10:17:00 Linus Walleij wrote:

Quoted 6 lines · Laurent Pinchart
I was thinking about letting Linus pick up the PFC patches again now that the flood is over. Of course, if it can help, I can still pick the patches up and submit pull requests to Linus.
Linus, what's your opinion on this ? Would you rather pick the patches directly after I've acked them, or process pull requests ?
As long as it's reasonable traffic I'll take it. :-)
I'll queue this one then, I guess only patch 2/3?

That's correct. Thank you.

Signature · 3 lines

Regards,

Laurent Pinchart

Linus Walleij

2014-02-10 09:19 UTC #13 lore raw

PATCH 2/3 pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

In reply to Magnus Damm

Quoted 6 lines · Magnus Damm

From: Magnus Damm <[email protected]>
Create a new group for the USB0 OVC/VBUS pin by itself. This allows us to monitor PWEN as GPIO on the Lager board.
Signed-off-by Magnus Damm <[email protected]>

Patch applied with Laurent's and Simon's ACKs.

Yours,
Linus Walleij

Magnus Damm

author maintainer

2014-01-29 23:10 UTC #14 lore raw

PATCH 3/3 ARM: shmobile: Lager USB0 cable detection workaround

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Add Lager board code to check the PWEN GPIO signal and refuse to allow probe of the USBHS driver in case of DIP misconfiguration.

For correct operation Lager DIP switches SW5 and SW6 shall be configured in 2-3 position to enable USB Function support.

If the DIP switch is configured incorrectly then the user can simply adjust the hardware and either reboot or use the bind interface to try to probe again:

# echo renesas_usbhs > /sys/bus/platform/drivers/renesas_usbhs/bind

Signed-off-by Magnus Damm <[email protected]>

---

Depends on "[PATCH 02/03] pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS"

 arch/arm/mach-shmobile/board-lager.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

arch/arm/mach-shmobile/board-lager.c +22 -3

@@ -406,13 +406,30 @@ static int usbhs_hardware_init(struct pl
 {
 	struct usbhs_private *priv = usbhs_get_priv(pdev);
 	struct usb_phy *phy;
+	int ret;
+
+	/* USB0 Function - use PWEN as GPIO input to detect DIP Switch SW5
+	 * setting to avoid VBUS short circuit due to wrong cable.
+	 * PWEN should be pulled up high if USB Function is selected by SW5
+	 */
+	gpio_request_one(RCAR_GP_PIN(5, 18), GPIOF_IN, NULL); /* USB0_PWEN */
+	if (!gpio_get_value(RCAR_GP_PIN(5, 18))) {
+		pr_warn("Error: USB Function not selected - check SW5 + SW6\n");
+		ret = -ENOTSUPP;
+		goto error;
+	}
 
 	phy = usb_get_phy_dev(&pdev->dev, 0);
-	if (IS_ERR(phy))
-		return PTR_ERR(phy);
+	if (IS_ERR(phy)) {
+		ret = PTR_ERR(phy);
+		goto error;
+	}
 
 	priv->phy = phy;
 	return 0;
+ error:
+	gpio_free(RCAR_GP_PIN(5, 18));
+	return ret;
 }
 
 static int usbhs_hardware_exit(struct platform_device *pdev)

@@ -424,6 +441,8 @@ static int usbhs_hardware_exit(struct pl
 
 	usb_put_phy(priv->phy);
 	priv->phy = NULL;
+
+	gpio_free(RCAR_GP_PIN(5, 18));
 	return 0;
 }

@@ -534,7 +553,7 @@ static const struct pinctrl_map lager_pi
 				  "vin1_clk", "vin1"),
 	/* USB0 */
 	PIN_MAP_MUX_GROUP_DEFAULT("renesas_usbhs", "pfc-r8a7790",
-				  "usb0", "usb0"),
+				  "usb0_ovc_vbus", "usb0"),
 };
 
 static void __init lager_add_standard_devices(void)

Laurent Pinchart

2014-01-31 01:14 UTC #15 lore raw

PATCH 3/3 ARM: shmobile: Lager USB0 cable detection workaround

Hi Magnus,

Thank you for the patch.

Quoted 5 lines · On Thursday 30 January 2014 08:10:29 Magnus Damm wrote:

From: Magnus Damm <[email protected]>
Add Lager board code to check the PWEN GPIO signal and refuse to allow probe of the USBHS driver in case of DIP misconfiguration.

Quoted 8 lines

For correct operation Lager DIP switches SW5 and SW6 shall be configured in 2-3 position to enable USB Function support.
If the DIP switch is configured incorrectly then the user can simply adjust the hardware and either reboot or use the bind interface to try to probe again:
# echo renesas_usbhs > /sys/bus/platform/drivers/renesas_usbhs/bind

Our of curiosity, and I know you will love the question, have you thought about how to implement this on multiplatform kernels without a board file ? :-)

This might be one of the valid cases where we won't be able to do without a board file. The callback functions in platform data, however, probably need to go.

arch/arm/mach-shmobile/board-lager.c

Quoted 11 lines

Signed-off-by Magnus Damm <[email protected]>
---
Depends on "[PATCH 02/03] pinctrl: sh-pfc: r8a7790: Break out USB0
OVC/VBUS"
 arch/arm/mach-shmobile/board-lager.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
arch/arm/mach-shmobile/board-lager.c +0 -0

10:17:20.000000000

Quoted 51 lines

+0900 @@ -406,13 +406,30 @@ static int usbhs_hardware_init(struct pl
{

arch/arm/mach-shmobile/board-lager.c +22 -3

 	struct usbhs_private *priv = usbhs_get_priv(pdev);
 	struct usb_phy *phy;
+	int ret;
+
+	/* USB0 Function - use PWEN as GPIO input to detect DIP Switch SW5
+	 * setting to avoid VBUS short circuit due to wrong cable.
+	 * PWEN should be pulled up high if USB Function is selected by SW5
+	 */
+	gpio_request_one(RCAR_GP_PIN(5, 18), GPIOF_IN, NULL); /* USB0_PWEN */
+	if (!gpio_get_value(RCAR_GP_PIN(5, 18))) {
+		pr_warn("Error: USB Function not selected - check SW5 + SW6\n");
+		ret = -ENOTSUPP;
+		goto error;
+	}
 
 	phy = usb_get_phy_dev(&pdev->dev, 0);
-	if (IS_ERR(phy))
-		return PTR_ERR(phy);
+	if (IS_ERR(phy)) {
+		ret = PTR_ERR(phy);
+		goto error;
+	}
 
 	priv->phy = phy;
 	return 0;
+ error:
+	gpio_free(RCAR_GP_PIN(5, 18));
+	return ret;
 }
 
 static int usbhs_hardware_exit(struct platform_device *pdev)

@@ -424,6 +441,8 @@ static int usbhs_hardware_exit(struct pl
 
 	usb_put_phy(priv->phy);
 	priv->phy = NULL;
+
+	gpio_free(RCAR_GP_PIN(5, 18));
 	return 0;
 }

@@ -534,7 +553,7 @@ static const struct pinctrl_map lager_pi
 				  "vin1_clk", "vin1"),
 	/* USB0 */
 	PIN_MAP_MUX_GROUP_DEFAULT("renesas_usbhs", "pfc-r8a7790",
-				  "usb0", "usb0"),
+				  "usb0_ovc_vbus", "usb0"),
 };
 
 static void __init lager_add_standard_devices(void)

Signature · 3 lines

Regards,

Laurent Pinchart

Magnus Damm

author maintainer

2014-01-31 03:35 UTC #16 lore raw

PATCH 3/3 ARM: shmobile: Lager USB0 cable detection workaround

Hi Laurent,

[CC Morimoto-san, the author of the USBHS driver]

Quoted 17 lines · Laurent Pinchart

Hi Magnus,
Thank you for the patch.
Quoted 12 lines · On Thursday 30 January 2014 08:10:29 Magnus Damm wrote:
From: Magnus Damm <[email protected]>
Add Lager board code to check the PWEN GPIO signal and refuse to allow probe of the USBHS driver in case of DIP misconfiguration.
For correct operation Lager DIP switches SW5 and SW6 shall be configured in 2-3 position to enable USB Function support.
If the DIP switch is configured incorrectly then the user can simply adjust the hardware and either reboot or use the bind interface to try to probe again:

Quoted 5 lines

Quoted 1 line
# echo renesas_usbhs > /sys/bus/platform/drivers/renesas_usbhs/bind
Our of curiosity, and I know you will love the question, have you thought about how to implement this on multiplatform kernels without a board file ? :-)

Thanks for asking. =)

Actually I've been giving this some thought already. And it happens to be that the sister board "Koelsch" has similar but slightly improved hardware.

In the Lager case we can only support USB Function properly and safely, and to avoid shooting ourselves in the foot we need a check like this patch implements. So on Lager RCAR_GP_PIN(5, 18) needs to be high for proper USB Function operation. If not we give up.

In case of Koelsch a MAX3355E chip is hooked up to handle dual function or OTG. So on that platform we can use either Function or Host. To figure out which cable is hooked up we can check the ID pin of the USB connector through the MAX3355E ID_OUT signal.

In general with a USB connector (and the MAX3355E chip) there are standard signal levels for ID pin detection. The ID signal for the USB micro-AB connector will be High in case of Function cable (pull-up, left open), and Low in case of Host (tied to GND).

So, to answer your question about how to support this without board-specific code written in C, I believe we need to add abstractions to support the MAX3355E chip and/or external ID pin somehow. Perhaps there is almost nothing to abstract there?

Regarding how to support Lager then I think we can use the same code as Koelsch because the ID pin signal is following the same signal level. So perhaps it is enough to add support for the USBHS driver to handle the ID pin as GPIO? If so, then we can use the same style for both Lager and Koelsch, but on Lager we also somehow need to tell the the USBHS driver that only Function is OK.

Quoted 3 lines

This might be one of the valid cases where we won't be able to do without a board file. The callback functions in platform data, however, probably need to go.

I think it is possible to get rid of the board file. And especially if we can reuse the code for multiple boards or SoC then this starts making sense - even to me! =)

Thanks,

/ magnus

Simon Horman

2014-02-06 07:00 UTC #17 lore raw

PATCH 3/3 ARM: shmobile: Lager USB0 cable detection workaround

In reply to Magnus Damm

Quoted 8 lines · Magnus Damm

From: Magnus Damm <[email protected]>
Add Lager board code to check the PWEN GPIO signal and refuse to allow probe of the USBHS driver in case of DIP misconfiguration.
For correct operation Lager DIP switches SW5 and SW6 shall be configured in 2-3 position to enable USB Function support.

Quoted 7 lines

If the DIP switch is configured incorrectly then the user can simply adjust the hardware and either reboot or use the bind interface to try to probe again:
# echo renesas_usbhs > /sys/bus/platform/drivers/renesas_usbhs/bind
Signed-off-by Magnus Damm <[email protected]>

Thanks, I have queue this up.

Simon Horman

2014-01-31 00:44 UTC #18 lore raw

cover ARM: shmobile: Lager USBHS update

In reply to Magnus Damm

Quoted 3 lines · Magnus Damm

ARM: shmobile: Lager USBHS update
[PATCH 01/03] ARM: shmobile: Remove Lager USBHS UDC ifdefs

Thanks, I will queue this one up.

Quoted 1 line

[PATCH 02/03] pinctrl: sh-pfc: r8a7790: Break out USB0 OVC/VBUS

I believe that this one is for Laurent.

Quoted 1 line

[PATCH 03/03] ARM: shmobile: Lager USB0 cable detection workaround

I will hold off on this one as it depends on the previous patch.

Quoted 24 lines

Update the Lager USB0 code to let the USBHS device always be present and check for DIP switch configuration. To be able to check the DIP switch configuration the r8a7790 PINCTRL bits need to be updated too.
With this series USB0 on Lager will be fixed to USB Function via the USBHS driver. In theory the SoC can also use USB Host on this port either via USBHS or PCI USB, but since cable detection is missing it is possible for the user to misconfigure USB Host and drive VBUS with the wrong cable. Because of that USB0 will be forced to stick to Function-only in the Lager board code.
Signed-off-by Magnus Damm <[email protected]>
---
Written against renesas.git tag renesas-devel-v3.13-20140127
 arch/arm/mach-shmobile/board-lager.c |   29 ++++++++++++++++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
 2 files changed, 31 insertions(+), 7 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html