* [PATCH] usb: cdns3: gadget: Don't manage pullups @ 2019-10-23 9:02 Roger Quadros 2019-10-23 9:17 ` Pawel Laszczak 0 siblings, 1 reply; 9+ messages in thread From: Roger Quadros @ 2019-10-23 9:02 UTC (permalink / raw) To: felipe.balbi, gregkh Cc: pawell, peter.chen, nsekhar, kurahul, linux-usb, linux-kernel, Roger Quadros The USB gadget core is supposed to manage pullups of the controller. Don't manage pullups from within the controller driver. Otherwise, function drivers are not able to keep the controller disconnected from the bus till they are ready. (e.g. g_webcam) Signed-off-by: Roger Quadros <rogerq@ti.com> --- Hi Greg/Felipe, This can be used for -rc as it is a bug fix. cheers, -roger drivers/usb/cdns3/gadget.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index 2ca280f4c054..714382d96055 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -2324,8 +2324,6 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev) writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf); cdns3_configure_dmult(priv_dev, NULL); - - cdns3_gadget_pullup(&priv_dev->gadget, 1); } /** @@ -2708,8 +2706,6 @@ static int cdns3_gadget_suspend(struct cdns3 *cdns, bool do_wakeup) /* disable interrupt for device */ writel(0, &priv_dev->regs->usb_ien); - cdns3_gadget_pullup(&priv_dev->gadget, 0); - return 0; } -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-23 9:02 [PATCH] usb: cdns3: gadget: Don't manage pullups Roger Quadros @ 2019-10-23 9:17 ` Pawel Laszczak 2019-10-25 3:13 ` Peter Chen 0 siblings, 1 reply; 9+ messages in thread From: Pawel Laszczak @ 2019-10-23 9:17 UTC (permalink / raw) To: Roger Quadros, felipe.balbi, gregkh Cc: peter.chen, nsekhar, Rahul Kumar, linux-usb, linux-kernel Hi, Reviewed-by: Pawel Laszczak <pawell@cadence.com> Regards, Pawel, >The USB gadget core is supposed to manage pullups >of the controller. Don't manage pullups from within >the controller driver. Otherwise, function drivers >are not able to keep the controller disconnected from >the bus till they are ready. (e.g. g_webcam) > >Signed-off-by: Roger Quadros <rogerq@ti.com> >--- >Hi Greg/Felipe, > >This can be used for -rc as it is a bug fix. > >cheers, >-roger > > drivers/usb/cdns3/gadget.c | 4 ---- > 1 file changed, 4 deletions(-) > >diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c >index 2ca280f4c054..714382d96055 100644 >--- a/drivers/usb/cdns3/gadget.c >+++ b/drivers/usb/cdns3/gadget.c >@@ -2324,8 +2324,6 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev) > writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf); > > cdns3_configure_dmult(priv_dev, NULL); >- >- cdns3_gadget_pullup(&priv_dev->gadget, 1); > } > > /** >@@ -2708,8 +2706,6 @@ static int cdns3_gadget_suspend(struct cdns3 *cdns, bool do_wakeup) > /* disable interrupt for device */ > writel(0, &priv_dev->regs->usb_ien); > >- cdns3_gadget_pullup(&priv_dev->gadget, 0); >- > return 0; > } > >-- >Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-23 9:17 ` Pawel Laszczak @ 2019-10-25 3:13 ` Peter Chen 2019-10-25 9:59 ` Roger Quadros 0 siblings, 1 reply; 9+ messages in thread From: Peter Chen @ 2019-10-25 3:13 UTC (permalink / raw) To: Pawel Laszczak Cc: Roger Quadros, felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel On 19-10-23 09:17:45, Pawel Laszczak wrote: > Hi, > > Reviewed-by: Pawel Laszczak <pawell@cadence.com> Hi Roger & Pawel, Assume gadget function has already enabled, if you switch host mode to device mode, with your changes, where the device mode will be enabled again? Peter > > Regards, > Pawel, > > >The USB gadget core is supposed to manage pullups > >of the controller. Don't manage pullups from within > >the controller driver. Otherwise, function drivers > >are not able to keep the controller disconnected from > >the bus till they are ready. (e.g. g_webcam) > > > >Signed-off-by: Roger Quadros <rogerq@ti.com> > >--- > >Hi Greg/Felipe, > > > >This can be used for -rc as it is a bug fix. > > > >cheers, > >-roger > > > > drivers/usb/cdns3/gadget.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > >diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > >index 2ca280f4c054..714382d96055 100644 > >--- a/drivers/usb/cdns3/gadget.c > >+++ b/drivers/usb/cdns3/gadget.c > >@@ -2324,8 +2324,6 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev) > > writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf); > > > > cdns3_configure_dmult(priv_dev, NULL); > >- > >- cdns3_gadget_pullup(&priv_dev->gadget, 1); > > } > > > > /** > >@@ -2708,8 +2706,6 @@ static int cdns3_gadget_suspend(struct cdns3 *cdns, bool do_wakeup) > > /* disable interrupt for device */ > > writel(0, &priv_dev->regs->usb_ien); > > > >- cdns3_gadget_pullup(&priv_dev->gadget, 0); > >- > > return 0; > > } > > > >-- > >Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > >Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > -- Thanks, Peter Chen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-25 3:13 ` Peter Chen @ 2019-10-25 9:59 ` Roger Quadros 2019-10-25 10:14 ` Felipe Balbi 2019-10-29 3:12 ` Peter Chen 0 siblings, 2 replies; 9+ messages in thread From: Roger Quadros @ 2019-10-25 9:59 UTC (permalink / raw) To: Peter Chen, Pawel Laszczak Cc: felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel Peter, On 25/10/2019 06:13, Peter Chen wrote: > On 19-10-23 09:17:45, Pawel Laszczak wrote: >> Hi, >> >> Reviewed-by: Pawel Laszczak <pawell@cadence.com> > > Hi Roger & Pawel, > > Assume gadget function has already enabled, if you switch host mode > to device mode, with your changes, where the device mode will be enabled > again? When it switches from device mode to host the UDC is removed. When we switch back from host to device mode the UDC is added, so, usb_add_gadget_udc_release()-> check_pending_gadget_drivers()-> udc_bind_to_driver()->usb_udc_connect_control()->usb_gadget_connect()-> gadget->ops->pullup() cheers, -roger > > Peter >> >> Regards, >> Pawel, >> >>> The USB gadget core is supposed to manage pullups >>> of the controller. Don't manage pullups from within >>> the controller driver. Otherwise, function drivers >>> are not able to keep the controller disconnected from >>> the bus till they are ready. (e.g. g_webcam) >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> Hi Greg/Felipe, >>> >>> This can be used for -rc as it is a bug fix. >>> >>> cheers, >>> -roger >>> >>> drivers/usb/cdns3/gadget.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c >>> index 2ca280f4c054..714382d96055 100644 >>> --- a/drivers/usb/cdns3/gadget.c >>> +++ b/drivers/usb/cdns3/gadget.c >>> @@ -2324,8 +2324,6 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev) >>> writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf); >>> >>> cdns3_configure_dmult(priv_dev, NULL); >>> - >>> - cdns3_gadget_pullup(&priv_dev->gadget, 1); >>> } >>> >>> /** >>> @@ -2708,8 +2706,6 @@ static int cdns3_gadget_suspend(struct cdns3 *cdns, bool do_wakeup) >>> /* disable interrupt for device */ >>> writel(0, &priv_dev->regs->usb_ien); >>> >>> - cdns3_gadget_pullup(&priv_dev->gadget, 0); >>> - >>> return 0; >>> } >>> >>> -- >>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >> > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-25 9:59 ` Roger Quadros @ 2019-10-25 10:14 ` Felipe Balbi 2019-10-29 3:12 ` Peter Chen 1 sibling, 0 replies; 9+ messages in thread From: Felipe Balbi @ 2019-10-25 10:14 UTC (permalink / raw) To: Roger Quadros, Peter Chen, Pawel Laszczak Cc: felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 907 bytes --] Hi, Roger Quadros <rogerq@ti.com> writes: > On 25/10/2019 06:13, Peter Chen wrote: >> On 19-10-23 09:17:45, Pawel Laszczak wrote: >>> Hi, >>> >>> Reviewed-by: Pawel Laszczak <pawell@cadence.com> >> >> Hi Roger & Pawel, >> >> Assume gadget function has already enabled, if you switch host mode >> to device mode, with your changes, where the device mode will be enabled >> again? > > When it switches from device mode to host the UDC is removed. When we switch > back from host to device mode the UDC is added, so, > > usb_add_gadget_udc_release()-> check_pending_gadget_drivers()-> > udc_bind_to_driver()->usb_udc_connect_control()->usb_gadget_connect()-> > gadget->ops->pullup() I agree with Roger here. UDC shouldn't try to manage pullups directly. If there are any bugs related to role switch, we should fix it in udc core, so the fix applies to everyone ;-) -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-25 9:59 ` Roger Quadros 2019-10-25 10:14 ` Felipe Balbi @ 2019-10-29 3:12 ` Peter Chen 2019-10-29 9:37 ` Roger Quadros 1 sibling, 1 reply; 9+ messages in thread From: Peter Chen @ 2019-10-29 3:12 UTC (permalink / raw) To: Roger Quadros Cc: Pawel Laszczak, felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel On 19-10-25 12:59:17, Roger Quadros wrote: > Peter, > > On 25/10/2019 06:13, Peter Chen wrote: > > On 19-10-23 09:17:45, Pawel Laszczak wrote: > > > Hi, > > > > > > Reviewed-by: Pawel Laszczak <pawell@cadence.com> > > > > Hi Roger & Pawel, > > > > Assume gadget function has already enabled, if you switch host mode > > to device mode, with your changes, where the device mode will be enabled > > again? > > When it switches from device mode to host the UDC is removed. When we switch > back from host to device mode the UDC is added, so, > > usb_add_gadget_udc_release()-> check_pending_gadget_drivers()-> > udc_bind_to_driver()->usb_udc_connect_control()->usb_gadget_connect()-> > gadget->ops->pullup() Thanks. I have another question how you decide when to store UDC name to /sys/kernel/config/usb_gadget/g1/UDC? Do you have a user daemon program to monitor VBUS or external connector? At host mode, the store operation will fail due to there is NO UDC. -- Thanks, Peter Chen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-29 3:12 ` Peter Chen @ 2019-10-29 9:37 ` Roger Quadros 2019-10-29 10:33 ` Felipe Balbi 2019-10-30 1:15 ` Peter Chen 0 siblings, 2 replies; 9+ messages in thread From: Roger Quadros @ 2019-10-29 9:37 UTC (permalink / raw) To: Peter Chen Cc: Pawel Laszczak, felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel On 29/10/2019 05:12, Peter Chen wrote: > On 19-10-25 12:59:17, Roger Quadros wrote: >> Peter, >> >> On 25/10/2019 06:13, Peter Chen wrote: >>> On 19-10-23 09:17:45, Pawel Laszczak wrote: >>>> Hi, >>>> >>>> Reviewed-by: Pawel Laszczak <pawell@cadence.com> >>> >>> Hi Roger & Pawel, >>> >>> Assume gadget function has already enabled, if you switch host mode >>> to device mode, with your changes, where the device mode will be enabled >>> again? >> >> When it switches from device mode to host the UDC is removed. When we switch >> back from host to device mode the UDC is added, so, >> >> usb_add_gadget_udc_release()-> check_pending_gadget_drivers()-> >> udc_bind_to_driver()->usb_udc_connect_control()->usb_gadget_connect()-> >> gadget->ops->pullup() > > Thanks. I have another question how you decide when to store UDC name > to /sys/kernel/config/usb_gadget/g1/UDC? Do you have a user daemon program > to monitor VBUS or external connector? At host mode, the store operation > will fail due to there is NO UDC. > Yes, user space needs to monitor /sys/class/usb_role/6000000.usb-role-switch/role When it becomes "device" the UDC is available and it can prepare to configure the UDC. Could you please give your Ack for this patch if it is OK? Thanks. cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-29 9:37 ` Roger Quadros @ 2019-10-29 10:33 ` Felipe Balbi 2019-10-30 1:15 ` Peter Chen 1 sibling, 0 replies; 9+ messages in thread From: Felipe Balbi @ 2019-10-29 10:33 UTC (permalink / raw) To: Roger Quadros, Peter Chen Cc: Pawel Laszczak, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1573 bytes --] Hi, Roger Quadros <rogerq@ti.com> writes: > On 29/10/2019 05:12, Peter Chen wrote: >> On 19-10-25 12:59:17, Roger Quadros wrote: >>> Peter, >>> >>> On 25/10/2019 06:13, Peter Chen wrote: >>>> On 19-10-23 09:17:45, Pawel Laszczak wrote: >>>>> Hi, >>>>> >>>>> Reviewed-by: Pawel Laszczak <pawell@cadence.com> >>>> >>>> Hi Roger & Pawel, >>>> >>>> Assume gadget function has already enabled, if you switch host mode >>>> to device mode, with your changes, where the device mode will be enabled >>>> again? >>> >>> When it switches from device mode to host the UDC is removed. When we switch >>> back from host to device mode the UDC is added, so, >>> >>> usb_add_gadget_udc_release()-> check_pending_gadget_drivers()-> >>> udc_bind_to_driver()->usb_udc_connect_control()->usb_gadget_connect()-> >>> gadget->ops->pullup() >> >> Thanks. I have another question how you decide when to store UDC name >> to /sys/kernel/config/usb_gadget/g1/UDC? Do you have a user daemon program >> to monitor VBUS or external connector? At host mode, the store operation >> will fail due to there is NO UDC. >> > > Yes, user space needs to monitor /sys/class/usb_role/6000000.usb-role-switch/role > > When it becomes "device" the UDC is available and it can prepare to configure > the UDC. > > Could you please give your Ack for this patch if it is OK? Thanks. this is already in its way to Linus' tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=f3fb802efaef3662744a2215a51294d52a7cfc0e -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: cdns3: gadget: Don't manage pullups 2019-10-29 9:37 ` Roger Quadros 2019-10-29 10:33 ` Felipe Balbi @ 2019-10-30 1:15 ` Peter Chen 1 sibling, 0 replies; 9+ messages in thread From: Peter Chen @ 2019-10-30 1:15 UTC (permalink / raw) To: Roger Quadros Cc: Pawel Laszczak, felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel On 19-10-29 11:37:31, Roger Quadros wrote: > > > On 29/10/2019 05:12, Peter Chen wrote: > > On 19-10-25 12:59:17, Roger Quadros wrote: > > > Peter, > > > > > > On 25/10/2019 06:13, Peter Chen wrote: > > > > On 19-10-23 09:17:45, Pawel Laszczak wrote: > > > > > Hi, > > > > > > > > > > Reviewed-by: Pawel Laszczak <pawell@cadence.com> > > > > > > > > Hi Roger & Pawel, > > > > > > > > Assume gadget function has already enabled, if you switch host mode > > > > to device mode, with your changes, where the device mode will be enabled > > > > again? > > > > > > When it switches from device mode to host the UDC is removed. When we switch > > > back from host to device mode the UDC is added, so, > > > > > > usb_add_gadget_udc_release()-> check_pending_gadget_drivers()-> > > > udc_bind_to_driver()->usb_udc_connect_control()->usb_gadget_connect()-> > > > gadget->ops->pullup() > > > > Thanks. I have another question how you decide when to store UDC name > > to /sys/kernel/config/usb_gadget/g1/UDC? Do you have a user daemon program > > to monitor VBUS or external connector? At host mode, the store operation > > will fail due to there is NO UDC. > > > > Yes, user space needs to monitor /sys/class/usb_role/6000000.usb-role-switch/role > > When it becomes "device" the UDC is available and it can prepare to configure > the UDC. > > Could you please give your Ack for this patch if it is OK? Thanks. > Acked-by: Peter Chen <peter.chen@nxp.com> -- Thanks, Peter Chen ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-10-30 1:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-23 9:02 [PATCH] usb: cdns3: gadget: Don't manage pullups Roger Quadros 2019-10-23 9:17 ` Pawel Laszczak 2019-10-25 3:13 ` Peter Chen 2019-10-25 9:59 ` Roger Quadros 2019-10-25 10:14 ` Felipe Balbi 2019-10-29 3:12 ` Peter Chen 2019-10-29 9:37 ` Roger Quadros 2019-10-29 10:33 ` Felipe Balbi 2019-10-30 1:15 ` Peter Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).