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

[Corey Minyard]

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@xxxxxxxxxx>
> > > ---
> > >   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);