<div dir="auto">I got what you meant. I"ll fix it</div><div class="gmail_extra"> <div class="gmail_quote">El dia 20/04/2017 08:47, "Oscar Salvador" <<a href="mailto:osalvador.vilardaga@gmail.com">osalvador.vilardaga@gmail.com</a>> va escriure: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Karol, I don't get what you mean with return due to fallthrough. I mean, I know what is it, but I don't see how I can do it there. Moving the check before the switch looks like that: if (!iccsense) return 0; switch (attr) { case hwmon_power_input: if (iccsense->data_valid && !list_empty(&iccsense->rails)) return 0444; case hwmon_power_max: if (iccsense->power_w_max) return 0444; case hwmon_power_crit: if (iccsense->power_w_crit) return 0444; default: return 0; } Could you drop me a hint? On 18 April 2017 at 09:56, Karol Herbst <<a href="mailto:karolherbst@gmail.com">karolherbst@gmail.com</a>> wrote: > 2017-04-17 9:47 GMT+02:00 Oscar Salvador <<a href="mailto:osalvador.vilardaga@gmail.com">osalvador.vilardaga@gmail.com</a>>: >> This patch introduces the nouveau_hwmon_ops structure, sets up >> .is_visible and .read_string operations and adds all the functions >> for these operations. >> This is also a preparation for the next patches, where most of the >> work is being done. >> This code doesn't interacture with the old one. >> It's just to make easier the review of all patches. >> --- >> drivers/gpu/drm/nouveau/nouveau_hwmon.c | 166 ++++++++++++++++++++++++++++++++ >> 1 file changed, 166 insertions(+) >> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c b/drivers/gpu/drm/nouveau/nouveau_hwmon.c >> index 24b40c5..9b6423c 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c >> @@ -764,6 +764,172 @@ static const struct hwmon_channel_info *nouveau_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 && > > move the pointer check before the switch, because you need to check it > in every case (or move it to every case, but doing it once is > simplier/cleaner) > >> + iccsense->data_valid && >> + !list_empty(&iccsense->rails)) >> + return 0444; > > add return due to fallthrough > >> + case hwmon_power_max: >> + if (iccsense->power_w_max) >> + return 0444; > > add return due to fallthrough > >> + case hwmon_power_crit: >> + if (iccsense->power_w_crit) >> + return 0444; > > add return due to fallthrough > >> + 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 && >> + nvkm_therm_temp_get(therm) < 0) >> + return 0; >> + >> + switch (attr) { >> + case hwmon_temp_input: >> + 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 0444; >> + 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 && >> + 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 >> -- >> 2.1.4 >> >> _______________________________________________ >> Nouveau mailing list >> <a href="mailto:Nouveau@lists.freedesktop.org">Nouveau@lists.freedesktop.org</a> >> <a href="https://lists.freedesktop.org/mailman/listinfo/nouveau" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/nouveau</a> </blockquote></div></div>