Thread (22 messages) 22 messages, 7 authors, 2011-02-24

Re: [PATCH v5] EHCI bus glue for on-chip PMC MSP USB controller

From: Anoop P A <hidden>
Date: 2011-02-24 09:55:47
Also in: lkml

On Wed, 2011-02-23 at 20:02 +0300, Dan Carpenter wrote:
quoted
quoted
It doesn't compile so I can't test this, but I think that this will
cause a sparse warning.  "base" should have an __iomem tag.  Please
run sparse on this driver.
Looks like mips platform build has been broken on linux-next ( unable to
configure) . However I have tested code with linux-queue tree ( mips)
and didn't see any such warnings
Sparse is an external tool.  You have to install it and build with
make C=1 drivers/usb/file.o
Ok. I found there was some issue in arch/mips/Makefile CHECKFLAGS
quoted
quoted
quoted
+	temp = HCS_N_CC(ehci->hcs_params) * HCS_N_PCC(ehci->hcs_params);
+	temp &= 0x0f;
companion HCs * ports per CC & 0xf?

What's the &= 0x0f for?  It's left out of the printk.
Code got carried forward from ehci-pci.c . Is that says ehci-pci.c is
uptodate? .  
My guess is that ehci-pci.c is buggy.  Anyway this is just a work around
to handle buggy hardware.  Since the code here just prints some debug
output and doesn't do the work arounds we might as well remove the whole
thing?  It seems unlikely that your mips hardware will have the same
bug.
Ok I will remove the code block
quoted
quoted
quoted
+static int ehci_msp_suspend(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+	unsigned long flags;
+	int rc;
+
+	return 0;
+	rc = 0;
+
+	if (time_before(jiffies, ehci->next_statechange))
+		usleep(10000);
Is there still a usleep() function?  Either way, can you send us
something that compiles on linux-next?
Again code got carried from ehci-pci.c .(changed msleep to usleep as
checkpatch complained about it). I am unable to compile mips targets in
linux-next tree . However this patch is tested with both linux-stable
and linux-queue tree of l-m-o
checkpatch.pl complains because msleep() is not accurate for tiny
amounts of time.  There is no usleep() function because trying to be
that accurate is a lot of work and causes a lot of interrupts.  There
is a usleep_range() function instead which lets the scheduler group
wakeups together.  This is documented in
Documentation/timers/timers-howto.txt

Also this is dead code.  No one will complain, if you just delete it.
You mean entire ehci_msp_suspend() ??
regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help