Re: [PATCH] EHCI: Update qTD next pointer in QH overlay region during unlink
On 9/7/2012 9:22 PM, Alan Stern wrote:
On Fri, 7 Sep 2012, Pavankumar Kondeti wrote:
There is a possibility of QH overlay region having reference to a stale
qTD pointer during unlink.
Consider an endpoint having two pending qTD before unlink process begins.
The endpoint's QH queue looks like this.
qTD1 -- qTD2 -- Dummy
To unlink qTD2, QH is removed from asynchronous list and Asynchronous
Advance Doorbell is programmed. The qTD1's next qTD pointer is set to
qTD2'2 next qTD pointer and qTD2 is retired upon controller's doorbell
interrupt. If QH's current qTD pointer points to qTD1, transfer overlay
region still have reference to qTD2. But qtD2 is just unlinked and freed.
This may cause EHCI system error. Fix this by updating qTD next pointer
in QH overlay region with the qTD next pointer of the current qTD.
Signed-off-by: Pavankumar Kondeti pkond...@codeaurora.org
---
drivers/usb/host/ehci-q.c | 12 ++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 9bc39ca..4b66374 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -128,9 +128,17 @@ qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
else {
qtd = list_entry (qh-qtd_list.next,
struct ehci_qtd, qtd_list);
-/* first qtd may already be partially processed */
-if (cpu_to_hc32(ehci, qtd-qtd_dma) == qh-hw-hw_current)
+/*
+ * first qtd may already be partially processed.
+ * If we come here during unlink, the QH overlay region
+ * might have reference to the just unlinked qtd. The
+ * qtd is updated in qh_completions(). Update the QH
+ * overlay here.
+ */
+if (cpu_to_hc32(ehci, qtd-qtd_dma) == qh-hw-hw_current) {
+qh-hw-hw_qtd_next = qtd-hw_next;
qtd = NULL;
+}
}
if (qtd)
Acked-by: Alan Stern st...@rowland.harvard.edu
Thanks Alan for reviewing the patch.
Have you been able to determine that this eliminates your host system
errors?
Yes. We are able to determine that this patch is fixing the EHCI system
error.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
USB support for device tree
Hi
I am working on adding USB device tree support for MSM platform. One of
our chip set has 2 hsusb cores. The first core is configured as otg and
the other core is configured in host only mode (EHCI compliant). Are the
below device node names Okay? Please suggest.
hsusb0-otg: usb@0xa600 {
compatible = qcom,hsusb-otg;
---
};
hsusb0-device: usb@0xa600 {
compatible = qcom,hsusb-device;
---
};
hsusb0-host: usb@0xa600 {
compatible = qcom,hsusb-host, usb-ehci;
---
};
hsusb1-host: usb@0xa600 {
compatible = qcom,hsusb-host, usb-ehci;
---
};
/* super speed support
ssusb0-device: usb@0xa600 {
compatible = qcom,ssusb-device;
};
*/
Thanks,
Pavan
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB support for device tree
Hi
On 11/4/2011 10:15 PM, Grant Likely wrote:
On Fri, Nov 4, 2011 at 4:25 AM, Pavan Kondeti pkond...@codeaurora.org wrote:
Hi
I am working on adding USB device tree support for MSM platform. One of
our chip set has 2 hsusb cores. The first core is configured as otg and
the other core is configured in host only mode (EHCI compliant). Are the
below device node names Okay? Please suggest.
hsusb0-otg: usb@0xa600 {
compatible = qcom,hsusb-otg;
---
};
hsusb0-device: usb@0xa600 {
compatible = qcom,hsusb-device;
---
};
hsusb0-host: usb@0xa600 {
compatible = qcom,hsusb-host, usb-ehci;
---
};
hsusb1-host: usb@0xa600 {
compatible = qcom,hsusb-host, usb-ehci;
---
};
/* super speed support
ssusb0-device: usb@0xa600 {
compatible = qcom,ssusb-device;
};
The host controller node names as usb@adddr as you have here is
exactly right. The driver doesn't care and will only look at the
compatible list. OTG controllers can also use usb@ as the prefix.
Controllers that are only in device mode should probably be called
something like usb-gadget@addr or similar, because usb@ is used
for host controllers.
In our case OTG, gadget and host controllers are part of a single core
i.e they all share the same register address space.
If I use usb@addr for OTG controller, then host and OTG device node
names becomes same. Is that okay? Can two devices have the same device
node name in device tree source file?
The label names (hsusb*-host, hsusb*-device) are completely irrelevant
since Linux never sees them. Use whatever you want for the label
names.
Is it a good practice to give label names? I thought, it improves a bit
readability of device tree source file.
Also, the form of the node name is: usb@a600 (without '0x' in
the address).
Thanks for the correction.
Thanks,
Pavan
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] mmc: core: Kill block requests if card is removed
Hello Sujit,
On 10/12/2011 1:06 PM, Sujit Reddy Thumma wrote:
Kill block requests when the host knows that the card is
removed from the slot and is sure that it can no longer
accept any requests.
Kill this silently so that the block layer don't output
error messages unnecessarily.
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
snip
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 0ea4a06..7cdbc14 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -196,6 +196,7 @@ struct mmc_card {
#define MMC_STATE_HIGHSPEED_DDR (14) /* card is in high
speed mode */
#define MMC_STATE_ULTRAHIGHSPEED (15) /* card is in ultra
high speed mode */
#define MMC_CARD_SDXC(16) /* card is SDXC */
+#define MMC_STATE_INSERTED (17) /* card present in the slot
*/
unsigned intquirks; /* card quirks */
#define MMC_QUIRK_LENIENT_FN0(10) /* allow SDIO FN0
writes outside of the VS CCCR range */
#define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (11) /* use func-cur_blksize */
@@ -344,6 +345,7 @@ static inline void __maybe_unused remove_quirk(struct
mmc_card *card, int data)
#define mmc_card_sdio(c) ((c)-type == MMC_TYPE_SDIO)
#define mmc_card_present(c) ((c)-state MMC_STATE_PRESENT)
+#define mmc_card_inserted(c) ((c)-state MMC_STATE_INSERTED)
#define mmc_card_readonly(c) ((c)-state MMC_STATE_READONLY)
#define mmc_card_highspeed(c)((c)-state MMC_STATE_HIGHSPEED)
#define mmc_card_blockaddr(c)((c)-state MMC_STATE_BLOCKADDR)
@@ -352,6 +354,7 @@ static inline void __maybe_unused remove_quirk(struct
mmc_card *card, int data)
#define mmc_card_ext_capacity(c) ((c)-state MMC_CARD_SDXC)
#define mmc_card_set_present(c) ((c)-state |= MMC_STATE_PRESENT)
+#define mmc_card_set_inserted(c) ((c)-state |= MMC_STATE_INSERTED)
#define mmc_card_set_readonly(c) ((c)-state |= MMC_STATE_READONLY)
#define mmc_card_set_highspeed(c) ((c)-state |= MMC_STATE_HIGHSPEED)
#define mmc_card_set_blockaddr(c) ((c)-state |= MMC_STATE_BLOCKADDR)
Why do we need another flag to indicate card's presence? can not we use
MMC_STATE_PRESENT flag? This flag is set in mmc_add_card(). But not
cleared any where...
Thanks,
Pavan
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 2/5] USB: OTG: msm: vote for dayatona fabric clock
Hi Sergei,
On 5/3/2011 4:44 PM, Sergei Shtylyov wrote:
Hello.
On 03-05-2011 9:13, Pavankumar Kondeti wrote:
From: Anji jonnala an...@codeaurora.org
HSUSB core clock is derived from daytona fabric clock and for
HSUSB operational require minimum core clock at 55MHz. Since, HSUSB
cannot tolerate daytona fabric clock change in the middle of HSUSB
operational, vote for maximum Daytona fabric clock
while usb is operational
Signed-off-by: Anji jonnalaan...@codeaurora.org
Signed-off-by: Pavankumar Kondetipkond...@codeaurora.org
[...]
diff --git a/drivers/usb/otg/msm_otg.c b/drivers/usb/otg/msm_otg.c
index 2965986..cfbb606 100644
--- a/drivers/usb/otg/msm_otg.c
+++ b/drivers/usb/otg/msm_otg.c
[...]
@@ -954,7 +979,11 @@ free_regs:
put_core_clk:
if (motg-core_clk)
clk_put(motg-core_clk);
-clk_put(motg-pclk);
Is it really correct to remove this line?
No. It is not correct. I will fix it in next version.
+put_pclk_src:
+if (!IS_ERR(motg-pclk_src)) {
+clk_disable(motg-pclk_src);
+clk_put(motg-pclk_src);
+}
put_clk:
clk_put(motg-clk);
put_phy_reset_clk:
WBR, Sergei
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 2/5] USB: OTG: msm: vote for dayatona fabric clock
Hi Greg, On 5/3/2011 10:45 PM, Greg KH wrote: On Tue, May 03, 2011 at 05:59:37PM +0530, Pavan Kondeti wrote: Hi Sergei, On 5/3/2011 4:44 PM, Sergei Shtylyov wrote: Hello. On 03-05-2011 9:13, Pavankumar Kondeti wrote: From: Anji jonnala an...@codeaurora.org HSUSB core clock is derived from daytona fabric clock and for HSUSB operational require minimum core clock at 55MHz. Since, HSUSB cannot tolerate daytona fabric clock change in the middle of HSUSB operational, vote for maximum Daytona fabric clock while usb is operational Signed-off-by: Anji jonnalaan...@codeaurora.org Signed-off-by: Pavankumar Kondetipkond...@codeaurora.org [...] diff --git a/drivers/usb/otg/msm_otg.c b/drivers/usb/otg/msm_otg.c index 2965986..cfbb606 100644 --- a/drivers/usb/otg/msm_otg.c +++ b/drivers/usb/otg/msm_otg.c [...] @@ -954,7 +979,11 @@ free_regs: put_core_clk: if (motg-core_clk) clk_put(motg-core_clk); - clk_put(motg-pclk); Is it really correct to remove this line? No. It is not correct. I will fix it in next version. Next version? I meant next patch set version. Again sorry for the confusion. Ok, I'll drop all of these then from my queue. Please resend when you have something working properly. I have tested the patches well. The driver probe() never failed in my testing. So could not found this BUG(). I am resending the patch series now. Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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/7] USB: gadget: Fix typo (s/EBUSY/-EBUSY) in ci13xxx_udc
Hi David, Thanks for your feedback. I will re-send the patch series. Thanks, Pavan On 4/29/2011 9:34 PM, David Brown wrote: On Thu, Apr 28 2011, Pavankumar Kondeti wrote: Signed-off-by: Pavankumar Kondeti pkond...@codeaurora.org It is helpful to maintains to describe second revisions of patches in the subject, something like: [PATCH v2 1/7] ... Also, for something this size, it would be helpful to have a coverletter explaining the what the series does. It is also useful to put a small history in the coverletter stating what is fixed in each update of the patch series. For example, this update fixes some small mistakes that were addressed on the list. Oh, and you should CC the people that mentioned things that need to be fixed so that they see the updates. David -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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 5/5] USB: OTG: msm: Add PHY suspend support for MSM8960
On 5/2/2011 5:52 PM, Sergei Shtylyov wrote:
Hello.
On 02-05-2011 10:36, Pavankumar Kondeti wrote:
Signed-off-by: Pavankumar Kondetipkond...@codeaurora.org
[...]
diff --git a/drivers/usb/otg/msm_otg.c b/drivers/usb/otg/msm_otg.c
index 66a1b7c..52853fd 100644
--- a/drivers/usb/otg/msm_otg.c
+++ b/drivers/usb/otg/msm_otg.c
@@ -163,6 +163,32 @@ put_3p3:
return rc;
}
+#ifdef CONFIG_PM_SLEEP
+#define USB_PHY_SUSP_DIG_VOL 50
+static int msm_hsusb_config_vddcx(int high)
+{
+int max_vol = USB_PHY_VDD_DIG_VOL_MAX;
+int min_vol;
+int ret;
+
+if (high)
+min_vol = USB_PHY_VDD_DIG_VOL_MIN;
+else
+min_vol = USB_PHY_SUSP_DIG_VOL;
+
+ret = regulator_set_voltage(hsusb_vddcx, min_vol, max_vol);
+if (ret) {
+pr_err(%s: unable to set the voltage for regulator
You missed space after regulator...
Thanks for pointing this out. The other patch (USB: OTG: msm: Configure
PHY Analog and Digital voltage domains) also has missed spaces. I will
fix it in next patch set.
+HSUSB_VDDCX\n, __func__);
+return ret;
+}
+
[...]
WBR, Sergei
Thanks,
Pavan
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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/5] USB: gadget: Fix unused variable warning in ci13xxx_udc
On 4/28/2011 3:31 PM, Michal Nazarewicz wrote:
On Thu, 28 Apr 2011 09:58:17 +0200, Pavankumar Kondeti
pkond...@codeaurora.org wrote:
Signed-off-by: Pavankumar Kondeti pkond...@codeaurora.org
---
drivers/usb/gadget/ci13xxx_udc.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/ci13xxx_udc.c
b/drivers/usb/gadget/ci13xxx_udc.c
index e09178b..6a9ad59 100644
--- a/drivers/usb/gadget/ci13xxx_udc.c
+++ b/drivers/usb/gadget/ci13xxx_udc.c
@@ -1843,7 +1843,7 @@ __releases(mEp-lock)
__acquires(mEp-lock)
{
struct ci13xxx_req *mReq, *mReqTemp;
-int retval;
+int uninitialized_var(retval);
trace(%p, mEp);
Could we instead set it to 0?
would it matter anyway? we return -EINVAL if the queue is empty.
Otherwise we assign retval = _hardware_dequeue().
Also, it seems this function has a bug where it compares retval
with EBUSY instead of -EBUSY:
if (retval == EBUSY)
retval = 0;
Just saying in case anyone wants to take care of it.
Thanks for pointing this out. I will send out a patch to fix this.
As of now isr_tr_complete_low() return value is simply ignored for
non-control endpoints. We never hit this case for control endpoint.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Fix trout build failure with ci13xxx_msm gadget
On 4/5/2011 6:25 PM, Felipe Balbi wrote: On Fri, Feb 04, 2011 at 10:08:18AM +0530, Pavankumar Kondeti wrote: This patch fixes the below compilation errors. CC drivers/usb/gadget/ci13xxx_msm.o CC net/mac80211/led.o drivers/usb/gadget/ci13xxx_msm.c: In function 'ci13xxx_msm_notify_event': drivers/usb/gadget/ci13xxx_msm.c:42: error: 'USB_AHBBURST' undeclared (first use in this function) drivers/usb/gadget/ci13xxx_msm.c:42: error: (Each undeclared identifier is reported only once drivers/usb/gadget/ci13xxx_msm.c:42: error: for each function it appears in.) drivers/usb/gadget/ci13xxx_msm.c:43: error: 'USB_AHBMODE' undeclared (first use in this function) make[4]: *** [drivers/usb/gadget/ci13xxx_msm.o] Error 1 make[3]: *** [drivers/usb/gadget] Error 2 MSM USB driver is not supported on boards like trout (MSM7201) which has an external PHY. Signed-off-by: Pavankumar Kondeti pkond...@codeaurora.org the real fix, though, would be to phase out the PHY to drivers/usb/otg/blablabla.c. ci13xxx_msm.c shouldn't assume the PHY it's using. That's wrong. Agreed. Thanks for pointing this out. I think OTG driver should bail out if the PHY (or SoC) is not supported; which means otg_get_transceiver() returns NULL and gadget/host driver's probe also will fail. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: finding out the attached device current requirement
On 3/3/2011 3:15 PM, Felipe Balbi wrote: On Thu, Mar 03, 2011 at 02:22:34PM +0530, Pavan Kondeti wrote: Hi I am adding ACA (Accessory Charger Adapter) detection support for MSM OTG driver. Battery charging spec introduced ACA to facilitate charging when acting as a host. The host does not have to supply power on VBUS when ACA is connected. Host device can also draw some amount of current. The amount of current is determined by subtracting the attached peripheral current requirement from the charger rating current. I am looking for a way to find out the attached peripheral's current requirement. We already DEVICE_ADD and DEVICE_REMOVE notifications. But bMaxPower from the device descriptor ?? You are referring to bMaxPower in Configuration descriptor right? When a multi configuration device is connected, configuration can be changed by user at any time. The same can also be possible with a user claimed port. So I am looking for a notification from USB core when a SET_CONFIGURATION request is sent to a device connected on the root hub. I will reduce my power (ACA rating current - bMaxpower of the config being selected) upon this notification. Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: msm gadget compilation failure with trout
On 1/28/2011 10:45 PM, Daniel Walker wrote: On Fri, 2011-01-28 at 12:31 +0530, Pavan Kondeti wrote: Hi Denis, On 1/28/2011 2:56 AM, Daniel Walker wrote: On Thu, 2011-01-27 at 22:12 +0100, Denis 'GNUtoo' Carikli wrote: Hi, I've that: drivers/usb/gadget/ci13xxx_msm.c: In function 'ci13xxx_msm_notify_event': drivers/usb/gadget/ci13xxx_msm.c:42:3: error: 'USB_AHBBURST' undeclared (first use in this function) drivers/usb/gadget/ci13xxx_msm.c:42:3: note: each undeclared identifier is reported only once for each function it appears in drivers/usb/gadget/ci13xxx_msm.c:43:3: error: 'USB_AHBMODE' undeclared (first use in this function) make[2]: *** [drivers/usb/gadget/ci13xxx_msm.o] Error 1 make[1]: *** [drivers/usb/gadget] Error 2 make[1]: *** Waiting for unfinished jobs here's my setup: * linux-next/master ( git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git ) at 47db3d1b0b3d69c6e977158ac354a972b81677bd * trout(also known as htc dream or G1) board Should I include my defconfig? Should I include more details? Am I using the gadget wrong driver? AHBMODE register offset is different for trout and AHBBURST register is not present in trout. MSM gadget driver is supported only for QSD8x50 and MSM7x30 boards. USB support is not added for trout which uses an external PHY unlike the other targets. Please disable USB support in your defconfig. If it's not supported don't allow it to be compiled, but you need to make some sort of fix for this. Agreed. I will submit a patch to fix this. btw, if it's not support why is there a 7X01 define used in the header ? Sorry. It should not have been there. It was a copy-paste mistake. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 3/5] USB: gadget: OTG supplement revision 2.0 updates
On 12/15/2010 5:48 PM, Felipe Balbi wrote: Hi, On Wed, Dec 15, 2010 at 04:44:12PM +0530, Pavankumar Kondeti wrote: @@ -1047,9 +1065,10 @@ composite_unbind(struct usb_gadget *gadget) kfree(cdev-req-buf); usb_ep_free_request(gadget-ep0, cdev-req); } +device_remove_file(gadget-dev, dev_attr_host_request); +device_remove_file(gadget-dev, dev_attr_suspended); kfree(cdev); set_gadget_data(gadget, NULL); -device_remove_file(gadget-dev, dev_attr_suspended); this looks like something that shouldn't be part of this patch ?!? Yeah. I will send another patch for removing suspended sysfs file before freeing the cdev. Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 4/5] USB: EHCI: Notify HCD about HNP enabled port suspend
On 12/15/2010 5:49 PM, Felipe Balbi wrote:
Hi,
On Wed, Dec 15, 2010 at 04:44:13PM +0530, Pavankumar Kondeti wrote:
Introduce start_hnp callback function for HCD to receive notification
from EHCI core that HNP enabled port is suspended. HCD may initiate
HNP or notify the same to OTG via otg_start_hnp().
This patch is inspired by USB: Hook start_hnp into ohci struct
(e8b24450).
Change-Id: I8e258a6fdf42c166ea9cb3a727e4d3d28a8adc72
Signed-off-by: Pavankumar Kondeti pkond...@codeaurora.org
---
drivers/usb/host/ehci-hub.c | 11 +++
drivers/usb/host/ehci.h |2 ++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 796ea0c..65bf104 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -1018,6 +1018,17 @@ static int ehci_hub_control (
|| (temp PORT_RESET) != 0)
goto error;
+#ifdef CONFIG_USB_OTG
+if (hcd-self.otg_port == (wIndex + 1)
+hcd-self.b_hnp_enable
+ehci-start_hnp) {
The pointer is checked here...
+ehci_writel(ehci, temp | PORT_SUSPEND,
+status_reg);
+set_bit(wIndex, ehci-suspended_ports);
+ehci-start_hnp(ehci);
should you check if this pointer is valid before ?
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/5] USB core changes for supporting OTG on MSM SoC
On 12/16/2010 3:08 AM, Alan Stern wrote: On Wed, 15 Dec 2010, Pavankumar Kondeti wrote: This patch series adds OTG 2.0 enhancements and misc changes required for supporting SRP HNP in MSM OTG driver. As these patches modify the USB core code, I would like to receive feedback before posting driver patches. The driver patches are available at https://www.codeaurora.org/gitweb/quic/la/?p=kernel/msm.git;a=commitdiff;h=05535a995e9eb34c3c41d874d09955a443008cd5 All those #ifdef CONFIG_USB_OTG lines you are sprinkling throughout the core are very annoying. A few simple inline routines should allow you to eliminate many of them. I am doing OTG specific operations only after checking bus-hnp_support, udev-port == bus-otg_port, which will be true only for OTG device and CONFIG_USB_OTG is enabled. So I guess I can put them directly without #ifdef. Also, in the 5/5 patch in hub_activate(), all those if (hdev-bus-is_b_host type == HUB_INIT) tests are distracting. You can make the test once, at the start of the function, and store the result in a local variable. Okay. I will fix this in V2. Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 5/5] USB: Eliminate delays involved in root hub initialization during HNP
On 12/16/2010 5:45 PM, Felipe Balbi wrote: On Thu, Dec 16, 2010 at 04:56:15PM +0530, Pavan Kondeti wrote: On 12/15/2010 5:51 PM, Felipe Balbi wrote: On Wed, Dec 15, 2010 at 04:44:14PM +0530, Pavankumar Kondeti wrote: Some USB controllers have common resources (IRQ, register address space) for Host, Peripheral and OTG. So HCD is added only before entering into Host mode. Root hub initialization is done in different steps to decrease boot up time. But this makes B-device difficult to meet HNP timings. Hence eliminate delays involved in root hub initialization for B-host. I wonder if this is the best approach. Would it be easier to not touch usbcore, probe the entire stack during boot but have a core layer handling synchronization to shared resources ? The implementation is like this: Actually OTG synchronizes UDC and HCD. OTG driver probe will be called first and it takes care of turning on clocks, resetting controller and PHY and enabling interrupts (VBUS and Id). UDC and HCD will not modify hardware registers in their probe functions. So HCD will not call usb_add_hcd() function in it's probe(). After HCD and UDC registers with OTG via otg_set_xxx(), OTG activates HCD (by calling usb_add_hcd()) or UDC (usb_gadget_connect()) based on Id/VBUS status. When gadget is active, HCD is detached from USB core (usb_remove_hcd()). So user space can not poke into sysfs/debugfs nodes provided in the usb core. Maybe you could make your device an MFD device that allocates platform_devices for its children (HCI, UDC, OTG, etc) and pass in a bunch of read/write functions as platform_data for them to use as accessors to shared register space ?!? Would that work ? The easy way to deal this problem is removing HCD when Mini/Micro -A cable is detached. But why do you need to do that isn't clear. What's the problem in keeping HCD always available ? If it's a mini/micro-B cable you will enter peripheral role anyway. If you keep the HCD, root hub auto/system suspend can happen, user may un-configure the root hub or remove the HCD etc. There is not standard way of guarding against PM user for this kind of HCD. We had a discussion about this some time back. Please see http://www.spinics.net/lists/linux-usb/msg35847.html Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 4/4] USB: Eliminate delays involved in root hub initialization during HNP
On 12/16/2010 6:37 PM, Felipe Balbi wrote: On Thu, Dec 16, 2010 at 03:39:35PM +0300, Sergei Shtylyov wrote: Hello. On 16-12-2010 14:09, Pavankumar Kondeti wrote: Some USB controllers have common resources (IRQ, register address space) for Host, Peripheral and OTG. So HCD is added only before entering into Host mode. Root hub initialization is done in different steps to decrease boot up time. But this makes B-device difficult to meet HNP timings. Hence eliminate delays involved in root hub initialization for B-host. This patch also marks hnp_supported flag TRUE for B-host while registering the bus. Change-Id: I821775e8c90bd71a7abbe17176f189664a1841e1 Signed-off-by: Pavankumar Kondetipkond...@codeaurora.org [...] index ac79fd5..2df61ba 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -680,6 +680,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) int status; bool need_debounce_delay = false; unsigned delay; + bool hnp_in_progress = hdev-bus-is_b_host (type == HUB_INIT); Parens here are not necessary. there's also a missing space before There is one. Please see my original patch. I am not sure why it is missing in Sergei's reply. Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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] MSM: Add USB suport for QSD8x50
Hi Daniel, On 12/8/2010 1:37 PM, Pavankumar Kondeti wrote: OTG driver takes care of putting hardware into low power mode. Hence make peripheral and host devices as children of OTG device and let runtime PM takes care of notifying peripheral and host state to OTG device. VBUS power up and shutdown routines are implemented by modem processor. As RPC infrastructure is not available, configure USB in peripheral only mode. Signed-off-by: Pavankumar Kondeti pkond...@codeaurora.org --- This patch depends on [PATCH V3 00/11] Add MSM USB controller driver patch series which is under review. USB driver patches are taken into Greg KH usb-next tree. Is it possible to include this patch for 2.6.38? Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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 00/11] Add MSM USB controller driver
Hi All, On 12/7/2010 5:53 PM, Pavankumar Kondeti wrote: This patch series adds USB peripheral, host and OTG (role switch based on Id pin status) support for MSM SoC. The peripheral controller driver patches include splitting ci13xxx_udc into core and bus glue layers. Peripheral and Host controller drivers depend on Transceiver driver for clock management and low power mode. Peripheral and Host devices are made children of OTG device when devices are registered. Runtime PM functions are used by Peripheral and Host controller drivers to tell their state to transceiver driver which actually takes care of putting hardware into low power mode. Change from V2: 1. Added core_clock in transceiver driver. This clock is introduced from MSM7x30 onwards to remove dependency on AXI bus frequency. 2. Use debugfs instead of sysfs for role switching option. 3. pm_runtime_nocallbacks() is used instead for nop runtime PM methods. 4. Peripheral controller driver makes use of ci13xxx_udc core. Pavankumar Kondeti (11): USB: Add MSM OTG Controller driver USB: EHCI: Add MSM Host Controller driver USB: EHCI: msm: Add support for power management USB: OTG: msm: Add support for power management USB: gadget: Separate out PCI bus code from ci13xxx_udc USB: gadget: Fix scheduling while atomic bugs in ci13xxx_udc USB: gadget: Initialize ci13xxx gadget device's coherent DMA mask USB: gadget: Introduce ci13xxx_udc_driver struct USB: gadget: Add USB controller driver for MSM SoC USB: gadget: Implement runtime PM for ci13xxx gadget USB: gadget: Implement runtime PM for MSM bus glue driver I have received feedback to re-use the ci13xxx_udc driver when I submitted a new device controller driver (msm72k_udc.c) for MSM SoC. http://www.mail-archive.com/linux-arm-msm@vger.kernel.org/msg0.html I have split ci13xxx_udc into generic ci13xxx_udc core and PCI bus glue driver. I have added a new bus glue driver for MSM. We need this bus glue driver to perform MSM specific quirks after controller is reset and stopped (i.e after disabling pull-up in device mode). I have verified ci13xxx_udc driver on MSM SoC. I don't have PCI ci13xxx hardware to test ci13xxx on PCI. I have not made any changes in PCI driver. So in theory, these patches should not break PCI driver. It would be really nice if any one can test 5-11 patches of this series on PCI ci13xxx hardware. Thanks in advance, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Add MSM USB Device Controller driver
Hi Matthieu, On 11/19/2010 10:46 PM, Matthieu CASTET wrote: The best way to handle this is to introduce flags in the driver. For example look at drivers/mmc/host/sdhci.c (quirk flags). But for now, let's make work msm version. We can add workaround for other controller later. Now you should check if it is better to start on ci13xxx_udc or make generic your driver. I had worked a bit on ci13xxx_udc, and the code is sometimes messy, hard to understand. Also to making work on our core, we need to the attached patch. ci13xxx_udc is working fine on MSM SoC without the attached patch. According to hardware doc, When endpoint is in primed state the corresponding bit of either ENDPTRPIME or ENDPTSTAT will be set. The driver is assuming the same. So I don't see a problem. The current driver, one dTD is given to the hardware at a time. We can improve it by adding hardware queuing functionality or adopt msm72k_udc algorithm. I have submitted patches to separate PCI stuff out of ci13xxx_udc. I have submitted MSM bus glue driver which makes use of ci13xxx_udc. I have tested it on MSM SoC. But I don't have ci13xxx PCI hardware. Can you help me in testing my patches (5-11 of [PATCH V3 00/11] Add MSM USB controller driver) Thanks in advance, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/2] USB: Notify OTG errors to user space via uevents
On 12/7/2010 8:02 AM, pkond...@codeaurora.org wrote: Hi Greg, On Mon, Dec 06, 2010 at 06:07:50PM +0530, Pavankumar Kondeti wrote: OTG specification mandates no silent failures and all errors should be reported to the user. The spec itself does not give the exact error description. But recommends the error message to be self explanatory. Provide otg_notify_error() utility for USB core and OTG driver to send the error codes to user space. All the error code values are described in include/linux/usb/ch9.h. The user space application can listen to netlink socket and parse the buffer for MODULE=OTG and ERROR=n, where 'n' contains the error code. How are you going to listen to the netlink socket that is already grabbed by libudev? Sorry. I never worked with udev. But I read udev documentation. I thought an external script can be invoked by adding a udev rule when MODULE=OTG is matched and ERROR value can be accessed in the script via env variable. I ran the sample program @ http://www.kernel.org/doc/pending/hotplug.txt and udevd concurrently. I am able to capture all the uevents in sample program. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/2] USB: Notify OTG errors to user space via uevents
On 12/7/2010 9:20 AM, Greg KH wrote:
On Mon, Dec 06, 2010 at 06:32:30PM -0800, pkond...@codeaurora.org wrote:
Hi Greg,
On Mon, Dec 06, 2010 at 06:07:50PM +0530, Pavankumar Kondeti wrote:
OTG specification mandates no silent failures and all errors should
be reported to the user. The spec itself does not give the exact
error description. But recommends the error message to be self
explanatory. Provide otg_notify_error() utility for USB core and
OTG driver to send the error codes to user space. All the error
code values are described in include/linux/usb/ch9.h. The user space
application can listen to netlink socket and parse the buffer for
MODULE=OTG and ERROR=n, where 'n' contains the error code.
How are you going to listen to the netlink socket that is already
grabbed by libudev?
Sorry. I never worked with udev. But I read udev documentation.
I thought an external script can be invoked by adding a udev rule
when MODULE=OTG is matched and ERROR value can be accessed
in the script via env variable.
So, you created a new kernel/user ABI and never even tried it out to see
how someone would even use it?
{sigh}
I'm afraid to ask if you actually tested this code, let alone compiled
the thing...
I have tested this patch with a sample program @
http://www.kernel.org/doc/pending/hotplug.txt on busybox environment
where udevd is not running. After seeing your comments, I ran the same
program concurrently
with udev on Linux box.
Please, if you really want to do this, create your own netlink socket,
don't create new uevent messages that will just confused the existing
tools out there, that's ripe for big problems.
I have seen examples in drivers sending uevents for notifying docking
station status (drivers/acpi/dock.c)
That is when the hardware configuration of the system changes.
and when backlight brightness changes
(drivers/video/backlight/backlight.c).
Again, hardware configuration status changed.
Documentation/filesystems/gfs2-uevents.txt says OFFLINE uevent is sent
upon file system errors.
So I have taken this approach.
Please don't, this is NOT the way to get errors from the kernel back out
to userspace, because, as you have noted, this is not how any other
subsystem does it.
Thanks for your feedback. Is there any other way kernel can send errors
to user space asynchronously? is /dev/ interface acceptable?
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/2] USB: Notify OTG errors to user space via uevents
On 12/7/2010 10:12 AM, Greg KH wrote: Please don't, this is NOT the way to get errors from the kernel back out to userspace, because, as you have noted, this is not how any other subsystem does it. Thanks for your feedback. Is there any other way kernel can send errors to user space asynchronously? is /dev/ interface acceptable? Andi Kleen was working on a unified error reporting system for the kernel for things like this and other subsystems. Try working with him on this as it sounds like it would be what you need. Thanks for your inputs. I have looked at Andi Kleen's Unified error reporting -- A worthy goal? But it talks mostly about platform errors. I will work with him and find out if we can use his approach for notifying OTG errors to user space. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Add MSM USB Device Controller driver
Hi Dave, On 11/28/2010 12:00 PM, David Brownell wrote: implementing specific hooks needed for MSM. I am relying on gadget_is_xxx() macro for MSM specific workarounds. NEVER DO THAT from inside controller drivers. Here we are using the macro in ci13xxx_udc core which is included by two gadget controller drivers. (If the code isn't specific to the msm silicon, and works on other vendors' implementations too, it'd misbehave anyway. Those calls are for gadget drivers, e.g. to embed knowledge about controller issues which can't be detected by looking at what's advertised by the gadget controller device. Using them isn't encouraged. Controller drivers should see what hardware they're talking to by actually talking to the chip and detecting any quirks (be they revision-specific or otherwise). In some cases platform_data will be the way to package such information. We need the following hooks/extension in ci13xxx_udc for MSM SoC. 1. MSM USB device controller depends on OTG driver for clocks, Low power mode, and PHY initialization. So udc_probe() must fail if otg_get_transceiver() fails. 2. By default streaming mode is enabled in ci13xxx controller. It has to be disabled due to some known issues. 3. Currently, pull-up is enabled upon gadget driver registration. But we want to enable only upon VBUS presence. also controller must be reset before enabling the pull-up as the controller was in host mode before. 4. Hardware registers should not be accessed in udc_probe() I have started with flags to achieve the above and ended up with following: TRANSCEIVER_IS_REQUIRED: Fail udc_probe() if otg_get_transceiver() == NULL SKIP_RESET_IN_PROBE: avoid resetting the hardware in udc_prob() ENABLE_PULLUP_UPON_VBUS: Enable pull-up upon VBUS RESET_BEFORE_PULLUP: Reset the hardware before pull-up DISABLE_STREAM_MODE: Disable streaming mode A post reset callback to write into special registers after reset I thought introducing this many flags may not be a good idea and used gadget_is_xxx() macro to achieve the same. if tomorrow, some gadget controller wants to fail udc_probe() if otg_get_tranceiver(), they can too OR their gadget_is_xxx() macro. If you say using flags is not _bad_, I will take that approach. Thanks, Pavan -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Add MSM USB Device Controller driver
On 11/19/2010 10:46 PM, Matthieu CASTET wrote: Hi, Brian Swetland a écrit : On Tue, Nov 9, 2010 at 6:54 PM, David Brownell davi...@pacbell.net wrote: --- On Tue, 11/9/10, Pavan Kondeti pkond...@codeaurora.org wrote: Hi Matthieu, This look like the arc/chipidea/mips ehci otg core. Yes. It is chipidea core for ARM. Why can't you reuse the ci13xxx_udc.c driver That basic approach is FAR PREFERABLE. Fix the bugs once, tune once, and so forth, reuse the ULPI support, etc. Work on more platforms, since the silicon IP is reused. You'll end up with more folk who can help maintain the driver too, since the pool of potential helpers won't be limited to those who have/use MSM hardware. Just be sure to cleanly factor the bus (PCI vs MSM-s ARM platform flavor and SoC glues (bus-related). That factoring will likely be the hardest part; but there are examples of similar stuff in Linux today. The main headache is that this particular IP has different bugs in different instantiations (I know, for example, it exists in Tegra with a different set of issues around fetching descriptor heads and cache alignment, on MSM7201A after extensive testing we discovered there was no reliable way of adding a descriptor to a list of transactions once that queue was active, etc...), so things that work in one SoC may break another, etc, etc, but that's part of the adventure I suppose. I certainly agree that one unified driver is the way to go if you can make it all work. The best way to handle this is to introduce flags in the driver. For example look at drivers/mmc/host/sdhci.c (quirk flags). But for now, let's make work msm version. We can add workaround for other controller later. I am working on separating PCI stuff from ci13xxx_udc driver and implementing specific hooks needed for MSM. I am relying on gadget_is_xxx() macro for MSM specific workarounds. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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] msm: gpio: Add v2 gpio support to MSM SoCs.
Hi Greg,
On Tue, Nov 16, 2010 at 01:38:35PM -0800, Gregory Bean wrote:
Beginning with the MSM8x60, the hardware block responsible for gpio
support changes. Provide gpiolib support for the new v2 architecture.
Cc: Baruch Siach bar...@tkos.co.il
Signed-off-by: Gregory Bean gb...@codeaurora.org
+static int __devinit msm_gpio_probe(struct platform_device *dev)
+{
+ int ret;
+
+ spin_lock_init(tlmm_lock);
Not required. DEFINE_SPINLOCK() would have initialized the spinlock.
+ msm_gpio.label = dev-name;
+ ret = gpiochip_add(msm_gpio);
+
+ return ret;
+}
+
+static int __devexit msm_gpio_remove(struct platform_device *dev)
+{
+ int ret = gpiochip_remove(msm_gpio);
+
+ if (ret 0)
+ return ret;
+
+ set_irq_handler(TLMM_SCSS_SUMMARY_IRQ, NULL);
+
+ return 0;
+}
+
+static struct platform_driver msm_gpio_driver = {
+ .probe = msm_gpio_probe,
+ .remove = __devexit_p(msm_gpio_remove),
+ .driver = {
+ .name = msmgpio,
+ .owner = THIS_MODULE,
+ },
+};
+
+static struct platform_device msm_device_gpio = {
+ .name = msmgpio,
+ .id = 0,
+};
id = -1 right?
+
+static int __init msm_gpio_init(void)
+{
+ int rc;
+
+ rc = platform_driver_register(msm_gpio_driver);
+ if (rc == 0)
+ rc = platform_device_register(msm_device_gpio);
Should not we unregister the platform driver in case the above call returns
failure?
+
+ return rc;
+}
+
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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/4] USB: Add MSM OTG Controller driver
On Wed, Nov 17, 2010 at 09:20:16PM -0800, Greg KH wrote: On Thu, Nov 18, 2010 at 10:30:16AM +0530, Pavan Kondeti wrote: Hi Greg, On Wed, Nov 17, 2010 at 08:16:37AM -0800, Greg KH wrote: On Wed, Nov 17, 2010 at 03:53:52PM +0530, Pavan Kondeti wrote: Hi Greg, On Tue, Nov 16, 2010 at 01:43:46PM -0800, Greg KH wrote: On Tue, Nov 09, 2010 at 04:49:48PM +0530, Pavankumar Kondeti wrote: This driver implements PHY initialization, clock management, memory mapping register address space, ULPI IO ops and simple OTG state machine to kick host/peripheral based on Id/VBUS line status. VBUS/Id lines are tied to a reference voltage on some boards. Hence provide a sysfs interface to select host/peripheral mode. As you are creating a new user/kernel abi, it MUST be documented in the Documentation/ABI/ directory. I can't take this patch set until that happens. Thanks for letting me know this. I will add the documentation for the sysfs file. Also note that if you are adding a new ABI like this one, it needs to work the same for the other existing OTG drivers as well. So please also work to fix them to do the same thing, or change your code to work like the existing drivers do (hint, do the latter one...) I am not sure if other OTG driver require this sysfs file. That's the point, why does this driver require something that no other driver does? USB mode i.e Host/Peripheral is changed based on Id/VBUS status. But on some of the MSM boards, Id/VBUS is connected to reference voltage and need an additional sysfs file for user to change the operation mode. Are you sure this should be user selectable? Who is going to do that selection and how is it going to happen automatically like OTG is supposed to handle? The board I am using is a reference board where Id is grounded always. But we use it in peripheral mode for ADB (Android debugging Bridge). So user is going to write into the sysfs file (host/peripheral/none). But this is mainly for debugging/testing purpose. Where should a driver specific sysfs file should go in Doc/ABI/ ? Where all others are, there are lots of examples in that directory, including a README, right? Did you read that and it not explain things sufficiently? If so, please let me know what can be expanded on in that file to make it easier for others in the future. I have read the README file and it only talks about different levels of stability. I have not looked at examples. After looking at examples I understand that testing/sysfs-driver-usb-msm_otg is the correct file. My main complaint here is that you are creating a brand new kernel/userspace ABI, and you bury it in a driver patch without giving really any warning or description of it at all. This is something that we need to make sure we get correct as you (yes you) will be maintaining it for the next 12+ years. This is not something to do lightly at all, as I'm sure you can imagine. Okay. I understand now. Thanks for the explanation. I should not have used a sysfs file, which creates a new kernel/userspace ABI. I will use debugfs as it is mainly intended for debugging purpose. The only reason I have gone for sysfs is that to have this option available without CONFIG_DEBUGFS. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Add MSM USB Device Controller driver
Hi Igor,
On Wed, Nov 10, 2010 at 08:47:16AM +0200, Igor Grinberg wrote:
On 11/10/10 04:19, Pavan Kondeti wrote:
On Tue, Nov 09, 2010 at 05:36:28PM +0200, Igor Grinberg wrote:
On 11/09/10 15:52, Matthieu CASTET wrote:
Hi,
Pavankumar Kondeti a écrit :
diff --git a/drivers/usb/gadget/msm72k_udc.c
b/drivers/usb/gadget/msm72k_udc.c
new file mode 100644
index 000..3fa4192
--- /dev/null
+++ b/drivers/usb/gadget/msm72k_udc.c
+
+static unsigned ulpi_read(struct usb_info *ui, unsigned reg)
+{
+ unsigned timeout = 10;
+
+ /* initiate read operation */
+ writel(ULPI_RUN | ULPI_READ | ULPI_ADDR(reg),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while ((readl(USB_ULPI_VIEWPORT) ULPI_RUN) (--timeout))
+ ;
+
+ if (timeout == 0) {
+ dev_err(ui-pdev-dev, ulpi_read: timeout %08x\n,
+ readl(USB_ULPI_VIEWPORT));
+ return 0x;
+ }
+ return ULPI_DATA_READ(readl(USB_ULPI_VIEWPORT));
+}
+
+static int ulpi_write(struct usb_info *ui, unsigned val, unsigned reg)
+{
+ unsigned timeout = 1;
+
+ /* initiate write operation */
+ writel(ULPI_RUN | ULPI_WRITE |
+ ULPI_ADDR(reg) | ULPI_DATA(val),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while ((readl(USB_ULPI_VIEWPORT) ULPI_RUN) (--timeout))
+ ;
+
+ if (timeout == 0) {
+ dev_err(ui-pdev-dev, ulpi_write: timeout\n);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void ulpi_init(struct usb_info *ui)
+{
+ int *seq = ui-phy_init_seq;
+
+ if (!seq)
+ return;
+
+ while (seq[0] = 0) {
+ dev_vdbg(ui-pdev-dev, ulpi: write 0x%02x to
0x%02x\n,
+ seq[0], seq[1]);
+ ulpi_write(ui, seq[0], seq[1]);
+ seq += 2;
+ }
+}
+
--- /dev/null
+++ b/drivers/usb/otg/msm72k_otg.c
+
+#define ULPI_IO_TIMEOUT_USEC (10 * 1000)
+static int ulpi_read(struct otg_transceiver *otg, u32 reg)
+{
+ int cnt = 0;
+
+ /* initiate read operation */
+ writel(ULPI_RUN | ULPI_READ | ULPI_ADDR(reg),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while (cnt ULPI_IO_TIMEOUT_USEC) {
+ if (!(readl(USB_ULPI_VIEWPORT) ULPI_RUN))
+ break;
+ udelay(1);
+ cnt++;
+ }
+
+ if (cnt = ULPI_IO_TIMEOUT_USEC) {
+ dev_err(otg-dev, ulpi_read: timeout %08x\n,
+ readl(USB_ULPI_VIEWPORT));
+ return -ETIMEDOUT;
+ }
+ return ULPI_DATA_READ(readl(USB_ULPI_VIEWPORT));
+}
+
+static int ulpi_write(struct otg_transceiver *otg, u32 val, u32 reg)
+{
+ int cnt = 0;
+
+ /* initiate write operation */
+ writel(ULPI_RUN | ULPI_WRITE |
+ ULPI_ADDR(reg) | ULPI_DATA(val),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while (cnt ULPI_IO_TIMEOUT_USEC) {
+ if (!(readl(USB_ULPI_VIEWPORT) ULPI_RUN))
+ break;
+ udelay(1);
+ cnt++;
+ }
+
+ if (cnt = ULPI_IO_TIMEOUT_USEC) {
+ dev_err(otg-dev, ulpi_write: timeout\n);
+ return -ETIMEDOUT;
+ }
+ return 0;
+}
+
+static struct otg_io_access_ops msm_otg_io_ops = {
+ .read = ulpi_read,
+ .write = ulpi_write,
+};
+
+static void ulpi_init(struct msm_otg *motg)
+{
+ struct msm_otg_platform_data *pdata = motg-pdata;
+ int *seq = pdata-phy_init_seq;
+
+ if (!seq)
+ return;
+
+ while (seq[0] = 0) {
+ dev_vdbg(motg-otg.dev, ulpi: write 0x%02x to 0x%02x\n,
+ seq[0], seq[1]);
+ ulpi_write(motg-otg, seq[0], seq[1]);
+ seq += 2;
+ }
+}
can't you share the ulpi fonctions from udc and otg ?
The best would be to use the usb/otg/ulpi.c driver for the ulpi access.
This driver will eventually use msm72k_otg.c transceiver.
Well, I mean, in addition to defining the struct otg_io_access_ops in some
place,
where both device and host drivers can share the same ops code, you can also
use the ulpi generic driver as the protocol driver, instead of writing
private code.
I have not looked at generic ULPI driver. I will see if we can re-use it in
our msm72k_otg.c.
If the generic driver still lack some functionality, I can try to help you
with this.
Sure. Thanks for offering help.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm
Re: [PATCH v2] USB: Add MSM USB Device Controller driver
On Tue, Nov 09, 2010 at 02:36:57PM +0100, Matthieu CASTET wrote: Hi Pavan, Pavan Kondeti a écrit : On Tue, Nov 09, 2010 at 12:40:17PM +0100, Matthieu CASTET wrote: Hi Matthieu, This look like the arc/chipidea/mips ehci otg core. Yes. It is chipidea core for ARM. Why can't you reuse the ci13xxx_udc.c driver ? Or if ci13xxx_udc.c is too bad, rewrite a new generic version for this core. ci13xxx_udc.c driver registers with PCI subsytem (ours is a platform subsystem), does not manage clocks. msm72k_udc.c also takes care of initializing the integrated PHY. We also need to program special registers (MSM-ARM specific) upon resetting the hardware. Yes, but why not adding this to ci13xxx_udc.c instead of doing a new driver. For example adding platform subsystem is less than 150 lines of code. I agree. I am not sure doing n drivers for a same hardware is a good idea. May be ci13xxx_udc.c isn't enough generic, but in this case don't do the same mistake with the new driver. Sure. Thanks for the advice. Make a generic core, and make the ulpi stuff (or other specific stuff) in a glue around this code. IMHO, generic core should handle the controller stuff (endpoint operations, interrupts processing etc...) and leave the transceiver interaction to the individual device controller driver. Look at host ehci : you don't need to rewrite ehci core. You only add a glue around it. Thanks for pointing to EHCI. ehci-hcd can support PCI/platform/PS3 buses. It takes care of most of the things and yet gives flexibility to HCD to implement their own methods. I will hack ci13xxx_udc.c (platform bus support) and see how it goes. If it works, I will see how ci13xxx_ stuff can be reused efficiently. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Add MSM USB Device Controller driver
On Tue, Nov 09, 2010 at 02:52:09PM +0100, Matthieu CASTET wrote:
Hi,
Pavankumar Kondeti a écrit :
diff --git a/drivers/usb/gadget/msm72k_udc.c
b/drivers/usb/gadget/msm72k_udc.c
new file mode 100644
index 000..3fa4192
--- /dev/null
+++ b/drivers/usb/gadget/msm72k_udc.c
+
+static unsigned ulpi_read(struct usb_info *ui, unsigned reg)
+{
+ unsigned timeout = 10;
+
+ /* initiate read operation */
+ writel(ULPI_RUN | ULPI_READ | ULPI_ADDR(reg),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while ((readl(USB_ULPI_VIEWPORT) ULPI_RUN) (--timeout))
+ ;
+
+ if (timeout == 0) {
+ dev_err(ui-pdev-dev, ulpi_read: timeout %08x\n,
+ readl(USB_ULPI_VIEWPORT));
+ return 0x;
+ }
+ return ULPI_DATA_READ(readl(USB_ULPI_VIEWPORT));
+}
+
+static int ulpi_write(struct usb_info *ui, unsigned val, unsigned reg)
+{
+ unsigned timeout = 1;
+
+ /* initiate write operation */
+ writel(ULPI_RUN | ULPI_WRITE |
+ ULPI_ADDR(reg) | ULPI_DATA(val),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while ((readl(USB_ULPI_VIEWPORT) ULPI_RUN) (--timeout))
+ ;
+
+ if (timeout == 0) {
+ dev_err(ui-pdev-dev, ulpi_write: timeout\n);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void ulpi_init(struct usb_info *ui)
+{
+ int *seq = ui-phy_init_seq;
+
+ if (!seq)
+ return;
+
+ while (seq[0] = 0) {
+ dev_vdbg(ui-pdev-dev, ulpi: write 0x%02x to 0x%02x\n,
+ seq[0], seq[1]);
+ ulpi_write(ui, seq[0], seq[1]);
+ seq += 2;
+ }
+}
+
--- /dev/null
+++ b/drivers/usb/otg/msm72k_otg.c
+
+#define ULPI_IO_TIMEOUT_USEC (10 * 1000)
+static int ulpi_read(struct otg_transceiver *otg, u32 reg)
+{
+ int cnt = 0;
+
+ /* initiate read operation */
+ writel(ULPI_RUN | ULPI_READ | ULPI_ADDR(reg),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while (cnt ULPI_IO_TIMEOUT_USEC) {
+ if (!(readl(USB_ULPI_VIEWPORT) ULPI_RUN))
+ break;
+ udelay(1);
+ cnt++;
+ }
+
+ if (cnt = ULPI_IO_TIMEOUT_USEC) {
+ dev_err(otg-dev, ulpi_read: timeout %08x\n,
+ readl(USB_ULPI_VIEWPORT));
+ return -ETIMEDOUT;
+ }
+ return ULPI_DATA_READ(readl(USB_ULPI_VIEWPORT));
+}
+
+static int ulpi_write(struct otg_transceiver *otg, u32 val, u32 reg)
+{
+ int cnt = 0;
+
+ /* initiate write operation */
+ writel(ULPI_RUN | ULPI_WRITE |
+ ULPI_ADDR(reg) | ULPI_DATA(val),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while (cnt ULPI_IO_TIMEOUT_USEC) {
+ if (!(readl(USB_ULPI_VIEWPORT) ULPI_RUN))
+ break;
+ udelay(1);
+ cnt++;
+ }
+
+ if (cnt = ULPI_IO_TIMEOUT_USEC) {
+ dev_err(otg-dev, ulpi_write: timeout\n);
+ return -ETIMEDOUT;
+ }
+ return 0;
+}
+
+static struct otg_io_access_ops msm_otg_io_ops = {
+ .read = ulpi_read,
+ .write = ulpi_write,
+};
+
+static void ulpi_init(struct msm_otg *motg)
+{
+ struct msm_otg_platform_data *pdata = motg-pdata;
+ int *seq = pdata-phy_init_seq;
+
+ if (!seq)
+ return;
+
+ while (seq[0] = 0) {
+ dev_vdbg(motg-otg.dev, ulpi: write 0x%02x to 0x%02x\n,
+ seq[0], seq[1]);
+ ulpi_write(motg-otg, seq[0], seq[1]);
+ seq += 2;
+ }
+}
can't you share the ulpi fonctions from udc and otg ?
Yes. udc can use the ulpi ops defined by the otg. But in udc patch series, otg
stuff is not integrated. I will post another patch to make udc work with
otg. or you want me to add otg integration in the 1st patch series it self?
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm 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: Add MSM USB Device Controller driver
On Tue, Nov 09, 2010 at 05:36:28PM +0200, Igor Grinberg wrote:
On 11/09/10 15:52, Matthieu CASTET wrote:
Hi,
Pavankumar Kondeti a écrit :
diff --git a/drivers/usb/gadget/msm72k_udc.c
b/drivers/usb/gadget/msm72k_udc.c
new file mode 100644
index 000..3fa4192
--- /dev/null
+++ b/drivers/usb/gadget/msm72k_udc.c
+
+static unsigned ulpi_read(struct usb_info *ui, unsigned reg)
+{
+ unsigned timeout = 10;
+
+ /* initiate read operation */
+ writel(ULPI_RUN | ULPI_READ | ULPI_ADDR(reg),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while ((readl(USB_ULPI_VIEWPORT) ULPI_RUN) (--timeout))
+ ;
+
+ if (timeout == 0) {
+ dev_err(ui-pdev-dev, ulpi_read: timeout %08x\n,
+ readl(USB_ULPI_VIEWPORT));
+ return 0x;
+ }
+ return ULPI_DATA_READ(readl(USB_ULPI_VIEWPORT));
+}
+
+static int ulpi_write(struct usb_info *ui, unsigned val, unsigned reg)
+{
+ unsigned timeout = 1;
+
+ /* initiate write operation */
+ writel(ULPI_RUN | ULPI_WRITE |
+ ULPI_ADDR(reg) | ULPI_DATA(val),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while ((readl(USB_ULPI_VIEWPORT) ULPI_RUN) (--timeout))
+ ;
+
+ if (timeout == 0) {
+ dev_err(ui-pdev-dev, ulpi_write: timeout\n);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void ulpi_init(struct usb_info *ui)
+{
+ int *seq = ui-phy_init_seq;
+
+ if (!seq)
+ return;
+
+ while (seq[0] = 0) {
+ dev_vdbg(ui-pdev-dev, ulpi: write 0x%02x to 0x%02x\n,
+ seq[0], seq[1]);
+ ulpi_write(ui, seq[0], seq[1]);
+ seq += 2;
+ }
+}
+
--- /dev/null
+++ b/drivers/usb/otg/msm72k_otg.c
+
+#define ULPI_IO_TIMEOUT_USEC (10 * 1000)
+static int ulpi_read(struct otg_transceiver *otg, u32 reg)
+{
+ int cnt = 0;
+
+ /* initiate read operation */
+ writel(ULPI_RUN | ULPI_READ | ULPI_ADDR(reg),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while (cnt ULPI_IO_TIMEOUT_USEC) {
+ if (!(readl(USB_ULPI_VIEWPORT) ULPI_RUN))
+ break;
+ udelay(1);
+ cnt++;
+ }
+
+ if (cnt = ULPI_IO_TIMEOUT_USEC) {
+ dev_err(otg-dev, ulpi_read: timeout %08x\n,
+ readl(USB_ULPI_VIEWPORT));
+ return -ETIMEDOUT;
+ }
+ return ULPI_DATA_READ(readl(USB_ULPI_VIEWPORT));
+}
+
+static int ulpi_write(struct otg_transceiver *otg, u32 val, u32 reg)
+{
+ int cnt = 0;
+
+ /* initiate write operation */
+ writel(ULPI_RUN | ULPI_WRITE |
+ ULPI_ADDR(reg) | ULPI_DATA(val),
+ USB_ULPI_VIEWPORT);
+
+ /* wait for completion */
+ while (cnt ULPI_IO_TIMEOUT_USEC) {
+ if (!(readl(USB_ULPI_VIEWPORT) ULPI_RUN))
+ break;
+ udelay(1);
+ cnt++;
+ }
+
+ if (cnt = ULPI_IO_TIMEOUT_USEC) {
+ dev_err(otg-dev, ulpi_write: timeout\n);
+ return -ETIMEDOUT;
+ }
+ return 0;
+}
+
+static struct otg_io_access_ops msm_otg_io_ops = {
+ .read = ulpi_read,
+ .write = ulpi_write,
+};
+
+static void ulpi_init(struct msm_otg *motg)
+{
+ struct msm_otg_platform_data *pdata = motg-pdata;
+ int *seq = pdata-phy_init_seq;
+
+ if (!seq)
+ return;
+
+ while (seq[0] = 0) {
+ dev_vdbg(motg-otg.dev, ulpi: write 0x%02x to 0x%02x\n,
+ seq[0], seq[1]);
+ ulpi_write(motg-otg, seq[0], seq[1]);
+ seq += 2;
+ }
+}
can't you share the ulpi fonctions from udc and otg ?
The best would be to use the usb/otg/ulpi.c driver for the ulpi access.
This driver will eventually use msm72k_otg.c transceiver.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Add MSM USB Device Controller support
On Thu, Sep 02, 2010 at 12:52:43AM +0200, David Lanzendörfer wrote: Hi I tried your patchset with https://www.codeaurora.org/gitweb/quic/kernel/?p=dwalker/linux-msm.git;a=shortlog;h=refs/heads/for-next But somehow, usb networking doesnt seem to come up? O_o I'm sure, if you have tested it, the mistake has to be on my side. Could I try /your/ kernel config? (With leviathan-incoming [htc-msm-2.6.32-fork for GNU/Linux] the device comes up on bootup) Do you know which chipset(7x01/7x27/8x50) is used in your device? We have to setup the platform data and device in the respective board file. I have tested USB on 8x50 and it works fine. I will post a patch to add usb support for 8x50 today. Which gadget (android/ethernet) are you using? Also VBUS detection happens on Peripheral processor and Apps processor gets a RPC notification. But RPC infrastructure is not present in upstream msm. So you need to call msm_hsusb_vbus_state(1) after adding your platform device. I will see if I can add a debugfs file in the driver to manipulate vbus state. -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
