lkml.org 
[lkml]   [2012]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v5 3/4] USB: ehci-s5p: Add phy driver support
    From
    On Wed, Dec 19, 2012 at 10:37 PM, Vivek Gautam
    <gautamvivek1987@gmail.com> wrote:
    >
    > On Thu, Dec 20, 2012 at 5:00 AM, Doug Anderson <dianders@chromium.org> wrote:
    >>
    >> On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
    >>> +static void s5p_ehci_phy_enable(struct s5p_ehci_hcd *s5p_ehci)
    >>> +{
    >>> + struct platform_device *pdev = to_platform_device(s5p_ehci->dev);
    >>> +
    >>> + if (s5p_ehci->phy) {
    >>> + samsung_usbphy_set_type(s5p_ehci->phy, USB_PHY_TYPE_HOST);
    >>
    >> This confuses me. Why are you setting the type to host here?
    >>
    > Being in host controller, before calling usb_phy_init() we set type to
    > Host since, with certain SOCs
    > like 4210, same register has different bit settings for HOST type and
    > device type. So setting this
    > to Host type here make the flow of usb_phy_init to go in the direction of Host.

    OK. I think I need to study the code more...

    >>>
    >>> + phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
    >>> + if (IS_ERR_OR_NULL(phy)) {
    >>> + /* Fallback to pdata */
    >>> + if (!pdata) {
    >>> + dev_err(&pdev->dev, "no platform data or transceiver defined\n");
    >>> + return -EPROBE_DEFER;
    >>
    >> Shouldn't you return -EINVAL like the old code did?
    >
    > We are deferring the probe since the usb-phy transceiver may get
    > probed after ehci/ohci controllers.
    > And if we return -EINVAL like the previous code, we would end up not
    > setting the phy.

    OK. Something is wrong here then, since this really isn't an error:
    * It should be commented about why you're deferring.
    * You shouldn't have a dev_err for something that's not fatal.

    Ideally we'd want something that would force probing to happen in the
    right order. I spent a little time looking and didn't see anything,
    but maybe I'm missing something obvious. If nothing pops out, the
    defer seems OK as long as it is commented well.


    \
     
     \ /
      Last update: 2012-12-20 19:41    [W:7.977 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site