update code in trezorhal for the new circuitry

[prusnak]

• [x] 1. switch on/off SD card circuitry
• [x] 2. set SD card pins (DAT3:0, CLK, CMD) to log 0 or high Z when SD card is switched off
• [x] 3. switch on/off CTP (capacitive touch) circuitry
• [ ] 4. set CTP I2C pins (SDA, SCL) to log 0, others to high Z, when CTP is switched off

[mcudev]

looks related to https://github.com/trezor/trezor-core/issues/86? i don't have great availability right now. but feel free to ping me if you need a second set of eyes or code review.

[prusnak]

Yes, it is related. Will ping you once it's implemented. Thanks!

[prusnak]

Pushed updated code to branch https://github.com/trezor/trezor-core/tree/new_hw

Applied points 1, 2 and 3. I don't do 4, because I start CPT immediately in touch_init.

The branch also contains the updated schematics.

[mcudev]

Thanks for the update! Looks pretty good!

Observations:

• Looks like you have pull-ups because the GPIOs default to being floating inputs. Seems correct.
• Looks like we're then driving some P-channel enhancement mode MOSFET switches (active low).
• Looks like more capacitance for the microSD power (would need to test and look at some datasheets to verify). Probably better.
• Looks like the CTP is now not powered during reset and all the way through the boardloader and into the bootloader. I like that a lot.
• Do we need a low-ohm series resistor between TOUCH_ON and the gate? It might not be necessary, I forget.

One thing I need to verify, but am pretty sure that I did on purpose for the display code after scoping it -- see this example where I make sure to write the pin state before I call HAL_GPIO_Init https://github.com/trezor/trezor-core/blob/new_hw/embed/extmod/modtrezorui/display-stm32.h#L170-L171

For SD_ON, we'd want to make sure that is GPIO_PIN_SET before init-ing it as an output. Now we might be ok if the default is to output a high. Easy enough too verify (maybe tomorrow).

Same feedback for CTP_ON. I'd probably prefer HAL_GPIO_WritePin before HAL_GPIO_Init.

Also, now that there is a switch and we can control the whole startup sequence of the CTP, there may be some more that we can do with that better control. I get the feeling that I had something in the back of my mind when I called adding the switch a potential upgrade. Would love some new dev hardware for testing, if you've got some extra prototypes :)

From #86 , did you guys consider: "potential upgrade: Add MCU controlled switch to allow control of LCD + ST7789V power. Potential benefits are control of display module startup sequence and startup screen flash/flicker."?

[prusnak]

Would love some new dev hardware for testing, if you've got some extra prototypes :)

Sure,was thinking about that already. I'll arrange this on Monday.

[mcudev]

Neat, thanks! If you have any screen modules, I could use some extras too. I only have one screen module that works nicely without problems. If you can send a few of the new prototype boards, that'd be helpful too so I can flash lock, tinker, solder on debug wires, etc... without worrying about killing "my only one". I really appreciate whatever you can hook me up with!

Regarding SD_ON and CTP_ON, as you have the code, immediately after calling HAL_GPIO_Init, they are left as floating outputs (verified with scope). So, the external pull-ups should keep the switch gate inputs high. Calling HAL_GPIO_WritePin before HAL_GPIO_Init just makes the pin drive the output as desired the whole time it's configured as an output. Should be no difference; just style, I guess. We could probably use slower speed for the outputs too. But again, it's probably a meaningless nit.

[mcudev]

I'm going to double check my scope on another board to make sure that the outputs are really left floating. It doesn't make sense to me that they would be left floating in push-pull mode. Maybe I have a bad port. I'll follow-up later.

[mcudev]

Regarding SD_ON, as you have the code, immediately after https://github.com/trezor/trezor-core/blob/new_hw/embed/trezorhal/sdcard.c#L92, it is driven low as an output (verified with scope on my original board and a second dev board -- with better connections this time, I guess) which could momentarily override the external pull-up until the sdcard_default_pin_state function runs. This is what I was expecting because the output data register value for that pin is zero after reset. So, we'd want to call HAL_GPIO_WritePin(GPIOC, GPIO_PIN_0, GPIO_PIN_SET); before that line to make the pin drive the output high (to keep the switch off) the whole time it's configured as an output.

Same could be done for CTP_ON, except right now the code wants to have the CTP powered on immediately, so it's a wash.

[prusnak]

I pushed minor improvements in bb94f81ee539947baff41de5a7e8cd5587b834bb

[mcudev]

yep, looks good!

[mcudev]

I just had the thought that one reason why I liked the idea of adding a switch to allow control of LCD + ST7789V power was to make it less likely that any useful data was retained across a soft reset. Just as another level of protection in-case there was some way to bypass the boardloader's display initialization.

[mcudev]

Following-up to last comment...This would also help if maybe the boardloader was not clearing all possible areas in the ST7789V memory (something undocumented maybe?), or if other vendor firmwares were displaying sensitive information more often, etc...

[prusnak]

Merged into master as https://github.com/trezor/trezor-core/commit/2b30cc16a21b9d6d4d72ec9eb6b0392ce18592d8 + https://github.com/trezor/trezor-core/commit/6ce106b54466b70f2540fbc19d85b2a366bf143b