atsamd icon indicating copy to clipboard operation
atsamd copied to clipboard

Port pygamer to v2

Open bradleyharden opened this issue 3 years ago • 16 comments

This relies on #467. Please merge it first.

Port the pygamer BSP to the gpio::v2 and sercom::v2::spi APIs. While implementing this, I discovered that the pygamer uses an undocumented combination of pins for Sercom1. It uses PA17, PB22 & PB23. The datasheet indicates that PA17 is in IoSet1, while the other two pins are in IoSet3. It shouldn't be possible to use these together, but it appears there is an undocumented IoSet. Add this as UndocIoSet1 for Sercom1.

Also fix a warning from a trinket_m0 example.

@jacobrosenthal, could you please look this over? In particular, I'd like to draw attention to the various structs within Sets. Since the bsp_pins! macro can provide aliases for pins in a corresponding pin mode, I decided to use those aliases to define the various structs. However, that introduces a change in behavior to the construction of the Sets type. Previously, Sets only distributed pins to the correct structs. But now, creating the Sets struct also changes the pin mode for many pins. Is that ok?

In general, the structure of this BSP might need some thought in light of the v2 changes. For example, the Joystick and JoystickReader structs are now identical, because they both use the pin aliases from bsp_pins!. The Battery and BatteryReader structs are also the same.

Finally, I'd also like to note that I changed the structure of the modules a bit. All of the examples now do this:

use pygamer as bsp;
use bsp::{hal, pac, ...};

I modified the crate to no longer re-export the HAL. I also changing it re-export * from both the buttons and pins modules. I think this is in line with the proposed changes in #357.

bradleyharden avatar Jun 20 '21 13:06 bradleyharden

@bradleyharden Thanks for the cleanup.

hrm.. it IS meaningfully different..

The previous idea of the sets were that it was naked pins just as if you restarted the board, but grouped for you by functionality and pinout but not configured or costing any more power or startup time than a fresh reset.

Now some stuff can still be init to truly enable it, but in the mean time its all configured which may very well be powering or leaking current into regulators and ics even though youre not using them.

Frankly thought that was true of the old model. Every pin floating could also not be ideal as well.. but at least you knew all the pins were floating..

Im not sure, I guess we have to decide as a repo what we want the bsps to do. To me this basically removes the usefullness of the 'sets' model which I dont know how I feel about (truly, not sure if its good or bad)

jacobrosenthal avatar Jun 20 '21 15:06 jacobrosenthal

Let me know what you decide is best. I don't really have an opinion. I can return it to the old approach. It just means more pin aliases.

Speaking of, in terms of the docs, it might make sense for me to create those aliases in a separate module. There can be a lot of them, and they can make it hard to find other items in the docs.

bradleyharden avatar Jun 20 '21 16:06 bradleyharden

One other thing to note. The v1 API had the pins in FloatingInput mode at reset. But that's actually wrong. They are in a floating disabled mode. I don't think it matters for the discussion above, but I wanted to clarify that point.

bradleyharden avatar Jun 20 '21 17:06 bradleyharden

I rebased this and tested stuff. Most stuff seems all working, but the screen is actually not working even with this ioset5.

Im still not sure on pre configured pins. But maybe the question first needs to be are we keeping the sets model at all or not? It seems like its mainly me using it at this point, and Id rather be consistent than right.

jacobrosenthal avatar Jun 21 '21 22:06 jacobrosenthal

@jacobrosenthal, wasn't the undocumented IOSET for the SD card, not the display? I'm not sure what's wrong with the display. Do you have any other symptoms? Maybe you could double check the code and make sure I didn't make any transposition errors?

bradleyharden avatar Jun 22 '21 00:06 bradleyharden

@jacobrosenthal, I returned the Sets to their original architecture. Could you take another look at the display problem? I have no way to test it.

bradleyharden avatar Jun 26 '21 02:06 bradleyharden

I forgot, I wanted to implement the IOSET solution in this PR. Let me do that before we merge.

bradleyharden avatar Jun 26 '21 03:06 bradleyharden

Ok, I think this should be good to go, pending approval by @jacobrosenthal.

bradleyharden avatar Jun 26 '21 14:06 bradleyharden

Whoops, hit the wrong button

bradleyharden avatar Jul 05 '21 23:07 bradleyharden

Im not sure I reported here, but yes this is still broken. I actually tried the sd card example again as well and thats broken too

jacobrosenthal avatar Aug 22 '21 01:08 jacobrosenthal

When last we spoke, i was seeing strange reboots on my pygamer, it turns out thats only one device... (the one that has my swd header soldered...) so just not using that one for now and ignoring that problem.

The ferris_img example draw a little garbage on the display then freezes. I cant really scope the screen well because pygamer doesnt break out the tft spi pins.

I bought a thing-plus bsp because it has access to the .1" pins so I could logic analyze it https://www.sparkfun.com/products/14713

and this very similar (not the exact same) tft screen as pygamer https://www.amazon.com/gp/product/B07BFV69DZ/ref=ppx_yo_dt_b_search_asin_title?ie=UTF8&psc=1

Needed a patch to hf2 to add the vid/pid https://github.com/jacobrosenthal/hf2-rs/tree/sparkfun

and ported the bsp and screen example and it works?! https://github.com/jacobrosenthal/atsamd/tree/port_pygamer_to_v2

technically that chip is the ATSAMD51J20A instead of the ATSAMD51J19A maybe an eratta? Or the different tft screen is less annoyed with some timing change??

jacobrosenthal avatar Sep 16 '21 20:09 jacobrosenthal

Another note, I stripped the embedded graphics from the sd_card example, and the sdcard spi seems to work fine. So it really is the displays SPI thats giving trouble here I believe (which doesnt use the undoc ioset or anything)

jacobrosenthal avatar Sep 16 '21 21:09 jacobrosenthal

So does this mean that spi::v2 is actually bug-free?

jbeaurivage avatar Sep 17 '21 15:09 jbeaurivage

Note that I refactored this to be based on #467, which merged the thumbv6m and thumbv7em versions. That could have fixed a bug in the latter.

bradleyharden avatar Sep 17 '21 16:09 bradleyharden

It seems to indicate its bug free except when interacting with at least the tft screen on the pygamer

jacobrosenthal avatar Sep 17 '21 16:09 jacobrosenthal

Grabbed a sensepeek so I could probe the FPC on the pygamer. ferris_img here currently just stops drawing writing some noise to like the first 1/8 inch of the screen wipe after leaving the bootloader screen. saleae captures look sane so its not obviously an spi issue (i can see the init sequence https://github.com/sajattack/st7735-lcd-rs/blob/master/src/lib.rs#L88-L90), just have to spot the difference in timing or whatever. logic.zip

jacobrosenthal avatar Sep 23 '21 00:09 jacobrosenthal