On Thu, Apr 18, 2019 at 02:02:48PM -0400, Jean-Francois Dagenais wrote: > > > On Apr 18, 2019, at 13:38, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > > >> +#if IS_ENABLED(CONFIG_THERMAL) > > > > This will result in missing symbols if THERMAL is built as module > > and this driver is built into the kernel. You'll have to adjust > > Kconfig dependencies accordingly. See other drivers for examples. > > Right! Was not a problem for me, but I do remember seing the "funny" > ifdefs around. > I know, it is annoying. There was an effort to make THERMAL boolean instead of tristate, but it looks like that has stalled or was rejected. > > > >> + data->cooling_dev = > >> + thermal_of_cooling_device_register(client->dev.of_node, > >> + id->name, data, > >> + &max6650_cooling_ops); > >> + if (IS_ERR(data->cooling_dev)) { > >> + err = PTR_ERR(data->cooling_dev); > >> + dev_err(&client->dev, > >> + "Failed to register as cooling device (%d)\n", err); > >> + return err; > > > > Why would it be fatal for the driver if this fails ? It wasn't > > fatal before. > > Mmmh, you are right. This assumes that all users of max6650 would now require to > be referred to in some thermal zone. Again, this was not a problem for my test > environment. > > Wow, two very egocentric issues. Will fix and send V3, thanks for the review! Not really egocentric. Just keep in mind that there are existing use cases. Thanks, Guenter