Use BDM bd.devNr for port numbers

[rickgaiser]

This PR fixes the use of the block_device devNr field. Whenever possible the devNr should hold a value that is persistent across reboots.

• ATA: master=0, slave=1 (was already implemented)
• USB: 0=first port, 1=second port, 2=hub, fixed in this PR
• MX4SIO: 1=second port, fixed in this PR
• IEEE1394: not implemented (incrementing number)
• UDPBD: not implemented (incrementing number)

The already existing ioctl can be used to query this device number:

ps2sdk/common/include/usbhdfsd-common.h

Lines 38 to 39 in 88fdeb0

	/** Get the device number for the block device backing the mass partition */
	#define USBMASS_IOCTL_GET_DEVICE_NUMBER 0x0006

@DKWDRV I have put this code together for you, to end the dicussion in #757. I only compile tested it. Please check this commit and test if this works for you.

[DKWDRV]

@rickgaiser ps2homebrew/Open-PS2-Loader#1316 should show improvements with this (after reboot part)?
Where can I get compiled irx?

I think the code is mostly fine. sceUsbdGetDeviceLocation should return 0 on success.

I don't think it's a good idea to default to devNr = 2 for other cases of path[0]. In the future if many devices will be registered it would be default to it. The two usb port are considered as "root usb hub" with port nr = 0. The ports are 01 and 02. If a hub is connected to any of the ports it will be the actual hub itself that takes 01 or 02 as device and any other found USB device will be 02 in this case.
I think it's better check for sceUsbdGetDeviceLocation return value and if that is valid either leave path[0] as it is 01 02 or if that causes problems then in the else clause check if the number is higher than 2 and just do number -1?

[DKWDRV]

@rickgaiser I hope #753 gets more attention too! Instead of just samples release having IRX package.

[rickgaiser]

@rickgaiser ps2homebrew/Open-PS2-Loader#1316 should show improvements with this (after reboot part)?

I know it will work for neutrino now but for OPL I don't know. That needs to be tested, but I see OPL using the USBMASS_IOCTL_GET_DEVICE_NUMBER ioctl, and that should return a reboot persistent number after this PR.

Where can I get compiled irx?

We are developers not users: git pull the source code to a separate local branch and compile.

I think the code is mostly fine. sceUsbdGetDeviceLocation should return 0 on success.

I don't think it's a good idea to default to devNr = 2 for other cases of path[0]. In the future if many devices will be registered it would be default to it. The two usb port are considered as "root usb hub" with port nr = 0. The ports are 01 and 02. If a hub is connected to any of the ports it will be the actual hub itself that takes 01 or 02 as device and any other found USB device will be 02 in this case. I think it's better check for sceUsbdGetDeviceLocation return value and if that is valid either leave path[0] as it is 01 02 or if that causes problems then in the else clause check if the number is higher than 2 and just do number -1?

I was thinking about that also. But if the function returns an error, then path should be unchanged and still contain the initialized value path[0] = 0;. Resulting in devNr = 2;, so I removed the return value check but I agree it's nicer to explicitly check the return value and act on it, preventing confusion.

When the usb stick cannot be identified as being in the first or the second port, then using devNr = 2; is a bit oversimplified, I admit. But becouse the usb driver only supports at maximum 2 usb devices it should be ok, as long as not both devices are in a hub. In that case they will both become "usb2" for BDM. But it should still work and they will also become "mass0" and "mass1". The only disadvantage is that they can not be uniquely identified.
A better solution would be to use an incrementing number for all usb drives that are not directly in one of the two ports. So they become "usb2" and "usb3".

If you can try the current code that would be nice. I'll try to make the above changes in the next few days but I don't have much time.

[DKWDRV]

@rickgaiser ps2homebrew/Open-PS2-Loader#1316 should show improvements with this (after reboot part)?

I know it will work for neutrino now but for OPL I don't know. That needs to be tested, but I see OPL using the USBMASS_IOCTL_GET_DEVICE_NUMBER ioctl, and that should return a reboot persistent number after this PR.

Yes this PR should fix that issue. That PR has two problems. Second one was random crashes after IOP reboot related to the device number.

Where can I get compiled irx?

We are developers not users: git pull the source code to a separate local branch and compile.

That seems so much for such a small irx.

I think the code is mostly fine. sceUsbdGetDeviceLocation should return 0 on success.
I don't think it's a good idea to default to devNr = 2 for other cases of path[0]. In the future if many devices will be registered it would be default to it. The two usb port are considered as "root usb hub" with port nr = 0. The ports are 01 and 02. If a hub is connected to any of the ports it will be the actual hub itself that takes 01 or 02 as device and any other found USB device will be 02 in this case. I think it's better check for sceUsbdGetDeviceLocation return value and if that is valid either leave path[0] as it is 01 02 or if that causes problems then in the else clause check if the number is higher than 2 and just do number -1?

I was thinking about that also. But if the function returns an error, then path should be unchanged and still contain the initialized value path[0] = 0;. Resulting in devNr = 2;, so I removed the return value check but I agree it's nicer to explicitly check the return value and act on it, preventing confusion.

Well there is nothing using or supporting hubs out there. Neither opl or DKWDRV support hubs so for now this will do for at least 2 USB devices support.

When the usb stick cannot be identified as being in the first or the second port, then using devNr = 2; is a bit oversimplified, I admit. But becouse the usb driver only supports at maximum 2 usb devices it should be ok, as long as not both devices are in a hub. In that case they will both become "usb2" for BDM. But it should still work and they will also become "mass0" and "mass1". The only disadvantage is that they can not be uniquely identified. A better solution would be to use an incrementing number for all usb drives that are not directly in one of the two ports. So they become "usb2" and "usb3".

I had given deep thoughts to this. My original PR would solve this perfectly. You could keep everything as is, and check the port number for everything without messing with a million device ids. Even with a hub that would still be the same unique port number since the port index number.

If you can try the current code that would be nice. I'll try to make the above changes in the next few days but I don't have much time.

It's ok, just merge it and close #757

[uyjulian]

How about, instead of massX: or trying to make new information about how to pass devNr to next program, what about disk ID/label

e.g. massbyuuid0:/0123-ABCD/yourfile.elf or massbyuuid0:/01234567-89AB-CDEF-0123-456789ABCDEF/yourfile.elf

The same thing linux does with /dev/disk/by-uuid/

[DKWDRV]

How about, instead of massX: or trying to make new information about how to pass devNr to next program, what about disk ID/label

e.g. massbyuuid0:/0123-ABCD/yourfile.elf or massbyuuid0:/01234567-89AB-CDEF-0123-456789ABCDEF/yourfile.elf

The same thing linux does with /dev/disk/by-uuid/

There are many solutions to get multiple support but almost all will break most homebrew relying on "mass0" specifically.

For now this is a good compromise for both OPL and DKWDRV. So merge?

In the future perhaps more can be done.

[rickgaiser]

I tested it and it does not work as expected.

sceUsbdGetDeviceLocation returns:
1:0:0:0... for the right port, I thought that was the second one
2:0:0:0... for the left port, I thought that was the first one

So... it's reversed? How can this be? Is this consistent across all ps2 models?

MC0 is left, MC1 is right. So I expect things to be counted left-to-right.

[DKWDRV]

I tested it and it does not work as expected.

sceUsbdGetDeviceLocation returns: 1:0:0:0... for the right port, I thought that was the second one 2:0:0:0... for the left port, I thought that was the first one

So... it's reversed? How can this be? Is this consistent across all ps2 models?

MC0 is left, MC1 is right. So I expect things to be counted left-to-right.

Ughhhhhhhhhh this will never end. Yes it does so and so it should be. See here
If your trying to make things fit the universal goodlines you will be dissapointed. It's the PS2 :)

Really my PR took care of everything and left everything as is, only the developer had to do extra (but safe check).

[DKWDRV]

Bump @rickgaiser @uyjulian
Can we merge this?

[uyjulian]

Suppress cppcheck unused struct member with // cppcheck-suppress unusedStructMember before the offending lines

[uyjulian]

Lgtm

[rickgaiser]

I was still testing with USB port numbers on FAT model, as they don't have left/right, but top/bottom.

In BDM, the USB numbers are:

• top/left = the first port = "usb0"
• bottom/right = the second port = "usb1"

USBMASS_IOCTL_GET_DEVICE_NUMBER will report these numbers as 0 and 1

This is how it should be, so ready to merge (but it's already merged I see).