[PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

Florian Preinstorfer
Hi,
I tried to invert the RTS line on a FT231X FTDI chip using ftdi_eeprom
and it did not work out. As it turned out, the rs232_inversion options
are not recognized by ftdi_eeprom. As it seems, rs232_inversion and
invert should accomplish the same thing and even write to the same
location. Therefore, I replaced rs232_inversion with invert.

Patch summary:
 * The field eeprom->rs232_inversion reads and writes to the same
   location as eeprom->invert already does.
 * The configuration values invert_*, as read by ftdi_eeprom, are
   working ootb with FT230X chips. No configuration values were
   available for rs232_inversion.
 * Replace a duplicated code block, that prints inverted bits with
   print_inverted_bits().

I tested this commit with FT231X, which identifies itself as VID 0x0403
and PID 0x6015.

Please find the patch attached.

Best regards,
Florian

0001-Replace-eeprom-rs232_inversion-with-eeprom-invert.patch (6K) Download Attachment
signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

Florian Preinstorfer
Hi,
any comments on the patch or objections against merging?

best regards,
Florian


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

E.S. Rosenberg
Doesn't that mean that an interface users are using changed?

2014-10-22 10:16 GMT+03:00 Florian Preinstorfer <[hidden email]>:
Hi,
any comments on the patch or objections against merging?

best regards,
Florian




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] Replace eeprom->rs232_inversion with eeprom->invert.

Florian Preinstorfer
On 22/10/14 10:51, E.S. Rosenberg wrote:
> Doesn't that mean that an interface users are using changed?
I don't think so. The rs232_inversion options were never (to my
knowledge and based on a few simple tests) processed by ftdi_eeprom.
Both, invert and rs232_inversion, seem to write to the same memory
location, so I thought merging them together might be a suitable option.


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

Uwe Bonnes
In reply to this post by Florian Preinstorfer
>>>>> "Florian" == Florian Preinstorfer <[hidden email]> writes:

    Florian> Hi, any comments on the patch or objections against merging?

Hi Florian,

you are right, eeprom->rs232_inversion is the same as eeprom->invert.
To replacing it is good. However i propose to split up yout patch in two
parts:
- first replacing rs232_inversio vs. invert.
- second introducing the printout function.

This will help regression testing.

Bye
--
Uwe Bonnes                [hidden email]

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------

--
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] Replace eeprom->rs232_inversion with eeprom->invert.

Florian Preinstorfer
> you are right, eeprom->rs232_inversion is the same as eeprom->invert.
> To replacing it is good. However i propose to split up yout patch in two
> parts:
> - first replacing rs232_inversio vs. invert.
> - second introducing the printout function.
>
> This will help regression testing.
Hi Uwe.

Good point, thanks for reviewing the patch. Please find two patches
attached, that split up the rs232_inversion replacement and the printout
function.

best regards,
Florian

0001-Replace-eeprom-rs232_inversion-with-eeprom-invert.patch (3K) Download Attachment
0002-Add-print_inverted_bits-to-replace-a-duplicated-code.patch (3K) Download Attachment
signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re: [PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

Thomas Jarosch
Hi Florian,

On Thursday, 23. October 2014 08:42:31 Florian Preinstorfer wrote:

> > you are right, eeprom->rs232_inversion is the same as eeprom->invert.
> > To replacing it is good. However i propose to split up yout patch in two
> > parts:
> > - first replacing rs232_inversio vs. invert.
> > - second introducing the printout function.
> >
> > This will help regression testing.
>
> Hi Uwe.
>
> Good point, thanks for reviewing the patch. Please find two patches
> attached, that split up the rs232_inversion replacement and the printout
> function.

thanks for splitting the patch and also Uwe for the review.
I'll take a look at them soonish.

[My planning got messed up again: First two shell shock updates and now an
SSLv3 poodle ;) You can't really plan for security issues / upgrades.]

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: Re: [PATCH] Replace eeprom->rs232_inversion with eeprom->invert.

Thomas Jarosch
In reply to this post by Florian Preinstorfer
Hi Florian,

On Thursday, 23. October 2014 08:42:31 Florian Preinstorfer wrote:

> > you are right, eeprom->rs232_inversion is the same as eeprom->invert.
> > To replacing it is good. However i propose to split up yout patch in two
> > parts:
> > - first replacing rs232_inversio vs. invert.
> > - second introducing the printout function.
> >
> > This will help regression testing.
>
> Hi Uwe.
>
> Good point, thanks for reviewing the patch. Please find two patches
> attached, that split up the rs232_inversion replacement and the printout
> function.
both patches have been applied. I've double checked the removal
of the "RS232_INVERSION" eeprom API definition and it was safe to remove it:
It was never part of an official release and also just added on 2014-09-14.

I guess the only one that needs to change his code is Uwe :D

Cheers,
Thomas

signature.asc (188 bytes) Download Attachment
Loading...