Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250
Hi,
On Wed, Dec 19, 2012 at 11:52:01AM +0530, Vivek Gautam wrote:
> >>> @@ -736,7 +1035,41 @@ static int __devinit samsung_usbphy_probe(struct
> >>> platform_device *pdev)
> >>>
> >>> sphy->clk = clk;
> >>>
> >>> - return usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
> >>> + sphy->has_usb3 = (sphy->cpu_type == TYPE_EXYNOS5250);
> >>> +
> >>> + if (sphy->has_usb3) {
> >>> + struct resource *usb3phy_mem;
> >>> + void __iomem*usb3phy_base;
> >>> +
> >>> + usb3phy_mem = platform_get_resource(pdev, IORESOURCE_MEM,
> >>> 1);
> >>> + if (!usb3phy_mem) {
> >>> + dev_err(dev, "%s: missing mem resource\n",
> >>> __func__);
> >>> + return -ENODEV;
> >>> + }
> >>> +
> >>> + usb3phy_base = devm_request_and_ioremap(dev, usb3phy_mem);
> >>> + if (!usb3phy_base) {
> >>> + dev_err(dev, "%s: register mapping failed\n",
> >>> __func__);
> >>> + return -ENXIO;
> >>> + }
> >>> +
> >>> + sphy->usb3phy.regs_phy = usb3phy_base;
> >>> + sphy->usb3phy.phy.dev = sphy->dev;
> >>> + sphy->usb3phy.phy.label = "samsung-usb3phy";
> >>> + sphy->usb3phy.phy.init = samsung_usb3phy_init;
> >>> + sphy->usb3phy.phy.shutdown = samsung_usb3phy_shutdown;
> >>> + }
> >>> +
> >>> + ret = usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
> >>> + if (ret)
> >>> + return ret;
> >>
> >> is this realy how your HW behaves ? USB2 and USB3 phys are a single HW
> >> entity ? I kinda doubt that :-s
> >>
>
> They are separate HW in fact.
> So, do you expect to see a separate driver interface for USB 3.0 type phy ?
yes. Just as we did on OMAP. One driver for the USB2 part and one driver
for USB3 part (which are actually two, but you can only talk to them as
one) :-)
> That will be quite similar architecturally to current samsung-usbphy
> driver for USB 2.0 type phy,
> and may require some code duplication too.
If it duplicates code, then perhaps it's best to keep it as is but I'm
actually surprised you guys have similar programming model on both
parts. I mean, the differences at HW behavior are huge: on one side you
use ULPI/UTMI+ on the other PIPE3, on one side you have 480Mbps
half-duplex signalling, on the other you have 5Gbps dual simplex
signalling, the differences go on and on.
Also, what you say about duplicating, it seems to me that it will
duplicate only the boylerplate part (allocating memory, having a
platform_driver, and so on), because you _do_ have completely separate
functions to handle usb3 part.
One more comment below:
> +static u32 exynos5_usb3phy_set_clock(struct samsung_usbphy *sphy)
> +{
> + u32 reg;
> + u32 refclk;
> +
> + refclk = sphy->ref_clk_freq;
> +
> + reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
> + PHYCLKRST_FSEL(refclk);
> +
> + switch (refclk) {
> + case HOST_CTRL0_FSEL_CLKSEL_50M:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x00));
> + break;
> + case HOST_CTRL0_FSEL_CLKSEL_20M:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x00));
> + break;
> + case HOST_CTRL0_FSEL_CLKSEL_19200K:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x88));
> + break;
> + case HOST_CTRL0_FSEL_CLKSEL_24M:
> + default:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x88));
> + break;
> + }
> +
> + return reg;
> +}
looks like this should be done by commong clock framework by clock
reparenting perhaps ?
--
balbi
signature.asc
Description: Digital signature
Re: help with f_loopback
On Tue, Dec 18, 2012 at 07:29:40PM -0600, ricardo garcia wrote: > > my controller its: PHY dwc_otg (Broadcom BCMxxx) That one is not in tree. I'm afraid you are on your own here. It might be the fault of the UDC. > im not currently able to test this with 3.7, can somebody give it a > try to rule out that > its a software problem??? (try sending more than 128 bytes to the > loopback gadget > driver function within g_zero and see if you receive it back...and trying with > 256, 512 , 1024 and 4096 buffer sizes would be nice as well.please > and thanks Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] staging: usbip: userspace: suppress a bogus error
Hi,
2012/12/8 Ilija Hadzic :
> If mkdir() of VHCI_STATE_PATH fails because the directory
> already exists, that's not an error. This patch fixes
> annoying "record connection" errors that would typically
> come up on attach.
>
> Signed-off-by: Ilija Hadzic
> ---
> drivers/staging/usbip/userspace/src/usbip_attach.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/src/usbip_attach.c
> b/drivers/staging/usbip/userspace/src/usbip_attach.c
> index bdf61c0..2da4e44 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_attach.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_attach.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
>
> #include "vhci_driver.h"
> #include "usbip_common.h"
> @@ -52,8 +53,18 @@ static int record_connection(char *host, char *port, char
> *busid, int rhport)
> int ret;
>
> ret = mkdir(VHCI_STATE_PATH, 0700);
> - if (ret < 0)
> - return -1;
> + if (ret < 0) {
> + /* if VHCI_STATE_PATH exists, then it better be a directory */
> + if (errno == EEXIST) {
> + struct stat s;
> + ret = stat(VHCI_STATE_PATH, &s);
> + if (ret < 0)
> + return -1;
> + if (!(s.st_mode & S_IFDIR))
> + return -1;
> + } else
> + return -1;
> + }
>
> snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d", rhport);
>
The patch looks good to me.
Acked-by: David Chang
> --
> 1.7.12
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2]linux-usb:optimize to match the Huawei USB storage devices and support new switch command
On Wed, Dec 19, 2012 at 03:13:32AM +, Fangxiaozhi (Franko) wrote: > By the way, I found the kernel is updated to 3.7.1 today. So I have to > update my patch based on 3.7.1, and resubmit it? > Right? You should rebase your patch on top of Greg's usb-next branch of his usb tree. http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=shortlog;h=refs/heads/usb-next but I guess that there are hardly any changes in that area. The last change in drivers/usb/storage/initializers.* is yours "USB: usb-storage fails to attach to Huawei Datacard cdrom device". If you call ./scripts/get_maintainer.pl on your patch you should learn that you miss |Matthew Dharm |usb-stor...@lists.one-eyed-alien.net > > And shouldn't you read something from the us->recv_bulk_pipe after > > that? > Well, because our device will re-connect to switch the ports if it > receives the command. > So it is not necessary to read the response of the command. Hmm. I guess this for Matthew / Greg to decide, I don't insist on anything. Maybe a comment would be nice because now it looks, atleast to me, that something is missing. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: help with f_loopback
On Wed, Dec 19, 2012 at 09:19:53AM +0100, Sebastian Andrzej Siewior wrote: > On Tue, Dec 18, 2012 at 07:29:40PM -0600, ricardo garcia wrote: > > > > my controller its: PHY dwc_otg (Broadcom BCMxxx) > > That one is not in tree. I'm afraid you are on your own here. It might be the > fault of the UDC. right, with such an old kernel and an out of tree driver, you're on your own. You could help samsung folks clean up their s3c_hsotg.c driver, since that's just a dwc_otg instance, so that it can be reused. -- balbi signature.asc Description: Digital signature
[PATCH V2] usb: gadget zero: avoid unnecessary reinit of data in f_sourcesink
In the IN case, since the USB request is allocated only when the source/sink function is started and never freed, the USB ept buffer needs to be initialized only at the beginning. This change results into a more performant g_zero module, especially when 'pattern=1' is selected. Signed-off-by: Armando Visconti Reviewed-by: Sebastian Andrzej Siewior --- This is patch V2, after Sbatian's review. Patch is very simple, but my assumption needs to be cross-verified. This patch has been quickly tested using a DWC3 controller gadget device. Felipe, if you can quicly take a look at it.. Armando drivers/usb/gadget/f_sourcesink.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/f_sourcesink.c b/drivers/usb/gadget/f_sourcesink.c index 3c126fd..534cf8c 100644 --- a/drivers/usb/gadget/f_sourcesink.c +++ b/drivers/usb/gadget/f_sourcesink.c @@ -536,8 +536,7 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req) check_read_data(ss, req); if (pattern != 2) memset(req->buf, 0x55, req->length); - } else - reinit_write_data(ep, req); + } break; /* this endpoint is normally active while we're configured */ -- 1.7.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] usb: add usb port auto power off mechanism
于 2012/12/18 23:58, Alan Stern 写道:
On Tue, 18 Dec 2012, lantianyu wrote:
What you want here is sort of an alternate debounce routine. The
normal routine waits until the connect status has been stable for 100
ms. But you want to wait until the status has stable in the
"connected" state for 100 ms.
Yesh.
Maybe it would be better to do this by passing an extra argument to the
regular debounce routine.
How about this?
It would be easier to read a patch, particularly if it wasn't
whitespace-damaged.
Oh. Sorry. I didn't notice this. I replied via a newly installed
thunder bird and forgot to unset html format.
static int hub_port_debounce(struct usb_hub *hub, int port1, bool
must_connected)
This should be "must_be_connected".
{
int ret;
int total_time, stable_time = 0;
u16 portchange, portstatus;
unsigned connection = 0x;
for (total_time = 0; ; total_time += HUB_DEBOUNCE_STEP) {
ret = hub_port_status(hub, port1, &portstatus, &portchange);
if (ret < 0)
return ret;
if (!(portchange & USB_PORT_STAT_C_CONNECTION) &&
(portstatus & USB_PORT_STAT_CONNECTION) == connection){
if (!must_connected || (connection == USB_PORT_STAT_CONNECTION))
stable_time += HUB_DEBOUNCE_STEP;
if (stable_time >= HUB_DEBOUNCE_STABLE)
break;
} else {
stable_time = 0;
connection = portstatus & USB_PORT_STAT_CONNECTION;
}
if (portchange & USB_PORT_STAT_C_CONNECTION) {
clear_port_feature(hub->hdev, port1,
USB_PORT_FEAT_C_CONNECTION);
}
if (total_time >= HUB_DEBOUNCE_TIMEOUT)
break;
msleep(HUB_DEBOUNCE_STEP);
}
dev_dbg (hub->intfdev,
"debounce: port %d: total %dms stable %dms status 0x%x\n",
port1, total_time, stable_time, portstatus);
if (stable_time < HUB_DEBOUNCE_STABLE)
return -ETIMEDOUT;
return portstatus;
}
Yes, that seems okay. You'll want to increase HUB_DEBOUNCE_TIMEOUT.
Yeah.
No, no, not at all. The set_bit and clear_bit operations don't need
any protection -- they are atomic. What we need to do is make sure
that two different threads don't try to manage the same port at the
same time. For example, it should not be possible for one thread to
reset the port while another thread is trying to suspend it.
I'm pretty sure that this can't happen with the existing code. The
only places where a port gets used are:
when a device is connected;
when a device is disconnected;
when a device is reset;
when a device is suspended or resumed.
We should check that these things really are mutually exclusive, and
that they remain mutually exclusive when your changes are added.
I just find busy_bits is set or clear in the usb_port_resume() and
usb_reset_and_verify_device(). So currently we should keep my changes
mutually exclusive with them, right?
Don't forget about what happens when a device is removed.
I am not very clear about this since busy_bits is not changed during device
being
removed. Could you elaborate? Thanks.
My changes add two new places: when port is resumed and when is suspended.
Port's resume/suspend may occur in device's resume/suspend and Pm Qos
change by
pm_runtime_get_sync/put(port_dev).
The case in device's resume/suspend will not conflict with
usb_port_resume() and
usb_reset_and_verify_device() since they are all in the same thread or
will not occur at
the same time.
Yes.
For the case of Pm Qos change, actually it only happens when user set
NO_POWER_OFF flag
after the device being power off. The port will be resumed at that time.
One case is
that device resume and Pm Qos change take place at the same time. The
usb_port_resume()
is calling pm_runtime_get_sync(port_dev) and PM core also is doing the
same thing. So two
port resume will be executed in the parallel. Assuming that the one
triggered by usb_port_resume()
is finished firstly and it goes back to usb_port_resume() position where
just before set busy_bits. The
second, port resume operation will be executed. Will this happen? If
yes, there will be a race problem.
Just one case I have found.
The PM core guarantees that runtime PM callbacks are mutually
exclusive. It also guarantees that the runtime_resume routine won't be
called if the device is currently active (and the runtime_suspend
routine won't be called if the device is currently suspended).
Yeah. I misread the code.
Therefore it suffices to make sure that usb_port_resume does
pm_runtime_get_sync(port_dev) before it touches busy_bits.
The other race, against usb_reset_and_verify_device, should also be
okay. Aside from the resume pathway, the only place that routine gets
called is from usb_reset_device, which is careful to prevent the device
from being suspended during the reset.
Is it possble to add a lock to prevent the busy_bits from being cleared
by other thread?
It looks like we don't need it.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci_hcd fails USB2.0 camera that worked on ehci_hcd
I am told by camera manufacturers that a first version of the Windows drivers also had such problems with their camera, but that it was fixed in subsequent Windows drivers of xHCI. I reattach the same ehci lsusb.txt. On Tuesday 18 December 2012 16:19:18 Sarah Sharp wrote: > On Tue, Dec 18, 2012 at 01:13:05AM -0500, Marius Silaghi wrote: > > Hi Marius, thanks for reporting this. > > > My USB2.0 cameras are enumerated fine on ehci systems. But not on my new > > Ubuntu 12.10: > > $uname -a > > Linux VAIO 3.5.0-18-generic #29-Ubuntu SMP Fri Oct 19 10:26:51 UTC 2012 > > x86_64 x86_64 x86_64 GNU/Linux > > > > here xhci_hcd is compiled in the kernel, and the device is not enumerated. > > I tried unbinding xhci using the sysfs and the messages dissapear, but > > ehci does not take over. Any idea? > > Most xHCI host controllers don't share ports with the EHCI controller, > so unbinding the xHCI driver doesn't switch the ports over. Even on the > Intel xHCI hosts that can switch ports between the xHCI host and the > EHCI host, we don't switch the ports back on xHCI driver unload. > > > [12241.536789] usb 3-1: new high-speed USB device number 8 using xhci_hcd > > [12241.553375] usb 3-1: device descriptor read/8, error -75 > > [12241.673302] usb 3-1: device descriptor read/8, error -75 > > [12241.888420] usb 3-1: new high-speed USB device number 9 using xhci_hcd > > [12241.905029] usb 3-1: device descriptor read/8, error -75 > > [12242.024764] usb 3-1: device descriptor read/8, error -75 > > [12242.240032] usb 3-1: new high-speed USB device number 10 using xhci_hcd > > [12242.256595] usb 3-1: device descriptor read/8, error -75 > > [12242.376524] usb 3-1: device descriptor read/8, error -75 > > [12242.591580] usb 3-1: new high-speed USB device number 11 using xhci_hcd > > [12242.608170] usb 3-1: device descriptor read/8, error -75 > > [12242.727987] usb 3-1: device descriptor read/8, error -75 > > [12242.831348] hub 3-0:1.0: unable to enumerate USB device on port 1 > > Can you recompile your kernel with CONFIG_USB_DEBUG and > CONFIG_USB_XHCI_HCD_DEBUGGING turned on? If you need instructions, > please follow the directions for compiling the latest stable kernel > here: http://kernelnewbies.org/KernelBuild > > Then send me the dmesg output from just before you plugged in the > device. Please also send me the output of `sudo lsusb -v` with the > device plugged into a system with an EHCI controller. > > However, I will say that at a quick glance, it looks like your hardware > may have electrical issues under the xHCI host controller. The -75 > error code is for EOVERFLOW, which means your device "babbled" on a > transfer. There's not much software can do about electrical issues, if > that is the true root cause. > > Sarah Sharp > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 0 Full speed (or root) hub bMaxPacketSize064 idVendor 0x1d6b Linux Foundation idProduct 0x0002 2.0 root hub bcdDevice3.02 iManufacturer 3 iProduct2 iSerial 1 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 25 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0004 1x 4 bytes bInterval 12 Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 0 Full speed (or root) hub bMaxPacketSize064 idVendor 0x1d6b Linux Foundation idProduct 0x0002 2.0 root hub bcdDevice3.02 iMan
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
于 2012/12/18 4:06, Alan Stern 写道:
On Mon, 17 Dec 2012, Octavio Alvarez wrote:
On Thu, 13 Dec 2012 00:45:05 -0800, Lan Tianyu
wrote:
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index f034716..9335f1b 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2509,7 +2509,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
* they only forward requests from the root hub. Therefore
* controllers should always be enabled for remote wakeup.
*/
- device_wakeup_enable(hcd->self.controller);
+ if (!usb_hcd_wakeup_quirks(hcd->self.controller))
+ device_wakeup_enable(hcd->self.controller);
return retval;
error_create_attr_group:
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index fdefd9c..ba847d3 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -12,6 +12,7 @@
*/
#include
+#include
#include
#include "usb.h"
@@ -226,3 +227,33 @@ void usb_detect_interface_quirks(struct usb_device
*udev)
quirks);
udev->quirks |= quirks;
}
+
+struct pci_hcd {
+ u32 vendor;
+ u32 device;
+};
+
+static struct pci_hcd hcd_wakeup_qrk[] = {
+ {PCI_VENDOR_ID_NVIDIA, 0x026d}, /* MCP51 OHCI */
+ {PCI_VENDOR_ID_NVIDIA, 0x0aa5}, /* MCP79 OHCI */
+ {PCI_VENDOR_ID_NVIDIA, 0x0aa7}, /* MCP79 OHCI */
+ { }
+};
+
+int usb_hcd_wakeup_quirks(struct device *dev)
+{
+ struct pci_dev *pdev;
+ int i;
+
+ if (dev->bus != (struct bus_type *)&pci_bus_type)
+ return 0;
+
+ pdev = to_pci_dev(dev);
+ for (i = 0; hcd_wakeup_qrk[i].vendor || hcd_wakeup_qrk[i].device; i++)
+ if ((hcd_wakeup_qrk[i].vendor == pdev->vendor) &&
+ (hcd_wakeup_qrk[i].device == pdev->device)) {
+ return 1;
+ }
+
+ return 0;
+}
I would informing the user via dmesg output about the applied quirk
and a point him to relevant documentation. Something like this:
"Detected OHCI controller ID :, which requires no-wakeup quirk.
See Documentation/quirks/ohci-no-wakeup.txt"
Incidentally, this patch should be written differently. Instead of a
quirks routine, there should simply be a bad_wakeup bitflag added to
the usb_hcd structure. The flag should be set in ohci-pci.c by
matching against nVidia's PCI vendor ID.
Oh. I forget to mention the issue also takes place on the uhci.
https://bugzilla.kernel.org/show_bug.cgi?id=42721
So we also should make such a patch for uhci.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: samsung: Add support to set pmu isolation
Hi Sylwester,
On Wed, Dec 19, 2012 at 11:05 AM, Vivek Gautam
wrote:
> CC: Doug Anderson
>
>
> On Wed, Dec 19, 2012 at 4:49 AM, Sylwester Nawrocki
> wrote:
>> Hi Vivek,
>>
>>
>> On 12/18/2012 02:56 PM, Vivek Gautam wrote:
>>>
>>> Adding support to parse device node data in order to get
>>> required properties to set pmu isolation for usb-phy.
>>>
>>> Signed-off-by: Vivek Gautam
>>> ---
>>>
>>> Changes from v1:
>>> - Changed the name of property for phy handler from'samsung,usb-phyctrl'
>>> to 'samsung,usb-phyhandle' to make it look more generic.
>>> - Similarly 'samsung,phyctrl-reg' is changed to 'samsung,phyhandle-reg'
>>> - Added a check for 'samsung,usb-phyhandle' before getting node from
>>> phandle.
>>> - Putting the node using 'of_node_put()' which had been missed.
>>> - Adding necessary check for the pointer in
>>> 'samsung_usbphy_set_isolation()'
>>> to avoid any NULL pointer dereferencing.
>>> - Unmapping the register ioremapped in
>>> 'samsung_usbphy_parse_dt_param()'.
>>>
>>>
>>> .../devicetree/bindings/usb/samsung-usbphy.txt | 12 +++
>>> drivers/usb/phy/samsung-usbphy.c | 94
>>> ++--
>>> 2 files changed, 98 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>>> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>>> index 7b26e2d..a7b28b2 100644
>>> --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>>> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>>> @@ -9,3 +9,15 @@ Required properties:
>>> - compatible : should be "samsung,exynos4210-usbphy"
>>> - reg : base physical address of the phy registers and length of memory
>>> mapped
>>> region.
>>> +
>>> +Optional properties:
>>> +- samsung,usb-phyhandle : should point to usb-phyhandle sub-node which
>>> provides
>>> + binding data to enable/disable device PHY handled
>>> by
>>> + PMU register.
>>> +
>>> + Required properties:
>>> + - compatible : should be "samsung,usbdev-phyctrl"
>>> for
>>> + DEVICE type phy.
>>> + - samsung,phyhandle-reg: base physical address of
>>> + PHY_CONTROL register in
>>> PMU.
>>> +- samsung,enable-mask : should be '1'
>>
>>
>> This should only be 1 for Exynos4210+ SoCs, right ?
Yes that's true Exynso4210+ SoCs have only single PHY for both host and device
which gets enabled by single bit.
>> S5PV210 uses bit 0 for OTG and bit1 for USB host, doesn't it ? And for
>> s3c64xx
>> it seems to be bit 16.
>>
True, S5PV210 uses two bits separately for OTG and HOST, so in that
case we would
require to set both these bits. Thanks for pointing out !!
I couldn't see device tree support for S5PV210 and S3C64xx so thought
of going for
4210+ SoCs. Better we make this more generic so that once these SoCs
are moved to
device tree we don't face any issue. Right ??
>> How about deriving this information from 'compatible' property instead ?
>>
It will definitely be good to use the compatible property to derive
such information,
Need to amend the current approach.
>> Maybe you could just encode the USB PMU registers (I assume those aren't
>> touched by anything but the usb drivers) in a regular 'reg' property in
>> an usbphy subnode. Then the driver could interpret it also with help
>> of 'compatible' property. And you could just use of_iomap(). E.g.
>>
>> usbphy@1213 {
>> compatible = "samsung,exynos5250-usbphy";
>> reg = <0x1213 0x100>, <0x1210 0x100>;
>> usbphy-pmu {
>> /* USB device and host PHY_CONTROL registers */
>> reg = <0x10040704 8>;
>> };
>> };
>>
This approach seems nice.
>> Your "samsung,usb-phyhandle" approach seems over-engineered to me.
>> I might be missing something though.
>>
The idea behind using phandles for usb-phy was to get the multiple
registers (2 in PMU
and 1 in SYSREG) and program them separately as required.
>>
>>> diff --git a/drivers/usb/phy/samsung-usbphy.c
>>> b/drivers/usb/phy/samsung-usbphy.c
>>> index 5c5e1bb5..4ceabe3 100644
>>> --- a/drivers/usb/phy/samsung-usbphy.c
>>> +++ b/drivers/usb/phy/samsung-usbphy.c
>>> @@ -72,6 +72,8 @@ enum samsung_cpu_type {
>>>* @dev: The parent device supplied to the probe function
>>>* @clk: usb phy clock
>>>* @regs: usb phy register memory base
>>> + * @devctrl_reg: usb device phy-control pmu register memory base
>>
>>
>> hum, this name is not really helpful in understanding what's going
>> on here.
>>
>> Looking at arch/arm/mach-s5pv210/setup-usb-phy.c, there is only one
>> PHY_CONTROL (Power Management Unit) register for both OTG and USB host
>> PHYs. So are you really taking care of that case as well ?
>>
This doesn't take care of s3pv210. Will amend this to ensure that.
>>
>>> + * @en_mask: enable
Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device
CC: Doug Anderson On Sat, Dec 15, 2012 at 12:50 PM, Grant Likely wrote: > On Thu, 13 Dec 2012 20:22:26 +0530, Vivek Gautam > wrote: >> Using chip specific compatible string as it should be. >> So fixing this for ehci-s5p, ohci-exynos and dwc3-exynos >> which till now used a generic 'exynos' in their compatible strings. >> >> This goes as per the discussion happened in the thread for >> [PATCH v2] ARM: Exynos5250: Enabling dwc3-exynos driver >> available at: >> http://www.spinics.net/lists/linux-usb/msg74145.html >> >> Vivek Gautam (2): >> usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device >> usb: dwc3-exynos: Fix compatible strings for the device > > for both patches: > Acked-by: Grant Likely > >> >> drivers/usb/dwc3/dwc3-exynos.c |2 +- >> drivers/usb/host/ehci-s5p.c|2 +- >> drivers/usb/host/ohci-exynos.c |2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> -- >> 1.7.6.5 >> > > -- > Grant Likely, B.Sc, P.Eng. > Secret Lab Technologies, Ltd. > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Thanks & Regards Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device
CC: Doug Anderson
On Thu, Dec 13, 2012 at 9:39 PM, Vivek Gautam wrote:
> On Thu, Dec 13, 2012 at 8:22 PM, Vivek Gautam
> wrote:
>> Using specific chip in compatible strings. Newer SOCs can claim
>> device by using older string in the compatible list.
>>
>> Signed-off-by: Vivek Gautam
>> ---
>> drivers/usb/host/ehci-s5p.c|2 +-
>> drivers/usb/host/ohci-exynos.c |2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
>> index 319dcfa..f18e6ac 100644
>> --- a/drivers/usb/host/ehci-s5p.c
>> +++ b/drivers/usb/host/ehci-s5p.c
>> @@ -266,7 +266,7 @@ static const struct dev_pm_ops s5p_ehci_pm_ops = {
>>
>> #ifdef CONFIG_OF
>> static const struct of_device_id exynos_ehci_match[] = {
>> - { .compatible = "samsung,exynos-ehci" },
>> + { .compatible = "samsung,exynos4210-ehci" },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, exynos_ehci_match);
>> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> index aa3b884..77f2017 100644
>> --- a/drivers/usb/host/ohci-exynos.c
>> +++ b/drivers/usb/host/ohci-exynos.c
>> @@ -267,7 +267,7 @@ static const struct dev_pm_ops exynos_ohci_pm_ops = {
>>
>> #ifdef CONFIG_OF
>> static const struct of_device_id exynos_ohci_match[] = {
>> - { .compatible = "samsung,exynos-ohci" },
>> + { .compatible = "samsung,exynos4210-ohci" },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, exynos_ohci_match);
>> --
>> 1.7.6.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Thanks & Regards
> Vivek
--
Thanks & Regards
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: dwc3-exynos: Fix compatible strings for the device
CC: Doug Anderson
On Thu, Dec 13, 2012 at 9:40 PM, Vivek Gautam wrote:
> CC: LKML
>
> On Thu, Dec 13, 2012 at 8:22 PM, Vivek Gautam
> wrote:
>> Using specific chip in compatible strings. Newer SOCs can claim
>> device by using older string in the compatible list.
>>
>> Signed-off-by: Vivek Gautam
>> ---
>> drivers/usb/dwc3/dwc3-exynos.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>> index aae5328..9dce99a 100644
>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>> @@ -188,7 +188,7 @@ static int dwc3_exynos_remove(struct platform_device
>> *pdev)
>>
>> #ifdef CONFIG_OF
>> static const struct of_device_id exynos_dwc3_match[] = {
>> - { .compatible = "samsung,exynos-dwc3" },
>> + { .compatible = "samsung,exynos5250-dwc3" },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, exynos_dwc3_match);
>> --
>> 1.7.6.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Thanks & Regards
> Vivek
--
Thanks & Regards
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] ARM: Exynos5250: Enabling ehci-s5p driver
CC: Doug Anderson
On Sat, Dec 15, 2012 at 12:53 PM, Grant Likely
wrote:
> On Thu, 13 Dec 2012 22:06:01 +0530, Vivek Gautam
> wrote:
>> Adding EHCI device tree node for Exynos5250 along with
>> the device base adress and gpio line for vbus.
>>
>> Signed-off-by: Vivek Gautam
>> Acked-by: Jingoo Han
>> ---
>> .../devicetree/bindings/usb/exynos-usb.txt | 25
>>
>> arch/arm/boot/dts/exynos5250-smdk5250.dts |4 +++
>> arch/arm/boot/dts/exynos5250.dtsi |6
>> arch/arm/mach-exynos/include/mach/map.h|1 +
>> arch/arm/mach-exynos/mach-exynos5-dt.c |2 +
>> 5 files changed, 38 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/usb/exynos-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> b/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> new file mode 100644
>> index 000..e8bbb47
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
>> @@ -0,0 +1,25 @@
>> +Samsung Exynos SoC USB controller
>> +
>> +The USB devices interface with USB controllers on Exynos SOCs.
>> +The device node has following properties.
>> +
>> +EHCI
>> +Required properties:
>> + - compatible: should be "samsung,exynos4210-ehci" for USB 2.0
>> + EHCI controller in host mode.
>> + - reg: physical base address of the controller and length of memory mapped
>> + region.
>> + - interrupts: interrupt number to the cpu.
>> +
>> +Optional properties:
>> + - samsung,vbus-gpio: if present, specifies the GPIO that
>> + needs to be pulled up for the bus to be powered.
>> +
>> +Example:
>> +
>> + usb@1211 {
>> + compatible = "samsung,exynos4210-ehci";
>> + reg = <0x1211 0x100>;
>> + interrupts = <0 71 0>;
>> + samsung,vbus-gpio = <&gpx2 6 1 3 3>;
>> + };
>> diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
>> b/arch/arm/boot/dts/exynos5250-smdk5250.dts
>> index 711b55f..f990086 100644
>> --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
>> +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
>> @@ -218,4 +218,8 @@
>> i2s_2: i2s@12D7 {
>> status = "disabled";
>> };
>> +
>> + usb@1211 {
>> + samsung,vbus-gpio = <&gpx2 6 1 3 3>;
>> + };
>> };
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>> b/arch/arm/boot/dts/exynos5250.dtsi
>> index 581e57a..584bb9a 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -299,6 +299,12 @@
>> rx-dma-channel = <&pdma0 11>; /* preliminary */
>> };
>>
>> + usb@1211 {
>> + compatible = "samsung,exynos4210-ehci";
>> + reg = <0x1211 0x100>;
>> + interrupts = <0 71 0>;
>> + };
>> +
>> amba {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> diff --git a/arch/arm/mach-exynos/include/mach/map.h
>> b/arch/arm/mach-exynos/include/mach/map.h
>> index cbb2852..b2c662f 100644
>> --- a/arch/arm/mach-exynos/include/mach/map.h
>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>> @@ -201,6 +201,7 @@
>> #define EXYNOS4_PA_EHCI 0x1258
>> #define EXYNOS4_PA_OHCI 0x1259
>> #define EXYNOS4_PA_HSPHY 0x125B
>> +#define EXYNOS5_PA_EHCI 0x1211
>> #define EXYNOS4_PA_MFC 0x1340
>>
>> #define EXYNOS4_PA_UART 0x1380
>> diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c
>> b/arch/arm/mach-exynos/mach-exynos5-dt.c
>> index 462e5ac..b3b9af1 100644
>> --- a/arch/arm/mach-exynos/mach-exynos5-dt.c
>> +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
>> @@ -110,6 +110,8 @@ static const struct of_dev_auxdata
>> exynos5250_auxdata_lookup[] __initconst = {
>> "samsung-i2s.1", NULL),
>> OF_DEV_AUXDATA("samsung,samsung-i2s", 0x12D7,
>> "samsung-i2s.2", NULL),
>> + OF_DEV_AUXDATA("samsung,exynos4210-ehci", EXYNOS5_PA_EHCI,
>> + "s5p-ehci", NULL),
>
> I'm assuming the above change is temporary. What is left to be done to
> drop the auxdata in theses two patches?
>
> Otherwise the patch looks fine.
>
> Acked-by: Grant Likely
> --
> 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/
--
Thanks & Regards
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ARM: Exynos5250: Enabling dwc3-exynos driver
CC: Doug Anderson
On Thu, Dec 13, 2012 at 10:17 PM, Vivek Gautam wrote:
> Adding DWC3 device tree node for Exynos5250 along with the
> device address and clock support needed for the controller.
>
> Signed-off-by: Vivek Gautam
> ---
> Changes from v2:
> - Changed the compatible string to chip specific(samsung,exynos5250),
>since dwc3-exynos is being used from exynso5250 onwards.
> - Based on changes for USB 2.0:
>
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-December/024413.html
>
> Changes from v1:
> - Changed the device node name from 'dwc3' to 'usb@1200'.
> - Added the documentation for device tree bindings for dwc3 controller.
>
>
> .../devicetree/bindings/usb/exynos-usb.txt | 14 +++
> arch/arm/boot/dts/exynos5250.dtsi |6 +
> arch/arm/mach-exynos/Kconfig |1 +
> arch/arm/mach-exynos/clock-exynos5.c | 24
>
> arch/arm/mach-exynos/include/mach/map.h|1 +
> arch/arm/mach-exynos/mach-exynos5-dt.c |2 +
> 6 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
> b/Documentation/devicetree/bindings/usb/exynos-usb.txt
> index f66fcdd..d660410 100644
> --- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
> @@ -38,3 +38,17 @@ Example:
> reg = <0x1212 0x100>;
> interrupts = <0 71 0>;
> };
> +
> +DWC3
> +Required properties:
> + - compatible: should be "samsung,exynos5250-dwc3" for USB 3.0 DWC3
> controller.
> + - reg: physical base address of the controller and length of memory mapped
> + region.
> + - interrupts: interrupt number to the cpu.
> +
> +Example:
> + usb@1200 {
> + compatible = "samsung,exynos5250-dwc3";
> + reg = <0x1200 0x1>;
> + interrupts = <0 72 0>;
> + };
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> index 75510d1..001a31b 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -299,6 +299,12 @@
> rx-dma-channel = <&pdma0 11>; /* preliminary */
> };
>
> + usb@1200 {
> + compatible = "samsung,exynos5250-dwc3";
> + reg = <0x1200 0x1>;
> + interrupts = <0 72 0>;
> + };
> +
> usb@1211 {
> compatible = "samsung,exynos4210-ehci";
> reg = <0x1211 0x100>;
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 91d5b6f..09f9587 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -426,6 +426,7 @@ config MACH_EXYNOS5_DT
> depends on ARCH_EXYNOS5
> select ARM_AMBA
> select USE_OF
> + select USB_ARCH_HAS_XHCI
> help
> Machine support for Samsung EXYNOS5 machine with device tree
> enabled.
> Select this if a fdt blob is available for the EXYNOS5 SoC based
> board.
> diff --git a/arch/arm/mach-exynos/clock-exynos5.c
> b/arch/arm/mach-exynos/clock-exynos5.c
> index 5c63bc7..f2214a0 100644
> --- a/arch/arm/mach-exynos/clock-exynos5.c
> +++ b/arch/arm/mach-exynos/clock-exynos5.c
> @@ -768,6 +768,11 @@ static struct clk exynos5_init_clocks_off[] = {
> .enable = exynos5_clk_ip_fsys_ctrl ,
> .ctrlbit= (1 << 18),
> }, {
> + .name = "usbdrd30",
> + .parent = &exynos5_clk_aclk_200.clk,
> + .enable = exynos5_clk_ip_fsys_ctrl,
> + .ctrlbit= (1 << 19),
> + }, {
> .name = "usbotg",
> .enable = exynos5_clk_ip_fsys_ctrl,
> .ctrlbit= (1 << 7),
> @@ -1121,6 +1126,16 @@ static struct clksrc_sources exynos5_clkset_group = {
> .nr_sources = ARRAY_SIZE(exynos5_clkset_group_list),
> };
>
> +struct clk *exynos5_clkset_usbdrd30_list[] = {
> + [0] = &exynos5_clk_mout_mpll.clk,
> + [1] = &exynos5_clk_mout_cpll.clk,
> +};
> +
> +struct clksrc_sources exynos5_clkset_usbdrd30 = {
> + .sources= exynos5_clkset_usbdrd30_list,
> + .nr_sources = ARRAY_SIZE(exynos5_clkset_usbdrd30_list),
> +};
> +
> /* Possible clock sources for aclk_266_gscl_sub Mux */
> static struct clk *clk_src_gscl_266_list[] = {
> [0] = &clk_ext_xtal_mux,
> @@ -1415,6 +1430,15 @@ static struct clksrc_clk exynos5_clksrcs[] = {
> .parent = &exynos5_clk_mout_cpll.clk,
> },
> .reg_div = { .reg = EXYNOS5_CLKDIV_GEN, .shift = 4, .size = 3
> },
> + }, {
> + .clk= {
> + .name = "sclk_usbdrd30",
> + .enable
[PATCH] USB: option: blacklist network interface on ZTE MF880
The driver description files gives these names to the vendor specific
functions on this modem:
diag: VID_19D2&PID_0284&MI_00
nmea: VID_19D2&PID_0284&MI_01
at: VID_19D2&PID_0284&MI_02
mdm: VID_19D2&PID_0284&MI_03
net: VID_19D2&PID_0284&MI_04
Signed-off-by: Bjørn Mork
---
drivers/usb/serial/option.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index e6f87b7..f8e1165 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -923,7 +923,8 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0257, 0xff, 0xff,
0xff), /* ZTE MF821 */
.driver_info = (kernel_ulong_t)&net_intf3_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0265, 0xff, 0xff,
0xff) },
- { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0284, 0xff, 0xff,
0xff) },
+ { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0284, 0xff, 0xff,
0xff), /* ZTE MF880 */
+ .driver_info = (kernel_ulong_t)&net_intf4_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0317, 0xff, 0xff,
0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0326, 0xff, 0xff,
0xff),
.driver_info = (kernel_ulong_t)&net_intf4_blacklist },
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net] net: qmi_wwan: add ZTE MF880
The driver description files gives these names to the vendor specific
functions on this modem:
diag: VID_19D2&PID_0284&MI_00
nmea: VID_19D2&PID_0284&MI_01
at: VID_19D2&PID_0284&MI_02
mdm: VID_19D2&PID_0284&MI_03
net: VID_19D2&PID_0284&MI_04
Signed-off-by: Bjørn Mork
---
drivers/net/usb/qmi_wwan.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 9b950f5..91d7cb9 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -433,6 +433,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x19d2, 0x0199, 1)},/* ZTE MF820S */
{QMI_FIXED_INTF(0x19d2, 0x0200, 1)},
{QMI_FIXED_INTF(0x19d2, 0x0257, 3)},/* ZTE MF821 */
+ {QMI_FIXED_INTF(0x19d2, 0x0284, 4)},/* ZTE MF880 */
{QMI_FIXED_INTF(0x19d2, 0x0326, 4)},/* ZTE MF821D */
{QMI_FIXED_INTF(0x19d2, 0x1008, 4)},/* ZTE (Vodafone) K3570-Z */
{QMI_FIXED_INTF(0x19d2, 0x1010, 4)},/* ZTE (Vodafone) K3571-Z */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Remove CONFIG_USB_SUSPEND?
Hi, I just subscribed to the linux-usb list as an occasional user ... I glanced through this thread backwards ... and am somehow surprised nobody objects. I am no expert but it is my impression a lot of us want CPU powersaving, maybe LCD powersaving and don't care about external USB devices. It is only causing a hassle if the mouse goes sleep every 2 seconds or first keyboard keystroke goes away because it just woke up the keyboard. Even worse if a usb-storage device goes sleep and kernel chokes on subsequent file access. Sure, it is because of broken devices, firmware, sometimes linux ... but why do I have to sacrifice all power-saving to avoid potential issues. I just do not want to hit new bugs here or there in powersaving features in USB devices, really. They are cheap, crappy and I accepted it. But please, don't make me either use their power-saving or disable powersaving altogether. I believe you know very well what in kernel is affected and if you say few lines of code are affected, but do you really want laptop-mode-tools and all other to change their documentation, config files and make all the whitelists and blacklists in their config files useless? If I get it right I either use the tool as a whole or not at all? Really? I must be missing something. sorry for my incompetence taht I am not so knowledgeable of power-saving in general. Am just a 'looser'. ;) Martin Greg KH wrote: > On Tue, Dec 18, 2012 at 11:11:15AM -0500, Alan Stern wrote: >> I suggest that we remove the CONFIG_USB_SUSPEND option, starting in >> 3.9. Practically everyone enables it, and the amount of code it >> protects is fairly small (just portions of usbcore, nothing in the >> drivers). >> >> Basically, if people don't want their kernels to save power then they >> should turn off CONFIG_PM. >> >> Objections, anyone? > > None from me. > > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.7 kernel hangs when doing scp
Hi Greg, On Tue, Dec 18, 2012 at 12:43 AM, Greg KH wrote: > On Tue, Dec 18, 2012 at 12:01:35AM -0200, Fabio Estevam wrote: >> Hi, >> >> Booting a 3.7 kernel on a mx6qsabrelite board via NFS and using an >> asix USB/Ethernet adapter and then I try to transfer a 10MB file from >> the target to the host: > > Has this ever worked before on any older kernel? We did more experiments on this and we noticed that: - mx51 running kernel 3.7 does not show this problem (tested a non-dt mx51 kernel, which uses drivers/usb/host/ehci-mxc.c instead of the chipidea driver.) - x86 running 3.2 kernel does not show this problem - usb support on mx6 has appeared on 3.6, but running an out-of-tree 3.0.35 kernel also shows the same issue. - Doing long file transfers via mass storage on mx6 running 3.7 works fine. Shawn/Sascha/Peter, Any suggestions as to how debug this? By looking at the kernel dumps it seems to point to 'gic_handle_irq' area. Thanks, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] drivers/usb/host/ohci* : set urb->hcpriv = NULL immediately, after free it
On Wed, 19 Dec 2012, Chen Gang wrote: > although we can not say it is surely a bug. > it is better to set urb->hcpriv = NULL, after finish calling urb_free_priv. > > > Signed-off-by: Chen Gang > --- > drivers/usb/host/ohci-q.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c > index 7482cfb..88731b7 100644 > --- a/drivers/usb/host/ohci-q.c > +++ b/drivers/usb/host/ohci-q.c > @@ -44,6 +44,7 @@ __acquires(ohci->lock) > // ASSERT (urb->hcpriv != 0); > > urb_free_priv (ohci, urb->hcpriv); > + urb->hcpriv = NULL; > if (likely(status == -EINPROGRESS)) > status = 0; > Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Suggestion] drivers/usb/host/uhci* : sprintf, need check len when use buf
On Wed, 19 Dec 2012, Chen Gang wrote: > Hello Alan Stern > > I finished constructing envrionments. > let uhci-debug.c has effect (#define DEBUG, debug = 3 in > drivers/usb/host/uhci-hcd.c) > build kernel and install, and restart machine. > can cat /sys/kernel/debug/usb/uhci/* to get full display contents > > for fixing this issue of uhci-debug.c, > > design: > A) I will let all are according to usb_device_dump in > drivers/usb/core/devices.c >it has effect for all sub call functions (uhci_show_td, > uhci_show_urbp...). > for originally, length judging of uhci_show_urbp seems need > improvement. >it needs line >= 200 (line 104) >it call uhci_show_td in a loop (line 132..144) >uhci_show_td nees line >= 160 ! > so I prefer to touch all sub functions. >if you have another suggestions, please tell me. > > B) I will not touch "original looping within 10 times" (if > 10 will be > skipped) >in function uhci_show_urbp line 132..148 >in function uhci_show_qh line 213..222 >in function uhci_sprint_schedule > line 460..470, > for nframes line 381..433. >if you think it is necessary to remove them, please tell me. > > test: > A) let MAX_OUTPUT as various values: >one value is enough for use, then can get full contents. >test 3 various values which small enough to let output truncate in > various location. > B) check the result: >I will cat the contents from the /sys/kernel/debug/usb/uhci/* >use wc -c to get the count of output contents, then judge whether > match the MAX_OUTPUT. > if you have additional completions, please tell me. > > > I will start the implementation when get your reply. Your plan sounds okay to me. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012, Lan Tianyu wrote:
> Hi Alan:
>
> How about this patch?
>
> Index: linux-pm/drivers/usb/host/ohci-pci.c
> ===
> --- linux-pm.orig/drivers/usb/host/ohci-pci.c 2012-11-01
> 18:21:33.604460469 +0800
> +++ linux-pm/drivers/usb/host/ohci-pci.c2012-12-19
> 14:39:07.081601806 +0800
> @@ -188,6 +188,15 @@
> pci_write_config_word(pdev, 0x50, misc | 0x0300);
> }
>
> +static int ohci_quirk_bad_wakeup(struct usb_hcd *hcd)
> +{
> + struct ohci_hcd *ohci = hcd_to_ohci (hcd);
> +
> + ohci_dbg(ohci, "marked as bad wakeup.\n");
I'd prefer the message to be something more like "enabled nVidia/SiS
wakeup quirk".
> + hcd->bad_wakeup = true;
> + return 0;
> +}
> +
> /* List of quirks for OHCI */
> static const struct pci_device_id ohci_pci_quirks[] = {
> {
> @@ -238,6 +247,31 @@
> PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
> .driver_data = (unsigned long)ohci_quirk_amd700,
> },
> + {
> + /* MCP51 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x026d),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> + {
> + /* MCP61 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x03f1),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> + {
> + /* MCP79 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0aa5),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> + {
> + /* MCP79 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0aa7),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
Since we don't know of any nVidia controllers that function correctly,
you might as well match any product ID.
> + {
> + /* SiS OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_SI, 7001),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
>
> /* FIXME for some of the early AMD 760 southbridges, OHCI
> * won't work at all. blacklist them.
> Index: linux-pm/include/linux/usb/hcd.h
> ===
> --- linux-pm.orig/include/linux/usb/hcd.h 2012-11-01
> 18:21:34.732460451 +0800
> +++ linux-pm/include/linux/usb/hcd.h2012-12-19 10:48:43.305822774 +0800
> @@ -138,6 +138,7 @@
> resource_size_t rsrc_start; /* memory/io resource
> start */
> resource_size_t rsrc_len; /* memory/io resource
> length */
> unsignedpower_budget; /* in mA, 0 = no limit */
> + boolbad_wakeup;
This should be a bitflag (i.e., bad_wakeup:1) and it should come
immediately after has_tt:1.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug. Can't read uvcvideo (usb video / webcams) devices
On Wed, 19 Dec 2012, Егор Орлов wrote: > Alan, thank you for your reply. May be in the future this bug will be > fixed. I understand it's not a simple bug. I will not send any > messages concerning this bug anymore. > > When I tested the camera on other two machines running Ubuntu it > showed the same log message but it worked: > not running at top speed; connect to a high speed hub The difference is those other two machines have a non-EHCI host controller (either UHCI or OHCI). Your first machine has only EHCI. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] usb: add usb port auto power off mechanism
On Wed, 19 Dec 2012, lantianyu wrote: > >> I just find busy_bits is set or clear in the usb_port_resume() and > >> usb_reset_and_verify_device(). So currently we should keep my changes > >> mutually exclusive with them, right? > > > > Don't forget about what happens when a device is removed. > I am not very clear about this since busy_bits is not changed during device > being > removed. Could you elaborate? Thanks. What happens if the device is unplugged while some thread is using busy_bits? Will the hub driver realize that the device is gone? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012, lantianyu wrote: > Oh. I forget to mention the issue also takes place on the uhci. > https://bugzilla.kernel.org/show_bug.cgi?id=42721 > So we also should make such a patch for uhci. That bug report shows clearly that it is a software problem or a device problem, not an error in the UHCI controller hardware. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2]linux-usb:optimize to match the Huawei USB storage devices and support new switch command
On Wed, Dec 19, 2012 at 12:34 AM, Sebastian Andrzej Siewior wrote: > On Wed, Dec 19, 2012 at 03:13:32AM +, Fangxiaozhi (Franko) wrote: >> > And shouldn't you read something from the us->recv_bulk_pipe after >> > that? >> Well, because our device will re-connect to switch the ports if it >> receives the command. >> So it is not necessary to read the response of the command. > > Hmm. I guess this for Matthew / Greg to decide, I don't insist on anything. > Maybe a comment would be nice because now it looks, atleast to me, that > something is missing. I think an unusual situation like that deserves a comment that explains that the device is about to disconnect / reconnect, so reading status is not necessary. I am also concerned about the error of using &bcbw instead of bcbw. I doubt this code would have worked with that typo in place. How was this patch tested? Also, the dongles_pid function is really just a different implementation of the unusual_devs.h table. I think that it is much easier for people to add new entries to the table, rather than edit your code, when new dongles are released. BUT, your code includes many more PIDs than the table did. Again, how was this tested for the new PIDs covered? At a minimum, some comment in dongles_pid is required to highlight this area of code for possible future expansion as new devices are released. Matt -- Matthew Dharm Maintainer, USB Mass Storage driver for Linux -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Remove CONFIG_USB_SUSPEND?
On Wed, 19 Dec 2012, Martin Mokrejs wrote: > Hi, > I just subscribed to the linux-usb list as an occasional user ... > I glanced through this thread backwards ... and am somehow surprised > nobody objects. I am no expert but it is my impression a lot of us > want CPU powersaving, maybe LCD powersaving and don't care about > external USB devices. It is only causing a hassle if the mouse goes > sleep every 2 seconds or first keyboard keystroke goes away because > it just woke up the keyboard. Even worse if a usb-storage device goes > sleep and kernel chokes on subsequent file access. Sure, it is because > of broken devices, firmware, sometimes linux ... but why do I have to > sacrifice all power-saving to avoid potential issues. I just do not want > to hit new bugs here or there in powersaving features in USB devices, > really. They are cheap, crappy and I accepted it. But please, don't > make me either use their power-saving or disable powersaving altogether. > I believe you know very well what in kernel is affected and if you say > few lines of code are affected, but do you really want laptop-mode-tools > and all other to change their documentation, config files and make > all the whitelists and blacklists in their config files useless? If I > get it right I either use the tool as a whole or not at all? Really? > > I must be missing something. sorry for my incompetence taht I am not so > knowledgeable of power-saving in general. Am just a 'looser'. ;) Don't worry -- asking questions like this is how you learn. Yes, you are indeed missing something. I didn't propose that the USB power-saving code be removed from the kernel. Rather, I suggested that the Kconfig option be removed so that it will no longer be possible to build a kernel _without_ the power-saving code. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Remove CONFIG_USB_SUSPEND?
On Wed, Dec 19, 2012 at 03:24:34PM +0100, Martin Mokrejs wrote: > Hi, > I just subscribed to the linux-usb list as an occasional user ... > I glanced through this thread backwards ... and am somehow surprised > nobody objects. I am no expert but it is my impression a lot of us > want CPU powersaving, maybe LCD powersaving and don't care about > external USB devices. It is only causing a hassle if the mouse goes > sleep every 2 seconds or first keyboard keystroke goes away because > it just woke up the keyboard. Even worse if a usb-storage device goes > sleep and kernel chokes on subsequent file access. Sure, it is because > of broken devices, firmware, sometimes linux ... Do you currently have this problem with your devices? Which ones are they, as we need to know in order to get this fixed. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ehci: make debug port in-use detection functional again
Debug port in-use determination must be done before the controller gets
reset the first time, i.e. before the call to ehci_setup() as of commit
1a49e2ac9651df7349867a5cf44e2c83de1046af. That commit effectively
rendered commit 9fa5780beea1274d498a224822397100022da7d4 useless.
While moving that code around, also fix the BAR determination - the
respective capability field is a 3- rather than a 2-bit one -, and use
PCI_CAP_ID_DBG instead of the literal 0x0a.
It's unclear to me whether the debug port functionality is important
enough to warrant fixing this in stable kernels too.
Signed-off-by: Jan Beulich
Cc: Alan Stern
Cc: Konrad Rzeszutek Wilk
---
drivers/usb/host/ehci-pci.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
--- 3.7/drivers/usb/host/ehci-pci.c
+++ 3.7-ehci-init-order/drivers/usb/host/ehci-pci.c
@@ -192,6 +192,26 @@ static int ehci_pci_setup(struct usb_hcd
break;
}
+ /* optional debug port, normally in the first BAR */
+ temp = pci_find_capability(pdev, PCI_CAP_ID_DBG);
+ if (temp) {
+ pci_read_config_dword(pdev, temp, &temp);
+ temp >>= 16;
+ if (((temp >> 13) & 7) == 1) {
+ u32 hcs_params = ehci_readl(ehci,
+ &ehci->caps->hcs_params);
+
+ temp &= 0x1fff;
+ ehci->debug = hcd->regs + temp;
+ temp = ehci_readl(ehci, &ehci->debug->control);
+ ehci_info(ehci, "debug port %d%s\n",
+ HCS_DEBUG_PORT(hcs_params),
+ (temp & DBGP_ENABLED) ? " IN USE" : "");
+ if (!(temp & DBGP_ENABLED))
+ ehci->debug = NULL;
+ }
+ }
+
retval = ehci_setup(hcd);
if (retval)
return retval;
@@ -226,25 +246,6 @@ static int ehci_pci_setup(struct usb_hcd
break;
}
- /* optional debug port, normally in the first BAR */
- temp = pci_find_capability(pdev, 0x0a);
- if (temp) {
- pci_read_config_dword(pdev, temp, &temp);
- temp >>= 16;
- if ((temp & (3 << 13)) == (1 << 13)) {
- temp &= 0x1fff;
- ehci->debug = hcd->regs + temp;
- temp = ehci_readl(ehci, &ehci->debug->control);
- ehci_info(ehci, "debug port %d%s\n",
- HCS_DEBUG_PORT(ehci->hcs_params),
- (temp & DBGP_ENABLED)
- ? " IN USE"
- : "");
- if (!(temp & DBGP_ENABLED))
- ehci->debug = NULL;
- }
- }
-
/* at least the Genesys GL880S needs fixup here */
temp = HCS_N_CC(ehci->hcs_params) * HCS_N_PCC(ehci->hcs_params);
temp &= 0x0f;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012 07:29:23 -0800, Alan Stern wrote: + ohci_dbg(ohci, "marked as bad wakeup.\n"); I'd prefer the message to be something more like "enabled nVidia/SiS wakeup quirk". To me, the stupid end-user, both messages are useless. I don't know that that means or implies. I would go with: "Disabled OHCI wakeup (USB) due to faulty controller (no-wakeup.txt)" and have a file named no-wakeup.txt under Documentation with this: " Users have reported OHCI misbehavior consisting on false wakeups right after suspend to RAM on some OHCI controllers, particularly from nVIDIA and SiS. For those controllers, wakeups has been disabled. The system will not be able to wake up the system from suspend to RAM from an OHCI (USB) device. To see the list of affected controllers do: grep -B 3 ohci_quirk_bad_wakeup linux-pm/drivers/usb/host/ohci-pci.c Bug is tracked at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677472 " -- Octavio. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Remove CONFIG_USB_SUSPEND?
Greg KH wrote: > On Wed, Dec 19, 2012 at 03:24:34PM +0100, Martin Mokrejs wrote: >> Hi, >> I just subscribed to the linux-usb list as an occasional user ... >> I glanced through this thread backwards ... and am somehow surprised >> nobody objects. I am no expert but it is my impression a lot of us >> want CPU powersaving, maybe LCD powersaving and don't care about >> external USB devices. It is only causing a hassle if the mouse goes >> sleep every 2 seconds or first keyboard keystroke goes away because >> it just woke up the keyboard. Even worse if a usb-storage device goes >> sleep and kernel chokes on subsequent file access. Sure, it is because >> of broken devices, firmware, sometimes linux ... > > Do you currently have this problem with your devices? Which ones are > they, as we need to know in order to get this fixed. Oh, thanks but luckily not. The missed first keystroke was about 3.2/3.3 series and I know I was not the only one. I don't have the problem now, with recent kernels. And the mouse going sleep ... I think it is my ignorance to configure kernel on the fly, or use the laptop-mode-tools to do it. I just haven't settled on a single power management tool. cpufreutils is now obsolete, I have some issues with laptop-mode-tools ... but, no, no kernel bug to report at the moment. Don't worry. ;-) I believe I can prevent mouse falling a sleep once I find where do I have to send the echo with it's USB ID if I don't go the laptop-mode-tools direction. But it is only an issue for me when on battery, so not urgent. Martin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ehci: make debug port in-use detection functional again
On Wed, 19 Dec 2012, Jan Beulich wrote: > Debug port in-use determination must be done before the controller gets > reset the first time, i.e. before the call to ehci_setup() as of commit > 1a49e2ac9651df7349867a5cf44e2c83de1046af. That commit effectively > rendered commit 9fa5780beea1274d498a224822397100022da7d4 useless. Quite right, thank you for finding and fixing this. > While moving that code around, also fix the BAR determination - the > respective capability field is a 3- rather than a 2-bit one -, and use > PCI_CAP_ID_DBG instead of the literal 0x0a. > > It's unclear to me whether the debug port functionality is important > enough to warrant fixing this in stable kernels too. I think we should. Greg, please queue this for all stable kernels that include the 1a49e2ac commit. > Signed-off-by: Jan Beulich > Cc: Alan Stern > Cc: Konrad Rzeszutek Wilk Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012, Octavio Alvarez wrote: > On Wed, 19 Dec 2012 07:29:23 -0800, Alan Stern > wrote: > >> + ohci_dbg(ohci, "marked as bad wakeup.\n"); > > > > I'd prefer the message to be something more like "enabled nVidia/SiS > > wakeup quirk". > > To me, the stupid end-user, both messages are useless. I don't know You, the stupid end-user, would not see this message at all under normal circumstances. It uses the ohci_dbg macro and therefore will not appear unless your kernel is built with CONFIG_USB_DEBUG enabled. > that that means or implies. I would go with: > "Disabled OHCI wakeup (USB) due to faulty controller (no-wakeup.txt)" > > and have a file named no-wakeup.txt under Documentation with this: > > " > Users have reported OHCI misbehavior consisting on false wakeups right > after suspend to RAM on some OHCI controllers, particularly from nVIDIA > and SiS. For those controllers, wakeups has been disabled. > > The system will not be able to wake up the system from suspend > to RAM from an OHCI (USB) device. > > To see the list of affected controllers do: > > grep -B 3 ohci_quirk_bad_wakeup linux-pm/drivers/usb/host/ohci-pci.c > > Bug is tracked at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677472 > " It wouldn't hurt to include a URL for the bug report in a comment. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012 08:57:00 -0800, Alan Stern wrote: You, the stupid end-user, would not see this message at all under normal circumstances. It uses the ohci_dbg macro and therefore will not appear unless your kernel is built with CONFIG_USB_DEBUG enabled. Shouldn't it be exposed to dmesg? -- Octavio. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ehci: make debug port in-use detection functional again
On Wed, Dec 19, 2012 at 11:52:42AM -0500, Alan Stern wrote: > On Wed, 19 Dec 2012, Jan Beulich wrote: > > > Debug port in-use determination must be done before the controller gets > > reset the first time, i.e. before the call to ehci_setup() as of commit > > 1a49e2ac9651df7349867a5cf44e2c83de1046af. That commit effectively > > rendered commit 9fa5780beea1274d498a224822397100022da7d4 useless. > > Quite right, thank you for finding and fixing this. > > > While moving that code around, also fix the BAR determination - the > > respective capability field is a 3- rather than a 2-bit one -, and use > > PCI_CAP_ID_DBG instead of the literal 0x0a. > > > > It's unclear to me whether the debug port functionality is important > > enough to warrant fixing this in stable kernels too. > > I think we should. Greg, please queue this for all stable kernels that > include the 1a49e2ac commit. Ok, will do. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation
On 12/18/2012 10:38 PM, Venu Byravarasu wrote: >> -Original Message- >> From: Stephen Warren [mailto:swar...@wwwdotorg.org] >> Sent: Tuesday, December 18, 2012 10:03 PM >> To: Venu Byravarasu >> Cc: ba...@ti.com; gre...@linuxfoundation.org; linux- >> ker...@vger.kernel.org; linux-usb@vger.kernel.org >> Subject: Re: [PATCH] usb: phy: tegra: Using devm API for memory allocation >> >> On 12/17/2012 11:21 PM, Venu Byravarasu wrote: >>> Using devm_kzalloc for allocating memory needed for PHY >>> pointer and hence removing kfree calls to PHY pointer. >> >> Since the kfree() here used to be in tegra_usb_phy_close() rather than >> any remove() function, does it actually make sense to use >> devm_kzalloc(); would plain using kzalloc() instead, and not removing >> the kfree() calls, be better? >> > > Stephen, > As you mentioned I can replace kmalloc with kzalloc in the original code > and push an updated patch. > However, I just wanted to understand if there exists any issue > in using devm_kzalloc instead of kzalloc? devm_* are intended for objects allocated during probe(), and free()d during remove(). The object you're allocating here isn't that case. Now, once you convert the Tegra PHY driver to be a true device, perhaps this object will be allocated/freed during probe/remove, so the devm_ functions will be useful then? The problem this may cause is a memory leak. Consider the Tegra EHCI and PHY drivers being built as modules, the PHY driver module being inserted and never removed, yet the EHCI driver being continually inserted and removed. Since the PHY is never removed, the memory allocated by its devm_kzalloc() call is never freed, but it's continually re-allocated since tegra_usb_phy_open() is called whenever the EHCI driver module is inserted. You need the explicit kfree() to avoid that, and since you're kfree()ing somewhere other than remove(), using devm_* to make the allocation isn't appropriate. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012, Octavio Alvarez wrote: > On Wed, 19 Dec 2012 08:57:00 -0800, Alan Stern > wrote: > > > You, the stupid end-user, would not see this message at all under > > normal circumstances. It uses the ohci_dbg macro and therefore will > > not appear unless your kernel is built with CONFIG_USB_DEBUG enabled. > > Shouldn't it be exposed to dmesg? None of the other quirk messages are. On the other hand, they don't affect the behavior as much as this quirk does. Still, if we do produce a message about it, I don't feel it is necessary to try and explain the whole situation. Something as simple as "Buggy wakeup support, disabling" should be enough for an end-user who wants to know why typing on the USB keyboard doesn't wake up a suspended system. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
Am 19.12.2012 16:29, schrieb Alan Stern:
> On Wed, 19 Dec 2012, Lan Tianyu wrote:
...
> /* List of quirks for OHCI */
> static const struct pci_device_id ohci_pci_quirks[] = {
> {
> @@ -238,6 +247,31 @@
> PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
> .driver_data = (unsigned long)ohci_quirk_amd700,
> },
> + {
> + /* MCP51 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x026d),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> + {
> + /* MCP61 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x03f1),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> + {
> + /* MCP79 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0aa5),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> + {
> + /* MCP79 OHCI */
> + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0aa7),
> + .driver_data = (unsigned long)ohci_quirk_bad_wakeup,
> + },
> Since we don't know of any nVidia controllers that function correctly,
> you might as well match any product ID.
>
Some more NVIDIA OHCI controllers:
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0067)/* nForce2 */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x01c2)/* nForce */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x036c)/* MCP55 */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0454)/* MCP65 */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x055e)/* MCP67 */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x077b)/* MCP78S */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x077d)/* MCP78S */
PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0d9c)/* MCP89 */
I can confirm that MCP55 has this bug and it should be safe to add
MCP65-78S, too, because MCP79 still has the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012, [ISO-8859-1] Frank Sch�fer wrote: > I can confirm that MCP55 has this bug and it should be safe to add > MCP65-78S, too, because MCP79 still has the bug. By the way, you mentioned that runtime suspend seemed to work okay, right? It might be worthwhile testing this again, just to be certain. In particular, I'd like to see what "lspci -vv" shows for the controller while it is runtime suspended with a device attached. As far as the OHCI hardware is concerned, there shouldn't be any difference between runtime suspend and system suspend. This strongly suggests that the bug doesn't lie in the controller itself but in the firmware (BIOS or ACPI). Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: samsung: Add support to set pmu isolation
Hi,
On 12/19/2012 02:44 PM, Vivek Gautam wrote:
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,15 @@ Required properties:
- compatible : should be "samsung,exynos4210-usbphy"
- reg : base physical address of the phy registers and length of memory
mapped
region.
+
+Optional properties:
+- samsung,usb-phyhandle : should point to usb-phyhandle sub-node which
provides
+ binding data to enable/disable device PHY handled
by
+ PMU register.
+
+ Required properties:
+ - compatible : should be "samsung,usbdev-phyctrl"
for
+ DEVICE type phy.
+ - samsung,phyhandle-reg: base physical address of
+ PHY_CONTROL register in
PMU.
+- samsung,enable-mask : should be '1'
This should only be 1 for Exynos4210+ SoCs, right ?
Yes that's true Exynso4210+ SoCs have only single PHY for both host and device
which gets enabled by single bit.
S5PV210 uses bit 0 for OTG and bit1 for USB host, doesn't it ? And for
s3c64xx
it seems to be bit 16.
True, S5PV210 uses two bits separately for OTG and HOST, so in that
case we would
require to set both these bits. Thanks for pointing out !!
I couldn't see device tree support for S5PV210 and S3C64xx so thought
of going for
4210+ SoCs. Better we make this more generic so that once these SoCs
are moved to
device tree we don't face any issue. Right ??
Fair enough. Yes, let's not make any future addition of the device tree
support for the older Samsung platforms unnecessarily difficult. It doesn't
take much effort, and there is many drivers that are shared by multiple
SoCs
already, starting from s3c64xx to exynos4 series.
How about deriving this information from 'compatible' property instead ?
It will definitely be good to use the compatible property to derive
such information,
Need to amend the current approach.
Maybe you could just encode the USB PMU registers (I assume those aren't
touched by anything but the usb drivers) in a regular 'reg' property in
an usbphy subnode. Then the driver could interpret it also with help
of 'compatible' property. And you could just use of_iomap(). E.g.
usbphy@1213 {
compatible = "samsung,exynos5250-usbphy";
reg =<0x1213 0x100>,<0x1210 0x100>;
usbphy-pmu {
/* USB device and host PHY_CONTROL registers */
reg =<0x10040704 8>;
};
};
This approach seems nice.
Your "samsung,usb-phyhandle" approach seems over-engineered to me.
I might be missing something though.
The idea behind using phandles for usb-phy was to get the multiple
registers (2 in PMU
and 1 in SYSREG) and program them separately as required.
You could still specify multiple pairs in 'reg' property
or perhaps use separate node for SYSREG. And if on some SoCs PHY_CONTROL
registers do not immediately follow each other in memory it might make
sense to define the bindings so that each register is specified separately,
e.g.
reg = <0x10040704 4>, <0x10040708 4>, <0x10050230 4>;
However AFAICS single region for the PHY_CONTROL registers should be
sufficient for all existing SoCs.
--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net] net: qmi_wwan: add ZTE MF880
From: Bjørn Mork Date: Wed, 19 Dec 2012 15:15:51 +0100 > The driver description files gives these names to the vendor specific > functions on this modem: > > diag: VID_19D2&PID_0284&MI_00 > nmea: VID_19D2&PID_0284&MI_01 > at: VID_19D2&PID_0284&MI_02 > mdm: VID_19D2&PID_0284&MI_03 > net: VID_19D2&PID_0284&MI_04 > > Signed-off-by: Bjørn Mork Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: xhci - fix bit definitions for IMAN register
According to XHCI specification (5.5.2.1) the IP is bit 0 and IE is bit 1
of IMAN register. Previously their definitions were reversed.
Signed-off-by: Dmitry Torokhov
---
Sarah,
I did not see any ill effects from using the old definitions (I think
because IMAN_IP is RW1C and in legacy PCI case we come in with it
already set to 1 so it was clearing itself even though we were setting
IMAN_IE instead of IMAN_IP), and the new ones seem to be working as
well.
Thanks,
Dmitry
drivers/usb/host/xhci.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f791bd0..2c510e4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -206,8 +206,8 @@ struct xhci_op_regs {
/* bits 12:31 are reserved (and should be preserved on writes). */
/* IMAN - Interrupt Management Register */
-#define IMAN_IP(1 << 1)
-#define IMAN_IE(1 << 0)
+#define IMAN_IE(1 << 1)
+#define IMAN_IP(1 << 0)
/* USBSTS - USB status - status bitmasks */
/* HC not running - set to 1 when run/stop bit is cleared. */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] ARM: EXYNOS: Update & move usb-phy types to generic include layer
Vivek,
On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam wrote:
> Updating the names of usb-phy types to more generic names:
> USB_PHY_TYPE_DEIVCE & USB_PHY_TYPE_HOST; and further update
> its dependencies.
Since you're changing the name, I would have expected to see a removal
of the old enum type in this patch. I don't see it. After applying
all of your patches I find that S5P_USB_PHY_HOST and
S5P_USB_PHY_DEVICE are still defined and used.
$ git grep S5P_USB_PHY_HOST
arch/arm/mach-exynos/setup-usb-phy.c: else if (type == S5P_USB_PHY_HOST)
arch/arm/mach-exynos/setup-usb-phy.c: else if (type == S5P_USB_PHY_HOST)
arch/arm/plat-samsung/include/plat/usb-phy.h: S5P_USB_PHY_HOST,
I believe that those usages of the old enum should also be switched
over and the old enum removed.
> Signed-off-by: Praveen Paneri
> Signed-off-by: Vivek Gautam
> ---
> drivers/usb/host/ehci-s5p.c |9 +
> drivers/usb/host/ohci-exynos.c |9 +
> include/linux/usb/samsung_usb_phy.h | 16
> 3 files changed, 26 insertions(+), 8 deletions(-)
> create mode 100644 include/linux/usb/samsung_usb_phy.h
>
> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
> index 319dcfa..46ca5ef 100644
> --- a/drivers/usb/host/ehci-s5p.c
> +++ b/drivers/usb/host/ehci-s5p.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #define EHCI_INSNREG00(base) (base + 0x90)
> @@ -164,7 +165,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> }
>
> if (pdata->phy_init)
> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>
> ehci = hcd_to_ehci(hcd);
> ehci->caps = hcd->regs;
> @@ -198,7 +199,7 @@ static int s5p_ehci_remove(struct platform_device *pdev)
> usb_remove_hcd(hcd);
>
> if (pdata && pdata->phy_exit)
> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>
> clk_disable_unprepare(s5p_ehci->clk);
>
> @@ -229,7 +230,7 @@ static int s5p_ehci_suspend(struct device *dev)
> rc = ehci_suspend(hcd, do_wakeup);
>
> if (pdata && pdata->phy_exit)
> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>
> clk_disable_unprepare(s5p_ehci->clk);
>
> @@ -246,7 +247,7 @@ static int s5p_ehci_resume(struct device *dev)
> clk_prepare_enable(s5p_ehci->clk);
>
> if (pdata && pdata->phy_init)
> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>
> /* DMA burst Enable */
> writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index aa3b884..804fb62 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> struct exynos_ohci_hcd {
> @@ -153,7 +154,7 @@ static int exynos_ohci_probe(struct platform_device *pdev)
> }
>
> if (pdata->phy_init)
> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>
> ohci = hcd_to_ohci(hcd);
> ohci_hcd_init(ohci);
> @@ -184,7 +185,7 @@ static int exynos_ohci_remove(struct platform_device
> *pdev)
> usb_remove_hcd(hcd);
>
> if (pdata && pdata->phy_exit)
> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>
> clk_disable_unprepare(exynos_ohci->clk);
>
> @@ -229,7 +230,7 @@ static int exynos_ohci_suspend(struct device *dev)
> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
>
> if (pdata && pdata->phy_exit)
> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>
> clk_disable_unprepare(exynos_ohci->clk);
>
> @@ -249,7 +250,7 @@ static int exynos_ohci_resume(struct device *dev)
> clk_prepare_enable(exynos_ohci->clk);
>
> if (pdata && pdata->phy_init)
> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>
> ohci_resume(hcd, false);
>
> diff --git a/include/linux/usb/samsung_usb_phy.h
> b/include/linux/usb/samsung_usb_phy.h
> new file mode 100644
> index 000..9167826
> --- /dev/null
> +++ b/include/linux/usb/samsung_usb_phy.h
> @@ -0,0 +1,16 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics Co.Ltd
> + * http://www.samsung.com/
> + *
> + * Defines phy types for samsung usb phy controllers - HOST or DEIVCE.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as publish
Re: [PATCH v5 2/4] usb: phy: samsung: Add host phy support to samsung-phy driver
Vivek, I don't really have a good 1 foot view about how all of the USB bits fit together, but a few detail-oriented comments below. On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam wrote: > This patch adds host phy support to samsung-usbphy.c and > further adds support for samsung's exynos5250 usb-phy. > > Signed-off-by: Praveen Paneri > Signed-off-by: Vivek Gautam > --- > .../devicetree/bindings/usb/samsung-usbphy.txt | 25 +- > drivers/usb/phy/Kconfig|2 +- > drivers/usb/phy/samsung-usbphy.c | 465 > ++-- > include/linux/usb/samsung_usb_phy.h| 13 + > 4 files changed, 454 insertions(+), 51 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt > b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt > index a7b28b2..2ec5400 100644 > --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt > +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt > @@ -1,23 +1,38 @@ > * Samsung's usb phy transceiver > > -The Samsung's phy transceiver is used for controlling usb otg phy for > -s3c-hsotg usb device controller. > +The Samsung's phy transceiver is used for controlling usb phy for > +s3c-hsotg as well as ehci-s5p and ohci-exynos usb controllers > +across Samsung SOCs. > TODO: Adding the PHY binding with controller(s) according to the under > developement generic PHY driver. > > Required properties: > + > +Exynos4210: > - compatible : should be "samsung,exynos4210-usbphy" > - reg : base physical address of the phy registers and length of memory > mapped > region. > > +Exynos5250: > +- compatible : should be "samsung,exynos5250-usbphy" > +- reg : base physical address of the phy registers and length of memory > mapped > + region. > + > Optional properties: > - samsung,usb-phyhandle : should point to usb-phyhandle sub-node which > provides > binding data to enable/disable device PHY handled by > - PMU register. > + PMU register; or to configure usb2.0 phy handled by > + SYSREG. > > Required properties: > - compatible : should be "samsung,usbdev-phyctrl" for > - DEVICE type phy. > + DEVICE type phy; or > + should be "samsung,usbhost-phyctrl" for > + HOST type phy; or > + should be "samsung,usb-phycfg" for > + USB2.0 PHY_CFG. > - samsung,phyhandle-reg: base physical address of > - PHY_CONTROL register in PMU. > +PHY_CONTROL register in PMU; > +or USB2.0 PHY_CFG register > +in SYSREG. > - samsung,enable-mask : should be '1' > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > index 17ad743..13c0eaf 100644 > --- a/drivers/usb/phy/Kconfig > +++ b/drivers/usb/phy/Kconfig > @@ -47,7 +47,7 @@ config USB_RCAR_PHY > > config SAMSUNG_USBPHY > bool "Samsung USB PHY controller Driver" > - depends on USB_S3C_HSOTG > + depends on USB_S3C_HSOTG || USB_EHCI_S5P || USB_OHCI_EXYNOS > select USB_OTG_UTILS > help > Enable this to support Samsung USB phy controller for samsung > diff --git a/drivers/usb/phy/samsung-usbphy.c > b/drivers/usb/phy/samsung-usbphy.c > index 4ceabe3..621348a 100644 > --- a/drivers/usb/phy/samsung-usbphy.c > +++ b/drivers/usb/phy/samsung-usbphy.c > @@ -5,7 +5,8 @@ > * > * Author: Praveen Paneri > * > - * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller > + * Samsung USB-PHY transceiver; talks to S3C HS OTG controller, EHCI-S5P and > + * OHCI-EXYNOS controllers. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 as > @@ -24,7 +25,7 @@ > #include > #include > #include > -#include > +#include > #include > > /* Register definitions */ > @@ -56,6 +57,103 @@ > #define RSTCON_HLINK_SWRST (0x1 << 1) > #define RSTCON_SWRST (0x1 << 0) > > +/* EXYNOS5 */ > +#define EXYNOS5_PHY_HOST_CTRL0 (0x00) > + > +#define HOST_CTRL0_PHYSWRSTALL (0x1 << 31) > + > +#define HOST_CTRL0_REFCLKSEL_MASK (0x3) > +#define HOST_CTRL0_REFCLKSEL_XTAL (0x0 << 19) > +#define HOST_CTRL0_REFCLKSEL_EXTL (0x1 << 19) > +#define HOST_CTRL0_REFCLKSEL_CLKCORE (0x2 << 19) > + > +#define HOST_CTRL0_FSEL_MASK (0x7 << 16) > +#define HOST_CTRL0_FSEL(_x)((_x)
Re: [PATCH v5 3/4] USB: ehci-s5p: Add phy driver support
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(&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 && pdat
Re: 3.4.17: OOPs in xhci_irq() probably due to sleeping disks on a Prolific USB-to-SATA dongle
Hi Martin, Thanks for the bug report. Yes, I would like full dmesg from your system, including the oops if you have it. Can you reproduce this bug on an updated 3.4.24 kernel? It sounds like a difficult bug to trigger, but the oops wasn't helpful enough for me to pinpoint the problem. If you can trigger the bug on the 3.4.24 kernel, please recompile with CONFIG_USB_XHCI_HCD_DEBUGGING turned on, and send me dmesg showing the oops. You may have to capture dmesg via netconsole to a separate box in order to capture the oops. Sarah Sharp On Wed, Dec 19, 2012 at 03:34:46PM +0100, Martin Mokrejs wrote: > Hi (resend with smaller image attached to pass mailing list filters), > I had a Prolific-based USB3.0-to-2xSATA dongle connected to my > Dell Vostro 3550 laptop via its Texas Instruments-based USB3.0 > chip connector. The dongle apparently made the disks sleep and once I > wanted to access the data I got: > > Dec 18 19:23:05 vostro kernel: [87422.577634] EXT4-fs error (device sdc1): > __ext4_get_inode_loc:3564: inode #2: block 1057: comm ls: unable to read > itable block > Dec 18 19:23:05 vostro kernel: [87422.577637] EXT4-fs error (device sdc1) in > ext4_reserve_inode_write:4382: IO failure > Dec 18 19:23:10 vostro kernel: [87427.816708] Buffer I/O error on device > sdc1, logical block 365985792 > Dec 18 19:23:10 vostro kernel: [87427.816711] lost page write due to I/O > error on sdc1 > Dec 18 19:23:10 vostro kernel: [87427.816714] JBD2: Error -5 detected when > updating journal superblock for sdc1-8. > Dec 18 19:23:10 vostro kernel: [87427.816732] Aborting journal on device > sdc1-8. > Dec 18 19:23:10 vostro kernel: [87427.816734] Buffer I/O error on device > sdc1, logical block 365985792 > Dec 18 19:23:10 vostro kernel: [87427.816736] lost page write due to I/O > error on sdc1 > Dec 18 19:23:10 vostro kernel: [87427.816738] JBD2: Error -5 detected when > updating journal superblock for sdc1-8. > Dec 18 19:23:35 vostro kernel: [87452.589793] EXT3-fs error (device sdd1): > ext3_get_inode_loc: unable to read inode block - inode=2, block=1027 > Dec 18 19:23:35 vostro kernel: [87452.589950] Buffer I/O error on device > sdd1, logical block 0 > Dec 18 19:23:35 vostro kernel: [87452.589952] lost page write due to I/O > error on sdd1 > Dec 18 19:23:35 vostro kernel: [87452.589954] EXT3-fs (sdd1): I/O error while > writing superblock > Dec 18 19:23:35 vostro kernel: [87452.589956] EXT3-fs (sdd1): error in > ext3_reserve_inode_write: IO failure > Dec 18 19:23:35 vostro kernel: [87452.590098] Buffer I/O error on device > sdd1, logical block 0 > Dec 18 19:23:35 vostro kernel: [87452.590100] lost page write due to I/O > error on sdd1 > Dec 18 19:23:35 vostro kernel: [87452.590101] EXT3-fs (sdd1): I/O error while > writing superblock > Dec 18 19:23:40 vostro kernel: [87457.731802] Buffer I/O error on device > sdd1, logical block 122061318 > Dec 18 19:23:40 vostro kernel: [87457.731804] lost page write due to I/O > error on sdd1 > Dec 18 19:23:40 vostro kernel: [87457.731807] Aborting journal on device sdd1. > Dec 18 19:23:40 vostro kernel: [87457.731810] Buffer I/O error on device > sdd1, logical block 122061314 > Dec 18 19:23:40 vostro kernel: [87457.731811] lost page write due to I/O > error on sdd1 > Dec 18 19:23:40 vostro kernel: [87457.731813] JBD: I/O error detected when > updating journal superblock for sdd1. > > > I think it was said on this list few times already that this is a Prolific > chip problem ignoring the host and powering off the disks. Actually, I am not > sure > they were really spun off but that is not the issue I am to report here. > However, > I decided to turn off the power from the dongle powering the disks and unplug > the > USB cable. I think the computer survived that but after powered back up the > dongle > with drives and re-plugged in the USB cable I got a kernel OOPs. My apologies > that I am not certain when it crashed exactly, took me a while to fight > through > some other obstackles and somehow forgot the details meanwhile. :( > The crash was with 3.4.17 kernel. I retyped the stacktrace from a camera > picture (attached), sadly, it did NOT fit onto the screen so it's NOT > complete. > Anyway, here it goes: > > resched_task > check_preempt_curr > xhci_irq > ttwu_do_activate.constprop > try_to_wake_up > flat_send_IPI_mask > trigger_load_balance > init_timer_deferrable_key > wake_up_process > _raw_spin_lock_irq > xhci_msi_irq > handle_irq_event_percpu > handle_irq_event > ack_apic_edge > handle_edge_irq > handle_irq > do_IRQ > common_interrupt > > ? put_page_testzero > release_pages > free_pages_and_swap_cache > tlb_flush_mmu > tlb_finish_mmu > exit_mmap > mmput > flush_old_exec > load_elf_binary > ? _raw_read_lock > ? load_misc_binary > ? get_user_pages > ? get_arg_page > ? put_page > search_binary_handler > ? elf_core_dump > do_execve_common.isra > do_execve > sys_execve > stub_execve > > RIP ring_doorbell_for_active_rings > > > Please let m
Re: xHCI immediately wakes up Asus P8Z68-V LX in S5
On Wed, Dec 19, 2012 at 03:15:09AM +, Ben Hutchings wrote: > On Mon, 2012-12-17 at 13:28 -0800, Sarah Sharp wrote: > > On Sun, Dec 16, 2012 at 12:37:00AM +, Ben Hutchings wrote: > > > Since Linux 3.2.16, my desktop with an Asus P8Z68-V LX motherboard > > > always wakes up a few seconds after I shutdown. I then have to switch > > > it off a second time. This still occurs in Linux 3.6.9 (haven't tried > > > 3.7 yet). > > > > Hmm. So if we enable PCI bus suspend for this xHCI host controller, > > then it will immediately wakeup the system on shutdown? > > So it seems. > > > What happens when runtime PM is enabled for the host (and any USB > > devices attached to it)? Does the host controller stay in D3hot, or > > does it immediately pop back into D0? > > It stays in D3hot. Also, this fixes the problem. > > > Can you turn on CONFIG_USB_XHCI_HCD_DEBUGGING and CONFIG_USB_DEBUG for > > your kernel and send me the dmesg? I'm wondering if the xHCI host > > controller is either failing to complete the host suspend command, or > > perhaps failing to be halted. > > This happens in S5 (soft-off), so there is no way to read dmesg > afterwards! I just checked S3 (mem) and S4 (disk), and the system stays > properly suspended in those states. > > I set sysctl kernel.printk=8 and recorded the screen during shutdown, > from which I transcribe the following: > > pcieport :00:1c.6: wake-up capability enabled by ACPI > xhci_hcd :04:00.0: // Halt the HC > xhci_hcd :04:00.0: xhci_shutdown completed - status = 1 Ok, then the xHCI host controller reported that it did halt successfully. It should not be sending any interrupts or PMEs when it's halted. > ACPI: Preparing to enter system sleep state S5 > Disabling non-boot CPUs ... > Power down. > acpi_power_off called > > The first line is related to wake-on-LAN as the specified device is the > downstream port connected to the network controller. > > [...] > > > No devices are connected to the xHCI. > > > > > > 'echo disabled > /sys/bus/pci/devices/:04:00.0/power/wakeup' does > > > *not* work around this. > > > > Does unloading the xHCI driver before shutdown fix this? > [...] > > Yes. > > Also, I tried plugging devices into the USB 3.0-capable ports. Plugging > in a mouse fixes this. Plugging in a super-speed SATA enclosure > doesn't. > > So, as far as I can see this problem is specific to: > - Sleep in S5 > - Driver loaded > - Run-time power management disabled > - No devices or super-speed device plugged in > > Any of these avoid the problem: > - Sleep in S3 or S4 > - Driver unloaded > - Run-time power management enabled > - Low-speed device plugged in Is it any low speed device, or just a mouse? Does a connected keyboard or a USB serial device make the system stay shutdown? The symptoms are pretty bizarre, and I honestly don't know what could be the root cause. I would suggest you try the BIOS update and see if it fixes the issue. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
On Wed, 19 Dec 2012 11:35:50 -0800, Alan Stern wrote: As far as the OHCI hardware is concerned, there shouldn't be any difference between runtime suspend and system suspend. This strongly suggests that the bug doesn't lie in the controller itself but in the firmware (BIOS or ACPI). Is there a way we can help to look for such a failing pattern, more directly related to BIOS/ACPI instead of the OHCI controller, in lspci, dmidecode or some other tool? -- Octavio. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
[CC: list trimmed slightly] On Wed, 19 Dec 2012, Octavio Alvarez wrote: > On Wed, 19 Dec 2012 11:35:50 -0800, Alan Stern > wrote: > > > As far as the OHCI hardware is concerned, there shouldn't be any > > difference between runtime suspend and system suspend. This strongly > > suggests that the bug doesn't lie in the controller itself but in the > > firmware (BIOS or ACPI). Did you actually do a runtime suspend test? What does "lspci -v" show for the controller while it is suspended with a device attached? > Is there a way we can help to look for such a failing pattern, more > directly related to BIOS/ACPI instead of the OHCI controller, in lspci, > dmidecode or some other tool? Not that I know of. Probably the only way we will find the answer to this problem is by getting in touch with an engineer at ASUS who knows what the BIOS is doing wrong. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.7 kernel hangs when doing scp
On Wed, Dec 19, 2012 at 12:56:12PM -0200, Fabio Estevam wrote: > Hi Greg, > > On Tue, Dec 18, 2012 at 12:43 AM, Greg KH wrote: > > On Tue, Dec 18, 2012 at 12:01:35AM -0200, Fabio Estevam wrote: > >> Hi, > >> > >> Booting a 3.7 kernel on a mx6qsabrelite board via NFS and using an > >> asix USB/Ethernet adapter and then I try to transfer a 10MB file from > >> the target to the host: > > > > Has this ever worked before on any older kernel? > > We did more experiments on this and we noticed that: > > - mx51 running kernel 3.7 does not show this problem (tested a non-dt > mx51 kernel, which uses drivers/usb/host/ehci-mxc.c instead of the > chipidea driver.) > - x86 running 3.2 kernel does not show this problem > - usb support on mx6 has appeared on 3.6, but running an out-of-tree > 3.0.35 kernel also shows the same issue. > - Doing long file transfers via mass storage on mx6 running 3.7 works fine. > > Shawn/Sascha/Peter, > > Any suggestions as to how debug this? By looking at the kernel dumps > it seems to point to 'gic_handle_irq' area. Hi Fabio, Please to see if the Stream mode disable (bit4, usbmode) is set or not? If it is not, please try below patch diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index c97503b..258bc02 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -227,6 +227,7 @@ static void tdi_reset (struct ehci_hcd *ehci) tmp = ehci_readl(ehci, &ehci->regs->usbmode); tmp |= USBMODE_CM_HC; + tmp |= (1 << 4); /* disable stream mode */ /* The default byte access to MMR space is LE after * controller reset. Set the required endian mode * for transfer buffers to match the host microprocessor > > Thanks, > > Fabio Estevam > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] ARM: EXYNOS: Update & move usb-phy types to generic include layer
Hi Doug,
On Thu, Dec 20, 2012 at 3:18 AM, Doug Anderson wrote:
> Vivek,
>
> On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam
> wrote:
>> Updating the names of usb-phy types to more generic names:
>> USB_PHY_TYPE_DEIVCE & USB_PHY_TYPE_HOST; and further update
>> its dependencies.
>
> Since you're changing the name, I would have expected to see a removal
> of the old enum type in this patch. I don't see it. After applying
> all of your patches I find that S5P_USB_PHY_HOST and
> S5P_USB_PHY_DEVICE are still defined and used.
>
True that we should have removed the earlier defined enums, infact i
did the same
in patchset v4 for this series. But since in this patchset i separated
out drivers and
architecture part, i thought it would be easier to remove them in one
shot in a latter commit
which will actually touch upon all architecture related code
(mach-exynos/setup-usb-phy.c,
mach-s5pv210/setup-usb-phy.c, and include/plat/usb-phy.h) once this
approach is accepted.
> $ git grep S5P_USB_PHY_HOST
> arch/arm/mach-exynos/setup-usb-phy.c: else if (type == S5P_USB_PHY_HOST)
> arch/arm/mach-exynos/setup-usb-phy.c: else if (type == S5P_USB_PHY_HOST)
> arch/arm/plat-samsung/include/plat/usb-phy.h: S5P_USB_PHY_HOST,
>
> I believe that those usages of the old enum should also be switched
> over and the old enum removed.
>
Sure, those enums need to be removed.
If you say i will submit a patch for the same.
>> Signed-off-by: Praveen Paneri
>> Signed-off-by: Vivek Gautam
>> ---
>> drivers/usb/host/ehci-s5p.c |9 +
>> drivers/usb/host/ohci-exynos.c |9 +
>> include/linux/usb/samsung_usb_phy.h | 16
>> 3 files changed, 26 insertions(+), 8 deletions(-)
>> create mode 100644 include/linux/usb/samsung_usb_phy.h
>>
>> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
>> index 319dcfa..46ca5ef 100644
>> --- a/drivers/usb/host/ehci-s5p.c
>> +++ b/drivers/usb/host/ehci-s5p.c
>> @@ -17,6 +17,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #define EHCI_INSNREG00(base) (base + 0x90)
>> @@ -164,7 +165,7 @@ static int s5p_ehci_probe(struct platform_device *pdev)
>> }
>>
>> if (pdata->phy_init)
>> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>>
>> ehci = hcd_to_ehci(hcd);
>> ehci->caps = hcd->regs;
>> @@ -198,7 +199,7 @@ static int s5p_ehci_remove(struct platform_device *pdev)
>> usb_remove_hcd(hcd);
>>
>> if (pdata && pdata->phy_exit)
>> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>>
>> clk_disable_unprepare(s5p_ehci->clk);
>>
>> @@ -229,7 +230,7 @@ static int s5p_ehci_suspend(struct device *dev)
>> rc = ehci_suspend(hcd, do_wakeup);
>>
>> if (pdata && pdata->phy_exit)
>> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>>
>> clk_disable_unprepare(s5p_ehci->clk);
>>
>> @@ -246,7 +247,7 @@ static int s5p_ehci_resume(struct device *dev)
>> clk_prepare_enable(s5p_ehci->clk);
>>
>> if (pdata && pdata->phy_init)
>> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>>
>> /* DMA burst Enable */
>> writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
>> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> index aa3b884..804fb62 100644
>> --- a/drivers/usb/host/ohci-exynos.c
>> +++ b/drivers/usb/host/ohci-exynos.c
>> @@ -15,6 +15,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> struct exynos_ohci_hcd {
>> @@ -153,7 +154,7 @@ static int exynos_ohci_probe(struct platform_device
>> *pdev)
>> }
>>
>> if (pdata->phy_init)
>> - pdata->phy_init(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
>>
>> ohci = hcd_to_ohci(hcd);
>> ohci_hcd_init(ohci);
>> @@ -184,7 +185,7 @@ static int exynos_ohci_remove(struct platform_device
>> *pdev)
>> usb_remove_hcd(hcd);
>>
>> if (pdata && pdata->phy_exit)
>> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>>
>> clk_disable_unprepare(exynos_ohci->clk);
>>
>> @@ -229,7 +230,7 @@ static int exynos_ohci_suspend(struct device *dev)
>> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
>>
>> if (pdata && pdata->phy_exit)
>> - pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
>> + pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
>>
>> clk_disable_unprepare(exynos_ohci->clk);
>>
>> @@ -249,7 +250,7 @@ static int exynos_ohci_resume(struct device *dev)
>> clk_prepare_enable(exynos_ohci->clk);
>>
>> if
Re: [PATCH v5 3/4] USB: ehci-s5p: Add phy driver support
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(&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);
>>
