From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752697AbcF1MLx (ORCPT ); Tue, 28 Jun 2016 08:11:53 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:36587 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752112AbcF1MLv (ORCPT ); Tue, 28 Jun 2016 08:11:51 -0400 Reply-To: minyard@acm.org Subject: Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init References: <1466619748-20610-1-git-send-email-tcamuso@redhat.com> <5771D40C.9070609@acm.org> <6ef8e084-e554-5f5c-23aa-a7224be275b2@redhat.com> To: Tony Camuso , openipmi-developer@lists.sourceforge.net Cc: linux-kernel@vger.kernel.org From: Corey Minyard Message-ID: <5772697E.3080404@acm.org> Date: Tue, 28 Jun 2016 07:11:42 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <6ef8e084-e554-5f5c-23aa-a7224be275b2@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/28/2016 05:14 AM, Tony Camuso wrote: > On 06/27/2016 09:34 PM, Corey Minyard wrote: >> Queued for 2.18, if that is ok. >> >> Thanks, >> >> -corey > > Yes, as long as there's enough time to shake it out before it's merged > with Linus. I've done some testing on a couple archs, and I'll do more > on some others as the opportunity arises. > It's in linux-next, hopefully that provides enough shaking out. -corey > Regards, > Tony > > >> >> On 06/22/2016 01:22 PM, Tony Camuso wrote: >>> Parameter trydefaults=1 causes the ipmi_init to initialize ipmi through >>> the legacy port io space that was designated for ipmi. Architectures >>> that do not map legacy port io can panic when trydefaults=1. >>> >>> Rather than implement build-time conditional exceptions for each >>> architecture that does not map legacy port io, we have removed legacy >>> port io from the driver. >>> >>> Parameter 'trydefaults' has been removed. Attempts to use it hereafter >>> will evoke the "Unknown symbol in module, or unknown parameter" >>> message. >>> >>> The patch was built against a number of architectures and tested for >>> regressions and functionality on x86_64 and ARM64. >>> >>> Signed-off-by: Tony Camuso >>> --- >>> drivers/char/ipmi/ipmi_si_intf.c | 73 >>> ---------------------------------------- >>> 1 file changed, 73 deletions(-) >>> >>> diff --git a/drivers/char/ipmi/ipmi_si_intf.c >>> b/drivers/char/ipmi/ipmi_si_intf.c >>> index 7b1c412..a112c01 100644 >>> --- a/drivers/char/ipmi/ipmi_si_intf.c >>> +++ b/drivers/char/ipmi/ipmi_si_intf.c >>> @@ -1322,7 +1322,6 @@ static bool si_tryplatform = true; >>> #ifdef CONFIG_PCI >>> static bool si_trypci = true; >>> #endif >>> -static bool si_trydefaults = >>> IS_ENABLED(CONFIG_IPMI_SI_PROBE_DEFAULTS); >>> static char *si_type[SI_MAX_PARMS]; >>> #define MAX_SI_TYPE_STR 30 >>> static char si_type_str[MAX_SI_TYPE_STR]; >>> @@ -1371,10 +1370,6 @@ module_param_named(trypci, si_trypci, bool, 0); >>> MODULE_PARM_DESC(trypci, "Setting this to zero will disable the" >>> " default scan of the interfaces identified via pci"); >>> #endif >>> -module_param_named(trydefaults, si_trydefaults, bool, 0); >>> -MODULE_PARM_DESC(trydefaults, "Setting this to 'false' will disable >>> the" >>> - " default scan of the KCS and SMIC interface at the standard" >>> - " address"); >>> module_param_string(type, si_type_str, MAX_SI_TYPE_STR, 0); >>> MODULE_PARM_DESC(type, "Defines the type of each interface, each" >>> " interface separated by commas. The types are 'kcs'," >>> @@ -3461,62 +3456,6 @@ static inline void >>> wait_for_timer_and_thread(struct smi_info *smi_info) >>> del_timer_sync(&smi_info->si_timer); >>> } >>> -static const struct ipmi_default_vals >>> -{ >>> - const int type; >>> - const int port; >>> -} ipmi_defaults[] = >>> -{ >>> - { .type = SI_KCS, .port = 0xca2 }, >>> - { .type = SI_SMIC, .port = 0xca9 }, >>> - { .type = SI_BT, .port = 0xe4 }, >>> - { .port = 0 } >>> -}; >>> - >>> -static void default_find_bmc(void) >>> -{ >>> - struct smi_info *info; >>> - int i; >>> - >>> - for (i = 0; ; i++) { >>> - if (!ipmi_defaults[i].port) >>> - break; >>> -#ifdef CONFIG_PPC >>> - if (check_legacy_ioport(ipmi_defaults[i].port)) >>> - continue; >>> -#endif >>> - info = smi_info_alloc(); >>> - if (!info) >>> - return; >>> - >>> - info->addr_source = SI_DEFAULT; >>> - >>> - info->si_type = ipmi_defaults[i].type; >>> - info->io_setup = port_setup; >>> - info->io.addr_data = ipmi_defaults[i].port; >>> - info->io.addr_type = IPMI_IO_ADDR_SPACE; >>> - >>> - info->io.addr = NULL; >>> - info->io.regspacing = DEFAULT_REGSPACING; >>> - info->io.regsize = DEFAULT_REGSPACING; >>> - info->io.regshift = 0; >>> - >>> - if (add_smi(info) == 0) { >>> - if ((try_smi_init(info)) == 0) { >>> - /* Found one... */ >>> - printk(KERN_INFO PFX "Found default %s" >>> - " state machine at %s address 0x%lx\n", >>> - si_to_str[info->si_type], >>> - addr_space_to_str[info->io.addr_type], >>> - info->io.addr_data); >>> - } else >>> - cleanup_one_si(info); >>> - } else { >>> - kfree(info); >>> - } >>> - } >>> -} >>> - >>> static int is_new_interface(struct smi_info *info) >>> { >>> struct smi_info *e; >>> @@ -3844,8 +3783,6 @@ static int init_ipmi_si(void) >>> #ifdef CONFIG_PARISC >>> register_parisc_driver(&ipmi_parisc_driver); >>> parisc_registered = true; >>> - /* poking PC IO addresses will crash machine, don't do it */ >>> - si_trydefaults = 0; >>> #endif >>> /* We prefer devices with interrupts, but in the case of a >>> machine >>> @@ -3885,16 +3822,6 @@ static int init_ipmi_si(void) >>> if (type) >>> return 0; >>> - if (si_trydefaults) { >>> - mutex_lock(&smi_infos_lock); >>> - if (list_empty(&smi_infos)) { >>> - /* No BMC was found, try defaults. */ >>> - mutex_unlock(&smi_infos_lock); >>> - default_find_bmc(); >>> - } else >>> - mutex_unlock(&smi_infos_lock); >>> - } >>> - >>> mutex_lock(&smi_infos_lock); >>> if (unload_when_empty && list_empty(&smi_infos)) { >>> mutex_unlock(&smi_infos_lock); >> >