Marc Hulsman wrote:
> Fan 4/5 and PWM 4/5 pins can be used for GPIO. If the bios initializes these
> pins as in use for GPIO we keep it that way and do not construct the
> sysfs-interface for them. I left the code for reading the corresponding
> registers during update_device unchanged as adding conditional terms would
> only complicate the code.
>
Looks good:
Acked-by: Hans de Goede <j.w.r.degoede at hhs.nl>
One note though (for a future patch) you say you keep reading the unused regs
to keep the code clean. I understand this, but please keep in mind that i2c is
pretty slow, so you might want todo another patch changing this.
Or even better write a patch where you stop re-reading never changing registers
completely, iow read config / min / max registers only once on probe and then
only read registers which actually measure things (and thus can change without
a write) in update_device (Yes there is the potential problem of acpi code
writing to the device underneath us but we aren't checking for that now anyways).
Regards,
Hans
> Marc
> ---
> Pins fan/pwm 4-5 can be in use as GPIO. If that is the case, do not
> create their sysfs-interface.
>
> Signed-off-by: Marc Hulsman <m.hulsman at tudelft.nl>
> ---
> drivers/hwmon/w83791d.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> ---
> Index: linux-2.6.27-rc1/drivers/hwmon/w83791d.c
> ===================================================================
> --- linux-2.6.27-rc1.orig/drivers/hwmon/w83791d.c
> +++ linux-2.6.27-rc1/drivers/hwmon/w83791d.c
> @@ -160,6 +160,7 @@ static const u8 W83791D_REG_BEEP_CTRL[3]
> 0xA3, /* BEEP Control Register 3 */
> };
>
> +#define W83791D_REG_GPIO 0x15
> #define W83791D_REG_CONFIG 0x40
> #define W83791D_REG_VID_FANDIV 0x47
> #define W83791D_REG_DID_VID4 0x49
> @@ -908,8 +909,6 @@ static struct attribute *w83791d_attribu
> FAN_UNIT_ATTRS(0),
> FAN_UNIT_ATTRS(1),
> FAN_UNIT_ATTRS(2),
> - FAN_UNIT_ATTRS(3),
> - FAN_UNIT_ATTRS(4),
> TEMP_UNIT_ATTRS(0),
> TEMP_UNIT_ATTRS(1),
> TEMP_UNIT_ATTRS(2),
> @@ -925,6 +924,18 @@ static const struct attribute_group w837
> .attrs = w83791d_attributes,
> };
>
> +/* Separate group of attributes for fan/pwm 4-5. Their pins can also be
> + in use for GPIO in which case their sysfs-interface should not be made
> + available */
> +static struct attribute *w83791d_attributes_fanpwm45[] = {
> + FAN_UNIT_ATTRS(3),
> + FAN_UNIT_ATTRS(4),
> + NULL
> +};
> +
> +static const struct attribute_group w83791d_group_fanpwm45 = {
> + .attrs = w83791d_attributes_fanpwm45,
> +};
>
> static int w83791d_detect_subclients(struct i2c_client *client)
> {
> @@ -1056,6 +1067,7 @@ static int w83791d_probe(struct i2c_clie
> struct w83791d_data *data;
> struct device *dev = &client->dev;
> int i, val1, err;
> + u8 has_fanpwm45;
>
> #ifdef DEBUG
> val1 = w83791d_read(client, W83791D_REG_DID_VID4);
> @@ -1089,15 +1101,27 @@ static int w83791d_probe(struct i2c_clie
> if ((err = sysfs_create_group(&client->dev.kobj, &w83791d_group)))
> goto error3;
>
> + /* Check if pins of fan/pwm 4-5 are in use as GPIO */
> + has_fanpwm45 = w83791d_read(client, W83791D_REG_GPIO) & 0x10;
> + if (has_fanpwm45) {
> + err = sysfs_create_group(&client->dev.kobj,
> + &w83791d_group_fanpwm45);
> + if (err)
> + goto error4;
> + }
> +
> /* Everything is ready, now register the working device */
> data->hwmon_dev = hwmon_device_register(dev);
> if (IS_ERR(data->hwmon_dev)) {
> err = PTR_ERR(data->hwmon_dev);
> - goto error4;
> + goto error5;
> }
>
> return 0;
>
> +error5:
> + if (has_fanpwm45)
> + sysfs_remove_group(&client->dev.kobj, &w83791d_group_fanpwm45);
> error4:
> sysfs_remove_group(&client->dev.kobj, &w83791d_group);
> error3:
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors at lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
