KakuteF7 board support #138

bkueng · 2019-07-09T08:45:19Z

This adds support for the Holybro KakuteF7 board.

There are 2*32KB sectors reserved for parameters - I used 2 to stay compatible with ArduPilot's configuration.

VBUS sensing

Looking through the datasheet, only PA9 can be used for VBUS sensing. If another pin is assigned (which is the case for KakuteF7), we can still use it for fast-boot detection. I did some changes to account for that in general. In case of disabled VBUS sensing, we need to force valid B-peripheral session, otherwise the USB device will not show up.
I think we could generally just disable VBUS sensing (but still use the pin for fast-boot detection). On the Firmware/NuttX we have it disabled as well.

The only other boards affected by this are OmnibusF4SD and Crazyflie. I cross-tested on Omnibus.

According to the datasheet only PA9 can be used for VBUS sensing on the OTG peripheral. If any other pin is used, we need to disable VBUS sensing but can still use the pin for fast boot detection (gpio input).

davids5

@bkueng - ST has multiple versions of USB OTG IP. The incorporation is based on release timeline not family. Please verify the USB IP on the supported Soc and insure we are not changing undefined bits.

davids5 · 2019-07-10T12:31:16Z

main_f7.c

 	rcc_peripheral_enable_clock(&RCC_AHB1ENR, RCC_AHB1ENR_GPIOAEN);
-#  if defined(USE_VBUS_PULL_DOWN)


This is needed on HW with interface issues raising VBUS above threshold do to leakage

I removed it because no board was using it. Do you want to keep it?

yes it is used out of tree

davids5 · 2019-07-10T12:40:01Z

stm32/cdcacm.c

@@ -316,6 +323,9 @@ usb_cinit(void)

 #if defined(BOARD_USB_VBUS_SENSE_DISABLED)
 	OTG_FS_GCCFG |= OTG_GCCFG_NOVBUSSENS;
+
+	/* Force valid B-peripheral session */
+	OTG_FS_GOTGCTL |= OTG_GOTGCTL_BVALOEN | OTG_GOTGCTL_BVALOVAL;


This is not for all USB IP in ST line and may need to be conditions based on CID of the IP block
F7,,F469, F446,

F427, F407:

Forgot to update comment

Ok, I found it, we can check for OTG_FS_CID >= 0x00002000.

davids5 · 2019-07-10T18:18:31Z

stm32/cdcacm.c

@@ -316,6 +323,12 @@ usb_cinit(void)

 #if defined(BOARD_USB_VBUS_SENSE_DISABLED)
 	OTG_FS_GCCFG |= OTG_GCCFG_NOVBUSSENS;
+


@bkueng Please see line 64 and use the same pattern to document the IP change and test for it.

…_DISABLED Required for example on STM32F745 boards.

davids5 · 2019-07-11T08:00:04Z

stm32/cdcacm.c

@@ -316,6 +323,12 @@ usb_cinit(void)

 #if defined(BOARD_USB_VBUS_SENSE_DISABLED)
 	OTG_FS_GCCFG |= OTG_GCCFG_NOVBUSSENS;
+


bkueng · 2019-07-11T10:04:59Z

Great, let's merge then! Thanks @davids5

tridge · 2019-08-01T06:03:38Z

@bkueng and @davids5 the change to board_types.txt should not have gone in. The layout for this bootloader was defined by the ardupilot bootloader in this case, and it started in the ardupilot bootloader.
Just because the px4 bootloader now implements it I don't think you should move it to the px4 section of the file.

davids5 · 2019-08-01T14:34:58Z

@tridge - Not having any procedural information in the file, nor knowing what you expect from the file. I would not know what was correct or incorrect.

I look at this as a reservation system for code points. Is it more than that?

From my perspective for this to be useful and serve that purpose the correct approach would be 3 columns in sorted order by code point.

|code point | PX4 ID | AP ID|

Freedom to add a row with a Col 1 and Col 2 or/and Col 3 would make sense.
Freedom to update a row with Col 2 or Col 3 would make sense.

What am I missing?

dagar · 2019-08-01T16:07:57Z

Yes it should be a single table sorted by id.

We can preserve the separate identifiers if people actually care, but given they don't seem to be consumed directly in any project why don't we just keep it simple?

bkueng · 2019-08-05T08:50:18Z

I wasn't aware of such a convention either. A single list would IMO make more sense as well.
The other important factor is the firmware offset in case of flash-based parameters.

tridge · 2019-09-10T21:38:22Z

@davids5 yes, it is a reservation system. And by adding AP_HW_KAKUTEF7 I was reserving that ID. Implicit in that is the particular layout of flash for that board. This PR took that reservation and grabbed it for px4.
If we have two different layouts for KakuteF7 then we need two separate IDs, as otherwise the ID is useless as a mechanism for the firmware loading script to determine if it the firmware it has is compatible. That includes the following info:

the starting flash page for the firmware
the number of pages allowed for the bootloader
any reserved pages (could be at front or back of flash)

It is in principle a good thing for the px4 and ardupilot bootloader to use common IDs for the same board where possible so that vendors can ship with one bootloader that supports both, but that only works if the above info lines up.
I don't know if px4 kakutef7 chose the same values as ArduPilot did. By moving the AP_HW_KAKUTEF7 from the ArduPilot section to the px4 section that seems to imply that px4 provides the canonical definition of the above information for that board. That is not reasonable.
We could instead have columns giving the above information, but given it takes months for me to get anything merged in this repo and we don't have that sort of timeline available when a vendor produces a new board I'll instead just use a big reserved range for ArduPilot. If px4 wants to use a layout compatible with the ardupilot layout (as defined in hwdef.dat) that would be great. If not then please don't use the IDs in the ardupilot range, and please don't shift AP_HW_* values into the px4 area.

dagar · 2019-09-10T22:34:09Z

@tridge There doesn't need to be a PX4 section and an Ardupilot section. A list of IDs (even including layout) can be shared. I don't think centralizing the layout is going to cause any real problems and it's certainly better than unnecessarily bifurcating. If you would like to move forward collaboratively I will make sure these PRs are merged immediately. If there are cases that truly can't be reconciled we can carry alternate IDs, but it's not too late to prevent the mess from getting worse.

bkueng · 2019-09-16T12:14:01Z

I don't know if px4 kakutef7 chose the same values as ArduPilot did. By moving the AP_HW_KAKUTEF7 from the ArduPilot section to the px4 section that seems to imply that px4 provides the canonical definition of the above information for that board.

I chose the same values, which is why I moved it in the first place. The intention was to stay compatible, not to grab ID's.

bkueng mentioned this pull request Jul 9, 2019

KakuteF7 board support PX4/PX4-Autopilot#12440

Merged

bkueng requested a review from davids5 July 10, 2019 05:36

VBUS sensing: either use PA9 if enabled, or only use the gpio

0dd6547

According to the datasheet only PA9 can be used for VBUS sensing on the OTG peripheral. If any other pin is used, we need to disable VBUS sensing but can still use the pin for fast boot detection (gpio input).

bkueng force-pushed the kakutef7 branch 2 times, most recently from b70ff10 to eccf9eb Compare July 10, 2019 06:53

davids5 requested changes Jul 10, 2019

View reviewed changes

bkueng force-pushed the kakutef7 branch from eccf9eb to a6df686 Compare July 10, 2019 16:12

davids5 requested changes Jul 10, 2019

View reviewed changes

bkueng added 2 commits July 11, 2019 09:57

fix cdcacm: need to force a valid OTG session if BOARD_USB_VBUS_SENSE…

a34cd91

…_DISABLED Required for example on STM32F745 boards.

hw_config: add KakuteF7 target

7067adf

bkueng force-pushed the kakutef7 branch from a6df686 to 7067adf Compare July 11, 2019 07:58

davids5 approved these changes Jul 11, 2019

View reviewed changes

bkueng merged commit b52fa20 into master Jul 11, 2019

bkueng deleted the kakutef7 branch July 11, 2019 10:05

dagar mentioned this pull request Sep 11, 2019

[RFC]: board_types.txt -> consolidated board_types.h #151

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KakuteF7 board support #138

KakuteF7 board support #138

bkueng commented Jul 9, 2019

davids5 left a comment

davids5 Jul 10, 2019

bkueng Jul 10, 2019

davids5 Jul 10, 2019

davids5 Jul 10, 2019 •

edited

Loading

davids5 Jul 10, 2019

bkueng Jul 10, 2019

davids5 Jul 10, 2019

bkueng Jul 11, 2019

davids5 Jul 11, 2019

davids5 Jul 11, 2019

bkueng commented Jul 11, 2019

tridge commented Aug 1, 2019

davids5 commented Aug 1, 2019

dagar commented Aug 1, 2019

bkueng commented Aug 5, 2019

tridge commented Sep 10, 2019 •

edited

Loading

dagar commented Sep 10, 2019

bkueng commented Sep 16, 2019

		rcc_peripheral_enable_clock(&RCC_AHB1ENR, RCC_AHB1ENR_GPIOAEN);
		# if defined(USE_VBUS_PULL_DOWN)

		@@ -316,6 +323,12 @@ usb_cinit(void)

		#if defined(BOARD_USB_VBUS_SENSE_DISABLED)
		OTG_FS_GCCFG \|= OTG_GCCFG_NOVBUSSENS;

KakuteF7 board support #138

KakuteF7 board support #138

Conversation

bkueng commented Jul 9, 2019

VBUS sensing

davids5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davids5 Jul 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkueng commented Jul 11, 2019

tridge commented Aug 1, 2019

davids5 commented Aug 1, 2019

dagar commented Aug 1, 2019

bkueng commented Aug 5, 2019

tridge commented Sep 10, 2019 • edited Loading

dagar commented Sep 10, 2019

bkueng commented Sep 16, 2019

davids5 Jul 10, 2019 •

edited

Loading

tridge commented Sep 10, 2019 •

edited

Loading