Skip to content

Commit

Permalink
pw_spi_linux: Avoid unneccessary ioctl()s in Configure()
Browse files Browse the repository at this point in the history
Initiator::Configure() is called on every Device::WriteRead() call, in
case different devices on the bus use different configuration.

Avoid unnecessary ioctl() calls by recording the current configuration
and skipping Configure() if the new config is not actually different.

If the pw::spi::Initiator base class were using the non-virtual
interface (NVI) pattern, this could be implemented there and apply to
all initiator implementations.

Test: Verified downstream project still works and redundant ioctls no
      longer happen
Bug: 366541694
Change-Id: Ic173cd1f895ea398c7e33df8fe8b4a5b05d2e8cf
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/235877
Reviewed-by: Andrew Harper <aharp@google.com>
Commit-Queue: Jonathon Reinhart <jrreinhart@google.com>
Docs-Not-Needed: Jonathon Reinhart <jrreinhart@google.com>
Reviewed-by: Jesus Sanchez-Palencia <jesussanp@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
  • Loading branch information
JonathonReinhart authored and CQ Bot Account committed Sep 16, 2024
1 parent f66eafb commit eefd313
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pw_spi_linux/public/pw_spi_linux/spi.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#pragma once

#include <cstdint>
#include <optional>

#include "pw_bytes/span.h"
#include "pw_spi/chip_selector.h"
Expand All @@ -39,6 +40,7 @@ class LinuxInitiator : public Initiator {
private:
uint32_t max_speed_hz_;
int fd_;
std::optional<Config> current_config_;
};

// Linux userspace implementation of SPI ChipSelector
Expand Down
6 changes: 6 additions & 0 deletions pw_spi_linux/spi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ LinuxInitiator::~LinuxInitiator() {
}

Status LinuxInitiator::Configure(const Config& config) {
if (current_config_ == config) {
// Don't waste time issuing ioctls if the config is not actually changing.
return OkStatus();
}

// Map clock polarity/phase to Linux userspace equivalents
uint32_t mode = 0;
if (config.polarity == ClockPolarity::kActiveLow) {
Expand Down Expand Up @@ -70,6 +75,7 @@ Status LinuxInitiator::Configure(const Config& config) {
return Status::InvalidArgument();
}

current_config_ = config;
return OkStatus();
}

Expand Down

0 comments on commit eefd313

Please sign in to comment.