drivers: gpio: Introduce SC18IS606 GPIO Extension drivers #100743

zacck · 2025-12-09T13:24:08Z

Introduce I2C to SPI bridge driver GPIO support for the SC18IS6906 bridge from NXP.

This adds an GPIO controller available on the bridge from NXP. Ideally this enables one to add an GPIO extension through i2c

zacck · 2025-12-09T13:50:39Z

Odd, tests and compliance are passing for me locally, Compliance is failing on a line 239 of a 234 line file too.

drivers/gpio/gpio_sc18is606.c

zacck · 2025-12-10T06:09:05Z

@kartben sorry for the ping but usually you can really see where I have messed up, please tell me how I could fix these compliance fails. Locally the test passes and the lines being complained about no longer exist, thank you so much for the time!

Added a driver implementation for the GPIO Controller on the sc18is606 Signed-off-by: Zacck Osiemo <[email protected]>

Apply device tree overlay for SC18IS606 Signed-off-by: Zacck Osiemo <[email protected]>

sonarqubecloud · 2025-12-11T14:33:27Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zacck · 2025-12-11T14:35:29Z

@mmahadevan108 it would be awesome if I could get your review on this please, Kind Regards

Copilot

Pull request overview

This PR introduces GPIO controller support for the NXP SC18IS606 I2C to SPI bridge device, enabling GPIO extension functionality through I2C.

Key Changes:

New GPIO driver implementation supporting 3 GPIO pins with configurable modes (input, push-pull output, open-drain output)
Device tree bindings and example overlays for the GPIO controller
Build system integration (Kconfig and CMakeLists)

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`drivers/gpio/gpio_sc18is606.c`	Main GPIO driver implementation with pin configuration, read/write operations, and hardware interface
`drivers/gpio/Kconfig.sc18is606`	Kconfig configuration for the GPIO driver with dependency on SPI_SC18IS606
`drivers/gpio/Kconfig`	Integration of the new Kconfig file into the GPIO subsystem
`drivers/gpio/CMakeLists.txt`	Build system integration to compile the driver when enabled
`dts/bindings/gpio/nxp,sc18is606-gpio.yaml`	Device tree binding definition for the GPIO controller
`dts/bindings/mfd/nxp,sc18is606.yaml`	Example usage in the MFD parent binding documentation
`tests/drivers/build_all/mfd/app.overlay`	Test overlay demonstrating GPIO controller instantiation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-14T07:49:04Z

drivers/gpio/Kconfig.sc18is606

@@ -0,0 +1,18 @@
+# Copyright (c) 2025 tinyvision.ai SPDX-License-Identifier: Apache-2.0


Missing space after copyright notice. There should be a space between "tinyvision.ai" and "SPDX-License-Identifier" on this line.

Copilot · 2025-12-14T07:49:05Z

drivers/gpio/gpio_sc18is606.c

+		LOG_ERR("Failed to enable GPIO (%d)", ret);
+	}
+
+	data->conf |= pin_conf << (pin * 2);


The configuration data stored in data->conf is never cleared for the specific pin before setting new configuration. This means old configuration bits will remain set when reconfiguring a pin. The correct approach is to first clear the two-bit field for the pin being configured, then set the new value.

Suggested change

data->conf |= pin_conf << (pin * 2);

data->conf &= ~(SC18IS606_GPIO_CONF_MASK << (pin * 2));

data->conf |= (pin_conf & SC18IS606_GPIO_CONF_MASK) << (pin * 2);

Copilot · 2025-12-14T07:49:05Z

drivers/gpio/gpio_sc18is606.c

+	ret = nxp_sc18is606_transfer(cfg->bridge, enable_buf, sizeof(enable_buf), NULL, 0, NULL);
+	if (ret < 0) {
+		LOG_ERR("Failed to enable GPIO (%d)", ret);
+	}


The bridge is claimed but not released if the enable GPIO transfer fails. This creates a resource leak where the bridge remains locked. The release should occur in all error paths, or error handling should properly unwind the claim.

Copilot · 2025-12-14T07:49:05Z

drivers/gpio/gpio_sc18is606.c

+	ret = nxp_sc18is606_transfer(cfg->bridge, buf, sizeof(buf), NULL, 0, NULL);
+	if (ret < 0) {
+		LOG_ERR("Failed to configure GPIO (%d)", ret);
+	}


The bridge is not released if the configure GPIO transfer fails. This creates a resource leak where the bridge remains locked. The release at line 160 will only execute if this function completes normally, but not if an early return occurs due to error.

Copilot · 2025-12-14T07:49:06Z

drivers/gpio/gpio_sc18is606.c

+	/*current port state*/
+	uint8_t output_state;
+
+	/*current port pin config*/


Missing space in comment. Should be "current port pin config" with spaces after the comment markers.

Suggested change

/*current port pin config*/

/* current port pin config */

Copilot · 2025-12-14T07:49:07Z

drivers/gpio/gpio_sc18is606.c

+	/*current port state*/
+	uint8_t output_state;
+
+	/*current port pin config*/


Missing space in comment. Should be "current port state" instead of running the comment directly into the code.

Suggested change

/*current port state*/

uint8_t output_state;

/*current port pin config*/

/* current port state */

uint8_t output_state;

/* current port pin config */

Copilot · 2025-12-14T07:49:07Z

drivers/gpio/gpio_sc18is606.c

+		}
+		if (flags & GPIO_OUTPUT_INIT_LOW) {


Both GPIO_OUTPUT_INIT_HIGH and GPIO_OUTPUT_INIT_LOW could theoretically be set together (though unlikely). The current implementation would set the pin high first, then low. Consider making these mutually exclusive checks with an else-if statement to clarify the intended behavior.

Suggested change

}

if (flags & GPIO_OUTPUT_INIT_LOW) {

} else if (flags & GPIO_OUTPUT_INIT_LOW) {

Copilot · 2025-12-14T07:49:07Z

dts/bindings/gpio/nxp,sc18is606-gpio.yaml

+# SPDX-License-Identifier: Apache-2.0
+
+
+description: GPIO Controller part for the SC18IS606 bridge


Inconsistent documentation format: The description should end with a period for consistency with standard documentation practices.

Suggested change

description: GPIO Controller part for the SC18IS606 bridge

description: GPIO Controller part for the SC18IS606 bridge.

Copilot · 2025-12-14T07:49:07Z

drivers/gpio/gpio_sc18is606.c

+	data->conf |= pin_conf << (pin * 2);
+	buf[1] = data->conf;


Potential race condition: The pin configuration function modifies data->conf without any locking mechanism. If multiple threads call gpio_sc18is606_pin_configure concurrently for different pins, they could overwrite each other's configuration updates since data->conf is shared state. The claim/release only protects the hardware access, not the software state.

Copilot · 2025-12-14T07:49:08Z

drivers/gpio/gpio_sc18is606.c

+	buf[1] &= ~mask;
+	buf[1] |= (value & mask);
+	buf[1] ^= toggle;
+
+	ret = nxp_sc18is606_transfer(cfg->bridge, buf, sizeof(buf), NULL, 0, NULL);
+
+	if (ret < 0) {
+		LOG_ERR("Failed to write to GPIO (%d)", ret);
+		return ret;
+	}
+
+	data->output_state = buf[1];


Potential race condition: The output_state is read, modified, and written without proper synchronization. If multiple threads call port_set_raw concurrently, they could overwrite each other's updates since they all read the same initial value of data->output_state before applying their changes.

zephyrbot added area: GPIO area: MFD area: Devicetree Bindings area: Boards/SoCs area: Tests Issues related to a particular existing or missing test platform: NXP NXP labels Dec 9, 2025

zephyrbot requested review from Holt-Sun, aasinclair, dbaluta, henrikbrixandersen, manuargue, mmahadevan108, nashif, nordic-auko, seov-nordic and zejiang0jason December 9, 2025 13:27

zephyrbot assigned mmahadevan108 Dec 9, 2025

zacck force-pushed the pr-sc18is606-gpio branch from f45ac52 to 8461744 Compare December 9, 2025 13:36

Holt-Sun reviewed Dec 9, 2025

View reviewed changes

drivers/gpio/gpio_sc18is606.c Show resolved Hide resolved

drivers/gpio/gpio_sc18is606.c Outdated Show resolved Hide resolved

zacck force-pushed the pr-sc18is606-gpio branch from 8461744 to 37a0517 Compare December 9, 2025 15:24

zacck force-pushed the pr-sc18is606-gpio branch from 37a0517 to fffe960 Compare December 11, 2025 14:16

zacck added 2 commits December 11, 2025 16:29

drivers: gpio: Introduce SC18IS606 GPIO Controller

fd53956

Added a driver implementation for the GPIO Controller on the sc18is606 Signed-off-by: Zacck Osiemo <[email protected]>

tests: gpio: Add overlay for sc18is606 gpio

2ec1456

Apply device tree overlay for SC18IS606 Signed-off-by: Zacck Osiemo <[email protected]>

zacck force-pushed the pr-sc18is606-gpio branch from fffe960 to 2ec1456 Compare December 11, 2025 14:29

zacck requested a review from Holt-Sun December 11, 2025 14:34

Holt-Sun approved these changes Dec 14, 2025

View reviewed changes

kartben requested a review from Copilot December 14, 2025 07:44

Copilot started reviewing on behalf of kartben December 14, 2025 07:44 View session

Copilot AI reviewed Dec 14, 2025

View reviewed changes

		@@ -0,0 +1,18 @@
		# Copyright (c) 2025 tinyvision.ai SPDX-License-Identifier: Apache-2.0

	data->conf \|= pin_conf << (pin * 2);
	data->conf &= ~(SC18IS606_GPIO_CONF_MASK << (pin * 2));
	data->conf \|= (pin_conf & SC18IS606_GPIO_CONF_MASK) << (pin * 2);

	}
	if (flags & GPIO_OUTPUT_INIT_LOW) {
	} else if (flags & GPIO_OUTPUT_INIT_LOW) {

		# SPDX-License-Identifier: Apache-2.0


		description: GPIO Controller part for the SC18IS606 bridge

drivers: gpio: Introduce SC18IS606 GPIO Extension drivers #100743

Are you sure you want to change the base?

drivers: gpio: Introduce SC18IS606 GPIO Extension drivers #100743

Uh oh!

Conversation

zacck commented Dec 9, 2025

Uh oh!

zacck commented Dec 9, 2025

Uh oh!

Uh oh!

Uh oh!

zacck commented Dec 10, 2025

Uh oh!

sonarqubecloud bot commented Dec 11, 2025

Quality Gate passed

Uh oh!

zacck commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants