[PATCH] uwb: Staticize local symbols
These local symbols are used only in this file.
Fix the following sparse warnings:
drivers/uwb/drp-ie.c:30:5: warning: symbol 'uwb_rsv_reason_code' was not
declared. Should it be static?
drivers/uwb/drp-ie.c:58:5: warning: symbol 'uwb_rsv_companion_reason_code' was
not declared. Should it be static?
Signed-off-by: Jingoo Han
---
drivers/uwb/drp-ie.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/uwb/drp-ie.c b/drivers/uwb/drp-ie.c
index 5206731..b7d4f6b 100644
--- a/drivers/uwb/drp-ie.c
+++ b/drivers/uwb/drp-ie.c
@@ -27,7 +27,7 @@
/*
* Return the reason code for a reservations's DRP IE.
*/
-int uwb_rsv_reason_code(struct uwb_rsv *rsv)
+static int uwb_rsv_reason_code(struct uwb_rsv *rsv)
{
static const int reason_codes[] = {
[UWB_RSV_STATE_O_INITIATED] = UWB_DRP_REASON_ACCEPTED,
@@ -55,7 +55,7 @@ int uwb_rsv_reason_code(struct uwb_rsv *rsv)
/*
* Return the reason code for a reservations's companion DRP IE .
*/
-int uwb_rsv_companion_reason_code(struct uwb_rsv *rsv)
+static int uwb_rsv_companion_reason_code(struct uwb_rsv *rsv)
{
static const int companion_reason_codes[] = {
[UWB_RSV_STATE_O_MOVE_EXPANDING] = UWB_DRP_REASON_ACCEPTED,
--
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: [PATCH v3 7/8] ARM: dts: omap4: update omap-control-usb nodes
Hi Benoit,
On 08/16/2013 05:30 PM, Benoit Cousson wrote:
> Hi Roger,
>
> Sorry I missed something in the previous revision :-(
no problem :)
>
> On 16/08/2013 15:09, Roger Quadros wrote:
>> Split otghs_ctrl and USB2 PHY power down into separate
>> omap-control-usb nodes. Get rid of "ti,type" property.
>
> You should add that you update the usb_otg_hs node accordingly as well.
OK.
>
>> CC: Benoit Cousson
>> Signed-off-by: Roger Quadros
>> ---
>> arch/arm/boot/dts/omap4.dtsi | 20
>> 1 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 22d9f2b..a77dd0a 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -519,7 +519,7 @@
>> usb2_phy: usb2phy@4a0ad080 {
>> compatible = "ti,omap-usb2";
>> reg = <0x4a0ad080 0x58>;
>> -ctrl-module = <&omap_control_usb>;
>> +ctrl-module = <&omap_control_usb2phy>;
>> };
>> };
>>
>> @@ -643,12 +643,16 @@
>> };
>> };
>>
>> -omap_control_usb: omap-control-usb@4a002300 {
>> -compatible = "ti,omap-control-usb";
>> -reg = <0x4a002300 0x4>,
>> - <0x4a00233c 0x4>;
>> -reg-names = "control_dev_conf", "otghs_control";
>> -ti,type = <1>;
>> +omap_control_usb2phy: omap-control-usb@4a002300 {
>> +compatible = "ti,usb2-control-usb";
>> +reg = <0x4a002300 0x4>;
>> +reg-names = "power";
>> +};
>> +
>> +omap_control_usbotg: omap-control-usb@4a00233c {
>> +compatible = "ti,omap4-control-usb";
>> +reg = <0x4a00233c 0x4>;
>> +reg-names = "otghs_control";
>> };
>>
>> usb_otg_hs: usb_otg_hs@4a0ab000 {
>> @@ -661,7 +665,7 @@
>> multipoint = <1>;
>> num-eps = <16>;
>> ram-bits = <12>;
>> -ti,has-mailbox;
>> +ctrl-module = <&omap_control_usbotg>;
>
> In omap-usb.txt, ti,has-mailbox is still marked as mandatory whereas the
> ctrl-module is optional.
>
> You should update the usb-otg-hs bindings as well.
Right, I removed it from the binding information, but missed the example.
>
> BTW, why is that property not prefixed with "ti,"? Is ctrl-module really
> meaningful for other arch?
The ctrl-module thing is TI specific, but otg_hs is used by other platforms,
maybe that's why
the "ti," prefix was used.
cheers,
-roger
--
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: am335x: add wakeup support
Hi Sebastian,
On 08/16/2013 08:04 PM, Sebastian Andrzej Siewior wrote:
> This is based on George Cherian's patch which added power & wakeup
> support to am335x and does no longer apply since I took some if the code
> apart in favor of the multi instance support.
> This compiles and I boots and later it detects a device after I plug it in :)
> Could somebody please test it so it does what it should?
>
> Cc: George Cherian
> Cc: Roger Quadros
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> drivers/usb/phy/phy-am335x-control.c | 42
>
> drivers/usb/phy/phy-am335x.c | 35 ++
> 2 files changed, 77 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-am335x-control.c
> b/drivers/usb/phy/phy-am335x-control.c
> index 22cf07d..0fac976 100644
> --- a/drivers/usb/phy/phy-am335x-control.c
> +++ b/drivers/usb/phy/phy-am335x-control.c
> @@ -26,6 +26,41 @@ struct am335x_control_usb {
> #define USBPHY_OTGVDET_EN(1 << 19)
> #define USBPHY_OTGSESSEND_EN (1 << 20)
>
> +#define AM335X_PHY0_WK_EN(1 << 0)
> +#define AM335X_PHY1_WK_EN(1 << 8)
> +
> +static void am335x_phy_wkup(struct phy_control *phy_ctrl, u32 id, bool on)
> +{
> + struct am335x_control_usb *usb_ctrl;
> + u32 val;
> + u32 reg;
> +
> + usb_ctrl = container_of(phy_ctrl, struct am335x_control_usb, phy_ctrl);
> +
> + switch (id) {
> + case 0:
> + reg = AM335X_PHY0_WK_EN;
> + break;
> + case 1:
> + reg = AM335X_PHY1_WK_EN;
> + break;
> + default:
> + WARN_ON(1);
> + return;
> + }
> +
> + spin_lock(&usb_ctrl->lock);
> + val = readl(usb_ctrl->wkup);
> +
> + if (on)
> + val |= reg;
> + else
> + val &= ~reg;
> +
> + writel(val, usb_ctrl->wkup);
> + spin_unlock(&usb_ctrl->lock);
> +}
> +
> static void am335x_phy_power(struct phy_control *phy_ctrl, u32 id, bool on)
> {
> struct am335x_control_usb *usb_ctrl;
> @@ -59,6 +94,7 @@ static void am335x_phy_power(struct phy_control *phy_ctrl,
> u32 id, bool on)
>
> static const struct phy_control ctrl_am335x = {
> .phy_power = am335x_phy_power,
> + .phy_wkup = am335x_phy_wkup,
> };
>
> static const struct of_device_id omap_control_usb_id_table[] = {
> @@ -117,6 +153,12 @@ static int am335x_control_usb_probe(struct
> platform_device *pdev)
> ctrl_usb->phy_reg = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(ctrl_usb->phy_reg))
> return PTR_ERR(ctrl_usb->phy_reg);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "wakeup");
> + ctrl_usb->wkup = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(ctrl_usb->wkup))
> + return PTR_ERR(ctrl_usb->wkup);
> +
> spin_lock_init(&ctrl_usb->lock);
> ctrl_usb->phy_ctrl = *phy_ctrl;
>
> diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
> index c4d614d..1d0cd96 100644
> --- a/drivers/usb/phy/phy-am335x.c
> +++ b/drivers/usb/phy/phy-am335x.c
> @@ -79,6 +79,40 @@ static int am335x_phy_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM_RUNTIME
> +
> +static int am335x_phy_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct am335x_phy *am_phy = platform_get_drvdata(pdev);
> +
> + phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
> + if (device_may_wakeup(dev))
> + phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
Is it OK to configure the wakeup after the PHY is powered down?
Maybe it is OK, but just doesn't sound logical ;).
> + return 0;
> +}
...
cheers,
-roger
--
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 0/9] USB: phy: phy-nop: Manage RESET GPIO in the driver
Hi Felipe, On 08/16/2013 01:52 PM, Benoit Cousson wrote: > Hi Roger, > > On 15/08/2013 12:18, Roger Quadros wrote: >> Hi, >> >> Modelling the RESET line as a regulator supply wasn't a good idea >> as it abuses the regulator framework and makes adaptation >> code/data more complex. >> >> Instead, manage the RESET gpio line directly in the driver. >> >> This also makes us easy to migrate to a dedicated GPIO RESET controller >> whenever it becomes available. As of now, it doesn't seem to be making >> it into 3.12. >> >> *NOTE:* As there are changes to platform data, Patch 1 needs to be shared >> between the arm-soc tree and usb tree. >> >> Patch 1 is available at repo >> git://github.com/rogerq/linux.git >> in branch >> phy-reset-common >> >> Patch 2 contains the phy-nop driver changes >> Patches 3 and 4 adapt legacy boot code to the phy-nop driver changes. >> Patches 5, 6 and 7 adapt DT data to the binding changes. >> Patch 8 is cleanup of omap3-beagle DT. >> Patch 9 adds USB host support to omap3-beagle-xm using the new binding. >> >> Patches are based on v3.11-rc5. >> Tested leacy boot on omap3-beagle and omap3-beagle-xm >> Tested DT boot on omap3-beagle, omap3-beagle-xm and omap4-panda-es >> >> v2: >> - Added RESET GPIO polarity feature >> - Changed to gpio_set_value_cansleep() >> >> cheers, >> -roger >> >> --- >> Roger Quadros (9): >>usb: phy: nop: Add gpio_reset to platform data >>usb: phy: nop: Don't use regulator framework for RESET line >>ARM: OMAP2+: omap-usb-host: Get rid of platform_data from struct >> usbhs_phy_data >>ARM: OMAP2+: usb-host: Adapt to USB phy-nop RESET line changes >>ARM: dts: omap3-beagle: Use reset-gpios for hsusb2_reset >>ARM: dts: omap4-panda: Use reset-gpios for hsusb1_reset >>ARM: dts: omap5-uevm: Use reset-gpios for hsusb2_reset >>ARM: dts: omap3-beagle: Make USB host pin naming consistent >>ARM: dts: omap3-beagle-xm: Add USB Host support > > The 5 DTS patches looks good to me, and I can apply them, but you need to be > sure that the driver will be merged for 3.12 as well. Could you please let us know if this series can make it to 3.12? Thanks. cheers, -roger -- 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: FUSB200 xhci issue
Am 10.08.2013 13:57, schrieb Alan Stern: On Sat, 10 Aug 2013, Oleksij Rempel wrote: usb reset do not affect behaviour of firmware. At least after i remove all attempts to reboot FW from driver. If adapter will got reset signal, FW will be notified about it. Then FW will remove reset flag and will just continue to work. After usb reset, lsusb show correct, update information - EP3 and EP4 was updated from INT to BULK. I assume, no i need to add to the driver some kind of firmware check. What is the proper way to do it? The simplest way is to put a new value for the device descriptor's bcdDevice value in the firmware. Then all you have to do is check that value. Hi Alan, I need some more help. I reworked some firmware related parts of ath9k_htc driver and added usb_reset_device. As well dummy pre_reset and post_reset functions. Right now it will do double reset. First time after usb_reset_device host will detect changes in usb descriptor and set flag for logical disconnect. In this case driver will fail to load and after this actual disconnect btw reset will happen. Then, after reset, driver will be automatically loaded second time. This time usb_reset_device will go without problems, since usb descriptor was updated on first round. How driver should behave in this situation? I prefer to update firmware on every module load. -- Regards, Oleksij -- 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: dealing with enclosures that don't handle READ_CAPACITY10
On 08/12/2013 03:17 PM, Oliver Neukum wrote: > Hi, > > I got a bug report about an enclosure that mishandles READ_CAPACITY10. > I don't want to introduce yet another quirk. So what about basing this > on USB version? > Hmm. You _sure_ it's restricted to USB 3.0? Seem like a generic USB firmware issue to me, so I'd rather have it handled via a quirk. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- 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: octeon-usb and dwc2 in staging are for the same hw
On 08/17/2013 10:44 PM, Paul Zimmerman wrote: > Hi Greg, all, > > After taking a look at the Octeon driver, it looks like that controller > uses a customized version of the DWC2 core - it has a different DMA > engine than the one provided by the standard hardware. So in fact these > two drivers are actually not "for the same hw". Is it just the DMA engine? musb provides support for more than one DMA engine. Plus you may still be able to hide the dma engine behind drivers/dma > > I think it would be very complicated to combine both of these into a > common driver. So in my opinion the best solution is to keep both drivers. > 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] usb: phy: am335x: add wakeup support
On 08/19/2013 09:34 AM, Roger Quadros wrote:
> Hi Sebastian,
Hi Roger,
>> diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
>> index c4d614d..1d0cd96 100644
>> --- a/drivers/usb/phy/phy-am335x.c
>> +++ b/drivers/usb/phy/phy-am335x.c
>> @@ -79,6 +79,40 @@ static int am335x_phy_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM_RUNTIME
>> +
>> +static int am335x_phy_runtime_suspend(struct device *dev)
>> +{
>> +struct platform_device *pdev = to_platform_device(dev);
>> +struct am335x_phy *am_phy = platform_get_drvdata(pdev);
>> +
>> +phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
>> +if (device_may_wakeup(dev))
>> +phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
>
> Is it OK to configure the wakeup after the PHY is powered down?
> Maybe it is OK, but just doesn't sound logical ;).
I have no idea. So you say either powerdown the phy _or_ leave it
powered and activate wake up?
>
>> +return 0;
>> +}
> ...
>
> cheers,
> -roger
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] usb: phy: am335x: add wakeup support
On 08/19/2013 10:58 AM, Sebastian Andrzej Siewior wrote:
> On 08/19/2013 09:34 AM, Roger Quadros wrote:
>> Hi Sebastian,
>
> Hi Roger,
>
>>> diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
>>> index c4d614d..1d0cd96 100644
>>> --- a/drivers/usb/phy/phy-am335x.c
>>> +++ b/drivers/usb/phy/phy-am335x.c
>>> @@ -79,6 +79,40 @@ static int am335x_phy_remove(struct platform_device
>>> *pdev)
>>> return 0;
>>> }
>>>
>>> +#ifdef CONFIG_PM_RUNTIME
>>> +
>>> +static int am335x_phy_runtime_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct am335x_phy *am_phy = platform_get_drvdata(pdev);
>>> +
>>> + phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
>>> + if (device_may_wakeup(dev))
>>> + phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
>>
>> Is it OK to configure the wakeup after the PHY is powered down?
>> Maybe it is OK, but just doesn't sound logical ;).
>
> I have no idea. So you say either powerdown the phy _or_ leave it
> powered and activate wake up?
No, I just meant about the sequence, i.e. first configure the wakeup
and then power down the phy.
e.g.
if (device_may_wakeup())
set wakeup;
power down;
cheers,
-roger
--
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 0/6] simplify platform_get_resource_byname/devm_ioremap_resource
devm_ioremap_resource often uses the result of a call to
platform_get_resource_byname as its last argument. devm_ioremap_resource
does appropriate error handling on this argument, so error handling can be
removed from the call to platform_get_resource_byname.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
//
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l,f;
expression list es;
@@
(
res = f(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
|
- res = f(es);
... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
... when != res
+ res = f(es);
e = devm_ioremap_resource(e1, res);
)
//
In practice, f is always platform_get_resource_byname (or
platform_get_resource, which was handled by a previous patch series). And
the call to platform_get_resource_byname always immediately precedes the
call to devm_ioremap_resource.
--
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 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
From: Julia Lawall
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
//
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
//
Signed-off-by: Julia Lawall
---
drivers/usb/musb/musb_dsps.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..f2f9710 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
- if (!r)
- return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
if (!musb->ctrl_base)
return -EINVAL;
--
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: am335x: add wakeup support
On 08/19/2013 10:04 AM, Roger Quadros wrote: > No, I just meant about the sequence, i.e. first configure the wakeup > and then power down the phy. > > e.g. > > if (device_may_wakeup()) > set wakeup; > power down; I don't see the difference but yes, this can be arranged. > > cheers, > -roger > 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
[PATCH v2] usb: phy: am335x: add wakeup support
This is based on George Cherian's patch which added power & wakeup
support to am335x and does no longer apply since I took some if the code
apart in favor of the multi instance support.
This compiles and I boots and later it detects a device after I plug it in :)
Could somebody please test it so it does what it should?
Cc: George Cherian
Cc: Roger Quadros
Signed-off-by: Sebastian Andrzej Siewior
---
v1…v2:
- first set the wakeup bit then poweroff the phy.
drivers/usb/phy/phy-am335x-control.c | 42
drivers/usb/phy/phy-am335x.c | 35 ++
2 files changed, 77 insertions(+)
diff --git a/drivers/usb/phy/phy-am335x-control.c
b/drivers/usb/phy/phy-am335x-control.c
index 22cf07d..0fac976 100644
--- a/drivers/usb/phy/phy-am335x-control.c
+++ b/drivers/usb/phy/phy-am335x-control.c
@@ -26,6 +26,41 @@ struct am335x_control_usb {
#define USBPHY_OTGVDET_EN (1 << 19)
#define USBPHY_OTGSESSEND_EN (1 << 20)
+#define AM335X_PHY0_WK_EN (1 << 0)
+#define AM335X_PHY1_WK_EN (1 << 8)
+
+static void am335x_phy_wkup(struct phy_control *phy_ctrl, u32 id, bool on)
+{
+ struct am335x_control_usb *usb_ctrl;
+ u32 val;
+ u32 reg;
+
+ usb_ctrl = container_of(phy_ctrl, struct am335x_control_usb, phy_ctrl);
+
+ switch (id) {
+ case 0:
+ reg = AM335X_PHY0_WK_EN;
+ break;
+ case 1:
+ reg = AM335X_PHY1_WK_EN;
+ break;
+ default:
+ WARN_ON(1);
+ return;
+ }
+
+ spin_lock(&usb_ctrl->lock);
+ val = readl(usb_ctrl->wkup);
+
+ if (on)
+ val |= reg;
+ else
+ val &= ~reg;
+
+ writel(val, usb_ctrl->wkup);
+ spin_unlock(&usb_ctrl->lock);
+}
+
static void am335x_phy_power(struct phy_control *phy_ctrl, u32 id, bool on)
{
struct am335x_control_usb *usb_ctrl;
@@ -59,6 +94,7 @@ static void am335x_phy_power(struct phy_control *phy_ctrl,
u32 id, bool on)
static const struct phy_control ctrl_am335x = {
.phy_power = am335x_phy_power,
+ .phy_wkup = am335x_phy_wkup,
};
static const struct of_device_id omap_control_usb_id_table[] = {
@@ -117,6 +153,12 @@ static int am335x_control_usb_probe(struct platform_device
*pdev)
ctrl_usb->phy_reg = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(ctrl_usb->phy_reg))
return PTR_ERR(ctrl_usb->phy_reg);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "wakeup");
+ ctrl_usb->wkup = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ctrl_usb->wkup))
+ return PTR_ERR(ctrl_usb->wkup);
+
spin_lock_init(&ctrl_usb->lock);
ctrl_usb->phy_ctrl = *phy_ctrl;
diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index c4d614d..0639061 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -79,6 +79,40 @@ static int am335x_phy_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_RUNTIME
+
+static int am335x_phy_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct am335x_phy *am_phy = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
+ phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
+ return 0;
+}
+
+static int am335x_phy_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct am335x_phy *am_phy = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, false);
+ phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, true);
+ return 0;
+}
+
+static const struct dev_pm_ops am335x_pm_ops = {
+ SET_RUNTIME_PM_OPS(am335x_phy_runtime_suspend,
+ am335x_phy_runtime_resume, NULL)
+};
+
+#define DEV_PM_OPS (&am335x_pm_ops)
+#else
+#define DEV_PM_OPS NULL
+#endif
+
static const struct of_device_id am335x_phy_ids[] = {
{ .compatible = "ti,am335x-usb-phy" },
{ }
@@ -91,6 +125,7 @@ static struct platform_driver am335x_phy_driver = {
.driver = {
.name = "am335x-phy-driver",
.owner = THIS_MODULE,
+ .pm = DEV_PM_OPS,
.of_match_table = of_match_ptr(am335x_phy_ids),
},
};
--
1.8.4.rc2
--
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: am335x: add wakeup support
Sebastian,
On 08/19/2013 01:12 PM, Sebastian Andrzej Siewior wrote:
> This is based on George Cherian's patch which added power & wakeup
> support to am335x and does no longer apply since I took some if the code
> apart in favor of the multi instance support.
> This compiles and I boots and later it detects a device after I plug it in :)
> Could somebody please test it so it does what it should?
>
> Cc: George Cherian
> Cc: Roger Quadros
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> v1…v2:
> - first set the wakeup bit then poweroff the phy.
>
> drivers/usb/phy/phy-am335x-control.c | 42
>
> drivers/usb/phy/phy-am335x.c | 35 ++
> 2 files changed, 77 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-am335x-control.c
> b/drivers/usb/phy/phy-am335x-control.c
> index 22cf07d..0fac976 100644
> --- a/drivers/usb/phy/phy-am335x-control.c
> +++ b/drivers/usb/phy/phy-am335x-control.c
> @@ -26,6 +26,41 @@ struct am335x_control_usb {
> #define USBPHY_OTGVDET_EN(1 << 19)
> #define USBPHY_OTGSESSEND_EN (1 << 20)
>
> +#define AM335X_PHY0_WK_EN(1 << 0)
> +#define AM335X_PHY1_WK_EN(1 << 8)
> +
> +static void am335x_phy_wkup(struct phy_control *phy_ctrl, u32 id, bool on)
> +{
> + struct am335x_control_usb *usb_ctrl;
> + u32 val;
> + u32 reg;
> +
> + usb_ctrl = container_of(phy_ctrl, struct am335x_control_usb, phy_ctrl);
> +
> + switch (id) {
> + case 0:
> + reg = AM335X_PHY0_WK_EN;
> + break;
> + case 1:
> + reg = AM335X_PHY1_WK_EN;
> + break;
> + default:
> + WARN_ON(1);
> + return;
> + }
> +
> + spin_lock(&usb_ctrl->lock);
> + val = readl(usb_ctrl->wkup);
> +
> + if (on)
> + val |= reg;
> + else
> + val &= ~reg;
> +
> + writel(val, usb_ctrl->wkup);
> + spin_unlock(&usb_ctrl->lock);
> +}
> +
> static void am335x_phy_power(struct phy_control *phy_ctrl, u32 id, bool on)
> {
> struct am335x_control_usb *usb_ctrl;
> @@ -59,6 +94,7 @@ static void am335x_phy_power(struct phy_control *phy_ctrl,
> u32 id, bool on)
>
> static const struct phy_control ctrl_am335x = {
> .phy_power = am335x_phy_power,
> + .phy_wkup = am335x_phy_wkup,
> };
>
> static const struct of_device_id omap_control_usb_id_table[] = {
> @@ -117,6 +153,12 @@ static int am335x_control_usb_probe(struct
> platform_device *pdev)
> ctrl_usb->phy_reg = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(ctrl_usb->phy_reg))
> return PTR_ERR(ctrl_usb->phy_reg);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "wakeup");
> + ctrl_usb->wkup = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(ctrl_usb->wkup))
> + return PTR_ERR(ctrl_usb->wkup);
> +
> spin_lock_init(&ctrl_usb->lock);
> ctrl_usb->phy_ctrl = *phy_ctrl;
>
> diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
> index c4d614d..0639061 100644
> --- a/drivers/usb/phy/phy-am335x.c
> +++ b/drivers/usb/phy/phy-am335x.c
> @@ -79,6 +79,40 @@ static int am335x_phy_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM_RUNTIME
> +
> +static int am335x_phy_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct am335x_phy *am_phy = platform_get_drvdata(pdev);
> +
> + if (device_may_wakeup(dev))
> + phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
> + phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
> + return 0;
> +}
> +
> +static int am335x_phy_runtime_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct am335x_phy *am_phy = platform_get_drvdata(pdev);
> +
> + if (device_may_wakeup(dev))
> + phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, false);
> + phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, true);
You should have kept the resume side as it was earlier.
i.e.
1) power up the phy.
2) configure the wakeup.
For any generic device, it must be powered before any setting is changed.
> + return 0;
> +}
cheers,
-roger
--
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 v4 8/8] ARM: dts: omap5: update omap-control-usb node
Split USB2 PHY and USB3 PHY into separate omap-control-usb
nodes. Get rid of "ti,type" property.
CC: Benoit Cousson
Signed-off-by: Roger Quadros
---
arch/arm/boot/dts/omap5.dtsi | 20
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 07be2cd..7cda18b 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -626,12 +626,16 @@
hw-caps-temp-alert;
};
- omap_control_usb: omap-control-usb@4a002300 {
- compatible = "ti,omap-control-usb";
- reg = <0x4a002300 0x4>,
- <0x4a002370 0x4>;
- reg-names = "control_dev_conf", "phy_power_usb";
- ti,type = <2>;
+ omap_control_usb2phy: omap-control-usb@4a002300 {
+ compatible = "ti,usb2-control-usb";
+ reg = <0x4a002300 0x4>;
+ reg-names = "power";
+ };
+
+ omap_control_usb3phy: omap-control-usb@0x4a002370 {
+ compatible = "ti,usb3-control-usb";
+ reg = <0x4a002370 0x4>;
+ reg-names = "power";
};
omap_dwc3@4a02 {
@@ -661,7 +665,7 @@
usb2_phy: usb2phy@4a084000 {
compatible = "ti,omap-usb2";
reg = <0x4a084000 0x7c>;
- ctrl-module = <&omap_control_usb>;
+ ctrl-module = <&omap_control_usb2phy>;
};
usb3_phy: usb3phy@4a084400 {
@@ -670,7 +674,7 @@
<0x4a084800 0x64>,
<0x4a084c00 0x40>;
reg-names = "phy_rx", "phy_tx", "pll_ctrl";
- ctrl-module = <&omap_control_usb>;
+ ctrl-module = <&omap_control_usb3phy>;
};
};
--
1.7.4.1
--
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 v4 1/8] usb: phy: omap-control: Get rid of platform data
omap-control device is present from OMAP4 onwards which
support device tree boots only. So get rid of platform data.
Signed-off-by: Roger Quadros
---
drivers/usb/phy/phy-omap-control.c | 18 +++---
include/linux/usb/omap_control_usb.h |4
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index a4dda8e..7f446c4 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -197,8 +197,13 @@ static int omap_control_usb_probe(struct platform_device
*pdev)
{
struct resource *res;
struct device_node *np = pdev->dev.of_node;
- struct omap_control_usb_platform_data *pdata =
- dev_get_platdata(&pdev->dev);
+
+ if (np) {
+ of_property_read_u32(np, "ti,type", &control_usb->type);
+ } else {
+ /* We only support DT boot */
+ return -EINVAL;
+ }
control_usb = devm_kzalloc(&pdev->dev, sizeof(*control_usb),
GFP_KERNEL);
@@ -207,15 +212,6 @@ static int omap_control_usb_probe(struct platform_device
*pdev)
return -ENOMEM;
}
- if (np) {
- of_property_read_u32(np, "ti,type", &control_usb->type);
- } else if (pdata) {
- control_usb->type = pdata->type;
- } else {
- dev_err(&pdev->dev, "no pdata present\n");
- return -EINVAL;
- }
-
control_usb->dev= &pdev->dev;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
diff --git a/include/linux/usb/omap_control_usb.h
b/include/linux/usb/omap_control_usb.h
index 27b5b8c..e2416b4 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -31,10 +31,6 @@ struct omap_control_usb {
u32 type;
};
-struct omap_control_usb_platform_data {
- u8 type;
-};
-
enum omap_control_usb_mode {
USB_MODE_UNDEFINED = 0,
USB_MODE_HOST,
--
1.7.4.1
--
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 v4 5/8] usb: musb: omap2430: Don't use omap_get_control_dev()
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
Also get rid of "ti,has-mailbox" property as it is redundant and
we can determine that from whether "ctrl-module" property is present
or not. Get rid of has_mailbox from musb_hdrc_platform_data as well.
Signed-off-by: Roger Quadros
---
Documentation/devicetree/bindings/usb/omap-usb.txt |4 ---
drivers/usb/musb/omap2430.c| 25 +++
include/linux/usb/musb.h |2 -
3 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index e24078f..4dc9100 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -3,9 +3,6 @@ OMAP GLUE AND OTHER OMAP SPECIFIC COMPONENTS
OMAP MUSB GLUE
- compatible : Should be "ti,omap4-musb" or "ti,omap3-musb"
- ti,hwmods : must be "usb_otg_hs"
- - ti,has-mailbox : to specify that omap uses an external mailbox
- (in control module) to communicate with the musb core during device connect
- and disconnect.
- multipoint : Should be "1" indicating the musb controller supports
multipoint. This is a MUSB configuration-specific setting.
- num-eps : Specifies the number of endpoints. This is also a
@@ -28,7 +25,6 @@ SOC specific device node entry
usb_otg_hs: usb_otg_hs@4a0ab000 {
compatible = "ti,omap4-musb";
ti,hwmods = "usb_otg_hs";
- ti,has-mailbox;
multipoint = <1>;
num-eps = <16>;
ram-bits = <12>;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index ebb46ec..516795b 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include "musb_core.h"
#include "omap2430.h"
@@ -509,8 +510,12 @@ static int omap2430_probe(struct platform_device *pdev)
glue->dev = &pdev->dev;
glue->musb = musb;
glue->status= OMAP_MUSB_UNKNOWN;
+ glue->control_otghs = ERR_PTR(-ENODEV);
if (np) {
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
+
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata) {
dev_err(&pdev->dev,
@@ -539,22 +544,20 @@ static int omap2430_probe(struct platform_device *pdev)
of_property_read_u32(np, "ram-bits", (u32 *)&config->ram_bits);
of_property_read_u32(np, "power", (u32 *)&pdata->power);
config->multipoint = of_property_read_bool(np, "multipoint");
- pdata->has_mailbox = of_property_read_bool(np,
- "ti,has-mailbox");
pdata->board_data = data;
pdata->config = config;
- }
- if (pdata->has_mailbox) {
- glue->control_otghs = omap_get_control_dev();
- if (IS_ERR(glue->control_otghs)) {
- dev_vdbg(&pdev->dev, "Failed to get control device\n");
- ret = PTR_ERR(glue->control_otghs);
- goto err2;
+ control_node = of_parse_phandle(np, "ctrl-module", 0);
+ if (control_node) {
+ control_pdev = of_find_device_by_node(control_node);
+ if (!control_pdev) {
+ dev_err(&pdev->dev, "Failed to get control
device\n");
+ ret = -EINVAL;
+ goto err2;
+ }
+ glue->control_otghs = &control_pdev->dev;
}
- } else {
- glue->control_otghs = ERR_PTR(-ENODEV);
}
pdata->platform_ops = &omap2430_ops;
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..eb50525 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -99,8 +99,6 @@ struct musb_hdrc_platform_data {
/* MUSB_HOST, MUSB_PERIPHERAL, or MUSB_OTG */
u8 mode;
- u8 has_mailbox:1;
-
/* for clk_get() */
const char *clock;
--
1.7.4.1
--
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 v4 3/8] usb: phy: omap-usb2: Don't use omap_get_control_dev()
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
As we don't support non-DT boot, we just bail out on probe
if device node is not present.
Signed-off-by: Roger Quadros
---
drivers/usb/phy/phy-omap-usb2.c | 26 --
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..e278a3d 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -121,8 +122,14 @@ static int omap_usb2_suspend(struct usb_phy *x, int
suspend)
static int omap_usb2_probe(struct platform_device *pdev)
{
- struct omap_usb *phy;
- struct usb_otg *otg;
+ struct omap_usb *phy;
+ struct usb_otg *otg;
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
+
+ if (!node)
+ return -EINVAL;
phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -144,11 +151,18 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->phy.otg= otg;
phy->phy.type = USB_PHY_TYPE_USB2;
- phy->control_dev = omap_get_control_dev();
- if (IS_ERR(phy->control_dev)) {
- dev_dbg(&pdev->dev, "Failed to get control device\n");
- return -ENODEV;
+ control_node = of_parse_phandle(node, "ctrl-module", 0);
+ if (!control_node) {
+ dev_err(&pdev->dev, "Failed to get control device phandle\n");
+ return -EINVAL;
}
+ control_pdev = of_find_device_by_node(control_node);
+ if (!control_pdev) {
+ dev_err(&pdev->dev, "Failed to get control device\n");
+ return -EINVAL;
+ }
+
+ phy->control_dev = &control_pdev->dev;
phy->is_suspended = 1;
omap_control_usb_phy_power(phy->control_dev, 0);
--
1.7.4.1
--
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 v4 7/8] ARM: dts: omap4: update omap-control-usb nodes
Split otghs_ctrl and USB2 PHY power down into separate
omap-control-usb nodes. Get rid of "ti,type" property.
Also get rid of "ti,has-mailbox" property from usb_otg_hs
node and provide the ctrl-module phandle.
CC: Benoit Cousson
Signed-off-by: Roger Quadros
---
arch/arm/boot/dts/omap4.dtsi | 20
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..a77dd0a 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -519,7 +519,7 @@
usb2_phy: usb2phy@4a0ad080 {
compatible = "ti,omap-usb2";
reg = <0x4a0ad080 0x58>;
- ctrl-module = <&omap_control_usb>;
+ ctrl-module = <&omap_control_usb2phy>;
};
};
@@ -643,12 +643,16 @@
};
};
- omap_control_usb: omap-control-usb@4a002300 {
- compatible = "ti,omap-control-usb";
- reg = <0x4a002300 0x4>,
- <0x4a00233c 0x4>;
- reg-names = "control_dev_conf", "otghs_control";
- ti,type = <1>;
+ omap_control_usb2phy: omap-control-usb@4a002300 {
+ compatible = "ti,usb2-control-usb";
+ reg = <0x4a002300 0x4>;
+ reg-names = "power";
+ };
+
+ omap_control_usbotg: omap-control-usb@4a00233c {
+ compatible = "ti,omap4-control-usb";
+ reg = <0x4a00233c 0x4>;
+ reg-names = "otghs_control";
};
usb_otg_hs: usb_otg_hs@4a0ab000 {
@@ -661,7 +665,7 @@
multipoint = <1>;
num-eps = <16>;
ram-bits = <12>;
- ti,has-mailbox;
+ ctrl-module = <&omap_control_usbotg>;
};
};
};
--
1.7.4.1
--
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 v4 4/8] usb: phy: omap-usb3: Don't use omap_get_control_dev()
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
As we don't support non-DT boot, we just bail out on probe
if device node is not present.
Signed-off-by: Roger Quadros
---
drivers/usb/phy/phy-omap-usb3.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index 4a7f27c..981313e 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#definePLL_STATUS 0x0004
#definePLL_GO 0x0008
@@ -198,6 +199,12 @@ static int omap_usb3_probe(struct platform_device *pdev)
{
struct omap_usb *phy;
struct resource *res;
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
+
+ if (!node)
+ return -EINVAL;
phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -239,11 +246,18 @@ static int omap_usb3_probe(struct platform_device *pdev)
return -EINVAL;
}
- phy->control_dev = omap_get_control_dev();
- if (IS_ERR(phy->control_dev)) {
- dev_dbg(&pdev->dev, "Failed to get control device\n");
- return -ENODEV;
+ control_node = of_parse_phandle(node, "ctrl-module", 0);
+ if (!control_node) {
+ dev_err(&pdev->dev, "Failed to get control device phandle\n");
+ return -EINVAL;
}
+ control_pdev = of_find_device_by_node(control_node);
+ if (!control_pdev) {
+ dev_err(&pdev->dev, "Failed to get control device\n");
+ return -EINVAL;
+ }
+
+ phy->control_dev = &control_pdev->dev;
omap_control_usb_phy_power(phy->control_dev, 0);
usb_add_phy_dev(&phy->phy);
--
1.7.4.1
--
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 v4 6/8] usb: phy: omap: get rid of omap_get_control_dev()
This function was preventing us from supporting multiple
instances. Get rid of it. Since we support DT boots only,
users can get the control device phandle from the DT node.
Signed-off-by: Roger Quadros
---
drivers/usb/phy/phy-omap-control.c | 31 ++-
include/linux/usb/omap_control_usb.h |5 -
2 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 4c4c85c..1a7e19a 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -26,26 +26,6 @@
#include
#include
-static struct omap_control_usb *control_usb;
-
-/**
- * omap_get_control_dev - returns the device pointer for this control device
- *
- * This API should be called to get the device pointer for this control
- * module device. This device pointer should be used for called other
- * exported API's in this driver.
- *
- * To be used by PHY driver and glue driver.
- */
-struct device *omap_get_control_dev(void)
-{
- if (!control_usb)
- return ERR_PTR(-ENODEV);
-
- return control_usb->dev;
-}
-EXPORT_SYMBOL_GPL(omap_get_control_dev);
-
/**
* omap_control_usb_phy_power - power on/off the phy using control module reg
* @dev: the control module device
@@ -188,11 +168,19 @@ void omap_control_usb_set_mode(struct device *dev,
{
struct omap_control_usb *ctrl_usb;
- if (IS_ERR(dev) || control_usb->type != OMAP_CTRL_TYPE_OMAP4)
+ if (IS_ERR(dev) || !dev)
return;
ctrl_usb = dev_get_drvdata(dev);
+ if (!ctrl_usb) {
+ dev_err(dev, "Invalid control usb device\n");
+ return;
+ }
+
+ if (ctrl_usb->type != OMAP_CTRL_TYPE_OMAP4)
+ return;
+
switch (mode) {
case USB_MODE_HOST:
omap_control_usb_host_mode(ctrl_usb);
@@ -215,6 +203,7 @@ static int omap_control_usb_probe(struct platform_device
*pdev)
{
struct resource *res;
const struct of_device_id *of_id;
+ struct omap_control_usb *control_usb;
of_id = of_match_device(omap_control_usb_id_table, &pdev->dev);
if (!of_id)
diff --git a/include/linux/usb/omap_control_usb.h
b/include/linux/usb/omap_control_usb.h
index 450b5c1..8008e8d 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -65,15 +65,10 @@ enum omap_control_usb_mode {
#define OMAP_CTRL_USB2_PHY_PD BIT(28)
#if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
-extern struct device *omap_get_control_dev(void);
extern void omap_control_usb_phy_power(struct device *dev, int on);
extern void omap_control_usb_set_mode(struct device *dev,
enum omap_control_usb_mode mode);
#else
-static inline struct device *omap_get_control_dev(void)
-{
- return ERR_PTR(-ENODEV);
-}
static inline void omap_control_usb_phy_power(struct device *dev, int on)
{
--
1.7.4.1
--
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 v4 0/8] phy: omap-usb: Support multiple instances and new types
Hi, This patchset does the following: * Get rid of omap_control_usb platform data as we support DT only. * Restructure and add support for new PHY types. We now support the follwing four types TYPE_OMAP - if it has otghs_control mailbox register (e.g. on OMAP4) TYPE_USB2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY TYPE_USB3 - if it has DPLL and individual Rx & Tx power control. e.g. USB3 PHY or SATA PHY TYPE_DRA7 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7 * Get rid of "ti,type" DT property and introduce compatible strings for each type. * Have only one power control API "omap_control_usb_phy_power()" instead of a different one for each PHY type. * Get rid of omap_get_control_dev() so that we can support multiple instances of the control device. We take advantage of the fact that omap control USB device is only present on OMAP4 onwards and hence only supports DT boot. The users of control USB device can get a reference to it from the device node's phandle. Patches are based on balbi/next. Smoke tested on OMAP4 panda with MUSB in gadget mode (g_zero). You can find the patches in branch usb-control-module in git tree git://github.com/rogerq/linux.git v4: - removed "ti,has-mailbox" from omap-usb binding document example. v3: - return -EINVAL instead of -ENODEV on probe failure. - pass device type data through of_device_id table. - get rid of "ti,mailbox" property and "has_mailbox" from musb platform data. v2: - get rid of "ti,type" property and introduce compatible strings for each device type. cheers, -roger --- Roger Quadros (8): usb: phy: omap-control: Get rid of platform data usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power() usb: phy: omap-usb2: Don't use omap_get_control_dev() usb: phy: omap-usb3: Don't use omap_get_control_dev() usb: musb: omap2430: Don't use omap_get_control_dev() usb: phy: omap: get rid of omap_get_control_dev() ARM: dts: omap4: update omap-control-usb nodes ARM: dts: omap5: update omap-control-usb node Documentation/devicetree/bindings/usb/omap-usb.txt | 33 ++-- arch/arm/boot/dts/omap4.dtsi | 20 ++- arch/arm/boot/dts/omap5.dtsi | 20 ++- drivers/usb/musb/omap2430.c| 25 ++- drivers/usb/phy/phy-omap-control.c | 210 +++- drivers/usb/phy/phy-omap-usb2.c| 26 ++- drivers/usb/phy/phy-omap-usb3.c| 28 ++- include/linux/usb/musb.h |2 - include/linux/usb/omap_control_usb.h | 33 ++-- 9 files changed, 224 insertions(+), 173 deletions(-) -- 1.7.4.1 -- 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 v4 2/8] usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()
Add support for new device types and in the process rid of "ti,type"
device tree property. The correct type of device will be determined
from the compatible string instead.
Introduce a compatible string for each device type. At the moment
we support 4 types Mailbox, USB2, USB3 and DRA7.
Update DT binding information to reflect these changes.
Also get rid of omap_control_usb3_phy_power(). Just one function
i.e. omap_control_usb_phy_power() will now take care of all PHY types.
Signed-off-by: Roger Quadros
---
Documentation/devicetree/bindings/usb/omap-usb.txt | 29 ++--
drivers/usb/phy/phy-omap-control.c | 173
drivers/usb/phy/phy-omap-usb3.c|6 +-
include/linux/usb/omap_control_usb.h | 24 ++--
4 files changed, 137 insertions(+), 95 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..e24078f 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -73,22 +73,23 @@ omap_dwc3 {
OMAP CONTROL USB
Required properties:
- - compatible: Should be "ti,omap-control-usb"
+ - compatible: Should be one of
+ "ti,omap4-control-usb" - if it has otghs_control mailbox register as on OMAP4.
+ "ti,usb2-control-usb" - if it has Power down bit in control_dev_conf register
+ e.g. USB2_PHY on OMAP5.
+ "ti,usb3-control-usb" - if it has DPLL and individual Rx & Tx power control
+ e.g. USB3 PHY and SATA PHY on OMAP5.
+ "ti,dra7-control-usb" - if it has both power down and power aux registers
+ e.g. USB2 PHY on DRA7 platform.
- reg : Address and length of the register set for the device. It contains
- the address of "control_dev_conf" and "otghs_control" or "phy_power_usb"
- depending upon omap4 or omap5.
- - reg-names: The names of the register addresses corresponding to the
registers
- filled in "reg".
- - ti,type: This is used to differentiate whether the control module has
- usb mailbox or usb3 phy power. omap4 has usb mailbox in control module to
- notify events to the musb core and omap5 has usb3 phy power register to
- power on usb3 phy. Should be "1" if it has mailbox and "2" if it has usb3
- phy power.
+ the address of "otghs_control" for omap4-control-usb or "power" register
+ for other types. For dra7-control-usb, it must also contain "power_aux"
+ register.
+ - reg-names: should be otghs_control for omap4-control-usb and "power" for
+ other types. For dra7-control-usb type it must also contain "power_aux".
omap_control_usb: omap-control-usb@4a002300 {
compatible = "ti,omap-control-usb";
- reg = <0x4a002300 0x4>,
- <0x4a00233c 0x4>;
- reg-names = "control_dev_conf", "otghs_control";
- ti,type = <1>;
+ reg = <0x4a00233c 0x4>;
+ reg-names = "otghs_control";
};
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 7f446c4..4c4c85c 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -46,61 +47,76 @@ struct device *omap_get_control_dev(void)
EXPORT_SYMBOL_GPL(omap_get_control_dev);
/**
- * omap_control_usb3_phy_power - power on/off the serializer using control
- * module
+ * omap_control_usb_phy_power - power on/off the phy using control module reg
* @dev: the control module device
- * @on: 0 to off and 1 to on based on powering on or off the PHY
- *
- * usb3 PHY driver should call this API to power on or off the PHY.
+ * @on: 0 or 1, based on powering on or off the PHY
*/
-void omap_control_usb3_phy_power(struct device *dev, bool on)
+void omap_control_usb_phy_power(struct device *dev, int on)
{
- u32 val;
+ u32 val, val_aux;
unsigned long rate;
- struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+ struct omap_control_usb *control_usb;
- if (control_usb->type != OMAP_CTRL_DEV_TYPE2)
+ if (IS_ERR(dev) || !dev) {
+ pr_err("%s: invalid device\n", __func__);
return;
+ }
- rate = clk_get_rate(control_usb->sys_clk);
- rate = rate/100;
+ control_usb = dev_get_drvdata(dev);
+ if (!control_usb) {
+ dev_err(dev, "%s: invalid control usb device\n", __func__);
+ return;
+ }
- val = readl(control_usb->phy_power);
+ if (control_usb->type == OMAP_CTRL_TYPE_OMAP4)
+ return;
- if (on) {
- val &= ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
- OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
- val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON <<
- OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
- val |= rate << OMAP_CTRL_USB_PWRCTL_CLK_FREQ_
[PATCH v3] usb: phy: am335x: add wakeup support
This is based on George Cherian's patch which added power & wakeup
support to am335x and does no longer apply since I took some if the code
apart in favor of the multi instance support.
This compiles and I boots and later it detects a device after I plug it in :)
Could somebody please test it so it does what it should?
Cc: George Cherian
Cc: Roger Quadros
Signed-off-by: Sebastian Andrzej Siewior
---
v2…v3:
- don't reverse the order on resume
v1…v2:
- first set the wakeup bit then poweroff the phy.
drivers/usb/phy/phy-am335x-control.c | 42
drivers/usb/phy/phy-am335x.c | 35 ++
2 files changed, 77 insertions(+)
diff --git a/drivers/usb/phy/phy-am335x-control.c
b/drivers/usb/phy/phy-am335x-control.c
index 22cf07d..0fac976 100644
--- a/drivers/usb/phy/phy-am335x-control.c
+++ b/drivers/usb/phy/phy-am335x-control.c
@@ -26,6 +26,41 @@ struct am335x_control_usb {
#define USBPHY_OTGVDET_EN (1 << 19)
#define USBPHY_OTGSESSEND_EN (1 << 20)
+#define AM335X_PHY0_WK_EN (1 << 0)
+#define AM335X_PHY1_WK_EN (1 << 8)
+
+static void am335x_phy_wkup(struct phy_control *phy_ctrl, u32 id, bool on)
+{
+ struct am335x_control_usb *usb_ctrl;
+ u32 val;
+ u32 reg;
+
+ usb_ctrl = container_of(phy_ctrl, struct am335x_control_usb, phy_ctrl);
+
+ switch (id) {
+ case 0:
+ reg = AM335X_PHY0_WK_EN;
+ break;
+ case 1:
+ reg = AM335X_PHY1_WK_EN;
+ break;
+ default:
+ WARN_ON(1);
+ return;
+ }
+
+ spin_lock(&usb_ctrl->lock);
+ val = readl(usb_ctrl->wkup);
+
+ if (on)
+ val |= reg;
+ else
+ val &= ~reg;
+
+ writel(val, usb_ctrl->wkup);
+ spin_unlock(&usb_ctrl->lock);
+}
+
static void am335x_phy_power(struct phy_control *phy_ctrl, u32 id, bool on)
{
struct am335x_control_usb *usb_ctrl;
@@ -59,6 +94,7 @@ static void am335x_phy_power(struct phy_control *phy_ctrl,
u32 id, bool on)
static const struct phy_control ctrl_am335x = {
.phy_power = am335x_phy_power,
+ .phy_wkup = am335x_phy_wkup,
};
static const struct of_device_id omap_control_usb_id_table[] = {
@@ -117,6 +153,12 @@ static int am335x_control_usb_probe(struct platform_device
*pdev)
ctrl_usb->phy_reg = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(ctrl_usb->phy_reg))
return PTR_ERR(ctrl_usb->phy_reg);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "wakeup");
+ ctrl_usb->wkup = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(ctrl_usb->wkup))
+ return PTR_ERR(ctrl_usb->wkup);
+
spin_lock_init(&ctrl_usb->lock);
ctrl_usb->phy_ctrl = *phy_ctrl;
diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index c4d614d..d8209f9 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -79,6 +79,40 @@ static int am335x_phy_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_RUNTIME
+
+static int am335x_phy_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct am335x_phy *am_phy = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
+ phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
+ return 0;
+}
+
+static int am335x_phy_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct am335x_phy *am_phy = platform_get_drvdata(pdev);
+
+ phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, true);
+ if (device_may_wakeup(dev))
+ phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, false);
+ return 0;
+}
+
+static const struct dev_pm_ops am335x_pm_ops = {
+ SET_RUNTIME_PM_OPS(am335x_phy_runtime_suspend,
+ am335x_phy_runtime_resume, NULL)
+};
+
+#define DEV_PM_OPS (&am335x_pm_ops)
+#else
+#define DEV_PM_OPS NULL
+#endif
+
static const struct of_device_id am335x_phy_ids[] = {
{ .compatible = "ti,am335x-usb-phy" },
{ }
@@ -91,6 +125,7 @@ static struct platform_driver am335x_phy_driver = {
.driver = {
.name = "am335x-phy-driver",
.owner = THIS_MODULE,
+ .pm = DEV_PM_OPS,
.of_match_table = of_match_ptr(am335x_phy_ids),
},
};
--
1.8.4.rc2
--
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] usb: phy: am335x: add wakeup support
On 08/19/2013 01:39 PM, Sebastian Andrzej Siewior wrote: > This is based on George Cherian's patch which added power & wakeup > support to am335x and does no longer apply since I took some if the code > apart in favor of the multi instance support. > This compiles and I boots and later it detects a device after I plug it in :) > Could somebody please test it so it does what it should? > > Cc: George Cherian > Cc: Roger Quadros > Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Roger Quadros > --- > v2…v3: > - don't reverse the order on resume > > v1…v2: > - first set the wakeup bit then poweroff the phy. > > drivers/usb/phy/phy-am335x-control.c | 42 > > drivers/usb/phy/phy-am335x.c | 35 ++ > 2 files changed, 77 insertions(+) > cheers, -roger -- 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 1/3] USB: use percpu counter to count submitted URBs per device
Because usb_hcd_submit_urb is in the hotest path of usb core,
so use percpu counter to count URB instead of using atomic variable
because atomic operations are much slower than percpu operations.
Cc: Oliver Neukum
Cc: Alan Stern
Signed-off-by: Ming Lei
---
drivers/usb/core/hcd.c |4 ++--
drivers/usb/core/sysfs.c |7 ++-
drivers/usb/core/usb.c |9 -
drivers/usb/core/usb.h |1 +
include/linux/usb.h |2 +-
5 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 19ad3d2..0b4d1ae 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1556,7 +1556,7 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
*/
usb_get_urb(urb);
atomic_inc(&urb->use_count);
- atomic_inc(&urb->dev->urbnum);
+ this_cpu_inc(*urb->dev->urbnum);
usbmon_urb_submit(&hcd->self, urb);
/* NOTE requirements on root-hub callers (usbfs and the hub
@@ -1583,7 +1583,7 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
urb->hcpriv = NULL;
INIT_LIST_HEAD(&urb->urb_list);
atomic_dec(&urb->use_count);
- atomic_dec(&urb->dev->urbnum);
+ this_cpu_dec(*urb->dev->urbnum);
if (atomic_read(&urb->reject))
wake_up(&usb_kill_urb_queue);
usb_put_urb(urb);
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d9284b9..707f2ca 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -237,9 +237,14 @@ static ssize_t
show_urbnum(struct device *dev, struct device_attribute *attr, char *buf)
{
struct usb_device *udev;
+ unsigned int cnt = 0;
+ int i;
udev = to_usb_device(dev);
- return sprintf(buf, "%d\n", atomic_read(&udev->urbnum));
+ for_each_possible_cpu(i)
+ cnt += *per_cpu_ptr(udev->urbnum, i);
+
+ return sprintf(buf, "%d\n", cnt);
}
static DEVICE_ATTR(urbnum, S_IRUGO, show_urbnum, NULL);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 0a6ee2e..5111edb 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -271,6 +271,7 @@ static void usb_release_dev(struct device *dev)
kfree(udev->product);
kfree(udev->manufacturer);
kfree(udev->serial);
+ free_percpu(udev->urbnum);
kfree(udev);
}
@@ -433,7 +434,13 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
set_dev_node(&dev->dev, dev_to_node(bus->controller));
dev->state = USB_STATE_ATTACHED;
dev->lpm_disable_count = 1;
- atomic_set(&dev->urbnum, 0);
+
+ dev->urbnum = alloc_percpu(typeof(*dev->urbnum));
+ if (!dev->urbnum) {
+ usb_put_hcd(bus_to_hcd(bus));
+ kfree(dev);
+ return NULL;
+ }
INIT_LIST_HEAD(&dev->ep0.urb_list);
dev->ep0.desc.bLength = USB_DT_ENDPOINT_SIZE;
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 8238577..12a0181 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -1,5 +1,6 @@
#include
#include
+#include
struct usb_hub_descriptor;
struct dev_state;
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 001629c..75332dc 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -561,7 +561,7 @@ struct usb_device {
int maxchild;
u32 quirks;
- atomic_t urbnum;
+ unsigned int __percpu *urbnum;
unsigned long active_duration;
--
1.7.9.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
[PATCH 3/3] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
This patch introduces ehci_disable_event(), which is applied on
IAA_WATCHDOG and START_UNLINK_INTR events in case that the two
events needn't to be handled, so that we may avoid unnecessary CPU
wakeup.
Cc: Alan Stern
Signed-off-by: Ming Lei
---
drivers/usb/host/ehci-hcd.c | 12 +---
drivers/usb/host/ehci-sched.c |4 +++-
drivers/usb/host/ehci-timer.c | 14 ++
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 73c7299..549da36 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -738,17 +738,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
if (status & STS_IAA) {
/* Turn off the IAA watchdog */
- ehci->enabled_hrtimer_events &= ~BIT(EHCI_HRTIMER_IAA_WATCHDOG);
-
- /*
-* Mild optimization: Allow another IAAD to reset the
-* hrtimer, if one occurs before the next expiration.
-* In theory we could always cancel the hrtimer, but
-* tests show that about half the time it will be reset
-* for some other event anyway.
-*/
- if (ehci->next_hrtimer_event == EHCI_HRTIMER_IAA_WATCHDOG)
- ++ehci->next_hrtimer_event;
+ ehci_disable_event(ehci, EHCI_HRTIMER_IAA_WATCHDOG);
/* guard against (alleged) silicon errata */
if (cmd & CMD_IAAD)
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 6631089..83be03f 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -610,9 +610,11 @@ static void cancel_unlink_wait_intr(struct ehci_hcd *ehci,
struct ehci_qh *qh)
list_del_init(&qh->unlink_node);
/*
-* TODO: disable the event of EHCI_HRTIMER_START_UNLINK_INTR for
+* disable the event of EHCI_HRTIMER_START_UNLINK_INTR for
* avoiding unnecessary CPU wakeup
*/
+ if (list_empty(&ehci->intr_unlink_wait))
+ ehci_disable_event(ehci, EHCI_HRTIMER_START_UNLINK_INTR);
}
static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
diff --git a/drivers/usb/host/ehci-timer.c b/drivers/usb/host/ehci-timer.c
index 424ac5d..89bce50 100644
--- a/drivers/usb/host/ehci-timer.c
+++ b/drivers/usb/host/ehci-timer.c
@@ -100,6 +100,20 @@ static void ehci_enable_event(struct ehci_hcd *ehci,
unsigned event,
}
+/* Warning: don't call this function from hrtimer handler context */
+static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event)
+{
+ ehci->enabled_hrtimer_events &= ~(1 << event);
+ if (!ehci->enabled_hrtimer_events) {
+ ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
+ hrtimer_cancel(&ehci->hrtimer);
+ } else if (ehci->next_hrtimer_event == event) {
+ ehci->next_hrtimer_event =
+ ffs(ehci->enabled_hrtimer_events) - 1;
+ }
+}
+
+
/* Poll the STS_ASS status bit; see when it agrees with CMD_ASE */
static void ehci_poll_ASS(struct ehci_hcd *ehci)
{
--
1.7.9.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
[PATCH] USB: mos7720: fix big-endian control requests
Fix endianess bugs in parallel-port code which caused corrupt control-requests to be issued on big-endian machines. Reported-by: kbuild test robot Cc: sta...@vger.kernel.org Signed-off-by: Johan Hovold --- Here's another endianess fix detected by sparse after a recent change to these lines which reproduced the endianess bugs. Thanks, Johan drivers/usb/serial/mos7720.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c index b013001..c724ed5 100644 --- a/drivers/usb/serial/mos7720.c +++ b/drivers/usb/serial/mos7720.c @@ -382,8 +382,8 @@ static int write_parport_reg_nonblock(struct mos7715_parport *mos_parport, } urbtrack->setup->bRequestType = (__u8)0x40; urbtrack->setup->bRequest = (__u8)0x0e; - urbtrack->setup->wValue = get_reg_value(reg, dummy); - urbtrack->setup->wIndex = get_reg_index(reg); + urbtrack->setup->wValue = cpu_to_le16(get_reg_value(reg, dummy)); + urbtrack->setup->wIndex = cpu_to_le16(get_reg_index(reg)); urbtrack->setup->wLength = 0; usb_fill_control_urb(urbtrack->urb, usbdev, usb_sndctrlpipe(usbdev, 0), -- 1.8.3.2 -- 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
[RFC PATCH 2/3] USB: kill urb->use_count atomic variable
This patch kills atomic_inc/atomic_dec operations on
urb->use_count in URB submit/complete path.
The urb->use_count is only used for unlinking URB, and it isn't
necessary defined as atomic counter, so the variable is renamed
as urb->use_flag for this purpose, then reading/writing the
flag is still kept as atomic but ARCH's atomic operations(atomic_inc/
atomic_dec) are saved.
Cc: Oliver Neukum
Cc: Alan Stern
Signed-off-by: Ming Lei
---
drivers/usb/core/hcd.c | 14 +-
drivers/usb/core/urb.c |8 ++--
drivers/usb/core/usb.h |5 +
include/linux/usb.h|2 +-
4 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 0b4d1ae..9457c4e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1555,7 +1555,7 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
* an error or calls giveback(), but not both.
*/
usb_get_urb(urb);
- atomic_inc(&urb->use_count);
+ atomic_set(&urb->use_flag, URB_USING);
this_cpu_inc(*urb->dev->urbnum);
usbmon_urb_submit(&hcd->self, urb);
@@ -1582,7 +1582,7 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
usbmon_urb_submit_error(&hcd->self, urb, status);
urb->hcpriv = NULL;
INIT_LIST_HEAD(&urb->urb_list);
- atomic_dec(&urb->use_count);
+ atomic_set(&urb->use_flag, URB_UNUSED);
this_cpu_dec(*urb->dev->urbnum);
if (atomic_read(&urb->reject))
wake_up(&usb_kill_urb_queue);
@@ -1628,11 +1628,11 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
/* Prevent the device and bus from going away while
* the unlink is carried out. If they are already gone
-* then urb->use_count must be 0, since disconnected
+* then urb->use_flag must be URB_UNUSED, since disconnected
* devices can't have any active URBs.
*/
spin_lock_irqsave(&hcd_urb_unlink_lock, flags);
- if (atomic_read(&urb->use_count) > 0) {
+ if (atomic_read(&urb->use_flag) != URB_UNUSED) {
retval = 0;
usb_get_dev(urb->dev);
}
@@ -1672,6 +1672,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
/* pass ownership to the completion handler */
urb->status = status;
+ atomic_set(&urb->use_flag, URB_UNUSING);
+
/*
* We disable local IRQs here avoid possible deadlock because
* drivers may call spin_lock() to hold lock which might be
@@ -1686,7 +1688,9 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
urb->complete(urb);
local_irq_restore(flags);
- atomic_dec(&urb->use_count);
+ if (atomic_read(&urb->use_flag) == URB_UNUSING)
+ atomic_set(&urb->use_flag, URB_UNUSED);
+
if (unlikely(atomic_read(&urb->reject)))
wake_up(&usb_kill_urb_queue);
usb_put_urb(urb);
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index c12bc79..79d2534 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -9,6 +9,8 @@
#include
#include
+#include "usb.h"
+
#define to_urb(d) container_of(d, struct urb, kref)
@@ -661,7 +663,8 @@ void usb_kill_urb(struct urb *urb)
atomic_inc(&urb->reject);
usb_hcd_unlink_urb(urb, -ENOENT);
- wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
+ wait_event(usb_kill_urb_queue,
+ atomic_read(&urb->use_flag) == URB_UNUSED);
atomic_dec(&urb->reject);
}
@@ -705,7 +708,8 @@ void usb_poison_urb(struct urb *urb)
return;
usb_hcd_unlink_urb(urb, -ENOENT);
- wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
+ wait_event(usb_kill_urb_queue,
+ atomic_read(&urb->use_flag) == URB_UNUSED);
}
EXPORT_SYMBOL_GPL(usb_poison_urb);
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 12a0181..bbdcf93 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -5,6 +5,11 @@
struct usb_hub_descriptor;
struct dev_state;
+/* urb use flag*/
+#define URB_UNUSED 0
+#define URB_USING 1
+#define URB_UNUSING2
+
/* Functions local to drivers/usb/core/ */
extern int usb_create_sysfs_dev_files(struct usb_device *dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 75332dc..7f5f629 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1408,7 +1408,7 @@ struct urb {
/* private: usb core and host controller only fields in the urb */
struct kref kref; /* reference count of the URB */
void *hcpriv; /* private data for host controller */
- atomic_t use_count; /* concurrent submissions counter */
+ atomic_t use_flag; /* urb using flag */
atomic_t reject;/* submissio
Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
On 19-08-2013 10:51, Julia Lawall wrote: diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 4ffbaac..f2f9710 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb) u32 rev, val; r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control"); - if (!r) - return -EINVAL; - reg_base = devm_ioremap_resource(dev, r); if (!musb->ctrl_base) return -EINVAL; Not really related to Julia's patch, apart from making it more obvious that there's a bug here. I believe it is reg_base that needs to be checked, right? Svenning -- 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 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
On Mon, 19 Aug 2013, Svenning Sørensen wrote: > On 19-08-2013 10:51, Julia Lawall wrote: > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > > index 4ffbaac..f2f9710 100644 > > --- a/drivers/usb/musb/musb_dsps.c > > +++ b/drivers/usb/musb/musb_dsps.c > > @@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb) > > u32 rev, val; > > r = platform_get_resource_byname(parent, IORESOURCE_MEM, > > "control"); > > - if (!r) > > - return -EINVAL; > > - > > reg_base = devm_ioremap_resource(dev, r); > > if (!musb->ctrl_base) > > return -EINVAL; > Not really related to Julia's patch, apart from making it more obvious that > there's a bug here. > I believe it is reg_base that needs to be checked, right? Indeed, it is all backwards. I could extend the patch, if you want. julia
[PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
From: Julia Lawall
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to
devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
//
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
//
This patch also adjusts the error-handling code on the call to
devm_ioremap_resource to check the right value, noticed by Svenning Sørensen.
Signed-off-by: Julia Lawall
---
v2: Fixed the bug on the test of the result of devm_ioremap_resource
drivers/usb/musb/musb_dsps.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..e60be6f 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
- if (!r)
- return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
- if (!musb->ctrl_base)
- return -EINVAL;
+if (IS_ERR(reg_base))
+ return PTR_ERR(reg_base);
musb->ctrl_base = reg_base;
/* NOP driver needs change if supporting dual instance */
Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
On 19-08-2013 13:35, Julia Lawall wrote: reg_base = devm_ioremap_resource(dev, r); if (!musb->ctrl_base) return -EINVAL; Not really related to Julia's patch, apart from making it more obvious that there's a bug here. I believe it is reg_base that needs to be checked, right? Indeed, it is all backwards. I could extend the patch, if you want. I'll let Felipe decide on that, but I can't imagine any objections. IS_ERR() is the proper test, of course :) Svenning -- 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 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
On Mon, 19 Aug 2013, Svenning Sørensen wrote: > > On 19-08-2013 13:35, Julia Lawall wrote: > > reg_base = devm_ioremap_resource(dev, r); > > if (!musb->ctrl_base) > > return -EINVAL; > > > Not really related to Julia's patch, apart from making it more obvious > > > that > > > there's a bug here. > > > I believe it is reg_base that needs to be checked, right? > > Indeed, it is all backwards. I could extend the patch, if you want. > > > I'll let Felipe decide on that, but I can't imagine any objections. > IS_ERR() is the proper test, of course :) I've already sent a patch, in case it is useful. julia
Re: Non-enumerable devices on USB and other enumerable buses
On Sat, Aug 17, 2013 at 9:29 AM, Alan Stern wrote: > On Fri, 16 Aug 2013, Mark Brown wrote: > >> > Besides, you need to get the platform information to the driver in any >> > case, no matter how you decide to solve the chicken-and-egg problem. >> > It shouldn't be a factor in deciding which solution to use. >> >> It's not that this is hard, it's that I don't see how if you already >> have some concept of the device in the kernel data structures (which you >> must have in order to be able to provide platform data when it's needed) >> anything is gained by not using that when dealing with bootstrapping >> issues. > > I agree. In fact, there's no choice but to use this device concept > during startup. Otherwise there's no way to get the platform data to > the driver when it is needed, because there's no way to tell which > device the data applies to. The question is how elaborate the concept > needs to be and how it gets used. > > Aong those lines, I would like to point out that the device concept > embodied in the kernel's data structures can be pretty thin. For > example, it might be little more than a port number or bus address. Maybe the principle behind drivers/usb/core/usb-acpi.c is helpful for the problem, and DT may refer to ACPI to describe on-board USB devices, and the way to retrieve platform data too. >> Anyway, I think it's time to try to implement something rather than talk >> about it. > > Hopefully this discussion has given you some ideas for alternative > approachs, or at least helped to solidify your ideas. Thanks, -- Ming Lei -- 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/3] usb: dwc3: msm: Add device tree binding information
Hi, On Fri, 2013-08-16 at 16:44 -0600, Stephen Warren wrote: > On 08/14/2013 06:59 AM, Ivan T. Ivanov wrote: > > From: "Ivan T. Ivanov" > > > > MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys > > (SNPS) and HS, SS PHY's control and configuration registers. > > > > It could operate in device mode (SS, HS, FS) and host > > mode (SS, HS, FS, LS). > > > diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt > > b/Documentation/devicetree/bindings/usb/msm-ssusb.txt > > > +- clock-names : > ... > > + "sleep_a_clk" : Sleep clock, used when USB3 core goes into low > ... > > + "ref_clk" : Reference clock - used in host mode. > ... > > + "core_clk" : Master/Core clock, have to be >= 125 MHz for SS > ... > > + "iface_clk" : System bus AXI clock > > + "sleep_clk" : Sleep clock, used when USB3 core goes into low > ... > > + "utmi_clk" : Generated by HS-PHY. Used to clock the low power > > I think it makes sense to remove "_clk" from all those names, unless the > HW documentation really talks about a clock named e.g. iface_clk yet > some other clock names in the documentation don't have the "_clk" > suffix, e.g. the "xo I didn't quote. >From limited information that I have, I could not say how clock inputs are named from the controller perspective, but I agree that "_clk" suffix looks redundant. Side question: if for example label in controller says "UTMI", should I also use capital letters for the resource or this could be "utmi"? > > > +Sub nodes: > > +== > > That section title is the same style as all the other section title, so > it's no obvious that this is a sub-node for the controller wrapper. > Instead, I would suggest something more like: > > Required child nodes: > > > +- Sub node for "DWC3 USB3 controller". > > Then you can drop that since it's obvious. > > > + This sub node is required property for device node. The properties > > + of this subnode are specified in dwc3.txt. > > That doesn't really say much. How about. > > -- > A child node must exist to represent the core DWC3 IP block. The name of > the node is not important. The content of the node is defined in dwc3.txt. > -- Thanks, I will use your suggestion. Regards, Ivan -- 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 v1 45/49] sound: usb: caiaq: prepare for enabling irq in complete()
At Sun, 18 Aug 2013 00:25:10 +0800,
Ming Lei wrote:
>
> Complete() will be run with interrupt enabled, so change to
> spin_lock_irqsave().
>
> Cc: Jaroslav Kysela
> Cc: Takashi Iwai
Acked-by: Takashi Iwai
thanks,
Takashi
> Cc: alsa-de...@alsa-project.org
> Acked-by: Daniel Mack
> Signed-off-by: Ming Lei
> ---
> sound/usb/caiaq/audio.c |5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
> index 7103b09..e5675ab 100644
> --- a/sound/usb/caiaq/audio.c
> +++ b/sound/usb/caiaq/audio.c
> @@ -672,10 +672,11 @@ static void read_completed(struct urb *urb)
> offset += len;
>
> if (len > 0) {
> - spin_lock(&cdev->spinlock);
> + unsigned long flags;
> + spin_lock_irqsave(&cdev->spinlock, flags);
> fill_out_urb(cdev, out, &out->iso_frame_desc[outframe]);
> read_in_urb(cdev, urb, &urb->iso_frame_desc[frame]);
> - spin_unlock(&cdev->spinlock);
> + spin_unlock_irqrestore(&cdev->spinlock, flags);
> check_for_elapsed_periods(cdev, cdev->sub_playback);
> check_for_elapsed_periods(cdev, cdev->sub_capture);
> send_it = 1;
> --
> 1.7.9.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
Re: [PATCH v1 44/49] sound: usb: midi: prepare for enabling irq in complete()
At Sun, 18 Aug 2013 00:25:09 +0800,
Ming Lei wrote:
>
> Complete() will be run with interrupt enabled, so change to
> spin_lock_irqsave().
>
> Cc: Jaroslav Kysela
> Cc: Takashi Iwai
Acked-by: Takashi Iwai
thanks,
Takashi
> Cc: Clemens Ladisch
> Cc: alsa-de...@alsa-project.org
> Signed-off-by: Ming Lei
> ---
> sound/usb/midi.c |5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> index b901f46..86af276 100644
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -279,15 +279,16 @@ static void snd_usbmidi_out_urb_complete(struct urb*
> urb)
> struct out_urb_context *context = urb->context;
> struct snd_usb_midi_out_endpoint* ep = context->ep;
> unsigned int urb_index;
> + unsigned long flags;
>
> - spin_lock(&ep->buffer_lock);
> + spin_lock_irqsave(&ep->buffer_lock, flags);
> urb_index = context - ep->urbs;
> ep->active_urbs &= ~(1 << urb_index);
> if (unlikely(ep->drain_urbs)) {
> ep->drain_urbs &= ~(1 << urb_index);
> wake_up(&ep->drain_wait);
> }
> - spin_unlock(&ep->buffer_lock);
> + spin_unlock_irqrestore(&ep->buffer_lock, flags);
> if (urb->status < 0) {
> int err = snd_usbmidi_urb_error(urb->status);
> if (err < 0) {
> --
> 1.7.9.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
Re: [PATCH 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: > Because usb_hcd_submit_urb is in the hotest path of usb core, > so use percpu counter to count URB instead of using atomic variable > because atomic operations are much slower than percpu operations. > > Cc: Oliver Neukum > Cc: Alan Stern > Signed-off-by: Ming Lei > --- > drivers/usb/core/hcd.c |4 ++-- > drivers/usb/core/sysfs.c |7 ++- > drivers/usb/core/usb.c |9 - > drivers/usb/core/usb.h |1 + > include/linux/usb.h |2 +- > 5 files changed, 18 insertions(+), 5 deletions(-) And this really speeds things up? Exactly what does it? And it's not that atomic operations are "slower", it's just that the barriers involved can be slower, depending on what else is happening. If you look, you are already hitting atomic variables in the same path, so how can this change speed anything up? 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
Re: [PATCH v4 4/8] usb: phy: omap-usb3: Don't use omap_get_control_dev()
Hello.
On 08/19/2013 02:37 PM, Roger Quadros wrote:
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
As we don't support non-DT boot, we just bail out on probe
if device node is not present.
Signed-off-by: Roger Quadros
---
drivers/usb/phy/phy-omap-usb3.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index 4a7f27c..981313e 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
[...]
@@ -198,6 +199,12 @@ static int omap_usb3_probe(struct platform_device *pdev)
{
struct omap_usb *phy;
struct resource *res;
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
Could you align the variable names with the above 2 variables?
Either that, or remove the alignment of those two.
WBR, Sergei
--
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 v4 4/8] usb: phy: omap-usb3: Don't use omap_get_control_dev()
Hi Sergei,
On 08/19/2013 05:23 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 08/19/2013 02:37 PM, Roger Quadros wrote:
>
>> omap_get_control_dev() is being deprecated as it doesn't support
>> multiple instances. As control device is present only from OMAP4
>> onwards which supports DT only, we use phandles to get the
>> reference to the control device.
>
>> As we don't support non-DT boot, we just bail out on probe
>> if device node is not present.
>
>> Signed-off-by: Roger Quadros
>> ---
>> drivers/usb/phy/phy-omap-usb3.c | 22 ++
>> 1 files changed, 18 insertions(+), 4 deletions(-)
>
>> diff --git a/drivers/usb/phy/phy-omap-usb3.c
>> b/drivers/usb/phy/phy-omap-usb3.c
>> index 4a7f27c..981313e 100644
>> --- a/drivers/usb/phy/phy-omap-usb3.c
>> +++ b/drivers/usb/phy/phy-omap-usb3.c
> [...]
>> @@ -198,6 +199,12 @@ static int omap_usb3_probe(struct platform_device *pdev)
>> {
>> struct omap_usb*phy;
>> struct resource*res;
>> +struct device_node *node = pdev->dev.of_node;
>> +struct device_node *control_node;
>> +struct platform_device *control_pdev;
>
>Could you align the variable names with the above 2 variables?
> Either that, or remove the alignment of those two.
OK, I'll remove the alignment of the 1st two lines.
cheers,
-roger
--
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 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
Hello.
On 08/19/2013 03:47 PM, Julia Lawall wrote:
From: Julia Lawall
Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to
devm_ioremap_resource.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
//
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@
res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
//
This patch also adjusts the error-handling code on the call to
devm_ioremap_resource to check the right value, noticed by Svenning Sørensen.
Saying "also" in the changelog is usuallly a good sign the patch should be
split.
Signed-off-by: Julia Lawall
---
v2: Fixed the bug on the test of the result of devm_ioremap_resource
drivers/usb/musb/musb_dsps.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..e60be6f 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
- if (!r)
- return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
- if (!musb->ctrl_base)
- return -EINVAL;
+if (IS_ERR(reg_base))
+ return PTR_ERR(reg_base);
This is indented with space, not tabs. And of course, this was a matter of
a separate *fix* patch, while the original patch was a *cleanup*.
WBR, Sergei
--
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 3/3] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Mon, Aug 19, 2013 at 07:04:20PM +0800, Ming Lei wrote: > This patch introduces ehci_disable_event(), which is applied on > IAA_WATCHDOG and START_UNLINK_INTR events in case that the two > events needn't to be handled, so that we may avoid unnecessary CPU > wakeup. Why would those events not need to be handled? What does this help with? Measurements please. 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 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
On Mon, 19 Aug 2013, Sergei Shtylyov wrote:
> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> > From: Julia Lawall
>
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource_byname when the value is passed to
> > devm_ioremap_resource.
>
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
>
> > //
> > @@
> > expression pdev,res,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> >res = platform_get_resource_byname(...);
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> >e = devm_ioremap_resource(e1, res);
> > //
>
> > This patch also adjusts the error-handling code on the call to
> > devm_ioremap_resource to check the right value, noticed by Svenning
> > Sørensen.
>
>Saying "also" in the changelog is usuallly a good sign the patch should be
> split.
OK, the original patch already does the first part. I will send a new
patch for the bug fix.
julia
>
> > Signed-off-by: Julia Lawall
>
> > ---
> > v2: Fixed the bug on the test of the result of devm_ioremap_resource
>
> > drivers/usb/musb/musb_dsps.c |7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 4ffbaac..e60be6f 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> >
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> > - if (!r)
> > - return -EINVAL;
> > -
> > reg_base = devm_ioremap_resource(dev, r);
> > - if (!musb->ctrl_base)
> > - return -EINVAL;
> > +if (IS_ERR(reg_base))
> > + return PTR_ERR(reg_base);
>
>This is indented with space, not tabs. And of course, this was a matter of
> a separate *fix* patch, while the original patch was a *cleanup*.
>
> WBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Re: FUSB200 xhci issue
On Mon, 19 Aug 2013, Oleksij Rempel wrote: > >> I assume, no i need to add to the driver some kind of firmware check. > >> What is the proper way to do it? > > > > The simplest way is to put a new value for the device descriptor's > > bcdDevice value in the firmware. Then all you have to do is check that > > value. > > Hi Alan, > > I need some more help. I reworked some firmware related parts of > ath9k_htc driver and added usb_reset_device. As well dummy pre_reset and > post_reset functions. > Right now it will do double reset. First time after usb_reset_device > host will detect changes in usb descriptor and set flag for logical > disconnect. In this case driver will fail to load and after this actual > disconnect btw reset will happen. Then, after reset, driver will be > automatically loaded second time. This time usb_reset_device will go > without problems, since usb descriptor was updated on first round. > > How driver should behave in this situation? I prefer to update firmware > on every module load. Why? If the firmware has already been updated, why waste time updating it again? If you don't update the firmware when it is already present, then the device won't get reset a second time. 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: [RFC PATCH 2/3] USB: kill urb->use_count atomic variable
On Mon, Aug 19, 2013 at 07:04:19PM +0800, Ming Lei wrote: > This patch kills atomic_inc/atomic_dec operations on > urb->use_count in URB submit/complete path. Any reason only this patch was "RFC"? And you didn't kill them all, please look closer, this should have no affect on "speed", did you measure it? > The urb->use_count is only used for unlinking URB, and it isn't > necessary defined as atomic counter, so the variable is renamed > as urb->use_flag for this purpose, then reading/writing the > flag is still kept as atomic but ARCH's atomic operations(atomic_inc/ > atomic_dec) are saved. That sentence made no sense to me at all :( And how is atomic_set() any faster/slower than atomic_inc/dec? The same memory barriers kick in, right? confused, 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, 19 Aug 2013, Ming Lei wrote: > Because usb_hcd_submit_urb is in the hotest path of usb core, > so use percpu counter to count URB instead of using atomic variable > because atomic operations are much slower than percpu operations. This seems like a ridiculous amount of additional overhead for a simple counter. The kernel doesn't even use this value for anything; it's only purpose is to allow userspace to see how many URBs have been transferred for a device. (I don't know what programs use this information. Powertop maybe?) Do you have any reason to believe this will really improve performance at all? 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 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource
On Mon, Aug 19, 2013 at 06:29:37PM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> >From: Julia Lawall
>
> >Remove unneeded error handling on the result of a call to
> >platform_get_resource_byname when the value is passed to
> >devm_ioremap_resource.
>
> >A simplified version of the semantic patch that makes this change is as
> >follows: (http://coccinelle.lip6.fr/)
>
> >//
> >@@
> >expression pdev,res,e,e1;
> >expression ret != 0;
> >identifier l;
> >@@
> >
> > res = platform_get_resource_byname(...);
> >- if (res == NULL) { ... \(goto l;\|return ret;\) }
> > e = devm_ioremap_resource(e1, res);
> >//
>
> >This patch also adjusts the error-handling code on the call to
> >devm_ioremap_resource to check the right value, noticed by Svenning Sørensen.
>
>Saying "also" in the changelog is usuallly a good sign the patch
> should be split.
>
> >Signed-off-by: Julia Lawall
>
> >---
> >v2: Fixed the bug on the test of the result of devm_ioremap_resource
>
> > drivers/usb/musb/musb_dsps.c |7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> >index 4ffbaac..e60be6f 100644
> >--- a/drivers/usb/musb/musb_dsps.c
> >+++ b/drivers/usb/musb/musb_dsps.c
> >@@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> >
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> >-if (!r)
> >-return -EINVAL;
> >-
> > reg_base = devm_ioremap_resource(dev, r);
> >-if (!musb->ctrl_base)
> >-return -EINVAL;
> >+if (IS_ERR(reg_base))
> >+return PTR_ERR(reg_base);
>
>This is indented with space, not tabs. And of course, this was a
> matter of a separate *fix* patch, while the original patch was a
> *cleanup*.
>
I would say this falls under the "trivial and closely related"
banner. But I would prefer if the changelog said "Fixed a bug,
also made some cleanups." Especially I wish the subject mentioned
the bugfix.
regards,
dan carpenter
--
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: [RFC PATCH 2/3] USB: kill urb->use_count atomic variable
On Mon, 19 Aug 2013, Ming Lei wrote: > This patch kills atomic_inc/atomic_dec operations on > urb->use_count in URB submit/complete path. > > The urb->use_count is only used for unlinking URB, and it isn't > necessary defined as atomic counter, so the variable is renamed > as urb->use_flag for this purpose, then reading/writing the > flag is still kept as atomic but ARCH's atomic operations(atomic_inc/ > atomic_dec) are saved. "Flag" is the wrong word. It implies a binary value, but the use_count can take on 3 values. Also, the names you selected aren't very good. I'd suggest something like the following: URB_INACTIVE (or URB_IDLE), URB_ACTIVE, URB_GIVING_BACK, URB_RESUBMITTED (or URB_ACTIVE_AND_GIVING_BACK) The state transitions will then be much clearer. But a counter seems equally clear to me. Besides, there's no way to avoid using atomic operations here. Your patch does this in __usb_hcd_giveback_urb(): > - atomic_dec(&urb->use_count); > + if (atomic_read(&urb->use_flag) == URB_UNUSING) > + atomic_set(&urb->use_flag, URB_UNUSED); This has a race. What happens if the driver submits the URB after the atomic_read and before the atomic_set? 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 10:00 PM, Greg Kroah-Hartman wrote: > On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: >> Because usb_hcd_submit_urb is in the hotest path of usb core, >> so use percpu counter to count URB instead of using atomic variable >> because atomic operations are much slower than percpu operations. >> >> Cc: Oliver Neukum >> Cc: Alan Stern >> Signed-off-by: Ming Lei >> --- >> drivers/usb/core/hcd.c |4 ++-- >> drivers/usb/core/sysfs.c |7 ++- >> drivers/usb/core/usb.c |9 - >> drivers/usb/core/usb.h |1 + >> include/linux/usb.h |2 +- >> 5 files changed, 18 insertions(+), 5 deletions(-) > > And this really speeds things up? Exactly what does it? > > And it's not that atomic operations are "slower", it's just that the For SMP, atomic_inc/atomic_dec are much slower than percpu variable inc/dec, see 4.1(Why Isn’t Concurrent Count-ing Trivial?) of [1]. However, it is slower: on a Intel Core Duo laptop, it is about six times slower than non-atomic increment when a single thread is incrementing, and more than ten times slower if two threads are incrementing. Considered that most of desktop & laptop are SMP now, and with USB3.0, the submitted URBs per second may reach tens of thousand or more, and we can remove the atomic inc/dec operations in the hot path, so why don't do it? > barriers involved can be slower, depending on what else is happening. > If you look, you are already hitting atomic variables in the same path, > so how can this change speed anything up? No, no barriers are involved in atomic_inc/atomic_dec at all. [1], Is Parallel Programming Hard, And, If So, What Can You Do About It? git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/perfbook.git Thanks, -- Ming Lei -- 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: USB host support should depend on HAS_DMA
On Sun, 18 Aug 2013, Geert Uytterhoeven wrote:
> On Thu, Jul 11, 2013 at 1:12 AM, Arnd Bergmann wrote:
> > On Wednesday 10 July 2013, Alan Stern wrote:
> >> This isn't right. There are USB host controllers that use PIO, not
> >> DMA. The HAS_DMA dependency should go with the controller driver, not
> >> the USB core.
> >>
> >> On the other hand, the USB core does call various routines like
> >> dma_unmap_single. It ought to be possible to compile these calls even
> >> when DMA isn't enabled. That is, they should be defined as do-nothing
> >> stubs.
> >
> > The asm-generic/dma-mapping-broken.h file intentionally causes link
> > errors, but that could be changed.
> >
> > The better approach in my mind would be to replace code like
> >
> >
> > if (hcd->self.uses_dma)
> >
> > with
> >
> > if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
> >
> > which will reliably cause that reference to be omitted from object code,
> > but not stop giving link errors for drivers that actually require
> > DMA.
>
> This can be done for drivers/usb/core/hcd.c.
>
> But I'm a bit puzzled by drivers/usb/core/buffer.c. E.g.
>
> void *hcd_buffer_alloc(...)
> {
>
> /* some USB hosts just use PIO */
> if (!bus->controller->dma_mask &&
> !(hcd->driver->flags & HCD_LOCAL_MEM)) {
I don't remember the full story. You should check with the person who
added the HCD_LOCAL_MEM flag originally.
> So if DMA is not used (!hcd->self.uses_dma, i.e. bus->controller->dma_mask
> is zero), and HCD_LOCAL_MEM is set, we still end up calling dma_pool_alloc()?
>
> (Naively, I'm not so familiar with the USB code) I'd expect it to use
> kmalloc() instead?
Maybe what happens in this case is that some sort of hook causes the
allocation to be made from a special memory-mapped region on board the
controller.
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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 10:42 PM, Alan Stern wrote: > On Mon, 19 Aug 2013, Ming Lei wrote: > >> Because usb_hcd_submit_urb is in the hotest path of usb core, >> so use percpu counter to count URB instead of using atomic variable >> because atomic operations are much slower than percpu operations. > > This seems like a ridiculous amount of additional overhead for a simple > counter. The kernel doesn't even use this value for anything; it's > only purpose is to allow userspace to see how many URBs have been > transferred for a device. (I don't know what programs use this > information. Powertop maybe?) That is why I want to remove the expensive atomic inc/dec, or can we remove the counter? > > Do you have any reason to believe this will really improve performance > at all? Please see my reply on Greg's comments. Thanks, -- Ming Lei -- 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/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 11:06:31PM +0800, Ming Lei wrote: > On Mon, Aug 19, 2013 at 10:00 PM, Greg Kroah-Hartman > wrote: > > On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: > >> Because usb_hcd_submit_urb is in the hotest path of usb core, > >> so use percpu counter to count URB instead of using atomic variable > >> because atomic operations are much slower than percpu operations. > >> > >> Cc: Oliver Neukum > >> Cc: Alan Stern > >> Signed-off-by: Ming Lei > >> --- > >> drivers/usb/core/hcd.c |4 ++-- > >> drivers/usb/core/sysfs.c |7 ++- > >> drivers/usb/core/usb.c |9 - > >> drivers/usb/core/usb.h |1 + > >> include/linux/usb.h |2 +- > >> 5 files changed, 18 insertions(+), 5 deletions(-) > > > > And this really speeds things up? Exactly what does it? > > > > And it's not that atomic operations are "slower", it's just that the > > For SMP, atomic_inc/atomic_dec are much slower than percpu > variable inc/dec, see 4.1(Why Isn’t Concurrent Count-ing Trivial?) > of [1]. > > However, it is slower: on a Intel Core Duo laptop, it is about six > times slower than non-atomic increment when a single thread > is incrementing, and more than ten times slower if two threads > are incrementing. > > Considered that most of desktop & laptop are SMP now, and with > USB3.0, the submitted URBs per second may reach tens of thousand > or more, and we can remove the atomic inc/dec operations in the hot > path, so why don't do it? Because you really didn't do it, there are lots of other atomic operations on that same path. And, thens of thousands of urbs should be trivial, did you measure this to see if it changed anything? I'm not taking patches like this that are not quantifiable, sorry. The gating problem in USB right now is the hardware, it's the slowest thing, not the kernel, from everything I have ever tested, or seen. Well, bad host controller silicon is also a problem (i.e. raspberry pi), but there's not much we can do about braindead problems like that... > > barriers involved can be slower, depending on what else is happening. > > If you look, you are already hitting atomic variables in the same path, > > so how can this change speed anything up? > > No, no barriers are involved in atomic_inc/atomic_dec at all. None? Hm, you might want to rethink that statement :) 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, 19 Aug 2013, Ming Lei wrote: > On Mon, Aug 19, 2013 at 10:42 PM, Alan Stern > wrote: > > On Mon, 19 Aug 2013, Ming Lei wrote: > > > >> Because usb_hcd_submit_urb is in the hotest path of usb core, > >> so use percpu counter to count URB instead of using atomic variable > >> because atomic operations are much slower than percpu operations. > > > > This seems like a ridiculous amount of additional overhead for a simple > > counter. The kernel doesn't even use this value for anything; it's > > only purpose is to allow userspace to see how many URBs have been > > transferred for a device. (I don't know what programs use this > > information. Powertop maybe?) > > That is why I want to remove the expensive atomic inc/dec, or can we > remove the counter? No doubt somebody would complain if the counter was removed. Who added it in the first place, and for what reason? > > Do you have any reason to believe this will really improve performance > > at all? > > Please see my reply on Greg's comments. As far as I can see, this counter does not need to be exact. Why not simply make it a non-atomic unsigned int? 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 11:17 PM, Alan Stern wrote: > On Mon, 19 Aug 2013, Ming Lei wrote: > >> On Mon, Aug 19, 2013 at 10:42 PM, Alan Stern >> wrote: >> > On Mon, 19 Aug 2013, Ming Lei wrote: >> > >> >> Because usb_hcd_submit_urb is in the hotest path of usb core, >> >> so use percpu counter to count URB instead of using atomic variable >> >> because atomic operations are much slower than percpu operations. >> > >> > This seems like a ridiculous amount of additional overhead for a simple >> > counter. The kernel doesn't even use this value for anything; it's >> > only purpose is to allow userspace to see how many URBs have been >> > transferred for a device. (I don't know what programs use this >> > information. Powertop maybe?) >> >> That is why I want to remove the expensive atomic inc/dec, or can we >> remove the counter? > > No doubt somebody would complain if the counter was removed. Who added > it in the first place, and for what reason? > >> > Do you have any reason to believe this will really improve performance >> > at all? >> >> Please see my reply on Greg's comments. > > As far as I can see, this counter does not need to be exact. Why not > simply make it a non-atomic unsigned int? It may becomes quite inaccurate, and 4.1 of the perfbook mentioned that half of counts might be lost with simple non-atomic unsigned int, so I think percpu variable is good choice. Thanks, -- Ming Lei -- 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 3/3] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Mon, 19 Aug 2013, Ming Lei wrote:
> This patch introduces ehci_disable_event(), which is applied on
> IAA_WATCHDOG and START_UNLINK_INTR events in case that the two
> events needn't to be handled, so that we may avoid unnecessary CPU
> wakeup.
> @@ -100,6 +100,20 @@ static void ehci_enable_event(struct ehci_hcd *ehci,
> unsigned event,
> }
>
>
Only one blank line here, please.
> +/* Warning: don't call this function from hrtimer handler context */
> +static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event)
> +{
> + ehci->enabled_hrtimer_events &= ~(1 << event);
> + if (!ehci->enabled_hrtimer_events) {
> + ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
> + hrtimer_cancel(&ehci->hrtimer);
> + } else if (ehci->next_hrtimer_event == event) {
> + ehci->next_hrtimer_event =
> + ffs(ehci->enabled_hrtimer_events) - 1;
Should the timer be rescheduled here? It's hard to say without seeing
some test results.
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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, 19 Aug 2013, Ming Lei wrote: > > As far as I can see, this counter does not need to be exact. Why not > > simply make it a non-atomic unsigned int? > > It may becomes quite inaccurate, and 4.1 of the perfbook mentioned > that half of counts might be lost with simple non-atomic unsigned int, > so I think percpu variable is good choice. In practice I think that is very unlikely to happen. There would have to be separate threads running on different CPUs, simultaneously submitting URBs for the same device and very closely synchronized. Also, we don't know how this number gets used. Quite possibly, losing half of the counts won't matter very much -- maybe the user cares only about the order of magnitude. 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 11:14 PM, Greg Kroah-Hartman wrote: > On Mon, Aug 19, 2013 at 11:06:31PM +0800, Ming Lei wrote: >> On Mon, Aug 19, 2013 at 10:00 PM, Greg Kroah-Hartman >> wrote: >> > On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: >> >> Because usb_hcd_submit_urb is in the hotest path of usb core, >> >> so use percpu counter to count URB instead of using atomic variable >> >> because atomic operations are much slower than percpu operations. >> >> >> >> Cc: Oliver Neukum >> >> Cc: Alan Stern >> >> Signed-off-by: Ming Lei >> >> --- >> >> drivers/usb/core/hcd.c |4 ++-- >> >> drivers/usb/core/sysfs.c |7 ++- >> >> drivers/usb/core/usb.c |9 - >> >> drivers/usb/core/usb.h |1 + >> >> include/linux/usb.h |2 +- >> >> 5 files changed, 18 insertions(+), 5 deletions(-) >> > >> > And this really speeds things up? Exactly what does it? >> > >> > And it's not that atomic operations are "slower", it's just that the >> >> For SMP, atomic_inc/atomic_dec are much slower than percpu >> variable inc/dec, see 4.1(Why Isn’t Concurrent Count-ing Trivial?) >> of [1]. >> >> However, it is slower: on a Intel Core Duo laptop, it is about six >> times slower than non-atomic increment when a single thread >> is incrementing, and more than ten times slower if two threads >> are incrementing. >> >> Considered that most of desktop & laptop are SMP now, and with >> USB3.0, the submitted URBs per second may reach tens of thousand >> or more, and we can remove the atomic inc/dec operations in the hot >> path, so why don't do it? > > Because you really didn't do it, there are lots of other atomic > operations on that same path. Not lots in the path of usbcore. > > And, thens of thousands of urbs should be trivial, did you measure this > to see if it changed anything? I'm not taking patches like this that > are not quantifiable, sorry. The number may be too trivial to measure, but I will try to test with perf. > > The gating problem in USB right now is the hardware, it's the slowest > thing, not the kernel, from everything I have ever tested, or seen. The problem may not speed up usb performance, but might decrease CPU utilization a bit, or cache miss. > > Well, bad host controller silicon is also a problem (i.e. raspberry pi), > but there's not much we can do about braindead problems like that... > >> > barriers involved can be slower, depending on what else is happening. >> > If you look, you are already hitting atomic variables in the same path, >> > so how can this change speed anything up? >> >> No, no barriers are involved in atomic_inc/atomic_dec at all. > > None? Hm, you might want to rethink that statement :) Please see Documentation/memory-barriers.txt: The following also do _not_ imply memory barriers, and so may require explicit memory barriers under some circumstances (smp_mb__before_atomic_dec() for instance): atomic_add(); atomic_sub(); atomic_inc(); atomic_dec(); Thanks, -- Ming Lei -- 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/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 11:33 PM, Alan Stern wrote: > On Mon, 19 Aug 2013, Ming Lei wrote: > >> > As far as I can see, this counter does not need to be exact. Why not >> > simply make it a non-atomic unsigned int? >> >> It may becomes quite inaccurate, and 4.1 of the perfbook mentioned >> that half of counts might be lost with simple non-atomic unsigned int, >> so I think percpu variable is good choice. > > In practice I think that is very unlikely to happen. There would have > to be separate threads running on different CPUs, simultaneously > submitting URBs for the same device and very closely synchronized. Right. > > Also, we don't know how this number gets used. Quite possibly, losing > half of the counts won't matter very much -- maybe the user cares only > about the order of magnitude. Another disadvantage is that accessing the shared variable is still slower than accessing one percpu variable in theory. Thanks, -- Ming Lei -- 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/3] USB: use percpu counter to count submitted URBs per device
Alan Stern writes: > No doubt somebody would complain if the counter was removed. Who added > it in the first place, and for what reason? Who and why is pretty well documented: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4d59d8a11383ebf0e0260ee481a4e766959fd7d9 Thanks to Sarah and git. Bjørn -- 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/3] of: add vendor prefix for Mentor Graphics
On 08/17/2013 03:27 AM, Sebastian Andrzej Siewior wrote: > On 08/17/2013 12:52 AM, Stephen Warren wrote: >> On 08/15/2013 07:13 AM, Sebastian Andrzej Siewior wrote: >>> This prefix is currently used for the musb driver. >> >>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt >>> b/Documentation/devicetree/bindings/vendor-prefixes.txt >> >>> +mg Mentor Graphics >> >> It's slightly short; I would have preferred "mentor" I think. but I >> guess it's fine. >> >> I see that both values are already used though: >> >>> arch/arm/boot/dts/am33xx.dtsi:375: >>> compatible = "mg,musbmhdrc"; >>> arch/arm/boot/dts/am33xx.dtsi:430: >>> compatible = "mg,musbmhdrc"; >> >>> arch/arm/boot/dts/dbx5x0.dtsi:181: "mentor,musb"; >> >> Should both be documented? Should the bindings for those devices be >> unified on one vendor prefix, with the old one perhaps still documented >> as deprecated depending on how long it's been around? > > I wasn't aware of the dbx5x0 mentor,usb binding. As far as the am33xx > is concerned, it has been prepared for the next merge window and can be > changed. OK, changing it to mentor, seems simplest and most convenient then. > However the mentor,usb binding isn't documented either. Presumably you can just adjust this patch to document it. -- 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: musb: dsps: fix devm_ioremap_resource error detection code
From: Julia Lawall devm_ioremap_resource returns an ERR_PTR value, not NULL, on failure. Furthermore, the value returned by devm_ioremap_resource should be tested. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @@ expression e,e1; statement S; @@ *e = devm_ioremap_resource(...); if (!e1) S // Signed-off-by: Julia Lawall --- drivers/usb/musb/musb_dsps.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 4ffbaac..4ad52e7 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -361,8 +361,8 @@ static int dsps_musb_init(struct musb *musb) return -EINVAL; reg_base = devm_ioremap_resource(dev, r); - if (!musb->ctrl_base) - return -EINVAL; + if (IS_ERR(reg_base)) + return PTR_ERR(reg_base); musb->ctrl_base = reg_base; /* NOP driver needs change if supporting dual instance */ -- 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/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 11:40:50PM +0800, Ming Lei wrote: > On Mon, Aug 19, 2013 at 11:14 PM, Greg Kroah-Hartman > wrote: > > On Mon, Aug 19, 2013 at 11:06:31PM +0800, Ming Lei wrote: > >> On Mon, Aug 19, 2013 at 10:00 PM, Greg Kroah-Hartman > >> wrote: > >> > On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: > >> >> Because usb_hcd_submit_urb is in the hotest path of usb core, > >> >> so use percpu counter to count URB instead of using atomic variable > >> >> because atomic operations are much slower than percpu operations. > >> >> > >> >> Cc: Oliver Neukum > >> >> Cc: Alan Stern > >> >> Signed-off-by: Ming Lei > >> >> --- > >> >> drivers/usb/core/hcd.c |4 ++-- > >> >> drivers/usb/core/sysfs.c |7 ++- > >> >> drivers/usb/core/usb.c |9 - > >> >> drivers/usb/core/usb.h |1 + > >> >> include/linux/usb.h |2 +- > >> >> 5 files changed, 18 insertions(+), 5 deletions(-) > >> > > >> > And this really speeds things up? Exactly what does it? > >> > > >> > And it's not that atomic operations are "slower", it's just that the > >> > >> For SMP, atomic_inc/atomic_dec are much slower than percpu > >> variable inc/dec, see 4.1(Why Isn’t Concurrent Count-ing Trivial?) > >> of [1]. > >> > >> However, it is slower: on a Intel Core Duo laptop, it is about six > >> times slower than non-atomic increment when a single thread > >> is incrementing, and more than ten times slower if two threads > >> are incrementing. > >> > >> Considered that most of desktop & laptop are SMP now, and with > >> USB3.0, the submitted URBs per second may reach tens of thousand > >> or more, and we can remove the atomic inc/dec operations in the hot > >> path, so why don't do it? > > > > Because you really didn't do it, there are lots of other atomic > > operations on that same path. > > Not lots in the path of usbcore. > > > > > And, thens of thousands of urbs should be trivial, did you measure this > > to see if it changed anything? I'm not taking patches like this that > > are not quantifiable, sorry. > > The number may be too trivial to measure, but I will try to test > with perf. > > > > > The gating problem in USB right now is the hardware, it's the slowest > > thing, not the kernel, from everything I have ever tested, or seen. > > The problem may not speed up usb performance, but might decrease > CPU utilization a bit, or cache miss. > > > > > Well, bad host controller silicon is also a problem (i.e. raspberry pi), > > but there's not much we can do about braindead problems like that... > > > >> > barriers involved can be slower, depending on what else is happening. > >> > If you look, you are already hitting atomic variables in the same path, > >> > so how can this change speed anything up? > >> > >> No, no barriers are involved in atomic_inc/atomic_dec at all. > > > > None? Hm, you might want to rethink that statement :) > > Please see Documentation/memory-barriers.txt: > > The following also do _not_ imply memory barriers, and so may require explicit > memory barriers under some circumstances (smp_mb__before_atomic_dec() for > instance): > > atomic_add(); > atomic_sub(); > atomic_inc(); > atomic_dec(); You are both right, each in your own way. Greg is correct that on x86 these operations do include memory barriers, even though Linux does not require them to do so. Ming is correct that Linux does not require it, and that there are in fact non-x86 architectures in which these operations do not include memory barriers. Thanx, Paul -- 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: Non-enumerable devices on USB and other enumerable buses
On Mon, Aug 19, 2013 at 08:17:53PM +0800, Ming Lei wrote: > On Sat, Aug 17, 2013 at 9:29 AM, Alan Stern wrote: > > Aong those lines, I would like to point out that the device concept > > embodied in the kernel's data structures can be pretty thin. For > > example, it might be little more than a port number or bus address. > Maybe the principle behind drivers/usb/core/usb-acpi.c is helpful > for the problem, and DT may refer to ACPI to describe on-board > USB devices, and the way to retrieve platform data too. I can't parse this at all well - why would DT want to refer to ACPI, do you mean people may wish to look at the code as an example? As Grant noted DT already has some mechanisms for enumerable buses which looking at the code appears to be broadly what that's doing. signature.asc Description: Digital signature
Re: [PATCH v3 1/3] usb: dwc3: msm: Add device tree binding information
On 08/19/2013 06:27 AM, Ivan T. Ivanov wrote: > > Hi, > > On Fri, 2013-08-16 at 16:44 -0600, Stephen Warren wrote: >> On 08/14/2013 06:59 AM, Ivan T. Ivanov wrote: >>> From: "Ivan T. Ivanov" >>> >>> MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys >>> (SNPS) and HS, SS PHY's control and configuration registers. >>> >>> It could operate in device mode (SS, HS, FS) and host >>> mode (SS, HS, FS, LS). >> >>> diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt >>> b/Documentation/devicetree/bindings/usb/msm-ssusb.txt >> >>> +- clock-names : >> ... >>> + "sleep_a_clk" : Sleep clock, used when USB3 core goes into low >> ... >>> + "ref_clk" : Reference clock - used in host mode. >> ... >>> + "core_clk" : Master/Core clock, have to be >= 125 MHz for SS >> ... >>> + "iface_clk" : System bus AXI clock >>> + "sleep_clk" : Sleep clock, used when USB3 core goes into low >> ... >>> + "utmi_clk" : Generated by HS-PHY. Used to clock the low power >> >> I think it makes sense to remove "_clk" from all those names, unless the >> HW documentation really talks about a clock named e.g. iface_clk yet >> some other clock names in the documentation don't have the "_clk" >> suffix, e.g. the "xo I didn't quote. > > From limited information that I have, I could not say how clock inputs > are named from the controller perspective, but I agree that "_clk" > suffix looks redundant. > > Side question: if for example label in controller says "UTMI", should I > also use capital letters for the resource or this could be "utmi"? All the clock-names entries I've seen so far have been lower-case, but I suppose there's no hard-and-fast rule that they couldn't be upper-/mixed-case if that best matched the HW documentation. -- 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/3] usb: dwc3: msm: Add device tree binding information
On 08/19/13 05:27, Ivan T. Ivanov wrote: > Hi, > > On Fri, 2013-08-16 at 16:44 -0600, Stephen Warren wrote: >> On 08/14/2013 06:59 AM, Ivan T. Ivanov wrote: >>> From: "Ivan T. Ivanov" >>> >>> MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys >>> (SNPS) and HS, SS PHY's control and configuration registers. >>> >>> It could operate in device mode (SS, HS, FS) and host >>> mode (SS, HS, FS, LS). >>> diff --git a/Documentation/devicetree/bindings/usb/msm-ssusb.txt >>> b/Documentation/devicetree/bindings/usb/msm-ssusb.txt >>> +- clock-names : >> ... >>> + "sleep_a_clk" : Sleep clock, used when USB3 core goes into low >> ... >>> + "ref_clk" : Reference clock - used in host mode. >> ... >>> + "core_clk" : Master/Core clock, have to be >= 125 MHz for SS >> ... >>> + "iface_clk" : System bus AXI clock >>> + "sleep_clk" : Sleep clock, used when USB3 core goes into low >> ... >>> + "utmi_clk" : Generated by HS-PHY. Used to clock the low power >> I think it makes sense to remove "_clk" from all those names, unless the >> HW documentation really talks about a clock named e.g. iface_clk yet >> some other clock names in the documentation don't have the "_clk" >> suffix, e.g. the "xo I didn't quote. > From limited information that I have, I could not say how clock inputs > are named from the controller perspective, but I agree that "_clk" > suffix looks redundant. In downstream trees we've tried to standardize the names on core_clk, iface_clk, bus_clk, etc. Historically the hardware designers have used the names from the clock controller instead of coming up with standard names of their own when they put the clock inputs in their data sheets (if they do at all). -- 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-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/3] of: add vendor prefix for Mentor Graphics
On 08/19/2013 05:50 PM, Stephen Warren wrote: arch/arm/boot/dts/am33xx.dtsi:375: compatible = "mg,musbmhdrc"; arch/arm/boot/dts/am33xx.dtsi:430: compatible = "mg,musbmhdrc"; >>> arch/arm/boot/dts/dbx5x0.dtsi:181: "mentor,musb"; >>> >>> Should both be documented? Should the bindings for those devices be >>> unified on one vendor prefix, with the old one perhaps still documented >>> as deprecated depending on how long it's been around? >> >> I wasn't aware of the dbx5x0 mentor,usb binding. As far as the am33xx >> is concerned, it has been prepared for the next merge window and can be >> changed. > > OK, changing it to mentor, seems simplest and most convenient then. Okay. >> However the mentor,usb binding isn't documented either. > > Presumably you can just adjust this patch to document it. Okay. That means if we stick to that hierarchy given my dbx5x0 (which makes sense after thinking for a while) I'm going to shift my devices a little to fit this. Once this is done I document it and post patches. 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 11:40:50PM +0800, Ming Lei wrote: > On Mon, Aug 19, 2013 at 11:14 PM, Greg Kroah-Hartman > wrote: > > On Mon, Aug 19, 2013 at 11:06:31PM +0800, Ming Lei wrote: > >> On Mon, Aug 19, 2013 at 10:00 PM, Greg Kroah-Hartman > >> wrote: > >> > On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: > >> >> Because usb_hcd_submit_urb is in the hotest path of usb core, > >> >> so use percpu counter to count URB instead of using atomic variable > >> >> because atomic operations are much slower than percpu operations. > >> >> > >> >> Cc: Oliver Neukum > >> >> Cc: Alan Stern > >> >> Signed-off-by: Ming Lei > >> >> --- > >> >> drivers/usb/core/hcd.c |4 ++-- > >> >> drivers/usb/core/sysfs.c |7 ++- > >> >> drivers/usb/core/usb.c |9 - > >> >> drivers/usb/core/usb.h |1 + > >> >> include/linux/usb.h |2 +- > >> >> 5 files changed, 18 insertions(+), 5 deletions(-) > >> > > >> > And this really speeds things up? Exactly what does it? > >> > > >> > And it's not that atomic operations are "slower", it's just that the > >> > >> For SMP, atomic_inc/atomic_dec are much slower than percpu > >> variable inc/dec, see 4.1(Why Isn’t Concurrent Count-ing Trivial?) > >> of [1]. > >> > >> However, it is slower: on a Intel Core Duo laptop, it is about six > >> times slower than non-atomic increment when a single thread > >> is incrementing, and more than ten times slower if two threads > >> are incrementing. > >> > >> Considered that most of desktop & laptop are SMP now, and with > >> USB3.0, the submitted URBs per second may reach tens of thousand > >> or more, and we can remove the atomic inc/dec operations in the hot > >> path, so why don't do it? > > > > Because you really didn't do it, there are lots of other atomic > > operations on that same path. > > Not lots in the path of usbcore. Did you look close? I see 2 more right there in the context of your patch alone. One you try to take care of later (but just do the same thing, no real change), the other you don't address at all. > > And, thens of thousands of urbs should be trivial, did you measure this > > to see if it changed anything? I'm not taking patches like this that > > are not quantifiable, sorry. > > The number may be too trivial to measure, but I will try to test > with perf. If it's too trivial to measure, then I can't accept the patch, nor should you expect it to be accepted, right? > > The gating problem in USB right now is the hardware, it's the slowest > > thing, not the kernel, from everything I have ever tested, or seen. > > The problem may not speed up usb performance, but might decrease > CPU utilization a bit, or cache miss. Do you have proof of this? Without that, why would you even want someone to accept such a patch? 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
Re: [PATCH 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, 19 Aug 2013, Bjørn Mork wrote: > Alan Stern writes: > > > No doubt somebody would complain if the counter was removed. Who added > > it in the first place, and for what reason? > > Who and why is pretty well documented: > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4d59d8a11383ebf0e0260ee481a4e766959fd7d9 > > Thanks to Sarah and git. Thanks for locating that (I'm not using at my regular computer today so I don't have access to my git repository). The commit message says clearly that urbnum was added specifically for use by powertop. It seems reasonable to assume that powertop won't mind very much if the number is off by some small factor. 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, 19 Aug 2013, Ming Lei wrote: > Another disadvantage is that accessing the shared variable is still > slower than accessing one percpu variable in theory. By that argument, _everything_ in the kernel should be percpu. There is a reason why atomic variables exist. 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 1/3] USB: use percpu counter to count submitted URBs per device
On Mon, Aug 19, 2013 at 10:14:57AM -0700, Greg Kroah-Hartman wrote: > On Mon, Aug 19, 2013 at 11:40:50PM +0800, Ming Lei wrote: > > On Mon, Aug 19, 2013 at 11:14 PM, Greg Kroah-Hartman > > wrote: > > > On Mon, Aug 19, 2013 at 11:06:31PM +0800, Ming Lei wrote: > > >> On Mon, Aug 19, 2013 at 10:00 PM, Greg Kroah-Hartman > > >> wrote: > > >> > On Mon, Aug 19, 2013 at 07:04:18PM +0800, Ming Lei wrote: > > >> >> Because usb_hcd_submit_urb is in the hotest path of usb core, > > >> >> so use percpu counter to count URB instead of using atomic variable > > >> >> because atomic operations are much slower than percpu operations. > > >> >> > > >> >> Cc: Oliver Neukum > > >> >> Cc: Alan Stern > > >> >> Signed-off-by: Ming Lei > > >> >> --- > > >> >> drivers/usb/core/hcd.c |4 ++-- > > >> >> drivers/usb/core/sysfs.c |7 ++- > > >> >> drivers/usb/core/usb.c |9 - > > >> >> drivers/usb/core/usb.h |1 + > > >> >> include/linux/usb.h |2 +- > > >> >> 5 files changed, 18 insertions(+), 5 deletions(-) > > >> > > > >> > And this really speeds things up? Exactly what does it? > > >> > > > >> > And it's not that atomic operations are "slower", it's just that the > > >> > > >> For SMP, atomic_inc/atomic_dec are much slower than percpu > > >> variable inc/dec, see 4.1(Why Isn’t Concurrent Count-ing Trivial?) > > >> of [1]. > > >> > > >> However, it is slower: on a Intel Core Duo laptop, it is about > > >> six > > >> times slower than non-atomic increment when a single thread > > >> is incrementing, and more than ten times slower if two threads > > >> are incrementing. > > >> > > >> Considered that most of desktop & laptop are SMP now, and with > > >> USB3.0, the submitted URBs per second may reach tens of thousand > > >> or more, and we can remove the atomic inc/dec operations in the hot > > >> path, so why don't do it? > > > > > > Because you really didn't do it, there are lots of other atomic > > > operations on that same path. > > > > Not lots in the path of usbcore. > > Did you look close? I see 2 more right there in the context of your > patch alone. One you try to take care of later (but just do the same > thing, no real change), the other you don't address at all. Ok, sorry, atomic_set() is, on some arches, "faster" than atomic_inc(), so you have sped up things there, my apologies. But given the other locks taken in this path, and other atomic operations, removing 1 seems like premature optimization. Please, use perf, and other things, to find out where real problems are in the USB stack. I'm sure there are locations that we can improve on, but until you get some real numbers, it's going to be hard to accept stuff like this. 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 v2] usb: xhci: Disable runtime PM suspend for quirky controllers
If a USB controller with XHCI_RESET_ON_RESUME goes to runtime suspend,
a reset will be performed upon runtime resume. Any previously suspended
devices attached to the controller will be re-enumerated at this time.
This will cause problems, for example, if an open system call on the
device triggered the resume (the open call will fail).
Note that this change is only relevant when persist_enabled is not set
for USB devices.
Signed-off-by: Shawn Nematbakhsh
---
Changes since v1:
- Only disable runtime suspend when devices are connected.
- Skip this change if CONFIG_USB_DEFAULT_PERSIST is enabled.
---
Change-Id: I964e5bdfec09d03fac8656aaf566bc48d34ef0f9
---
drivers/usb/host/xhci.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9478caa..8adbab2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3581,10 +3581,21 @@ void xhci_free_dev(struct usb_hcd *hcd, struct
usb_device *udev)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_virt_device *virt_dev;
+ struct device *dev = hcd->self.controller;
unsigned long flags;
u32 state;
int i, ret;
+#ifndef CONFIG_USB_DEFAULT_PERSIST
+ /*
+* We called pm_runtime_get_noresume when the device was attached.
+* Decrement the counter here to allow controller to runtime suspend
+* if no devices remain.
+*/
+ if (xhci->quirks & XHCI_RESET_ON_RESUME)
+ pm_runtime_put_noidle(dev);
+#endif
+
ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__);
/* If the host is halted due to driver unload, we still need to free the
* device.
@@ -3656,6 +3667,7 @@ static int xhci_reserve_host_control_ep_resources(struct
xhci_hcd *xhci)
int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct device *dev = hcd->self.controller;
unsigned long flags;
int timeleft;
int ret;
@@ -3708,6 +3720,16 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct
usb_device *udev)
goto disable_slot;
}
udev->slot_id = xhci->slot_id;
+
+#ifndef CONFIG_USB_DEFAULT_PERSIST
+ /*
+* If resetting upon resume, we can't put the controller into runtime
+* suspend if there is a device attached.
+*/
+ if (xhci->quirks & XHCI_RESET_ON_RESUME)
+ pm_runtime_get_noresume(dev);
+#endif
+
/* Is this a LS or FS device under a HS hub? */
/* Hub or peripherial? */
return 1;
--
1.7.12.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] i2c-tiny-usb: do not use stack as URB transfer_buffer
On Thu, Aug 15, 2013 at 10:46:32PM +0300, Jussi Kivilinna wrote: > On 15.08.2013 20:55, Wolfram Sang wrote: > > On Tue, Aug 06, 2013 at 02:17:42PM +0300, Jussi Kivilinna wrote: > >> Patch fixes i2c-tiny-usb not to use stack as URB transfer_buffer. URB > >> buffers > >> need to be DMA-able, which stack is not. > >> > >> Patch is only compile tested. > >> > >> Cc: sta...@vger.kernel.org > >> Signed-off-by: Jussi Kivilinna > > > > Looks OK to me. Till, are you there? > > That cc to stable should probably be removed if patch is not tested on hw. Applied to for-next, thanks! And removed stable for now... signature.asc Description: Digital signature
Re: [PATCH] usb: phy: Cleanup error code in **_usb_get_phy_**() APIs
>> I liked the ones where we had IS_ERR_OR_NULL() here (and in all the >> ones below)... you sometimes have to handle PHYs in >> platform-independent code where you don't want to worry about if this >> platform actually has a PHY driver there or not. Any reason you >> changed that? > > The **get_phy_*() APIs never return a NULL pointer now, do we still > need to handle that in that case. > Or are we assuming that code will use these phy operations without > getting a phy in the first place ? In our 5420 PHY tune patch (which I think has not made it upstream yet), we're calling usb_phy_tune(hcd->phy) from the USB core. This pointer is usually NULL unless it has been explicitly set by the platform specific HCD driver. For situations like that I think it's convenient if you can just fire-and-forget a generic PHY method without worrying whether the particular PHY implements it or whether it has a driver at all. -- 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
USB gadget on mx6
Hi,
I am running today's linux-next and I am trying to enable usb gadget on mx6:
diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
index a55113e..ef757d2 100644
--- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
@@ -115,6 +115,14 @@
status = "okay";
};
+&usbotg {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbotg_1>;
+ disable-over-current;
+ dr_mode = "peripheral";
+ status = "okay";
+};
+
&usdhc1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usdhc1_2>;
However, I am not able to get USB ether to work.
Any suggestions?
Regards,
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
[PATCH] chipidea: ci_hdrc_imx: Fix MODULE_ALIAS()
The MODULE_ALIAS string should match the '.name' field of the platform driver.
In this case we have '.name = "imx_usb"', so adapt MODULE_ALIAS() accordingly.
Signed-off-by: Fabio Estevam
---
drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 74d998d..2aea048 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -202,7 +202,7 @@ static struct platform_driver ci_hdrc_imx_driver = {
module_platform_driver(ci_hdrc_imx_driver);
-MODULE_ALIAS("platform:imx-usb");
+MODULE_ALIAS("platform:imx_usb");
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("CI HDRC i.MX USB binding");
MODULE_AUTHOR("Marek Vasut ");
--
1.8.1.2
--
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] extcon: extcon-dra7xx: Add extcon driver for USB ID detection
On 08/16/2013 04:13 AM, George Cherian wrote:
> Adding extcon driver for USB ID detection to dynamically
> configure USB Host/Peripheral mode.
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
> b/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
> +EXTCON FOR DRA7xx
> +
> +Required Properties:
Please at lest explain what a DRA7xxx is, and the purpose of the HW
module this binding describes.
> + - compatible : Should be "ti,dra7xx-usb"
If this is a USB VID detector rather than a full USB host controller,
then "usb" in the binding is a bit over-reaching. Perhaps "-usb-vid" or
"-usb-vid-detector" would be more accurate.
> + - gpios : phandle to ID pin and interrupt gpio.
This isn't just a phandle; it's phandle+args, or a GPIO specifier. Some
reference should be made to ../gpio/gpio.txt for the format.
Why does the interrupt line need to be included in a list of GPIOs?
If the DRA7xxx is a VID detector, why does it even need/have any GPIOs
associated with it; surely it has a dedicated VID input pin. Can you
provide more details re: how the HW is structured.
> +Optional Properties:
> + - interrupt-parent : interrupt controller phandle
> + - interrupts : interrupt number
> +
> +
It's typical insert the following between those two blank lines:
Example:
... or delete one of the blank lines.
> +dra7x_extcon1 {
> + compatible = "ti,dra7xx-usb";
> + gpios = <&pcf_usb 1 0>,
> + <&gpio6 11 2>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <11>;
> + };
> +
No need for that trailing blank line.
--
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: USB gadget on mx6
Dear Fabio Estevam,
> Hi,
>
> I am running today's linux-next and I am trying to enable usb gadget on
> mx6:
>
> diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> index a55113e..ef757d2 100644
> --- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
> @@ -115,6 +115,14 @@
> status = "okay";
> };
>
> +&usbotg {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usbotg_1>;
> + disable-over-current;
> + dr_mode = "peripheral";
> + status = "okay";
> +};
> +
> &usdhc1 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc1_2>;
>
>
> However, I am not able to get USB ether to work.
>
> Any suggestions?
Are you not missing usbmisc or vbus regulator ?
Best regards,
Marek Vasut
--
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/6] for fixing xhci_readl call in xhci_hub.c after removing xhci_hcd from function definition
On Sat, Aug 17, 2013 at 07:08:15PM +0300, Dan Carpenter wrote: > On Fri, Aug 16, 2013 at 12:24:34PM -0700, Sarah Sharp wrote: > > In general, please keep the short descriptions of your patches (which > > turn into the subject lines of your mails) limited to around 55 > > characters. > > 55 is a very austere limit. That's the limit before git-format patch starts chopping things off in the generated file name, and it's the limit when vim git commit syntax highlighting stops highlighting the short description. > I've been telling people 72 character the same as email. > `git citool` has a fixed width of 75 characters so that's what I > normally use in practice. But hey, 72 is a fine limit too. I'll start telling people that. :) 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: USB gadget on mx6
Hi Marek, On Mon, Aug 19, 2013 at 4:43 PM, Marek Vasut wrote: > Are you not missing usbmisc or vbus regulator ? usbmisc is in the .dtsi otg node. vbus is only needed on usb host or otg modes, right? I would like to setup usb otg port in peripheral mode only. -- 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] Fix stack corruption on some architectures
There is no need to get an interface specification if we know it's the
wrong one; trivial change. The big thing, though, was explained in the
#mipslinux IRC channel:
[Mon 2013-08-19 12:28:21 PM PDT] guys, are you sure it's not "DMA
off stack" case?
[Mon 2013-08-19 12:28:35 PM PDT] it's a known stack corruptor on
non-coherent arches
[Mon 2013-08-19 12:31:48 PM PDT] headless: for usb/ehci?
[Mon 2013-08-19 12:34:11 PM PDT] headless: explain
[Mon 2013-08-19 12:35:38 PM PDT] usb_control_msg() (or other such
func) should not use buffer on stack. DMA from/to stack is prohibited
[Mon 2013-08-19 12:35:58 PM PDT] and EHCI uses DMA on control xfers
(as well as all the others)
Signed-off-by: Daniel Gimpelevich
---
drivers/net/usb/hso.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cba1d46..86292e6 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2816,13 +2816,16 @@ exit:
static int hso_get_config_data(struct usb_interface *interface)
{
struct usb_device *usbdev = interface_to_usbdev(interface);
- u8 config_data[17];
+ u8 *config_data = kmalloc(17, GFP_KERNEL);
u32 if_num = interface->altsetting->desc.bInterfaceNumber;
s32 result;
+ if (!config_data)
+ return -ENOMEM;
if (usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
0x86, 0xC0, 0, 0, config_data, 17,
USB_CTRL_SET_TIMEOUT) != 0x11) {
+ kfree(config_data);
return -EIO;
}
@@ -2873,6 +2876,7 @@ static int hso_get_config_data(struct usb_interface
*interface)
if (config_data[16] & 0x1)
result |= HSO_INFO_CRC_BUG;
+ kfree(config_data);
return result;
}
@@ -2886,6 +2890,11 @@ static int hso_probe(struct usb_interface *interface,
struct hso_shared_int *shared_int;
struct hso_device *tmp_dev = NULL;
+ if (interface->cur_altsetting->desc.bInterfaceClass != 0xFF) {
+ dev_err(&interface->dev, "Not our interface\n");
+ return -ENODEV;
+ }
+
if_num = interface->altsetting->desc.bInterfaceNumber;
/* Get the interface/port specification from either driver_info or from
@@ -2895,10 +2904,6 @@ static int hso_probe(struct usb_interface *interface,
else
port_spec = hso_get_config_data(interface);
- if (interface->cur_altsetting->desc.bInterfaceClass != 0xFF) {
- dev_err(&interface->dev, "Not our interface\n");
- return -ENODEV;
- }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
if (interface->num_altsetting > 1)
--
1.7.9.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
Re: [PATCH] Fix stack corruption on some architectures
On Mon, Aug 19, 2013 at 01:37:27PM -0700, Daniel Gimpelevich wrote: > There is no need to get an interface specification if we know it's the > wrong one; trivial change. The big thing, though, was explained in the > #mipslinux IRC channel: > [Mon 2013-08-19 12:28:21 PM PDT] guys, are you sure it's not "DMA > off stack" case? > [Mon 2013-08-19 12:28:35 PM PDT] it's a known stack corruptor on > non-coherent arches > [Mon 2013-08-19 12:31:48 PM PDT] headless: for usb/ehci? > [Mon 2013-08-19 12:34:11 PM PDT] headless: explain > [Mon 2013-08-19 12:35:38 PM PDT] usb_control_msg() (or other such > func) should not use buffer on stack. DMA from/to stack is prohibited > [Mon 2013-08-19 12:35:58 PM PDT] and EHCI uses DMA on control > xfers (as well as all the others) > > Signed-off-by: Daniel Gimpelevich Acked-by: Greg Kroah-Hartman -- 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] Fix stack corruption on some architectures
Hello. On 08/20/2013 12:37 AM, Daniel Gimpelevich wrote: There is no need to get an interface specification if we know it's the wrong one; trivial change. Is it related to stack corruption? If not, it's asking to be in a separate patch. The big thing, though, was explained in the #mipslinux IRC channel: [Mon 2013-08-19 12:28:21 PM PDT] guys, are you sure it's not "DMA off stack" case? [Mon 2013-08-19 12:28:35 PM PDT] it's a known stack corruptor on non-coherent arches [Mon 2013-08-19 12:31:48 PM PDT] headless: for usb/ehci? [Mon 2013-08-19 12:34:11 PM PDT] headless: explain [Mon 2013-08-19 12:35:38 PM PDT] usb_control_msg() (or other such func) should not use buffer on stack. DMA from/to stack is prohibited [Mon 2013-08-19 12:35:58 PM PDT] and EHCI uses DMA on control xfers (as well as all the others) That headless was me. :-) Signed-off-by: Daniel Gimpelevich WBR, Sergei -- 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 1/4] staging: usbip: add "-P" / "--pid" option to save usbipd process id
On Sat, Aug 17, 2013 at 03:34:31PM -0600, Anthony Foiani wrote: > Introduce option "-P" / "--pid" to request that usbipd save its PID to > a file while running. Shouldn't this be the requirement of the tool that starts it up (i.e. systemd or sysvinit)? Putting stuff like /var/run into the daemon seems like a bad idea. 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
Re: [PATCH v2 2/4] staging: usbip: set usbipd server port via "-t" / "--tcp-port" option.
On Sat, Aug 17, 2013 at 03:34:32PM -0600, Anthony Foiani wrote: > +int USBIP_PORT = 3240; > +char *USBIP_PORT_STRING = "3240"; Variables shouldn't be ALL_CAPS, those are what #defines are for. Yes, I know you moved from a define to a variable, but please make it obvious and change the name as well, otherwise stuff like: > + USBIP_PORT = port; > + USBIP_PORT_STRING = arg; Just makes my head hurt. 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
Re: [PATCH] Fix stack corruption on some architectures
On Tue, 2013-08-20 at 02:31 +0400, Sergei Shtylyov wrote: > Hello. > > On 08/20/2013 12:37 AM, Daniel Gimpelevich wrote: > > > There is no need to get an interface specification if we know it's the > > wrong one; trivial change. > > Is it related to stack corruption? If not, it's asking to be in a > separate > patch. Perhaps, but I'll leave that up to the maintainer(s). You should be credited as co-author of the patch. -- 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 2/4] staging: usbip: set usbipd server port via "-t" / "--tcp-port" option.
Greg KH writes: > On Sat, Aug 17, 2013 at 03:34:32PM -0600, Anthony Foiani wrote: > > +int USBIP_PORT = 3240; > > +char *USBIP_PORT_STRING = "3240"; > > Variables shouldn't be ALL_CAPS, those are what #defines are for. > Yes, I know you moved from a define to a variable, Right, I was trying to minimize churn. Guess I should have followed with another patch to convert to sane usage. > but please make it obvious and change the name as well, otherwise > stuff like: > >> +USBIP_PORT = port; >> +USBIP_PORT_STRING = arg; > > Just makes my head hurt. Ok. Will try to respin in the next few days. Thanks! Tony -- 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 1/4] staging: usbip: add "-P" / "--pid" option to save usbipd process id
Greg KH writes: > On Sat, Aug 17, 2013 at 03:34:31PM -0600, Anthony Foiani wrote: > > Introduce option "-P" / "--pid" to request that usbipd save its > > PID to a file while running. > > Shouldn't this be the requirement of the tool that starts it up > (i.e. systemd or sysvinit)? Putting stuff like /var/run into the > daemon seems like a bad idea. By that logic, we should remove the daemonize code entirely, since one can't otherwise get the PID post-daemonize. (I'll admit that I'm using it in a project that has no init scripts, so I'd carry this change regardless.) Finally, the "/var/run/usbipd.pid" is just a default; the user is welcome to specify whatever path their distribution or layout requires. (Assuming I coded the getopt bits correctly!) As always, feel free to drop. I just wanted to follow up on my previous patches with a more final version, in case anyone else needs or wants these features. Thanks, Tony -- 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: [Pull Request] xhci: Step 2 to fix usb-linus and usb-next.
On Thu, Aug 15, 2013 at 06:44:03PM -0700, Sarah Sharp wrote: > The following changes since commit 224563b6ce034b82f8511969d9496113da34fb2c: > > Merge tag 'for-usb-next-2013-08-15' of > git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci into usb-next > (2013-08-15 17:33:16 -0700) > > are available in the git repository at: > > > gitol...@ra.kernel.org:/pub/scm/linux/kernel/git/sarah/xhci.git > tags/for-usb-2013-08-15-step-2 What's with the gitolite@ address here? Anyway, I've just pulled this branch into my usb-next branch, which should be everything that is needed, right? 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
[BUG] ohci-pci: devices not detected after hibernate
Last working release: 3.10 Bisected earliest broken: c1117afb85 Broken in: 3.11-rc1, 3.11-rc6, bd479f2933 (the current usb-next) Once my PC has been hibernated and resumed, some devices plugged in to the on-motherboard ports stop working, and don't show up in lsusb. The pattern seems to be that ports with USB 1.1 devices are affected, but ports with USB 2.0 devices are not. It's an x86-amd64 desktop, with an MSI motherboard based on the AMD 970 chipset. Ports in USB 1.1 mode use the ohci-pci driver. I've bisected the problem to c1117afb85, included in v3.11-rc1. c1117afb85 USB: OHCI: make ohci-pci a separate driver If the PC is booted without a resume image, everything works. It's running Debian unstable with a local kernel build, and to hibernate I'm using the "pm-hibernate" script from pm-utils. For a sanity check of my local builds, I've seen the same behaviour in the Debian archive's build of 3.11-rc4. Using an optical mouse as my main test device: The mouse doesn't show up in lsusb, and the light (optical mouse) is off. It doesn't sent any input to X11. Plugging the mouse in to a different USB port on the motherboard will show brief activity in usbmon, but again it doesn't work, doesn't show in lsusb and doesn't light up. Once the mouse has been plugged in to a USB port, no device will be detected in that physical port again until the PC is rebooted. Nothing will show in dmesg or usbmon when another device is plugged or unplugged (even with CONFIG_USB_DEBUG). USB 2.0 devices still work (but not in ports that a 1.1 device has previously been plugged in to). An external USB 2.0 hub does still show up after hibernation, and the same USB 1.1 mouse works if plugged in to this hub. Attached are logs from bd479f2933 with CONFIG_USB_DEBUG. There's no physical movement of plugs in this log, just hibernating and resuming with the same devices plugged in. The keyboard is PS/2, not USB. Affected devices in the log: 046d:c03d Logitech, Inc. M-BT96a Pilot Optical Mouse 04b8:0101 Seiko Epson Corp. GT-7000U [Perfection 636] 046d:c216 Logitech, Inc. Dual Action Gamepad Unaffected devices in the log: 0424:2514 Standard Microsystems Corp. USB 2.0 Hub 04cb:01c1 Fuji Photo Film Co., Ltd FinePix F31fd (PTP) 04e8:681c Samsung Electronics Co., Ltd Galaxy Portal/Spica/S 04e8:6860 Samsung Electronics Co., Ltd GT-I9100 Phone In the lsusb output, the USB 1.1 root hubs' power settings change (diff of before and after hibernation): Hub Descriptor: bLength 9 bDescriptorType 41 nNbrPorts 5 - wHubCharacteristic 0x0012 -No power switching (usb 1.0) + wHubCharacteristic 0x0011 +Per-port power switching No overcurrent protection bPwrOn2PwrGood2 * 2 milli seconds bHubContrCurrent 0 milli Ampere DeviceRemovable0x00 PortPwrCtrlMask0xff Hub Port Status: - Port 1: .0100 power - Port 2: .0100 power - Port 3: .0303 lowspeed power enable connect - Port 4: .0100 power - Port 5: .0100 power + Port 1: . + Port 2: . + Port 3: . + Port 4: . + Port 5: . Device Status: 0x0001 Self Powered usb_next_bd479f2__no_x11.tar.bz2 Description: Binary data
Re: RE: [PATCH v2 0/8] Common Clock Framework support for Samsung S3C64xx
Quoting Kukjin Kim (2013-08-17 03:30:11) > Tomasz Figa wrote: > > [...] > > > > > > > > > > > > > Basically, this series looks good to me, but I'm not sure how this > > > > > > should be handled because of dependency with PWM cleanup and clk > > > > > > stuff > > > > > > in clk tree now... > > > > > > > > > > Patches 1-3 can go into the clk tree. 4-6 should go through their > > > > > respective trees. > > > > > > > > It looks like version 2 of patch 2/8 has been applied by mistake, > > > > breaking compilation (and operation) of the clock driver added in > > > > patch 3/8. > > > Ugh. My mistake. > > > > Happens. Thanks for fast response. > > > Sorry for late ;-) > > > > > Could you please fix this up? Thanks in advance. > > > > > > This is a little tricky since I published the clk-next-s3c64xx branch as > > > a stable branch for Samsung which I think has been merged to the > > > Samsung tree already. > > > > Right, this somewhat limits our options. Although I'm not really sure > > whether Kukjin already has pushed it to his public tree. > > > Yeah, I already did sort out in my local but not public tree because of some > problem. > > > > So what are the options? > > > > > > One option is to create a fixup patch that just manages the delta > > > between V2 and V3. I can then add this to the top of clk-next-s3c64xx > > > and re-merge it into clk-next. Then the Samsung tree will need to > > > re-merge that dependency branch. > > > > Well, I can make a "convert PLL65xx to new registration method" patch, > > that would be basically the delta. If this could be merged before patch > > 7/8, no regression would be introduced. > > > > > Do you have a better idea? > > > > Not really. Maybe let's ask Kukjin whether he has already merged it to his > > tree. Kukjin, have you? > > > OK, if new branch is ready, I will replace with that or if re-merge is > required, I will. Either way, I'm fine and can handle. Mike, let me know > your choice :-) Since I have already published it let's just go with the delta patch. I can create another stable branch named clk-next-s3c64xx-delta that just has this patch on top of clk-next-s3c64xx OR I can apply it on top of the existing clk-next-s3c64xx and re-merge it. I'm trying to think on whether there are any weird git corner cases with re-merging clk-next-s3c64xx. Let me know if re-merging is somehow unsafe (makes history weird, or whatever). Let me know what option is better for you. I'll publish as soon as I get the delta patch. Apologies again for creating some extra work! Thanks, Mike > > Thanks, > Kukjin -- 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] extcon: extcon-dra7xx: Add extcon driver for USB ID detection
Hi George,
On 08/16/2013 07:13 PM, George Cherian wrote:
> Adding extcon driver for USB ID detection to dynamically
> configure USB Host/Peripheral mode.
>
> Signed-off-by: George Cherian
> ---
> .../devicetree/bindings/extcon/extcon-dra7xx.txt | 19 ++
> drivers/extcon/Kconfig | 7 +
> drivers/extcon/Makefile| 1 +
> drivers/extcon/extcon-dra7xx.c | 234
> +
> 4 files changed, 261 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
> create mode 100644 drivers/extcon/extcon-dra7xx.c
>
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
> b/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
> new file mode 100644
> index 000..37e4c22
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
> @@ -0,0 +1,19 @@
> +EXTCON FOR DRA7xx
> +
> +Required Properties:
> + - compatible : Should be "ti,dra7xx-usb"
> + - gpios : phandle to ID pin and interrupt gpio.
> +
> +Optional Properties:
> + - interrupt-parent : interrupt controller phandle
> + - interrupts : interrupt number
> +
> +
> +dra7x_extcon1 {
You used 'dra7xx-usb' device name. Why did you use 'dra7x_extcon1' name?
What is meaning 'dra7x_extcon1'?
> + compatible = "ti,dra7xx-usb";
> + gpios = <&pcf_usb 1 0>,
> + <&gpio6 11 2>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <11>;
> + };
You have to keep indentation rule.
> +
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index f1d54a3..b9cf0b2 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -64,4 +64,11 @@ config EXTCON_PALMAS
> Say Y here to enable support for USB peripheral and USB host
> detection by palmas usb.
>
> +config EXTCON_DRA7XX
> + tristate "DRA7XX EXTCON support"
> + help
> + Say Y here to enable support for USB peripheral and USB host
> + detection by pcf8575 using DRA7XX extcon.
You should explain detailed description about pcf8575 on patch description
and change description of EXTCON_DRA7xx as following:
"using DRA7XX extcon" -> "using DRA7XX device" or "using DRA7XX usb"
> +
> +
Remove unnecessary blank line.
> endif # MULTISTATE_SWITCH
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index e4fa8ba..e4778f9 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o
> obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
> obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o
> obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o
> +obj-$(CONFIG_EXTCON_DRA7XX) += extcon-dra7xx.o
> diff --git a/drivers/extcon/extcon-dra7xx.c b/drivers/extcon/extcon-dra7xx.c
> new file mode 100644
> index 000..268c25e
> --- /dev/null
> +++ b/drivers/extcon/extcon-dra7xx.c
> @@ -0,0 +1,234 @@
> +/*
> + * DRA7XX USB ID pin detection driver
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Author: George Cherian
> + *
> + * Based on extcon-palmas.c
> + *
> + * Author: Kishon Vijay Abraham I
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct dra7xx_usb {
> + struct device *dev;
> +
> + struct extcon_dev edev;
> + struct task_struct *thread_task;
> +
> + /*GPIO pin */
Add space between "/*" and "GPIO".
> + int id_gpio;
> + int irq_gpio;
> +
> + int id_prev;
> + int id_current;
> +
> +};
> +
> +static const char *dra7xx_extcon_cable[] = {
> + [0] = "USB",
> + [1] = "USB-HOST",
> + NULL,
> +};
> +
> +static const int mutually_exclusive[] = {0x3, 0x0};
> +
> +static int id_poll_func(void *data)
> +{
> + struct dra7xx_usb *dra7xx_usb = (struct dra7xx_usb *) data;
> +
> + allow_signal(SIGINT);
> + allow_signal(SIGTERM);
> + allow_signal(SIGKILL);
> + allow_signal(SIGUSR1);
> +
> + set_freezable();
> +
> + while (!kthread_should_stop()) {
> + dra7xx_usb->id_current = gpio_get_value_cansleep
> + (dra7xx_usb->id_gpio);
> + if (dra7xx_usb->id_current == dra7xx_usb->id_prev) {
> + schedule_timeout_interruptible
>
Re: [PATCH] chipidea: ci_hdrc_imx: Fix MODULE_ALIAS()
On Mon, Aug 19, 2013 at 03:36:33PM -0300, Fabio Estevam wrote:
> The MODULE_ALIAS string should match the '.name' field of the platform driver.
>
> In this case we have '.name = "imx_usb"', so adapt MODULE_ALIAS() accordingly.
>
> Signed-off-by: Fabio Estevam
> ---
> drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 74d998d..2aea048 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -202,7 +202,7 @@ static struct platform_driver ci_hdrc_imx_driver = {
>
> module_platform_driver(ci_hdrc_imx_driver);
>
> -MODULE_ALIAS("platform:imx-usb");
> +MODULE_ALIAS("platform:imx_usb");
> MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("CI HDRC i.MX USB binding");
> MODULE_AUTHOR("Marek Vasut ");
> --
> 1.8.1.2
>
Acked-by: Peter Chen
--
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 1/2] extcon: extcon-dra7xx: Add extcon driver for USB ID detection
Hi Stephen,
Thanks for your review.
On 8/20/2013 1:01 AM, Stephen Warren wrote:
On 08/16/2013 04:13 AM, George Cherian wrote:
Adding extcon driver for USB ID detection to dynamically
configure USB Host/Peripheral mode.
diff --git a/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
b/Documentation/devicetree/bindings/extcon/extcon-dra7xx.txt
+EXTCON FOR DRA7xx
+
+Required Properties:
Please at lest explain what a DRA7xxx is, and the purpose of the HW
module this binding describes.
DRA7xx is the SoC name and the USB VID detection is implemented via gpio's
Basically it does only ID detection via GPIO and there is no VBUS
detection in h/w.
+ - compatible : Should be "ti,dra7xx-usb"
If this is a USB VID detector rather than a full USB host controller,
then "usb" in the binding is a bit over-reaching. Perhaps "-usb-vid" or
"-usb-vid-detector" would be more accurate.
This will be renamed to dra7xx-extcon.
+ - gpios : phandle to ID pin and interrupt gpio.
This isn't just a phandle; it's phandle+args, or a GPIO specifier. Some
reference should be made to ../gpio/gpio.txt for the format.
okay
Why does the interrupt line need to be included in a list of GPIOs?
ID pins are connected to pcf8575, and the pcf8575's interrupt line is
inturn connected to
gpio bank6 pin 11, we use this gpio interrupt to detect the ID pin change.
If the DRA7xxx is a VID detector, why does it even need/have any GPIOs
associated with it; surely it has a dedicated VID input pin. Can you
provide more details re: how the HW is structured.
+Optional Properties:
+ - interrupt-parent : interrupt controller phandle
+ - interrupts : interrupt number
+
+
It's typical insert the following between those two blank lines:
Example:
... or delete one of the blank lines.
+dra7x_extcon1 {
+ compatible = "ti,dra7xx-usb";
+ gpios = <&pcf_usb 1 0>,
+ <&gpio6 11 2>;
+ interrupt-parent = <&gpio6>;
+ interrupts = <11>;
+ };
+
No need for that trailing blank line.
okay
--
-George
--
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
