Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aerofc: Add and use a new I2C bus #7070

Merged
merged 6 commits into from
Aug 2, 2017

Conversation

zehortigoza
Copy link
Contributor

To this work is necessary also update the FPGA RTL(intel-aero/intel-aero-fpga#2)

CONFIG_STM32_UART7_SERIALDRIVER=y
# CONFIG_STM32_UART7_1WIREDRIVER is not set
# CONFIG_UART7_RS485 is not set
CONFIG_UART7_RXDMA=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you check the dma channel does not conflict with other channels?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check now, UART7 is the only user of his DMA stream.

@@ -1279,6 +1280,9 @@ struct ist8310_bus_option {
#ifdef PX4_I2C_BUS_ONBOARD
{ IST8310_BUS_I2C_INTERNAL, "/dev/ist8310_int", PX4_I2C_BUS_ONBOARD, NULL },
#endif
#ifdef PX4_I2C_BUS_EXPANSION2
{ IST8310_BUS_I2C_EXTERNAL2, "/dev/inst8310_ext", PX4_I2C_BUS_EXPANSION2, NULL },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: should be ist8310_ext... what if you have both connected? It should probably be ist8310_ext2 otherwise would conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh thanks, ok I will rename to ext2.

@lucasdemarchi
Copy link
Contributor

Overall LGTM. Some minor comments above.

LorenzMeier
LorenzMeier previously approved these changes Apr 19, 2017
@LorenzMeier
Copy link
Member

Please merge when all things are ready including the FPGA image for a complete upgrade.

@@ -1279,6 +1280,9 @@ struct ist8310_bus_option {
#ifdef PX4_I2C_BUS_ONBOARD
{ IST8310_BUS_I2C_INTERNAL, "/dev/ist8310_int", PX4_I2C_BUS_ONBOARD, NULL },
#endif
#ifdef PX4_I2C_BUS_EXPANSION2
{ IST8310_BUS_I2C_EXTERNAL2, "/dev/ist8310_ext2", PX4_I2C_BUS_EXPANSION2, NULL },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should stay on the current PX4_I2C_BUS_EXPANSION

@LorenzMeier
Copy link
Member

Note that we will need this in for the v1.6 release. Do you have the FPGA image and a BSP update ready?

@lucasdemarchi
Copy link
Contributor

This shouldn't be in 1.6 yet. The latest release of the BSP doesn't contain the needed changes.

@lucasdemarchi
Copy link
Contributor

Btw last BSP is 1.3.1: https://github.com/intel-aero/meta-intel-aero/releases

Should be available for download next Monday.

@zehortigoza
Copy link
Contributor Author

@lucasdemarchi now should be a good time to get back to this? I just rebased and tested this

@lucasdemarchi
Copy link
Contributor

Yes. And remember to leave the current i2c bus as is

@zehortigoza
Copy link
Contributor Author

zehortigoza commented Jun 13, 2017

Yes. And remember to leave the current i2c bus as is

It was already fixed.

@zehortigoza zehortigoza force-pushed the additional_i2c_bus branch 3 times, most recently from fc1a8c1 to 4f1b07a Compare July 21, 2017 20:04
The UART3 also have the I2C bus 2 functions so moving GPS to UART7 to
have one additional I2C.
To keep GPS working is also necessary update the FPGA RTL to version
0xC1 or higher.
Nothing use this define right now so lets remove it.
Several other boards also have this defines that can also
be removed.
Now that UART3 is no longer in use we can use this I2C bus.
This change plus the new FPGA RTL(version 0xC1 or higher) will make
use of the new I2C bus, this new bus will be shared between aerofc_adc
and ll40ls(if connected) and leaving the old bus just to IST8310.
@zehortigoza
Copy link
Contributor Author

FPGA firmware updated and binary sent to be integrated on Aero image.
intel-aero/meta-intel-aero#226

@zehortigoza
Copy link
Contributor Author

@LorenzMeier @lucasdemarchi ping

@LorenzMeier
Copy link
Member

@zehortigoza Could you explain how this should be merged? If we merge now, does master work with the latest Aero BSP or where should we point users?

@zehortigoza
Copy link
Contributor Author

@LorenzMeier PX4 will boot and run ok with the FPGA firmware from the latest Aero BSP release(1.4v) but the user will lose GPS. Users can download the updated FPGA firmware before the release from here: https://github.com/intel-aero/meta-intel-aero/tree/master/recipes-support/jam-stapl/jam-stapl

@lucasdemarchi
Copy link
Contributor

@LorenzMeier I don't have powers in this repo to merge this. Can you do so, please?

@LorenzMeier LorenzMeier merged commit 7d46858 into PX4:master Aug 2, 2017
@LorenzMeier
Copy link
Member

Sorry for the oversight - and we can certainly talk about a more central role in the maintenance team. It would be good to have you being able to do more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants