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

2012-12-20 Thread Doug Anderson
On Wed, Dec 19, 2012 at 10:37 PM, Vivek Gautam
 wrote:
>
> On Thu, Dec 20, 2012 at 5:00 AM, Doug Anderson  wrote:
>>
>> On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam  
>> 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(>dev, USB_PHY_TYPE_USB2);
>>> +   if (IS_ERR_OR_NULL(phy)) {
>>> +   /* Fallback to pdata */
>>> +   if (!pdata) {
>>> +   dev_err(>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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-12-20 Thread Doug Anderson
On Wed, Dec 19, 2012 at 10:37 PM, Vivek Gautam
gautamvivek1...@gmail.com wrote:

 On Thu, Dec 20, 2012 at 5:00 AM, Doug Anderson diand...@chromium.org wrote:

 On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam gautam.vi...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-12-19 Thread Vivek Gautam
Hi Doug,


On Thu, Dec 20, 2012 at 5:00 AM, Doug Anderson  wrote:
> Vivek,
>
> Again, not a high-level review, but...
>
Thanks for reviewing. :-)

>
> On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam  
> wrote:
>> Adding the phy driver to ehci-s5p. Keeping the platform data
>> for continuing the smooth operation for boards which still uses it
>>
>> Signed-off-by: Vivek Gautam 
>> Acked-by: Jingoo Han 
>> ---
>>  drivers/usb/host/ehci-s5p.c |   70 
>> ++-
>>  1 files changed, 49 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
>> index 46ca5ef..50c93af 100644
>> --- a/drivers/usb/host/ehci-s5p.c
>> +++ b/drivers/usb/host/ehci-s5p.c
>> @@ -17,6 +17,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>
>> @@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
>> struct device *dev;
>> struct usb_hcd *hcd;
>> struct clk *clk;
>> +   struct usb_phy *phy;
>> +   struct s5p_ehci_platdata *pdata;
>>  };
>>
>>  static const struct hc_driver s5p_ehci_hc_driver = {
>> @@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
>> .clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
>>  };
>>
>> +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.

>> +   usb_phy_init(s5p_ehci->phy);
>> +   } else if (s5p_ehci->pdata->phy_init) {
>> +   s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>> +   }
>> +}
>> +
>> +static void s5p_ehci_phy_disable(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);
>
> ...and why set it to host here?
>
Same here...

>> +   usb_phy_shutdown(s5p_ehci->phy);
>> +   } else if (s5p_ehci->pdata->phy_exit) {
>> +   s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>> +   }
>> +}
>> +
>>  static void s5p_setup_vbus_gpio(struct platform_device *pdev)
>>  {
>> int err;
>> @@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
>>
>>  static int s5p_ehci_probe(struct platform_device *pdev)
>>  {
>> -   struct s5p_ehci_platdata *pdata;
>> +   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
>> struct s5p_ehci_hcd *s5p_ehci;
>> struct usb_hcd *hcd;
>> struct ehci_hcd *ehci;
>> struct resource *res;
>> +   struct usb_phy *phy;
>> int irq;
>> int err;
>>
>> -   pdata = pdev->dev.platform_data;
>> -   if (!pdata) {
>> -   dev_err(>dev, "No platform data defined\n");
>> -   return -EINVAL;
>> -   }
>> -
>> /*
>>  * Right now device-tree probed devices don't get dma_mask set.
>>  * Since shared usb code relies on it, set it here for now.
>> @@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
>> if (!s5p_ehci)
>> return -ENOMEM;
>>
>> +   phy = devm_usb_get_phy(>dev, USB_PHY_TYPE_USB2);
>> +   if (IS_ERR_OR_NULL(phy)) {
>> +   /* Fallback to pdata */
>> +   if (!pdata) {
>> +   dev_err(>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.

>
>> +   } else {
>> +   s5p_ehci->pdata = pdata;
>> +   }
>> +   } else {
>> +   s5p_ehci->phy = phy;
>> +   }
>> +
>> s5p_ehci->dev = >dev;
>>
>> hcd = usb_create_hcd(_ehci_hc_driver, >dev,
>> @@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
>> goto fail_io;
>> }
>>
>> -   if (pdata->phy_init)
>> -   pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>> +   s5p_ehci_phy_enable(s5p_ehci);
>>
>> ehci = hcd_to_ehci(hcd);
>> ehci->caps = hcd->regs;
>> @@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
>> err = usb_add_hcd(hcd, irq, IRQF_SHARED);
>> if (err) {
>>  

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

2012-12-19 Thread Doug Anderson
Vivek,

Again, not a high-level review, but...


On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam  wrote:
> Adding the phy driver to ehci-s5p. Keeping the platform data
> for continuing the smooth operation for boards which still uses it
>
> Signed-off-by: Vivek Gautam 
> Acked-by: Jingoo Han 
> ---
>  drivers/usb/host/ehci-s5p.c |   70 
> ++-
>  1 files changed, 49 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
> index 46ca5ef..50c93af 100644
> --- a/drivers/usb/host/ehci-s5p.c
> +++ b/drivers/usb/host/ehci-s5p.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
> struct device *dev;
> struct usb_hcd *hcd;
> struct clk *clk;
> +   struct usb_phy *phy;
> +   struct s5p_ehci_platdata *pdata;
>  };
>
>  static const struct hc_driver s5p_ehci_hc_driver = {
> @@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
> .clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
>  };
>
> +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?

> +   usb_phy_init(s5p_ehci->phy);
> +   } else if (s5p_ehci->pdata->phy_init) {
> +   s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
> +   }
> +}
> +
> +static void s5p_ehci_phy_disable(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);

...and why set it to host here?

> +   usb_phy_shutdown(s5p_ehci->phy);
> +   } else if (s5p_ehci->pdata->phy_exit) {
> +   s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
> +   }
> +}
> +
>  static void s5p_setup_vbus_gpio(struct platform_device *pdev)
>  {
> int err;
> @@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
>
>  static int s5p_ehci_probe(struct platform_device *pdev)
>  {
> -   struct s5p_ehci_platdata *pdata;
> +   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
> struct s5p_ehci_hcd *s5p_ehci;
> struct usb_hcd *hcd;
> struct ehci_hcd *ehci;
> struct resource *res;
> +   struct usb_phy *phy;
> int irq;
> int err;
>
> -   pdata = pdev->dev.platform_data;
> -   if (!pdata) {
> -   dev_err(>dev, "No platform data defined\n");
> -   return -EINVAL;
> -   }
> -
> /*
>  * Right now device-tree probed devices don't get dma_mask set.
>  * Since shared usb code relies on it, set it here for now.
> @@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> if (!s5p_ehci)
> return -ENOMEM;
>
> +   phy = devm_usb_get_phy(>dev, USB_PHY_TYPE_USB2);
> +   if (IS_ERR_OR_NULL(phy)) {
> +   /* Fallback to pdata */
> +   if (!pdata) {
> +   dev_err(>dev, "no platform data or transceiver 
> defined\n");
> +   return -EPROBE_DEFER;

Shouldn't you return -EINVAL like the old code did?

> +   } else {
> +   s5p_ehci->pdata = pdata;
> +   }
> +   } else {
> +   s5p_ehci->phy = phy;
> +   }
> +
> s5p_ehci->dev = >dev;
>
> hcd = usb_create_hcd(_ehci_hc_driver, >dev,
> @@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> goto fail_io;
> }
>
> -   if (pdata->phy_init)
> -   pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
> +   s5p_ehci_phy_enable(s5p_ehci);
>
> ehci = hcd_to_ehci(hcd);
> ehci->caps = hcd->regs;
> @@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> err = usb_add_hcd(hcd, irq, IRQF_SHARED);
> if (err) {
> dev_err(>dev, "Failed to add USB HCD\n");
> -   goto fail_io;
> +   goto fail_add_hcd;
> }
>
> platform_set_drvdata(pdev, s5p_ehci);
>
> return 0;
>
> +fail_add_hcd:
> +   s5p_ehci_phy_disable(s5p_ehci);
>  fail_io:
> clk_disable_unprepare(s5p_ehci->clk);
>  fail_clk:
> @@ -192,14 +228,12 @@ fail_clk:
>
>  static int s5p_ehci_remove(struct platform_device *pdev)
>  {
> -   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
> struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
> struct usb_hcd *hcd = s5p_ehci->hcd;
>
> usb_remove_hcd(hcd);
>
> -   if (pdata && pdata->phy_exit)
> -   

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

2012-12-19 Thread Doug Anderson
Vivek,

Again, not a high-level review, but...


On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam gautam.vi...@samsung.com wrote:
 Adding the phy driver to ehci-s5p. Keeping the platform data
 for continuing the smooth operation for boards which still uses it

 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 Acked-by: Jingoo Han jg1@samsung.com
 ---
  drivers/usb/host/ehci-s5p.c |   70 
 ++-
  1 files changed, 49 insertions(+), 21 deletions(-)

 diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
 index 46ca5ef..50c93af 100644
 --- a/drivers/usb/host/ehci-s5p.c
 +++ b/drivers/usb/host/ehci-s5p.c
 @@ -17,6 +17,7 @@
  #include linux/platform_device.h
  #include linux/of_gpio.h
  #include linux/platform_data/usb-ehci-s5p.h
 +#include linux/usb/phy.h
  #include linux/usb/samsung_usb_phy.h
  #include plat/usb-phy.h

 @@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
 struct device *dev;
 struct usb_hcd *hcd;
 struct clk *clk;
 +   struct usb_phy *phy;
 +   struct s5p_ehci_platdata *pdata;
  };

  static const struct hc_driver s5p_ehci_hc_driver = {
 @@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
 .clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
  };

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

 +   usb_phy_init(s5p_ehci-phy);
 +   } else if (s5p_ehci-pdata-phy_init) {
 +   s5p_ehci-pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
 +   }
 +}
 +
 +static void s5p_ehci_phy_disable(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);

...and why set it to host here?

 +   usb_phy_shutdown(s5p_ehci-phy);
 +   } else if (s5p_ehci-pdata-phy_exit) {
 +   s5p_ehci-pdata-phy_exit(pdev, USB_PHY_TYPE_HOST);
 +   }
 +}
 +
  static void s5p_setup_vbus_gpio(struct platform_device *pdev)
  {
 int err;
 @@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);

  static int s5p_ehci_probe(struct platform_device *pdev)
  {
 -   struct s5p_ehci_platdata *pdata;
 +   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
 struct s5p_ehci_hcd *s5p_ehci;
 struct usb_hcd *hcd;
 struct ehci_hcd *ehci;
 struct resource *res;
 +   struct usb_phy *phy;
 int irq;
 int err;

 -   pdata = pdev-dev.platform_data;
 -   if (!pdata) {
 -   dev_err(pdev-dev, No platform data defined\n);
 -   return -EINVAL;
 -   }
 -
 /*
  * Right now device-tree probed devices don't get dma_mask set.
  * Since shared usb code relies on it, set it here for now.
 @@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 if (!s5p_ehci)
 return -ENOMEM;

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

 +   } else {
 +   s5p_ehci-pdata = pdata;
 +   }
 +   } else {
 +   s5p_ehci-phy = phy;
 +   }
 +
 s5p_ehci-dev = pdev-dev;

 hcd = usb_create_hcd(s5p_ehci_hc_driver, pdev-dev,
 @@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 goto fail_io;
 }

 -   if (pdata-phy_init)
 -   pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
 +   s5p_ehci_phy_enable(s5p_ehci);

 ehci = hcd_to_ehci(hcd);
 ehci-caps = hcd-regs;
 @@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 if (err) {
 dev_err(pdev-dev, Failed to add USB HCD\n);
 -   goto fail_io;
 +   goto fail_add_hcd;
 }

 platform_set_drvdata(pdev, s5p_ehci);

 return 0;

 +fail_add_hcd:
 +   s5p_ehci_phy_disable(s5p_ehci);
  fail_io:
 clk_disable_unprepare(s5p_ehci-clk);
  fail_clk:
 @@ -192,14 +228,12 @@ fail_clk:

  static int s5p_ehci_remove(struct platform_device *pdev)
  {
 -   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
 struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
 struct usb_hcd *hcd = s5p_ehci-hcd;

 usb_remove_hcd(hcd);

 -   if (pdata  

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

2012-12-19 Thread Vivek Gautam
Hi Doug,


On Thu, Dec 20, 2012 at 5:00 AM, Doug Anderson diand...@chromium.org wrote:
 Vivek,

 Again, not a high-level review, but...

Thanks for reviewing. :-)


 On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam gautam.vi...@samsung.com 
 wrote:
 Adding the phy driver to ehci-s5p. Keeping the platform data
 for continuing the smooth operation for boards which still uses it

 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 Acked-by: Jingoo Han jg1@samsung.com
 ---
  drivers/usb/host/ehci-s5p.c |   70 
 ++-
  1 files changed, 49 insertions(+), 21 deletions(-)

 diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
 index 46ca5ef..50c93af 100644
 --- a/drivers/usb/host/ehci-s5p.c
 +++ b/drivers/usb/host/ehci-s5p.c
 @@ -17,6 +17,7 @@
  #include linux/platform_device.h
  #include linux/of_gpio.h
  #include linux/platform_data/usb-ehci-s5p.h
 +#include linux/usb/phy.h
  #include linux/usb/samsung_usb_phy.h
  #include plat/usb-phy.h

 @@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
 struct device *dev;
 struct usb_hcd *hcd;
 struct clk *clk;
 +   struct usb_phy *phy;
 +   struct s5p_ehci_platdata *pdata;
  };

  static const struct hc_driver s5p_ehci_hc_driver = {
 @@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
 .clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
  };

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

 +   usb_phy_init(s5p_ehci-phy);
 +   } else if (s5p_ehci-pdata-phy_init) {
 +   s5p_ehci-pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
 +   }
 +}
 +
 +static void s5p_ehci_phy_disable(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);

 ...and why set it to host here?

Same here...

 +   usb_phy_shutdown(s5p_ehci-phy);
 +   } else if (s5p_ehci-pdata-phy_exit) {
 +   s5p_ehci-pdata-phy_exit(pdev, USB_PHY_TYPE_HOST);
 +   }
 +}
 +
  static void s5p_setup_vbus_gpio(struct platform_device *pdev)
  {
 int err;
 @@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);

  static int s5p_ehci_probe(struct platform_device *pdev)
  {
 -   struct s5p_ehci_platdata *pdata;
 +   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
 struct s5p_ehci_hcd *s5p_ehci;
 struct usb_hcd *hcd;
 struct ehci_hcd *ehci;
 struct resource *res;
 +   struct usb_phy *phy;
 int irq;
 int err;

 -   pdata = pdev-dev.platform_data;
 -   if (!pdata) {
 -   dev_err(pdev-dev, No platform data defined\n);
 -   return -EINVAL;
 -   }
 -
 /*
  * Right now device-tree probed devices don't get dma_mask set.
  * Since shared usb code relies on it, set it here for now.
 @@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 if (!s5p_ehci)
 return -ENOMEM;

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


 +   } else {
 +   s5p_ehci-pdata = pdata;
 +   }
 +   } else {
 +   s5p_ehci-phy = phy;
 +   }
 +
 s5p_ehci-dev = pdev-dev;

 hcd = usb_create_hcd(s5p_ehci_hc_driver, pdev-dev,
 @@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 goto fail_io;
 }

 -   if (pdata-phy_init)
 -   pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
 +   s5p_ehci_phy_enable(s5p_ehci);

 ehci = hcd_to_ehci(hcd);
 ehci-caps = hcd-regs;
 @@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 if (err) {
 dev_err(pdev-dev, Failed to add USB 

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

2012-12-18 Thread Vivek Gautam
CC: Doug Anderson.


On Tue, Dec 18, 2012 at 8:13 PM, Vivek Gautam  wrote:
> Adding the phy driver to ehci-s5p. Keeping the platform data
> for continuing the smooth operation for boards which still uses it
>
> Signed-off-by: Vivek Gautam 
> Acked-by: Jingoo Han 
> ---
>  drivers/usb/host/ehci-s5p.c |   70 
> ++-
>  1 files changed, 49 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
> index 46ca5ef..50c93af 100644
> --- a/drivers/usb/host/ehci-s5p.c
> +++ b/drivers/usb/host/ehci-s5p.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
> struct device *dev;
> struct usb_hcd *hcd;
> struct clk *clk;
> +   struct usb_phy *phy;
> +   struct s5p_ehci_platdata *pdata;
>  };
>
>  static const struct hc_driver s5p_ehci_hc_driver = {
> @@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
> .clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
>  };
>
> +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);
> +   usb_phy_init(s5p_ehci->phy);
> +   } else if (s5p_ehci->pdata->phy_init) {
> +   s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
> +   }
> +}
> +
> +static void s5p_ehci_phy_disable(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);
> +   usb_phy_shutdown(s5p_ehci->phy);
> +   } else if (s5p_ehci->pdata->phy_exit) {
> +   s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
> +   }
> +}
> +
>  static void s5p_setup_vbus_gpio(struct platform_device *pdev)
>  {
> int err;
> @@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
>
>  static int s5p_ehci_probe(struct platform_device *pdev)
>  {
> -   struct s5p_ehci_platdata *pdata;
> +   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
> struct s5p_ehci_hcd *s5p_ehci;
> struct usb_hcd *hcd;
> struct ehci_hcd *ehci;
> struct resource *res;
> +   struct usb_phy *phy;
> int irq;
> int err;
>
> -   pdata = pdev->dev.platform_data;
> -   if (!pdata) {
> -   dev_err(>dev, "No platform data defined\n");
> -   return -EINVAL;
> -   }
> -
> /*
>  * Right now device-tree probed devices don't get dma_mask set.
>  * Since shared usb code relies on it, set it here for now.
> @@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> if (!s5p_ehci)
> return -ENOMEM;
>
> +   phy = devm_usb_get_phy(>dev, USB_PHY_TYPE_USB2);
> +   if (IS_ERR_OR_NULL(phy)) {
> +   /* Fallback to pdata */
> +   if (!pdata) {
> +   dev_err(>dev, "no platform data or transceiver 
> defined\n");
> +   return -EPROBE_DEFER;
> +   } else {
> +   s5p_ehci->pdata = pdata;
> +   }
> +   } else {
> +   s5p_ehci->phy = phy;
> +   }
> +
> s5p_ehci->dev = >dev;
>
> hcd = usb_create_hcd(_ehci_hc_driver, >dev,
> @@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> goto fail_io;
> }
>
> -   if (pdata->phy_init)
> -   pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
> +   s5p_ehci_phy_enable(s5p_ehci);
>
> ehci = hcd_to_ehci(hcd);
> ehci->caps = hcd->regs;
> @@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> err = usb_add_hcd(hcd, irq, IRQF_SHARED);
> if (err) {
> dev_err(>dev, "Failed to add USB HCD\n");
> -   goto fail_io;
> +   goto fail_add_hcd;
> }
>
> platform_set_drvdata(pdev, s5p_ehci);
>
> return 0;
>
> +fail_add_hcd:
> +   s5p_ehci_phy_disable(s5p_ehci);
>  fail_io:
> clk_disable_unprepare(s5p_ehci->clk);
>  fail_clk:
> @@ -192,14 +228,12 @@ fail_clk:
>
>  static int s5p_ehci_remove(struct platform_device *pdev)
>  {
> -   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
> struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
> struct usb_hcd *hcd = s5p_ehci->hcd;
>
> usb_remove_hcd(hcd);
>
> -   if (pdata && pdata->phy_exit)
> -   pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
> +   s5p_ehci_phy_disable(s5p_ehci);
>
> clk_disable_unprepare(s5p_ehci->clk);
>
> @@ -223,14 +257,11 @@ static int 

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

2012-12-18 Thread Vivek Gautam
Adding the phy driver to ehci-s5p. Keeping the platform data
for continuing the smooth operation for boards which still uses it

Signed-off-by: Vivek Gautam 
Acked-by: Jingoo Han 
---
 drivers/usb/host/ehci-s5p.c |   70 ++-
 1 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 46ca5ef..50c93af 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
struct device *dev;
struct usb_hcd *hcd;
struct clk *clk;
+   struct usb_phy *phy;
+   struct s5p_ehci_platdata *pdata;
 };
 
 static const struct hc_driver s5p_ehci_hc_driver = {
@@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
.clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
 };
 
+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);
+   usb_phy_init(s5p_ehci->phy);
+   } else if (s5p_ehci->pdata->phy_init) {
+   s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
+   }
+}
+
+static void s5p_ehci_phy_disable(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);
+   usb_phy_shutdown(s5p_ehci->phy);
+   } else if (s5p_ehci->pdata->phy_exit) {
+   s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
+   }
+}
+
 static void s5p_setup_vbus_gpio(struct platform_device *pdev)
 {
int err;
@@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
 
 static int s5p_ehci_probe(struct platform_device *pdev)
 {
-   struct s5p_ehci_platdata *pdata;
+   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
struct s5p_ehci_hcd *s5p_ehci;
struct usb_hcd *hcd;
struct ehci_hcd *ehci;
struct resource *res;
+   struct usb_phy *phy;
int irq;
int err;
 
-   pdata = pdev->dev.platform_data;
-   if (!pdata) {
-   dev_err(>dev, "No platform data defined\n");
-   return -EINVAL;
-   }
-
/*
 * Right now device-tree probed devices don't get dma_mask set.
 * Since shared usb code relies on it, set it here for now.
@@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
if (!s5p_ehci)
return -ENOMEM;
 
+   phy = devm_usb_get_phy(>dev, USB_PHY_TYPE_USB2);
+   if (IS_ERR_OR_NULL(phy)) {
+   /* Fallback to pdata */
+   if (!pdata) {
+   dev_err(>dev, "no platform data or transceiver 
defined\n");
+   return -EPROBE_DEFER;
+   } else {
+   s5p_ehci->pdata = pdata;
+   }
+   } else {
+   s5p_ehci->phy = phy;
+   }
+
s5p_ehci->dev = >dev;
 
hcd = usb_create_hcd(_ehci_hc_driver, >dev,
@@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
goto fail_io;
}
 
-   if (pdata->phy_init)
-   pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
+   s5p_ehci_phy_enable(s5p_ehci);
 
ehci = hcd_to_ehci(hcd);
ehci->caps = hcd->regs;
@@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err) {
dev_err(>dev, "Failed to add USB HCD\n");
-   goto fail_io;
+   goto fail_add_hcd;
}
 
platform_set_drvdata(pdev, s5p_ehci);
 
return 0;
 
+fail_add_hcd:
+   s5p_ehci_phy_disable(s5p_ehci);
 fail_io:
clk_disable_unprepare(s5p_ehci->clk);
 fail_clk:
@@ -192,14 +228,12 @@ fail_clk:
 
 static int s5p_ehci_remove(struct platform_device *pdev)
 {
-   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
struct usb_hcd *hcd = s5p_ehci->hcd;
 
usb_remove_hcd(hcd);
 
-   if (pdata && pdata->phy_exit)
-   pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
+   s5p_ehci_phy_disable(s5p_ehci);
 
clk_disable_unprepare(s5p_ehci->clk);
 
@@ -223,14 +257,11 @@ static int s5p_ehci_suspend(struct device *dev)
struct s5p_ehci_hcd *s5p_ehci = dev_get_drvdata(dev);
struct usb_hcd *hcd = s5p_ehci->hcd;
bool do_wakeup = device_may_wakeup(dev);
-   struct platform_device *pdev = to_platform_device(dev);
-   struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
int rc;
 
rc = 

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

2012-12-18 Thread Vivek Gautam
Adding the phy driver to ehci-s5p. Keeping the platform data
for continuing the smooth operation for boards which still uses it

Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Acked-by: Jingoo Han jg1@samsung.com
---
 drivers/usb/host/ehci-s5p.c |   70 ++-
 1 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 46ca5ef..50c93af 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -17,6 +17,7 @@
 #include linux/platform_device.h
 #include linux/of_gpio.h
 #include linux/platform_data/usb-ehci-s5p.h
+#include linux/usb/phy.h
 #include linux/usb/samsung_usb_phy.h
 #include plat/usb-phy.h
 
@@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
struct device *dev;
struct usb_hcd *hcd;
struct clk *clk;
+   struct usb_phy *phy;
+   struct s5p_ehci_platdata *pdata;
 };
 
 static const struct hc_driver s5p_ehci_hc_driver = {
@@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
.clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
 };
 
+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);
+   usb_phy_init(s5p_ehci-phy);
+   } else if (s5p_ehci-pdata-phy_init) {
+   s5p_ehci-pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
+   }
+}
+
+static void s5p_ehci_phy_disable(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);
+   usb_phy_shutdown(s5p_ehci-phy);
+   } else if (s5p_ehci-pdata-phy_exit) {
+   s5p_ehci-pdata-phy_exit(pdev, USB_PHY_TYPE_HOST);
+   }
+}
+
 static void s5p_setup_vbus_gpio(struct platform_device *pdev)
 {
int err;
@@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
 
 static int s5p_ehci_probe(struct platform_device *pdev)
 {
-   struct s5p_ehci_platdata *pdata;
+   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
struct s5p_ehci_hcd *s5p_ehci;
struct usb_hcd *hcd;
struct ehci_hcd *ehci;
struct resource *res;
+   struct usb_phy *phy;
int irq;
int err;
 
-   pdata = pdev-dev.platform_data;
-   if (!pdata) {
-   dev_err(pdev-dev, No platform data defined\n);
-   return -EINVAL;
-   }
-
/*
 * Right now device-tree probed devices don't get dma_mask set.
 * Since shared usb code relies on it, set it here for now.
@@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
if (!s5p_ehci)
return -ENOMEM;
 
+   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;
+   } else {
+   s5p_ehci-pdata = pdata;
+   }
+   } else {
+   s5p_ehci-phy = phy;
+   }
+
s5p_ehci-dev = pdev-dev;
 
hcd = usb_create_hcd(s5p_ehci_hc_driver, pdev-dev,
@@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
goto fail_io;
}
 
-   if (pdata-phy_init)
-   pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
+   s5p_ehci_phy_enable(s5p_ehci);
 
ehci = hcd_to_ehci(hcd);
ehci-caps = hcd-regs;
@@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err) {
dev_err(pdev-dev, Failed to add USB HCD\n);
-   goto fail_io;
+   goto fail_add_hcd;
}
 
platform_set_drvdata(pdev, s5p_ehci);
 
return 0;
 
+fail_add_hcd:
+   s5p_ehci_phy_disable(s5p_ehci);
 fail_io:
clk_disable_unprepare(s5p_ehci-clk);
 fail_clk:
@@ -192,14 +228,12 @@ fail_clk:
 
 static int s5p_ehci_remove(struct platform_device *pdev)
 {
-   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
struct usb_hcd *hcd = s5p_ehci-hcd;
 
usb_remove_hcd(hcd);
 
-   if (pdata  pdata-phy_exit)
-   pdata-phy_exit(pdev, USB_PHY_TYPE_HOST);
+   s5p_ehci_phy_disable(s5p_ehci);
 
clk_disable_unprepare(s5p_ehci-clk);
 
@@ -223,14 +257,11 @@ static int s5p_ehci_suspend(struct device *dev)
struct s5p_ehci_hcd *s5p_ehci = dev_get_drvdata(dev);
struct usb_hcd *hcd = s5p_ehci-hcd;
bool do_wakeup = device_may_wakeup(dev);
-  

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

2012-12-18 Thread Vivek Gautam
CC: Doug Anderson.


On Tue, Dec 18, 2012 at 8:13 PM, Vivek Gautam gautam.vi...@samsung.com wrote:
 Adding the phy driver to ehci-s5p. Keeping the platform data
 for continuing the smooth operation for boards which still uses it

 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 Acked-by: Jingoo Han jg1@samsung.com
 ---
  drivers/usb/host/ehci-s5p.c |   70 
 ++-
  1 files changed, 49 insertions(+), 21 deletions(-)

 diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
 index 46ca5ef..50c93af 100644
 --- a/drivers/usb/host/ehci-s5p.c
 +++ b/drivers/usb/host/ehci-s5p.c
 @@ -17,6 +17,7 @@
  #include linux/platform_device.h
  #include linux/of_gpio.h
  #include linux/platform_data/usb-ehci-s5p.h
 +#include linux/usb/phy.h
  #include linux/usb/samsung_usb_phy.h
  #include plat/usb-phy.h

 @@ -33,6 +34,8 @@ struct s5p_ehci_hcd {
 struct device *dev;
 struct usb_hcd *hcd;
 struct clk *clk;
 +   struct usb_phy *phy;
 +   struct s5p_ehci_platdata *pdata;
  };

  static const struct hc_driver s5p_ehci_hc_driver = {
 @@ -66,6 +69,30 @@ static const struct hc_driver s5p_ehci_hc_driver = {
 .clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
  };

 +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);
 +   usb_phy_init(s5p_ehci-phy);
 +   } else if (s5p_ehci-pdata-phy_init) {
 +   s5p_ehci-pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
 +   }
 +}
 +
 +static void s5p_ehci_phy_disable(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);
 +   usb_phy_shutdown(s5p_ehci-phy);
 +   } else if (s5p_ehci-pdata-phy_exit) {
 +   s5p_ehci-pdata-phy_exit(pdev, USB_PHY_TYPE_HOST);
 +   }
 +}
 +
  static void s5p_setup_vbus_gpio(struct platform_device *pdev)
  {
 int err;
 @@ -88,20 +115,15 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);

  static int s5p_ehci_probe(struct platform_device *pdev)
  {
 -   struct s5p_ehci_platdata *pdata;
 +   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
 struct s5p_ehci_hcd *s5p_ehci;
 struct usb_hcd *hcd;
 struct ehci_hcd *ehci;
 struct resource *res;
 +   struct usb_phy *phy;
 int irq;
 int err;

 -   pdata = pdev-dev.platform_data;
 -   if (!pdata) {
 -   dev_err(pdev-dev, No platform data defined\n);
 -   return -EINVAL;
 -   }
 -
 /*
  * Right now device-tree probed devices don't get dma_mask set.
  * Since shared usb code relies on it, set it here for now.
 @@ -119,6 +141,19 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 if (!s5p_ehci)
 return -ENOMEM;

 +   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;
 +   } else {
 +   s5p_ehci-pdata = pdata;
 +   }
 +   } else {
 +   s5p_ehci-phy = phy;
 +   }
 +
 s5p_ehci-dev = pdev-dev;

 hcd = usb_create_hcd(s5p_ehci_hc_driver, pdev-dev,
 @@ -164,8 +199,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 goto fail_io;
 }

 -   if (pdata-phy_init)
 -   pdata-phy_init(pdev, USB_PHY_TYPE_HOST);
 +   s5p_ehci_phy_enable(s5p_ehci);

 ehci = hcd_to_ehci(hcd);
 ehci-caps = hcd-regs;
 @@ -176,13 +210,15 @@ static int s5p_ehci_probe(struct platform_device *pdev)
 err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 if (err) {
 dev_err(pdev-dev, Failed to add USB HCD\n);
 -   goto fail_io;
 +   goto fail_add_hcd;
 }

 platform_set_drvdata(pdev, s5p_ehci);

 return 0;

 +fail_add_hcd:
 +   s5p_ehci_phy_disable(s5p_ehci);
  fail_io:
 clk_disable_unprepare(s5p_ehci-clk);
  fail_clk:
 @@ -192,14 +228,12 @@ fail_clk:

  static int s5p_ehci_remove(struct platform_device *pdev)
  {
 -   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
 struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
 struct usb_hcd *hcd = s5p_ehci-hcd;

 usb_remove_hcd(hcd);

 -   if (pdata  pdata-phy_exit)
 -   pdata-phy_exit(pdev, USB_PHY_TYPE_HOST);
 +   s5p_ehci_phy_disable(s5p_ehci);

 clk_disable_unprepare(s5p_ehci-clk);

 @@ -223,14