add ready cb to bdm #668

KrahJohlito · 2024-10-08T23:14:05Z

So this is basically open for review more so than anything.. this coupled with this "fixes" the issue with OPL not launching from mass0 when dual USB is connected by not allowing reads in cdvdman until both devices are init see log after this change compared to before:

THIS COMMIT:

connecting device usb0p0
attaching to usb0p0
usbmass_bd: probe: devId=1
usbmass_bd: usb_mass_findDevice devId 1
usbmass_bd: bNumInterfaces 1
usbmass_bd: bInterfaceClass 8 bInterfaceSubClass 6 bInterfaceProusbmass_bd: connect: devId=1
usbmass_bd: usb_mass_findDevice devId 1
usbmass_bd: register Input endpoint id =4 addr=81 packetSize=64
usbmass_bd: register Output endpoint id =5 addr=02 packetSize=64
usbmass_bd: connect ok: epI=4, epO=5
usbmass_bd: Waiting... devices_initialising: 1
usbmass_bd: Detected another device initialising. Breaking wait.
usbmass_bd: setting configuration controlEp=0, confNum=1
usbmass_bd: callback: res 0, bytes 0, arg 1f2b88
usbmass_bd: setting interface controlEp=0, interface=0 altSetting=0
usbmass_bd: callback: res 0, bytes 0, arg 1f2b88
usbmass_bd: callback: res 0, bytes 1, arg 1f2ac0
usbmass_bd: usb_queue_cmd
usbmass_bd: callback: res 0, bytes 31, arg 1f2a78
usbmass_bd: callback: res 0, bytes 13, arg 1f2a40
usbmass_bd: bulk csw result: 0, csw.status: 0
usbmass_bd: Vendor: PNY
usbmass_bd: Product: USB 3.0 FD
usbmass_bd: Revision: PMAP
usbmass_bd: usb_queue_cmd
usbmass_bd: callback: res 0, bytes 31, arg 1f2a78
usbmass_bd: callback: res 0, bytes 13, arg 1f2a40
usbmass_bd: bulk csw result: 0, csw.status: 0
usbmass_bd: usb_queue_cmd
usbmass_bd: callback: res 0, bytes 31, arg 1f2a78
usbmass_bd: callback: res 0, bytes 13, arg 1f2a40
usbmass_bd: bulk csw result: 0, csw.status: 0
usbmass_bd: 0 483950591-byte logical blocks: (0MB / 512MiB)
connecting device usb1p0
Device usb0p0 is now ready. UNLOCKING...
usbmass_bd: Device 2 is now ready
usbmass_bd: Device 1 is now ready
Waiting for device...done!
sceCdRead lsn=16 sectors=1 sector_size=2048 buf=00029304
cdvdman_read lsn=16 sectors=1 buf=29304
usbmass_bd: usb_queue_cmd
sceCdSync 0 sync flag = 1

CURRENT:

connecting device usb0p0
attaching to usb0p0
Waiting for device...done!
sceCdRead lsn=16 sectors=1 sector_size=2048 buf=00024b14
cdvdman_read lsn=16 sectors=1 buf=24b14
sceCdSync 0 sync flag = 1
cdvdman_searchfile_init mediaLsnCount=1971232
sceCdLayerSearchFile \SLUS_214.19;1
cdvdman_findfile \SLUS_214.19;1 layer0
cdvdman_findfile cdvdman_filepath=\SLUS_214.19;1
cdvdman_locatefile start locating \SLUS_214.19;1, layer=0
sceCdRead lsn=261 sectors=1 sector_size=2048 buf=00024b14
cdvdman_read lsn=261 sectors=1 buf=24b14
sceCdSync 0 sync flag = 1
cdvdman_locatefile tocLBA read done
cdvdman_locatefile strcmp SLUS_214.19;1
cdvdman_locatefile strcmp SLUS_214.19;1 ☺
cdvdman_locatefile strcmp SLUS_214.19;1 FILELIST.BIN;1
cdvdman_locatefile strcmp SLUS_214.19;1 SYSTEM.CNF;1
cdvdman_locatefile strcmp SLUS_214.19;1 E419C51B
cdvdman_locatefile strcmp SLUS_214.19;1 SLUS_214.19;1
cdvdman_locatefile found file! LBA=278 size=4733144
cdvdman_findfile found \SLUS_214.19;1
open ret=0 lsn=278 size=4733144
cdrom_read size=65536b (32s) file_position=0
sceCdRead lsn=278 sectors=32 sector_size=2048 buf=001c1400
cdvdman_read lsn=278 sectors=32 buf=1c1400
sceCdSync 0 sync flag = 1

usbmass_bd: connect: devId=1
usbmass_bd: Vendor: PNY
usbmass_bd: Product: USB 3.0 FD
usbmass_bd: Revision: PMAP
connecting device usb1p0
usbmass_bd: ERROR: unable to read sector after 16 tries (sector=0x0000000000f77ad8, count=128)
cdrom_read ret=65536
Input ELF format filename = cdrom0:\SLUS_214.19;1
..cdrom_read size=65536b (32s) file_position=65536
sceCdRead lsn=310 sectors=32 sector_size=2048 buf=001d1400
cdvdman_read lsn=310 sectors=32 buf=1d1400
usbmass_bd: ERROR: unable to read sector after 16 tries (sector=BDM: bd_defrag_read: ERROR: read failed!
sceCdSync 0 sync flag = 0
cdrom_read ret=65536
.cdrom_read size=65536b (32s) file_position=131072
sceCdRead lsn=342 sectors=32 sector_size=2048 buf=001c1400
cdvdman_read lsn=342 sectors=32 buf=1c1400
BDM: bd_defrag_read: ERROR: read failed!
sceCdSync 0 sync flag = 0
cdrom_read ret=65536
sceCdRead lsn=374 sectors=32 sector_size=2048 buf=001d1400
0x0000000000f77c58, count=128)
BDM: bd_defrag_read: ERROR: read failed!
sceCdSync 0 sync flag = 0

Not sure if this is the best way to go about it but its the only thing I could get to work, I tried multiple different things like adding a cb to scsi_connect instead of that workaround DelayThread.. a separate thread for ready setting etc etc basically a lot of different things with no success and this finally worked..

It does add a delay which increases boot times slightly which is not ideal I know but during tests it only seemed to iterate one cycle so really it should just be a second or two difference but a full five if only one device is connected so wait cycles could probably be reduced to 2 or 3.

Anyway that is why it is open for review, if there's improvements to be made here let me know or if its just outright the wrong way to go about it; let me know because there's not much point trying to refine it if its just the wrong way to go about solving the issue.
Thanks

fjtrujy · 2024-10-09T06:46:03Z

iop/fs/bdm/src/bdm.c

@@ -47,6 +42,11 @@ void bdm_RegisterCallback(bdm_cb cb)
    }
 }

+void bdm_on_device_ready(int event)


The whole bdm_on_device_ready is not doing anything, should we keep this logic then?

I know it seems pointless, it’s there so I can export the function to cdvdman in OPL; in the exported function I actually signal bdm io sema to start reads.

KrahJohlito/Open-PS2-Loader@c1b44bf

fjtrujy · 2024-10-09T06:46:25Z

iop/usb/usbmass_bd/src/usb_mass.c

@@ -15,7 +15,7 @@
 #include <usbhdfsd-common.h>

 // #define ASYNC
-// #define DEBUG  //comment out this line when not debugging


This line I suppose needs to keep commented

Yes you’re right, this pr isn’t “complete” but more so just to get feedback on the overall changes

AKuHAK · 2024-10-09T08:18:52Z

iop/usb/usbmass_bd/src/usb_mass.c

@@ -705,6 +710,9 @@ static void usb_mass_release(mass_dev *dev)
    dev->bulkEpO   = -1;
    dev->controlEp = -1;
    dev->status    = 0;
+
+    if (devices_initialising > 0) // should handle all errors?
+        devices_initialising--;
 }

 static int usb_mass_disconnect(int devId)


not sure, but is the devId now pointless as we anyway will go through all devIDs?

No I don’t think so but this devices_initialising flag could probably done better with a dev->status flag rather than how I’m doing it currently, just waiting to hear if this method is reasonable or not before I spend more time on it

AKuHAK · 2024-10-09T08:19:45Z

just curious what cb is standing for? :) callback?

KrahJohlito · 2024-10-09T09:42:36Z

just curious what cb is standing for? :) callback?

Yea callback, sorry lol this wasn’t really meant to be merged as is so I used shorthand

fjtrujy · 2024-10-09T10:11:15Z

I'm happy we are around these kinds of improvements, it makes the SDK more friendly and reactive.

However, I'm not even sure what is the issue we are trying to fix, I suppose some kind of race condition when several USBs are connected.

Can we create a dummy sample app located in ps2sdk where with the -ldebug we print on the screen the number of USBs connected and the status?
In this way, we can also check the delay between being connected and being ready, and the other way around the delay after disconnecting it.

I think that some of the logic we have in usbmass_bd could be improved.

What do you think @KrahJohlito .

Cheers

rickgaiser · 2024-10-14T19:48:52Z

However, I'm not even sure what is the issue we are trying to fix, I suppose some kind of race condition when several USBs are connected.

Exactly, I think we need to find out the root cause of the issue first.

The block device (BD) drivers I would like to keep as simple as possible. As soon as they call bdm_connect_bd, the block device should be ready for communication. If it's not then it's an issue inside the block device driver that should be fixed there.

Any reactive events can be fired from BDM (the manager) if needed, but I know that's not possible in the case of OPL ingame, becouse OPL itself acts as a simplified BDM in that case.

add ready cb to bdm

5974ac7

fjtrujy reviewed Oct 9, 2024

View reviewed changes

AKuHAK reviewed Oct 9, 2024

View reviewed changes

KrahJohlito closed this by deleting the head repository Nov 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add ready cb to bdm #668

add ready cb to bdm #668

KrahJohlito commented Oct 8, 2024

fjtrujy Oct 9, 2024

KrahJohlito Oct 9, 2024

fjtrujy Oct 9, 2024 •

edited

Loading

KrahJohlito Oct 9, 2024

AKuHAK Oct 9, 2024

KrahJohlito Oct 9, 2024

AKuHAK commented Oct 9, 2024

KrahJohlito commented Oct 9, 2024 •

edited

Loading

fjtrujy commented Oct 9, 2024

rickgaiser commented Oct 14, 2024

add ready cb to bdm #668

add ready cb to bdm #668

Conversation

KrahJohlito commented Oct 8, 2024

fjtrujy Oct 9, 2024

Choose a reason for hiding this comment

KrahJohlito Oct 9, 2024

Choose a reason for hiding this comment

fjtrujy Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

KrahJohlito Oct 9, 2024

Choose a reason for hiding this comment

AKuHAK Oct 9, 2024

Choose a reason for hiding this comment

KrahJohlito Oct 9, 2024

Choose a reason for hiding this comment

AKuHAK commented Oct 9, 2024

KrahJohlito commented Oct 9, 2024 • edited Loading

fjtrujy commented Oct 9, 2024

rickgaiser commented Oct 14, 2024

fjtrujy Oct 9, 2024 •

edited

Loading

KrahJohlito commented Oct 9, 2024 •

edited

Loading