Quantcast

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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]  

Loading...