linux-kernel - Re: [PATCH v2] USB: ehci-s5p: Fix phy reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJuYYwRYp0dvf=veg1UVAj8rJ2CdpFy32LUE6zeq1sH9FbZ=KQ@mail.gmail.com>
Date:	Thu, 14 Mar 2013 09:49:11 +0530
From:	Thomas Abraham <thomas.abraham@...aro.org>
To:	Alexander Graf <agraf@...e.de>
Cc:	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, dmueller@...e.de,
	Vivek Gautam <gautam.vivek@...sung.com>,
	Jingoo Han <jg1.han@...sung.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Felipe Balbi <balbi@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Doug Anderson <dianders@...omium.org>
Subject: Re: [PATCH v2] USB: ehci-s5p: Fix phy reset

On 14 March 2013 05:29, Alexander Graf <agraf@...e.de> wrote:
> On my Exynos 5 based Arndale system, I need to pull the reset line down
> and then let it go up again to actually perform a reset. Without that
> reset, I can't find any USB hubs on my bus, rendering the USB controller
> useless.
>
> We also only need to reset the line after the phy node has been found.
> This way we don't accidently reserve the vbus GPIO pin, but later on
> defer the creation of our controller, because the phy device tree node
> hasn't been probed yet.
>
> This patch implements the above logic, making EHCI and OHCI work on
> Arndale systems for me.
>
> Signed-off-by: Alexander Graf <agraf@...e.de>
> CC: Vivek Gautam <gautam.vivek@...sung.com>
> CC: Jingoo Han <jg1.han@...sung.com>
> CC: Alan Stern <stern@...land.harvard.edu>
> CC: Kukjin Kim <kgene.kim@...sung.com>
> CC: Felipe Balbi <balbi@...com>
> CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> CC: Doug Anderson <dianders@...omium.org>
>
> ---
>
> v1 -> v2:
>
>   - remove gpio_free call
>   - move reset logic after phy node search
>
> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
> index 20ebf6a..b29b2b8 100644
> --- a/drivers/usb/host/ehci-s5p.c
> +++ b/drivers/usb/host/ehci-s5p.c
> @@ -103,9 +103,14 @@ static void s5p_setup_vbus_gpio(struct platform_device *pdev)
>         if (!gpio_is_valid(gpio))
>                 return;
>
> -       err = gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, "ehci_vbus_gpio");
> -       if (err)
> +       /* reset pulls the line down, then up again */
> +       err = gpio_request_one(gpio, GPIOF_OUT_INIT_LOW, "ehci_vbus_gpio");
> +       if (err) {
>                 dev_err(&pdev->dev, "can't request ehci vbus gpio %d", gpio);
> +               return;
> +       }
> +       mdelay(1);
> +       __gpio_set_value(gpio, 1);
>  }
>
>  static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
> @@ -131,8 +136,6 @@ static int s5p_ehci_probe(struct platform_device *pdev)
>         if (!pdev->dev.coherent_dma_mask)
>                 pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>
> -       s5p_setup_vbus_gpio(pdev);
> -
>         s5p_ehci = devm_kzalloc(&pdev->dev, sizeof(struct s5p_ehci_hcd),
>                                 GFP_KERNEL);
>         if (!s5p_ehci)
> @@ -152,6 +155,8 @@ static int s5p_ehci_probe(struct platform_device *pdev)
>                 s5p_ehci->otg = phy->otg;
>         }
>
> +       s5p_setup_vbus_gpio(pdev);
> +
>         s5p_ehci->dev = &pdev->dev;
>
>         hcd = usb_create_hcd(&s5p_ehci_hc_driver, &pdev->dev,

Hi Alexander,

This change, though it works for Exynos5250 based Arndale board, does
not actually seem correct. On Arndale board, the on-board 4-port usb
hub is self powered and hence the vbus 'enable' gpio line from
Exynos5250 SoC is instead used as a "reset" signal for the on-board
usb hub (and not as the vbus enable signal).

Whereas, the driver uses the gpio used in 's5p_setup_vbus_gpio'
function as just a mechanism to enable vbus for downstream devices. So
the driver should not be modified as above to handle the board
specific behavior.

Instead, what needs to be done is, remove the "samsung,vbus-gpio"
property from the usb2.0 node in dts files (this property is optional)
for Arndale board. Then, during the machine_init, perform the reset
sequencing as required.

Ideally, the reset sequencing for the on-board AX88760 usb hub should
have been handled in the driver for this device. I have not checked if
there is a driver for this in the kernel.

Thanks,
Thomas.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/