This is an automated email from the ASF dual-hosted git repository. cederom pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 30735584a4d8a3287b7d206792c5a29586d05997 Author: Jorge Guzman <[email protected]> AuthorDate: Fri Jul 24 14:11:36 2026 -0300 risc-v/gd32vw55x: fix I2C master and add gd32vw553k-start sht3x example The GD32VW55x I2C master (the STM32-style "v2" IP) never completed a real transfer. Two bugs: 1. Wrong kernel clock. The protocol state machine is clocked by the I2C kernel clock selected in RCU_CFG1.I2C0SEL, not by the APB1 bus clock that only feeds the register interface. The driver left it at the APB1 default and computed TIMING for PCLK1 (~80 MHz); the resulting prescaled period is so short that the SDADEL/SCLDEL setup and hold times fall below the analog-filter minimum of the IP, so the master latches START but never drives SCL (STAT stuck with BUSY set). Route I2C0 to IRC16M (16 MHz) and use it as clk_freq, and never let the prescaler drop below the value that keeps the prescaled clock at/under 4 MHz (250 ns) so SDADEL/SCLDEL stay in spec. This matches the vendor BSP (IRC16M, PSC=3). 2. Transfer timeout was zero. CONFIG_GD32VW55X_I2C_TIMEOTICKS has a Kconfig default of 0 ("override when non-zero"), but the driver derived the timeout with a plain #ifndef, which never triggers because the symbol is always defined. The polled wait loop therefore ran a single iteration and gave up. Address probes (NACK on the first pass) still worked and masked it; only multi-byte transfers exercised the loop. Honour the "0 means derive from seconds/milliseconds" contract. Also make the interrupt wait immune to the ISR completing between startmsg() and the wait (do not clobber a posted DONE), and drop a redundant cast. To exercise this on hardware, add an "sht3x" configuration to the gd32vw553k-start: the nsh base plus I2C0, the i2ctool and the Sensirion SHT3x temperature/humidity driver, registered as /dev/i2c0 and /dev/temp0. I2C0 is routed to PA2 (SCL) / PA3 (SDA) on AF4, the pins broken out on the J1 header (datasheet Table 2-5); when SPI is enabled it claims PA2, so I2C0 falls back to PB0/PB1, whose SDA pin is not broken out (the periph case). Signed-off-by: Jorge Guzman <[email protected]> --- .../gd32vw55x/boards/gd32vw553k-start/index.rst | 44 ++++++++++ arch/risc-v/src/gd32vw55x/gd32vw55x_i2c.c | 94 +++++++++++++++++----- .../gd32vw553k-start/configs/sht3x/defconfig | 57 +++++++++++++ .../gd32vw55x/gd32vw553k-start/include/board.h | 16 +++- .../gd32vw55x/gd32vw553k-start/src/gd32_bringup.c | 29 +++++-- 5 files changed, 211 insertions(+), 29 deletions(-) diff --git a/Documentation/platforms/risc-v/gd32vw55x/boards/gd32vw553k-start/index.rst b/Documentation/platforms/risc-v/gd32vw55x/boards/gd32vw553k-start/index.rst index 896eb68eebc..73906282b7e 100644 --- a/Documentation/platforms/risc-v/gd32vw55x/boards/gd32vw553k-start/index.rst +++ b/Documentation/platforms/risc-v/gd32vw55x/boards/gd32vw553k-start/index.rst @@ -194,6 +194,50 @@ capture, both watchdogs, progmem, TRNG and CRC), registered as ``/dev/i2c0``, ``/dev/spi0``, ``/dev/adc0``, ``/dev/pwm0``, ``/dev/capture0``, ``/dev/watchdog0`` and ``/dev/watchdog1``. No radio. +sht3x +----- + +NSH plus I2C0, the i2ctool and the Sensirion SHT3x temperature/humidity +driver -- a small example to validate the I2C master port against a real +device. Because SPI is *off* here, I2C0 is routed to the J1 header on +**PA2 (SCL)** and **PA3 (SDA)** (adjacent pins on the second row of J1, both +AF4); with SPI on (the ``periph`` case) it falls back to PB0/PB1, whose SDA +pin is not broken out. See the pin-header tables above. + +Wire the sensor to J1: SCL -> PA2, SDA -> PA3, VCC -> +3V3 (J2) or the module +3.3 V, GND -> GND. A breakout with its own pull-ups is assumed; the pins are +configured with the internal pull-ups as a fallback. + +The bus is registered as ``/dev/i2c0`` and the sensor as ``/dev/temp0`` +(``gd32_bringup()`` registers it at address 0x44). Confirm the address with +the i2ctool using a zero-byte write probe (``-z``); the SHT3x NACKs the +default one-byte read probe when it has no measurement pending, so plain +``i2c dev`` would not show it:: + + nsh> i2c dev -z 0x03 0x77 + 0 1 2 3 4 5 6 7 8 9 a b c d e f + 00: -- -- -- -- -- -- -- -- -- -- -- -- + 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- + 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- + 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- + 40: -- -- -- -- 44 -- -- -- -- -- -- -- -- -- -- -- + 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- + 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- + 70: -- -- -- -- -- -- -- -- + +The ``44`` above is the SHT3x acknowledging. Then read it through the +driver:: + + nsh> sht3x + Converting... + Temperature = 26.530098 + Humidity = 47.998001 + +.. note:: + If the sensor sits at 0x45, change the address passed to + ``sht3x_register()`` in ``gd32_bringup.c`` (the i2ctool still finds it at + its real address regardless). + littlefs -------- diff --git a/arch/risc-v/src/gd32vw55x/gd32vw55x_i2c.c b/arch/risc-v/src/gd32vw55x/gd32vw55x_i2c.c index aaaa23189a4..20cb186f636 100644 --- a/arch/risc-v/src/gd32vw55x/gd32vw55x_i2c.c +++ b/arch/risc-v/src/gd32vw55x/gd32vw55x_i2c.c @@ -80,8 +80,16 @@ # define CONFIG_GD32VW55X_I2C_TIMEOMS (500) #endif -#ifndef CONFIG_GD32VW55X_I2C_TIMEOTICKS -# define CONFIG_GD32VW55X_I2C_TIMEOTICKS \ +/* CONFIG_GD32VW55X_I2C_TIMEOTICKS overrides the seconds/milliseconds pair + * only when non-zero (its Kconfig default is 0); otherwise the timeout is + * derived from the seconds/milliseconds values. Note the Kconfig symbol is + * always defined, so a plain #ifndef would leave it at the useless 0. + */ + +#if CONFIG_GD32VW55X_I2C_TIMEOTICKS != 0 +# define I2C_TIMEOUT_TICKS CONFIG_GD32VW55X_I2C_TIMEOTICKS +#else +# define I2C_TIMEOUT_TICKS \ (SEC2TICK(CONFIG_GD32VW55X_I2C_TIMEOSEC) + \ MSEC2TICK(CONFIG_GD32VW55X_I2C_TIMEOMS)) #endif @@ -113,6 +121,15 @@ #define I2C_SCLDELY_VALUE (4) #define I2C_SDADELY_VALUE (2) +/* Lower bound for the prescaled clock (4 MHz -> 250 ns period). The + * prescaler is never allowed below this, so the SDADEL/SCLDEL setup and hold + * times stay above the analog-filter minimum of the IP; otherwise the master + * hangs before the first clock. At the 16 MHz kernel clock this forces + * PSC = 3, as in the vendor BSP. + */ + +#define I2C_PRESC_FREQ_MAX (4000000) + /**************************************************************************** * Private Types ****************************************************************************/ @@ -218,7 +235,7 @@ static const struct i2c_ops_s g_i2c_ops = static const struct gd32_i2c_config_s g_i2c0_config = { .i2c_base = GD32VW55X_I2C0_BASE, - .clk_freq = GD32VW55X_PCLK1_FREQ, + .clk_freq = 16000000, /* IRC16M kernel clock */ .scl_pin = GPIO_I2C0_SCL, .sda_pin = GPIO_I2C0_SDA, .rcu_en = RCU_APB1EN_I2C0EN, @@ -252,7 +269,7 @@ static struct gd32_i2c_priv_s g_i2c0_priv = static const struct gd32_i2c_config_s g_i2c1_config = { .i2c_base = GD32VW55X_I2C1_BASE, - .clk_freq = GD32VW55X_PCLK1_FREQ, + .clk_freq = GD32VW55X_PCLK1_FREQ, /* I2C1 has no clock mux: CK_APB1 */ .scl_pin = GPIO_I2C1_SCL, .sda_pin = GPIO_I2C1_SDA, .rcu_en = RCU_APB1EN_I2C1EN, @@ -352,23 +369,30 @@ static int gd32_i2c_sem_waitdone(struct gd32_i2c_priv_s *priv) /* Signal the interrupt handler that we are waiting. Interrupts are * currently disabled but will be re-enabled while - * nxsem_tickwait_uninterruptible() sleeps. + * nxsem_tickwait_uninterruptible() sleeps. Only arm the wait if the ISR + * has not already completed the transfer between startmsg() and here -- + * otherwise WAITING would clobber a DONE that was already posted and the + * wait would block until the timeout. */ - priv->intstate = INTSTATE_WAITING; - - do + ret = OK; + if (priv->intstate != INTSTATE_DONE) { - ret = nxsem_tickwait_uninterruptible(&priv->sem_isr, - CONFIG_GD32VW55X_I2C_TIMEOTICKS); - if (ret < 0) + priv->intstate = INTSTATE_WAITING; + + do { - /* Break out of the loop on irrecoverable errors */ + ret = nxsem_tickwait_uninterruptible(&priv->sem_isr, + I2C_TIMEOUT_TICKS); + if (ret < 0) + { + /* Break out of the loop on irrecoverable errors */ - break; + break; + } } + while (priv->intstate != INTSTATE_DONE); } - while (priv->intstate != INTSTATE_DONE); /* Disable the I2C interrupts */ @@ -386,17 +410,19 @@ static int gd32_i2c_sem_waitdone(struct gd32_i2c_priv_s *priv) clock_t start; clock_t elapsed; - timeout = CONFIG_GD32VW55X_I2C_TIMEOTICKS; + timeout = I2C_TIMEOUT_TICKS; start = clock_systime_ticks(); priv->intstate = INTSTATE_WAITING; do { - /* Poll by simply calling the timer interrupt handler with the - * interrupts disabled. + /* Poll by simply calling the interrupt handler. A short pause keeps + * the polling from hammering the status register faster than the bus + * can advance between events. */ gd32_i2c_isr_process(priv); + up_udelay(2); elapsed = clock_systime_ticks() - start; } @@ -479,9 +505,18 @@ static void gd32_i2c_setclock(struct gd32_i2c_priv_s *priv, i2cclk = priv->config->clk_freq; - /* Find the smallest prescaler that keeps SCLL and SCLH in range */ + /* Find the smallest prescaler that keeps SCLL and SCLH in range, but never + * below the one that bounds the prescaled clock to I2C_PRESC_FREQ_MAX (see + * that definition for why). + */ + + psc = 0; + if (i2cclk > I2C_PRESC_FREQ_MAX) + { + psc = (i2cclk + I2C_PRESC_FREQ_MAX - 1) / I2C_PRESC_FREQ_MAX - 1; + } - for (psc = 0; psc < 16; psc++) + for (; psc < 16; psc++) { cycles = (i2cclk / (psc + 1)) / frequency; @@ -742,9 +777,8 @@ static int gd32_i2c_isr_process(struct gd32_i2c_priv_s *priv) if ((status & I2C_STAT_RBNE) != 0 && priv->dcnt > 0) { - *priv->ptr++ = (uint8_t)(gd32_i2c_getreg(priv, - GD32VW55X_I2C_RDATA_OFFSET) & - I2C_RDATA_MASK); + *priv->ptr++ = gd32_i2c_getreg(priv, GD32VW55X_I2C_RDATA_OFFSET) & + I2C_RDATA_MASK; priv->dcnt--; } @@ -857,6 +891,22 @@ static int gd32_i2c_init(struct gd32_i2c_priv_s *priv) modifyreg32(GD32VW55X_RCU_APB1EN, 0, config->rcu_en); + /* Source the I2C0 kernel clock from IRC16M (16 MHz). The protocol state + * machine is clocked by this kernel clock (not the APB1 bus clock that + * feeds the register interface). With the APB1 default the prescaled + * period is so short that the SDADEL/SCLDEL setup and hold times fall + * below the analog-filter minimum of this I2C v2 IP, and the master hangs + * with the START bit set and BUSY never asserting. IRC16M gives a fixed, + * in-spec 16 MHz reference (matching the vendor BSP), which is why + * clk_freq is 16 MHz. + */ + + if (config->i2c_base == GD32VW55X_I2C0_BASE) + { + modifyreg32(GD32VW55X_RCU_CFG1, RCU_CFG1_I2C0SEL_MASK, + RCU_CFG1_I2C0SEL_IRC16M); + } + /* Reset the peripheral to be sure that it is in a known state */ modifyreg32(GD32VW55X_RCU_APB1RST, 0, config->rcu_rst); diff --git a/boards/risc-v/gd32vw55x/gd32vw553k-start/configs/sht3x/defconfig b/boards/risc-v/gd32vw55x/gd32vw553k-start/configs/sht3x/defconfig new file mode 100644 index 00000000000..da64b0b57cf --- /dev/null +++ b/boards/risc-v/gd32vw55x/gd32vw553k-start/configs/sht3x/defconfig @@ -0,0 +1,57 @@ +# +# This file is autogenerated: PLEASE DO NOT EDIT IT. +# +# You can use "make menuconfig" to make any modifications to the installed .config file. +# You can then do "make savedefconfig" to generate a new defconfig file that includes your +# modifications. +# +# CONFIG_ARCH_LEDS is not set +CONFIG_ARCH="risc-v" +CONFIG_ARCH_BOARD="gd32vw553k-start" +CONFIG_ARCH_BOARD_GD32VW553K_START=y +CONFIG_ARCH_CHIP="gd32vw55x" +CONFIG_ARCH_CHIP_GD32VW553KM=y +CONFIG_ARCH_CHIP_GD32VW55X=y +CONFIG_ARCH_INTERRUPTSTACK=2048 +CONFIG_ARCH_RISCV=y +CONFIG_ARCH_STACKDUMP=y +CONFIG_BOARDCTL_RESET=y +CONFIG_BOARD_LOOPSPERMSEC=16000 +CONFIG_BUILTIN=y +CONFIG_DEBUG_FEATURES=y +CONFIG_DEBUG_FULLOPT=y +CONFIG_DEBUG_SYMBOLS=y +CONFIG_EXAMPLES_HELLO=y +CONFIG_EXAMPLES_SHT3X=y +CONFIG_FS_PROCFS=y +CONFIG_GD32VW55X_I2C0=y +CONFIG_GD32VW55X_UART2=y +CONFIG_I2C_POLLED=y +CONFIG_IDLETHREAD_STACKSIZE=2048 +CONFIG_INIT_ENTRYPOINT="nsh_main" +CONFIG_INIT_STACKSIZE=3072 +CONFIG_INTELHEX_BINARY=y +CONFIG_LIBC_PERROR_STDOUT=y +CONFIG_LIBC_STRERROR=y +CONFIG_NFILE_DESCRIPTORS_PER_BLOCK=6 +CONFIG_NSH_BUILTIN_APPS=y +CONFIG_NSH_FILEIOSIZE=64 +CONFIG_NSH_READLINE=y +CONFIG_NSH_STRERROR=y +CONFIG_RAM_SIZE=294400 +CONFIG_RAM_START=0x20000200 +CONFIG_RAW_BINARY=y +CONFIG_RR_INTERVAL=200 +CONFIG_SCHED_WAITPID=y +CONFIG_SENSORS=y +CONFIG_SENSORS_SHT3X=y +CONFIG_STACK_COLORATION=y +CONFIG_START_DAY=13 +CONFIG_START_MONTH=7 +CONFIG_START_YEAR=2026 +CONFIG_SYSTEM_I2CTOOL=y +CONFIG_SYSTEM_NSH=y +CONFIG_TASK_NAME_SIZE=12 +CONFIG_UART2_RXBUFSIZE=128 +CONFIG_UART2_SERIAL_CONSOLE=y +CONFIG_UART2_TXBUFSIZE=128 diff --git a/boards/risc-v/gd32vw55x/gd32vw553k-start/include/board.h b/boards/risc-v/gd32vw55x/gd32vw553k-start/include/board.h index 66ee1d6b107..f5760c59eb6 100644 --- a/boards/risc-v/gd32vw55x/gd32vw553k-start/include/board.h +++ b/boards/risc-v/gd32vw55x/gd32vw553k-start/include/board.h @@ -73,9 +73,21 @@ # define GPIO_SPI_MOSI GPIO_SPI_MOSI_1 /* PA0, AF5 */ #endif +/* I2C0 uses PA2 (SCL) / PA3 (SDA) on AF4, the pins broken out on the J1 + * header (datasheet Table 2-5), so a sensor such as the SHT3x can be wired + * there directly. When SPI is enabled it claims PA2 (SPI SCK), so I2C0 + * falls back to PB0/PB1 (AF6); PB1 (SDA) is not broken out on J1/J2, so that + * pairing only registers the bus (the periph case). + */ + #ifdef CONFIG_GD32VW55X_I2C0 -# define GPIO_I2C0_SCL GPIO_I2C0_SCL_3 /* PB0, AF6 */ -# define GPIO_I2C0_SDA GPIO_I2C0_SDA_3 /* PB1, AF6 */ +# ifdef CONFIG_GD32VW55X_SPI +# define GPIO_I2C0_SCL GPIO_I2C0_SCL_3 /* PB0, AF6 */ +# define GPIO_I2C0_SDA GPIO_I2C0_SDA_3 /* PB1, AF6 (not on a header) */ +# else +# define GPIO_I2C0_SCL GPIO_I2C0_SCL_1 /* PA2, AF4 (J1) */ +# define GPIO_I2C0_SDA GPIO_I2C0_SDA_1 /* PA3, AF4 (J1) */ +# endif #endif #ifdef CONFIG_GD32VW55X_I2C1 diff --git a/boards/risc-v/gd32vw55x/gd32vw553k-start/src/gd32_bringup.c b/boards/risc-v/gd32vw55x/gd32vw553k-start/src/gd32_bringup.c index 0f4b5f8f11f..939e36e8c74 100644 --- a/boards/risc-v/gd32vw55x/gd32vw553k-start/src/gd32_bringup.c +++ b/boards/risc-v/gd32vw55x/gd32vw553k-start/src/gd32_bringup.c @@ -49,6 +49,9 @@ #ifdef CONFIG_USERLED_LOWER # include <nuttx/leds/userled.h> #endif +#ifdef CONFIG_SENSORS_SHT3X +# include <nuttx/sensors/sht3x.h> +#endif #ifdef CONFIG_MTD_PROGMEM # include <nuttx/mtd/mtd.h> #endif @@ -142,13 +145,29 @@ int gd32_bringup(void) { ferr("ERROR: failed to initialize I2C0\n"); } -#ifdef CONFIG_I2C_DRIVER - else if (i2c_register(i2c, 0) < 0) + else { - ferr("ERROR: failed to register /dev/i2c0\n"); - gd32_i2cbus_uninitialize(i2c); - } +#ifdef CONFIG_I2C_DRIVER + /* Expose the bus as /dev/i2c0 for the i2ctool (i2c dev/get/set...) */ + + if (i2c_register(i2c, 0) < 0) + { + ferr("ERROR: failed to register /dev/i2c0\n"); + } #endif + +#ifdef CONFIG_SENSORS_SHT3X + /* Sensirion SHT3x on I2C0 -> /dev/temp0. The default I2C address is + * 0x44 (0x45 with ADDR tied high); confirm it first with + * "i2c dev 0x44 0x45" from NSH. + */ + + if (sht3x_register("/dev/temp0", i2c, 0x44) < 0) + { + ferr("ERROR: failed to register SHT3x at 0x44 on I2C0\n"); + } +#endif + } #endif #ifdef CONFIG_GD32VW55X_SPI
