1*4882a593SmuzhiyunHow to Get Your Patch Accepted Into the Hwmon Subsystem 2*4882a593Smuzhiyun======================================================= 3*4882a593Smuzhiyun 4*4882a593SmuzhiyunThis text is a collection of suggestions for people writing patches or 5*4882a593Smuzhiyundrivers for the hwmon subsystem. Following these suggestions will greatly 6*4882a593Smuzhiyunincrease the chances of your change being accepted. 7*4882a593Smuzhiyun 8*4882a593Smuzhiyun 9*4882a593Smuzhiyun1. General 10*4882a593Smuzhiyun---------- 11*4882a593Smuzhiyun 12*4882a593Smuzhiyun* It should be unnecessary to mention, but please read and follow: 13*4882a593Smuzhiyun 14*4882a593Smuzhiyun - Documentation/process/submit-checklist.rst 15*4882a593Smuzhiyun - Documentation/process/submitting-drivers.rst 16*4882a593Smuzhiyun - Documentation/process/submitting-patches.rst 17*4882a593Smuzhiyun - Documentation/process/coding-style.rst 18*4882a593Smuzhiyun 19*4882a593Smuzhiyun* Please run your patch through 'checkpatch --strict'. There should be no 20*4882a593Smuzhiyun errors, no warnings, and few if any check messages. If there are any 21*4882a593Smuzhiyun messages, please be prepared to explain. 22*4882a593Smuzhiyun 23*4882a593Smuzhiyun* Please use the standard multi-line comment style. Do not mix C and C++ 24*4882a593Smuzhiyun style comments in a single driver (with the exception of the SPDX license 25*4882a593Smuzhiyun identifier). 26*4882a593Smuzhiyun 27*4882a593Smuzhiyun* If your patch generates checkpatch errors, warnings, or check messages, 28*4882a593Smuzhiyun please refrain from explanations such as "I prefer that coding style". 29*4882a593Smuzhiyun Keep in mind that each unnecessary message helps hiding a real problem, 30*4882a593Smuzhiyun and a consistent coding style makes it easier for others to understand 31*4882a593Smuzhiyun and review the code. 32*4882a593Smuzhiyun 33*4882a593Smuzhiyun* Please test your patch thoroughly. We are not your test group. 34*4882a593Smuzhiyun Sometimes a patch can not or not completely be tested because of missing 35*4882a593Smuzhiyun hardware. In such cases, you should test-build the code on at least one 36*4882a593Smuzhiyun architecture. If run-time testing was not achieved, it should be written 37*4882a593Smuzhiyun explicitly below the patch header. 38*4882a593Smuzhiyun 39*4882a593Smuzhiyun* If your patch (or the driver) is affected by configuration options such as 40*4882a593Smuzhiyun CONFIG_SMP, make sure it compiles for all configuration variants. 41*4882a593Smuzhiyun 42*4882a593Smuzhiyun 43*4882a593Smuzhiyun2. Adding functionality to existing drivers 44*4882a593Smuzhiyun------------------------------------------- 45*4882a593Smuzhiyun 46*4882a593Smuzhiyun* Make sure the documentation in Documentation/hwmon/<driver_name>.rst is up to 47*4882a593Smuzhiyun date. 48*4882a593Smuzhiyun 49*4882a593Smuzhiyun* Make sure the information in Kconfig is up to date. 50*4882a593Smuzhiyun 51*4882a593Smuzhiyun* If the added functionality requires some cleanup or structural changes, split 52*4882a593Smuzhiyun your patch into a cleanup part and the actual addition. This makes it easier 53*4882a593Smuzhiyun to review your changes, and to bisect any resulting problems. 54*4882a593Smuzhiyun 55*4882a593Smuzhiyun* Never mix bug fixes, cleanup, and functional enhancements in a single patch. 56*4882a593Smuzhiyun 57*4882a593Smuzhiyun 58*4882a593Smuzhiyun3. New drivers 59*4882a593Smuzhiyun-------------- 60*4882a593Smuzhiyun 61*4882a593Smuzhiyun* Running your patch or driver file(s) through checkpatch does not mean its 62*4882a593Smuzhiyun formatting is clean. If unsure about formatting in your new driver, run it 63*4882a593Smuzhiyun through Lindent. Lindent is not perfect, and you may have to do some minor 64*4882a593Smuzhiyun cleanup, but it is a good start. 65*4882a593Smuzhiyun 66*4882a593Smuzhiyun* Consider adding yourself to MAINTAINERS. 67*4882a593Smuzhiyun 68*4882a593Smuzhiyun* Document the driver in Documentation/hwmon/<driver_name>.rst. 69*4882a593Smuzhiyun 70*4882a593Smuzhiyun* Add the driver to Kconfig and Makefile in alphabetical order. 71*4882a593Smuzhiyun 72*4882a593Smuzhiyun* Make sure that all dependencies are listed in Kconfig. 73*4882a593Smuzhiyun 74*4882a593Smuzhiyun* Please list include files in alphabetic order. 75*4882a593Smuzhiyun 76*4882a593Smuzhiyun* Please align continuation lines with '(' on the previous line. 77*4882a593Smuzhiyun 78*4882a593Smuzhiyun* Avoid forward declarations if you can. Rearrange the code if necessary. 79*4882a593Smuzhiyun 80*4882a593Smuzhiyun* Avoid macros to generate groups of sensor attributes. It not only confuses 81*4882a593Smuzhiyun checkpatch, but also makes it more difficult to review the code. 82*4882a593Smuzhiyun 83*4882a593Smuzhiyun* Avoid calculations in macros and macro-generated functions. While such macros 84*4882a593Smuzhiyun may save a line or so in the source, it obfuscates the code and makes code 85*4882a593Smuzhiyun review more difficult. It may also result in code which is more complicated 86*4882a593Smuzhiyun than necessary. Use inline functions or just regular functions instead. 87*4882a593Smuzhiyun 88*4882a593Smuzhiyun* Limit the number of kernel log messages. In general, your driver should not 89*4882a593Smuzhiyun generate an error message just because a runtime operation failed. Report 90*4882a593Smuzhiyun errors to user space instead, using an appropriate error code. Keep in mind 91*4882a593Smuzhiyun that kernel error log messages not only fill up the kernel log, but also are 92*4882a593Smuzhiyun printed synchronously, most likely with interrupt disabled, often to a serial 93*4882a593Smuzhiyun console. Excessive logging can seriously affect system performance. 94*4882a593Smuzhiyun 95*4882a593Smuzhiyun* Use devres functions whenever possible to allocate resources. For rationale 96*4882a593Smuzhiyun and supported functions, please see Documentation/driver-api/driver-model/devres.rst. 97*4882a593Smuzhiyun If a function is not supported by devres, consider using devm_add_action(). 98*4882a593Smuzhiyun 99*4882a593Smuzhiyun* If the driver has a detect function, make sure it is silent. Debug messages 100*4882a593Smuzhiyun and messages printed after a successful detection are acceptable, but it 101*4882a593Smuzhiyun must not print messages such as "Chip XXX not found/supported". 102*4882a593Smuzhiyun 103*4882a593Smuzhiyun Keep in mind that the detect function will run for all drivers supporting an 104*4882a593Smuzhiyun address if a chip is detected on that address. Unnecessary messages will just 105*4882a593Smuzhiyun pollute the kernel log and not provide any value. 106*4882a593Smuzhiyun 107*4882a593Smuzhiyun* Provide a detect function if and only if a chip can be detected reliably. 108*4882a593Smuzhiyun 109*4882a593Smuzhiyun* Only the following I2C addresses shall be probed: 0x18-0x1f, 0x28-0x2f, 110*4882a593Smuzhiyun 0x48-0x4f, 0x58, 0x5c, 0x73 and 0x77. Probing other addresses is strongly 111*4882a593Smuzhiyun discouraged as it is known to cause trouble with other (non-hwmon) I2C 112*4882a593Smuzhiyun chips. If your chip lives at an address which can't be probed then the 113*4882a593Smuzhiyun device will have to be instantiated explicitly (which is always better 114*4882a593Smuzhiyun anyway.) 115*4882a593Smuzhiyun 116*4882a593Smuzhiyun* Avoid writing to chip registers in the detect function. If you have to write, 117*4882a593Smuzhiyun only do it after you have already gathered enough data to be certain that the 118*4882a593Smuzhiyun detection is going to be successful. 119*4882a593Smuzhiyun 120*4882a593Smuzhiyun Keep in mind that the chip might not be what your driver believes it is, and 121*4882a593Smuzhiyun writing to it might cause a bad misconfiguration. 122*4882a593Smuzhiyun 123*4882a593Smuzhiyun* Make sure there are no race conditions in the probe function. Specifically, 124*4882a593Smuzhiyun completely initialize your chip and your driver first, then register with 125*4882a593Smuzhiyun the hwmon subsystem. 126*4882a593Smuzhiyun 127*4882a593Smuzhiyun* Use devm_hwmon_device_register_with_info() or, if your driver needs a remove 128*4882a593Smuzhiyun function, hwmon_device_register_with_info() to register your driver with the 129*4882a593Smuzhiyun hwmon subsystem. Try using devm_add_action() instead of a remove function if 130*4882a593Smuzhiyun possible. Do not use hwmon_device_register(). 131*4882a593Smuzhiyun 132*4882a593Smuzhiyun* Your driver should be buildable as module. If not, please be prepared to 133*4882a593Smuzhiyun explain why it has to be built into the kernel. 134*4882a593Smuzhiyun 135*4882a593Smuzhiyun* Do not provide support for deprecated sysfs attributes. 136*4882a593Smuzhiyun 137*4882a593Smuzhiyun* Do not create non-standard attributes unless really needed. If you have to use 138*4882a593Smuzhiyun non-standard attributes, or you believe you do, discuss it on the mailing list 139*4882a593Smuzhiyun first. Either case, provide a detailed explanation why you need the 140*4882a593Smuzhiyun non-standard attribute(s). 141*4882a593Smuzhiyun Standard attributes are specified in Documentation/hwmon/sysfs-interface.rst. 142*4882a593Smuzhiyun 143*4882a593Smuzhiyun* When deciding which sysfs attributes to support, look at the chip's 144*4882a593Smuzhiyun capabilities. While we do not expect your driver to support everything the 145*4882a593Smuzhiyun chip may offer, it should at least support all limits and alarms. 146*4882a593Smuzhiyun 147*4882a593Smuzhiyun* Last but not least, please check if a driver for your chip already exists 148*4882a593Smuzhiyun before starting to write a new driver. Especially for temperature sensors, 149*4882a593Smuzhiyun new chips are often variants of previously released chips. In some cases, 150*4882a593Smuzhiyun a presumably new chip may simply have been relabeled. 151