Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966118AbcKXP7U (ORCPT ); Thu, 24 Nov 2016 10:59:20 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:33594 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965905AbcKXP7S (ORCPT ); Thu, 24 Nov 2016 10:59:18 -0500 Date: Thu, 24 Nov 2016 16:59:14 +0100 From: Corentin Labbe To: minyard@acm.org Cc: openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org, Corey Minyard Subject: Re: [PATCH] Move platform device creation earlier in the initialization Message-ID: <20161124155914.GA27993@Red> References: <20161107095744.GA20037@Red> <1478543711-1132-1-git-send-email-minyard@acm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1478543711-1132-1-git-send-email-minyard@acm.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4934 Lines: 136 On Mon, Nov 07, 2016 at 12:35:10PM -0600, minyard@acm.org wrote: > From: Corey Minyard > > 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 > --- > > 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 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 >