* [PATCH 1/3] arm64: dts: socfpga: agilex5: add the pinctrl node @ 2026-10-29 21:31 Dinh Nguyen 2026-10-29 21:32 ` [PATCH 2/3] arm64: dts: agilex5: define the i2c0 pins for pinctrl Dinh Nguyen 2026-10-29 21:32 ` [PATCH 3/3] arm64: dts: stratix10: define the i2c1 " Dinh Nguyen 0 siblings, 2 replies; 5+ messages in thread From: Dinh Nguyen @ 2026-10-29 21:31 UTC (permalink / raw) To: robh, krzk+dt, conor+dt; +Cc: dinguyen, devicetree The Agilex5 also has a pinctrl module that does the pinmuxing for the various pins on the platform. Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> --- arch/arm64/boot/dts/altera/socfpga_agilex5.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/altera/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/altera/socfpga_agilex5.dtsi index 0c501a93a8127..161f3d4bbefe8 100644 --- a/arch/arm64/boot/dts/altera/socfpga_agilex5.dtsi +++ b/arch/arm64/boot/dts/altera/socfpga_agilex5.dtsi @@ -446,6 +446,14 @@ spi1: spi@10da5000 { status = "disabled"; }; + pinctrl0: pinctrl@10d13000 { + compatible = "pinctrl-single"; + #pinctrl-cells = <1>; + reg = <0x10d13000 0x120>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0x0000000f>; + }; + sysmgr: sysmgr@10d12000 { compatible = "altr,sys-mgr-s10","altr,sys-mgr"; reg = <0x10d12000 0x500>; -- 2.42.0.411.g813d9a9188 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] arm64: dts: agilex5: define the i2c0 pins for pinctrl 2026-10-29 21:31 [PATCH 1/3] arm64: dts: socfpga: agilex5: add the pinctrl node Dinh Nguyen @ 2026-10-29 21:32 ` Dinh Nguyen 2026-10-29 21:50 ` sashiko-bot 2026-10-29 21:32 ` [PATCH 3/3] arm64: dts: stratix10: define the i2c1 " Dinh Nguyen 1 sibling, 1 reply; 5+ messages in thread From: Dinh Nguyen @ 2026-10-29 21:32 UTC (permalink / raw) To: robh, krzk+dt, conor+dt; +Cc: dinguyen, devicetree Add the I2C pins definition to the Agilex5 SoCDK NAND daughter card. This allows the I2C driver to use pinctrl on the pins to perform I2C recovery. Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> --- .../dts/altera/socfpga_agilex5_socdk_nand.dts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts b/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts index 81443096cf991..346a4ee22f95b 100644 --- a/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts +++ b/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts @@ -39,6 +39,22 @@ memory@80000000 { }; }; +&pinctrl0 { + i2c0_pmx_func: i2c0-pmx-func-pins { + pinctrl-single,pins = < + 0x10 0x4 /* I2C0_SDA (IOA4) PIN4SEL) */ + 0x14 0x4 /* I2C0_SCL (IOA5) PIN5SEL */ + >; + }; + + i2c0_pmx_func_gpio: i2c0-pmx-func-gpio-pins { + pinctrl-single,pins = < + 0x10 0x8 /* I2C0_SDA (IOA4) PIN4SEL) */ + 0x14 0x8 /* I2C0_SCL (IOA5) PIN5SEL */ + >; + }; +}; + &gmac0 { status = "okay"; phy-mode = "rgmii"; /* TX/RX clock delays provided by Agilex5 I/O hardware */ @@ -66,6 +82,10 @@ &gpio1 { &i2c0 { status = "okay"; + + pinctrl-names = "default", "gpio"; + pinctrl-0 = <&i2c0_pmx_func>; + pinctrl-1 = <&i2c0_pmx_func_gpio>; }; &i3c0 { -- 2.42.0.411.g813d9a9188 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] arm64: dts: agilex5: define the i2c0 pins for pinctrl 2026-10-29 21:32 ` [PATCH 2/3] arm64: dts: agilex5: define the i2c0 pins for pinctrl Dinh Nguyen @ 2026-10-29 21:50 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-10-29 21:50 UTC (permalink / raw) To: Dinh Nguyen; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] arm64: dts: agilex5: missing scl-gpios and sda-gpios on i2c0 prevents recovery -- commit 4fe1ecb21c260e09211a4f5ec837b9b80b38c36d Author: Dinh Nguyen <dinguyen@kernel.org> arm64: dts: agilex5: define the i2c0 pins for pinctrl This commit adds pinctrl definitions for i2c0 default and gpio states on the Agilex5 SoCDK NAND daughter card to support I2C bus recovery. > diff --git a/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts b/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts > index 81443096cf991..346a4ee22f95b 100644 > --- a/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts > +++ b/arch/arm64/boot/dts/altera/socfpga_agilex5_socdk_nand.dts > @@ -39,6 +39,22 @@ memory@80000000 { > }; > }; > > +&pinctrl0 { > + i2c0_pmx_func: i2c0-pmx-func-pins { > + pinctrl-single,pins = < > + 0x10 0x4 /* I2C0_SDA (IOA4) PIN4SEL) */ > + 0x14 0x4 /* I2C0_SCL (IOA5) PIN5SEL */ > + >; > + }; > + > + i2c0_pmx_func_gpio: i2c0-pmx-func-gpio-pins { > + pinctrl-single,pins = < > + 0x10 0x8 /* I2C0_SDA (IOA4) PIN4SEL) */ > + 0x14 0x8 /* I2C0_SCL (IOA5) PIN5SEL */ > + >; > + }; > +}; > + > &gmac0 { > status = "okay"; > phy-mode = "rgmii"; /* TX/RX clock delays provided by Agilex5 I/O hardware */ > @@ -66,6 +82,10 @@ &gpio1 { > > &i2c0 { > status = "okay"; > + > + pinctrl-names = "default", "gpio"; > + pinctrl-0 = <&i2c0_pmx_func>; > + pinctrl-1 = <&i2c0_pmx_func_gpio>; > }; [Severity: Medium] Does i2c0 also need scl-gpios and sda-gpios properties for I2C recovery to work? When probing i2c0, i2c_dw_probe_master() calls i2c_dw_init_recovery_info() in drivers/i2c/busses/i2c-designware-master.c: gpio = devm_gpiod_get_optional(dev->dev, "scl", GPIOD_OUT_HIGH); if (IS_ERR_OR_NULL(gpio)) return PTR_ERR_OR_ZERO(gpio); rinfo->scl_gpiod = gpio; Without scl-gpios in the i2c0 node, devm_gpiod_get_optional() returns NULL and i2c_dw_init_recovery_info() returns early before calling devm_pinctrl_get() or populating adap->bus_recovery_info. Consequently, i2c_init_recovery() in drivers/i2c/i2c-core-base.c sees adap->bus_recovery_info as NULL and skips i2c_gpio_init_recovery(), leaving the gpio pinctrl state unused and bus recovery disabled. In arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts, the pinctrl states are paired with the GPIO descriptors: pinctrl-names = "default", "gpio"; pinctrl-0 = <&i2c1_pmx_func>; pinctrl-1 = <&i2c1_pmx_func_gpio>; scl-gpios = <&portb 6 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; sda-gpios = <&portb 7 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; Should i2c0 here also specify scl-gpios and sda-gpios (for example on &porta 5 and &porta 4 for IOA5 and IOA4)? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261029213201.718625-1-dinguyen@kernel.org?part=2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] arm64: dts: stratix10: define the i2c1 pins for pinctrl 2026-10-29 21:31 [PATCH 1/3] arm64: dts: socfpga: agilex5: add the pinctrl node Dinh Nguyen 2026-10-29 21:32 ` [PATCH 2/3] arm64: dts: agilex5: define the i2c0 pins for pinctrl Dinh Nguyen @ 2026-10-29 21:32 ` Dinh Nguyen 2026-10-29 22:02 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: Dinh Nguyen @ 2026-10-29 21:32 UTC (permalink / raw) To: robh, krzk+dt, conor+dt; +Cc: dinguyen, devicetree Add the I2C pins definition to the Stratix10 SoCDK NAND and eMMC daughter cards. This allows the I2C driver to use pinctrl on the pins to perform I2C recovery. Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> --- .../altera/socfpga_stratix10_socdk_emmc.dts | 20 +++++++++++++++++++ .../altera/socfpga_stratix10_socdk_nand.dts | 20 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts index bda2fe059beba..c63af8fa0917a 100644 --- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts @@ -12,6 +12,22 @@ / { "altr,socfpga-stratix10"; }; +&pinctrl0 { + i2c1_pmx_func: i2c1-pmx-func-pins { + pinctrl-single,pins = < + 0x78 0x4 /* I2C1_SDA (IO6) PIN30SEL) */ + 0x7c 0x4 /* I2C1_SCL (IO7) PIN31SEL */ + >; + }; + + i2c1_pmx_func_gpio: i2c1-pmx-func-gpio-pins { + pinctrl-single,pins = < + 0x78 0x8 /* I2C1_SDA (IO6) PIN30SEL) */ + 0x7c 0x8 /* I2C1_SCL (IO7) PIN31SEL */ + >; + }; +}; + &gmac2 { status = "okay"; /* PHY delays is configured via skew properties */ @@ -57,6 +73,10 @@ &i2c1 { i2c-sda-falling-time-ns = <890>; /* hcnt */ i2c-scl-falling-time-ns = <890>; /* lcnt */ + pinctrl-names = "default", "gpio"; + pinctrl-0 = <&i2c1_pmx_func>; + pinctrl-1 = <&i2c1_pmx_func_gpio>; + adc@14 { compatible = "lltc,ltc2497"; reg = <0x14>; diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts index a450c42a6b35f..f69ec2f5b94b6 100644 --- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts @@ -56,6 +56,22 @@ &gpio1 { status = "okay"; }; +&pinctrl0 { + i2c1_pmx_func: i2c1-pmx-func-pins { + pinctrl-single,pins = < + 0x78 0x4 /* I2C1_SDA (IO6) PIN30SEL) */ + 0x7c 0x4 /* I2C1_SCL (IO7) PIN31SEL */ + >; + }; + + i2c1_pmx_func_gpio: i2c1-pmx-func-gpio-pins { + pinctrl-single,pins = < + 0x78 0x8 /* I2C1_SDA (IO6) PIN30SEL) */ + 0x1c 0x8 /* I2C1_SCL (IO7) PIN31SEL */ + >; + }; +}; + &gmac2 { status = "okay"; phy-mode = "rgmii"; @@ -130,6 +146,10 @@ &i2c1 { i2c-sda-falling-time-ns = <890>; /* hcnt */ i2c-scl-falling-time-ns = <890>; /* lcnt */ + pinctrl-names = "default", "gpio"; + pinctrl-0 = <&i2c1_pmx_func>; + pinctrl-1 = <&i2c1_pmx_func_gpio>; + adc@14 { compatible = "lltc,ltc2497"; reg = <0x14>; -- 2.42.0.411.g813d9a9188 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] arm64: dts: stratix10: define the i2c1 pins for pinctrl 2026-10-29 21:32 ` [PATCH 3/3] arm64: dts: stratix10: define the i2c1 " Dinh Nguyen @ 2026-10-29 22:02 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-10-29 22:02 UTC (permalink / raw) To: Dinh Nguyen; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] arm64: dts: stratix10: wrong I2C1_SCL pinmux offset in nand GPIO state - [High] arm64: dts: stratix10: i2c1 pinmux nodes attached to i2c2 controller - [Medium] arm64: dts: stratix10: missing scl-gpios and sda-gpios for I2C recovery -- commit 6b60cc5dbb08c4ec389238c564d8da6f01a7cc28 Author: Dinh Nguyen <dinguyen@kernel.org> arm64: dts: stratix10: define the i2c1 pins for pinctrl This commit adds I2C1 pinctrl pin definitions and recovery states to the Stratix10 SoCDK eMMC and NAND daughter card device trees to support I2C bus recovery. > diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts > index b2a3449638dd3..743a4db0423d8 100644 > --- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts > +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_emmc.dts > @@ -12,6 +12,22 @@ / { > "altr,socfpga-stratix10"; > }; > > +&pinctrl0 { > + i2c1_pmx_func: i2c1-pmx-func-pins { > + pinctrl-single,pins = < > + 0x78 0x4 /* I2C1_SDA (IO6) PIN30SEL) */ > + 0x7c 0x4 /* I2C1_SCL (IO7) PIN31SEL */ > + >; > + }; > + > + i2c1_pmx_func_gpio: i2c1-pmx-func-gpio-pins { > + pinctrl-single,pins = < > + 0x78 0x8 /* I2C1_SDA (IO6) PIN30SEL) */ > + 0x7c 0x8 /* I2C1_SCL (IO7) PIN31SEL */ > + >; > + }; > +}; > + > &gmac2 { > status = "okay"; > /* PHY delays is configured via skew properties */ > @@ -57,6 +73,10 @@ &i2c2 { > i2c-sda-falling-time-ns = <890>; /* hcnt */ > i2c-scl-falling-time-ns = <890>; /* lcnt */ > > + pinctrl-names = "default", "gpio"; > + pinctrl-0 = <&i2c1_pmx_func>; > + pinctrl-1 = <&i2c1_pmx_func_gpio>; [Severity: High] Are the i2c1_pmx_func and i2c1_pmx_func_gpio pinctrl states attached to the right I2C controller node here and in socfpga_stratix10_socdk_nand.dts? In arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi, i2c1 is i2c@ffc02900 and i2c2 is i2c@ffc02a00. In socfpga_stratix10_socdk.dts, i2c1_pmx_func and i2c1_pmx_func_gpio configure PIN30SEL (0x78, I2C1_SDA) and PIN31SEL (0x7c, I2C1_SCL) and are attached to &i2c1. Here (and in socfpga_stratix10_socdk_nand.dts), those I2C1 pinmux states are attached to &i2c2 instead of &i2c1, so when &i2c2 probes via really_probe() -> pinctrl_bind_pins(), it muxes the I2C1 pins rather than configuring pins for i2c2 or using the &i2c1 controller. [ ... ] > diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts > index 7951ce46ae1f6..21d25a10e472a 100644 > --- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts > +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk_nand.dts > @@ -56,6 +56,22 @@ &gpio1 { > status = "okay"; > }; > > +&pinctrl0 { > + i2c1_pmx_func: i2c1-pmx-func-pins { > + pinctrl-single,pins = < > + 0x78 0x4 /* I2C1_SDA (IO6) PIN30SEL) */ > + 0x7c 0x4 /* I2C1_SCL (IO7) PIN31SEL */ > + >; > + }; > + > + i2c1_pmx_func_gpio: i2c1-pmx-func-gpio-pins { > + pinctrl-single,pins = < > + 0x78 0x8 /* I2C1_SDA (IO6) PIN30SEL) */ > + 0x1c 0x8 /* I2C1_SCL (IO7) PIN31SEL */ [Severity: High] Should the register offset for I2C1_SCL (PIN31SEL) in i2c1_pmx_func_gpio be 0x7c instead of 0x1c? In i2c1_pmx_func right above (and in socfpga_stratix10_socdk_emmc.dts), PIN31SEL is at offset 0x7c. Using 0x1c in i2c1_pmx_func_gpio writes 0x8 to PIN7SEL when the "gpio" pinctrl state is selected via pinctrl_select_state(), leaving I2C1_SCL (PIN31SEL) in I2C mode and leaving PIN7SEL overwritten even after switching back to the "default" state. > + >; > + }; > +}; > + > &gmac2 { > status = "okay"; > phy-mode = "rgmii"; > @@ -130,6 +146,10 @@ &i2c2 { > i2c-sda-falling-time-ns = <890>; /* hcnt */ > i2c-scl-falling-time-ns = <890>; /* lcnt */ > > + pinctrl-names = "default", "gpio"; > + pinctrl-0 = <&i2c1_pmx_func>; > + pinctrl-1 = <&i2c1_pmx_func_gpio>; [Severity: Medium] Can DesignWare I2C bus recovery initialize without scl-gpios and sda-gpios defined in the I2C node in both socfpga_stratix10_socdk_emmc.dts and socfpga_stratix10_socdk_nand.dts? When the controller probes via i2c_dw_probe_master() -> i2c_dw_init_recovery_info() in drivers/i2c/busses/i2c-designware-master.c, it checks for the "scl" GPIO first: gpio = devm_gpiod_get_optional(dev->dev, "scl", GPIOD_OUT_HIGH); if (IS_ERR_OR_NULL(gpio)) return PTR_ERR_OR_ZERO(gpio); Without scl-gpios (and sda-gpios, as present in socfpga_stratix10_socdk.dts: scl-gpios = <&portb 6 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; sda-gpios = <&portb 7 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; ), i2c_dw_init_recovery_info() returns early before calling devm_pinctrl_get() or setting adap->bus_recovery_info, so i2c_init_recovery() never sets up GPIO bus recovery. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261029213201.718625-1-dinguyen@kernel.org?part=3 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-29 22:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-29 21:31 [PATCH 1/3] arm64: dts: socfpga: agilex5: add the pinctrl node Dinh Nguyen 2026-10-29 21:32 ` [PATCH 2/3] arm64: dts: agilex5: define the i2c0 pins for pinctrl Dinh Nguyen 2026-10-29 21:50 ` sashiko-bot 2026-10-29 21:32 ` [PATCH 3/3] arm64: dts: stratix10: define the i2c1 " Dinh Nguyen 2026-10-29 22:02 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox