rts-gpio DT binding

rts-gpio DT binding

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

Hi Mark,

I'm looking at the omap-serial driver and saw that you added rts-gpio
binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
actual RTS signals.

Instead of adding that extra GPIO handling, why didn't you just mux
those signals as RTS and enable auto-RTS/auto-CTS feature ?

It looks to me like that's highly unnecessary binding.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: rts-gpio DT binding

[Mark Jackson]

On 18/03/14 16:55, Felipe Balbi wrote:
> Hi Mark,
> 
> I'm looking at the omap-serial driver and saw that you added rts-gpio
> binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
> RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
> actual RTS signals.
> 
> Instead of adding that extra GPIO handling, why didn't you just mux
> those signals as RTS and enable auto-RTS/auto-CTS feature ?
> 
> It looks to me like that's highly unnecessary binding.

I agree !!

I think it was to allow delays pre- and post- sending the comms data.

Several RS485 drivers require a "warm up" time before they will
transmit data correctly, and also need a "cool down" time to prevent
clipping of the last few bits of data.

IIRC the built-in RTS handling did not allow for this.

It also allows the RTS signal to be "inverted", again required for
some RS485 driver chips.

However, I'll dig into why I did this and report back.

Mark J.

Re: rts-gpio DT binding

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

On Tue, Mar 18, 2014 at 05:04:47PM +0000, Mark Jackson wrote:
> On 18/03/14 16:55, Felipe Balbi wrote:
> > Hi Mark,
> > 
> > I'm looking at the omap-serial driver and saw that you added rts-gpio
> > binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
> > RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
> > actual RTS signals.
> > 
> > Instead of adding that extra GPIO handling, why didn't you just mux
> > those signals as RTS and enable auto-RTS/auto-CTS feature ?
> > 
> > It looks to me like that's highly unnecessary binding.
> 
> I agree !!
> 
> I think it was to allow delays pre- and post- sending the comms data.
> 
> Several RS485 drivers require a "warm up" time before they will
> transmit data correctly, and also need a "cool down" time to prevent
> clipping of the last few bits of data.
> 
> IIRC the built-in RTS handling did not allow for this.

you might be right here. I can't find anywhere to write rts delays.
Weird... digging TRM.

> It also allows the RTS signal to be "inverted", again required for
> some RS485 driver chips.
> 
> However, I'll dig into why I did this and report back.

cool, thanks

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: rts-gpio DT binding

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

hi,

On Tue, Mar 18, 2014 at 12:11:10PM -0500, Felipe Balbi wrote:
> On Tue, Mar 18, 2014 at 05:04:47PM +0000, Mark Jackson wrote:
> > On 18/03/14 16:55, Felipe Balbi wrote:
> > > Hi Mark,
> > > 
> > > I'm looking at the omap-serial driver and saw that you added rts-gpio
> > > binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
> > > RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
> > > actual RTS signals.
> > > 
> > > Instead of adding that extra GPIO handling, why didn't you just mux
> > > those signals as RTS and enable auto-RTS/auto-CTS feature ?
> > > 
> > > It looks to me like that's highly unnecessary binding.
> > 
> > I agree !!
> > 
> > I think it was to allow delays pre- and post- sending the comms data.
> > 
> > Several RS485 drivers require a "warm up" time before they will
> > transmit data correctly, and also need a "cool down" time to prevent
> > clipping of the last few bits of data.
> > 
> > IIRC the built-in RTS handling did not allow for this.
> 
> you might be right here. I can't find anywhere to write rts delays.
> Weird... digging TRM.

hmm, you can change RTS value manually if you don't use auto-RTS. Just
write MCR BIT(1) to toggle that line.

The "warm up" time you mention, however, can be coped with by means of
RX FIFO trigger level. TRM mentions that far end could send another byte
after RTS has been deasserted. So if we set RX trigger level to always
have room for one extra byte, then we will never loose data and the
"warm up" time is probably negligible.

Am I missing something ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: rts-gpio DT binding

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

Hi,

On Tue, Mar 18, 2014 at 12:18:41PM -0500, Felipe Balbi wrote:
> hi,
> 
> On Tue, Mar 18, 2014 at 12:11:10PM -0500, Felipe Balbi wrote:
> > On Tue, Mar 18, 2014 at 05:04:47PM +0000, Mark Jackson wrote:
> > > On 18/03/14 16:55, Felipe Balbi wrote:
> > > > Hi Mark,
> > > > 
> > > > I'm looking at the omap-serial driver and saw that you added rts-gpio
> > > > binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
> > > > RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
> > > > actual RTS signals.
> > > > 
> > > > Instead of adding that extra GPIO handling, why didn't you just mux
> > > > those signals as RTS and enable auto-RTS/auto-CTS feature ?
> > > > 
> > > > It looks to me like that's highly unnecessary binding.
> > > 
> > > I agree !!
> > > 
> > > I think it was to allow delays pre- and post- sending the comms data.
> > > 
> > > Several RS485 drivers require a "warm up" time before they will
> > > transmit data correctly, and also need a "cool down" time to prevent
> > > clipping of the last few bits of data.
> > > 
> > > IIRC the built-in RTS handling did not allow for this.
> > 
> > you might be right here. I can't find anywhere to write rts delays.
> > Weird... digging TRM.
> 
> hmm, you can change RTS value manually if you don't use auto-RTS. Just
> write MCR BIT(1) to toggle that line.
> 
> The "warm up" time you mention, however, can be coped with by means of
> RX FIFO trigger level. TRM mentions that far end could send another byte
> after RTS has been deasserted. So if we set RX trigger level to always
> have room for one extra byte, then we will never loose data and the
> "warm up" time is probably negligible.
> 
> Am I missing something ?

another small gem hidden in the TRM:

	"You must ensure that TCR[3:0] > TCR[7:4], whenever auto-RTS or
	software flow control is enabled to avoid a misoperation of the
	device. In FIFO interrupt mode with flow control, you have to
	also ensure that the trigger level to HALT transmission is
	greater or equal to receive FIFO trigger level (either TLR[7:4]
	or FCR[7:6]); otherwise, FIFO operation stalls."

so there are quite a few things needed to get this working properly.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: rts-gpio DT binding

[Mark Jackson]

On 18/03/14 17:11, Felipe Balbi wrote:
> On Tue, Mar 18, 2014 at 05:04:47PM +0000, Mark Jackson wrote:
>> On 18/03/14 16:55, Felipe Balbi wrote:
>>> Hi Mark,
>>>
>>> I'm looking at the omap-serial driver and saw that you added rts-gpio
>>> binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
>>> RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
>>> actual RTS signals.
>>>
>>> Instead of adding that extra GPIO handling, why didn't you just mux
>>> those signals as RTS and enable auto-RTS/auto-CTS feature ?
>>>
>>> It looks to me like that's highly unnecessary binding.
>>
>> I agree !!
>>
>> I think it was to allow delays pre- and post- sending the comms data.
>>
>> Several RS485 drivers require a "warm up" time before they will
>> transmit data correctly, and also need a "cool down" time to prevent
>> clipping of the last few bits of data.
>>
>> IIRC the built-in RTS handling did not allow for this.
> 
> you might be right here. I can't find anywhere to write rts delays.
> Weird... digging TRM.
> 
>> It also allows the RTS signal to be "inverted", again required for
>> some RS485 driver chips.
>>
>> However, I'll dig into why I did this and report back.
> 
> cool, thanks

Okay ... it comes back to me now.

When using RS485 drivers, we're not actually using RTS as a "Ready
To Send", we're really using it as an "enable RS485 driver".

I just used the "RTS" mnemonic as "we're now wanting to send some
data so please now enable the RS485 driver", rather than the normal
"I'm ready for your to send me some data".

So it's the opposite function.

Maybe it was a poor choice of abbreviation ?

As I said before, RS485 drivers might have active or active low
enables, so we might need to invert the "RTS" polarity.  This is
not handled by the hardware RTS signal.

Is that a good enough explanation ?

Mark J.

Re: rts-gpio DT binding

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]

Hi,

On Wed, Mar 19, 2014 at 09:15:03AM +0000, Mark Jackson wrote:
> On 18/03/14 17:11, Felipe Balbi wrote:
> > On Tue, Mar 18, 2014 at 05:04:47PM +0000, Mark Jackson wrote:
> >> On 18/03/14 16:55, Felipe Balbi wrote:
> >>> Hi Mark,
> >>>
> >>> I'm looking at the omap-serial driver and saw that you added rts-gpio
> >>> binding in commit 4a0ac0f55b18dc297a87a85417fcf068658bf103 (OMAP: add
> >>> RS485 support) but, as it turns out, gpio0_13 and gpio2_15 are both
> >>> actual RTS signals.
> >>>
> >>> Instead of adding that extra GPIO handling, why didn't you just mux
> >>> those signals as RTS and enable auto-RTS/auto-CTS feature ?
> >>>
> >>> It looks to me like that's highly unnecessary binding.
> >>
> >> I agree !!
> >>
> >> I think it was to allow delays pre- and post- sending the comms data.
> >>
> >> Several RS485 drivers require a "warm up" time before they will
> >> transmit data correctly, and also need a "cool down" time to prevent
> >> clipping of the last few bits of data.
> >>
> >> IIRC the built-in RTS handling did not allow for this.
> > 
> > you might be right here. I can't find anywhere to write rts delays.
> > Weird... digging TRM.
> > 
> >> It also allows the RTS signal to be "inverted", again required for
> >> some RS485 driver chips.
> >>
> >> However, I'll dig into why I did this and report back.
> > 
> > cool, thanks
> 
> Okay ... it comes back to me now.
> 
> When using RS485 drivers, we're not actually using RTS as a "Ready
> To Send", we're really using it as an "enable RS485 driver".
> 
> I just used the "RTS" mnemonic as "we're now wanting to send some
> data so please now enable the RS485 driver", rather than the normal
> "I'm ready for your to send me some data".
> 
> So it's the opposite function.
> 
> Maybe it was a poor choice of abbreviation ?
> 
> As I said before, RS485 drivers might have active or active low
> enables, so we might need to invert the "RTS" polarity.  This is
> not handled by the hardware RTS signal.
> 
> Is that a good enough explanation ?

fair, but considering you can toggle RTS by hand with writes to MCR
register, I still don't see the need for remuxing the lines as GPIOs.

Just don't use auto-RTS and toggle the line by hand with MCR, no ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: rts-gpio DT binding

[Mark Jackson]

On 19/03/14 14:59, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Mar 19, 2014 at 09:15:03AM +0000, Mark Jackson wrote:

[snip]

>>
>> Okay ... it comes back to me now.
>>
>> When using RS485 drivers, we're not actually using RTS as a "Ready
>> To Send", we're really using it as an "enable RS485 driver".
>>
>> I just used the "RTS" mnemonic as "we're now wanting to send some
>> data so please now enable the RS485 driver", rather than the normal
>> "I'm ready for your to send me some data".
>>
>> So it's the opposite function.
>>
>> Maybe it was a poor choice of abbreviation ?
>>
>> As I said before, RS485 drivers might have active or active low
>> enables, so we might need to invert the "RTS" polarity.  This is
>> not handled by the hardware RTS signal.
>>
>> Is that a good enough explanation ?
> 
> fair, but considering you can toggle RTS by hand with writes to MCR
> register, I still don't see the need for remuxing the lines as GPIOs.
> 
> Just don't use auto-RTS and toggle the line by hand with MCR, no ?

TBH I hadn't realised you could do that, so I guess you could.

But my solution seems more flexible as it would allow you to add an RTS
signal to UART0 and UART5 which don't have any hardware handshaking lines.

Mark J.

Re: rts-gpio DT binding

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

Hi,

On Wed, Mar 19, 2014 at 05:02:56PM +0000, Mark Jackson wrote:
> >> Okay ... it comes back to me now.
> >>
> >> When using RS485 drivers, we're not actually using RTS as a "Ready
> >> To Send", we're really using it as an "enable RS485 driver".
> >>
> >> I just used the "RTS" mnemonic as "we're now wanting to send some
> >> data so please now enable the RS485 driver", rather than the normal
> >> "I'm ready for your to send me some data".
> >>
> >> So it's the opposite function.
> >>
> >> Maybe it was a poor choice of abbreviation ?
> >>
> >> As I said before, RS485 drivers might have active or active low
> >> enables, so we might need to invert the "RTS" polarity.  This is
> >> not handled by the hardware RTS signal.
> >>
> >> Is that a good enough explanation ?
> > 
> > fair, but considering you can toggle RTS by hand with writes to MCR
> > register, I still don't see the need for remuxing the lines as GPIOs.
> > 
> > Just don't use auto-RTS and toggle the line by hand with MCR, no ?
> 
> TBH I hadn't realised you could do that, so I guess you could.
> 
> But my solution seems more flexible as it would allow you to add an RTS
> signal to UART0 and UART5 which don't have any hardware handshaking lines.

fair, but at least turn into helper functions part of serial core so
that other serial drivers have a chance of reusing this.

IMHO, it's a pretty ugly hack, but then again, a bunch of current
in-kernel GPIO usage is, anyway.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]