drivers: gpio: silabs-siwx91x: Add device runtime support #96491

smalae · 2025-09-24T12:50:55Z

Add the port_count field to the configuration structure and allocate different sizes for the ports array based
on whether the node is HP or ULP.
Add Device runtime PM for GPIO driver

jhedberg · 2025-09-24T14:00:00Z

@smalae for the future, please use titles that give enough context for the entire Zephyr project, as issues & PRs are visible to everyone. I fixed it now for this PR.

dts/bindings/gpio/silabs,siwx91x-gpio.yaml

asmellby · 2025-09-25T14:07:08Z

dts/arm/silabs/siwg917.dtsi


 			#address-cells = <1>;
 			#size-cells = <0>;
+			silabs,port-count = <4>;


Code should use DT_CHILD_NUM() instead of this being a property in dts

asmellby · 2025-09-25T14:13:05Z

dts/arm/silabs/siwg917.dtsi

 			#address-cells = <1>;
 			#size-cells = <0>;
+			silabs,port-count = <4>;
+			silabs,max-pin-usage-count = <8>;


Code should use the pin mask constructed from ngpios and gpio-reserved-ranges, which is already available as the .common.port_pin_mask member of the config struct on each port. No need for another dts property. If you need a compile time constant, it should be possible to do some macrobatics to create a number that is ngpios - popcnt(reserved_ranges).

Updated accordingly

asmellby · 2025-09-25T14:15:17Z

drivers/gpio/gpio_silabs_siwx91x.c

+		pdata->pin_config_info[pdata->cur_pin].port_dev = dev;
+		pdata->pin_config_info[pdata->cur_pin].pin = pin;
+		pdata->pin_config_info[pdata->cur_pin].flags = flags;
+		pdata->cur_pin++;


A pin can be configured multiple times, this needs to reuse an existing allocation if the same pin is reconfigured.

Add soc power domain and zephyr,pm-device-runtime-auto Signed-off-by: Sai Santhosh Malae <[email protected]>

jhedberg · 2025-10-06T11:24:05Z

drivers/gpio/gpio_silabs_siwx91x.c

+			while (port_data->pin_config_info[i].port_dev != NULL &&
+			       pin_cnt < __builtin_popcount(port_cfg->common.port_pin_mask)) {
+				ret = gpio_siwx91x_pin_configure(
+					port_data->pin_config_info[pin_cnt].port_dev,
+					port_data->pin_config_info[pin_cnt].pin,
+					port_data->pin_config_info[pin_cnt].flags);


Is it really correct that you're using a different index for port_data->pin_config_info in the while loop condition and inside the loop? Looks a bit suspicious.

You're right — that was missed. I've corrected it now.

jhedberg · 2025-10-06T11:25:30Z

drivers/gpio/gpio_silabs_siwx91x.c

+	const struct gpio_siwx91x_common_config *pcfg = dev->config;
+	struct gpio_siwx91x_common_data *pdata = dev->data;


Does the p prefix for these variable names stand for "port", or are you using Hungarian notation? In both cases I think you should just drop it.

Added the 'port' prefix to port-specific config and data, and used generic cfg and data labels for common configurations.

sonarqubecloud · 2025-10-07T06:55:26Z

Quality Gate passed

Issues
2 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

Add the port_count field to the configuration structure and allocate different sizes for the ports array based on whether the node is HP or ULP. Signed-off-by: Sai Santhosh Malae <[email protected]>

This commit enables the pm device runtime support for the siwx91x gpio driver. Signed-off-by: Sai Santhosh Malae <[email protected]>

Apply clang format corrections Signed-off-by: Sai Santhosh Malae <[email protected]>

Drop usage of `pcfg` and `pdata` in favor of unified `cfg` for common configuration and `port_cfg`, `port_data` for port-specific configurations. Signed-off-by: Sai Santhosh Malae <[email protected]>

Martinhoff-maker · 2025-10-07T15:20:26Z

drivers/gpio/gpio_silabs_siwx91x.c

-	uint16_t pin_direction[MAX_PIN_COUNT];
-#endif
+	struct gpio_siwx91x_pin_config_info *pin_config_info;
+	uint8_t next_pin;


next_pin feels not the good name for this variable. If I understand right, it saves the number of configured pin in the port ? would rather use "pin_ctn" or "pin_count" to have link with port variable.

Martinhoff-maker · 2025-10-07T15:25:46Z

drivers/gpio/gpio_silabs_siwx91x.c

+		port_data->next_pin++;
+	}

+	if (cur_cfg_pin < __builtin_popcount(port_cfg->common.port_pin_mask)) {


This value is already known and constant, it is ARRAY_SIZE(port_data->pin_config_info)

Martinhoff-maker · 2025-10-07T15:32:28Z

drivers/gpio/gpio_silabs_siwx91x.c

+		port_data->pin_config_info[cur_cfg_pin].pin = pin;
+		port_data->pin_config_info[cur_cfg_pin].flags = flags;
+	} else {
+		return -EIO;


-EINVAL feels to be a better return value.
-EIO is for: "I/O error when accessing an external GPIO chip."

Martinhoff-maker · 2025-10-07T15:41:42Z

drivers/gpio/gpio_silabs_siwx91x.c

+			while (port_data->pin_config_info[pin_cnt].port_dev != NULL &&
+			       pin_cnt < __builtin_popcount(port_cfg->common.port_pin_mask)) {
+				ret = gpio_siwx91x_pin_configure(
+					port_data->pin_config_info[pin_cnt].port_dev,
+					port_data->pin_config_info[pin_cnt].pin,
+					port_data->pin_config_info[pin_cnt].flags);
+				if (ret) {
+					return ret;
+				}
+				pin_cnt++;
+			}


can't you use the fact that you knows how much pin has been configured with your next_pin variable in port data ?

Suggested change

while (port_data->pin_config_info[pin_cnt].port_dev != NULL &&

pin_cnt < __builtin_popcount(port_cfg->common.port_pin_mask)) {

ret = gpio_siwx91x_pin_configure(

port_data->pin_config_info[pin_cnt].port_dev,

port_data->pin_config_info[pin_cnt].pin,

port_data->pin_config_info[pin_cnt].flags);

if (ret) {

return ret;

}

pin_cnt++;

}

for(int i = 0; i < port_data->next_pin; i++) {

ret = gpio_siwx91x_pin_configure(

port_data->pin_config_info[i].port_dev,

port_data->pin_config_info[i].pin,

port_data->pin_config_info[i].flags);

if (ret) {

return ret;

}

}

or something like this ? renaming to "pin_ctn" make more sense too here.

Martinhoff-maker · 2025-10-07T15:53:54Z

drivers/gpio/gpio_silabs_siwx91x.c

+					port_data->pin_config_info[pin_cnt].pin,
+					port_data->pin_config_info[pin_cnt].flags);
+				if (ret) {
+					return ret;


maybe a stupid question question but how could it failed since you already managed to configure this pin this way ? and even if it failed, what our power domain driver will do about that info ? (more open question)

zephyrbot added area: GPIO platform: Silabs Silicon Labs labels Sep 24, 2025

zephyrbot requested review from asmellby, henrikbrixandersen, jerome-pouiller, jhedberg, Martinhoff-maker and mnkp September 24, 2025 12:52

zephyrbot assigned jhedberg and asmellby Sep 24, 2025

jhedberg changed the title ~~Device runtime support for gpio driver~~ drivers: gpio: silabs-siwx91x: Add device runtime support Sep 24, 2025

smalae force-pushed the device-runtime-support-for-gpio-driver branch from 862d501 to ea23cd1 Compare September 25, 2025 06:58

jhedberg reviewed Sep 25, 2025

View reviewed changes

dts/bindings/gpio/silabs,siwx91x-gpio.yaml Outdated Show resolved Hide resolved

asmellby requested changes Sep 25, 2025

View reviewed changes

smalae force-pushed the device-runtime-support-for-gpio-driver branch from ea23cd1 to e2e9218 Compare October 3, 2025 17:29

zephyrbot added the area: Boards/SoCs label Oct 3, 2025

smalae force-pushed the device-runtime-support-for-gpio-driver branch from e2e9218 to c892a57 Compare October 3, 2025 17:34

smalae requested review from asmellby and jhedberg October 3, 2025 17:35

dts: arm: silabs-siwx91x: Update GPIO nodes

5c61cc4

Add soc power domain and zephyr,pm-device-runtime-auto Signed-off-by: Sai Santhosh Malae <[email protected]>

jhedberg requested changes Oct 6, 2025

View reviewed changes

smalae force-pushed the device-runtime-support-for-gpio-driver branch 2 times, most recently from 3e9475b to c6c07bf Compare October 7, 2025 06:29

smalae requested a review from jhedberg October 7, 2025 06:49

smalae added 4 commits October 7, 2025 17:29

drivers: gpio: silabs-siwx91x: Differentiate port count

c62b8bb

Add the port_count field to the configuration structure and allocate different sizes for the ports array based on whether the node is HP or ULP. Signed-off-by: Sai Santhosh Malae <[email protected]>

drivers: gpio: silabs-siwx91x: Device runtime PM

e4de085

This commit enables the pm device runtime support for the siwx91x gpio driver. Signed-off-by: Sai Santhosh Malae <[email protected]>

drivers: gpio: silabs-siwx91x: clang format corrections

2ad2a3d

Apply clang format corrections Signed-off-by: Sai Santhosh Malae <[email protected]>

drivers: gpio: silabs-siwx91x: Refactor config structures

c6c07bf

Drop usage of `pcfg` and `pdata` in favor of unified `cfg` for common configuration and `port_cfg`, `port_data` for port-specific configurations. Signed-off-by: Sai Santhosh Malae <[email protected]>

Martinhoff-maker reviewed Oct 7, 2025

View reviewed changes

		const struct gpio_siwx91x_common_config *pcfg = dev->config;
		struct gpio_siwx91x_common_data *pdata = dev->data;

drivers: gpio: silabs-siwx91x: Add device runtime support #96491

Are you sure you want to change the base?

drivers: gpio: silabs-siwx91x: Add device runtime support #96491

Conversation

smalae commented Sep 24, 2025

Uh oh!

jhedberg commented Sep 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmellby Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 7, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asmellby Sep 25, 2025 •

edited

Loading