2022-02-01 20:40:29

by Robert Marko

[permalink] [raw]
Subject: [PATCH v10 0/6] Add Delta TN48M CPLD support

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

This patch series focuses on providing support for the CPLD provided
peripherals that are needed for the switch to function.

The series has been refined over the last 9 months or so and currently
all of the code has been reviewed and is ready for merge.
We are still hovewer waiting for the DT bindings to be reviewed for a
while, but hopefully Rob will provide feedback soon.

---
Changes in v10:
* Rebase onto 5.17-rc1

Robert Marko (6):
mfd: simple-mfd-i2c: Add Delta TN48M CPLD support
gpio: Add Delta TN48M CPLD GPIO driver
dt-bindings: reset: Add Delta TN48M
reset: Add Delta TN48M CPLD reset controller
dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings
MAINTAINERS: Add Delta Networks TN48M CPLD drivers

.../bindings/gpio/delta,tn48m-gpio.yaml | 39 ++++++
.../bindings/mfd/delta,tn48m-cpld.yaml | 90 ++++++++++++
.../bindings/reset/delta,tn48m-reset.yaml | 35 +++++
MAINTAINERS | 9 ++
drivers/gpio/Kconfig | 12 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tn48m.c | 100 ++++++++++++++
drivers/mfd/Kconfig | 11 ++
drivers/mfd/simple-mfd-i2c.c | 1 +
drivers/reset/Kconfig | 13 ++
drivers/reset/Makefile | 1 +
drivers/reset/reset-tn48m.c | 128 ++++++++++++++++++
include/dt-bindings/reset/delta,tn48m-reset.h | 20 +++
13 files changed, 460 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
create mode 100644 drivers/gpio/gpio-tn48m.c
create mode 100644 drivers/reset/reset-tn48m.c
create mode 100644 include/dt-bindings/reset/delta,tn48m-reset.h

--
2.34.1


2022-02-01 20:40:32

by Robert Marko

[permalink] [raw]
Subject: [PATCH v10 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 ea3e6c914384..04baac692330 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5488,6 +5488,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.34.1

2022-02-01 20:40:32

by Robert Marko

[permalink] [raw]
Subject: [PATCH v10 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.34.1

2022-02-01 20:40:33

by Robert Marko

[permalink] [raw]
Subject: [PATCH v10 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]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
Changes in v10:
* Rebase onto 5.17-rc1

Changes in v9:
* Expand KConfig help per Andys comment
* Drop the comma in of_device_id per Andys comment

Changes in v8:
* Drop of.h and include mod_devicetable.h per Andys comment
* Mark the units used in timeout and sleep defines for the timeout poller

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.

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

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 6f8ba0ddc05f..b496028b6bfa 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -256,6 +256,19 @@ 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.
+
+ This driver can also be built as a module. If so, the module will be
+ called reset-tn48m.
+
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 bd0a97be18b5..a80a9c4008a7 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_RESET_STARFIVE_JH7100) += reset-starfive-jh7100.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..130027291b6e
--- /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/mod_devicetable.h>
+#include <linux/module.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_US 125000
+#define TN48M_RESET_SLEEP_US 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_US,
+ TN48M_RESET_TIMEOUT_US);
+}
+
+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.34.1

2022-03-02 15:50:34

by Lee Jones

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

On Mon, 31 Jan 2022, Robert Marko wrote:

> Add binding documents for the Delta TN48M CPLD drivers.
>
> Signed-off-by: Robert Marko <[email protected]>

This is missing a DT review.

> ---
> 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

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

2022-03-02 23:46:42

by Rob Herring

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

On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> On Mon, 31 Jan 2022, Robert Marko wrote:
>
> > Add binding documents for the Delta TN48M CPLD drivers.
> >
> > Signed-off-by: Robert Marko <[email protected]>
>
> This is missing a DT review.

How about this one[1]?

Rob

[1] https://lore.kernel.org/all/[email protected]/

2022-03-03 10:58:52

by Lee Jones

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

On Wed, 02 Mar 2022, Rob Herring wrote:

> On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > On Mon, 31 Jan 2022, Robert Marko wrote:
> >
> > > Add binding documents for the Delta TN48M CPLD drivers.
> > >
> > > Signed-off-by: Robert Marko <[email protected]>
> >
> > This is missing a DT review.
>
> How about this one[1]?
>
> Rob
>
> [1] https://lore.kernel.org/all/[email protected]/

Looks like the ball is in your court Robert.

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

2022-03-03 12:48:26

by Robert Marko

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

On Wed, Mar 2, 2022 at 10:39 PM Rob Herring <[email protected]> wrote:
>
> On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > On Mon, 31 Jan 2022, Robert Marko wrote:
> >
> > > Add binding documents for the Delta TN48M CPLD drivers.
> > >
> > > Signed-off-by: Robert Marko <[email protected]>
> >
> > This is missing a DT review.
>
> How about this one[1]?
>
> Rob
>
> [1] https://lore.kernel.org/all/[email protected]/

Hi Rob,
Thanks for reaching out.

As you can see the bindings have evolved since v6,
GPIO driver now only uses 2 distinct compatibles.

I am open to discussion in order to reach some kind of a consensus on
this matter
as the whole series is currently unfortunately stalled.

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

2022-03-04 22:15:50

by Rob Herring

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

On Thu, Mar 03, 2022 at 01:41:13PM +0100, Robert Marko wrote:
> On Wed, Mar 2, 2022 at 10:39 PM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > > On Mon, 31 Jan 2022, Robert Marko wrote:
> > >
> > > > Add binding documents for the Delta TN48M CPLD drivers.
> > > >
> > > > Signed-off-by: Robert Marko <[email protected]>
> > >
> > > This is missing a DT review.
> >
> > How about this one[1]?
> >
> > Rob
> >
> > [1] https://lore.kernel.org/all/[email protected]/
>
> Hi Rob,
> Thanks for reaching out.
>
> As you can see the bindings have evolved since v6,
> GPIO driver now only uses 2 distinct compatibles.

Fundamentally, it hasn't really changed.

There's 2 main issues. First, I don't see the need for any child nodes.
This would be sufficient:

cpld@41 {
compatible = "delta,tn48m-cpld";
reg = <0x41>;
#reset-cells = <1>;
#gpio-cells = <2>;
gpio-controller;
};

You only need child nodes if the sub-blocks have their own resources or
are widely reused in different configurations.

The 2nd issue is whether GPIOs are even GPIOs at all. I don't recall
that Linus ever agreed.

Both issues kind of boil down to is there even more that 1 variation of
this h/w where you have differing connections? AFAICT, Delta tn48m is a
pretty specific device and I would guess something implemented in a CPLD
is likely to change on every board design. At least that's my experience
with 'board level logic'.

Rob

2022-03-17 18:10:02

by Robert Marko

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

On Fri, Mar 4, 2022 at 10:47 PM Rob Herring <[email protected]> wrote:
>
> On Thu, Mar 03, 2022 at 01:41:13PM +0100, Robert Marko wrote:
> > On Wed, Mar 2, 2022 at 10:39 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Wed, Mar 02, 2022 at 08:47:32AM +0000, Lee Jones wrote:
> > > > On Mon, 31 Jan 2022, Robert Marko wrote:
> > > >
> > > > > Add binding documents for the Delta TN48M CPLD drivers.
> > > > >
> > > > > Signed-off-by: Robert Marko <[email protected]>
> > > >
> > > > This is missing a DT review.
> > >
> > > How about this one[1]?
> > >
> > > Rob
> > >
> > > [1] https://lore.kernel.org/all/[email protected]/
> >
> > Hi Rob,
> > Thanks for reaching out.
> >
> > As you can see the bindings have evolved since v6,
> > GPIO driver now only uses 2 distinct compatibles.
>
> Fundamentally, it hasn't really changed.
>
> There's 2 main issues. First, I don't see the need for any child nodes.
> This would be sufficient:
>
> cpld@41 {
> compatible = "delta,tn48m-cpld";
> reg = <0x41>;
> #reset-cells = <1>;
> #gpio-cells = <2>;
> gpio-controller;
> };
>
> You only need child nodes if the sub-blocks have their own resources or
> are widely reused in different configurations.
>
> The 2nd issue is whether GPIOs are even GPIOs at all. I don't recall
> that Linus ever agreed.
>
> Both issues kind of boil down to is there even more that 1 variation of
> this h/w where you have differing connections? AFAICT, Delta tn48m is a
> pretty specific device and I would guess something implemented in a CPLD
> is likely to change on every board design. At least that's my experience
> with 'board level logic'.

Hi Rob, sorry for the late reply.

Having one node was the route I went in v1, but that was rejected as
it would mean
having an MFD driver that just registers the sub-drivers.
That is what the simple-mfd-i2c driver was designed to get rid of and
inherit the regmap
from the parent.
For this to work, subnodes are required as we need to match on compatibles.

Using subnodes for GPIO-s also gets rid of hardcoding the register
layout in the driver per board,
as Delta chose to use a weird register layout in which the GPIO
registers have completely random offsets.
The layout is even weirder in the TN4810M which uses the same CPLD but
expanded and is easily
supportable in the same driver in the current form.
My goal and the requirement from the community was to make the GPIO
driver as simple as possible
and extendable so that boards like TN4810M can be easily added.

Also, the Kontron SL28CPLD does pretty much the same in regards to DT
as well as other things.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml?h=v5.16.15

It uses the same logic with different compatibles for GPIO to be able
to inform the kernel of the certain
bank capabilities.
I mean, using one compatible would be possible by using a boolean
property for example that tells you
that its output capable as well.

Regards,
Robert

>
> Rob



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