On Thu, Apr 18, 2019 at 02:02:48PM -0400, Jean-Francois Dagenais wrote:
> 
> > On Apr 18, 2019, at 13:38, Guenter Roeck <[email protected]> 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

Reply via email to