Re: [PATCH v5 3/4] USB: ehci-s5p: Add phy driver support

From: Doug Anderson
Date: Thu Dec 20 2012 - 12:51:54 EST

On Wed, Dec 19, 2012 at 10:37 PM, Vivek Gautam
<gautamvivek1987@xxxxxxxxx> wrote:
> On Thu, Dec 20, 2012 at 5:00 AM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>> On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam <gautam.vivek@xxxxxxxxxxx> 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.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at