Re: [PATCH v2 1/2] usb: ehci-mx6: move mode set/detect to probe
On Fri, Jul 2, 2021 at 12:17 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 2:15 AM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat
Are you absolutely sure of that ?
It used to be necessary to know the core mode early to determine which driver (the gadget or host) to probe, that's why this code was in of_to_plat.
Marek,
I can't find a reason for the core mode to be known early and have tested it both with host and gadget mode on IMX6 and IMX8MM. Fabio has tested this change also (from another thread) with SPL SDP on IMX8MM (granted the SPL is not using DM_USB for that).
It appears I accidentally forgot to cc the uboot list so I'll need to resend this at any rate.
Best regards,
Tim
On 7/2/21 9:43 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:17 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 2:15 AM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat
Are you absolutely sure of that ?
It used to be necessary to know the core mode early to determine which driver (the gadget or host) to probe, that's why this code was in of_to_plat.
Marek,
I can't find a reason for the core mode to be known early and have tested it both with host and gadget mode on IMX6 and IMX8MM. Fabio has tested this change also (from another thread) with SPL SDP on IMX8MM (granted the SPL is not using DM_USB for that).
It appears I accidentally forgot to cc the uboot list so I'll need to resend this at any rate.
If I recall it correctly, the mode determination happened early because it was necessary to find out which driver to probe (this one, or the samsung gadget one) before it actually probed. I'm not sure myself if this is still even applicable.
On Fri, Jul 2, 2021 at 12:53 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 9:43 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:17 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 2:15 AM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat
Are you absolutely sure of that ?
It used to be necessary to know the core mode early to determine which driver (the gadget or host) to probe, that's why this code was in of_to_plat.
Marek,
I can't find a reason for the core mode to be known early and have tested it both with host and gadget mode on IMX6 and IMX8MM. Fabio has tested this change also (from another thread) with SPL SDP on IMX8MM (granted the SPL is not using DM_USB for that).
It appears I accidentally forgot to cc the uboot list so I'll need to resend this at any rate.
If I recall it correctly, the mode determination happened early because it was necessary to find out which driver to probe (this one, or the samsung gadget one) before it actually probed. I'm not sure myself if this is still even applicable.
Marek,
When I tested UMS with this series on imx6 based gwventana I had to troubleshoot a regression caused when I switched to DM_USB and found that the gadget driver isn't probed until you issue the ums command. I don't know if this helps reduce your hesitation here.
I'm not sure what else you want me to do with this - personally I would like to see some more maintainers of boards using ehci-mx6 chime in (mx7?) with testing results showing host and device un-affected. I only have imx6q/imx6dl/imx8mm/imx8mn to test with here.
Tim
On 7/2/21 11:44 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:53 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 9:43 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:17 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 2:15 AM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat
Are you absolutely sure of that ?
It used to be necessary to know the core mode early to determine which driver (the gadget or host) to probe, that's why this code was in of_to_plat.
Marek,
I can't find a reason for the core mode to be known early and have tested it both with host and gadget mode on IMX6 and IMX8MM. Fabio has tested this change also (from another thread) with SPL SDP on IMX8MM (granted the SPL is not using DM_USB for that).
It appears I accidentally forgot to cc the uboot list so I'll need to resend this at any rate.
If I recall it correctly, the mode determination happened early because it was necessary to find out which driver to probe (this one, or the samsung gadget one) before it actually probed. I'm not sure myself if this is still even applicable.
Marek,
When I tested UMS with this series on imx6 based gwventana I had to troubleshoot a regression caused when I switched to DM_USB and found that the gadget driver isn't probed until you issue the ums command. I don't know if this helps reduce your hesitation here.
Do I understand it correctly that, because you had to debug a regression, this patch should be OK ? That implication there is a bit odd.
What I am asking is whether you considered all the things which can go wrong with this still messy driver, that's all.
I'm not sure what else you want me to do with this - personally I would like to see some more maintainers of boards using ehci-mx6 chime in (mx7?) with testing results showing host and device un-affected. I only have imx6q/imx6dl/imx8mm/imx8mn to test with here.
The other option is to put it into next and see whether someone complains. I am banking toward that option.
On Fri, Jul 2, 2021 at 3:18 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 11:44 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:53 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 9:43 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:17 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 2:15 AM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat
Are you absolutely sure of that ?
It used to be necessary to know the core mode early to determine which driver (the gadget or host) to probe, that's why this code was in of_to_plat.
Marek,
I can't find a reason for the core mode to be known early and have tested it both with host and gadget mode on IMX6 and IMX8MM. Fabio has tested this change also (from another thread) with SPL SDP on IMX8MM (granted the SPL is not using DM_USB for that).
It appears I accidentally forgot to cc the uboot list so I'll need to resend this at any rate.
If I recall it correctly, the mode determination happened early because it was necessary to find out which driver to probe (this one, or the samsung gadget one) before it actually probed. I'm not sure myself if this is still even applicable.
Marek,
When I tested UMS with this series on imx6 based gwventana I had to troubleshoot a regression caused when I switched to DM_USB and found that the gadget driver isn't probed until you issue the ums command. I don't know if this helps reduce your hesitation here.
Do I understand it correctly that, because you had to debug a regression, this patch should be OK ? That implication there is a bit odd.
Not at all... My regression had to do with the fact that DM_USB uses the 'usb0' alias to get to the USB OTG controller (which I didn't have defined right). I merely pointed out that I had to poke around at the code enough to realize the device controller didn't get probed until you use the ums command. I didn't realize my regression until I set out to test gadget mode so that I could resubmit this series and let you know that I had tested both host and gadget mode on the boards I have.
What I am asking is whether you considered all the things which can go wrong with this still messy driver, that's all.
Oh I most definitely will never be able to consider everything as you point out the driver is a complete mess and it supports a large variety of SoC's and configurations. All I can do is make my best effort to look for possible issues (which I've done), test (which I've done), and post for review.
I'm not sure what else you want me to do with this - personally I would like to see some more maintainers of boards using ehci-mx6 chime in (mx7?) with testing results showing host and device un-affected. I only have imx6q/imx6dl/imx8mm/imx8mn to test with here.
The other option is to put it into next and see whether someone complains. I am banking toward that option.
By 'next' do you mean it sits in a branch that never gets merged or are you saying it gets merged after v2021.07? I would agree it should not be merged this late into v2021.07 but if it sits in a branch that's never merged to master it will never get tested.
Tim
On 7/3/21 12:36 AM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 3:18 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 11:44 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:53 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 9:43 PM, Tim Harvey wrote:
On Fri, Jul 2, 2021 at 12:17 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 2:15 AM, Tim Harvey wrote: > There is no need to set and/or detect mode in of_to_plat
Are you absolutely sure of that ?
It used to be necessary to know the core mode early to determine which driver (the gadget or host) to probe, that's why this code was in of_to_plat.
Marek,
I can't find a reason for the core mode to be known early and have tested it both with host and gadget mode on IMX6 and IMX8MM. Fabio has tested this change also (from another thread) with SPL SDP on IMX8MM (granted the SPL is not using DM_USB for that).
It appears I accidentally forgot to cc the uboot list so I'll need to resend this at any rate.
If I recall it correctly, the mode determination happened early because it was necessary to find out which driver to probe (this one, or the samsung gadget one) before it actually probed. I'm not sure myself if this is still even applicable.
Marek,
When I tested UMS with this series on imx6 based gwventana I had to troubleshoot a regression caused when I switched to DM_USB and found that the gadget driver isn't probed until you issue the ums command. I don't know if this helps reduce your hesitation here.
Do I understand it correctly that, because you had to debug a regression, this patch should be OK ? That implication there is a bit odd.
Not at all... My regression had to do with the fact that DM_USB uses the 'usb0' alias to get to the USB OTG controller (which I didn't have defined right). I merely pointed out that I had to poke around at the code enough to realize the device controller didn't get probed until you use the ums command. I didn't realize my regression until I set out to test gadget mode so that I could resubmit this series and let you know that I had tested both host and gadget mode on the boards I have.
What I am asking is whether you considered all the things which can go wrong with this still messy driver, that's all.
Oh I most definitely will never be able to consider everything as you point out the driver is a complete mess and it supports a large variety of SoC's and configurations. All I can do is make my best effort to look for possible issues (which I've done), test (which I've done), and post for review.
All right
I'm not sure what else you want me to do with this - personally I would like to see some more maintainers of boards using ehci-mx6 chime in (mx7?) with testing results showing host and device un-affected. I only have imx6q/imx6dl/imx8mm/imx8mn to test with here.
The other option is to put it into next and see whether someone complains. I am banking toward that option.
By 'next' do you mean it sits in a branch that never gets merged or are you saying it gets merged after v2021.07? I would agree it should not be merged this late into v2021.07 but if it sits in a branch that's never merged to master it will never get tested.
The later, i.e. scheduled for 2021.10 .
Hi Tim,
On Fri, Jul 2, 2021 at 6:44 PM Tim Harvey tharvey@gateworks.com wrote:
Marek,
When I tested UMS with this series on imx6 based gwventana I had to troubleshoot a regression caused when I switched to DM_USB and found that the gadget driver isn't probed until you issue the ums command. I don't know if this helps reduce your hesitation here.
I'm not sure what else you want me to do with this - personally I would like to see some more maintainers of boards using ehci-mx6 chime in (mx7?) with testing results showing host and device un-affected. I only have imx6q/imx6dl/imx8mm/imx8mn to test with here.
I have successfully tested your series on a imx7s-warp board too (besides imx8mm-evk).
Thanks
participants (3)
-
Fabio Estevam -
Marek Vasut -
Tim Harvey