On 04/27/2016 08:24 AM, Mark Salter wrote:
> On Tue, 2016-04-26 at 23:31 -0500, [email protected] wrote:
>> From: Corey Minyard <[email protected]>
>>
>> Commit d61a3ead2680 ("[PATCH] IPMI: reserve I/O ports separately")
>> changed the way I/O ports were reserved and includes this comment in
>> log:
>>
>>   Some BIOSes reserve disjoint I/O regions in their ACPI tables for the IPMI
>>   controller.  This causes problems when trying to register the entire I/O
>>   region.  Therefore we must register each I/O port separately.
>>
>> There is a similar problem with memio regions on an arm64 platform
>> (AMD Seattle). Where I see:
>>
>>   ipmi message handler version 39.2
>>   ipmi_si AMDI0300:00: probing via device tree
>>   ipmi_si AMDI0300:00: ipmi_si: probing via ACPI
>>   ipmi_si AMDI0300:00: [mem 0xe0010000] regsize 1 spacing 4 irq 23
>>   ipmi_si: Adding ACPI-specified kcs state machine
>>   IPMI System Interface driver.
>>   ipmi_si: Trying ACPI-specified kcs state machine at mem \
>>            address 0xe0010000, slave address 0x0, irq 23
>>   ipmi_si: Could not set up I/O space
>>
>> The problem is that the ACPI core registers disjoint regions for the
>> platform device:
>>
>> e0010000-e0010000 : AMDI0300:00
>> e0010004-e0010004 : AMDI0300:00
>>
>> and the ipmi_si driver tries to register one region e0010000-e0010004.
>>
>> Based on a patch from Mark Salter <[email protected]>
>>
>> Signed-off-by: Corey Minyard <[email protected]>
>> ---
> This works for me as well.
>
> Tested-by: Mark Salter <[email protected]>

Thanks a bunch for testing this.  Queued for 4.7 and in linux-next.

-corey

>>   drivers/char/ipmi/ipmi_si_intf.c | 40 
>> +++++++++++++++++++++++++++-------------
>>   1 file changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/ipmi_si_intf.c 
>> b/drivers/char/ipmi/ipmi_si_intf.c
>> index 6ecf9af..a815044 100644
>> --- a/drivers/char/ipmi/ipmi_si_intf.c
>> +++ b/drivers/char/ipmi/ipmi_si_intf.c
>> @@ -1637,25 +1637,28 @@ static void mem_outq(const struct si_sm_io *io, 
>> unsigned int offset,
>>   }
>>   #endif
>>   
>> -static void mem_cleanup(struct smi_info *info)
>> +static void mem_region_cleanup(struct smi_info *info, int num)
>>   {
>>      unsigned long addr = info->io.addr_data;
>> -    int           mapsize;
>> +    int idx;
>> +
>> +    for (idx = 0; idx < num; idx++)
>> +            release_mem_region(addr + idx * info->io.regspacing,
>> +                               info->io.regsize);
>> +}
>>   
>> +static void mem_cleanup(struct smi_info *info)
>> +{
>>      if (info->io.addr) {
>>              iounmap(info->io.addr);
>> -
>> -            mapsize = ((info->io_size * info->io.regspacing)
>> -                       - (info->io.regspacing - info->io.regsize));
>> -
>> -            release_mem_region(addr, mapsize);
>> +            mem_region_cleanup(info, info->io_size);
>>      }
>>   }
>>   
>>   static int mem_setup(struct smi_info *info)
>>   {
>>      unsigned long addr = info->io.addr_data;
>> -    int           mapsize;
>> +    int           mapsize, idx;
>>   
>>      if (!addr)
>>              return -ENODEV;
>> @@ -1692,6 +1695,21 @@ static int mem_setup(struct smi_info *info)
>>      }
>>   
>>      /*
>> +     * Some BIOSes reserve disjoint memory regions in their ACPI
>> +     * tables.  This causes problems when trying to request the
>> +     * entire region.  Therefore we must request each register
>> +     * separately.
>> +     */
>> +    for (idx = 0; idx < info->io_size; idx++) {
>> +            if (request_mem_region(addr + idx * info->io.regspacing,
>> +                                   info->io.regsize, DEVICE_NAME) == NULL) {
>> +                    /* Undo allocations */
>> +                    mem_region_cleanup(info, idx);
>> +                    return -EIO;
>> +            }
>> +    }
>> +
>> +    /*
>>       * Calculate the total amount of memory to claim.  This is an
>>       * unusual looking calculation, but it avoids claiming any
>>       * more memory than it has to.  It will claim everything
>> @@ -1700,13 +1718,9 @@ static int mem_setup(struct smi_info *info)
>>       */
>>      mapsize = ((info->io_size * info->io.regspacing)
>>                 - (info->io.regspacing - info->io.regsize));
>> -
>> -    if (request_mem_region(addr, mapsize, DEVICE_NAME) == NULL)
>> -            return -EIO;
>> -
>>      info->io.addr = ioremap(addr, mapsize);
>>      if (info->io.addr == NULL) {
>> -            release_mem_region(addr, mapsize);
>> +            mem_region_cleanup(info, info->io_size);
>>              return -EIO;
>>      }
>>      return 0;


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Reply via email to