* [BUG] ipmi: NULL device printing in check_set_rcv_irq()
@ 2016-11-07 9:57 Corentin Labbe
2016-11-07 18:35 ` [PATCH] Move platform device creation earlier in the initialization minyard
0 siblings, 1 reply; 3+ messages in thread
From: Corentin Labbe @ 2016-11-07 9:57 UTC (permalink / raw)
To: minyard; +Cc: openipmi-developer, linux-kernel
Hello
On one of my servers I got this line in dmesg
(NULL device *): The BMC does not support setting the recv irq bit, compensating, but the BMC needs to be fixed.
This printing is done via a dev_warn(smi_info->dev,) in drivers/char/ipmi/ipmi_si_intf.c:check_clr_rcv_irq() but smi_info->dev is set only latter in try_smi_init().
I do not know which way is better to fix it, either move the code block which register the device before check_for_broken_irqs(), or perhaps replace dev_warn by pr_warn.
Regards
Corentin Labbe
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Move platform device creation earlier in the initialization
2016-11-07 9:57 [BUG] ipmi: NULL device printing in check_set_rcv_irq() Corentin Labbe
@ 2016-11-07 18:35 ` minyard
2016-11-24 15:59 ` Corentin Labbe
0 siblings, 1 reply; 3+ messages in thread
From: minyard @ 2016-11-07 18:35 UTC (permalink / raw)
To: Corentin Labbe; +Cc: openipmi-developer, linux-kernel, Corey Minyard
From: Corey Minyard <cminyard@mvista.com>
Some logs are printed out early using smi->dev, but on a platform device
that is not created until later. So move the creation of that device
structure earlier in the sequence so it can be used for printing.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
Corentin, can you try the attached patch? It moves the platform device
allocation earlier, but I don't really want to actually add the platform
device until later.
drivers/char/ipmi/ipmi_si_intf.c | 46 +++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index cb45108..751c281 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3502,6 +3502,7 @@ static int try_smi_init(struct smi_info *new_smi)
{
int rv = 0;
int i;
+ char *init_name = NULL;
printk(KERN_INFO PFX "Trying %s-specified %s state"
" machine at %s address 0x%lx, slave address 0x%x,"
@@ -3531,6 +3532,26 @@ static int try_smi_init(struct smi_info *new_smi)
goto out_err;
}
+ /* Do this early so it's available for logs. */
+ if (!new_smi->dev) {
+ init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", 0);
+
+ /*
+ * If we don't already have a device from something
+ * else (like PCI), then register a new one.
+ */
+ new_smi->pdev = platform_device_alloc("ipmi_si",
+ new_smi->intf_num);
+ if (!new_smi->pdev) {
+ pr_err(PFX "Unable to allocate platform device\n");
+ goto out_err;
+ }
+ new_smi->dev = &new_smi->pdev->dev;
+ new_smi->dev->driver = &ipmi_driver.driver;
+ /* Nulled by device_add() */
+ new_smi->dev->init_name = init_name;
+ }
+
/* Allocate the state machine's data and initialize it. */
new_smi->si_sm = kmalloc(new_smi->handlers->size(), GFP_KERNEL);
if (!new_smi->si_sm) {
@@ -3604,21 +3625,7 @@ static int try_smi_init(struct smi_info *new_smi)
atomic_set(&new_smi->req_events, 1);
}
- if (!new_smi->dev) {
- /*
- * If we don't already have a device from something
- * else (like PCI), then register a new one.
- */
- new_smi->pdev = platform_device_alloc("ipmi_si",
- new_smi->intf_num);
- if (!new_smi->pdev) {
- printk(KERN_ERR PFX
- "Unable to allocate platform device\n");
- goto out_err;
- }
- new_smi->dev = &new_smi->pdev->dev;
- new_smi->dev->driver = &ipmi_driver.driver;
-
+ if (new_smi->pdev) {
rv = platform_device_add(new_smi->pdev);
if (rv) {
printk(KERN_ERR PFX
@@ -3668,6 +3675,9 @@ static int try_smi_init(struct smi_info *new_smi)
dev_info(new_smi->dev, "IPMI %s interface initialized\n",
si_to_str[new_smi->si_type]);
+ WARN_ON(new_smi->dev->init_name != NULL);
+ kfree(init_name);
+
return 0;
out_err_stop_timer:
@@ -3712,8 +3722,14 @@ static int try_smi_init(struct smi_info *new_smi)
if (new_smi->dev_registered) {
platform_device_unregister(new_smi->pdev);
new_smi->dev_registered = false;
+ new_smi->pdev = NULL;
+ } else if (new_smi->pdev) {
+ platform_device_put(new_smi->pdev);
+ new_smi->pdev = NULL;
}
+ kfree(init_name);
+
return rv;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Move platform device creation earlier in the initialization
2016-11-07 18:35 ` [PATCH] Move platform device creation earlier in the initialization minyard
@ 2016-11-24 15:59 ` Corentin Labbe
0 siblings, 0 replies; 3+ messages in thread
From: Corentin Labbe @ 2016-11-24 15:59 UTC (permalink / raw)
To: minyard; +Cc: openipmi-developer, linux-kernel, Corey Minyard
On Mon, Nov 07, 2016 at 12:35:10PM -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> Some logs are printed out early using smi->dev, but on a platform device
> that is not created until later. So move the creation of that device
> structure earlier in the sequence so it can be used for printing.
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>
> Corentin, can you try the attached patch? It moves the platform device
> allocation earlier, but I don't really want to actually add the platform
> device until later.
>
It works.
After patching:
[30786.391668] IPMI System Interface driver.
[30786.391706] ipmi_si: probing via SMBIOS
[30786.391709] ipmi_si: SMBIOS: io 0xca8 regsize 1 spacing 4 irq 10
[30786.391712] ipmi_si: Adding SMBIOS-specified kcs state machine
[30786.391717] ipmi_si: Trying SMBIOS-specified kcs state machine at i/o address 0xca8, slave address 0x20, irq 10
[30786.582985] ipmi_si ipmi_si.0: The BMC does not support setting the recv irq bit, compensating, but the BMC needs to be fixed.
[30786.615091] ipmi_si ipmi_si.0: Using irq 10
[30786.645423] ipmi_si ipmi_si.0: Found new BMC (man_id: 0x0002a2, prod_id: 0x0100, dev_id: 0x20)
[30786.645481] ipmi_si ipmi_si.0: IPMI kcs interface initialized
Retrying with original ipmi_si:
[30813.525306] IPMI System Interface driver.
[30813.525363] ipmi_si: probing via SMBIOS
[30813.525368] ipmi_si: SMBIOS: io 0xca8 regsize 1 spacing 4 irq 10
[30813.525372] ipmi_si: Adding SMBIOS-specified kcs state machine
[30813.525379] ipmi_si: Trying SMBIOS-specified kcs state machine at i/o address 0xca8, slave address 0x20, irq 10
[30813.708519] (NULL device *): The BMC does not support setting the recv irq bit, compensating, but the BMC needs to be fixed.
[30813.740591] ipmi_si ipmi_si.0: Using irq 10
[30813.773238] ipmi_si ipmi_si.0: Found new BMC (man_id: 0x0002a2, prod_id: 0x0100, dev_id: 0x20)
[30813.773286] ipmi_si ipmi_si.0: IPMI kcs interface initialized
So Tested-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Thanks
> drivers/char/ipmi/ipmi_si_intf.c | 46 +++++++++++++++++++++++++++-------------
> 1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index cb45108..751c281 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3502,6 +3502,7 @@ static int try_smi_init(struct smi_info *new_smi)
> {
> int rv = 0;
> int i;
> + char *init_name = NULL;
>
> printk(KERN_INFO PFX "Trying %s-specified %s state"
> " machine at %s address 0x%lx, slave address 0x%x,"
> @@ -3531,6 +3532,26 @@ static int try_smi_init(struct smi_info *new_smi)
> goto out_err;
> }
>
> + /* Do this early so it's available for logs. */
> + if (!new_smi->dev) {
> + init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", 0);
> +
> + /*
> + * If we don't already have a device from something
> + * else (like PCI), then register a new one.
> + */
> + new_smi->pdev = platform_device_alloc("ipmi_si",
> + new_smi->intf_num);
> + if (!new_smi->pdev) {
> + pr_err(PFX "Unable to allocate platform device\n");
> + goto out_err;
> + }
> + new_smi->dev = &new_smi->pdev->dev;
> + new_smi->dev->driver = &ipmi_driver.driver;
> + /* Nulled by device_add() */
> + new_smi->dev->init_name = init_name;
> + }
> +
> /* Allocate the state machine's data and initialize it. */
> new_smi->si_sm = kmalloc(new_smi->handlers->size(), GFP_KERNEL);
> if (!new_smi->si_sm) {
> @@ -3604,21 +3625,7 @@ static int try_smi_init(struct smi_info *new_smi)
> atomic_set(&new_smi->req_events, 1);
> }
>
> - if (!new_smi->dev) {
> - /*
> - * If we don't already have a device from something
> - * else (like PCI), then register a new one.
> - */
> - new_smi->pdev = platform_device_alloc("ipmi_si",
> - new_smi->intf_num);
> - if (!new_smi->pdev) {
> - printk(KERN_ERR PFX
> - "Unable to allocate platform device\n");
> - goto out_err;
> - }
> - new_smi->dev = &new_smi->pdev->dev;
> - new_smi->dev->driver = &ipmi_driver.driver;
> -
> + if (new_smi->pdev) {
> rv = platform_device_add(new_smi->pdev);
> if (rv) {
> printk(KERN_ERR PFX
> @@ -3668,6 +3675,9 @@ static int try_smi_init(struct smi_info *new_smi)
> dev_info(new_smi->dev, "IPMI %s interface initialized\n",
> si_to_str[new_smi->si_type]);
>
> + WARN_ON(new_smi->dev->init_name != NULL);
> + kfree(init_name);
> +
> return 0;
>
> out_err_stop_timer:
> @@ -3712,8 +3722,14 @@ static int try_smi_init(struct smi_info *new_smi)
> if (new_smi->dev_registered) {
> platform_device_unregister(new_smi->pdev);
> new_smi->dev_registered = false;
> + new_smi->pdev = NULL;
> + } else if (new_smi->pdev) {
> + platform_device_put(new_smi->pdev);
> + new_smi->pdev = NULL;
> }
>
> + kfree(init_name);
> +
> return rv;
> }
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-24 15:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-07 9:57 [BUG] ipmi: NULL device printing in check_set_rcv_irq() Corentin Labbe
2016-11-07 18:35 ` [PATCH] Move platform device creation earlier in the initialization minyard
2016-11-24 15:59 ` Corentin Labbe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).