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

i2c: introduce struct i2c_dt_spec #37359

Merged
merged 4 commits into from
Aug 3, 2021
Merged

i2c: introduce struct i2c_dt_spec #37359

merged 4 commits into from
Aug 3, 2021

Conversation

JordanYates
Copy link
Collaborator

Introduces the struct i2c_dt_spec type, which contains the complete I2C bus information derived from devicetree. It serves the same purpose as struct spi_dt_spec in that it can be constructed automatically in DEVICE_DT_INST_DEFINE macros and provided as a single handle to I2C API calls.

It encapsulates less information than struct spi_dt_spec, but it still reduces boilerplate and chances for error in device drivers.
It also allows the following pattern to be used for drivers that support both SPI and I2C buses.

struct device_config {
    union {
        struct spi_dt_spec spi;
        struct i2c_dt_spec i2c;
    };
};

@github-actions github-actions bot added the area: API Changes to public APIs label Aug 1, 2021
@JordanYates
Copy link
Collaborator Author

If an i2c_is_ready function is desired (equivalent to spi_is_ready) I can add that.
Currently drivers need to dereference an additional layer:

int device_init(const struct device *dev) {
    const struct device_config *config = dev->config;

    if (!device_is_ready(config->bus.bus)) {
        ...
    }
}

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Putting a -1 on to give me time to review, which I am doing right now.

Introduces the `struct i2c_dt_spec` type, which contains the complete
I2c bus information derived from devicetree. It serves the same purpose
as `struct spi_dt_spec` in that it can be constructed automatically in
`DEVICE_DT_INST_DEFINE` macros and provided as a single handle to I2C
API calls. While I2C has much less instance configuration than SPI, this
is still useful to enable the following pattern in device drivers that
support both I2C and SPI comms:

```
struct config {
    union {
        struct spi_dt_spec spi;
        struct i2c_dt_spec i2c;
    };
};
```

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Jordan Yates added 3 commits August 3, 2021 19:52
Add variants of all i2c transfer functions that accept an `i2c_dt_spec`
as the bus specifier. This helps reduce code duplication in device
drivers.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Convert bme680 driver to `struct i2c_dt_spec` as a demonstration.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Convert sht3xd driver to `struct i2c_dt_spec` as a demonstration.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

struct device_config {
    union {
        struct spi_dt_spec spi;
        struct i2c_dt_spec i2c;
    };
};

Regarding this pattern from the PR description, I'm guessing you're omitting the extra ifdeffery for the sake of clarity.

When you're going to do it "for real" I assume you'll do something similar to what's in bme280.h that avoids wasting flash on room for a SPI config in the case of I2C-only systems, like this:

#define BME280_BUS_SPI DT_ANY_INST_ON_BUS_STATUS_OKAY(spi)
#define BME280_BUS_I2C DT_ANY_INST_ON_BUS_STATUS_OKAY(i2c)

union bme280_bus_config {
#if BME280_BUS_SPI
	struct spi_config spi_cfg;
#endif
#if BME280_BUS_I2C
	uint16_t i2c_addr;
#endif
};

That is, you could do

union bme280_bus_config {
#if BME280_BUS_SPI
	struct spi_dt_spec spi_spec;
#endif
#if BME280_BUS_I2C
	struct i2c_dt_spec i2c_spec;
#endif
};

And then remove the bus pointer from struct bme280_config and access it through the spec structure instead or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: I2C area: Sensors Sensors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants