diff mbox

usb: tegra: moving phy driver into drivers directory

Message ID 1346146338-4996-1-git-send-email-vbyravarasu@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Venu Byravarasu Aug. 28, 2012, 9:32 a.m. UTC
In order to keep up with the USB driver files organization,
moving USB phy driver from mach-tegra to drivers/USB directory.

Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
---
 arch/arm/mach-tegra/Makefile                       |    1 -
 arch/arm/mach-tegra/devices.c                      |    8 +-------
 arch/arm/mach-tegra/devices.h                      |    2 --
 drivers/usb/host/ehci-tegra.c                      |    2 +-
 drivers/usb/phy/Makefile                           |    1 +
 .../usb_phy.c => drivers/usb/phy/tegra_usb_phy.c   |    4 +---
 .../usb_phy.h => drivers/usb/phy/tegra_usb_phy.h   |    2 --
 7 files changed, 4 insertions(+), 16 deletions(-)
 rename arch/arm/mach-tegra/usb_phy.c => drivers/usb/phy/tegra_usb_phy.c (99%)
 rename arch/arm/mach-tegra/include/mach/usb_phy.h => drivers/usb/phy/tegra_usb_phy.h (97%)

Comments

Felipe Balbi Aug. 28, 2012, 9:49 a.m. UTC | #1
Hi,

On Tue, Aug 28, 2012 at 03:02:18PM +0530, Venu Byravarasu wrote:
> In order to keep up with the USB driver files organization,
> moving USB phy driver from mach-tegra to drivers/USB directory.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> ---
>  arch/arm/mach-tegra/Makefile                       |    1 -
>  arch/arm/mach-tegra/devices.c                      |    8 +-------
>  arch/arm/mach-tegra/devices.h                      |    2 --
>  drivers/usb/host/ehci-tegra.c                      |    2 +-
>  drivers/usb/phy/Makefile                           |    1 +
>  .../usb_phy.c => drivers/usb/phy/tegra_usb_phy.c   |    4 +---
>  .../usb_phy.h => drivers/usb/phy/tegra_usb_phy.h   |    2 --
>  7 files changed, 4 insertions(+), 16 deletions(-)
>  rename arch/arm/mach-tegra/usb_phy.c => drivers/usb/phy/tegra_usb_phy.c (99%)
>  rename arch/arm/mach-tegra/include/mach/usb_phy.h => drivers/usb/phy/tegra_usb_phy.h (97%)
> 
> diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
> index b94858f..b542dac 100644
> --- a/arch/arm/mach-tegra/Makefile
> +++ b/arch/arm/mach-tegra/Makefile
> @@ -25,7 +25,6 @@ obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
>  obj-$(CONFIG_TEGRA_SYSTEM_DMA)		+= dma.o
>  obj-$(CONFIG_CPU_FREQ)                  += cpu-tegra.o
>  obj-$(CONFIG_TEGRA_PCI)			+= pcie.o
> -obj-$(CONFIG_USB_SUPPORT)		+= usb_phy.o
>  
>  obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-dt-tegra20.o
>  obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
> diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-tegra/devices.c
> index 61e9603..232d7e1 100644
> --- a/arch/arm/mach-tegra/devices.c
> +++ b/arch/arm/mach-tegra/devices.c
> @@ -26,7 +26,6 @@
>  #include <mach/irqs.h>
>  #include <mach/iomap.h>
>  #include <mach/dma.h>
> -#include <mach/usb_phy.h>
>  
>  #include "gpio-names.h"
>  #include "devices.h"
> @@ -438,11 +437,6 @@ static struct resource tegra_usb3_resources[] = {
>  	},
>  };
>  
> -struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
> -	.reset_gpio = -1,
> -	.clk = "cdev2",
> -};
> -
>  struct tegra_ehci_platform_data tegra_ehci1_pdata = {
>  	.operating_mode = TEGRA_USB_OTG,
>  	.power_down_on_bus_suspend = 1,
> @@ -450,7 +444,7 @@ struct tegra_ehci_platform_data tegra_ehci1_pdata = {
>  };
>  
>  struct tegra_ehci_platform_data tegra_ehci2_pdata = {
> -	.phy_config = &tegra_ehci2_ulpi_phy_config,
> +	.phy_config = NULL,
>  	.operating_mode = TEGRA_USB_HOST,
>  	.power_down_on_bus_suspend = 1,
>  	.vbus_gpio = -1,
> diff --git a/arch/arm/mach-tegra/devices.h b/arch/arm/mach-tegra/devices.h
> index 4f50527..6bac5e6 100644
> --- a/arch/arm/mach-tegra/devices.h
> +++ b/arch/arm/mach-tegra/devices.h
> @@ -22,8 +22,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/platform_data/tegra_usb.h>
>  
> -#include <mach/usb_phy.h>
> -
>  extern struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config;
>  
>  extern struct tegra_ehci_platform_data tegra_ehci1_pdata;
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 75eca42..a03e279 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -27,7 +27,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pm_runtime.h>
>  
> -#include <mach/usb_phy.h>
> +#include "../phy/tegra_usb_phy.h"
>  #include <mach/iomap.h>
>  
>  #define TEGRA_USB_DMA_ALIGN 32
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index eca095b..663164f 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -5,3 +5,4 @@
>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>  
>  obj-$(CONFIG_USB_ISP1301)		+= isp1301.o
> +obj-$(CONFIG_USB_EHCI_TEGRA)		+= tegra_usb_phy.o
> diff --git a/arch/arm/mach-tegra/usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
> similarity index 99%
> rename from arch/arm/mach-tegra/usb_phy.c
> rename to drivers/usb/phy/tegra_usb_phy.c
> index 022b33a..c856716 100644
> --- a/arch/arm/mach-tegra/usb_phy.c
> +++ b/drivers/usb/phy/tegra_usb_phy.c
> @@ -1,6 +1,4 @@
>  /*
> - * arch/arm/mach-tegra/usb_phy.c
> - *
>   * Copyright (C) 2010 Google, Inc.
>   *
>   * Author:
> @@ -31,7 +29,7 @@
>  #include <linux/usb/ulpi.h>
>  #include <asm/mach-types.h>
>  #include <mach/gpio-tegra.h>
> -#include <mach/usb_phy.h>
> +#include "tegra_usb_phy.h"
>  #include <mach/iomap.h>
>  
>  #define ULPI_VIEWPORT		0x170

NAK, needs to be converted to proper PHY driver (misnamed as
include/linux/usb/otg.h). You will need proper phy->init, phy->suspend,
phy->resume, etc callbacks.
Venu Byravarasu Aug. 28, 2012, 12:36 p.m. UTC | #2
> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@ti.com]
> Sent: Tuesday, August 28, 2012 3:19 PM
> To: Venu Byravarasu
> Cc: ccross@android.com; olof@lixom.net; swarren@wwwdotorg.org;
> linux@arm.linux.org.uk; stern@rowland.harvard.edu;
> gregkh@linuxfoundation.org; balbi@ti.com; linux-kernel@vger.kernel.org;
> linux-tegra@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> usb@vger.kernel.org
> Subject: Re: [PATCH] usb: tegra: moving phy driver into drivers directory
> 
> * PGP Signed by an unknown key
> 
> Hi,
> 
> On Tue, Aug 28, 2012 at 03:02:18PM +0530, Venu Byravarasu wrote:
> > In order to keep up with the USB driver files organization,
> > moving USB phy driver from mach-tegra to drivers/USB directory.
> >
> > Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/Makefile                       |    1 -
> >  arch/arm/mach-tegra/devices.c                      |    8 +-------
> >  arch/arm/mach-tegra/devices.h                      |    2 --
> >  drivers/usb/host/ehci-tegra.c                      |    2 +-
> >  drivers/usb/phy/Makefile                           |    1 +
> >  .../usb_phy.c => drivers/usb/phy/tegra_usb_phy.c   |    4 +---
> >  .../usb_phy.h => drivers/usb/phy/tegra_usb_phy.h   |    2 --
> >  7 files changed, 4 insertions(+), 16 deletions(-)
> >  rename arch/arm/mach-tegra/usb_phy.c =>
> drivers/usb/phy/tegra_usb_phy.c (99%)
> >  rename arch/arm/mach-tegra/include/mach/usb_phy.h =>
> drivers/usb/phy/tegra_usb_phy.h (97%)
> >
> > diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-
> tegra/Makefile
> > index b94858f..b542dac 100644
> > --- a/arch/arm/mach-tegra/Makefile
> > +++ b/arch/arm/mach-tegra/Makefile
> > @@ -25,7 +25,6 @@ obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
> >  obj-$(CONFIG_TEGRA_SYSTEM_DMA)		+= dma.o
> >  obj-$(CONFIG_CPU_FREQ)                  += cpu-tegra.o
> >  obj-$(CONFIG_TEGRA_PCI)			+= pcie.o
> > -obj-$(CONFIG_USB_SUPPORT)		+= usb_phy.o
> >
> >  obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-dt-
> tegra20.o
> >  obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-
> tegra30.o
> > diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-
> tegra/devices.c
> > index 61e9603..232d7e1 100644
> > --- a/arch/arm/mach-tegra/devices.c
> > +++ b/arch/arm/mach-tegra/devices.c
> > @@ -26,7 +26,6 @@
> >  #include <mach/irqs.h>
> >  #include <mach/iomap.h>
> >  #include <mach/dma.h>
> > -#include <mach/usb_phy.h>
> >
> >  #include "gpio-names.h"
> >  #include "devices.h"
> > @@ -438,11 +437,6 @@ static struct resource tegra_usb3_resources[] = {
> >  	},
> >  };
> >
> > -struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
> > -	.reset_gpio = -1,
> > -	.clk = "cdev2",
> > -};
> > -
> >  struct tegra_ehci_platform_data tegra_ehci1_pdata = {
> >  	.operating_mode = TEGRA_USB_OTG,
> >  	.power_down_on_bus_suspend = 1,
> > @@ -450,7 +444,7 @@ struct tegra_ehci_platform_data tegra_ehci1_pdata
> = {
> >  };
> >
> >  struct tegra_ehci_platform_data tegra_ehci2_pdata = {
> > -	.phy_config = &tegra_ehci2_ulpi_phy_config,
> > +	.phy_config = NULL,
> >  	.operating_mode = TEGRA_USB_HOST,
> >  	.power_down_on_bus_suspend = 1,
> >  	.vbus_gpio = -1,
> > diff --git a/arch/arm/mach-tegra/devices.h b/arch/arm/mach-
> tegra/devices.h
> > index 4f50527..6bac5e6 100644
> > --- a/arch/arm/mach-tegra/devices.h
> > +++ b/arch/arm/mach-tegra/devices.h
> > @@ -22,8 +22,6 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/platform_data/tegra_usb.h>
> >
> > -#include <mach/usb_phy.h>
> > -
> >  extern struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config;
> >
> >  extern struct tegra_ehci_platform_data tegra_ehci1_pdata;
> > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> > index 75eca42..a03e279 100644
> > --- a/drivers/usb/host/ehci-tegra.c
> > +++ b/drivers/usb/host/ehci-tegra.c
> > @@ -27,7 +27,7 @@
> >  #include <linux/of_gpio.h>
> >  #include <linux/pm_runtime.h>
> >
> > -#include <mach/usb_phy.h>
> > +#include "../phy/tegra_usb_phy.h"
> >  #include <mach/iomap.h>
> >
> >  #define TEGRA_USB_DMA_ALIGN 32
> > diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> > index eca095b..663164f 100644
> > --- a/drivers/usb/phy/Makefile
> > +++ b/drivers/usb/phy/Makefile
> > @@ -5,3 +5,4 @@
> >  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
> >
> >  obj-$(CONFIG_USB_ISP1301)		+= isp1301.o
> > +obj-$(CONFIG_USB_EHCI_TEGRA)		+= tegra_usb_phy.o
> > diff --git a/arch/arm/mach-tegra/usb_phy.c
> b/drivers/usb/phy/tegra_usb_phy.c
> > similarity index 99%
> > rename from arch/arm/mach-tegra/usb_phy.c
> > rename to drivers/usb/phy/tegra_usb_phy.c
> > index 022b33a..c856716 100644
> > --- a/arch/arm/mach-tegra/usb_phy.c
> > +++ b/drivers/usb/phy/tegra_usb_phy.c
> > @@ -1,6 +1,4 @@
> >  /*
> > - * arch/arm/mach-tegra/usb_phy.c
> > - *
> >   * Copyright (C) 2010 Google, Inc.
> >   *
> >   * Author:
> > @@ -31,7 +29,7 @@
> >  #include <linux/usb/ulpi.h>
> >  #include <asm/mach-types.h>
> >  #include <mach/gpio-tegra.h>
> > -#include <mach/usb_phy.h>
> > +#include "tegra_usb_phy.h"
> >  #include <mach/iomap.h>
> >
> >  #define ULPI_VIEWPORT		0x170
> 
> NAK, needs to be converted to proper PHY driver (misnamed as
> include/linux/usb/otg.h). You will need proper phy->init, phy->suspend,
> phy->resume, etc callbacks.
> 

Thanks Felipe for your comments.
Created a patch to separate out phy related stuff to phy.h with you as a reviewer.
Plz let me know your comments. 

Thanks,
Venu

> --
> balbi
> 
> * Unknown Key
> * 0x35CAA444
Stephen Warren Aug. 28, 2012, 2:07 p.m. UTC | #3
On 08/28/2012 02:32 AM, Venu Byravarasu wrote:
> In order to keep up with the USB driver files organization,
> moving USB phy driver from mach-tegra to drivers/USB directory.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>

> diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-tegra/devices.c

> -struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
> -	.reset_gpio = -1,
> -	.clk = "cdev2",
> -};
> -
>  struct tegra_ehci_platform_data tegra_ehci1_pdata = {
>  	.operating_mode = TEGRA_USB_OTG,
>  	.power_down_on_bus_suspend = 1,
> @@ -450,7 +444,7 @@ struct tegra_ehci_platform_data tegra_ehci1_pdata = {
>  };
>  
>  struct tegra_ehci_platform_data tegra_ehci2_pdata = {
> -	.phy_config = &tegra_ehci2_ulpi_phy_config,
> +	.phy_config = NULL,

The PHY driver checks that field isn't NULL, and fails if it is:

> struct tegra_usb_phy *tegra_usb_phy_open(struct device *dev, int instance,
>         void __iomem *regs, void *config, enum tegra_usb_phy_mode phy_mode)
> {
...
>         phy->config = config;
>         phy->mode = phy_mode;
> 
>         if (!phy->config) {
>                 if (phy_is_ulpi(phy)) {
>                         pr_err("%s: ulpi phy configuration missing", __func__);
>                         err = -EINVAL;
>                         goto err0;

So, this change will completely break ULPI support, which currently
works fine. So, NAK.

I also plan on deleting devices.[ch] in kernel 3.7, and moving the USB
platform data into board-dt-tegra20.c, since that's the only place it's
used right now. So, this patch would conflict with that rather badly. I
just posted the patches for that to the linux-tegra mailing list last
night. Do you have better proposals for that? Perhaps usb_phy.c should
set phy->config to &ulpi_default in a similar fashion to how it works
for UTMI; that would remove some of the coupling between the changes.

BTW, in your response to Felipe, you said...

> Thanks Felipe for your comments.
> Created a patch to separate out phy related stuff to phy.h with you as a reviewer.
> Plz let me know your comments. 

... where is that patch?
Venu Byravarasu Aug. 29, 2012, 5:17 a.m. UTC | #4
> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Tuesday, August 28, 2012 7:37 PM
> To: Venu Byravarasu
> Cc: ccross@android.com; olof@lixom.net; linux@arm.linux.org.uk;
> stern@rowland.harvard.edu; gregkh@linuxfoundation.org; balbi@ti.com;
> linux-kernel@vger.kernel.org; linux-tegra@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH] usb: tegra: moving phy driver into drivers directory
> 
> On 08/28/2012 02:32 AM, Venu Byravarasu wrote:
> > In order to keep up with the USB driver files organization,
> > moving USB phy driver from mach-tegra to drivers/USB directory.
> >
> > Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> 
> > diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-
> tegra/devices.c
> 
> > -struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
> > -	.reset_gpio = -1,
> > -	.clk = "cdev2",
> > -};
> > -
> >  struct tegra_ehci_platform_data tegra_ehci1_pdata = {
> >  	.operating_mode = TEGRA_USB_OTG,
> >  	.power_down_on_bus_suspend = 1,
> > @@ -450,7 +444,7 @@ struct tegra_ehci_platform_data tegra_ehci1_pdata
> = {
> >  };
> >
> >  struct tegra_ehci_platform_data tegra_ehci2_pdata = {
> > -	.phy_config = &tegra_ehci2_ulpi_phy_config,
> > +	.phy_config = NULL,
> 
> The PHY driver checks that field isn't NULL, and fails if it is:
> 
> > struct tegra_usb_phy *tegra_usb_phy_open(struct device *dev, int
> instance,
> >         void __iomem *regs, void *config, enum tegra_usb_phy_mode
> phy_mode)
> > {
> ...
> >         phy->config = config;
> >         phy->mode = phy_mode;
> >
> >         if (!phy->config) {
> >                 if (phy_is_ulpi(phy)) {
> >                         pr_err("%s: ulpi phy configuration missing", __func__);
> >                         err = -EINVAL;
> >                         goto err0;
> 
> So, this change will completely break ULPI support, which currently
> works fine. So, NAK.

My initial plan was to add support for phy interfaces one by one.
As part of that thought of UTMI only support at first and then add
ULPI and HSIC in next patches.
However as you were mentioning that it is not correct way, will
push ULPI & UTMI support at once in next patches.

> 
> I also plan on deleting devices.[ch] in kernel 3.7, and moving the USB
> platform data into board-dt-tegra20.c, since that's the only place it's
> used right now. So, this patch would conflict with that rather badly. I
> just posted the patches for that to the linux-tegra mailing list last
> night. Do you have better proposals for that? Perhaps usb_phy.c should
> set phy->config to &ulpi_default in a similar fashion to how it works
> for UTMI; that would remove some of the coupling between the changes.
> 
> BTW, in your response to Felipe, you said...
> 
> > Thanks Felipe for your comments.
> > Created a patch to separate out phy related stuff to phy.h with you as a
> reviewer.
> > Plz let me know your comments.
> 
> ... where is that patch?

Plz see https://lkml.org/lkml/2012/8/28/58
Stephen Warren Aug. 29, 2012, 5:30 p.m. UTC | #5
On 08/28/12 22:17, Venu Byravarasu wrote:
> Stephen Warren wrote at Tuesday, August 28, 2012 7:37 PM:
>> On 08/28/2012 02:32 AM, Venu Byravarasu wrote:
>>> In order to keep up with the USB driver files organization,
>>> moving USB phy driver from mach-tegra to drivers/USB directory.
>>>
>>> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
>>
>>> diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-
>> tegra/devices.c
>>
>>> -struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
>>> -	.reset_gpio = -1,
>>> -	.clk = "cdev2",
>>> -};
>>> -
>>>   struct tegra_ehci_platform_data tegra_ehci1_pdata = {
>>>   	.operating_mode = TEGRA_USB_OTG,
>>>   	.power_down_on_bus_suspend = 1,
>>> @@ -450,7 +444,7 @@ struct tegra_ehci_platform_data tegra_ehci1_pdata
>> = {
>>>   };
>>>
>>>   struct tegra_ehci_platform_data tegra_ehci2_pdata = {
>>> -	.phy_config = &tegra_ehci2_ulpi_phy_config,
>>> +	.phy_config = NULL,
>>
>> The PHY driver checks that field isn't NULL, and fails if it is:
>>
>>> struct tegra_usb_phy *tegra_usb_phy_open(struct device *dev, int
>> instance,
>>>          void __iomem *regs, void *config, enum tegra_usb_phy_mode
>> phy_mode)
>>> {
>> ...
>>>          phy->config = config;
>>>          phy->mode = phy_mode;
>>>
>>>          if (!phy->config) {
>>>                  if (phy_is_ulpi(phy)) {
>>>                          pr_err("%s: ulpi phy configuration missing", __func__);
>>>                          err = -EINVAL;
>>>                          goto err0;
>>
>> So, this change will completely break ULPI support, which currently
>> works fine. So, NAK.
>
> My initial plan was to add support for phy interfaces one by one.
> As part of that thought of UTMI only support at first and then add
> ULPI and HSIC in next patches.
> However as you were mentioning that it is not correct way, will
> push ULPI & UTMI support at once in next patches.

But with the existing code, both ULPI and UTMI work. This patch breaks 
something that already works.

>> I also plan on deleting devices.[ch] in kernel 3.7, and moving the USB
>> platform data into board-dt-tegra20.c, since that's the only place it's
>> used right now. So, this patch would conflict with that rather badly. I
>> just posted the patches for that to the linux-tegra mailing list last
>> night. Do you have better proposals for that? Perhaps usb_phy.c should
>> set phy->config to &ulpi_default in a similar fashion to how it works
>> for UTMI; that would remove some of the coupling between the changes.
>>
>> BTW, in your response to Felipe, you said...
>>
>>> Thanks Felipe for your comments.
>>> Created a patch to separate out phy related stuff to phy.h with you as a
>> reviewer.
>>> Plz let me know your comments.
>>
>> ... where is that patch?
>
> Plz see https://lkml.org/lkml/2012/8/28/58

Doesn't that link point at the patch I replied to?
Venu Byravarasu Aug. 30, 2012, 5:16 a.m. UTC | #6
> -----Original Message-----
> From: linux-tegra-owner@vger.kernel.org [mailto:linux-tegra-
> owner@vger.kernel.org] On Behalf Of Stephen Warren
> Sent: Wednesday, August 29, 2012 11:01 PM
> To: Venu Byravarasu
> Cc: ccross@android.com; olof@lixom.net; linux@arm.linux.org.uk;
> stern@rowland.harvard.edu; gregkh@linuxfoundation.org; balbi@ti.com;
> linux-kernel@vger.kernel.org; linux-tegra@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH] usb: tegra: moving phy driver into drivers directory
> 
> On 08/28/12 22:17, Venu Byravarasu wrote:
> > Stephen Warren wrote at Tuesday, August 28, 2012 7:37 PM:
> >> On 08/28/2012 02:32 AM, Venu Byravarasu wrote:
> >>> In order to keep up with the USB driver files organization,
> >>> moving USB phy driver from mach-tegra to drivers/USB directory.
> >>>
> >>> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> >>
> >>> diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-
> >> tegra/devices.c
> >>
> >>> -struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
> >>> -	.reset_gpio = -1,
> >>> -	.clk = "cdev2",
> >>> -};
> >>> -
> >>>   struct tegra_ehci_platform_data tegra_ehci1_pdata = {
> >>>   	.operating_mode = TEGRA_USB_OTG,
> >>>   	.power_down_on_bus_suspend = 1,
> >>> @@ -450,7 +444,7 @@ struct tegra_ehci_platform_data
> tegra_ehci1_pdata
> >> = {
> >>>   };
> >>>
> >>>   struct tegra_ehci_platform_data tegra_ehci2_pdata = {
> >>> -	.phy_config = &tegra_ehci2_ulpi_phy_config,
> >>> +	.phy_config = NULL,
> >>
> >> The PHY driver checks that field isn't NULL, and fails if it is:
> >>
> >>> struct tegra_usb_phy *tegra_usb_phy_open(struct device *dev, int
> >> instance,
> >>>          void __iomem *regs, void *config, enum tegra_usb_phy_mode
> >> phy_mode)
> >>> {
> >> ...
> >>>          phy->config = config;
> >>>          phy->mode = phy_mode;
> >>>
> >>>          if (!phy->config) {
> >>>                  if (phy_is_ulpi(phy)) {
> >>>                          pr_err("%s: ulpi phy configuration missing", __func__);
> >>>                          err = -EINVAL;
> >>>                          goto err0;
> >>
> >> So, this change will completely break ULPI support, which currently
> >> works fine. So, NAK.
> >
> > My initial plan was to add support for phy interfaces one by one.
> > As part of that thought of UTMI only support at first and then add
> > ULPI and HSIC in next patches.
> > However as you were mentioning that it is not correct way, will
> > push ULPI & UTMI support at once in next patches.
> 
> But with the existing code, both ULPI and UTMI work. This patch breaks
> something that already works.
> 
> >> I also plan on deleting devices.[ch] in kernel 3.7, and moving the USB
> >> platform data into board-dt-tegra20.c, since that's the only place it's
> >> used right now. So, this patch would conflict with that rather badly. I
> >> just posted the patches for that to the linux-tegra mailing list last
> >> night. Do you have better proposals for that? Perhaps usb_phy.c should
> >> set phy->config to &ulpi_default in a similar fashion to how it works
> >> for UTMI; that would remove some of the coupling between the changes.
> >>
> >> BTW, in your response to Felipe, you said...
> >>
> >>> Thanks Felipe for your comments.
> >>> Created a patch to separate out phy related stuff to phy.h with you as a
> >> reviewer.
> >>> Plz let me know your comments.
> >>
> >> ... where is that patch?
> >
> > Plz see https://lkml.org/lkml/2012/8/28/58
> 
> Doesn't that link point at the patch I replied to?

Sorry, by mistake I sent the wrong one.
Here is the correct one: https://lkml.org/lkml/2012/8/29/40  

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" 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

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index b94858f..b542dac 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -25,7 +25,6 @@  obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
 obj-$(CONFIG_TEGRA_SYSTEM_DMA)		+= dma.o
 obj-$(CONFIG_CPU_FREQ)                  += cpu-tegra.o
 obj-$(CONFIG_TEGRA_PCI)			+= pcie.o
-obj-$(CONFIG_USB_SUPPORT)		+= usb_phy.o
 
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-dt-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-tegra/devices.c
index 61e9603..232d7e1 100644
--- a/arch/arm/mach-tegra/devices.c
+++ b/arch/arm/mach-tegra/devices.c
@@ -26,7 +26,6 @@ 
 #include <mach/irqs.h>
 #include <mach/iomap.h>
 #include <mach/dma.h>
-#include <mach/usb_phy.h>
 
 #include "gpio-names.h"
 #include "devices.h"
@@ -438,11 +437,6 @@  static struct resource tegra_usb3_resources[] = {
 	},
 };
 
-struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config = {
-	.reset_gpio = -1,
-	.clk = "cdev2",
-};
-
 struct tegra_ehci_platform_data tegra_ehci1_pdata = {
 	.operating_mode = TEGRA_USB_OTG,
 	.power_down_on_bus_suspend = 1,
@@ -450,7 +444,7 @@  struct tegra_ehci_platform_data tegra_ehci1_pdata = {
 };
 
 struct tegra_ehci_platform_data tegra_ehci2_pdata = {
-	.phy_config = &tegra_ehci2_ulpi_phy_config,
+	.phy_config = NULL,
 	.operating_mode = TEGRA_USB_HOST,
 	.power_down_on_bus_suspend = 1,
 	.vbus_gpio = -1,
diff --git a/arch/arm/mach-tegra/devices.h b/arch/arm/mach-tegra/devices.h
index 4f50527..6bac5e6 100644
--- a/arch/arm/mach-tegra/devices.h
+++ b/arch/arm/mach-tegra/devices.h
@@ -22,8 +22,6 @@ 
 #include <linux/platform_device.h>
 #include <linux/platform_data/tegra_usb.h>
 
-#include <mach/usb_phy.h>
-
 extern struct tegra_ulpi_config tegra_ehci2_ulpi_phy_config;
 
 extern struct tegra_ehci_platform_data tegra_ehci1_pdata;
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 75eca42..a03e279 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -27,7 +27,7 @@ 
 #include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 
-#include <mach/usb_phy.h>
+#include "../phy/tegra_usb_phy.h"
 #include <mach/iomap.h>
 
 #define TEGRA_USB_DMA_ALIGN 32
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index eca095b..663164f 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,3 +5,4 @@ 
 ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
 
 obj-$(CONFIG_USB_ISP1301)		+= isp1301.o
+obj-$(CONFIG_USB_EHCI_TEGRA)		+= tegra_usb_phy.o
diff --git a/arch/arm/mach-tegra/usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
similarity index 99%
rename from arch/arm/mach-tegra/usb_phy.c
rename to drivers/usb/phy/tegra_usb_phy.c
index 022b33a..c856716 100644
--- a/arch/arm/mach-tegra/usb_phy.c
+++ b/drivers/usb/phy/tegra_usb_phy.c
@@ -1,6 +1,4 @@ 
 /*
- * arch/arm/mach-tegra/usb_phy.c
- *
  * Copyright (C) 2010 Google, Inc.
  *
  * Author:
@@ -31,7 +29,7 @@ 
 #include <linux/usb/ulpi.h>
 #include <asm/mach-types.h>
 #include <mach/gpio-tegra.h>
-#include <mach/usb_phy.h>
+#include "tegra_usb_phy.h"
 #include <mach/iomap.h>
 
 #define ULPI_VIEWPORT		0x170
diff --git a/arch/arm/mach-tegra/include/mach/usb_phy.h b/drivers/usb/phy/tegra_usb_phy.h
similarity index 97%
rename from arch/arm/mach-tegra/include/mach/usb_phy.h
rename to drivers/usb/phy/tegra_usb_phy.h
index 935ce9f..516c724 100644
--- a/arch/arm/mach-tegra/include/mach/usb_phy.h
+++ b/drivers/usb/phy/tegra_usb_phy.h
@@ -1,6 +1,4 @@ 
 /*
- * arch/arm/mach-tegra/include/mach/usb_phy.h
- *
  * Copyright (C) 2010 Google, Inc.
  *
  * This software is licensed under the terms of the GNU General Public