[PATCH v6 1/6] mfd: simple-mfd-i2c: Add Delta TN48M CPLD support

[PATCH v6 1/6] mfd: simple-mfd-i2c: Add Delta TN48M CPLD support

[Robert Marko]

Delta TN48M switches have a Lattice CPLD that serves
multiple purposes including being a GPIO expander.

So, lets use the simple I2C MFD driver to provide the MFD core.

Also add a virtual symbol which pulls in the simple-mfd-i2c driver and
provide a common symbol on which the subdevice drivers can depend on.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v2:
* Drop the custom MFD driver and header
* Use simple I2C MFD driver

 drivers/mfd/Kconfig          | 10 ++++++++++
 drivers/mfd/simple-mfd-i2c.c |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b74efa469e90..733c2f9adb15 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -297,6 +297,16 @@ config MFD_ASIC3
 	  This driver supports the ASIC3 multifunction chip found on many
 	  PDAs (mainly iPAQ and HTC based ones)
 
+config MFD_TN48M_CPLD
+	tristate "Delta Networks TN48M switch CPLD driver"
+	depends on I2C
+	select MFD_SIMPLE_MFD_I2C
+	help
+	  Select this option to enable support for Delta Networks TN48M switch
+	  CPLD. It consists of reset and GPIO drivers. CPLD provides GPIOS-s
+	  for the SFP slots as well as power supply related information.
+	  SFP support depends on the GPIO driver being selected.
+
 config PMIC_DA903X
 	bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"
 	depends on I2C=y
diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 87f684cff9a1..af8e91781417 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
 
 static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "kontron,sl28cpld" },
+	{ .compatible = "delta,tn48m-cpld" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
-- 
2.31.1

[PATCH v6 2/6] gpio: Add Delta TN48M CPLD GPIO driver

[Robert Marko]

Delta TN48M CPLD is used as a GPIO expander for the SFP GPIOs.

It is a mix of input only and output only pins.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v6:
* Drop unused header
* Return the return value of device_property_read_u32()
instead of a hardcoded return

Changes in v2:
* Rewrite to use simple I2C MFD and GPIO regmap
* Drop DT bindings for pin numbering

 drivers/gpio/Kconfig      | 12 ++++++
 drivers/gpio/Makefile     |  1 +
 drivers/gpio/gpio-tn48m.c | 88 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 drivers/gpio/gpio-tn48m.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e3607ec4c2e8..472f7764508e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1310,6 +1310,18 @@ config GPIO_TIMBERDALE
 	help
 	Add support for the GPIO IP in the timberdale FPGA.
 
+config GPIO_TN48M_CPLD
+	tristate "Delta Networks TN48M switch CPLD GPIO driver"
+	depends on MFD_TN48M_CPLD
+	select GPIO_REGMAP
+	help
+	  This enables support for the GPIOs found on the Delta
+	  Networks TN48M switch CPLD.
+	  They are used for inputs and outputs on the SFP slots.
+
+	  This driver can also be built as a module. If so, the
+	  module will be called gpio-tn48m.
+
 config GPIO_TPS65086
 	tristate "TI TPS65086 GPO"
 	depends on MFD_TPS65086
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c58a90a3c3b1..271fb806475e 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_GPIO_TEGRA186)		+= gpio-tegra186.o
 obj-$(CONFIG_GPIO_TEGRA)		+= gpio-tegra.o
 obj-$(CONFIG_GPIO_THUNDERX)		+= gpio-thunderx.o
 obj-$(CONFIG_GPIO_TIMBERDALE)		+= gpio-timberdale.o
+obj-$(CONFIG_GPIO_TN48M_CPLD)		+= gpio-tn48m.o
 obj-$(CONFIG_GPIO_TPIC2810)		+= gpio-tpic2810.o
 obj-$(CONFIG_GPIO_TPS65086)		+= gpio-tps65086.o
 obj-$(CONFIG_GPIO_TPS65218)		+= gpio-tps65218.o
diff --git a/drivers/gpio/gpio-tn48m.c b/drivers/gpio/gpio-tn48m.c
new file mode 100644
index 000000000000..b12a6b4bc4b3
--- /dev/null
+++ b/drivers/gpio/gpio-tn48m.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Delta TN48M CPLD GPIO driver
+ *
+ * Copyright (C) 2021 Sartura Ltd.
+ *
+ * Author: Robert Marko <robert.marko@sartura.hr>
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+enum tn48m_gpio_type {
+	TN48M_SFP_TX_DISABLE = 1,
+	TN48M_SFP_PRESENT,
+	TN48M_SFP_LOS,
+};
+
+static int tn48m_gpio_probe(struct platform_device *pdev)
+{
+	struct gpio_regmap_config config = {0};
+	enum tn48m_gpio_type type;
+	struct regmap *regmap;
+	u32 base;
+	int ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
+	type = (uintptr_t)device_get_match_data(&pdev->dev);
+	if (!type)
+		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)
+		return -ENODEV;
+
+	config.regmap = regmap;
+	config.parent = &pdev->dev;
+	config.ngpio = 4;
+
+	switch (type) {
+	case TN48M_SFP_TX_DISABLE:
+		config.reg_set_base = base;
+		break;
+	case TN48M_SFP_PRESENT:
+		config.reg_dat_base = base;
+		break;
+	case TN48M_SFP_LOS:
+		config.reg_dat_base = base;
+		break;
+	default:
+		dev_err(&pdev->dev, "unknown type %d\n", type);
+		return -ENODEV;
+	}
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&pdev->dev, &config));
+}
+
+static const struct of_device_id tn48m_gpio_of_match[] = {
+	{ .compatible = "delta,tn48m-gpio-sfp-tx-disable", .data = (void *)TN48M_SFP_TX_DISABLE },
+	{ .compatible = "delta,tn48m-gpio-sfp-present", .data = (void *)TN48M_SFP_PRESENT },
+	{ .compatible = "delta,tn48m-gpio-sfp-los", .data = (void *)TN48M_SFP_LOS },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tn48m_gpio_of_match);
+
+static struct platform_driver tn48m_gpio_driver = {
+	.driver = {
+		.name = "delta-tn48m-gpio",
+		.of_match_table = tn48m_gpio_of_match,
+	},
+	.probe = tn48m_gpio_probe,
+};
+module_platform_driver(tn48m_gpio_driver);
+
+MODULE_AUTHOR("Robert Marko <robert.marko@sartura.hr>");
+MODULE_DESCRIPTION("Delta TN48M CPLD GPIO driver");
+MODULE_LICENSE("GPL");
-- 
2.31.1

[PATCH v6 3/6] dt-bindings: reset: Add Delta TN48M

[Robert Marko]

Add header for the Delta TN48M CPLD provided
resets.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 include/dt-bindings/reset/delta,tn48m-reset.h | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 include/dt-bindings/reset/delta,tn48m-reset.h

diff --git a/include/dt-bindings/reset/delta,tn48m-reset.h b/include/dt-bindings/reset/delta,tn48m-reset.h
new file mode 100644
index 000000000000..d4e9ed12de3e
--- /dev/null
+++ b/include/dt-bindings/reset/delta,tn48m-reset.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Delta TN48M CPLD GPIO driver
+ *
+ * Copyright (C) 2021 Sartura Ltd.
+ *
+ * Author: Robert Marko <robert.marko@sartura.hr>
+ */
+
+#ifndef _DT_BINDINGS_RESET_TN48M_H
+#define _DT_BINDINGS_RESET_TN48M_H
+
+#define CPU_88F7040_RESET	0
+#define CPU_88F6820_RESET	1
+#define MAC_98DX3265_RESET	2
+#define PHY_88E1680_RESET	3
+#define PHY_88E1512_RESET	4
+#define POE_RESET		5
+
+#endif /* _DT_BINDINGS_RESET_TN48M_H */
-- 
2.31.1

[PATCH v6 4/6] reset: Add Delta TN48M CPLD reset controller

[Robert Marko]

Delta TN48M CPLD exposes resets for the following:
* 88F7040 SoC
* 88F6820 SoC
* 98DX3265 switch MAC-s
* 88E1680 PHY-s
* 88E1512 PHY
* PoE PSE controller

Controller supports only self clearing resets.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes in v5:
* Allow COMPILE_TEST as well
* Default to MFD_TN48M_CPLD

Changes in v4:
* Drop assert and deassert as only self-clearing
resets are support by the HW
* Make sure that reset is cleared before returning
from reset.

 drivers/reset/Kconfig       |  10 +++
 drivers/reset/Makefile      |   1 +
 drivers/reset/reset-tn48m.c | 128 ++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 drivers/reset/reset-tn48m.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 4171c6f76385..b647bb945597 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -237,6 +237,16 @@ config RESET_TI_SYSCON
 	  you wish to use the reset framework for such memory-mapped devices,
 	  say Y here. Otherwise, say N.
 
+config RESET_TN48M_CPLD
+	tristate "Delta Networks TN48M switch CPLD reset controller"
+	depends on MFD_TN48M_CPLD || COMPILE_TEST
+	default MFD_TN48M_CPLD
+	help
+	  This enables the reset controller driver for the Delta TN48M CPLD.
+	  It provides reset signals for Armada 7040 and 385 SoC-s, Alleycat 3X
+	  switch MAC-s, Alaska OOB ethernet PHY, Quad Alaska ethernet PHY-s and
+	  Microchip PD69200 PoE PSE controller.
+
 config RESET_UNIPHIER
 	tristate "Reset controller driver for UniPhier SoCs"
 	depends on ARCH_UNIPHIER || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 65a118a91b27..d048498e6566 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
+obj-$(CONFIG_RESET_TN48M_CPLD) += reset-tn48m.o
 obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
 obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
 obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
diff --git a/drivers/reset/reset-tn48m.c b/drivers/reset/reset-tn48m.c
new file mode 100644
index 000000000000..8b58685f4043
--- /dev/null
+++ b/drivers/reset/reset-tn48m.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Delta TN48M CPLD reset driver
+ *
+ * Copyright (C) 2021 Sartura Ltd.
+ *
+ * Author: Robert Marko <robert.marko@sartura.hr>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+#include <dt-bindings/reset/delta,tn48m-reset.h>
+
+#define TN48M_RESET_REG		0x10
+
+#define TN48M_RESET_TIMEOUT	125000
+#define TN48M_RESET_SLEEP	10
+
+struct tn48_reset_map {
+	u8 bit;
+};
+
+struct tn48_reset_data {
+	struct reset_controller_dev rcdev;
+	struct regmap *regmap;
+};
+
+static const struct tn48_reset_map tn48m_resets[] = {
+	[CPU_88F7040_RESET] = {0},
+	[CPU_88F6820_RESET] = {1},
+	[MAC_98DX3265_RESET] = {2},
+	[PHY_88E1680_RESET] = {4},
+	[PHY_88E1512_RESET] = {6},
+	[POE_RESET] = {7},
+};
+
+static inline struct tn48_reset_data *to_tn48_reset_data(
+			struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct tn48_reset_data, rcdev);
+}
+
+static int tn48m_control_reset(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct tn48_reset_data *data = to_tn48_reset_data(rcdev);
+	unsigned int val;
+
+	regmap_update_bits(data->regmap, TN48M_RESET_REG,
+			   BIT(tn48m_resets[id].bit), 0);
+
+	return regmap_read_poll_timeout(data->regmap,
+					TN48M_RESET_REG,
+					val,
+					val & BIT(tn48m_resets[id].bit),
+					TN48M_RESET_SLEEP,
+					TN48M_RESET_TIMEOUT);
+}
+
+static int tn48m_control_status(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct tn48_reset_data *data = to_tn48_reset_data(rcdev);
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(data->regmap, TN48M_RESET_REG, &regval);
+	if (ret < 0)
+		return ret;
+
+	if (BIT(tn48m_resets[id].bit) & regval)
+		return 0;
+	else
+		return 1;
+}
+
+static const struct reset_control_ops tn48_reset_ops = {
+	.reset		= tn48m_control_reset,
+	.status		= tn48m_control_status,
+};
+
+static int tn48m_reset_probe(struct platform_device *pdev)
+{
+	struct tn48_reset_data *data;
+	struct regmap *regmap;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap)
+		return -ENODEV;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->regmap = regmap;
+
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.ops = &tn48_reset_ops;
+	data->rcdev.nr_resets = ARRAY_SIZE(tn48m_resets);
+	data->rcdev.of_node = pdev->dev.of_node;
+
+	return devm_reset_controller_register(&pdev->dev, &data->rcdev);
+}
+
+static const struct of_device_id tn48m_reset_of_match[] = {
+	{ .compatible = "delta,tn48m-reset", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tn48m_reset_of_match);
+
+static struct platform_driver tn48m_reset_driver = {
+	.driver = {
+		.name = "delta-tn48m-reset",
+		.of_match_table = tn48m_reset_of_match,
+	},
+	.probe = tn48m_reset_probe,
+};
+module_platform_driver(tn48m_reset_driver);
+
+MODULE_AUTHOR("Robert Marko <robert.marko@sartura.hr>");
+MODULE_DESCRIPTION("Delta TN48M CPLD reset driver");
+MODULE_LICENSE("GPL");
-- 
2.31.1

[PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

Add binding documents for the Delta TN48M CPLD drivers.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v3:
* Include bindings for reset driver

Changes in v2:
* Implement MFD as a simple I2C MFD
* Add GPIO bindings as separate

 .../bindings/gpio/delta,tn48m-gpio.yaml       | 42 +++++++++
 .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
 .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
 3 files changed, 167 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
 create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml

diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
new file mode 100644
index 000000000000..aca646aecb12
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Delta Networks TN48M CPLD GPIO controller
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+description: |
+  This module is part of the Delta TN48M multi-function device. For more
+  details see ../mfd/delta,tn48m-cpld.yaml.
+
+  GPIO controller module provides GPIO-s for the SFP slots.
+  It is split into 3 controllers, one output only for the SFP TX disable
+  pins, one input only for the SFP present pins and one input only for
+  the SFP LOS pins.
+
+properties:
+  compatible:
+    enum:
+      - delta,tn48m-gpio-sfp-tx-disable
+      - delta,tn48m-gpio-sfp-present
+      - delta,tn48m-gpio-sfp-los
+
+  reg:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - "#gpio-cells"
+  - gpio-controller
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
new file mode 100644
index 000000000000..2c6e2adf73ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Delta Networks TN48M CPLD controller
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+description: |
+  Lattice CPLD onboard the TN48M switches is used for system
+  management.
+
+  It provides information about the hardware model, revision,
+  PSU status etc.
+
+  It is also being used as a GPIO expander for the SFP slots and
+  reset controller for the switch MAC-s and other peripherals.
+
+properties:
+  compatible:
+    const: delta,tn48m-cpld
+
+  reg:
+    description:
+      I2C device address.
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+patternProperties:
+  "^gpio(@[0-9a-f]+)?$":
+    $ref: ../gpio/delta,tn48m-gpio.yaml
+
+  "^reset-controller?$":
+    $ref: ../reset/delta,tn48m-reset.yaml
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cpld@41 {
+            compatible = "delta,tn48m-cpld";
+            reg = <0x41>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            gpio@31 {
+                compatible = "delta,tn48m-gpio-sfp-tx-disable";
+                reg = <0x31>;
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            gpio@3a {
+                compatible = "delta,tn48m-gpio-sfp-present";
+                reg = <0x3a>;
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            gpio@40 {
+                compatible = "delta,tn48m-gpio-sfp-los";
+                reg = <0x40>;
+                gpio-controller;
+                #gpio-cells = <2>;
+            };
+
+            reset-controller {
+              compatible = "delta,tn48m-reset";
+              #reset-cells = <1>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
new file mode 100644
index 000000000000..0e5ee8decc0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Delta Networks TN48M CPLD reset controller
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+description: |
+  This module is part of the Delta TN48M multi-function device. For more
+  details see ../mfd/delta,tn48m-cpld.yaml.
+
+  Reset controller modules provides resets for the following:
+  * 88F7040 SoC
+  * 88F6820 SoC
+  * 98DX3265 switch MAC-s
+  * 88E1680 PHY-s
+  * 88E1512 PHY
+  * PoE PSE controller
+
+properties:
+  compatible:
+    const: delta,tn48m-reset
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - "#reset-cells"
+
+additionalProperties: false
-- 
2.31.1

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Mon, Jun 7, 2021 at 2:33 PM Robert Marko <robert.marko@sartura.hr> wrote:
>
> Add binding documents for the Delta TN48M CPLD drivers.
>
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
> Changes in v3:
> * Include bindings for reset driver
>
> Changes in v2:
> * Implement MFD as a simple I2C MFD
> * Add GPIO bindings as separate
>
>  .../bindings/gpio/delta,tn48m-gpio.yaml       | 42 +++++++++
>  .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
>  .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
>  3 files changed, 167 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
>  create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
>  create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> new file mode 100644
> index 000000000000..aca646aecb12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Delta Networks TN48M CPLD GPIO controller
> +
> +maintainers:
> +  - Robert Marko <robert.marko@sartura.hr>
> +
> +description: |
> +  This module is part of the Delta TN48M multi-function device. For more
> +  details see ../mfd/delta,tn48m-cpld.yaml.
> +
> +  GPIO controller module provides GPIO-s for the SFP slots.
> +  It is split into 3 controllers, one output only for the SFP TX disable
> +  pins, one input only for the SFP present pins and one input only for
> +  the SFP LOS pins.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - delta,tn48m-gpio-sfp-tx-disable
> +      - delta,tn48m-gpio-sfp-present
> +      - delta,tn48m-gpio-sfp-los
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#gpio-cells"
> +  - gpio-controller
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> new file mode 100644
> index 000000000000..2c6e2adf73ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Delta Networks TN48M CPLD controller
> +
> +maintainers:
> +  - Robert Marko <robert.marko@sartura.hr>
> +
> +description: |
> +  Lattice CPLD onboard the TN48M switches is used for system
> +  management.
> +
> +  It provides information about the hardware model, revision,
> +  PSU status etc.
> +
> +  It is also being used as a GPIO expander for the SFP slots and
> +  reset controller for the switch MAC-s and other peripherals.
> +
> +properties:
> +  compatible:
> +    const: delta,tn48m-cpld
> +
> +  reg:
> +    description:
> +      I2C device address.
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +patternProperties:
> +  "^gpio(@[0-9a-f]+)?$":
> +    $ref: ../gpio/delta,tn48m-gpio.yaml
> +
> +  "^reset-controller?$":
> +    $ref: ../reset/delta,tn48m-reset.yaml
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        cpld@41 {
> +            compatible = "delta,tn48m-cpld";
> +            reg = <0x41>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            gpio@31 {
> +                compatible = "delta,tn48m-gpio-sfp-tx-disable";
> +                reg = <0x31>;
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            gpio@3a {
> +                compatible = "delta,tn48m-gpio-sfp-present";
> +                reg = <0x3a>;
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            gpio@40 {
> +                compatible = "delta,tn48m-gpio-sfp-los";
> +                reg = <0x40>;
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            reset-controller {
> +              compatible = "delta,tn48m-reset";
> +              #reset-cells = <1>;
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> new file mode 100644
> index 000000000000..0e5ee8decc0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Delta Networks TN48M CPLD reset controller
> +
> +maintainers:
> +  - Robert Marko <robert.marko@sartura.hr>
> +
> +description: |
> +  This module is part of the Delta TN48M multi-function device. For more
> +  details see ../mfd/delta,tn48m-cpld.yaml.
> +
> +  Reset controller modules provides resets for the following:
> +  * 88F7040 SoC
> +  * 88F6820 SoC
> +  * 98DX3265 switch MAC-s
> +  * 88E1680 PHY-s
> +  * 88E1512 PHY
> +  * PoE PSE controller
> +
> +properties:
> +  compatible:
> +    const: delta,tn48m-reset
> +
> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - "#reset-cells"
> +
> +additionalProperties: false
> --
> 2.31.1
>

Are there any issues with the bindings?
The patch series is depending on this as the rest has been reviewed.

Regards,
Robert
-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Rob Herring]

On Fri, Jun 25, 2021 at 01:46:08PM +0200, Robert Marko wrote:
> On Mon, Jun 7, 2021 at 2:33 PM Robert Marko <robert.marko@sartura.hr> wrote:
> >
> > Add binding documents for the Delta TN48M CPLD drivers.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > Changes in v3:
> > * Include bindings for reset driver
> >
> > Changes in v2:
> > * Implement MFD as a simple I2C MFD
> > * Add GPIO bindings as separate
> >
> >  .../bindings/gpio/delta,tn48m-gpio.yaml       | 42 +++++++++
> >  .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
> >  .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
> >  3 files changed, 167 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> >  create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> >  create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > new file mode 100644
> > index 000000000000..aca646aecb12
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > @@ -0,0 +1,42 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Delta Networks TN48M CPLD GPIO controller
> > +
> > +maintainers:
> > +  - Robert Marko <robert.marko@sartura.hr>
> > +
> > +description: |
> > +  This module is part of the Delta TN48M multi-function device. For more
> > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > +
> > +  GPIO controller module provides GPIO-s for the SFP slots.
> > +  It is split into 3 controllers, one output only for the SFP TX disable
> > +  pins, one input only for the SFP present pins and one input only for
> > +  the SFP LOS pins.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - delta,tn48m-gpio-sfp-tx-disable
> > +      - delta,tn48m-gpio-sfp-present
> > +      - delta,tn48m-gpio-sfp-los
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio-controller: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#gpio-cells"
> > +  - gpio-controller
> > +
> > +additionalProperties: false
> > diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > new file mode 100644
> > index 000000000000..2c6e2adf73ca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > @@ -0,0 +1,90 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Delta Networks TN48M CPLD controller
> > +
> > +maintainers:
> > +  - Robert Marko <robert.marko@sartura.hr>
> > +
> > +description: |
> > +  Lattice CPLD onboard the TN48M switches is used for system
> > +  management.
> > +
> > +  It provides information about the hardware model, revision,
> > +  PSU status etc.
> > +
> > +  It is also being used as a GPIO expander for the SFP slots and
> > +  reset controller for the switch MAC-s and other peripherals.
> > +
> > +properties:
> > +  compatible:
> > +    const: delta,tn48m-cpld
> > +
> > +  reg:
> > +    description:
> > +      I2C device address.
> > +    maxItems: 1
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +
> > +patternProperties:
> > +  "^gpio(@[0-9a-f]+)?$":
> > +    $ref: ../gpio/delta,tn48m-gpio.yaml
> > +
> > +  "^reset-controller?$":
> > +    $ref: ../reset/delta,tn48m-reset.yaml
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        cpld@41 {
> > +            compatible = "delta,tn48m-cpld";
> > +            reg = <0x41>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            gpio@31 {
> > +                compatible = "delta,tn48m-gpio-sfp-tx-disable";
> > +                reg = <0x31>;
> > +                gpio-controller;
> > +                #gpio-cells = <2>;
> > +            };
> > +
> > +            gpio@3a {
> > +                compatible = "delta,tn48m-gpio-sfp-present";
> > +                reg = <0x3a>;
> > +                gpio-controller;
> > +                #gpio-cells = <2>;
> > +            };
> > +
> > +            gpio@40 {
> > +                compatible = "delta,tn48m-gpio-sfp-los";
> > +                reg = <0x40>;
> > +                gpio-controller;
> > +                #gpio-cells = <2>;
> > +            };
> > +
> > +            reset-controller {
> > +              compatible = "delta,tn48m-reset";
> > +              #reset-cells = <1>;
> > +            };
> > +        };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > new file mode 100644
> > index 000000000000..0e5ee8decc0d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > @@ -0,0 +1,35 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Delta Networks TN48M CPLD reset controller
> > +
> > +maintainers:
> > +  - Robert Marko <robert.marko@sartura.hr>
> > +
> > +description: |
> > +  This module is part of the Delta TN48M multi-function device. For more
> > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > +
> > +  Reset controller modules provides resets for the following:
> > +  * 88F7040 SoC
> > +  * 88F6820 SoC
> > +  * 98DX3265 switch MAC-s
> > +  * 88E1680 PHY-s
> > +  * 88E1512 PHY
> > +  * PoE PSE controller
> > +
> > +properties:
> > +  compatible:
> > +    const: delta,tn48m-reset
> > +
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > --
> > 2.31.1
> >
> 
> Are there any issues with the bindings?

Yes. Primarily the GPIO function being part of the compatible. I'm 
surprised Linus W is okay with that.

> The patch series is depending on this as the rest has been reviewed.

The bindings have been reviewed too, you just didn't like my comments...

Rob

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Wed, Jul 14, 2021 at 12:25 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Jun 25, 2021 at 01:46:08PM +0200, Robert Marko wrote:
> > On Mon, Jun 7, 2021 at 2:33 PM Robert Marko <robert.marko@sartura.hr> wrote:
> > >
> > > Add binding documents for the Delta TN48M CPLD drivers.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > ---
> > > Changes in v3:
> > > * Include bindings for reset driver
> > >
> > > Changes in v2:
> > > * Implement MFD as a simple I2C MFD
> > > * Add GPIO bindings as separate
> > >
> > >  .../bindings/gpio/delta,tn48m-gpio.yaml       | 42 +++++++++
> > >  .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
> > >  .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
> > >  3 files changed, 167 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > >  create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > >  create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > new file mode 100644
> > > index 000000000000..aca646aecb12
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > @@ -0,0 +1,42 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Delta Networks TN48M CPLD GPIO controller
> > > +
> > > +maintainers:
> > > +  - Robert Marko <robert.marko@sartura.hr>
> > > +
> > > +description: |
> > > +  This module is part of the Delta TN48M multi-function device. For more
> > > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > > +
> > > +  GPIO controller module provides GPIO-s for the SFP slots.
> > > +  It is split into 3 controllers, one output only for the SFP TX disable
> > > +  pins, one input only for the SFP present pins and one input only for
> > > +  the SFP LOS pins.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - delta,tn48m-gpio-sfp-tx-disable
> > > +      - delta,tn48m-gpio-sfp-present
> > > +      - delta,tn48m-gpio-sfp-los
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  "#gpio-cells":
> > > +    const: 2
> > > +
> > > +  gpio-controller: true
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - "#gpio-cells"
> > > +  - gpio-controller
> > > +
> > > +additionalProperties: false
> > > diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > new file mode 100644
> > > index 000000000000..2c6e2adf73ca
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > @@ -0,0 +1,90 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Delta Networks TN48M CPLD controller
> > > +
> > > +maintainers:
> > > +  - Robert Marko <robert.marko@sartura.hr>
> > > +
> > > +description: |
> > > +  Lattice CPLD onboard the TN48M switches is used for system
> > > +  management.
> > > +
> > > +  It provides information about the hardware model, revision,
> > > +  PSU status etc.
> > > +
> > > +  It is also being used as a GPIO expander for the SFP slots and
> > > +  reset controller for the switch MAC-s and other peripherals.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: delta,tn48m-cpld
> > > +
> > > +  reg:
> > > +    description:
> > > +      I2C device address.
> > > +    maxItems: 1
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> > > +
> > > +patternProperties:
> > > +  "^gpio(@[0-9a-f]+)?$":
> > > +    $ref: ../gpio/delta,tn48m-gpio.yaml
> > > +
> > > +  "^reset-controller?$":
> > > +    $ref: ../reset/delta,tn48m-reset.yaml
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        cpld@41 {
> > > +            compatible = "delta,tn48m-cpld";
> > > +            reg = <0x41>;
> > > +            #address-cells = <1>;
> > > +            #size-cells = <0>;
> > > +
> > > +            gpio@31 {
> > > +                compatible = "delta,tn48m-gpio-sfp-tx-disable";
> > > +                reg = <0x31>;
> > > +                gpio-controller;
> > > +                #gpio-cells = <2>;
> > > +            };
> > > +
> > > +            gpio@3a {
> > > +                compatible = "delta,tn48m-gpio-sfp-present";
> > > +                reg = <0x3a>;
> > > +                gpio-controller;
> > > +                #gpio-cells = <2>;
> > > +            };
> > > +
> > > +            gpio@40 {
> > > +                compatible = "delta,tn48m-gpio-sfp-los";
> > > +                reg = <0x40>;
> > > +                gpio-controller;
> > > +                #gpio-cells = <2>;
> > > +            };
> > > +
> > > +            reset-controller {
> > > +              compatible = "delta,tn48m-reset";
> > > +              #reset-cells = <1>;
> > > +            };
> > > +        };
> > > +    };
> > > diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > new file mode 100644
> > > index 000000000000..0e5ee8decc0d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > @@ -0,0 +1,35 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Delta Networks TN48M CPLD reset controller
> > > +
> > > +maintainers:
> > > +  - Robert Marko <robert.marko@sartura.hr>
> > > +
> > > +description: |
> > > +  This module is part of the Delta TN48M multi-function device. For more
> > > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > > +
> > > +  Reset controller modules provides resets for the following:
> > > +  * 88F7040 SoC
> > > +  * 88F6820 SoC
> > > +  * 98DX3265 switch MAC-s
> > > +  * 88E1680 PHY-s
> > > +  * 88E1512 PHY
> > > +  * PoE PSE controller
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: delta,tn48m-reset
> > > +
> > > +  "#reset-cells":
> > > +    const: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - "#reset-cells"
> > > +
> > > +additionalProperties: false
> > > --
> > > 2.31.1
> > >
> >
> > Are there any issues with the bindings?
>
> Yes. Primarily the GPIO function being part of the compatible. I'm
> surprised Linus W is okay with that.

I think I already explained this before, having a single compatible
won't work here.
Then there would not be anything to know whether its input or output
only as the pins
have specific purpose.
And knowing the capabilites is a requirment of the GPIO regmap driver
and the GPIO
core itself as it exposes that information in a generic manner and
driver like for the
SFP bus use that.

Maybe Linus W can chime in here as well.

>
> > The patch series is depending on this as the rest has been reviewed.
>
> The bindings have been reviewed too, you just didn't like my comments...

Sorry, I did not pick that up as after my replies there was no further
discussion.

I am really hoping that we can find a middle ground here and get this
merged as the driver code itself has been revied and ACK-ed.

Regards,
Robert
>
> Rob



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Lee Jones]

On Sun, 18 Jul 2021, Robert Marko wrote:

> On Wed, Jul 14, 2021 at 12:25 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Jun 25, 2021 at 01:46:08PM +0200, Robert Marko wrote:
> > > On Mon, Jun 7, 2021 at 2:33 PM Robert Marko <robert.marko@sartura.hr> wrote:
> > > >
> > > > Add binding documents for the Delta TN48M CPLD drivers.
> > > >
> > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > > ---
> > > > Changes in v3:
> > > > * Include bindings for reset driver
> > > >
> > > > Changes in v2:
> > > > * Implement MFD as a simple I2C MFD
> > > > * Add GPIO bindings as separate
> > > >
> > > >  .../bindings/gpio/delta,tn48m-gpio.yaml       | 42 +++++++++
> > > >  .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
> > > >  .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
> > > >  3 files changed, 167 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > > new file mode 100644
> > > > index 000000000000..aca646aecb12
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > > @@ -0,0 +1,42 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Delta Networks TN48M CPLD GPIO controller
> > > > +
> > > > +maintainers:
> > > > +  - Robert Marko <robert.marko@sartura.hr>
> > > > +
> > > > +description: |
> > > > +  This module is part of the Delta TN48M multi-function device. For more
> > > > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > > > +
> > > > +  GPIO controller module provides GPIO-s for the SFP slots.
> > > > +  It is split into 3 controllers, one output only for the SFP TX disable
> > > > +  pins, one input only for the SFP present pins and one input only for
> > > > +  the SFP LOS pins.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - delta,tn48m-gpio-sfp-tx-disable
> > > > +      - delta,tn48m-gpio-sfp-present
> > > > +      - delta,tn48m-gpio-sfp-los
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  "#gpio-cells":
> > > > +    const: 2
> > > > +
> > > > +  gpio-controller: true
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - "#gpio-cells"
> > > > +  - gpio-controller
> > > > +
> > > > +additionalProperties: false
> > > > diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > > new file mode 100644
> > > > index 000000000000..2c6e2adf73ca
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > > @@ -0,0 +1,90 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Delta Networks TN48M CPLD controller
> > > > +
> > > > +maintainers:
> > > > +  - Robert Marko <robert.marko@sartura.hr>
> > > > +
> > > > +description: |
> > > > +  Lattice CPLD onboard the TN48M switches is used for system
> > > > +  management.
> > > > +
> > > > +  It provides information about the hardware model, revision,
> > > > +  PSU status etc.
> > > > +
> > > > +  It is also being used as a GPIO expander for the SFP slots and
> > > > +  reset controller for the switch MAC-s and other peripherals.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: delta,tn48m-cpld
> > > > +
> > > > +  reg:
> > > > +    description:
> > > > +      I2C device address.
> > > > +    maxItems: 1
> > > > +
> > > > +  "#address-cells":
> > > > +    const: 1
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 0
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - "#address-cells"
> > > > +  - "#size-cells"
> > > > +
> > > > +patternProperties:
> > > > +  "^gpio(@[0-9a-f]+)?$":
> > > > +    $ref: ../gpio/delta,tn48m-gpio.yaml
> > > > +
> > > > +  "^reset-controller?$":
> > > > +    $ref: ../reset/delta,tn48m-reset.yaml
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    i2c {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        cpld@41 {
> > > > +            compatible = "delta,tn48m-cpld";
> > > > +            reg = <0x41>;
> > > > +            #address-cells = <1>;
> > > > +            #size-cells = <0>;
> > > > +
> > > > +            gpio@31 {
> > > > +                compatible = "delta,tn48m-gpio-sfp-tx-disable";
> > > > +                reg = <0x31>;
> > > > +                gpio-controller;
> > > > +                #gpio-cells = <2>;
> > > > +            };
> > > > +
> > > > +            gpio@3a {
> > > > +                compatible = "delta,tn48m-gpio-sfp-present";
> > > > +                reg = <0x3a>;
> > > > +                gpio-controller;
> > > > +                #gpio-cells = <2>;
> > > > +            };
> > > > +
> > > > +            gpio@40 {
> > > > +                compatible = "delta,tn48m-gpio-sfp-los";
> > > > +                reg = <0x40>;
> > > > +                gpio-controller;
> > > > +                #gpio-cells = <2>;
> > > > +            };
> > > > +
> > > > +            reset-controller {
> > > > +              compatible = "delta,tn48m-reset";
> > > > +              #reset-cells = <1>;
> > > > +            };
> > > > +        };
> > > > +    };
> > > > diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > > new file mode 100644
> > > > index 000000000000..0e5ee8decc0d
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > > @@ -0,0 +1,35 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Delta Networks TN48M CPLD reset controller
> > > > +
> > > > +maintainers:
> > > > +  - Robert Marko <robert.marko@sartura.hr>
> > > > +
> > > > +description: |
> > > > +  This module is part of the Delta TN48M multi-function device. For more
> > > > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > > > +
> > > > +  Reset controller modules provides resets for the following:
> > > > +  * 88F7040 SoC
> > > > +  * 88F6820 SoC
> > > > +  * 98DX3265 switch MAC-s
> > > > +  * 88E1680 PHY-s
> > > > +  * 88E1512 PHY
> > > > +  * PoE PSE controller
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: delta,tn48m-reset
> > > > +
> > > > +  "#reset-cells":
> > > > +    const: 1
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - "#reset-cells"
> > > > +
> > > > +additionalProperties: false
> > > >
> > >
> > > Are there any issues with the bindings?
> >
> > Yes. Primarily the GPIO function being part of the compatible. I'm
> > surprised Linus W is okay with that.
> 
> I think I already explained this before, having a single compatible
> won't work here.
> Then there would not be anything to know whether its input or output
> only as the pins have specific purpose.

Properties?

> And knowing the capabilites is a requirment of the GPIO regmap driver
> and the GPIO
> core itself as it exposes that information in a generic manner and
> driver like for the
> SFP bus use that.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Rob Herring]

On Sun, Jul 18, 2021 at 11:15:38AM +0200, Robert Marko wrote:
> On Wed, Jul 14, 2021 at 12:25 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Jun 25, 2021 at 01:46:08PM +0200, Robert Marko wrote:
> > > On Mon, Jun 7, 2021 at 2:33 PM Robert Marko <robert.marko@sartura.hr> wrote:
> > > >
> > > > Add binding documents for the Delta TN48M CPLD drivers.
> > > >
> > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > > ---
> > > > Changes in v3:
> > > > * Include bindings for reset driver
> > > >
> > > > Changes in v2:
> > > > * Implement MFD as a simple I2C MFD
> > > > * Add GPIO bindings as separate
> > > >
> > > >  .../bindings/gpio/delta,tn48m-gpio.yaml       | 42 +++++++++
> > > >  .../bindings/mfd/delta,tn48m-cpld.yaml        | 90 +++++++++++++++++++
> > > >  .../bindings/reset/delta,tn48m-reset.yaml     | 35 ++++++++
> > > >  3 files changed, 167 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > >  create mode 100644 Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > > new file mode 100644
> > > > index 000000000000..aca646aecb12
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
> > > > @@ -0,0 +1,42 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/gpio/delta,tn48m-gpio.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Delta Networks TN48M CPLD GPIO controller
> > > > +
> > > > +maintainers:
> > > > +  - Robert Marko <robert.marko@sartura.hr>
> > > > +
> > > > +description: |
> > > > +  This module is part of the Delta TN48M multi-function device. For more
> > > > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > > > +
> > > > +  GPIO controller module provides GPIO-s for the SFP slots.
> > > > +  It is split into 3 controllers, one output only for the SFP TX disable
> > > > +  pins, one input only for the SFP present pins and one input only for
> > > > +  the SFP LOS pins.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - delta,tn48m-gpio-sfp-tx-disable
> > > > +      - delta,tn48m-gpio-sfp-present
> > > > +      - delta,tn48m-gpio-sfp-los
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  "#gpio-cells":
> > > > +    const: 2
> > > > +
> > > > +  gpio-controller: true
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - "#gpio-cells"
> > > > +  - gpio-controller
> > > > +
> > > > +additionalProperties: false
> > > > diff --git a/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > > new file mode 100644
> > > > index 000000000000..2c6e2adf73ca
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
> > > > @@ -0,0 +1,90 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mfd/delta,tn48m-cpld.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Delta Networks TN48M CPLD controller
> > > > +
> > > > +maintainers:
> > > > +  - Robert Marko <robert.marko@sartura.hr>
> > > > +
> > > > +description: |
> > > > +  Lattice CPLD onboard the TN48M switches is used for system
> > > > +  management.
> > > > +
> > > > +  It provides information about the hardware model, revision,
> > > > +  PSU status etc.
> > > > +
> > > > +  It is also being used as a GPIO expander for the SFP slots and
> > > > +  reset controller for the switch MAC-s and other peripherals.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: delta,tn48m-cpld
> > > > +
> > > > +  reg:
> > > > +    description:
> > > > +      I2C device address.
> > > > +    maxItems: 1
> > > > +
> > > > +  "#address-cells":
> > > > +    const: 1
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 0
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - "#address-cells"
> > > > +  - "#size-cells"
> > > > +
> > > > +patternProperties:
> > > > +  "^gpio(@[0-9a-f]+)?$":
> > > > +    $ref: ../gpio/delta,tn48m-gpio.yaml
> > > > +
> > > > +  "^reset-controller?$":
> > > > +    $ref: ../reset/delta,tn48m-reset.yaml
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    i2c {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        cpld@41 {
> > > > +            compatible = "delta,tn48m-cpld";
> > > > +            reg = <0x41>;
> > > > +            #address-cells = <1>;
> > > > +            #size-cells = <0>;
> > > > +
> > > > +            gpio@31 {
> > > > +                compatible = "delta,tn48m-gpio-sfp-tx-disable";
> > > > +                reg = <0x31>;
> > > > +                gpio-controller;
> > > > +                #gpio-cells = <2>;
> > > > +            };
> > > > +
> > > > +            gpio@3a {
> > > > +                compatible = "delta,tn48m-gpio-sfp-present";
> > > > +                reg = <0x3a>;
> > > > +                gpio-controller;
> > > > +                #gpio-cells = <2>;
> > > > +            };
> > > > +
> > > > +            gpio@40 {
> > > > +                compatible = "delta,tn48m-gpio-sfp-los";
> > > > +                reg = <0x40>;
> > > > +                gpio-controller;
> > > > +                #gpio-cells = <2>;
> > > > +            };
> > > > +
> > > > +            reset-controller {
> > > > +              compatible = "delta,tn48m-reset";
> > > > +              #reset-cells = <1>;
> > > > +            };
> > > > +        };
> > > > +    };
> > > > diff --git a/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > > new file mode 100644
> > > > index 000000000000..0e5ee8decc0d
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
> > > > @@ -0,0 +1,35 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/reset/delta,tn48m-reset.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Delta Networks TN48M CPLD reset controller
> > > > +
> > > > +maintainers:
> > > > +  - Robert Marko <robert.marko@sartura.hr>
> > > > +
> > > > +description: |
> > > > +  This module is part of the Delta TN48M multi-function device. For more
> > > > +  details see ../mfd/delta,tn48m-cpld.yaml.
> > > > +
> > > > +  Reset controller modules provides resets for the following:
> > > > +  * 88F7040 SoC
> > > > +  * 88F6820 SoC
> > > > +  * 98DX3265 switch MAC-s
> > > > +  * 88E1680 PHY-s
> > > > +  * 88E1512 PHY
> > > > +  * PoE PSE controller
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: delta,tn48m-reset
> > > > +
> > > > +  "#reset-cells":
> > > > +    const: 1
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - "#reset-cells"
> > > > +
> > > > +additionalProperties: false
> > > > --
> > > > 2.31.1
> > > >
> > >
> > > Are there any issues with the bindings?
> >
> > Yes. Primarily the GPIO function being part of the compatible. I'm
> > surprised Linus W is okay with that.
> 
> I think I already explained this before, having a single compatible
> won't work here.
> Then there would not be anything to know whether its input or output
> only as the pins
> have specific purpose.

The client side should tell the direction. Are you using the SFP 
binding?: Documentation/devicetree/bindings/net/sff,sfp.txt

Specific purpose IOs are not general purpose IOs. Repeating Linus W 
here. Maybe his opinion has evolved...

If the programming model of each instance is different, then different 
compatibles are justified. But describe what the difference is, not the 
connection.

> And knowing the capabilites is a requirment of the GPIO regmap driver
> and the GPIO
> core itself as it exposes that information in a generic manner and
> driver like for the
> SFP bus use that.

Driver details aren't justification for bindings.

> Maybe Linus W can chime in here as well.
> 
> >
> > > The patch series is depending on this as the rest has been reviewed.
> >
> > The bindings have been reviewed too, you just didn't like my comments...
> 
> Sorry, I did not pick that up as after my replies there was no further
> discussion.
> 
> I am really hoping that we can find a middle ground here and get this
> merged as the driver code itself has been revied and ACK-ed.
> 
> Regards,
> Robert
> >
> > Rob
> 
> 
> 
> -- 
> Robert Marko
> Staff Embedded Linux Engineer
> Sartura Ltd.
> Lendavska ulica 16a
> 10000 Zagreb, Croatia
> Email: robert.marko@sartura.hr
> Web: www.sartura.hr
> 

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Linus Walleij]

On Tue, Jul 20, 2021 at 12:59 AM Rob Herring <robh@kernel.org> wrote:

> > > > Are there any issues with the bindings?
> > >
> > > Yes. Primarily the GPIO function being part of the compatible. I'm
> > > surprised Linus W is okay with that.
> >
> > I think I already explained this before, having a single compatible
> > won't work here.
> > Then there would not be anything to know whether its input or output
> > only as the pins
> > have specific purpose.
>
> The client side should tell the direction. Are you using the SFP
> binding?: Documentation/devicetree/bindings/net/sff,sfp.txt
>
> Specific purpose IOs are not general purpose IOs. Repeating Linus W
> here. Maybe his opinion has evolved...

Nah. I think at one time or two I was convinced to let something
special purpose slip through as "GPIO".

Typical case: LED control lines that were in practice used for other
things, such as controlling motors.

Here there is a pin named "SFP TX disable" which is suspicious.
Why isn't whatever is now managing SFP just read/write this bit
without going through the GPIO abstraction to disable TX?

If it is a regmap in Linux then that is fine, just pass the regmap
around inside the kernel, OK finished. But really that is an OS
detail.

If the pin is in practice used for other things, say connected
to a LED, I would soften up and accept it as a GPIO compatible.

> If the programming model of each instance is different, then different
> compatibles are justified. But describe what the difference is, not the
> connection.

IIRC that is the case as the instances are different.

So those differences should be described for each compatible in the
bindings.

So there is this:

> +  GPIO controller module provides GPIO-s for the SFP slots.
> +  It is split into 3 controllers, one output only for the SFP TX disable
> +  pins, one input only for the SFP present pins and one input only for
> +  the SFP LOS pins.

This should read "the hardware instances are different in such way
that the first can only (by hardware restrictions) be used as output..."
etc, so that it is crystal clear what this means.

But if the lines are special purpose not general purpose, they
should not be GPIOs to begin with.

Yours,
Linus Walleij

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Wed, Jul 21, 2021 at 4:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Jul 20, 2021 at 12:59 AM Rob Herring <robh@kernel.org> wrote:
>
> > > > > Are there any issues with the bindings?
> > > >
> > > > Yes. Primarily the GPIO function being part of the compatible. I'm
> > > > surprised Linus W is okay with that.
> > >
> > > I think I already explained this before, having a single compatible
> > > won't work here.
> > > Then there would not be anything to know whether its input or output
> > > only as the pins
> > > have specific purpose.
> >
> > The client side should tell the direction. Are you using the SFP
> > binding?: Documentation/devicetree/bindings/net/sff,sfp.txt
> >
> > Specific purpose IOs are not general purpose IOs. Repeating Linus W
> > here. Maybe his opinion has evolved...
>
> Nah. I think at one time or two I was convinced to let something
> special purpose slip through as "GPIO".
>
> Typical case: LED control lines that were in practice used for other
> things, such as controlling motors.
>
> Here there is a pin named "SFP TX disable" which is suspicious.
> Why isn't whatever is now managing SFP just read/write this bit
> without going through the GPIO abstraction to disable TX?

Hi Linus,
The pins that this driver wants to expose are used for SFP-s only,
they are provided by the Lattice CPLD which also does other things.

Linux has a generic SFP driver which is used to manage these SFP
ports, but it only supports GPIO-s, it has no concept of anything else.
Since the driver is fully generic, I have no idea how could one extend it
to effectively handle these pins internally, especially since I have more
switches that use the CPLD for SFP-s as well, even for 48 ports and 192
pins for them.

GPIO regmap works perfectly for this as its generic enough to cover all of
these cases.
CPLD also provides pins to test the port LED-s per color as well,
but I have chosen not to expose them so far.

>
> If it is a regmap in Linux then that is fine, just pass the regmap
> around inside the kernel, OK finished. But really that is an OS
> detail.

Yes, its regmap but I cant really pass it to the SFP driver as I don't have
special driver handling the SFP but rather the generic kernel one.
It only knows how to handle GPIO-s.

>
> If the pin is in practice used for other things, say connected
> to a LED, I would soften up and accept it as a GPIO compatible.
>
> > If the programming model of each instance is different, then different
> > compatibles are justified. But describe what the difference is, not the
> > connection.
>
> IIRC that is the case as the instances are different.
>
> So those differences should be described for each compatible in the
> bindings.

Ok, makes sense, I will make it clear in the bindings.
>
> So there is this:
>
> > +  GPIO controller module provides GPIO-s for the SFP slots.
> > +  It is split into 3 controllers, one output only for the SFP TX disable
> > +  pins, one input only for the SFP present pins and one input only for
> > +  the SFP LOS pins.
>
> This should read "the hardware instances are different in such way
> that the first can only (by hardware restrictions) be used as output..."
> etc, so that it is crystal clear what this means.

Ok, makes sense, I will make it clear in the bindings.
>
> But if the lines are special purpose not general purpose, they
> should not be GPIOs to begin with.
>
> Yours,
> Linus Walleij

Looking forward to your reply.

Regards,
Robert.
-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Linus Walleij]

On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@sartura.hr> wrote:

> The pins that this driver wants to expose are used for SFP-s only,
> they are provided by the Lattice CPLD which also does other things.
>
> Linux has a generic SFP driver which is used to manage these SFP
> ports, but it only supports GPIO-s, it has no concept of anything else.
> Since the driver is fully generic, I have no idea how could one extend it
> to effectively handle these pins internally, especially since I have more
> switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> pins for them.

Which file is this driver in so I can look?

Maybe it is not a good idea to look for generic code just because
it is convenient? I have had this problem before with GPIO, along
the lines "lemme just do this dirty thing this one time because it
is so convenient for me" (more or less) and the answer is still
"no".

Can you either augment the driver to handle a regmap with bit indices
instead or write a new similar driver for that or refactor it some other
way?

It is not a simple solution to your problem, but it might be the right
solution even if it means some more work.

> GPIO regmap works perfectly for this as its generic enough to cover all of
> these cases.

Yeah but it might be the wrong thing to do even if it is simple
to use and works.

> CPLD also provides pins to test the port LED-s per color as well,
> but I have chosen not to expose them so far.

Have you considered
Documentation/devicetree/bindings/leds/register-bit-led.txt

> > If it is a regmap in Linux then that is fine, just pass the regmap
> > around inside the kernel, OK finished. But really that is an OS
> > detail.
>
> Yes, its regmap but I cant really pass it to the SFP driver as I don't have
> special driver handling the SFP but rather the generic kernel one.
> It only knows how to handle GPIO-s.

Of course you have to program it. If I know which driver it
is it is easier to provide architecture ideas.

Yours,
Linus Walleij

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Wed, Aug 11, 2021 at 2:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@sartura.hr> wrote:
>
> > The pins that this driver wants to expose are used for SFP-s only,
> > they are provided by the Lattice CPLD which also does other things.
> >
> > Linux has a generic SFP driver which is used to manage these SFP
> > ports, but it only supports GPIO-s, it has no concept of anything else.
> > Since the driver is fully generic, I have no idea how could one extend it
> > to effectively handle these pins internally, especially since I have more
> > switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> > pins for them.
>
> Which file is this driver in so I can look?

Hi Linus,
Sorry for the late reply.

Sure, here is the generic Linux driver that is used for SFP handling:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/sfp.c?h=v5.14-rc7

>
> Maybe it is not a good idea to look for generic code just because
> it is convenient? I have had this problem before with GPIO, along
> the lines "lemme just do this dirty thing this one time because it
> is so convenient for me" (more or less) and the answer is still
> "no".
>
> Can you either augment the driver to handle a regmap with bit indices
> instead or write a new similar driver for that or refactor it some other
> way?
>
> It is not a simple solution to your problem, but it might be the right
> solution even if it means some more work.

I understand your position, believe me, I spend some time looking at
what would be the logical way for these switches.
But I see no way how could the SFP driver be extended in a generic way
that would allow supporting different register layouts when it comes to pins.

>
> > GPIO regmap works perfectly for this as its generic enough to cover all of
> > these cases.
>
> Yeah but it might be the wrong thing to do even if it is simple
> to use and works.
>
> > CPLD also provides pins to test the port LED-s per color as well,
> > but I have chosen not to expose them so far.
>
> Have you considered
> Documentation/devicetree/bindings/leds/register-bit-led.txt

Yeah, but unfortunately in this case it wont work as the LED-s
are for debugging/test purposes only and you first need to switch
the CPLD out of it interpreting the LED state with a BIT flip.

Regards,
Robert
>
> > > If it is a regmap in Linux then that is fine, just pass the regmap
> > > around inside the kernel, OK finished. But really that is an OS
> > > detail.
> >
> > Yes, its regmap but I cant really pass it to the SFP driver as I don't have
> > special driver handling the SFP but rather the generic kernel one.
> > It only knows how to handle GPIO-s.
>
> Of course you have to program it. If I know which driver it
> is it is easier to provide architecture ideas.
>
> Yours,
> Linus Walleij



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Tue, Aug 24, 2021 at 10:03 AM Robert Marko <robert.marko@sartura.hr> wrote:
>
> On Wed, Aug 11, 2021 at 2:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@sartura.hr> wrote:
> >
> > > The pins that this driver wants to expose are used for SFP-s only,
> > > they are provided by the Lattice CPLD which also does other things.
> > >
> > > Linux has a generic SFP driver which is used to manage these SFP
> > > ports, but it only supports GPIO-s, it has no concept of anything else.
> > > Since the driver is fully generic, I have no idea how could one extend it
> > > to effectively handle these pins internally, especially since I have more
> > > switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> > > pins for them.
> >
> > Which file is this driver in so I can look?
>
> Hi Linus,
> Sorry for the late reply.
>
> Sure, here is the generic Linux driver that is used for SFP handling:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/sfp.c?h=v5.14-rc7
>
> >
> > Maybe it is not a good idea to look for generic code just because
> > it is convenient? I have had this problem before with GPIO, along
> > the lines "lemme just do this dirty thing this one time because it
> > is so convenient for me" (more or less) and the answer is still
> > "no".
> >
> > Can you either augment the driver to handle a regmap with bit indices
> > instead or write a new similar driver for that or refactor it some other
> > way?
> >
> > It is not a simple solution to your problem, but it might be the right
> > solution even if it means some more work.
>
> I understand your position, believe me, I spend some time looking at
> what would be the logical way for these switches.
> But I see no way how could the SFP driver be extended in a generic way
> that would allow supporting different register layouts when it comes to pins.
>
> >
> > > GPIO regmap works perfectly for this as its generic enough to cover all of
> > > these cases.
> >
> > Yeah but it might be the wrong thing to do even if it is simple
> > to use and works.
> >
> > > CPLD also provides pins to test the port LED-s per color as well,
> > > but I have chosen not to expose them so far.
> >
> > Have you considered
> > Documentation/devicetree/bindings/leds/register-bit-led.txt
>
> Yeah, but unfortunately in this case it wont work as the LED-s
> are for debugging/test purposes only and you first need to switch
> the CPLD out of it interpreting the LED state with a BIT flip.
>
> Regards,
> Robert
> >
> > > > If it is a regmap in Linux then that is fine, just pass the regmap
> > > > around inside the kernel, OK finished. But really that is an OS
> > > > detail.
> > >
> > > Yes, its regmap but I cant really pass it to the SFP driver as I don't have
> > > special driver handling the SFP but rather the generic kernel one.
> > > It only knows how to handle GPIO-s.
> >
> > Of course you have to program it. If I know which driver it
> > is it is easier to provide architecture ideas.
> >
> > Yours,
> > Linus Walleij

Linus,

can I offer some further explanation?

Regards,
Robert
>
>
>
> --
> Robert Marko
> Staff Embedded Linux Engineer
> Sartura Ltd.
> Lendavska ulica 16a
> 10000 Zagreb, Croatia
> Email: robert.marko@sartura.hr
> Web: www.sartura.hr



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Tue, Sep 7, 2021 at 11:02 PM Robert Marko <robert.marko@sartura.hr> wrote:
>
> On Tue, Aug 24, 2021 at 10:03 AM Robert Marko <robert.marko@sartura.hr> wrote:
> >
> > On Wed, Aug 11, 2021 at 2:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > > On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@sartura.hr> wrote:
> > >
> > > > The pins that this driver wants to expose are used for SFP-s only,
> > > > they are provided by the Lattice CPLD which also does other things.
> > > >
> > > > Linux has a generic SFP driver which is used to manage these SFP
> > > > ports, but it only supports GPIO-s, it has no concept of anything else.
> > > > Since the driver is fully generic, I have no idea how could one extend it
> > > > to effectively handle these pins internally, especially since I have more
> > > > switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> > > > pins for them.
> > >
> > > Which file is this driver in so I can look?
> >
> > Hi Linus,
> > Sorry for the late reply.
> >
> > Sure, here is the generic Linux driver that is used for SFP handling:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/sfp.c?h=v5.14-rc7
> >
> > >
> > > Maybe it is not a good idea to look for generic code just because
> > > it is convenient? I have had this problem before with GPIO, along
> > > the lines "lemme just do this dirty thing this one time because it
> > > is so convenient for me" (more or less) and the answer is still
> > > "no".
> > >
> > > Can you either augment the driver to handle a regmap with bit indices
> > > instead or write a new similar driver for that or refactor it some other
> > > way?
> > >
> > > It is not a simple solution to your problem, but it might be the right
> > > solution even if it means some more work.
> >
> > I understand your position, believe me, I spend some time looking at
> > what would be the logical way for these switches.
> > But I see no way how could the SFP driver be extended in a generic way
> > that would allow supporting different register layouts when it comes to pins.
> >
> > >
> > > > GPIO regmap works perfectly for this as its generic enough to cover all of
> > > > these cases.
> > >
> > > Yeah but it might be the wrong thing to do even if it is simple
> > > to use and works.
> > >
> > > > CPLD also provides pins to test the port LED-s per color as well,
> > > > but I have chosen not to expose them so far.
> > >
> > > Have you considered
> > > Documentation/devicetree/bindings/leds/register-bit-led.txt
> >
> > Yeah, but unfortunately in this case it wont work as the LED-s
> > are for debugging/test purposes only and you first need to switch
> > the CPLD out of it interpreting the LED state with a BIT flip.
> >
> > Regards,
> > Robert
> > >
> > > > > If it is a regmap in Linux then that is fine, just pass the regmap
> > > > > around inside the kernel, OK finished. But really that is an OS
> > > > > detail.
> > > >
> > > > Yes, its regmap but I cant really pass it to the SFP driver as I don't have
> > > > special driver handling the SFP but rather the generic kernel one.
> > > > It only knows how to handle GPIO-s.
> > >
> > > Of course you have to program it. If I know which driver it
> > > is it is easier to provide architecture ideas.
> > >
> > > Yours,
> > > Linus Walleij
>
> Linus,
>
> can I offer some further explanation?
>
> Regards,
> Robert

Hi Linus,

I would really like to move forward with this somehow.

I have multiple switches depending on the outcome of this series.

Regards,
Robert
> >
> >
> >
> > --
> > Robert Marko
> > Staff Embedded Linux Engineer
> > Sartura Ltd.
> > Lendavska ulica 16a
> > 10000 Zagreb, Croatia
> > Email: robert.marko@sartura.hr
> > Web: www.sartura.hr
>
>
>
> --
> Robert Marko
> Staff Embedded Linux Engineer
> Sartura Ltd.
> Lendavska ulica 16a
> 10000 Zagreb, Croatia
> Email: robert.marko@sartura.hr
> Web: www.sartura.hr



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Linus Walleij]

Hi Robert,

sorry for slow reply, I am a bit busy.

On Tue, Aug 24, 2021 at 10:03 AM Robert Marko <robert.marko@sartura.hr> wrote:
> On Wed, Aug 11, 2021 at 2:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@sartura.hr> wrote:
> >
> > > The pins that this driver wants to expose are used for SFP-s only,
> > > they are provided by the Lattice CPLD which also does other things.
> > >
> > > Linux has a generic SFP driver which is used to manage these SFP
> > > ports, but it only supports GPIO-s, it has no concept of anything else.
> > > Since the driver is fully generic, I have no idea how could one extend it
> > > to effectively handle these pins internally, especially since I have more
> > > switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> > > pins for them.
> >
> > Which file is this driver in so I can look?
>
> Hi Linus,
> Sorry for the late reply.
>
> Sure, here is the generic Linux driver that is used for SFP handling:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/sfp.c?h=v5.14-rc7

So this has this:

enum {
        GPIO_MODDEF0,
        GPIO_LOS,
        GPIO_TX_FAULT,
        GPIO_TX_DISABLE,
        GPIO_RATE_SELECT,
        GPIO_MAX,

        SFP_F_PRESENT = BIT(GPIO_MODDEF0),
        SFP_F_LOS = BIT(GPIO_LOS),
        SFP_F_TX_FAULT = BIT(GPIO_TX_FAULT),
        SFP_F_TX_DISABLE = BIT(GPIO_TX_DISABLE),
        SFP_F_RATE_SELECT = BIT(GPIO_RATE_SELECT),

        SFP_E_INSERT = 0,
        SFP_E_REMOVE,

This does not look general purpose to me at all?
It's just some hardware engineer that thougt "GPIO"
was a nice thing to call this.

> > Maybe it is not a good idea to look for generic code just because
> > it is convenient? I have had this problem before with GPIO, along
> > the lines "lemme just do this dirty thing this one time because it
> > is so convenient for me" (more or less) and the answer is still
> > "no".
> >
> > Can you either augment the driver to handle a regmap with bit indices
> > instead or write a new similar driver for that or refactor it some other
> > way?
> >
> > It is not a simple solution to your problem, but it might be the right
> > solution even if it means some more work.
>
> I understand your position, believe me, I spend some time looking at
> what would be the logical way for these switches.
> But I see no way how could the SFP driver be extended in a generic way
> that would allow supporting different register layouts when it comes to pins.

Why do you think you have to use the GPIO abstraction and bindings?
Just invent something that satisfy your needs, the bindings are just
strings. Why does the consumer have to use the GPIO binding?
They can just use phandle named anything. Some "sfp-foo-resource = <&...>
or so.

For example I created this:
Documentation/devicetree/bindings/firmware/intel,ixp4xx-network-processing-engine.yaml
It's handling out a resource using a phandle. Nothing different than
GPIO, regulator, clock etc. Just invent something for SFP?

Yours,
Linus Walleij

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Mon, Oct 4, 2021 at 12:48 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Robert,
>
> sorry for slow reply, I am a bit busy.
>
> On Tue, Aug 24, 2021 at 10:03 AM Robert Marko <robert.marko@sartura.hr> wrote:
> > On Wed, Aug 11, 2021 at 2:17 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > > On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@sartura.hr> wrote:
> > >
> > > > The pins that this driver wants to expose are used for SFP-s only,
> > > > they are provided by the Lattice CPLD which also does other things.
> > > >
> > > > Linux has a generic SFP driver which is used to manage these SFP
> > > > ports, but it only supports GPIO-s, it has no concept of anything else.
> > > > Since the driver is fully generic, I have no idea how could one extend it
> > > > to effectively handle these pins internally, especially since I have more
> > > > switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> > > > pins for them.
> > >
> > > Which file is this driver in so I can look?
> >
> > Hi Linus,
> > Sorry for the late reply.
> >
> > Sure, here is the generic Linux driver that is used for SFP handling:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/sfp.c?h=v5.14-rc7
>
> So this has this:
>
> enum {
>         GPIO_MODDEF0,
>         GPIO_LOS,
>         GPIO_TX_FAULT,
>         GPIO_TX_DISABLE,
>         GPIO_RATE_SELECT,
>         GPIO_MAX,
>
>         SFP_F_PRESENT = BIT(GPIO_MODDEF0),
>         SFP_F_LOS = BIT(GPIO_LOS),
>         SFP_F_TX_FAULT = BIT(GPIO_TX_FAULT),
>         SFP_F_TX_DISABLE = BIT(GPIO_TX_DISABLE),
>         SFP_F_RATE_SELECT = BIT(GPIO_RATE_SELECT),
>
>         SFP_E_INSERT = 0,
>         SFP_E_REMOVE,
>
> This does not look general purpose to me at all?
> It's just some hardware engineer that thougt "GPIO"
> was a nice thing to call this.

Hi Linus.
These were historically always just regular GPIO-s, usually connected
directly to the SoC
GPIO controller or some kind of a GPIO expander and thus it uses gpiod.

>
> > > Maybe it is not a good idea to look for generic code just because
> > > it is convenient? I have had this problem before with GPIO, along
> > > the lines "lemme just do this dirty thing this one time because it
> > > is so convenient for me" (more or less) and the answer is still
> > > "no".
> > >
> > > Can you either augment the driver to handle a regmap with bit indices
> > > instead or write a new similar driver for that or refactor it some other
> > > way?
> > >
> > > It is not a simple solution to your problem, but it might be the right
> > > solution even if it means some more work.
> >
> > I understand your position, believe me, I spend some time looking at
> > what would be the logical way for these switches.
> > But I see no way how could the SFP driver be extended in a generic way
> > that would allow supporting different register layouts when it comes to pins.
>
> Why do you think you have to use the GPIO abstraction and bindings?
> Just invent something that satisfy your needs, the bindings are just
> strings. Why does the consumer have to use the GPIO binding?
> They can just use phandle named anything. Some "sfp-foo-resource = <&...>
> or so.
>
> For example I created this:
> Documentation/devicetree/bindings/firmware/intel,ixp4xx-network-processing-engine.yaml
> It's handling out a resource using a phandle. Nothing different than
> GPIO, regulator, clock etc. Just invent something for SFP?

The SFP driver requires GPIO-s, it only knows how to use GPIO-s, and
its a generic driver,
it covers any device that has an SFP port that is implemented per spec.
So, I cannot just extend it to suit my devices needs and I don't see
how can I extend it in
a generic manner so that it controls the pins directly via a regmap
for example, especially since
each switch has a different number of SFP ports and thus pins and a
different register layout.

I have added Andrew Lunn as he is one of the maintainers of PHYLIB
under which the SFP driver
is, he may have some input on how to proceed with this.

I honestly think that we have some kind of misunderstanding here and
look forward to resolving it.

Regards,
Robert

>
> Yours,
> Linus Walleij



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Andrew Lunn]

> The SFP driver requires GPIO-s, it only knows how to use GPIO-s, and
> its a generic driver,
> it covers any device that has an SFP port that is implemented per spec.
> So, I cannot just extend it to suit my devices needs and I don't see
> how can I extend it in
> a generic manner so that it controls the pins directly via a regmap
> for example, especially since
> each switch has a different number of SFP ports and thus pins and a
> different register layout.
> 
> I have added Andrew Lunn as he is one of the maintainers of PHYLIB
> under which the SFP driver
> is, he may have some input on how to proceed with this.
> 
> I honestly think that we have some kind of misunderstanding here and
> look forward to resolving it.

Hi Robert

Can you describe your hardware and regmap in a bit more detail. What
do these registers look like? How do they map to the SFP cage pins?

	  Andrew

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Tue, Oct 19, 2021 at 3:40 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > The SFP driver requires GPIO-s, it only knows how to use GPIO-s, and
> > its a generic driver,
> > it covers any device that has an SFP port that is implemented per spec.
> > So, I cannot just extend it to suit my devices needs and I don't see
> > how can I extend it in
> > a generic manner so that it controls the pins directly via a regmap
> > for example, especially since
> > each switch has a different number of SFP ports and thus pins and a
> > different register layout.
> >
> > I have added Andrew Lunn as he is one of the maintainers of PHYLIB
> > under which the SFP driver
> > is, he may have some input on how to proceed with this.
> >
> > I honestly think that we have some kind of misunderstanding here and
> > look forward to resolving it.
>
> Hi Robert
>
> Can you describe your hardware and regmap in a bit more detail. What
> do these registers look like? How do they map to the SFP cage pins?

Hi Andrew,
This board is simple as it only has 4 SFP ports so they have split the
respective
pins into individual registers per their purpose.

So TX disable pins have their own 8bit register and they map pins
using individual bits.
This is how the register looks:
+-----+---------------+-----+-------------------+---------------+
| Bit |     Name      | R/W |    Description    | Default value |
+-----+---------------+-----+-------------------+---------------+
| 7:4 | Reserved         | R/W | Not used                |     0 |
| 3   | TX_Disable_52 | R/W | Enable/disable       |     0 |
| 2   | TX_Disable_51 | R/W | SFP TX transmiter |     0 |
| 1   | TX_Disable_50 | R/W | 1 = TX off               |     0 |
| 0   | TX_Disable_49 | R/W | 0 = TX on               |     0 |
+-----+---------------+-----+-------------------+---------------+

Presence and LOS pins also have their respective registers in
the same format.
So you can see that the register bits map directly to the SFP cage pins.

Regards,
Robert
>
>           Andrew



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Andrew Lunn]

> > > > +  GPIO controller module provides GPIO-s for the SFP slots.
> > > > +  It is split into 3 controllers, one output only for the SFP TX disable
> > > > +  pins, one input only for the SFP present pins and one input only for
> > > > +  the SFP LOS pins.

Late to the conversation, so i might be asking questions already
asked...

So the PLD has restrictions? You have a collection of GPOs and a
collection of GPIs? You don't have an GPIOs?

> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - delta,tn48m-gpio-sfp-tx-disable
> > > > +      - delta,tn48m-gpio-sfp-present
> > > > +      - delta,tn48m-gpio-sfp-los

Do these names have any real significant? Are you really forced to
connect the SFP cage in this dedicated manor? Is there any reason why
i cannot use a GPO to control an LED? A GPI for a button?

	Andrew

Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Robert Marko]

On Tue, Oct 19, 2021 at 4:05 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > > +  GPIO controller module provides GPIO-s for the SFP slots.
> > > > > +  It is split into 3 controllers, one output only for the SFP TX disable
> > > > > +  pins, one input only for the SFP present pins and one input only for
> > > > > +  the SFP LOS pins.
>
> Late to the conversation, so i might be asking questions already
> asked...
>
> So the PLD has restrictions? You have a collection of GPOs and a
> collection of GPIs? You don't have an GPIOs?

Yes, the CPLD is hardwired per board with the specific FW for it.
There are no true GPIO-s, only input or output pins depending on their
purpose in the SFP cage.
>
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - delta,tn48m-gpio-sfp-tx-disable
> > > > > +      - delta,tn48m-gpio-sfp-present
> > > > > +      - delta,tn48m-gpio-sfp-los
>
> Do these names have any real significant? Are you really forced to
> connect the SFP cage in this dedicated manor? Is there any reason why
> i cannot use a GPO to control an LED? A GPI for a button?

Yes, there are connected like this on the TN48M switch, names directly
match their
purpose. The CPLD is customized per each switch model, so TN4810M which is the
48 x 10G SFP+ uses the same CPLD model but is wired differently and
with a different FW.
Since it's a CPLD you technically use whatever pin to connect stuff,
but it completely depends
on the FW implementation as there is no traditional GPIO block with
XYZ number of pins.
I have multiple vendors and models using the same CPLD but its wired
completely different
and the register layout is different.

So you cant force any of the input pins to output mode or change their
value at all, its all hardwired.

Regards,
Robert
>
>         Andrew



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

[PATCH v6 6/6] MAINTAINERS: Add Delta Networks TN48M CPLD drivers

[Robert Marko]

Add maintainers entry for the Delta Networks TN48M
CPLD MFD drivers.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v3:
* Add reset driver documentation

Changes in v2:
* Drop no more existing files

 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9450e052f1b1..82d9c2943c34 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5096,6 +5096,15 @@ W:	https://linuxtv.org
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/platform/sti/delta
 
+DELTA NETWORKS TN48M CPLD DRIVERS
+M:	Robert Marko <robert.marko@sartura.hr>
+S:	Maintained
+F:	Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
+F:	Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
+F:	Documentation/devicetree/bindings/reset/delta,tn48m-reset.yaml
+F:	drivers/gpio/gpio-tn48m.c
+F:	include/dt-bindings/reset/delta,tn48m-reset.h
+
 DENALI NAND DRIVER
 L:	linux-mtd@lists.infradead.org
 S:	Orphan
-- 
2.31.1