Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USTX in U0S/U1S is broken #7256

Closed
xsrf opened this issue May 1, 2020 · 9 comments
Closed

USTX in U0S/U1S is broken #7256

xsrf opened this issue May 1, 2020 · 9 comments

Comments

@xsrf
Copy link
Contributor

xsrf commented May 1, 2020

Hi,
I found USTX in registers U0S/U1S to be broken. It's always 0.
You can read about how I tested it here: https://www.esp8266.com/viewtopic.php?f=13&t=21169
I've also reported it here: https://bbs.espressif.com/viewtopic.php?f=66&t=56814

However, nobody confirmed it so far.

What's the best way to document this? I've used https://github.com/esp8266/Arduino/blob/master/cores/esp8266/esp8266_peri.h#L252 a lot as reference for developing, so I'd appreciate a comment there.

A possible workaround is to enable Loopback mode and read USRXD instead.

//UART STATUS Registers Bits
#define USTX    31 //TX PIN Level (Bug: Always 0! Enable loopback UxC0 |= 1<<UCLBE and read USRXD)
#define USRTS   30 //RTS PIN Level
#define USDTR   39 //DTR PIN Level
#define USTXC   16 //TX FIFO COUNT (8bit)
#define USRXD   15 //RX PIN Level
#define USCTS   14 //CTS PIN Level
#define USDSR   13 //DSR PIN Level
#define USRXC    0 //RX FIFO COUNT (8bit)

MCVE

#include <Arduino.h>
#include <Ticker.h>
Ticker tick;
// U1 TX: GPIO_02 / D4
#define P_DEBUG 14 // D5
#define DEBUG_HIGH GPOS = 1<<P_DEBUG
#define DEBUG_LOW GPOC = 1<<P_DEBUG

void ICACHE_RAM_ATTR send() {
  DEBUG_HIGH;
  DEBUG_LOW;
  U1F = 0x80;
}

void setup() {
  Serial1.begin(9600);
  pinMode(P_DEBUG,OUTPUT);
  U1S |= 0x01 << USTXC;
  U1D = 10*100;
  tick.attach(0.01, send);
}

void loop() {
  if(U1S & (1<<USTX)) DEBUG_HIGH;
  else DEBUG_LOW;
}

// What I get:
// IO_02: ‾‾‾‾|_______________|‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾|___
// DEBUG: ____|_________________________________|___
// What I expect:
// IO_02: ‾‾‾‾|_______________|‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾|___
// DEBUG: ‾‾‾‾|_______________|‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾|___
@devyte
Copy link
Collaborator

devyte commented May 1, 2020

I read through that. Please edit your op and add the MCVE, and also explain what you expect to happen and what actually happens. We can discuss further at that point.
As a side note, I believe we check the tx queue for empty plus a specific elapsed time calculated based on the serial config params. You can look at our uart.h/cpp and HardwareSerial class if you haven't already.

@xsrf
Copy link
Contributor Author

xsrf commented May 1, 2020

I've added the MCVE from the forum post I've linked. I'm not interested in the uart tx queue or if the buffer is empty, because I don't use uart1 to send serial data. I just use it to create a timed low pulse.
The issue I'm reporting is that the register is not mirroring the tx level, as it should.

@xsrf xsrf changed the title USTX in UxS is broken USTX in U0S/U1S is broken May 1, 2020
@devyte
Copy link
Collaborator

devyte commented May 1, 2020

I'm not convinced that what you observe is a hardware bug. It could be that. Or it could be our core has some wrong register definition. Or our core has some bug in the init of serial1. Or there could be some assumption that isn't being met with your code. Or something else. That is why I suggested looking at what we do in our uart code.
I don't think I've ever seen code trying to read the uart Tx pin level. Example questions that come to mind (off the top of my head and without thinking my way through it):

  • are you sure that your sketch writes to the uart the same way our core does? at a glance you're using U1F while we're using USF in uart.cpp.
  • do you observe the same issue with uart 0?
  • are you sure that your sketch reads the status the same way our core does? at a glance you're using U1S while we're using USS in uart.cpp.
  • are you sure that use of USTX does what you think? is it really the output pin level, or is it some intermediate register to be output which nobody is using, or something else?

Some notes about your apparent need for accuracy/cpu load/whatever

  • you're relying on Ticker, which is a scheduled os task that executes at the next possible chance in the SYS context. The mechanism works by detecting whether the relevant sw timer has expired by polling, then a task get created and queued (that happens at some point when it can, it's not precise), then that task executes which would call your cb and hence write to the fifo (that is also not precise). If you need timing precision, you should use a hw timer.
  • depending on your application, there could be non-maskable interrupts at play. Also there is the wifi handling. this could introduce jitter.
  • you're assuming that the tx fifo always has space, i.e.: you're not checking whether it's empty before writing to it. This is likely always true given your shown speeds, but it's not a guarantee if you change your application parameters, e.g. slower baudrate or higher output frequency. Some off-the-top-of-my-head calculations:
    • ticker at 0.01 => 100 bytes per second => 1000bps average (each byte at 9600bps + pauses in between)
    • change ticker to 0.001 => 1000 bytes per second => 10000bps average which is already more than 9600bps baudrate => I suspect your fifo would fill up.
      I just thought I'd point it out.

@xsrf
Copy link
Contributor Author

xsrf commented May 1, 2020

Or it could be our core has some wrong register definition.

Possible, but I've checked all bits of U0S/U1S. If the TX level is mirrored in a register, then it's not in U0S/U1S.

Or our core has some bug in the init of serial1.

I've tested pinMode(2, SPECIAL); // Set GPIO2 as UART1 TX instead of Serial1.begin(). Same result.

Or there could be some assumption that isn't being met with your code.

Well, my only assumption is that #define USTX 31 //TX PIN Level means that this bit should mirror the signal level of TX.

That is why I suggested looking at what we do in our uart code.

I did, e.g. to find out how to replace Serial1.begin with pinMode.

I don't think I've ever seen code trying to read the uart Tx pin level.

I guess I'm the first one using it then ;)
https://github.com/xsrf/ESP8266Matrix/blob/master/ESP8266Matrix.h#L344

  • are you sure that your sketch writes to the uart the same way our core does? at a glance you're using U1F while we're using USF in uart.cpp.

I'm sure it does not, but I'm not using uart for communication, so why would I?
USF(x) is just an alias for UxF.

  • do you observe the same issue with uart 0?

Yes

  • are you sure that your sketch reads the status the same way our core does? at a glance you're using U1S while we're using USS in uart.cpp.

USS(x) is just an alias for UxS. And I'm pretty sure you are not interested in the level of the tx line if you want to know if the uart is ready for communication etc. You are not reading the tx level.

  • are you sure that use of USTX does what you think? is it really the output pin level, or is it some intermediate register to be output which nobody is using, or something else?

No, I'm not sure. Can you point me to better documentation? ;)
My only documentation is the comment in the header file.

  • you're relying on Ticker

this is just a demo code, I don't rely on ticker for precise timing.

  • you're assuming that the tx fifo always has space

right, but that's the case in my application an the demo code. This is not relevant to the issue.

@devyte
Copy link
Collaborator

devyte commented May 1, 2020

Can you point me to better documentation?

Our core is built on Espressif's SDK, and does weird things to accomodate. A whole lot of our code comes from extracts found elsewhere and reverse engineering done by others. Our docs and core code has been found to have fundamental issues in the past. That means don't necessarily take our code/docs/comments at face value in cases like these, but compare against pure-sdk applications out there and notes written by others, and Do Your Research.

Having said that, I suspect you've already done so. I did a quick search and didn't find anything worthwhile for the Tx level.
So about documenting this. I think something like the following at the reg definition for USTX:

//Doesn't seem to work, always reads as 0 for both uarts. HW bug? Possible workaround: Enable loopback UxC0 |= 1<<UCLBE and read USRXD, see https://github.com/esp8266/Arduino/issues/7256 for discussion. 

and then mention the bit 31 at USS, U0S and U1S, something like:

//Bit 31 (Tx level read) doesn't seem to work, see USTX comment 

@xsrf
Copy link
Contributor Author

xsrf commented May 2, 2020

I know that the code and documentation is mainly reverse engineering, so I'm not blaming you at all for the "bad" documentation. I'm quite happy it exists :)
The only other real source I was pointed to is https://www.espressif.com/sites/default/files/documentation/esp8266-technical_reference_en.pdf but it says the same about bit 31 in the status register on page 115.

After spending many days on this, I came to the conclusion that the bit is not doing what it should do. But sure, this is open for discussion.
I hope that some day espressif will confirm this, or correct me.

Until then, any comment in the header file that helps others so they don't have to spend as many hours scratching their head about this, like I did, is appreciated. Thx :)

@devyte
Copy link
Collaborator

devyte commented May 2, 2020

Do you want to make the PR?

@xsrf
Copy link
Contributor Author

xsrf commented May 3, 2020

Sure, can do that. But do you really also want a comment on USS, U0S and U1S? I don't think anyone will use these registers without reading the bit definitions. I think the comment on USTX should be enough. Or what do you think may be the benefit of a comment on USS etc?

xsrf added a commit to xsrf/Arduino that referenced this issue May 3, 2020
Documentation that bit USTX in UART status register USS(u) / U0S / U1S is not mirroring TX level as expected but always reads 0, see issue esp8266#7256
devyte added a commit that referenced this issue May 3, 2020
Documentation that bit USTX in UART status register USS(u) / U0S / U1S is not mirroring TX level as expected but always reads 0, see issue #7256

Co-authored-by: Develo <deveyes@gmail.com>
@devyte
Copy link
Collaborator

devyte commented May 3, 2020

Closed via #7265.

@devyte devyte closed this as completed May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants