On Tue, Jan 23, 2024 at 11:05:26PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@xxxxxxx> > > The sensor maybe disabled before kernel boot, so add change_mode > to support configuring the sensor to enabled state. > As mentioned by others, this will require a better explanation. It only affects thermal sensors, and the scope is not provided. Specifically, neither subject nor description explain that this change is primarily for thermal subsystem functionality, and the (non ?) impact on the hwmon device is not explained. > Signed-off-by: Peng Fan <peng.fan@xxxxxxx> > Reviewed-by: Cristian Marussi <cristian.marussi@xxxxxxx> As a general comment, I have not received the original patch, and instead pulled it from patchwork. I also did not receive the initial version of this patch. Maybe it is just me, but e-mail from kernel.org has been sporadic in the last month or so and I have been missing various e-mails. I would suggest for everyone to copy me directly if a response from me is expected or desired. Anyway, this change looks like it enables / disables individual temperature sensors. What is the expected result for the hwmon device, or in other words what happens if a sensor is disabled through the thermal subsystem and the "sensors" command is executed ? The impact (or lack of it) should be explained. Also, if my interpretation is correct, you'll need to explain why you did not (also) implement the hwmon "enable" attribute for temperature (and possibly other) sensors. Thanks, Guenter
