soc: sifli: sf32lb52: Add pinctrl definition #100283

HalfSweet · 2025-12-01T08:29:38Z

This PR fixes several pinctrl issues in the sf32lb52 platform, primarily including the following:

Added analog attributes
Modified the drive-strength definition to reflect actual register values
Preserved default configurations in the .c driver after removal
Added pinmux definitions for the SA port

gmarull · 2025-12-01T09:10:41Z

dts/bindings/pinctrl/sifli,sf32lb52x-pinmux.yaml

+    - drive-strength: 0, 1, 2 (default), or 3. Maps to {DS0, DS1} register bits
+      where DS0 is the high bit. Default is 2 (DS0=1, DS1=0).
+      Note: PA39-PA42 only have DS1 bit (no DS0), so drive-strength must be
+      0 or 1 for these pins (default 0), where 0=4mA and 1=20mA.


drive-strength is documented as:

drive-strength: type: int description: maximum sink or source current in mA

Is this defined in the Zephyr specification? I modified it to the actual register values because there are differences in a few I/O pins

yes, see pincfg-node.yaml. Maybe handle this at runtime, or find some macrology to convert at compile time things to the appropriate register value? That's an unfortunate hw design.

dts/bindings/pinctrl/sifli,sf32lb52x-pinmux.yaml

gmarull · 2025-12-01T09:14:21Z

dts/bindings/pinctrl/sifli,sf32lb52x-pinmux.yaml

+      sifli,analog:
+        type: boolean
+        description: |
+          Configure the pin as analog mode. When set, the pin function select
+          is set to analog (FSEL=15), and pull-enable and input-enable are
+          disabled. This is typically used for power supply pins or ADC inputs.


if this is an FSEL, simply modify sf32lb52x-pinctrl.h to add an entry for each pin, e.g. PA00_ANALOG.

In addition to FSEL, analog mode also requires disabling IE and PE. I believe that setting a single attribute could simplify the writing of the device tree

if that's the case, encode all this in the macro (as we encode many things already)

gmarull · 2025-12-01T09:14:57Z

drivers/pinctrl/pinctrl_sf32lb52x.c

+	/*
+	 * PA39-PA42 only have DS1 bit (no DS0), drive-strength must be 0 or 1.
+	 * Check and return error if invalid configuration.
+	 */
+	if ((port == SF32LB_PORT_PA) && (pad_num >= 39U) && (pad_num <= 42U) && (ds > 1U)) {
+		return -EINVAL;
+	}


isn't this checked at compile time?

You're right, my compile-time code wasn't working properly, so I created a runtime check but forgot to remove the compile-time detection. I should delete it

- Add sifli and analog definitions - Modify the drive-strength definition to use the original register definition instead of the physical current value, as different I/O pins may have different definitions Signed-off-by: Haoran Jiang <[email protected]>

- Add helper macros related to sifli,analog - Standardize driver level descriptions to `DS` instead of `DS0` Signed-off-by: Haoran Jiang <[email protected]>

…ster Only modify the definitions that need to be changed in the pinmux register; SR/IS will retain their default values after reset. Signed-off-by: Haoran Jiang <[email protected]>

Add SA port pinmux definition Signed-off-by: Haoran Jiang <[email protected]>

sonarqubecloud · 2025-12-01T11:40:23Z

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

gmarull · 2025-12-01T14:19:46Z

dts/bindings/pinctrl/sifli,sf32lb52x-pinmux.yaml

+        description: |
+          Drive strength in milliamps. Default is 4mA.
+          Enum index maps directly to register value {DS0,DS1}:
+          Index 0 (2mA) → 0, Index 1 (8mA) → 1, Index 2 (4mA) → 2, Index 3 (12mA) → 3.
+          Note: PA39-PA42 only support 4mA or 20mA (DS1 bit only). 20mA (index 4) is only valid
+          for PA39-PA42.


description of inherited params should not be changed, maybe keep notes in the binding desc

gmarull · 2025-12-01T14:20:50Z

soc/sifli/sf32/sf32lb52x/pinctrl_soc.h

+#define SF32LB_PE_MSK BIT(4U)
+#define SF32LB_PS_MSK BIT(5U)
+#define SF32LB_IE_MSK BIT(6U)
+#define SF32LB_IS_MSK BIT(7U)


shouldn't IS be removed now that is not used?

gmarull · 2025-12-01T14:21:13Z

soc/sifli/sf32/sf32lb52x/pinctrl_soc.h

+#define SF32LB_PS_MSK BIT(5U)
+#define SF32LB_IE_MSK BIT(6U)
+#define SF32LB_IS_MSK BIT(7U)
+#define SF32LB_SR_MSK BIT(8U)


same for SR

gmarull · 2025-12-01T14:22:03Z

drivers/pinctrl/pinctrl_sf32lb52x.c

+			/* 20mA is valid for PA39-42 */
+			ds_reg = 1U;
+		} else if ((ds_idx == 0U) || (ds_idx == 2U)) {
+			/* 2mA/4mA → 4mA (DS1=0) */


→ -> ->

gmarull · 2025-12-01T14:22:37Z

drivers/pinctrl/pinctrl_sf32lb52x.c

+	val = sys_read32(pad);
+	val &= ~SF32LB_PINMUX_CFG_MSK;
+	val |= (pin & (SF32LB_PINMUX_CFG_MSK & ~SF32LB_DS_MSK));
+	val |= FIELD_PREP(SF32LB_DS_MSK, ds_reg);
+
+	sys_write32(val, pad);


no longer atomic, maybe spinlock?

gmarull · 2025-12-01T14:23:27Z

include/zephyr/dt-bindings/pinctrl/sf32lb52x-pinctrl.h

+/*
+ * PA port analog mode definitions
+ */
+#define PA00_ANALOG             SF32LB_PINMUX_ANALOG(PA, 0U)
+#define PA01_ANALOG             SF32LB_PINMUX_ANALOG(PA, 1U)
+#define PA02_ANALOG             SF32LB_PINMUX_ANALOG(PA, 2U)
+#define PA03_ANALOG             SF32LB_PINMUX_ANALOG(PA, 3U)
+#define PA04_ANALOG             SF32LB_PINMUX_ANALOG(PA, 4U)
+#define PA05_ANALOG             SF32LB_PINMUX_ANALOG(PA, 5U)
+#define PA06_ANALOG             SF32LB_PINMUX_ANALOG(PA, 6U)
+#define PA07_ANALOG             SF32LB_PINMUX_ANALOG(PA, 7U)
+#define PA08_ANALOG             SF32LB_PINMUX_ANALOG(PA, 8U)
+#define PA09_ANALOG             SF32LB_PINMUX_ANALOG(PA, 9U)
+#define PA10_ANALOG             SF32LB_PINMUX_ANALOG(PA, 10U)
+#define PA11_ANALOG             SF32LB_PINMUX_ANALOG(PA, 11U)
+#define PA12_ANALOG             SF32LB_PINMUX_ANALOG(PA, 12U)
+#define PA13_ANALOG             SF32LB_PINMUX_ANALOG(PA, 13U)
+#define PA14_ANALOG             SF32LB_PINMUX_ANALOG(PA, 14U)
+#define PA15_ANALOG             SF32LB_PINMUX_ANALOG(PA, 15U)
+#define PA16_ANALOG             SF32LB_PINMUX_ANALOG(PA, 16U)
+#define PA17_ANALOG             SF32LB_PINMUX_ANALOG(PA, 17U)
+#define PA18_ANALOG             SF32LB_PINMUX_ANALOG(PA, 18U)
+#define PA19_ANALOG             SF32LB_PINMUX_ANALOG(PA, 19U)
+#define PA20_ANALOG             SF32LB_PINMUX_ANALOG(PA, 20U)
+#define PA21_ANALOG             SF32LB_PINMUX_ANALOG(PA, 21U)
+#define PA22_ANALOG             SF32LB_PINMUX_ANALOG(PA, 22U)
+#define PA23_ANALOG             SF32LB_PINMUX_ANALOG(PA, 23U)
+#define PA24_ANALOG             SF32LB_PINMUX_ANALOG(PA, 24U)
+#define PA25_ANALOG             SF32LB_PINMUX_ANALOG(PA, 25U)
+#define PA26_ANALOG             SF32LB_PINMUX_ANALOG(PA, 26U)
+#define PA27_ANALOG             SF32LB_PINMUX_ANALOG(PA, 27U)
+#define PA28_ANALOG             SF32LB_PINMUX_ANALOG(PA, 28U)
+#define PA29_ANALOG             SF32LB_PINMUX_ANALOG(PA, 29U)
+#define PA30_ANALOG             SF32LB_PINMUX_ANALOG(PA, 30U)
+#define PA31_ANALOG             SF32LB_PINMUX_ANALOG(PA, 31U)
+#define PA32_ANALOG             SF32LB_PINMUX_ANALOG(PA, 32U)
+#define PA33_ANALOG             SF32LB_PINMUX_ANALOG(PA, 33U)
+#define PA34_ANALOG             SF32LB_PINMUX_ANALOG(PA, 34U)
+#define PA35_ANALOG             SF32LB_PINMUX_ANALOG(PA, 35U)
+#define PA36_ANALOG             SF32LB_PINMUX_ANALOG(PA, 36U)
+#define PA37_ANALOG             SF32LB_PINMUX_ANALOG(PA, 37U)
+#define PA38_ANALOG             SF32LB_PINMUX_ANALOG(PA, 38U)
+#define PA39_ANALOG             SF32LB_PINMUX_ANALOG(PA, 39U)
+#define PA40_ANALOG             SF32LB_PINMUX_ANALOG(PA, 40U)
+#define PA41_ANALOG             SF32LB_PINMUX_ANALOG(PA, 41U)
+#define PA42_ANALOG             SF32LB_PINMUX_ANALOG(PA, 42U)
+#define PA43_ANALOG             SF32LB_PINMUX_ANALOG(PA, 43U)
+#define PA44_ANALOG             SF32LB_PINMUX_ANALOG(PA, 44U)


better add this together with each pin "group" to keep consistency (as done below actualy)

zephyrbot added platform: SF32LB area: Pinctrl area: Devicetree Bindings area: Boards/SoCs labels Dec 1, 2025

zephyrbot requested review from cameled, ck-telecom and gmarull December 1, 2025 08:31

zephyrbot assigned gmarull Dec 1, 2025

HalfSweet force-pushed the sf32lb/pinctrl branch from 4309d41 to 3b90c44 Compare December 1, 2025 08:55

gmarull requested changes Dec 1, 2025

View reviewed changes

HalfSweet force-pushed the sf32lb/pinctrl branch 5 times, most recently from 83c64be to 6f90673 Compare December 1, 2025 11:06

HalfSweet added 4 commits December 1, 2025 19:13

soc: sifli: sf32lb52x: Add pinctrl definition

d5e6b2f

- Add helper macros related to sifli,analog - Standardize driver level descriptions to `DS` instead of `DS0` Signed-off-by: Haoran Jiang <[email protected]>

drivers: pinctrl: sf32lb: Preserve the SR/IS definition for each regi…

f9c71aa

…ster Only modify the definitions that need to be changed in the pinmux register; SR/IS will retain their default values after reset. Signed-off-by: Haoran Jiang <[email protected]>

dt-bindings: pinctrl: sf32lb52: Add SA port pinmux definition

3c3af4b

Add SA port pinmux definition Signed-off-by: Haoran Jiang <[email protected]>

HalfSweet force-pushed the sf32lb/pinctrl branch from 6f90673 to 3c3af4b Compare December 1, 2025 11:13

gmarull reviewed Dec 1, 2025

View reviewed changes

soc: sifli: sf32lb52: Add pinctrl definition #100283

Are you sure you want to change the base?

soc: sifli: sf32lb52: Add pinctrl definition #100283

Conversation

HalfSweet commented Dec 1, 2025

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!

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 Dec 1, 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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants