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

spi_master for AVR #8299

Merged
merged 13 commits into from
Apr 8, 2020
Merged

spi_master for AVR #8299

merged 13 commits into from
Apr 8, 2020

Conversation

fauxpark
Copy link
Member

@fauxpark fauxpark commented Mar 3, 2020

Description

Pulling the SPI code up out of the BLE stuff so we can reuse it. Also replaced as much of the AVR-specific code as I could see, to facilitate Arm support by simply dropping in an appropriate spi_master.

Feel free to point out any dumb mistakes.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@drashna
Copy link
Member

drashna commented Mar 4, 2020

Need to test the Ploopy with this, but otherwise, I think it looks good, and travis doesn't object.

@drashna drashna requested a review from a team March 4, 2020 09:38
@fauxpark fauxpark mentioned this pull request Mar 4, 2020
13 tasks
@tzarc
Copy link
Member

tzarc commented Mar 8, 2020

I think we should mirror the i2c driver, and add spi_start/spi_stop.
There will be cases where we might need to change the SPI speed, polarity, and mode, depending on the device that's being talked to.
It would also allow us to specify the pin for slave select, so that manual handling of the slave select isn't required.

@tzarc
Copy link
Member

tzarc commented Mar 8, 2020

Speed

  • in the case of Arm we'd be setting a divisor; I'm not sure what AVR requires
  • accept an uint8_t

Polarity/phase:

Suggested API:

  • void spi_start(some_pin_t slavePin, uint8_t clockDivisor, int8_t spiMode);
  • void spi_stop(void);

@fauxpark
Copy link
Member Author

fauxpark commented Mar 8, 2020

LUFA's implementation of I2C has equivalents for i2c_start()/i2c_stop() in TWI_StartTransmission() and TWI_StopTransmission(), but it seems like it's not really a "thing" for SPI: https://github.com/abcminiuser/lufa/tree/master/LUFA/Drivers/Peripheral/AVR8
I'm not really sure what a stop function would do in the context of SPI. There is no real setup or teardown to speak of, you just put a byte in the register, wait for the SPIF flag, then take a byte out.

@tzarc
Copy link
Member

tzarc commented Mar 8, 2020

Yep, that's fine, but in the context of whether we want to start and stop communicating with a device, the slave select pin needs to be toggled.

I'm also thinking from the Arm side of things -- ChibiOS's SPI driver does indeed need start/stop, so if we "train" people to use the start/transmit/receive/stop methodology then anyone who wants to swap from an AVR-based design to an Arm-based design won't require extra messing about trying to understand why it works for one and not the other.

@fauxpark
Copy link
Member Author

fauxpark commented Mar 8, 2020

Got it. How's this for a start:

static pin_t currentSlavePin = NO_PIN;

void spi_start(pin_t slavePin) {
    if (currentSlavePin == NO_PIN && slavePin != NO_PIN) {
        currentSlavePin = slavePin;
        writePinLow(currentSlavePin);
    }
}

void spi_stop(void) {
    if (currentSlavePin != NO_PIN) {
        writePinHigh(currentSlavePin);
        currentSlavePin = NO_PIN;
    }
}

@fauxpark
Copy link
Member Author

fauxpark commented Mar 9, 2020

With this last commit I've added spi_start(slavePin) and spi_stop(void), however they don't seem to be working... any ideas?

Debug console just keeps printing this:

  > sdep_recv_pkt failed
    failed BLE command: ATE=0: 

and the board will occasionally spam a or b (the two keys I have mapped for testing) briefly, only over USB.

@fauxpark
Copy link
Member Author

fauxpark commented Mar 9, 2020

Okay, strange. A sprinkling of dprintf in spi_start/spi_stop shows they are both being called, but the sanity check in spi_start is not passing, even though currentSlavePin == NO_PIN (0xFF) and slavePin == 0x34 (B4). Wrapping each side of the && in parentheses didn't help. I'm missing something obvious here.

@fauxpark
Copy link
Member Author

fauxpark commented Mar 9, 2020

Figured it out. The NO_PIN define was not cast to pin_t, causing it to default to int16. Assigning it to currentSlavePin truncated it to 0xFF. So it was trying to compare that to the original 0xFFFF, and of course, failing.

@fauxpark fauxpark marked this pull request as ready for review March 9, 2020 06:17
drivers/avr/spi_master.c Show resolved Hide resolved
drivers/avr/spi_master.c Show resolved Hide resolved
drivers/avr/spi_master.c Show resolved Hide resolved
drivers/avr/spi_master.h Show resolved Hide resolved
@tzarc tzarc requested a review from a team March 9, 2020 20:16
@fauxpark fauxpark requested review from drashna, tzarc and a team March 9, 2020 21:56
Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

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

Docs look good.

@tzarc tzarc requested a review from a team March 9, 2020 21:57
@fauxpark fauxpark changed the title [WIP] spi_master for AVR spi_master for AVR Mar 9, 2020
@fauxpark fauxpark mentioned this pull request Mar 10, 2020
13 tasks
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

Brief test on my 34u2 bluefruit - seems happy enough.

@yanfali any chance you could test your fruity60?

@tzarc tzarc removed the pending_user_approval Awaiting feedback from a user before merge is allowed label Apr 8, 2020
@tzarc tzarc merged commit 400ca2d into qmk:master Apr 8, 2020
@tzarc
Copy link
Member

tzarc commented Apr 8, 2020

@yanfali confirmed fruity60 working.

@fauxpark fauxpark deleted the spi-master branch April 8, 2020 04:37
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Apr 9, 2020
* 'master' of https://github.com/qmk/qmk_firmware: (53 commits)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  DennyTom's buttery_engine (qmk#8138)
  add via support for kira80 (qmk#8677)
  [Keyboard] Wheatfield Split75 (qmk#8511)
  Correctly handle json keymaps with ANY()
  ...
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Apr 9, 2020
* master: (3035 commits)
  [Keymap] Update personal userspace and keymaps (qmk#8747)
  Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650)
  Add RGB support in via to launchpad (qmk#8621)
  VIA support for the KBDFans KBD6x (qmk#8680)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  ...

# Conflicts:
#	layouts/community/ortho_4x12/grahampheath/keymap.c
#	lib/lufa
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Apr 9, 2020
* master: (208 commits)
  [Keymap] Update personal userspace and keymaps (qmk#8747)
  Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650)
  Add RGB support in via to launchpad (qmk#8621)
  VIA support for the KBDFans KBD6x (qmk#8680)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  ...
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Apr 9, 2020
* master: (208 commits)
  [Keymap] Update personal userspace and keymaps (qmk#8747)
  Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650)
  Add RGB support in via to launchpad (qmk#8621)
  VIA support for the KBDFans KBD6x (qmk#8680)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  ...
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Apr 10, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
violet-fish pushed a commit to violet-fish/qmk_firmware that referenced this pull request Apr 12, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
Quarren42 pushed a commit to Quarren42/qmk_firmware that referenced this pull request Apr 15, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
bitherder pushed a commit to bitherder/qmk_firmware that referenced this pull request May 15, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request May 24, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request May 24, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Jun 12, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
turky pushed a commit to turky/qmk_firmware that referenced this pull request Jun 13, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
thorstenweber83 pushed a commit to thorstenweber83/qmk_firmware that referenced this pull request Sep 2, 2020
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Change _delay_ms/us() to wait_ms/us()

* Switch to platform-agnostic GPIO macros

* Add AVR spi_master and migrate Adafruit BLE code

* Set verbose back to false

* Add clock divisor, bit order and SPI mode configuration for init

* Add start and stop functions

* Move configuration of mode, endianness and speed to `spi_start()`

* Some breaks here would be good

* Default Adafruit BLE clock divisor to 4 (2MHz on the Feather 32U4)

* Remove mode and divisor enums

* Add some docs

* No hr at EOF

* Add links in sidebar
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.

Changing SS Pin for SPI Bluetooth Communication
6 participants