Patch to include arbitrary user data to be added to the EEPROM

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Patch to include arbitrary user data to be added to the EEPROM

Salvador Eduardo Tropea
Hi!

I'm trying to include some data inside the EEPROM generated by
ftdi_eeprom tool.
I modified the ftdi_eeprom tool and libftdi to:

1) Support a "--build-eeprom" command line option that just generates
the EEPROM image (only flashed if --flash-eeprom is provided)
2) Support two configuration options:
- user_data_addr  an integer indicating the offset where we want to put
the user provided data.
- user_data_file a string indicating the filename that contains the
binary data to be added.
3) Extended libftdi API to store the above mentioned data inside the
eeprom struct.
4) Extended ftdi_eeprom_build to include this data.

I'm attaching the output of "git diff".
Is this patch acceptable?

Regards, SET

PS: I fixed a couple of computations that assumed the EEPROM size is 128.

--
Ing. Salvador Eduardo Tropea          http://utic.inti.gob.ar/
INTI - Micro y Nanoelectrónica (CMNB) http://www.inti.gob.ar/
Unidad Técnica Sistemas Inteligentes  Av. General Paz 5445
Tel: (+54 11) 4724 6300 ext. 6919     San Martín - B1650KNA
FAX: (+54 11) 4754 5194               Buenos Aires * Argentina






--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

user-data.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Patch to include arbitrary user data to be added to the EEPROM

Thomas Jarosch
Hi SET,

Am 22.10.2015 um 21:11 schrieb Salvador Eduardo Tropea:

> Hi!
>
> I'm trying to include some data inside the EEPROM generated by ftdi_eeprom tool.
> I modified the ftdi_eeprom tool and libftdi to:
>
> 1) Support a "--build-eeprom" command line option that just generates the EEPROM image (only flashed if --flash-eeprom is provided)
> 2) Support two configuration options:
> - user_data_addr  an integer indicating the offset where we want to put the user provided data.
> - user_data_file a string indicating the filename that contains the binary data to be added.
> 3) Extended libftdi API to store the above mentioned data inside the eeprom struct.
> 4) Extended ftdi_eeprom_build to include this data.
>
> I'm attaching the output of "git diff".
> Is this patch acceptable?
>
> Regards, SET
>
> PS: I fixed a couple of computations that assumed the EEPROM size is 128.

thanks for your patch! It looks quite good and
just needs a few minor tweaks:

- ftdi_eeprom: Warn the user if the supplied user data
  is too big for the storage area. Right now we silently truncate it.

- Do we need to hardcode the hex offsets in "free_start"?
  Could we also do "free_start += 1;" and so on?

- Hardcoding of 128 byte limit again in this line:

  if (eeprom->size>128) user_area_size+=eeprom->size-128;

  ?

  Would be good to add a comment here that explains why
  it's there.


- Why not ftdi_error_return() in dangerous cases like this:

  fprintf(stderr,"Warning, user data overlaps the strings area!\n");

  ?

- ftdi_set_eeprom_user_buf() -> rename to "ftdi_set_eeprom_user_data()"


Let me know what you think.

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|

Re: Patch to include arbitrary user data to be added to the EEPROM

Salvador Eduardo Tropea
Hi Thomas:

El 02/11/15 a las 19:01, Thomas Jarosch escibió:

> Hi SET,
>
> Am 22.10.2015 um 21:11 schrieb Salvador Eduardo Tropea:
>> Hi!
>>
>> I'm trying to include some data inside the EEPROM generated by ftdi_eeprom tool.
>> I modified the ftdi_eeprom tool and libftdi to:
>>
>> 1) Support a "--build-eeprom" command line option that just generates the EEPROM image (only flashed if --flash-eeprom is provided)
>> 2) Support two configuration options:
>> - user_data_addr  an integer indicating the offset where we want to put the user provided data.
>> - user_data_file a string indicating the filename that contains the binary data to be added.
>> 3) Extended libftdi API to store the above mentioned data inside the eeprom struct.
>> 4) Extended ftdi_eeprom_build to include this data.
>>
>> I'm attaching the output of "git diff".
>> Is this patch acceptable?
>>
>> Regards, SET
>>
>> PS: I fixed a couple of computations that assumed the EEPROM size is 128.
> thanks for your patch! It looks quite good and
> just needs a few minor tweaks:
>
> - ftdi_eeprom: Warn the user if the supplied user data
>    is too big for the storage area. Right now we silently truncate it.
Ok, I added a warning about it.

> - Do we need to hardcode the hex offsets in "free_start"?
>    Could we also do "free_start += 1;" and so on?

Hmmm ... not sure. I just added this value after the last index used for
each part.
Now I moved it to a switch/case adjusting for each part.

>
> - Hardcoding of 128 byte limit again in this line:
>
>    if (eeprom->size>128) user_area_size+=eeprom->size-128;
>
>    ?
>
>    Would be good to add a comment here that explains why
>    it's there.

I think it was wrong. I eliminated it.


>
>
> - Why not ftdi_error_return() in dangerous cases like this:
>
>    fprintf(stderr,"Warning, user data overlaps the strings area!\n");
>
>    ?

Because the user could be doing it on purpose ;-)
Suppose the user knows that ftdi_eeprom lacks some obscure feature and
wants to overwrite an address that is supposed to be properly filled by
ftdi_eeprom.


> - ftdi_set_eeprom_user_buf() -> rename to "ftdi_set_eeprom_user_data()"

Ok!

I'm attaching a patch with the above changes.

Regards, SET

--
Ing. Salvador Eduardo Tropea          http://utic.inti.gob.ar/
INTI - Micro y Nanoelectrónica (CMNB) http://www.inti.gob.ar/
Unidad Técnica Sistemas Inteligentes  Av. General Paz 5445
Tel: (+54 11) 4724 6300 ext. 6919     San Martín - B1650KNA
FAX: (+54 11) 4754 5194               Buenos Aires * Argentina






--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

user_data.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Patch to include arbitrary user data to be added to the EEPROM

Thomas Jarosch
Hi Set,

Am 03.11.2015 um 16:43 schrieb Salvador Eduardo Tropea:
> I'm attaching a patch with the above changes.

patch applied, thanks! Finally :)

I've split the --build-eeprom support into an own commit
and also massaged the commit messages of both commits.

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|

Re: Patch to include arbitrary user data to be added to the EEPROM

Salvador Eduardo Tropea
Thanks!!

El 20/11/15 a las 19:32, Thomas Jarosch escibió:

> Hi Set,
>
> Am 03.11.2015 um 16:43 schrieb Salvador Eduardo Tropea:
>> I'm attaching a patch with the above changes.
> patch applied, thanks! Finally :)
>
> I've split the --build-eeprom support into an own commit
> and also massaged the commit messages of both commits.
>
>

--
Ing. Salvador Eduardo Tropea          http://utic.inti.gob.ar/
INTI - Micro y Nanoelectrónica (CMNB) http://www.inti.gob.ar/
Unidad Técnica Sistemas Inteligentes  Av. General Paz 5445
Tel: (+54 11) 4724 6300 ext. 6919     San Martín - B1650KNA
FAX: (+54 11) 4754 5194               Buenos Aires * Argentina





--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]