[PATCH 0/9] drivers: gpio: and the QIXIS FPGA GPIO controller

[PATCH 0/9] drivers: gpio: and the QIXIS FPGA GPIO controller

[Ioana Ciornei]

This patch set adds support for the GPIO controllers on the QIXIS FPGAs
found on some Layerscape boards such as LX2160ARDB and LS1046AQDS.

The first patch adds the necessary dt-binding for the new GPIO
controller driver. The filename of the new dt-binding was chosen as the
common part between all the compatible strings added.

The next two patches extend the fsl,fpga-qixis-i2c dt-binding and the
simple-mfd-i2c driver so that the LX2160ARDB FPGA is also probed by said
driver.

Patches 3/9 and 4/9 extend gpio-regmap and add the actual new GPIO
driver for these fixed direction GPIO controllers.

The last 4 patches extend the device-tree for the LX2160ARDB and
LS1046AQDS boards by describing the QIXIS FPGAs, when necessary, adding
the GPIO controller nodes and also using said GPIO lines to describe the
on-board SFP cages.

This patch set depends on the following in-flight patch:
 https://lore.kernel.org/all/20250707153120.1371719-1-ioana.ciornei@nxp.com/

Ioana Ciornei (9):
  dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO
    controller
  dt-bindings: fsl,fpga-qixis-i2c: extend support to also cover the
    LX2160ARDB FPGA
  mfd: simple-mfd-i2c: add compatible string for LX2160ARDB
  gpio: regmap: add the .get_direction() callback
  drivers: gpio: add QIXIS FPGA GPIO controller
  arm64: dts: lx2160a-rdb: describe the QIXIS FPGA and two child GPIO
    controllers
  arm64: dts: ls1046a-qds: describe the FPGA based GPIO controller
  arm64: dts: lx2160a-rdb: fully describe the two SFP+ cages
  arm64: dts: ls1046a-qds: describe the two on-board SFP+ cages

 .../bindings/board/fsl,fpga-qixis-i2c.yaml    |  35 +++++
 .../bindings/gpio/fsl,fpga-gpio.yaml          |  44 ++++++
 .../boot/dts/freescale/fsl-ls1046a-qds.dts    |  52 +++++++
 .../boot/dts/freescale/fsl-lx2160a-rdb.dts    |  76 ++++++++++
 drivers/gpio/Kconfig                          |   9 ++
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-qixis-fpga.c                | 141 ++++++++++++++++++
 drivers/gpio/gpio-regmap.c                    |  17 ++-
 drivers/mfd/simple-mfd-i2c.c                  |   1 +
 include/linux/gpio/regmap.h                   |   3 +
 10 files changed, 378 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
 create mode 100644 drivers/gpio/gpio-qixis-fpga.c

-- 
2.25.1

[PATCH 1/9] dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO controller

[Ioana Ciornei]

Add a device tree binding for the QIXIS FPGA based GPIO controller.
Depending on the board, the QIXIS FPGA exposes registers which act as a
GPIO controller, each with 8 GPIO lines of fixed direction.

Since each QIXIS FPGA layout has its particularities, add a separate
compatible string for each board/GPIO register combination supported.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../bindings/gpio/fsl,fpga-gpio.yaml          | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
new file mode 100644
index 000000000000..dc7b6c0d9b40
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/fsl,fpga-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO controller embedded in the NXP QIXIS FPGA
+
+maintainers:
+  - Ioana Ciornei <ioana.ciornei@nxp.com>
+
+description: |
+  This module is part of the QIXIS FPGA found on some Layerscape boards such as
+  LX2160ARDB and LS1046AQDS. For more details see
+  ../board/fsl,fpga-qixis-i2c.yaml.
+
+  Each controller supports a maximum of 8 GPIO lines and each line has a fixed
+  direction which cannot be changed using a direction register.
+
+properties:
+  compatible:
+    enum:
+      - fsl,lx2160ardb-fpga-gpio-sfp2
+      - fsl,lx2160ardb-fpga-gpio-sfp3
+      - fsl,ls1046aqds-fpga-gpio-stat-pres2
+
+  reg:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  gpio-line-names:
+    minItems: 1
+    maxItems: 8
+
+required:
+  - compatible
+  - "#gpio-cells"
+  - gpio-controller
+
+additionalProperties: false
-- 
2.25.1

[PATCH 2/9] dt-bindings: fsl,fpga-qixis-i2c: extend support to also cover the LX2160ARDB FPGA

[Ioana Ciornei]

Extend the list of supported compatible strings with fsl,lx2160ardb-fpga.

Since the register map exposed by the LX2160ARDB's FPGA also contains
two GPIO controllers, accept the necessary GPIO pattern property. At the
same time, add the #address-cells and #size-cells properties as valid
ones.

This is needed because when defining child devices such as the GPIO
controller described in the added example, the child device needs a the
reg property to properly identify its register location.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../bindings/board/fsl,fpga-qixis-i2c.yaml    | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
index 28b37772fb65..e8981f974210 100644
--- a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
+++ b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
@@ -22,6 +22,13 @@ properties:
               - fsl,lx2160aqds-fpga
           - const: fsl,fpga-qixis-i2c
           - const: simple-mfd
+      - const: fsl,lx2160ardb-fpga
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
 
   interrupts:
     maxItems: 1
@@ -32,6 +39,10 @@ properties:
   mux-controller:
     $ref: /schemas/mux/reg-mux.yaml
 
+patternProperties:
+  "^gpio(@[0-9a-f]+)?$":
+    $ref: /schemas/gpio/fsl,fpga-gpio.yaml
+
 required:
   - compatible
   - reg
@@ -68,3 +79,27 @@ examples:
         };
     };
 
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        board-control@66 {
+            compatible = "fsl,lx2160ardb-fpga";
+            reg = <0x66>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            gpio@19 {
+                compatible = "fsl,lx2160ardb-fpga-gpio-sfp2";
+                reg = <0x19>;
+                gpio-controller;
+                #gpio-cells = <2>;
+                gpio-line-names =
+                    "SFP2_TX_EN", "",
+                    "", "",
+                    "SFP2_RX_LOS", "SFP2_TX_FAULT",
+                    "", "SFP2_MOD_ABS";
+            };
+        };
+    };
-- 
2.25.1

[PATCH 3/9] mfd: simple-mfd-i2c: add compatible string for LX2160ARDB

[Ioana Ciornei]

Extend the list of supported devices with the QIXIS FPGA found on the
LX2160ARDB board.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/mfd/simple-mfd-i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index f7798bd92222..9e6bdb0e1796 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -101,6 +101,7 @@ static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705},
 	{ .compatible = "fsl,lx2160aqds-fpga" },
 	{ .compatible = "fsl,ls1028aqds-fpga" },
+	{ .compatible = "fsl,lx2160ardb-fpga" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
-- 
2.25.1

[PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Ioana Ciornei]

There are GPIO controllers such as the one present in the LX2160ARDB
QIXIS CPLD which are single register fixed-direction. This cannot be
modeled using the gpio-regmap as-is since there is no way to
present the true direction of a GPIO line.

In order to make this use case possible, add a new callback to the
gpio_config structure - .get_direction() - which can be used by user
drivers to provide the fixed direction per GPIO line.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/gpio/gpio-regmap.c  | 17 ++++++++++++++++-
 include/linux/gpio/regmap.h |  3 +++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 87c4225784cf..dac2acb26655 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -32,6 +32,8 @@ struct gpio_regmap {
 	unsigned int reg_dir_in_base;
 	unsigned int reg_dir_out_base;
 
+	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
+
 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
 			      unsigned int offset, unsigned int *reg,
 			      unsigned int *mask);
@@ -129,6 +131,9 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
 	unsigned int base, val, reg, mask;
 	int invert, ret;
 
+	if (gpio->get_direction)
+		return gpio->get_direction(gpio, offset);
+
 	if (gpio->reg_dat_base && !gpio->reg_set_base)
 		return GPIO_LINE_DIRECTION_IN;
 	if (gpio->reg_set_base && !gpio->reg_dat_base)
@@ -163,7 +168,16 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip,
 {
 	struct gpio_regmap *gpio = gpiochip_get_data(chip);
 	unsigned int base, val, reg, mask;
-	int invert, ret;
+	int invert, ret, dir;
+
+	if (gpio->get_direction) {
+		dir = gpio->get_direction(gpio, offset);
+		if (dir == GPIO_LINE_DIRECTION_IN && output)
+			return -EOPNOTSUPP;
+		if (dir == GPIO_LINE_DIRECTION_OUT && !output)
+			return -EOPNOTSUPP;
+		return 0;
+	}
 
 	if (gpio->reg_dir_out_base) {
 		base = gpio_regmap_addr(gpio->reg_dir_out_base);
@@ -247,6 +261,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 	gpio->reg_clr_base = config->reg_clr_base;
 	gpio->reg_dir_in_base = config->reg_dir_in_base;
 	gpio->reg_dir_out_base = config->reg_dir_out_base;
+	gpio->get_direction = config->get_direction;
 
 	chip = &gpio->gpio_chip;
 	chip->parent = config->parent;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index c722c67668c6..99fd973e61fa 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -37,6 +37,8 @@ struct regmap;
  *			offset to a register/bitmask pair. If not
  *			given the default gpio_regmap_simple_xlate()
  *			is used.
+ * @get_direction:	(Optional) Callback to the user driver to return the
+ *			fixed direction of the GPIO line
  * @drvdata:		(Optional) Pointer to driver specific data which is
  *			not used by gpio-remap but is provided "as is" to the
  *			driver callback(s).
@@ -81,6 +83,7 @@ struct gpio_regmap_config {
 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
 			      unsigned int offset, unsigned int *reg,
 			      unsigned int *mask);
+	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
 
 	void *drvdata;
 };
-- 
2.25.1

[PATCH 5/9] drivers: gpio: add QIXIS FPGA GPIO controller

[Ioana Ciornei]

Add support for the GPIO controller found on some QIXIS FPGAs in
Layerscape boards such as LX2160ARDB and LS1046AQDS. This driver is
using gpio-regmap.

A GPIO controller has a maximum of 8 lines (all found in the same
register). Even within the same controller, the GPIO lines' direction is
fixed, either output or input, without the possibility to change it.
This is why the driver also implements the newly added .get_diretion()
callback exposed by gpio-regmap.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/gpio/Kconfig           |   9 +++
 drivers/gpio/Makefile          |   1 +
 drivers/gpio/gpio-qixis-fpga.c | 141 +++++++++++++++++++++++++++++++++
 3 files changed, 151 insertions(+)
 create mode 100644 drivers/gpio/gpio-qixis-fpga.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 6802e549621b..fe69b9fa12eb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1926,6 +1926,15 @@ config GPIO_LATCH
 	  Say yes here to enable a driver for GPIO multiplexers based on latches
 	  connected to other GPIOs.
 
+config GPIO_QIXIS_FPGA
+	tristate "NXP QIXIS FPGA GPIO support"
+	depends on MFD_SIMPLE_MFD_I2C || COMPILE_TEST
+	select GPIO_REGMAP
+	help
+	  This enables support for the GPIOs found in the QIXIS FPGA which is
+	  integrated on some NXP Layerscape boards such as LX2160ARDB and
+	  LS1046AQDS.
+
 config GPIO_MOCKUP
 	tristate "GPIO Testing Driver (DEPRECATED)"
 	select IRQ_SIM
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 88dedd298256..b8dbd1aa2c85 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_GPIO_PL061)		+= gpio-pl061.o
 obj-$(CONFIG_GPIO_PMIC_EIC_SPRD)	+= gpio-pmic-eic-sprd.o
 obj-$(CONFIG_GPIO_POLARFIRE_SOC)	+= gpio-mpfs.o
 obj-$(CONFIG_GPIO_PXA)			+= gpio-pxa.o
+obj-$(CONFIG_GPIO_QIXIS_FPGA)		+= gpio-qixis-fpga.o
 obj-$(CONFIG_GPIO_RASPBERRYPI_EXP)	+= gpio-raspberrypi-exp.o
 obj-$(CONFIG_GPIO_RC5T583)		+= gpio-rc5t583.o
 obj-$(CONFIG_GPIO_RCAR)			+= gpio-rcar.o
diff --git a/drivers/gpio/gpio-qixis-fpga.c b/drivers/gpio/gpio-qixis-fpga.c
new file mode 100644
index 000000000000..d5b7619970e9
--- /dev/null
+++ b/drivers/gpio/gpio-qixis-fpga.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Layerscape GPIO QIXIS FPGA driver
+ *
+ * Copyright 2025 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum qixis_cpld_gpio_type {
+	LX2160ARDB_GPIO_SFP2 = 0,
+	LX2160ARDB_GPIO_SFP3,
+	LS1046AQDS_GPIO_STAT_PRES2,
+};
+
+struct qixis_cpld_gpio_config {
+	enum qixis_cpld_gpio_type type;
+	unsigned int input_lines;
+};
+
+static struct qixis_cpld_gpio_config lx2160ardb_sfp2_cfg = {
+	.type = LX2160ARDB_GPIO_SFP2,
+	.input_lines = GENMASK(7, 1),
+};
+
+static struct qixis_cpld_gpio_config lx2160ardb_sfp3_cfg = {
+	.type = LX2160ARDB_GPIO_SFP3,
+	.input_lines = GENMASK(7, 1),
+};
+
+static struct qixis_cpld_gpio_config ls1046aqds_stat_pres2_cfg = {
+	.type = LS1046AQDS_GPIO_STAT_PRES2,
+	.input_lines = GENMASK(7, 0),
+};
+
+static int qixis_cpld_gpio_get_direction(struct gpio_regmap *gpio, unsigned int offset)
+{
+	struct qixis_cpld_gpio_config *cfg = gpio_regmap_get_drvdata(gpio);
+
+	if (cfg->input_lines & BIT(offset))
+		return GPIO_LINE_DIRECTION_IN;
+	else
+		return GPIO_LINE_DIRECTION_OUT;
+}
+
+static const struct regmap_config regmap_config_8r_8v = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int qixis_cpld_gpio_probe(struct platform_device *pdev)
+{
+	const struct qixis_cpld_gpio_config *cfg;
+	struct gpio_regmap_config config = {0};
+	struct regmap *regmap;
+	void __iomem *reg;
+	u32 base;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
+	cfg = device_get_match_data(&pdev->dev);
+	if (!cfg)
+		return -ENODEV;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &base);
+	if (ret)
+		return ret;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap) {
+		/* In case there is no regmap configured by the parent device,
+		 * create our own.
+		 */
+		reg = devm_platform_ioremap_resource(pdev, 0);
+		if (!reg)
+			return -ENODEV;
+
+		regmap = devm_regmap_init_mmio(&pdev->dev, reg, &regmap_config_8r_8v);
+		if (!regmap)
+			return -ENODEV;
+
+		/* In this case, the offset of our register is 0 inside the
+		 * regmap area that we just created.
+		 */
+		base = 0;
+	}
+
+	config.get_direction = qixis_cpld_gpio_get_direction;
+	config.drvdata = (void *)cfg;
+	config.regmap = regmap;
+	config.parent = &pdev->dev;
+	config.ngpio_per_reg = 8;
+	config.ngpio = 8;
+
+	switch (cfg->type) {
+	case LX2160ARDB_GPIO_SFP2:
+	case LX2160ARDB_GPIO_SFP3:
+	case LS1046AQDS_GPIO_STAT_PRES2:
+		config.reg_dat_base = GPIO_REGMAP_ADDR(base);
+		config.reg_set_base = GPIO_REGMAP_ADDR(base);
+		break;
+	}
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&pdev->dev, &config));
+}
+
+static const struct of_device_id qixis_cpld_gpio_of_match[] = {
+	{
+		.compatible = "fsl,lx2160ardb-fpga-gpio-sfp2",
+		.data = &lx2160ardb_sfp2_cfg,
+	},
+	{
+		.compatible = "fsl,lx2160ardb-fpga-gpio-sfp3",
+		.data = &lx2160ardb_sfp3_cfg,
+	},
+	{
+		.compatible = "fsl,ls1046aqds-fpga-gpio-stat-pres2",
+		.data = &ls1046aqds_stat_pres2_cfg,
+	},
+
+	{}
+};
+MODULE_DEVICE_TABLE(of, qixis_cpld_gpio_of_match);
+
+static struct platform_driver qixis_cpld_gpio_driver = {
+	.probe = qixis_cpld_gpio_probe,
+	.driver = {
+		.name = "gpio-qixis-cpld",
+		.of_match_table = qixis_cpld_gpio_of_match,
+	},
+};
+module_platform_driver(qixis_cpld_gpio_driver);
-- 
2.25.1

[PATCH 6/9] arm64: dts: lx2160a-rdb: describe the QIXIS FPGA and two child GPIO controllers

[Ioana Ciornei]

Describe the FPGA present on the LX2160ARDB board as a simple-mfd I2C
device. The FPGA presents registers that deal with power-on-reset
timing, muxing, SFP cage monitoring and control etc.

Also add the two GPIO controllers responsible for monitoring and
controlling the SFP+ cages used for MAC5 and MAC6.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../boot/dts/freescale/fsl-lx2160a-rdb.dts    | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts
index 0c44b3cbef77..8209dafd7768 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts
@@ -170,6 +170,37 @@ mt35xu512aba1: flash@1 {
 &i2c0 {
 	status = "okay";
 
+	cpld@66 {
+		compatible = "fsl,lx2160ardb-fpga";
+		reg = <0x66>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		sfp2_csr: gpio@19 {
+			compatible = "fsl,lx2160ardb-fpga-gpio-sfp2";
+			reg = <0x19>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"SFP2_TX_EN", "",
+				"", "",
+				"SFP2_RX_LOS", "SFP2_TX_FAULT",
+				"", "SFP2_MOD_ABS";
+		};
+
+		sfp3_csr: gpio@1a {
+			compatible = "fsl,lx2160ardb-fpga-gpio-sfp2";
+			reg = <0x1a>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"SFP3_TX_EN", "",
+				"", "",
+				"SFP3_RX_LOS", "SFP3_TX_FAULT",
+				"", "SFP3_MOD_ABS";
+		};
+	};
+
 	i2c-mux@77 {
 		compatible = "nxp,pca9547";
 		reg = <0x77>;
-- 
2.25.1

[PATCH 7/9] arm64: dts: ls1046a-qds: describe the FPGA based GPIO controller

[Ioana Ciornei]

The QIXIS FPGA node is extended so that it describes the GPIO controller
responsible for all the status presence lines on both SFP+ cages as well
as the IO SLOTs present on the board.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
index 736722b58e77..64133e63da96 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
@@ -166,8 +166,20 @@ nand@1,0 {
 
 	fpga: board-control@2,0 {
 		compatible = "fsl,ls1046aqds-fpga", "fsl,fpga-qixis", "simple-mfd";
+		#address-cells = <1>;
+		#size-cells = <1>;
 		reg = <0x2 0x0 0x0000100>;
 		ranges = <0 2 0 0x100>;
+
+		stat_pres2: gpio-stat-pres2@c {
+			compatible = "fsl,ls1046aqds-fpga-gpio-stat-pres2";
+			reg = <0xc 1>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"SLOT1", "SLOT2", "SLOT3", "SLOT4", "SLOT5", "SLOT6",
+				"SFP1_MOD_DEF", "SFP2_MOD_DEF";
+		};
 	};
 };
 
-- 
2.25.1

[PATCH 8/9] arm64: dts: lx2160a-rdb: fully describe the two SFP+ cages

[Ioana Ciornei]

Describe the two SFP+ cages found on the LX2160ARDB board with their
respective I2C buses and GPIO lines.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../boot/dts/freescale/fsl-lx2160a-rdb.dts    | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts
index 8209dafd7768..762c38fc6fa0 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-rdb.dts
@@ -31,6 +31,26 @@ sb_3v3: regulator-sb3v3 {
 		regulator-boot-on;
 		regulator-always-on;
 	};
+
+	sfp2: sfp-2 {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp2_i2c>;
+		maximum-power-milliwatt = <2000>;
+		tx-disable-gpios = <&sfp2_csr 0 GPIO_ACTIVE_HIGH>;
+		los-gpios = <&sfp2_csr 4 GPIO_ACTIVE_HIGH>;
+		tx-fault-gpios = <&sfp2_csr 5 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&sfp2_csr 7 GPIO_ACTIVE_LOW>;
+	};
+
+	sfp3: sfp-3 {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp3_i2c>;
+		maximum-power-milliwatt = <2000>;
+		tx-disable-gpios = <&sfp3_csr 0 GPIO_ACTIVE_HIGH>;
+		los-gpios = <&sfp3_csr 4 GPIO_ACTIVE_HIGH>;
+		tx-fault-gpios = <&sfp3_csr 5 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&sfp3_csr 7 GPIO_ACTIVE_LOW>;
+	};
 };
 
 &crypto {
@@ -236,6 +256,31 @@ temperature-sensor@4d {
 				vcc-supply = <&sb_3v3>;
 			};
 		};
+
+		i2c@7 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x7>;
+
+			i2c-mux@75 {
+				compatible = "nxp,pca9547";
+				reg = <0x75>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sfp2_i2c: i2c@4 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0x4>;
+				};
+
+				sfp3_i2c: i2c@5 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0x5>;
+				};
+			};
+		};
 	};
 };
 
-- 
2.25.1

[PATCH 9/9] arm64: dts: ls1046a-qds: describe the two on-board SFP+ cages

[Ioana Ciornei]

Describe the two SFP+ cages present on the LS1046AQDS board and their
associated I2C buses and GPIO lines.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../boot/dts/freescale/fsl-ls1046a-qds.dts    | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
index 64133e63da96..c188977a901e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
@@ -42,6 +42,21 @@ aliases {
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	sfp1: sfp-1 {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp1_i2c>;
+		maximum-power-milliwatt = <2000>;
+		mod-def0-gpios = <&stat_pres2 6 GPIO_ACTIVE_LOW>;
+	};
+
+	sfp2: sfp-2 {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp2_i2c>;
+		maximum-power-milliwatt = <2000>;
+		mod-def0-gpios = <&stat_pres2 7 GPIO_ACTIVE_LOW>;
+	};
+
 };
 
 &dspi {
@@ -139,6 +154,31 @@ temp-sensor@4c {
 				reg = <0x4c>;
 			};
 		};
+
+		i2c@7 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x7>;
+
+			i2c-mux@76 {
+				compatible = "nxp,pca9547";
+				reg = <0x76>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sfp1_i2c: i2c@6 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0x6>;
+				};
+
+				sfp2_i2c: i2c@7 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0x7>;
+				};
+			};
+		};
 	};
 };
 
-- 
2.25.1

Re: [PATCH 1/9] dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO controller

[Krzysztof Kozlowski]

On 09/07/2025 13:26, Ioana Ciornei wrote:
> Add a device tree binding for the QIXIS FPGA based GPIO controller.
> Depending on the board, the QIXIS FPGA exposes registers which act as a
> GPIO controller, each with 8 GPIO lines of fixed direction.
> 
> Since each QIXIS FPGA layout has its particularities, add a separate
> compatible string for each board/GPIO register combination supported.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Your changelog explains patches, which is kind of redundant - we see
that - but does not explain the dependency you have here between patches.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> ---
>  .../bindings/gpio/fsl,fpga-gpio.yaml          | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> new file mode 100644
> index 000000000000..dc7b6c0d9b40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/fsl,fpga-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO controller embedded in the NXP QIXIS FPGA
> +
> +maintainers:
> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
> +
> +description: |
> +  This module is part of the QIXIS FPGA found on some Layerscape boards such as
> +  LX2160ARDB and LS1046AQDS. For more details see
> +  ../board/fsl,fpga-qixis-i2c.yaml.

There are no "board" bindings, so this does not feel like correct path.

> +
> +  Each controller supports a maximum of 8 GPIO lines and each line has a fixed
> +  direction which cannot be changed using a direction register.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,lx2160ardb-fpga-gpio-sfp2
> +      - fsl,lx2160ardb-fpga-gpio-sfp3

What is the difference between these?

> +      - fsl,ls1046aqds-fpga-gpio-stat-pres2

Keep list sorted.


Best regards,
Krzysztof

Re: [PATCH 2/9] dt-bindings: fsl,fpga-qixis-i2c: extend support to also cover the LX2160ARDB FPGA

[Krzysztof Kozlowski]

On 09/07/2025 13:26, Ioana Ciornei wrote:
> Extend the list of supported compatible strings with fsl,lx2160ardb-fpga.
> 
> Since the register map exposed by the LX2160ARDB's FPGA also contains
> two GPIO controllers, accept the necessary GPIO pattern property. At the
> same time, add the #address-cells and #size-cells properties as valid
> ones.
> 
> This is needed because when defining child devices such as the GPIO
> controller described in the added example, the child device needs a the
> reg property to properly identify its register location.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  .../bindings/board/fsl,fpga-qixis-i2c.yaml    | 35 +++++++++++++++++++

So here is the board? Why FPGA is in the board...

>  1 file changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> index 28b37772fb65..e8981f974210 100644
> --- a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> +++ b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> @@ -22,6 +22,13 @@ properties:
>                - fsl,lx2160aqds-fpga
>            - const: fsl,fpga-qixis-i2c
>            - const: simple-mfd
> +      - const: fsl,lx2160ardb-fpga

Weird, your first patch added three compatibles, this adds only one.

> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
>  
>    interrupts:
>      maxItems: 1
> @@ -32,6 +39,10 @@ properties:
>    mux-controller:
>      $ref: /schemas/mux/reg-mux.yaml
>  
> +patternProperties:
> +  "^gpio(@[0-9a-f]+)?$":

Why unit address is optional? Anyway, this is wrong. You do not have
ranges here and earlier you already said children do not have any
addressing. Look at mux.

> +    $ref: /schemas/gpio/fsl,fpga-gpio.yaml
> +
>  required:
>    - compatible
>    - reg
> @@ -68,3 +79,27 @@ examples:
>          };
>      };
>  
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        board-control@66 {
> +            compatible = "fsl,lx2160ardb-fpga";
> +            reg = <0x66>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            gpio@19 {

And what is the meaning of @19?


Best regards,
Krzysztof

Re: [PATCH 1/9] dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO controller

[Ioana Ciornei]

On Wed, Jul 09, 2025 at 02:14:47PM +0200, Krzysztof Kozlowski wrote:
> On 09/07/2025 13:26, Ioana Ciornei wrote:
> > Add a device tree binding for the QIXIS FPGA based GPIO controller.
> > Depending on the board, the QIXIS FPGA exposes registers which act as a
> > GPIO controller, each with 8 GPIO lines of fixed direction.
> > 
> > Since each QIXIS FPGA layout has its particularities, add a separate
> > compatible string for each board/GPIO register combination supported.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> Your changelog explains patches, which is kind of redundant - we see
> that - but does not explain the dependency you have here between patches.
> 

Do you mean the logical dependency between all the components like
FPGAs, GPIOs etc? I can expand on that, sure. I will also update the
cover letter with some of the information below.
If this is not what you are looking for, please let me know.

Layerscape boards such as those that I update here have a QIXIS FPGA
accessible through I2C. This FPGA exposes a set of registers which can
be used to monitor the status of different components, configure muxing,
act as GPIO controllers etc.

Since the register layout that this device exposes is different on a per
board basis, each board has a different compatible string such as the
one that patch 2/9 adds - fsl,lx2160ardb-fpga.

Going deeper, some of these registers are acting as GPIO controllers
exposing status/control of different SFP cages on the board. For these
kind of registers the new gpio-regmap driver is added.

> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 

Sure. Will fix.

> > ---
> >  .../bindings/gpio/fsl,fpga-gpio.yaml          | 44 +++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> > new file mode 100644
> > index 000000000000..dc7b6c0d9b40
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/fsl,fpga-gpio.yaml
> > +$schema: http://devicetree.org/meta-schemas/core.yaml
> > +
> > +title: GPIO controller embedded in the NXP QIXIS FPGA
> > +
> > +maintainers:
> > +  - Ioana Ciornei <ioana.ciornei@nxp.com>
> > +
> > +description: |
> > +  This module is part of the QIXIS FPGA found on some Layerscape boards such as
> > +  LX2160ARDB and LS1046AQDS. For more details see
> > +  ../board/fsl,fpga-qixis-i2c.yaml.
> 
> There are no "board" bindings, so this does not feel like correct path.

As you have seen already in patch 2/9 there is already a dt-binding in
the board/ folder.

> 
> > +
> > +  Each controller supports a maximum of 8 GPIO lines and each line has a fixed
> > +  direction which cannot be changed using a direction register.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,lx2160ardb-fpga-gpio-sfp2
> > +      - fsl,lx2160ardb-fpga-gpio-sfp3
> 
> What is the difference between these?

The layout of the registers backing these two GPIO controllers is the
same but they each expose status/control of different SFP cages.

> 
> > +      - fsl,ls1046aqds-fpga-gpio-stat-pres2
> 
> Keep list sorted.
> 

Sure. Will fix.

Re: [PATCH 1/9] dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO controller

[Krzysztof Kozlowski]

On 09/07/2025 15:55, Ioana Ciornei wrote:
> On Wed, Jul 09, 2025 at 02:14:47PM +0200, Krzysztof Kozlowski wrote:
>> On 09/07/2025 13:26, Ioana Ciornei wrote:
>>> Add a device tree binding for the QIXIS FPGA based GPIO controller.
>>> Depending on the board, the QIXIS FPGA exposes registers which act as a
>>> GPIO controller, each with 8 GPIO lines of fixed direction.
>>>
>>> Since each QIXIS FPGA layout has its particularities, add a separate
>>> compatible string for each board/GPIO register combination supported.
>>>
>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>>
>> Your changelog explains patches, which is kind of redundant - we see
>> that - but does not explain the dependency you have here between patches.
>>
> 
> Do you mean the logical dependency between all the components like
> FPGAs, GPIOs etc? I can expand on that, sure. I will also update the
> cover letter with some of the information below.
> If this is not what you are looking for, please let me know.

I meant here cover letter, not changelog. It does not explain
dependencies between patches. You just explain what each patch is doing
- this is completely redundant cover letter.

> 
> Layerscape boards such as those that I update here have a QIXIS FPGA
> accessible through I2C. This FPGA exposes a set of registers which can
> be used to monitor the status of different components, configure muxing,
> act as GPIO controllers etc.
> 
> Since the register layout that this device exposes is different on a per
> board basis, each board has a different compatible string such as the
> one that patch 2/9 adds - fsl,lx2160ardb-fpga.
> 
> Going deeper, some of these registers are acting as GPIO controllers
> exposing status/control of different SFP cages on the board. For these
> kind of registers the new gpio-regmap driver is added.
> 
>> A nit, subject: drop second/last, redundant "bindings". The
>> "dt-bindings" prefix is already stating that these are bindings.
>> See also:
>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>>
> 
> Sure. Will fix.
> 
>>> ---
>>>  .../bindings/gpio/fsl,fpga-gpio.yaml          | 44 +++++++++++++++++++
>>>  1 file changed, 44 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
>>> new file mode 100644
>>> index 000000000000..dc7b6c0d9b40
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
>>> @@ -0,0 +1,44 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/gpio/fsl,fpga-gpio.yaml
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml
>>> +
>>> +title: GPIO controller embedded in the NXP QIXIS FPGA
>>> +
>>> +maintainers:
>>> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
>>> +
>>> +description: |
>>> +  This module is part of the QIXIS FPGA found on some Layerscape boards such as
>>> +  LX2160ARDB and LS1046AQDS. For more details see
>>> +  ../board/fsl,fpga-qixis-i2c.yaml.
>>
>> There are no "board" bindings, so this does not feel like correct path.
> 
> As you have seen already in patch 2/9 there is already a dt-binding in
> the board/ folder.
> 
>>
>>> +
>>> +  Each controller supports a maximum of 8 GPIO lines and each line has a fixed
>>> +  direction which cannot be changed using a direction register.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - fsl,lx2160ardb-fpga-gpio-sfp2
>>> +      - fsl,lx2160ardb-fpga-gpio-sfp3
>>
>> What is the difference between these?
> 
> The layout of the registers backing these two GPIO controllers is the
> same but they each expose status/control of different SFP cages.

So same devices? Why do they need separate compatibles?



Best regards,
Krzysztof

Re: [PATCH 2/9] dt-bindings: fsl,fpga-qixis-i2c: extend support to also cover the LX2160ARDB FPGA

[Ioana Ciornei]

On Wed, Jul 09, 2025 at 02:17:27PM +0200, Krzysztof Kozlowski wrote:
> On 09/07/2025 13:26, Ioana Ciornei wrote:
> > Extend the list of supported compatible strings with fsl,lx2160ardb-fpga.
> > 
> > Since the register map exposed by the LX2160ARDB's FPGA also contains
> > two GPIO controllers, accept the necessary GPIO pattern property. At the
> > same time, add the #address-cells and #size-cells properties as valid
> > ones.
> > 
> > This is needed because when defining child devices such as the GPIO
> > controller described in the added example, the child device needs a the
> > reg property to properly identify its register location.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  .../bindings/board/fsl,fpga-qixis-i2c.yaml    | 35 +++++++++++++++++++
> 
> So here is the board? Why FPGA is in the board...

I think because its usage and integration is very much dependant on the
board? I am really not sure why it was added there in the first place as
a .txt file.

> 
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> > index 28b37772fb65..e8981f974210 100644
> > --- a/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/board/fsl,fpga-qixis-i2c.yaml
> > @@ -22,6 +22,13 @@ properties:
> >                - fsl,lx2160aqds-fpga
> >            - const: fsl,fpga-qixis-i2c
> >            - const: simple-mfd
> > +      - const: fsl,lx2160ardb-fpga
> 
> Weird, your first patch added three compatibles, this adds only one.

The first patch added 3 compatibles for the registers exposed by this
FPGA that act as a GPIO controller. There are 3 compatibles and not just
one because the registers backing them have different layouts, each
exposing different control/status bits.
As you have pointed out in your last reply on patch 1/9, two of those
compatibles can be merged into a single one.

In this patch I am adding a new compatible for the QIXIS FPGA found on
the LX2160ARDB board so that the simple-mfd-i2c driver has something to
probe on and expose its regmap to the child devices - the gpio
controllers.

> 
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> >  
> >    interrupts:
> >      maxItems: 1
> > @@ -32,6 +39,10 @@ properties:
> >    mux-controller:
> >      $ref: /schemas/mux/reg-mux.yaml
> >  
> > +patternProperties:
> > +  "^gpio(@[0-9a-f]+)?$":
> 
> Why unit address is optional? Anyway, this is wrong. You do not have
> ranges here and earlier you already said children do not have any
> addressing. Look at mux.

Agree on the '?' not being needed here since my plan is to enforce that
if the dts has a GPIO controller defined as a child device then it needs
a unit address.

The unit address is there to convey to the driver what is the address of
the register backing the GPIO controller. I am not sure how else could I
cleanly do that.

My current plan is to:
 - Not change how the board DT files which already define their QIXIS FPGAs
   look like, meaning that they will keep their FPGA child nodes without
   addressing. Very much like the mux is used currently in the
   fsl-lx2160a-qds.dts.
 - For any new boards that need a GPIO driver to be probed on one of the
   FPGA's registers, impose the use of the unit address.

I acknowledge the fact that this a bit confusing, I am open to
suggestions, but I currently do not know another way forward which
cleanly does what I need.

> 
> > +    $ref: /schemas/gpio/fsl,fpga-gpio.yaml
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -68,3 +79,27 @@ examples:
> >          };
> >      };
> >  
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        board-control@66 {
> > +            compatible = "fsl,lx2160ardb-fpga";
> > +            reg = <0x66>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            gpio@19 {
> 
> And what is the meaning of @19?

The register found at address 0x19 is the one backing this GPIO
controller.

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Michael Walle]

Hi Ioana,

great to see another user of gpio-regmap.

On Wed Jul 9, 2025 at 1:26 PM CEST, Ioana Ciornei wrote:
> There are GPIO controllers such as the one present in the LX2160ARDB
> QIXIS CPLD which are single register fixed-direction. This cannot be
> modeled using the gpio-regmap as-is since there is no way to
> present the true direction of a GPIO line.

You mean input and output mixed together in one register? At least
to me, that wasn't so obvious by the commit message, I had to look
at the actual driver.

> In order to make this use case possible, add a new callback to the
> gpio_config structure - .get_direction() - which can be used by user
> drivers to provide the fixed direction per GPIO line.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  drivers/gpio/gpio-regmap.c  | 17 ++++++++++++++++-
>  include/linux/gpio/regmap.h |  3 +++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 87c4225784cf..dac2acb26655 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -32,6 +32,8 @@ struct gpio_regmap {
>  	unsigned int reg_dir_in_base;
>  	unsigned int reg_dir_out_base;
>  
> +	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> +
>  	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
>  			      unsigned int offset, unsigned int *reg,
>  			      unsigned int *mask);
> @@ -129,6 +131,9 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
>  	unsigned int base, val, reg, mask;
>  	int invert, ret;
>  
> +	if (gpio->get_direction)
> +		return gpio->get_direction(gpio, offset);
> +
>  	if (gpio->reg_dat_base && !gpio->reg_set_base)
>  		return GPIO_LINE_DIRECTION_IN;
>  	if (gpio->reg_set_base && !gpio->reg_dat_base)
> @@ -163,7 +168,16 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip,
>  {
>  	struct gpio_regmap *gpio = gpiochip_get_data(chip);
>  	unsigned int base, val, reg, mask;
> -	int invert, ret;
> +	int invert, ret, dir;
> +
> +	if (gpio->get_direction) {
> +		dir = gpio->get_direction(gpio, offset);
> +		if (dir == GPIO_LINE_DIRECTION_IN && output)
> +			return -EOPNOTSUPP;
> +		if (dir == GPIO_LINE_DIRECTION_OUT && !output)
> +			return -EOPNOTSUPP;
> +		return 0;
> +	}

What is the intention here? Shouldn't there be just a .set_direction
op and if there isn't one, return EOPNOTSUPP?

In any case, that is unused code for your driver as far as I see,
because you neither set .reg_dir_in_base nor .reg_dir_out_base and
thus, .direction_input nor .direction_output are set within the
gpio_chip struct (see gpio_regmap_register()).

>  	if (gpio->reg_dir_out_base) {
>  		base = gpio_regmap_addr(gpio->reg_dir_out_base);
> @@ -247,6 +261,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
>  	gpio->reg_clr_base = config->reg_clr_base;
>  	gpio->reg_dir_in_base = config->reg_dir_in_base;
>  	gpio->reg_dir_out_base = config->reg_dir_out_base;
> +	gpio->get_direction = config->get_direction;
>  
>  	chip = &gpio->gpio_chip;
>  	chip->parent = config->parent;
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index c722c67668c6..99fd973e61fa 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -37,6 +37,8 @@ struct regmap;
>   *			offset to a register/bitmask pair. If not
>   *			given the default gpio_regmap_simple_xlate()
>   *			is used.
> + * @get_direction:	(Optional) Callback to the user driver to return the
> + *			fixed direction of the GPIO line
>   * @drvdata:		(Optional) Pointer to driver specific data which is
>   *			not used by gpio-remap but is provided "as is" to the
>   *			driver callback(s).
> @@ -81,6 +83,7 @@ struct gpio_regmap_config {
>  	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
>  			      unsigned int offset, unsigned int *reg,
>  			      unsigned int *mask);
> +	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
>  
>  	void *drvdata;
>  };

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Andrew Lunn]

On Wed, Jul 09, 2025 at 02:26:53PM +0300, Ioana Ciornei wrote:
> There are GPIO controllers such as the one present in the LX2160ARDB
> QIXIS CPLD which are single register fixed-direction. This cannot be
> modeled using the gpio-regmap as-is since there is no way to
> present the true direction of a GPIO line.
> 
> In order to make this use case possible, add a new callback to the
> gpio_config structure - .get_direction() - which can be used by user
> drivers to provide the fixed direction per GPIO line.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  drivers/gpio/gpio-regmap.c  | 17 ++++++++++++++++-
>  include/linux/gpio/regmap.h |  3 +++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 87c4225784cf..dac2acb26655 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -32,6 +32,8 @@ struct gpio_regmap {
>  	unsigned int reg_dir_in_base;
>  	unsigned int reg_dir_out_base;
>  
> +	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> +

This is not my area, so i will deffer to the GPIO
Maintainers. However, it is not clear to me what get_direction()
should return. Is it the current direction, or the supported
directions? Maybe it should be called get_fixed_direction()?

I then wounder how it will be implemented. Since it is fixed, it is
probably just a constant bitmap, and you look at the offset bit in
this batmap? At minimum a ready made helper could be provided, or
rather than have this op, just provide the bitmap, and gpio-regmap.c
can look at the bit in the bitmap?

	Andrew

Re: [PATCH 5/9] drivers: gpio: add QIXIS FPGA GPIO controller

[Andrew Lunn]

> A GPIO controller has a maximum of 8 lines (all found in the same
> register). Even within the same controller, the GPIO lines' direction is
> fixed, either output or input, without the possibility to change it.

Since this is an FPGA, not silicon, is the selection of output or
input a syntheses option?

> +static const struct of_device_id qixis_cpld_gpio_of_match[] = {
> +	{
> +		.compatible = "fsl,lx2160ardb-fpga-gpio-sfp2",
> +		.data = &lx2160ardb_sfp2_cfg,
> +	},
> +	{
> +		.compatible = "fsl,lx2160ardb-fpga-gpio-sfp3",
> +		.data = &lx2160ardb_sfp3_cfg,
> +	},
> +	{
> +		.compatible = "fsl,ls1046aqds-fpga-gpio-stat-pres2",
> +		.data = &ls1046aqds_stat_pres2_cfg,
> +	},

Does the FPGA have an ID register you can read to confirm it is what
you think it is?

Or is the bitstream downloaded at boot by another driver? Can you ask
that driver what bitstream it downloaded?

Given how similar these devices are, it seems like a typ0 could give a
mostly working device which passes testing, so doing some validation
of the compatible against the actual FPGA would be nice.

	Andrew

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Michael Walle]

> just provide the bitmap, and gpio-regmap.c can look at the bit in
> the bitmap?

I like that idea.

-michael

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Ioana Ciornei]

On Wed, Jul 09, 2025 at 05:09:17PM +0200, Andrew Lunn wrote:
> On Wed, Jul 09, 2025 at 02:26:53PM +0300, Ioana Ciornei wrote:
> > There are GPIO controllers such as the one present in the LX2160ARDB
> > QIXIS CPLD which are single register fixed-direction. This cannot be
> > modeled using the gpio-regmap as-is since there is no way to
> > present the true direction of a GPIO line.
> > 
> > In order to make this use case possible, add a new callback to the
> > gpio_config structure - .get_direction() - which can be used by user
> > drivers to provide the fixed direction per GPIO line.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  drivers/gpio/gpio-regmap.c  | 17 ++++++++++++++++-
> >  include/linux/gpio/regmap.h |  3 +++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> > index 87c4225784cf..dac2acb26655 100644
> > --- a/drivers/gpio/gpio-regmap.c
> > +++ b/drivers/gpio/gpio-regmap.c
> > @@ -32,6 +32,8 @@ struct gpio_regmap {
> >  	unsigned int reg_dir_in_base;
> >  	unsigned int reg_dir_out_base;
> >  
> > +	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> > +
> 
> This is not my area, so i will deffer to the GPIO
> Maintainers. However, it is not clear to me what get_direction()
> should return. Is it the current direction, or the supported
> directions? Maybe it should be called get_fixed_direction()?
> 
> I then wounder how it will be implemented. Since it is fixed, it is
> probably just a constant bitmap, and you look at the offset bit in
> this batmap? At minimum a ready made helper could be provided, or
> rather than have this op, just provide the bitmap, and gpio-regmap.c
> can look at the bit in the bitmap?
>

That is indeed possible and would save us from an extra callback that
doesn't do much, it just looks into the same kind of bitmap that
gpio-regmap could just have access to directly.

I will wait for feedback from the GPIO maintainers but I do like the
idea.

Thanks!

Re: [PATCH 5/9] drivers: gpio: add QIXIS FPGA GPIO controller

[Ioana Ciornei]

On Wed, Jul 09, 2025 at 05:17:18PM +0200, Andrew Lunn wrote:
> > A GPIO controller has a maximum of 8 lines (all found in the same
> > register). Even within the same controller, the GPIO lines' direction is
> > fixed, either output or input, without the possibility to change it.
> 
> Since this is an FPGA, not silicon, is the selection of output or
> input a syntheses option?

I suppose so, yes. The idea is that in this particular case, the fixed
direction for each GPIO line (bit in the register) matches the use case
and will not be changed. For example, the presence detect or rx los
GPIOs for the SFP cages are only input, while the tx enable one is
output always.

>
> > +static const struct of_device_id qixis_cpld_gpio_of_match[] = {
> > +	{
> > +		.compatible = "fsl,lx2160ardb-fpga-gpio-sfp2",
> > +		.data = &lx2160ardb_sfp2_cfg,
> > +	},
> > +	{
> > +		.compatible = "fsl,lx2160ardb-fpga-gpio-sfp3",
> > +		.data = &lx2160ardb_sfp3_cfg,
> > +	},
> > +	{
> > +		.compatible = "fsl,ls1046aqds-fpga-gpio-stat-pres2",
> > +		.data = &ls1046aqds_stat_pres2_cfg,
> > +	},
> 
> Does the FPGA have an ID register you can read to confirm it is what
> you think it is?
> 
> Or is the bitstream downloaded at boot by another driver? Can you ask
> that driver what bitstream it downloaded?
> 
> Given how similar these devices are, it seems like a typ0 could give a
> mostly working device which passes testing, so doing some validation
> of the compatible against the actual FPGA would be nice.
> 

The FPGA does have an ID register that we could verify and match against
the board type that we expect.

On the other hand, I am not 100% on board with the idea to check this
from the GPIO driver which teoretically should only touch its one
register. Maybe from the parent's driver we could do that and prevent
the probing of children if things don't match up. But this does prove to
be complicated since those drivers are simple-mfd (for LS1046AQDS) and
simple-mfd-i2c (for LX2160ARDB). And I don't think it would be wise to
add some specific board logic into of/platform.c.

Ioana

Re: [PATCH 5/9] drivers: gpio: add QIXIS FPGA GPIO controller

[Andrew Lunn]

> The FPGA does have an ID register that we could verify and match against
> the board type that we expect.
> 
> On the other hand, I am not 100% on board with the idea to check this
> from the GPIO driver which teoretically should only touch its one
> register. Maybe from the parent's driver we could do that and prevent
> the probing of children if things don't match up. But this does prove to
> be complicated since those drivers are simple-mfd (for LS1046AQDS) and
> simple-mfd-i2c (for LX2160ARDB). And I don't think it would be wise to
> add some specific board logic into of/platform.c.

My experience is, DT authors will mess up and put in the wrong
compatible. And the wrong compatible might be enough for it to mostly
work, so you end up with deployed systems with wrong compatibles. It
then becomes difficult to actually extend the use of the compatible,
without causing regressions.

Also, checking will catch putting the wrong bitstream into the FPGA.

So if you can check it, do check it, and return -ENODEV.

   Andrew

Re: [PATCH 1/9] dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO controller

[Rob Herring]

On Wed, Jul 09, 2025 at 02:26:50PM +0300, Ioana Ciornei wrote:
> Add a device tree binding for the QIXIS FPGA based GPIO controller.
> Depending on the board, the QIXIS FPGA exposes registers which act as a
> GPIO controller, each with 8 GPIO lines of fixed direction.
> 
> Since each QIXIS FPGA layout has its particularities, add a separate
> compatible string for each board/GPIO register combination supported.

This could be covered in my proposed trivial gpio schema[1].


> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
>  .../bindings/gpio/fsl,fpga-gpio.yaml          | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> new file mode 100644
> index 000000000000..dc7b6c0d9b40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/fsl,fpga-gpio.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/fsl,fpga-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO controller embedded in the NXP QIXIS FPGA
> +
> +maintainers:
> +  - Ioana Ciornei <ioana.ciornei@nxp.com>
> +
> +description: |
> +  This module is part of the QIXIS FPGA found on some Layerscape boards such as
> +  LX2160ARDB and LS1046AQDS. For more details see
> +  ../board/fsl,fpga-qixis-i2c.yaml.

Or this is simple enough, just add this as a child node in that schema.

Rob

[1] https://lore.kernel.org/all/20250701225355.2977294-1-robh@kernel.org/

Re: [PATCH 2/9] dt-bindings: fsl,fpga-qixis-i2c: extend support to also cover the LX2160ARDB FPGA

[Rob Herring]

On Wed, Jul 09, 2025 at 02:17:27PM +0200, Krzysztof Kozlowski wrote:
> On 09/07/2025 13:26, Ioana Ciornei wrote:
> > Extend the list of supported compatible strings with fsl,lx2160ardb-fpga.
> > 
> > Since the register map exposed by the LX2160ARDB's FPGA also contains
> > two GPIO controllers, accept the necessary GPIO pattern property. At the
> > same time, add the #address-cells and #size-cells properties as valid
> > ones.
> > 
> > This is needed because when defining child devices such as the GPIO
> > controller described in the added example, the child device needs a the
> > reg property to properly identify its register location.
> > 
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  .../bindings/board/fsl,fpga-qixis-i2c.yaml    | 35 +++++++++++++++++++
> 
> So here is the board? Why FPGA is in the board...

This directory is for board level custom logic in FPGA, CPLD, etc.

Rob

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Linus Walleij]

On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote:

> This is not my area, so i will deffer to the GPIO
> Maintainers. However, it is not clear to me what get_direction()
> should return.

This callback should return the current direction as set up
in the hardware.

A major usecase is that this is called when the gpiochip is
registered to read out all the current directions of the GPIO
lines, so the kernel has a clear idea of the state of the
hardware.

Calling this should ideally result in a read of the status from
a hardware register.

Yours,
Linus Walleij

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Andrew Lunn]

On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote:
> On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > This is not my area, so i will deffer to the GPIO
> > Maintainers. However, it is not clear to me what get_direction()
> > should return.
> 
> This callback should return the current direction as set up
> in the hardware.
> 
> A major usecase is that this is called when the gpiochip is
> registered to read out all the current directions of the GPIO
> lines, so the kernel has a clear idea of the state of the
> hardware.
> 
> Calling this should ideally result in a read of the status from
> a hardware register.

O.K, so completely different to what is proposed in this patch.

Maybe you can suggest a better name.

	Andrew

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Linus Walleij]

On Fri, Jul 11, 2025 at 7:45 PM Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote:
> > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > This is not my area, so i will deffer to the GPIO
> > > Maintainers. However, it is not clear to me what get_direction()
> > > should return.
> >
> > This callback should return the current direction as set up
> > in the hardware.
> >
> > A major usecase is that this is called when the gpiochip is
> > registered to read out all the current directions of the GPIO
> > lines, so the kernel has a clear idea of the state of the
> > hardware.
> >
> > Calling this should ideally result in a read of the status from
> > a hardware register.
>
> O.K, so completely different to what is proposed in this patch.
>
> Maybe you can suggest a better name.

If the hardware only supports one direction, then .get_direction()
should return that direction.

What the patch does is to
read the direction from the hardware and use that in the
set_direction() callback, as if all regmapped hardware in the
world had fixed direction, that's wrong.

I'd just add something custom in gpio-regmap if this is
something reoccuring in regmapped GPIO drivers.

bool is_fixed_direction(struct gpio_regmap *gpio, unsigned int offset)

or so?

Then the core can use is_fixed_direction() together
with gpio_get_direction() to check if it can do
a certain set_direction().

Pseudocode:

mydir = get_direction(line)
if (is_fixed_direction(line) && (mydir != requested_dir)
  return -ERROR;

Yours,
Linus Walleij

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 2261 bytes --]

On Fri Jul 11, 2025 at 8:06 PM CEST, Linus Walleij wrote:
> On Fri, Jul 11, 2025 at 7:45 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote:
> > > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > This is not my area, so i will deffer to the GPIO
> > > > Maintainers. However, it is not clear to me what get_direction()
> > > > should return.
> > >
> > > This callback should return the current direction as set up
> > > in the hardware.
> > >
> > > A major usecase is that this is called when the gpiochip is
> > > registered to read out all the current directions of the GPIO
> > > lines, so the kernel has a clear idea of the state of the
> > > hardware.
> > >
> > > Calling this should ideally result in a read of the status from
> > > a hardware register.
> >
> > O.K, so completely different to what is proposed in this patch.
> >
> > Maybe you can suggest a better name.
>
> If the hardware only supports one direction, then .get_direction()
> should return that direction.
>
> What the patch does is to
> read the direction from the hardware and use that in the
> set_direction() callback, as if all regmapped hardware in the
> world had fixed direction, that's wrong.
>
> I'd just add something custom in gpio-regmap if this is
> something reoccuring in regmapped GPIO drivers.
>
> bool is_fixed_direction(struct gpio_regmap *gpio, unsigned int offset)
>
> or so?
>
> Then the core can use is_fixed_direction() together
> with gpio_get_direction() to check if it can do
> a certain set_direction().
>
> Pseudocode:
>
> mydir = get_direction(line)
> if (is_fixed_direction(line) && (mydir != requested_dir)
>   return -ERROR;

You don't need a .is_fixed_direction(). You can deduce that if only
.get_direction() is set in the gpio-regmap config.

mydir = get_direction(line)
if (!config->set_direction && mydir != requested_dir)
  return -ERROR;

That or either Andrew's idea of setting a bitmap within the
gpio-regmap config which already tells the gpio-regmap core and then
amend gpio_regmap_get_direction() to return that fixed direction if
that bitmap is not NULL.

I'm fine with both.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Ioana Ciornei]

On Wed, Jul 09, 2025 at 05:01:38PM +0200, Michael Walle wrote:
> Hi Ioana,
> 
> great to see another user of gpio-regmap.
> 
> On Wed Jul 9, 2025 at 1:26 PM CEST, Ioana Ciornei wrote:
> > There are GPIO controllers such as the one present in the LX2160ARDB
> > QIXIS CPLD which are single register fixed-direction. This cannot be
> > modeled using the gpio-regmap as-is since there is no way to
> > present the true direction of a GPIO line.
> 
> You mean input and output mixed together in one register? At least
> to me, that wasn't so obvious by the commit message, I had to look
> at the actual driver.
> 

Yes, that is right. I will update the commit message to make it more
clear for everybody.

> > In order to make this use case possible, add a new callback to the
> > gpio_config structure - .get_direction() - which can be used by user
> > drivers to provide the fixed direction per GPIO line.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> >  drivers/gpio/gpio-regmap.c  | 17 ++++++++++++++++-
> >  include/linux/gpio/regmap.h |  3 +++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> > index 87c4225784cf..dac2acb26655 100644
> > --- a/drivers/gpio/gpio-regmap.c
> > +++ b/drivers/gpio/gpio-regmap.c
> > @@ -32,6 +32,8 @@ struct gpio_regmap {
> >  	unsigned int reg_dir_in_base;
> >  	unsigned int reg_dir_out_base;
> >  
> > +	int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> > +
> >  	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> >  			      unsigned int offset, unsigned int *reg,
> >  			      unsigned int *mask);
> > @@ -129,6 +131,9 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
> >  	unsigned int base, val, reg, mask;
> >  	int invert, ret;
> >  
> > +	if (gpio->get_direction)
> > +		return gpio->get_direction(gpio, offset);
> > +
> >  	if (gpio->reg_dat_base && !gpio->reg_set_base)
> >  		return GPIO_LINE_DIRECTION_IN;
> >  	if (gpio->reg_set_base && !gpio->reg_dat_base)
> > @@ -163,7 +168,16 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip,
> >  {
> >  	struct gpio_regmap *gpio = gpiochip_get_data(chip);
> >  	unsigned int base, val, reg, mask;
> > -	int invert, ret;
> > +	int invert, ret, dir;
> > +
> > +	if (gpio->get_direction) {
> > +		dir = gpio->get_direction(gpio, offset);
> > +		if (dir == GPIO_LINE_DIRECTION_IN && output)
> > +			return -EOPNOTSUPP;
> > +		if (dir == GPIO_LINE_DIRECTION_OUT && !output)
> > +			return -EOPNOTSUPP;
> > +		return 0;
> > +	}
> 
> What is the intention here? Shouldn't there be just a .set_direction
> op and if there isn't one, return EOPNOTSUPP?
> 
> In any case, that is unused code for your driver as far as I see,
> because you neither set .reg_dir_in_base nor .reg_dir_out_base and
> thus, .direction_input nor .direction_output are set within the
> gpio_chip struct (see gpio_regmap_register()).
> 

Yes, you are right. I did want to return ealier an EOPNOTSUPP to make
sure that in my specific case no directions would be changed. But that
is not needed indeed since, as you said, I do not use .reg_dir_in_base
nor .reg_dir_out_base.

I will remove this in v2.

And since we are on the subject, I will try out Andrew's suggestion with
giving gpio-regmap a bitmap to use directly.

Thanks!
Ioana

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Ioana Ciornei]

On Mon, Jul 14, 2025 at 08:36:03AM +0200, Michael Walle wrote:
> On Fri Jul 11, 2025 at 8:06 PM CEST, Linus Walleij wrote:
> > On Fri, Jul 11, 2025 at 7:45 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote:
> > > > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > >
> > > > > This is not my area, so i will deffer to the GPIO
> > > > > Maintainers. However, it is not clear to me what get_direction()
> > > > > should return.
> > > >
> > > > This callback should return the current direction as set up
> > > > in the hardware.
> > > >
> > > > A major usecase is that this is called when the gpiochip is
> > > > registered to read out all the current directions of the GPIO
> > > > lines, so the kernel has a clear idea of the state of the
> > > > hardware.
> > > >
> > > > Calling this should ideally result in a read of the status from
> > > > a hardware register.
> > >
> > > O.K, so completely different to what is proposed in this patch.
> > >
> > > Maybe you can suggest a better name.
> >
> > If the hardware only supports one direction, then .get_direction()
> > should return that direction.
> >
> > What the patch does is to
> > read the direction from the hardware and use that in the
> > set_direction() callback, as if all regmapped hardware in the
> > world had fixed direction, that's wrong.
> >
> > I'd just add something custom in gpio-regmap if this is
> > something reoccuring in regmapped GPIO drivers.
> >
> > bool is_fixed_direction(struct gpio_regmap *gpio, unsigned int offset)
> >
> > or so?
> >
> > Then the core can use is_fixed_direction() together
> > with gpio_get_direction() to check if it can do
> > a certain set_direction().
> >
> > Pseudocode:
> >
> > mydir = get_direction(line)
> > if (is_fixed_direction(line) && (mydir != requested_dir)
> >   return -ERROR;
> 
> You don't need a .is_fixed_direction(). You can deduce that if only
> .get_direction() is set in the gpio-regmap config.
> 
> mydir = get_direction(line)
> if (!config->set_direction && mydir != requested_dir)
>   return -ERROR;

This implies that gpio_regmap_config gets two new callbacks
.get_direction() and .set_direction() and that in case .set_direction()
is set in gpio-regmap config, then its used directly from
gpio_regmap_set_direction(), right?

>
> That or either Andrew's idea of setting a bitmap within the
> gpio-regmap config which already tells the gpio-regmap core and then
> amend gpio_regmap_get_direction() to return that fixed direction if
> that bitmap is not NULL.

Even though at first glance I was under the impression that the bitmap
solution is cleaner, how big should the bitmap be knows only the final
gpio driver. Without this information, we cannot know the bitmap size so
that we can use the DECLARE_BITMAP macro in gpio-regmap config.

Ioana

Re: [PATCH 1/9] dt-bindings: gpio: add bindings for the QIXIS FPGA based GPIO controller

[Ioana Ciornei]

On Thu, Jul 10, 2025 at 05:01:32PM -0500, Rob Herring wrote:
> On Wed, Jul 09, 2025 at 02:26:50PM +0300, Ioana Ciornei wrote:
> > Add a device tree binding for the QIXIS FPGA based GPIO controller.
> > Depending on the board, the QIXIS FPGA exposes registers which act as a
> > GPIO controller, each with 8 GPIO lines of fixed direction.
> > 
> > Since each QIXIS FPGA layout has its particularities, add a separate
> > compatible string for each board/GPIO register combination supported.
> 
> This could be covered in my proposed trivial gpio schema[1].

Indeed. I will update the trivial-gpio.yaml schema (once that gets
merged) with these new compatible strings.

Thanks,
Ioana

Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 2430 bytes --]

Hi,

> > > Then the core can use is_fixed_direction() together
> > > with gpio_get_direction() to check if it can do
> > > a certain set_direction().
> > >
> > > Pseudocode:
> > >
> > > mydir = get_direction(line)
> > > if (is_fixed_direction(line) && (mydir != requested_dir)
> > >   return -ERROR;
> > 
> > You don't need a .is_fixed_direction(). You can deduce that if only
> > .get_direction() is set in the gpio-regmap config.
> > 
> > mydir = get_direction(line)
> > if (!config->set_direction && mydir != requested_dir)
> >   return -ERROR;
>
> This implies that gpio_regmap_config gets two new callbacks
> .get_direction() and .set_direction() and that in case .set_direction()
> is set in gpio-regmap config, then its used directly from
> gpio_regmap_set_direction(), right?

Yes. Or just .get_direction() for now and assume that .set_direction
is NULL, i.e. it just covers your use case for the fixed direction.

.. Oh I just noticed that this will really limit the use to either
all or nothing. You cannot mix set user defined directions with
fixed directions. Linus' .is_fixed_direction() will allow that.

Though I wonder if we really want to let the user override
.get_direction() and .set_direction(). I still prefer the bitmap.

> > That or either Andrew's idea of setting a bitmap within the
> > gpio-regmap config which already tells the gpio-regmap core and then
> > amend gpio_regmap_get_direction() to return that fixed direction if
> > that bitmap is not NULL.
>
> Even though at first glance I was under the impression that the bitmap
> solution is cleaner, how big should the bitmap be knows only the final
> gpio driver. Without this information, we cannot know the bitmap size so
> that we can use the DECLARE_BITMAP macro in gpio-regmap config.

Actually, I had the same thought. But there is also bitmap_alloc()
and friends, no? And the gpio-regmap config contains the ngpios.

In gpio_regmap_get_direction():

if (gpio->fixed_direction_output && test_bit(offset, gpio->fixed_direction_output))
	return GPIO_LINE_DIRECTION_OUT;

Which implies that once .fixed_direction is set it will always be
checked. So if someone in the future wants to mix and match
.fixed_direction with .reg_dir_{in,out}_base we have to add a second
bitmap which tells you what pins are fixed.

You'd probably need to make sure offset is smaller than ngpio.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]