Re: [PATCH v2] usb: serial: Repair FTDI FT232R bricked eeprom

From: James Hilliard
Date: Wed Sep 09 2020 - 23:46:48 EST


On Wed, Sep 9, 2020 at 9:17 PM Hector Martin "marcan"
<hector@xxxxxxxxxxxxxx> wrote:
>
>
>
> On September 10, 2020 12:02:34 PM GMT+09:00, Oliver Neukum <oneukum@xxxxxxx> wrote:
> >Am Mittwoch, den 09.09.2020, 13:34 -0600 schrieb James Hilliard:
> >> This patch detects and reverses the effects of the malicious FTDI
> >> Windows driver version 2.12.00(FTDIgate).
> >
> >Hi,
> >
> >this raises questions.
> >Should we do this unconditionally without asking?
> >Does this belong into kernel space?
>
> I agree; this is very cute, but does it really need to be an automatic Linux feature? Presumably someone looking to fix a bricked FTDI chip can just run my script, and those who just want to use those chips with Linux already can since the driver binds to the zero PID.
Well for one your script is not easily useable with embedded platforms
like mine where I ran into this issue, I have no python2 interpreter
available in my production builds.
>
> I am deeply amused by the idea of Linux automatically fixing problems caused by malicious Windows drivers, but thinking objectively, I'm not sure if that's the right thing to do.
>From my understanding Linux fixing up hardware issues caused
by faulty/weird Windows drivers isn't exactly unusual.
>
> >
> >> +static int ftdi_repair_brick(struct usb_serial_port *port)
> >> +{
> >> + struct ftdi_private *priv = usb_get_serial_port_data(port);
> >> + int orig_latency;
> >> + int rv;
> >> + u16 *eeprom_data;
> >> + u16 checksum;
> >> + int eeprom_size;
> >> + int result;
> >> +
> >> + switch (priv->chip_type) {
> >> + case FT232RL:
> >> + eeprom_size = 0x40;
> >> + break;
> >> + default:
> >> + /* Unsupported for brick repair */
> >> + return 0;
> >> + }
> >> +
> >> + /* Latency timer needs to be 0x77 to unlock EEPROM programming */
> >> + if (priv->latency != 0x77) {
> >> + orig_latency = priv->latency;
> >> + priv->latency = 0x77;
> >> + rv = write_latency_timer(port);
> >> + priv->latency = orig_latency;
> >> + if (rv < 0)
> >> + return -EIO;
> >> + }
> >
> >Do you really want to change this without returning to the original?
> >
> > Regards
> > Oliver
>
> --
> Hector Martin "marcan" (hector@xxxxxxxxxxxxxx)
> Public key: https://mrcn.st/pub