From: Stephen Warren <swarren@wwwdotorg.org>
To: Venu Byravarasu <vbyravarasu@nvidia.com>
Cc: "ccross@android.com" <ccross@android.com>,
"olof@lixom.net" <olof@lixom.net>,
"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
"stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"balbi@ti.com" <balbi@ti.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: Re: [PATCH] usb: tegra: moving phy driver into drivers directory
Date: Wed, 29 Aug 2012 10:30:49 -0700 [thread overview]
Message-ID: <503E51C9.7000606@wwwdotorg.org> (raw)
In-Reply-To: <D958900912E20642BCBC71664EFECE3E6DDE14DD7C@BGMAIL02.nvidia.com>
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?
next prev parent reply other threads:[~2012-08-29 17:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-28 9:32 [PATCH] usb: tegra: moving phy driver into drivers directory Venu Byravarasu
2012-08-28 9:49 ` Felipe Balbi
2012-08-28 12:36 ` Venu Byravarasu
2012-08-28 14:07 ` Stephen Warren
2012-08-29 5:17 ` Venu Byravarasu
2012-08-29 17:30 ` Stephen Warren [this message]
2012-08-30 5:16 ` Venu Byravarasu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=503E51C9.7000606@wwwdotorg.org \
--to=swarren@wwwdotorg.org \
--cc=balbi@ti.com \
--cc=ccross@android.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=olof@lixom.net \
--cc=stern@rowland.harvard.edu \
--cc=vbyravarasu@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).