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

drivers: i2c: Move "Use 10-bit addressing" option to i2c_msg #9334

Merged
merged 2 commits into from
Sep 18, 2018

Conversation

Mierunski
Copy link
Collaborator

@Mierunski Mierunski commented Aug 8, 2018

Fixes #3806
Added new message flag I2C_MSG_ADDR_10_BITS while keeping the old flag for 10 bit addressing for backward compatibility.
Modified nrf i2c driver to check this flag instead, now all other drivers should be modified in the same way.

@Mierunski Mierunski requested review from galak and anangl August 8, 2018 07:51
@codecov-io
Copy link

codecov-io commented Aug 8, 2018

Codecov Report

Merging #9334 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9334   +/-   ##
=======================================
  Coverage   52.28%   52.28%           
=======================================
  Files         213      213           
  Lines       25964    25964           
  Branches     5598     5598           
=======================================
  Hits        13576    13576           
  Misses      10150    10150           
  Partials     2238     2238

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38a7950...af6c270. Read the comment docs.

@@ -90,10 +94,6 @@ static int i2c_nrfx_twi_configure(struct device *dev, u32_t dev_config)
{
nrfx_twi_t const *inst = &(get_dev_config(dev)->twi);

if (I2C_ADDR_10_BITS & dev_config) {
return -EINVAL;
Copy link
Member

Choose a reason for hiding this comment

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

I think this should stay. As long as the flag is available in the API, though it is marked as deprecated, it should be handled correctly to keep the backward compatibility.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added it back

@Mierunski
Copy link
Collaborator Author

@carlescufi @nashif Could we get this merged?

Mieszko Mierunski added 2 commits August 30, 2018 15:47
This commit adds I2C_MSG_ADDR_10_BITS flag, while keeping
I2C_ADDR_10_BITS for backward compatibility.

I2C_ADDR_10_BITS flag should be removed as soon as all drivers
are modified to use message flag.

Signed-off-by: Mieszko Mierunski <[email protected]>
10 bit addressing is still not supported on nrf chips, but now
I2C_MSG_ADDR_10_BITS flag is checked.

Signed-off-by: Mieszko Mierunski <[email protected]>
@@ -52,7 +52,7 @@ extern "C" {
#define I2C_SPEED_GET(cfg) (((cfg) & I2C_SPEED_MASK) \
>> I2C_SPEED_SHIFT)

/** Use 10-bit addressing. */
/** Use 10-bit addressing. DEPRECATED - Use I2C_MSG_ADDR_10_BITS instead. */
#define I2C_ADDR_10_BITS (1 << 0)
Copy link
Member

Choose a reason for hiding this comment

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

I2C_SLAVE_FLAGS_ADDR_10_BITS references this macro in line 66 (I can't put the comment there). I guess (1 << 0) should be used there directly.

@carlescufi
Copy link
Member

@MaureenHelm @galak can you take a look at this one?

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

What about all the other controllers? I wondering if we should say if the controller supports the feature at all, and than handle the error checking there?

@nashif nashif merged commit fd04e7d into zephyrproject-rtos:master Sep 18, 2018
@Mierunski Mierunski deleted the i2c_10_bit_addr branch September 24, 2018 06:47
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.

8 participants