Re: [PATCH 2/4] hwmon: (coretemp) update hotplug condition check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chen,

On Thu, 15 Jul 2010 10:46:42 +0800, Chen Gong wrote:
> this patch fixes two errors about hotplug. One is for
> hotplug notifier. The other is unnecessary driver unregister.
> Because even none of online cpus supports coretemp, we can't
> assume new onlined cpu doesn't support it either. If related
> driver is unregistered there we have no chance to use coretemp
> from then on.
> 
> Signed-off-by: Chen Gong <gong.chen@xxxxxxxxxxxxxxx>
> ---
>  drivers/hwmon/coretemp.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index b89f6a2..7b7c5b8 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -514,10 +514,13 @@ static int __cpuinit coretemp_cpu_callback(struct notifier_block *nfb,
>  
>  	switch (action) {
>  	case CPU_ONLINE:
> +	case CPU_ONLINE_FROZEN:
>  	case CPU_DOWN_FAILED:
> +	case CPU_DOWN_FAILED_FROZEN:
>  		coretemp_device_add(cpu);
>  		break;
>  	case CPU_DOWN_PREPARE:
> +	case CPU_DOWN_PREPARE_FROZEN:
>  		coretemp_device_remove(cpu);
>  		break;
>  	}

Hmm. Please discuss this with Rafael (Cc'd). We already had 2 changes
in this area:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8bb7844286fb8c9fce6f65d8288aeb09d03a5e0d#patch22
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=561d9a969455cb009bb15b63e1d925dc527e7a9d

As you can see, Rafael has been removing what you are trying to add
back. And Rafael knows quite a bit when it comes to power management.
So I will not accept this patch unless he approves them.

> @@ -559,10 +562,6 @@ static int __init coretemp_init(void)
>  				" has no thermal sensor.\n", c->x86_model);
>  		}
>  	}
> -	if (list_empty(&pdev_list)) {
> -		err = -ENODEV;
> -		goto exit_driver_unreg;
> -	}
>  
>  #ifdef CONFIG_HOTPLUG_CPU
>  	register_hotcpu_notifier(&coretemp_cpu_notifier);
> @@ -577,8 +576,6 @@ exit_devices_unreg:
>  		kfree(p);
>  	}
>  	mutex_unlock(&pdev_list_mutex);
> -exit_driver_unreg:
> -	platform_driver_unregister(&coretemp_driver);
>  exit:
>  	return err;
>  }

I think the code is that way on purpose. The problem you mention is
theoretical and can't happen in practice. In practice, either all the
processor cores on the system are supported by the driver, or none is.
And if are running some code, then at least one core is up, so it will
detect itself.

If I'm wrong, then please present a realistic case where we may find no
supported core at some point in time, and one shows up later.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux