raspberrypi / pico-feedback

25 stars 2 forks source link

Collection of lacking information im RP2040 manual #222

Open MarcVonWindscooting opened 2 years ago

MarcVonWindscooting commented 2 years ago

While working with the manual on my own build environment I noticed, that essential information is lacking and some things plain wrong. I recorded these for some time. Here they are:

  1. (2.15. CLOCKS) Does (only) PERI CLK really NOT have a divider ? The register documentation is missing completely. Figure 28: clk_sys also drives PWM - this is missing!
  2. (4.1. USB) SIE_STATUS . DATA_SEQ_ERROR cannot be cleared by writing a 1 (this is how I understand 'WC') but by REALLY clearing it, e.g. through CLR alias. SIE_STATUS . SPEED is 'WC' too? That seems odd to me. Completely lacking: the meaning of USB_MUXING and USB_PWR registers. At least I could not guess the meaning of the bits.
  3. (4.5. PWM) no clear mention of clock source. 4.5.2.4 calculations suggests SYS CLK. 4.5.1 is where one searches for the information.
liamfraser commented 2 years ago

Hi, thanks for the feedback. You are right on all points.

rewolff commented 2 years ago

USB muxing is really for internal testing / fpga. On the real chip you would always pick the "to_phy" option. I would suggest to document them as: "reserved, leave at reset state" if possible. I appreciate the honesty that there really IS a register there. For us mortals out in the real world (WITH real chips, WITHOUT the FPGA-source) "reserved" is sufficient.

On the other hand.... documenting the debug features may help someone come up with a clever hack.

So documenting as "Reserved, leave at reset value. These bits were used to mux the USB output during development of the chip." might be in line with raspberry pi policy.

Reading between the lines, the chip would have gone out even when PIO wasn't working. Why else would there be SPI, I2C and Uart modules? I'd say a third and fourth PIO woul'dve been about the same area, but more functionality.

LukasGossmann commented 1 year ago

As MarcVonWindscooting already noticed correctly clk_peri does not have a divider so it should be removed from the diagram. There is a related issue for the SDK where i also noticed that.

Copy paste of the info from there:

Here is what needs to be changed:
1: On page 181 the Figure 28 needs to have its box with the division sign removed in the path of clk_peri.
(build-date: 2023-03-02, build-version: ae3b121-clean)