Delta TN48M switches have a Lattice CPLD that serves
multiple purposes including being a GPIO expander.
So lets add the MFD core driver for it.
Signed-off-by: Robert Marko <[email protected]>
---
drivers/mfd/Kconfig | 13 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/tn48m-cpld.c | 181 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/tn48m.h | 30 +++++++
4 files changed, 225 insertions(+)
create mode 100644 drivers/mfd/tn48m-cpld.c
create mode 100644 include/linux/mfd/tn48m.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b74efa469e90..809041f98d71 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -297,6 +297,19 @@ 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_CORE
+ select REGMAP_I2C
+ help
+ Select this option to enable support for Delta Networks TN48M switch
+ CPLD. It consists of GPIO and hwmon drivers.
+ CPLD provides GPIOS-s for the SFP slots as well as power supply
+ related information.
+ Driver provides debugfs information about the board model as
+ well as hardware and CPLD revision information.
+
config PMIC_DA903X
bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 834f5463af28..974663341f08 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_MFD_TI_LP87565) += lp87565.o
obj-$(CONFIG_MFD_DAVINCI_VOICECODEC) += davinci_voicecodec.o
obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o
obj-$(CONFIG_MFD_TI_AM335X_TSCADC) += ti_am335x_tscadc.o
+obj-$(CONFIG_MFD_TN48M_CPLD) += tn48m-cpld.o
obj-$(CONFIG_MFD_STA2X11) += sta2x11-mfd.o
obj-$(CONFIG_MFD_STMPE) += stmpe.o
diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
new file mode 100644
index 000000000000..b84510fb630a
--- /dev/null
+++ b/drivers/mfd/tn48m-cpld.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Delta TN48M CPLD parent driver
+ *
+ * Copyright 2020 Sartura Ltd
+ *
+ * Author: Robert Marko <[email protected]>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/tn48m.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+static const struct mfd_cell tn48m_cell[] = {};
+
+static const struct regmap_config tn48m_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x40,
+};
+
+static int hardware_version_show(struct seq_file *s, void *data)
+{
+ struct tn48m_data *priv = s->private;
+ unsigned int regval;
+ char *buf;
+
+ regmap_read(priv->regmap, HARDWARE_VERSION_ID, ®val);
+
+ switch (FIELD_GET(HARDWARE_VERSION_MASK, regval)) {
+ case HARDWARE_VERSION_EVT1:
+ buf = "EVT1";
+ break;
+ case HARDWARE_VERSION_EVT2:
+ buf = "EVT2";
+ break;
+ case HARDWARE_VERSION_DVT:
+ buf = "DVT";
+ break;
+ case HARDWARE_VERSION_PVT:
+ buf = "PVT";
+ break;
+ default:
+ buf = "Unknown";
+ break;
+ }
+
+ seq_printf(s, "%s\n", buf);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(hardware_version);
+
+static int board_id_show(struct seq_file *s, void *data)
+{
+ struct tn48m_data *priv = s->private;
+ unsigned int regval;
+ char *buf;
+
+ regmap_read(priv->regmap, BOARD_ID, ®val);
+
+ switch (regval) {
+ case BOARD_ID_TN48M:
+ buf = "TN48M";
+ break;
+ case BOARD_ID_TN48M_P:
+ buf = "TN48-P";
+ break;
+ default:
+ buf = "Unknown";
+ break;
+ }
+
+ seq_printf(s, "%s\n", buf);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(board_id);
+
+static int code_version_show(struct seq_file *s, void *data)
+{
+ struct tn48m_data *priv = s->private;
+ unsigned int regval;
+
+ regmap_read(priv->regmap, CPLD_CODE_VERSION, ®val);
+
+ seq_printf(s, "%d\n", regval);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(code_version);
+
+static void tn48m_init_debugfs(struct tn48m_data *data)
+{
+ data->debugfs_dir = debugfs_create_dir(data->client->name, NULL);
+
+ debugfs_create_file("hardware_version",
+ 0400,
+ data->debugfs_dir,
+ data,
+ &hardware_version_fops);
+
+ debugfs_create_file("board_id",
+ 0400,
+ data->debugfs_dir,
+ data,
+ &board_id_fops);
+
+ debugfs_create_file("code_version",
+ 0400,
+ data->debugfs_dir,
+ data,
+ &code_version_fops);
+}
+
+static int tn48m_probe(struct i2c_client *client)
+{
+ struct tn48m_data *data;
+ int ret;
+
+ data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->client = client;
+ data->dev = &client->dev;
+ i2c_set_clientdata(client, data);
+
+ data->regmap = devm_regmap_init_i2c(client, &tn48m_regmap_config);
+ if (IS_ERR(data->regmap)) {
+ dev_err(data->dev, "Failed to allocate regmap\n");
+ return PTR_ERR(data->regmap);
+ }
+
+ ret = devm_mfd_add_devices(data->dev, PLATFORM_DEVID_AUTO, tn48m_cell,
+ ARRAY_SIZE(tn48m_cell), NULL, 0, NULL);
+ if (ret)
+ dev_err(data->dev, "Failed to register sub-devices %d\n", ret);
+
+ tn48m_init_debugfs(data);
+
+ return ret;
+}
+
+static int tn48m_remove(struct i2c_client *client)
+{
+ struct tn48m_data *data = i2c_get_clientdata(client);
+
+ debugfs_remove_recursive(data->debugfs_dir);
+
+ return 0;
+}
+
+static const struct of_device_id tn48m_of_match[] = {
+ { .compatible = "delta,tn48m-cpld"},
+ { }
+};
+MODULE_DEVICE_TABLE(of, tn48m_of_match);
+
+static struct i2c_driver tn48m_driver = {
+ .driver = {
+ .name = "tn48m-cpld",
+ .of_match_table = tn48m_of_match,
+ },
+ .probe_new = tn48m_probe,
+ .remove = tn48m_remove,
+};
+module_i2c_driver(tn48m_driver);
+
+MODULE_AUTHOR("Robert Marko <[email protected]>");
+MODULE_DESCRIPTION("Delta TN48M CPLD parent driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
new file mode 100644
index 000000000000..551c550efa54
--- /dev/null
+++ b/include/linux/mfd/tn48m.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Sartura Ltd
+ */
+
+#ifndef __TN48M_H__
+#define __TN48M_H__
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+#define HARDWARE_VERSION_ID 0x0
+#define HARDWARE_VERSION_MASK GENMASK(3, 0)
+#define HARDWARE_VERSION_EVT1 0
+#define HARDWARE_VERSION_EVT2 1
+#define HARDWARE_VERSION_DVT 2
+#define HARDWARE_VERSION_PVT 3
+#define BOARD_ID 0x1
+#define BOARD_ID_TN48M 0xa
+#define BOARD_ID_TN48M_P 0xb
+#define CPLD_CODE_VERSION 0x2
+
+struct tn48m_data {
+ struct device *dev;
+ struct regmap *regmap;
+ struct i2c_client *client;
+ struct dentry *debugfs_dir;
+};
+
+#endif
--
2.31.1
Add binding document for the Delta TN48M CPLD drivers.
Signed-off-by: Robert Marko <[email protected]>
---
.../bindings/mfd/delta,tn48m-cpld.yaml | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
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..7d81943d3d27
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
@@ -0,0 +1,59 @@
+# 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 for the SFP slots.
+
+properties:
+ compatible:
+ const: delta,tn48m-cpld
+
+ reg:
+ description:
+ I2C device address.
+ maxItems: 1
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+ description:
+ The first cell is the pin number and the second cell is used to specify
+ the gpio active state.
+
+required:
+ - compatible
+ - reg
+ - gpio-controller
+ - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpld@41 {
+ compatible = "delta,tn48m-cpld";
+ reg = <0x41>;
+
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };
--
2.31.1
Delta TN48M CPLD is used as a GPIO expander for the SFP GPIOs.
It is a mix of input only and output only pins.
Since there is no logical GPIO numbering arbitrary one is used
along dt-bindings to make it humanly readable.
Signed-off-by: Robert Marko <[email protected]>
---
drivers/gpio/Kconfig | 12 +++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tn48m.c | 191 ++++++++++++++++++++++++++++++++++++++
drivers/mfd/tn48m-cpld.c | 6 +-
include/linux/mfd/tn48m.h | 3 +
5 files changed, 212 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpio/gpio-tn48m.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e3607ec4c2e8..e17d4416786a 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
+ depends on OF_GPIO
+ 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..4e859e4c339c
--- /dev/null
+++ b/drivers/gpio/gpio-tn48m.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Delta TN48M CPLD GPIO driver
+ *
+ * Copyright 2020 Sartura Ltd
+ *
+ * Author: Robert Marko <[email protected]>
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/tn48m.h>
+#include <dt-bindings/gpio/tn48m-gpio.h>
+
+struct tn48m_gpio {
+ struct gpio_chip chip;
+ struct tn48m_data *data;
+};
+
+static int tn48m_gpio_get_direction(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ switch (offset) {
+ case SFP_TX_DISABLE_52:
+ case SFP_TX_DISABLE_51:
+ case SFP_TX_DISABLE_50:
+ case SFP_TX_DISABLE_49:
+ return GPIO_LINE_DIRECTION_OUT;
+ case SFP_PRESENT_52:
+ case SFP_PRESENT_51:
+ case SFP_PRESENT_50:
+ case SFP_PRESENT_49:
+ case SFP_LOS_52:
+ case SFP_LOS_51:
+ case SFP_LOS_50:
+ case SFP_LOS_49:
+ return GPIO_LINE_DIRECTION_IN;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int tn48m_gpio_get_reg(unsigned int offset)
+{
+ switch (offset) {
+ case SFP_TX_DISABLE_52:
+ case SFP_TX_DISABLE_51:
+ case SFP_TX_DISABLE_50:
+ case SFP_TX_DISABLE_49:
+ return SFP_TX_DISABLE;
+ case SFP_PRESENT_52:
+ case SFP_PRESENT_51:
+ case SFP_PRESENT_50:
+ case SFP_PRESENT_49:
+ return SFP_PRESENT;
+ case SFP_LOS_52:
+ case SFP_LOS_51:
+ case SFP_LOS_50:
+ case SFP_LOS_49:
+ return SFP_LOS;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int tn48m_gpio_get_mask(unsigned int offset)
+{
+ switch (offset) {
+ case SFP_TX_DISABLE_52:
+ case SFP_PRESENT_52:
+ case SFP_LOS_52:
+ return BIT(3);
+ case SFP_TX_DISABLE_51:
+ case SFP_PRESENT_51:
+ case SFP_LOS_51:
+ return BIT(2);
+ case SFP_TX_DISABLE_50:
+ case SFP_PRESENT_50:
+ case SFP_LOS_50:
+ return BIT(1);
+ case SFP_TX_DISABLE_49:
+ case SFP_PRESENT_49:
+ case SFP_LOS_49:
+ return BIT(0);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int tn48m_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct tn48m_gpio *gpio = gpiochip_get_data(chip);
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(gpio->data->regmap,
+ tn48m_gpio_get_reg(offset),
+ ®val);
+ if (ret < 0)
+ return ret;
+
+ return regval & tn48m_gpio_get_mask(offset);
+}
+
+static void tn48m_gpio_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct tn48m_gpio *gpio = gpiochip_get_data(chip);
+
+ regmap_update_bits(gpio->data->regmap,
+ tn48m_gpio_get_reg(offset),
+ tn48m_gpio_get_mask(offset),
+ value ? tn48m_gpio_get_mask(offset) : 0);
+}
+
+static int tn48m_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int offset, int value)
+{
+ tn48m_gpio_set(chip, offset, value);
+
+ return 0;
+}
+
+/*
+ * Required for SFP as it calls gpiod_direction_input()
+ * and if its missing TX disable GPIO will print an
+ * error and not be controlled anymore.
+ */
+static int tn48m_gpio_direction_input(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ return 0;
+}
+
+static const struct gpio_chip tn48m_template_chip = {
+ .label = "tn48m-gpio",
+ .owner = THIS_MODULE,
+ .get_direction = tn48m_gpio_get_direction,
+ .direction_output = tn48m_gpio_direction_output,
+ .direction_input = tn48m_gpio_direction_input,
+ .get = tn48m_gpio_get,
+ .set = tn48m_gpio_set,
+ .base = -1,
+ .ngpio = 12,
+ .can_sleep = true,
+};
+
+static int tn48m_gpio_probe(struct platform_device *pdev)
+{
+ struct tn48m_gpio *gpio;
+ int ret;
+
+ gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+ if (!gpio)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, gpio);
+
+ gpio->data = dev_get_drvdata(pdev->dev.parent);
+ gpio->chip = tn48m_template_chip;
+ gpio->chip.parent = gpio->data->dev;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct platform_device_id tn48m_gpio_id_table[] = {
+ { "delta,tn48m-gpio", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, tn48m_gpio_id_table);
+
+static struct platform_driver tn48m_gpio_driver = {
+ .driver = {
+ .name = "tn48m-gpio",
+ },
+ .probe = tn48m_gpio_probe,
+ .id_table = tn48m_gpio_id_table,
+};
+module_platform_driver(tn48m_gpio_driver);
+
+MODULE_AUTHOR("Robert Marko <[email protected]>");
+MODULE_DESCRIPTION("Delta TN48M CPLD GPIO driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
index b84510fb630a..f22a15ddd22d 100644
--- a/drivers/mfd/tn48m-cpld.c
+++ b/drivers/mfd/tn48m-cpld.c
@@ -17,7 +17,11 @@
#include <linux/regmap.h>
#include <linux/slab.h>
-static const struct mfd_cell tn48m_cell[] = {};
+static const struct mfd_cell tn48m_cell[] = {
+ {
+ .name = "delta,tn48m-gpio",
+ }
+};
static const struct regmap_config tn48m_regmap_config = {
.reg_bits = 8,
diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
index 551c550efa54..9cc2b04c8d69 100644
--- a/include/linux/mfd/tn48m.h
+++ b/include/linux/mfd/tn48m.h
@@ -19,6 +19,9 @@
#define BOARD_ID_TN48M 0xa
#define BOARD_ID_TN48M_P 0xb
#define CPLD_CODE_VERSION 0x2
+#define SFP_TX_DISABLE 0x31
+#define SFP_PRESENT 0x3a
+#define SFP_LOS 0x40
struct tn48m_data {
struct device *dev;
--
2.31.1
CPLD inside of the Delta TN48M does not number GPIOs
at all, so in order to ensure numbering lets use bindigs.
Signed-off-by: Robert Marko <[email protected]>
---
include/dt-bindings/gpio/tn48m-gpio.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 include/dt-bindings/gpio/tn48m-gpio.h
diff --git a/include/dt-bindings/gpio/tn48m-gpio.h b/include/dt-bindings/gpio/tn48m-gpio.h
new file mode 100644
index 000000000000..4ece4826d746
--- /dev/null
+++ b/include/dt-bindings/gpio/tn48m-gpio.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * GPIO definitions for Delta TN48M CPLD GPIO driver
+ *
+ * Copyright 2020 Sartura Ltd
+ *
+ * Author: Robert Marko <[email protected]>
+ */
+
+#ifndef _DT_BINDINGS_TN48M_GPIO_H
+#define _DT_BINDINGS_TN48M_GPIO_H
+
+#define SFP_TX_DISABLE_52 0
+#define SFP_TX_DISABLE_51 1
+#define SFP_TX_DISABLE_50 2
+#define SFP_TX_DISABLE_49 3
+#define SFP_PRESENT_52 4
+#define SFP_PRESENT_51 5
+#define SFP_PRESENT_50 6
+#define SFP_PRESENT_49 7
+#define SFP_LOS_52 8
+#define SFP_LOS_51 9
+#define SFP_LOS_50 10
+#define SFP_LOS_49 11
+
+#endif /* _DT_BINDINGS_TN48M_GPIO_H */
--
2.31.1
Add maintainers entry for the Delta Networks TN48M
CPLD MFD drivers.
Signed-off-by: Robert Marko <[email protected]>
---
MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9450e052f1b1..7193624868c7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5096,6 +5096,16 @@ 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 <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
+F: drivers/gpio/gpio-tn48m.c
+F: drivers/hwmon/tn48m-hwmon.c
+F: drivers/mfd/tn48m-cpld.c
+F: include/dt-bindings/gpio/tn48m-gpio.h
+F: include/linux/mfd/tn48m.h
+
DENALI NAND DRIVER
L: [email protected]
S: Orphan
--
2.31.1
On Fri, Apr 30, 2021 at 02:35:07PM +0200, Robert Marko wrote:
> CPLD inside of the Delta TN48M does not number GPIOs
> at all, so in order to ensure numbering lets use bindigs.
Looking at the code, I'd make the gpio number something like '(offset <<
8) | bit' rather than just making up an index.
We don't normally have defines for GPIO numbers either.
>
> Signed-off-by: Robert Marko <[email protected]>
> ---
> include/dt-bindings/gpio/tn48m-gpio.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
> create mode 100644 include/dt-bindings/gpio/tn48m-gpio.h
>
> diff --git a/include/dt-bindings/gpio/tn48m-gpio.h b/include/dt-bindings/gpio/tn48m-gpio.h
> new file mode 100644
> index 000000000000..4ece4826d746
> --- /dev/null
> +++ b/include/dt-bindings/gpio/tn48m-gpio.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * GPIO definitions for Delta TN48M CPLD GPIO driver
> + *
> + * Copyright 2020 Sartura Ltd
> + *
> + * Author: Robert Marko <[email protected]>
> + */
> +
> +#ifndef _DT_BINDINGS_TN48M_GPIO_H
> +#define _DT_BINDINGS_TN48M_GPIO_H
> +
> +#define SFP_TX_DISABLE_52 0
> +#define SFP_TX_DISABLE_51 1
> +#define SFP_TX_DISABLE_50 2
> +#define SFP_TX_DISABLE_49 3
> +#define SFP_PRESENT_52 4
> +#define SFP_PRESENT_51 5
> +#define SFP_PRESENT_50 6
> +#define SFP_PRESENT_49 7
> +#define SFP_LOS_52 8
> +#define SFP_LOS_51 9
> +#define SFP_LOS_50 10
> +#define SFP_LOS_49 11
> +
> +#endif /* _DT_BINDINGS_TN48M_GPIO_H */
> --
> 2.31.1
>
On Fri, 30 Apr 2021 14:35:10 +0200, Robert Marko wrote:
> Add binding document for the Delta TN48M CPLD drivers.
>
> Signed-off-by: Robert Marko <[email protected]>
> ---
> .../bindings/mfd/delta,tn48m-cpld.yaml | 59 +++++++++++++++++++
> 1 file changed, 59 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/delta,tn48m-cpld.yaml
>
Reviewed-by: Rob Herring <[email protected]>
Am 2021-04-30 14:35, schrieb 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.
>
> Since there is no logical GPIO numbering arbitrary one is used
> along dt-bindings to make it humanly readable.
Can gpio/gpio-regmap.c be used here? See gpio/gpio-sl28cpld.c
for an example.
-michael
Am 2021-05-06 16:00, schrieb Rob Herring:
> On Fri, Apr 30, 2021 at 02:35:08PM +0200, Robert Marko wrote:
>> Delta TN48M CPLD is used as a GPIO expander for the SFP GPIOs.
>>
>> It is a mix of input only and output only pins.
>>
>> Since there is no logical GPIO numbering arbitrary one is used
>> along dt-bindings to make it humanly readable.
>>
>> Signed-off-by: Robert Marko <[email protected]>
>> ---
>> drivers/gpio/Kconfig | 12 +++
>> drivers/gpio/Makefile | 1 +
>> drivers/gpio/gpio-tn48m.c | 191
>> ++++++++++++++++++++++++++++++++++++++
>> drivers/mfd/tn48m-cpld.c | 6 +-
>> include/linux/mfd/tn48m.h | 3 +
>> 5 files changed, 212 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpio/gpio-tn48m.c
>
>
>> +static const struct platform_device_id tn48m_gpio_id_table[] = {
>> + { "delta,tn48m-gpio", },
>
> Looks like a compatible, but is not. I think you can drop this and just
> use 'tm48m-gpio' (the driver name).
I'm just curious, why isn't the vendor included here (as there
might be a chance for name clashes in the future).
-michael
On Fri, Apr 30, 2021 at 02:35:08PM +0200, Robert Marko wrote:
> Delta TN48M CPLD is used as a GPIO expander for the SFP GPIOs.
>
> It is a mix of input only and output only pins.
>
> Since there is no logical GPIO numbering arbitrary one is used
> along dt-bindings to make it humanly readable.
>
> Signed-off-by: Robert Marko <[email protected]>
> ---
> drivers/gpio/Kconfig | 12 +++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-tn48m.c | 191 ++++++++++++++++++++++++++++++++++++++
> drivers/mfd/tn48m-cpld.c | 6 +-
> include/linux/mfd/tn48m.h | 3 +
> 5 files changed, 212 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpio/gpio-tn48m.c
> +static const struct platform_device_id tn48m_gpio_id_table[] = {
> + { "delta,tn48m-gpio", },
Looks like a compatible, but is not. I think you can drop this and just
use 'tm48m-gpio' (the driver name).
Same for hwmon.
Rob
> + { }
> +};
> +MODULE_DEVICE_TABLE(platform, tn48m_gpio_id_table);
> +
> +static struct platform_driver tn48m_gpio_driver = {
> + .driver = {
> + .name = "tn48m-gpio",
> + },
> + .probe = tn48m_gpio_probe,
> + .id_table = tn48m_gpio_id_table,
> +};
> +module_platform_driver(tn48m_gpio_driver);
Hi Robert,
Am 2021-04-30 14:35, schrieb Robert Marko:
> Delta TN48M switches have a Lattice CPLD that serves
> multiple purposes including being a GPIO expander.
> So lets add the MFD core driver for it.
Did you have a look at mfd/simple-mfd-i2c.c?
-michael
Hi Robert,
I love your patch! Yet something to improve:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on hwmon/hwmon-next gpio/for-next v5.12 next-20210506]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Robert-Marko/mfd-Add-Delta-TN48M-CPLD-driver/20210430-203709
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/4a3a37836c56a383a726a52892d69bfdd1cf5d4c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Robert-Marko/mfd-Add-Delta-TN48M-CPLD-driver/20210430-203709
git checkout 4a3a37836c56a383a726a52892d69bfdd1cf5d4c
# save the attached .config to linux build tree
make W=1 W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/mfd/tn48m-cpld.c: In function 'hardware_version_show':
>> drivers/mfd/tn48m-cpld.c:36:10: error: implicit declaration of function 'FIELD_GET' [-Werror=implicit-function-declaration]
36 | switch (FIELD_GET(HARDWARE_VERSION_MASK, regval)) {
| ^~~~~~~~~
cc1: some warnings being treated as errors
vim +/FIELD_GET +36 drivers/mfd/tn48m-cpld.c
27
28 static int hardware_version_show(struct seq_file *s, void *data)
29 {
30 struct tn48m_data *priv = s->private;
31 unsigned int regval;
32 char *buf;
33
34 regmap_read(priv->regmap, HARDWARE_VERSION_ID, ®val);
35
> 36 switch (FIELD_GET(HARDWARE_VERSION_MASK, regval)) {
37 case HARDWARE_VERSION_EVT1:
38 buf = "EVT1";
39 break;
40 case HARDWARE_VERSION_EVT2:
41 buf = "EVT2";
42 break;
43 case HARDWARE_VERSION_DVT:
44 buf = "DVT";
45 break;
46 case HARDWARE_VERSION_PVT:
47 buf = "PVT";
48 break;
49 default:
50 buf = "Unknown";
51 break;
52 }
53
54 seq_printf(s, "%s\n", buf);
55
56 return 0;
57 }
58
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hello Michael,
On Thu, May 6, 2021 at 6:40 PM Michael Walle <[email protected]> wrote:
>
> Am 2021-05-06 16:00, schrieb Rob Herring:
> > On Fri, Apr 30, 2021 at 02:35:08PM +0200, Robert Marko wrote:
> >> Delta TN48M CPLD is used as a GPIO expander for the SFP GPIOs.
> >>
> >> It is a mix of input only and output only pins.
> >>
> >> Since there is no logical GPIO numbering arbitrary one is used
> >> along dt-bindings to make it humanly readable.
> >>
> >> Signed-off-by: Robert Marko <[email protected]>
> >> ---
> >> drivers/gpio/Kconfig | 12 +++
> >> drivers/gpio/Makefile | 1 +
> >> drivers/gpio/gpio-tn48m.c | 191
> >> ++++++++++++++++++++++++++++++++++++++
> >> drivers/mfd/tn48m-cpld.c | 6 +-
> >> include/linux/mfd/tn48m.h | 3 +
> >> 5 files changed, 212 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/gpio/gpio-tn48m.c
> >
> >
> >> +static const struct platform_device_id tn48m_gpio_id_table[] = {
> >> + { "delta,tn48m-gpio", },
> >
> > Looks like a compatible, but is not. I think you can drop this and just
> > use 'tm48m-gpio' (the driver name).
>
> I'm just curious, why isn't the vendor included here (as there
> might be a chance for name clashes in the future).
I'm looping in Clement from Delta as well as the Upstream Working
Group from DENT [0].
Thanks,
Luka
[0] https://dent.dev/
On Fri, 30 Apr 2021, Robert Marko wrote:
> Delta TN48M switches have a Lattice CPLD that serves
> multiple purposes including being a GPIO expander.
> So lets add the MFD core driver for it.
>
> Signed-off-by: Robert Marko <[email protected]>
> ---
> drivers/mfd/Kconfig | 13 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/tn48m-cpld.c | 181 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/tn48m.h | 30 +++++++
> 4 files changed, 225 insertions(+)
> create mode 100644 drivers/mfd/tn48m-cpld.c
> create mode 100644 include/linux/mfd/tn48m.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b74efa469e90..809041f98d71 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -297,6 +297,19 @@ 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_CORE
> + select REGMAP_I2C
> + help
> + Select this option to enable support for Delta Networks TN48M switch
> + CPLD. It consists of GPIO and hwmon drivers.
> + CPLD provides GPIOS-s for the SFP slots as well as power supply
> + related information.
> + Driver provides debugfs information about the board model as
> + well as hardware and CPLD revision information.
No need for every sentence to be it's own paragraphs.
Please re-align to be a single chunk.
> config PMIC_DA903X
> bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"
> depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 834f5463af28..974663341f08 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_MFD_TI_LP87565) += lp87565.o
> obj-$(CONFIG_MFD_DAVINCI_VOICECODEC) += davinci_voicecodec.o
> obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o
> obj-$(CONFIG_MFD_TI_AM335X_TSCADC) += ti_am335x_tscadc.o
> +obj-$(CONFIG_MFD_TN48M_CPLD) += tn48m-cpld.o
>
> obj-$(CONFIG_MFD_STA2X11) += sta2x11-mfd.o
> obj-$(CONFIG_MFD_STMPE) += stmpe.o
> diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
> new file mode 100644
> index 000000000000..b84510fb630a
> --- /dev/null
> +++ b/drivers/mfd/tn48m-cpld.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Delta TN48M CPLD parent driver
> + *
> + * Copyright 2020 Sartura Ltd
This is out of date.
> + * Author: Robert Marko <[email protected]>
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/tn48m.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +static const struct mfd_cell tn48m_cell[] = {};
Please populate this.
Without it, this is not an MFD.
> +static const struct regmap_config tn48m_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x40,
> +};
> +
> +static int hardware_version_show(struct seq_file *s, void *data)
> +{
> + struct tn48m_data *priv = s->private;
> + unsigned int regval;
> + char *buf;
> +
> + regmap_read(priv->regmap, HARDWARE_VERSION_ID, ®val);
> +
> + switch (FIELD_GET(HARDWARE_VERSION_MASK, regval)) {
> + case HARDWARE_VERSION_EVT1:
> + buf = "EVT1";
> + break;
> + case HARDWARE_VERSION_EVT2:
> + buf = "EVT2";
> + break;
> + case HARDWARE_VERSION_DVT:
> + buf = "DVT";
> + break;
> + case HARDWARE_VERSION_PVT:
> + buf = "PVT";
> + break;
> + default:
> + buf = "Unknown";
> + break;
> + }
> +
> + seq_printf(s, "%s\n", buf);
> +
> + return 0;
> +}
> +
Please drop this '\n'.
> +DEFINE_SHOW_ATTRIBUTE(hardware_version);
> +
> +static int board_id_show(struct seq_file *s, void *data)
> +{
> + struct tn48m_data *priv = s->private;
> + unsigned int regval;
> + char *buf;
> +
> + regmap_read(priv->regmap, BOARD_ID, ®val);
> +
> + switch (regval) {
> + case BOARD_ID_TN48M:
> + buf = "TN48M";
> + break;
> + case BOARD_ID_TN48M_P:
> + buf = "TN48-P";
> + break;
> + default:
> + buf = "Unknown";
> + break;
> + }
> +
> + seq_printf(s, "%s\n", buf);
> +
> + return 0;
> +}
> +
Please drop this '\n'.
> +DEFINE_SHOW_ATTRIBUTE(board_id);
> +
> +static int code_version_show(struct seq_file *s, void *data)
> +{
> + struct tn48m_data *priv = s->private;
> + unsigned int regval;
> +
> + regmap_read(priv->regmap, CPLD_CODE_VERSION, ®val);
> +
> + seq_printf(s, "%d\n", regval);
> +
> + return 0;
> +}
> +
Please drop this '\n'.
> +DEFINE_SHOW_ATTRIBUTE(code_version);
> +
> +static void tn48m_init_debugfs(struct tn48m_data *data)
> +{
> + data->debugfs_dir = debugfs_create_dir(data->client->name, NULL);
> +
> + debugfs_create_file("hardware_version",
> + 0400,
> + data->debugfs_dir,
> + data,
> + &hardware_version_fops);
> +
> + debugfs_create_file("board_id",
> + 0400,
> + data->debugfs_dir,
> + data,
> + &board_id_fops);
> +
> + debugfs_create_file("code_version",
> + 0400,
> + data->debugfs_dir,
> + data,
> + &code_version_fops);
> +}
Does S/W actually do anything useful with these files?
Or are they just there for the sake of it?
If the latter, just print them to the kernel log and have done.
> +static int tn48m_probe(struct i2c_client *client)
> +{
> + struct tn48m_data *data;
'ddata' for both please.
> + int ret;
> +
> + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->client = client;
> + data->dev = &client->dev;
> + i2c_set_clientdata(client, data);
> +
> + data->regmap = devm_regmap_init_i2c(client, &tn48m_regmap_config);
> + if (IS_ERR(data->regmap)) {
> + dev_err(data->dev, "Failed to allocate regmap\n");
> + return PTR_ERR(data->regmap);
> + }
> +
> + ret = devm_mfd_add_devices(data->dev, PLATFORM_DEVID_AUTO, tn48m_cell,
> + ARRAY_SIZE(tn48m_cell), NULL, 0, NULL);
> + if (ret)
> + dev_err(data->dev, "Failed to register sub-devices %d\n", ret);
> +
> + tn48m_init_debugfs(data);
> +
> + return ret;
> +}
> +
> +static int tn48m_remove(struct i2c_client *client)
> +{
> + struct tn48m_data *data = i2c_get_clientdata(client);
> +
> + debugfs_remove_recursive(data->debugfs_dir);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id tn48m_of_match[] = {
> + { .compatible = "delta,tn48m-cpld"},
Missing ' ' before the '}'.
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, tn48m_of_match);
> +
> +static struct i2c_driver tn48m_driver = {
> + .driver = {
> + .name = "tn48m-cpld",
> + .of_match_table = tn48m_of_match,
> + },
> + .probe_new = tn48m_probe,
> + .remove = tn48m_remove,
> +};
> +module_i2c_driver(tn48m_driver);
> +
> +MODULE_AUTHOR("Robert Marko <[email protected]>");
> +MODULE_DESCRIPTION("Delta TN48M CPLD parent driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
> new file mode 100644
> index 000000000000..551c550efa54
> --- /dev/null
> +++ b/include/linux/mfd/tn48m.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Sartura Ltd
Out of date.
> + */
> +
> +#ifndef __TN48M_H__
> +#define __TN48M_H__
Better prefix with MFD.
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +
> +#define HARDWARE_VERSION_ID 0x0
> +#define HARDWARE_VERSION_MASK GENMASK(3, 0)
> +#define HARDWARE_VERSION_EVT1 0
> +#define HARDWARE_VERSION_EVT2 1
> +#define HARDWARE_VERSION_DVT 2
> +#define HARDWARE_VERSION_PVT 3
> +#define BOARD_ID 0x1
> +#define BOARD_ID_TN48M 0xa
> +#define BOARD_ID_TN48M_P 0xb
> +#define CPLD_CODE_VERSION 0x2
> +
> +struct tn48m_data {
> + struct device *dev;
> + struct regmap *regmap;
> + struct i2c_client *client;
You don't need both 'dev' and 'client'.
> + struct dentry *debugfs_dir;
> +};
> +
> +#endif
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Hi,
Am 2021-05-19 13:53, schrieb Robert Marko:
> On Thu, May 6, 2021 at 6:34 PM Michael Walle <[email protected]> wrote:
>> Am 2021-04-30 14:35, schrieb Robert Marko:
>> > Delta TN48M switches have a Lattice CPLD that serves
>> > multiple purposes including being a GPIO expander.
>> > So lets add the MFD core driver for it.
>>
>> Did you have a look at mfd/simple-mfd-i2c.c?
>
> Yes, that was my first idea but we have a requirement to expose CPLD
> information via debugfs as there are userspace applications using it.
> And simple-mfd-i2c does not allow us to do so.
Mh, last time Lee wasn't very fond of having a driver that just
populates
sub-drivers while doing almost nothing itself. See
https://lore.kernel.org/lkml/20200605065709.GD3714@dell/
That being said, I'd also like to expose our CPLD version, but until now
haven't found a good solution.
-michael
On Thu, May 6, 2021 at 6:34 PM Michael Walle <[email protected]> wrote:
>
> Hi Robert,
>
> Am 2021-04-30 14:35, schrieb Robert Marko:
> > Delta TN48M switches have a Lattice CPLD that serves
> > multiple purposes including being a GPIO expander.
> > So lets add the MFD core driver for it.
>
> Did you have a look at mfd/simple-mfd-i2c.c?
Hi Michael,
Yes, that was my first idea but we have a requirement to expose CPLD
information via debugfs as there are userspace applications using it.
And simple-mfd-i2c does not allow us to do so.
Regards,
Robert
>
> -michael
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Wed, 19 May 2021, Michael Walle wrote:
> Hi,
>
> Am 2021-05-19 13:53, schrieb Robert Marko:
> > On Thu, May 6, 2021 at 6:34 PM Michael Walle <[email protected]> wrote:
> > > Am 2021-04-30 14:35, schrieb Robert Marko:
> > > > Delta TN48M switches have a Lattice CPLD that serves
> > > > multiple purposes including being a GPIO expander.
> > > > So lets add the MFD core driver for it.
> > >
> > > Did you have a look at mfd/simple-mfd-i2c.c?
> >
> > Yes, that was my first idea but we have a requirement to expose CPLD
> > information via debugfs as there are userspace applications using it.
> > And simple-mfd-i2c does not allow us to do so.
>
> Mh, last time Lee wasn't very fond of having a driver that just populates
> sub-drivers while doing almost nothing itself. See
> https://lore.kernel.org/lkml/20200605065709.GD3714@dell/
Right. I still feel that way.
> That being said, I'd also like to expose our CPLD version, but until now
> haven't found a good solution.
Why though? Does S/W *need* it?
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, May 20, 2021 at 8:49 AM Lee Jones <[email protected]> wrote:
>
> On Wed, 19 May 2021, Michael Walle wrote:
>
> > Hi,
> >
> > Am 2021-05-19 13:53, schrieb Robert Marko:
> > > On Thu, May 6, 2021 at 6:34 PM Michael Walle <[email protected]> wrote:
> > > > Am 2021-04-30 14:35, schrieb Robert Marko:
> > > > > Delta TN48M switches have a Lattice CPLD that serves
> > > > > multiple purposes including being a GPIO expander.
> > > > > So lets add the MFD core driver for it.
> > > >
> > > > Did you have a look at mfd/simple-mfd-i2c.c?
> > >
> > > Yes, that was my first idea but we have a requirement to expose CPLD
> > > information via debugfs as there are userspace applications using it.
> > > And simple-mfd-i2c does not allow us to do so.
> >
> > Mh, last time Lee wasn't very fond of having a driver that just populates
> > sub-drivers while doing almost nothing itself. See
> > https://lore.kernel.org/lkml/20200605065709.GD3714@dell/
>
> Right. I still feel that way.
>
> > That being said, I'd also like to expose our CPLD version, but until now
> > haven't found a good solution.
>
> Why though? Does S/W *need* it?
Because we have userspace S/W that uses it as the same CPLD is in
multiple variants of the board but the correct board model is set during
manufacturing and we can read it from the CPLD.
We also have information about PSU1 and PSU2(Some models only)
power good, whether they are present and some other info that I need
to expose as these are monitored in userspace.
I planned to do that via the hwmon driver but according to Guenther they
are not hwmon attributes and I agree.
Would it be possible to have a dedicated driver that would only expose
the required information via debugfs?
Then I could simply use the simple I2C MFD driver with only a GPIO
driver on top of it.
Regards,
Robert
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Fri, May 21, 2021 at 11:04 AM Lee Jones <[email protected]> wrote:
>
> On Fri, 21 May 2021, Robert Marko wrote:
>
> > On Thu, May 20, 2021 at 8:49 AM Lee Jones <[email protected]> wrote:
> > >
> > > On Wed, 19 May 2021, Michael Walle wrote:
> > >
> > > > Hi,
> > > >
> > > > Am 2021-05-19 13:53, schrieb Robert Marko:
> > > > > On Thu, May 6, 2021 at 6:34 PM Michael Walle <[email protected]> wrote:
> > > > > > Am 2021-04-30 14:35, schrieb Robert Marko:
> > > > > > > Delta TN48M switches have a Lattice CPLD that serves
> > > > > > > multiple purposes including being a GPIO expander.
> > > > > > > So lets add the MFD core driver for it.
> > > > > >
> > > > > > Did you have a look at mfd/simple-mfd-i2c.c?
> > > > >
> > > > > Yes, that was my first idea but we have a requirement to expose CPLD
> > > > > information via debugfs as there are userspace applications using it.
> > > > > And simple-mfd-i2c does not allow us to do so.
> > > >
> > > > Mh, last time Lee wasn't very fond of having a driver that just populates
> > > > sub-drivers while doing almost nothing itself. See
> > > > https://lore.kernel.org/lkml/20200605065709.GD3714@dell/
> > >
> > > Right. I still feel that way.
> > >
> > > > That being said, I'd also like to expose our CPLD version, but until now
> > > > haven't found a good solution.
> > >
> > > Why though? Does S/W *need* it?
> > Because we have userspace S/W that uses it as the same CPLD is in
> > multiple variants of the board but the correct board model is set during
> > manufacturing and we can read it from the CPLD.
> >
> > We also have information about PSU1 and PSU2(Some models only)
> > power good, whether they are present and some other info that I need
> > to expose as these are monitored in userspace.
> >
> > I planned to do that via the hwmon driver but according to Guenther they
> > are not hwmon attributes and I agree.
> >
> > Would it be possible to have a dedicated driver that would only expose
> > the required information via debugfs?
> > Then I could simply use the simple I2C MFD driver with only a GPIO
> > driver on top of it.
>
> Yes, I was going to suggest that.
>
> It should probably live in drivers/misc.
OK, that works for me.
I will then rework this for v2.
Regards,
Robert
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Fri, 21 May 2021, Robert Marko wrote:
> On Thu, May 20, 2021 at 8:49 AM Lee Jones <[email protected]> wrote:
> >
> > On Wed, 19 May 2021, Michael Walle wrote:
> >
> > > Hi,
> > >
> > > Am 2021-05-19 13:53, schrieb Robert Marko:
> > > > On Thu, May 6, 2021 at 6:34 PM Michael Walle <[email protected]> wrote:
> > > > > Am 2021-04-30 14:35, schrieb Robert Marko:
> > > > > > Delta TN48M switches have a Lattice CPLD that serves
> > > > > > multiple purposes including being a GPIO expander.
> > > > > > So lets add the MFD core driver for it.
> > > > >
> > > > > Did you have a look at mfd/simple-mfd-i2c.c?
> > > >
> > > > Yes, that was my first idea but we have a requirement to expose CPLD
> > > > information via debugfs as there are userspace applications using it.
> > > > And simple-mfd-i2c does not allow us to do so.
> > >
> > > Mh, last time Lee wasn't very fond of having a driver that just populates
> > > sub-drivers while doing almost nothing itself. See
> > > https://lore.kernel.org/lkml/20200605065709.GD3714@dell/
> >
> > Right. I still feel that way.
> >
> > > That being said, I'd also like to expose our CPLD version, but until now
> > > haven't found a good solution.
> >
> > Why though? Does S/W *need* it?
> Because we have userspace S/W that uses it as the same CPLD is in
> multiple variants of the board but the correct board model is set during
> manufacturing and we can read it from the CPLD.
>
> We also have information about PSU1 and PSU2(Some models only)
> power good, whether they are present and some other info that I need
> to expose as these are monitored in userspace.
>
> I planned to do that via the hwmon driver but according to Guenther they
> are not hwmon attributes and I agree.
>
> Would it be possible to have a dedicated driver that would only expose
> the required information via debugfs?
> Then I could simply use the simple I2C MFD driver with only a GPIO
> driver on top of it.
Yes, I was going to suggest that.
It should probably live in drivers/misc.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, May 6, 2021 at 6:38 PM Michael Walle <[email protected]> wrote:
>
> Am 2021-04-30 14:35, schrieb 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.
> >
> > Since there is no logical GPIO numbering arbitrary one is used
> > along dt-bindings to make it humanly readable.
>
> Can gpio/gpio-regmap.c be used here? See gpio/gpio-sl28cpld.c
> for an example.
I suppose it can be used, I need to look into it more to see how could
translation to register and mask be done with it.
I have adapted the driver to work with simple I2C MFD so far.
It still leaves me with a finding the logical way to number the GPIO-s.
Robert
>
> -michael
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Thu, May 6, 2021 at 6:40 PM Michael Walle <[email protected]> wrote:
>
> Am 2021-05-06 16:00, schrieb Rob Herring:
> > On Fri, Apr 30, 2021 at 02:35:08PM +0200, Robert Marko wrote:
> >> Delta TN48M CPLD is used as a GPIO expander for the SFP GPIOs.
> >>
> >> It is a mix of input only and output only pins.
> >>
> >> Since there is no logical GPIO numbering arbitrary one is used
> >> along dt-bindings to make it humanly readable.
> >>
> >> Signed-off-by: Robert Marko <[email protected]>
> >> ---
> >> drivers/gpio/Kconfig | 12 +++
> >> drivers/gpio/Makefile | 1 +
> >> drivers/gpio/gpio-tn48m.c | 191
> >> ++++++++++++++++++++++++++++++++++++++
> >> drivers/mfd/tn48m-cpld.c | 6 +-
> >> include/linux/mfd/tn48m.h | 3 +
> >> 5 files changed, 212 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/gpio/gpio-tn48m.c
> >
> >
> >> +static const struct platform_device_id tn48m_gpio_id_table[] = {
> >> + { "delta,tn48m-gpio", },
> >
> > Looks like a compatible, but is not. I think you can drop this and just
> > use 'tm48m-gpio' (the driver name).
>
> I'm just curious, why isn't the vendor included here (as there
> might be a chance for name clashes in the future).
It's my oversight, I have converted it to use simple I2C MFD so its
OF based now.
I will update the driver's name with the vendor's name.
Robert
>
> -michael
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr