libftdi Archives

Subject: Re: ftdi_eeprom: setting cha_vcp=false has no effect on generated eeprom binary

From: Thomas Jarosch <thomas.jarosch@xxxxxxxxxxxxx>
To: libftdi@xxxxxxxxxxxxxxxxxxxxxxx
Cc: Holger Mößinger <H.Moessinger@xxxxxxxxx>, Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
Date: Mon, 13 Apr 2020 23:32:47 +0200
Hi Holger,

You wrote on Fri, Feb 07, 2020 at 02:01:04PM +0000:
> I was able to spare some time and looked a bit more into this.
> I found that channel_a_driver config was not really implemented in 
> ftdi_eeprom for Type_R chips.
> Also handling of the type_R high_current_drive parameter was missing or not 
> working.
> I have also added some more information to the ftdi_eeprom/example.conf file 
> and added a --verbose option to get some more output when using the 
> --eeprom-read option.
> 
> The changes are based on commit 0209a3633dc877a577af07d883eb5059e22f6a91 and 
> are currently available here: https://github.com/5inf/libftdi/commits/master
> 
> If there is interest in integrating any of these changes into the official 
> codebase, what is the preferred procedure for sending a patch?

thank you for your patches, I've applied all of them (with minor tweaks).

Regarding the wrong high_current_drive parameter:

I think you unearthed a trove of issues here. There were fixes in this area
in 2015, but nobody understood it's a more generic issue:

**************************
http://developer.intra2net.com/git/?p=libftdi;a=commitdiff;h=3986243d4e7f366001276d7246e7b9822f7e39c4;hp=a9b9673b87526a7ffb583b1f7a299f7639e9a240
commit 3986243d4e7f366001276d7246e7b9822f7e39c4
Refs: v1.2-18-g3986243
Author:     Thilo Schulz <thilo@xxxxxxx>
AuthorDate: Mon Oct 12 19:07:23 2015 +0200

    Fix useless use_usb_version config file option is useless due to incorrect 
checking of boolean flag
**************************

Here's a problematic example from ftdi_eeprom_decode():

    eeprom->in_is_isochronous  = buf[0x0A]&0x01;
    eeprom->out_is_isochronous = buf[0x0A]&0x02;
    eeprom->suspend_pull_downs = buf[0x0A]&0x04;
    eeprom->use_serial         = !!(buf[0x0A] & USE_SERIAL_NUM);
    eeprom->use_usb_version    = !!(buf[0x0A] & USE_USB_VERSION_BIT);
..
    eeprom->high_current_a   = buf[0x00] & HIGH_CURRENT_DRIVE;


I think the decoding of boolean bit fields all need to be changed
to "!!(bit test)", so the in memory value will truely be one or zero.

Then we can apply changes like this everywhere:

-            if ( eeprom->high_current_b == HIGH_CURRENT_DRIVE)
+            if (eeprom->high_current_b)
                 output[0x01] |= HIGH_CURRENT_DRIVE;


Raw list of possible candidates to check (edited already):
**************************
$ grep getbool main.c
    eeprom_set_value(ftdi, SELF_POWERED, cfg_getbool(cfg, "self_powered"));
    eeprom_set_value(ftdi, REMOTE_WAKEUP, cfg_getbool(cfg, "remote_wakeup"));
    eeprom_set_value(ftdi, IN_IS_ISOCHRONOUS, cfg_getbool(cfg, 
"in_is_isochronous"));
    eeprom_set_value(ftdi, OUT_IS_ISOCHRONOUS, cfg_getbool(cfg, 
"out_is_isochronous"));
    eeprom_set_value(ftdi, SUSPEND_PULL_DOWNS, cfg_getbool(cfg, 
"suspend_pull_downs"));
    eeprom_set_value(ftdi, HIGH_CURRENT, cfg_getbool(cfg, "high_current"));
                     cfg_getbool(cfg, "cha_vcp") ? DRIVER_VCP : 0);
                     cfg_getbool(cfg, "chb_vcp") ? DRIVER_VCP : 0);
                     cfg_getbool(cfg, "chc_vcp") ? DRIVER_VCP : 0);
                     cfg_getbool(cfg, "chd_vcp") ? DRIVER_VCP : 0);
    eeprom_set_value(ftdi, CHANNEL_A_RS485, cfg_getbool(cfg, "cha_rs485"));
    eeprom_set_value(ftdi, CHANNEL_B_RS485, cfg_getbool(cfg, "chb_rs485"));
    eeprom_set_value(ftdi, CHANNEL_C_RS485, cfg_getbool(cfg, "chc_rs485"));
    eeprom_set_value(ftdi, CHANNEL_D_RS485, cfg_getbool(cfg, "chd_rs485"));
**************************

Is the analysis above correct?

Cheers,
Thomas

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to libftdi+unsubscribe@xxxxxxxxxxxxxxxxxxxxxxx   

Current Thread