hwmon: Versatile Express hwmon driver

From: Pawel Moll <[email protected]>

hwmon framework driver for Versatile Express sensors, providing information about board level voltage (only when regulator driver is not configured), currents, temperature and power/energy usage. Labels for the values can be defined as DT properties.

Signed-off-by Pawel Moll <[email protected]>

---
 .../devicetree/bindings/hwmon/vexpress.txt         |   23 ++
 Documentation/hwmon/vexpress                       |   34 +++
 drivers/hwmon/Kconfig                              |    8 +
 drivers/hwmon/Makefile                             |    1 +
 drivers/hwmon/vexpress.c                           |  229 ++++++++++++++++++++
 5 files changed, 295 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/vexpress.txt
 create mode 100644 Documentation/hwmon/vexpress
 create mode 100644 drivers/hwmon/vexpress.c

drivers/hwmon/vexpress.c new file +46 -0

+
+	data->hwmon_dev = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto error;
+	}
+
+	return 0;
+
+error:
+	sysfs_remove_group(&pdev->dev.kobj, match->data);
+	vexpress_config_func_put(data->func);
+	return err;
+}
+
+static int __devexit vexpress_hwmon_remove(struct platform_device *pdev)
+{
+	struct vexpress_hwmon_data *data = platform_get_drvdata(pdev);
+	const struct of_device_id *match;
+
+	hwmon_device_unregister(data->hwmon_dev);
+
+	match = of_match_device(vexpress_hwmon_of_match, &pdev->dev);
+	sysfs_remove_group(&pdev->dev.kobj, match->data);
+
+	vexpress_config_func_put(data->func);
+
+	return 0;
+}
+
+static struct platform_driver vexpress_hwmon_driver = {
+	.probe = vexpress_hwmon_probe,
+	.remove = __devexit_p(vexpress_hwmon_remove),
+	.driver	= {
+		.name = DRVNAME,
+		.owner = THIS_MODULE,
+		.of_match_table = vexpress_hwmon_of_match,
+	},
+};
+
+module_platform_driver(vexpress_hwmon_driver);
+
+MODULE_AUTHOR("Pawel Moll <[email protected]>");
+MODULE_DESCRIPTION("Versatile Express hwmon sensors driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:vexpress-hwmon");

Signature · 1 line

1.7.9.5

Quoted 19 lines · Pawel Moll

From: Pawel Moll <[email protected]>

hwmon framework driver for Versatile Express sensors, providing information about board level voltage (only when regulator driver is not configured), currents, temperature and power/energy usage. Labels for the values can be defined as DT properties.

Signed-off-by Pawel Moll <[email protected]>

---
 .../devicetree/bindings/hwmon/vexpress.txt         |   23 ++
 Documentation/hwmon/vexpress                       |   34 +++
 drivers/hwmon/Kconfig                              |    8 +
 drivers/hwmon/Makefile                             |    1 +
 drivers/hwmon/vexpress.c                           |  229 ++++++++++++++++++++
 5 files changed, 295 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/vexpress.txt
 create mode 100644 Documentation/hwmon/vexpress
 create mode 100644 drivers/hwmon/vexpress.c

[ ... ]

drivers/hwmon/vexpress.c line 183

Quoted 4 lines

drivers/hwmon/vexpress.c new file +4 -0

+
+	err = sysfs_create_group(&pdev->dev.kobj, match->data);
+	if (err)
+		goto error;

You'll need a second label for that. Since the group was not created, you can not delete it.

Quoted 8 lines · Guenter Roeck

drivers/hwmon/vexpress.c line 183

Quoted 4 lines · Pawel Moll

drivers/hwmon/vexpress.c new file +4 -0

+
+	err = sysfs_create_group(&pdev->dev.kobj, match->data);
+	if (err)
+		goto error;

You'll need a second label for that. Since the group was not created, you can not delete it.

Actually I think I can... The sysfs_remove_group() effectively is a wrapper for sysfs_hash_and_remove() which acts like "rm -f" - does nothing if the file doesn't exist. Even drivers/hwmon/lm83.c (which, by pure coincidence, I was looking at when re-working this driver) does that in exit_remove_files...

Anyway, it's nothing to argue about, so I'll change it and send (hopefully ;-) final version in a jiffy.

Quoted 10 lines · Pawel Moll

Quoted 8 lines · Guenter Roeck

drivers/hwmon/vexpress.c line 183

Quoted 4 lines · Pawel Moll

drivers/hwmon/vexpress.c new file +4 -0

+
+	err = sysfs_create_group(&pdev->dev.kobj, match->data);
+	if (err)
+		goto error;

You'll need a second label for that. Since the group was not created, you can not delete it.

Quoted 5 lines

Actually I think I can... The sysfs_remove_group() effectively is a wrapper for sysfs_hash_and_remove() which acts like "rm -f" - does nothing if the file doesn't exist. Even drivers/hwmon/lm83.c (which, by pure coincidence, I was looking at when re-working this driver) does that in exit_remove_files...

You are right, and many hwmon drivers do exactly that, to limit the number of labels. This is code which isn't executed when all is fine, so we don't care about performance. Plus, in many cases the file removals are unconditional in the remove function anyway.

Quoted 2 lines

Anyway, it's nothing to argue about, so I'll change it and send (hopefully ;-) final version in a jiffy.

You can keep the code as is too, if you prefer.

From: Pawel Moll <[email protected]>

hwmon framework driver for Versatile Express sensors, providing information about board level voltage (only when regulator driver is not configured), currents, temperature and power/energy usage. Labels for the values can be defined as DT properties.

Signed-off-by Pawel Moll <[email protected]>

Quoted 7 lines · Pawel Moll

From: Pawel Moll <[email protected]>

hwmon framework driver for Versatile Express sensors, providing information about board level voltage (only when regulator driver is not configured), currents, temperature and power/energy usage. Labels for the values can be defined as DT properties.

Quoted 1 line

Signed-off-by Pawel Moll <[email protected]>

After my concern was clarified ...

Acked-by Guenter Roeck <[email protected]>

I assume this will go upstream through the arm tree ?

Quoted 8 lines · Guenter Roeck

Quoted 7 lines · Pawel Moll

From: Pawel Moll <[email protected]>

hwmon framework driver for Versatile Express sensors, providing information about board level voltage (only when regulator driver is not configured), currents, temperature and power/energy usage. Labels for the values can be defined as DT properties.

Quoted 5 lines

Quoted 1 line

Signed-off-by Pawel Moll <[email protected]>

After my concern was clarified ...

Acked-by Guenter Roeck <[email protected]>

Thanks a lot!

Quoted 1 line

I assume this will go upstream through the arm tree ?

I was planning to push the subsystem patches (hwmon, regulator etc.) through respective maintainers, so through your tree - the driver depends on VEXPRESS_CONFIG so won't cause any build issues before the infrastructure is in place. Of course if you'd rather see it go through arm-soc, I'll add it to that branch. Just let me know what is your preference.

Quoted 7 lines

I was planning to push the subsystem patches (hwmon, regulator etc.) through respective maintainers, so through your tree - the driver depends on VEXPRESS_CONFIG so won't cause any build issues before the infrastructure is in place. Of course if you'd rather see it go through arm-soc, I'll add it to that branch. Just let me know what is your preference.

Since the driver can not be built w/o the rest of the infrastructure, I think it should be pushed through a tree where it can. This way we also make sure that it is in sync with its infrastructure, and avoid bad surprises late in the game.