Skip to content

Conversation

@michaelstoops
Copy link
Contributor

@michaelstoops michaelstoops commented Mar 20, 2021

Like it says, this is a major rewrite of the UART documentation. Nominally, this is because of /#1754. But also, I found this documentation confusing when I needed it a couple weeks ago, so I gave a shot at fixing the whole thing.

I got rid of the terminology of "primary" and "secondary" devices, which seemed to appear only in this doc, not in Broadcom's docs or others. Instead, I attempted to consistently refer to hardware devices by their canonical hardware names: UART0, etc. When discussing the serial port on the GPIO header, I referred to it as /dev/serial0, because that's how it will appear to the reader in Linux.

As you review this, please look first at the structure and clarity for the expected audience. Does this meet the reader's need? My intent was to give:

  • a brief introduction to what a UART is in general, and relate it to other things they may know
  • then to discuss the UARTs present in Raspberry Pi devices
  • then to discuss applications and other how-to instructions

Once we've settled on the structure, I think we need to look at the accuracy of the information. This is a big change and it contains many assertions of fact. I've attempted to validate them against reality. You can see my methodology and data at https://github.com/michaelstoops/documentation/blob/1754-testing/configuration/uart-testing.md. It should be easy to reproduce if you have the appropriate tools.

Lastly, let's look at minor issues of style: formatting and the like. I had to decide which things to 'quote' and which to mark as inline code, which I don't think is quite clear in the style guide. I gave a reasonable attempt, but I also edited and re-edited a lot, so I'm not sure they're all consistent in the end.

Your feedback is welcome.

@JamesH65
Copy link
Contributor

@pelwell Since you, IIRC, were responsible for the previous content (maybe not the actual writing?), what are your thoughts on this PR? The UART stuff has always been a bit difficult to understand, a quick read of this and its does seem to be am improvement.

@pelwell
Copy link
Contributor

pelwell commented Mar 20, 2021

As the author of the original I'm the wrong person to review this.

@ghost
Copy link

ghost commented Mar 20, 2021

Hello. I'm the last person to make any significant changes to this page. The issue we seem to have is that at some point in the past, someone decided it would be a good idea to come up with the concept of 'primary' and 'secondary' UARTs. In reality these designations don't really add much to the documentation, and actually just add extra unnecessary complexity, however I kept them as it actually makes some aspects of the UART config slightly easier to explain.

You don't need to explain what a UART is, or some of the other explanatory text you have added, e.g. baud rate. In fact, explanation of non-Pi specific technical details is generally discouraged in this repo.

Taking the proposed changes as a whole, my view is that you have added more complexity in some areas, and removed complexity from others.

@michaelstoops
Copy link
Contributor Author

As the author of the original I'm the wrong person to review this.

I'd still be glad to hear your point of view. 😊 If I've crossed you by trashing your work, I apologize. I assume that the choices you made were in good faith. Where I chose differently, combining our views should produce something that's better as a whole.

So let's hear it!

@ghost
Copy link

ghost commented Mar 20, 2021

See #1850 which addresses #1754.

(This PR does not actually address that issue).

@michaelstoops
Copy link
Contributor Author

Hello. I'm the last person to make any significant changes to this page. The issue we seem to have is that at some point in the past, someone decided it would be a good idea to come up with the concept of 'primary' and 'secondary' UARTs. In reality these designations don't really add much to the documentation, and actually just add extra unnecessary complexity, however I kept them as it actually makes some aspects of the UART config slightly easier to explain.

You don't need to explain what a UART is, or some of the other explanatory text you have added, e.g. baud rate. In fact, explanation of non-Pi specific technical details is generally discouraged in this repo.

Taking the proposed changes as a whole, my view is that you have added more complexity in some areas, and removed complexity from others.

I figure that the casual technologist may benefit from the background info, but I appreciate the point that it's really not configuration information, and this is under the configuration directory. Is there a better place for such content? If not, should there be?

@michaelstoops
Copy link
Contributor Author

See #1850 which addresses #1754.

(This PR does not actually address that issue).

Thank you. I had a slightly different understanding of the solution to 1754, which I thought was covered.

It seems that your changes might be lost in the re-structuring, so I'm integrating it into my next revision.

@ghost
Copy link

ghost commented Mar 20, 2021

Is there a better place for such content? If not, should there be?

The general rule with this repo is to not document things that are not Pi-specific, and to be reasonably succinct in doing so. Per the contribution policy at https://www.raspberrypi.org/documentation/CONTRIBUTING.md:

Note that this documentation is intended to be a short and concise set of helpful resources aimed at the majority of users.

(Also note that while there is some content in the repo that is not Pi-specific, e.g. the Linux tutorial section at documentation/linux, this is historical content which was written before there was any other content on the official website other than the blog and the forums. We now have MagPi articles and the Projects website at projects.raspberrypi.org, among others. The documentation will shortly be getting a major overhaul by Alasdair Allan, who wrote the very excellent docs for the Pico and RP2040).

@JamesH65
Copy link
Contributor

Just a small clarification - Alasdair is the technical docs manager, and whilst wrote many parts of the Pico docs, there was a huge contribution from the engineers involved, from the HW team who implemented the RP2040 to the software team who wrote the SDK, examples etc, plus input from a graphics designers in the comms team. And some engineers did both HW design and software, so got to write docs for both! Alasdair managed all of us and provided the overall concepts and took in 'suggestions' from above (i.e. Eben and other directors!). Worked really well, the docs are great.

@aallan
Copy link
Contributor

aallan commented Mar 22, 2021

Alasdair is the technical docs manager, and whilst wrote many parts of the Pico docs, there was a huge contribution from the engineers involved, from the HW team who implemented the RP2040 to the software team who wrote the SDK, examples etc, plus input from a graphics designers in the comms team. And some engineers did both HW design and software, so got to write docs for both!

Good god. I don't think a single person on the planet has the breadth of knowledge to write documentation all the way from getting started with the C SDK, through hardware design, to register level description of silicon. If such a person exists, it certainly isn't me.

The RP2040 documentation took a huge team effort, by most of the hardware and software team behind Pico, over the course of an entire year. As @JamesH65 said I wrote some of it — chunks of things — mostly around the getting started documentation so that would be written from the perspective of someone that hadn't just spent 4 years building the chip, but I certainly didn't do it all!

Also note that while there is some content in the repo that is not Pi-specific, e.g. the Linux tutorial section at documentation/linux, this is historical content which was written before there was any other content on the official website other than the blog and the forums. We now have MagPi articles and the Projects website at projects.raspberrypi.org, among others. The documentation will shortly be getting a major overhaul…

That said. Yes. There is a major overhaul coming and one of the primary aims of this is to reduce the amount of documentation. More is not necessarily better. The documentation here will be significantly reduced — a process that I'm sure will cause absolutely no gnashing of teeth or outcries from anybody, and will be totally painless — err, right?

In any case a lot (all)of the not Pi specific documentation will be going away, and the remaining documentation will get "tightened" up and be more considered.

The documentation repo here grew (and is still growing) organically. Time is soon coming for a prune. Although it'll be a little while yet, while I get toolchain issues sorted out, before content is affected. I figure we'll go with a Big Bang approach to toolchain and then start back in on the content.

@lurch
Copy link
Contributor

lurch commented Mar 22, 2021

The changes in this PR definitely feel far too wordy - have a look at some of the other documentation pages to get a feel for the typical level of detail used.

@michaelstoops
Copy link
Contributor Author

Thank you all for the feedback. I'll give this another edit and come back.

@michaelstoops
Copy link
Contributor Author

OK, revision made! I took out the background info. I also took out some of the wordy prose and replaced it with brief statements of fact. Hopefully that's a bit closer to where we're trying to go, and also an improvement over the past.

Feedback welcome. :)

@lurch
Copy link
Contributor

lurch commented Mar 27, 2021

Thanks for your extensive edits @michaelstoops , this "feels" much better now 👍

@michaelstoops
Copy link
Contributor Author

I think we're closing in on it. :)

Feedback still welcome, as always.

@michaelstoops
Copy link
Contributor Author

@aallan, can I get a signal from you on whether this edit falls in line with where the documentation is going? Is this one headed for a merge?

@JamesH65
Copy link
Contributor

This reads better than the original and has more information, so its a good change in my book, but I noted a couple of issues that I have commented on inline.

@michaelstoops
Copy link
Contributor Author

This reads better than the original and has more information, so its a good change in my book, but I noted a couple of issues that I have commented on inline.

Fixed. Thanks!

@aallan
Copy link
Contributor

aallan commented Jun 7, 2021

There's been a lot of back and forth on this one. What still needs doing here?

I'd encourage you to wrap this PR up in the next week or so as we're in the process of transitioning the documentation from the current Markdown-based source format to Asciidoc. At some point soon — probably around the end of June, beginning of July — we will freeze the current documentation repo. After that time contributions and PRs based on the Markdown source will not be accepted, and any PRs that are still open will be closed.

See #1911 for full details.

@aallan
Copy link
Contributor

aallan commented Jun 7, 2021

There is an alternative being offered to this PR in #1850. So we merge this, that, or neither. Anyone have any thoughts?

@aallan aallan linked an issue Jun 7, 2021 that may be closed by this pull request
@aallan
Copy link
Contributor

aallan commented Jul 10, 2021

Merged #1850 instead.

@aallan aallan closed this Jul 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UART: Enabling of UART1 for CM3+ does not work

5 participants