Messages in this thread | | | Subject | Re: [PATCH 1/2] hwmon: (k8temp) update to use new hwmon registration API | From | Guenter Roeck <> | Date | Sat, 20 Jul 2019 14:12:08 -0700 |
| |
On 7/20/19 2:05 PM, Robert Karszniewicz wrote: > Hello Guenter. > > Thank you for your feedback! I am working on a version 2. > > On Sat Jul 20, 2019 at 7:08 AM Guenter Roeck wrote: >>> +static umode_t >>> +k8temp_is_visible(const void *drvdata, enum hwmon_sensor_types type, >>> + u32 attr, int channel) >>> +{ >>> + if (type != hwmon_temp) >>> + return 0; >> >> Not really needed since only temperature sensors are registered in the first place. >> >>> + >>> + if (attr != hwmon_temp_input) >>> + return 0; >>> + >> >> The idea here is to only create sensors if they actually exist, so I would expect >> something like the following here. > > I realised that in the probe() function I deleted all the code which > conditionally creates the sysfs files. Is that how it's supposed to be > done and that's exactly what is_visible() is for, or is the proper way > to still create the sysfs files conditionally, too? >
Yes, that is what the function is for. sysfs files are created in the core, based on the result from the call to the is_visible function. If the function returns 0, the respective sysfs file won't be created.
Thanks, Guenter
>> struct k8temp_data *data = drvdata; >> >> if (attr != hwmon_temp_input) >> return 0; >> >> if ((channel & 1) && !(data->sensorsp & SEL_PLACE)) >> return 0; >> >> if ((channel & 2) && !(data->sensorsp & SEL_CORE)) >> return 0; >> >>> + return 0444; >>> +} > >>> + if (data->swap_core_select) >>> + core = core ? 0 : 1; >> >> core = 1 - core; >> >> would accomplish the same without conditional. > > How do you like > core ^= 1; > ? > >>> static int k8temp_probe(struct pci_dev *pdev, >>> const struct pci_device_id *id) >>> { >>> - int err; >>> u8 scfg; >>> u32 temp; >>> u8 model, stepping; >>> struct k8temp_data *data; >>> + struct device *hwmon_dev; >>> >>> data = devm_kzalloc(&pdev->dev, sizeof(struct k8temp_data), GFP_KERNEL); >>> if (!data) >>> @@ -233,84 +274,23 @@ static int k8temp_probe(struct pci_dev *pdev, >>> >>> data->name = "k8temp"; >>> mutex_init(&data->update_lock); >>> - pci_set_drvdata(pdev, data); >>> - >>> - /* Register sysfs hooks */ >>> - err = device_create_file(&pdev->dev, >>> - &sensor_dev_attr_temp1_input.dev_attr); >>> - if (err) >>> - goto exit_remove; >>> - >>> - /* sensor can be changed and reports something */ >>> - if (data->sensorsp & SEL_PLACE) { >>> - err = device_create_file(&pdev->dev, >>> - &sensor_dev_attr_temp2_input.dev_attr); >>> - if (err) >>> - goto exit_remove; >>> - } >>> - >>> - /* core can be changed and reports something */ >>> - if (data->sensorsp & SEL_CORE) { >>> - err = device_create_file(&pdev->dev, >>> - &sensor_dev_attr_temp3_input.dev_attr); >>> - if (err) >>> - goto exit_remove; >>> - if (data->sensorsp & SEL_PLACE) { >>> - err = device_create_file(&pdev->dev, >>> - &sensor_dev_attr_temp4_input. >>> - dev_attr); >>> - if (err) >>> - goto exit_remove; >>> - } >>> - } >
| |