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

[WIP] boards name and number external SPI consistently #11336

Closed
wants to merge 1 commit into from

Conversation

dagar
Copy link
Member

@dagar dagar commented Jan 29, 2019

This PR unifies the mess of different external SPI bus naming and numbering across boards.

Master

  • PX4_SPI_BUS_EXT
  • PX4_SPI_BUS_EXT0
  • PX4_SPI_BUS_EXT1
  • PX4_SPI_BUS_EXTERNAL1
  • PX4_SPI_BUS_EXTERNAL2
  • PX4_SPI_BUS_EXPANSION

PR

  • PX4_SPI_BUS_EXTERNALx (starts at 1)

Gradually working towards a simple unified mechanism for handling optional external peripherals without pushing the burden into every single driver and board.

@dagar dagar requested a review from davids5 January 29, 2019 19:23
@dagar dagar force-pushed the pr-ext_spi_consistency branch 4 times, most recently from cbfeefa to b582f08 Compare January 29, 2019 20:37
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

Do we need to state that bus and CS numbering are 1 based?

See Questions

@@ -244,17 +243,17 @@ __BEGIN_DECLS
#define PX4_SENSOR_BUS_FIRST_CS PX4_SPIDEV_ACCEL_MAG
#define PX4_SENSOR_BUS_LAST_CS PX4_SPIDEV_GYRO

#define PX4_SPIDEV_EXTERNAL1 PX4_MK_SPI_SEL(PX4_SPI_BUS_EXTERNAL,0)
#define PX4_SPIDEV_EXTERNAL2 PX4_MK_SPI_SEL(PX4_SPI_BUS_EXTERNAL,1)
#define PX4_SPIDEV_EXTERNAL1_1 PX4_MK_SPI_SEL(PX4_SPI_BUS_EXTERNAL1,0)
Copy link
Member

Choose a reason for hiding this comment

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

formatting

boards/nxp/fmuk66-v3/src/board_config.h Show resolved Hide resolved
#ifdef PX4_SPI_BUS_EXT
#define px4_spi_bus_external(bus) (bus == PX4_SPI_BUS_EXT)
#ifdef PX4_SPI_BUS_EXTERNAL1
#define px4_spi_bus_external(bus) (bus == PX4_SPI_BUS_EXTERNAL1)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a complex with all the externals?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes


if (g_dev == nullptr) {
goto fail;
}

if (OK != g_dev->init()) {

#ifdef PX4_SPIDEV_EXTERNAL1_2
Copy link
Member

Choose a reason for hiding this comment

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

Is this the same functionality as it was?

@dagar dagar self-assigned this Feb 4, 2019
@dagar dagar changed the title boards name and number external SPI consistently [WIP] boards name and number external SPI consistently Feb 4, 2019
@dagar dagar force-pushed the pr-ext_spi_consistency branch from b582f08 to fa54cbc Compare February 11, 2019 00:53
@stale stale bot closed this Jul 24, 2019
@julianoes julianoes reopened this Jul 25, 2019
@stale stale bot removed the Admin: Wont fix label Jul 25, 2019
@PX4 PX4 deleted a comment from stale bot Jul 25, 2019
@PX4 PX4 deleted a comment from stale bot Jul 25, 2019
@julianoes
Copy link
Contributor

@davids5 what's up with this?

@davids5
Copy link
Member

davids5 commented Jul 25, 2019

@julianoes - it was @dagar work and has rotted. If if cleans it up and resolve the open, we can review it again.

I would make the CS as external in the ID - i.e PX4_MK_SPI_EXT_SEL - then they are self describing.

@stale
Copy link

stale bot commented Oct 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants