Re: [PATCH 1/2] tty: add bits to manage multidrop mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18/04/2017 17:37, Rodolfo Giometti wrote:
> On 04/18/17 11:21, Richard Genoud wrote:
>> On 14/04/2017 16:28, Rodolfo Giometti wrote:
>>> On 04/14/17 16:24, Richard Genoud wrote:
>>>> On 13/04/2017 17:12, Rodolfo Giometti wrote:
>>>>> On 04/13/17 16:53, Richard Genoud wrote:
>>>>>> On 13/04/2017 14:19, Rodolfo Giometti wrote:
>>>>>>> On 04/13/17 12:43, Richard Genoud wrote:
>>>>>>>> On 11/04/2017 22:12, Rodolfo Giometti wrote:
>>>>>>>>> Multidrop mode differentiates the data characters and the address
>>>>>>>>> characters. Data is transmitted with the parity bit to 0 and
>>>>>>>>> addresses
>>>>>>>>> are transmitted with the parity bit to 1. However this usually
>>>>>>>>> slow
>>>>>>>>> down communication by adding a delay between the first byte and
>>>>>>>>> the
>>>>>>>>> others.
>>>>>>>>>
>>>>>>>>> This patch defines two non-stadard bits PARMD (that enables
>>>>>>>>> multidrop)
>>>>>>>>> and SENDA (that marks the next transmitted byte as address)
>>>>>>>>> that can
>>>>>>>>> be used to completely remove the delay during transmission by
>>>>>>>>> correctly managing the parity bit generation in hardware.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Rodolfo Giometti <giometti@xxxxxxxx>
>>>>>>>>> ---
>>>>>>>>>  include/linux/tty.h                 | 2 ++
>>>>>>>>>  include/uapi/asm-generic/termbits.h | 2 ++
>>>>>>>>>  2 files changed, 4 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/include/linux/tty.h b/include/linux/tty.h
>>>>>>>>> index 83b264c..85238ff 100644
>>>>>>>>> --- a/include/linux/tty.h
>>>>>>>>> +++ b/include/linux/tty.h
>>>>>>>>> @@ -166,6 +166,8 @@ struct tty_bufhead {
>>>>>>>>>  #define C_CIBAUD(tty)    _C_FLAG((tty), CIBAUD)
>>>>>>>>>  #define C_CRTSCTS(tty)    _C_FLAG((tty), CRTSCTS)
>>>>>>>>>  #define C_CMSPAR(tty)    _C_FLAG((tty), CMSPAR)
>>>>>>>>> +#define C_PARMD(tty)    _C_FLAG((tty), PARMD)
>>>>>>>>> +#define C_SENDA(tty)    _C_FLAG((tty), SENDA)
>>>>>>>>>
>>>>>>>>>  #define L_ISIG(tty)    _L_FLAG((tty), ISIG)
>>>>>>>>>  #define L_ICANON(tty)    _L_FLAG((tty), ICANON)
>>>>>>>>> diff --git a/include/uapi/asm-generic/termbits.h
>>>>>>>>> b/include/uapi/asm-generic/termbits.h
>>>>>>>>> index 232b478..7e82859 100644
>>>>>>>>> --- a/include/uapi/asm-generic/termbits.h
>>>>>>>>> +++ b/include/uapi/asm-generic/termbits.h
>>>>>>>>> @@ -140,6 +140,8 @@ struct ktermios {
>>>>>>>>>  #define HUPCL    0002000
>>>>>>>>>  #define CLOCAL    0004000
>>>>>>>>>  #define CBAUDEX 0010000
>>>>>>>>> +#define PARMD   0100000
>>>>>>>>> +#define SENDA   0200000
>>>>>>>>>  #define    BOTHER 0010000
>>>>>>>>>  #define    B57600 0010001
>>>>>>>>>  #define   B115200 0010002
>>>>>>>>>
>>>>>>>> I guess there's a problem with SENDA (bit 16)
>>>>>>>> It's overlapping with CIBAUD (002003600000 (bits 28,19,18,17,16))
>>>>>>>
>>>>>>> You're right! In this case which bit should I use for SENDA? :(
>>>>>>>
>>>>>>> Since it is referred to output data maybe can we move it into
>>>>>>> c_oflag
>>>>>>> bits?
>>>>>>
>>>>>> Well, bits 23 and 24 are free in asm-generic/c_cflag, and since
>>>>>> multidrop uses the parity bit, I'd say it fits better in c_cflag
>>>>>> than in
>>>>>> c_oflag.
>>>>>
>>>>> I see, but while PARMD defines a special parity usage the SENDA bit is
>>>>> dedicated to output data only... however if you prefer I use bits
>>>>> 23 and
>>>>> 24 it's OK for me! :-)
>>>>>
>>>>>> But honestly, I find the SENDA bit logic disturbing and not compliant
>>>>>> with the way tc{g,s}etattr() works.
>>>>>>
>>>>>> Because, usually, a tcsetattr() call is followed by a tcgetattr()
>>>>>> call
>>>>>> to check if everything went well. (tcsetattr return 0 if *any* of the
>>>>>> requested change succeeded).
>>>>>> So, with the proposed implementation, we will have
>>>>>
>>>>>>     struct termios term, check;
>>>>>>
>>>>>>     ret = tcgetattr(fd, &term);
>>>>>>     if (ret < 0) {
>>>>>>         perror("tcgetattr");
>>>>>>         exit(-1);
>>>>>>     }
>>>>>>     term.c_cflag |= PARENB | CMSPAR | PARMD | SENDA;
>>>>>>     ret = tcsetattr(fd, TCSADRAIN, &term);
>>>>>>     if (ret < 0) {
>>>>>>         perror("tcsetattr");
>>>>>>         exit(-1);
>>>>>>     }
>>>>>>     ret = tcgetattr(fd, &check);
>>>>>>     if (term.c_cflag != check.c_cflag) {
>>>>>>         /* fail to set all c_cflags */
>>>>>>         exit(-1);
>>>>>>     }
>>>>>> => it will always fail.
>>>>>
>>>>> This should be another reason because we should use c_oflag instead of
>>>>> c_cflag! :-P
>>>> Actually it isn't, I gave the example of c_cflag, but it's the same
>>>> with
>>>> c_{i,o,l}flag
>>>
>>> I see... so any advice about how to manage SENDA bit in order to avoid
>>> this drawback?
>>
>> I'm not a tty guru, but I think the proper way to implement this would
>> be with a new line discipline.
> 
> Gosh! A new line discipline for a special parity settings? =8-o
> 
> In any case there are no methods to directly manage tty parity settings
> within a line discipline. The developer still needs to use the
> tcsetattr() to set the PARMD parity mode.
> 
> Maybe we can solve the problem by keeping the multidrop settings within
> the tcsetattr()+PARMD and then using something similar to the
> send_break() mechanism called with the TCSBRK ioctl() command. We can
> add a TCSSENDA ioctl() commands which in turn calls a send_md_address()
> function implemented as follow:
> 
> static int send_md_address(struct tty_struct *tty, unsigned int bytes)
> {
>         int retval;
> 
>         if (tty->ops->senda_ctl == NULL)
>                 return 0;
> 
>         return tty->ops->senda_ctl(tty, bytes);
> }
> 
> Where the senda_ctl (one per serial driver) do the job! The bytes
> parameters is just because we may have more than one address bytes...
> 
> Do you think that this could be a reasonable solution?

As I'm not familiar with the multidrop, I read a little bit about it here :
https://www.namanow.org/images/pdfs/technology/mdb_version_4-2.pdf
(p29-32)

And I'm starting to understand the whole problem here.

Correct me if I'm wrong, but as I understood it, multidrop is a
messaging protocol (like i2c or modbus) used on RS485 to address only
one device on the bus.
The "mode bit" is used to differentiate the address/command byte from
the data/checksum
Now, this mode bit can be implemented by using a 9bit data mode (but
this mode is not supported by tcsetattr())
And it can also be implemented by using the parity bit, leading to quite
a lot of tcsetattr() calls (2 by message).

This mode bit is also used in the response to indicate the last byte
sent from the slave to the master.

So, with 9bit mode, multidrop protocol could be fully handled in user
space (no need to call tcsetattr() twice per message, and the mode bit
could be tested in the slave response !). But 9bit mode is not supported
by tcsetattr().

With parity bit, mode bit can be emulated for sending a block, and it
can also be reconstructed when reading a block by setting IGNPAR=0 and
PARMRK=1 (but it's a pain !).

The 3rd method is the use of the multidrop mode of the serial controller
(what you are proposing).
For Atmel controllers, when this mode is enabled, the parity bit is used
as a mode bit.
So, before transmitting a message, the SENDA bit should be set, and
then, the transmitter will set the mode bit for the next byte, leaving
it at 0 for all the following ones.

When receiving, the parity error interrupt will only be fired if the
mode bit is high (meaning that a whole block has been received).


The first question I would ask is do we want this protocol to be handled
in the kernel or in user-space ?

If it's in kernel, it may be something like the I2C protocol, the
attached devices (coin device, bill validator, display or whatever)
would also have a kernel drivel, with a right interface.
The mode bit will directly be handled in the kernel, using one of the 3
methods described above, depending of the underlying controller
capabilities.

If it's in user-space, there's already a solution with MARK/SPACE parity
bit and parity bit reconstruction with PARMRK (not the easiest way, but
it can work with every serial controller).
Adding the 9 data-bits support would give another solution. (but the
controller would have to support 9bits mode, and dealing with 9bit data
buffer may be not so fun).

And there's the solution you propose, with a PARMD parity bit.
It will also change the behaviour of PARMRK / IGNPAR since the notion of
parity error would actually be "end of block reception".

And do you know if some other controllers implement this multidrop mode ?
Because this solution seems to be Atmel specific.

Regards,
Richard






--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux