[PATCH 00/03] ARM: shmobile: Lager USBHS update

[PATCH 00/03] ARM: shmobile: Lager USBHS update

[Magnus Damm]

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 <damm@opensource.se>
---

 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(-)

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

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

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 <damm@opensource.se>
---

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

--- 0003/arch/arm/mach-shmobile/board-lager.c
+++ work/arch/arm/mach-shmobile/board-lager.c	2014-01-24 10:16:15.000000000 +0900
@@ -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 = {

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

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

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 <damm@opensource.se>
---

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

--- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c	2014-01-24 10:23:32.000000000 +0900
@@ -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[] = {

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

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> 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 <damm@opensource.se>
> ---
> 
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c	2014-01-24 
10:23:32.000000000
> +0900 @@ -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,
> +};

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 ?

>  /* - 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[] = {

-- 
Regards,

Laurent Pinchart

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

[Magnus Damm]

Hi Laurent,

On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> 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 <damm@opensource.se>
>> ---
>>
>>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
> 10:23:32.000000000
>> +0900 @@ -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,
>> +};
>
> 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.

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

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

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

[Simon Horman]

Hi Laurent,

On Fri, Jan 31, 2014 at 12:10:05PM +0900, Magnus Damm wrote:
> Hi Laurent,
> 
> On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Magnus,
> >
> > Thank you for the patch.
> >
> > On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> >> From: Magnus Damm <damm@opensource.se>
> >>
> >> 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 <damm@opensource.se>
> >> ---
> >>
> >>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
> > 10:23:32.000000000
> >> +0900 @@ -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,
> >> +};
> >
> > 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.
> 
> > 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?
> 
> >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

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

[Laurent Pinchart]

Hi Magnus,

On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
> On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart wrote:
> > On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> >> From: Magnus Damm <damm@opensource.se>
> >> 
> >> 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 <damm@opensource.se>
> >> ---
> >> 
> >>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >> 
> >> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
> > 10:23:32.000000000
> >> +0900 @@ -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,
> >> +};
> > 
> > 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.

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

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

> 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 <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

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

[Magnus Damm]

Hi Laurent,

On Thu, Feb 6, 2014 at 8:01 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
>> On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart wrote:
>> > On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
>> >> From: Magnus Damm <damm@opensource.se>
>> >>
>> >> 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 <damm@opensource.se>
>> >> ---
>> >>
>> >>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
>> >>  1 file changed, 9 insertions(+)
>> >>
>> >> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>> >> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
>> > 10:23:32.000000000
>> >> +0900 @@ -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,
>> >> +};
>> >
>> > 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.
>
> OK. I assume that using PWEN without OVC/VBUS doesn't make sense, right ?

Correct!

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

>> 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 <laurent.pinchart@ideasonboard.com>

Thanks,

/ magnus

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

[Simon Horman]

On Thu, Feb 06, 2014 at 10:34:27PM +0900, Magnus Damm wrote:
> Hi Laurent,
> 
> On Thu, Feb 6, 2014 at 8:01 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Magnus,
> >
> > On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
> >> On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart wrote:
> >> > On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> >> >> From: Magnus Damm <damm@opensource.se>
> >> >>
> >> >> 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 <damm@opensource.se>
> >> >> ---
> >> >>
> >> >>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |    9 +++++++++
> >> >>  1 file changed, 9 insertions(+)
> >> >>
> >> >> --- 0001/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >> >> +++ work/drivers/pinctrl/sh-pfc/pfc-r8a7790.c 2014-01-24
> >> > 10:23:32.000000000
> >> >> +0900 @@ -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,
> >> >> +};
> >> >
> >> > 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.
> >
> > OK. I assume that using PWEN without OVC/VBUS doesn't make sense, right ?
> 
> Correct!
> 
> >> > 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. =)
> 
> >> 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 <laurent.pinchart@ideasonboard.com>
> 
> Thanks,

Thanks Magnus.

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

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

[Laurent Pinchart]

Hi Simon,

On Friday 07 February 2014 09:15:44 Simon Horman wrote:
> On Thu, Feb 06, 2014 at 10:34:27PM +0900, Magnus Damm wrote:
> > On Thu, Feb 6, 2014 at 8:01 PM, Laurent Pinchart wrote:
> > > On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
> > >> On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart wrote:
> > >> > On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> > >> >> From: Magnus Damm <damm@opensource.se>
> > >> >> 
> > >> >> 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 <damm@opensource.se>

[snip]

> > > 
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > 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 ?

-- 
Regards,

Laurent Pinchart

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

[Simon Horman]

On Fri, Feb 07, 2014 at 01:36:35AM +0100, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Friday 07 February 2014 09:15:44 Simon Horman wrote:
> > On Thu, Feb 06, 2014 at 10:34:27PM +0900, Magnus Damm wrote:
> > > On Thu, Feb 6, 2014 at 8:01 PM, Laurent Pinchart wrote:
> > > > On Friday 31 January 2014 12:10:05 Magnus Damm wrote:
> > > >> On Fri, Jan 31, 2014 at 10:17 AM, Laurent Pinchart wrote:
> > > >> > On Thursday 30 January 2014 08:10:19 Magnus Damm wrote:
> > > >> >> From: Magnus Damm <damm@opensource.se>
> > > >> >> 
> > > >> >> 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 <damm@opensource.se>
> 
> [snip]
> 
> > > > 
> > > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Simon Horman <horms+renesas@verge.net.au>

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

Either way is fine for me :)

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

[Linus Walleij]

On Fri, Feb 7, 2014 at 1:36 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> 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

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

[Laurent Pinchart]

Hi Linus,

On Monday 10 February 2014 10:17:00 Linus Walleij wrote:
> On Fri, Feb 7, 2014 at 1:36 AM, Laurent Pinchart wrote:
> > 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.

-- 
Regards,

Laurent Pinchart

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

[Linus Walleij]

On Thu, Jan 30, 2014 at 12:10 AM, Magnus Damm <magnus.damm@gmail.com> wrote:

> From: Magnus Damm <damm@opensource.se>
>
> 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 <damm@opensource.se>

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

Yours,
Linus Walleij

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

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

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 <damm@opensource.se>
---

 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(-)

--- 0004/arch/arm/mach-shmobile/board-lager.c
+++ work/arch/arm/mach-shmobile/board-lager.c	2014-01-24 10:17:20.000000000 +0900
@@ -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)

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

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Thursday 30 January 2014 08:10:29 Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> 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

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.

> Signed-off-by: Magnus Damm <damm@opensource.se>
> ---
> 
>  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(-)
> 
> --- 0004/arch/arm/mach-shmobile/board-lager.c
> +++ work/arch/arm/mach-shmobile/board-lager.c	2014-01-24 
10:17:20.000000000
> +0900 @@ -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)

-- 
Regards,

Laurent Pinchart

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

[Magnus Damm]

Hi Laurent,

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

On Fri, Jan 31, 2014 at 10:14 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Thursday 30 January 2014 08:10:29 Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> 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
>
> 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.

> 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

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

[Simon Horman]

On Thu, Jan 30, 2014 at 08:10:29AM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> 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 <damm@opensource.se>

Thanks, I have queue this up.

Re: [PATCH 00/03] ARM: shmobile: Lager USBHS update

[Simon Horman]

On Thu, Jan 30, 2014 at 08:09:59AM +0900, Magnus Damm wrote:
> ARM: shmobile: Lager USBHS update
> 
> [PATCH 01/03] ARM: shmobile: Remove Lager USBHS UDC ifdefs

Thanks, I will queue this one up.

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

I believe that this one is for Laurent.

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

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

> 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 <damm@opensource.se>
> ---
> 
>  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>