-
Notifications
You must be signed in to change notification settings - Fork 178
android-tools-conf-configfs: handle multiple UDC controllers on IQ-X7181 #1355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| while true; do | ||
| count=`ls /sys/class/udc | wc -l` | ||
| if [ $count -gt 1 ]; then | ||
| udcname=a600000.usb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we pick up this name? Can we pick up the first one instead of hardcoding it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked all qcom targets supported in meta-qcom, for the targets with multiple USB ports, usb@a600000 was configured to the same USB port to download images (in EDL mode). Fixed the udcname here for easy use during development.
Take Hamoa EVK as an example, if we pick up the first one (which is a000000.usb not a600000.usb), developer have to plug out/in USB cable and try out the third type-c usb port for adb connection after images flashed. not friendly for use.
root@iq-x7181-evk:~# ls /sys/class/udc/
a000000.usb a600000.usb a800000.usb
qcom-next/arch/arm64/boot/dts/qcom$ grep usb_1_ss[0-2]_dwc3 x1e80100.dtsi
remote-endpoint = <&usb_1_ss0_dwc3_ss>;
remote-endpoint = <&usb_1_ss1_dwc3_ss>;
remote-endpoint = <&usb_1_ss2_dwc3_ss>;
usb_1_ss2_dwc3: usb@a000000 { => 3rd type-C usb port
usb_1_ss2_dwc3_hs: endpoint {
usb_1_ss2_dwc3_ss: endpoint {
usb_1_ss0_dwc3: usb@a600000 { => 1st type-C usb port, used to flash images in EDL mode as well
usb_1_ss0_dwc3_hs: endpoint {
usb_1_ss0_dwc3_ss: endpoint {
usb_1_ss1_dwc3: usb@a800000 {
usb_1_ss1_dwc3_hs: endpoint {
qcom-next/arch/arm64/boot/dts/qcom$ grep usb@a600000 *.dtsi
lemans.dtsi: usb_0: usb@a600000 {
qcs8300.dtsi: usb_1: usb@a600000 { => first USB port, there is no usb_0
qdu1000.dtsi: usb_1_dwc3: usb@a600000 {
sar2130p.dtsi: usb_1_dwc3: usb@a600000 {
sc7180.dtsi: usb_1_dwc3: usb@a600000 {
sc7280.dtsi: usb_1: usb@a600000 {
sc8180x.dtsi: usb_prim_dwc3: usb@a600000 {
sc8280xp.dtsi: usb_0_dwc3: usb@a600000 {
sdm670.dtsi: usb_1_dwc3: usb@a600000 {
sdm845.dtsi: usb_1_dwc3: usb@a600000 {
sdx75.dtsi: usb_dwc3: usb@a600000 {
sm6350.dtsi: usb_1_dwc3: usb@a600000 {
sm8150.dtsi: usb_1_dwc3: usb@a600000 {
sm8250.dtsi: usb_1_dwc3: usb@a600000 {
sm8350.dtsi: usb_1_dwc3: usb@a600000 {
sm8450.dtsi: usb_1: usb@a600000 {
sm8550.dtsi: usb_1: usb@a600000 {
sm8650.dtsi: usb_1: usb@a600000 {
sm8750.dtsi: usb_1: usb@a600000 {
talos.dtsi: usb_1_dwc3: usb@a600000 {
x1e80100.dtsi: usb_1_ss0_dwc3: usb@a600000 {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IPQ5424 has two USB controllers, usb@1e00000 and usb@8a00000. Likewise IPQ6018, IPQ8074 have two controllers, none of which match this address. MSM8996 has two USB controllers, both useable as OTG devices and their addresses don't match this one.
So, we need to find a way to find a better UDC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these targets enabled in meta-qcom ? I checked the boards under meta-qcom/conf/machine folder and did not find them under this folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qcom-armv8a machine covers generic configuration, which might be used for those machines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At given point, only one write to this node /sys/kernel/config/usb_gadget/adb/UDC is supported. without usb_gadget driver change, can we implement multiple adbd on UDC ports ? and do we really need multiple adb instances during development, once adb supported, what about the support of "adb reboot bootloader" "adb reboot edl", things become more complicated ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it become more complicated? ADB will still handle those commands.
By running ADB only on one port, you are limiting our customers to be able to use only one USB-C port for debugging. It might be undesirable. If we have multiple OTG or peripheral ports, there should be no difference in their functionality.
Regarding /sys/kernel/config/usb_gadget/adb/UDC. Of course there should be a separate configuration per each port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lumag,
Locally I tried to modify the configuration in android-gadget-setup/start scripts for multiple UDC ports, looks not function as expected. currently there are several high priority tasks on-going. would you mind to merge this PR if I update to only pick the first UDC name to unblock test from APT side ?
same as:
https://github.com/openembedded/meta-openembedded/blob/master/meta-oe/recipes-devtools/android-tools/android-tools-conf-configfs/android-gadget-start
ls /sys/class/udc/ | head -n 1 | xargs echo -n > /sys/kernel/config/usb_gadget/adb/UDC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's fine with me. Please open a ticket against meta-qcom to support multiple ADB instances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's fine with me. Please open a ticket against meta-qcom to support multiple ADB instances.
Create an issue to track: #1414
d6357f6 to
5751ff2
Compare
On IQ-X7181 EVK board, below command in android-gadget-start script will hit issue as there are multiple UDC controllers. Overwrite the script to ensure adb work normally. ls /sys/class/udc/ > /sys/kernel/config/usb_gadget/adb/UDC in https://github.com/openembedded/meta-openembedded/blob/master/meta-oe/dynamic-layers/selinux/recipes-devtool/android-tools/android-tools-conf-configfs/android-gadget-start Error: root@iq-x7181-evk:/# ls /sys/class/udc/ > /sys/kernel/config/usb_gadget/adb/UDC ls: write error: Device or resource busy Signed-off-by: Shoudi Li <shoudil@qti.qualcomm.com>
5751ff2 to
04cbcaf
Compare
Test run workflowTest jobs for commit 04cbcaf
|
Test Results 14 files ±0 14 suites ±0 1h 57m 39s ⏱️ + 57m 24s For more details on these failures, see this check. Results for commit 04cbcaf. ± Comparison against base commit 3cfd302. |
|
There are no differences between your file and the one provided via meta-oe, is this really needed? |
|
Ah, I get what you raised before, the issue is that meta-oe/dynamic-layers/selinux/android-tools/android-tools-conf-configfs/android-gadget-start is getting selected instead, which is basically an older version of the same script, which wasn't updated in meta-clang before. I believe the right fix here would be to send a patch to meta-oe to update meta-oe/dynamic-layers/selinux/android-tools/android-tools-conf-configfs/android-gadget-start based on the latest from meta-oe/recipes-devtools/android-tools/android-tools-conf-configfs/android-gadget-start (or drop and find a way to reuse the file from the other recipe). |
Yes, agree. That's why I tried to understand the background to maintain another version under dynamic-layer/selinux/xx in #1355 (comment). |
On IQ-X7181 EVK board, below command in android-gadget-start script will hit issue as there are multiple UDC controllers. Overwrite the script to ensure adb work normally.
ls /sys/class/udc/ > /sys/kernel/config/usb_gadget/adb/UDC
in https://github.com/openembedded/meta-openembedded/blob/master/meta-oe/dynamic-layers/selinux/recipes-devtool/android-tools/android-tools-conf-configfs/android-gadget-start
Error:
root@iq-x7181-evk:/# ls /sys/class/udc/ > /sys/kernel/config/usb_gadget/adb/UDC
ls: write error: Device or resource busy