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

[Nicolas Ferre <nicolas.ferre@atmel.com>]

On 11/14/2011 01:37 AM, 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.

Darron, Claudio,

This explanation makes sense. Thanks for this.

Bye,


>>> 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
>>
> 
> 


-- 
Nicolas Ferre