1c3a2f0adSGuenter RoeckHow to Get Your Patch Accepted Into the Hwmon Subsystem 2b04f2f7dSMauro Carvalho Chehab======================================================= 3c3a2f0adSGuenter Roeck 4ac20ad14SMasanari IidaThis text is a collection of suggestions for people writing patches or 5c3a2f0adSGuenter Roeckdrivers for the hwmon subsystem. Following these suggestions will greatly 6c3a2f0adSGuenter Roeckincrease the chances of your change being accepted. 7c3a2f0adSGuenter Roeck 8c3a2f0adSGuenter Roeck 9c3a2f0adSGuenter Roeck1. General 10c3a2f0adSGuenter Roeck---------- 11c3a2f0adSGuenter Roeck 12b04f2f7dSMauro Carvalho Chehab* It should be unnecessary to mention, but please read and follow: 13b04f2f7dSMauro Carvalho Chehab 14b04f2f7dSMauro Carvalho Chehab - Documentation/process/submit-checklist.rst 15b04f2f7dSMauro Carvalho Chehab - Documentation/process/submitting-patches.rst 16b04f2f7dSMauro Carvalho Chehab - Documentation/process/coding-style.rst 17c3a2f0adSGuenter Roeck 18165720d9SGuenter Roeck* Please run your patch through 'checkpatch --strict'. There should be no 19165720d9SGuenter Roeck errors, no warnings, and few if any check messages. If there are any 20165720d9SGuenter Roeck messages, please be prepared to explain. 21165720d9SGuenter Roeck 224e19e72fSGuenter Roeck* Please use the standard multi-line comment style. Do not mix C and C++ 234e19e72fSGuenter Roeck style comments in a single driver (with the exception of the SPDX license 244e19e72fSGuenter Roeck identifier). 254e19e72fSGuenter Roeck 26165720d9SGuenter Roeck* If your patch generates checkpatch errors, warnings, or check messages, 27165720d9SGuenter Roeck please refrain from explanations such as "I prefer that coding style". 28165720d9SGuenter Roeck Keep in mind that each unnecessary message helps hiding a real problem, 29165720d9SGuenter Roeck and a consistent coding style makes it easier for others to understand 30165720d9SGuenter Roeck and review the code. 31c3a2f0adSGuenter Roeck 32c3a2f0adSGuenter Roeck* Please test your patch thoroughly. We are not your test group. 33c3a2f0adSGuenter Roeck Sometimes a patch can not or not completely be tested because of missing 34c3a2f0adSGuenter Roeck hardware. In such cases, you should test-build the code on at least one 35c3a2f0adSGuenter Roeck architecture. If run-time testing was not achieved, it should be written 36c3a2f0adSGuenter Roeck explicitly below the patch header. 37c3a2f0adSGuenter Roeck 38c3a2f0adSGuenter Roeck* If your patch (or the driver) is affected by configuration options such as 3940b31360SStephen Rothwell CONFIG_SMP, make sure it compiles for all configuration variants. 40c3a2f0adSGuenter Roeck 41c3a2f0adSGuenter Roeck 42c3a2f0adSGuenter Roeck2. Adding functionality to existing drivers 43c3a2f0adSGuenter Roeck------------------------------------------- 44c3a2f0adSGuenter Roeck 457ebd8b66SMauro Carvalho Chehab* Make sure the documentation in Documentation/hwmon/<driver_name>.rst is up to 46c3a2f0adSGuenter Roeck date. 47c3a2f0adSGuenter Roeck 48c3a2f0adSGuenter Roeck* Make sure the information in Kconfig is up to date. 49c3a2f0adSGuenter Roeck 50c3a2f0adSGuenter Roeck* If the added functionality requires some cleanup or structural changes, split 51c3a2f0adSGuenter Roeck your patch into a cleanup part and the actual addition. This makes it easier 52c3a2f0adSGuenter Roeck to review your changes, and to bisect any resulting problems. 53c3a2f0adSGuenter Roeck 54c3a2f0adSGuenter Roeck* Never mix bug fixes, cleanup, and functional enhancements in a single patch. 55c3a2f0adSGuenter Roeck 56c3a2f0adSGuenter Roeck 57c3a2f0adSGuenter Roeck3. New drivers 58c3a2f0adSGuenter Roeck-------------- 59c3a2f0adSGuenter Roeck 60c3a2f0adSGuenter Roeck* Running your patch or driver file(s) through checkpatch does not mean its 61c3a2f0adSGuenter Roeck formatting is clean. If unsure about formatting in your new driver, run it 62c3a2f0adSGuenter Roeck through Lindent. Lindent is not perfect, and you may have to do some minor 63c3a2f0adSGuenter Roeck cleanup, but it is a good start. 64c3a2f0adSGuenter Roeck 65c3a2f0adSGuenter Roeck* Consider adding yourself to MAINTAINERS. 66c3a2f0adSGuenter Roeck 677ebd8b66SMauro Carvalho Chehab* Document the driver in Documentation/hwmon/<driver_name>.rst. 68c3a2f0adSGuenter Roeck 69c3a2f0adSGuenter Roeck* Add the driver to Kconfig and Makefile in alphabetical order. 70c3a2f0adSGuenter Roeck 7107d33600SKees Cook* Make sure that all dependencies are listed in Kconfig. 72c3a2f0adSGuenter Roeck 73165720d9SGuenter Roeck* Please list include files in alphabetic order. 74165720d9SGuenter Roeck 75165720d9SGuenter Roeck* Please align continuation lines with '(' on the previous line. 76165720d9SGuenter Roeck 77c3a2f0adSGuenter Roeck* Avoid forward declarations if you can. Rearrange the code if necessary. 78c3a2f0adSGuenter Roeck 79165720d9SGuenter Roeck* Avoid macros to generate groups of sensor attributes. It not only confuses 80165720d9SGuenter Roeck checkpatch, but also makes it more difficult to review the code. 81165720d9SGuenter Roeck 82c3a2f0adSGuenter Roeck* Avoid calculations in macros and macro-generated functions. While such macros 83c3a2f0adSGuenter Roeck may save a line or so in the source, it obfuscates the code and makes code 84c3a2f0adSGuenter Roeck review more difficult. It may also result in code which is more complicated 85c3a2f0adSGuenter Roeck than necessary. Use inline functions or just regular functions instead. 86c3a2f0adSGuenter Roeck 87165720d9SGuenter Roeck* Limit the number of kernel log messages. In general, your driver should not 88165720d9SGuenter Roeck generate an error message just because a runtime operation failed. Report 89165720d9SGuenter Roeck errors to user space instead, using an appropriate error code. Keep in mind 90165720d9SGuenter Roeck that kernel error log messages not only fill up the kernel log, but also are 91165720d9SGuenter Roeck printed synchronously, most likely with interrupt disabled, often to a serial 92165720d9SGuenter Roeck console. Excessive logging can seriously affect system performance. 93165720d9SGuenter Roeck 943fafb0ceSGuenter Roeck* Use devres functions whenever possible to allocate resources. For rationale 95fe34c89dSMauro Carvalho Chehab and supported functions, please see Documentation/driver-api/driver-model/devres.rst. 96165720d9SGuenter Roeck If a function is not supported by devres, consider using devm_add_action(). 973fafb0ceSGuenter Roeck 98c3a2f0adSGuenter Roeck* If the driver has a detect function, make sure it is silent. Debug messages 99c3a2f0adSGuenter Roeck and messages printed after a successful detection are acceptable, but it 100c3a2f0adSGuenter Roeck must not print messages such as "Chip XXX not found/supported". 101c3a2f0adSGuenter Roeck 102c3a2f0adSGuenter Roeck Keep in mind that the detect function will run for all drivers supporting an 103c3a2f0adSGuenter Roeck address if a chip is detected on that address. Unnecessary messages will just 104c3a2f0adSGuenter Roeck pollute the kernel log and not provide any value. 105c3a2f0adSGuenter Roeck 106c3a2f0adSGuenter Roeck* Provide a detect function if and only if a chip can be detected reliably. 107c3a2f0adSGuenter Roeck 10897607f98SJean Delvare* Only the following I2C addresses shall be probed: 0x18-0x1f, 0x28-0x2f, 10997607f98SJean Delvare 0x48-0x4f, 0x58, 0x5c, 0x73 and 0x77. Probing other addresses is strongly 11097607f98SJean Delvare discouraged as it is known to cause trouble with other (non-hwmon) I2C 11197607f98SJean Delvare chips. If your chip lives at an address which can't be probed then the 11297607f98SJean Delvare device will have to be instantiated explicitly (which is always better 11397607f98SJean Delvare anyway.) 11497607f98SJean Delvare 115c3a2f0adSGuenter Roeck* Avoid writing to chip registers in the detect function. If you have to write, 116c3a2f0adSGuenter Roeck only do it after you have already gathered enough data to be certain that the 117c3a2f0adSGuenter Roeck detection is going to be successful. 118c3a2f0adSGuenter Roeck 119c3a2f0adSGuenter Roeck Keep in mind that the chip might not be what your driver believes it is, and 120c3a2f0adSGuenter Roeck writing to it might cause a bad misconfiguration. 121c3a2f0adSGuenter Roeck 122c3a2f0adSGuenter Roeck* Make sure there are no race conditions in the probe function. Specifically, 123165720d9SGuenter Roeck completely initialize your chip and your driver first, then register with 124165720d9SGuenter Roeck the hwmon subsystem. 125165720d9SGuenter Roeck 1269b0cffa6SGuenter Roeck* Use devm_hwmon_device_register_with_info() or, if your driver needs a remove 1279b0cffa6SGuenter Roeck function, hwmon_device_register_with_info() to register your driver with the 128165720d9SGuenter Roeck hwmon subsystem. Try using devm_add_action() instead of a remove function if 129*5720a18bSGuenter Roeck possible. Do not use any of the deprecated registration functions. 130165720d9SGuenter Roeck 131165720d9SGuenter Roeck* Your driver should be buildable as module. If not, please be prepared to 132165720d9SGuenter Roeck explain why it has to be built into the kernel. 133c3a2f0adSGuenter Roeck 134c3a2f0adSGuenter Roeck* Do not provide support for deprecated sysfs attributes. 135c3a2f0adSGuenter Roeck 136c3a2f0adSGuenter Roeck* Do not create non-standard attributes unless really needed. If you have to use 137c3a2f0adSGuenter Roeck non-standard attributes, or you believe you do, discuss it on the mailing list 138c3a2f0adSGuenter Roeck first. Either case, provide a detailed explanation why you need the 139c3a2f0adSGuenter Roeck non-standard attribute(s). 1407ebd8b66SMauro Carvalho Chehab Standard attributes are specified in Documentation/hwmon/sysfs-interface.rst. 141c3a2f0adSGuenter Roeck 142c3a2f0adSGuenter Roeck* When deciding which sysfs attributes to support, look at the chip's 143c3a2f0adSGuenter Roeck capabilities. While we do not expect your driver to support everything the 144c3a2f0adSGuenter Roeck chip may offer, it should at least support all limits and alarms. 145c3a2f0adSGuenter Roeck 146c3a2f0adSGuenter Roeck* Last but not least, please check if a driver for your chip already exists 147c3a2f0adSGuenter Roeck before starting to write a new driver. Especially for temperature sensors, 148c3a2f0adSGuenter Roeck new chips are often variants of previously released chips. In some cases, 149c3a2f0adSGuenter Roeck a presumably new chip may simply have been relabeled. 150