On Mon, Apr 29 2024 at 12:45:19 PM -05:00:00, Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
On 4/29/2024 11:48, Lyndon Sanche wrote:
#include <linux/i8042.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/platform_profile.h>
+#include <linux/bitfield.h>
These should be inserted in alphabetical order.
Agree
+
+ // Clean up but do not fail
Switch comment style to /* */
Agree
+ if (platform_profile_register(thermal_handler))
+ kfree(thermal_handler);
Don't you also want to return an error in this case? Because this
means that the platform supports thermal modes but it failed to setup
due to other issues.
It's different than the case of no supported modes in which case
returning 0 makes sense.
Maybe like this:
ret = platform_profile_register(thermal_handler);
if (ret)
kfree(thermal_handler);
return ret;
Good idea, I will propogate this error. Failure of module will then
occur on allocation or platform_profile error.
goto fail_rfkill;
}
+ // Do not fail module if thermal modes not supported, just skip
Switch comment style to /* */
Agree.
Thank you for this feedback.
Lyndon
