All of lore.kernel.org
 help / color / mirror / Atom feed
From: Corey Minyard <minyard@acm.org>
To: "Darrick J. Wong" <djwong@us.ibm.com>
Cc: Jean Delvare <khali@linux-fr.org>,
	openipmi-developer@lists.sourceforge.net,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	lm-sensors <lm-sensors@lm-sensors.org>
Subject: Re: [lm-sensors] [Openipmi-developer] [PATCH] Fix platform drivers
Date: Fri, 07 Nov 2008 21:52:44 +0000	[thread overview]
Message-ID: <4914B8AC.7070202@acm.org> (raw)
In-Reply-To: <20081107191636.13288.17003.stgit@elm3a70.beaverton.ibm.com>

Ok, thanks for this.

Might this be a candidate for the stable-kernel?

Acked-by: Corey Minyard <cminyard@mvista.com>

Darrick J. Wong wrote:
> It turns out that if one registers a struct platform_device, the platform
> device code expects that platform_device.device->driver points to a struct
> driver inside a struct platform_driver.  This is not the case with the ipmi-si,
> ipmi-msghandler and ibmaem drivers, which causes the suspend/resume hook
> functions to jump off into nowhere, causing a crash.  Make this assumption hold
> true for these three drivers.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
>
>  drivers/char/ipmi/ipmi_msghandler.c |   20 +++++++++++---------
>  drivers/char/ipmi/ipmi_si_intf.c    |   16 +++++++++-------
>  drivers/hwmon/ibmaem.c              |   18 ++++++++++--------
>  3 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 8a59aaa..7a88dfd 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -422,9 +422,11 @@ struct ipmi_smi {
>  /**
>   * The driver model view of the IPMI messaging driver.
>   */
> -static struct device_driver ipmidriver = {
> -	.name = "ipmi",
> -	.bus = &platform_bus_type
> +static struct platform_driver ipmidriver = {
> +	.driver = {
> +		.name = "ipmi",
> +		.bus = &platform_bus_type
> +	}
>  };
>  static DEFINE_MUTEX(ipmidriver_mutex);
>  
> @@ -2384,9 +2386,9 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
>  	 * representing the interfaced BMC already
>  	 */
>  	if (bmc->guid_set)
> -		old_bmc = ipmi_find_bmc_guid(&ipmidriver, bmc->guid);
> +		old_bmc = ipmi_find_bmc_guid(&ipmidriver.driver, bmc->guid);
>  	else
> -		old_bmc = ipmi_find_bmc_prod_dev_id(&ipmidriver,
> +		old_bmc = ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
>  						    bmc->id.product_id,
>  						    bmc->id.device_id);
>  
> @@ -2416,7 +2418,7 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
>  		snprintf(name, sizeof(name),
>  			 "ipmi_bmc.%4.4x", bmc->id.product_id);
>  
> -		while (ipmi_find_bmc_prod_dev_id(&ipmidriver,
> +		while (ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
>  						 bmc->id.product_id,
>  						 bmc->id.device_id)) {
>  			if (!warn_printed) {
> @@ -2446,7 +2448,7 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
>  			       " Unable to allocate platform device\n");
>  			return -ENOMEM;
>  		}
> -		bmc->dev->dev.driver = &ipmidriver;
> +		bmc->dev->dev.driver = &ipmidriver.driver;
>  		dev_set_drvdata(&bmc->dev->dev, bmc);
>  		kref_init(&bmc->refcount);
>  
> @@ -4247,7 +4249,7 @@ static int ipmi_init_msghandler(void)
>  	if (initialized)
>  		return 0;
>  
> -	rv = driver_register(&ipmidriver);
> +	rv = driver_register(&ipmidriver.driver);
>  	if (rv) {
>  		printk(KERN_ERR PFX "Could not register IPMI driver\n");
>  		return rv;
> @@ -4308,7 +4310,7 @@ static __exit void cleanup_ipmi(void)
>  	remove_proc_entry(proc_ipmi_root->name, NULL);
>  #endif /* CONFIG_PROC_FS */
>  
> -	driver_unregister(&ipmidriver);
> +	driver_unregister(&ipmidriver.driver);
>  
>  	initialized = 0;
>  
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 3123bf5..3000135 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -114,9 +114,11 @@ static char *si_to_str[] = { "kcs", "smic", "bt" };
>  
>  #define DEVICE_NAME "ipmi_si"
>  
> -static struct device_driver ipmi_driver = {
> -	.name = DEVICE_NAME,
> -	.bus = &platform_bus_type
> +static struct platform_driver ipmi_driver = {
> +	.driver = {
> +		.name = DEVICE_NAME,
> +		.bus = &platform_bus_type
> +	}
>  };
>  
>  
> @@ -2868,7 +2870,7 @@ static int try_smi_init(struct smi_info *new_smi)
>  			goto out_err;
>  		}
>  		new_smi->dev = &new_smi->pdev->dev;
> -		new_smi->dev->driver = &ipmi_driver;
> +		new_smi->dev->driver = &ipmi_driver.driver;
>  
>  		rv = platform_device_add(new_smi->pdev);
>  		if (rv) {
> @@ -2983,7 +2985,7 @@ static __devinit int init_ipmi_si(void)
>  	initialized = 1;
>  
>  	/* Register the device drivers. */
> -	rv = driver_register(&ipmi_driver);
> +	rv = driver_register(&ipmi_driver.driver);
>  	if (rv) {
>  		printk(KERN_ERR
>  		       "init_ipmi_si: Unable to register driver: %d\n",
> @@ -3052,7 +3054,7 @@ static __devinit int init_ipmi_si(void)
>  #ifdef CONFIG_PPC_OF
>  		of_unregister_platform_driver(&ipmi_of_platform_driver);
>  #endif
> -		driver_unregister(&ipmi_driver);
> +		driver_unregister(&ipmi_driver.driver);
>  		printk(KERN_WARNING
>  		       "ipmi_si: Unable to find any System Interface(s)\n");
>  		return -ENODEV;
> @@ -3151,7 +3153,7 @@ static __exit void cleanup_ipmi_si(void)
>  		cleanup_one_si(e);
>  	mutex_unlock(&smi_infos_lock);
>  
> -	driver_unregister(&ipmi_driver);
> +	driver_unregister(&ipmi_driver.driver);
>  }
>  module_exit(cleanup_ipmi_si);
>  
> diff --git a/drivers/hwmon/ibmaem.c b/drivers/hwmon/ibmaem.c
> index 7b0ed5d..fe74609 100644
> --- a/drivers/hwmon/ibmaem.c
> +++ b/drivers/hwmon/ibmaem.c
> @@ -88,9 +88,11 @@
>  static DEFINE_IDR(aem_idr);
>  static DEFINE_SPINLOCK(aem_idr_lock);
>  
> -static struct device_driver aem_driver = {
> -	.name = DRVNAME,
> -	.bus = &platform_bus_type,
> +static struct platform_driver aem_driver = {
> +	.driver = {
> +		.name = DRVNAME,
> +		.bus = &platform_bus_type,
> +	}
>  };
>  
>  struct aem_ipmi_data {
> @@ -583,7 +585,7 @@ static int aem_init_aem1_inst(struct aem_ipmi_data *probe, u8 module_handle)
>  	data->pdev = platform_device_alloc(DRVNAME, data->id);
>  	if (!data->pdev)
>  		goto dev_err;
> -	data->pdev->dev.driver = &aem_driver;
> +	data->pdev->dev.driver = &aem_driver.driver;
>  
>  	res = platform_device_add(data->pdev);
>  	if (res)
> @@ -716,7 +718,7 @@ static int aem_init_aem2_inst(struct aem_ipmi_data *probe,
>  	data->pdev = platform_device_alloc(DRVNAME, data->id);
>  	if (!data->pdev)
>  		goto dev_err;
> -	data->pdev->dev.driver = &aem_driver;
> +	data->pdev->dev.driver = &aem_driver.driver;
>  
>  	res = platform_device_add(data->pdev);
>  	if (res)
> @@ -1085,7 +1087,7 @@ static int __init aem_init(void)
>  {
>  	int res;
>  
> -	res = driver_register(&aem_driver);
> +	res = driver_register(&aem_driver.driver);
>  	if (res) {
>  		printk(KERN_ERR "Can't register aem driver\n");
>  		return res;
> @@ -1097,7 +1099,7 @@ static int __init aem_init(void)
>  	return 0;
>  
>  ipmi_reg_err:
> -	driver_unregister(&aem_driver);
> +	driver_unregister(&aem_driver.driver);
>  	return res;
>  
>  }
> @@ -1107,7 +1109,7 @@ static void __exit aem_exit(void)
>  	struct aem_data *p1, *next1;
>  
>  	ipmi_smi_watcher_unregister(&driver_data.bmc_events);
> -	driver_unregister(&aem_driver);
> +	driver_unregister(&aem_driver.driver);
>  	list_for_each_entry_safe(p1, next1, &driver_data.aem_devices, list)
>  		aem_delete(p1);
>  }
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id\x100&url=/
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
>
>   


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

WARNING: multiple messages have this Message-ID (diff)
From: Corey Minyard <minyard@acm.org>
To: "Darrick J. Wong" <djwong@us.ibm.com>
Cc: Jean Delvare <khali@linux-fr.org>,
	openipmi-developer@lists.sourceforge.net,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	lm-sensors <lm-sensors@lm-sensors.org>
Subject: Re: [Openipmi-developer] [PATCH] Fix platform drivers that crash on suspend/resume
Date: Fri, 07 Nov 2008 15:52:44 -0600	[thread overview]
Message-ID: <4914B8AC.7070202@acm.org> (raw)
In-Reply-To: <20081107191636.13288.17003.stgit@elm3a70.beaverton.ibm.com>

Ok, thanks for this.

Might this be a candidate for the stable-kernel?

Acked-by: Corey Minyard <cminyard@mvista.com>

Darrick J. Wong wrote:
> It turns out that if one registers a struct platform_device, the platform
> device code expects that platform_device.device->driver points to a struct
> driver inside a struct platform_driver.  This is not the case with the ipmi-si,
> ipmi-msghandler and ibmaem drivers, which causes the suspend/resume hook
> functions to jump off into nowhere, causing a crash.  Make this assumption hold
> true for these three drivers.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
>
>  drivers/char/ipmi/ipmi_msghandler.c |   20 +++++++++++---------
>  drivers/char/ipmi/ipmi_si_intf.c    |   16 +++++++++-------
>  drivers/hwmon/ibmaem.c              |   18 ++++++++++--------
>  3 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 8a59aaa..7a88dfd 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -422,9 +422,11 @@ struct ipmi_smi {
>  /**
>   * The driver model view of the IPMI messaging driver.
>   */
> -static struct device_driver ipmidriver = {
> -	.name = "ipmi",
> -	.bus = &platform_bus_type
> +static struct platform_driver ipmidriver = {
> +	.driver = {
> +		.name = "ipmi",
> +		.bus = &platform_bus_type
> +	}
>  };
>  static DEFINE_MUTEX(ipmidriver_mutex);
>  
> @@ -2384,9 +2386,9 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
>  	 * representing the interfaced BMC already
>  	 */
>  	if (bmc->guid_set)
> -		old_bmc = ipmi_find_bmc_guid(&ipmidriver, bmc->guid);
> +		old_bmc = ipmi_find_bmc_guid(&ipmidriver.driver, bmc->guid);
>  	else
> -		old_bmc = ipmi_find_bmc_prod_dev_id(&ipmidriver,
> +		old_bmc = ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
>  						    bmc->id.product_id,
>  						    bmc->id.device_id);
>  
> @@ -2416,7 +2418,7 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
>  		snprintf(name, sizeof(name),
>  			 "ipmi_bmc.%4.4x", bmc->id.product_id);
>  
> -		while (ipmi_find_bmc_prod_dev_id(&ipmidriver,
> +		while (ipmi_find_bmc_prod_dev_id(&ipmidriver.driver,
>  						 bmc->id.product_id,
>  						 bmc->id.device_id)) {
>  			if (!warn_printed) {
> @@ -2446,7 +2448,7 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
>  			       " Unable to allocate platform device\n");
>  			return -ENOMEM;
>  		}
> -		bmc->dev->dev.driver = &ipmidriver;
> +		bmc->dev->dev.driver = &ipmidriver.driver;
>  		dev_set_drvdata(&bmc->dev->dev, bmc);
>  		kref_init(&bmc->refcount);
>  
> @@ -4247,7 +4249,7 @@ static int ipmi_init_msghandler(void)
>  	if (initialized)
>  		return 0;
>  
> -	rv = driver_register(&ipmidriver);
> +	rv = driver_register(&ipmidriver.driver);
>  	if (rv) {
>  		printk(KERN_ERR PFX "Could not register IPMI driver\n");
>  		return rv;
> @@ -4308,7 +4310,7 @@ static __exit void cleanup_ipmi(void)
>  	remove_proc_entry(proc_ipmi_root->name, NULL);
>  #endif /* CONFIG_PROC_FS */
>  
> -	driver_unregister(&ipmidriver);
> +	driver_unregister(&ipmidriver.driver);
>  
>  	initialized = 0;
>  
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 3123bf5..3000135 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -114,9 +114,11 @@ static char *si_to_str[] = { "kcs", "smic", "bt" };
>  
>  #define DEVICE_NAME "ipmi_si"
>  
> -static struct device_driver ipmi_driver = {
> -	.name = DEVICE_NAME,
> -	.bus = &platform_bus_type
> +static struct platform_driver ipmi_driver = {
> +	.driver = {
> +		.name = DEVICE_NAME,
> +		.bus = &platform_bus_type
> +	}
>  };
>  
>  
> @@ -2868,7 +2870,7 @@ static int try_smi_init(struct smi_info *new_smi)
>  			goto out_err;
>  		}
>  		new_smi->dev = &new_smi->pdev->dev;
> -		new_smi->dev->driver = &ipmi_driver;
> +		new_smi->dev->driver = &ipmi_driver.driver;
>  
>  		rv = platform_device_add(new_smi->pdev);
>  		if (rv) {
> @@ -2983,7 +2985,7 @@ static __devinit int init_ipmi_si(void)
>  	initialized = 1;
>  
>  	/* Register the device drivers. */
> -	rv = driver_register(&ipmi_driver);
> +	rv = driver_register(&ipmi_driver.driver);
>  	if (rv) {
>  		printk(KERN_ERR
>  		       "init_ipmi_si: Unable to register driver: %d\n",
> @@ -3052,7 +3054,7 @@ static __devinit int init_ipmi_si(void)
>  #ifdef CONFIG_PPC_OF
>  		of_unregister_platform_driver(&ipmi_of_platform_driver);
>  #endif
> -		driver_unregister(&ipmi_driver);
> +		driver_unregister(&ipmi_driver.driver);
>  		printk(KERN_WARNING
>  		       "ipmi_si: Unable to find any System Interface(s)\n");
>  		return -ENODEV;
> @@ -3151,7 +3153,7 @@ static __exit void cleanup_ipmi_si(void)
>  		cleanup_one_si(e);
>  	mutex_unlock(&smi_infos_lock);
>  
> -	driver_unregister(&ipmi_driver);
> +	driver_unregister(&ipmi_driver.driver);
>  }
>  module_exit(cleanup_ipmi_si);
>  
> diff --git a/drivers/hwmon/ibmaem.c b/drivers/hwmon/ibmaem.c
> index 7b0ed5d..fe74609 100644
> --- a/drivers/hwmon/ibmaem.c
> +++ b/drivers/hwmon/ibmaem.c
> @@ -88,9 +88,11 @@
>  static DEFINE_IDR(aem_idr);
>  static DEFINE_SPINLOCK(aem_idr_lock);
>  
> -static struct device_driver aem_driver = {
> -	.name = DRVNAME,
> -	.bus = &platform_bus_type,
> +static struct platform_driver aem_driver = {
> +	.driver = {
> +		.name = DRVNAME,
> +		.bus = &platform_bus_type,
> +	}
>  };
>  
>  struct aem_ipmi_data {
> @@ -583,7 +585,7 @@ static int aem_init_aem1_inst(struct aem_ipmi_data *probe, u8 module_handle)
>  	data->pdev = platform_device_alloc(DRVNAME, data->id);
>  	if (!data->pdev)
>  		goto dev_err;
> -	data->pdev->dev.driver = &aem_driver;
> +	data->pdev->dev.driver = &aem_driver.driver;
>  
>  	res = platform_device_add(data->pdev);
>  	if (res)
> @@ -716,7 +718,7 @@ static int aem_init_aem2_inst(struct aem_ipmi_data *probe,
>  	data->pdev = platform_device_alloc(DRVNAME, data->id);
>  	if (!data->pdev)
>  		goto dev_err;
> -	data->pdev->dev.driver = &aem_driver;
> +	data->pdev->dev.driver = &aem_driver.driver;
>  
>  	res = platform_device_add(data->pdev);
>  	if (res)
> @@ -1085,7 +1087,7 @@ static int __init aem_init(void)
>  {
>  	int res;
>  
> -	res = driver_register(&aem_driver);
> +	res = driver_register(&aem_driver.driver);
>  	if (res) {
>  		printk(KERN_ERR "Can't register aem driver\n");
>  		return res;
> @@ -1097,7 +1099,7 @@ static int __init aem_init(void)
>  	return 0;
>  
>  ipmi_reg_err:
> -	driver_unregister(&aem_driver);
> +	driver_unregister(&aem_driver.driver);
>  	return res;
>  
>  }
> @@ -1107,7 +1109,7 @@ static void __exit aem_exit(void)
>  	struct aem_data *p1, *next1;
>  
>  	ipmi_smi_watcher_unregister(&driver_data.bmc_events);
> -	driver_unregister(&aem_driver);
> +	driver_unregister(&aem_driver.driver);
>  	list_for_each_entry_safe(p1, next1, &driver_data.aem_devices, list)
>  		aem_delete(p1);
>  }
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
>
>   


  reply	other threads:[~2008-11-07 21:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-07 19:16 [lm-sensors] [PATCH] Fix platform drivers that crash on Darrick J. Wong
2008-11-07 19:16 ` [PATCH] Fix platform drivers that crash on suspend/resume Darrick J. Wong
2008-11-07 21:52 ` Corey Minyard [this message]
2008-11-07 21:52   ` [Openipmi-developer] " Corey Minyard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4914B8AC.7070202@acm.org \
    --to=minyard@acm.org \
    --cc=akpm@linux-foundation.org \
    --cc=djwong@us.ibm.com \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.