Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. -- balbi -- 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 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 01:49:51PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 1:59 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:49:51PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); -- balbi -- 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 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 02:04:37PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:59 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:49:51PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* -* When a babble condition occurs, the musb controller -* removes the session bit and the endpoint config is lost. -*/ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
Hi again, On Thu, Feb 26, 2015 at 02:15:40PM -0600, Felipe Balbi wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* -* When a babble condition occurs, the musb controller -* removes the session bit and the endpoint config is lost. -*/ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. here's the final version: commit 3db11e7da5938bbd955410355194625f699a424c Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. while at that, make sure to tell usbcore that we're dropping the session and, thus, disconnecting the device. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..64cb242d92f7 100644 --- a/drivers/usb/musb/musb_core.c +++
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 02:25:20PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 2:19 PM, Felipe Balbi ba...@ti.com wrote: Hi again, On Thu, Feb 26, 2015 at 02:15:40PM -0600, Felipe Balbi wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* -* When a babble condition occurs, the musb controller -* removes the session bit and the endpoint config is lost. -*/ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. here's the final version: commit 3db11e7da5938bbd955410355194625f699a424c Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. while at that, make sure to tell usbcore that we're dropping the session and, thus, disconnecting
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 2:04 PM, Bin Liu binml...@gmail.com wrote: On Thu, Feb 26, 2015 at 1:59 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:49:51PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) I meant your idea is better. I will drop my patch. We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); -- balbi -- 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 03/15] usb: musb: core: fix highspeed check
Hi, On Thu, Feb 26, 2015 at 02:11:15PM -0600, Felipe Balbi wrote: On Thu, Feb 26, 2015 at 02:04:37PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:59 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:49:51PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote: Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* - * When a babble condition occurs, the musb controller - * removes the session bit and the endpoint config is lost. - */ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. here's the final version: commit 3db11e7da5938bbd955410355194625f699a424c Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. while at that, make sure to tell usbcore
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 2:19 PM, Felipe Balbi ba...@ti.com wrote: Hi again, On Thu, Feb 26, 2015 at 02:15:40PM -0600, Felipe Balbi wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* -* When a babble condition occurs, the musb controller -* removes the session bit and the endpoint config is lost. -*/ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. here's the final version: commit 3db11e7da5938bbd955410355194625f699a424c Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. while at that, make sure to tell usbcore that we're dropping the session and, thus, disconnecting the device. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..64cb242d92f7 100644 --- a/drivers/usb/musb/musb_core.c +++
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
Felipe, On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. -- balbi -- 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 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 02:38:07PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 2:31 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 02:25:20PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 2:19 PM, Felipe Balbi ba...@ti.com wrote: Hi again, On Thu, Feb 26, 2015 at 02:15:40PM -0600, Felipe Balbi wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* -* When a babble condition occurs, the musb controller -* removes the session bit and the endpoint config is lost. -*/ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. here's the final version: commit 3db11e7da5938bbd955410355194625f699a424c Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at
Re: [PATCH v2 03/15] usb: musb: core: fix highspeed check
On Thu, Feb 26, 2015 at 2:31 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Feb 26, 2015 at 02:25:20PM -0600, Bin Liu wrote: On Thu, Feb 26, 2015 at 2:19 PM, Felipe Balbi ba...@ti.com wrote: Hi again, On Thu, Feb 26, 2015 at 02:15:40PM -0600, Felipe Balbi wrote: On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: FSDEV is set for both HIGH and FULL speeds, the correct HIGHSPEED check is done through power register's HSMODE bit. Signed-off-by: Felipe Balbi ba...@ti.com I'm still unsure if we should really ignore babble on FS/LS. It seems to me we should never ignore it, but I really don't have a way to prove this statement. For the sake of reducing impact, we will just fix HS check here. I believe we should drop speed check in here and not ignore babble regardless. We have seen many cases that full-speed babble causes MUSB stop working. I'll make that as a separate patch then, just to make sure we can revert it later if something goes wrong ;-) Agreed. I noticed something else. If we really don't need to reset musb in case of babble, then we can drop that recover_work completely which simplifies babble handling quite a bit. I'll fiddle with that, if you don't mind. That is fine with me. I am writing the comments for the dropping reset patch right now ;) We only need the following in musb_recover_work() for the recovery. 1852 /* 1853 * When a babble condition occurs, the musb controller 1854 * removes the session bit and the endpoint config is lost. 1855 */ 1856 if (musb-dyn_fifo) 1857 status = ep_config_from_table(musb); 1858 else 1859 status = ep_config_from_hw(musb); 1860 1861 /* start the session again */ 1862 if (status == 0) 1863 musb_start(musb); the statement that musb looses ep configuration seems bogus to me. I dropped that too: commit 4a07d415bf5894c66f61827c30053a65d6dfce26 Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d9c627d54db6..219636c1b020 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) static void musb_recover_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, recover_work.work); - int status, ret; + int ret; + u8 devctl; ret = musb_platform_reset(musb); if (ret) { @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct *data) return; } - usb_phy_vbus_off(musb-xceiv); - usleep_range(100, 200); - - usb_phy_vbus_on(musb-xceiv); - usleep_range(100, 200); + /* drop session bit */ + devctl = musb_readb(musb-mregs, MUSB_DEVCTL); + devctl = ~MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); - /* -* When a babble condition occurs, the musb controller -* removes the session bit and the endpoint config is lost. -*/ - if (musb-dyn_fifo) - status = ep_config_from_table(musb); - else - status = ep_config_from_hw(musb); + /* wait for session to really drop */ + udelay(100); - /* start the session again */ - if (status == 0) - musb_start(musb); + /* restart session */ + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb-mregs, MUSB_DEVCTL, devctl); } /* -- the only thing that we're still missing is a notification to usbcore that we have a port being disabled, that's the only way to have testusb know that it has been disconnected by the host. here's the final version: commit 3db11e7da5938bbd955410355194625f699a424c Author: Felipe Balbi ba...@ti.com Date: Thu Feb 26 14:02:35 2015 -0600 usb: musb: core: simplify musb_recover_work() we're not resetting musb at all, just restarting the session. This means we don't need to touch PHYs or VBUS or anything like that. Just make sure session bit is reenabled after MUSB dropped it. while at that, make sure to tell