On 05/02/2024 17:15, Guenter Roeck wrote: > On Mon, Feb 05, 2024 at 04:26:08PM +0100, Krzysztof Kozlowski wrote: >> On 05/02/2024 16:20, Cosmo Chou wrote: >>> This driver implements support for temperature monitoring of Astera Labs >>> PT5161L series PCIe retimer chips. >>> >>> This driver implementation originates from the CSDK available at >>> Link: https://github.com/facebook/openbmc/tree/helium/common/recipes-lib/retimer-v2.14 >>> The communication protocol utilized is based on the I2C/SMBus standard. >>> >>> Signed-off-by: Cosmo Chou <chou.cosmo@xxxxxxxxx> >>> --- > [ ... ] > >>> + >>> +static int __init pt5161l_init(void) >>> +{ >>> + pt5161l_debugfs_dir = debugfs_create_dir("pt5161l", NULL); >> >> Drivers don't need initcalls. For sure any debugfs should not be handled >> here but in probe. >> > > Lots of hwmon drivers have init functions, for basic chip detection of > Super-I/O chips (example: drivers/hwmon/nct6775-platform.c) and to create > a parent debugfs subdirectory for the driver. The probe function then adds > subdirecties per chip instantiation. Example for pmbus, in > drivers/hwmon/pmbus/pmbus_core.c: Core bus components are a bit different... > > static int __init pmbus_core_init(void) > { > pmbus_debugfs_dir = debugfs_create_dir("pmbus", NULL); > if (IS_ERR(pmbus_debugfs_dir)) > pmbus_debugfs_dir = NULL; > > return 0; > } > > static void __exit pmbus_core_exit(void) > { > debugfs_remove_recursive(pmbus_debugfs_dir); > } > > Are you saying this is all wrong ? What alternative would you suggest ? Just create parent directory in probe and only keep remove in __exit. But you are right that might not be much better approach. Best regards, Krzysztof
