Bootloader broken on latest master

The bootloader on latest master always boots into MSC mode. Might have been introduced with the portexpander changeset.

added 1-Bug 3-Bootloader labels

Bisected to commit c084a687. Cc @Woazboat.

I guess the error might be in portexpander_init. It works when the old bootloader initializes the portexpander correctly before this code runs, but fails if this is the first function to attempt initialization.

I don't have a debugger, so I unfortunately can't test the bootloader. What's changed in that commit is:

1. The portexpander now latches input changes until the next time it is queried
2. Slightly changed portexpander configuration order

• Previously: enable pull-ups for inputs -> set outputs to open drain -> enable outputs -> set output state to high
• Now: set outputs to open drain -> set outputs to high (not enabled yet) -> enable outputs -> enable pull-ups -> enable latch for inputs

3. Enable interrupts for the microcontroller GPIO pin (interrupts are still masked on the portexpander)

I suspect 1. is the culprit. Can you try adding a portexpander read at the end of the init function to clear the latches? (portexpander_read(PE_C_INPUT_PORT, &buf);)

assigned to @rahix

The following patch seems to make it work. Any ideas?

diff --git a/lib/card10/portexpander.c b/lib/card10/portexpander.c
index 52eb8897..fca443b9 100644
--- a/lib/card10/portexpander.c
+++ b/lib/card10/portexpander.c
@@ -107,6 +107,11 @@ int portexpander_init(void)
 		);
 	}
 
+	portexpander_write(PE_C_PULL_ENABLE, PE_INPUT_MASK);
+
+	// Enable outputs for the transistors, the LED and the LCD reset
+	portexpander_write(PE_C_CONFIG, PE_INPUT_MASK);
+
 	// Latch inputs so we can figure out whether an interrupt was caused by a rising or falling edge
 	portexpander_write(PE_C_INPUT_LATCH, PE_INPUT_MASK);
 

I have now seen two things actually: In a faulty bootloader, the button is assumed pressed and the display is staying off (LCD reset is my guess).

I am also wondering whether this code:

/* lib/card10/portexpander.c L166+ */
		return E_BAD_PARAM;
	}

	portexpander_write(PE_C_PULL_ENABLE, pull_selection_state);
	portexpander_write(PE_C_PULL_ENABLE, pull_enable_state);

	return E_NO_ERROR;

is actually correct. Do you really want to write to the same register twice?

*facepalm* no, the first one should be PE_C_PULL_SEL. (although technically that shouldn't actually change anything in this case since pull up is the default according to the datasheet?)

Enabling the output another time shouldn't really be necessary and I can't see any errors in that part of the config function, so it kind of looks like portexpander_config isn't run at all.

Is MXC_ASSERT disabled in the bootloader? Because that would explain a lot of things. (The code should probably be changed so that only the return value check happens inside the assert and not the whole function call.)

mentioned in commit 609e3947

mentioned in merge request !323 (merged)

Seems the MXC_ASSERT was the issue! Thank you very much for helping out!

I've opened a MR (!323 (merged)) to fix this, I'll wait for someone else to test the changes and we will hopefully have this merged for 1.12.

changed milestone to %v1.12

closed via merge request !323 (merged)

mentioned in commit fd5ad609