diff mbox

[V4,3/4] ARM: shmobile: koelsch: Add USBHS and internal PCI USB support

Message ID 1393178459-14637-4-git-send-email-vladimir.barinov@cogentembedded.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Vladimir Barinov Feb. 23, 2014, 6 p.m. UTC
From: Vladimir Barinov <vladimir.barinov@cogentembedded.com>

This adds the following to R-Car M2 Koelsch board:
- USBHS PHY
- USBHS device
- internal PCI USB host devices

Depending on state of ID pin from MAX3355E chip the usb0 is configured
ether as host or gadget.
In case of gadget the USBHS device is registered.
In case of host the PCI USB is registered.

The USB phy is bound to either USB host or USBHS device respectively, hence
configured to ether channel 0 or 2.

The USBHS can act as USB Host, and this can be easily configured.
But the simplest test with storage stick connected to USBHS Host provides
IP resets and system hangs. Even the PWEN pin is not handled and it is nessasary to
provide VBUS using gpio. It is easy to see in RCAR H2/M2 documentation
that the USBHS IP changed. F.e. the PWEN/EXTLP pins are no more presented in
USBHS via DVSTCTR register. And others.

Since the USBHS is not stable in Host mode lets connect fully tested PCI USB IP to usb0.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>

Changes in V4:
* folded USBHS and internal PCI USB related patches together
* added handling of ID pin from MAX3355E chip
* removed ifdefs

Changes in V3:
* fixed a typo in the log message;
* fixed the USB1 device name in the pinmux table.

Changes in V2:
* capitalized ARM in the subject;
* rebased on top the latest devel tag;
* added pipe_type array to the usbhs platform info since it differs from the default.

* capitalized ARM in the subject;
* rebased on top the latest devel tag.

---
 arch/arm/mach-shmobile/board-koelsch.c |  183 +++++++++++++++++++++++++++++++++
 1 file changed, 183 insertions(+)

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

Comments

Magnus Damm Feb. 24, 2014, 3:52 a.m. UTC | #1
On Mon, Feb 24, 2014 at 3:00 AM,  <vladimir.barinov@cogentembedded.com> wrote:
> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
>
> This adds the following to R-Car M2 Koelsch board:
> - USBHS PHY
> - USBHS device
> - internal PCI USB host devices
>
> Depending on state of ID pin from MAX3355E chip the usb0 is configured
> ether as host or gadget.
> In case of gadget the USBHS device is registered.
> In case of host the PCI USB is registered.
>
> The USB phy is bound to either USB host or USBHS device respectively, hence
> configured to ether channel 0 or 2.
>
> The USBHS can act as USB Host, and this can be easily configured.
> But the simplest test with storage stick connected to USBHS Host provides
> IP resets and system hangs. Even the PWEN pin is not handled and it is nessasary to
> provide VBUS using gpio. It is easy to see in RCAR H2/M2 documentation
> that the USBHS IP changed. F.e. the PWEN/EXTLP pins are no more presented in
> USBHS via DVSTCTR register. And others.
>
> Since the USBHS is not stable in Host mode lets connect fully tested PCI USB IP to usb0.
>
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
>
> Changes in V4:
> * folded USBHS and internal PCI USB related patches together
> * added handling of ID pin from MAX3355E chip
> * removed ifdefs

Thanks for removing the ifdefs and adding some initial implementation
for the MAX chip.

Please see my comments below.

> --- build.orig/arch/arm/mach-shmobile/board-koelsch.c   2014-02-23 21:47:44.510571967 +0400
> +++ build/arch/arm/mach-shmobile/board-koelsch.c        2014-02-23 21:47:59.358571662 +0400
> @@ -367,6 +370,177 @@
>         DEFINE_RES_IRQ(gic_spi(168)),
>  };
>
> +/* USBHS */
> +static const struct resource usbhs_resources[] __initconst = {
> +       DEFINE_RES_MEM(0xe6590000, 0x100),
> +       DEFINE_RES_IRQ(gic_spi(107)),
> +};
> +
> +struct usbhs_private {
> +       struct renesas_usbhs_platform_info info;
> +       struct usb_phy *phy;
> +};
> +
> +#define usbhs_get_priv(pdev) \
> +       container_of(renesas_usbhs_get_info(pdev), struct usbhs_private, info)
> +
> +static int usbhs_power_ctrl(struct platform_device *pdev,
> +                               void __iomem *base, int enable)
> +{
> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
> +
> +       if (!priv->phy)
> +               return -ENODEV;
> +
> +       if (enable) {
> +               int retval = usb_phy_init(priv->phy);
> +
> +               if (!retval)
> +                       retval = usb_phy_set_suspend(priv->phy, 0);
> +               return retval;
> +       }
> +
> +       usb_phy_set_suspend(priv->phy, 1);
> +       usb_phy_shutdown(priv->phy);
> +       return 0;
> +}
> +
> +static int usbhs_hardware_init(struct platform_device *pdev)
> +{
> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
> +       struct usb_phy *phy;
> +
> +       phy = usb_get_phy_dev(&pdev->dev, 0);
> +       if (IS_ERR(phy))
> +               return PTR_ERR(phy);
> +
> +       priv->phy = phy;
> +       return 0;
> +}
> +
> +static int usbhs_hardware_exit(struct platform_device *pdev)
> +{
> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
> +
> +       if (!priv->phy)
> +               return 0;
> +
> +       usb_put_phy(priv->phy);
> +       priv->phy = NULL;
> +       return 0;
> +}
> +
> +static int usbhs_get_id(struct platform_device *pdev)
> +{
> +       return USBHS_GADGET;
> +}

Uhm, I sort of expected this place to be where you read out the ID pin
state from the MAX device.

> +static u32 koelsch_usbhs_pipe_type[] = {
> +       USB_ENDPOINT_XFER_CONTROL,
> +       USB_ENDPOINT_XFER_ISOC,
> +       USB_ENDPOINT_XFER_ISOC,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_INT,
> +       USB_ENDPOINT_XFER_INT,
> +       USB_ENDPOINT_XFER_INT,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +       USB_ENDPOINT_XFER_BULK,
> +};
> +
> +static struct usbhs_private usbhs_priv __initdata = {
> +       .info = {
> +               .platform_callback = {
> +                       .power_ctrl     = usbhs_power_ctrl,
> +                       .hardware_init  = usbhs_hardware_init,
> +                       .hardware_exit  = usbhs_hardware_exit,
> +                       .get_id         = usbhs_get_id,
> +               },
> +               .driver_param = {
> +                       .buswait_bwait  = 4,
> +                       .pipe_type = koelsch_usbhs_pipe_type,
> +                       .pipe_size = ARRAY_SIZE(koelsch_usbhs_pipe_type),
> +               },
> +       }
> +};
> +
> +static void __init koelsch_add_usb0_gadget(void)
> +{
> +       usb_bind_phy("renesas_usbhs", 0, "usb_phy_rcar_gen2");
> +       platform_device_register_resndata(&platform_bus,
> +                                         "renesas_usbhs", -1,
> +                                         usbhs_resources,
> +                                         ARRAY_SIZE(usbhs_resources),
> +                                         &usbhs_priv.info,
> +                                         sizeof(usbhs_priv.info));
> +}
> +
> +/* Internal PCI0 */
> +static const struct resource pci0_resources[] __initconst = {
> +       DEFINE_RES_MEM(0xee090000, 0x10000),    /* CFG */
> +       DEFINE_RES_MEM(0xee080000, 0x10000),    /* MEM */
> +       DEFINE_RES_IRQ(gic_spi(108)),
> +};
> +
> +static void __init koelsch_add_usb0_host(void)
> +{
> +       usb_bind_phy("0000:00:01.0", 0, "usb_phy_rcar_gen2");
> +       usb_bind_phy("0000:00:02.0", 0, "usb_phy_rcar_gen2");
> +       platform_device_register_simple("pci-rcar-gen2",
> +                                       0, pci0_resources,
> +                                       ARRAY_SIZE(pci0_resources));
> +}
> +
> +/* Internal PCI1 */
> +static const struct resource pci1_resources[] __initconst = {
> +       DEFINE_RES_MEM(0xee0d0000, 0x10000),    /* CFG */
> +       DEFINE_RES_MEM(0xee0c0000, 0x10000),    /* MEM */
> +       DEFINE_RES_IRQ(gic_spi(113)),
> +};
> +
> +static void __init koelsch_add_usb1_host(void)
> +{
> +       platform_device_register_simple("pci-rcar-gen2",
> +                                       1, pci1_resources,
> +                                       ARRAY_SIZE(pci1_resources));
> +}
> +
> +/* USBHS PHY */
> +static struct rcar_gen2_phy_platform_data usbhs_phy_pdata = {
> +       .chan2_pci = 1, /* Channel 2 is PCI USB host */
> +};
> +
> +static const struct resource usbhs_phy_resources[] __initconst = {
> +       DEFINE_RES_MEM(0xe6590100, 0x100),
> +};
> +
> +/* Add all available USB devices */
> +static void __init koelsch_add_usb_devices(void)
> +{
> +       /* MAX3355E ID pin */
> +       gpio_request_one(RCAR_GP_PIN(5, 31), GPIOF_IN, NULL);
> +       if (!gpio_get_value(RCAR_GP_PIN(5, 31))) {
> +               usbhs_phy_pdata.chan0_pci = 1; /* Channel 0 is PCI USB host */
> +               koelsch_add_usb0_host();
> +       } else {
> +               usbhs_phy_pdata.chan0_pci = 0; /* Channel 0 is USBHS */
> +               koelsch_add_usb0_gadget();
> +       }

I don't think we should perform this kind of check at boot-up. This
goes without saying, but normal USB supports hot-plug so we should
check the cable type when the cable insertion event happens. Please
see my comment above for USBHS-specific location.

I do however understand that according to your investigation you
cannot use USBHS in host mode. I believe further investigation is
needed in that area to determine what is the best way forward.
Regarding VBUS control, I believe it should be possible to drive the
USB0_VBUS as GPIO and manually control the power.

Would it be possible for you to break out USB PCI support for USB1 and
resend that so we can begin by merging that?

Thanks,

/ magnus
--
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
Vladimir Barinov Feb. 24, 2014, 7:34 a.m. UTC | #2
Hello Magnus,

Thank you for the quick response.

On 02/24/2014 07:52 AM, Magnus Damm wrote:
> +static int usbhs_hardware_init(struct platform_device *pdev)
> +{
> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
> +       struct usb_phy *phy;
> +
> +       phy = usb_get_phy_dev(&pdev->dev, 0);
> +       if (IS_ERR(phy))
> +               return PTR_ERR(phy);
> +
> +       priv->phy = phy;
> +       return 0;
> +}
> +
> +static int usbhs_hardware_exit(struct platform_device *pdev)
> +{
> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
> +
> +       if (!priv->phy)
> +               return 0;
> +
> +       usb_put_phy(priv->phy);
> +       priv->phy = NULL;
> +       return 0;
> +}
> +
> +static int usbhs_get_id(struct platform_device *pdev)
> +{
> +       return USBHS_GADGET;
> +}
> Uhm, I sort of expected this place to be where you read out the ID pin
> state from the MAX device.
Yes, I've seen your work for lager board.
I did differently then you beacause of problem in USBHS Host driver, 
hence the need of setup PHY at initial stage to PCI USB and not to USBHS.
>
>> +static u32 koelsch_usbhs_pipe_type[] = {
>> +
>> +/* Add all available USB devices */
>> +static void __init koelsch_add_usb_devices(void)
>> +{
>> +       /* MAX3355E ID pin */
>> +       gpio_request_one(RCAR_GP_PIN(5, 31), GPIOF_IN, NULL);
>> +       if (!gpio_get_value(RCAR_GP_PIN(5, 31))) {
>> +               usbhs_phy_pdata.chan0_pci = 1; /* Channel 0 is PCI USB host */
>> +               koelsch_add_usb0_host();
>> +       } else {
>> +               usbhs_phy_pdata.chan0_pci = 0; /* Channel 0 is USBHS */
>> +               koelsch_add_usb0_gadget();
>> +       }
> I don't think we should perform this kind of check at boot-up. This
> goes without saying, but normal USB supports hot-plug so we should
> check the cable type when the cable insertion event happens. Please
> see my comment above for USBHS-specific location.
>
> I do however understand that according to your investigation you
> cannot use USBHS in host mode. I believe further investigation is
> needed in that area to determine what is the best way forward.
> Regarding VBUS control, I believe it should be possible to drive the
> USB0_VBUS as GPIO and manually control the power.
I see.
>
> Would it be possible for you to break out USB PCI support for USB1 and
> resend that so we can begin by merging that?
Wouldn't you like me also add USBHS in gadget mode for USB0 with the 
similar check like you did on lager (with ID pin),
since it does not need the gpio-rcar changes too.

Also if you'd like I can add the USBHS in Host mode with the ID pin 
check like you suggested, but the usb0 host will not be stable.
Probably this could speed up the USBHS Host development/fixing.

Regards,
Vladimir
--
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
Magnus Damm Feb. 24, 2014, 8:05 a.m. UTC | #3
Hi Vladimir,

On Mon, Feb 24, 2014 at 4:34 PM, Vladimir Barinov
<vladimir.barinov@cogentembedded.com> wrote:
> Hello Magnus,
>
> Thank you for the quick response.
>
>
> On 02/24/2014 07:52 AM, Magnus Damm wrote:
>>
>> +static int usbhs_hardware_init(struct platform_device *pdev)
>> +{
>> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
>> +       struct usb_phy *phy;
>> +
>> +       phy = usb_get_phy_dev(&pdev->dev, 0);
>> +       if (IS_ERR(phy))
>> +               return PTR_ERR(phy);
>> +
>> +       priv->phy = phy;
>> +       return 0;
>> +}
>> +
>> +static int usbhs_hardware_exit(struct platform_device *pdev)
>> +{
>> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
>> +
>> +       if (!priv->phy)
>> +               return 0;
>> +
>> +       usb_put_phy(priv->phy);
>> +       priv->phy = NULL;
>> +       return 0;
>> +}
>> +
>> +static int usbhs_get_id(struct platform_device *pdev)
>> +{
>> +       return USBHS_GADGET;
>> +}
>> Uhm, I sort of expected this place to be where you read out the ID pin
>> state from the MAX device.
>
> Yes, I've seen your work for lager board.
> I did differently then you beacause of problem in USBHS Host driver, hence
> the need of setup PHY at initial stage to PCI USB and not to USBHS.

Yeah, I understand. But with the current patches I wonder, isn't there
risk for short circuit ? Say that a USB host cable is connected during
boot and the PCI USB host controller is hooked up, what is stopping us
from switching the cable and driving VBUS from two sides using a USB
function cable? So the current patches seem a bit unsafe to me.

>>> +static u32 koelsch_usbhs_pipe_type[] = {
>>> +
>>> +/* Add all available USB devices */
>>> +static void __init koelsch_add_usb_devices(void)
>>> +{
>>> +       /* MAX3355E ID pin */
>>> +       gpio_request_one(RCAR_GP_PIN(5, 31), GPIOF_IN, NULL);
>>> +       if (!gpio_get_value(RCAR_GP_PIN(5, 31))) {
>>> +               usbhs_phy_pdata.chan0_pci = 1; /* Channel 0 is PCI USB
>>> host */
>>> +               koelsch_add_usb0_host();
>>> +       } else {
>>> +               usbhs_phy_pdata.chan0_pci = 0; /* Channel 0 is USBHS */
>>> +               koelsch_add_usb0_gadget();
>>> +       }
>>
>> I don't think we should perform this kind of check at boot-up. This
>> goes without saying, but normal USB supports hot-plug so we should
>> check the cable type when the cable insertion event happens. Please
>> see my comment above for USBHS-specific location.
>>
>> I do however understand that according to your investigation you
>> cannot use USBHS in host mode. I believe further investigation is
>> needed in that area to determine what is the best way forward.
>> Regarding VBUS control, I believe it should be possible to drive the
>> USB0_VBUS as GPIO and manually control the power.
>
> I see.

To control USB0_VBUS as GPIO you may need to adjust the PFC tables for
pinctrl. At some point, would it be possible for you to cook up some
prototype code to try to control the USB0_VBUS signal via GPIO? This
may be pointless if the USBHS hardware cannot operate in Host mode
though.

>> Would it be possible for you to break out USB PCI support for USB1 and
>> resend that so we can begin by merging that?
>
> Wouldn't you like me also add USBHS in gadget mode for USB0 with the similar
> check like you did on lager (with ID pin),
> since it does not need the gpio-rcar changes too.

Thanks for your offer. Yes, that would be nice. May I suggest doing it
on two levels:
1) Gadget-only support (is it possible to perform runtime check of ID
pin value at insert event and give error in case of Host?)
2) Incremental USBHS host patch

Using two incremental patches like above we can begin by merging 1)
and keep on working on 2).

> Also if you'd like I can add the USBHS in Host mode with the ID pin check
> like you suggested, but the usb0 host will not be stable.
> Probably this could speed up the USBHS Host development/fixing.

Please add it as a separate incremental patch if possible. I'd like to
have some kind of stable level of support without any funky
dependencies as a first goal, then keep on trying to get USBHS
working.

I think PCI USB for the micro USB port can simply be dropped now and
only use USBHS.

Thank you!

/ magnus
--
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
Vladimir Barinov Feb. 24, 2014, 8:29 a.m. UTC | #4
Hi Magnus,

On 02/24/2014 12:05 PM, Magnus Damm wrote:
> Hi Vladimir,
>
> On Mon, Feb 24, 2014 at 4:34 PM, Vladimir Barinov
> <vladimir.barinov@cogentembedded.com>  wrote:
>> Hello Magnus,
>>
>> Thank you for the quick response.
>>
>>
>> On 02/24/2014 07:52 AM, Magnus Damm wrote:
>>> +static int usbhs_hardware_init(struct platform_device *pdev)
>>> +{
>>> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
>>> +       struct usb_phy *phy;
>>> +
>>> +       phy = usb_get_phy_dev(&pdev->dev, 0);
>>> +       if (IS_ERR(phy))
>>> +               return PTR_ERR(phy);
>>> +
>>> +       priv->phy = phy;
>>> +       return 0;
>>> +}
>>> +
>>> +static int usbhs_hardware_exit(struct platform_device *pdev)
>>> +{
>>> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
>>> +
>>> +       if (!priv->phy)
>>> +               return 0;
>>> +
>>> +       usb_put_phy(priv->phy);
>>> +       priv->phy = NULL;
>>> +       return 0;
>>> +}
>>> +
>>> +static int usbhs_get_id(struct platform_device *pdev)
>>> +{
>>> +       return USBHS_GADGET;
>>> +}
>>> Uhm, I sort of expected this place to be where you read out the ID pin
>>> state from the MAX device.
>> Yes, I've seen your work for lager board.
>> I did differently then you beacause of problem in USBHS Host driver, hence
>> the need of setup PHY at initial stage to PCI USB and not to USBHS.
> Yeah, I understand. But with the current patches I wonder, isn't there
> risk for short circuit ? Say that a USB host cable is connected during
> boot and the PCI USB host controller is hooked up, what is stopping us
> from switching the cable and driving VBUS from two sides using a USB
> function cable? So the current patches seem a bit unsafe to me.
Yes.
In case of such condition, when the usb cable changed after host device 
probe the
risk of VBUS collision is obvious.
The interrupt driven ID pin monitoring in board file could help?

Probably the separate driver for the MAX3355 should not be added since 
this provides
poor information, most significant is cable ID.
>
>>>> +static u32 koelsch_usbhs_pipe_type[] = {
>>>> +
>>>> +/* Add all available USB devices */
>>>> +static void __init koelsch_add_usb_devices(void)
>>>> +{
>>>> +       /* MAX3355E ID pin */
>>>> +       gpio_request_one(RCAR_GP_PIN(5, 31), GPIOF_IN, NULL);
>>>> +       if (!gpio_get_value(RCAR_GP_PIN(5, 31))) {
>>>> +               usbhs_phy_pdata.chan0_pci = 1; /* Channel 0 is PCI USB
>>>> host */
>>>> +               koelsch_add_usb0_host();
>>>> +       } else {
>>>> +               usbhs_phy_pdata.chan0_pci = 0; /* Channel 0 is USBHS */
>>>> +               koelsch_add_usb0_gadget();
>>>> +       }
>>> I don't think we should perform this kind of check at boot-up. This
>>> goes without saying, but normal USB supports hot-plug so we should
>>> check the cable type when the cable insertion event happens. Please
>>> see my comment above for USBHS-specific location.
>>>
>>> I do however understand that according to your investigation you
>>> cannot use USBHS in host mode. I believe further investigation is
>>> needed in that area to determine what is the best way forward.
>>> Regarding VBUS control, I believe it should be possible to drive the
>>> USB0_VBUS as GPIO and manually control the power.
>> I see.
> To control USB0_VBUS as GPIO you may need to adjust the PFC tables for
> pinctrl. At some point, would it be possible for you to cook up some
> prototype code to try to control the USB0_VBUS signal via GPIO? This
> may be pointless if the USBHS hardware cannot operate in Host mode
> though.
No need, there is a set_vbus callback in HSBHS platform code for this 
purpose.
>
>>> Would it be possible for you to break out USB PCI support for USB1 and
>>> resend that so we can begin by merging that?
>> Wouldn't you like me also add USBHS in gadget mode for USB0 with the similar
>> check like you did on lager (with ID pin),
>> since it does not need the gpio-rcar changes too.
> Thanks for your offer. Yes, that would be nice. May I suggest doing it
> on two levels:
> 1) Gadget-only support (is it possible to perform runtime check of ID
> pin value at insert event and give error in case of Host?)
> 2) Incremental USBHS host patch
>
> Using two incremental patches like above we can begin by merging 1)
> and keep on working on 2).
>
>> Also if you'd like I can add the USBHS in Host mode with the ID pin check
>> like you suggested, but the usb0 host will not be stable.
>> Probably this could speed up the USBHS Host development/fixing.
> Please add it as a separate incremental patch if possible. I'd like to
> have some kind of stable level of support without any funky
> dependencies as a first goal, then keep on trying to get USBHS
> working.
>
> I think PCI USB for the micro USB port can simply be dropped now and
> only use USBHS.
Sure, will do.

Regards,
Vladimir
--
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
Magnus Damm Feb. 24, 2014, 8:57 a.m. UTC | #5
Hi Vladimir,

On Mon, Feb 24, 2014 at 5:29 PM, Vladimir Barinov
<vladimir.barinov@cogentembedded.com> wrote:
> Hi Magnus,
>
>
> On 02/24/2014 12:05 PM, Magnus Damm wrote:
>>
>> Hi Vladimir,
>>
>> On Mon, Feb 24, 2014 at 4:34 PM, Vladimir Barinov
>> <vladimir.barinov@cogentembedded.com>  wrote:
>>>
>>> Hello Magnus,
>>>
>>> Thank you for the quick response.
>>>
>>>
>>> On 02/24/2014 07:52 AM, Magnus Damm wrote:
>>>>
>>>> +static int usbhs_hardware_init(struct platform_device *pdev)
>>>> +{
>>>> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
>>>> +       struct usb_phy *phy;
>>>> +
>>>> +       phy = usb_get_phy_dev(&pdev->dev, 0);
>>>> +       if (IS_ERR(phy))
>>>> +               return PTR_ERR(phy);
>>>> +
>>>> +       priv->phy = phy;
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int usbhs_hardware_exit(struct platform_device *pdev)
>>>> +{
>>>> +       struct usbhs_private *priv = usbhs_get_priv(pdev);
>>>> +
>>>> +       if (!priv->phy)
>>>> +               return 0;
>>>> +
>>>> +       usb_put_phy(priv->phy);
>>>> +       priv->phy = NULL;
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int usbhs_get_id(struct platform_device *pdev)
>>>> +{
>>>> +       return USBHS_GADGET;
>>>> +}
>>>> Uhm, I sort of expected this place to be where you read out the ID pin
>>>> state from the MAX device.
>>>
>>> Yes, I've seen your work for lager board.
>>> I did differently then you beacause of problem in USBHS Host driver,
>>> hence
>>> the need of setup PHY at initial stage to PCI USB and not to USBHS.
>>
>> Yeah, I understand. But with the current patches I wonder, isn't there
>> risk for short circuit ? Say that a USB host cable is connected during
>> boot and the PCI USB host controller is hooked up, what is stopping us
>> from switching the cable and driving VBUS from two sides using a USB
>> function cable? So the current patches seem a bit unsafe to me.
>
> Yes.
> In case of such condition, when the usb cable changed after host device
> probe the
> risk of VBUS collision is obvious.
> The interrupt driven ID pin monitoring in board file could help?

Well, it the board code may be one place to workaround things, but we
probably want to reuse this code somehow. And we already have some
kind of handling in the USBHS code.

When I grep in the directory drivers/usb/phy I can see those drivers
requesting IRQs. Perhaps this IRQ is for hotplug purpose? Maybe we can
have reusable GPIO interrupt driven hotplug detection in some generic
place?

The question is just how to let the OHCI/EHCI code coexist together
with the PHY IRQ code.

> Probably the separate driver for the MAX3355 should not be added since this
> provides
> poor information, most significant is cable ID.

Eventually we may end up with a separate driver, but to begin with it
is probably fine just to monitor GPIO pin states.

>>>>> +static u32 koelsch_usbhs_pipe_type[] = {
>>>>> +
>>>>> +/* Add all available USB devices */
>>>>> +static void __init koelsch_add_usb_devices(void)
>>>>> +{
>>>>> +       /* MAX3355E ID pin */
>>>>> +       gpio_request_one(RCAR_GP_PIN(5, 31), GPIOF_IN, NULL);
>>>>> +       if (!gpio_get_value(RCAR_GP_PIN(5, 31))) {
>>>>> +               usbhs_phy_pdata.chan0_pci = 1; /* Channel 0 is PCI USB
>>>>> host */
>>>>> +               koelsch_add_usb0_host();
>>>>> +       } else {
>>>>> +               usbhs_phy_pdata.chan0_pci = 0; /* Channel 0 is USBHS */
>>>>> +               koelsch_add_usb0_gadget();
>>>>> +       }
>>>>
>>>> I don't think we should perform this kind of check at boot-up. This
>>>> goes without saying, but normal USB supports hot-plug so we should
>>>> check the cable type when the cable insertion event happens. Please
>>>> see my comment above for USBHS-specific location.
>>>>
>>>> I do however understand that according to your investigation you
>>>> cannot use USBHS in host mode. I believe further investigation is
>>>> needed in that area to determine what is the best way forward.
>>>> Regarding VBUS control, I believe it should be possible to drive the
>>>> USB0_VBUS as GPIO and manually control the power.
>>>
>>> I see.
>>
>> To control USB0_VBUS as GPIO you may need to adjust the PFC tables for
>> pinctrl. At some point, would it be possible for you to cook up some
>> prototype code to try to control the USB0_VBUS signal via GPIO? This
>> may be pointless if the USBHS hardware cannot operate in Host mode
>> though.
>
> No need, there is a set_vbus callback in HSBHS platform code for this
> purpose.

I guess that callback is coming handy now. But I wonder how to control
the VBUS signal? You may now this already, but in some cases the PFC
tables need to be adjusted to allow GPIO control of a certain
function. Please see this patch for r8a7790, maybe something similar
is needed for r8a7791?

http://www.spinics.net/lists/linux-sh/msg28034.html

>>>> Would it be possible for you to break out USB PCI support for USB1 and
>>>> resend that so we can begin by merging that?
>>>
>>> Wouldn't you like me also add USBHS in gadget mode for USB0 with the
>>> similar
>>> check like you did on lager (with ID pin),
>>> since it does not need the gpio-rcar changes too.
>>
>> Thanks for your offer. Yes, that would be nice. May I suggest doing it
>> on two levels:
>> 1) Gadget-only support (is it possible to perform runtime check of ID
>> pin value at insert event and give error in case of Host?)
>> 2) Incremental USBHS host patch
>>
>> Using two incremental patches like above we can begin by merging 1)
>> and keep on working on 2).
>>
>>> Also if you'd like I can add the USBHS in Host mode with the ID pin check
>>> like you suggested, but the usb0 host will not be stable.
>>> Probably this could speed up the USBHS Host development/fixing.
>>
>> Please add it as a separate incremental patch if possible. I'd like to
>> have some kind of stable level of support without any funky
>> dependencies as a first goal, then keep on trying to get USBHS
>> working.
>>
>> I think PCI USB for the micro USB port can simply be dropped now and
>> only use USBHS.
>
> Sure, will do.

Great, thanks for your help!

/ magnus
--
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
diff mbox

Patch

Index: build/arch/arm/mach-shmobile/board-koelsch.c
===================================================================
--- build.orig/arch/arm/mach-shmobile/board-koelsch.c	2014-02-23 21:47:44.510571967 +0400
+++ build/arch/arm/mach-shmobile/board-koelsch.c	2014-02-23 21:47:59.358571662 +0400
@@ -36,12 +36,15 @@ 
 #include <linux/pinctrl/machine.h>
 #include <linux/platform_data/gpio-rcar.h>
 #include <linux/platform_data/rcar-du.h>
+#include <linux/platform_data/usb-rcar-gen2-phy.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/gpio-regulator.h>
 #include <linux/regulator/machine.h>
 #include <linux/sh_eth.h>
+#include <linux/usb/phy.h>
+#include <linux/usb/renesas_usbhs.h>
 #include <linux/spi/flash.h>
 #include <linux/spi/rspi.h>
 #include <linux/spi/spi.h>
@@ -367,6 +370,177 @@ 
 	DEFINE_RES_IRQ(gic_spi(168)),
 };
 
+/* USBHS */
+static const struct resource usbhs_resources[] __initconst = {
+	DEFINE_RES_MEM(0xe6590000, 0x100),
+	DEFINE_RES_IRQ(gic_spi(107)),
+};
+
+struct usbhs_private {
+	struct renesas_usbhs_platform_info info;
+	struct usb_phy *phy;
+};
+
+#define usbhs_get_priv(pdev) \
+	container_of(renesas_usbhs_get_info(pdev), struct usbhs_private, info)
+
+static int usbhs_power_ctrl(struct platform_device *pdev,
+				void __iomem *base, int enable)
+{
+	struct usbhs_private *priv = usbhs_get_priv(pdev);
+
+	if (!priv->phy)
+		return -ENODEV;
+
+	if (enable) {
+		int retval = usb_phy_init(priv->phy);
+
+		if (!retval)
+			retval = usb_phy_set_suspend(priv->phy, 0);
+		return retval;
+	}
+
+	usb_phy_set_suspend(priv->phy, 1);
+	usb_phy_shutdown(priv->phy);
+	return 0;
+}
+
+static int usbhs_hardware_init(struct platform_device *pdev)
+{
+	struct usbhs_private *priv = usbhs_get_priv(pdev);
+	struct usb_phy *phy;
+
+	phy = usb_get_phy_dev(&pdev->dev, 0);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	priv->phy = phy;
+	return 0;
+}
+
+static int usbhs_hardware_exit(struct platform_device *pdev)
+{
+	struct usbhs_private *priv = usbhs_get_priv(pdev);
+
+	if (!priv->phy)
+		return 0;
+
+	usb_put_phy(priv->phy);
+	priv->phy = NULL;
+	return 0;
+}
+
+static int usbhs_get_id(struct platform_device *pdev)
+{
+	return USBHS_GADGET;
+}
+
+static u32 koelsch_usbhs_pipe_type[] = {
+	USB_ENDPOINT_XFER_CONTROL,
+	USB_ENDPOINT_XFER_ISOC,
+	USB_ENDPOINT_XFER_ISOC,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_INT,
+	USB_ENDPOINT_XFER_INT,
+	USB_ENDPOINT_XFER_INT,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+	USB_ENDPOINT_XFER_BULK,
+};
+
+static struct usbhs_private usbhs_priv __initdata = {
+	.info = {
+		.platform_callback = {
+			.power_ctrl	= usbhs_power_ctrl,
+			.hardware_init	= usbhs_hardware_init,
+			.hardware_exit	= usbhs_hardware_exit,
+			.get_id		= usbhs_get_id,
+		},
+		.driver_param = {
+			.buswait_bwait	= 4,
+			.pipe_type = koelsch_usbhs_pipe_type,
+			.pipe_size = ARRAY_SIZE(koelsch_usbhs_pipe_type),
+		},
+	}
+};
+
+static void __init koelsch_add_usb0_gadget(void)
+{
+	usb_bind_phy("renesas_usbhs", 0, "usb_phy_rcar_gen2");
+	platform_device_register_resndata(&platform_bus,
+					  "renesas_usbhs", -1,
+					  usbhs_resources,
+					  ARRAY_SIZE(usbhs_resources),
+					  &usbhs_priv.info,
+					  sizeof(usbhs_priv.info));
+}
+
+/* Internal PCI0 */
+static const struct resource pci0_resources[] __initconst = {
+	DEFINE_RES_MEM(0xee090000, 0x10000),	/* CFG */
+	DEFINE_RES_MEM(0xee080000, 0x10000),	/* MEM */
+	DEFINE_RES_IRQ(gic_spi(108)),
+};
+
+static void __init koelsch_add_usb0_host(void)
+{
+	usb_bind_phy("0000:00:01.0", 0, "usb_phy_rcar_gen2");
+	usb_bind_phy("0000:00:02.0", 0, "usb_phy_rcar_gen2");
+	platform_device_register_simple("pci-rcar-gen2",
+					0, pci0_resources,
+					ARRAY_SIZE(pci0_resources));
+}
+
+/* Internal PCI1 */
+static const struct resource pci1_resources[] __initconst = {
+	DEFINE_RES_MEM(0xee0d0000, 0x10000),	/* CFG */
+	DEFINE_RES_MEM(0xee0c0000, 0x10000),	/* MEM */
+	DEFINE_RES_IRQ(gic_spi(113)),
+};
+
+static void __init koelsch_add_usb1_host(void)
+{
+	platform_device_register_simple("pci-rcar-gen2",
+					1, pci1_resources,
+					ARRAY_SIZE(pci1_resources));
+}
+
+/* USBHS PHY */
+static struct rcar_gen2_phy_platform_data usbhs_phy_pdata = {
+	.chan2_pci = 1,	/* Channel 2 is PCI USB host */
+};
+
+static const struct resource usbhs_phy_resources[] __initconst = {
+	DEFINE_RES_MEM(0xe6590100, 0x100),
+};
+
+/* Add all available USB devices */
+static void __init koelsch_add_usb_devices(void)
+{
+	/* MAX3355E ID pin */
+	gpio_request_one(RCAR_GP_PIN(5, 31), GPIOF_IN, NULL);
+	if (!gpio_get_value(RCAR_GP_PIN(5, 31))) {
+		usbhs_phy_pdata.chan0_pci = 1; /* Channel 0 is PCI USB host */
+		koelsch_add_usb0_host();
+	} else {
+		usbhs_phy_pdata.chan0_pci = 0; /* Channel 0 is USBHS */
+		koelsch_add_usb0_gadget();
+	}
+
+	platform_device_register_resndata(&platform_bus, "usb_phy_rcar_gen2",
+					  -1, usbhs_phy_resources,
+					  ARRAY_SIZE(usbhs_phy_resources),
+					  &usbhs_phy_pdata,
+					  sizeof(usbhs_phy_pdata));
+	koelsch_add_usb1_host();
+}
+
 static const struct pinctrl_map koelsch_pinctrl_map[] = {
 	/* DU */
 	PIN_MAP_MUX_GROUP_DEFAULT("rcar-du-r8a7791", "pfc-r8a7791",
@@ -429,6 +603,14 @@ 
 				  "sdhi2_ctrl", "sdhi2"),
 	PIN_MAP_MUX_GROUP_DEFAULT("sh_mobile_sdhi.2", "pfc-r8a7791",
 				  "sdhi2_cd", "sdhi2"),
+	/* USB0 */
+	PIN_MAP_MUX_GROUP_DEFAULT("renesas_usbhs", "pfc-r8a7791",
+				  "usb0", "usb0"),
+	PIN_MAP_MUX_GROUP_DEFAULT("pci-rcar-gen2.0", "pfc-r8a7791",
+				  "usb0", "usb0"),
+	/* USB1 */
+	PIN_MAP_MUX_GROUP_DEFAULT("pci-rcar-gen2.1", "pfc-r8a7791",
+				  "usb1", "usb1"),
 };
 
 static void __init koelsch_add_standard_devices(void)
@@ -485,6 +667,7 @@ 
 					  sdhi2_resources, ARRAY_SIZE(sdhi2_resources),
 					  &sdhi2_info, sizeof(struct sh_mobile_sdhi_info));
 
+	koelsch_add_usb_devices();
 }
 
 /*