On Mon, 2009-10-26 at 23:20 +0800, Bjorn Helgaas wrote: > On Monday 26 October 2009 07:33:45 am [email protected] wrote: > > From: Zhao Yakui <[email protected]> > > > > According to the IPMI 2.0 spec the IPMI system interface can be located with > > ACPI. One is located in SPMI table(Service Processor Management Interface > > table). Another is located in ACPI namespace. > > This patch is to locate the IPMI system interface in ACPI namespace and > > register it. > > It includes the following two steps: > > 1. enumerate the ACPI device tree to find the IPMI system interface > > The IPMI device type is IPI0001. When the device is found, it > > will continue to parse the corresponding resources. > > For example: > > interface type (KCS, BT, SMIC) (SSIF is not supported) > > interrupt number and type (_GPE or GSI) > > Memory or IO base address > > 2. register the IPMI system interface
> As far as I can tell, these patches are unchanged from the original > ones you posted here: > http://marc.info/?l=linux-acpi&m=125420227326811&w=2 > http://marc.info/?l=linux-acpi&m=125420227426814&w=2 I refresh them based on the latest upstream branch. Of course I do the small change about the second patch. It is mainly about the coding style. In fact you mention two issues about the two patches: 1: Load a PNP driver for it to register the IPMI system interface. This is about the first patch. 2. coding style( for example: comments, the definition about some variables). For the first issue: Before I start the first patch, I consider using the PNP device driver. But I find that it is so complex because of the following two points: 1. One is that we can't register the IPMI system interface if the boot option of "pnpacpi=off" is added. This will also depend on the PNP module. 2. The second is that there exist so many cases about the IPMI IO/memory resource definition. Maybe there exist both IO/memory resource definition for one IPMI device. In such case we can't know which should be selected. At the same time we have similar issues about the interrupt type. So I decide to parse the IO/memory/interrupt resource independently. Very sorry that I don't reply the above points in the discussion. For the second patch: I update the coding style about the comments. But I don't change the coding style about the variable definition as it doesn't matter. At the same time it is easy to understand by using the explicit definition. For example: if (acpi_disabled) return; thanks. yakui > Either you mistakenly posted the original patches again, or you > didn't address a single piece of the feedback we gave you. > > Please don't waste my time asking for comments if you're just going > to ignore them. > > It's OK if you think my suggestions are incorrect, but I expect you > to at least acknowledge them and explain why you disagree with me, so > we can have a discussion about it. > > Bjorn > > > Signed-off-by: Zhao Yakui <[email protected]> > > --- > > drivers/char/ipmi/ipmi_si_intf.c | 394 > > ++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 394 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > > b/drivers/char/ipmi/ipmi_si_intf.c > > index d2e6980..94d14bc 100644 > > --- a/drivers/char/ipmi/ipmi_si_intf.c > > +++ b/drivers/char/ipmi/ipmi_si_intf.c > > @@ -1813,6 +1813,35 @@ static __devinit void hardcode_find_bmc(void) > > * are no more. > > */ > > static int acpi_failure; > > +static LIST_HEAD(acpi_ipmi); > > + > > +struct acpi_device_ipmi { > > + struct list_head link; > > + u8 interfacetype; > > + /* > > + * Bit 0 - SCI interrupt supported > > + * Bit 1 - I/O APIC/SAPIC > > + */ > > + u8 interrupttype; > > + /* > > + * If bit 0 of InterruptType is set, then this is the SCI > > + * interrupt in the GPEx_STS register. > > + */ > > + u8 gpe; > > + /* > > + * If bit 1 of InterruptType is set, then this is the I/O > > + * APIC/SAPIC interrupt. > > + */ > > + u32 global_interrupt; > > + > > + /* The actual register address. */ > > + struct acpi_generic_address addr; > > + struct acpi_generic_address sm_addr; > > + > > + u16 ipmi_revision; > > + u8 resource_count; > > + struct device *dev; > > +}; > > > > /* For GPE-type interrupts. */ > > static u32 ipmi_acpi_gpe(void *context) > > @@ -2002,6 +2031,367 @@ static __devinit int try_init_acpi(struct SPMITable > > *spmi) > > return 0; > > } > > > > +static __devinit int try_init_acpi_device(struct acpi_device_ipmi *spmi) > > +{ > > + struct smi_info *info; > > + u8 addr_space; > > + > > + if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > > + addr_space = IPMI_MEM_ADDR_SPACE; > > + else > > + addr_space = IPMI_IO_ADDR_SPACE; > > + > > + info = kzalloc(sizeof(*info), GFP_KERNEL); > > + if (!info) { > > + printk(KERN_ERR "ipmi_si: Could not allocate SI data (3)\n"); > > + return -ENOMEM; > > + } > > + > > + info->addr_source = "ACPI"; > > + > > + /* Figure out the interface type. */ > > + switch (spmi->interfacetype) { > > + case 1: /* KCS */ > > + info->si_type = SI_KCS; > > + break; > > + case 2: /* SMIC */ > > + info->si_type = SI_SMIC; > > + break; > > + case 3: /* BT */ > > + info->si_type = SI_BT; > > + break; > > + default: > > + printk(KERN_INFO "ipmi_si: Unknown ACPI/SPMI SI type %d\n", > > + spmi->interfacetype); > > + kfree(info); > > + return -EIO; > > + } > > + > > + if (spmi->interrupttype & 1) { > > + /* We've got a GPE interrupt. */ > > + info->irq = spmi->gpe; > > + info->irq_setup = acpi_gpe_irq_setup; > > + } else if (spmi->interrupttype & 2) { > > + /* We've got an APIC/SAPIC interrupt. */ > > + info->irq = spmi->global_interrupt; > > + info->irq_setup = std_irq_setup; > > + } else { > > + /* Use the default interrupt setting. */ > > + info->irq = 0; > > + info->irq_setup = NULL; > > + } > > + > > + if (spmi->addr.bit_width) { > > + /* A (hopefully) properly formed register bit width. */ > > + info->io.regspacing = spmi->addr.bit_width / 8; > > + } else { > > + info->io.regspacing = DEFAULT_REGSPACING; > > + } > > + info->io.regsize = info->io.regspacing; > > + info->io.regshift = spmi->addr.bit_offset; > > + > > + if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { > > + info->io_setup = mem_setup; > > + info->io.addr_type = IPMI_MEM_ADDR_SPACE; > > + } else if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_IO) { > > + info->io_setup = port_setup; > > + info->io.addr_type = IPMI_IO_ADDR_SPACE; > > + } else { > > + kfree(info); > > + printk(KERN_WARNING > > + "ipmi_si: Unknown ACPI I/O Address type\n"); > > + return -EIO; > > + } > > + info->io.addr_data = spmi->addr.address; > > + info->dev = spmi->dev; > > + > > + try_smi_init(info); > > + > > + return 0; > > +} > > + > > +static acpi_status > > +acpi_parse_io_ports(struct acpi_resource *resource, void *context) > > +{ > > + struct acpi_device_ipmi *p_ipmi = context; > > + > > + if (resource->type == ACPI_RESOURCE_TYPE_EXTENDED_IRQ || > > + resource->type == ACPI_RESOURCE_TYPE_IRQ) { > > + unsigned int irq_number; > > + if (p_ipmi->interrupttype) { > > + /* > > + * If it already support the interrupt through GPE, > > + * it is unnecessary to get this interrupt again. > > + */ > > + printk(KERN_WARNING "Interrupt through GPE is already" > > + " supported.\n"); > > + return AE_OK; > > + } > > + if (resource->type == ACPI_RESOURCE_TYPE_IRQ) { > > + struct acpi_resource_irq *irq; > > + irq = &resource->data.irq; > > + if (irq->interrupt_count != 1) { > > + printk(KERN_WARNING "incorrect IRQ is " > > + "defined in _CRS\n"); > > + return AE_OK; > > + } > > + irq_number = irq->interrupts[0]; > > + } else { > > + struct acpi_resource_extended_irq *extended_irq; > > + extended_irq = &resource->data.extended_irq; > > + if (extended_irq->interrupt_count != 1) { > > + printk(KERN_WARNING "incorrect IRQ is " > > + "defined in _CRS\n"); > > + return AE_OK; > > + } > > + irq_number = extended_irq->interrupts[0]; > > + } > > + p_ipmi->global_interrupt = irq_number; > > + if (p_ipmi->global_interrupt) { > > + /* GSI interrupt type */ > > + p_ipmi->interrupttype |= 0x02; > > + } > > + return AE_OK; > > + } > > + if (resource->type == ACPI_RESOURCE_TYPE_IO || > > + resource->type == ACPI_RESOURCE_TYPE_FIXED_IO) { > > + u16 address; > > + struct acpi_resource_io *io; > > + struct acpi_resource_fixed_io *fixed_io; > > + > > + fixed_io = &resource->data.fixed_io; > > + if (p_ipmi->resource_count) { > > + /* > > + * Multiply definitions of IO/memory address are > > + * obtained. It is incorrect. We will continue > > + * to use the first IO/memory definition. > > + * If not correct, please fix me. > > + */ > > + return AE_OK; > > + } > > + if (resource->type == ACPI_RESOURCE_TYPE_IO) { > > + io = &resource->data.io; > > + if (!io->minimum) { > > + /* when IO address is zero, return */ > > + return AE_OK; > > + } > > + address = io->minimum; > > + } else { > > + fixed_io = &resource->data.fixed_io; > > + if (!fixed_io->address) > > + return AE_OK; > > + address = fixed_io->address; > > + } > > + p_ipmi->resource_count++; > > + p_ipmi->addr.space_id = ACPI_ADR_SPACE_SYSTEM_IO; > > + p_ipmi->addr.address = address; > > + return AE_OK; > > + } > > + > > + if (resource->type == ACPI_RESOURCE_TYPE_MEMORY32 || > > + resource->type == ACPI_RESOURCE_TYPE_MEMORY24 || > > + resource->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) { > > + u32 address; > > + if (p_ipmi->resource_count) { > > + /* > > + * Multiply definitions of IO/memory address are > > + * obtained. It is incorrect. We will continue > > + * to use the first IO/memory definition. > > + * If not correct, please fix me. > > + */ > > + return AE_OK; > > + } > > + if (resource->type == ACPI_RESOURCE_TYPE_MEMORY32) { > > + struct acpi_resource_memory32 *memory32; > > + memory32 = &resource->data.memory32; > > + address = memory32->minimum; > > + } else if (resource->type == ACPI_RESOURCE_TYPE_MEMORY24) { > > + struct acpi_resource_memory24 *memory24; > > + memory24 = &resource->data.memory24; > > + address = memory24->minimum; > > + } else { > > + struct acpi_resource_fixed_memory32 *fixed_memory32; > > + fixed_memory32 = &resource->data.fixed_memory32; > > + address = fixed_memory32->address; > > + } > > + p_ipmi->resource_count++; > > + p_ipmi->addr.address = (u64) address; > > + p_ipmi->addr.space_id = ACPI_ADR_SPACE_SYSTEM_MEMORY; > > + return AE_OK; > > + } > > + if (resource->type == ACPI_RESOURCE_TYPE_ADDRESS16 || > > + resource->type == ACPI_RESOURCE_TYPE_ADDRESS32 || > > + resource->type == ACPI_RESOURCE_TYPE_ADDRESS64) { > > + struct acpi_resource_address64 address64; > > + acpi_resource_to_address64(resource, &address64); > > + if (p_ipmi->resource_count) { > > + /* > > + * Multiply definitions of IO/memory address are > > + * obtained. It is incorrect. We will continue > > + * to use the first IO/memory definition. > > + * If not correct, please fix me. > > + */ > > + return AE_OK; > > + } > > + if (address64.resource_type != ACPI_MEMORY_RANGE && > > + address64.resource_type != ACPI_IO_RANGE) { > > + /* ignore the incorrect resource type */ > > + return AE_OK; > > + } > > + p_ipmi->addr.address = address64.minimum; > > + p_ipmi->resource_count++; > > + if (address64.resource_type == ACPI_MEMORY_RANGE) > > + p_ipmi->addr.space_id = ACPI_ADR_SPACE_SYSTEM_MEMORY; > > + else > > + p_ipmi->addr.space_id = ACPI_ADR_SPACE_SYSTEM_IO; > > + > > + return AE_OK; > > + } > > + > > + return AE_OK; > > +} > > + > > +/* > > + * acpi_parse_bmc_resource -- parse the BMC resources from ACPI > > + * @p_ipmi: the memory to store the BCM resource > > + * @handle: ACPI device handle > > + */ > > +static int acpi_parse_bmc_resource(struct acpi_device_ipmi *p_ipmi, > > + acpi_handle handle) > > +{ > > + int parse_ok = false; > > + unsigned long long temp_data; > > + acpi_status status; > > + > > + /* According to IPMI spec there should exist the _IFT method > > + * for the IPMI device. So when there is no _IFT, it is regarded > > + * as the incorrect BMC device and won't parse the resource again. > > + */ > > + status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp_data); > > + if (ACPI_FAILURE(status)) > > + return parse_ok; > > + > > + p_ipmi->interfacetype = temp_data; > > + /* Figure out the interface type. If the interface type is not > > + * KCS/SMIC/BT, it is regared as the incorrect IPMI device. > > + * Of course the SSIF interface type is also defined, but we > > + * can't handle it. So it is not supported */ > > + switch (temp_data) { > > + case 1: /* KCS */ > > + case 2: /* SMIC */ > > + case 3: /* BT */ > > + break; > > + default: > > + printk(KERN_INFO "ipmi_si: Unknown ACPI/SPMI SI type %d\n", > > + p_ipmi->interfacetype); > > + return parse_ok; > > + } > > + /* check whether there exists the _GPE method. If it exists, it > > + * means that interrupt through GPE is supported. > > + */ > > + temp_data = 0; > > + status = acpi_evaluate_integer(handle, "_GPE", NULL, &temp_data); > > + if (ACPI_SUCCESS(status)) { > > + p_ipmi->gpe = temp_data; > > + /* set the GPE interrupt type */ > > + p_ipmi->interrupttype |= 0x01; > > + } > > + /* get the IPMI revision */ > > + temp_data = 0; > > + status = acpi_evaluate_integer(handle, "_SRV", NULL, &temp_data); > > + if (ACPI_SUCCESS(status)) > > + p_ipmi->ipmi_revision = temp_data; > > + > > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > > + acpi_parse_io_ports, p_ipmi); > > + if (ACPI_FAILURE(status)) { > > + printk(KERN_WARNING "Can't parse the _CRS object \n"); > > + return parse_ok; > > + } > > + if (!p_ipmi->resource_count) { > > + /* The incorrect IO/Memory address is parsed */ > > + printk(KERN_ERR "Incorrect IO/Memory address is parsed\n"); > > + return parse_ok; > > + } > > + parse_ok = true; > > + > > + return parse_ok; > > +} > > + > > +const struct acpi_device_id ipmi_ids[] = { > > + {"IPI0001", 0}, > > + {"", 0}, > > +}; > > + > > +/* > > + * acpi_check_bmc_device -- check whether @handle is a BMC device and then > > + * get its corresponding resource. For example: IO/Mem > > + * address, interface type > > + * @handle: ACPI device handle > > + * @level : depth in the ACPI namespace tree > > + * @context: the number of bmc device. In theory there is not more than > > + * one ACPI BMC device. > > + * @rv: a return value to fill if desired (Not use) > > + */ > > +static acpi_status > > +acpi_check_bmc_device(acpi_handle handle, u32 level, void *context, > > + void **return_value) > > +{ > > + struct acpi_device *acpi_dev; > > + struct acpi_device_ipmi *p_ipmi = NULL; > > + int *count = (int *)context; > > + > > + acpi_dev = NULL; > > + /* Get the acpi device for device handle */ > > + if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev) { > > + /* If there is no ACPI device for handle, return */ > > + return AE_OK; > > + } > > + > > + if (acpi_match_device_ids(acpi_dev, ipmi_ids)) > > + return AE_OK; > > + > > + p_ipmi = kzalloc(sizeof(*p_ipmi), GFP_KERNEL); > > + if (!p_ipmi) { > > + printk(KERN_ERR "Can't allocate memory for IPMI device\n"); > > + return AE_OK; > > + } > > + p_ipmi->dev = &acpi_dev->dev; > > + if (!acpi_parse_bmc_resource(p_ipmi, handle)) { > > + kfree(p_ipmi); > > + } else { > > + list_add_tail(&p_ipmi->link, &acpi_ipmi); > > + *count = *count + 1; > > + } > > + > > + return AE_OK; > > +} > > + > > +static __devinit void acpi_device_find_bmc(void) > > +{ > > + acpi_status status; > > + int device_count = 0; > > + struct acpi_device_ipmi *p_ipmi, *p_ipmi2; > > + > > + if (acpi_disabled) > > + return; > > + > > + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, > > + ACPI_UINT32_MAX, > > + acpi_check_bmc_device, &device_count, NULL); > > + if (!device_count) { > > + /* when no IPMI device is found in ACPI namespace, return */ > > + return; > > + } > > + list_for_each_entry_safe(p_ipmi, p_ipmi2, &acpi_ipmi, link) { > > + try_init_acpi_device(p_ipmi); > > + list_del(&p_ipmi->link); > > + kfree(p_ipmi); > > + } > > + > > + return; > > +} > > + > > static __devinit void acpi_find_bmc(void) > > { > > acpi_status status; > > @@ -2014,6 +2404,7 @@ static __devinit void acpi_find_bmc(void) > > if (acpi_failure) > > return; > > > > + /* locate the IPMI system interface in ACPI SPMI table */ > > for (i = 0; ; i++) { > > status = acpi_get_table(ACPI_SIG_SPMI, i+1, > > (struct acpi_table_header **)&spmi); > > @@ -2022,6 +2413,9 @@ static __devinit void acpi_find_bmc(void) > > > > try_init_acpi(spmi); > > } > > + > > + /* locate the IPMI system interface in ACPI device */ > > + acpi_device_find_bmc(); > > } > > #endif > > > > ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
