2021-11-02 16:58:31

by Robert Marko

[permalink] [raw]
Subject: [PATCH v7 1/6] mfd: simple-mfd-i2c: Add Delta TN48M CPLD support

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 <[email protected]>
Acked-for-MFD-by: Lee Jones <[email protected]>
---
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 ad15be6b86bc..3701657e831d 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.33.1


2021-11-02 16:58:31

by Robert Marko

[permalink] [raw]
Subject: [PATCH v7 2/6] gpio: Add Delta TN48M CPLD GPIO driver

Delta TN48M switch has an onboard Lattice CPLD that is used as a GPIO
expander.

The CPLD provides 12 pins in total on the TN48M, but on more advanced
switch models it provides up to 192 pins, so the driver is extendable
to support more switches.

Signed-off-by: Robert Marko <[email protected]>
---
Changes in v7:
* Change compatibles, reduce their number
* Rework the driver to be easily extendible to support more devices
* Use match data to populate configuration
* Drop reviews and ACK-s as the driver changed

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 | 100 ++++++++++++++++++++++++++++++++++++++
3 files changed, 113 insertions(+)
create mode 100644 drivers/gpio/gpio-tn48m.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index fab571016adf..5ccdf0636fad 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1344,6 +1344,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 32a32659866a..93abc7461e45 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -148,6 +148,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..08909555e73d
--- /dev/null
+++ b/drivers/gpio/gpio-tn48m.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Delta TN48M CPLD GPIO driver
+ *
+ * Copyright (C) 2021 Sartura Ltd.
+ *
+ * Author: Robert Marko <[email protected]>
+ */
+
+#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_GP0 = 1,
+ TN48M_GPI,
+};
+
+struct tn48m_gpio_config {
+ int ngpio;
+ int ngpio_per_reg;
+ enum tn48m_gpio_type type;
+};
+
+static const struct tn48m_gpio_config tn48m_gpo_config = {
+ .ngpio = 4,
+ .ngpio_per_reg = 4,
+ .type = TN48M_GP0,
+};
+
+static const struct tn48m_gpio_config tn48m_gpi_config = {
+ .ngpio = 4,
+ .ngpio_per_reg = 4,
+ .type = TN48M_GPI,
+};
+
+static int tn48m_gpio_probe(struct platform_device *pdev)
+{
+ const struct tn48m_gpio_config *gpio_config = NULL;
+ struct gpio_regmap_config config = {0};
+ struct regmap *regmap;
+ u32 base;
+ int ret;
+
+ if (!pdev->dev.parent)
+ return -ENODEV;
+
+ gpio_config = device_get_match_data(&pdev->dev);
+ if (!gpio_config)
+ 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 = gpio_config->ngpio;
+ config.ngpio_per_reg = gpio_config->ngpio_per_reg;
+ switch (gpio_config->type) {
+ case TN48M_GP0:
+ config.reg_set_base = base;
+ break;
+ case TN48M_GPI:
+ config.reg_dat_base = base;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ 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-gpo", .data = &tn48m_gpo_config },
+ { .compatible = "delta,tn48m-gpi", .data = &tn48m_gpi_config },
+ { }
+};
+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 <[email protected]>");
+MODULE_DESCRIPTION("Delta TN48M CPLD GPIO driver");
+MODULE_LICENSE("GPL");
--
2.33.1

2021-11-02 16:58:51

by Robert Marko

[permalink] [raw]
Subject: [PATCH v7 4/6] reset: Add Delta TN48M CPLD reset controller

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 <[email protected]>
Reviewed-by: Philipp Zabel <[email protected]>
---
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 5656cac04b4c..e76aba5f4c84 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -243,6 +243,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 ea8b8d9ca565..79beab92324f 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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_DESCRIPTION("Delta TN48M CPLD reset driver");
+MODULE_LICENSE("GPL");
--
2.33.1

2021-11-02 16:59:04

by Robert Marko

[permalink] [raw]
Subject: [PATCH v7 3/6] dt-bindings: reset: Add Delta TN48M

Add header for the Delta TN48M CPLD provided
resets.

Signed-off-by: Robert Marko <[email protected]>
---
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 <[email protected]>
+ */
+
+#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.33.1

2021-11-02 16:59:51

by Robert Marko

[permalink] [raw]
Subject: [PATCH v7 6/6] MAINTAINERS: Add Delta Networks TN48M CPLD drivers

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

Signed-off-by: Robert Marko <[email protected]>
---
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 d7b4f32875a9..92747bfc01db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5289,6 +5289,15 @@ S: Maintained
F: Documentation/hwmon/dps920ab.rst
F: drivers/hwmon/pmbus/dps920ab.c

+DELTA NETWORKS TN48M CPLD DRIVERS
+M: Robert Marko <[email protected]>
+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: [email protected]
S: Orphan
--
2.33.1

2021-11-02 17:00:29

by Robert Marko

[permalink] [raw]
Subject: [PATCH v7 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

Add binding documents for the Delta TN48M CPLD drivers.

Signed-off-by: Robert Marko <[email protected]>
---
Changes in v7:
* Update bindings to reflect driver updates

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 | 39 ++++++++
.../bindings/mfd/delta,tn48m-cpld.yaml | 90 +++++++++++++++++++
.../bindings/reset/delta,tn48m-reset.yaml | 35 ++++++++
3 files changed, 164 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..e3e668a12091
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/delta,tn48m-gpio.yaml
@@ -0,0 +1,39 @@
+# 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 <[email protected]>
+
+description: |
+ This module is part of the Delta TN48M multi-function device. For more
+ details see ../mfd/delta,tn48m-cpld.yaml.
+
+ Delta TN48M has an onboard Lattice CPLD that is used as an GPIO expander.
+ It provides 12 pins in total, they are input-only or ouput-only type.
+
+properties:
+ compatible:
+ enum:
+ - delta,tn48m-gpo
+ - delta,tn48m-gpi
+
+ 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..f6967c1f6235
--- /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 <[email protected]>
+
+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 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-gpo";
+ reg = <0x31>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio@3a {
+ compatible = "delta,tn48m-gpi";
+ reg = <0x3a>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio@40 {
+ compatible = "delta,tn48m-gpi";
+ 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 <[email protected]>
+
+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.33.1

2021-11-02 19:30:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v7 2/6] gpio: Add Delta TN48M CPLD GPIO driver

On Tue, Nov 2, 2021 at 6:57 PM Robert Marko <[email protected]> wrote:
>
> Delta TN48M switch has an onboard Lattice CPLD that is used as a GPIO
> expander.
>
> The CPLD provides 12 pins in total on the TN48M, but on more advanced
> switch models it provides up to 192 pins, so the driver is extendable
> to support more switches.

...

> +static int tn48m_gpio_probe(struct platform_device *pdev)
> +{

> + const struct tn48m_gpio_config *gpio_config = NULL;
> + struct gpio_regmap_config config = {0};

I don't see a point in assigning these variables.

> + struct regmap *regmap;
> + u32 base;
> + int ret;
> +
> + if (!pdev->dev.parent)
> + return -ENODEV;
> +
> + gpio_config = device_get_match_data(&pdev->dev);
> + if (!gpio_config)
> + 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;

> +}

--
With Best Regards,
Andy Shevchenko

2021-11-02 19:32:50

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v7 4/6] reset: Add Delta TN48M CPLD reset controller

On Tue, Nov 2, 2021 at 6:57 PM Robert Marko <[email protected]> wrote:
>
> 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.

...

> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>

> +#include <linux/of.h>

I haven't found any user of this header, but mod_devicetable.h is missing.

> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>

> +#include <dt-bindings/reset/delta,tn48m-reset.h>

Shouldn't this go before linux/* as it provides more generic definitions?

...

> +#define TN48M_RESET_TIMEOUT 125000
> +#define TN48M_RESET_SLEEP 10

In which units? (both)

--
With Best Regards,
Andy Shevchenko

2021-11-05 15:43:33

by Robert Marko

[permalink] [raw]
Subject: Re: [PATCH v7 2/6] gpio: Add Delta TN48M CPLD GPIO driver

On Tue, Nov 2, 2021 at 8:28 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Nov 2, 2021 at 6:57 PM Robert Marko <[email protected]> wrote:
> >
> > Delta TN48M switch has an onboard Lattice CPLD that is used as a GPIO
> > expander.
> >
> > The CPLD provides 12 pins in total on the TN48M, but on more advanced
> > switch models it provides up to 192 pins, so the driver is extendable
> > to support more switches.
>
> ...
>
> > +static int tn48m_gpio_probe(struct platform_device *pdev)
> > +{
>
> > + const struct tn48m_gpio_config *gpio_config = NULL;
> > + struct gpio_regmap_config config = {0};
>
> I don't see a point in assigning these variables.

Hi,
The NULL one isn't required, but the regmap one is, otherwise the
devm_gpio_regmap_register call will
return -EINVAL.

It will fail here:
https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpio-regmap.c#L209

I see that Michael Walle who did the GPIO regmap uses it as well in
the Kontron CPLD driver.
Michael, can you shed some light on this?

Regards,
Robert
>
> > + struct regmap *regmap;
> > + u32 base;
> > + int ret;
> > +
> > + if (!pdev->dev.parent)
> > + return -ENODEV;
> > +
> > + gpio_config = device_get_match_data(&pdev->dev);
> > + if (!gpio_config)
> > + 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;
>
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko



--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr

2021-11-05 16:00:43

by Robert Marko

[permalink] [raw]
Subject: Re: [PATCH v7 4/6] reset: Add Delta TN48M CPLD reset controller

On Tue, Nov 2, 2021 at 8:31 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Nov 2, 2021 at 6:57 PM Robert Marko <[email protected]> wrote:
> >
> > 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.
>
> ...
>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
>
> > +#include <linux/of.h>
>
> I haven't found any user of this header, but mod_devicetable.h is missing.

Hi,
thanks will fix up in v8.

>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset-controller.h>
>
> > +#include <dt-bindings/reset/delta,tn48m-reset.h>
>
> Shouldn't this go before linux/* as it provides more generic definitions?

I have looked at other drivers and I would say that almost all of them
do it this way,
putting it after other includes.
I also personally prefer it this way.

>
> ...
>
> > +#define TN48M_RESET_TIMEOUT 125000
> > +#define TN48M_RESET_SLEEP 10
>
> In which units? (both)

Its microseconds, will make it clear in v8.

Regards,
Robert
>
> --
> With Best Regards,
> Andy Shevchenko



--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr

2021-11-06 01:36:20

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v7 2/6] gpio: Add Delta TN48M CPLD GPIO driver

Am 2021-11-05 12:34, schrieb Robert Marko:
> On Tue, Nov 2, 2021 at 8:28 PM Andy Shevchenko
> <[email protected]> wrote:
>>
>> On Tue, Nov 2, 2021 at 6:57 PM Robert Marko <[email protected]>
>> wrote:
>> >
>> > Delta TN48M switch has an onboard Lattice CPLD that is used as a GPIO
>> > expander.
>> >
>> > The CPLD provides 12 pins in total on the TN48M, but on more advanced
>> > switch models it provides up to 192 pins, so the driver is extendable
>> > to support more switches.
>>
>> ...
>>
>> > +static int tn48m_gpio_probe(struct platform_device *pdev)
>> > +{
>>
>> > + const struct tn48m_gpio_config *gpio_config = NULL;
>> > + struct gpio_regmap_config config = {0};
>>
>> I don't see a point in assigning these variables.
>
> Hi,
> The NULL one isn't required, but the regmap one is, otherwise the
> devm_gpio_regmap_register call will
> return -EINVAL.
>
> It will fail here:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpio-regmap.c#L209
>
> I see that Michael Walle who did the GPIO regmap uses it as well in
> the Kontron CPLD driver.
> Michael, can you shed some light on this?

I guess this was already clarified? Because you don't set
all the properties of the config you have to initialize it.
Otherwise anything could happen.

-michael

2021-11-09 20:54:30

by Robert Marko

[permalink] [raw]
Subject: Re: [PATCH v7 2/6] gpio: Add Delta TN48M CPLD GPIO driver

On Sat, Nov 6, 2021 at 12:19 AM Michael Walle <[email protected]> wrote:
>
> Am 2021-11-05 12:34, schrieb Robert Marko:
> > On Tue, Nov 2, 2021 at 8:28 PM Andy Shevchenko
> > <[email protected]> wrote:
> >>
> >> On Tue, Nov 2, 2021 at 6:57 PM Robert Marko <[email protected]>
> >> wrote:
> >> >
> >> > Delta TN48M switch has an onboard Lattice CPLD that is used as a GPIO
> >> > expander.
> >> >
> >> > The CPLD provides 12 pins in total on the TN48M, but on more advanced
> >> > switch models it provides up to 192 pins, so the driver is extendable
> >> > to support more switches.
> >>
> >> ...
> >>
> >> > +static int tn48m_gpio_probe(struct platform_device *pdev)
> >> > +{
> >>
> >> > + const struct tn48m_gpio_config *gpio_config = NULL;
> >> > + struct gpio_regmap_config config = {0};
> >>
> >> I don't see a point in assigning these variables.
> >
> > Hi,
> > The NULL one isn't required, but the regmap one is, otherwise the
> > devm_gpio_regmap_register call will
> > return -EINVAL.
> >
> > It will fail here:
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpio-regmap.c#L209
> >
> > I see that Michael Walle who did the GPIO regmap uses it as well in
> > the Kontron CPLD driver.
> > Michael, can you shed some light on this?
>
> I guess this was already clarified? Because you don't set
> all the properties of the config you have to initialize it.
> Otherwise anything could happen.

Yeah, I think it's clear now.
Thanks,
Robert
>
> -michael



--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr