[PATCH] RS485: fix inconsistencies in the meaning of some variables

[darron@griffin.net (Darron Black)]

On 11/13/2011 03:53 PM, Wolfram Sang wrote:
> Hi,
>
> I have been working on a patch series which adds hardware RS485 to the 8250
> according to the latest developments. The series will be posted tomorrow after
> some more tests. However, there is one thing I wondered about:
>
>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
> probably break even more users?)
It allows the application to configure RTS to not toggle at all in one 
of those two scenarios.

Perhaps the RTS toggle after transmit delay needs to be large, and 
they'd rather do it in userspace than block in the driver. I also recall 
a protocol that would send a master assertion command and hold on to RTS 
afterwards. I can easily imagine needing to quickly transmit something, 
hold on to RTS for a while, then finish your transmit later.

However, I don't have any concrete examples of needing this outside that 
vague recollection of a master assertion sequence in an old embedded 
platform far away from Linux.

>> diff --git a/include/linux/serial.h b/include/linux/serial.h
>> index 97ff8e2..3d86517 100644
>> --- a/include/linux/serial.h
>> +++ b/include/linux/serial.h
>> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>>
>>   struct serial_rs485 {
>>   	__u32	flags;			/* RS485 feature flags */
>> -#define SER_RS485_ENABLED		(1<<  0)
>> -#define SER_RS485_RTS_ON_SEND		(1<<  1)
>> -#define SER_RS485_RTS_AFTER_SEND	(1<<  2)
>> -#define SER_RS485_RTS_BEFORE_SEND	(1<<  3)
>> +#define SER_RS485_ENABLED		(1<<  0)	/* If enabled */
>> +#define SER_RS485_RTS_ON_SEND		(1<<  1)	/* Logical level for
>> +							   RTS pin when
>> +							   sending */
>> +#define SER_RS485_RTS_AFTER_SEND	(1<<  2)	/* Logical level for
>> +							   RTS pin after sent*/
> Nit: 80 char should be broken here, because that is not readable. Or put the
> comment above the define.
>
> Thanks,
>
>     Wolfram
>