* [PATCH 1/4] nouveau_hwmon: migrate to hwmon_device_register_with_info
@ 2017-04-13 9:08 Oscar Salvador
2017-04-13 23:21 ` Karol Herbst
0 siblings, 1 reply; 3+ messages in thread
From: Oscar Salvador @ 2017-04-13 9:08 UTC (permalink / raw)
To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
This patch introduces the structure "struct hwmon_ops" and sets up the ".visible" operation.
Is also a preparation for the next patch where all work is being done.
--- linux/drivers/gpu/drm/nouveau/nouveau_hwmon.c.orig 2017-04-12 19:22:29.070573187 +0200
+++ linux/drivers/gpu/drm/nouveau/nouveau_hwmon.c 2017-04-12 19:21:32.391338011 +0200
@@ -764,6 +764,166 @@ static const struct hwmon_channel_info *
&nouveau_power,
NULL
};
+
+static umode_t
+nouveau_chip_is_visible(const void *data, u32 attr, int channel)
+{
+ switch (attr) {
+ case hwmon_chip_update_interval:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static umode_t
+nouveau_power_is_visible(const void *data, u32 attr, int channel)
+{
+ struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+ struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
+
+ switch (attr) {
+ case hwmon_power_input:
+ if (iccsense && iccsense->data_valid &&
+ !list_empty(&iccsense->rails))
+ return 0444;
+ case hwmon_power_max:
+ case hwmon_power_crit:
+ if (iccsense->power_w_max && iccsense->power_w_crit)
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static umode_t
+nouveau_temp_is_visible(const void *data, u32 attr, int channel)
+{
+ struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+ struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
+
+ if (therm && therm->attr_get && therm->attr_set)
+ if (nvkm_therm_temp_get(therm) < 0)
+ return 0;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ return 0444;
+ case hwmon_temp_max:
+ case hwmon_temp_max_hyst:
+ case hwmon_temp_crit:
+ case hwmon_temp_crit_hyst:
+ case hwmon_temp_emergency:
+ case hwmon_temp_emergency_hyst:
+ return 0644;
+ default:
+ return 0;
+ }
+}
+
+static umode_t
+nouveau_pwm_is_visible(const void *data, u32 attr, int channel)
+{
+ struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+ struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
+
+ if (therm && therm->attr_get)
+ if (therm->fan_get && therm->fan_get(therm) < 0)
+ return 0;
+
+ switch (attr) {
+ case hwmon_pwm_enable:
+ case hwmon_pwm_input:
+ return 0644;
+ default:
+ return 0;
+ }
+}
+
+static umode_t
+nouveau_input_is_visible(const void *data, u32 attr, int channel)
+{
+ struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+ struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
+
+ if (!volt || nvkm_volt_get(volt) < 0)
+ return 0;
+
+ switch (attr) {
+ case hwmon_in_input:
+ case hwmon_in_label:
+ case hwmon_in_min:
+ case hwmon_in_max:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static umode_t
+nouveau_fan_is_visible(const void *data, u32 attr, int channel)
+{
+ struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+ struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
+
+ if (!therm || !therm->attr_get || nvkm_therm_fan_sense(therm) < 0)
+ return 0;
+
+ switch (attr) {
+ case hwmon_fan_input:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static umode_t
+nouveau_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
+ int channel)
+{
+ switch (type) {
+ case hwmon_chip:
+ return nouveau_chip_is_visible(data, attr, channel);
+ case hwmon_temp:
+ return nouveau_temp_is_visible(data, attr, channel);
+ case hwmon_fan:
+ return nouveau_fan_is_visible(data, attr, channel);
+ case hwmon_in:
+ return nouveau_input_is_visible(data, attr, channel);
+ case hwmon_pwm:
+ return nouveau_pwm_is_visible(data, attr, channel);
+ case hwmon_power:
+ return nouveau_power_is_visible(data, attr, channel);
+ default:
+ return 0;
+ }
+}
+
+static char *input_label = "GPU core";
+
+static int
+nouveau_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, char **buf)
+{
+ if (type == hwmon_in && attr == hwmon_in_label) {
+ *buf = input_label;
+ return 0;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops nouveau_hwmon_ops = {
+ .is_visible = nouveau_is_visible,
+ .read = NULL,
+ .read_string = nouveau_read_string,
+ .write = NULL,
+};
+
+static const struct hwmon_chip_info nouveau_chip_info = {
+ .ops = &nouveau_hwmon_ops,
+ .info = nouveau_info,
+};
#endif
int
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/4] nouveau_hwmon: migrate to hwmon_device_register_with_info
2017-04-13 9:08 [PATCH 1/4] nouveau_hwmon: migrate to hwmon_device_register_with_info Oscar Salvador
@ 2017-04-13 23:21 ` Karol Herbst
2017-04-14 7:14 ` [Nouveau] " Oscar Salvador
0 siblings, 1 reply; 3+ messages in thread
From: Karol Herbst @ 2017-04-13 23:21 UTC (permalink / raw)
To: Oscar Salvador; +Cc: ML nouveau, dri-devel
2017-04-13 11:08 GMT+02:00 Oscar Salvador <osalvador.vilardaga@gmail.com>:
> This patch introduces the structure "struct hwmon_ops" and sets up the ".visible" operation.
> Is also a preparation for the next patch where all work is being done.
>
> --- linux/drivers/gpu/drm/nouveau/nouveau_hwmon.c.orig 2017-04-12 19:22:29.070573187 +0200
> +++ linux/drivers/gpu/drm/nouveau/nouveau_hwmon.c 2017-04-12 19:21:32.391338011 +0200
> @@ -764,6 +764,166 @@ static const struct hwmon_channel_info *
> &nouveau_power,
> NULL
> };
> +
> +static umode_t
> +nouveau_chip_is_visible(const void *data, u32 attr, int channel)
> +{
> + switch (attr) {
> + case hwmon_chip_update_interval:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> +static umode_t
> +nouveau_power_is_visible(const void *data, u32 attr, int channel)
> +{
> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
> + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
> +
> + switch (attr) {
> + case hwmon_power_input:
> + if (iccsense && iccsense->data_valid &&
> + !list_empty(&iccsense->rails))
> + return 0444;
no fallthrough here.
> + case hwmon_power_max:
> + case hwmon_power_crit:
> + if (iccsense->power_w_max && iccsense->power_w_crit)
> + return 0444;
it makes sense to split those
> + default:
> + return 0;
> + }
> +}
> +
> +static umode_t
> +nouveau_temp_is_visible(const void *data, u32 attr, int channel)
> +{
> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> +
> + if (therm && therm->attr_get && therm->attr_set)
> + if (nvkm_therm_temp_get(therm) < 0)
> + return 0;
I think you can merge those if statements
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + return 0444;
> + case hwmon_temp_max:
> + case hwmon_temp_max_hyst:
> + case hwmon_temp_crit:
> + case hwmon_temp_crit_hyst:
> + case hwmon_temp_emergency:
> + case hwmon_temp_emergency_hyst:
> + return 0644;
I doubt we ever want to support changing those, please leave them as read only
> + default:
> + return 0;
> + }
> +}
> +
> +static umode_t
> +nouveau_pwm_is_visible(const void *data, u32 attr, int channel)
> +{
> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> +
> + if (therm && therm->attr_get)
> + if (therm->fan_get && therm->fan_get(therm) < 0)
> + return 0;
merge the ifs
> +
> + switch (attr) {
> + case hwmon_pwm_enable:
> + case hwmon_pwm_input:
> + return 0644;
> + default:
> + return 0;
> + }
> +}
> +
> +static umode_t
> +nouveau_input_is_visible(const void *data, u32 attr, int channel)
> +{
> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
> + struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
> +
> + if (!volt || nvkm_volt_get(volt) < 0)
> + return 0;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + case hwmon_in_label:
> + case hwmon_in_min:
> + case hwmon_in_max:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> +static umode_t
> +nouveau_fan_is_visible(const void *data, u32 attr, int channel)
> +{
> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> +
> + if (!therm || !therm->attr_get || nvkm_therm_fan_sense(therm) < 0)
> + return 0;
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> +static umode_t
> +nouveau_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> + int channel)
Indentation
> +{
> + switch (type) {
> + case hwmon_chip:
> + return nouveau_chip_is_visible(data, attr, channel);
> + case hwmon_temp:
> + return nouveau_temp_is_visible(data, attr, channel);
> + case hwmon_fan:
> + return nouveau_fan_is_visible(data, attr, channel);
> + case hwmon_in:
> + return nouveau_input_is_visible(data, attr, channel);
> + case hwmon_pwm:
> + return nouveau_pwm_is_visible(data, attr, channel);
> + case hwmon_power:
> + return nouveau_power_is_visible(data, attr, channel);
> + default:
> + return 0;
> + }
> +}
> +
> +static char *input_label = "GPU core";
> +
> +static int
> +nouveau_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, char **buf)
Indentation
> +{
> + if (type == hwmon_in && attr == hwmon_in_label) {
> + *buf = input_label;
> + return 0;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_ops nouveau_hwmon_ops = {
> + .is_visible = nouveau_is_visible,
> + .read = NULL,
> + .read_string = nouveau_read_string,
> + .write = NULL,
> +};
> +
> +static const struct hwmon_chip_info nouveau_chip_info = {
> + .ops = &nouveau_hwmon_ops,
> + .info = nouveau_info,
> +};
> #endif
>
> int
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Nouveau] [PATCH 1/4] nouveau_hwmon: migrate to hwmon_device_register_with_info
2017-04-13 23:21 ` Karol Herbst
@ 2017-04-14 7:14 ` Oscar Salvador
0 siblings, 0 replies; 3+ messages in thread
From: Oscar Salvador @ 2017-04-14 7:14 UTC (permalink / raw)
To: Karol Herbst; +Cc: ML nouveau, dri-devel
Sorry, It's my first time sending a patch and I'm a bit lost. I didn't
know the first one must be a cover letter.
I'll write another series of patch doing it right and with the changes
you suggested in it.
I'll put v2 after PATCH.
Thanks
On 14 April 2017 at 01:21, Karol Herbst <karolherbst@gmail.com> wrote:
> 2017-04-13 11:08 GMT+02:00 Oscar Salvador <osalvador.vilardaga@gmail.com>:
>> This patch introduces the structure "struct hwmon_ops" and sets up the ".visible" operation.
>> Is also a preparation for the next patch where all work is being done.
>>
>> --- linux/drivers/gpu/drm/nouveau/nouveau_hwmon.c.orig 2017-04-12 19:22:29.070573187 +0200
>> +++ linux/drivers/gpu/drm/nouveau/nouveau_hwmon.c 2017-04-12 19:21:32.391338011 +0200
>> @@ -764,6 +764,166 @@ static const struct hwmon_channel_info *
>> &nouveau_power,
>> NULL
>> };
>> +
>> +static umode_t
>> +nouveau_chip_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + switch (attr) {
>> + case hwmon_chip_update_interval:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_power_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
>> +
>> + switch (attr) {
>> + case hwmon_power_input:
>> + if (iccsense && iccsense->data_valid &&
>> + !list_empty(&iccsense->rails))
>> + return 0444;
>
> no fallthrough here.
>
>> + case hwmon_power_max:
>> + case hwmon_power_crit:
>> + if (iccsense->power_w_max && iccsense->power_w_crit)
>> + return 0444;
>
> it makes sense to split those
>
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_temp_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>> +
>> + if (therm && therm->attr_get && therm->attr_set)
>> + if (nvkm_therm_temp_get(therm) < 0)
>> + return 0;
>
> I think you can merge those if statements
>
>> +
>> + switch (attr) {
>> + case hwmon_temp_input:
>> + return 0444;
>> + case hwmon_temp_max:
>> + case hwmon_temp_max_hyst:
>> + case hwmon_temp_crit:
>> + case hwmon_temp_crit_hyst:
>> + case hwmon_temp_emergency:
>> + case hwmon_temp_emergency_hyst:
>> + return 0644;
>
> I doubt we ever want to support changing those, please leave them as read only
>
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_pwm_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>> +
>> + if (therm && therm->attr_get)
>> + if (therm->fan_get && therm->fan_get(therm) < 0)
>> + return 0;
>
> merge the ifs
>
>> +
>> + switch (attr) {
>> + case hwmon_pwm_enable:
>> + case hwmon_pwm_input:
>> + return 0644;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_input_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
>> +
>> + if (!volt || nvkm_volt_get(volt) < 0)
>> + return 0;
>> +
>> + switch (attr) {
>> + case hwmon_in_input:
>> + case hwmon_in_label:
>> + case hwmon_in_min:
>> + case hwmon_in_max:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_fan_is_visible(const void *data, u32 attr, int channel)
>> +{
>> + struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>> + struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>> +
>> + if (!therm || !therm->attr_get || nvkm_therm_fan_sense(therm) < 0)
>> + return 0;
>> +
>> + switch (attr) {
>> + case hwmon_fan_input:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static umode_t
>> +nouveau_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
>> + int channel)
>
> Indentation
>
>> +{
>> + switch (type) {
>> + case hwmon_chip:
>> + return nouveau_chip_is_visible(data, attr, channel);
>> + case hwmon_temp:
>> + return nouveau_temp_is_visible(data, attr, channel);
>> + case hwmon_fan:
>> + return nouveau_fan_is_visible(data, attr, channel);
>> + case hwmon_in:
>> + return nouveau_input_is_visible(data, attr, channel);
>> + case hwmon_pwm:
>> + return nouveau_pwm_is_visible(data, attr, channel);
>> + case hwmon_power:
>> + return nouveau_power_is_visible(data, attr, channel);
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static char *input_label = "GPU core";
>> +
>> +static int
>> +nouveau_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>> + int channel, char **buf)
>
> Indentation
>
>> +{
>> + if (type == hwmon_in && attr == hwmon_in_label) {
>> + *buf = input_label;
>> + return 0;
>> + }
>> +
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static const struct hwmon_ops nouveau_hwmon_ops = {
>> + .is_visible = nouveau_is_visible,
>> + .read = NULL,
>> + .read_string = nouveau_read_string,
>> + .write = NULL,
>> +};
>> +
>> +static const struct hwmon_chip_info nouveau_chip_info = {
>> + .ops = &nouveau_hwmon_ops,
>> + .info = nouveau_info,
>> +};
>> #endif
>>
>> int
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-14 7:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-13 9:08 [PATCH 1/4] nouveau_hwmon: migrate to hwmon_device_register_with_info Oscar Salvador
2017-04-13 23:21 ` Karol Herbst
2017-04-14 7:14 ` [Nouveau] " Oscar Salvador
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.