Awesome Work, and small updates

bm16ton commented 3 years ago

Hello, first let me say thank you! I really realy enjoyed reading your ftdi reverse breakdown. I am in awe and jealous of your skills. So i too your kernel driver, my first attempt it didnt work failed the check in probe. I immediately went to sniffing a working userland tool (should have tried more with stock code) and found a fair amount of differences, here ill list sum;

const u16 FTDI_BIT_MODE_RESET = 0x00ff; const u16 FTDI_BIT_MODE_MPSSE = 0x02ff;

ret = usb_control_msg(
    ftdi->udev, usb_sndctrlpipe(ftdi->udev, 0),
    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0000,
    /* wIndex =  */0x0000,
    /* data = */NULL,
    /* size = */0,
    ftdi->io_timeout);
if (ret < 0)
    return ret;

ret = usb_control_msg(
    ftdi->udev, usb_sndctrlpipe(ftdi->udev, 0),
    /* bRequest = */0x03,
    /* bRequestType = */0x40,
    /* wValue = */0x4138,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,
    ftdi->io_timeout);
if (ret < 0)
    return ret;

ret = usb_control_msg(
    ftdi->udev, usb_sndctrlpipe(ftdi->udev, 0),
    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0000,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,
    ftdi->io_timeout);
if (ret < 0)
    return ret;

ret = usb_control_msg(
    ftdi->udev, usb_sndctrlpipe(ftdi->udev, 0),
    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0001,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,
    ftdi->io_timeout);
if (ret < 0)
    return ret;

ret = usb_control_msg(
    ftdi->udev, usb_sndctrlpipe(ftdi->udev, 0),
    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0002,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,
    ftdi->io_timeout);
if (ret < 0)
    return ret;

As you can see I had to change the RESET, MPSSE and wIindex also added a baudrate change (I dunno either), and still had to remove the check ftdi_mpsse_verify. I then added smbus support and blocked binding to bank B (Im using ft2232h) and everything is working great! Will add kernel mods for speed and bank A/B options and /sys entries for changing speed etc as well when I find time. I realize you probly already know but in case someone stumbles here;

PURGE RX AND TX BUFFERS

    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0000,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,

PURGE RX BUFFERS

    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0001,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,

PURGE TX BUFFERS

    /* bRequest = */0x00,
    /* bRequestType = */0x40,
    /* wValue = */0x0002,
    /* wIndex =  */0x0001,
    /* data = */NULL,
    /* size = */0,

Ialso get extra 2 bytes on invalid/bad commands i guess an error code. My versions is located here; https://github.com/bm16ton/ft2232-mpsse-i2c-spi-kern-drivers and integrated into my kernel here; https://github.com/bm16ton/yoga-c630-linux-kernel with hopefully more updates coming soon. Is there any reason this couldnt be submitted for mainline kernel? Again great work and thank you!

krinkinmu commented 3 years ago

Glad that you liked it, as to your question

Is there any reason this couldnt be submitted for mainline kernel?

I think there might be a problem with submitting this code to the mainline kernel, but the explanation is a bit long.

The way kernel recognizes what driver handles what USB device is by vendor/product ID (I'm sure there are other ways as well, but this seem to be the main option), so you'd need to have vendor/product ID for your device and driver. And when it comes to vendor/product IDs for FTDI based devices there are a couple of options:

you can just use FTDI vendor/product ID
you can request your own range of vendor/product IDs from USB org and flush the FTDI chip to use those (I don't know about all FTDI chips, but it's definitely an option for some)

Let's first take a look at the second option. The problem with the second option is you have to pay for vendor/product IDs - those are not free. The amount of money might be insignificant for a company that manufactures 100000s of devices, but for individual it's a bit too much and just isn't worth it.

So now to the first option. Let's just reuse FTDI vendor/product ID and submit a mainline driver using those. The problem with this option is that FTDI MPSSE chips are sort of universal. They can operate as a serial interface, as I2C, as SPI and probably a bunch of others. And for many popular FTDI devices that can operate as a serial interface, mainline kernel already has drivers that just expose them as a serial interface.

Case in point is the device you seem to use, see: https://elixir.bootlin.com/linux/v5.12.9/source/drivers/usb/serial/ftdi_sio_ids.h#L24). If you create an I2C or SPI driver bound to the FTDI vendor/product ID, it will just cause a conflict with an already existing driver. So in it's current form submitting the driver to the mainline kernel doesn't seem to be an option.

Then you may ask about adding support for I2C/SPI/etc to the existing kernel driver?

And indeed, in general, in Linux kernel a driver for one physical device can expose different interfaces. However, FTDI, to the best of my knowledge, can only operate in one mode at a time. In other words, when FTDI chip is configured in serial interface mode, it cannot do I2C or SPI. So now you have to be able to detect in what mode a device is now and expose the right interface depending on that.

While it's not an impossible problem to solve, it's tricky and requires a bit of work and it doesn't seem to be worth it.

Manufactures that want to produce I2C/SPI devices based on FTDI chips can just buy vendor/product ID range and use those, instead of going through all the trouble of supporting multiple modes of operation.

And individual entuthiasts who just want to use an FTDI device they have in I2C/SPI mode is a bit too niche of use-case to warrant adding complexity to the kernel. Besides, enthusiasts can just flush FTDI device with whatever vendor/product ID they want, as long as it doesn't conflict with anything else in the system, and use their custom driver for that.

That's all only my opinion though, so if you want you might ask this question in the kernel mailing list.

bm16ton commented 3 years ago

I see, the ftdi-sio driver has support already for not binding to bank A on boards with specific product I'd. I added my own product string to the list and updated the eeprom to match, and then made the i2c driver only bind to bank 1. I haven't seen anyone else use this technique so its probly something the kernel devs dont like. I suppose not updating the eeprom and reusing one of the already included "white listed" ones may work. Well hell as long as I have your permission and no weird licensing things I dont know know about, I'll try and finish it up to a full featured driver. Never submitted to kernel before directly but I'll do whatever it takes to make sure you get the credit for your amazing work.

On Wed, Jun 9, 2021, 4:50 PM Krinkin, Mike @.***> wrote:

Glad that you liked it, as to your question

Is there any reason this couldnt be submitted for mainline kernel?

I think there might be a problem with submitting this code to the mainline kernel, but the explanation is a bit long.

The way kernel recognizes what driver handles what USB device is by vendor/product ID (I'm sure there are other ways as well, but this seem to be the main option), so you'd need to have vendor/product ID for your device and driver. And when it comes to vendor/product IDs for FTDI based devices there are a couple of options:

you can just use FTDI vendor/product ID

you can request your own range of vendor/product IDs from USB org and flush the FTDI chip to use those (I don't know about all FTDI chips, but it's definitely an option for some)

Let's first take a look at the second option. The problem with the second option is you have to pay for vendor/product IDs - those are not free. The amount of money might be insignificant for a company that manufactures 100000s of devices, but for individual it's a bit too much and just isn't worth it.

So now to the first option. Let's just reuse FTDI vendor/product ID and submit a mainline driver using those. The problem with this option is that FTDI MPSSE chips are sort of universal. They can operate as a serial interface, as I2C, as SPI and probably a bunch of others. And for many popular FTDI devices that can operate as a serial interface, mainline kernel already has drivers that just expose them as a serial interface.

Case in point is the device you seem to use, see: https://elixir.bootlin.com/linux/v5.12.9/source/drivers/usb/serial/ftdi_sio_ids.h#L24). If you create an I2C or SPI driver bound to the FTDI vendor/product ID, it will just cause a conflict with an already existing driver. So in it's current form submitting the driver to the mainline kernel doesn't seem to be an option.

Then you may ask about adding support for I2C/SPI/etc to the existing kernel driver?

And indeed, in general, in Linux kernel a driver for one physical device can expose different interfaces. However, FTDI, to the best of my knowledge, can only operate in one mode at a time. In other words, when FTDI chip is configured in serial interface mode, it cannot do I2C or SPI. So now you have to be able to detect in what mode a device is now and expose the right interface depending on that.

While it's not an impossible problem to solve, it's tricky and requires a bit of work and it doesn't seem to be worth it.

Manufactures that want to produce I2C/SPI devices based on FTDI chips can just buy vendor/product ID range and use those, instead of going through all the trouble of supporting multiple modes of operation.

And individual entuthiasts who just want to use an FTDI device they have in I2C/SPI mode is a bit too niche of use-case to warrant adding complexity to the kernel. Besides, enthusiasts can just flush FTDI device with whatever vendor/product ID they want, as long as it doesn't conflict with anything else in the system, and use their custom driver for that.

That's all only my opinion though, so if you want you might ask this question in the kernel mailing list.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/krinkinmu/bootlin/issues/1#issuecomment-858090752, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAWMP3DNS7AQQRRPUWDSLLTR7HXZANCNFSM46MWYCWA .

krinkinmu / bootlin

Awesome Work, and small updates #1