From 8ac3d392a229128bb558e39ee2f907302e1643ca Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Thu, 5 May 2022 14:43:36 +0800 Subject: [PATCH 1/2] I2C: Fix SCL period timings on ESP targets The output frequency is now more accurate as the SCL period timings have been fixed. This fix applies for ESP32, ESP32S3, ESP32C3 and ESP32H2 --- components/hal/esp32/include/hal/i2c_ll.h | 44 +++++++++++++-------- components/hal/esp32c3/include/hal/i2c_ll.h | 32 ++++++--------- components/hal/esp32h2/include/hal/i2c_ll.h | 14 +++---- components/hal/esp32s3/include/hal/i2c_ll.h | 34 +++++++--------- 4 files changed, 61 insertions(+), 63 deletions(-) diff --git a/components/hal/esp32/include/hal/i2c_ll.h b/components/hal/esp32/include/hal/i2c_ll.h index 650dbbaa04..be0640b24f 100644 --- a/components/hal/esp32/include/hal/i2c_ll.h +++ b/components/hal/esp32/include/hal/i2c_ll.h @@ -1,16 +1,8 @@ -// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ // The LL layer for I2C register operations @@ -125,9 +117,29 @@ static inline void i2c_ll_cal_bus_clk(uint32_t source_clk, uint32_t bus_freq, i2 */ static inline void i2c_ll_set_bus_timing(i2c_dev_t *hw, i2c_clk_cal_t *bus_cfg) { - //scl period - hw->scl_low_period.period = bus_cfg->scl_low; - hw->scl_high_period.period = bus_cfg->scl_high; + /* SCL period. According to the TRM, we should always subtract 1 to SCL low period */ + assert(bus_cfg->scl_low > 0); + hw->scl_low_period.period = bus_cfg->scl_low - 1; + /* Still according to the TRM, if filter is not enbled, we have to subtract 7, + * if SCL filter is enabled, we have to subtract: + * 8 if SCL filter is between 0 and 2 (included) + * 6 + SCL threshold if SCL filter is between 3 and 7 (included) + * to SCL high period */ + uint16_t scl_high = bus_cfg->scl_high; + /* In the "worst" case, we will subtract 13, make sure the result will still be correct */ + assert(scl_high > 13); + if (hw->scl_filter_cfg.en) { + if (hw->scl_filter_cfg.thres <= 2) { + scl_high -= 8; + } else if (hw->scl_filter_cfg.thres <= 7) { + scl_high -= hw->scl_filter_cfg.thres + 6; + } else { + assert(false); + } + } else { + scl_high -= 7; + } + hw->scl_high_period.period = scl_high; //sda sample hw->sda_hold.time = bus_cfg->sda_hold; hw->sda_sample.time = bus_cfg->sda_sample; diff --git a/components/hal/esp32c3/include/hal/i2c_ll.h b/components/hal/esp32c3/include/hal/i2c_ll.h index 31743005b6..a4b3662fb5 100644 --- a/components/hal/esp32c3/include/hal/i2c_ll.h +++ b/components/hal/esp32c3/include/hal/i2c_ll.h @@ -1,16 +1,8 @@ -// Copyright 2020 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ // The LL layer for I2C register operations @@ -159,17 +151,17 @@ static inline void i2c_ll_set_bus_timing(i2c_dev_t *hw, i2c_clk_cal_t *bus_cfg) { HAL_FORCE_MODIFY_U32_REG_FIELD(hw->clk_conf, sclk_div_num, bus_cfg->clkm_div - 1); //scl period - hw->scl_low_period.period = bus_cfg->scl_low - 1; - hw->scl_high_period.period = bus_cfg->scl_high; + hw->scl_low_period.period = bus_cfg->scl_low - 2; + hw->scl_high_period.period = bus_cfg->scl_high - 3; //sda sample - hw->sda_hold.time = bus_cfg->sda_hold; - hw->sda_sample.time = bus_cfg->sda_sample; + hw->sda_hold.time = bus_cfg->sda_hold - 1; + hw->sda_sample.time = bus_cfg->sda_sample - 1; //setup - hw->scl_rstart_setup.time = bus_cfg->setup; - hw->scl_stop_setup.time = bus_cfg->setup; + hw->scl_rstart_setup.time = bus_cfg->setup - 1; + hw->scl_stop_setup.time = bus_cfg->setup - 1; //hold hw->scl_start_hold.time = bus_cfg->hold - 1; - hw->scl_stop_hold.time = bus_cfg->hold; + hw->scl_stop_hold.time = bus_cfg->hold - 1; hw->timeout.time_out_value = bus_cfg->tout; hw->timeout.time_out_en = 1; } diff --git a/components/hal/esp32h2/include/hal/i2c_ll.h b/components/hal/esp32h2/include/hal/i2c_ll.h index 2e34e4a55c..8685551444 100644 --- a/components/hal/esp32h2/include/hal/i2c_ll.h +++ b/components/hal/esp32h2/include/hal/i2c_ll.h @@ -159,17 +159,17 @@ static inline void i2c_ll_set_bus_timing(i2c_dev_t *hw, i2c_clk_cal_t *bus_cfg) { HAL_FORCE_MODIFY_U32_REG_FIELD(hw->clk_conf, sclk_div_num, bus_cfg->clkm_div - 1); //scl period - hw->scl_low_period.period = bus_cfg->scl_low - 1; - hw->scl_high_period.period = bus_cfg->scl_high; + hw->scl_low_period.period = bus_cfg->scl_low - 2; + hw->scl_high_period.period = bus_cfg->scl_high - 3; //sda sample - hw->sda_hold.time = bus_cfg->sda_hold; - hw->sda_sample.time = bus_cfg->sda_sample; + hw->sda_hold.time = bus_cfg->sda_hold - 1; + hw->sda_sample.time = bus_cfg->sda_sample - 1; //setup - hw->scl_rstart_setup.time = bus_cfg->setup; - hw->scl_stop_setup.time = bus_cfg->setup; + hw->scl_rstart_setup.time = bus_cfg->setup - 1; + hw->scl_stop_setup.time = bus_cfg->setup - 1; //hold hw->scl_start_hold.time = bus_cfg->hold - 1; - hw->scl_stop_hold.time = bus_cfg->hold; + hw->scl_stop_hold.time = bus_cfg->hold - 1; hw->timeout.time_out_value = bus_cfg->tout; hw->timeout.time_out_en = 1; } diff --git a/components/hal/esp32s3/include/hal/i2c_ll.h b/components/hal/esp32s3/include/hal/i2c_ll.h index eae2a660fb..ec209daf2e 100644 --- a/components/hal/esp32s3/include/hal/i2c_ll.h +++ b/components/hal/esp32s3/include/hal/i2c_ll.h @@ -1,16 +1,8 @@ -// Copyright 2020 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ // The LL layer for I2C register operations @@ -152,18 +144,20 @@ static inline void i2c_ll_update(i2c_dev_t *hw) static inline void i2c_ll_set_bus_timing(i2c_dev_t *hw, i2c_clk_cal_t *bus_cfg) { HAL_FORCE_MODIFY_U32_REG_FIELD(hw->clk_conf, sclk_div_num, bus_cfg->clkm_div - 1); + /* According to the Technical Reference Manual, the following timings must be subtracted by 1. + * Moreover, the frequency calculation also shows that we must subtract 3 to the total SCL */ //scl period - hw->scl_low_period.scl_low_period = bus_cfg->scl_low - 1; - hw->scl_high_period.scl_high_period = bus_cfg->scl_high; + hw->scl_low_period.scl_low_period = bus_cfg->scl_low - 1 - 2; + hw->scl_high_period.scl_high_period = bus_cfg->scl_high - 1 - 1; //sda sample - hw->sda_hold.sda_hold_time = bus_cfg->sda_hold; - hw->sda_sample.sda_sample_time = bus_cfg->sda_sample; + hw->sda_hold.sda_hold_time = bus_cfg->sda_hold - 1; + hw->sda_sample.sda_sample_time = bus_cfg->sda_sample - 1; //setup - hw->scl_rstart_setup.scl_rstart_setup_time = bus_cfg->setup; - hw->scl_stop_setup.scl_stop_setup_time = bus_cfg->setup; + hw->scl_rstart_setup.scl_rstart_setup_time = bus_cfg->setup - 1; + hw->scl_stop_setup.scl_stop_setup_time = bus_cfg->setup - 1; //hold hw->scl_start_hold.scl_start_hold_time = bus_cfg->hold - 1; - hw->scl_stop_hold.scl_stop_hold_time = bus_cfg->hold; + hw->scl_stop_hold.scl_stop_hold_time = bus_cfg->hold - 1; hw->to.time_out_value = bus_cfg->tout; hw->to.time_out_en = 1; } From 279ec3eb35c37e0fc4c7bc61df3dbd82a1cc5373 Mon Sep 17 00:00:00 2001 From: Anton Maklakov Date: Mon, 23 May 2022 11:38:19 +0700 Subject: [PATCH 2/2] i2c: fix 'comparision is always true' warning --- components/hal/esp32/include/hal/i2c_ll.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/hal/esp32/include/hal/i2c_ll.h b/components/hal/esp32/include/hal/i2c_ll.h index be0640b24f..9a99341c59 100644 --- a/components/hal/esp32/include/hal/i2c_ll.h +++ b/components/hal/esp32/include/hal/i2c_ll.h @@ -131,10 +131,9 @@ static inline void i2c_ll_set_bus_timing(i2c_dev_t *hw, i2c_clk_cal_t *bus_cfg) if (hw->scl_filter_cfg.en) { if (hw->scl_filter_cfg.thres <= 2) { scl_high -= 8; - } else if (hw->scl_filter_cfg.thres <= 7) { - scl_high -= hw->scl_filter_cfg.thres + 6; } else { - assert(false); + assert(hw->scl_filter_cfg.thres <= 7); + scl_high -= hw->scl_filter_cfg.thres + 6; } } else { scl_high -= 7;