Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1432388 > unrolled thread

Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init

Started byCorey Minyard <minyard@acm.org>
First post2016-06-28 03:40 +0200
Last post2016-06-28 14:20 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init Corey Minyard <minyard@acm.org> - 2016-06-28 03:40 +0200
    Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init Tony Camuso <tcamuso@redhat.com> - 2016-06-28 12:30 +0200
      Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init Tony Camuso <tcamuso@redhat.com> - 2016-06-28 14:20 +0200
      Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init Corey Minyard <minyard@acm.org> - 2016-06-28 14:20 +0200

#1432388 — Re: [PATCH 1/1] ipmi: remove trydefaults parameter and default init

FromCorey Minyard <minyard@acm.org>
Date2016-06-28 03:40 +0200
SubjectRe: [PATCH 1/1] ipmi: remove trydefaults parameter and default init
Message-ID<rOQam-8n3-13@gated-at.bofh.it>
Queued for 2.18, if that is ok.

Thanks,

-corey

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 <tcamuso@redhat.com>
> ---
>   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);

[toc] | [next] | [standalone]


#1432740

FromTony Camuso <tcamuso@redhat.com>
Date2016-06-28 12:30 +0200
Message-ID<rOYrh-5Ep-83@gated-at.bofh.it>
In reply to#1432388
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.

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 <tcamuso@redhat.com>
>> ---
>>   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);
>

[toc] | [prev] | [next] | [standalone]


#1432924

FromTony Camuso <tcamuso@redhat.com>
Date2016-06-28 14:20 +0200
Message-ID<rP09H-6V8-5@gated-at.bofh.it>
In reply to#1432740
On 06/28/2016 08:11 AM, Corey Minyard wrote:
> 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

That should suffice.

[toc] | [prev] | [next] | [standalone]


#1432927

FromCorey Minyard <minyard@acm.org>
Date2016-06-28 14:20 +0200
Message-ID<rP09H-6V8-7@gated-at.bofh.it>
In reply to#1432740
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 <tcamuso@redhat.com>
>>> ---
>>>   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);
>>
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web