diff mbox series

[v2,2/4] hwmon: pwm-fan: Dynamically setup attribute groups

Message ID 20201113150853.155495-3-pbarker@konsulko.com (mailing list archive)
State Changes Requested
Headers show
Series pwm-fan: Support multiple tachometer inputs | expand

Commit Message

Paul Barker Nov. 13, 2020, 3:08 p.m. UTC 
Instead of implementing an is_visible function we can dynamically
populate the attribute group for the device based on whether a fan
tachometer input is configured or not.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 drivers/hwmon/pwm-fan.c | 55 +++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

Comments

Paul Barker Nov. 26, 2020, 9:07 a.m. UTC | #1 
On Thu, 26 Nov 2020 at 01:45, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Nov 13, 2020 at 03:08:51PM +0000, Paul Barker wrote:
> > Instead of implementing an is_visible function we can dynamically
> > populate the attribute group for the device based on whether a fan
> > tachometer input is configured or not.
> >
> > Signed-off-by: Paul Barker <pbarker@konsulko.com>
>
> This will make it more difficult to ever convert the driver to use the
> devm_hwmon_device_register_with_info() API. I'd rather see a conversion
> to that API now instead of making it more difficult to implement in the
> future.

Ah I hadn't realised that was the preferred API. I'll take a look and
see if I can convert the driver, perhaps as a standalone patch so we
can get this right before I re-work my implementation of multiple
tachometer input support.

Thanks,
diff mbox series

Patch

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index efe2764f42d3..c4e0059ccaec 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -31,6 +31,8 @@  struct pwm_fan_ctx {
 	atomic_t pulses;
 	unsigned int rpm;
 	u8 pulses_per_revolution;
+	struct sensor_device_attribute sensor_attr;
+
 	ktime_t sample_start;
 	struct timer_list rpm_timer;
 
@@ -39,6 +41,9 @@  struct pwm_fan_ctx {
 	unsigned int pwm_fan_max_state;
 	unsigned int *pwm_fan_cooling_levels;
 	struct thermal_cooling_device *cdev;
+
+	struct attribute_group fan_group;
+	struct attribute_group *fan_groups[2];
 };
 
 /* This handler assumes self resetting edge triggered interrupt. */
@@ -138,36 +143,6 @@  static ssize_t rpm_show(struct device *dev,
 }
 
 static SENSOR_DEVICE_ATTR_RW(pwm1, pwm, 0);
-static SENSOR_DEVICE_ATTR_RO(fan1_input, rpm, 0);
-
-static struct attribute *pwm_fan_attrs[] = {
-	&sensor_dev_attr_pwm1.dev_attr.attr,
-	&sensor_dev_attr_fan1_input.dev_attr.attr,
-	NULL,
-};
-
-static umode_t pwm_fan_attrs_visible(struct kobject *kobj, struct attribute *a,
-				     int n)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
-
-	/* Hide fan_input in case no interrupt is available  */
-	if (n == 1 && ctx->irq <= 0)
-		return 0;
-
-	return a->mode;
-}
-
-static const struct attribute_group pwm_fan_group = {
-	.attrs = pwm_fan_attrs,
-	.is_visible = pwm_fan_attrs_visible,
-};
-
-static const struct attribute_group *pwm_fan_groups[] = {
-	&pwm_fan_group,
-	NULL,
-};
 
 /* thermal cooling device callbacks */
 static int pwm_fan_get_max_state(struct thermal_cooling_device *cdev,
@@ -287,6 +262,7 @@  static int pwm_fan_probe(struct platform_device *pdev)
 	int ret;
 	struct pwm_state state = { };
 	int tach_count;
+	size_t sz;
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
@@ -340,6 +316,13 @@  static int pwm_fan_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, tach_count,
 				     "Could not get number of fan tachometer inputs\n");
 
+	sz = (2 + tach_count) * sizeof(struct attribute *);
+	ctx->fan_group.attrs = devm_kzalloc(dev, sz, GFP_KERNEL);
+	if (!ctx->fan_group.attrs)
+		return -ENOMEM;
+
+	ctx->fan_group.attrs[0] = &sensor_dev_attr_pwm1.dev_attr.attr;
+
 	if (tach_count > 0) {
 		u32 ppr = 2;
 
@@ -366,6 +349,13 @@  static int pwm_fan_probe(struct platform_device *pdev)
 			return -EINVAL;
 		}
 
+		sysfs_attr_init(&ctx->sensor_attr.dev_attr.attr);
+
+		ctx->sensor_attr.dev_attr.attr.name = "fan1_input";
+		ctx->sensor_attr.dev_attr.attr.mode = 0444;
+		ctx->sensor_attr.dev_attr.show = rpm_show;
+		ctx->fan_group.attrs[1] = &ctx->sensor_attr.dev_attr.attr;
+
 		dev_dbg(dev, "tach: irq=%d, pulses_per_revolution=%d\n",
 			ctx->irq, ctx->pulses_per_revolution);
 
@@ -373,8 +363,9 @@  static int pwm_fan_probe(struct platform_device *pdev)
 		mod_timer(&ctx->rpm_timer, jiffies + HZ);
 	}
 
-	hwmon = devm_hwmon_device_register_with_groups(dev, "pwmfan",
-						       ctx, pwm_fan_groups);
+	ctx->fan_groups[0] = &ctx->fan_group;
+	hwmon = devm_hwmon_device_register_with_groups(dev, "pwmfan", ctx,
+			(const struct attribute_group **)ctx->fan_groups);
 	if (IS_ERR(hwmon)) {
 		dev_err(dev, "Failed to register hwmon device\n");
 		return PTR_ERR(hwmon);