linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
@ 2021-04-30 12:35 Robert Marko
  2021-04-30 12:35 ` [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings Robert Marko
                   ` (7 more replies)
  0 siblings, 8 replies; 26+ messages in thread
From: Robert Marko @ 2021-04-30 12:35 UTC (permalink / raw)
  To: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek, 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.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 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 <robert.marko@sartura.hr>
+ */
+
+#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, &regval);
+
+	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, &regval);
+
+	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, &regval);
+
+	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 <robert.marko@sartura.hr>");
+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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
@ 2021-04-30 12:35 ` Robert Marko
  2021-05-06 13:57   ` Rob Herring
  2021-04-30 12:35 ` [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver Robert Marko
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Robert Marko @ 2021-04-30 12:35 UTC (permalink / raw)
  To: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek, Robert Marko

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 <robert.marko@sartura.hr>
---
 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 <robert.marko@sartura.hr>
+ */
+
+#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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
  2021-04-30 12:35 ` [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings Robert Marko
@ 2021-04-30 12:35 ` Robert Marko
  2021-05-06 14:00   ` Rob Herring
  2021-05-06 16:38   ` Michael Walle
  2021-04-30 12:35 ` [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver Robert Marko
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 26+ messages in thread
From: Robert Marko @ 2021-04-30 12:35 UTC (permalink / raw)
  To: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek, 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.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 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 <robert.marko@sartura.hr>
+ */
+
+#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),
+			  &regval);
+	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 <robert.marko@sartura.hr>");
+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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
  2021-04-30 12:35 ` [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings Robert Marko
  2021-04-30 12:35 ` [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver Robert Marko
@ 2021-04-30 12:35 ` Robert Marko
  2021-04-30 13:12   ` Guenter Roeck
  2021-04-30 12:35 ` [PATCH 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings Robert Marko
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Robert Marko @ 2021-04-30 12:35 UTC (permalink / raw)
  To: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek, Robert Marko

Delta TN48M has a CPLD that also monitors the power supply
statuses.

These are useful to be presented to the users, so lets
add a driver for HWMON part of the CPLD.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/hwmon/Kconfig       |  10 +++
 drivers/hwmon/Makefile      |   1 +
 drivers/hwmon/tn48m-hwmon.c | 148 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/tn48m-cpld.c    |   3 +
 include/linux/mfd/tn48m.h   |   1 +
 5 files changed, 163 insertions(+)
 create mode 100644 drivers/hwmon/tn48m-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 54f04e61fb83..89271dfeb775 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1924,6 +1924,16 @@ config SENSORS_TMP513
 	  This driver can also be built as a module. If so, the module
 	  will be called tmp513.
 
+config SENSORS_TN48M
+	tristate "Delta Networks TN48M switch CPLD HWMON driver"
+	depends on MFD_TN48M_CPLD
+	help
+	  If you say yes here you get support for Delta Networks TN48M
+	  CPLD.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called tn48m-hwmon.
+
 config SENSORS_VEXPRESS
 	tristate "Versatile Express"
 	depends on VEXPRESS_CONFIG
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index fe38e8a5c979..22e470057ffe 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -187,6 +187,7 @@ obj-$(CONFIG_SENSORS_TMP108)	+= tmp108.o
 obj-$(CONFIG_SENSORS_TMP401)	+= tmp401.o
 obj-$(CONFIG_SENSORS_TMP421)	+= tmp421.o
 obj-$(CONFIG_SENSORS_TMP513)	+= tmp513.o
+obj-$(CONFIG_SENSORS_TN48M)	+= tn48m-hwmon.o
 obj-$(CONFIG_SENSORS_VEXPRESS)	+= vexpress-hwmon.o
 obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
 obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
diff --git a/drivers/hwmon/tn48m-hwmon.c b/drivers/hwmon/tn48m-hwmon.c
new file mode 100644
index 000000000000..80fd18d74f1d
--- /dev/null
+++ b/drivers/hwmon/tn48m-hwmon.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Delta TN48M CPLD HWMON driver
+ *
+ * Copyright 2020 Sartura Ltd
+ *
+ * Author: Robert Marko <robert.marko@sartura.hr>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/tn48m.h>
+
+#define PSU1_PRESENT_MASK	BIT(0)
+#define PSU2_PRESENT_MASK	BIT(1)
+#define PSU1_POWERGOOD_MASK	BIT(2)
+#define PSU2_POWERGOOD_MASK	BIT(3)
+#define PSU1_ALERT_MASK		BIT(4)
+#define PSU2_ALERT_MASK		BIT(5)
+
+static int board_id_get(struct tn48m_data *data)
+{
+	unsigned int regval;
+
+	regmap_read(data->regmap, BOARD_ID, &regval);
+
+	switch (regval) {
+	case BOARD_ID_TN48M:
+		return BOARD_ID_TN48M;
+	case BOARD_ID_TN48M_P:
+		return BOARD_ID_TN48M_P;
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t psu_present_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
+	struct tn48m_data *data = dev_get_drvdata(dev);
+	unsigned int regval, status;
+
+	if (board_id_get(data) == BOARD_ID_TN48M_P) {
+		regmap_read(data->regmap, attr2->nr, &regval);
+
+		if (attr2->index == 1)
+			status = !FIELD_GET(PSU1_PRESENT_MASK, regval);
+		else
+			status = !FIELD_GET(PSU2_PRESENT_MASK, regval);
+	} else
+		status = 0;
+
+	return sprintf(buf, "%d\n", status);
+}
+
+static ssize_t psu_pg_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
+	struct tn48m_data *data = dev_get_drvdata(dev);
+	unsigned int regval, status;
+
+	regmap_read(data->regmap, attr2->nr, &regval);
+
+	if (attr2->index == 1)
+		status = FIELD_GET(PSU1_POWERGOOD_MASK, regval);
+	else
+		status = FIELD_GET(PSU2_POWERGOOD_MASK, regval);
+
+	return sprintf(buf, "%d\n", status);
+}
+
+static ssize_t psu_alert_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
+	struct tn48m_data *data = dev_get_drvdata(dev);
+	unsigned int regval, status;
+
+	if (board_id_get(data) == BOARD_ID_TN48M_P) {
+		regmap_read(data->regmap, attr2->nr, &regval);
+
+		if (attr2->index == 1)
+			status = !FIELD_GET(PSU1_ALERT_MASK, regval);
+		else
+			status = !FIELD_GET(PSU2_ALERT_MASK, regval);
+	} else
+		status = 0;
+
+	return sprintf(buf, "%d\n", status);
+}
+
+static SENSOR_DEVICE_ATTR_2_RO(psu1_present, psu_present, PSU_STATUS, 1);
+static SENSOR_DEVICE_ATTR_2_RO(psu2_present, psu_present, PSU_STATUS, 2);
+static SENSOR_DEVICE_ATTR_2_RO(psu1_pg, psu_pg, PSU_STATUS, 1);
+static SENSOR_DEVICE_ATTR_2_RO(psu2_pg, psu_pg, PSU_STATUS, 2);
+static SENSOR_DEVICE_ATTR_2_RO(psu1_alert, psu_alert, PSU_STATUS, 1);
+static SENSOR_DEVICE_ATTR_2_RO(psu2_alert, psu_alert, PSU_STATUS, 2);
+
+static struct attribute *tn48m_hwmon_attrs[] = {
+	&sensor_dev_attr_psu1_present.dev_attr.attr,
+	&sensor_dev_attr_psu2_present.dev_attr.attr,
+	&sensor_dev_attr_psu1_pg.dev_attr.attr,
+	&sensor_dev_attr_psu2_pg.dev_attr.attr,
+	&sensor_dev_attr_psu1_alert.dev_attr.attr,
+	&sensor_dev_attr_psu2_alert.dev_attr.attr,
+	NULL
+};
+
+ATTRIBUTE_GROUPS(tn48m_hwmon);
+
+static int tn48m_hwmon_probe(struct platform_device *pdev)
+{
+	struct tn48m_data *data = dev_get_drvdata(pdev->dev.parent);
+	struct device *hwmon_dev;
+
+	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
+							   "tn48m_hwmon",
+							   data,
+							   tn48m_hwmon_groups);
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct platform_device_id tn48m_hwmon_id_table[] = {
+	{ "delta,tn48m-hwmon", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, tn48m_hwmon_id_table);
+
+static struct platform_driver tn48m_hwmon_driver = {
+	.driver = {
+		.name = "tn48m-hwmon",
+	},
+	.probe = tn48m_hwmon_probe,
+	.id_table = tn48m_hwmon_id_table,
+};
+module_platform_driver(tn48m_hwmon_driver);
+
+MODULE_AUTHOR("Robert Marko <robert.marko@sartura.hr>");
+MODULE_DESCRIPTION("Delta TN48M CPLD HWMON driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
index f22a15ddd22d..4d837aca01e7 100644
--- a/drivers/mfd/tn48m-cpld.c
+++ b/drivers/mfd/tn48m-cpld.c
@@ -20,6 +20,9 @@
 static const struct mfd_cell tn48m_cell[] = {
 	{
 		.name = "delta,tn48m-gpio",
+	},
+	{
+		.name = "delta,tn48m-hwmon",
 	}
 };
 
diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
index 9cc2b04c8d69..eb2cfc3a5db7 100644
--- a/include/linux/mfd/tn48m.h
+++ b/include/linux/mfd/tn48m.h
@@ -22,6 +22,7 @@
 #define SFP_TX_DISABLE		0x31
 #define SFP_PRESENT		0x3a
 #define SFP_LOS			0x40
+#define PSU_STATUS		0xa
 
 struct tn48m_data {
 	struct device *dev;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
                   ` (2 preceding siblings ...)
  2021-04-30 12:35 ` [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver Robert Marko
@ 2021-04-30 12:35 ` Robert Marko
  2021-05-06 14:01   ` Rob Herring
  2021-04-30 12:35 ` [PATCH 6/6] MAINTAINERS: Add Delta Networks TN48M CPLD drivers Robert Marko
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Robert Marko @ 2021-04-30 12:35 UTC (permalink / raw)
  To: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek, Robert Marko

Add binding document for the Delta TN48M CPLD drivers.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 .../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 <robert.marko@sartura.hr>
+
+description: |
+  Lattice CPLD onboard the TN48M switches is used for system
+  management.
+
+  It provides information about the hardware model, revision,
+  PSU status etc.
+
+  It is also being used as a GPIO expander for the SFP slots.
+
+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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 6/6] MAINTAINERS: Add Delta Networks TN48M CPLD drivers
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
                   ` (3 preceding siblings ...)
  2021-04-30 12:35 ` [PATCH 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings Robert Marko
@ 2021-04-30 12:35 ` Robert Marko
  2021-05-06 16:34 ` [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Michael Walle
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 26+ messages in thread
From: Robert Marko @ 2021-04-30 12:35 UTC (permalink / raw)
  To: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek, Robert Marko

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

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 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 <robert.marko@sartura.hr>
+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:	linux-mtd@lists.infradead.org
 S:	Orphan
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver
  2021-04-30 12:35 ` [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver Robert Marko
@ 2021-04-30 13:12   ` Guenter Roeck
  2021-05-19 12:01     ` Robert Marko
  0 siblings, 1 reply; 26+ messages in thread
From: Guenter Roeck @ 2021-04-30 13:12 UTC (permalink / raw)
  To: Robert Marko, lee.jones, robh+dt, linus.walleij, bgolaszewski,
	jdelvare, devicetree, linux-kernel, linux-gpio, linux-hwmon
  Cc: luka.perkov, jmp, pmenzel, buczek

On 4/30/21 5:35 AM, Robert Marko wrote:
> Delta TN48M has a CPLD that also monitors the power supply
> statuses.
> 
> These are useful to be presented to the users, so lets
> add a driver for HWMON part of the CPLD.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  drivers/hwmon/Kconfig       |  10 +++
>  drivers/hwmon/Makefile      |   1 +
>  drivers/hwmon/tn48m-hwmon.c | 148 ++++++++++++++++++++++++++++++++++++
>  drivers/mfd/tn48m-cpld.c    |   3 +
>  include/linux/mfd/tn48m.h   |   1 +
>  5 files changed, 163 insertions(+)
>  create mode 100644 drivers/hwmon/tn48m-hwmon.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 54f04e61fb83..89271dfeb775 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1924,6 +1924,16 @@ config SENSORS_TMP513
>  	  This driver can also be built as a module. If so, the module
>  	  will be called tmp513.
>  
> +config SENSORS_TN48M
> +	tristate "Delta Networks TN48M switch CPLD HWMON driver"
> +	depends on MFD_TN48M_CPLD
> +	help
> +	  If you say yes here you get support for Delta Networks TN48M
> +	  CPLD.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called tn48m-hwmon.
> +
>  config SENSORS_VEXPRESS
>  	tristate "Versatile Express"
>  	depends on VEXPRESS_CONFIG
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index fe38e8a5c979..22e470057ffe 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -187,6 +187,7 @@ obj-$(CONFIG_SENSORS_TMP108)	+= tmp108.o
>  obj-$(CONFIG_SENSORS_TMP401)	+= tmp401.o
>  obj-$(CONFIG_SENSORS_TMP421)	+= tmp421.o
>  obj-$(CONFIG_SENSORS_TMP513)	+= tmp513.o
> +obj-$(CONFIG_SENSORS_TN48M)	+= tn48m-hwmon.o
>  obj-$(CONFIG_SENSORS_VEXPRESS)	+= vexpress-hwmon.o
>  obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
>  obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
> diff --git a/drivers/hwmon/tn48m-hwmon.c b/drivers/hwmon/tn48m-hwmon.c
> new file mode 100644
> index 000000000000..80fd18d74f1d
> --- /dev/null
> +++ b/drivers/hwmon/tn48m-hwmon.c
> @@ -0,0 +1,148 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Delta TN48M CPLD HWMON driver
> + *
> + * Copyright 2020 Sartura Ltd
> + *
> + * Author: Robert Marko <robert.marko@sartura.hr>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/hwmon.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/mfd/tn48m.h>
> +
> +#define PSU1_PRESENT_MASK	BIT(0)
> +#define PSU2_PRESENT_MASK	BIT(1)
> +#define PSU1_POWERGOOD_MASK	BIT(2)
> +#define PSU2_POWERGOOD_MASK	BIT(3)
> +#define PSU1_ALERT_MASK		BIT(4)
> +#define PSU2_ALERT_MASK		BIT(5)
> +
> +static int board_id_get(struct tn48m_data *data)
> +{
> +	unsigned int regval;
> +
> +	regmap_read(data->regmap, BOARD_ID, &regval);
> +
> +	switch (regval) {
> +	case BOARD_ID_TN48M:
> +		return BOARD_ID_TN48M;
> +	case BOARD_ID_TN48M_P:
> +		return BOARD_ID_TN48M_P;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static ssize_t psu_present_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
> +	struct tn48m_data *data = dev_get_drvdata(dev);
> +	unsigned int regval, status;
> +
> +	if (board_id_get(data) == BOARD_ID_TN48M_P) {
> +		regmap_read(data->regmap, attr2->nr, &regval);
> +
> +		if (attr2->index == 1)
> +			status = !FIELD_GET(PSU1_PRESENT_MASK, regval);
> +		else
> +			status = !FIELD_GET(PSU2_PRESENT_MASK, regval);
> +	} else
> +		status = 0;
> +
> +	return sprintf(buf, "%d\n", status);
> +}
> +
> +static ssize_t psu_pg_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
> +	struct tn48m_data *data = dev_get_drvdata(dev);
> +	unsigned int regval, status;
> +
> +	regmap_read(data->regmap, attr2->nr, &regval);
> +
> +	if (attr2->index == 1)
> +		status = FIELD_GET(PSU1_POWERGOOD_MASK, regval);
> +	else
> +		status = FIELD_GET(PSU2_POWERGOOD_MASK, regval);
> +
> +	return sprintf(buf, "%d\n", status);
> +}
> +
> +static ssize_t psu_alert_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
> +	struct tn48m_data *data = dev_get_drvdata(dev);
> +	unsigned int regval, status;
> +
> +	if (board_id_get(data) == BOARD_ID_TN48M_P) {
> +		regmap_read(data->regmap, attr2->nr, &regval);
> +
> +		if (attr2->index == 1)
> +			status = !FIELD_GET(PSU1_ALERT_MASK, regval);
> +		else
> +			status = !FIELD_GET(PSU2_ALERT_MASK, regval);
> +	} else
> +		status = 0;
> +
> +	return sprintf(buf, "%d\n", status);
> +}
> +
> +static SENSOR_DEVICE_ATTR_2_RO(psu1_present, psu_present, PSU_STATUS, 1);
> +static SENSOR_DEVICE_ATTR_2_RO(psu2_present, psu_present, PSU_STATUS, 2);
> +static SENSOR_DEVICE_ATTR_2_RO(psu1_pg, psu_pg, PSU_STATUS, 1);
> +static SENSOR_DEVICE_ATTR_2_RO(psu2_pg, psu_pg, PSU_STATUS, 2);
> +static SENSOR_DEVICE_ATTR_2_RO(psu1_alert, psu_alert, PSU_STATUS, 1);
> +static SENSOR_DEVICE_ATTR_2_RO(psu2_alert, psu_alert, PSU_STATUS, 2);
> +
> +static struct attribute *tn48m_hwmon_attrs[] = {
> +	&sensor_dev_attr_psu1_present.dev_attr.attr,
> +	&sensor_dev_attr_psu2_present.dev_attr.attr,
> +	&sensor_dev_attr_psu1_pg.dev_attr.attr,
> +	&sensor_dev_attr_psu2_pg.dev_attr.attr,
> +	&sensor_dev_attr_psu1_alert.dev_attr.attr,
> +	&sensor_dev_attr_psu2_alert.dev_attr.attr,

Literally none of those attributes are standard hwmon attributes.
I don't know what this is, but it is not a hardware monitoring driver.

> +	NULL
> +};
> +
> +ATTRIBUTE_GROUPS(tn48m_hwmon);
> +
> +static int tn48m_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct tn48m_data *data = dev_get_drvdata(pdev->dev.parent);
> +	struct device *hwmon_dev;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
> +							   "tn48m_hwmon",
> +							   data,
> +							   tn48m_hwmon_groups);

Please use devm_hwmon_device_register_with_info() to register hwmon devices.
Of course, that only makes sense for actual hardware monitoring drivers
which do support standard attributes.

> +	return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct platform_device_id tn48m_hwmon_id_table[] = {
> +	{ "delta,tn48m-hwmon", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, tn48m_hwmon_id_table);
> +
> +static struct platform_driver tn48m_hwmon_driver = {
> +	.driver = {
> +		.name = "tn48m-hwmon",
> +	},
> +	.probe = tn48m_hwmon_probe,
> +	.id_table = tn48m_hwmon_id_table,
> +};
> +module_platform_driver(tn48m_hwmon_driver);
> +
> +MODULE_AUTHOR("Robert Marko <robert.marko@sartura.hr>");
> +MODULE_DESCRIPTION("Delta TN48M CPLD HWMON driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
> index f22a15ddd22d..4d837aca01e7 100644
> --- a/drivers/mfd/tn48m-cpld.c
> +++ b/drivers/mfd/tn48m-cpld.c
> @@ -20,6 +20,9 @@
>  static const struct mfd_cell tn48m_cell[] = {
>  	{
>  		.name = "delta,tn48m-gpio",
> +	},
> +	{
> +		.name = "delta,tn48m-hwmon",
>  	}
>  };
>  
> diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
> index 9cc2b04c8d69..eb2cfc3a5db7 100644
> --- a/include/linux/mfd/tn48m.h
> +++ b/include/linux/mfd/tn48m.h
> @@ -22,6 +22,7 @@
>  #define SFP_TX_DISABLE		0x31
>  #define SFP_PRESENT		0x3a
>  #define SFP_LOS			0x40
> +#define PSU_STATUS		0xa
>  
>  struct tn48m_data {
>  	struct device *dev;
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings
  2021-04-30 12:35 ` [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings Robert Marko
@ 2021-05-06 13:57   ` Rob Herring
  0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2021-05-06 13:57 UTC (permalink / raw)
  To: Robert Marko
  Cc: lee.jones, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, luka.perkov,
	jmp, pmenzel, buczek

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 <robert.marko@sartura.hr>
> ---
>  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 <robert.marko@sartura.hr>
> + */
> +
> +#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
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-04-30 12:35 ` [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver Robert Marko
@ 2021-05-06 14:00   ` Rob Herring
  2021-05-06 16:40     ` Michael Walle
  2021-05-06 16:38   ` Michael Walle
  1 sibling, 1 reply; 26+ messages in thread
From: Rob Herring @ 2021-05-06 14:00 UTC (permalink / raw)
  To: Robert Marko
  Cc: lee.jones, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, luka.perkov,
	jmp, pmenzel, buczek

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 <robert.marko@sartura.hr>
> ---
>  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);

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings
  2021-04-30 12:35 ` [PATCH 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings Robert Marko
@ 2021-05-06 14:01   ` Rob Herring
  0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2021-05-06 14:01 UTC (permalink / raw)
  To: Robert Marko
  Cc: linux-gpio, bgolaszewski, linux-hwmon, jmp, linus.walleij,
	linux-kernel, luka.perkov, linux, lee.jones, buczek, jdelvare,
	pmenzel, devicetree, robh+dt

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 <robert.marko@sartura.hr>
> ---
>  .../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 <robh@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
                   ` (4 preceding siblings ...)
  2021-04-30 12:35 ` [PATCH 6/6] MAINTAINERS: Add Delta Networks TN48M CPLD drivers Robert Marko
@ 2021-05-06 16:34 ` Michael Walle
  2021-05-19 11:53   ` Robert Marko
  2021-05-06 16:50 ` kernel test robot
  2021-05-19 11:11 ` Lee Jones
  7 siblings, 1 reply; 26+ messages in thread
From: Michael Walle @ 2021-05-06 16:34 UTC (permalink / raw)
  To: Robert Marko
  Cc: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, luka.perkov,
	jmp, pmenzel, buczek

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-04-30 12:35 ` [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver Robert Marko
  2021-05-06 14:00   ` Rob Herring
@ 2021-05-06 16:38   ` Michael Walle
  2021-05-21 13:21     ` Robert Marko
  1 sibling, 1 reply; 26+ messages in thread
From: Michael Walle @ 2021-05-06 16:38 UTC (permalink / raw)
  To: Robert Marko
  Cc: lee.jones, robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, luka.perkov,
	jmp, pmenzel, buczek

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-05-06 14:00   ` Rob Herring
@ 2021-05-06 16:40     ` Michael Walle
  2021-05-06 17:21       ` Luka Perkov
  2021-05-21 13:22       ` Robert Marko
  0 siblings, 2 replies; 26+ messages in thread
From: Michael Walle @ 2021-05-06 16:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Robert Marko, lee.jones, linus.walleij, bgolaszewski, jdelvare,
	linux, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	luka.perkov, jmp, pmenzel, buczek

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 <robert.marko@sartura.hr>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
                   ` (5 preceding siblings ...)
  2021-05-06 16:34 ` [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Michael Walle
@ 2021-05-06 16:50 ` kernel test robot
  2021-05-19 11:11 ` Lee Jones
  7 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-05-06 16:50 UTC (permalink / raw)
  To: Robert Marko, lee.jones, robh+dt, linus.walleij, bgolaszewski,
	jdelvare, linux, devicetree, linux-kernel, linux-gpio,
	linux-hwmon
  Cc: kbuild-all

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

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 <lkp@intel.com>

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, &regval);
    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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64733 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-05-06 16:40     ` Michael Walle
@ 2021-05-06 17:21       ` Luka Perkov
  2021-05-21 13:22       ` Robert Marko
  1 sibling, 0 replies; 26+ messages in thread
From: Luka Perkov @ 2021-05-06 17:21 UTC (permalink / raw)
  To: Michael Walle
  Cc: Rob Herring, Robert Marko, lee.jones, linus.walleij,
	bgolaszewski, jdelvare, linux, devicetree, linux-kernel,
	linux-gpio, linux-hwmon, Jonathan Polom, Paul Menzel,
	Donald Buczek, CLEMENT.CHANG 張弘慶,
	upstream-wg

Hello Michael,

On Thu, May 6, 2021 at 6:40 PM Michael Walle <michael@walle.cc> 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 <robert.marko@sartura.hr>
> >> ---
> >>  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/

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
                   ` (6 preceding siblings ...)
  2021-05-06 16:50 ` kernel test robot
@ 2021-05-19 11:11 ` Lee Jones
  7 siblings, 0 replies; 26+ messages in thread
From: Lee Jones @ 2021-05-19 11:11 UTC (permalink / raw)
  To: Robert Marko
  Cc: robh+dt, linus.walleij, bgolaszewski, jdelvare, linux,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, luka.perkov,
	jmp, pmenzel, buczek

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 <robert.marko@sartura.hr>
> ---
>  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 <robert.marko@sartura.hr>
> + */
> +
> +#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, &regval);
> +
> +	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, &regval);
> +
> +	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, &regval);
> +
> +	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 <robert.marko@sartura.hr>");
> +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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-05-06 16:34 ` [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Michael Walle
@ 2021-05-19 11:53   ` Robert Marko
  2021-05-19 19:42     ` Michael Walle
  0 siblings, 1 reply; 26+ messages in thread
From: Robert Marko @ 2021-05-19 11:53 UTC (permalink / raw)
  To: Michael Walle
  Cc: Lee Jones, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

On Thu, May 6, 2021 at 6:34 PM Michael Walle <michael@walle.cc> 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: robert.marko@sartura.hr
Web: www.sartura.hr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver
  2021-04-30 13:12   ` Guenter Roeck
@ 2021-05-19 12:01     ` Robert Marko
  2021-05-19 13:40       ` Guenter Roeck
  0 siblings, 1 reply; 26+ messages in thread
From: Robert Marko @ 2021-05-19 12:01 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Lee Jones, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, Luka Perkov,
	jmp, Paul Menzel, Donald Buczek

On Fri, Apr 30, 2021 at 3:12 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 4/30/21 5:35 AM, Robert Marko wrote:
> > Delta TN48M has a CPLD that also monitors the power supply
> > statuses.
> >
> > These are useful to be presented to the users, so lets
> > add a driver for HWMON part of the CPLD.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> >  drivers/hwmon/Kconfig       |  10 +++
> >  drivers/hwmon/Makefile      |   1 +
> >  drivers/hwmon/tn48m-hwmon.c | 148 ++++++++++++++++++++++++++++++++++++
> >  drivers/mfd/tn48m-cpld.c    |   3 +
> >  include/linux/mfd/tn48m.h   |   1 +
> >  5 files changed, 163 insertions(+)
> >  create mode 100644 drivers/hwmon/tn48m-hwmon.c
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 54f04e61fb83..89271dfeb775 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -1924,6 +1924,16 @@ config SENSORS_TMP513
> >         This driver can also be built as a module. If so, the module
> >         will be called tmp513.
> >
> > +config SENSORS_TN48M
> > +     tristate "Delta Networks TN48M switch CPLD HWMON driver"
> > +     depends on MFD_TN48M_CPLD
> > +     help
> > +       If you say yes here you get support for Delta Networks TN48M
> > +       CPLD.
> > +
> > +       This driver can also be built as a module. If so, the module
> > +       will be called tn48m-hwmon.
> > +
> >  config SENSORS_VEXPRESS
> >       tristate "Versatile Express"
> >       depends on VEXPRESS_CONFIG
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index fe38e8a5c979..22e470057ffe 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -187,6 +187,7 @@ obj-$(CONFIG_SENSORS_TMP108)      += tmp108.o
> >  obj-$(CONFIG_SENSORS_TMP401) += tmp401.o
> >  obj-$(CONFIG_SENSORS_TMP421) += tmp421.o
> >  obj-$(CONFIG_SENSORS_TMP513) += tmp513.o
> > +obj-$(CONFIG_SENSORS_TN48M)  += tn48m-hwmon.o
> >  obj-$(CONFIG_SENSORS_VEXPRESS)       += vexpress-hwmon.o
> >  obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
> >  obj-$(CONFIG_SENSORS_VIA686A)        += via686a.o
> > diff --git a/drivers/hwmon/tn48m-hwmon.c b/drivers/hwmon/tn48m-hwmon.c
> > new file mode 100644
> > index 000000000000..80fd18d74f1d
> > --- /dev/null
> > +++ b/drivers/hwmon/tn48m-hwmon.c
> > @@ -0,0 +1,148 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Delta TN48M CPLD HWMON driver
> > + *
> > + * Copyright 2020 Sartura Ltd
> > + *
> > + * Author: Robert Marko <robert.marko@sartura.hr>
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <linux/mfd/tn48m.h>
> > +
> > +#define PSU1_PRESENT_MASK    BIT(0)
> > +#define PSU2_PRESENT_MASK    BIT(1)
> > +#define PSU1_POWERGOOD_MASK  BIT(2)
> > +#define PSU2_POWERGOOD_MASK  BIT(3)
> > +#define PSU1_ALERT_MASK              BIT(4)
> > +#define PSU2_ALERT_MASK              BIT(5)
> > +
> > +static int board_id_get(struct tn48m_data *data)
> > +{
> > +     unsigned int regval;
> > +
> > +     regmap_read(data->regmap, BOARD_ID, &regval);
> > +
> > +     switch (regval) {
> > +     case BOARD_ID_TN48M:
> > +             return BOARD_ID_TN48M;
> > +     case BOARD_ID_TN48M_P:
> > +             return BOARD_ID_TN48M_P;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> > +static ssize_t psu_present_show(struct device *dev,
> > +                             struct device_attribute *attr, char *buf)
> > +{
> > +     struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
> > +     struct tn48m_data *data = dev_get_drvdata(dev);
> > +     unsigned int regval, status;
> > +
> > +     if (board_id_get(data) == BOARD_ID_TN48M_P) {
> > +             regmap_read(data->regmap, attr2->nr, &regval);
> > +
> > +             if (attr2->index == 1)
> > +                     status = !FIELD_GET(PSU1_PRESENT_MASK, regval);
> > +             else
> > +                     status = !FIELD_GET(PSU2_PRESENT_MASK, regval);
> > +     } else
> > +             status = 0;
> > +
> > +     return sprintf(buf, "%d\n", status);
> > +}
> > +
> > +static ssize_t psu_pg_show(struct device *dev,
> > +                        struct device_attribute *attr, char *buf)
> > +{
> > +     struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
> > +     struct tn48m_data *data = dev_get_drvdata(dev);
> > +     unsigned int regval, status;
> > +
> > +     regmap_read(data->regmap, attr2->nr, &regval);
> > +
> > +     if (attr2->index == 1)
> > +             status = FIELD_GET(PSU1_POWERGOOD_MASK, regval);
> > +     else
> > +             status = FIELD_GET(PSU2_POWERGOOD_MASK, regval);
> > +
> > +     return sprintf(buf, "%d\n", status);
> > +}
> > +
> > +static ssize_t psu_alert_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +     struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
> > +     struct tn48m_data *data = dev_get_drvdata(dev);
> > +     unsigned int regval, status;
> > +
> > +     if (board_id_get(data) == BOARD_ID_TN48M_P) {
> > +             regmap_read(data->regmap, attr2->nr, &regval);
> > +
> > +             if (attr2->index == 1)
> > +                     status = !FIELD_GET(PSU1_ALERT_MASK, regval);
> > +             else
> > +                     status = !FIELD_GET(PSU2_ALERT_MASK, regval);
> > +     } else
> > +             status = 0;
> > +
> > +     return sprintf(buf, "%d\n", status);
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR_2_RO(psu1_present, psu_present, PSU_STATUS, 1);
> > +static SENSOR_DEVICE_ATTR_2_RO(psu2_present, psu_present, PSU_STATUS, 2);
> > +static SENSOR_DEVICE_ATTR_2_RO(psu1_pg, psu_pg, PSU_STATUS, 1);
> > +static SENSOR_DEVICE_ATTR_2_RO(psu2_pg, psu_pg, PSU_STATUS, 2);
> > +static SENSOR_DEVICE_ATTR_2_RO(psu1_alert, psu_alert, PSU_STATUS, 1);
> > +static SENSOR_DEVICE_ATTR_2_RO(psu2_alert, psu_alert, PSU_STATUS, 2);
> > +
> > +static struct attribute *tn48m_hwmon_attrs[] = {
> > +     &sensor_dev_attr_psu1_present.dev_attr.attr,
> > +     &sensor_dev_attr_psu2_present.dev_attr.attr,
> > +     &sensor_dev_attr_psu1_pg.dev_attr.attr,
> > +     &sensor_dev_attr_psu2_pg.dev_attr.attr,
> > +     &sensor_dev_attr_psu1_alert.dev_attr.attr,
> > +     &sensor_dev_attr_psu2_alert.dev_attr.attr,
>
> Literally none of those attributes are standard hwmon attributes.
> I don't know what this is, but it is not a hardware monitoring driver.

Yes, I agree that it does not expose any of the standard attributes, but these
are the only ones the CPLD exposes.

I don't know where else to put them, MFD driver did not seem logical to me.
>
> > +     NULL
> > +};
> > +
> > +ATTRIBUTE_GROUPS(tn48m_hwmon);
> > +
> > +static int tn48m_hwmon_probe(struct platform_device *pdev)
> > +{
> > +     struct tn48m_data *data = dev_get_drvdata(pdev->dev.parent);
> > +     struct device *hwmon_dev;
> > +
> > +     hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
> > +                                                        "tn48m_hwmon",
> > +                                                        data,
> > +                                                        tn48m_hwmon_groups);
>
> Please use devm_hwmon_device_register_with_info() to register hwmon devices.
> Of course, that only makes sense for actual hardware monitoring drivers
> which do support standard attributes.

Yes, devm_hwmon_device_register_with_info() made no sense without any of the
standard attributes.

Robert
>
> > +     return PTR_ERR_OR_ZERO(hwmon_dev);
> > +}
> > +
> > +static const struct platform_device_id tn48m_hwmon_id_table[] = {
> > +     { "delta,tn48m-hwmon", },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(platform, tn48m_hwmon_id_table);
> > +
> > +static struct platform_driver tn48m_hwmon_driver = {
> > +     .driver = {
> > +             .name = "tn48m-hwmon",
> > +     },
> > +     .probe = tn48m_hwmon_probe,
> > +     .id_table = tn48m_hwmon_id_table,
> > +};
> > +module_platform_driver(tn48m_hwmon_driver);
> > +
> > +MODULE_AUTHOR("Robert Marko <robert.marko@sartura.hr>");
> > +MODULE_DESCRIPTION("Delta TN48M CPLD HWMON driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
> > index f22a15ddd22d..4d837aca01e7 100644
> > --- a/drivers/mfd/tn48m-cpld.c
> > +++ b/drivers/mfd/tn48m-cpld.c
> > @@ -20,6 +20,9 @@
> >  static const struct mfd_cell tn48m_cell[] = {
> >       {
> >               .name = "delta,tn48m-gpio",
> > +     },
> > +     {
> > +             .name = "delta,tn48m-hwmon",
> >       }
> >  };
> >
> > diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
> > index 9cc2b04c8d69..eb2cfc3a5db7 100644
> > --- a/include/linux/mfd/tn48m.h
> > +++ b/include/linux/mfd/tn48m.h
> > @@ -22,6 +22,7 @@
> >  #define SFP_TX_DISABLE               0x31
> >  #define SFP_PRESENT          0x3a
> >  #define SFP_LOS                      0x40
> > +#define PSU_STATUS           0xa
> >
> >  struct tn48m_data {
> >       struct device *dev;
> >
>


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver
  2021-05-19 12:01     ` Robert Marko
@ 2021-05-19 13:40       ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-05-19 13:40 UTC (permalink / raw)
  To: Robert Marko
  Cc: Lee Jones, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	devicetree, linux-kernel, linux-gpio, linux-hwmon, Luka Perkov,
	jmp, Paul Menzel, Donald Buczek

On 5/19/21 5:01 AM, Robert Marko wrote:
> On Fri, Apr 30, 2021 at 3:12 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 4/30/21 5:35 AM, Robert Marko wrote:
>>> Delta TN48M has a CPLD that also monitors the power supply
>>> statuses.
>>>
>>> These are useful to be presented to the users, so lets
>>> add a driver for HWMON part of the CPLD.
>>>
>>> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>>> ---
>>>   drivers/hwmon/Kconfig       |  10 +++
>>>   drivers/hwmon/Makefile      |   1 +
>>>   drivers/hwmon/tn48m-hwmon.c | 148 ++++++++++++++++++++++++++++++++++++
>>>   drivers/mfd/tn48m-cpld.c    |   3 +
>>>   include/linux/mfd/tn48m.h   |   1 +
>>>   5 files changed, 163 insertions(+)
>>>   create mode 100644 drivers/hwmon/tn48m-hwmon.c
>>>
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 54f04e61fb83..89271dfeb775 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -1924,6 +1924,16 @@ config SENSORS_TMP513
>>>          This driver can also be built as a module. If so, the module
>>>          will be called tmp513.
>>>
>>> +config SENSORS_TN48M
>>> +     tristate "Delta Networks TN48M switch CPLD HWMON driver"
>>> +     depends on MFD_TN48M_CPLD
>>> +     help
>>> +       If you say yes here you get support for Delta Networks TN48M
>>> +       CPLD.
>>> +
>>> +       This driver can also be built as a module. If so, the module
>>> +       will be called tn48m-hwmon.
>>> +
>>>   config SENSORS_VEXPRESS
>>>        tristate "Versatile Express"
>>>        depends on VEXPRESS_CONFIG
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index fe38e8a5c979..22e470057ffe 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -187,6 +187,7 @@ obj-$(CONFIG_SENSORS_TMP108)      += tmp108.o
>>>   obj-$(CONFIG_SENSORS_TMP401) += tmp401.o
>>>   obj-$(CONFIG_SENSORS_TMP421) += tmp421.o
>>>   obj-$(CONFIG_SENSORS_TMP513) += tmp513.o
>>> +obj-$(CONFIG_SENSORS_TN48M)  += tn48m-hwmon.o
>>>   obj-$(CONFIG_SENSORS_VEXPRESS)       += vexpress-hwmon.o
>>>   obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
>>>   obj-$(CONFIG_SENSORS_VIA686A)        += via686a.o
>>> diff --git a/drivers/hwmon/tn48m-hwmon.c b/drivers/hwmon/tn48m-hwmon.c
>>> new file mode 100644
>>> index 000000000000..80fd18d74f1d
>>> --- /dev/null
>>> +++ b/drivers/hwmon/tn48m-hwmon.c
>>> @@ -0,0 +1,148 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Delta TN48M CPLD HWMON driver
>>> + *
>>> + * Copyright 2020 Sartura Ltd
>>> + *
>>> + * Author: Robert Marko <robert.marko@sartura.hr>
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/mod_devicetable.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include <linux/mfd/tn48m.h>
>>> +
>>> +#define PSU1_PRESENT_MASK    BIT(0)
>>> +#define PSU2_PRESENT_MASK    BIT(1)
>>> +#define PSU1_POWERGOOD_MASK  BIT(2)
>>> +#define PSU2_POWERGOOD_MASK  BIT(3)
>>> +#define PSU1_ALERT_MASK              BIT(4)
>>> +#define PSU2_ALERT_MASK              BIT(5)
>>> +
>>> +static int board_id_get(struct tn48m_data *data)
>>> +{
>>> +     unsigned int regval;
>>> +
>>> +     regmap_read(data->regmap, BOARD_ID, &regval);
>>> +
>>> +     switch (regval) {
>>> +     case BOARD_ID_TN48M:
>>> +             return BOARD_ID_TN48M;
>>> +     case BOARD_ID_TN48M_P:
>>> +             return BOARD_ID_TN48M_P;
>>> +     default:
>>> +             return -EINVAL;
>>> +     }
>>> +}
>>> +
>>> +static ssize_t psu_present_show(struct device *dev,
>>> +                             struct device_attribute *attr, char *buf)
>>> +{
>>> +     struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
>>> +     struct tn48m_data *data = dev_get_drvdata(dev);
>>> +     unsigned int regval, status;
>>> +
>>> +     if (board_id_get(data) == BOARD_ID_TN48M_P) {
>>> +             regmap_read(data->regmap, attr2->nr, &regval);
>>> +
>>> +             if (attr2->index == 1)
>>> +                     status = !FIELD_GET(PSU1_PRESENT_MASK, regval);
>>> +             else
>>> +                     status = !FIELD_GET(PSU2_PRESENT_MASK, regval);
>>> +     } else
>>> +             status = 0;
>>> +
>>> +     return sprintf(buf, "%d\n", status);
>>> +}
>>> +
>>> +static ssize_t psu_pg_show(struct device *dev,
>>> +                        struct device_attribute *attr, char *buf)
>>> +{
>>> +     struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
>>> +     struct tn48m_data *data = dev_get_drvdata(dev);
>>> +     unsigned int regval, status;
>>> +
>>> +     regmap_read(data->regmap, attr2->nr, &regval);
>>> +
>>> +     if (attr2->index == 1)
>>> +             status = FIELD_GET(PSU1_POWERGOOD_MASK, regval);
>>> +     else
>>> +             status = FIELD_GET(PSU2_POWERGOOD_MASK, regval);
>>> +
>>> +     return sprintf(buf, "%d\n", status);
>>> +}
>>> +
>>> +static ssize_t psu_alert_show(struct device *dev,
>>> +                           struct device_attribute *attr, char *buf)
>>> +{
>>> +     struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
>>> +     struct tn48m_data *data = dev_get_drvdata(dev);
>>> +     unsigned int regval, status;
>>> +
>>> +     if (board_id_get(data) == BOARD_ID_TN48M_P) {
>>> +             regmap_read(data->regmap, attr2->nr, &regval);
>>> +
>>> +             if (attr2->index == 1)
>>> +                     status = !FIELD_GET(PSU1_ALERT_MASK, regval);
>>> +             else
>>> +                     status = !FIELD_GET(PSU2_ALERT_MASK, regval);
>>> +     } else
>>> +             status = 0;
>>> +
>>> +     return sprintf(buf, "%d\n", status);
>>> +}
>>> +
>>> +static SENSOR_DEVICE_ATTR_2_RO(psu1_present, psu_present, PSU_STATUS, 1);
>>> +static SENSOR_DEVICE_ATTR_2_RO(psu2_present, psu_present, PSU_STATUS, 2);
>>> +static SENSOR_DEVICE_ATTR_2_RO(psu1_pg, psu_pg, PSU_STATUS, 1);
>>> +static SENSOR_DEVICE_ATTR_2_RO(psu2_pg, psu_pg, PSU_STATUS, 2);
>>> +static SENSOR_DEVICE_ATTR_2_RO(psu1_alert, psu_alert, PSU_STATUS, 1);
>>> +static SENSOR_DEVICE_ATTR_2_RO(psu2_alert, psu_alert, PSU_STATUS, 2);
>>> +
>>> +static struct attribute *tn48m_hwmon_attrs[] = {
>>> +     &sensor_dev_attr_psu1_present.dev_attr.attr,
>>> +     &sensor_dev_attr_psu2_present.dev_attr.attr,
>>> +     &sensor_dev_attr_psu1_pg.dev_attr.attr,
>>> +     &sensor_dev_attr_psu2_pg.dev_attr.attr,
>>> +     &sensor_dev_attr_psu1_alert.dev_attr.attr,
>>> +     &sensor_dev_attr_psu2_alert.dev_attr.attr,
>>
>> Literally none of those attributes are standard hwmon attributes.
>> I don't know what this is, but it is not a hardware monitoring driver.
> 
> Yes, I agree that it does not expose any of the standard attributes, but these
> are the only ones the CPLD exposes.
> 
> I don't know where else to put them, MFD driver did not seem logical to me.
>>
>>> +     NULL
>>> +};
>>> +
>>> +ATTRIBUTE_GROUPS(tn48m_hwmon);
>>> +
>>> +static int tn48m_hwmon_probe(struct platform_device *pdev)
>>> +{
>>> +     struct tn48m_data *data = dev_get_drvdata(pdev->dev.parent);
>>> +     struct device *hwmon_dev;
>>> +
>>> +     hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
>>> +                                                        "tn48m_hwmon",
>>> +                                                        data,
>>> +                                                        tn48m_hwmon_groups);
>>
>> Please use devm_hwmon_device_register_with_info() to register hwmon devices.
>> Of course, that only makes sense for actual hardware monitoring drivers
>> which do support standard attributes.
> 
> Yes, devm_hwmon_device_register_with_info() made no sense without any of the
> standard attributes.
> 

I would suggest to expose the information using debugfs.
Again, this is not a hardware monitoring driver.

Guenter

> Robert
>>
>>> +     return PTR_ERR_OR_ZERO(hwmon_dev);
>>> +}
>>> +
>>> +static const struct platform_device_id tn48m_hwmon_id_table[] = {
>>> +     { "delta,tn48m-hwmon", },
>>> +     { }
>>> +};
>>> +MODULE_DEVICE_TABLE(platform, tn48m_hwmon_id_table);
>>> +
>>> +static struct platform_driver tn48m_hwmon_driver = {
>>> +     .driver = {
>>> +             .name = "tn48m-hwmon",
>>> +     },
>>> +     .probe = tn48m_hwmon_probe,
>>> +     .id_table = tn48m_hwmon_id_table,
>>> +};
>>> +module_platform_driver(tn48m_hwmon_driver);
>>> +
>>> +MODULE_AUTHOR("Robert Marko <robert.marko@sartura.hr>");
>>> +MODULE_DESCRIPTION("Delta TN48M CPLD HWMON driver");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/mfd/tn48m-cpld.c b/drivers/mfd/tn48m-cpld.c
>>> index f22a15ddd22d..4d837aca01e7 100644
>>> --- a/drivers/mfd/tn48m-cpld.c
>>> +++ b/drivers/mfd/tn48m-cpld.c
>>> @@ -20,6 +20,9 @@
>>>   static const struct mfd_cell tn48m_cell[] = {
>>>        {
>>>                .name = "delta,tn48m-gpio",
>>> +     },
>>> +     {
>>> +             .name = "delta,tn48m-hwmon",
>>>        }
>>>   };
>>>
>>> diff --git a/include/linux/mfd/tn48m.h b/include/linux/mfd/tn48m.h
>>> index 9cc2b04c8d69..eb2cfc3a5db7 100644
>>> --- a/include/linux/mfd/tn48m.h
>>> +++ b/include/linux/mfd/tn48m.h
>>> @@ -22,6 +22,7 @@
>>>   #define SFP_TX_DISABLE               0x31
>>>   #define SFP_PRESENT          0x3a
>>>   #define SFP_LOS                      0x40
>>> +#define PSU_STATUS           0xa
>>>
>>>   struct tn48m_data {
>>>        struct device *dev;
>>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-05-19 11:53   ` Robert Marko
@ 2021-05-19 19:42     ` Michael Walle
  2021-05-20  6:49       ` Lee Jones
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Walle @ 2021-05-19 19:42 UTC (permalink / raw)
  To: Robert Marko
  Cc: Lee Jones, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

Hi,

Am 2021-05-19 13:53, schrieb Robert Marko:
> On Thu, May 6, 2021 at 6:34 PM Michael Walle <michael@walle.cc> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-05-19 19:42     ` Michael Walle
@ 2021-05-20  6:49       ` Lee Jones
  2021-05-21  8:19         ` Robert Marko
  0 siblings, 1 reply; 26+ messages in thread
From: Lee Jones @ 2021-05-20  6:49 UTC (permalink / raw)
  To: Michael Walle
  Cc: Robert Marko, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

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 <michael@walle.cc> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-05-20  6:49       ` Lee Jones
@ 2021-05-21  8:19         ` Robert Marko
  2021-05-21  9:03           ` Lee Jones
  0 siblings, 1 reply; 26+ messages in thread
From: Robert Marko @ 2021-05-21  8:19 UTC (permalink / raw)
  To: Lee Jones
  Cc: Michael Walle, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

On Thu, May 20, 2021 at 8:49 AM Lee Jones <lee.jones@linaro.org> 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 <michael@walle.cc> 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: robert.marko@sartura.hr
Web: www.sartura.hr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-05-21  8:19         ` Robert Marko
@ 2021-05-21  9:03           ` Lee Jones
  2021-05-21  9:06             ` Robert Marko
  0 siblings, 1 reply; 26+ messages in thread
From: Lee Jones @ 2021-05-21  9:03 UTC (permalink / raw)
  To: Robert Marko
  Cc: Michael Walle, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

On Fri, 21 May 2021, Robert Marko wrote:

> On Thu, May 20, 2021 at 8:49 AM Lee Jones <lee.jones@linaro.org> 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 <michael@walle.cc> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/6] mfd: Add Delta TN48M CPLD driver
  2021-05-21  9:03           ` Lee Jones
@ 2021-05-21  9:06             ` Robert Marko
  0 siblings, 0 replies; 26+ messages in thread
From: Robert Marko @ 2021-05-21  9:06 UTC (permalink / raw)
  To: Lee Jones
  Cc: Michael Walle, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

On Fri, May 21, 2021 at 11:04 AM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Fri, 21 May 2021, Robert Marko wrote:
>
> > On Thu, May 20, 2021 at 8:49 AM Lee Jones <lee.jones@linaro.org> 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 <michael@walle.cc> 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: robert.marko@sartura.hr
Web: www.sartura.hr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-05-06 16:38   ` Michael Walle
@ 2021-05-21 13:21     ` Robert Marko
  0 siblings, 0 replies; 26+ messages in thread
From: Robert Marko @ 2021-05-21 13:21 UTC (permalink / raw)
  To: Michael Walle
  Cc: Lee Jones, robh+dt, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

On Thu, May 6, 2021 at 6:38 PM Michael Walle <michael@walle.cc> 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: robert.marko@sartura.hr
Web: www.sartura.hr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver
  2021-05-06 16:40     ` Michael Walle
  2021-05-06 17:21       ` Luka Perkov
@ 2021-05-21 13:22       ` Robert Marko
  1 sibling, 0 replies; 26+ messages in thread
From: Robert Marko @ 2021-05-21 13:22 UTC (permalink / raw)
  To: Michael Walle
  Cc: Rob Herring, Lee Jones, Linus Walleij, bgolaszewski, jdelvare,
	Guenter Roeck, devicetree, linux-kernel, linux-gpio, linux-hwmon,
	Luka Perkov, jmp, Paul Menzel, Donald Buczek

On Thu, May 6, 2021 at 6:40 PM Michael Walle <michael@walle.cc> 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 <robert.marko@sartura.hr>
> >> ---
> >>  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: robert.marko@sartura.hr
Web: www.sartura.hr

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-05-21 13:23 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 12:35 [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Robert Marko
2021-04-30 12:35 ` [PATCH 2/6] dt-bindings: gpio: Add Delta TN48M CPLD GPIO bindings Robert Marko
2021-05-06 13:57   ` Rob Herring
2021-04-30 12:35 ` [PATCH 3/6] gpio: Add Delta TN48M CPLD GPIO driver Robert Marko
2021-05-06 14:00   ` Rob Herring
2021-05-06 16:40     ` Michael Walle
2021-05-06 17:21       ` Luka Perkov
2021-05-21 13:22       ` Robert Marko
2021-05-06 16:38   ` Michael Walle
2021-05-21 13:21     ` Robert Marko
2021-04-30 12:35 ` [PATCH 4/6] hwmon: Add Delta TN48M CPLD HWMON driver Robert Marko
2021-04-30 13:12   ` Guenter Roeck
2021-05-19 12:01     ` Robert Marko
2021-05-19 13:40       ` Guenter Roeck
2021-04-30 12:35 ` [PATCH 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings Robert Marko
2021-05-06 14:01   ` Rob Herring
2021-04-30 12:35 ` [PATCH 6/6] MAINTAINERS: Add Delta Networks TN48M CPLD drivers Robert Marko
2021-05-06 16:34 ` [PATCH 1/6] mfd: Add Delta TN48M CPLD driver Michael Walle
2021-05-19 11:53   ` Robert Marko
2021-05-19 19:42     ` Michael Walle
2021-05-20  6:49       ` Lee Jones
2021-05-21  8:19         ` Robert Marko
2021-05-21  9:03           ` Lee Jones
2021-05-21  9:06             ` Robert Marko
2021-05-06 16:50 ` kernel test robot
2021-05-19 11:11 ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).